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 5, 2024
1 parent df8c168 commit 1aa8c57
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 54 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: 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
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;
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
* 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/. */

//! ## Nimbus SDK App Version Comparison
//! The Nimbus SDK supports comparing app versions that follow the Firefox versioning scheme.
//! ## Firefox Version Comparison
//! 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)
//! There's also some more documentation in the [IDL](https://searchfox.org/mozilla-central/rev/468a65168dd0bc3c7d602211a566c16e66416cce/xpcom/base/nsIVersionComparator.idl#9-31)
Expand Down Expand Up @@ -49,7 +48,7 @@
//! ## 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)
//! ```
//! use nimbus::versioning::Version;
//! use firefox_versioning::version::Version;
//! let v1 = Version::try_from("1.0pre1").unwrap();
//! let v2 = Version::try_from("1.0pre2").unwrap();
//! let v3 = Version::try_from("1.0").unwrap();
Expand Down Expand Up @@ -84,19 +83,24 @@
//! < 1.1pre10a
//! < 1.1pre10
use crate::NimbusError;
use crate::error::VersionParsingError;
use std::cmp::Ordering;

#[derive(Debug, Default, Clone, PartialEq)]
pub(crate) struct VersionPart {
pub(crate) num_a: i32,
pub(crate) str_b: String,
pub(crate) num_c: i32,
pub(crate) extra_d: String,
pub struct VersionPart {
pub num_a: i32,
pub str_b: String,
pub num_c: i32,
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)]
pub struct Version(pub(crate) Vec<VersionPart>);
pub struct Version(pub Vec<VersionPart>);

impl PartialEq for Version {
fn eq(&self, other: &Self) -> bool {
Expand Down Expand Up @@ -172,7 +176,7 @@ impl PartialOrd for VersionPart {
}

impl TryFrom<&'_ str> for Version {
type Error = NimbusError;
type Error = VersionParsingError;
fn try_from(value: &'_ str) -> Result<Self, Self::Error> {
let versions = value
.split('.')
Expand All @@ -183,15 +187,15 @@ impl TryFrom<&'_ str> for Version {
}

impl TryFrom<String> for Version {
type Error = NimbusError;
fn try_from(curr_part: String) -> std::result::Result<Self, Self::Error> {
type Error = VersionParsingError;
fn try_from(curr_part: String) -> Result<Self, Self::Error> {
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(|| {
NimbusError::VersionParsingError(format!(
VersionParsingError::Overflow(format!(
"Tried to access character {} in string {}, but it has size {}",
idx,
value,
Expand All @@ -209,13 +213,13 @@ fn is_num_c(c: char) -> bool {
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 {
*res = val;
} else {
let res_l = *res 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(),
));
}
Expand All @@ -226,11 +230,11 @@ fn parse_version_num(val: i32, res: &mut i32) -> Result<(), NimbusError> {
}

impl TryFrom<&'_ str> for VersionPart {
type Error = NimbusError;
type Error = VersionParsingError;

fn try_from(value: &'_ str) -> Result<Self, Self::Error> {
if value.chars().any(|c| !c.is_ascii()) {
return Err(NimbusError::VersionParsingError(format!(
return Err(VersionParsingError::ParseError(format!(
"version string {} contains non-ascii characters",
value
)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
* 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::*;
use crate::{error::Result, NimbusError};
use firefox_versioning::error::VersionParsingError;
use firefox_versioning::version::{Version, VersionPart};

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

#[test]
fn test_wild_card_to_version_part() -> Result<()> {
Expand Down Expand Up @@ -162,7 +164,7 @@ fn test_compare_wild_card() -> Result<()> {
#[test]
fn test_non_ascii_throws_error() -> Result<()> {
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!
} else {
panic!("Expected VersionParsingError, got {:?}", err)
Expand All @@ -178,7 +180,7 @@ fn test_version_number_parsing_overflows() -> Result<()> {
// this is greater than i32::MAX, should return an error
let err =
VersionPart::try_from("2147483648").expect_err("Should throw error, it overflows an i32");
if let NimbusError::VersionParsingError(_) = err {
if let VersionParsingError::Overflow(_) = err {
// OK
} else {
panic!("Expected a VersionParsingError, got {:?}", err)
Expand Down

0 comments on commit 1aa8c57

Please sign in to comment.