Close #9829: Move IO work in FxaPushSupportFeature to a coroutine

The `FxaPushSupportFeature` needs to generate, read and write a push
scope (identifier) to disk. This work was meant to be done lazily and
not during app startup. However, if a Sync account is setup then the IO
work is done immediately during the initial account manager
initialization at startup.

In this patch, we delegate this IO work to a new `PushScopeProperty`
that retrieves the value from disk using a provided `CoroutineScope`.

components/feature/accounts-push/README.md

@@ -51,7 +51,10 @@ in order to fix the problem. Therefore, if FxA and Push are used together,
 include the support feature as well:
 
 ```kotlin
-FxaPushSupportFeature(context, fxaAccountManager, autoPushFeature)
+val feature = FxaPushSupportFeature(context, fxaAccountManager, autoPushFeature)
+
+// initialize the feature; this can be done immediately if needed.
+feature.initalize()
 ```
 
 ## License

components/feature/accounts-push/build.gradle

@@ -27,6 +27,12 @@ android {
     }
 }
 
+tasks.withType(org.jetbrains.kotlin.gradle.tasks.KotlinCompile).configureEach {
+    kotlinOptions {
+        freeCompilerArgs += "-Xopt-in=kotlinx.coroutines.ExperimentalCoroutinesApi"
+    }
+}
+
 dependencies {
     implementation project(':service-firefox-accounts')
     implementation project(':support-ktx')

components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/FxaPushSupportFeature.kt

@@ -13,6 +13,7 @@ import androidx.lifecycle.ProcessLifecycleOwner
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.DelicateCoroutinesApi
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.Dispatchers.Main
 import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.launch
 import mozilla.components.concept.base.crash.Breadcrumb
@@ -24,6 +25,7 @@ import mozilla.components.concept.sync.DeviceConstellation
 import mozilla.components.concept.sync.DeviceConstellationObserver
 import mozilla.components.concept.sync.DevicePushSubscription
 import mozilla.components.concept.sync.OAuthAccount
+import mozilla.components.feature.accounts.push.cache.PushScopeProperty
 import mozilla.components.feature.push.AutoPushFeature
 import mozilla.components.feature.push.AutoPushSubscription
 import mozilla.components.feature.push.PushScope
@@ -32,7 +34,6 @@ import mozilla.components.service.fxa.manager.ext.withConstellation
 import mozilla.components.support.base.log.logger.Logger
 import mozilla.components.support.base.utils.SharedPreferencesCache
 import org.json.JSONObject
-import java.util.UUID
 import mozilla.components.concept.sync.AccountObserver as SyncAccountObserver
 
 internal const val PREFERENCE_NAME = "mozac_feature_accounts_push"
@@ -48,61 +49,49 @@ internal const val PREF_FXA_SCOPE = "fxa_push_scope"
  * @param accountManager The FxaAccountManager.
  * @param pushFeature The [AutoPushFeature] if that is setup for observing push events.
  * @param crashReporter Instance of `CrashReporting` to record unexpected caught exceptions.
+ * @param coroutineScope The scope in which IO work within the feature should be performed on.
  * @param owner the lifecycle owner for the observer. Defaults to [ProcessLifecycleOwner].
  * @param autoPause whether to stop notifying the observer during onPause lifecycle events.
  * Defaults to false so that observers are always notified.
  */
 class FxaPushSupportFeature(
     private val context: Context,
-    accountManager: FxaAccountManager,
-    pushFeature: AutoPushFeature,
+    private val accountManager: FxaAccountManager,
+    private val pushFeature: AutoPushFeature,
     private val crashReporter: CrashReporting? = null,
-    owner: LifecycleOwner = ProcessLifecycleOwner.get(),
-    autoPause: Boolean = false
+    private val coroutineScope: CoroutineScope = CoroutineScope(Dispatchers.IO),
+    private val owner: LifecycleOwner = ProcessLifecycleOwner.get(),
+    private val autoPause: Boolean = false
 ) {
 
     /**
      * A unique scope for the FxA push subscription that is generated once and stored in SharedPreferences.
      *
-     * This scope is randomly generated and unique to the account on that particular device.
+     * This scope is randomly generated and unique to the app install.
      * (why this uuid? Note it is *not* reset on logout!)
-     * (isn't this racey? Different observers reference it)
-     * (maybe you just want the acct uid?)
      */
-    private val fxaPushScope: String by lazy {
-        val prefs = preference(context)
-
-        // Generate a unique scope if one doesn't exist.
-        val randomUuid = UUID.randomUUID().toString().replace("-", "")
-
-        // Return a scope in the format example: "fxa_push_scope_a62d5f27c9d74af4996d057f0e0e9c38"
-        val scope = PUSH_SCOPE_PREFIX + randomUuid
-
-        if (!prefs.contains(PREF_FXA_SCOPE)) {
-            prefs.edit().putString(PREF_FXA_SCOPE, scope).apply()
-
-            return@lazy scope
-        }
+    private val pushScope = PushScopeProperty(context, coroutineScope)
 
-        // The default string is non-null, so we can safely cast.
-        prefs.getString(PREF_FXA_SCOPE, scope) as String
-    }
-
-    init {
-        val autoPushObserver = AutoPushObserver(accountManager, pushFeature, fxaPushScope)
+    /**
+     * Initialize the support feature to launch the appropriate observers.
+     */
+    fun initialize() = coroutineScope.launch {
+        val autoPushObserver = AutoPushObserver(accountManager, pushFeature, pushScope.value())
 
         val accountObserver = AccountObserver(
             context,
             pushFeature,
-            fxaPushScope,
+            pushScope.value(),
             crashReporter,
             owner,
             autoPause
         )
 
-        accountManager.register(accountObserver)
+        coroutineScope.launch(Main) {
+            accountManager.register(accountObserver)
 
-        pushFeature.register(autoPushObserver, owner, autoPause)
+            pushFeature.register(autoPushObserver, owner, autoPause)
+        }
     }
 
     companion object {
@@ -128,7 +117,6 @@ internal class AccountObserver(
 
     @OptIn(DelicateCoroutinesApi::class) // GlobalScope usage
     override fun onAuthenticated(account: OAuthAccount, authType: AuthType) {
-
         val constellationObserver = ConstellationObserver(
             context = context,
             push = push,
@@ -395,7 +383,6 @@ internal class VerificationDelegate(
 
 internal data class VerificationState(val timestamp: Long, val totalCount: Int)
 
-@VisibleForTesting
 internal fun preference(context: Context) = context.getSharedPreferences(PREFERENCE_NAME, Context.MODE_PRIVATE)
 
 internal fun AutoPushSubscription.into() = DevicePushSubscription(

components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/cache/PushScopeProperty.kt

@@ -0,0 +1,43 @@
+/* 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 mozilla.components.feature.accounts.push.cache
+
+import android.content.Context
+import android.content.SharedPreferences
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.withContext
+import mozilla.components.feature.accounts.push.FxaPushSupportFeature
+import mozilla.components.feature.accounts.push.PREF_FXA_SCOPE
+import mozilla.components.feature.accounts.push.preference
+import mozilla.components.feature.push.PushScope
+import java.util.UUID
+
+/**
+ * An implementation of a [ScopeProperty] that generates and stores a scope in [SharedPreferences].
+ */
+internal class PushScopeProperty(
+    private val context: Context,
+    private val coroutineScope: CoroutineScope,
+) : ScopeProperty {
+
+    override suspend fun value(): PushScope = withContext(coroutineScope.coroutineContext) {
+        val prefs = preference(context)
+
+        // Generate a unique scope if one doesn't exist.
+        val randomUuid = UUID.randomUUID().toString().replace("-", "")
+
+        // Return a scope in the format example: "fxa_push_scope_a62d5f27c9d74af4996d057f0e0e9c38"
+        val scope = FxaPushSupportFeature.PUSH_SCOPE_PREFIX + randomUuid
+
+        if (!prefs.contains(PREF_FXA_SCOPE)) {
+            prefs.edit().putString(PREF_FXA_SCOPE, scope).apply()
+
+            return@withContext scope
+        }
+
+        // The default string is non-null, so we can safely cast.
+        prefs.getString(PREF_FXA_SCOPE, scope) as String
+    }
+}

components/feature/accounts-push/src/main/java/mozilla/components/feature/accounts/push/cache/ScopeProperty.kt

@@ -0,0 +1,18 @@
+/* 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 mozilla.components.feature.accounts.push.cache
+
+import mozilla.components.feature.push.PushScope
+
+/**
+ * A [ScopeProperty] implementation generates and holds the [PushScope].
+ */
+interface ScopeProperty {
+
+    /**
+     * Returns the [PushScope] value.
+     */
+    suspend fun value(): PushScope
+}

components/feature/accounts-push/src/test/java/mozilla/components/feature/accounts/push/FxaPushSupportFeatureTest.kt

@@ -12,9 +12,11 @@ import mozilla.components.service.fxa.manager.FxaAccountManager
 import mozilla.components.support.test.any
 import mozilla.components.support.test.mock
 import mozilla.components.support.test.robolectric.testContext
+import mozilla.components.support.test.rule.MainCoroutineRule
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
 import org.junit.Before
+import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
 import org.mockito.ArgumentMatchers.anyBoolean
@@ -25,26 +27,39 @@ import org.robolectric.RobolectricTestRunner
 @RunWith(RobolectricTestRunner::class)
 class FxaPushSupportFeatureTest {
 
+    @get:Rule
+    val coroutineTestRule = MainCoroutineRule()
+
     private val pushFeature: AutoPushFeature = mock()
     private val accountManager: FxaAccountManager = mock()
 
+    private lateinit var feature: FxaPushSupportFeature
+
     @Before
     fun setup() {
         preference(testContext).edit().remove(PREF_FXA_SCOPE).apply()
         `when`(pushFeature.config).thenReturn(mock())
+
+        feature = FxaPushSupportFeature(
+            context = testContext,
+            accountManager = accountManager,
+            pushFeature = pushFeature,
+            crashReporter = null,
+            coroutineScope = coroutineTestRule.scope
+        )
     }
 
     @Test
     fun `account observer registered`() {
-        FxaPushSupportFeature(testContext, accountManager, pushFeature)
+        feature.initialize()
 
         verify(accountManager).register(any())
         verify(pushFeature).register(any(), any(), anyBoolean())
     }
 
     @Test
     fun `feature generates and caches a scope`() {
-        FxaPushSupportFeature(testContext, accountManager, pushFeature)
+        feature.initialize()
 
         assertTrue(preference(testContext).contains(PREF_FXA_SCOPE))
     }
@@ -53,15 +68,15 @@ class FxaPushSupportFeatureTest {
     fun `feature does not generate a scope if one already exists`() {
         preference(testContext).edit().putString(PREF_FXA_SCOPE, "testScope").apply()
 
-        FxaPushSupportFeature(testContext, accountManager, pushFeature)
+        feature.initialize()
 
         val cachedScope = preference(testContext).getString(PREF_FXA_SCOPE, "")
         assertEquals("testScope", cachedScope!!)
     }
 
     @Test
     fun `feature generates a partially predictable push scope`() {
-        FxaPushSupportFeature(testContext, accountManager, pushFeature)
+        feature.initialize()
 
         val cachedScope = preference(testContext).getString(PREF_FXA_SCOPE, "")
 

docs/changelog.md

@@ -11,6 +11,9 @@ permalink: /changelog/
 * [Gecko](https://github.com/mozilla-mobile/android-components/blob/main/buildSrc/src/main/java/Gecko.kt)
 * [Configuration](https://github.com/mozilla-mobile/android-components/blob/main/.config.yml)
 
+* **feature-accounts-push**:
+  * ⚠️ **This is a breaking change**: `FxaPushSupportFeature` now requires to be explicitly started with `initialize`.
+  * The constructor for `FxaPushSupportFeature` has a `coroutineScope` parameter that defaults to a `CoroutineScope(Dispatchers.IO)`.
 
 # 105.0.0
 * [Commits](https://github.com/mozilla-mobile/android-components/compare/v104.0.0...v105.0.0)