For #12565 - Clean up controller and add tests

app/src/main/java/org/mozilla/fenix/search/SearchController.kt

@@ -8,6 +8,7 @@ import android.content.Intent
 import androidx.navigation.NavController
 import mozilla.components.browser.search.SearchEngine
 import mozilla.components.browser.session.Session
+import mozilla.components.browser.session.SessionManager
 import mozilla.components.support.ktx.kotlin.isUrl
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
@@ -16,15 +17,14 @@ import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint.ACTION
 import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint.NONE
 import org.mozilla.fenix.components.metrics.Event.PerformedSearch.SearchAccessPoint.SUGGESTION
+import org.mozilla.fenix.components.metrics.MetricController
 import org.mozilla.fenix.components.metrics.MetricsUtils
 import org.mozilla.fenix.components.searchengine.CustomSearchEngineStore
 import org.mozilla.fenix.crashes.CrashListActivity
-import org.mozilla.fenix.ext.components
-import org.mozilla.fenix.ext.metrics
 import org.mozilla.fenix.ext.navigateSafe
-import org.mozilla.fenix.ext.settings
 import org.mozilla.fenix.settings.SupportUtils
 import org.mozilla.fenix.settings.SupportUtils.MozillaPage.MANIFESTO
+import org.mozilla.fenix.utils.Settings
 
 /**
  * An interface that handles the view manipulation of the Search, triggered by the Interactor
@@ -42,11 +42,14 @@ interface SearchController {
     fun handleSearchShortcutsButtonClicked()
 }
 
-@Suppress("TooManyFunctions")
+@Suppress("TooManyFunctions", "LongParameterList")
 class DefaultSearchController(
     private val activity: HomeActivity,
+    private val sessionManager: SessionManager,
     private val store: SearchFragmentStore,
     private val navController: NavController,
+    private val settings: Settings,
+    private val metrics: MetricController,
     private val clearToolbarFocus: () -> Unit
 ) : SearchController {
 
@@ -76,7 +79,7 @@ class DefaultSearchController(
         val event = if (url.isUrl()) {
             Event.EnteredUrl(false)
         } else {
-            activity.settings().incrementActiveSearchCount()
+            settings.incrementActiveSearchCount()
 
             val searchAccessPoint = when (store.state.searchAccessPoint) {
                 NONE -> ACTION
@@ -92,7 +95,7 @@ class DefaultSearchController(
             }
         }
 
-        event?.let { activity.metrics.track(it) }
+        event?.let { metrics.track(it) }
     }
 
     override fun handleEditingCancelled() {
@@ -100,7 +103,6 @@ class DefaultSearchController(
     }
 
     override fun handleTextChanged(text: String) {
-        val settings = activity.settings()
         // Display the search shortcuts on each entry of the search fragment (see #5308)
         val textMatchesCurrentUrl = store.state.url == text
         val textMatchesCurrentSearch = store.state.searchTerms == text
@@ -129,11 +131,11 @@ class DefaultSearchController(
             from = BrowserDirection.FromSearch
         )
 
-        activity.metrics.track(Event.EnteredUrl(false))
+        metrics.track(Event.EnteredUrl(false))
     }
 
     override fun handleSearchTermsTapped(searchTerms: String) {
-        activity.settings().incrementActiveSearchCount()
+        settings.incrementActiveSearchCount()
 
         activity.openToBrowserAndLoad(
             searchTermOrURL = searchTerms,
@@ -155,14 +157,14 @@ class DefaultSearchController(
                 sap
             )
         }
-        event?.let { activity.metrics.track(it) }
+        event?.let { metrics.track(it) }
     }
 
     override fun handleSearchShortcutEngineSelected(searchEngine: SearchEngine) {
         store.dispatch(SearchFragmentAction.SearchShortcutEngineSelected(searchEngine))
         val isCustom =
             CustomSearchEngineStore.isCustomSearchEngine(activity, searchEngine.identifier)
-        activity.metrics.track(Event.SearchShortcutSelected(searchEngine, isCustom))
+        metrics.track(Event.SearchShortcutSelected(searchEngine, isCustom))
     }
 
     override fun handleSearchShortcutsButtonClicked() {
@@ -176,14 +178,14 @@ class DefaultSearchController(
     }
 
     override fun handleExistingSessionSelected(session: Session) {
-        activity.components.core.sessionManager.select(session)
+        sessionManager.select(session)
         activity.openToBrowser(
             from = BrowserDirection.FromSearch
         )
     }
 
     override fun handleExistingSessionSelected(tabId: String) {
-        val session = activity.components.core.sessionManager.findSessionById(tabId)
+        val session = sessionManager.findSessionById(tabId)
         if (session != null) {
             handleExistingSessionSelected(session)
         }

app/src/main/java/org/mozilla/fenix/search/SearchFragment.kt

@@ -86,6 +86,7 @@ class SearchFragment : Fragment(), UserInteractionHandler {
         savedInstanceState: Bundle?
     ): View? {
         val activity = activity as HomeActivity
+        val settings = activity.settings()
         val args by navArgs<SearchFragmentArgs>()
 
         val tabId = args.sessionId
@@ -112,13 +113,13 @@ class SearchFragment : Fragment(), UserInteractionHandler {
                     defaultEngineSource = currentSearchEngine,
                     showSearchSuggestions = shouldShowSearchSuggestions(isPrivate),
                     showSearchSuggestionsHint = false,
-                    showSearchShortcuts = requireContext().settings().shouldShowSearchShortcuts &&
+                    showSearchShortcuts = settings.shouldShowSearchShortcuts &&
                             url.isEmpty() &&
                             areShortcutsAvailable,
                     areShortcutsAvailable = areShortcutsAvailable,
-                    showClipboardSuggestions = requireContext().settings().shouldShowClipboardSuggestions,
-                    showHistorySuggestions = requireContext().settings().shouldShowHistorySuggestions,
-                    showBookmarkSuggestions = requireContext().settings().shouldShowBookmarkSuggestions,
+                    showClipboardSuggestions = settings.shouldShowClipboardSuggestions,
+                    showHistorySuggestions = settings.shouldShowHistorySuggestions,
+                    showBookmarkSuggestions = settings.shouldShowBookmarkSuggestions,
                     tabId = tabId,
                     pastedText = args.pastedText,
                     searchAccessPoint = args.searchAccessPoint
@@ -128,8 +129,11 @@ class SearchFragment : Fragment(), UserInteractionHandler {
 
         val searchController = DefaultSearchController(
             activity = activity,
+            sessionManager = requireComponents.core.sessionManager,
             store = searchStore,
             navController = findNavController(),
+            settings = settings,
+            metrics = requireComponents.analytics.metrics,
             clearToolbarFocus = ::clearToolbarFocus
         )
 
@@ -163,7 +167,7 @@ class SearchFragment : Fragment(), UserInteractionHandler {
                 visible = {
                     currentSearchEngine.searchEngine.identifier.contains("google") &&
                         speechIsAvailable() &&
-                        requireContext().settings().shouldShowVoiceSearch
+                        settings.shouldShowVoiceSearch
                 },
                 listener = ::launchVoiceSearch
             )

app/src/test/java/org/mozilla/fenix/search/DefaultSearchControllerTest.kt

@@ -4,71 +4,75 @@
 
 package org.mozilla.fenix.search
 
-import android.content.Intent
 import androidx.navigation.NavController
 import androidx.navigation.NavDirections
+import io.mockk.MockKAnnotations
+import io.mockk.Runs
 import io.mockk.every
+import io.mockk.impl.annotations.MockK
+import io.mockk.just
 import io.mockk.mockk
+import io.mockk.mockkObject
+import io.mockk.unmockkObject
 import io.mockk.verify
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.runBlockingTest
 import mozilla.components.browser.search.SearchEngine
 import mozilla.components.browser.session.Session
 import mozilla.components.browser.session.SessionManager
-import mozilla.components.support.test.robolectric.testContext
-import org.junit.Assert.assertFalse
+import org.junit.After
 import org.junit.Before
 import org.junit.Test
-import org.junit.runner.RunWith
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.MetricController
-import org.mozilla.fenix.crashes.CrashListActivity
-import org.mozilla.fenix.ext.components
-import org.mozilla.fenix.ext.intentFilterEq
-import org.mozilla.fenix.ext.metrics
+import org.mozilla.fenix.components.metrics.MetricsUtils
 import org.mozilla.fenix.ext.navigateSafe
-import org.mozilla.fenix.ext.searchEngineManager
-import org.mozilla.fenix.ext.settings
-import org.mozilla.fenix.helpers.FenixRobolectricTestRunner
 import org.mozilla.fenix.settings.SupportUtils
 import org.mozilla.fenix.utils.Settings
 
 @ExperimentalCoroutinesApi
-@RunWith(FenixRobolectricTestRunner::class)
 class DefaultSearchControllerTest {
 
-    private val activity: HomeActivity = mockk(relaxed = true)
-    private val store: SearchFragmentStore = mockk(relaxed = true)
-    private val navController: NavController = mockk(relaxed = true)
-    private val defaultSearchEngine: SearchEngine? = mockk(relaxed = true)
-    private val searchEngine: SearchEngine = mockk(relaxed = true)
-    private val metrics: MetricController = mockk(relaxed = true)
-    private val sessionManager: SessionManager = mockk(relaxed = true)
-    private val settings: Settings = mockk(relaxed = true)
-    private val clearToolbarFocus: (() -> Unit) = mockk(relaxed = true)
+    @MockK(relaxed = true) private lateinit var activity: HomeActivity
+    @MockK(relaxed = true) private lateinit var store: SearchFragmentStore
+    @MockK(relaxed = true) private lateinit var navController: NavController
+    @MockK private lateinit var searchEngine: SearchEngine
+    @MockK(relaxed = true) private lateinit var metrics: MetricController
+    @MockK(relaxed = true) private lateinit var settings: Settings
+    @MockK private lateinit var sessionManager: SessionManager
+    @MockK(relaxed = true) private lateinit var clearToolbarFocus: () -> Unit
 
     private lateinit var controller: DefaultSearchController
 
     @Before
     fun setUp() {
-        every { activity.searchEngineManager.defaultSearchEngine } returns defaultSearchEngine
+        MockKAnnotations.init(this)
+        mockkObject(MetricsUtils)
+
         every { store.state.tabId } returns "test-tab-id"
         every { store.state.searchEngineSource.searchEngine } returns searchEngine
-        every { activity.metrics } returns metrics
-        every { activity.components.core.sessionManager } returns sessionManager
-        every { activity.components.settings } returns settings
+        every { sessionManager.select(any()) } just Runs
+        every { MetricsUtils.createSearchEvent(searchEngine, activity, any()) } returns null
 
         controller = DefaultSearchController(
             activity = activity,
+            sessionManager = sessionManager,
             store = store,
             navController = navController,
+            settings = settings,
+            metrics = metrics,
             clearToolbarFocus = clearToolbarFocus
         )
     }
 
+    @After
+    fun teardown() {
+        unmockkObject(MetricsUtils)
+    }
+
     @Test
     fun handleUrlCommitted() {
         val url = "https://www.google.com/"
@@ -86,15 +90,32 @@ class DefaultSearchControllerTest {
         verify { metrics.track(Event.EnteredUrl(false)) }
     }
 
+    @Test
+    fun handleSearchCommitted() {
+        val searchTerm = "Firefox"
+
+        controller.handleUrlCommitted(searchTerm)
+
+        verify {
+            activity.openToBrowserAndLoad(
+                searchTermOrURL = searchTerm,
+                newTab = false,
+                from = BrowserDirection.FromSearch,
+                engine = searchEngine
+            )
+        }
+        verify { settings.incrementActiveSearchCount() }
+    }
+
     @Test
     fun handleCrashesUrlCommitted() {
         val url = "about:crashes"
-        every { activity.packageName } returns testContext.packageName
+        every { activity.packageName } returns "org.mozilla.fenix"
 
         controller.handleUrlCommitted(url)
 
         verify {
-            activity.startActivity(intentFilterEq(Intent(testContext, CrashListActivity::class.java)))
+            activity.startActivity(any())
         }
     }
 
@@ -117,13 +138,6 @@ class DefaultSearchControllerTest {
 
     @Test
     fun handleEditingCancelled() = runBlockingTest {
-        controller = DefaultSearchController(
-            activity = activity,
-            store = store,
-            navController = navController,
-            clearToolbarFocus = clearToolbarFocus
-        )
-
         controller.handleEditingCancelled()
 
         verify {
@@ -183,8 +197,6 @@ class DefaultSearchControllerTest {
     fun `do not show search shortcuts when setting disabled AND query empty AND url not matching query`() {
         every { settings.shouldShowSearchShortcuts } returns false
 
-        assertFalse(testContext.settings().shouldShowSearchShortcuts)
-
         val text = ""
 
         controller.handleTextChanged(text)
@@ -196,8 +208,6 @@ class DefaultSearchControllerTest {
     fun `do not show search shortcuts when setting disabled AND query non-empty`() {
         every { settings.shouldShowSearchShortcuts } returns false
 
-        assertFalse(testContext.settings().shouldShowSearchShortcuts)
-
         val text = "mozilla"
 
         controller.handleTextChanged(text)
@@ -278,11 +288,32 @@ class DefaultSearchControllerTest {
 
     @Test
     fun handleExistingSessionSelected() {
-        val session: Session = mockk(relaxed = true)
+        val session = mockk<Session>()
 
         controller.handleExistingSessionSelected(session)
 
         verify { sessionManager.select(session) }
         verify { activity.openToBrowser(from = BrowserDirection.FromSearch) }
     }
+
+    @Test
+    fun handleExistingSessionSelected_tabId_nullSession() {
+        every { sessionManager.findSessionById("tab-id") } returns null
+
+        controller.handleExistingSessionSelected("tab-id")
+
+        verify(inverse = true) { sessionManager.select(any()) }
+        verify(inverse = true) { activity.openToBrowser(from = BrowserDirection.FromSearch) }
+    }
+
+    @Test
+    fun handleExistingSessionSelected_tabId() {
+        val session = mockk<Session>()
+        every { sessionManager.findSessionById("tab-id") } returns session
+
+        controller.handleExistingSessionSelected("tab-id")
+
+        verify { sessionManager.select(any()) }
+        verify { activity.openToBrowser(from = BrowserDirection.FromSearch) }
+    }
 }