diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesStorage.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesStorage.kt index 150c003e027..905368f6b9c 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesStorage.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/PlacesStorage.kt @@ -30,14 +30,16 @@ abstract class PlacesStorage( context: Context, val crashReporter: CrashReporting? = null ) : Storage, SyncableStore { - internal val writeScope by lazy { + internal var writeScope = CoroutineScope( Executors.newSingleThreadExecutor( NamedThreadFactory("PlacesStorageWriteScope") ).asCoroutineDispatcher() ) - } - internal val readScope by lazy { CoroutineScope(Dispatchers.IO) } + @VisibleForTesting internal set + + internal var readScope = CoroutineScope(Dispatchers.IO) + @VisibleForTesting internal set private val storageDir by lazy { context.filesDir } abstract val logger: Logger @@ -48,8 +50,8 @@ abstract class PlacesStorage( RustPlacesConnection } - internal val writer: PlacesWriterConnection by lazy { places.writer() } - internal val reader: PlacesReaderConnection by lazy { places.reader() } + internal open val writer: PlacesWriterConnection by lazy { places.writer() } + internal open val reader: PlacesReaderConnection by lazy { places.reader() } override suspend fun warmUp() { logElapsedTime(logger, "Warming up places storage") { @@ -67,15 +69,51 @@ abstract class PlacesStorage( } } - /** - * Cleans up background work and database connections - */ + @Deprecated( + "Use `cleanupWrites` and `cleanupReads` to get a similar functionality. " + + "See https://github.com/mozilla-mobile/android-components/issues/7348 for a description of the issues" + + "for when using this method" + ) override fun cleanup() { writeScope.coroutineContext.cancelChildren() readScope.coroutineContext.cancelChildren() places.close() } + override fun cleanupWrites() { + interruptCurrentWrites() + writeScope.coroutineContext.cancelChildren() + } + + override fun cleanupReads() { + interruptCurrentReads() + readScope.coroutineContext.cancelChildren() + } + + /** + * Stop all current write operations. + * Allows immediately dismissing all write operations and clearing the write queue. + */ + internal fun interruptCurrentWrites() { + try { + writer.interrupt() + } catch (e: PlacesException.OperationInterrupted) { + logger.debug("Ignoring expected OperationInterrupted exception for explicit writer interrupt call", e) + } + } + + /** + * Stop all current read queries. + * Allows avoiding having to wait for stale queries responses and clears the queries queue. + */ + internal fun interruptCurrentReads() { + try { + reader.interrupt() + } catch (e: PlacesException.OperationInterrupted) { + logger.debug("Ignoring expected OperationInterrupted exception for explicit reader interrupt call", e) + } + } + /** * Runs [block] described by [operation], ignoring and logging non-fatal exceptions. * diff --git a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/RemoteTabsStorage.kt b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/RemoteTabsStorage.kt index e70bbff3624..4ce5e7d7612 100644 --- a/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/RemoteTabsStorage.kt +++ b/components/browser/storage-sync/src/main/java/mozilla/components/browser/storage/sync/RemoteTabsStorage.kt @@ -30,7 +30,8 @@ open class RemoteTabsStorage( private val crashReporter: CrashReporting? = null ) : Storage, SyncableStore { internal val api by lazy { RemoteTabsProvider(File(context.filesDir, TABS_DB_NAME).canonicalPath) } - private val scope by lazy { CoroutineScope(Dispatchers.IO) } + private val readScope by lazy { CoroutineScope(Dispatchers.IO) } + private val writeScope by lazy { CoroutineScope(Dispatchers.IO) } internal val logger = Logger("RemoteTabsStorage") override suspend fun warmUp() { @@ -42,7 +43,7 @@ open class RemoteTabsStorage( * @param tabs The list of opened tabs, for all opened non-private windows, on this device. */ suspend fun store(tabs: List) { - return withContext(scope.coroutineContext) { + return withContext(writeScope.coroutineContext) { try { api.setLocalTabs( tabs.map { @@ -62,7 +63,7 @@ open class RemoteTabsStorage( * @return A mapping of opened tabs per device. */ suspend fun getAll(): Map> { - return withContext(scope.coroutineContext) { + return withContext(readScope.coroutineContext) { try { api.getAll().map { device -> val tabs = device.remoteTabs.map { tab -> @@ -88,7 +89,16 @@ open class RemoteTabsStorage( } override fun cleanup() { - scope.coroutineContext.cancelChildren() + cleanupWrites() + cleanupReads() + } + + override fun cleanupWrites() { + writeScope.coroutineContext.cancelChildren() + } + + override fun cleanupReads() { + readScope.coroutineContext.cancelChildren() } override fun registerWithSyncManager() { diff --git a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt index 4f312347ebd..b5282f60d2e 100644 --- a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt +++ b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesBookmarksStorageTest.kt @@ -42,6 +42,7 @@ class PlacesBookmarksStorageTest { } @After + @Suppress("DEPRECATION") fun cleanup() = runTestOnMain { bookmarks.cleanup() } diff --git a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt index 656e9a17423..1426e6280af 100644 --- a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt +++ b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesHistoryStorageTest.kt @@ -56,6 +56,7 @@ class PlacesHistoryStorageTest { } @After + @Suppress("DEPRECATION") fun cleanup() = runTestOnMain { history.cleanup() } diff --git a/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesStorageTest.kt b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesStorageTest.kt new file mode 100644 index 00000000000..3c5648ebf95 --- /dev/null +++ b/components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesStorageTest.kt @@ -0,0 +1,78 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.browser.storage.sync + +import android.content.Context +import kotlinx.coroutines.cancelChildren +import mozilla.appservices.places.PlacesReaderConnection +import mozilla.appservices.places.PlacesWriterConnection +import mozilla.appservices.places.uniffi.PlacesException +import mozilla.components.support.base.log.logger.Logger +import mozilla.components.support.test.mock +import org.junit.Test +import org.mockito.Mockito.doAnswer +import org.mockito.Mockito.doReturn +import org.mockito.Mockito.verify +import kotlin.coroutines.CoroutineContext + +class PlacesStorageTest { + private val storage = FakePlacesStorage() + + @Test + fun `WHEN all reads are interrupted THEN no exception is thrown`() { + doAnswer { + throw PlacesException.OperationInterrupted("This should be caught") + }.`when`(storage.reader).interrupt() + + storage.interruptCurrentReads() + + verify(storage.reader).interrupt() + } + + @Test + fun `WHEN all writes are interrupted THEN no exception is thrown`() { + doAnswer { + throw PlacesException.OperationInterrupted("This should be caught") + }.`when`(storage.writer).interrupt() + + storage.interruptCurrentWrites() + + verify(storage.writer).interrupt() + } + + @Test + fun `WHEN a call is made to clean all reads THEN they are cancelled`() { + storage.readScope = mock { + doReturn(mock()).`when`(this).coroutineContext + } + + storage.cleanupReads() + + verify(storage.reader).interrupt() + verify(storage.readScope.coroutineContext).cancelChildren() + } + + @Test + fun `WHEN a call is made to clean all writes THEN they are cancelled`() { + storage.writeScope = mock { + doReturn(mock()).`when`(this).coroutineContext + } + + storage.cleanupWrites() + + verify(storage.writer).interrupt() + verify(storage.writeScope.coroutineContext).cancelChildren() + } +} + +class FakePlacesStorage( + context: Context = mock() +) : PlacesStorage(context) { + override val logger = Logger("FakePlacesStorage") + override fun registerWithSyncManager() {} + + override val writer: PlacesWriterConnection = mock() + override val reader: PlacesReaderConnection = mock() +} diff --git a/components/concept/storage/src/main/java/mozilla/components/concept/storage/CleanableStorage.kt b/components/concept/storage/src/main/java/mozilla/components/concept/storage/CleanableStorage.kt new file mode 100644 index 00000000000..0fdbbf4f16d --- /dev/null +++ b/components/concept/storage/src/main/java/mozilla/components/concept/storage/CleanableStorage.kt @@ -0,0 +1,25 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package mozilla.components.concept.storage + +/** + * Storage that allows to stop and clean in progress operations. + */ +interface CleanableStorage { + /** + * Cleans up all background work and operations queue. + */ + fun cleanup() + + /** + * Cleans up all pending write operations. + */ + fun cleanupWrites() + + /** + * Cleans up all pending read operations. + */ + fun cleanupReads() +} diff --git a/components/concept/storage/src/main/java/mozilla/components/concept/storage/HistoryMetadataStorage.kt b/components/concept/storage/src/main/java/mozilla/components/concept/storage/HistoryMetadataStorage.kt index 65abfef7d32..0d7e2db1f66 100644 --- a/components/concept/storage/src/main/java/mozilla/components/concept/storage/HistoryMetadataStorage.kt +++ b/components/concept/storage/src/main/java/mozilla/components/concept/storage/HistoryMetadataStorage.kt @@ -110,7 +110,7 @@ data class HistoryHighlightWeights( /** * An interface for interacting with a storage that manages [HistoryMetadata]. */ -interface HistoryMetadataStorage { +interface HistoryMetadataStorage : CleanableStorage { /** * Returns the most recent [HistoryMetadata] for the provided [url]. * diff --git a/components/concept/storage/src/main/java/mozilla/components/concept/storage/Storage.kt b/components/concept/storage/src/main/java/mozilla/components/concept/storage/Storage.kt index c6874a2ce61..35a316bebe1 100644 --- a/components/concept/storage/src/main/java/mozilla/components/concept/storage/Storage.kt +++ b/components/concept/storage/src/main/java/mozilla/components/concept/storage/Storage.kt @@ -7,7 +7,7 @@ package mozilla.components.concept.storage /** * An interface which provides generic operations for storing browser data like history and bookmarks. */ -interface Storage { +interface Storage : CleanableStorage { /** * Make sure underlying database connections are established. */ @@ -17,9 +17,4 @@ interface Storage { * Runs internal database maintenance tasks */ suspend fun runMaintenance() - - /** - * Cleans up background work and database connections - */ - fun cleanup() } diff --git a/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProvider.kt b/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProvider.kt index 92097814d1e..5519e48fa0e 100644 --- a/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProvider.kt +++ b/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProvider.kt @@ -5,6 +5,7 @@ package mozilla.components.feature.awesomebar.provider import android.graphics.drawable.Drawable +import androidx.annotation.VisibleForTesting import mozilla.components.browser.icons.BrowserIcons import mozilla.components.browser.icons.IconRequest import mozilla.components.concept.awesomebar.AwesomeBar @@ -34,7 +35,7 @@ private const val BOOKMARKS_SUGGESTION_LIMIT = 20 * @param suggestionsHeader optional parameter to specify if the suggestion should have a header */ class BookmarksStorageSuggestionProvider( - private val bookmarksStorage: BookmarksStorage, + @get:VisibleForTesting internal val bookmarksStorage: BookmarksStorage, private val loadUrlUseCase: SessionUseCases.LoadUrlUseCase, private val icons: BrowserIcons? = null, private val indicatorIcon: Drawable? = null, @@ -49,6 +50,8 @@ class BookmarksStorageSuggestionProvider( } override suspend fun onInputChanged(text: String): List { + bookmarksStorage.cleanupReads() + if (text.isEmpty()) { return emptyList() } diff --git a/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProvider.kt b/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProvider.kt index ed565459b28..c6b5c0051e2 100644 --- a/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProvider.kt +++ b/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProvider.kt @@ -61,6 +61,9 @@ class CombinedHistorySuggestionProvider( } override suspend fun onInputChanged(text: String): List = coroutineScope { + historyStorage.cleanupReads() + historyMetadataStorage.cleanupReads() + if (text.isBlank()) { return@coroutineScope emptyList() } diff --git a/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/HistoryMetadataSuggestionProvider.kt b/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/HistoryMetadataSuggestionProvider.kt index fd4b6fc7eb0..68c9bdcf038 100644 --- a/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/HistoryMetadataSuggestionProvider.kt +++ b/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/HistoryMetadataSuggestionProvider.kt @@ -38,11 +38,11 @@ const val DEFAULT_METADATA_SUGGESTION_LIMIT = 5 * @param suggestionsHeader optional parameter to specify if the suggestion should have a header */ class HistoryMetadataSuggestionProvider( - private val historyStorage: HistoryMetadataStorage, + @get:VisibleForTesting internal val historyStorage: HistoryMetadataStorage, private val loadUrlUseCase: SessionUseCases.LoadUrlUseCase, private val icons: BrowserIcons? = null, internal val engine: Engine? = null, - @VisibleForTesting internal val maxNumberOfSuggestions: Int = DEFAULT_METADATA_SUGGESTION_LIMIT, + @get:VisibleForTesting internal val maxNumberOfSuggestions: Int = DEFAULT_METADATA_SUGGESTION_LIMIT, private val showEditSuggestion: Boolean = true, private val suggestionsHeader: String? = null, ) : AwesomeBar.SuggestionProvider { @@ -53,6 +53,8 @@ class HistoryMetadataSuggestionProvider( } override suspend fun onInputChanged(text: String): List { + historyStorage.cleanupReads() + if (text.isNullOrBlank()) { return emptyList() } diff --git a/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProvider.kt b/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProvider.kt index 7bab35367eb..96fb0020a1d 100644 --- a/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProvider.kt +++ b/components/feature/awesomebar/src/main/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProvider.kt @@ -38,11 +38,11 @@ const val DEFAULT_HISTORY_SUGGESTION_LIMIT = 20 * @param suggestionsHeader optional parameter to specify if the suggestion should have a header */ class HistoryStorageSuggestionProvider( - private val historyStorage: HistoryStorage, + @get:VisibleForTesting internal val historyStorage: HistoryStorage, private val loadUrlUseCase: SessionUseCases.LoadUrlUseCase, private val icons: BrowserIcons? = null, internal val engine: Engine? = null, - @VisibleForTesting internal var maxNumberOfSuggestions: Int = DEFAULT_HISTORY_SUGGESTION_LIMIT, + @get:VisibleForTesting internal var maxNumberOfSuggestions: Int = DEFAULT_HISTORY_SUGGESTION_LIMIT, private val showEditSuggestion: Boolean = true, private val suggestionsHeader: String? = null, ) : AwesomeBar.SuggestionProvider { @@ -54,6 +54,8 @@ class HistoryStorageSuggestionProvider( } override suspend fun onInputChanged(text: String): List { + historyStorage.cleanupReads() + if (text.isEmpty()) { return emptyList() } diff --git a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt index 34143ac0dce..8afb7cc132a 100644 --- a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt +++ b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/BookmarksStorageSuggestionProviderTest.kt @@ -20,8 +20,11 @@ import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.anyString +import org.mockito.Mockito.inOrder import org.mockito.Mockito.never +import org.mockito.Mockito.spy import org.mockito.Mockito.times import org.mockito.Mockito.verify import java.util.UUID @@ -45,6 +48,28 @@ class BookmarksStorageSuggestionProviderTest { assertTrue(suggestions.isEmpty()) } + @Test + fun `Provider cleanups all previous read operations when text is empty`() = runTest { + val provider = BookmarksStorageSuggestionProvider(mock(), mock()) + + provider.onInputChanged("") + + verify(provider.bookmarksStorage).cleanupReads() + } + + @Test + fun `Provider cleanups all previous read operations when text is not empty`() = runTest { + val storage = spy(bookmarks) + val provider = BookmarksStorageSuggestionProvider(storage, mock()) + storage.addItem("Mobile", newItem.url!!, newItem.title!!, null) + val orderVerifier = inOrder(storage) + + provider.onInputChanged("moz") + + orderVerifier.verify(provider.bookmarksStorage).cleanupReads() + orderVerifier.verify(provider.bookmarksStorage).searchBookmarks(eq("moz"), anyInt()) + } + @Test fun `Provider returns suggestions from configured bookmarks storage`() = runTest { val provider = BookmarksStorageSuggestionProvider(bookmarks, mock()) @@ -224,5 +249,14 @@ class BookmarksStorageSuggestionProviderTest { // "Not needed for the test" throw NotImplementedError() } + + override fun cleanupWrites() { + // "Not needed for the test" + throw NotImplementedError() + } + + override fun cleanupReads() { + // no-op + } } } diff --git a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProviderTest.kt b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProviderTest.kt index 76f1d19fdcd..f133305a4ef 100644 --- a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProviderTest.kt +++ b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/CombinedHistorySuggestionProviderTest.kt @@ -20,8 +20,10 @@ import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Test import org.junit.runner.RunWith +import org.mockito.Mockito import org.mockito.Mockito.anyInt import org.mockito.Mockito.doReturn +import org.mockito.Mockito.verify @ExperimentalCoroutinesApi // for runTest @RunWith(AndroidJUnit4::class) @@ -49,6 +51,18 @@ class CombinedHistorySuggestionProviderTest { assertTrue(provider.onInputChanged(" ").isEmpty()) } + @Test + fun `WHEN onInputChanged is called with empty text THEN cleanup all previous read operations`() = runTest { + val history: HistoryStorage = mock() + val metadata: HistoryMetadataStorage = mock() + val provider = CombinedHistorySuggestionProvider(history, metadata, mock()) + + provider.onInputChanged("") + + verify(history).cleanupReads() + verify(metadata).cleanupReads() + } + @Test fun `GIVEN more suggestions asked than metadata items exist WHEN user changes input THEN return a combined list of suggestions`() = runTest { val storage: HistoryMetadataStorage = mock() @@ -64,6 +78,23 @@ class CombinedHistorySuggestionProviderTest { assertEquals("http://www.mozilla.com/firefox/", result[1].description) } + @Test + fun `WHEN onInputChanged is called with non empty text THEN cleanup all previous read operations`() = runTest { + val storage: HistoryMetadataStorage = mock() + doReturn(listOf(historyEntry)).`when`(storage).queryHistoryMetadata(eq("moz"), anyInt()) + val history: HistoryStorage = mock() + doReturn(emptyList()).`when`(history).getSuggestions(eq("moz"), anyInt()) + val provider = CombinedHistorySuggestionProvider(history, storage, mock()) + val orderVerifier = Mockito.inOrder(storage, history) + + provider.onInputChanged("moz") + + orderVerifier.verify(history).cleanupReads() + orderVerifier.verify(storage).cleanupReads() + orderVerifier.verify(storage).queryHistoryMetadata(eq("moz"), anyInt()) + orderVerifier.verify(history).getSuggestions(eq("moz"), anyInt()) + } + @Test fun `GIVEN fewer suggestions asked than metadata items exist WHEN user changes input THEN return suggestions only based on metadata items`() = runTest { val storage: HistoryMetadataStorage = mock() diff --git a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/HistoryMetadataSuggestionProviderTest.kt b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/HistoryMetadataSuggestionProviderTest.kt index 5790c6b04f7..21dce2847ed 100644 --- a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/HistoryMetadataSuggestionProviderTest.kt +++ b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/HistoryMetadataSuggestionProviderTest.kt @@ -17,6 +17,7 @@ import mozilla.components.support.base.facts.Action import mozilla.components.support.base.facts.Fact import mozilla.components.support.base.facts.FactProcessor import mozilla.components.support.base.facts.Facts +import mozilla.components.support.test.eq import mozilla.components.support.test.mock import mozilla.components.support.test.whenever import org.junit.Assert.assertEquals @@ -27,6 +28,7 @@ import org.junit.Test import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.anyString import org.mockito.Mockito.doReturn +import org.mockito.Mockito.inOrder import org.mockito.Mockito.never import org.mockito.Mockito.times import org.mockito.Mockito.verify @@ -56,6 +58,28 @@ class HistoryMetadataSuggestionProviderTest { assertTrue(suggestions.isEmpty()) } + @Test + fun `provider cleanups all previous read operations when text is empty`() = runTest { + val provider = HistoryMetadataSuggestionProvider(mock(), mock()) + + provider.onInputChanged("") + + verify(provider.historyStorage).cleanupReads() + } + + @Test + fun `provider cleanups all previous read operations when text is not empty`() = runTest { + val storage: HistoryMetadataStorage = mock() + doReturn(listOf(historyEntry)).`when`(storage).queryHistoryMetadata(anyString(), anyInt()) + val provider = HistoryMetadataSuggestionProvider(storage, mock()) + val orderVerifier = inOrder(storage) + + provider.onInputChanged("moz") + + orderVerifier.verify(provider.historyStorage).cleanupReads() + orderVerifier.verify(provider.historyStorage).queryHistoryMetadata(eq("moz"), anyInt()) + } + @Test fun `provider returns suggestions from configured history storage`() = runTest { val storage: HistoryMetadataStorage = mock() diff --git a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProviderTest.kt b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProviderTest.kt index c0f91961780..af0b00dddfc 100644 --- a/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProviderTest.kt +++ b/components/feature/awesomebar/src/test/java/mozilla/components/feature/awesomebar/provider/HistoryStorageSuggestionProviderTest.kt @@ -24,9 +24,12 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.anyString import org.mockito.Mockito import org.mockito.Mockito.`when` +import org.mockito.Mockito.doReturn +import org.mockito.Mockito.inOrder import org.mockito.Mockito.never import org.mockito.Mockito.times import org.mockito.Mockito.verify @@ -48,6 +51,29 @@ class HistoryStorageSuggestionProviderTest { assertTrue(suggestions.isEmpty()) } + @Test + fun `provider cleanups all previous read operations when text is empty`() = runTest { + val provider = HistoryStorageSuggestionProvider(mock(), mock()) + + provider.onInputChanged("") + + verify(provider.historyStorage).cleanupReads() + } + + @Test + fun `provider cleanups all previous read operations when text is not empty`() = runTest { + val history: HistoryStorage = mock() + doReturn(listOf(SearchResult("id", "http://www.mozilla.com/", 10))) + .`when`(history).getSuggestions(anyString(), anyInt()) + val provider = HistoryStorageSuggestionProvider(history, mock()) + val orderVerifier = inOrder(history) + + provider.onInputChanged("moz") + + orderVerifier.verify(provider.historyStorage).cleanupReads() + orderVerifier.verify(provider.historyStorage).getSuggestions(eq("moz"), anyInt()) + } + @Test fun `Provider returns suggestions from configured history storage`() = runTest { val history: HistoryStorage = mock() diff --git a/components/feature/session/src/test/java/mozilla/components/feature/session/HistoryDelegateTest.kt b/components/feature/session/src/test/java/mozilla/components/feature/session/HistoryDelegateTest.kt index c0687803833..313fc2ae5bd 100644 --- a/components/feature/session/src/test/java/mozilla/components/feature/session/HistoryDelegateTest.kt +++ b/components/feature/session/src/test/java/mozilla/components/feature/session/HistoryDelegateTest.kt @@ -181,5 +181,13 @@ class HistoryDelegateTest { override fun cleanup() { fail() } + + override fun cleanupWrites() { + fail() + } + + override fun cleanupReads() { + fail() + } } } diff --git a/docs/changelog.md b/docs/changelog.md index 14260c17b27..0c695a473a0 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -11,6 +11,9 @@ permalink: /changelog/ * [Gecko](https://github.com/mozilla-mobile/android-components/blob/main/buildSrc/src/main/java/Gecko.kt) * [Configuration](https://github.com/mozilla-mobile/android-components/blob/main/.config.yml) +* **browser-awesomebar**: + * 🚒 Bug fixed [issue #12469](https://github.com/mozilla-mobile/android-components/issues/12469) Cancel previous queries from the application-services persistence layer before new suggestions requests. + * **service-firefox-accounts** * `SyncStatus` can now be `LoggedOut`. * `SyncStoreSupport` will update the `SyncStore` with `LoggedOut` when observed.