commit 5fd8f62d62034715acb81fe9b7294222d3790a52
parent d0f27a2fe758a1098430a3e670e709e13d15a55c
Author: mcarare <48995920+mcarare@users.noreply.github.com>
Date: Tue, 2 Dec 2025 13:50:07 +0000
Bug 2002956 - Refactor SyncStoreSupport to use injected dispatchers. r=android-reviewers,jonalmeida
`FxaAccountObserver` now creates its own `CoroutineScope` backed by the injected IO dispatcher and a `SupervisorJob`.
Differential Revision: https://phabricator.services.mozilla.com/D274391
Diffstat:
2 files changed, 36 insertions(+), 26 deletions(-)
diff --git a/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/store/SyncStoreSupport.kt b/mobile/android/android-components/components/service/firefox-accounts/src/main/java/mozilla/components/service/fxa/store/SyncStoreSupport.kt
@@ -6,8 +6,10 @@ package mozilla.components.service.fxa.store
import androidx.lifecycle.LifecycleOwner
import androidx.lifecycle.ProcessLifecycleOwner
+import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
import kotlinx.coroutines.launch
import mozilla.components.concept.sync.AccountObserver
import mozilla.components.concept.sync.AuthFlowError
@@ -19,7 +21,6 @@ import mozilla.components.concept.sync.Profile
import mozilla.components.service.fxa.manager.AccountState
import mozilla.components.service.fxa.manager.FxaAccountManager
import mozilla.components.service.fxa.sync.SyncStatusObserver
-import java.lang.Exception
/**
* Connections an [FxaAccountManager] with a [SyncStore], so that updates to Sync
@@ -30,14 +31,16 @@ import java.lang.Exception
* @param lifecycleOwner The lifecycle owner that will tie to the when account manager observations.
* Recommended that this be an Application or at minimum a persistent Activity.
* @param autoPause Whether the account manager observations will stop between onPause and onResume.
- * @param coroutineScope Scope used to launch various suspending operations.
+ * @param ioDispatcher Dispatcher used for background operations.
+ * @param mainDispatcher Dispatcher used for main thread operations.
*/
class SyncStoreSupport(
private val store: SyncStore,
private val fxaAccountManager: Lazy<FxaAccountManager>,
private val lifecycleOwner: LifecycleOwner = ProcessLifecycleOwner.get(),
private val autoPause: Boolean = false,
- private val coroutineScope: CoroutineScope = CoroutineScope(Dispatchers.IO),
+ private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO,
+ private val mainDispatcher: CoroutineDispatcher = Dispatchers.Main,
) {
/**
* Initialize the integration. This will cause it to register itself as an observer
@@ -56,7 +59,8 @@ class SyncStoreSupport(
ConstellationObserver(store),
lifecycleOwner,
autoPause,
- coroutineScope,
+ ioDispatcher,
+ mainDispatcher,
)
accountManager.register(accountObserver, owner = lifecycleOwner, autoPause = autoPause)
}
@@ -87,26 +91,31 @@ internal class AccountSyncObserver(private val store: SyncStore) : SyncStatusObs
* @param store The [SyncStore] that updates will be dispatched to.
* @param deviceConstellationObserver Will be registered as an observer to any constellations
* received in [AccountObserver.onAuthenticated].
- *
- * See [SyncStoreSupport] for the rest of the param definitions.
+ * @param lifecycleOwner The lifecycle owner that will tie to the when account manager observations.
+ * @param autoPause Whether the account manager observations will stop between onPause and onResume.
+ * @param ioDispatcher Dispatcher used for background operations.
+ * @param mainDispatcher Dispatcher used for main thread operations.
*/
internal class FxaAccountObserver(
private val store: SyncStore,
private val deviceConstellationObserver: DeviceConstellationObserver,
private val lifecycleOwner: LifecycleOwner,
private val autoPause: Boolean,
- private val coroutineScope: CoroutineScope,
+ ioDispatcher: CoroutineDispatcher,
+ private val mainDispatcher: CoroutineDispatcher,
) : AccountObserver {
+ private val scope = CoroutineScope(ioDispatcher + SupervisorJob())
+
override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
- coroutineScope.launch(Dispatchers.Main) {
+ scope.launch(mainDispatcher) {
account.deviceConstellation().registerDeviceObserver(
deviceConstellationObserver,
owner = lifecycleOwner,
autoPause = autoPause,
)
}
- coroutineScope.launch {
+ scope.launch {
val syncAccount = account.getProfile()?.toAccount(account) ?: return@launch
store.dispatch(SyncAction.UpdateAccount(syncAccount))
store.dispatch(SyncAction.UpdateAccountState(AccountState.Authenticated))
diff --git a/mobile/android/android-components/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/store/SyncStoreSupportTest.kt b/mobile/android/android-components/components/service/firefox-accounts/src/test/java/mozilla/components/service/fxa/store/SyncStoreSupportTest.kt
@@ -5,6 +5,7 @@
package mozilla.components.service.fxa.store
import androidx.lifecycle.LifecycleOwner
+import kotlinx.coroutines.test.StandardTestDispatcher
import kotlinx.coroutines.test.runTest
import mozilla.components.concept.sync.AuthFlowError
import mozilla.components.concept.sync.AuthType
@@ -20,21 +21,16 @@ import mozilla.components.support.test.any
import mozilla.components.support.test.coMock
import mozilla.components.support.test.eq
import mozilla.components.support.test.mock
-import mozilla.components.support.test.rule.MainCoroutineRule
import mozilla.components.support.test.whenever
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
import org.junit.Before
-import org.junit.Rule
import org.junit.Test
import org.mockito.Mockito.verify
import org.mockito.Mockito.`when`
class SyncStoreSupportTest {
-
- @get:Rule
- val mainCoroutineRule = MainCoroutineRule()
-
+ private val testDispatcher = StandardTestDispatcher()
private val accountManager = mock<FxaAccountManager>()
private val lifecycleOwner = mock<LifecycleOwner>()
private val autoPause = false
@@ -55,7 +51,8 @@ class SyncStoreSupportTest {
deviceConstellationObserver = constellationObserver,
lifecycleOwner = lifecycleOwner,
autoPause = autoPause,
- coroutineScope = mainCoroutineRule.scope,
+ ioDispatcher = testDispatcher,
+ mainDispatcher = testDispatcher,
)
integration = SyncStoreSupport(
@@ -63,7 +60,8 @@ class SyncStoreSupportTest {
fxaAccountManager = lazyOf(accountManager),
lifecycleOwner = lifecycleOwner,
autoPause = autoPause,
- coroutineScope = mainCoroutineRule.scope,
+ ioDispatcher = testDispatcher,
+ mainDispatcher = testDispatcher,
)
}
@@ -97,7 +95,7 @@ class SyncStoreSupportTest {
}
@Test
- fun `GIVEN account observer WHEN onAuthenticated observed THEN device observer registered`() = runTest {
+ fun `GIVEN account observer WHEN onAuthenticated observed THEN device observer registered`() = runTest(testDispatcher) {
val constellation = mock<DeviceConstellation>()
val account = mock<OAuthAccount> {
whenever(deviceConstellation()).thenReturn(constellation)
@@ -105,11 +103,13 @@ class SyncStoreSupportTest {
accountObserver.onAuthenticated(account, mock<AuthType.Existing>())
+ testDispatcher.scheduler.advanceUntilIdle()
+
verify(constellation).registerDeviceObserver(constellationObserver, lifecycleOwner, autoPause)
}
@Test
- fun `GIVEN account observer WHEN onAuthenticated observed with profile THEN account and account state are updated`() = runTest {
+ fun `GIVEN account observer WHEN onAuthenticated observed with profile THEN account and account state are updated`() = runTest(testDispatcher) {
val profile = generateProfile()
val constellation = mock<DeviceConstellation>()
val account = coMock<OAuthAccount> {
@@ -122,6 +122,7 @@ class SyncStoreSupportTest {
assertEquals(AccountState.NotAuthenticated, store.state.accountState)
accountObserver.onAuthenticated(account, AuthType.Existing)
+ testDispatcher.scheduler.advanceUntilIdle()
val expected = Account(
profile.uid,
@@ -136,7 +137,7 @@ class SyncStoreSupportTest {
}
@Test
- fun `GIVEN account observer WHEN onAuthenticated observed without profile THEN account and account state are not updated`() = runTest {
+ fun `GIVEN account observer WHEN onAuthenticated observed without profile THEN account and account state are not updated`() = runTest(testDispatcher) {
val constellation = mock<DeviceConstellation>()
val account = coMock<OAuthAccount> {
whenever(deviceConstellation()).thenReturn(constellation)
@@ -150,7 +151,7 @@ class SyncStoreSupportTest {
}
@Test
- fun `GIVEN user is logged in WHEN onLoggedOut observed THEN sync status and account states are updated`() = runTest {
+ fun `GIVEN user is logged in WHEN onLoggedOut observed THEN sync status and account states are updated`() = runTest(testDispatcher) {
val account = coMock<OAuthAccount> {
whenever(deviceConstellation()).thenReturn(mock())
whenever(getProfile()).thenReturn(null)
@@ -165,7 +166,7 @@ class SyncStoreSupportTest {
}
@Test
- fun `GIVEN account observer WHEN onAuthenticationProblems observed THEN account state is updated`() = runTest {
+ fun `GIVEN account observer WHEN onAuthenticationProblems observed THEN account state is updated`() {
assertEquals(AccountState.NotAuthenticated, store.state.accountState)
accountObserver.onAuthenticationProblems()
@@ -174,7 +175,7 @@ class SyncStoreSupportTest {
}
@Test
- fun `GIVEN account observer WHEN onFlowError observed THEN account state is updated`() = runTest {
+ fun `GIVEN account observer WHEN onFlowError observed THEN account state is updated`() {
assertNull(store.state.account)
assertEquals(AccountState.NotAuthenticated, store.state.accountState)
@@ -185,7 +186,7 @@ class SyncStoreSupportTest {
}
@Test
- fun `GIVEN account observer WHEN onProfileUpdated then update the account state`() = runTest {
+ fun `GIVEN account observer WHEN onProfileUpdated then update the account state`() {
// Prerequisite is having a non-null account already.
store.dispatch(SyncAction.UpdateAccount(Account(null, null, null, null, null, null)))
@@ -199,7 +200,7 @@ class SyncStoreSupportTest {
}
@Test
- fun `GIVEN account observer WHEN onReady is triggered THEN do nothing`() = runTest {
+ fun `GIVEN account observer WHEN onReady is triggered THEN do nothing`() = runTest(testDispatcher) {
// `onReady` is too early for us (today) to try and get the auth status from the cached value.
// See: https://bugzilla.mozilla.org/show_bug.cgi?id=1909779
val currentDeviceId = "id"
@@ -229,7 +230,7 @@ class SyncStoreSupportTest {
}
@Test
- fun `GIVEN account observer WHEN onReady observed without profile THEN account states are not updated`() = runTest {
+ fun `GIVEN account observer WHEN onReady observed without profile THEN account states are not updated`() = runTest(testDispatcher) {
val constellation = mock<DeviceConstellation>()
val account = coMock<OAuthAccount> {
whenever(deviceConstellation()).thenReturn(constellation)
