Revert "For #12565: Pass bookmark storage to controller" for debug test failures.

This reverts commit 3c22100b84.
pull/184/head
Sebastian Kaspari 4 years ago
parent cfbad1dae9
commit 4de466883b

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

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

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

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

Loading…
Cancel
Save