Closes #4396 - Add a Bookmarks Controller (#4593)

* For #4396 - Rename BookmarkInteractor methods

Following the naming model used in other Interactors this too will use reactive
method names in the form of "on..." instead of the previous imperative model.

Kept the imperative naming model for the methods from `SelectionInteractor` as
they are a new addition and I'm not sure about the future direction.

* For #4396 - Add a BookmarkController

It abstracts the Fragment behavior in a contract through which various
Interactors can inform about the specific View changes and can ask for
modifications in their container Fragment.

This contract and it's implementation - `DefaultBookmarkController` are the
result of extracting the container Fragment's business logic from
`BookmarkFragmentInteractor` in it's own standalone component.

* For #4396 - Refactored Bookmark related tests

Added a new `BookmarkControllerTest` tests class which complements the new
`BookmarkController` to ensure that it properly operates on `BookmarkFragment`

Also refactored the existing `BookmarkFragmentInteractorTest` to accommodate
`BookmarkFragmentInteractor`'s now more specialized behavior.

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

@@ -0,0 +1,118 @@
+/* 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 org.mozilla.fenix.library.bookmarks
+
+import android.content.Context
+import android.content.res.Resources
+import androidx.navigation.NavController
+import androidx.navigation.NavDirections
+import mozilla.components.concept.storage.BookmarkNode
+import org.mozilla.fenix.BrowserDirection
+import org.mozilla.fenix.BrowsingMode
+import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.R
+import org.mozilla.fenix.components.FenixSnackbarPresenter
+import org.mozilla.fenix.components.Services
+import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.ext.components
+import org.mozilla.fenix.ext.copyUrl
+import org.mozilla.fenix.ext.nav
+
+/**
+ * [BookmarkFragment] controller.
+ * Delegated by View Interactors, handles container business logic and operates changes on it.
+ */
+@SuppressWarnings("TooManyFunctions")
+interface BookmarkController {
+    fun handleBookmarkTapped(item: BookmarkNode)
+    fun handleBookmarkExpand(folder: BookmarkNode)
+    fun handleSelectionModeSwitch()
+    fun handleBookmarkEdit(node: BookmarkNode)
+    fun handleBookmarkSelected(node: BookmarkNode)
+    fun handleCopyUrl(item: BookmarkNode)
+    fun handleBookmarkSharing(item: BookmarkNode)
+    fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode)
+    fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event)
+    fun handleBackPressed()
+    fun handleSigningIn()
+}
+
+@SuppressWarnings("TooManyFunctions")
+class DefaultBookmarkController(
+    private val context: Context,
+    private val navController: NavController,
+    private val snackbarPresenter: FenixSnackbarPresenter,
+    private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit
+) : BookmarkController {
+
+    private val activity: HomeActivity = context as HomeActivity
+    private val resources: Resources = context.resources
+    private val services: Services = activity.components.services
+
+    override fun handleBookmarkTapped(item: BookmarkNode) {
+        openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, BrowsingMode.Normal)
+    }
+
+    override fun handleBookmarkExpand(folder: BookmarkNode) {
+        navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(folder.guid))
+    }
+
+    override fun handleSelectionModeSwitch() {
+        activity.invalidateOptionsMenu()
+    }
+
+    override fun handleBookmarkEdit(node: BookmarkNode) {
+        navigate(BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(node.guid))
+    }
+
+    override fun handleBookmarkSelected(node: BookmarkNode) {
+        snackbarPresenter.present(resources.getString(R.string.bookmark_cannot_edit_root))
+    }
+
+    override fun handleCopyUrl(item: BookmarkNode) {
+        item.copyUrl(context)
+        snackbarPresenter.present(resources.getString(R.string.url_copied))
+    }
+
+    override fun handleBookmarkSharing(item: BookmarkNode) {
+        navigate(BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment(
+                url = item.url!!,
+                title = item.title
+            )
+        )
+    }
+
+    override fun handleOpeningBookmark(item: BookmarkNode, mode: BrowsingMode) {
+        openInNewTab(item.url!!, true, BrowserDirection.FromBookmarks, mode)
+    }
+
+    override fun handleBookmarkDeletion(nodes: Set<BookmarkNode>, eventType: Event) {
+        deleteBookmarkNodes(nodes, eventType)
+    }
+
+    override fun handleBackPressed() {
+        navController.popBackStack()
+    }
+
+    override fun handleSigningIn() {
+        services.launchPairingSignIn(context, navController)
+    }
+
+    private fun openInNewTab(
+        searchTermOrURL: String,
+        newTab: Boolean,
+        from: BrowserDirection,
+        mode: BrowsingMode
+    ) {
+        with(activity) {
+            browsingModeManager.mode = mode
+            openToBrowserAndLoad(searchTermOrURL, newTab, from)
+        }
+    }
+
+    private fun navigate(directions: NavDirections) {
+        navController.nav(R.id.bookmarkFragment, directions)
+    }
+}

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

@@ -64,9 +64,10 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
     private val onDestinationChangedListener =
         NavController.OnDestinationChangedListener { _, destination, args ->
             if (destination.id != R.id.bookmarkFragment ||
-                args != null && BookmarkFragmentArgs.fromBundle(args).currentRoot != currentRoot?.guid
+                args != null && BookmarkFragmentArgs.fromBundle(args).currentRoot != currentRoot?.guid) {
-            )
+
-                bookmarkInteractor.deselectAll()
+                bookmarkInteractor.onAllBookmarksDeselected()
+            }
         }
     lateinit var initialJob: Job
     private var pendingBookmarkDeletionJob: (suspend () -> Unit)? = null
@@ -84,12 +85,15 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
             BookmarkStore(BookmarkState(null))
         }
         bookmarkInteractor = BookmarkFragmentInteractor(
-            context!!,
+            bookmarkStore = bookmarkStore,
-            findNavController(),
+            viewModel = sharedViewModel,
-            bookmarkStore,
+            bookmarksController = DefaultBookmarkController(
-            sharedViewModel,
+                context = context!!,
-            FenixSnackbarPresenter(view),
+                navController = findNavController(),
-            ::deleteMulti
+                snackbarPresenter = FenixSnackbarPresenter(view),
+                deleteBookmarkNodes = ::deleteMulti
+            ),
+            metrics = metrics!!
         )
 
         bookmarkView = BookmarkView(view.bookmarkLayout, bookmarkInteractor)
@@ -137,7 +141,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
 
             if (!isActive) return@launch
             launch(Main) {
-                bookmarkInteractor.change(currentRoot!!)
+                bookmarkInteractor.onBookmarksChanged(currentRoot!!)
                 sharedViewModel.selectedFolder = currentRoot
             }
         }
@@ -146,8 +150,8 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
     private fun checkIfSignedIn() {
         context?.components?.backgroundServices?.accountManager?.let {
             it.register(this, owner = this)
-            it.authenticatedAccount()?.let { bookmarkInteractor.signedIn() }
+            it.authenticatedAccount()?.let { bookmarkInteractor.onSignedIn() }
-                ?: bookmarkInteractor.signedOut()
+                ?: bookmarkInteractor.onSignedOut()
         }
     }
 
@@ -226,14 +230,14 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
     override fun onBackPressed(): Boolean = bookmarkView.onBackPressed()
 
     override fun onAuthenticated(account: OAuthAccount, newAccount: Boolean) {
-        bookmarkInteractor.signedIn()
+        bookmarkInteractor.onSignedIn()
         lifecycleScope.launch {
             refreshBookmarks()
         }
     }
 
     override fun onLoggedOut() {
-        bookmarkInteractor.signedOut()
+        bookmarkInteractor.onSignedOut()
     }
 
     private suspend fun refreshBookmarks() {
@@ -247,7 +251,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
                 pendingBookmarksToDelete.forEach {
                     rootNode -= it.guid
                 }
-                bookmarkInteractor.change(rootNode)
+                bookmarkInteractor.onBookmarksChanged(rootNode)
             }
     }
 
@@ -269,7 +273,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
         pendingBookmarksToDelete.forEach {
             bookmarkTree -= it.guid
         }
-        bookmarkInteractor.change(bookmarkTree!!)
+        bookmarkInteractor.onBookmarksChanged(bookmarkTree!!)
 
         val deleteOperation: (suspend () -> Unit) = {
             deleteSelectedBookmarks(selected)
@@ -293,7 +297,7 @@ class BookmarkFragment : LibraryPageFragment<BookmarkNode>(), BackHandler, Accou
                     bookmarkNode.url?.urlToTrimmedHost(context!!) ?: bookmarkNode.title
                 )
             }
-            else -> throw IllegalStateException("Illegal event type in deleteMulti")
+            else -> throw IllegalStateException("Illegal event type in onDeleteSome")
         }
 
         lifecycleScope.allowUndo(

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

@@ -4,142 +4,79 @@
 
 package org.mozilla.fenix.library.bookmarks
 
-import android.content.Context
-import androidx.navigation.NavController
 import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
-import org.mozilla.fenix.BrowserDirection
 import org.mozilla.fenix.BrowsingMode
-import org.mozilla.fenix.HomeActivity
-import org.mozilla.fenix.R
-import org.mozilla.fenix.components.FenixSnackbarPresenter
 import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.MetricController
-import org.mozilla.fenix.ext.asActivity
+import org.mozilla.fenix.lib.Do
-import org.mozilla.fenix.ext.components
-import org.mozilla.fenix.ext.copyUrl
-import org.mozilla.fenix.ext.nav
 
 /**
  * Interactor for the Bookmarks screen.
  * Provides implementations for the BookmarkViewInteractor.
  *
- * @property context The current Android Context
+ * @property bookmarkStore bookmarks state
- * @property navController The Android Navigation NavController
+ * @property viewModel view state
- * @property bookmarkStore The BookmarkStore
+ * @property bookmarksController view controller
- * @property sharedViewModel The shared ViewModel used between the Bookmarks screens
+ * @property metrics telemetry controller
- * @property snackbarPresenter A presenter for the FenixSnackBar
- * @property deleteBookmarkNodes A lambda function for deleting bookmark nodes with undo
  */
 @SuppressWarnings("TooManyFunctions")
 class BookmarkFragmentInteractor(
-    private val context: Context,
-    private val navController: NavController,
     private val bookmarkStore: BookmarkStore,
-    private val sharedViewModel: BookmarksSharedViewModel,
+    private val viewModel: BookmarksSharedViewModel,
-    private val snackbarPresenter: FenixSnackbarPresenter,
+    private val bookmarksController: BookmarkController,
-    private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit
+    private val metrics: MetricController
 ) : BookmarkViewInteractor, SignInInteractor {
 
-    val activity: HomeActivity?
+    override fun onBookmarksChanged(node: BookmarkNode) {
-        get() = context.asActivity() as? HomeActivity
-    val metrics: MetricController
-        get() = context.components.analytics.metrics
-
-    override fun change(node: BookmarkNode) {
         bookmarkStore.dispatch(BookmarkAction.Change(node))
     }
 
-    override fun open(item: BookmarkNode) {
+    override fun onSelectionModeSwitch(mode: BookmarkState.Mode) {
-        when (item.type) {
+        bookmarksController.handleSelectionModeSwitch()
-            BookmarkNodeType.ITEM -> openItem(item)
-            BookmarkNodeType.FOLDER -> {
-                navController.nav(
-                    R.id.bookmarkFragment,
-                    BookmarkFragmentDirections.actionBookmarkFragmentSelf(item.guid)
-                )
-            }
-            BookmarkNodeType.SEPARATOR -> throw IllegalStateException("Cannot open separators")
-        }
     }
 
-    override fun switchMode(mode: BookmarkState.Mode) {
+    override fun onEditPressed(node: BookmarkNode) {
-        activity?.invalidateOptionsMenu()
+        bookmarksController.handleBookmarkEdit(node)
     }
 
-    override fun edit(node: BookmarkNode) {
+    override fun onAllBookmarksDeselected() {
-        navController.nav(
-            R.id.bookmarkFragment,
-            BookmarkFragmentDirections
-                .actionBookmarkFragmentToBookmarkEditFragment(node.guid)
-        )
-    }
-
-    override fun select(item: BookmarkNode) {
-        if (item.inRoots()) {
-            snackbarPresenter.present(context.getString(R.string.bookmark_cannot_edit_root))
-            return
-        }
-        bookmarkStore.dispatch(BookmarkAction.Select(item))
-    }
-
-    override fun deselect(item: BookmarkNode) {
-        bookmarkStore.dispatch(BookmarkAction.Deselect(item))
-    }
-
-    override fun deselectAll() {
         bookmarkStore.dispatch(BookmarkAction.DeselectAll)
     }
 
-    override fun copy(item: BookmarkNode) {
+    override fun onCopyPressed(item: BookmarkNode) {
         require(item.type == BookmarkNodeType.ITEM)
-        item.copyUrl(activity!!)
+        item.url?.let {
-        snackbarPresenter.present(context.getString(R.string.url_copied))
+            bookmarksController.handleCopyUrl(item)
-        metrics.track(Event.CopyBookmark)
+            metrics.track(Event.CopyBookmark)
+        }
     }
 
-    override fun share(item: BookmarkNode) {
+    override fun onSharePressed(item: BookmarkNode) {
         require(item.type == BookmarkNodeType.ITEM)
-        item.url?.apply {
+        item.url?.let {
-            navController.nav(
+            bookmarksController.handleBookmarkSharing(item)
-                R.id.bookmarkFragment,
-                BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment(
-                    url = this,
-                    title = item.title
-                )
-            )
             metrics.track(Event.ShareBookmark)
         }
     }
 
-    override fun openInNewTab(item: BookmarkNode) {
+    override fun onOpenInNormalTab(item: BookmarkNode) {
-        openItem(item, BrowsingMode.Normal)
-    }
-
-    override fun openInPrivateTab(item: BookmarkNode) {
-        openItem(item, BrowsingMode.Private)
-    }
-
-    private fun openItem(item: BookmarkNode, tabMode: BrowsingMode? = null) {
         require(item.type == BookmarkNodeType.ITEM)
-        item.url?.let { url ->
+        item.url?.let {
-            tabMode?.let { activity?.browsingModeManager?.mode = it }
+            bookmarksController.handleOpeningBookmark(item, BrowsingMode.Normal)
-            activity?.openToBrowserAndLoad(
+            metrics.track(Event.OpenedBookmarkInNewTab)
-                searchTermOrURL = url,
-                newTab = true,
-                from = BrowserDirection.FromBookmarks
-            )
-            metrics.track(
-                when (tabMode) {
-                    BrowsingMode.Private -> Event.OpenedBookmarkInPrivateTab
-                    BrowsingMode.Normal -> Event.OpenedBookmarkInNewTab
-                    null -> Event.OpenedBookmark
-                }
-            )
         }
     }
 
-    override fun delete(nodes: Set<BookmarkNode>) {
+    override fun onOpenInPrivateTab(item: BookmarkNode) {
+        require(item.type == BookmarkNodeType.ITEM)
+        item.url?.let {
+            bookmarksController.handleOpeningBookmark(item, BrowsingMode.Private)
+            metrics.track(Event.OpenedBookmarkInPrivateTab)
+        }
+    }
+
+    override fun onDelete(nodes: Set<BookmarkNode>) {
         if (nodes.find { it.type == BookmarkNodeType.SEPARATOR } != null) {
             throw IllegalStateException("Cannot delete separators")
         }
@@ -149,22 +86,44 @@ class BookmarkFragmentInteractor(
             BookmarkNodeType.FOLDER -> Event.RemoveBookmarkFolder
             null -> Event.RemoveBookmarks
         }
-        deleteBookmarkNodes(nodes, eventType)
+        bookmarksController.handleBookmarkDeletion(nodes, eventType)
     }
 
-    override fun backPressed() {
+    override fun onBackPressed() {
-        navController.popBackStack()
+        bookmarksController.handleBackPressed()
     }
 
-    override fun clickedSignIn() {
+    override fun onSignInPressed() {
-        context.components.services.launchPairingSignIn(context, navController)
+        bookmarksController.handleSigningIn()
     }
 
-    override fun signedIn() {
+    override fun onSignedIn() {
-        sharedViewModel.signedIn.postValue(true)
+        viewModel.signedIn.postValue(true)
     }
 
-    override fun signedOut() {
+    override fun onSignedOut() {
-        sharedViewModel.signedIn.postValue(false)
+        viewModel.signedIn.postValue(false)
+    }
+
+    override fun open(item: BookmarkNode) {
+        Do exhaustive when (item.type) {
+            BookmarkNodeType.ITEM -> {
+                bookmarksController.handleBookmarkTapped(item)
+                metrics.track(Event.OpenedBookmark)
+            }
+            BookmarkNodeType.FOLDER -> bookmarksController.handleBookmarkExpand(item)
+            BookmarkNodeType.SEPARATOR -> throw IllegalStateException("Cannot open separators")
+        }
+    }
+
+    override fun select(item: BookmarkNode) {
+        when (item.inRoots()) {
+            true -> bookmarksController.handleBookmarkSelected(item)
+            false -> bookmarkStore.dispatch(BookmarkAction.Select(item))
+        }
+    }
+
+    override fun deselect(item: BookmarkNode) {
+        bookmarkStore.dispatch(BookmarkAction.Deselect(item))
     }
 }

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

@@ -27,68 +27,68 @@ interface BookmarkViewInteractor : SelectionInteractor<BookmarkNode> {
      *
      * @param node the head node of the new bookmarks tree
      */
-    fun change(node: BookmarkNode)
+    fun onBookmarksChanged(node: BookmarkNode)
 
     /**
      * Switches the current bookmark multi-selection mode.
      *
      * @param mode the multi-select mode to switch to
      */
-    fun switchMode(mode: BookmarkState.Mode)
+    fun onSelectionModeSwitch(mode: BookmarkState.Mode)
 
     /**
      * Opens up an interface to edit a bookmark node.
      *
      * @param node the bookmark node to edit
      */
-    fun edit(node: BookmarkNode)
+    fun onEditPressed(node: BookmarkNode)
 
     /**
      * De-selects all bookmark nodes, clearing the multi-selection mode.
      *
      */
-    fun deselectAll()
+    fun onAllBookmarksDeselected()
 
     /**
      * Copies the URL of a bookmark item to the copy-paste buffer.
      *
      * @param item the bookmark item to copy the URL from
      */
-    fun copy(item: BookmarkNode)
+    fun onCopyPressed(item: BookmarkNode)
 
     /**
      * Opens the share sheet for a bookmark item.
      *
      * @param item the bookmark item to share
      */
-    fun share(item: BookmarkNode)
+    fun onSharePressed(item: BookmarkNode)
 
     /**
      * Opens a bookmark item in a new tab.
      *
      * @param item the bookmark item to open in a new tab
      */
-    fun openInNewTab(item: BookmarkNode)
+    fun onOpenInNormalTab(item: BookmarkNode)
 
     /**
      * Opens a bookmark item in a private tab.
      *
      * @param item the bookmark item to open in a private tab
      */
-    fun openInPrivateTab(item: BookmarkNode)
+    fun onOpenInPrivateTab(item: BookmarkNode)
 
     /**
-     * Deletes a set of bookmark node.
+     * Deletes a set of bookmark nodes.
      *
      * @param nodes the bookmark nodes to delete
      */
-    fun delete(nodes: Set<BookmarkNode>)
+    fun onDelete(nodes: Set<BookmarkNode>)
 
     /**
      * Handles back presses for the bookmark screen, so navigation up the tree is possible.
      *
      */
-    fun backPressed()
+    fun onBackPressed()
 }
 
 class BookmarkView(
@@ -117,7 +117,7 @@ class BookmarkView(
         tree = state.tree
         if (state.mode != mode) {
             mode = state.mode
-            interactor.switchMode(mode)
+            interactor.onSelectionModeSwitch(mode)
         }
 
         bookmarkAdapter.updateData(state.tree, mode)
@@ -132,11 +132,11 @@ class BookmarkView(
     override fun onBackPressed(): Boolean {
         return when {
             mode is BookmarkState.Mode.Selecting -> {
-                interactor.deselectAll()
+                interactor.onAllBookmarksDeselected()
                 true
             }
             canGoBack -> {
-                interactor.backPressed()
+                interactor.onBackPressed()
                 true
             }
             else -> false

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

@@ -12,9 +12,9 @@ import kotlinx.android.extensions.LayoutContainer
 import org.mozilla.fenix.R
 
 interface SignInInteractor {
-    fun clickedSignIn()
+    fun onSignInPressed()
-    fun signedIn()
+    fun onSignedIn()
-    fun signedOut()
+    fun onSignedOut()
 }
 
 class SignInView(
@@ -31,7 +31,7 @@ class SignInView(
 
     init {
         view.setOnClickListener {
-            interactor.clickedSignIn()
+            interactor.onSignInPressed()
         }
     }
 

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

@@ -96,8 +96,8 @@ class SelectBookmarkFolderFragment : Fragment(), AccountObserver {
     private fun checkIfSignedIn() {
         val accountManager = requireComponents.backgroundServices.accountManager
         accountManager.register(this, owner = this)
-        accountManager.authenticatedAccount()?.let { bookmarkInteractor.signedIn() }
+        accountManager.authenticatedAccount()?.let { bookmarkInteractor.onSignedIn() }
-            ?: bookmarkInteractor.signedOut()
+            ?: bookmarkInteractor.onSignedOut()
     }
 
     override fun onCreateOptionsMenu(menu: Menu, inflater: MenuInflater) {
@@ -125,10 +125,10 @@ class SelectBookmarkFolderFragment : Fragment(), AccountObserver {
         }
     }
     override fun onAuthenticated(account: OAuthAccount, newAccount: Boolean) {
-        bookmarkInteractor.signedIn()
+        bookmarkInteractor.onSignedIn()
     }
 
     override fun onLoggedOut() {
-        bookmarkInteractor.signedOut()
+        bookmarkInteractor.onSignedOut()
     }
 }

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

@@ -16,15 +16,15 @@ class SelectBookmarkFolderInteractor(
     private val sharedViewModel: BookmarksSharedViewModel
 ) : SignInInteractor {
 
-    override fun clickedSignIn() {
+    override fun onSignInPressed() {
         context.components.services.launchPairingSignIn(context, navController)
     }
 
-    override fun signedIn() {
+    override fun onSignedIn() {
         sharedViewModel.signedIn.postValue(true)
     }
 
-    override fun signedOut() {
+    override fun onSignedOut() {
         sharedViewModel.signedIn.postValue(false)
     }
 }

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

@@ -30,13 +30,13 @@ abstract class BookmarkNodeViewHolder(
     protected fun setupMenu(item: BookmarkNode) {
         val bookmarkItemMenu = BookmarkItemMenu(containerView.context, item) {
             when (it) {
-                BookmarkItemMenu.Item.Edit -> interactor.edit(item)
+                BookmarkItemMenu.Item.Edit -> interactor.onEditPressed(item)
                 BookmarkItemMenu.Item.Select -> interactor.select(item)
-                BookmarkItemMenu.Item.Copy -> interactor.copy(item)
+                BookmarkItemMenu.Item.Copy -> interactor.onCopyPressed(item)
-                BookmarkItemMenu.Item.Share -> interactor.share(item)
+                BookmarkItemMenu.Item.Share -> interactor.onSharePressed(item)
-                BookmarkItemMenu.Item.OpenInNewTab -> interactor.openInNewTab(item)
+                BookmarkItemMenu.Item.OpenInNewTab -> interactor.onOpenInNormalTab(item)
-                BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.openInPrivateTab(item)
+                BookmarkItemMenu.Item.OpenInPrivateTab -> interactor.onOpenInPrivateTab(item)
-                BookmarkItemMenu.Item.Delete -> interactor.delete(setOf(item))
+                BookmarkItemMenu.Item.Delete -> interactor.onDelete(setOf(item))
             }
         }
 

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

@@ -0,0 +1,217 @@
+/* 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 org.mozilla.fenix.library.bookmarks
+
+import android.content.ClipData
+import android.content.ClipboardManager
+import android.content.Context
+import androidx.core.content.getSystemService
+import androidx.navigation.NavController
+import androidx.navigation.NavDestination
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.mockkStatic
+import io.mockk.verify
+import io.mockk.verifyOrder
+import mozilla.appservices.places.BookmarkRoot
+import mozilla.components.concept.storage.BookmarkNode
+import mozilla.components.concept.storage.BookmarkNodeType
+import org.junit.Before
+import org.junit.Test
+import org.mozilla.fenix.BrowserDirection
+import org.mozilla.fenix.BrowsingMode
+import org.mozilla.fenix.HomeActivity
+import org.mozilla.fenix.R
+import org.mozilla.fenix.components.FenixSnackbarPresenter
+import org.mozilla.fenix.components.Services
+import org.mozilla.fenix.components.metrics.Event
+import org.mozilla.fenix.ext.components
+
+@SuppressWarnings("TooManyFunctions", "LargeClass")
+class BookmarkControllerTest {
+
+    private lateinit var controller: BookmarkController
+
+    private val context: Context = mockk(relaxed = true)
+    private val navController: NavController = mockk(relaxed = true)
+    private val snackbarPresenter: FenixSnackbarPresenter = mockk(relaxed = true)
+    private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true)
+
+    private val homeActivity: HomeActivity = mockk(relaxed = true)
+    private val services: Services = mockk(relaxed = true)
+
+    private val item = BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0, "Mozilla", "http://mozilla.org", null)
+    private val subfolder = BookmarkNode(BookmarkNodeType.FOLDER, "987", "123", 0, "Subfolder", null, listOf())
+    private val childItem = BookmarkNode(
+        BookmarkNodeType.ITEM, "987", "123", 2, "Firefox", "https://www.mozilla.org/en-US/firefox/", null
+    )
+    private val tree = BookmarkNode(
+        BookmarkNodeType.FOLDER, "123", null, 0, "Mobile", null, listOf(item, item, childItem, subfolder)
+    )
+    private val root = BookmarkNode(
+        BookmarkNodeType.FOLDER, BookmarkRoot.Root.id, null, 0, BookmarkRoot.Root.name, null, null
+    )
+
+    @Before
+    fun setup() {
+        // needed for mocking 'getSystemService<ClipboardManager>()'
+        mockkStatic(
+            "androidx.core.content.ContextCompat",
+            "android.content.ClipData"
+        )
+
+        every { homeActivity.components.services } returns services
+        every { navController.currentDestination } returns NavDestination("").apply { id = R.id.bookmarkFragment }
+
+        controller = DefaultBookmarkController(
+            context = homeActivity,
+            navController = navController,
+            snackbarPresenter = snackbarPresenter,
+            deleteBookmarkNodes = deleteBookmarkNodes
+        )
+    }
+
+    @Test
+    fun `handleBookmarkTapped should load the bookmark in a new tab`() {
+        controller.handleBookmarkTapped(item)
+
+        verifyOrder {
+            homeActivity.browsingModeManager.mode = BrowsingMode.Normal
+            homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks)
+        }
+    }
+
+    @Test
+    fun `handleBookmarkExpand should navigate to the 'Bookmark' fragment`() {
+        controller.handleBookmarkExpand(tree)
+
+        verify {
+            navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(tree.guid))
+        }
+    }
+
+    @Test
+    fun `handleSelectionModeSwitch should invalidateOptionsMenu`() {
+        controller.handleSelectionModeSwitch()
+
+        verify {
+            homeActivity.invalidateOptionsMenu()
+        }
+    }
+
+    @Test
+    fun `handleBookmarkEdit should navigate to the 'Edit' fragment`() {
+        controller.handleBookmarkEdit(item)
+
+        verify {
+            navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(item.guid))
+        }
+    }
+
+    @Test
+    fun `handleBookmarkSelected should show a toast when selecting a folder`() {
+        val errorMessage = context.getString(R.string.bookmark_cannot_edit_root)
+
+        controller.handleBookmarkSelected(root)
+
+        verify {
+            snackbarPresenter.present(errorMessage, any(), any(), any())
+        }
+    }
+
+    @Test
+    fun `handleCopyUrl should copy bookmark url to clipboard and show a toast`() {
+        val clipboardManager: ClipboardManager = mockk(relaxed = true)
+        val urlCopiedMessage = context.getString(R.string.url_copied)
+        every { any<Context>().getSystemService<ClipboardManager>() } returns clipboardManager
+        every { ClipData.newPlainText(any(), any()) } returns mockk(relaxed = true)
+
+        controller.handleCopyUrl(item)
+
+        verifyOrder {
+            ClipData.newPlainText(item.url, item.url)
+            snackbarPresenter.present(urlCopiedMessage, any(), any(), any())
+        }
+    }
+
+    @Test
+    fun `handleBookmarkSharing should navigate to the 'Share' fragment`() {
+        controller.handleBookmarkSharing(item)
+
+        verify {
+            navController.navigate(
+                BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment(
+                    item.url,
+                    item.title
+                )
+            )
+        }
+    }
+
+    @Test
+    fun `handleOpeningBookmark should open the bookmark a new 'Normal' tab`() {
+        controller.handleOpeningBookmark(item, BrowsingMode.Normal)
+
+        verifyOrder {
+            homeActivity.browsingModeManager.mode = BrowsingMode.Normal
+            homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks)
+        }
+    }
+
+    @Test
+    fun `handleOpeningBookmark should open the bookmark a new 'Private' tab`() {
+        controller.handleOpeningBookmark(item, BrowsingMode.Private)
+
+        verifyOrder {
+            homeActivity.browsingModeManager.mode = BrowsingMode.Private
+            homeActivity.openToBrowserAndLoad(item.url!!, true, BrowserDirection.FromBookmarks)
+        }
+    }
+
+    @Test
+    fun `handleBookmarkDeletion for an item should properly call a passed in delegate`() {
+        controller.handleBookmarkDeletion(setOf(item), Event.RemoveBookmark)
+
+        verify {
+            deleteBookmarkNodes(setOf(item), Event.RemoveBookmark)
+        }
+    }
+
+    @Test
+    fun `handleBookmarkDeletion for multiple bookmarks should properly call a passed in delegate`() {
+        controller.handleBookmarkDeletion(setOf(item, subfolder), Event.RemoveBookmarks)
+
+        verify {
+            deleteBookmarkNodes(setOf(item, subfolder), Event.RemoveBookmarks)
+        }
+    }
+
+    @Test
+    fun `handleBookmarkDeletion for a folder should properly call a passed in delegate`() {
+        controller.handleBookmarkDeletion(setOf(subfolder), Event.RemoveBookmarkFolder)
+
+        verify {
+            deleteBookmarkNodes(setOf(subfolder), Event.RemoveBookmarkFolder)
+        }
+    }
+
+    @Test
+    fun `handleBackPressed should trigger handleBackPressed in NavController`() {
+        controller.handleBackPressed()
+
+        verify {
+            navController.popBackStack()
+        }
+    }
+
+    @Test
+    fun `handleSigningIn should trigger 'PairingSignIn`() {
+        controller.handleSigningIn()
+
+        verify {
+            services.launchPairingSignIn(homeActivity, navController)
+        }
+    }
+}

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

@@ -4,16 +4,9 @@
 
 package org.mozilla.fenix.library.bookmarks
 
-import android.content.ClipData
-import android.content.ClipboardManager
-import android.content.Context
-import androidx.core.content.getSystemService
-import androidx.navigation.NavController
-import androidx.navigation.NavDestination
 import io.mockk.called
 import io.mockk.every
 import io.mockk.mockk
-import io.mockk.mockkStatic
 import io.mockk.spyk
 import io.mockk.verify
 import io.mockk.verifyOrder
@@ -22,46 +15,25 @@ import mozilla.components.concept.storage.BookmarkNode
 import mozilla.components.concept.storage.BookmarkNodeType
 import org.junit.Before
 import org.junit.Test
-import org.mozilla.fenix.BrowserDirection
+import org.mozilla.fenix.BrowsingMode
-import org.mozilla.fenix.FenixApplication
-import org.mozilla.fenix.HomeActivity
-import org.mozilla.fenix.R
-import org.mozilla.fenix.components.FenixSnackbarPresenter
-import org.mozilla.fenix.components.Services
 import org.mozilla.fenix.components.metrics.Event
 import org.mozilla.fenix.components.metrics.MetricController
-import org.mozilla.fenix.ext.asActivity
-import org.mozilla.fenix.ext.components
 
+@SuppressWarnings("TooManyFunctions", "LargeClass")
 class BookmarkFragmentInteractorTest {
 
     private lateinit var interactor: BookmarkFragmentInteractor
 
-    private val context: Context = mockk(relaxed = true)
-    private val navController: NavController = mockk(relaxed = true)
     private val bookmarkStore = spyk(BookmarkStore(BookmarkState(null)))
     private val sharedViewModel: BookmarksSharedViewModel = mockk(relaxed = true)
-    private val snackbarPresenter: FenixSnackbarPresenter = mockk(relaxed = true)
+    private val bookmarkController: DefaultBookmarkController = mockk(relaxed = true)
-    private val deleteBookmarkNodes: (Set<BookmarkNode>, Event) -> Unit = mockk(relaxed = true)
-
-    private val applicationContext: FenixApplication = mockk(relaxed = true)
-    private val homeActivity: HomeActivity = mockk(relaxed = true)
     private val metrics: MetricController = mockk(relaxed = true)
 
     private val item = BookmarkNode(BookmarkNodeType.ITEM, "456", "123", 0, "Mozilla", "http://mozilla.org", null)
     private val separator = BookmarkNode(BookmarkNodeType.SEPARATOR, "789", "123", 1, null, null, null)
     private val subfolder = BookmarkNode(BookmarkNodeType.FOLDER, "987", "123", 0, "Subfolder", null, listOf())
-    private val childItem = BookmarkNode(
+    private val tree: BookmarkNode = BookmarkNode(
-        BookmarkNodeType.ITEM,
+        BookmarkNodeType.FOLDER, "123", null, 0, "Mobile", null, listOf(item, separator, item, subfolder)
-        "987",
-        "123",
-        2,
-        "Firefox",
-        "https://www.mozilla.org/en-US/firefox/",
-        null
-    )
-    private val tree = BookmarkNode(
-        BookmarkNodeType.FOLDER, "123", null, 0, "Mobile", null, listOf(item, separator, childItem, subfolder)
     )
     private val root = BookmarkNode(
         BookmarkNodeType.FOLDER, BookmarkRoot.Root.id, null, 0, BookmarkRoot.Root.name, null, null
@@ -69,31 +41,20 @@ class BookmarkFragmentInteractorTest {
 
     @Before
     fun setup() {
-        mockkStatic(
-            "org.mozilla.fenix.ext.ContextKt",
-            "androidx.core.content.ContextCompat",
-            "android.content.ClipData"
-        )
-        every { any<Context>().asActivity() } returns homeActivity
-        every { context.applicationContext } returns applicationContext
-        every { applicationContext.components.analytics.metrics } returns metrics
-        every { navController.currentDestination } returns NavDestination("").apply { id = R.id.bookmarkFragment }
         every { bookmarkStore.dispatch(any()) } returns mockk()
 
         interactor =
             BookmarkFragmentInteractor(
-                context,
+                bookmarkStore = bookmarkStore,
-                navController,
+                viewModel = sharedViewModel,
-                bookmarkStore,
+                bookmarksController = bookmarkController,
-                sharedViewModel,
+                metrics = metrics
-                snackbarPresenter,
-                deleteBookmarkNodes
             )
     }
 
     @Test
     fun `update bookmarks tree`() {
-        interactor.change(tree)
+        interactor.onBookmarksChanged(tree)
 
         verify {
             bookmarkStore.dispatch(BookmarkAction.Change(tree))
@@ -104,15 +65,10 @@ class BookmarkFragmentInteractorTest {
     fun `open a bookmark item`() {
         interactor.open(item)
 
-        val url = item.url!!
         verifyOrder {
-            homeActivity.openToBrowserAndLoad(
+            bookmarkController.handleBookmarkTapped(item)
-                searchTermOrURL = url,
+            metrics.track(Event.OpenedBookmark)
-                newTab = true,
-                from = BrowserDirection.FromBookmarks
-            )
         }
-        metrics.track(Event.OpenedBookmark)
     }
 
     @Test(expected = IllegalStateException::class)
@@ -125,25 +81,25 @@ class BookmarkFragmentInteractorTest {
         interactor.open(tree)
 
         verify {
-            navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentSelf(tree.guid))
+            bookmarkController.handleBookmarkExpand(tree)
         }
     }
 
     @Test
     fun `switch between bookmark selection modes`() {
-        interactor.switchMode(BookmarkState.Mode.Normal)
+        interactor.onSelectionModeSwitch(BookmarkState.Mode.Normal)
 
         verify {
-            homeActivity.invalidateOptionsMenu()
+            bookmarkController.handleSelectionModeSwitch()
         }
     }
 
     @Test
     fun `press the edit bookmark button`() {
-        interactor.edit(item)
+        interactor.onEditPressed(item)
 
         verify {
-            navController.navigate(BookmarkFragmentDirections.actionBookmarkFragmentToBookmarkEditFragment(item.guid))
+            bookmarkController.handleBookmarkEdit(item)
         }
     }
 
@@ -167,7 +123,7 @@ class BookmarkFragmentInteractorTest {
 
     @Test
     fun `deselectAll bookmark items`() {
-        interactor.deselectAll()
+        interactor.onAllBookmarksDeselected()
 
         verify {
             bookmarkStore.dispatch(BookmarkAction.DeselectAll)
@@ -183,119 +139,97 @@ class BookmarkFragmentInteractorTest {
 
     @Test
     fun `copy a bookmark item`() {
-        val clipboardManager: ClipboardManager = mockk(relaxed = true)
+        interactor.onCopyPressed(item)
-        every { any<Context>().getSystemService<ClipboardManager>() } returns clipboardManager
-        every { ClipData.newPlainText(any(), any()) } returns mockk(relaxed = true)
 
-        interactor.copy(item)
+        verifyOrder {
-
+            bookmarkController.handleCopyUrl(item)
-        verify {
             metrics.track(Event.CopyBookmark)
         }
     }
 
     @Test
     fun `share a bookmark item`() {
-        interactor.share(item)
+        interactor.onSharePressed(item)
 
         verifyOrder {
-            navController.navigate(
+            bookmarkController.handleBookmarkSharing(item)
-                BookmarkFragmentDirections.actionBookmarkFragmentToShareFragment(
-                    item.url,
-                    item.title
-                )
-            )
             metrics.track(Event.ShareBookmark)
         }
     }
 
     @Test
     fun `open a bookmark item in a new tab`() {
-        interactor.openInNewTab(item)
+        interactor.onOpenInNormalTab(item)
 
-        val url = item.url!!
         verifyOrder {
-            homeActivity.openToBrowserAndLoad(
+            bookmarkController.handleOpeningBookmark(item, BrowsingMode.Normal)
-                searchTermOrURL = url,
-                newTab = true,
-                from = BrowserDirection.FromBookmarks
-            )
             metrics.track(Event.OpenedBookmarkInNewTab)
         }
     }
 
     @Test
     fun `open a bookmark item in a private tab`() {
-        interactor.openInPrivateTab(item)
+        interactor.onOpenInPrivateTab(item)
 
-        val url = item.url!!
         verifyOrder {
-            homeActivity.openToBrowserAndLoad(
+            bookmarkController.handleOpeningBookmark(item, BrowsingMode.Private)
-                searchTermOrURL = url,
-                newTab = true,
-                from = BrowserDirection.FromBookmarks
-            )
             metrics.track(Event.OpenedBookmarkInPrivateTab)
         }
     }
 
     @Test
     fun `delete a bookmark item`() {
-        interactor.delete(setOf(item))
+        interactor.onDelete(setOf(item))
 
         verify {
-            deleteBookmarkNodes(setOf(item), Event.RemoveBookmark)
+            bookmarkController.handleBookmarkDeletion(setOf(item), Event.RemoveBookmark)
         }
     }
 
     @Test(expected = IllegalStateException::class)
     fun `delete a separator`() {
-        interactor.delete(setOf(item, item.copy(type = BookmarkNodeType.SEPARATOR)))
+        interactor.onDelete(setOf(item, item.copy(type = BookmarkNodeType.SEPARATOR)))
     }
 
     @Test
     fun `delete a bookmark folder`() {
-        interactor.delete(setOf(subfolder))
+        interactor.onDelete(setOf(subfolder))
 
         verify {
-            deleteBookmarkNodes(setOf(subfolder), Event.RemoveBookmarkFolder)
+            bookmarkController.handleBookmarkDeletion(setOf(subfolder), Event.RemoveBookmarkFolder)
         }
     }
 
     @Test
     fun `delete multiple bookmarks`() {
-        interactor.delete(setOf(item, subfolder))
+        interactor.onDelete(setOf(item, subfolder))
 
         verify {
-            deleteBookmarkNodes(setOf(item, subfolder), Event.RemoveBookmarks)
+            bookmarkController.handleBookmarkDeletion(setOf(item, subfolder), Event.RemoveBookmarks)
         }
     }
 
     @Test
     fun `press the back button`() {
-        interactor.backPressed()
+        interactor.onBackPressed()
 
         verify {
-            navController.popBackStack()
+            bookmarkController.handleBackPressed()
         }
     }
 
     @Test
     fun `clicked sign in on bookmarks screen`() {
-        val services: Services = mockk(relaxed = true)
+        interactor.onSignInPressed()
-        every { context.components.services } returns services
-
-        interactor.clickedSignIn()
 
         verify {
-            context.components.services
+            bookmarkController.handleSigningIn()
-            services.launchPairingSignIn(context, navController)
         }
     }
 
     @Test
     fun `got signed in signal on bookmarks screen`() {
-        interactor.signedIn()
+        interactor.onSignedIn()
 
         verify {
             sharedViewModel.signedIn.postValue(true)
@@ -304,7 +238,7 @@ class BookmarkFragmentInteractorTest {
 
     @Test
     fun `got signed out signal on bookmarks screen`() {
-        interactor.signedOut()
+        interactor.onSignedOut()
 
         verify {
             sharedViewModel.signedIn.postValue(false)