Skip to content

Commit 40275a1

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 40275a1

File tree

14 files changed

+756
-20
lines changed

14 files changed

+756
-20
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: 45 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,29 @@ 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 Gecko pref before enrollment took place.
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+
}
138147

148+
// The previous state of a given feature before enrollment.
139149
// ⚠️ 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. ⚠️
150+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
151+
#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq, uniffi::Enum)]
152+
#[cfg(feature = "stateful")]
153+
pub enum PreviousState {
154+
GeckoPref(PreviousGeckoPrefState),
155+
}
156+
157+
// Every experiment has an ExperimentEnrollment, even when we aren't enrolled.
158+
// ⚠️ Attention : Changes to this type should be accompanied by a new test ⚠️
159+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
141160
#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
142161
pub struct ExperimentEnrollment {
143162
pub slug: String,
@@ -430,6 +449,20 @@ impl ExperimentEnrollment {
430449
}
431450
}
432451

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

533566
// ⚠️ 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. ⚠️
567+
// ⚠️ in `src/stateful/tests/test_enrollment_bw_compat.rs` below, and may require a DB migration. ⚠️
535568
#[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
536569
pub enum EnrollmentStatus {
537570
Enrolled {
538571
reason: EnrolledReason,
539572
branch: String,
573+
#[cfg(feature = "stateful")]
574+
#[serde(skip_serializing_if = "Option::is_none")]
575+
previous_state: Option<PreviousState>,
540576
},
541577
NotEnrolled {
542578
reason: NotEnrolledReason,
@@ -577,6 +613,8 @@ impl EnrollmentStatus {
577613
EnrollmentStatus::Enrolled {
578614
reason,
579615
branch: branch.to_owned(),
616+
#[cfg(feature = "stateful")]
617+
previous_state: None,
580618
}
581619
}
582620

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 {

0 commit comments

Comments
 (0)