Unverified Commit 3f8f06ae authored by Beth Rennie's avatar Beth Rennie Committed by GitHub
Browse files

Bug 2045853 - Trigger postEnrolmentCalculation() on all enrollment changes (#7405) 

`postEnrolmentCalculation()` is responsible for recording the active
experiments and triggering the `onUpdatesApplied` observer -- which is
in turn used to invalidate the FML feature cache. Several enrolment
change mechanisms were not properly triggering `onUpdatesApplied`, which
has lead to stale feature value caching issues (see-also
https://github.com/mozilla-mobile/fenix/issues/21838). These issues have
been band-aided by calling applyPendingExperiments() in most cases,
which results in ~correct behaviour but does a double update.

Now we trigger `postEnrolmentCalculation()` and therefore feature
invalidation on all enrollment changes.
parent fcc5382d

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

+37 −19
Original line number Diff line number Diff line
@@ -165,7 +165,7 @@ open class Nimbus( 
        get() = nimbusClient.getExperimentParticipation()

        set(active) {

            dbScope.launch {

                nimbusClient.setExperimentParticipation(active)

                setExperimentParticipationOnThisThread(active)

                applyPendingExperimentsOnThisThread()

            }

        }
@@ -174,7 +174,7 @@ open class Nimbus( 
        get() = nimbusClient.getRolloutParticipation()

        set(active) {

            dbScope.launch {

                nimbusClient.setRolloutParticipation(active)

                setRolloutParticipationOnThisThread(active)

                applyPendingExperimentsOnThisThread()

            }

        }
@@ -458,21 +458,22 @@ open class Nimbus( 
        prefUnenrollReason: PrefUnenrollReason,

    ) {

        dbScope.launch {

            withCatchAll("unenrollForGeckoPref") {

            unenrollForGeckoPrefOnThisThread(geckoPrefState, prefUnenrollReason)

        }

    }

    }



    @WorkerThread

    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)

    internal fun unenrollForGeckoPrefOnThisThread(

        geckoPrefState: GeckoPrefState,

        prefUnenrollReason: PrefUnenrollReason,

    ): List<EnrollmentChangeEvent> {

        val events = nimbusClient.unenrollForGeckoPref(geckoPrefState, prefUnenrollReason)

        recordExperimentTelemetryEvents(events)

        return events

    ): List<EnrollmentChangeEvent>? {

        return withCatchAll("unenrollForGeckoPref") {

            val enrollmentChangeEvents = nimbusClient.unenrollForGeckoPref(geckoPrefState, prefUnenrollReason)

            recordExperimentTelemetryEvents(enrollmentChangeEvents)

            postEnrolmentCalculation()

            enrollmentChangeEvents

        }

    }



    override fun registerPreviousGeckoPrefStates(geckoPrefStates: List<GeckoPrefState>) {
@@ -491,25 +492,42 @@ open class Nimbus( 


    @WorkerThread

    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)

    internal fun optOutOnThisThread(experimentId: String) = withCatchAll("optOut") {

    internal fun optOutOnThisThread(experimentId: String) {

        withCatchAll("optOut") {

            nimbusClient.optOut(experimentId).also(::recordExperimentTelemetryEvents)

            postEnrolmentCalculation()

        }

    }



    @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") {

            val enrollmentChangeEvents = nimbusClient.resetTelemetryIdentifiers()

            recordExperimentTelemetryEvents(enrollmentChangeEvents)

            postEnrolmentCalculation()

        }

    }



    override fun optInWithBranch(experimentId: String, branch: String) {

        dbScope.launch {

            optInWithBranchOnThisThread(experimentId, branch)

        }

    }



    @WorkerThread

    @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)

    internal fun optInWithBranchOnThisThread(experimentId: String, branch: String) {

        withCatchAll("optIn") {

            nimbusClient.optInWithBranch(experimentId, branch).also(::recordExperimentTelemetryEvents)

            }

            postEnrolmentCalculation()

        }

    }

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

+2 −1
Original line number Diff line number Diff line
@@ -942,7 +942,8 @@ class NimbusTests { 
            PrefUnenrollReason.FAILED_TO_SET,

        )



        assertEquals(1, events.size)

        assertNotNull(events)

        assertEquals(1, events!!.size)

        assertEquals(EnrollmentChangeEventType.DISQUALIFICATION, events[0].change)

        assertEquals(0, handler.setValues?.size)

    }