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..9cd180c670 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 @@ -43,11 +44,15 @@ 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.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,7 +854,7 @@ 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, ), ), @@ -911,6 +916,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..ec0c8f0702 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::{OriginalGeckoPref, PrefUnenrollReason}; use crate::{ defaults::Defaults, error::{debug, warn, NimbusError, Result}, @@ -21,7 +21,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 +45,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 +99,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 +134,29 @@ 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), +} + +// 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, @@ -430,6 +449,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 @@ -531,12 +564,15 @@ impl ExperimentEnrollment { } // ⚠️ 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 +613,8 @@ impl EnrollmentStatus { EnrollmentStatus::Enrolled { reason, branch: branch.to_owned(), + #[cfg(feature = "stateful")] + previous_state: None, } } 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..9f5d402879 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 { @@ -139,12 +149,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 +380,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/gecko_prefs.rs b/components/nimbus/src/stateful/gecko_prefs.rs index 9a20cff338..9eb80cda2b 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>; @@ -279,6 +300,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 +357,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..6086b18070 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; @@ -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..b441fe6bc0 100644 --- a/components/nimbus/src/tests/helpers.rs +++ b/components/nimbus/src/tests/helpers.rs @@ -440,6 +440,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_gecko_prefs.rs b/components/nimbus/src/tests/stateful/test_gecko_prefs.rs index 2b138a5f61..499bf7879c 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, PrefBranch, PrefEnrollmentData, }, tests::helpers::{get_multi_feature_experiment, TestGeckoPrefHandler}, EnrolledExperiment, @@ -219,3 +219,122 @@ 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(()) +} 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..b956070b3d 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, @@ -670,6 +671,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 @@ -713,6 +716,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 @@ -728,6 +733,8 @@ fn test_evolver_experiment_update_enrolled_then_experiment_paused() -> Result<() if let EnrollmentStatus::Enrolled { reason: EnrolledReason::Qualified, branch, + #[cfg(feature = "stateful")] + previous_state: None, .. } = enrollment.status { @@ -753,6 +760,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 @@ -799,6 +808,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 @@ -1043,6 +1054,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 @@ -1078,6 +1091,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 @@ -2276,6 +2291,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, }, }]; @@ -2435,6 +2452,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 @@ -2915,6 +2934,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 +2944,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]; @@ -2962,6 +2985,8 @@ fn test_enrollment_explicit_opt_in() -> Result<()> { enrollment.status, EnrollmentStatus::Enrolled { reason: EnrolledReason::OptIn, + #[cfg(feature = "stateful")] + previous_state: None, .. } )); @@ -2990,6 +3015,8 @@ 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); @@ -3227,3 +3254,52 @@ 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!"); + } +} 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 + )); +}