diff --git a/CHANGELOG.md b/CHANGELOG.md index b51545a608..79e3985c6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ ### Nimbus - Reasons are now reported in unenrollment events. ([#7480](https://github.com/mozilla/application-services/pull/7480)) +- The available Firefox Labs are now filtered by targeting and bucketing. ([#7481](https://github.com/mozilla/application-services/pull/7481)) # v153.0 (_2026-06-15_) diff --git a/components/nimbus/src/evaluator.rs b/components/nimbus/src/evaluator.rs index 553830ef3c..1404f07722 100644 --- a/components/nimbus/src/evaluator.rs +++ b/components/nimbus/src/evaluator.rs @@ -50,79 +50,123 @@ pub fn split_locale(locale: String) -> (Option, Option) { /// Determine the enrolment status for an experiment. /// -/// # Arguments: -/// - `available_randomization_units` The app provided available randomization units -/// - `targeting_attributes` The attributes to use when evaluating targeting -/// - `exp` The `Experiment` to evaluate. -/// -/// # Returns: -/// An `ExperimentEnrollment` - you need to inspect the EnrollmentStatus to -/// determine if the user is actually enrolled. -/// -/// # Errors: -/// -/// The function can return errors in one of the following cases (but not limited to): +/// # Errors /// -/// - If the bucket sampling failed (i.e we could not find if the user should or should not be enrolled in the experiment based on the bucketing) -/// - If an error occurs while determining the branch the user should be enrolled in any of the experiments +/// The function can return an error when branch selection fails due to an +/// invalid bucketing configuration. pub fn evaluate_enrollment( available_randomization_units: &AvailableRandomizationUnits, - exp: &Experiment, - th: &NimbusTargetingHelper, + experiment: &Experiment, + targeting_helper: &NimbusTargetingHelper, ) -> Result { - if let ExperimentAvailable::Unavailable { reason } = is_experiment_available(th, exp, true) { - return Ok(ExperimentEnrollment { - slug: exp.slug.clone(), - status: EnrollmentStatus::NotEnrolled { reason }, - }); - } + let status = match can_enroll(available_randomization_units, targeting_helper, experiment) { + CanEnrollResult::Unavailable { reason } => EnrollmentStatus::NotEnrolled { reason }, + CanEnrollResult::NotTargeted => EnrollmentStatus::NotEnrolled { + reason: NotEnrolledReason::NotTargeted, + }, + CanEnrollResult::NotSelected => EnrollmentStatus::NotEnrolled { + reason: NotEnrolledReason::NotSelected, + }, + CanEnrollResult::TargetingError { reason } => EnrollmentStatus::Error { reason }, + CanEnrollResult::NoRandomizationUnit => { + info!( + "Could not find a suitable randomization unit for {}. Skipping experiment.", + experiment.slug, + ); + EnrollmentStatus::Error { + reason: "No randomization unit".into(), + } + } + + CanEnrollResult::Enrollable { randomization_id } => EnrollmentStatus::new_enrolled( + EnrolledReason::Qualified, + &choose_branch(&experiment.slug, &experiment.branches, randomization_id)?.slug, + ), + }; + + Ok(ExperimentEnrollment { + slug: experiment.slug.clone(), + status, + }) +} + +/// Whether or not an experiment can be enrolled. +pub enum CanEnrollResult<'aru> { + /// The experiment is enrollable. + Enrollable { + /// The randomization ID that should be used for branch selection. + randomization_id: &'aru str, + }, + + /// The experiment is not available for a reason outlined in [`NotEnrolledReason`] + Unavailable { + /// The reason the enrollment is not available. + reason: NotEnrolledReason, + }, + + /// The experiment is not enrollable due to a targeting error. + TargetingError { + /// The stringified error. + reason: String, + }, + + /// The experiment is not enrollable because targeting expression evaluated + /// to false. + NotTargeted, - // Get targeting out of the way - "if let chains" are experimental, - // otherwise we could improve this. - if let Some(expr) = &exp.targeting - && let Some(status) = targeting(expr, th) + /// The experiment is not enrollable because randomization ID did not fall + /// into a selected bucket. + NotSelected, + + /// The experiment is not enrollable because it requires a randomization + /// unit that is not available. + NoRandomizationUnit, +} + +/// Determine whether or not it is possible to enroll in the given experiment. +pub fn can_enroll<'aru>( + available_randomization_units: &'aru AvailableRandomizationUnits, + targeting_helper: &NimbusTargetingHelper, + experiment: &Experiment, +) -> CanEnrollResult<'aru> { + if let ExperimentAvailable::Unavailable { reason } = + is_experiment_available(targeting_helper, experiment, true) { - return Ok(ExperimentEnrollment { - slug: exp.slug.clone(), - status, - }); + return CanEnrollResult::Unavailable { reason }; } - Ok(ExperimentEnrollment { - slug: exp.slug.clone(), - status: { - let bucket_config = exp.bucket_config.clone(); - match available_randomization_units.get_value(&bucket_config.randomization_unit) { - Some(id) => { - if sampling::bucket_sample( - vec![id.to_owned(), bucket_config.namespace], - bucket_config.start, - bucket_config.count, - bucket_config.total, - )? { - EnrollmentStatus::new_enrolled( - EnrolledReason::Qualified, - &choose_branch(&exp.slug, &exp.branches, id)?.clone().slug, - ) - } else { - EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotSelected, - } - } - } - None => { - // XXX: When we link in glean, it would be nice if we could emit - // a failure telemetry event here. - info!( - "Could not find a suitable randomization unit for {}. Skipping experiment.", - &exp.slug - ); - EnrollmentStatus::Error { - reason: "No randomization unit".into(), - } - } + + if let Some(targeting_expression) = &experiment.targeting { + match targeting_helper.eval_jexl(targeting_expression) { + Err(e) => { + return CanEnrollResult::TargetingError { + reason: e.to_string(), + }; } - }, - }) + Ok(false) => return CanEnrollResult::NotTargeted, + Ok(true) => {} + }; + } + + let Some(randomization_id) = + available_randomization_units.get_value(&experiment.bucket_config.randomization_unit) + else { + return CanEnrollResult::NoRandomizationUnit; + }; + + let Ok(is_sampled) = sampling::bucket_sample( + [randomization_id, &experiment.bucket_config.namespace], + experiment.bucket_config.start, + experiment.bucket_config.count, + experiment.bucket_config.total, + ) else { + return CanEnrollResult::NoRandomizationUnit; + }; + + if is_sampled { + CanEnrollResult::Enrollable { randomization_id } + } else { + CanEnrollResult::NotSelected + } } /// Whether or not an experiment is available. @@ -222,41 +266,6 @@ pub(crate) fn choose_branch<'a>( branches.get(index).ok_or(NimbusError::OutOfBoundsError) } -/// Checks if the client is targeted by an experiment -/// This api evaluates the JEXL statement retrieved from the server -/// against the application context provided by the client -/// -/// # Arguments -/// - `expression_statement`: The JEXL statement provided by the server -/// - `targeting_attributes`: The client attributes to target against -/// -/// If this app can not be targeted, returns an EnrollmentStatus to indicate -/// why. Returns None if we should continue to evaluate the enrollment status. -/// -/// In practice, if this returns an EnrollmentStatus, it will be either -/// EnrollmentStatus::NotEnrolled, or EnrollmentStatus::Error in the following -/// cases (But not limited to): -/// - The `expression_statement` is not a valid JEXL statement -/// - The `expression_statement` expects fields that do not exist in the AppContext definition -/// - The result of evaluating the statement against the context is not a boolean -/// - jexl-rs returned an error -pub(crate) fn targeting( - expression_statement: &str, - targeting_helper: &NimbusTargetingHelper, -) -> Option { - match targeting_helper.eval_jexl(expression_statement.to_string()) { - Ok(res) => match res { - true => None, - false => Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted, - }), - }, - Err(e) => Some(EnrollmentStatus::Error { - reason: e.to_string(), - }), - } -} - #[cfg(test)] mod unit_tests { use super::*; diff --git a/components/nimbus/src/nimbus.udl b/components/nimbus/src/nimbus.udl index 62b8919088..7eb73f04bc 100644 --- a/components/nimbus/src/nimbus.udl +++ b/components/nimbus/src/nimbus.udl @@ -154,7 +154,7 @@ interface GeckoPrefHandler { void set_gecko_prefs_state(sequence new_prefs_state); void set_gecko_prefs_original_values(sequence original_gecko_prefs); - + }; dictionary GeckoPref { @@ -469,12 +469,12 @@ interface NimbusTargetingHelper { /// Execute the given jexl expression and evaluate against the existing targeting parameters and context passed to /// the helper at construction. [Throws=NimbusError] - boolean eval_jexl(string expression); + boolean eval_jexl([ByRef] string expression); /// Evaluate a JEXL expression and return debug results as JSON. /// For CLI testing and debugging. [Throws=NimbusError] - string eval_jexl_debug(string expression); + string eval_jexl_debug([ByRef] string expression); }; interface NimbusStringHelper { diff --git a/components/nimbus/src/schema.rs b/components/nimbus/src/schema.rs index cd80626540..43f9be77cc 100644 --- a/components/nimbus/src/schema.rs +++ b/components/nimbus/src/schema.rs @@ -347,7 +347,7 @@ pub enum RandomizationUnit { UserId, } -#[derive(Default)] +#[derive(Clone, Default)] pub struct AvailableRandomizationUnits { pub user_id: Option, pub nimbus_id: Option, @@ -377,7 +377,7 @@ impl AvailableRandomizationUnits { } } - pub fn get_value<'a>(&'a self, wanted: &'a RandomizationUnit) -> Option<&'a str> { + pub fn get_value(&self, wanted: &RandomizationUnit) -> Option<&str> { match wanted { RandomizationUnit::NimbusId => self.nimbus_id.as_deref(), RandomizationUnit::UserId => self.user_id.as_deref(), diff --git a/components/nimbus/src/stateful/dbcache.rs b/components/nimbus/src/stateful/dbcache.rs index 2d91068ae4..e68d685cd4 100644 --- a/components/nimbus/src/stateful/dbcache.rs +++ b/components/nimbus/src/stateful/dbcache.rs @@ -9,13 +9,13 @@ use crate::enrollment::{ EnrolledFeature, EnrolledFeatureConfig, ExperimentEnrollment, map_features_by_feature_id, }; use crate::error::{NimbusError, Result, warn}; -use crate::evaluator::{ExperimentAvailable, is_experiment_available}; +use crate::evaluator::{CanEnrollResult, can_enroll}; use crate::stateful::enrollment::get_enrollments; use crate::stateful::firefox_labs::FirefoxLabsMetadata; use crate::stateful::gecko_prefs::GeckoPrefStore; use crate::stateful::persistence::{Database, StoreId, Writer}; use crate::targeting::NimbusTargetingHelper; -use crate::{EnrolledExperiment, Experiment}; +use crate::{AvailableRandomizationUnits, EnrolledExperiment, Experiment}; // This module manages an in-memory cache of the database, so that some // functions exposed by nimbus can return results without blocking on any @@ -219,6 +219,7 @@ impl DatabaseCache { pub fn get_available_firefox_labs_metadata( &self, + available_randomization_units: &AvailableRandomizationUnits, targeting_helper: &NimbusTargetingHelper, coenrolling_feature_ids: &[String], ) -> Result> { @@ -237,13 +238,12 @@ impl DatabaseCache { } let enrolled = data.experiments_by_slug.contains_key(&experiment.slug); + let enrollable = matches!( + can_enroll(available_randomization_units, targeting_helper, experiment,), + CanEnrollResult::Enrollable { .. } + ); - // We call is_experiment_available with is_release=true - // because being able to enroll in experiments for different channels is a bug. - // - // See-also https://bugzilla.mozilla.org/show_bug.cgi?id=1909348 - if is_experiment_available(targeting_helper, experiment, true) - == ExperimentAvailable::Available + if enrollable && (enrolled || (features_available( experiment, diff --git a/components/nimbus/src/stateful/nimbus_client.rs b/components/nimbus/src/stateful/nimbus_client.rs index 05ce8f1201..f8df9442df 100644 --- a/components/nimbus/src/stateful/nimbus_client.rs +++ b/components/nimbus/src/stateful/nimbus_client.rs @@ -1003,14 +1003,20 @@ impl NimbusClient { } pub fn get_available_firefox_labs(&self) -> Result> { - let targeting_attributes = self.get_targeting_attributes(); + let mut state = self.mutable_state.lock().unwrap(); + state.update_time_to_now(Utc::now()); + let targeting_helper = NimbusTargetingHelper::with_targeting_attributes( - &targeting_attributes, + &state.targeting_attributes, self.event_store.clone(), self.gecko_prefs.clone(), ); - self.database_cache - .get_available_firefox_labs_metadata(&targeting_helper, &self.coenrolling_feature_ids) + + self.database_cache.get_available_firefox_labs_metadata( + &state.available_randomization_units, + &targeting_helper, + &self.coenrolling_feature_ids, + ) } pub fn enroll_in_firefox_lab(&self, slug: &str) -> Result { diff --git a/components/nimbus/src/targeting.rs b/components/nimbus/src/targeting.rs index 107c038e3d..3e676124f3 100644 --- a/components/nimbus/src/targeting.rs +++ b/components/nimbus/src/targeting.rs @@ -49,20 +49,20 @@ impl NimbusTargetingHelper { } } - pub fn eval_jexl(&self, expr: String) -> Result { + pub fn eval_jexl(&self, expr: &str) -> Result { cfg_if::cfg_if! { if #[cfg(feature = "stateful")] { - jexl_eval(&expr, &self.context, self.event_store.clone(), self.gecko_pref_store.clone()) + jexl_eval(expr, &self.context, self.event_store.clone(), self.gecko_pref_store.clone()) } else { - jexl_eval(&expr, &self.context) + jexl_eval(expr, &self.context) } } } #[cfg(feature = "stateful")] - pub fn eval_jexl_debug(&self, expression: String) -> Result { + pub fn eval_jexl_debug(&self, expression: &str) -> Result { let eval_result = jexl_eval_raw( - &expression, + expression, &self.context, self.event_store.clone(), self.gecko_pref_store.clone(), diff --git a/components/nimbus/src/tests/stateful/test_behavior.rs b/components/nimbus/src/tests/stateful/test_behavior.rs index b11b405c32..6abd1573d4 100644 --- a/components/nimbus/src/tests/stateful/test_behavior.rs +++ b/components/nimbus/src/tests/stateful/test_behavior.rs @@ -1459,72 +1459,76 @@ mod event_store_tests { let th = NimbusTargetingHelper::from(store); assert!( - th.eval_jexl(format!("'{event_id}'|eventSum('Minutes') >= 0")) + th.eval_jexl(&format!("'{event_id}'|eventSum('Minutes') >= 0")) .is_err() ); - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Minutes', 1) == 1"))?); - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Minutes', 1, 1) == 0"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Minutes', 1) == 1"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Minutes', 1, 1) == 0"))?); // This is one minute bucket 24h ago. We error out at zero. - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Minutes', 1, 24 * 60) == 0"))?); + assert!(th.eval_jexl(&format!( + "'{event_id}'|eventSum('Minutes', 1, 24 * 60) == 0" + ))?); // This is the last 24 hours of one minute buckets. This is the same as the first 60. - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Minutes', 24 * 60) == 1"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Minutes', 24 * 60) == 1"))?); assert!( - th.eval_jexl(format!("'{event_id}'|eventSum('Years') >= 0")) + th.eval_jexl(&format!("'{event_id}'|eventSum('Years') >= 0")) .is_err() ); - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Years', 1) == 1"))?); - assert!(th.eval_jexl(format!("'{event_id}'|eventSum('Years', 1, 1) == 0"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Years', 1) == 1"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventSum('Years', 1, 1) == 0"))?); assert!( - th.eval_jexl(format!("'{event_id}'|eventCountNonZero('Minutes') >= 0")) + th.eval_jexl(&format!("'{event_id}'|eventCountNonZero('Minutes') >= 0")) .is_err() ); - assert!(th.eval_jexl(format!("'{event_id}'|eventCountNonZero('Minutes', 1) == 1"))?); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( + "'{event_id}'|eventCountNonZero('Minutes', 1) == 1" + ))?); + assert!(th.eval_jexl(&format!( "'{event_id}'|eventCountNonZero('Minutes', 1, 1) == 0" ))?); assert!( - th.eval_jexl(format!( + th.eval_jexl(&format!( "'{event_id}'|eventAveragePerInterval('Minutes') >= 0" )) .is_err() ); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventAveragePerInterval('Minutes', 1) == 1" ))?); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventAveragePerInterval('Minutes', 1, 1) == 0" ))?); assert!( - th.eval_jexl(format!( + th.eval_jexl(&format!( "'{event_id}'|eventAveragePerNonZeroInterval('Minutes') >= 0" )) .is_err() ); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventAveragePerNonZeroInterval('Minutes', 1) >= 0" ))?); - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventAveragePerNonZeroInterval('Minutes', 1, 1) >= 0" ))?); // When was this event last seen? It was seen zero minutes ago. - assert!(th.eval_jexl(format!("'{event_id}'|eventLastSeen('Minutes') == 0"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventLastSeen('Minutes') == 0"))?); // Before this last minute, when was this event last seen? It was at least 60 minutes ago, if ever - assert!(th.eval_jexl(format!("'{event_id}'|eventLastSeen('Minutes', 1) > 60"))?); + assert!(th.eval_jexl(&format!("'{event_id}'|eventLastSeen('Minutes', 1) > 60"))?); // LastSeen doesn't support a fourth argument. assert!( - th.eval_jexl(format!("'{event_id}'|eventLastSeen('Minutes', 1, 1) >= 0")) + th.eval_jexl(&format!("'{event_id}'|eventLastSeen('Minutes', 1, 1) >= 0")) .is_err() ); // Q: Before 24 hours ago, when did we last see this event? // A: it was greater than 24h, but likely never. - assert!(th.eval_jexl(format!( + assert!(th.eval_jexl(&format!( "'{event_id}'|eventLastSeen('Minutes', 24 * 60) > 24 * 60" ))?); diff --git a/components/nimbus/src/tests/stateful/test_evaluator.rs b/components/nimbus/src/tests/stateful/test_evaluator.rs index 187935ffaf..eb702a83d7 100644 --- a/components/nimbus/src/tests/stateful/test_evaluator.rs +++ b/components/nimbus/src/tests/stateful/test_evaluator.rs @@ -8,14 +8,13 @@ use std::sync::Arc; use chrono::Utc; use serde_json::json; -use crate::enrollment::NotEnrolledReason; -use crate::evaluator::targeting; +use crate::error::{NimbusError, Result}; use crate::stateful::behavior::{ EventStore, Interval, IntervalConfig, IntervalData, MultiIntervalCounter, SingleIntervalCounter, }; use crate::stateful::targeting::RecordedContext; use crate::tests::helpers::TestRecordedContext; -use crate::{AppContext, EnrollmentStatus, TargetingAttributes}; +use crate::{AppContext, NimbusTargetingHelper, TargetingAttributes}; #[test] fn test_event_sum_transform() { @@ -29,16 +28,14 @@ fn test_event_sum_transform() { }]); let event_store = EventStore::from(vec![("app.foregrounded".to_string(), counter)]); - let th = event_store.into(); - assert_eq!( - targeting("'app.foregrounded'|eventSum('Days', 3, 0) > 2", &th), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + let th = NimbusTargetingHelper::from(event_store); + assert!( + !th.eval_jexl("'app.foregrounded'|eventSum('Days', 3, 0) > 2") + .unwrap(), ); - assert_eq!( - targeting("'app.foregrounded'|eventSum('Days', 3, 0) > 1", &th,), - None + assert!( + th.eval_jexl("'app.foregrounded'|eventSum('Days', 3, 0) > 1") + .unwrap(), ); } @@ -54,22 +51,14 @@ fn test_event_count_non_zero_transform() { }]); let event_store = EventStore::from(vec![("app.foregrounded".to_string(), counter)]); - let th = event_store.into(); - assert_eq!( - targeting( - "'app.foregrounded'|eventCountNonZero('Days', 3, 0) > 2", - &th, - ), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + let th = NimbusTargetingHelper::from(event_store); + assert!( + !th.eval_jexl("'app.foregrounded'|eventCountNonZero('Days', 3, 0) > 2") + .unwrap(), ); - assert_eq!( - targeting( - "'app.foregrounded'|eventCountNonZero('Days', 3, 0) > 1", - &th, - ), - None + assert!( + th.eval_jexl("'app.foregrounded'|eventCountNonZero('Days', 3, 0) > 1") + .unwrap() ); } @@ -85,22 +74,14 @@ fn test_event_average_per_interval_transform() { }]); let event_store = EventStore::from(vec![("app.foregrounded".to_string(), counter)]); - let th = event_store.into(); - assert_eq!( - targeting( - "'app.foregrounded'|eventAveragePerInterval('Days', 7, 0) > 2", - &th, - ), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + let th = NimbusTargetingHelper::from(event_store); + assert!( + !th.eval_jexl("'app.foregrounded'|eventAveragePerInterval('Days', 7, 0) > 2") + .unwrap(), ); - assert_eq!( - targeting( - "'app.foregrounded'|eventAveragePerInterval('Days', 7, 0) > 1.14", - &th, - ), - None + assert!( + th.eval_jexl("'app.foregrounded'|eventAveragePerInterval('Days', 7, 0) > 1.14") + .unwrap() ); } @@ -116,159 +97,91 @@ fn test_event_average_per_non_zero_interval_transform() { }]); let event_store = EventStore::from(vec![("app.foregrounded".to_string(), counter)]); - let th = event_store.into(); - assert_eq!( - targeting( - "'app.foregrounded'|eventAveragePerNonZeroInterval('Days', 7, 0) == 1", - &th, - ), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + let th = NimbusTargetingHelper::from(event_store); + assert!( + !th.eval_jexl("'app.foregrounded'|eventAveragePerNonZeroInterval('Days', 7, 0) == 1") + .unwrap(), ); - assert_eq!( - targeting( - "'app.foregrounded'|eventAveragePerNonZeroInterval('Days', 7, 0) == 2.25", - &th, - ), - None + assert!( + th.eval_jexl("'app.foregrounded'|eventAveragePerNonZeroInterval('Days', 7, 0) == 2.25") + .unwrap() ); } #[test] fn test_event_transform_sum_cnz_avg_avgnz_parameters() { - let th = Default::default(); - - assert_eq!( - targeting( - "'app.foregrounded'|eventSum('Days') > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform Sum requires 2-3 parameters" - .to_string() - }) - ); - assert_eq!( - targeting( - "1|eventSum('Days', 3, 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventSum(1, 3, 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventSum('Day', 3, 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Behavior error: IntervalParseError: Day is not a valid Interval" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventSum('Days', 'test', 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform Sum requires a positive number as the second parameter" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventSum('Days', 3, 'test') > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform Sum requires a positive number as the third parameter" - .to_string() - }) - ); + let th = NimbusTargetingHelper::default(); + + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum('Days') > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform Sum requires 2-3 parameters" + )); + assert!(matches!( + th.eval_jexl("1|eventSum('Days', 3, 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum(1, 3, 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum('Day', 3, 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Behavior error: IntervalParseError: Day is not a valid Interval" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum('Days', 'test', 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform Sum requires a positive number as the second parameter" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventSum('Days', 3, 'test') > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform Sum requires a positive number as the third parameter" + )); } #[test] fn test_event_transform_last_seen_parameters() { - let th = Default::default(); - - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen() > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform LastSeen requires 1-2 parameters" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen('Days', 0, 10) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform LastSeen requires 1-2 parameters" - .to_string() - }) - ); - assert_eq!( - targeting( - "1|eventLastSeen('Days', 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen(1, 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen('Day', 0) > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Behavior error: IntervalParseError: Day is not a valid Interval" - .to_string() - }) - ); - assert_eq!( - targeting( - "'app.foregrounded'|eventLastSeen('Days', 'test') > 1", - &th, - ), - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: Transform parameter error: event transform LastSeen requires a positive number as the second parameter" - .to_string() - }) - ); - - assert_eq!( - targeting("'app_cycle.foreground1'|eventLastSeen('Days', 2) > 1", &th), - None + let th = NimbusTargetingHelper::default(); + + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen() > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform LastSeen requires 1-2 parameters" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen('Days', 0, 10) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform LastSeen requires 1-2 parameters" + )); + assert!(matches!( + th.eval_jexl("1|eventLastSeen('Days', 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen(1, 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen('Day', 0) > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Behavior error: IntervalParseError: Day is not a valid Interval" + )); + assert!(matches!( + th.eval_jexl("'app.foregrounded'|eventLastSeen('Days', 'test') > 1").unwrap_err(), + NimbusError::EvaluationError(e) + if e == "Custom error: Transform parameter error: event transform LastSeen requires a positive number as the second parameter" + )); + + assert!( + th.eval_jexl("'app_cycle.foreground1'|eventLastSeen('Days', 2) > 1") + .unwrap() ); } @@ -277,7 +190,7 @@ fn test_targeting_active_experiments_equivalency() { // Here's our valid jexl statement let expression_statement = "'test' in active_experiments"; // A matching context that includes the appropriate specific context - let mut targeting_attributes: TargetingAttributes = AppContext { + let mut targeting_attributes = TargetingAttributes::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -293,37 +206,36 @@ fn test_targeting_active_experiments_equivalency() { debug_tag: None, custom_targeting_attributes: None, ..Default::default() - } - .into(); + }); + let mut set = HashSet::::new(); set.insert("test".into()); targeting_attributes.active_experiments = set; // The targeting should pass! - assert_eq!( - targeting(expression_statement, &targeting_attributes.clone().into()), - None + assert!( + NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap() ); // We set active_experiment treatment to something not expected and try again let mut set = HashSet::::new(); set.insert("test1".into()); targeting_attributes.active_experiments = set; - assert_eq!( - targeting(expression_statement, &targeting_attributes.clone().into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + assert!( + !NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); // We set active_experiments to None and try again let set = HashSet::::new(); targeting_attributes.active_experiments = set; - assert_eq!( - targeting(expression_statement, &targeting_attributes.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + assert!( + !NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); } @@ -332,7 +244,7 @@ fn test_targeting_active_experiments_exists() { // Here's our valid jexl statement let expression_statement = "'test' in active_experiments"; // A matching context that includes the appropriate specific context - let mut targeting_attributes: TargetingAttributes = AppContext { + let mut targeting_attributes = TargetingAttributes::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -348,26 +260,26 @@ fn test_targeting_active_experiments_exists() { debug_tag: None, custom_targeting_attributes: None, ..Default::default() - } - .into(); + }); + let mut set = HashSet::::new(); set.insert("test".into()); targeting_attributes.active_experiments = set; // The targeting should pass! - assert_eq!( - targeting(expression_statement, &targeting_attributes.clone().into()), - None + assert!( + NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap() ); // We set active_experiment treatment to something not expected and try again let set = HashSet::::new(); targeting_attributes.active_experiments = set; - assert_eq!( - targeting(expression_statement, &targeting_attributes.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + assert!( + !NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); } @@ -376,7 +288,7 @@ fn test_targeting_is_already_enrolled() { // Here's our valid jexl statement let expression_statement = "is_already_enrolled"; // A matching context that includes the appropriate specific context - let ac = AppContext { + let mut targeting_attributes = TargetingAttributes::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -384,110 +296,94 @@ fn test_targeting_is_already_enrolled() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - let mut targeting_attributes = TargetingAttributes::from(ac); + }); targeting_attributes.is_already_enrolled = true; // The targeting should pass! - assert_eq!( - targeting(expression_statement, &targeting_attributes.clone().into(),), - None + assert!( + NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); // We make the is_already_enrolled false and try again targeting_attributes.is_already_enrolled = false; - assert_eq!( - targeting(expression_statement, &targeting_attributes.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) + assert!( + !NimbusTargetingHelper::from(targeting_attributes.clone()) + .eval_jexl(expression_statement) + .unwrap(), ); } #[test] fn test_bucket_sample() { - let cases = [ - ("1.1", "1000", "1000", None), - ( - "0", - "1.1", - "1000", - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted, - }), - ), - ( - "0", - "0", - "1000.1", - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted, - }), - ), + fn eval_bucketsample_expr(start: &str, count: &str, total: &str) -> Result { + let expr = format!("0|bucketSample({start}, {count}, {total})"); + NimbusTargetingHelper::from(AppContext { + app_name: "nimbus_test".into(), + app_id: "nimbus-test".into(), + channel: "test".into(), + ..Default::default() + }) + .eval_jexl(&expr) + } + + let ok_cases = [ + ("1.1", "1000", "1000", true), + ("0", "1.1", "1000", false), + ("0", "0", "1000.1", false), + ]; + + for (start, count, total, expected) in ok_cases { + assert_eq!( + eval_bucketsample_expr(start, count, total).unwrap(), + expected + ); + } + + let err_cases = [ ( "4294967296", "1", "4294967297", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: start is out of range".into(), - }), + "Custom error: start is out of range", ), ( "0", "4294967296", "4294967296", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: count is out of range".into(), - }), + "Custom error: count is out of range", ), ( "0", "0", "4294967296", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: total is out of range".into(), - }), + "Custom error: total is out of range", ), ( r#""hello""#, "0", "1000", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: start is not a number".into(), - }), + "Custom error: start is not a number", ), ( "0", r#""hello""#, "1000", - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: count is not a number".into(), - }), + "Custom error: count is not a number", ), ( "0", "1000", r#""hello""#, - Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: total is not a number".into(), - }), + "Custom error: total is not a number", ), ]; - for (start, count, total, expected) in cases { - let expr = format!("0|bucketSample({start}, {count}, {total})"); - println!("{}", expr); - let targeting_attributes: TargetingAttributes = AppContext { - app_name: "nimbus_test".into(), - app_id: "nimbus-test".into(), - channel: "test".into(), - ..Default::default() - } - .into(); - - let result = targeting(&expr, &targeting_attributes.clone().into()); - - assert_eq!(result, expected); + for (start, count, total, expected) in err_cases { + assert!(matches!( + eval_bucketsample_expr(start, count, total).unwrap_err(), + NimbusError::EvaluationError(e) if e == expected)); } } diff --git a/components/nimbus/src/tests/stateful/test_nimbus.rs b/components/nimbus/src/tests/stateful/test_nimbus.rs index e865b33a6f..5a36fd33e2 100644 --- a/components/nimbus/src/tests/stateful/test_nimbus.rs +++ b/components/nimbus/src/tests/stateful/test_nimbus.rs @@ -1031,7 +1031,7 @@ fn test_active_enrollment_in_targeting() -> Result<()> { assert_eq!(active_experiments.len(), 1); let targeting_helper = client.create_targeting_helper(None)?; - assert!(targeting_helper.eval_jexl("'test-1' in active_experiments".to_string())?); + assert!(targeting_helper.eval_jexl("'test-1' in active_experiments")?); // Apply experiment that targets the above experiment is in enrollments let exp = get_targeted_experiment("test-2", "'test-1' in enrollments"); @@ -1042,12 +1042,12 @@ fn test_active_enrollment_in_targeting() -> Result<()> { assert_eq!(active_experiments.len(), 1); let targeting_helper = client.create_targeting_helper(None)?; - assert!(!targeting_helper.eval_jexl("'test-1' in active_experiments".to_string())?); - assert!(targeting_helper.eval_jexl("'test-2' in active_experiments".to_string())?); - assert!(targeting_helper.eval_jexl("'test-1' in enrollments".to_string())?); - assert!(targeting_helper.eval_jexl("'test-2' in enrollments".to_string())?); - assert!(targeting_helper.eval_jexl("enrollments_map['test-1'] == 'treatment'".to_string())?); - assert!(targeting_helper.eval_jexl("enrollments_map['test-2'] == 'control'".to_string())?); + assert!(!targeting_helper.eval_jexl("'test-1' in active_experiments")?); + assert!(targeting_helper.eval_jexl("'test-2' in active_experiments")?); + assert!(targeting_helper.eval_jexl("'test-1' in enrollments")?); + assert!(targeting_helper.eval_jexl("'test-2' in enrollments")?); + assert!(targeting_helper.eval_jexl("enrollments_map['test-1'] == 'treatment'")?); + assert!(targeting_helper.eval_jexl("enrollments_map['test-2'] == 'control'")?); Ok(()) } @@ -1104,16 +1104,16 @@ fn test_previous_enrollments_in_targeting() -> Result<()> { assert_eq!(active_experiments.len(), 5); let targeting_helper = client.create_targeting_helper(None)?; - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_3))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_4))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_5))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_2))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_3))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_4))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_5))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_3))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_4))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_5))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_2))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_3))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_4))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_5))?); // Apply empty first experiment, disqualifying second experiment, and decreased bucket rollout let exp_2 = get_targeted_experiment_with_feature(slug_2, "false", "feature-2"); @@ -1184,16 +1184,16 @@ fn test_previous_enrollments_in_targeting() -> Result<()> { )); let targeting_helper = client.create_targeting_helper(None)?; - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_3))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_4))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_5))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_2))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_3))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_4))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in enrollments", slug_5))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_3))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_4))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_5))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_2))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_3))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_4))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in enrollments", slug_5))?); Ok(()) } @@ -1234,20 +1234,20 @@ fn test_opt_out_multiple_experiments_same_feature_does_not_re_enroll() -> Result client.apply_pending_experiments()?; let targeting_helper = client.create_targeting_helper(None)?; - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); client.opt_out(slug_1.into())?; let targeting_helper = client.create_targeting_helper(None)?; - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); client.opt_out(slug_2.into())?; let targeting_helper = client.create_targeting_helper(None)?; - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_1))?); - assert!(!targeting_helper.eval_jexl(format!("'{}' in active_experiments", slug_2))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_1))?); + assert!(!targeting_helper.eval_jexl(&format!("'{}' in active_experiments", slug_2))?); Ok(()) } @@ -2597,6 +2597,17 @@ fn test_firefox_labs_enroll_unenroll() -> Result<()> { .patch(json!({ "firefoxLabsDescription": null })), get_firefox_lab_with_feature("lab-different-channel", "lab-feature-7") .patch(json!({ "channel": "mystery" })), + get_firefox_lab_with_feature("false-targeting", "lab-feature-8") + .patch(json!({ "targeting": "false" })), + get_firefox_lab_with_feature("false-bucketing", "lab-feature-9").patch(json!({ + "bucketConfig": { + "randomizationUnit": "nimbus_id", + "start": 0, + "count": 0, + "total": 10000, + "namespace": "firefox-labs-test", + } + })), ])?; assert_eq!( @@ -2639,6 +2650,18 @@ fn test_firefox_labs_enroll_unenroll() -> Result<()> { branch: Some("control".into()), ..Default::default() }, + EnrollmentStatusExtraDef { + slug: Some("false-bucketing".into()), + status: Some("NotEnrolled".into()), + reason: Some("FirefoxLabs".into()), + ..Default::default() + }, + EnrollmentStatusExtraDef { + slug: Some("false-targeting".into()), + status: Some("NotEnrolled".into()), + reason: Some("FirefoxLabs".into()), + ..Default::default() + }, EnrollmentStatusExtraDef { slug: Some("lab".into()), status: Some("NotEnrolled".into()), diff --git a/components/nimbus/src/tests/stateful/test_targeting.rs b/components/nimbus/src/tests/stateful/test_targeting.rs index 14db35aef6..08a3db2ef7 100644 --- a/components/nimbus/src/tests/stateful/test_targeting.rs +++ b/components/nimbus/src/tests/stateful/test_targeting.rs @@ -83,9 +83,7 @@ fn test_eval_jexl_debug_success() { "locale": "en-US", })); - let result = helper - .eval_jexl_debug("locale == 'en-US'".to_string()) - .unwrap(); + let result = helper.eval_jexl_debug("locale == 'en-US'").unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); assert_eq!(parsed["success"], true); @@ -98,7 +96,7 @@ fn test_eval_jexl_debug_error() { let helper = create_helper(json!({})); - let result = helper.eval_jexl_debug("invalid {{".to_string()).unwrap(); + let result = helper.eval_jexl_debug("invalid {{").unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); assert_eq!(parsed["success"], false); @@ -111,7 +109,7 @@ fn test_eval_jexl_debug_json_structure() { let helper = create_helper(json!({"test": true})); - let result = helper.eval_jexl_debug("test".to_string()).unwrap(); + let result = helper.eval_jexl_debug("test").unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); @@ -128,7 +126,7 @@ fn test_eval_jexl_debug_returns_pretty_json() { let helper = create_helper(json!({"locale": "en-US"})); - let result = helper.eval_jexl_debug("locale".to_string()).unwrap(); + let result = helper.eval_jexl_debug("locale").unwrap(); // Pretty JSON should have newlines assert!(result.contains('\n')); @@ -149,7 +147,7 @@ fn test_eval_jexl_debug_with_version_compare() { let helper = create_helper(serde_json::to_value(&targeting_attributes).unwrap()); let result = helper - .eval_jexl_debug("app_version|versionCompare('114.0') > 0".to_string()) + .eval_jexl_debug("app_version|versionCompare('114.0') > 0") .unwrap(); let parsed: serde_json::Value = serde_json::from_str(&result).unwrap(); diff --git a/components/nimbus/src/tests/test_evaluator.rs b/components/nimbus/src/tests/test_evaluator.rs index 58fb18b8a6..1bbb6d5548 100644 --- a/components/nimbus/src/tests/test_evaluator.rs +++ b/components/nimbus/src/tests/test_evaluator.rs @@ -7,10 +7,10 @@ use serde_json::{Map, Value, json}; use crate::enrollment::{EnrolledReason, EnrollmentStatus, NotEnrolledReason}; -use crate::evaluator::{ExperimentAvailable, choose_branch, is_experiment_available, targeting}; +use crate::evaluator::{ExperimentAvailable, choose_branch, is_experiment_available}; use crate::{ - AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, RandomizationUnit, - Result, TargetingAttributes, evaluate_enrollment, + AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, NimbusError, + NimbusTargetingHelper, RandomizationUnit, Result, TargetingAttributes, evaluate_enrollment, }; pub fn ta_with_locale(locale: String) -> TargetingAttributes { @@ -32,29 +32,19 @@ pub fn ta_with_locale(locale: String) -> TargetingAttributes { } #[test] -fn test_locale_substring() -> Result<()> { +fn test_locale_substring() { let expression_statement = "'en' in locale || 'de' in locale"; - let ta = ta_with_locale("de-US".to_string()); + let targeting_helper: NimbusTargetingHelper = ta_with_locale("de-US".to_string()).into(); - assert_eq!(targeting(expression_statement, &ta.into()), None); - Ok(()) + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); } #[test] -fn test_locale_substring_fails() -> Result<()> { +fn test_locale_substring_fails() { let expression_statement = "'en' in locale || 'de' in locale"; - let ta = ta_with_locale("cz-US".to_string()); - let enrollment_status = targeting(expression_statement, &ta.into()).unwrap(); - if let EnrollmentStatus::NotEnrolled { reason } = enrollment_status { - if let NotEnrolledReason::NotTargeted = reason { - // OK - } else { - panic!("Expected to fail on NotTargeted reason, got: {:?}", reason) - } - } else { - panic! {"Expected to fail targeting with NotEnrolled, got: {:?}", enrollment_status} - } - Ok(()) + let targeting_helper: NimbusTargetingHelper = ta_with_locale("cz-US".to_string()).into(); + + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap()); } #[test] @@ -79,35 +69,25 @@ fn test_language_region_from_locale() { #[test] fn test_geo_targeting_one_locale() -> Result<()> { let expression_statement = "language in ['ro']"; - let ta = ta_with_locale("ro".to_string()); + let targeting_helper: NimbusTargetingHelper = ta_with_locale("ro".to_string()).into(); - assert_eq!(targeting(expression_statement, &ta.into()), None); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); Ok(()) } #[test] -fn test_geo_targeting_multiple_locales() -> Result<()> { +fn test_geo_targeting_multiple_locales() { let expression_statement = "language in ['en', 'ro']"; - let ta = ta_with_locale("ro".to_string()); - assert_eq!(targeting(expression_statement, &ta.into()), None); - Ok(()) + let targeting_helper: NimbusTargetingHelper = ta_with_locale("ro".to_string()).into(); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); } #[test] -fn test_geo_targeting_fails_properly() -> Result<()> { +fn test_geo_targeting_fails_properly() { let expression_statement = "language in ['en', 'ro']"; - let ta = ta_with_locale("ar".to_string()); - let enrollment_status = targeting(expression_statement, &ta.into()).unwrap(); - if let EnrollmentStatus::NotEnrolled { reason } = enrollment_status { - if let NotEnrolledReason::NotTargeted = reason { - // OK - } else { - panic!("Expected to fail on NotTargeted reason, got: {:?}", reason) - } - } else { - panic! {"Expected to fail targeting with NotEnrolled, got: {:?}", enrollment_status} - } - Ok(()) + let targeting_helper: NimbusTargetingHelper = ta_with_locale("ar".to_string()).into(); + + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap()); } #[cfg(feature = "stateful")] @@ -115,11 +95,11 @@ fn test_geo_targeting_fails_properly() -> Result<()> { fn test_minimum_version_targeting_passes() -> Result<()> { // Here's our valid jexl statement let expression_statement = "app_version|versionCompare('96.!') >= 0"; - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("97pre.1.0-beta.1".into()), ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); Ok(()) } @@ -128,76 +108,52 @@ fn test_minimum_version_targeting_passes() -> Result<()> { fn test_minimum_version_targeting_fails() -> Result<()> { // Here's our valid jexl statement let expression_statement = "app_version|versionCompare('96+.0') >= 0"; - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("96.1".into()), ..Default::default() - }; - assert_eq!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - ); + }); + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap(),); Ok(()) } #[cfg(feature = "stateful")] #[test] -fn test_targeting_specific_version() -> Result<()> { +fn test_targeting_specific_version() { // Here's our valid jexl statement that targets **only** 96 versions let expression_statement = "(app_version|versionCompare('96.!') >= 0) && (app_version|versionCompare('97.!') < 0)"; - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("96.1".into()), ..Default::default() - }; + }); // OK 96.1 is a 96 version - assert_eq!(targeting(expression_statement, &ctx.into()), None); - let ctx = AppContext { + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("97.1".into()), ..Default::default() - }; + }); // Not targeted, version is 97 - assert_eq!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - ); + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap(),); - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("95.1".into()), ..Default::default() - }; + }); // Not targeted, version is 95 - assert_eq!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - ); - - Ok(()) + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap(),); } #[test] -fn test_targeting_invalid_transform() -> Result<()> { +fn test_targeting_invalid_transform() { let expression_statement = "app_version|invalid_transform('96+.0')"; - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_version: Some("96.1".into()), ..Default::default() - }; - let err = targeting(expression_statement, &ctx.into()); - if let Some(e) = err { - if let EnrollmentStatus::Error { reason: _ } = e { - // OK - } else { - panic!("Should have returned an error since the transform doesn't exist") - } - } else { - panic!("Should not have been targeted") - } - Ok(()) + }); + assert!(matches!( + targeting_helper.eval_jexl(expression_statement).unwrap_err(), + NimbusError::EvaluationError(e) if e == "Unknown transform: invalid_transform", + )); } #[cfg(feature = "stateful")] @@ -208,7 +164,7 @@ fn test_targeting() { "app_id == '1010' && (app_version|versionCompare('4.0') >= 0 || app_build == \"1234\")"; // A matching context testing the logical AND + OR of the expression - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -216,11 +172,11 @@ fn test_targeting() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); // A matching context testing the logical OR of the expression - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -228,11 +184,11 @@ fn test_targeting() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); // A matching context testing the other branch of the logical OR - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -240,11 +196,11 @@ fn test_targeting() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); // A non-matching context testing the logical AND of the expression - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "not_nimbus_test".to_string(), app_id: "org.example.app".to_string(), channel: "test".to_string(), @@ -252,16 +208,11 @@ fn test_targeting() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert!(matches!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - )); + }); + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap()); // A non-matching context testing the logical OR of the expression - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "not_nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -269,13 +220,8 @@ fn test_targeting() { app_build: Some("12345".to_string()), custom_targeting_attributes: None, ..Default::default() - }; - assert!(matches!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::NotEnrolled { - reason: NotEnrolledReason::NotTargeted - }) - )); + }); + assert!(!targeting_helper.eval_jexl(expression_statement).unwrap()) } #[test] @@ -287,7 +233,7 @@ fn test_targeting_custom_targeting_attributes() { custom_targeting_attributes.insert("is_first_run".into(), json!(true)); custom_targeting_attributes.insert("ios_version".into(), json!("8.8")); // A matching context that includes the appropriate specific context - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -295,11 +241,11 @@ fn test_targeting_custom_targeting_attributes() { app_build: Some("1234".to_string()), custom_targeting_attributes: Some(custom_targeting_attributes), ..Default::default() - }; - assert_eq!(targeting(expression_statement, &ctx.into()), None); + }); + assert!(targeting_helper.eval_jexl(expression_statement).unwrap()); // A matching context without the specific context - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "nimbus_test".to_string(), app_id: "1010".to_string(), channel: "test".to_string(), @@ -307,11 +253,11 @@ fn test_targeting_custom_targeting_attributes() { app_build: Some("1234".to_string()), custom_targeting_attributes: None, ..Default::default() - }; + }); // We haven't defined `is_first_run` here, so this should error out, i.e. return an error. assert!(matches!( - targeting(expression_statement, &ctx.into()), - Some(EnrollmentStatus::Error { .. }) + targeting_helper.eval_jexl(expression_statement).unwrap_err(), + NimbusError::EvaluationError(e) if e == "Identifier 'is_first_run' is undefined", )); } @@ -319,23 +265,26 @@ fn test_targeting_custom_targeting_attributes() { fn test_invalid_expression() { // This expression doesn't return a bool let expression_statement = "2.0"; + let targeting_helper = NimbusTargetingHelper::default(); - assert_eq!( - targeting(expression_statement, &Default::default()), - Some(EnrollmentStatus::Error { - reason: "Invalid Expression - didn't evaluate to a bool".to_string() - }) - ) + assert!(matches!( + targeting_helper + .eval_jexl(expression_statement) + .unwrap_err(), + NimbusError::InvalidExpression + )); } #[test] fn test_evaluation_error() { // This is an invalid JEXL statement let expression_statement = "This is not a valid JEXL expression"; + let targeting_helper = NimbusTargetingHelper::default(); assert!(matches!( - targeting(expression_statement, &Default::default()), - Some(EnrollmentStatus::Error { reason }) if reason.starts_with("EvaluationError:"))) + targeting_helper.eval_jexl(expression_statement).inspect_err(|e| eprintln!("{:?}", e)).unwrap_err(), + NimbusError::EvaluationError(e) if e.starts_with("Parsing error: Unrecognized token `is`"), + )); } #[test] @@ -574,12 +523,12 @@ fn test_wrong_randomization_units() { // Application context for matching the above experiment. If any of the `app_name`, `app_id`, // or `channel` doesn't match the experiment, then the client won't be enrolled. - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_name: "NimbusTest".to_string(), app_id: "org.example.app".to_string(), channel: "nightly".to_string(), ..Default::default() - }; + }); // We won't be enrolled in the experiment because we don't have the right randomization units since the // experiment is requesting the `UserId` and the `Default::default()` here will just have the @@ -587,7 +536,7 @@ fn test_wrong_randomization_units() { let enrollment = evaluate_enrollment( &AvailableRandomizationUnits::with_nimbus_id(&uuid::Uuid::new_v4()), &experiment, - &ctx.clone().into(), + &targeting_helper, ) .unwrap(); // The status should be `Error` @@ -595,8 +544,12 @@ fn test_wrong_randomization_units() { // Fits because of the user_id. let available_randomization_units = AvailableRandomizationUnits::with_user_id("bobo"); - let enrollment = - evaluate_enrollment(&available_randomization_units, &experiment, &ctx.into()).unwrap(); + let enrollment = evaluate_enrollment( + &available_randomization_units, + &experiment, + &targeting_helper, + ) + .unwrap(); assert!(matches!( enrollment.status, EnrollmentStatus::Enrolled { @@ -726,16 +679,16 @@ fn test_enrollment_bucketing() { // Tested against the desktop implementation let id = uuid::Uuid::parse_str("299eed1e-be6d-457d-9e53-da7b1a03f10d").unwrap(); // Application context for matching exp3 - let ctx = AppContext { + let targeting_helper = NimbusTargetingHelper::from(AppContext { app_id: "org.example.app".to_string(), channel: "nightly".to_string(), ..Default::default() - }; + }); let enrollment = evaluate_enrollment( &available_randomization_units.apply_nimbus_id(&id), &experiment, - &ctx.into(), + &targeting_helper, ) .unwrap(); assert!(matches!( diff --git a/components/nimbus/tests/test_message_helpers.rs b/components/nimbus/tests/test_message_helpers.rs index 9151e36762..9d136adb97 100644 --- a/components/nimbus/tests/test_message_helpers.rs +++ b/components/nimbus/tests/test_message_helpers.rs @@ -22,18 +22,18 @@ fn test_jexl_expression() -> Result<()> { let helper = nimbus.create_targeting_helper(None)?; // We get a boolean back from a string! - assert!(helper.eval_jexl("app_name == 'fenix'".to_string())?); + assert!(helper.eval_jexl("app_name == 'fenix'")?); // We get true and false back from two similar JEXL expressions! // I think we can convince ourselves that JEXL is being evaluated against the // AppContext. - assert!(!helper.eval_jexl("app_name == 'xinef'".to_string())?); + assert!(!helper.eval_jexl("app_name == 'xinef'")?); // The expression contains a variable not declared (snek_case Good, camelCase Bad) - assert!(helper.eval_jexl("appName == 'fenix'".to_string()).is_err()); + assert!(helper.eval_jexl("appName == 'fenix'").is_err()); // This validates that helpers created from the create_targeting_helper have the event_store present in jexl operations - assert!(helper.eval_jexl("'test'|eventSum('Days', 1, 0) == 1".to_string())?); + assert!(helper.eval_jexl("'test'|eventSum('Days', 1, 0) == 1")?); let helper = nimbus.create_targeting_helper( json!( @@ -46,12 +46,12 @@ fn test_jexl_expression() -> Result<()> { // Check the versionCompare function, just to prove to ourselves that it's the same JEXL evaluator. assert!(helper.eval_jexl( - "(version|versionCompare('95.!') >= 0) && (version|versionCompare('96.!') < 0)".to_string(), + "(version|versionCompare('95.!') >= 0) && (version|versionCompare('96.!') < 0)", )?); // Check the versionCompare function, just to prove to ourselves that it's the same JEXL evaluator. assert!(!helper.eval_jexl( - "(version|versionCompare('96.!') >= 0) && (version|versionCompare('97.!') < 0)".to_string(), + "(version|versionCompare('96.!') >= 0) && (version|versionCompare('97.!') < 0)", )?); Ok(()) @@ -64,11 +64,11 @@ fn test_derived_targeting_attributes_available() -> Result<()> { let helper = nimbus.create_targeting_helper(None)?; - assert!(helper.eval_jexl("locale == 'en-GB'".to_string())?); + assert!(helper.eval_jexl("locale == 'en-GB'")?); - assert!(helper.eval_jexl("language == 'en'".to_string())?); + assert!(helper.eval_jexl("language == 'en'")?); - assert!(helper.eval_jexl("region == 'GB'".to_string())?); + assert!(helper.eval_jexl("region == 'GB'")?); Ok(()) } @@ -82,7 +82,7 @@ fn test_derived_targeting_attributes_none() -> Result<()> { let helper = nimbus.create_targeting_helper(None)?; - assert!(!helper.eval_jexl("(locale||'NONE') == 'en'".to_string())?); + assert!(!helper.eval_jexl("(locale||'NONE') == 'en'")?); // assert!(helper.eval_jexl( // "language == null".to_string() @@ -101,17 +101,17 @@ fn test_jexl_expression_with_targeting_attributes() -> Result<()> { let helper = nimbus.create_targeting_helper(None)?; - assert!(helper.eval_jexl("days_since_install == 0".to_string())?); + assert!(helper.eval_jexl("days_since_install == 0")?); - assert!(helper.eval_jexl("days_since_update == 0".to_string())?); + assert!(helper.eval_jexl("days_since_update == 0")?); nimbus.set_install_time(Utc::now() - Duration::days(10)); nimbus.set_update_time(Utc::now() - Duration::days(5)); let helper = nimbus.create_targeting_helper(None)?; - assert!(helper.eval_jexl("days_since_install == 10".to_string())?); + assert!(helper.eval_jexl("days_since_install == 10")?); - assert!(helper.eval_jexl("days_since_update == 5".to_string())?); + assert!(helper.eval_jexl("days_since_update == 5")?); Ok(()) } diff --git a/components/nimbus/tests/test_restart.rs b/components/nimbus/tests/test_restart.rs index c9e36f86e0..cdb588feee 100644 --- a/components/nimbus/tests/test_restart.rs +++ b/components/nimbus/tests/test_restart.rs @@ -211,9 +211,9 @@ fn test_targeting_attributes_active_experiments() -> Result<()> { assert_eq!(ta.active_experiments, expected); let eval = client.create_targeting_helper(None)?; - assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments".to_string())?); - assert!(eval.eval_jexl("'experiment_target_false' in active_experiments".to_string())?); - assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments".to_string())?); + assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments")?); + assert!(eval.eval_jexl("'experiment_target_false' in active_experiments")?); + assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments")?); drop(client); @@ -224,9 +224,9 @@ fn test_targeting_attributes_active_experiments() -> Result<()> { assert_eq!(ta.active_experiments, expected); let eval = client.create_targeting_helper(None)?; - assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments".to_string())?); - assert!(eval.eval_jexl("'experiment_target_false' in active_experiments".to_string())?); - assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments".to_string())?); + assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments")?); + assert!(eval.eval_jexl("'experiment_target_false' in active_experiments")?); + assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments")?); drop(client); @@ -237,9 +237,9 @@ fn test_targeting_attributes_active_experiments() -> Result<()> { assert_eq!(ta.active_experiments, expected); let eval = client.create_targeting_helper(None)?; - assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments".to_string())?); - assert!(eval.eval_jexl("'experiment_target_false' in active_experiments".to_string())?); - assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments".to_string())?); + assert!(eval.eval_jexl("'experiment_always_enroll' in active_experiments")?); + assert!(eval.eval_jexl("'experiment_target_false' in active_experiments")?); + assert!(!eval.eval_jexl("'experiment_zero_buckets' in active_experiments")?); Ok(()) }