For mozilla-mobile/fenix#9056: Search with custom tab

components/feature/search/src/main/java/mozilla/components/feature/search/BrowserStoreSearchAdapter.kt

@@ -5,7 +5,7 @@
 package mozilla.components.feature.search
 
 import mozilla.components.browser.state.action.ContentAction
-import mozilla.components.browser.state.selector.selectedTab
+import mozilla.components.browser.state.selector.findTabOrCustomTabOrSelectedTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.search.SearchRequest
 
@@ -15,18 +15,27 @@ import mozilla.components.concept.engine.search.SearchRequest
  * This class uses the [browserStore] to determine when private mode is active, and updates the
  * [browserStore] whenever a new search has been requested.
  *
- * NOTE: this will add [SearchRequest]s to [browserStore.state] when [sendSearch] is called. Client
+ * NOTE: this will add [SearchRequest]s to [BrowserStore.state] when [sendSearch] is called. Client
  * code is responsible for consuming these requests and displaying something to the user.
  *
  * NOTE: client code is responsible for sending [ContentAction.ConsumeSearchRequestAction]s
  * after consuming events. See [SearchFeature] for a component that will handle this for you.
+ *
+ * @param browserStore The application's [BrowserStore].
+ * @param tabId ID of the tab that requests the search, or null to use the selected tab.
  */
-class BrowserStoreSearchAdapter(private val browserStore: BrowserStore) : SearchAdapter {
+class BrowserStoreSearchAdapter(
+    private val browserStore: BrowserStore,
+    private val tabId: String? = null
+) : SearchAdapter {
 
     override fun sendSearch(isPrivate: Boolean, text: String) {
-        val selectedTabId = browserStore.state.selectedTabId ?: return
-        browserStore.dispatch(ContentAction.UpdateSearchRequestAction(selectedTabId, SearchRequest(isPrivate, text)))
+        val selectedTabId = tabId ?: browserStore.state.selectedTabId ?: return
+        browserStore.dispatch(
+            ContentAction.UpdateSearchRequestAction(selectedTabId, SearchRequest(isPrivate, text))
+        )
     }
 
-    override fun isPrivateSession(): Boolean = browserStore.state.selectedTab?.content?.private ?: false
+    override fun isPrivateSession(): Boolean =
+        browserStore.state.findTabOrCustomTabOrSelectedTab(tabId)?.content?.private ?: false
 }

components/feature/search/src/main/java/mozilla/components/feature/search/SearchFeature.kt

@@ -11,7 +11,7 @@ import kotlinx.coroutines.flow.distinctUntilChangedBy
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.mapNotNull
 import mozilla.components.browser.state.action.ContentAction
-import mozilla.components.browser.state.selector.selectedTab
+import mozilla.components.browser.state.selector.findTabOrCustomTabOrSelectedTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.search.SearchRequest
 import mozilla.components.lib.state.ext.flowScoped
@@ -23,25 +23,31 @@ import mozilla.components.support.utils.ext.toNullablePair
  * then consumes them.
  *
  * NOTE: that this only consumes SearchRequests, and will not hook up the [store] to a source of
- * SearchRequests. See [SelectionActionDelegate] for use in generating SearchRequests.
+ * SearchRequests. See [mozilla.components.concept.engine.selection.SelectionActionDelegate] for use
+ * in generating SearchRequests.
  */
 class SearchFeature(
     private val store: BrowserStore,
-    private val performSearch: (SearchRequest) -> Unit
+    private val tabId: String? = null,
+    private val performSearch: (SearchRequest, tabId: String) -> Unit
 ) : LifecycleAwareFeature {
+
     private var scope: CoroutineScope? = null
 
     override fun start() {
         scope = store.flowScoped { flow ->
             flow
-                .map { it.selectedTab?.content?.searchRequest to it.selectedTabId }
+                .map { state ->
+                    val tab = state.findTabOrCustomTabOrSelectedTab(tabId)
+                    tab?.content?.searchRequest to tab?.id
+                }
                 // Do nothing if searchRequest or sessionId is null
                 .mapNotNull { pair -> pair.toNullablePair() }
                 // We may see repeat values if other state changes before we handle the request.
                 // Filter these out.
                 .distinctUntilChangedBy { (searchRequest, _) -> searchRequest }
                 .collect { (searchRequest, sessionId) ->
-                    performSearch(searchRequest)
+                    performSearch(searchRequest, sessionId)
                     store.dispatch(ContentAction.ConsumeSearchRequestAction(sessionId))
                 }
         }

components/feature/search/src/test/java/mozilla/components/feature/search/BrowserStoreSeachAdapterTest.kt

+/* 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.feature.search
+
+import mozilla.components.browser.state.action.ContentAction
+import mozilla.components.browser.state.state.BrowserState
+import mozilla.components.browser.state.state.createCustomTab
+import mozilla.components.browser.state.state.createTab
+import mozilla.components.browser.state.store.BrowserStore
+import mozilla.components.concept.engine.search.SearchRequest
+import mozilla.components.support.test.any
+import mozilla.components.support.test.mock
+import mozilla.components.support.test.whenever
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import org.mockito.Mockito.never
+import org.mockito.Mockito.verify
+
+private const val SELECTED_TAB_ID = "1"
+private const val CUSTOM_TAB_ID = "2"
+
+class BrowserStoreSearchAdapterTest {
+
+    private lateinit var browserStore: BrowserStore
+    private val state = BrowserState(
+        tabs = listOf(createTab(id = SELECTED_TAB_ID, url = "https://mozilla.org", private = true)),
+        customTabs = listOf(createCustomTab(id = CUSTOM_TAB_ID, url = "https://firefox.com")),
+        selectedTabId = SELECTED_TAB_ID
+    )
+
+    @Before
+    fun setup() {
+        browserStore = mock()
+        whenever(browserStore.state).thenReturn(state)
+    }
+
+    @Test
+    fun `adapter does nothing with null tab`() {
+        whenever(browserStore.state).thenReturn(BrowserState())
+        val searchAdapter = BrowserStoreSearchAdapter(browserStore)
+
+        searchAdapter.sendSearch(isPrivate = false, text = "normal search")
+        searchAdapter.sendSearch(isPrivate = true, text = "private search")
+
+        verify(browserStore, never()).dispatch(any())
+        assertFalse(searchAdapter.isPrivateSession())
+    }
+
+    @Test
+    fun `sendSearch with selected tab`() {
+        val searchAdapter = BrowserStoreSearchAdapter(browserStore)
+        searchAdapter.sendSearch(isPrivate = false, text = "normal search")
+        verify(browserStore).dispatch(
+            ContentAction.UpdateSearchRequestAction(
+                SELECTED_TAB_ID,
+                SearchRequest(isPrivate = false, query = "normal search")
+            )
+        )
+
+        searchAdapter.sendSearch(isPrivate = true, text = "private search")
+        verify(browserStore).dispatch(
+            ContentAction.UpdateSearchRequestAction(
+                SELECTED_TAB_ID,
+                SearchRequest(isPrivate = true, query = "private search")
+            )
+        )
+
+        assertTrue(searchAdapter.isPrivateSession())
+    }
+
+    @Test
+    fun `sendSearch with custom tab`() {
+        val searchAdapter = BrowserStoreSearchAdapter(browserStore, CUSTOM_TAB_ID)
+        searchAdapter.sendSearch(isPrivate = false, text = "normal search")
+        verify(browserStore).dispatch(
+            ContentAction.UpdateSearchRequestAction(
+                CUSTOM_TAB_ID,
+                SearchRequest(isPrivate = false, query = "normal search")
+            )
+        )
+
+        searchAdapter.sendSearch(isPrivate = true, text = "private search")
+        verify(browserStore).dispatch(
+            ContentAction.UpdateSearchRequestAction(
+                CUSTOM_TAB_ID,
+                SearchRequest(isPrivate = true, query = "private search")
+            )
+        )
+
+        assertFalse(searchAdapter.isPrivateSession())
+    }
+}

components/feature/search/src/test/java/mozilla/components/feature/search/SearchFeatureTest.kt

@@ -9,7 +9,9 @@ import mozilla.components.browser.state.state.createTab
 import mozilla.components.browser.state.store.BrowserStore
 import mozilla.components.concept.engine.search.SearchRequest
 import mozilla.components.support.test.any
+import mozilla.components.support.test.eq
 import mozilla.components.support.test.ext.joinBlocking
+import mozilla.components.support.test.libstate.ext.waitUntilIdle
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.rule.MainCoroutineRule
 import org.junit.After
@@ -20,8 +22,6 @@ import org.junit.Test
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
 
-import mozilla.components.support.test.libstate.ext.waitUntilIdle
-
 private const val SELECTED_TAB_ID = "1"
 
 class SearchFeatureTest {
@@ -31,7 +31,7 @@ class SearchFeatureTest {
     @get:Rule
     val coroutinesTestRule = MainCoroutineRule(testDispatcher)
 
-    private lateinit var performSearch: (SearchRequest) -> Unit
+    private lateinit var performSearch: (SearchRequest, String) -> Unit
     private lateinit var store: BrowserStore
     private lateinit var searchFeature: SearchFeature
 
@@ -41,7 +41,7 @@ class SearchFeatureTest {
             mockBrowserState()
         )
         performSearch = mock()
-        searchFeature = SearchFeature(store, performSearch).apply {
+        searchFeature = SearchFeature(store, null, performSearch).apply {
             start()
         }
     }
@@ -64,38 +64,38 @@ class SearchFeatureTest {
 
     @Test
     fun `GIVEN a tab is selected WHEN a search request is sent THEN a search should be performed`() {
-        verify(performSearch, times(0)).invoke(any())
+        verify(performSearch, times(0)).invoke(any(), eq(SELECTED_TAB_ID))
 
         val normalSearchRequest = SearchRequest(isPrivate = false, query = "query")
         store.dispatch(ContentAction.UpdateSearchRequestAction(SELECTED_TAB_ID, normalSearchRequest)).joinBlocking()
 
-        verify(performSearch, times(1)).invoke(any())
-        verify(performSearch, times(1)).invoke(normalSearchRequest)
+        verify(performSearch, times(1)).invoke(any(), eq(SELECTED_TAB_ID))
+        verify(performSearch, times(1)).invoke(normalSearchRequest, SELECTED_TAB_ID)
 
         val privateSearchRequest = SearchRequest(isPrivate = true, query = "query")
         store.dispatch(ContentAction.UpdateSearchRequestAction(SELECTED_TAB_ID, privateSearchRequest)).joinBlocking()
 
-        verify(performSearch, times(2)).invoke(any())
-        verify(performSearch, times(1)).invoke(privateSearchRequest)
+        verify(performSearch, times(2)).invoke(any(), eq(SELECTED_TAB_ID))
+        verify(performSearch, times(1)).invoke(privateSearchRequest, SELECTED_TAB_ID)
     }
 
     @Test
     fun `GIVEN no tab is selected WHEN a search request is sent THEN no search should be performed`() {
         store.dispatch(TabListAction.RemoveTabAction(tabId = SELECTED_TAB_ID, selectParentIfExists = false))
 
-        verify(performSearch, times(0)).invoke(any())
+        verify(performSearch, times(0)).invoke(any(), eq(SELECTED_TAB_ID))
 
         val normalSearchRequest = SearchRequest(isPrivate = false, query = "query")
         store.dispatch(ContentAction.UpdateSearchRequestAction(SELECTED_TAB_ID, normalSearchRequest)).joinBlocking()
 
-        verify(performSearch, times(0)).invoke(any())
-        verify(performSearch, times(0)).invoke(normalSearchRequest)
+        verify(performSearch, times(0)).invoke(any(), eq(SELECTED_TAB_ID))
+        verify(performSearch, times(0)).invoke(normalSearchRequest, SELECTED_TAB_ID)
 
         val privateSearchRequest = SearchRequest(isPrivate = true, query = "query")
         store.dispatch(ContentAction.UpdateSearchRequestAction(SELECTED_TAB_ID, privateSearchRequest)).joinBlocking()
 
-        verify(performSearch, times(0)).invoke(any())
-        verify(performSearch, times(0)).invoke(privateSearchRequest)
+        verify(performSearch, times(0)).invoke(any(), eq(SELECTED_TAB_ID))
+        verify(performSearch, times(0)).invoke(privateSearchRequest, SELECTED_TAB_ID)
     }
 
     @Test

docs/changelog.md

@@ -12,6 +12,10 @@ permalink: /changelog/
 * [Gecko](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Gecko.kt)
 * [Configuration](https://github.com/mozilla-mobile/android-components/blob/master/buildSrc/src/main/java/Config.kt)
 
+* **feature-search**
+  * ⚠️ **This is a breaking change**: `SearchFeature.performSearch` now takes a second parameter.
+  * `BrowserStoreSearchAdapter` and `SearchFeature` can now take a `tabId` parameter.
+
 # 56.0.0
 
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v55.0.0...v56.0.0)

samples/browser/src/main/java/org/mozilla/samples/browser/BrowserFragment.kt

@@ -142,10 +142,11 @@ class BrowserFragment : BaseBrowserFragment(), UserInteractionHandler {
         )
 
         searchFeature.set(
-            feature = SearchFeature(components.store) {
-                when (it.isPrivate) {
-                    true -> components.searchUseCases.newPrivateTabSearch.invoke(it.query)
-                    false -> components.searchUseCases.newTabSearch.invoke(it.query)
+            feature = SearchFeature(components.store) { request, _ ->
+                if (request.isPrivate) {
+                    components.searchUseCases.newPrivateTabSearch.invoke(request.query)
+                } else {
+                    components.searchUseCases.newTabSearch.invoke(request.query)
                 }
             },
             owner = this,