From f513de5cdbd1fcd9aec44bfe03a1e21f27e2ecbb Mon Sep 17 00:00:00 2001 From: MatthewTighe Date: Thu, 27 Jul 2023 10:13:48 -0700 Subject: [PATCH] Bug 1842249 - Refactor HistoryFragment sync handlers to lib-state --- .../fenix/library/history/HistoryFragment.kt | 45 +++++++++------- .../fenix/library/history/HistoryView.kt | 7 ++- .../history/state/HistorySyncMiddleware.kt | 51 ++++++++++++++++++ .../state/HistorySyncMiddlewareTest.kt | 53 +++++++++++++++++++ 4 files changed, 136 insertions(+), 20 deletions(-) create mode 100644 app/src/main/java/org/mozilla/fenix/library/history/state/HistorySyncMiddleware.kt create mode 100644 app/src/test/java/org/mozilla/fenix/library/history/state/HistorySyncMiddlewareTest.kt diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt index 006e39ef9..6da347e9a 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryFragment.kt @@ -59,6 +59,7 @@ import org.mozilla.fenix.ext.setTextColor import org.mozilla.fenix.home.Mode import org.mozilla.fenix.library.LibraryPageFragment import org.mozilla.fenix.library.history.state.HistoryNavigationMiddleware +import org.mozilla.fenix.library.history.state.HistorySyncMiddleware import org.mozilla.fenix.library.history.state.HistoryTelemetryMiddleware import org.mozilla.fenix.tabstray.Page import org.mozilla.fenix.utils.allowUndo @@ -97,15 +98,24 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, historyStore = StoreProvider.get(this) { HistoryFragmentStore( initialState = HistoryFragmentState.initial, - middleware = listOf( - HistoryNavigationMiddleware( - navController = findNavController(), - openToBrowser = ::openItem, - ), - HistoryTelemetryMiddleware( - isInPrivateMode = requireComponents.appStore.state.mode == Mode.Private, - ), - ), + middleware = if (FeatureFlags.historyFragmentLibStateRefactor) { + listOf( + HistoryNavigationMiddleware( + navController = findNavController(), + openToBrowser = ::openItem, + ), + HistoryTelemetryMiddleware( + isInPrivateMode = requireComponents.appStore.state.mode == Mode.Private, + ), + HistorySyncMiddleware( + accountManager = requireContext().components.backgroundServices.accountManager, + refreshView = { historyView.historyAdapter.refresh() }, + scope = lifecycleScope, + ), + ) + } else { + listOf() + }, ) } val historyController: HistoryController = DefaultHistoryController( @@ -361,16 +371,13 @@ class HistoryFragment : LibraryPageFragment(), UserInteractionHandler, } private fun openItem(item: History.Regular) { - // This telemetry is recorded by the middleware if the refactor is enabled - if (!FeatureFlags.historyFragmentLibStateRefactor) { - GleanHistory.openedItem.record( - GleanHistory.OpenedItemExtra( - isRemote = item.isRemote, - timeGroup = item.historyTimeGroup.toString(), - isPrivate = (activity as HomeActivity).browsingModeManager.mode == BrowsingMode.Private, - ), - ) - } + GleanHistory.openedItem.record( + GleanHistory.OpenedItemExtra( + isRemote = item.isRemote, + timeGroup = item.historyTimeGroup.toString(), + isPrivate = (activity as HomeActivity).browsingModeManager.mode == BrowsingMode.Private, + ), + ) (activity as HomeActivity).openToBrowserAndLoad( searchTermOrURL = item.url, diff --git a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt index db7fa01f7..a29de3161 100644 --- a/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt +++ b/app/src/main/java/org/mozilla/fenix/library/history/HistoryView.kt @@ -12,6 +12,7 @@ import androidx.paging.LoadState import androidx.recyclerview.widget.LinearLayoutManager import androidx.recyclerview.widget.SimpleItemAnimator import mozilla.components.support.base.feature.UserInteractionHandler +import org.mozilla.fenix.FeatureFlags import org.mozilla.fenix.R import org.mozilla.fenix.databinding.ComponentHistoryBinding import org.mozilla.fenix.ext.components @@ -69,7 +70,11 @@ class HistoryView( val primaryTextColor = ThemeManager.resolveAttribute(R.attr.textPrimary, context) binding.swipeRefresh.setColorSchemeColors(primaryTextColor) binding.swipeRefresh.setOnRefreshListener { - interactor.onRequestSync() + if (FeatureFlags.historyFragmentLibStateRefactor) { + store.dispatch(HistoryFragmentAction.StartSync) + } else { + interactor.onRequestSync() + } } } diff --git a/app/src/main/java/org/mozilla/fenix/library/history/state/HistorySyncMiddleware.kt b/app/src/main/java/org/mozilla/fenix/library/history/state/HistorySyncMiddleware.kt new file mode 100644 index 000000000..775bb8161 --- /dev/null +++ b/app/src/main/java/org/mozilla/fenix/library/history/state/HistorySyncMiddleware.kt @@ -0,0 +1,51 @@ +/* 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.history.state + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch +import mozilla.components.lib.state.Middleware +import mozilla.components.lib.state.MiddlewareContext +import mozilla.components.service.fxa.SyncEngine +import mozilla.components.service.fxa.manager.FxaAccountManager +import mozilla.components.service.fxa.sync.SyncReason +import org.mozilla.fenix.library.history.HistoryFragmentAction +import org.mozilla.fenix.library.history.HistoryFragmentState + +/** + * A [Middleware] for handling Sync side-effects based on [HistoryFragmentAction]s that are dispatched + * to the [HistoryFragmentStore]. + * + * @property accountManager The [FxaAccountManager] for handling Sync operations. + * @property refreshView Callback to refresh the view once a Sync is completed. + * @property scope Coroutine scope to launch Sync operations into. + */ +class HistorySyncMiddleware( + private val accountManager: FxaAccountManager, + private val refreshView: () -> Unit, + private val scope: CoroutineScope, +) : Middleware { + override fun invoke( + context: MiddlewareContext, + next: (HistoryFragmentAction) -> Unit, + action: HistoryFragmentAction, + ) { + next(action) + when (action) { + is HistoryFragmentAction.StartSync -> { + scope.launch { + accountManager.syncNow( + reason = SyncReason.User, + debounce = true, + customEngineSubset = listOf(SyncEngine.History), + ) + refreshView() + context.store.dispatch(HistoryFragmentAction.FinishSync) + } + } + else -> Unit + } + } +} diff --git a/app/src/test/java/org/mozilla/fenix/library/history/state/HistorySyncMiddlewareTest.kt b/app/src/test/java/org/mozilla/fenix/library/history/state/HistorySyncMiddlewareTest.kt new file mode 100644 index 000000000..b185136e9 --- /dev/null +++ b/app/src/test/java/org/mozilla/fenix/library/history/state/HistorySyncMiddlewareTest.kt @@ -0,0 +1,53 @@ +/* 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.history.state + +import mozilla.components.service.fxa.SyncEngine +import mozilla.components.service.fxa.manager.FxaAccountManager +import mozilla.components.service.fxa.sync.SyncReason +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 mozilla.components.support.test.rule.runTestOnMain +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.mockito.Mockito.verify +import org.mozilla.fenix.library.history.HistoryFragmentAction +import org.mozilla.fenix.library.history.HistoryFragmentState +import org.mozilla.fenix.library.history.HistoryFragmentStore + +class HistorySyncMiddlewareTest { + @get:Rule + val coroutinesTestRule = MainCoroutineRule() + + @Test + fun `WHEN sync is started THEN account manager handles sync, the view is refreshed, and the sync is finished`() = runTestOnMain { + var viewIsRefreshed = false + val accountManager = mock() + val middleware = HistorySyncMiddleware( + accountManager = accountManager, + refreshView = { viewIsRefreshed = true }, + scope = this, + ) + val store = HistoryFragmentStore( + initialState = HistoryFragmentState.initial, + middleware = listOf(middleware), + ) + + store.dispatch(HistoryFragmentAction.StartSync).joinBlocking() + store.waitUntilIdle() + + assertTrue(viewIsRefreshed) + assertEquals(HistoryFragmentState.Mode.Normal, store.state.mode) + verify(accountManager).syncNow( + reason = SyncReason.User, + debounce = true, + customEngineSubset = listOf(SyncEngine.History), + ) + } +}