Skip to content

Commit fb0f02b

Browse files
committed
Bug 1997373 - Store original pref values for Fenix Gecko integration
This patch begins storing a `PreviousState` on `ExperimentEnrollment` when it is of type `EnrollmentStatus::Enrolled`. It also introduces `PreviousState::GeckoPref` to hold Gecko original preference values for Gecko pref based experiments. The public APIs it opens are `registerPreviousGeckoPrefStates` and `getPreviousState`.
1 parent caf03ed commit fb0f02b

File tree

15 files changed

+878
-22
lines changed

15 files changed

+878
-22
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
# v148.0 (In progress)
2+
### Nimbus
3+
* 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.
24

35
[Full Changelog](In progress)
46

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import org.mozilla.experiments.nimbus.internal.NimbusClient
4444
import org.mozilla.experiments.nimbus.internal.NimbusClientInterface
4545
import org.mozilla.experiments.nimbus.internal.NimbusException
4646
import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason
47+
import org.mozilla.experiments.nimbus.internal.PreviousState
4748
import org.mozilla.experiments.nimbus.internal.RecordedContext
4849
import java.io.File
4950
import java.io.IOException
@@ -438,6 +439,18 @@ open class Nimbus(
438439
return nimbusClient.unenrollForGeckoPref(geckoPrefState, prefUnenrollReason)
439440
}
440441

442+
override fun registerPreviousGeckoPrefStates(geckoPrefStates: List<GeckoPrefState>) {
443+
dbScope.launch {
444+
withCatchAll("registerPreviousGeckoPrefStates") {
445+
nimbusClient.registerPreviousGeckoPrefStates(geckoPrefStates)
446+
}
447+
}
448+
}
449+
450+
override fun getPreviousState(experimentSlug: String): PreviousState? {
451+
return nimbusClient.getPreviousState(experimentSlug)
452+
}
453+
441454
@WorkerThread
442455
@VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
443456
internal fun optOutOnThisThread(experimentId: String) = withCatchAll("optOut") {

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import org.mozilla.experiments.nimbus.internal.EnrollmentChangeEvent
1717
import org.mozilla.experiments.nimbus.internal.ExperimentBranch
1818
import org.mozilla.experiments.nimbus.internal.GeckoPrefState
1919
import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason
20+
import org.mozilla.experiments.nimbus.internal.PreviousState
2021
import java.time.Duration
2122
import java.util.concurrent.TimeUnit
2223

@@ -191,6 +192,26 @@ interface NimbusInterface : FeaturesInterface, NimbusMessagingInterface, NimbusE
191192
prefUnenrollReason: PrefUnenrollReason,
192193
): List<EnrollmentChangeEvent> = listOf()
193194

195+
/**
196+
* Add the original Gecko pref values as a previous state on each involved enrollment.
197+
*
198+
* @param geckoPrefStates The list of items that should have their enrollment state updated with
199+
* original Gecko pref previous state information.
200+
*/
201+
fun registerPreviousGeckoPrefStates(
202+
geckoPrefStates: List<GeckoPrefState>,
203+
) = Unit
204+
205+
/**
206+
* Retrieves a previous state, if available on an enrolled experiment, from a given slug.
207+
*
208+
* @param experimentSlug The slug of the experiment.
209+
* @return The previous state of the given slug. Will return null if not available or invalid slug.
210+
*/
211+
fun getPreviousState(
212+
experimentSlug: String,
213+
): PreviousState? = null
214+
194215
/**
195216
* Reset internal state in response to application-level telemetry reset.
196217
* Consumers should call this method when the user resets the telemetry state of the

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

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package org.mozilla.experiments.nimbus
66

77
import android.content.Context
8+
import android.os.Looper
89
import android.util.Log
910
import androidx.test.core.app.ApplicationProvider
1011
import kotlinx.coroutines.CancellationException
@@ -43,11 +44,15 @@ import org.mozilla.experiments.nimbus.internal.GeckoPrefState
4344
import org.mozilla.experiments.nimbus.internal.JsonObject
4445
import org.mozilla.experiments.nimbus.internal.NimbusException
4546
import org.mozilla.experiments.nimbus.internal.PrefBranch
47+
import org.mozilla.experiments.nimbus.internal.PrefEnrollmentData
4648
import org.mozilla.experiments.nimbus.internal.PrefUnenrollReason
49+
import org.mozilla.experiments.nimbus.internal.PreviousGeckoPrefState
50+
import org.mozilla.experiments.nimbus.internal.PreviousState
4751
import org.mozilla.experiments.nimbus.internal.RecordedContext
4852
import org.mozilla.experiments.nimbus.internal.getCalculatedAttributes
4953
import org.mozilla.experiments.nimbus.internal.validateEventQueries
5054
import org.robolectric.RobolectricTestRunner
55+
import org.robolectric.Shadows.shadowOf
5156
import java.io.File
5257
import java.util.Calendar
5358
import java.util.concurrent.Executors
@@ -849,7 +854,7 @@ class NimbusTests {
849854
"number" to GeckoPrefState(
850855
geckoPref = GeckoPref("pref.number", PrefBranch.DEFAULT),
851856
geckoValue = "1",
852-
enrollmentValue = null,
857+
enrollmentValue = PrefEnrollmentData("test-experiment", "42", "about_welcome", "number"),
853858
isUserSet = false,
854859
),
855860
),
@@ -911,6 +916,36 @@ class NimbusTests {
911916
assertEquals(EnrollmentChangeEventType.DISQUALIFICATION, events[0].change)
912917
assertEquals(0, handler.setValues?.size)
913918
}
919+
920+
@Test
921+
fun `register previous gecko states and check values`() {
922+
val handler = TestGeckoPrefHandler()
923+
924+
val nimbus = createNimbus(geckoPrefHandler = handler)
925+
926+
suspend fun getString(): String {
927+
return testExperimentsJsonString(appInfo, packageName)
928+
}
929+
930+
val job = nimbus.applyLocalExperiments(::getString)
931+
runBlocking {
932+
job.join()
933+
}
934+
935+
assertEquals(1, handler.setValues?.size)
936+
assertEquals("42", handler.setValues?.get(0)?.enrollmentValue?.prefValue)
937+
938+
nimbus.registerPreviousGeckoPrefStates(handler.setValues!!)
939+
shadowOf(Looper.getMainLooper()).idle()
940+
941+
val previousState = nimbus.getPreviousState("test-experiment")
942+
shadowOf(Looper.getMainLooper()).idle()
943+
944+
assertNotNull(previousState)
945+
val geckoPreviousState = previousState as PreviousState.GeckoPref
946+
assertEquals("1", geckoPreviousState!!.v1.originalValues[0].value)
947+
assertEquals("pref.number", geckoPreviousState!!.v1.originalValues[0].pref)
948+
}
914949
}
915950

916951
// Mocking utilities, from mozilla.components.support.test

components/nimbus/src/enrollment.rs

Lines changed: 42 additions & 7 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::PrefUnenrollReason;
5+
use crate::stateful::gecko_prefs::{OriginalGeckoPref, PrefUnenrollReason};
66
use crate::{
77
defaults::Defaults,
88
error::{debug, warn, NimbusError, Result},
@@ -21,7 +21,7 @@ pub(crate) const PREVIOUS_ENROLLMENTS_GC_TIME: Duration = Duration::from_secs(36
2121

2222
// These are types we use internally for managing enrollments.
2323
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
24-
// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️
24+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
2525
#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
2626
pub enum EnrolledReason {
2727
/// A normal enrollment as per the experiment's rules.
@@ -45,7 +45,7 @@ impl Display for EnrolledReason {
4545
// These are types we use internally for managing non-enrollments.
4646

4747
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
48-
// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️
48+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
4949
#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
5050
pub enum NotEnrolledReason {
5151
/// The experiment targets a different application.
@@ -99,7 +99,7 @@ impl Default for Participation {
9999
// These are types we use internally for managing disqualifications.
100100

101101
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
102-
// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️
102+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
103103
#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
104104
pub enum DisqualifiedReason {
105105
/// There was an error.
@@ -134,10 +134,26 @@ impl Display for DisqualifiedReason {
134134
}
135135
}
136136

137-
// Every experiment has an ExperimentEnrollment, even when we aren't enrolled.
137+
// The previous state of a feature before enrollment.
138+
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
139+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
140+
#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
141+
#[cfg(feature = "stateful")]
142+
pub struct PreviousGeckoPrefState {
143+
pub original_values: Vec<OriginalGeckoPref>,
144+
pub feature_id: String,
145+
pub variable: String,
146+
}
147+
148+
#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq, uniffi::Enum)]
149+
#[cfg(feature = "stateful")]
150+
pub enum PreviousState {
151+
GeckoPref(PreviousGeckoPrefState),
152+
}
138153

154+
// Every experiment has an ExperimentEnrollment, even when we aren't enrolled.
139155
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
140-
// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️
156+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
141157
#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
142158
pub struct ExperimentEnrollment {
143159
pub slug: String,
@@ -430,6 +446,20 @@ impl ExperimentEnrollment {
430446
}
431447
}
432448

449+
// Previous state is only settable on Enrolled experiments
450+
#[cfg(feature = "stateful")]
451+
pub(crate) fn on_add_state(&self, previous_state: PreviousState) -> ExperimentEnrollment {
452+
let mut next = self.clone();
453+
if let EnrollmentStatus::Enrolled { reason, branch, .. } = &self.status {
454+
next.status = EnrollmentStatus::Enrolled {
455+
previous_state: Some(previous_state),
456+
reason: reason.clone(),
457+
branch: branch.clone(),
458+
};
459+
}
460+
next
461+
}
462+
433463
/// Reset identifiers in response to application-level telemetry reset.
434464
///
435465
/// We move any enrolled experiments to the "disqualified" state, since their further
@@ -531,12 +561,15 @@ impl ExperimentEnrollment {
531561
}
532562

533563
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
534-
// ⚠️ in `mod test_schema_bw_compat` below, and may require a DB migration. ⚠️
564+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
535565
#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
536566
pub enum EnrollmentStatus {
537567
Enrolled {
538568
reason: EnrolledReason,
539569
branch: String,
570+
#[cfg(feature = "stateful")]
571+
#[serde(skip_serializing_if = "Option::is_none")]
572+
previous_state: Option<PreviousState>,
540573
},
541574
NotEnrolled {
542575
reason: NotEnrolledReason,
@@ -577,6 +610,8 @@ impl EnrollmentStatus {
577610
EnrollmentStatus::Enrolled {
578611
reason,
579612
branch: branch.to_owned(),
613+
#[cfg(feature = "stateful")]
614+
previous_state: None,
580615
}
581616
}
582617

components/nimbus/src/metrics.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
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

5+
#[cfg(feature = "stateful")]
6+
use crate::enrollment::PreviousState;
7+
58
use crate::{enrollment::ExperimentEnrollment, EnrolledFeature, EnrollmentStatus};
69
use serde_derive::{Deserialize, Serialize};
710

@@ -28,6 +31,9 @@ pub struct EnrollmentStatusExtraDef {
2831
pub status: Option<String>,
2932
#[cfg(not(feature = "stateful"))]
3033
pub user_id: Option<String>,
34+
#[cfg(feature = "stateful")]
35+
#[serde(skip_serializing_if = "Option::is_none")]
36+
pub previous_state: Option<PreviousState>,
3137
}
3238

3339
#[cfg(test)]
@@ -93,6 +99,8 @@ impl From<ExperimentEnrollment> for EnrollmentStatusExtraDef {
9399
status: Some(enrollment.status.name()),
94100
#[cfg(not(feature = "stateful"))]
95101
user_id: None,
102+
#[cfg(feature = "stateful")]
103+
previous_state: None,
96104
}
97105
}
98106
}

components/nimbus/src/nimbus.udl

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,23 @@ callback interface MetricsHandler {
9494
void record_malformed_feature_config(MalformedFeatureConfigExtraDef event);
9595
};
9696

97+
typedef enum PreviousState;
98+
9799
dictionary EnrollmentStatusExtraDef {
98100
string? branch;
99101
string? conflict_slug;
100102
string? error_string;
101103
string? reason;
102104
string? slug;
103105
string? status;
106+
PreviousState? previous_state;
107+
};
108+
109+
110+
dictionary PreviousGeckoPrefState {
111+
sequence<OriginalGeckoPref> original_values;
112+
string feature_id;
113+
string variable;
104114
};
105115

106116
dictionary FeatureExposureExtraDef {
@@ -139,12 +149,20 @@ enum PrefBranch {
139149
"User",
140150
};
141151

152+
dictionary OriginalGeckoPref {
153+
string pref;
154+
PrefBranch branch;
155+
PrefValue? value;
156+
};
157+
158+
142159
enum PrefUnenrollReason {
143160
"Changed",
144161
"FailedToSet",
145162
};
146163

147164
dictionary PrefEnrollmentData {
165+
string experiment_slug;
148166
PrefValue pref_value;
149167
string feature_id;
150168
string variable;
@@ -362,6 +380,11 @@ interface NimbusClient {
362380
[Throws=NimbusError]
363381
sequence<EnrollmentChangeEvent> unenroll_for_gecko_pref(GeckoPrefState pref_state, PrefUnenrollReason pref_unenroll_reason);
364382

383+
[Throws=NimbusError]
384+
void register_previous_gecko_pref_states([ByRef] sequence<GeckoPrefState> gecko_pref_states);
385+
386+
[Throws=NimbusError]
387+
PreviousState? get_previous_state(string experiment_slug);
365388
};
366389

367390
interface NimbusTargetingHelper {

components/nimbus/src/stateful/enrollment.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* This Source Code Form is subject to the terms of the Mozilla Public
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/. */
4-
use crate::enrollment::Participation;
4+
use crate::enrollment::{Participation, PreviousState};
55
use crate::stateful::persistence::{
66
DB_KEY_EXPERIMENT_PARTICIPATION, DB_KEY_ROLLOUT_PARTICIPATION,
77
DEFAULT_EXPERIMENT_PARTICIPATION, DEFAULT_ROLLOUT_PARTICIPATION,
@@ -180,6 +180,24 @@ pub fn unenroll_for_pref(
180180
Ok(events)
181181
}
182182

183+
#[inline]
184+
pub fn get_previous_state_for_experiment<'r>(
185+
db: &Database,
186+
reader: &'r impl Readable<'r>,
187+
experiment_slug: &str,
188+
) -> Result<Option<PreviousState>> {
189+
Ok(db
190+
.get_store(StoreId::Enrollments)
191+
.get::<ExperimentEnrollment, _>(reader, experiment_slug)?
192+
.and_then(|enrollment| {
193+
if let EnrollmentStatus::Enrolled { previous_state, .. } = enrollment.status {
194+
previous_state
195+
} else {
196+
None
197+
}
198+
}))
199+
}
200+
183201
pub fn get_experiment_participation<'r>(
184202
db: &Database,
185203
reader: &'r impl Readable<'r>,

0 commit comments

Comments
 (0)