diff --git a/CHANGELOG.md b/CHANGELOG.md index a39a1c87b2..35d3447c12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ # v148.0 (In progress) +### Nimbus +* Adds `PreviousState` on `ExperimentEnrollment` when it is of type `EnrollmentStatus::Enrolled` and getters and setters. `PreviousState::GeckoPref` is added to support previous states for Gecko pref experiments. [Full Changelog](In progress) diff --git a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt index 0fed3ef9a0..494c473219 100644 --- a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt +++ b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/Nimbus.kt @@ -44,6 +44,7 @@ import org.mozilla.experiments.nimbus.internal.NimbusClient import org.mozilla.experiments.nimbus.internal.NimbusClientInterface import org.mozilla.experiments.nimbus.internal.NimbusException import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason +import org.mozilla.experiments.nimbus.internal.PreviousState import org.mozilla.experiments.nimbus.internal.RecordedContext import java.io.File import java.io.IOException @@ -438,6 +439,18 @@ open class Nimbus( return nimbusClient.unenrollForGeckoPref(geckoPrefState, prefUnenrollReason) } + override fun registerPreviousGeckoPrefStates(geckoPrefStates: List) { + dbScope.launch { + withCatchAll("registerPreviousGeckoPrefStates") { + nimbusClient.registerPreviousGeckoPrefStates(geckoPrefStates) + } + } + } + + override fun getPreviousState(experimentSlug: String): PreviousState? { + return nimbusClient.getPreviousState(experimentSlug) + } + @WorkerThread @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) internal fun optOutOnThisThread(experimentId: String) = withCatchAll("optOut") { diff --git a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusInterface.kt b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusInterface.kt index 451490b20d..6dc96d1f84 100644 --- a/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusInterface.kt +++ b/components/nimbus/android/src/main/java/org/mozilla/experiments/nimbus/NimbusInterface.kt @@ -17,6 +17,7 @@ import org.mozilla.experiments.nimbus.internal.EnrollmentChangeEvent import org.mozilla.experiments.nimbus.internal.ExperimentBranch import org.mozilla.experiments.nimbus.internal.GeckoPrefState import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason +import org.mozilla.experiments.nimbus.internal.PreviousState import java.time.Duration import java.util.concurrent.TimeUnit @@ -191,6 +192,26 @@ interface NimbusInterface : FeaturesInterface, NimbusMessagingInterface, NimbusE prefUnenrollReason: PrefUnenrollReason, ): List = listOf() + /** + * Add the original Gecko pref values as a previous state on each involved enrollment. + * + * @param geckoPrefStates The list of items that should have their enrollment state updated with + * original Gecko pref previous state information. + */ + fun registerPreviousGeckoPrefStates( + geckoPrefStates: List, + ) = Unit + + /** + * Retrieves a previous state, if available on an enrolled experiment, from a given slug. + * + * @param experimentSlug The slug of the experiment. + * @return The previous state of the given slug. Will return null if not available or invalid slug. + */ + fun getPreviousState( + experimentSlug: String, + ): PreviousState? = null + /** * Reset internal state in response to application-level telemetry reset. * Consumers should call this method when the user resets the telemetry state of the diff --git a/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusTests.kt b/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusTests.kt index 0fa4443fe3..d7c16c09fb 100644 --- a/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusTests.kt +++ b/components/nimbus/android/src/test/java/org/mozilla/experiments/nimbus/NimbusTests.kt @@ -5,6 +5,7 @@ package org.mozilla.experiments.nimbus import android.content.Context +import android.os.Looper import android.util.Log import androidx.test.core.app.ApplicationProvider import kotlinx.coroutines.CancellationException @@ -42,12 +43,17 @@ import org.mozilla.experiments.nimbus.internal.GeckoPrefHandler import org.mozilla.experiments.nimbus.internal.GeckoPrefState import org.mozilla.experiments.nimbus.internal.JsonObject import org.mozilla.experiments.nimbus.internal.NimbusException +import org.mozilla.experiments.nimbus.internal.OriginalGeckoPref import org.mozilla.experiments.nimbus.internal.PrefBranch +import org.mozilla.experiments.nimbus.internal.PrefEnrollmentData import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason +import org.mozilla.experiments.nimbus.internal.PreviousGeckoPrefState +import org.mozilla.experiments.nimbus.internal.PreviousState import org.mozilla.experiments.nimbus.internal.RecordedContext import org.mozilla.experiments.nimbus.internal.getCalculatedAttributes import org.mozilla.experiments.nimbus.internal.validateEventQueries import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf import java.io.File import java.util.Calendar import java.util.concurrent.Executors @@ -849,12 +855,13 @@ class NimbusTests { "number" to GeckoPrefState( geckoPref = GeckoPref("pref.number", PrefBranch.DEFAULT), geckoValue = "1", - enrollmentValue = null, + enrollmentValue = PrefEnrollmentData("test-experiment", "42", "about_welcome", "number"), isUserSet = false, ), ), ), var setValues: List? = null, + var originalGeckoPrefValues: List? = null, ) : GeckoPrefHandler { override fun getPrefsWithState(): Map> { return internalMap @@ -863,6 +870,10 @@ class NimbusTests { override fun setGeckoPrefsState(newPrefsState: List) { setValues = newPrefsState } + + override fun setGeckoPrefsOriginalValues(originalGeckoPrefs: List) { + originalGeckoPrefValues = originalGeckoPrefs + } } @Test @@ -884,6 +895,21 @@ class NimbusTests { assertEquals("42", handler.setValues?.get(0)?.enrollmentValue?.prefValue) } + @Test + fun `GeckoPrefHandler setGeckoPrefsOriginalValues function`() { + val handler = TestGeckoPrefHandler() + val originalValues = listOf( + OriginalGeckoPref( + pref = "pref.number", + branch = PrefBranch.DEFAULT, + value = "1", + ), + ) + handler.setGeckoPrefsOriginalValues(originalValues) + assertEquals(1, handler.originalGeckoPrefValues?.size) + assertEquals("pref.number", handler.originalGeckoPrefValues?.get(0)?.pref) + } + @Test fun `unenroll for gecko pref functions`() { val handler = TestGeckoPrefHandler() @@ -911,6 +937,36 @@ class NimbusTests { assertEquals(EnrollmentChangeEventType.DISQUALIFICATION, events[0].change) assertEquals(0, handler.setValues?.size) } + + @Test + fun `register previous gecko states and check values`() { + val handler = TestGeckoPrefHandler() + + val nimbus = createNimbus(geckoPrefHandler = handler) + + suspend fun getString(): String { + return testExperimentsJsonString(appInfo, packageName) + } + + val job = nimbus.applyLocalExperiments(::getString) + runBlocking { + job.join() + } + + assertEquals(1, handler.setValues?.size) + assertEquals("42", handler.setValues?.get(0)?.enrollmentValue?.prefValue) + + nimbus.registerPreviousGeckoPrefStates(handler.setValues!!) + shadowOf(Looper.getMainLooper()).idle() + + val previousState = nimbus.getPreviousState("test-experiment") + shadowOf(Looper.getMainLooper()).idle() + + assertNotNull(previousState) + val geckoPreviousState = previousState as PreviousState.GeckoPref + assertEquals("1", geckoPreviousState!!.v1.originalValues[0].value) + assertEquals("pref.number", geckoPreviousState!!.v1.originalValues[0].pref) + } } // Mocking utilities, from mozilla.components.support.test diff --git a/components/nimbus/src/enrollment.rs b/components/nimbus/src/enrollment.rs index b9dccd38ec..44706e7d7f 100644 --- a/components/nimbus/src/enrollment.rs +++ b/components/nimbus/src/enrollment.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. #[cfg(feature = "stateful")] -use crate::stateful::gecko_prefs::PrefUnenrollReason; +use crate::stateful::gecko_prefs::{GeckoPrefStore, OriginalGeckoPref, PrefUnenrollReason}; use crate::{ defaults::Defaults, error::{debug, warn, NimbusError, Result}, @@ -11,6 +11,8 @@ use crate::{ SLUG_REPLACEMENT_PATTERN, }; use serde_derive::*; +#[cfg(feature = "stateful")] +use std::sync::Arc; use std::{ collections::{HashMap, HashSet}, fmt::{Display, Formatter, Result as FmtResult}, @@ -21,7 +23,7 @@ pub(crate) const PREVIOUS_ENROLLMENTS_GC_TIME: Duration = Duration::from_secs(36 // These are types we use internally for managing enrollments. // ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️ -// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️ +// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️ #[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)] pub enum EnrolledReason { /// A normal enrollment as per the experiment's rules. @@ -45,7 +47,7 @@ impl Display for EnrolledReason { // These are types we use internally for managing non-enrollments. // ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️ -// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️ +// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️ #[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)] pub enum NotEnrolledReason { /// The experiment targets a different application. @@ -99,7 +101,7 @@ impl Default for Participation { // These are types we use internally for managing disqualifications. // ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️ -// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️ +// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️ #[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)] pub enum DisqualifiedReason { /// There was an error. @@ -134,10 +136,47 @@ impl Display for DisqualifiedReason { } } -// Every experiment has an ExperimentEnrollment, even when we aren't enrolled. +// The previous state of a Gecko pref before enrollment took place. +// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️ +// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️ +#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)] +#[cfg(feature = "stateful")] +pub struct PreviousGeckoPrefState { + pub original_values: Vec, + pub feature_id: String, + pub variable: String, +} +// The previous state of a given feature before enrollment. // ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️ -// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️ +// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️ +#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq, uniffi::Enum)] +#[cfg(feature = "stateful")] +pub enum PreviousState { + GeckoPref(PreviousGeckoPrefState), +} + +#[cfg(feature = "stateful")] +impl PreviousState { + #[cfg(feature = "stateful")] + pub(crate) fn on_revert_to_previous_state( + &self, + #[cfg(feature = "stateful")] gecko_pref_store: &Option>, + ) { + match self { + PreviousState::GeckoPref(previous_gecko_pref_state) => { + if let Some(store) = gecko_pref_store { + store.handler.set_gecko_prefs_original_values( + previous_gecko_pref_state.original_values.clone(), + ) + } + } + } + } +} +// Every experiment has an ExperimentEnrollment, even when we aren't enrolled. +// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️ +// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️ #[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)] pub struct ExperimentEnrollment { pub slug: String, @@ -222,6 +261,7 @@ impl ExperimentEnrollment { available_randomization_units: &AvailableRandomizationUnits, updated_experiment: &Experiment, targeting_helper: &NimbusTargetingHelper, + #[cfg(feature = "stateful")] gecko_pref_store: &Option>, out_enrollment_events: &mut Vec, ) -> Result { Ok(match &self.status { @@ -255,6 +295,14 @@ impl ExperimentEnrollment { "Existing experiment enrollment '{}' is now disqualified (global opt-out)", &self.slug ); + #[cfg(feature = "stateful")] + if let EnrollmentStatus::Enrolled { + previous_state: Some(previous_state), + .. + } = &self.status + { + previous_state.on_revert_to_previous_state(gecko_pref_store); + } let updated_enrollment = self.disqualify_from_enrolled(DisqualifiedReason::OptOut); out_enrollment_events.push(updated_enrollment.get_change_event()); @@ -275,6 +323,17 @@ impl ExperimentEnrollment { updated_experiment, targeting_helper, )?; + + #[cfg(feature = "stateful")] + if self.will_pref_experiment_change(updated_experiment, &evaluated_enrollment) { + if let EnrollmentStatus::Enrolled { + previous_state: Some(previous_state), + .. + } = &self.status + { + previous_state.on_revert_to_previous_state(gecko_pref_store); + } + } match evaluated_enrollment.status { EnrollmentStatus::Error { .. } => { let updated_enrollment = @@ -359,6 +418,7 @@ impl ExperimentEnrollment { /// from the database after `PREVIOUS_ENROLLMENTS_GC_TIME`. fn on_experiment_ended( &self, + #[cfg(feature = "stateful")] gecko_prefs: &Option>, out_enrollment_events: &mut Vec, ) -> Option { debug!( @@ -372,6 +432,16 @@ impl ExperimentEnrollment { | EnrollmentStatus::WasEnrolled { .. } | EnrollmentStatus::Error { .. } => return None, // We were never enrolled anyway, simply delete the enrollment record from the DB. }; + + #[cfg(feature = "stateful")] + if let EnrollmentStatus::Enrolled { + previous_state: Some(previous_state), + .. + } = &self.status + { + previous_state.on_revert_to_previous_state(gecko_prefs); + } + let enrollment = Self { slug: self.slug.clone(), status: EnrollmentStatus::WasEnrolled { @@ -389,9 +459,18 @@ impl ExperimentEnrollment { pub(crate) fn on_explicit_opt_out( &self, out_enrollment_events: &mut Vec, + #[cfg(feature = "stateful")] gecko_prefs: &Option>, ) -> ExperimentEnrollment { - match self.status { - EnrollmentStatus::Enrolled { .. } => { + match &self.status { + EnrollmentStatus::Enrolled { + #[cfg(feature = "stateful")] + previous_state, + .. + } => { + #[cfg(feature = "stateful")] + if let Some(previous_state) = previous_state { + previous_state.on_revert_to_previous_state(gecko_prefs); + } let enrollment = self.disqualify_from_enrolled(DisqualifiedReason::OptOut); out_enrollment_events.push(enrollment.get_change_event()); enrollment @@ -430,6 +509,20 @@ impl ExperimentEnrollment { } } + // Previous state is only settable on Enrolled experiments + #[cfg(feature = "stateful")] + pub(crate) fn on_add_state(&self, previous_state: PreviousState) -> ExperimentEnrollment { + let mut next = self.clone(); + if let EnrollmentStatus::Enrolled { reason, branch, .. } = &self.status { + next.status = EnrollmentStatus::Enrolled { + previous_state: Some(previous_state), + reason: reason.clone(), + branch: branch.clone(), + }; + } + next + } + /// Reset identifiers in response to application-level telemetry reset. /// /// We move any enrolled experiments to the "disqualified" state, since their further @@ -528,15 +621,69 @@ impl ExperimentEnrollment { | EnrollmentStatus::Error { .. } => self.clone(), } } + + #[cfg(feature = "stateful")] + pub(crate) fn will_pref_experiment_change( + &self, + updated_experiment: &Experiment, + updated_enrollment: &ExperimentEnrollment, + ) -> bool { + let (original_feature_id, original_branch_slug) = match &self.status { + EnrollmentStatus::Enrolled { + previous_state: Some(PreviousState::GeckoPref(previous_state)), + branch, + .. + } => (&previous_state.feature_id, branch), + // Can't change if it isn't a pref experiment + _ => { + return false; + } + }; + + let updated_branch_slug = match &updated_enrollment.status { + EnrollmentStatus::Enrolled { branch, .. } => branch, + // If we are no longer going to be enrolled, then a change happened + _ => { + return true; + } + }; + + // Branch changed + if updated_branch_slug != original_branch_slug { + return true; + } + + let Some(branch) = updated_experiment.get_branch(updated_branch_slug) else { + return true; + }; + + // Feature changed + match &branch.feature { + Some(updated_feature) => { + if updated_feature.feature_id != *original_feature_id { + return true; + } + } + None => { + return true; + } + }; + + // ToDo: In review ask about checking variable - unsure of what that corresponds to. + false + } } // ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️ -// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️ +// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️ #[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)] pub enum EnrollmentStatus { Enrolled { reason: EnrolledReason, branch: String, + #[cfg(feature = "stateful")] + #[serde(skip_serializing_if = "Option::is_none")] + previous_state: Option, }, NotEnrolled { reason: NotEnrolledReason, @@ -577,6 +724,8 @@ impl EnrollmentStatus { EnrollmentStatus::Enrolled { reason, branch: branch.to_owned(), + #[cfg(feature = "stateful")] + previous_state: None, } } @@ -618,6 +767,7 @@ impl<'a> EnrollmentsEvolver<'a> { prev_experiments: &[E], next_experiments: &[Experiment], prev_enrollments: &[ExperimentEnrollment], + #[cfg(feature = "stateful")] gecko_pref_store: &Option>, ) -> Result<(Vec, Vec)> where E: ExperimentMetadata + Clone, @@ -639,6 +789,8 @@ impl<'a> EnrollmentsEvolver<'a> { &prev_rollouts, &next_rollouts, &ro_enrollments, + #[cfg(feature = "stateful")] + gecko_pref_store, )?; enrollments.extend(next_ro_enrollments); @@ -663,6 +815,8 @@ impl<'a> EnrollmentsEvolver<'a> { &prev_experiments, &next_experiments, &prev_enrollments, + #[cfg(feature = "stateful")] + gecko_pref_store, )?; enrollments.extend(next_exp_enrollments); @@ -679,6 +833,7 @@ impl<'a> EnrollmentsEvolver<'a> { prev_experiments: &[E], next_experiments: &[Experiment], prev_enrollments: &[ExperimentEnrollment], + #[cfg(feature = "stateful")] gecko_pref_store: &Option>, ) -> Result<(Vec, Vec)> where E: ExperimentMetadata + Clone, @@ -718,6 +873,8 @@ impl<'a> EnrollmentsEvolver<'a> { next_experiments_map.get(slug).copied(), Some(prev_enrollment), &mut enrollment_events, + #[cfg(feature = "stateful")] + gecko_pref_store, ) { Ok(enrollment) => enrollment, Err(e) => { @@ -819,6 +976,8 @@ impl<'a> EnrollmentsEvolver<'a> { Some(next_experiment), prev_enrollment, &mut enrollment_events, + #[cfg(feature = "stateful")] + gecko_pref_store, ) { Ok(enrollment) => enrollment, Err(e) => { @@ -912,6 +1071,7 @@ impl<'a> EnrollmentsEvolver<'a> { next_experiment: Option<&Experiment>, prev_enrollment: Option<&ExperimentEnrollment>, out_enrollment_events: &mut Vec, // out param containing the events we'd like to emit to glean. + #[cfg(feature = "stateful")] gecko_pref_store: &Option>, ) -> Result> where E: ExperimentMetadata + Clone, @@ -940,9 +1100,11 @@ impl<'a> EnrollmentsEvolver<'a> { out_enrollment_events, )?), // Experiment deleted remotely. - (Some(_), None, Some(enrollment)) => { - enrollment.on_experiment_ended(out_enrollment_events) - } + (Some(_), None, Some(enrollment)) => enrollment.on_experiment_ended( + #[cfg(feature = "stateful")] + gecko_pref_store, + out_enrollment_events, + ), // Known experiment. (Some(_), Some(experiment), Some(enrollment)) => { Some(enrollment.on_experiment_updated( @@ -950,6 +1112,8 @@ impl<'a> EnrollmentsEvolver<'a> { self.available_randomization_units, experiment, &targeting_helper, + #[cfg(feature = "stateful")] + gecko_pref_store, out_enrollment_events, )?) } diff --git a/components/nimbus/src/metrics.rs b/components/nimbus/src/metrics.rs index 6092c218ec..5c3cb1b1ef 100644 --- a/components/nimbus/src/metrics.rs +++ b/components/nimbus/src/metrics.rs @@ -2,6 +2,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +#[cfg(feature = "stateful")] +use crate::enrollment::PreviousState; + use crate::{enrollment::ExperimentEnrollment, EnrolledFeature, EnrollmentStatus}; use serde_derive::{Deserialize, Serialize}; @@ -28,6 +31,9 @@ pub struct EnrollmentStatusExtraDef { pub status: Option, #[cfg(not(feature = "stateful"))] pub user_id: Option, + #[cfg(feature = "stateful")] + #[serde(skip_serializing_if = "Option::is_none")] + pub previous_state: Option, } #[cfg(test)] @@ -93,6 +99,8 @@ impl From for EnrollmentStatusExtraDef { status: Some(enrollment.status.name()), #[cfg(not(feature = "stateful"))] user_id: None, + #[cfg(feature = "stateful")] + previous_state: None, } } } diff --git a/components/nimbus/src/nimbus.udl b/components/nimbus/src/nimbus.udl index 0b0cc1567e..4cb31b69d7 100644 --- a/components/nimbus/src/nimbus.udl +++ b/components/nimbus/src/nimbus.udl @@ -94,6 +94,8 @@ callback interface MetricsHandler { void record_malformed_feature_config(MalformedFeatureConfigExtraDef event); }; +typedef enum PreviousState; + dictionary EnrollmentStatusExtraDef { string? branch; string? conflict_slug; @@ -101,6 +103,14 @@ dictionary EnrollmentStatusExtraDef { string? reason; string? slug; string? status; + PreviousState? previous_state; +}; + + +dictionary PreviousGeckoPrefState { + sequence original_values; + string feature_id; + string variable; }; dictionary FeatureExposureExtraDef { @@ -120,6 +130,9 @@ callback interface GeckoPrefHandler { record> get_prefs_with_state(); void set_gecko_prefs_state(sequence new_prefs_state); + + void set_gecko_prefs_original_values(sequence original_gecko_prefs); + }; dictionary GeckoPref { @@ -139,12 +152,20 @@ enum PrefBranch { "User", }; +dictionary OriginalGeckoPref { + string pref; + PrefBranch branch; + PrefValue? value; +}; + + enum PrefUnenrollReason { "Changed", "FailedToSet", }; dictionary PrefEnrollmentData { + string experiment_slug; PrefValue pref_value; string feature_id; string variable; @@ -362,6 +383,11 @@ interface NimbusClient { [Throws=NimbusError] sequence unenroll_for_gecko_pref(GeckoPrefState pref_state, PrefUnenrollReason pref_unenroll_reason); + [Throws=NimbusError] + void register_previous_gecko_pref_states([ByRef] sequence gecko_pref_states); + + [Throws=NimbusError] + PreviousState? get_previous_state(string experiment_slug); }; interface NimbusTargetingHelper { diff --git a/components/nimbus/src/stateful/enrollment.rs b/components/nimbus/src/stateful/enrollment.rs index 566f1cc74e..bfb6e31e72 100644 --- a/components/nimbus/src/stateful/enrollment.rs +++ b/components/nimbus/src/stateful/enrollment.rs @@ -1,7 +1,10 @@ +use std::sync::Arc; + /* 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 https://mozilla.org/MPL/2.0/. */ use crate::enrollment::Participation; +use crate::stateful::gecko_prefs::GeckoPrefStore; use crate::stateful::persistence::{ DB_KEY_EXPERIMENT_PARTICIPATION, DB_KEY_ROLLOUT_PARTICIPATION, DEFAULT_EXPERIMENT_PARTICIPATION, DEFAULT_ROLLOUT_PARTICIPATION, @@ -27,6 +30,7 @@ impl EnrollmentsEvolver<'_> { db: &Database, writer: &mut Writer, next_experiments: &[Experiment], + gecko_pref_store: &Option>, ) -> Result> { // Get separate participation states from the db let is_participating_in_experiments = get_experiment_participation(db, writer)?; @@ -47,6 +51,7 @@ impl EnrollmentsEvolver<'_> { &prev_experiments, next_experiments, &prev_enrollments, + gecko_pref_store, )?; let next_enrollments = map_enrollments(&next_enrollments); // Write the changes to the Database. @@ -134,13 +139,14 @@ pub fn opt_out( db: &Database, writer: &mut Writer, experiment_slug: &str, + gecko_prefs: &Option>, ) -> Result> { let mut events = vec![]; let enr_store = db.get_store(StoreId::Enrollments); if let Ok(Some(existing_enrollment)) = enr_store.get::(writer, experiment_slug) { - let updated_enrollment = &existing_enrollment.on_explicit_opt_out(&mut events); + let updated_enrollment = &existing_enrollment.on_explicit_opt_out(&mut events, gecko_prefs); enr_store.put(writer, experiment_slug, updated_enrollment)?; } else { events.push(EnrollmentChangeEvent { diff --git a/components/nimbus/src/stateful/gecko_prefs.rs b/components/nimbus/src/stateful/gecko_prefs.rs index 9a20cff338..a7d67d14ee 100644 --- a/components/nimbus/src/stateful/gecko_prefs.rs +++ b/components/nimbus/src/stateful/gecko_prefs.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use crate::{ - enrollment::{EnrollmentStatus, ExperimentEnrollment}, + enrollment::{EnrollmentStatus, ExperimentEnrollment, PreviousGeckoPrefState}, error::Result, json::PrefValue, EnrolledExperiment, Experiment, NimbusError, @@ -14,7 +14,7 @@ use std::collections::{HashMap, HashSet}; use std::fmt::{Display, Formatter}; use std::sync::{Arc, Mutex, MutexGuard}; -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Copy)] +#[derive(Debug, Clone, Serialize, Deserialize, Hash, PartialEq, Eq, Copy)] #[serde(rename_all = "lowercase")] pub enum PrefBranch { Default, @@ -38,6 +38,7 @@ pub struct GeckoPref { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PrefEnrollmentData { + pub experiment_slug: String, pub pref_value: PrefValue, pub feature_id: String, pub variable: String, @@ -86,6 +87,26 @@ pub enum PrefUnenrollReason { FailedToSet, } +// The pre-experiment original state of a Gecko pref. Values may be used to set on Gecko to restore the pref to the original state. +// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️ +// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️ +#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)] +pub struct OriginalGeckoPref { + pub pref: String, + pub branch: PrefBranch, + pub value: Option, +} + +impl<'a> From<&'a GeckoPrefState> for OriginalGeckoPref { + fn from(state: &'a GeckoPrefState) -> Self { + Self { + pref: state.gecko_pref.pref.clone(), + branch: state.gecko_pref.branch, + value: state.gecko_value.clone(), + } + } +} + pub type MapOfFeatureIdToPropertyNameToGeckoPrefState = HashMap>; @@ -110,6 +131,9 @@ pub trait GeckoPrefHandler: Send + Sync { /// Used to set the state for each pref based on enrollments fn set_gecko_prefs_state(&self, new_prefs_state: Vec); + + /// Used to set back to the original state for each pref based on the original gecko value + fn set_gecko_prefs_original_values(&self, original_gecko_prefs: Vec); } #[derive(Default)] @@ -279,6 +303,7 @@ impl GeckoPrefStore { // experiments. props.entry(prop_name.clone()).and_modify(|pref_state| { pref_state.enrollment_value = Some(PrefEnrollmentData { + experiment_slug: slug.clone(), pref_value: prop_value.clone(), feature_id: feature, variable: prop_name, @@ -335,3 +360,24 @@ pub fn query_gecko_pref_store( .map(|store| Value::Bool(store.pref_is_user_set(&gecko_pref))) .unwrap_or(Value::Bool(false))) } + +pub type MapOfExperimentSlugToPreviousState = HashMap; +pub fn build_previous_states(states: &[GeckoPrefState]) -> MapOfExperimentSlugToPreviousState { + let mut original_gecko_states = MapOfExperimentSlugToPreviousState::new(); + + for state in states { + let Some(enrollment_value) = &state.enrollment_value else { + continue; + }; + + original_gecko_states + .entry(enrollment_value.experiment_slug.clone()) + .and_modify(|st| st.original_values.push(state.into())) + .or_insert_with(|| PreviousGeckoPrefState { + original_values: vec![state.into()], + feature_id: enrollment_value.feature_id.clone(), + variable: enrollment_value.variable.clone(), + }); + } + original_gecko_states +} diff --git a/components/nimbus/src/stateful/nimbus_client.rs b/components/nimbus/src/stateful/nimbus_client.rs index 85edcfce85..f4cc1964e5 100644 --- a/components/nimbus/src/stateful/nimbus_client.rs +++ b/components/nimbus/src/stateful/nimbus_client.rs @@ -8,7 +8,7 @@ use crate::{ defaults::Defaults, enrollment::{ EnrolledFeature, EnrollmentChangeEvent, EnrollmentChangeEventType, EnrollmentsEvolver, - ExperimentEnrollment, + ExperimentEnrollment, PreviousGeckoPrefState, PreviousState, }, error::{info, BehaviorError}, evaluator::{ @@ -31,8 +31,8 @@ use crate::{ unenroll_for_pref, }, gecko_prefs::{ - GeckoPref, GeckoPrefHandler, GeckoPrefState, GeckoPrefStore, PrefBranch, - PrefEnrollmentData, PrefUnenrollReason, + GeckoPref, GeckoPrefHandler, GeckoPrefState, GeckoPrefStore, OriginalGeckoPref, + PrefBranch, PrefEnrollmentData, PrefUnenrollReason, }, matcher::AppContext, persistence::{Database, StoreId, Writer}, @@ -40,8 +40,8 @@ use crate::{ updating::{read_and_remove_pending_experiments, write_pending_experiments}, }, strings::fmt_with_map, - AvailableExperiment, AvailableRandomizationUnits, EnrolledExperiment, Experiment, - ExperimentBranch, NimbusError, NimbusTargetingHelper, Result, + AvailableExperiment, AvailableRandomizationUnits, EnrolledExperiment, EnrollmentStatus, + Experiment, ExperimentBranch, NimbusError, NimbusTargetingHelper, Result, }; use chrono::{DateTime, NaiveDateTime, Utc}; use once_cell::sync::OnceCell; @@ -347,7 +347,7 @@ impl NimbusClient { pub fn opt_out(&self, experiment_slug: String) -> Result> { let db = self.db()?; let mut writer = db.write()?; - let result = opt_out(db, &mut writer, &experiment_slug)?; + let result = opt_out(db, &mut writer, &experiment_slug, &self.gecko_prefs)?; let mut state = self.mutable_state.lock().unwrap(); self.end_initialize(db, writer, &mut state)?; Ok(result) @@ -456,7 +456,7 @@ impl NimbusClient { &mut targeting_helper, &coenrolling_feature_ids, ); - evolver.evolve_enrollments_in_db(db, writer, experiments) + evolver.evolve_enrollments_in_db(db, writer, experiments, &self.gecko_prefs) } pub fn apply_pending_experiments(&self) -> Result> { @@ -803,6 +803,63 @@ impl NimbusClient { Ok(Vec::new()) } + pub fn register_previous_gecko_pref_states( + &self, + gecko_pref_states: &[GeckoPrefState], + ) -> Result<()> { + let previous_states = super::gecko_prefs::build_previous_states(gecko_pref_states); + + let db = self.db()?; + let mut writer = db.write()?; + + for (experiment_slug, previous_state) in previous_states { + Self::add_previous_state_for_experiment( + db, + &mut writer, + &experiment_slug, + PreviousState::GeckoPref(previous_state), + )?; + } + + let mut state = self.mutable_state.lock().unwrap(); + self.end_initialize(db, writer, &mut state)?; + Ok(()) + } + + pub(crate) fn add_previous_state_for_experiment( + db: &Database, + writer: &mut Writer, + experiment_slug: &str, + previous_state: PreviousState, + ) -> Result<()> { + let enrollments = db.get_store(StoreId::Enrollments); + + if let Ok(Some(existing_enrollment)) = + enrollments.get::(writer, experiment_slug) + { + // Previous state is only valid on Enrolled experiments + let updated_state = existing_enrollment.on_add_state(previous_state); + enrollments.put(writer, experiment_slug, &updated_state)?; + } + Ok(()) + } + + pub fn get_previous_state(&self, experiment_slug: String) -> Result> { + let db = self.db()?; + let reader = db.read()?; + + Ok(db + .get_store(StoreId::Enrollments) + .get::(&reader, &experiment_slug)? + .and_then(|enrollment| { + if let EnrollmentStatus::Enrolled { previous_state, .. } = enrollment.status { + previous_state + } else { + None + } + })) + } + #[cfg(test)] pub fn get_metrics_handler(&self) -> &&TestMetrics { let metrics = &**self.metrics_handler; diff --git a/components/nimbus/src/tests/helpers.rs b/components/nimbus/src/tests/helpers.rs index 59fe3ef0ca..2045f1060e 100644 --- a/components/nimbus/src/tests/helpers.rs +++ b/components/nimbus/src/tests/helpers.rs @@ -4,6 +4,8 @@ #![allow(unexpected_cfgs)] +#[cfg(feature = "stateful")] +use crate::stateful::gecko_prefs::OriginalGeckoPref; use crate::{ enrollment::{EnrolledFeatureConfig, EnrolledReason, ExperimentEnrollment, NotEnrolledReason}, metrics::{EnrollmentStatusExtraDef, MetricsHandler}, @@ -223,6 +225,7 @@ impl MetricsHandler for TestMetrics { #[cfg(feature = "stateful")] pub struct TestGeckoPrefHandlerState { pub prefs_set: Option>, + pub original_prefs_state: Option>, } #[cfg(feature = "stateful")] @@ -236,7 +239,10 @@ impl TestGeckoPrefHandler { pub(crate) fn new(prefs: MapOfFeatureIdToPropertyNameToGeckoPrefState) -> Self { Self { prefs, - state: Mutex::new(TestGeckoPrefHandlerState { prefs_set: None }), + state: Mutex::new(TestGeckoPrefHandlerState { + prefs_set: None, + original_prefs_state: None, + }), } } } @@ -253,6 +259,13 @@ impl GeckoPrefHandler for TestGeckoPrefHandler { .expect("Unable to lock TestGeckoPrefHandler state") .prefs_set = Some(new_prefs_state); } + + fn set_gecko_prefs_original_values(&self, original_prefs_state: Vec) { + self.state + .lock() + .expect("Unable to lock TestGeckoPrefHandler state") + .original_prefs_state = Some(original_prefs_state); + } } pub(crate) fn get_test_experiments() -> Vec { @@ -440,6 +453,8 @@ impl ExperimentEnrollment { status: EnrollmentStatus::Enrolled { branch: "control".to_string(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, } } diff --git a/components/nimbus/src/tests/stateful/test_enrollment.rs b/components/nimbus/src/tests/stateful/test_enrollment.rs index d314a39b30..2b771873f3 100644 --- a/components/nimbus/src/tests/stateful/test_enrollment.rs +++ b/components/nimbus/src/tests/stateful/test_enrollment.rs @@ -59,7 +59,7 @@ fn test_enrollments() -> Result<()> { let ids = no_coenrolling_features(); let mut evolver = EnrollmentsEvolver::new(&aru, &mut targeting_attributes, &ids); - let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &[exp1])?; + let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &[exp1], &None)?; let enrollments = get_enrollments(&db, &writer)?; assert_eq!(enrollments.len(), 1); @@ -95,7 +95,7 @@ fn test_enrollments() -> Result<()> { )); // Now opt-out. - opt_out(&db, &mut writer, "secure-gold")?; + opt_out(&db, &mut writer, "secure-gold", &None)?; assert_eq!(get_enrollments(&db, &writer)?.len(), 0); // check we recorded the "why" correctly. let ee: ExperimentEnrollment = db @@ -142,7 +142,7 @@ fn test_updates() -> Result<()> { let ids = no_coenrolling_features(); let mut targeting_helper = th.clone(); let mut evolver = EnrollmentsEvolver::new(&aru, &mut targeting_helper, &ids); - let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps)?; + let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps, &None)?; let enrollments = get_enrollments(&db, &writer)?; assert_eq!(enrollments.len(), 2); @@ -151,7 +151,7 @@ fn test_updates() -> Result<()> { // pretend we just updated from the server and one of the 2 is missing. let exps = &[exps[1].clone()]; let mut evolver = EnrollmentsEvolver::new(&aru, &mut th, &ids); - let events = evolver.evolve_enrollments_in_db(&db, &mut writer, exps)?; + let events = evolver.evolve_enrollments_in_db(&db, &mut writer, exps, &None)?; // should only have 1 now. let enrollments = get_enrollments(&db, &writer)?; @@ -192,7 +192,7 @@ fn test_global_opt_out() -> Result<()> { let ids = no_coenrolling_features(); let mut targeting_helper = th.clone(); let mut evolver = EnrollmentsEvolver::new(&aru, &mut targeting_helper, &ids); - let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps)?; + let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps, &None)?; let enrollments = get_enrollments(&db, &writer)?; assert_eq!(enrollments.len(), 0); @@ -217,7 +217,7 @@ fn test_global_opt_out() -> Result<()> { let mut targeting_helper = th.clone(); let mut evolver = EnrollmentsEvolver::new(&aru, &mut targeting_helper, &ids); - let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps)?; + let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps, &None)?; let enrollments = get_enrollments(&db, &writer)?; assert_eq!(enrollments.len(), 2); @@ -235,7 +235,7 @@ fn test_global_opt_out() -> Result<()> { let mut targeting_helper = th.clone(); let mut evolver = EnrollmentsEvolver::new(&aru, &mut targeting_helper, &ids); - let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps)?; + let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps, &None)?; let enrollments = get_enrollments(&db, &writer)?; assert_eq!(enrollments.len(), 0); @@ -263,7 +263,7 @@ fn test_global_opt_out() -> Result<()> { set_experiment_participation(&db, &mut writer, true)?; let mut evolver = EnrollmentsEvolver::new(&aru, &mut th, &ids); - let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps)?; + let events = evolver.evolve_enrollments_in_db(&db, &mut writer, &exps, &None)?; let enrollments = get_enrollments(&db, &writer)?; assert_eq!(enrollments.len(), 0); @@ -424,7 +424,7 @@ fn test_experiments_opt_out_with_rollouts_opt_in() -> Result<()> { let ids = no_coenrolling_features(); let mut evolver = EnrollmentsEvolver::new(&aru, &mut th, &ids); - let _ = evolver.evolve_enrollments_in_db(&db, &mut writer, &[experiment, rollout])?; + let _ = evolver.evolve_enrollments_in_db(&db, &mut writer, &[experiment, rollout], &None)?; let enrollments = get_experiment_enrollments(&db, &writer)?; println!("Total enrollments: {}", enrollments.len()); @@ -500,7 +500,8 @@ fn test_rollouts_opt_out_with_experiments_opt_in() -> Result<()> { let ids = no_coenrolling_features(); let mut evolver = EnrollmentsEvolver::new(&aru, &mut th, &ids); - let _events = evolver.evolve_enrollments_in_db(&db, &mut writer, &[experiment, rollout])?; + let _events = + evolver.evolve_enrollments_in_db(&db, &mut writer, &[experiment, rollout], &None)?; // Use the same helper function as the working test let enrollments = get_experiment_enrollments(&db, &writer)?; diff --git a/components/nimbus/src/tests/stateful/test_gecko_prefs.rs b/components/nimbus/src/tests/stateful/test_gecko_prefs.rs index 2b138a5f61..41e260d2c8 100644 --- a/components/nimbus/src/tests/stateful/test_gecko_prefs.rs +++ b/components/nimbus/src/tests/stateful/test_gecko_prefs.rs @@ -3,12 +3,12 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use crate::{ - enrollment::ExperimentEnrollment, + enrollment::{ExperimentEnrollment, PreviousGeckoPrefState}, error::Result, json::PrefValue, stateful::gecko_prefs::{ create_feature_prop_pref_map, GeckoPrefHandler, GeckoPrefState, GeckoPrefStore, - GeckoPrefStoreState, + GeckoPrefStoreState, OriginalGeckoPref, PrefBranch, PrefEnrollmentData, }, tests::helpers::{get_multi_feature_experiment, TestGeckoPrefHandler}, EnrolledExperiment, @@ -219,3 +219,154 @@ fn test_gecko_pref_store_pref_is_user_set() -> Result<()> { Ok(()) } + +#[test] +fn test_build_previous_states() -> Result<()> { + let pref_state_1 = GeckoPrefState::new("test.some.pref.1", Some(PrefBranch::Default)) + .with_gecko_value(json!("gecko-pref-value-1")) + .with_enrollment_value(PrefEnrollmentData { + experiment_slug: "experiment-slug-1".to_string(), + pref_value: json!("pref-value-1"), + feature_id: "feature-id-1".to_string(), + variable: "variable-1".to_string(), + }); + + // Belongs to the same experiment variable as 1, but a different pref + let pref_state_2 = GeckoPrefState::new("test.some.pref.2", Some(PrefBranch::User)) + .with_gecko_value(json!("gecko-pref-value-2")) + .with_enrollment_value(PrefEnrollmentData { + experiment_slug: "experiment-slug-1".to_string(), + pref_value: json!("pref-value-2"), + feature_id: "feature-id-1".to_string(), + variable: "variable-1".to_string(), + }); + + // Other random independent experiment + let pref_state_3 = GeckoPrefState::new("test.some.pref.3", Some(PrefBranch::Default)) + .with_gecko_value(json!("gecko-pref-value-3")) + .with_enrollment_value(PrefEnrollmentData { + experiment_slug: "experiment-slug-3".to_string(), + pref_value: json!("experiment-pref-value-3"), + feature_id: "feature-id-3".to_string(), + variable: "variable-3".to_string(), + }); + + // Experiment missing gecko value + let pref_state_4 = GeckoPrefState::new("test.some.pref.4", Some(PrefBranch::Default)) + .with_enrollment_value(PrefEnrollmentData { + experiment_slug: "experiment-slug-4".to_string(), + pref_value: json!("experiment-pref-value-4"), + feature_id: "feature-id-4".to_string(), + variable: "variable-4".to_string(), + }); + + // Experiment missing enrollment data + let pref_state_5 = GeckoPrefState::new("test.some.pref.5", Some(PrefBranch::Default)) + .with_gecko_value(json!("gecko-pref-value-5")); + + let pref_states = vec![ + pref_state_1, + pref_state_2, + pref_state_3, + pref_state_4, + pref_state_5, + ]; + let previous_states = crate::stateful::gecko_prefs::build_previous_states(&pref_states); + + assert_eq!(3, previous_states.len()); + assert!(previous_states.contains_key("experiment-slug-1")); + assert!(previous_states.contains_key("experiment-slug-3")); + + let PreviousGeckoPrefState { + original_values: original_values_1, + feature_id: feature_id_1, + variable: variable_1, + } = previous_states + .get("experiment-slug-1") + .expect("Missing slug"); + + assert_eq!("feature-id-1", feature_id_1); + assert_eq!("variable-1", variable_1); + assert_eq!(2, original_values_1.len()); + + assert_eq!("test.some.pref.1", original_values_1[0].pref); + assert_eq!(PrefBranch::Default, original_values_1[0].branch); + assert_eq!( + "gecko-pref-value-1", + original_values_1[0].value.clone().unwrap() + ); + + assert_eq!("test.some.pref.2", original_values_1[1].pref); + assert_eq!(PrefBranch::User, original_values_1[1].branch); + assert_eq!( + "gecko-pref-value-2", + original_values_1[1].value.clone().unwrap() + ); + + let PreviousGeckoPrefState { + original_values: original_values_3, + feature_id: feature_id_3, + variable: variable_3, + } = previous_states + .get("experiment-slug-3") + .expect("Missing slug"); + + assert_eq!("feature-id-3", feature_id_3); + assert_eq!("variable-3", variable_3); + assert_eq!(1, original_values_3.len()); + assert_eq!("test.some.pref.3", original_values_3[0].pref); + assert_eq!(PrefBranch::Default, original_values_3[0].branch); + assert_eq!( + "gecko-pref-value-3", + original_values_3[0].value.clone().unwrap() + ); + + let PreviousGeckoPrefState { + original_values: original_values_4, + feature_id: feature_id_4, + variable: variable_4, + } = previous_states + .get("experiment-slug-4") + .expect("Missing slug"); + + assert_eq!("feature-id-4", feature_id_4); + assert_eq!("variable-4", variable_4); + assert_eq!(1, original_values_4.len()); + assert_eq!("test.some.pref.4", original_values_4[0].pref); + assert_eq!(PrefBranch::Default, original_values_4[0].branch); + assert_eq!(None, original_values_4[0].value.clone()); + + Ok(()) +} + +#[test] +fn test_set_gecko_prefs_original_values() { + let pref_state_1 = GeckoPrefState::new("test.some.pref.1", Some(PrefBranch::Default)) + .with_gecko_value(serde_json::Value::String(String::from( + "some-gecko-value-1", + ))); + let original_gecko_prefs = vec![OriginalGeckoPref::from(&pref_state_1)]; + let handler = TestGeckoPrefHandler::new(create_feature_prop_pref_map(vec![( + "feature-id", + "test_prop", + pref_state_1.clone(), + )])); + let handler: Arc> = Arc::new(Box::new(handler)); + let store = Arc::new(GeckoPrefStore::new(handler.clone())); + let _ = store.initialize(); + + handler.set_gecko_prefs_original_values(original_gecko_prefs.clone()); + let test_handler = unsafe { + std::mem::transmute::>, Arc>>( + handler, + ) + }; + let test_handler_state = test_handler + .state + .lock() + .expect("Unable to lock transmuted handler state"); + + let original_prefs_stored = test_handler_state.original_prefs_state.clone().unwrap(); + assert_eq!(1, original_prefs_stored.len()); + assert_eq!(&original_gecko_prefs, &original_prefs_stored); +} diff --git a/components/nimbus/src/tests/stateful/test_nimbus.rs b/components/nimbus/src/tests/stateful/test_nimbus.rs index cb9c17d1b3..051fdd6717 100644 --- a/components/nimbus/src/tests/stateful/test_nimbus.rs +++ b/components/nimbus/src/tests/stateful/test_nimbus.rs @@ -3,7 +3,10 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use crate::{ - enrollment::{DisqualifiedReason, EnrolledReason, EnrollmentStatus, ExperimentEnrollment}, + enrollment::{ + DisqualifiedReason, EnrolledReason, EnrollmentStatus, ExperimentEnrollment, + PreviousGeckoPrefState, PreviousState, + }, error::{info, Result}, json::PrefValue, metrics::MalformedFeatureConfigExtraDef, @@ -12,7 +15,10 @@ use crate::{ EventStore, Interval, IntervalConfig, IntervalData, MultiIntervalCounter, SingleIntervalCounter, }, - gecko_prefs::{create_feature_prop_pref_map, GeckoPrefState, PrefUnenrollReason}, + gecko_prefs::{ + create_feature_prop_pref_map, GeckoPrefState, OriginalGeckoPref, PrefBranch, + PrefEnrollmentData, PrefUnenrollReason, + }, persistence::{Database, StoreId}, targeting::RecordedContext, }, @@ -1977,3 +1983,252 @@ fn test_gecko_pref_unenrollment() -> Result<()> { Ok(()) } + +#[test] +fn register_previous_gecko_pref_states() -> Result<()> { + let metrics = TestMetrics::new(); + let temp_dir = tempfile::tempdir()?; + let app_context = AppContext { + app_name: "fenix".to_string(), + app_id: "org.mozilla.fenix".to_string(), + channel: "nightly".to_string(), + app_version: Some("124.0.0".to_string()), + ..Default::default() + }; + let recorded_context = Arc::new(TestRecordedContext::new()); + let pref_state = GeckoPrefState::new("test.pref", None).with_gecko_value(PrefValue::Null); + let handler = TestGeckoPrefHandler::new(create_feature_prop_pref_map(vec![( + "test_feature", + "test_prop", + pref_state.clone(), + )])); + let client = NimbusClient::new( + app_context.clone(), + Some(recorded_context), + Default::default(), + temp_dir.path(), + Box::new(metrics.clone()), + Some(Box::new(handler)), + None, + None, + )?; + client.set_nimbus_id(&Uuid::from_str("00000000-0000-0000-0000-000000000004")?)?; + client.initialize()?; + + let experiment_slug_1 = "exp-1"; + let experiment_1 = get_multi_feature_experiment( + experiment_slug_1, + vec![( + "test_feature", + json!({ + "test_prop": "some-experiment-value" + }), + )], + ) + .with_targeting("true"); + + let experiment_slug_2 = "exp-2"; + let experiment_2 = get_multi_feature_experiment( + experiment_slug_2, + vec![( + "test_feature_2", + json!({ + "test_prop": "some-experiment-value" + }), + )], + ) + .with_targeting("true"); + + client.set_experiments_locally(to_local_experiments_string(&[experiment_1, experiment_2])?)?; + client.apply_pending_experiments()?; + + let mut active_experiments = client.get_active_experiments()?; + active_experiments.sort_by(|a, b| a.slug.cmp(&b.slug)); + assert_eq!(active_experiments.len(), 2); + assert_eq!(active_experiments[0].slug, experiment_slug_1); + assert_eq!(active_experiments[1].slug, experiment_slug_2); + + // Shouldn't have a previous state yet + { + let db = client.db()?; + let reader = db.read()?; + + let enrollments: Vec = + db.get_store(StoreId::Enrollments).collect_all(&reader)?; + + assert_eq!(enrollments.len(), 2); + let enrollment_1 = enrollments + .iter() + .find(|e| e.slug == experiment_slug_1) + .expect("Should have an ExperimentEnrollment present."); + assert!(matches!( + enrollment_1.status, + EnrollmentStatus::Enrolled { + previous_state: None, + .. + } + )); + } + + let gecko_pref_state_1 = GeckoPrefState::new("some.pref", Some(PrefBranch::Default)) + .with_gecko_value(json!("some-gecko-value")) + .with_enrollment_value(PrefEnrollmentData { + experiment_slug: experiment_slug_1.to_string(), + pref_value: json!("enrollment-pref-value"), + feature_id: "feature_id".into(), + variable: "variable".into(), + }); + + let gecko_pref_state_2 = GeckoPrefState::new("some.pref", Some(PrefBranch::Default)) + .with_gecko_value(json!("some-gecko-value")) + .with_enrollment_value(PrefEnrollmentData { + experiment_slug: experiment_slug_2.to_string(), + pref_value: json!("enrollment-pref-value"), + feature_id: "feature_id".into(), + variable: "variable".into(), + }); + + let gecko_pref_states = vec![gecko_pref_state_1, gecko_pref_state_2]; + let registration = client.register_previous_gecko_pref_states(&gecko_pref_states); + assert!(registration.is_ok()); + + let db = client.db()?; + let reader = db.read()?; + let mut enrollments: Vec = + db.get_store(StoreId::Enrollments).collect_all(&reader)?; + enrollments.sort_by(|a, b| a.slug.cmp(&b.slug)); + assert_eq!(active_experiments.len(), 2); + + let previous_state_1 = PreviousGeckoPrefState { + original_values: vec![OriginalGeckoPref { + pref: "some.pref".into(), + branch: PrefBranch::Default, + value: Some(serde_json::Value::String(String::from("some-gecko-value"))), + }], + feature_id: "feature_id".into(), + variable: "variable".into(), + }; + + assert!(matches!( + enrollments[0].clone().status, + EnrollmentStatus::Enrolled { previous_state, .. } + if previous_state == Some(PreviousState::GeckoPref(previous_state_1.clone())) + )); + + let previous_state_1_using_get = client.get_previous_state(experiment_slug_1.to_string()); + + assert_eq!( + Some(PreviousState::GeckoPref(previous_state_1)), + previous_state_1_using_get.unwrap() + ); + + assert!(matches!( + enrollments[1].clone().status, + EnrollmentStatus::Enrolled { + previous_state: Some(_), + .. + } + )); + + Ok(()) +} + +#[test] +fn test_add_previous_state_for_experiment() -> Result<()> { + let metrics = TestMetrics::new(); + let temp_dir = tempfile::tempdir()?; + let app_context = AppContext { + app_name: "fenix".to_string(), + app_id: "org.mozilla.fenix".to_string(), + channel: "nightly".to_string(), + app_version: Some("124.0.0".to_string()), + ..Default::default() + }; + let recorded_context = Arc::new(TestRecordedContext::new()); + let pref_state = GeckoPrefState::new("test.pref", None).with_gecko_value(PrefValue::Null); + let handler = TestGeckoPrefHandler::new(create_feature_prop_pref_map(vec![( + "test_feature", + "test_prop", + pref_state.clone(), + )])); + let client = NimbusClient::new( + app_context.clone(), + Some(recorded_context), + Default::default(), + temp_dir.path(), + Box::new(metrics.clone()), + Some(Box::new(handler)), + None, + None, + )?; + client.set_nimbus_id(&Uuid::from_str("00000000-0000-0000-0000-000000000004")?)?; + client.initialize()?; + + let experiment_slug = "exp-1"; + let experiment = get_multi_feature_experiment( + experiment_slug, + vec![( + "test_feature", + json!({ + "test_prop": "some-experiment-value" + }), + )], + ) + .with_targeting("true"); + + client.set_experiments_locally(to_local_experiments_string(&[experiment])?)?; + client.apply_pending_experiments()?; + + let active_experiments = client.get_active_experiments()?; + assert_eq!(active_experiments.len(), 1); + + let original_previous_state = PreviousGeckoPrefState { + original_values: vec![OriginalGeckoPref { + pref: "some.pref".into(), + branch: PrefBranch::Default, + value: Some(serde_json::Value::String(String::from("some-gecko-value"))), + }], + feature_id: "some_control".into(), + variable: "test_variable".into(), + }; + + let db = client.db()?; + let mut writer = db.write()?; + NimbusClient::add_previous_state_for_experiment( + db, + &mut writer, + experiment_slug, + PreviousState::GeckoPref(original_previous_state.clone()), + )?; + let enrollments: Vec = + db.get_store(StoreId::Enrollments).collect_all(&writer)?; + + let experiment_result = enrollments + .into_iter() + .find(|e| e.slug == experiment_slug) + .expect("Should have an Experiment present."); + + assert!(matches!( + experiment_result.status, + EnrollmentStatus::Enrolled { previous_state, .. } + if previous_state == Some(PreviousState::GeckoPref(original_previous_state.clone())) + )); + + let reader_result = db + .get_store(StoreId::Enrollments) + .get::(&writer, experiment_slug)? + .and_then(|enrollment| { + if let EnrollmentStatus::Enrolled { previous_state, .. } = enrollment.status { + previous_state + } else { + None + } + }) + .unwrap(); + + assert_eq!( + reader_result, + PreviousState::GeckoPref(original_previous_state.clone()) + ); + Ok(()) +} diff --git a/components/nimbus/src/tests/test_enrollment.rs b/components/nimbus/src/tests/test_enrollment.rs index ecb7655860..8bba4d02df 100644 --- a/components/nimbus/src/tests/test_enrollment.rs +++ b/components/nimbus/src/tests/test_enrollment.rs @@ -3,7 +3,8 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ // Testing enrollment.rs - +#[cfg(feature = "stateful")] +use crate::stateful::gecko_prefs::{OriginalGeckoPref, PrefBranch}; use crate::tests::helpers::{get_bucketed_rollout, get_experiment_with_published_date}; use crate::{ defaults::Defaults, @@ -16,6 +17,15 @@ use crate::{ AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, FeatureConfig, NimbusTargetingHelper, TargetingAttributes, }; +#[cfg(feature = "stateful")] +use crate::{ + stateful::gecko_prefs::{ + create_feature_prop_pref_map, GeckoPrefHandler, GeckoPrefState, GeckoPrefStore, + }, + tests::helpers::TestGeckoPrefHandler, +}; +#[cfg(feature = "stateful")] +use std::sync::Arc; cfg_if::cfg_if! { if #[cfg(feature = "stateful")] { @@ -434,7 +444,15 @@ fn test_ios_rollout_experiment() -> Result<()> { let mut evolver = enrollment_evolver(&mut th, &aru, &ids); let mut events = vec![]; let enrollment = evolver - .evolve_enrollment::(true, None, Some(exp), None, &mut events)? + .evolve_enrollment::( + true, + None, + Some(exp), + None, + &mut events, + #[cfg(feature = "stateful")] + &None, + )? .unwrap(); println!("Enrollment: {:?}", &enrollment.status); assert!(matches!( @@ -453,7 +471,15 @@ fn test_evolver_new_experiment_enrolled() -> Result<()> { let mut evolver = enrollment_evolver(&mut th, &aru, &ids); let mut events = vec![]; let enrollment = evolver - .evolve_enrollment::(true, None, Some(exp), None, &mut events)? + .evolve_enrollment::( + true, + None, + Some(exp), + None, + &mut events, + #[cfg(feature = "stateful")] + &None, + )? .unwrap(); assert!(matches!( enrollment.status, @@ -475,7 +501,15 @@ fn test_evolver_new_experiment_not_enrolled() -> Result<()> { let mut evolver = enrollment_evolver(&mut th, &aru, &ids); let mut events = vec![]; let enrollment = evolver - .evolve_enrollment::(true, None, Some(&exp), None, &mut events)? + .evolve_enrollment::( + true, + None, + Some(&exp), + None, + &mut events, + #[cfg(feature = "stateful")] + &None, + )? .unwrap(); assert!(matches!( enrollment.status, @@ -496,7 +530,15 @@ fn test_evolver_new_experiment_globally_opted_out() -> Result<()> { let mut evolver = enrollment_evolver(&mut th, &aru, &ids); let mut events = vec![]; let enrollment = evolver - .evolve_enrollment::(false, None, Some(&exp), None, &mut events)? + .evolve_enrollment::( + false, + None, + Some(&exp), + None, + &mut events, + #[cfg(feature = "stateful")] + &None, + )? .unwrap(); assert!(matches!( enrollment.status, @@ -518,7 +560,15 @@ fn test_evolver_new_experiment_enrollment_paused() -> Result<()> { let mut evolver = enrollment_evolver(&mut th, &aru, &ids); let mut events = vec![]; let enrollment = evolver - .evolve_enrollment::(true, None, Some(&exp), None, &mut events)? + .evolve_enrollment::( + true, + None, + Some(&exp), + None, + &mut events, + #[cfg(feature = "stateful")] + &None, + )? .unwrap(); assert!(matches!( enrollment.status, @@ -551,6 +601,8 @@ fn test_evolver_experiment_update_not_enrolled_opted_out() -> Result<()> { Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert_eq!(enrollment.status, existing_enrollment.status); @@ -580,6 +632,8 @@ fn test_evolver_experiment_update_not_enrolled_enrollment_paused() -> Result<()> Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert_eq!(enrollment.status, existing_enrollment.status); @@ -609,6 +663,8 @@ fn test_evolver_experiment_update_not_enrolled_resuming_not_selected() -> Result Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert!(matches!( @@ -642,6 +698,8 @@ fn test_evolver_experiment_update_not_enrolled_resuming_selected() -> Result<()> Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert!(matches!( @@ -670,6 +728,8 @@ fn test_evolver_experiment_update_enrolled_then_opted_out() -> Result<()> { status: EnrollmentStatus::Enrolled { branch: "control".to_owned(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; let enrollment = evolver @@ -679,6 +739,8 @@ fn test_evolver_experiment_update_enrolled_then_opted_out() -> Result<()> { Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert!(matches!( @@ -713,6 +775,8 @@ fn test_evolver_experiment_update_enrolled_then_experiment_paused() -> Result<() status: EnrollmentStatus::Enrolled { branch: "control".to_owned(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; let enrollment = evolver @@ -722,12 +786,16 @@ fn test_evolver_experiment_update_enrolled_then_experiment_paused() -> Result<() Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); dbg!(&enrollment.status); if let EnrollmentStatus::Enrolled { reason: EnrolledReason::Qualified, branch, + #[cfg(feature = "stateful")] + previous_state: None, .. } = enrollment.status { @@ -753,6 +821,8 @@ fn test_evolver_experiment_update_enrolled_then_targeting_changed() -> Result<() status: EnrollmentStatus::Enrolled { branch: "control".to_owned(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; let enrollment = evolver @@ -762,6 +832,8 @@ fn test_evolver_experiment_update_enrolled_then_targeting_changed() -> Result<() Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); @@ -799,6 +871,8 @@ fn test_evolver_experiment_update_enrolled_then_bucketing_changed() -> Result<() status: EnrollmentStatus::Enrolled { branch: "control".to_owned(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; let observed = evolver @@ -808,6 +882,8 @@ fn test_evolver_experiment_update_enrolled_then_bucketing_changed() -> Result<() Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert!(matches!( @@ -837,8 +913,14 @@ fn test_rollout_unenrolls_when_bucketing_changes() -> Result<()> { let ro = get_bucketed_rollout(slug, 0); let recipes = [ro]; - let (enrollments, _) = - evolver.evolve_enrollments::(Participation::default(), &[], &recipes, &[])?; + let (enrollments, _) = evolver.evolve_enrollments::( + Participation::default(), + &[], + &recipes, + &[], + #[cfg(feature = "stateful")] + &None, + )?; assert_eq!(enrollments.len(), 1); let enr = enrollments.first().unwrap(); @@ -855,6 +937,8 @@ fn test_rollout_unenrolls_when_bucketing_changes() -> Result<()> { &prev_recipes, &recipes, enrollments.as_slice(), + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(enrollments.len(), 1); let enr = enrollments.first().unwrap(); @@ -871,6 +955,8 @@ fn test_rollout_unenrolls_when_bucketing_changes() -> Result<()> { &prev_recipes, &recipes, enrollments.as_slice(), + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(enrollments.len(), 1); let enr = enrollments.first().unwrap(); @@ -899,8 +985,14 @@ fn test_rollout_unenrolls_then_reenrolls_when_bucketing_changes() -> Result<()> let ro = get_bucketed_rollout(slug, 0); let recipes = [ro]; - let (enrollments, _) = - evolver.evolve_enrollments::(Participation::default(), &[], &recipes, &[])?; + let (enrollments, _) = evolver.evolve_enrollments::( + Participation::default(), + &[], + &recipes, + &[], + #[cfg(feature = "stateful")] + &None, + )?; assert_eq!(enrollments.len(), 1); let enr = enrollments.first().unwrap(); @@ -917,6 +1009,8 @@ fn test_rollout_unenrolls_then_reenrolls_when_bucketing_changes() -> Result<()> &prev_recipes, &recipes, enrollments.as_slice(), + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(enrollments.len(), 1); let enr = enrollments.first().unwrap(); @@ -933,6 +1027,8 @@ fn test_rollout_unenrolls_then_reenrolls_when_bucketing_changes() -> Result<()> &prev_recipes, &recipes, enrollments.as_slice(), + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(enrollments.len(), 1); let enr = enrollments.first().unwrap(); @@ -955,6 +1051,8 @@ fn test_rollout_unenrolls_then_reenrolls_when_bucketing_changes() -> Result<()> &prev_recipes, &recipes, enrollments.as_slice(), + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(enrollments.len(), 1); let enr = enrollments.first().unwrap(); @@ -1001,6 +1099,8 @@ fn test_experiment_does_not_reenroll_from_disqualified_not_selected_or_not_targe }, }, ], + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(enrollments.len(), 2); @@ -1043,6 +1143,8 @@ fn test_evolver_experiment_update_enrolled_then_branches_changed() -> Result<()> status: EnrollmentStatus::Enrolled { branch: "control".to_owned(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; let enrollment = evolver @@ -1052,6 +1154,8 @@ fn test_evolver_experiment_update_enrolled_then_branches_changed() -> Result<()> Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert_eq!(enrollment, existing_enrollment); @@ -1078,6 +1182,8 @@ fn test_evolver_experiment_update_enrolled_then_branch_disappears() -> Result<() status: EnrollmentStatus::Enrolled { branch: "control".to_owned(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; let enrollment = evolver @@ -1087,6 +1193,8 @@ fn test_evolver_experiment_update_enrolled_then_branch_disappears() -> Result<() Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert!(matches!( @@ -1128,6 +1236,8 @@ fn test_evolver_experiment_update_disqualified_then_opted_out() -> Result<()> { Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert!(matches!( @@ -1163,6 +1273,8 @@ fn test_evolver_experiment_update_disqualified_then_bucketing_ok() -> Result<()> Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert_eq!(enrollment, existing_enrollment); @@ -1187,6 +1299,8 @@ fn test_evolver_feature_can_have_only_one_experiment() -> Result<()> { &existing_experiments, &updated_experiments, &existing_enrollments, + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(2, enrollments.len()); @@ -1227,6 +1341,8 @@ fn test_evolver_feature_can_have_only_one_experiment() -> Result<()> { &existing_experiments, &updated_experiments, &existing_enrollments, + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(2, enrollments.len()); @@ -1254,6 +1370,8 @@ fn test_evolver_feature_can_have_only_one_experiment() -> Result<()> { &existing_experiments, &updated_experiments, &existing_enrollments, + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(2, enrollments.len()); @@ -1277,6 +1395,8 @@ fn test_evolver_feature_can_have_only_one_experiment() -> Result<()> { &existing_experiments, &updated_experiments, &existing_enrollments, + #[cfg(feature = "stateful")] + &None, )?; // There should be one WasEnrolled; the NotEnrolled will have been @@ -1340,6 +1460,8 @@ fn test_evolver_experiment_not_enrolled_feature_conflict() -> Result<()> { &[], &test_experiments, &[], + #[cfg(feature = "stateful")] + &None, )?; assert_eq!( @@ -1406,6 +1528,8 @@ fn test_multi_feature_per_branch_conflict() -> Result<()> { &[], &test_experiments, &[], + #[cfg(feature = "stateful")] + &None, )?; let enrolled_count = enrollments @@ -1450,6 +1574,8 @@ fn test_evolver_feature_id_reuse() -> Result<()> { &[], &test_experiments, &[], + #[cfg(feature = "stateful")] + &None, )?; let enrolled_count = enrollments @@ -1467,6 +1593,8 @@ fn test_evolver_feature_id_reuse() -> Result<()> { &test_experiments, &[test_experiments[1].clone(), conflicting_experiment.clone()], &enrollments, + #[cfg(feature = "stateful")] + &None, )?; debug!("events = {:?}", events); @@ -1517,6 +1645,8 @@ fn test_evolver_multi_feature_experiments() -> Result<()> { &[], &next_experiments, &[], + #[cfg(feature = "stateful")] + &None, )?; let feature_map = @@ -1559,6 +1689,8 @@ fn test_evolver_multi_feature_experiments() -> Result<()> { &prev_experiments, &next_experiments, &prev_enrollments, + #[cfg(feature = "stateful")] + &None, )?; assert_eq!( @@ -1603,6 +1735,8 @@ fn test_evolver_multi_feature_experiments() -> Result<()> { &prev_experiments, &next_experiments, &prev_enrollments, + #[cfg(feature = "stateful")] + &None, )?; let feature_map = @@ -1635,6 +1769,8 @@ fn test_evolver_multi_feature_experiments() -> Result<()> { &prev_experiments, &next_experiments, &prev_enrollments, + #[cfg(feature = "stateful")] + &None, )?; let feature_map = @@ -1671,6 +1807,8 @@ fn test_evolver_multi_feature_experiments() -> Result<()> { &prev_experiments, &next_experiments, &prev_enrollments, + #[cfg(feature = "stateful")] + &None, )?; // 4b. Add the single feature experiments. @@ -1686,6 +1824,8 @@ fn test_evolver_multi_feature_experiments() -> Result<()> { &prev_experiments, &next_experiments, &prev_enrollments, + #[cfg(feature = "stateful")] + &None, )?; assert_eq!( @@ -1727,6 +1867,8 @@ fn test_evolver_multi_feature_experiments() -> Result<()> { &prev_experiments, &next_experiments, &prev_enrollments, + #[cfg(feature = "stateful")] + &None, )?; assert_eq!( @@ -1767,6 +1909,8 @@ fn test_evolver_multi_feature_experiments() -> Result<()> { &prev_experiments, &next_experiments, &prev_enrollments, + #[cfg(feature = "stateful")] + &None, )?; let feature_map = @@ -1823,6 +1967,8 @@ fn test_evolver_experiment_update_was_enrolled() -> Result<()> { Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert_eq!(enrollment, existing_enrollment); @@ -2038,8 +2184,14 @@ fn test_evolve_enrollments_with_coenrolling_features() -> Result<()> { let all_experiments = [exp1, exp2, exp3.clone(), exp4.clone()]; let no_experiments: [Experiment; 0] = []; - let (enrollments, _) = - evolver.evolve_enrollment_recipes(true, &no_experiments, &all_experiments, &[])?; + let (enrollments, _) = evolver.evolve_enrollment_recipes( + true, + &no_experiments, + &all_experiments, + &[], + #[cfg(feature = "stateful")] + &None, + )?; let observed = map_features_by_feature_id(&enrollments, &all_experiments, &ids); let expected = HashMap::from([ @@ -2064,8 +2216,14 @@ fn test_evolve_enrollments_with_coenrolling_features() -> Result<()> { assert_eq!(observed, expected); let experiments = [exp3, exp4]; - let (enrollments, _) = - evolver.evolve_enrollment_recipes(true, &all_experiments, &experiments, &enrollments)?; + let (enrollments, _) = evolver.evolve_enrollment_recipes( + true, + &all_experiments, + &experiments, + &enrollments, + #[cfg(feature = "stateful")] + &None, + )?; let observed = map_features_by_feature_id(&enrollments, &all_experiments, &ids); let expected = HashMap::from([ @@ -2124,8 +2282,14 @@ fn test_evolve_enrollments_with_coenrolling_multi_features() -> Result<()> { let all_experiments = [exp1, exp2, exp3.clone(), exp4.clone()]; let no_experiments: [Experiment; 0] = []; - let (enrollments, _) = - evolver.evolve_enrollment_recipes(true, &no_experiments, &all_experiments, &[])?; + let (enrollments, _) = evolver.evolve_enrollment_recipes( + true, + &no_experiments, + &all_experiments, + &[], + #[cfg(feature = "stateful")] + &None, + )?; let observed = map_features_by_feature_id(&enrollments, &all_experiments, &ids); let expected = HashMap::from([ @@ -2156,8 +2320,14 @@ fn test_evolve_enrollments_with_coenrolling_multi_features() -> Result<()> { assert_eq!(observed, expected); let experiments = [exp3, exp4]; - let (enrollments, _) = - evolver.evolve_enrollment_recipes(true, &all_experiments, &experiments, &enrollments)?; + let (enrollments, _) = evolver.evolve_enrollment_recipes( + true, + &all_experiments, + &experiments, + &enrollments, + #[cfg(feature = "stateful")] + &None, + )?; let observed = map_features_by_feature_id(&enrollments, &all_experiments, &ids); let expected = HashMap::from([ @@ -2276,6 +2446,8 @@ fn test_evolve_enrollments_error_handling() -> Result<()> { status: EnrollmentStatus::Enrolled { branch: "hello".to_owned(), // XXX this OK? reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }]; @@ -2295,6 +2467,8 @@ fn test_evolve_enrollments_error_handling() -> Result<()> { &test_experiments, &test_experiments, &[], + #[cfg(feature = "stateful")] + &None, )?; assert_eq!( @@ -2316,6 +2490,8 @@ fn test_evolve_enrollments_error_handling() -> Result<()> { &[], &test_experiments, &existing_enrollments[..], + #[cfg(feature = "stateful")] + &None, )?; assert_eq!( @@ -2352,6 +2528,8 @@ fn test_evolve_enrollments_is_already_enrolled_targeting() -> Result<()> { &[], test_experiments, &[], + #[cfg(feature = "stateful")] + &None, )?; assert_eq!( enrollments.len(), @@ -2375,6 +2553,8 @@ fn test_evolve_enrollments_is_already_enrolled_targeting() -> Result<()> { test_experiments, test_experiments, &enrollments, + #[cfg(feature = "stateful")] + &None, )?; assert_eq!( enrollments.len(), @@ -2412,6 +2592,8 @@ fn test_evolver_experiment_update_error() -> Result<()> { Some(&exp), Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); assert!(matches!( @@ -2435,6 +2617,8 @@ fn test_evolver_experiment_ended_was_enrolled() -> Result<()> { status: EnrollmentStatus::Enrolled { branch: "control".to_owned(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; let enrollment = evolver @@ -2444,6 +2628,8 @@ fn test_evolver_experiment_ended_was_enrolled() -> Result<()> { None, Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); if let EnrollmentStatus::WasEnrolled { branch, .. } = enrollment.status { @@ -2480,6 +2666,8 @@ fn test_evolver_experiment_ended_was_disqualified() -> Result<()> { None, Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )? .unwrap(); if let EnrollmentStatus::WasEnrolled { branch, .. } = enrollment.status { @@ -2514,6 +2702,8 @@ fn test_evolver_experiment_ended_was_not_enrolled() -> Result<()> { None, Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )?; assert!(enrollment.is_none()); assert!(events.is_empty()); @@ -2540,6 +2730,8 @@ fn test_evolver_garbage_collection_before_threshold() -> Result<()> { None, Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )?; assert_eq!(enrollment.unwrap(), existing_enrollment); assert!(events.is_empty()); @@ -2566,6 +2758,8 @@ fn test_evolver_garbage_collection_after_threshold() -> Result<()> { None, Some(&existing_enrollment), &mut events, + #[cfg(feature = "stateful")] + &None, )?; assert!(enrollment.is_none()); assert!(events.is_empty()); @@ -2592,6 +2786,8 @@ fn test_evolver_new_experiment_enrollment_already_exists() { Some(&exp), Some(&existing_enrollment), &mut vec![], + #[cfg(feature = "stateful")] + &None, ); assert!(res.is_err()); } @@ -2603,7 +2799,15 @@ fn test_evolver_existing_experiment_has_no_enrollment() { let mut th = app_ctx.into(); let ids = no_coenrolling_features(); let mut evolver = enrollment_evolver(&mut th, &aru, &ids); - let res = evolver.evolve_enrollment(true, Some(&exp), Some(&exp), None, &mut vec![]); + let res = evolver.evolve_enrollment( + true, + Some(&exp), + Some(&exp), + None, + &mut vec![], + #[cfg(feature = "stateful")] + &None, + ); assert!(res.is_err()); } @@ -2615,7 +2819,15 @@ fn test_evolver_no_experiments_no_enrollment() { let ids = no_coenrolling_features(); let mut evolver = enrollment_evolver(&mut th, &aru, &ids); evolver - .evolve_enrollment::(true, None, None, None, &mut vec![]) + .evolve_enrollment::( + true, + None, + None, + None, + &mut vec![], + #[cfg(feature = "stateful")] + &None, + ) .unwrap(); } @@ -2656,8 +2868,14 @@ fn test_evolver_rollouts_do_not_conflict_with_experiments() -> Result<()> { let mut th = app_ctx.into(); let ids = no_coenrolling_features(); let mut evolver = enrollment_evolver(&mut th, &aru, &ids); - let (enrollments, events) = - evolver.evolve_enrollments::(Participation::default(), &[], recipes, &[])?; + let (enrollments, events) = evolver.evolve_enrollments::( + Participation::default(), + &[], + recipes, + &[], + #[cfg(feature = "stateful")] + &None, + )?; assert_eq!(enrollments.len(), 2); assert_eq!(events.len(), 2); @@ -2717,8 +2935,14 @@ fn test_evolver_rollouts_do_not_conflict_with_rollouts() -> Result<()> { let mut th = app_ctx.into(); let ids = no_coenrolling_features(); let mut evolver = enrollment_evolver(&mut th, &aru, &ids); - let (enrollments, events) = - evolver.evolve_enrollments::(Participation::default(), &[], recipes, &[])?; + let (enrollments, events) = evolver.evolve_enrollments::( + Participation::default(), + &[], + recipes, + &[], + #[cfg(feature = "stateful")] + &None, + )?; assert_eq!(enrollments.len(), 3); assert_eq!(events.len(), 3); @@ -2915,6 +3139,8 @@ fn test_evolver_map_features_by_feature_id_merges_rollouts() -> Result<()> { status: EnrollmentStatus::Enrolled { branch: exp_slug, reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; @@ -2923,6 +3149,8 @@ fn test_evolver_map_features_by_feature_id_merges_rollouts() -> Result<()> { status: EnrollmentStatus::Enrolled { branch: ro_slug, reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; let enrollments = &[ro_enrollment, exp_enrollment]; @@ -2943,8 +3171,14 @@ fn test_rollouts_end_to_end() -> Result<()> { let ids = no_coenrolling_features(); let mut evolver = enrollment_evolver(&mut th, &aru, &ids); - let (enrollments, _events) = - evolver.evolve_enrollments::(Participation::default(), &[], recipes, &[])?; + let (enrollments, _events) = evolver.evolve_enrollments::( + Participation::default(), + &[], + recipes, + &[], + #[cfg(feature = "stateful")] + &None, + )?; let features = map_features_by_feature_id(&enrollments, recipes, &no_coenrolling_features()); @@ -2962,6 +3196,8 @@ fn test_enrollment_explicit_opt_in() -> Result<()> { enrollment.status, EnrollmentStatus::Enrolled { reason: EnrolledReason::OptIn, + #[cfg(feature = "stateful")] + previous_state: None, .. } )); @@ -2990,9 +3226,15 @@ fn test_enrollment_enrolled_explicit_opt_out() { status: EnrollmentStatus::Enrolled { branch: "control".to_owned(), reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, }, }; - let enrollment = existing_enrollment.on_explicit_opt_out(&mut events); + let enrollment = existing_enrollment.on_explicit_opt_out( + &mut events, + #[cfg(feature = "stateful")] + &None, + ); if let EnrollmentStatus::Disqualified { branch, .. } = enrollment.status { assert_eq!(branch, "control"); } else { @@ -3015,7 +3257,11 @@ fn test_enrollment_not_enrolled_explicit_opt_out() { reason: NotEnrolledReason::NotTargeted, }, }; - let enrollment = existing_enrollment.on_explicit_opt_out(&mut events); + let enrollment = existing_enrollment.on_explicit_opt_out( + &mut events, + #[cfg(feature = "stateful")] + &None, + ); assert!(matches!( enrollment.status, EnrollmentStatus::NotEnrolled { @@ -3037,7 +3283,11 @@ fn test_enrollment_disqualified_explicit_opt_out() { reason: DisqualifiedReason::NotTargeted, }, }; - let enrollment = existing_enrollment.on_explicit_opt_out(&mut events); + let enrollment = existing_enrollment.on_explicit_opt_out( + &mut events, + #[cfg(feature = "stateful")] + &None, + ); assert_eq!(enrollment, existing_enrollment); assert!(events.is_empty()); } @@ -3210,8 +3460,14 @@ fn test_evolve_enrollments_ordering() -> Result<()> { let all_experiments = [exp1, exp2]; let no_experiments: [Experiment; 0] = []; - let (enrollments, _) = - evolver.evolve_enrollment_recipes(true, &no_experiments, &all_experiments, &[])?; + let (enrollments, _) = evolver.evolve_enrollment_recipes( + true, + &no_experiments, + &all_experiments, + &[], + #[cfg(feature = "stateful")] + &None, + )?; let observed = map_features_by_feature_id(&enrollments, &all_experiments, &ids); let expected = HashMap::from([( @@ -3227,3 +3483,261 @@ fn test_evolve_enrollments_ordering() -> Result<()> { Ok(()) } + +#[test] +#[cfg(feature = "stateful")] +fn test_on_add_state() { + let exp = get_test_experiments()[0].clone(); + let existing_enrollment = ExperimentEnrollment { + slug: exp.slug, + status: EnrollmentStatus::Enrolled { + branch: "control".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, + }, + }; + let original_previous_state: PreviousState = PreviousState::GeckoPref(PreviousGeckoPrefState { + original_values: vec![OriginalGeckoPref { + pref: "test.some.pref".to_string(), + branch: PrefBranch::Default, + value: Some(serde_json::Value::String(String::from("some-gecko-value"))), + }], + feature_id: "feature-id".to_string(), + variable: "variable".to_string(), + }); + + let enrollment = existing_enrollment.on_add_state(original_previous_state); + + if let EnrollmentStatus::Enrolled { + previous_state: + Some(PreviousState::GeckoPref(PreviousGeckoPrefState { + original_values, + feature_id, + variable, + })), + .. + } = &enrollment.status + { + assert_eq!("feature-id", feature_id); + assert_eq!("variable", variable); + assert_eq!(1, original_values.len()); + assert_eq!("test.some.pref", original_values[0].pref); + assert_eq!( + "some-gecko-value", + original_values[0].value.clone().unwrap() + ); + assert_eq!(PrefBranch::Default, original_values[0].branch); + } else { + panic!("Wrong variant!"); + } +} + +#[cfg(feature = "stateful")] +#[test] +fn test_on_revert_to_previous_state_with_gecko_prefs() { + let exp = get_test_experiments()[0].clone(); + let existing_enrollment = ExperimentEnrollment { + slug: exp.slug, + status: EnrollmentStatus::Enrolled { + branch: "control".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, + }, + }; + let pref_state_1 = GeckoPrefState::new("test.some.pref.1", Some(PrefBranch::Default)) + .with_gecko_value(serde_json::Value::String(String::from( + "some-gecko-value-1", + ))); + let original_previous_state: PreviousState = PreviousState::GeckoPref(PreviousGeckoPrefState { + original_values: vec![OriginalGeckoPref::from(&pref_state_1)], + feature_id: "feature-id".to_string(), + variable: "variable".to_string(), + }); + let enrollment = existing_enrollment.on_add_state(original_previous_state); + let handler = TestGeckoPrefHandler::new(create_feature_prop_pref_map(vec![( + "feature-id", + "test_prop", + pref_state_1.clone(), + )])); + let handler: Arc> = Arc::new(Box::new(handler)); + let store = Arc::new(GeckoPrefStore::new(handler.clone())); + let _ = store.initialize(); + let gecko_pref_store = Some(store); + + if let EnrollmentStatus::Enrolled { + previous_state: Some(previous_state), + .. + } = &enrollment.status + { + previous_state.on_revert_to_previous_state(&gecko_pref_store); + let test_handler = unsafe { + std::mem::transmute::>, Arc>>( + handler, + ) + }; + let test_handler_state = test_handler + .state + .lock() + .expect("Unable to lock transmuted handler state"); + let original_prefs_stored = test_handler_state.original_prefs_state.clone().unwrap(); + + let PreviousState::GeckoPref(gecko_pref_state) = previous_state; + assert_eq!(1, original_prefs_stored.len()); + assert_eq!(gecko_pref_state.original_values, original_prefs_stored); + } +} + +#[cfg(feature = "stateful")] +#[test] +fn test_will_pref_experiment_change_no_change() { + let original_experiment = get_test_experiments()[0].clone(); + let pref_state_1 = GeckoPrefState::new("test.some.pref.1", Some(PrefBranch::Default)) + .with_gecko_value(serde_json::Value::String(String::from( + "some-gecko-value-1", + ))); + let original_previous_state: PreviousState = PreviousState::GeckoPref(PreviousGeckoPrefState { + original_values: vec![OriginalGeckoPref::from(&pref_state_1)], + feature_id: "some_control".to_string(), + variable: "variable".to_string(), + }); + let original_enrollment = ExperimentEnrollment { + slug: original_experiment.slug.clone(), + status: EnrollmentStatus::Enrolled { + branch: "control".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: Some(original_previous_state), + }, + }; + + let updated_experiment = get_test_experiments()[0].clone(); + let updated_enrollment = ExperimentEnrollment { + slug: original_experiment.slug.clone(), + status: EnrollmentStatus::Enrolled { + branch: "control".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, + }, + }; + + assert!( + !original_enrollment.will_pref_experiment_change(&updated_experiment, &updated_enrollment) + ); +} + +#[cfg(feature = "stateful")] +#[test] +fn test_will_pref_experiment_change_branch_changed() { + let original_experiment = get_test_experiments()[0].clone(); + let pref_state_1 = GeckoPrefState::new("test.some.pref.1", Some(PrefBranch::Default)) + .with_gecko_value(serde_json::Value::String(String::from( + "some-gecko-value-1", + ))); + let original_previous_state: PreviousState = PreviousState::GeckoPref(PreviousGeckoPrefState { + original_values: vec![OriginalGeckoPref::from(&pref_state_1)], + feature_id: "some_control".to_string(), + variable: "variable".to_string(), + }); + let original_enrollment = ExperimentEnrollment { + slug: original_experiment.slug.clone(), + status: EnrollmentStatus::Enrolled { + branch: "control".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: Some(original_previous_state), + }, + }; + + let updated_experiment = get_test_experiments()[0].clone(); + let updated_enrollment = ExperimentEnrollment { + slug: original_experiment.slug.clone(), + status: EnrollmentStatus::Enrolled { + branch: "changed_branch".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, + }, + }; + + assert!( + original_enrollment.will_pref_experiment_change(&updated_experiment, &updated_enrollment) + ); +} + +#[cfg(feature = "stateful")] +#[test] +fn test_will_pref_experiment_change_feature_id_changed() { + let original_experiment = get_test_experiments()[0].clone(); + let pref_state_1 = GeckoPrefState::new("test.some.pref.1", Some(PrefBranch::Default)) + .with_gecko_value(serde_json::Value::String(String::from( + "some-gecko-value-1", + ))); + let original_previous_state: PreviousState = PreviousState::GeckoPref(PreviousGeckoPrefState { + original_values: vec![OriginalGeckoPref::from(&pref_state_1)], + feature_id: "changed_feature_id".to_string(), + variable: "variable".to_string(), + }); + let original_enrollment = ExperimentEnrollment { + slug: original_experiment.slug.clone(), + status: EnrollmentStatus::Enrolled { + branch: "control".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: Some(original_previous_state), + }, + }; + + let updated_experiment = get_test_experiments()[0].clone(); + let updated_enrollment = ExperimentEnrollment { + slug: original_experiment.slug.clone(), + status: EnrollmentStatus::Enrolled { + branch: "control".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: None, + }, + }; + + assert!( + original_enrollment.will_pref_experiment_change(&updated_experiment, &updated_enrollment) + ); +} + +#[cfg(feature = "stateful")] +#[test] +fn test_will_pref_experiment_change_not_enrolled() { + let original_experiment = get_test_experiments()[0].clone(); + let pref_state_1 = GeckoPrefState::new("test.some.pref.1", Some(PrefBranch::Default)) + .with_gecko_value(serde_json::Value::String(String::from( + "some-gecko-value-1", + ))); + let original_previous_state: PreviousState = PreviousState::GeckoPref(PreviousGeckoPrefState { + original_values: vec![OriginalGeckoPref::from(&pref_state_1)], + feature_id: "some_control".to_string(), + variable: "variable".to_string(), + }); + let original_enrollment = ExperimentEnrollment { + slug: original_experiment.slug.clone(), + status: EnrollmentStatus::Enrolled { + branch: "control".to_owned(), + reason: EnrolledReason::Qualified, + #[cfg(feature = "stateful")] + previous_state: Some(original_previous_state), + }, + }; + + let updated_experiment = get_test_experiments()[0].clone(); + let updated_enrollment = ExperimentEnrollment { + slug: original_experiment.slug.clone(), + status: EnrollmentStatus::Error { + reason: "Something went wrong".to_string(), + }, + }; + + assert!( + original_enrollment.will_pref_experiment_change(&updated_experiment, &updated_enrollment) + ); +} diff --git a/components/nimbus/src/tests/test_enrollment_bw_compat.rs b/components/nimbus/src/tests/test_enrollment_bw_compat.rs index e8cf2138e7..758a8a976d 100644 --- a/components/nimbus/src/tests/test_enrollment_bw_compat.rs +++ b/components/nimbus/src/tests/test_enrollment_bw_compat.rs @@ -69,3 +69,47 @@ fn test_not_enrolled_reason_schema_with_feature_conflict() { matches!(non_enrollment.status, EnrollmentStatus::NotEnrolled{ ref reason, ..} if reason == &NotEnrolledReason::FeatureConflict) ); } + +// In bug 1997373, we added a `previous_state` field to the EnrollmentStatus schema. +// This test check tht the data deserializes correctly both with and without the new field. +#[cfg(feature = "stateful")] +#[test] +fn test_experiment_schema_with_previous_state() { + // ⚠️ Warning : Do not change the JSON data used by this test. ⚠️ + let previous_state_empty: EnrollmentStatus = serde_json::from_value(json!({ + "Enrolled": { + "reason": "Qualified", + "branch": "some_branch", + } + })) + .unwrap(); + assert!( + matches!(previous_state_empty, EnrollmentStatus::Enrolled {ref previous_state, ..} if previous_state.is_none()) + ); + + let previous_state_exists: EnrollmentStatus = serde_json::from_value(json!({ + "Enrolled": { + "reason": "Qualified", + "branch": "some_branch", + "previous_state": { + "GeckoPref": { + "original_values": [{ + "pref": "some_pref", + "branch": "default", + "value": 5 + }], + "feature_id": "some_control", + "variable": "some_variable" + } + } + } + })) + .unwrap(); + assert!(matches!( + previous_state_exists, + EnrollmentStatus::Enrolled { + previous_state: Some(PreviousState::GeckoPref(PreviousGeckoPrefState { ref original_values, .. })), + .. + } if original_values[0].pref == "some_pref" && original_values[0].value.clone().unwrap() == 5 + )); +}