Skip to content

Commit

Permalink
feat: move versioning into its own support crate
Browse files Browse the repository at this point in the history
Multiple components in application-services need to filter for the exact firefox version.
Instead of duplicating the code, we move it into its own support crate, which other components
can now make use of.
  • Loading branch information
gruberb committed Nov 6, 2024
1 parent 6a7df1d commit 2d3ae02
Show file tree
Hide file tree
Showing 18 changed files with 142 additions and 56 deletions.
9 changes: 9 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ members = [

"examples/*/",
"testing/separated/*/",
"components/support/firefox-versioning",
]

exclude = [
Expand Down
3 changes: 2 additions & 1 deletion components/nimbus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ name = "nimbus"
default=["stateful"]
rkv-safe-mode = ["dep:rkv"]
stateful-uniffi-bindings = []
stateful = ["rkv-safe-mode", "stateful-uniffi-bindings", "dep:remote_settings", "dep:regex"]
stateful = ["rkv-safe-mode", "stateful-uniffi-bindings", "dep:remote_settings", "dep:regex", "dep:firefox-versioning"]

[dependencies]
anyhow = "1"
Expand All @@ -40,6 +40,7 @@ error-support = { path = "../support/error" }
remote_settings = { path = "../remote_settings", optional = true }
cfg-if = "1.0.0"
regex = { version = "1.10.5", optional = true }
firefox-versioning = { path = "../support/firefox-versioning", optional = true }

[build-dependencies]
uniffi = { workspace = true, features = ["build"] }
Expand Down
9 changes: 9 additions & 0 deletions components/nimbus/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
//! TODO: Implement proper error handling, this would include defining the error enum,
//! impl std::error::Error using `thiserror` and ensuring all errors are handled appropriately
#[cfg(feature = "stateful")]
use firefox_versioning::error::VersionParsingError;
use std::num::{ParseIntError, TryFromIntError};

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -107,4 +109,11 @@ impl<'a> From<jexl_eval::error::EvaluationError<'a>> for NimbusError {
}
}

#[cfg(feature = "stateful")]
impl From<VersionParsingError> for NimbusError {
fn from(eval_error: VersionParsingError) -> Self {
NimbusError::VersionParsingError(eval_error.to_string())
}
}

pub type Result<T, E = NimbusError> = std::result::Result<T, E>;
1 change: 0 additions & 1 deletion components/nimbus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ mod targeting;
pub mod error;
pub mod metrics;
pub mod schema;
pub mod versioning;

pub use enrollment::{EnrolledFeature, EnrollmentStatus};
pub use error::{NimbusError, Result};
Expand Down
34 changes: 6 additions & 28 deletions components/nimbus/src/targeting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,18 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use crate::{versioning::Version, NimbusError, Result};
use crate::{NimbusError, Result};

use jexl_eval::Evaluator;
use serde::Serialize;
use serde_json::{json, Value};
use serde_json::Value;

cfg_if::cfg_if! {
if #[cfg(feature = "stateful")] {
use anyhow::anyhow;
use crate::{TargetingAttributes, stateful::behavior::{EventStore, EventQueryType, query_event_store}};
use std::sync::{Arc, Mutex};
use firefox_versioning::compare::version_compare;
}
}

Expand Down Expand Up @@ -82,11 +84,11 @@ pub fn jexl_eval_raw<Context: serde::Serialize>(
context: &Context,
#[cfg(feature = "stateful")] event_store: Arc<Mutex<EventStore>>,
) -> Result<Value> {
let evaluator =
Evaluator::new().with_transform("versionCompare", |args| Ok(version_compare(args)?));
let evaluator = Evaluator::new();

#[cfg(feature = "stateful")]
let evaluator = evaluator
.with_transform("versionCompare", |args| Ok(version_compare(args)?))
.with_transform("eventSum", |args| {
Ok(query_event_store(
event_store.clone(),
Expand Down Expand Up @@ -150,30 +152,6 @@ pub fn jexl_eval<Context: serde::Serialize>(
}
}

fn version_compare(args: &[Value]) -> Result<Value> {
let curr_version = args.first().ok_or_else(|| {
NimbusError::VersionParsingError("current version doesn't exist in jexl transform".into())
})?;
let curr_version = curr_version.as_str().ok_or_else(|| {
NimbusError::VersionParsingError("current version in jexl transform is not a string".into())
})?;
let min_version = args.get(1).ok_or_else(|| {
NimbusError::VersionParsingError("minimum version doesn't exist in jexl transform".into())
})?;
let min_version = min_version.as_str().ok_or_else(|| {
NimbusError::VersionParsingError("minimum version is not a string in jexl transform".into())
})?;
let min_version = Version::try_from(min_version)?;
let curr_version = Version::try_from(curr_version)?;
Ok(json!(if curr_version > min_version {
1
} else if curr_version < min_version {
-1
} else {
0
}))
}

#[cfg(feature = "stateful")]
fn bucket_sample(args: &[Value]) -> anyhow::Result<Value> {
fn get_arg_as_u32(args: &[Value], idx: usize, name: &str) -> anyhow::Result<u32> {
Expand Down
1 change: 1 addition & 0 deletions components/nimbus/src/tests/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ pub(crate) fn get_test_experiments() -> Vec<Experiment> {
]
}

#[cfg(feature = "stateful")]
pub fn get_ios_rollout_experiment() -> Experiment {
serde_json::from_value(json!(
{
Expand Down
1 change: 0 additions & 1 deletion components/nimbus/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ mod test_evaluator;
mod test_lib_bw_compat;
mod test_sampling;
mod test_schema;
mod test_versioning;

#[cfg(feature = "stateful")]
mod stateful {
Expand Down
2 changes: 2 additions & 0 deletions components/nimbus/src/tests/stateful/test_nimbus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,7 @@ fn delete_test_creation_date<P: AsRef<Path>>(path: P) -> Result<()> {
Ok(())
}

#[cfg(feature = "stateful")]
#[test]
fn test_ios_rollout() -> Result<()> {
let metrics = TestMetrics::new();
Expand Down Expand Up @@ -1645,6 +1646,7 @@ fn test_new_enrollment_in_targeting_mid_run() -> Result<()> {
Ok(())
}

#[cfg(feature = "stateful")]
#[test]
fn test_recorded_context_recorded() -> Result<()> {
let metrics = TestMetrics::new();
Expand Down
12 changes: 10 additions & 2 deletions components/nimbus/src/tests/test_enrollment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,19 @@ use crate::{
enrollment::*,
error::Result,
tests::helpers::{
get_ios_rollout_experiment, get_multi_feature_experiment, get_single_feature_experiment,
get_test_experiments, no_coenrolling_features,
get_multi_feature_experiment, get_single_feature_experiment, get_test_experiments,
no_coenrolling_features,
},
AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, FeatureConfig,
NimbusTargetingHelper, TargetingAttributes,
};

cfg_if::cfg_if! {
if #[cfg(feature = "stateful")] {
use crate::tests::helpers::get_ios_rollout_experiment;

}
}
use serde_json::{json, Value};
use std::collections::{HashMap, HashSet};
use uuid::Uuid;
Expand Down Expand Up @@ -411,6 +418,7 @@ fn enrollment_evolver<'a>(
EnrollmentsEvolver::new(aru, targeting_helper, ids)
}

#[cfg(feature = "stateful")]
#[test]
fn test_ios_rollout_experiment() -> Result<()> {
let exp = &get_ios_rollout_experiment();
Expand Down
4 changes: 4 additions & 0 deletions components/nimbus/src/tests/test_evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ fn test_geo_targeting_fails_properly() -> Result<()> {
Ok(())
}

#[cfg(feature = "stateful")]
#[test]
fn test_minimum_version_targeting_passes() -> Result<()> {
// Here's our valid jexl statement
Expand All @@ -121,6 +122,7 @@ fn test_minimum_version_targeting_passes() -> Result<()> {
Ok(())
}

#[cfg(feature = "stateful")]
#[test]
fn test_minimum_version_targeting_fails() -> Result<()> {
// Here's our valid jexl statement
Expand All @@ -138,6 +140,7 @@ fn test_minimum_version_targeting_fails() -> Result<()> {
Ok(())
}

#[cfg(feature = "stateful")]
#[test]
fn test_targeting_specific_version() -> Result<()> {
// Here's our valid jexl statement that targets **only** 96 versions
Expand Down Expand Up @@ -196,6 +199,7 @@ fn test_targeting_invalid_transform() -> Result<()> {
Ok(())
}

#[cfg(feature = "stateful")]
#[test]
fn test_targeting() {
// Here's our valid jexl statement
Expand Down
1 change: 1 addition & 0 deletions components/nimbus/tests/test_message_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ mod message_tests {

use super::*;

#[cfg(feature = "stateful")]
#[test]
fn test_jexl_expression() -> Result<()> {
let nimbus = common::new_test_client("jexl_test")?;
Expand Down
10 changes: 10 additions & 0 deletions components/support/firefox-versioning/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "firefox-versioning"
version = "0.1.0"
authors = ["Nimbus Team <[email protected]>"]
license = "MPL-2.0"
edition = "2021"

[dependencies]
serde_json = "1.0"
thiserror = "1.0"
34 changes: 34 additions & 0 deletions components/support/firefox-versioning/src/compare.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use crate::error::VersionParsingError;
use crate::version::Version;
use serde_json::{json, Value};
use std::convert::TryFrom;

pub type Result<T, E = VersionParsingError> = std::result::Result<T, E>;

pub fn version_compare(args: &[Value]) -> Result<Value> {
let curr_version = args.first().ok_or_else(|| {
VersionParsingError::ParseError("current version doesn't exist in jexl transform".into())
})?;
let curr_version = curr_version.as_str().ok_or_else(|| {
VersionParsingError::ParseError("current version in jexl transform is not a string".into())
})?;
let min_version = args.get(1).ok_or_else(|| {
VersionParsingError::ParseError("minimum version doesn't exist in jexl transform".into())
})?;
let min_version = min_version.as_str().ok_or_else(|| {
VersionParsingError::ParseError("minimum version is not a string in jexl transform".into())
})?;
let min_version = Version::try_from(min_version)?;
let curr_version = Version::try_from(curr_version)?;
Ok(json!(if curr_version > min_version {
1
} else if curr_version < min_version {
-1
} else {
0
}))
}
17 changes: 17 additions & 0 deletions components/support/firefox-versioning/src/error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

/// The `VersioningError` indicates that a Firefox Version, being passed down as a `String`,
/// cannot be parsed into a `Version`.
///
/// It can either be caused by a non-ASCII character or a integer overflow.
#[derive(Debug, PartialEq, thiserror::Error)]
pub enum VersionParsingError {
/// Indicates that a number overflowed.
#[error("Overflow Error: {0}")]
Overflow(String),
/// Indicates a general parsing error.
#[error("Parsing Error: {0}")]
ParseError(String),
}
7 changes: 7 additions & 0 deletions components/support/firefox-versioning/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

pub mod compare;
pub mod error;
pub mod version;
Loading

0 comments on commit 2d3ae02

Please sign in to comment.