Bug 2037747 - Add per-feature update notifications to the Android Nimbus client [ci full]

components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt

@@ -20,11 +20,11 @@ import androidx.core.content.pm.PackageInfoCompat
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Deferred
+import kotlinx.coroutines.GlobalScope
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.NonCancellable
 import kotlinx.coroutines.async
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.withContext
 import mozilla.appservices.remotesettings.RemoteSettingsService
 import mozilla.telemetry.glean.Glean
@@ -41,6 +41,7 @@ import org.mozilla.experiments.nimbus.internal.EnrollmentChangeEvent
 import org.mozilla.experiments.nimbus.internal.EnrollmentChangeEventType
 import org.mozilla.experiments.nimbus.internal.EnrollmentStatusExtraDef
 import org.mozilla.experiments.nimbus.internal.FeatureExposureExtraDef
+import org.mozilla.experiments.nimbus.internal.FeatureUpdateDispatcher
 import org.mozilla.experiments.nimbus.internal.GeckoPrefHandler
 import org.mozilla.experiments.nimbus.internal.GeckoPrefState
 import org.mozilla.experiments.nimbus.internal.MalformedFeatureConfigExtraDef
@@ -86,6 +87,8 @@ open class Nimbus(
 
     private val logger = delegate.logger
 
+    private val updateDispatcher = FeatureUpdateDispatcher()
+
     private val metricsHandler = object : MetricsHandler {
         override fun recordDatabaseLoad(event: DatabaseLoadExtraDef) {
             NimbusEvents.databaseLoad.record(
@@ -205,6 +208,8 @@ open class Nimbus(
         )
     }
 
+    override fun getFeatureUpdateDispatcher(): FeatureUpdateDispatcher? = updateDispatcher
+
     // This is currently not available from the main thread.
     // see https://jira.mozilla.com/browse/SDK-191
     @WorkerThread
@@ -279,7 +284,7 @@ open class Nimbus(
     @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
     internal fun initializeOnThisThread() = withCatchAll("initialize") {
         nimbusClient.initialize()
-        postEnrolmentCalculation()
+        postEnrolmentCalculation(true)
     }
 
     override fun fetchExperiments() {
@@ -342,9 +347,15 @@ open class Nimbus(
                 events = nimbusClient.applyPendingExperiments()
             }
             NimbusHealth.applyPendingExperimentsTime.accumulateSingleSample(time)
+
+            // SAFETY: events is only null at declaration time and is
+            // immediately assigned a non-null value inside the
+            // measureTimeMillis lambda.
             recordExperimentTelemetryEvents(events!!)
+
             // Get the experiments to record in telemetry
-            postEnrolmentCalculation()
+            postEnrolmentCalculation(false)
+            updateDispatcher.notifyChanged(events)
         } catch (e: NimbusException.InvalidExperimentFormat) {
             reportError("Invalid experiment format", e)
         }
@@ -379,11 +390,22 @@ open class Nimbus(
         }
 
     @WorkerThread
-    private fun postEnrolmentCalculation() {
-        nimbusClient.getActiveExperiments().let {
-            recordExperimentTelemetry(it)
+    private fun postEnrolmentCalculation(initial: Boolean) {
+        nimbusClient.getActiveExperiments().also { experiments ->
+            recordExperimentTelemetry(experiments)
             updateObserver { observer ->
-                observer.onUpdatesApplied(it)
+                observer.onUpdatesApplied(experiments)
+            }
+
+            if (initial) {
+                val featureIds = mutableSetOf<String>()
+                for (experiment in experiments) {
+                    for (featureId in experiment.featureIds) {
+                        featureIds.add(featureId)
+                    }
+                }
+
+                updateDispatcher.notifyFeatures(featureIds)
             }
         }
     }
@@ -431,7 +453,8 @@ open class Nimbus(
             val enrolmentChanges = nimbusClient.setExperimentParticipation(active)
             if (enrolmentChanges.isNotEmpty()) {
                 recordExperimentTelemetryEvents(enrolmentChanges)
-                postEnrolmentCalculation()
+                postEnrolmentCalculation(false)
+                updateDispatcher.notifyChanged(enrolmentChanges)
             }
         }
 
@@ -442,7 +465,8 @@ open class Nimbus(
             val enrolmentChanges = nimbusClient.setRolloutParticipation(active)
             if (enrolmentChanges.isNotEmpty()) {
                 recordExperimentTelemetryEvents(enrolmentChanges)
-                postEnrolmentCalculation()
+                postEnrolmentCalculation(false)
+                updateDispatcher.notifyChanged(enrolmentChanges)
             }
         }
 
@@ -495,24 +519,35 @@ open class Nimbus(
         nimbusClient.optOut(experimentId).also(::recordExperimentTelemetryEvents)
     }
 
+    @AnyThread
     override fun resetTelemetryIdentifiers() {
         dbScope.launch {
-            withCatchAll("resetTelemetryIdentifiers") {
-                nimbusClient.resetTelemetryIdentifiers().also { enrollmentChangeEvents ->
-                    recordExperimentTelemetryEvents(enrollmentChangeEvents)
-                }
-            }
+            resetTelemetryIdentifiersOnThisThread()
         }
     }
 
+    @WorkerThread
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    internal fun resetTelemetryIdentifiersOnThisThread() = withCatchAll("resetTelemetryIdentifiers") {
+        nimbusClient.resetTelemetryIdentifiers().also { enrollmentChangeEvents ->
+            recordExperimentTelemetryEvents(enrollmentChangeEvents)
+            updateDispatcher.notifyChanged(enrollmentChangeEvents)
+        }
+    }
+
+    @AnyThread
     override fun optInWithBranch(experimentId: String, branch: String) {
         dbScope.launch {
-            withCatchAll("optIn") {
-                nimbusClient.optInWithBranch(experimentId, branch).also(::recordExperimentTelemetryEvents)
-            }
+            optInWithBranchOnThisThread(experimentId, branch)
         }
     }
 
+    @WorkerThread
+    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
+    internal fun optInWithBranchOnThisThread(experimentId: String, branch: String) = withCatchAll("optIn") {
+        nimbusClient.optInWithBranch(experimentId, branch).also(::recordExperimentTelemetryEvents)
+    }
+
     override fun recordExposureEvent(featureId: String, experimentSlug: String?) {
         recordExposureOnThisThread(featureId, experimentSlug)
     }

components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusInterface.kt

@@ -17,6 +17,7 @@ import org.mozilla.experiments.nimbus.internal.AvailableExperiment
 import org.mozilla.experiments.nimbus.internal.EnrolledExperiment
 import org.mozilla.experiments.nimbus.internal.EnrollmentChangeEvent
 import org.mozilla.experiments.nimbus.internal.ExperimentBranch
+import org.mozilla.experiments.nimbus.internal.FeatureUpdateDispatcher
 import org.mozilla.experiments.nimbus.internal.GeckoPrefState
 import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason
 import org.mozilla.experiments.nimbus.internal.PreviousGeckoPrefState
@@ -244,6 +245,11 @@ interface NimbusInterface : FeaturesInterface, NimbusMessagingInterface, NimbusE
     override val events: NimbusEventStore
         get() = this
 
+    /**
+     * Return the feature update dispatcher.
+     */
+    fun getFeatureUpdateDispatcher(): FeatureUpdateDispatcher? = null
+
     /**
      * Interface to be implemented by classes that want to observe experiment updates
      */

components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/internal/FeatureUpdateDispatcher.kt

@@ -0,0 +1,90 @@
+/* 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.experiments.nimbus.internal
+
+import androidx.annotation.AnyThread
+import kotlinx.coroutines.DelicateCoroutinesApi
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.GlobalScope
+import kotlinx.coroutines.launch
+import org.mozilla.experiments.nimbus.internal.EnrollmentChangeEvent
+import java.util.concurrent.locks.ReentrantLock
+import kotlin.concurrent.withLock
+
+/**
+ * The feature update dispatcher dispatches callbacks when feature
+ * configurations change.
+ */
+class FeatureUpdateDispatcher {
+    private val lock = ReentrantLock()
+    private val callbackMap: MutableMap<String, MutableSet<() -> Unit>> = mutableMapOf()
+
+    /**
+     * Register a callback to be called when the feature value changes.
+     */
+    @AnyThread
+    public fun register(featureId: String, callback: () -> Unit) {
+        lock.withLock {
+            callbackMap
+                .getOrPut(featureId, { mutableSetOf<() -> Unit>() })
+                .add(callback)
+        }
+    }
+
+    /**
+     * Remove a callback registration for a feature.
+     */
+    @AnyThread
+    public fun unregister(featureId: String, callback: () -> Unit) {
+        lock.withLock {
+            callbackMap.get(featureId)?.run { remove(callback) }
+        }
+    }
+
+    /**
+     * Trigger the callbacks for all the features that have changed.
+     */
+    @AnyThread
+    internal fun notifyChanged(events: List<EnrollmentChangeEvent>) {
+        if (events.isEmpty()) {
+            return
+        }
+
+        val featureIds = mutableSetOf<String>()
+
+        for (event in events) {
+            for (featureId in event.featureIds) {
+                featureIds.add(featureId)
+            }
+        }
+
+        notifyFeatures(featureIds)
+    }
+
+    /**
+     * Trigger the callbacks for the given features.
+     */
+    @AnyThread
+    @OptIn(DelicateCoroutinesApi::class)
+    internal fun notifyFeatures(featureIds: Set<String>) {
+        val toUpdate = mutableSetOf<() -> Unit>()
+
+        lock.withLock {
+            for (featureId in featureIds) {
+                callbackMap.get(featureId)?.also { callbacks ->
+                    for (callback in callbacks) {
+                        toUpdate.add(callback)
+                    }
+                }
+            }
+        }
+
+        GlobalScope.launch(Dispatchers.Main) {
+            for (callback in toUpdate) {
+                callback()
+            }
+        }
+    }
+}

components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/FeatureUpdateDispatcherTests.kt

@@ -0,0 +1,104 @@
+/* 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.experiments.nimbus
+
+import android.os.Looper
+import org.junit.Assert.assertEquals
+import org.junit.runner.RunWith
+import org.mozilla.experiments.nimbus.internal.NimbusUpdateDispatcher
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.Shadows.shadowOf
+
+@RunWith(RobolectricTestRunner::class)
+class FeatureUpdateDispatcherTests {
+    @Test
+    fun `test update registration`() {
+        val updates = FeatureUpdateDispatcher()
+
+        var fooCalls = 0
+        var barCalls = 0
+        var bazCalls = 0
+
+        fun assertCalls(expectedFoo: Int, expectedBar: Int, expectedBaz: Int) {
+            shadowOf(Looper.getMainLooper()).idle()
+
+            assertEquals(expectedFoo, fooCalls)
+            assertEquals(expectedBar, barCalls)
+            assertEquals(expectedBaz, bazCalls)
+        }
+
+        val fooCallback = { fooCalls++ }
+        val barCallback = { barCalls++ }
+        val bazCallback = { bazCalls++ }
+
+        assertCalls(0, 0, 0)
+
+        updates.notifyFeatures(setOf("foo", "bar", "baz"))
+        assertCalls(0, 0, 0)
+
+        updates.register("foo", fooCallback)
+        updates.notifyFeatures(setOf("foo", "bar", "baz"))
+        assertCalls(1, 0, 0)
+
+        updates.register("bar", barCallback)
+        updates.notifyFeatures(setOf("foo", "bar", "baz"))
+        assertCalls(2, 1, 0)
+
+        updates.register("baz", bazCallback)
+        updates.notifyFeatures(setOf("foo", "bar", "baz"))
+        assertCalls(3, 2, 1)
+
+        updates.unregister("foo", fooCallback)
+        updates.notifyFeatures(setOf("foo", "bar", "baz"))
+        assertCalls(3, 3, 2)
+
+        updates.unregister("bar", barCallback)
+        updates.notifyFeatures(setOf("foo", "bar", "baz"))
+        assertCalls(3, 3, 3)
+
+        updates.unregister("baz", bazCallback)
+        updates.notifyFeatures(setOf("foo", "bar", "baz"))
+        assertCalls(3, 3, 3)
+    }
+
+    @Test
+    fun `multiple callbacks for feature`() {
+        val updates = FeatureUpdateDispatcher()
+
+        var aCalls = 0
+        var bCalls = 0
+
+        fun assertCalls(expectedA: Int, expectedB: Int) {
+            shadowOf(Looper.getMainLooper()).idle()
+
+            assertEquals(expectedA, aCalls)
+            assertEquals(expectedB, bCalls)
+        }
+
+        val callbackA = { aCalls++ }
+        val callbackB = { bCalls++ }
+
+        assertCalls(0, 0)
+
+        updates.notifyFeatures(setOf("foo"))
+        assertCalls(0, 0)
+
+        updates.register("foo", callbackA)
+        updates.notifyFeatures(setOf("foo"))
+        assertCalls(1, 0)
+
+        updates.register("foo", callbackB)
+        updates.notifyFeatures(setOf("foo"))
+        assertCalls(2, 1)
+
+        updates.unregister("foo", callbackA)
+        updates.notifyFeatures(setOf("foo"))
+        assertCalls(2, 2)
+
+        updates.unregister("foo", callbackB)
+        updates.notifyFeatures(setOf("foo"))
+        assertCalls(2, 2)
+    }
+}