Skip to content

Commit 231ae50

Browse files
committed
Bug 2003370 - Use and react to Gecko preferences stored values
This patch adds functionality to use the new GeckoPref's PreviousState. It adds: * `set_gecko_prefs_original_values` for external handling of setting prefs back to original values * Mechanisms to return to a previous states when: * `on_experiment_updated` * Certain situations and as determined in `will_pref_experiment_change` * `on_experiment_ended` * `on_opt_out`
1 parent 111c827 commit 231ae50

File tree

10 files changed

+533
-46
lines changed

10 files changed

+533
-46
lines changed

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import org.mozilla.experiments.nimbus.internal.GeckoPrefHandler
4343
import org.mozilla.experiments.nimbus.internal.GeckoPrefState
4444
import org.mozilla.experiments.nimbus.internal.JsonObject
4545
import org.mozilla.experiments.nimbus.internal.NimbusException
46+
import org.mozilla.experiments.nimbus.internal.OriginalGeckoPref
4647
import org.mozilla.experiments.nimbus.internal.PrefBranch
4748
import org.mozilla.experiments.nimbus.internal.PrefEnrollmentData
4849
import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason
@@ -860,6 +861,7 @@ class NimbusTests {
860861
),
861862
),
862863
var setValues: List<GeckoPrefState>? = null,
864+
var originalGeckoPrefs: List<OriginalGeckoPref>? = null,
863865
) : GeckoPrefHandler {
864866
override fun getPrefsWithState(): Map<String, Map<String, GeckoPrefState>> {
865867
return internalMap
@@ -868,6 +870,10 @@ class NimbusTests {
868870
override fun setGeckoPrefsState(newPrefsState: List<GeckoPrefState>) {
869871
setValues = newPrefsState
870872
}
873+
874+
override fun setGeckoPrefsOriginalValues(originalGeckoPrefs: List<OriginalGeckoPref>) {
875+
originalGeckoPrefs = originalGeckoPrefs
876+
}
871877
}
872878

873879
@Test
@@ -889,6 +895,21 @@ class NimbusTests {
889895
assertEquals("42", handler.setValues?.get(0)?.enrollmentValue?.prefValue)
890896
}
891897

898+
@Test
899+
fun `GeckoPrefHandler setGeckoPrefsOriginalValues function`() {
900+
val handler = TestGeckoPrefHandler()
901+
val originalValues = listOf(
902+
OriginalGeckoPref(
903+
pref = "pref.number",
904+
branch = PrefBranch.DEFAULT,
905+
value = "1",
906+
),
907+
)
908+
handler.setGeckoPrefsOriginalValues(originalValues)
909+
assertEquals(1, handler.originalGeckoPrefs?.size)
910+
assertEquals("pref.number", handler.originalGeckoPrefs?.get(0)?.pref)
911+
}
912+
892913
@Test
893914
fun `unenroll for gecko pref functions`() {
894915
val handler = TestGeckoPrefHandler()

components/nimbus/src/enrollment.rs

Lines changed: 115 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// License, v. 2.0. If a copy of the MPL was not distributed with this
33
// file, You can obtain one at https://mozilla.org/MPL/2.0/.
44
#[cfg(feature = "stateful")]
5-
use crate::stateful::gecko_prefs::{OriginalGeckoPref, PrefUnenrollReason};
5+
use crate::stateful::gecko_prefs::{GeckoPrefStore, OriginalGeckoPref, PrefUnenrollReason};
66
use crate::{
77
defaults::Defaults,
88
error::{debug, warn, NimbusError, Result},
@@ -11,6 +11,8 @@ use crate::{
1111
SLUG_REPLACEMENT_PATTERN,
1212
};
1313
use serde_derive::*;
14+
#[cfg(feature = "stateful")]
15+
use std::sync::Arc;
1416
use std::{
1517
collections::{HashMap, HashSet},
1618
fmt::{Display, Formatter, Result as FmtResult},
@@ -151,6 +153,24 @@ pub enum PreviousState {
151153
GeckoPref(PreviousGeckoPrefState),
152154
}
153155

156+
#[cfg(feature = "stateful")]
157+
impl PreviousState {
158+
#[cfg(feature = "stateful")]
159+
pub(crate) fn on_revert_to_previous_state(
160+
&self,
161+
gecko_pref_store: &Option<Arc<GeckoPrefStore>>,
162+
) {
163+
match self {
164+
PreviousState::GeckoPref(previous_gecko_pref_state) => {
165+
if let Some(store) = gecko_pref_store {
166+
store.handler.set_gecko_prefs_original_values(
167+
previous_gecko_pref_state.original_values.clone(),
168+
)
169+
}
170+
}
171+
}
172+
}
173+
}
154174
// Every experiment has an ExperimentEnrollment, even when we aren't enrolled.
155175
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
156176
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
@@ -238,6 +258,7 @@ impl ExperimentEnrollment {
238258
available_randomization_units: &AvailableRandomizationUnits,
239259
updated_experiment: &Experiment,
240260
targeting_helper: &NimbusTargetingHelper,
261+
gecko_pref_store: &Option<Arc<GeckoPrefStore>>,
241262
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
242263
) -> Result<Self> {
243264
Ok(match &self.status {
@@ -271,6 +292,13 @@ impl ExperimentEnrollment {
271292
"Existing experiment enrollment '{}' is now disqualified (global opt-out)",
272293
&self.slug
273294
);
295+
if let EnrollmentStatus::Enrolled {
296+
previous_state: Some(previous_state),
297+
..
298+
} = &self.status
299+
{
300+
previous_state.on_revert_to_previous_state(gecko_pref_store);
301+
}
274302
let updated_enrollment =
275303
self.disqualify_from_enrolled(DisqualifiedReason::OptOut);
276304
out_enrollment_events.push(updated_enrollment.get_change_event());
@@ -291,6 +319,16 @@ impl ExperimentEnrollment {
291319
updated_experiment,
292320
targeting_helper,
293321
)?;
322+
323+
if self.will_pref_experiment_change(updated_experiment, &evaluated_enrollment) {
324+
if let EnrollmentStatus::Enrolled {
325+
previous_state: Some(previous_state),
326+
..
327+
} = &self.status
328+
{
329+
previous_state.on_revert_to_previous_state(gecko_pref_store);
330+
}
331+
}
294332
match evaluated_enrollment.status {
295333
EnrollmentStatus::Error { .. } => {
296334
let updated_enrollment =
@@ -375,6 +413,7 @@ impl ExperimentEnrollment {
375413
/// from the database after `PREVIOUS_ENROLLMENTS_GC_TIME`.
376414
fn on_experiment_ended(
377415
&self,
416+
gecko_prefs: &Option<Arc<GeckoPrefStore>>,
378417
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
379418
) -> Option<Self> {
380419
debug!(
@@ -388,6 +427,15 @@ impl ExperimentEnrollment {
388427
| EnrollmentStatus::WasEnrolled { .. }
389428
| EnrollmentStatus::Error { .. } => return None, // We were never enrolled anyway, simply delete the enrollment record from the DB.
390429
};
430+
431+
if let EnrollmentStatus::Enrolled {
432+
previous_state: Some(previous_state),
433+
..
434+
} = &self.status
435+
{
436+
previous_state.on_revert_to_previous_state(gecko_prefs);
437+
}
438+
391439
let enrollment = Self {
392440
slug: self.slug.clone(),
393441
status: EnrollmentStatus::WasEnrolled {
@@ -405,9 +453,13 @@ impl ExperimentEnrollment {
405453
pub(crate) fn on_explicit_opt_out(
406454
&self,
407455
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>,
456+
gecko_prefs: &Option<Arc<GeckoPrefStore>>,
408457
) -> ExperimentEnrollment {
409-
match self.status {
410-
EnrollmentStatus::Enrolled { .. } => {
458+
match &self.status {
459+
EnrollmentStatus::Enrolled { previous_state, .. } => {
460+
if let Some(previous_state) = previous_state {
461+
previous_state.on_revert_to_previous_state(gecko_prefs);
462+
}
411463
let enrollment = self.disqualify_from_enrolled(DisqualifiedReason::OptOut);
412464
out_enrollment_events.push(enrollment.get_change_event());
413465
enrollment
@@ -558,6 +610,57 @@ impl ExperimentEnrollment {
558610
| EnrollmentStatus::Error { .. } => self.clone(),
559611
}
560612
}
613+
614+
#[cfg(feature = "stateful")]
615+
pub(crate) fn will_pref_experiment_change(
616+
&self,
617+
updated_experiment: &Experiment,
618+
updated_enrollment: &ExperimentEnrollment,
619+
) -> bool {
620+
let (original_feature_id, original_branch_slug) = match &self.status {
621+
EnrollmentStatus::Enrolled {
622+
previous_state: Some(PreviousState::GeckoPref(previous_state)),
623+
branch,
624+
..
625+
} => (&previous_state.feature_id, branch),
626+
// Can't change if it isn't a pref experiment
627+
_ => {
628+
return false;
629+
}
630+
};
631+
632+
let updated_branch_slug = match &updated_enrollment.status {
633+
EnrollmentStatus::Enrolled { branch, .. } => branch,
634+
// If we are no longer going to be enrolled, then a change happened
635+
_ => {
636+
return true;
637+
}
638+
};
639+
640+
// Branch changed
641+
if updated_branch_slug != original_branch_slug {
642+
return true;
643+
}
644+
645+
let Some(branch) = updated_experiment.get_branch(updated_branch_slug) else {
646+
return true;
647+
};
648+
649+
// Feature changed
650+
match &branch.feature {
651+
Some(updated_feature) => {
652+
if updated_feature.feature_id != *original_feature_id {
653+
return true;
654+
}
655+
}
656+
None => {
657+
return true;
658+
}
659+
};
660+
661+
// ToDo: In review ask about checking variable - unsure of what that corresponds to.
662+
false
663+
}
561664
}
562665

563666
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
@@ -653,6 +756,7 @@ impl<'a> EnrollmentsEvolver<'a> {
653756
prev_experiments: &[E],
654757
next_experiments: &[Experiment],
655758
prev_enrollments: &[ExperimentEnrollment],
759+
gecko_pref_store: &Option<Arc<GeckoPrefStore>>,
656760
) -> Result<(Vec<ExperimentEnrollment>, Vec<EnrollmentChangeEvent>)>
657761
where
658762
E: ExperimentMetadata + Clone,
@@ -674,6 +778,7 @@ impl<'a> EnrollmentsEvolver<'a> {
674778
&prev_rollouts,
675779
&next_rollouts,
676780
&ro_enrollments,
781+
gecko_pref_store,
677782
)?;
678783

679784
enrollments.extend(next_ro_enrollments);
@@ -698,6 +803,7 @@ impl<'a> EnrollmentsEvolver<'a> {
698803
&prev_experiments,
699804
&next_experiments,
700805
&prev_enrollments,
806+
gecko_pref_store,
701807
)?;
702808

703809
enrollments.extend(next_exp_enrollments);
@@ -714,6 +820,7 @@ impl<'a> EnrollmentsEvolver<'a> {
714820
prev_experiments: &[E],
715821
next_experiments: &[Experiment],
716822
prev_enrollments: &[ExperimentEnrollment],
823+
gecko_pref_store: &Option<Arc<GeckoPrefStore>>,
717824
) -> Result<(Vec<ExperimentEnrollment>, Vec<EnrollmentChangeEvent>)>
718825
where
719826
E: ExperimentMetadata + Clone,
@@ -753,6 +860,7 @@ impl<'a> EnrollmentsEvolver<'a> {
753860
next_experiments_map.get(slug).copied(),
754861
Some(prev_enrollment),
755862
&mut enrollment_events,
863+
gecko_pref_store,
756864
) {
757865
Ok(enrollment) => enrollment,
758866
Err(e) => {
@@ -854,6 +962,7 @@ impl<'a> EnrollmentsEvolver<'a> {
854962
Some(next_experiment),
855963
prev_enrollment,
856964
&mut enrollment_events,
965+
gecko_pref_store,
857966
) {
858967
Ok(enrollment) => enrollment,
859968
Err(e) => {
@@ -947,6 +1056,7 @@ impl<'a> EnrollmentsEvolver<'a> {
9471056
next_experiment: Option<&Experiment>,
9481057
prev_enrollment: Option<&ExperimentEnrollment>,
9491058
out_enrollment_events: &mut Vec<EnrollmentChangeEvent>, // out param containing the events we'd like to emit to glean.
1059+
gecko_pref_store: &Option<Arc<GeckoPrefStore>>,
9501060
) -> Result<Option<ExperimentEnrollment>>
9511061
where
9521062
E: ExperimentMetadata + Clone,
@@ -976,7 +1086,7 @@ impl<'a> EnrollmentsEvolver<'a> {
9761086
)?),
9771087
// Experiment deleted remotely.
9781088
(Some(_), None, Some(enrollment)) => {
979-
enrollment.on_experiment_ended(out_enrollment_events)
1089+
enrollment.on_experiment_ended(gecko_pref_store, out_enrollment_events)
9801090
}
9811091
// Known experiment.
9821092
(Some(_), Some(experiment), Some(enrollment)) => {
@@ -985,6 +1095,7 @@ impl<'a> EnrollmentsEvolver<'a> {
9851095
self.available_randomization_units,
9861096
experiment,
9871097
&targeting_helper,
1098+
gecko_pref_store,
9881099
out_enrollment_events,
9891100
)?)
9901101
}

components/nimbus/src/nimbus.udl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ callback interface GeckoPrefHandler {
130130
record<string, record<string, GeckoPrefState>> get_prefs_with_state();
131131

132132
void set_gecko_prefs_state(sequence<GeckoPrefState> new_prefs_state);
133+
134+
void set_gecko_prefs_original_values(sequence<OriginalGeckoPref> original_gecko_prefs);
135+
133136
};
134137

135138
dictionary GeckoPref {

components/nimbus/src/stateful/enrollment.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
use std::sync::Arc;
2+
13
/* This Source Code Form is subject to the terms of the Mozilla Public
24
* License, v. 2.0. If a copy of the MPL was not distributed with this
35
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */
46
use crate::enrollment::Participation;
7+
use crate::stateful::gecko_prefs::GeckoPrefStore;
58
use crate::stateful::persistence::{
69
DB_KEY_EXPERIMENT_PARTICIPATION, DB_KEY_ROLLOUT_PARTICIPATION,
710
DEFAULT_EXPERIMENT_PARTICIPATION, DEFAULT_ROLLOUT_PARTICIPATION,
@@ -27,6 +30,7 @@ impl EnrollmentsEvolver<'_> {
2730
db: &Database,
2831
writer: &mut Writer,
2932
next_experiments: &[Experiment],
33+
gecko_pref_store: &Option<Arc<GeckoPrefStore>>,
3034
) -> Result<Vec<EnrollmentChangeEvent>> {
3135
// Get separate participation states from the db
3236
let is_participating_in_experiments = get_experiment_participation(db, writer)?;
@@ -47,6 +51,7 @@ impl EnrollmentsEvolver<'_> {
4751
&prev_experiments,
4852
next_experiments,
4953
&prev_enrollments,
54+
gecko_pref_store,
5055
)?;
5156
let next_enrollments = map_enrollments(&next_enrollments);
5257
// Write the changes to the Database.
@@ -134,13 +139,14 @@ pub fn opt_out(
134139
db: &Database,
135140
writer: &mut Writer,
136141
experiment_slug: &str,
142+
gecko_prefs: &Option<Arc<GeckoPrefStore>>,
137143
) -> Result<Vec<EnrollmentChangeEvent>> {
138144
let mut events = vec![];
139145
let enr_store = db.get_store(StoreId::Enrollments);
140146
if let Ok(Some(existing_enrollment)) =
141147
enr_store.get::<ExperimentEnrollment, Writer>(writer, experiment_slug)
142148
{
143-
let updated_enrollment = &existing_enrollment.on_explicit_opt_out(&mut events);
149+
let updated_enrollment = &existing_enrollment.on_explicit_opt_out(&mut events, gecko_prefs);
144150
enr_store.put(writer, experiment_slug, updated_enrollment)?;
145151
} else {
146152
events.push(EnrollmentChangeEvent {

components/nimbus/src/stateful/gecko_prefs.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ pub trait GeckoPrefHandler: Send + Sync {
131131

132132
/// Used to set the state for each pref based on enrollments
133133
fn set_gecko_prefs_state(&self, new_prefs_state: Vec<GeckoPrefState>);
134+
135+
/// Used to set back to the original state for each pref based on the original gecko value
136+
fn set_gecko_prefs_original_values(&self, original_gecko_prefs: Vec<OriginalGeckoPref>);
134137
}
135138

136139
#[derive(Default)]

components/nimbus/src/stateful/nimbus_client.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ impl NimbusClient {
347347
pub fn opt_out(&self, experiment_slug: String) -> Result<Vec<EnrollmentChangeEvent>> {
348348
let db = self.db()?;
349349
let mut writer = db.write()?;
350-
let result = opt_out(db, &mut writer, &experiment_slug)?;
350+
let result = opt_out(db, &mut writer, &experiment_slug, &self.gecko_prefs)?;
351351
let mut state = self.mutable_state.lock().unwrap();
352352
self.end_initialize(db, writer, &mut state)?;
353353
Ok(result)
@@ -456,7 +456,7 @@ impl NimbusClient {
456456
&mut targeting_helper,
457457
&coenrolling_feature_ids,
458458
);
459-
evolver.evolve_enrollments_in_db(db, writer, experiments)
459+
evolver.evolve_enrollments_in_db(db, writer, experiments, &self.gecko_prefs)
460460
}
461461

462462
pub fn apply_pending_experiments(&self) -> Result<Vec<EnrollmentChangeEvent>> {

0 commit comments

Comments
 (0)