Bug 2041810 - populate conflict_slug on mobile

components/nimbus/src/enrollment.rs

@@ -57,7 +57,7 @@ pub enum NotEnrolledReason {
     /// The experiment enrollment is paused.
     EnrollmentsPaused,
     /// The experiment used a feature that was already under experiment.
-    FeatureConflict,
+    FeatureConflict { conflict_slug: Option<String> },
     /// The evaluator bucketing did not choose us.
     NotSelected,
     /// We are not being targeted for this experiment.
@@ -66,14 +66,116 @@ pub enum NotEnrolledReason {
     OptOut,
 }
 
+#[cfg(feature = "stateful")]
+pub mod v3 {
+    use super::*;
+    use serde::{Deserialize, Serialize};
+
+    #[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
+    pub enum LegacyNotEnrolledReason {
+        DifferentAppName,
+        DifferentChannel,
+        EnrollmentsPaused,
+        FeatureConflict,
+        NotSelected,
+        NotTargeted,
+        OptOut,
+    }
+
+    #[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
+    pub struct LegacyExperimentEnrollment {
+        pub slug: String,
+        pub status: LegacyEnrollmentStatus,
+    }
+
+    #[derive(Deserialize, Serialize, Debug, Clone, Hash, Eq, PartialEq)]
+    pub enum LegacyEnrollmentStatus {
+        Enrolled {
+            reason: EnrolledReason,
+            branch: String,
+            #[serde(skip_serializing_if = "Option::is_none")]
+            prev_gecko_pref_states: Option<Vec<PreviousGeckoPrefState>>,
+        },
+        NotEnrolled {
+            reason: LegacyNotEnrolledReason,
+        },
+        Disqualified {
+            reason: DisqualifiedReason,
+            branch: String,
+        },
+        WasEnrolled {
+            branch: String,
+            experiment_ended_at: u64,
+        },
+        Error {
+            reason: String,
+        },
+    }
+
+    impl From<LegacyNotEnrolledReason> for NotEnrolledReason {
+        fn from(value: LegacyNotEnrolledReason) -> Self {
+            match value {
+                LegacyNotEnrolledReason::DifferentAppName => NotEnrolledReason::DifferentAppName,
+                LegacyNotEnrolledReason::DifferentChannel => NotEnrolledReason::DifferentChannel,
+                LegacyNotEnrolledReason::EnrollmentsPaused => NotEnrolledReason::EnrollmentsPaused,
+                LegacyNotEnrolledReason::FeatureConflict => NotEnrolledReason::FeatureConflict {
+                    conflict_slug: None,
+                },
+                LegacyNotEnrolledReason::NotSelected => NotEnrolledReason::NotSelected,
+                LegacyNotEnrolledReason::NotTargeted => NotEnrolledReason::NotTargeted,
+                LegacyNotEnrolledReason::OptOut => NotEnrolledReason::OptOut,
+            }
+        }
+    }
+
+    impl From<LegacyEnrollmentStatus> for EnrollmentStatus {
+        fn from(value: LegacyEnrollmentStatus) -> Self {
+            match value {
+                LegacyEnrollmentStatus::Enrolled {
+                    reason,
+                    branch,
+                    prev_gecko_pref_states,
+                } => EnrollmentStatus::Enrolled {
+                    reason,
+                    branch,
+                    prev_gecko_pref_states,
+                },
+                LegacyEnrollmentStatus::NotEnrolled { reason } => EnrollmentStatus::NotEnrolled {
+                    reason: reason.into(),
+                },
+                LegacyEnrollmentStatus::Disqualified { reason, branch } => {
+                    EnrollmentStatus::Disqualified { reason, branch }
+                }
+                LegacyEnrollmentStatus::WasEnrolled {
+                    branch,
+                    experiment_ended_at,
+                } => EnrollmentStatus::WasEnrolled {
+                    branch,
+                    experiment_ended_at,
+                },
+                LegacyEnrollmentStatus::Error { reason } => EnrollmentStatus::Error { reason },
+            }
+        }
+    }
+
+    impl From<LegacyExperimentEnrollment> for ExperimentEnrollment {
+        fn from(value: LegacyExperimentEnrollment) -> Self {
+            ExperimentEnrollment {
+                slug: value.slug,
+                status: value.status.into(),
+            }
+        }
+    }
+}
+
 impl Display for NotEnrolledReason {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         Display::fmt(
             match self {
                 NotEnrolledReason::DifferentAppName => "DifferentAppName",
                 NotEnrolledReason::DifferentChannel => "DifferentChannel",
                 NotEnrolledReason::EnrollmentsPaused => "EnrollmentsPaused",
-                NotEnrolledReason::FeatureConflict => "FeatureConflict",
+                NotEnrolledReason::FeatureConflict { .. } => "FeatureConflict",
                 NotEnrolledReason::NotSelected => "NotSelected",
                 NotEnrolledReason::NotTargeted => "NotTargeted",
                 NotEnrolledReason::OptOut => "OptOut",
@@ -905,7 +1007,7 @@ impl<'a> EnrollmentsEvolver<'a> {
             if matches!(
                 prev_enrollment.status,
                 EnrollmentStatus::NotEnrolled {
-                    reason: NotEnrolledReason::FeatureConflict
+                    reason: NotEnrolledReason::FeatureConflict { .. },
                 }
             ) {
                 continue;
@@ -985,7 +1087,9 @@ impl<'a> EnrollmentsEvolver<'a> {
                     next_enrollments.push(ExperimentEnrollment {
                         slug: slug.clone(),
                         status: EnrollmentStatus::NotEnrolled {
-                            reason: NotEnrolledReason::FeatureConflict,
+                            reason: NotEnrolledReason::FeatureConflict {
+                                conflict_slug: Some(needed_features_in_use[0].slug.clone()),
+                            },
                         },
                     });
 
@@ -1014,7 +1118,7 @@ impl<'a> EnrollmentsEvolver<'a> {
                 || matches!(
                     prev_enrollment.unwrap().status,
                     EnrollmentStatus::NotEnrolled {
-                        reason: NotEnrolledReason::FeatureConflict
+                        reason: NotEnrolledReason::FeatureConflict { .. }
                     }
                 )
             {

components/nimbus/src/metrics.rs

@@ -30,6 +30,7 @@ impl From<ExperimentEnrollment> for EnrollmentStatusExtraDef {
         let mut branch_value: Option<String> = None;
         let mut reason_value: Option<String> = None;
         let mut error_value: Option<String> = None;
+        let mut conflict_slug_value: Option<String> = None;
         match &enrollment.status {
             EnrollmentStatus::Enrolled { reason, branch, .. } => {
                 branch_value = Some(branch.to_owned());
@@ -41,6 +42,12 @@ impl From<ExperimentEnrollment> for EnrollmentStatusExtraDef {
             }
             EnrollmentStatus::NotEnrolled { reason } => {
                 reason_value = Some(reason.to_string());
+                conflict_slug_value = match reason {
+                    crate::enrollment::NotEnrolledReason::FeatureConflict { conflict_slug } => {
+                        conflict_slug.to_owned()
+                    }
+                    _ => None,
+                };
             }
             EnrollmentStatus::WasEnrolled { branch, .. } => branch_value = Some(branch.to_owned()),
             EnrollmentStatus::Error { reason } => {
@@ -49,7 +56,7 @@ impl From<ExperimentEnrollment> for EnrollmentStatusExtraDef {
         }
         EnrollmentStatusExtraDef {
             branch: branch_value,
-            conflict_slug: None,
+            conflict_slug: conflict_slug_value,
             error_string: error_value,
             reason: reason_value,
             slug: Some(enrollment.slug),

components/nimbus/src/stateful/persistence.rs

@@ -10,6 +10,8 @@ use std::fs;
 use std::path::Path;
 use std::sync::Arc;
 
+use crate::enrollment::ExperimentEnrollment;
+use crate::enrollment::v3;
 use crate::error::{ErrorCode, NimbusError, Result, debug, info, warn};
 use crate::metrics::{DatabaseLoadExtraDef, DatabaseMigrationExtraDef, MetricsHandler};
 
@@ -27,7 +29,7 @@ use crate::metrics::{DatabaseLoadExtraDef, DatabaseMigrationExtraDef, MetricsHan
 pub(crate) const DB_KEY_DB_VERSION: &str = "db_version";
 
 /// The current database version.
-pub(crate) const DB_VERSION: u16 = 3;
+pub(crate) const DB_VERSION: u16 = 4;
 
 pub(crate) const DB_KEY_DB_WAS_CORRUPT: &str = "db-was-corrupt";
 
@@ -554,6 +556,14 @@ impl Database {
             DatabaseMigrationReason::Upgrade,
         )?;
 
+        self.apply_migration(
+            writer,
+            |writer| self.migrate_v3_to_v4(writer),
+            &mut current_version,
+            4,
+            DatabaseMigrationReason::Upgrade,
+        )?;
+
         Ok(())
     }
 
@@ -677,6 +687,23 @@ impl Database {
         Ok(())
     }
 
+    fn migrate_v3_to_v4(&self, writer: &mut Writer) -> Result<()> {
+        info!("Upgrading from version 3 to version 4");
+
+        let enrollment_store = self.get_store(StoreId::Enrollments);
+        let enrollments: Vec<v3::LegacyExperimentEnrollment> =
+            enrollment_store.try_collect_all(writer)?;
+
+        let enrollments: Vec<ExperimentEnrollment> =
+            enrollments.into_iter().map(Into::into).collect();
+
+        for enrollment in enrollments {
+            enrollment_store.put(writer, &enrollment.slug, &enrollment)?;
+        }
+
+        Ok(())
+    }
+
     /// Gets a Store object, which used with the writer returned by
     /// `self.write()` to update the database in a transaction.
     pub fn get_store(&self, store_id: StoreId) -> &SingleStore {