For #12565: Pass bookmark storage to controller

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkController.kt

@@ -14,14 +14,14 @@ import kotlinx.coroutines.launch
 import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.engine.prompt.ShareData
 import mozilla.components.concept.storage.BookmarkNode
+import mozilla.components.concept.storage.BookmarksStorage
+import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.service.fxa.sync.SyncReason
 import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
 import org.mozilla.fenix.components.metrics.Event
-import org.mozilla.fenix.ext.bookmarkStorage
-import org.mozilla.fenix.ext.components
 import org.mozilla.fenix.ext.nav
 
 /**
@@ -52,6 +52,8 @@ interface BookmarkController {
 @Suppress("TooManyFunctions")
 class DefaultBookmarkController(
     private val activity: HomeActivity,
+    private val bookmarkStorage: BookmarksStorage,
+    private val accountManager: FxaAccountManager,
     private val navController: NavController,
     private val clipboardManager: ClipboardManager?,
     private val scope: CoroutineScope,
@@ -143,14 +145,14 @@ class DefaultBookmarkController(
         scope.launch {
             store.dispatch(BookmarkFragmentAction.StartSync)
             invokePendingDeletion()
-            activity.components.backgroundServices.accountManager.syncNow(SyncReason.User)
+            accountManager.syncNow(SyncReason.User)
             // The current bookmark node we are viewing may be made invalid after syncing so we
             // check if the current node is valid and if it isn't we find the nearest valid ancestor
             // and open it
             val validAncestorGuid = store.state.guidBackstack.findLast { guid ->
-                activity.bookmarkStorage.getBookmark(guid) != null
+                bookmarkStorage.getBookmark(guid) != null
             } ?: BookmarkRoot.Mobile.id
-            val node = activity.bookmarkStorage.getBookmark(validAncestorGuid)!!
+            val node = bookmarkStorage.getBookmark(validAncestorGuid)!!
             handleBookmarkExpand(node)
             store.dispatch(BookmarkFragmentAction.FinishSync)
         }
@@ -160,12 +162,12 @@ class DefaultBookmarkController(
         invokePendingDeletion.invoke()
         scope.launch {
             val parentGuid = store.state.guidBackstack.findLast { guid ->
-                store.state.tree?.guid != guid && activity.bookmarkStorage.getBookmark(guid) != null
+                store.state.tree?.guid != guid && bookmarkStorage.getBookmark(guid) != null
             }
             if (parentGuid == null) {
                 navController.popBackStack()
             } else {
-                val parent = activity.bookmarkStorage.getBookmark(parentGuid)!!
+                val parent = bookmarkStorage.getBookmark(parentGuid)!!
                 handleBookmarkExpand(parent)
             }
         }

app/src/main/java/org/mozilla/fenix/library/bookmarks/BookmarkFragment.kt

@@ -88,6 +88,8 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), UserInteractionHan
         _bookmarkInteractor = BookmarkFragmentInteractor(
             bookmarksController = DefaultBookmarkController(
                 activity = requireActivity() as HomeActivity,
+                bookmarkStorage = requireComponents.core.bookmarksStorage,
+                accountManager = requireComponents.backgroundServices.accountManager,
                 navController = findNavController(),
                 clipboardManager = requireContext().getSystemService(),
                 scope = viewLifecycleOwner.lifecycleScope,

app/src/test/java/org/mozilla/fenix/library/bookmarks/BookmarkControllerTest.kt

@@ -6,20 +6,17 @@ package org.mozilla.fenix.library.bookmarks
 
 import android.content.ClipData
 import android.content.ClipboardManager
-import android.content.Context
 import androidx.navigation.NavController
-import androidx.navigation.NavDestination
 import androidx.navigation.NavDirections
-import io.mockk.Runs
+import io.mockk.MockKAnnotations
 import io.mockk.called
 import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.every
+import io.mockk.impl.annotations.MockK
 import io.mockk.just
 import io.mockk.mockk
 import io.mockk.runs
-import io.mockk.slot
-import io.mockk.spyk
 import io.mockk.verify
 import io.mockk.verifyOrder
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -27,6 +24,8 @@ import kotlinx.coroutines.test.TestCoroutineScope
 import mozilla.appservices.places.BookmarkRoot
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
+import mozilla.components.concept.storage.BookmarksStorage
+import mozilla.components.service.fxa.manager.FxaAccountManager
 import org.junit.After
 import org.junit.Assert.assertEquals
 import org.junit.Before
@@ -35,31 +34,27 @@ import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.HomeActivity
 import org.mozilla.fenix.R
 import org.mozilla.fenix.browser.browsingmode.BrowsingMode
-import org.mozilla.fenix.components.Services
 import org.mozilla.fenix.components.metrics.Event
-import org.mozilla.fenix.ext.bookmarkStorage
-import org.mozilla.fenix.ext.components
 
-@Suppress("TooManyFunctions", "LargeClass")
 @ExperimentalCoroutinesApi
 class BookmarkControllerTest {
 
-    private lateinit var controller: BookmarkController
-
-    private val bookmarkStore = spyk(BookmarkFragmentStore(BookmarkFragmentState(null)))
-    private val context: Context = mockk(relaxed = true)
     private val scope = TestCoroutineScope()
-    private val clipboardManager: ClipboardManager = mockk(relaxUnitFun = true)
+
-    private val navController: NavController = mockk(relaxed = true)
+    @MockK private lateinit var bookmarkStore: BookmarkFragmentStore
-    private val sharedViewModel: BookmarksSharedViewModel = mockk()
+    @MockK private lateinit var sharedViewModel: BookmarksSharedViewModel
-    private val loadBookmarkNode: suspend (String) -> BookmarkNode? = mockk(relaxed = true)
+    @MockK(relaxUnitFun = true) private lateinit var clipboardManager: ClipboardManager
-    private val showSnackbar: (String) -> Unit = mockk(relaxed = true)
+    @MockK(relaxed = true) private lateinit var homeActivity: HomeActivity
-    private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true)
+    @MockK(relaxed = true) private lateinit var bookmarkStorage: BookmarksStorage
-    private val deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit = mockk(relaxed = true)
+    @MockK(relaxed = true) private lateinit var accountManager: FxaAccountManager
-    private val invokePendingDeletion: () -> Unit = mockk(relaxed = true)
+    @MockK(relaxed = true) private lateinit var navController: NavController
-
+    @MockK(relaxed = true) private lateinit var loadBookmarkNode: suspend (String) -> BookmarkNode?
-    private val homeActivity: HomeActivity = mockk(relaxed = true)
+    @MockK(relaxed = true) private lateinit var showSnackbar: (String) -> Unit
-    private val services: Services = mockk(relaxed = true)
+    @MockK(relaxed = true) private lateinit var deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit
+    @MockK(relaxed = true) private lateinit var deleteBookmarkFolder: (Set<BookmarkNode>) -> Unit
+    @MockK(relaxed = true) private lateinit var invokePendingDeletion: () -> Unit
+
+    private lateinit var controller: BookmarkController
 
     private val item =
         BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0, "Mozilla", "http://mozilla.org", null)
@@ -89,15 +84,20 @@ class BookmarkControllerTest {
 
     @Before
     fun setup() {
-        every { homeActivity.components.services } returns services
+        MockKAnnotations.init(this)
-        every { navController.currentDestination } returns NavDestination("").apply {
+        loadBookmarkNode = mockk(relaxed = true)
-            id = R.id.bookmarkFragment
+
+        every { navController.currentDestination } returns mockk {
+            every { id } returns R.id.bookmarkFragment
         }
+        every { bookmarkStore.state } returns BookmarkFragmentState(null)
         every { bookmarkStore.dispatch(any()) } returns mockk()
         every { sharedViewModel.selectedFolder = any() } just runs
 
         controller = DefaultBookmarkController(
             activity = homeActivity,
+            bookmarkStorage = bookmarkStorage,
+            accountManager = accountManager,
             navController = navController,
             clipboardManager = clipboardManager,
             scope = scope,
@@ -211,12 +211,12 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleBookmarkSelected should show a toast when selecting a root folder`() {
-        val errorMessage = context.getString(R.string.bookmark_cannot_edit_root)
+        every { homeActivity.resources.getString(R.string.bookmark_cannot_edit_root) } returns "Can't edit default folders"
 
         controller.handleBookmarkSelected(root)
 
         verify {
-            showSnackbar(errorMessage)
+            showSnackbar("Can't edit default folders")
         }
     }
 
@@ -240,25 +240,24 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleCopyUrl should copy bookmark url to clipboard and show a toast`() {
-        val urlCopiedMessage = context.getString(R.string.url_copied)
+        every { homeActivity.resources.getString(R.string.url_copied) } returns "URL copied"
 
         controller.handleCopyUrl(item)
 
         verifyOrder {
             ClipData.newPlainText(item.url, item.url)
-            showSnackbar(urlCopiedMessage)
+            showSnackbar("URL copied")
         }
     }
 
     @Test
     fun `handleBookmarkSharing should navigate to the 'Share' fragment`() {
-        val navDirectionsSlot = slot<NavDirections>()
-        every { navController.navigate(capture(navDirectionsSlot), null) } just Runs
-
         controller.handleBookmarkSharing(item)
 
         verify {
-            navController.navigate(navDirectionsSlot.captured, null)
+            navController.navigate(withArg<NavDirections> {
+                assertEquals(R.id.action_global_shareFragment, it.actionId)
+            }, null)
         }
     }
 
@@ -313,8 +312,7 @@ class BookmarkControllerTest {
 
     @Test
     fun `handleRequestSync dispatches actions in the correct order`() {
-        every { homeActivity.components.backgroundServices.accountManager } returns mockk(relaxed = true)
+        coEvery { bookmarkStorage.getBookmark(any()) } returns tree
-        coEvery { homeActivity.bookmarkStorage.getBookmark(any()) } returns tree
         coEvery { loadBookmarkNode.invoke(any()) } returns tree
 
         controller.handleRequestSync()

app/src/test/java/org/mozilla/fenix/share/ShareControllerTest.kt

@@ -83,11 +83,13 @@ class ShareControllerTest {
 
     @Test
     fun `handleShareToApp should start a new sharing activity and close this`() = runBlocking {
+        val appPackageName = "package"
+        val appClassName = "activity"
         val appShareOption = AppShareOption(
             name = "app",
             icon = mockk(),
-            packageName = "package",
+            packageName = appPackageName,
-            activityName = "activity"
+            activityName = appClassName
         )
         val shareIntent = slot<Intent>()
         // Our share Intent uses `FLAG_ACTIVITY_NEW_TASK` but when resolving the startActivity call
@@ -108,8 +110,8 @@ class ShareControllerTest {
         assertEquals(textToShare, shareIntent.captured.extras!![Intent.EXTRA_TEXT])
         assertEquals("text/plain", shareIntent.captured.type)
         assertEquals(Intent.FLAG_ACTIVITY_NEW_TASK, shareIntent.captured.flags)
-        assertEquals("package", shareIntent.captured.component!!.packageName)
+        assertEquals(appPackageName, shareIntent.captured.component!!.packageName)
-        assertEquals("activity", shareIntent.captured.component!!.className)
+        assertEquals(appClassName, shareIntent.captured.component!!.className)
 
         verify { recentAppStorage.updateRecentApp(appShareOption.activityName) }
         verifyOrder {