From b9d28c5f750d9c8b91182c15e499c4371e45dc29 Mon Sep 17 00:00:00 2001 From: Mugurell Date: Fri, 15 Jul 2022 18:33:57 +0300 Subject: [PATCH] For #12469 - Cancel in progress storage requests before new awesomebar suggestions --- .../browser/storage/sync/PlacesStorage.kt | 60 ++++++++++++-- .../sync/PlacesBookmarksStorageTest.kt | 1 + .../storage/sync/PlacesHistoryStorageTest.kt | 1 + .../browser/storage/sync/PlacesStorageTest.kt | 78 +++++++++++++++++++ .../components/concept/storage/Cancellable.kt | 31 ++++++++ .../concept/storage/HistoryMetadataStorage.kt | 2 +- .../components/concept/storage/Storage.kt | 7 +- .../BookmarksStorageSuggestionProvider.kt | 5 +- .../CombinedHistorySuggestionProvider.kt | 4 +- .../HistoryMetadataSuggestionProvider.kt | 6 +- .../HistoryStorageSuggestionProvider.kt | 6 +- .../BookmarksStorageSuggestionProviderTest.kt | 34 ++++++++ .../CombinedHistorySuggestionProviderTest.kt | 31 ++++++++ .../HistoryMetadataSuggestionProviderTest.kt | 24 ++++++ .../HistoryStorageSuggestionProviderTest.kt | 26 +++++++ .../feature/session/HistoryDelegateTest.kt | 8 ++ docs/changelog.md | 3 + 17 files changed, 306 insertions(+), 21 deletions(-) create mode 100644 components/browser/storage-sync/src/test/java/mozilla/components/browser/storage/sync/PlacesStorageTest.kt create mode 100644 components/concept/storage/src/main/java/mozilla/components/concept/storage/Cancellable.kt 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..dd9b79a9148 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,57 @@ abstract class PlacesStorage( } } - /** - * Cleans up background work and database connections - */ + @Deprecated( + "Use `cancelWrites` and `cancelReads` 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 cancelWrites() { + interruptCurrentWrites() + writeScope.coroutineContext.cancelChildren() + } + + override fun cancelReads() { + 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) + } catch (e: PlacesException) { + crashReporter?.submitCaughtException(e) + logger.warn("Ignoring PlacesException while interrupting writes", 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) + } catch (e: PlacesException) { + crashReporter?.submitCaughtException(e) + logger.warn("Ignoring PlacesException while interrupting reads", e) + } + } + /** * Runs [block] described by [operation], ignoring and logging non-fatal exceptions. * 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..bff7c7e5d5f --- /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.cancelReads() + + 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.cancelWrites() + + 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/Cancellable.kt b/components/concept/storage/src/main/java/mozilla/components/concept/storage/Cancellable.kt new file mode 100644 index 00000000000..ac66f627053 --- /dev/null +++ b/components/concept/storage/src/main/java/mozilla/components/concept/storage/Cancellable.kt @@ -0,0 +1,31 @@ +/* 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 Cancellable { + /** + * Cleans up all background work and operations queue. + */ + fun cleanup() { + // no-op + } + + /** + * Cleans up all pending write operations. + */ + fun cancelWrites() { + // no-op + } + + /** + * Cleans up all pending read operations. + */ + fun cancelReads() { + // no-op + } +} 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..19a8275fe29 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 : Cancellable { /** * 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..c4dd01399bf 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 : Cancellable { /** * 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..c5b05a3f276 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.cancelReads() + 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..85ea69af970 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,11 +61,13 @@ class CombinedHistorySuggestionProvider( } override suspend fun onInputChanged(text: String): List = coroutineScope { + historyStorage.cancelReads() + historyMetadataStorage.cancelReads() + if (text.isBlank()) { return@coroutineScope emptyList() } - // Do both queries in parallel to optimize for speed. val metadataSuggestionsAsync = async { historyMetadataStorage .queryHistoryMetadata(text, maxNumberOfSuggestions) 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..4164c51db04 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.cancelReads() + 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..7a6ed2c6041 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.cancelReads() + 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..decc1d8c540 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).cancelReads() + } + + @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).cancelReads() + 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 cancelWrites() { + // "Not needed for the test" + throw NotImplementedError() + } + + override fun cancelReads() { + // 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..9baeef54212 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 cancel all previous read operations`() = runTest { + val history: HistoryStorage = mock() + val metadata: HistoryMetadataStorage = mock() + val provider = CombinedHistorySuggestionProvider(history, metadata, mock()) + + provider.onInputChanged("") + + verify(history).cancelReads() + verify(metadata).cancelReads() + } + @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 cancel 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).cancelReads() + orderVerifier.verify(storage).cancelReads() + 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..d4e78dac506 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).cancelReads() + } + + @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).cancelReads() + 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..a2010baf6c9 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).cancelReads() + } + + @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).cancelReads() + 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..7b6c3a217ba 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 cancelWrites() { + fail() + } + + override fun cancelReads() { + 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.