For mozilla-mobile#12469 - Cancel previous queries before new suggest…

…ions requests
Mugurell · Jul 11, 2022 · 3c676fe · 3c676fe
1 parent 4696f89
commit 3c676fe
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 3 deletions.
diff --git a/...src/main/java/mozilla/components/compose/browser/awesomebar/internal/SuggestionFetcher.kt b/...src/main/java/mozilla/components/compose/browser/awesomebar/internal/SuggestionFetcher.kt
@@ -5,10 +5,12 @@
 package mozilla.components.compose.browser.awesomebar.internal
 
 import android.os.SystemClock
+import androidx.annotation.VisibleForTesting
 import androidx.compose.runtime.RememberObserver
 import androidx.compose.runtime.mutableStateOf
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.asCoroutineDispatcher
-import kotlinx.coroutines.coroutineScope
 import kotlinx.coroutines.launch
 import mozilla.components.compose.browser.awesomebar.AwesomeBarFacts
 import mozilla.components.compose.browser.awesomebar.AwesomeBarFacts.emitAwesomeBarFact
@@ -32,6 +34,9 @@ internal class SuggestionFetcher(
         NamedThreadFactory("SuggestionFetcher")
     ).asCoroutineDispatcher()
 
+    @VisibleForTesting
+    internal var fetchJob: Job? = null
+
     /**
      * The current list of suggestions as an observable list.
      */
@@ -45,7 +50,9 @@ internal class SuggestionFetcher(
     suspend fun fetch(text: String) {
         profiler?.addMarker("SuggestionFetcher.fetch") // DO NOT ADD ANYTHING ABOVE THIS addMarker CALL.
 
-        coroutineScope {
+        fetchJob?.cancel()
+
+        fetchJob = CoroutineScope(dispatcher).launch {
             groups.forEach { group ->
                 group.providers.forEach { provider ->
                     val profilerStartTime = profiler?.getProfilerTime() // DO NOT ADD ANYTHING ABOVE getProfilerTime.
@@ -58,7 +65,8 @@ internal class SuggestionFetcher(
     /**
      * Fetches suggestions from [provider].
      */
-    private suspend fun fetchFrom(
+    @VisibleForTesting
+    internal suspend fun fetchFrom(
         group: AwesomeBar.SuggestionProviderGroup,
         provider: AwesomeBar.SuggestionProvider,
         text: String,

diff --git a/...test/java/mozilla/components/compose/browser/awesomebar/internal/SuggestionFetcherTest.kt b/...test/java/mozilla/components/compose/browser/awesomebar/internal/SuggestionFetcherTest.kt
@@ -0,0 +1,41 @@
+/* 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.compose.browser.awesomebar.internal
+
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.Job
+import mozilla.components.concept.awesomebar.AwesomeBar.SuggestionProvider
+import mozilla.components.concept.awesomebar.AwesomeBar.SuggestionProviderGroup
+import mozilla.components.support.test.any
+import mozilla.components.support.test.mock
+import mozilla.components.support.test.rule.MainCoroutineRule
+import mozilla.components.support.test.rule.runTestOnMain
+import org.junit.Rule
+import org.junit.Test
+import org.mockito.Mockito.doAnswer
+import org.mockito.Mockito.inOrder
+import org.mockito.Mockito.spy
+
+@ExperimentalCoroutinesApi // for runTestOnMain
+class SuggestionFetcherTest {
+    @get:Rule
+    val coroutinesTestRule = MainCoroutineRule()
+
+    @Test
+    fun `GIVEN a new fetch request THEN all previous queries are cancelled`() = runTestOnMain {
+        val provider: SuggestionProvider = mock()
+        val providerGroup = SuggestionProviderGroup(listOf(provider))
+        val fetcher = spy(SuggestionFetcher(listOf(providerGroup), null))
+        val previousFetchJob: Job = mock()
+        fetcher.fetchJob = previousFetchJob
+        doAnswer {}.`when`(fetcher).fetchFrom(any(), any(), any(), any())
+        val orderVerifier = inOrder(previousFetchJob, fetcher)
+
+        fetcher.fetch("test")
+
+        orderVerifier.verify(previousFetchJob)!!.cancel()
+        orderVerifier.verify(fetcher).fetchFrom(providerGroup, provider, "test", null)
+    }
+}
diff --git 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 before new suggestions requests.
+
 * **service-pocket**
   * Add an index for sponsored stories foreign key to resolve a compilation warning. [#12406](https://github.com/mozilla-mobile/android-components/issues/12406)
 

diff --git a/samples/browser/build.gradle b/samples/browser/build.gradle
@@ -150,6 +150,12 @@ dependencies {
 
     debugImplementation Dependencies.leakcanary
 
+    testImplementation Dependencies.androidx_test_core
+    testImplementation Dependencies.androidx_test_junit
+    testImplementation Dependencies.testing_robolectric
+    testImplementation Dependencies.testing_mockito
+    testImplementation Dependencies.testing_coroutines
+
     androidTestImplementation project(':support-android-test')
     androidTestImplementation Dependencies.androidx_test_core
     androidTestImplementation Dependencies.androidx_test_runner