feat: move versioning into its own support crate

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.
This commit is contained in:
Bastian Gruber 2024-11-04 10:35:53 -04:00
Родитель 6a7df1d54b
Коммит 2d3ae0252b
18 изменённых файлов: 142 добавлений и 56 удалений

9
Cargo.lock сгенерированный
Просмотреть файл

@ -1522,6 +1522,14 @@ dependencies = [
"log", "log",
] ]
[[package]]
name = "firefox-versioning"
version = "0.1.0"
dependencies = [
"serde_json",
"thiserror",
]
[[package]] [[package]]
name = "fixedbitset" name = "fixedbitset"
version = "0.4.2" version = "0.4.2"
@ -2697,6 +2705,7 @@ dependencies = [
"ctor 0.2.8", "ctor 0.2.8",
"env_logger", "env_logger",
"error-support", "error-support",
"firefox-versioning",
"glean-build", "glean-build",
"hex", "hex",
"jexl-eval", "jexl-eval",

Просмотреть файл

@ -55,6 +55,7 @@ members = [
"examples/*/", "examples/*/",
"testing/separated/*/", "testing/separated/*/",
"components/support/firefox-versioning",
] ]
exclude = [ exclude = [

Просмотреть файл

@ -17,7 +17,7 @@ name = "nimbus"
default=["stateful"] default=["stateful"]
rkv-safe-mode = ["dep:rkv"] rkv-safe-mode = ["dep:rkv"]
stateful-uniffi-bindings = [] 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] [dependencies]
anyhow = "1" anyhow = "1"
@ -40,6 +40,7 @@ error-support = { path = "../support/error" }
remote_settings = { path = "../remote_settings", optional = true } remote_settings = { path = "../remote_settings", optional = true }
cfg-if = "1.0.0" cfg-if = "1.0.0"
regex = { version = "1.10.5", optional = true } regex = { version = "1.10.5", optional = true }
firefox-versioning = { path = "../support/firefox-versioning", optional = true }
[build-dependencies] [build-dependencies]
uniffi = { workspace = true, features = ["build"] } uniffi = { workspace = true, features = ["build"] }

Просмотреть файл

@ -8,6 +8,8 @@
//! TODO: Implement proper error handling, this would include defining the error enum, //! 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 //! 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}; use std::num::{ParseIntError, TryFromIntError};
#[derive(Debug, thiserror::Error)] #[derive(Debug, thiserror::Error)]
@ -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>; pub type Result<T, E = NimbusError> = std::result::Result<T, E>;

Просмотреть файл

@ -15,7 +15,6 @@ mod targeting;
pub mod error; pub mod error;
pub mod metrics; pub mod metrics;
pub mod schema; pub mod schema;
pub mod versioning;
pub use enrollment::{EnrolledFeature, EnrollmentStatus}; pub use enrollment::{EnrolledFeature, EnrollmentStatus};
pub use error::{NimbusError, Result}; pub use error::{NimbusError, Result};

Просмотреть файл

@ -2,16 +2,18 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this // 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/. // 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 jexl_eval::Evaluator;
use serde::Serialize; use serde::Serialize;
use serde_json::{json, Value}; use serde_json::Value;
cfg_if::cfg_if! { cfg_if::cfg_if! {
if #[cfg(feature = "stateful")] { if #[cfg(feature = "stateful")] {
use anyhow::anyhow; use anyhow::anyhow;
use crate::{TargetingAttributes, stateful::behavior::{EventStore, EventQueryType, query_event_store}}; use crate::{TargetingAttributes, stateful::behavior::{EventStore, EventQueryType, query_event_store}};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use firefox_versioning::compare::version_compare;
} }
} }
@ -82,11 +84,11 @@ pub fn jexl_eval_raw<Context: serde::Serialize>(
context: &Context, context: &Context,
#[cfg(feature = "stateful")] event_store: Arc<Mutex<EventStore>>, #[cfg(feature = "stateful")] event_store: Arc<Mutex<EventStore>>,
) -> Result<Value> { ) -> Result<Value> {
let evaluator = let evaluator = Evaluator::new();
Evaluator::new().with_transform("versionCompare", |args| Ok(version_compare(args)?));
#[cfg(feature = "stateful")] #[cfg(feature = "stateful")]
let evaluator = evaluator let evaluator = evaluator
.with_transform("versionCompare", |args| Ok(version_compare(args)?))
.with_transform("eventSum", |args| { .with_transform("eventSum", |args| {
Ok(query_event_store( Ok(query_event_store(
event_store.clone(), event_store.clone(),
@ -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")] #[cfg(feature = "stateful")]
fn bucket_sample(args: &[Value]) -> anyhow::Result<Value> { fn bucket_sample(args: &[Value]) -> anyhow::Result<Value> {
fn get_arg_as_u32(args: &[Value], idx: usize, name: &str) -> anyhow::Result<u32> { fn get_arg_as_u32(args: &[Value], idx: usize, name: &str) -> anyhow::Result<u32> {

Просмотреть файл

@ -342,6 +342,7 @@ pub(crate) fn get_test_experiments() -> Vec<Experiment> {
] ]
} }
#[cfg(feature = "stateful")]
pub fn get_ios_rollout_experiment() -> Experiment { pub fn get_ios_rollout_experiment() -> Experiment {
serde_json::from_value(json!( serde_json::from_value(json!(
{ {

Просмотреть файл

@ -10,7 +10,6 @@ mod test_evaluator;
mod test_lib_bw_compat; mod test_lib_bw_compat;
mod test_sampling; mod test_sampling;
mod test_schema; mod test_schema;
mod test_versioning;
#[cfg(feature = "stateful")] #[cfg(feature = "stateful")]
mod stateful { mod stateful {

Просмотреть файл

@ -917,6 +917,7 @@ fn delete_test_creation_date<P: AsRef<Path>>(path: P) -> Result<()> {
Ok(()) Ok(())
} }
#[cfg(feature = "stateful")]
#[test] #[test]
fn test_ios_rollout() -> Result<()> { fn test_ios_rollout() -> Result<()> {
let metrics = TestMetrics::new(); let metrics = TestMetrics::new();
@ -1645,6 +1646,7 @@ fn test_new_enrollment_in_targeting_mid_run() -> Result<()> {
Ok(()) Ok(())
} }
#[cfg(feature = "stateful")]
#[test] #[test]
fn test_recorded_context_recorded() -> Result<()> { fn test_recorded_context_recorded() -> Result<()> {
let metrics = TestMetrics::new(); let metrics = TestMetrics::new();

Просмотреть файл

@ -10,12 +10,19 @@ use crate::{
enrollment::*, enrollment::*,
error::Result, error::Result,
tests::helpers::{ tests::helpers::{
get_ios_rollout_experiment, get_multi_feature_experiment, get_single_feature_experiment, get_multi_feature_experiment, get_single_feature_experiment, get_test_experiments,
get_test_experiments, no_coenrolling_features, no_coenrolling_features,
}, },
AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, FeatureConfig, AppContext, AvailableRandomizationUnits, Branch, BucketConfig, Experiment, FeatureConfig,
NimbusTargetingHelper, TargetingAttributes, NimbusTargetingHelper, TargetingAttributes,
}; };
cfg_if::cfg_if! {
if #[cfg(feature = "stateful")] {
use crate::tests::helpers::get_ios_rollout_experiment;
}
}
use serde_json::{json, Value}; use serde_json::{json, Value};
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use uuid::Uuid; use uuid::Uuid;
@ -411,6 +418,7 @@ fn enrollment_evolver<'a>(
EnrollmentsEvolver::new(aru, targeting_helper, ids) EnrollmentsEvolver::new(aru, targeting_helper, ids)
} }
#[cfg(feature = "stateful")]
#[test] #[test]
fn test_ios_rollout_experiment() -> Result<()> { fn test_ios_rollout_experiment() -> Result<()> {
let exp = &get_ios_rollout_experiment(); let exp = &get_ios_rollout_experiment();

Просмотреть файл

@ -109,6 +109,7 @@ fn test_geo_targeting_fails_properly() -> Result<()> {
Ok(()) Ok(())
} }
#[cfg(feature = "stateful")]
#[test] #[test]
fn test_minimum_version_targeting_passes() -> Result<()> { fn test_minimum_version_targeting_passes() -> Result<()> {
// Here's our valid jexl statement // Here's our valid jexl statement
@ -121,6 +122,7 @@ fn test_minimum_version_targeting_passes() -> Result<()> {
Ok(()) Ok(())
} }
#[cfg(feature = "stateful")]
#[test] #[test]
fn test_minimum_version_targeting_fails() -> Result<()> { fn test_minimum_version_targeting_fails() -> Result<()> {
// Here's our valid jexl statement // Here's our valid jexl statement
@ -138,6 +140,7 @@ fn test_minimum_version_targeting_fails() -> Result<()> {
Ok(()) Ok(())
} }
#[cfg(feature = "stateful")]
#[test] #[test]
fn test_targeting_specific_version() -> Result<()> { fn test_targeting_specific_version() -> Result<()> {
// Here's our valid jexl statement that targets **only** 96 versions // Here's our valid jexl statement that targets **only** 96 versions
@ -196,6 +199,7 @@ fn test_targeting_invalid_transform() -> Result<()> {
Ok(()) Ok(())
} }
#[cfg(feature = "stateful")]
#[test] #[test]
fn test_targeting() { fn test_targeting() {
// Here's our valid jexl statement // Here's our valid jexl statement

Просмотреть файл

@ -20,6 +20,7 @@ mod message_tests {
use super::*; use super::*;
#[cfg(feature = "stateful")]
#[test] #[test]
fn test_jexl_expression() -> Result<()> { fn test_jexl_expression() -> Result<()> {
let nimbus = common::new_test_client("jexl_test")?; let nimbus = common::new_test_client("jexl_test")?;

Просмотреть файл

@ -0,0 +1,10 @@
[package]
name = "firefox-versioning"
version = "0.1.0"
authors = ["Nimbus Team <project-nimbus@mozilla.com>"]
license = "MPL-2.0"
edition = "2021"
[dependencies]
serde_json = "1.0"
thiserror = "1.0"

Просмотреть файл

@ -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
}))
}

Просмотреть файл

@ -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),
}

Просмотреть файл

@ -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;

Просмотреть файл

@ -2,8 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this * 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/. */ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
//! ## Nimbus SDK App Version Comparison //! ## Firefox Version Comparison
//! The Nimbus SDK supports comparing app versions that follow the Firefox versioning scheme.
//! This module was ported from the Firefox Desktop implementation. You can find the Desktop implementation //! This module was ported from the Firefox Desktop implementation. You can find the Desktop implementation
//! in [this C++ file](https://searchfox.org/mozilla-central/rev/468a65168dd0bc3c7d602211a566c16e66416cce/xpcom/base/nsVersionComparator.cpp) //! in [this C++ file](https://searchfox.org/mozilla-central/rev/468a65168dd0bc3c7d602211a566c16e66416cce/xpcom/base/nsVersionComparator.cpp)
//! There's also some more documentation in the [IDL](https://searchfox.org/mozilla-central/rev/468a65168dd0bc3c7d602211a566c16e66416cce/xpcom/base/nsIVersionComparator.idl#9-31) //! There's also some more documentation in the [IDL](https://searchfox.org/mozilla-central/rev/468a65168dd0bc3c7d602211a566c16e66416cce/xpcom/base/nsIVersionComparator.idl#9-31)
@ -49,7 +48,7 @@
//! ## Example version comparisons //! ## Example version comparisons
//! The following comparisons are taken directly from [the brief documentation in Mozilla-Central](https://searchfox.org/mozilla-central/rev/468a65168dd0bc3c7d602211a566c16e66416cce/xpcom/base/nsIVersionComparator.idl#9-31) //! The following comparisons are taken directly from [the brief documentation in Mozilla-Central](https://searchfox.org/mozilla-central/rev/468a65168dd0bc3c7d602211a566c16e66416cce/xpcom/base/nsIVersionComparator.idl#9-31)
//! ``` //! ```
//! use nimbus::versioning::Version; //! use firefox_versioning::version::Version;
//! let v1 = Version::try_from("1.0pre1").unwrap(); //! let v1 = Version::try_from("1.0pre1").unwrap();
//! let v2 = Version::try_from("1.0pre2").unwrap(); //! let v2 = Version::try_from("1.0pre2").unwrap();
//! let v3 = Version::try_from("1.0").unwrap(); //! let v3 = Version::try_from("1.0").unwrap();
@ -84,19 +83,24 @@
//! < 1.1pre10a //! < 1.1pre10a
//! < 1.1pre10 //! < 1.1pre10
use crate::NimbusError; use crate::error::VersionParsingError;
use std::cmp::Ordering; use std::cmp::Ordering;
#[derive(Debug, Default, Clone, PartialEq)] #[derive(Debug, Default, Clone, PartialEq)]
pub(crate) struct VersionPart { pub struct VersionPart {
pub(crate) num_a: i32, pub num_a: i32,
pub(crate) str_b: String, pub str_b: String,
pub(crate) num_c: i32, pub num_c: i32,
pub(crate) extra_d: String, pub extra_d: String,
} }
/// Represents a version in the form of a sequence of version parts.
///
/// The `Version` struct is used to compare application versions that follow
/// a dot-separated format (e.g., `1.0.0`, `98.2pre1.0-beta`). Each part of the version
/// is represented by a `VersionPart`.
#[derive(Debug, Default, Clone)] #[derive(Debug, Default, Clone)]
pub struct Version(pub(crate) Vec<VersionPart>); pub struct Version(pub Vec<VersionPart>);
impl PartialEq for Version { impl PartialEq for Version {
fn eq(&self, other: &Self) -> bool { fn eq(&self, other: &Self) -> bool {
@ -172,7 +176,7 @@ impl PartialOrd for VersionPart {
} }
impl TryFrom<&'_ str> for Version { impl TryFrom<&'_ str> for Version {
type Error = NimbusError; type Error = VersionParsingError;
fn try_from(value: &'_ str) -> Result<Self, Self::Error> { fn try_from(value: &'_ str) -> Result<Self, Self::Error> {
let versions = value let versions = value
.split('.') .split('.')
@ -183,15 +187,15 @@ impl TryFrom<&'_ str> for Version {
} }
impl TryFrom<String> for Version { impl TryFrom<String> for Version {
type Error = NimbusError; type Error = VersionParsingError;
fn try_from(curr_part: String) -> std::result::Result<Self, Self::Error> { fn try_from(curr_part: String) -> Result<Self, Self::Error> {
curr_part.as_str().try_into() curr_part.as_str().try_into()
} }
} }
fn char_at(value: &str, idx: usize) -> Result<char, NimbusError> { fn char_at(value: &str, idx: usize) -> Result<char, VersionParsingError> {
value.chars().nth(idx).ok_or_else(|| { value.chars().nth(idx).ok_or_else(|| {
NimbusError::VersionParsingError(format!( VersionParsingError::Overflow(format!(
"Tried to access character {} in string {}, but it has size {}", "Tried to access character {} in string {}, but it has size {}",
idx, idx,
value, value,
@ -209,13 +213,13 @@ fn is_num_c(c: char) -> bool {
c.is_numeric() || c == '+' || c == '-' c.is_numeric() || c == '+' || c == '-'
} }
fn parse_version_num(val: i32, res: &mut i32) -> Result<(), NimbusError> { fn parse_version_num(val: i32, res: &mut i32) -> Result<(), VersionParsingError> {
if *res == 0 { if *res == 0 {
*res = val; *res = val;
} else { } else {
let res_l = *res as i64; let res_l = *res as i64;
if (res_l * 10) + val as i64 > i32::MAX as i64 { if (res_l * 10) + val as i64 > i32::MAX as i64 {
return Err(NimbusError::VersionParsingError( return Err(VersionParsingError::Overflow(
"Number parsing overflows an i32".into(), "Number parsing overflows an i32".into(),
)); ));
} }
@ -226,11 +230,11 @@ fn parse_version_num(val: i32, res: &mut i32) -> Result<(), NimbusError> {
} }
impl TryFrom<&'_ str> for VersionPart { impl TryFrom<&'_ str> for VersionPart {
type Error = NimbusError; type Error = VersionParsingError;
fn try_from(value: &'_ str) -> Result<Self, Self::Error> { fn try_from(value: &'_ str) -> Result<Self, Self::Error> {
if value.chars().any(|c| !c.is_ascii()) { if value.chars().any(|c| !c.is_ascii()) {
return Err(NimbusError::VersionParsingError(format!( return Err(VersionParsingError::ParseError(format!(
"version string {} contains non-ascii characters", "version string {} contains non-ascii characters",
value value
))); )));

Просмотреть файл

@ -2,8 +2,10 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this * 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/. */ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
use crate::versioning::*; use firefox_versioning::error::VersionParsingError;
use crate::{error::Result, NimbusError}; use firefox_versioning::version::{Version, VersionPart};
pub type Result<T, E = VersionParsingError> = std::result::Result<T, E>;
#[test] #[test]
fn test_wild_card_to_version_part() -> Result<()> { fn test_wild_card_to_version_part() -> Result<()> {
@ -162,7 +164,7 @@ fn test_compare_wild_card() -> Result<()> {
#[test] #[test]
fn test_non_ascii_throws_error() -> Result<()> { fn test_non_ascii_throws_error() -> Result<()> {
let err = Version::try_from("92🥲.1.2pre").expect_err("Should have thrown error"); let err = Version::try_from("92🥲.1.2pre").expect_err("Should have thrown error");
if let NimbusError::VersionParsingError(_) = err { if let VersionParsingError::ParseError(_) = err {
// Good! // Good!
} else { } else {
panic!("Expected VersionParsingError, got {:?}", err) panic!("Expected VersionParsingError, got {:?}", err)
@ -178,7 +180,7 @@ fn test_version_number_parsing_overflows() -> Result<()> {
// this is greater than i32::MAX, should return an error // this is greater than i32::MAX, should return an error
let err = let err =
VersionPart::try_from("2147483648").expect_err("Should throw error, it overflows an i32"); VersionPart::try_from("2147483648").expect_err("Should throw error, it overflows an i32");
if let NimbusError::VersionParsingError(_) = err { if let VersionParsingError::Overflow(_) = err {
// OK // OK
} else { } else {
panic!("Expected a VersionParsingError, got {:?}", err) panic!("Expected a VersionParsingError, got {:?}", err)