diff --git a/CHANGELOG.md b/CHANGELOG.md index 984795dd21..138a41d123 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ ### Swift - Added `@unchecked Sendable` to classes that conform to `FeatureManifestInterface`. ([#6963](https://github.com/mozilla/application-services/pull/6963) + ### Ads Client - Added the Ads Client component to the Megazord. - Updated the ApiError enum to AdsClientApiError to avoid naming collision. +- The `context_id` is now generated and rotated via the existing eponym component crate. # v144.0 (_2025-09-15_) diff --git a/Cargo.lock b/Cargo.lock index a446e481e5..5a554e0ca5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,6 +21,7 @@ checksum = "512761e0bb2578dd7380c6baaa0f4ce03e84f95e960231d1dec8bf4d7d6e2627" name = "ads-client" version = "0.1.0" dependencies = [ + "context_id", "error-support", "mockall", "mockito", diff --git a/components/ads-client/Cargo.toml b/components/ads-client/Cargo.toml index 9dd5c9e18e..a526bc322f 100644 --- a/components/ads-client/Cargo.toml +++ b/components/ads-client/Cargo.toml @@ -5,19 +5,20 @@ edition = "2021" license = "MPL-2.0" [dependencies] +context_id = { path = "../context_id" } +error-support = { path = "../support/error" } parking_lot = "0.12" serde = "1" serde_json = "1" -url = "2" +thiserror = "2" uniffi = { version = "0.29.0" } +url = "2" uuid = { version = "1.3", features = ["v4"] } viaduct = { path = "../viaduct" } -error-support = { path = "../support/error" } -thiserror = "2" [dev-dependencies] -mockito = { version = "0.31", default-features = false} mockall = "0.12" +mockito = { version = "0.31", default-features = false } viaduct-dev = { path = "../support/viaduct-dev" } [build-dependencies] diff --git a/components/ads-client/src/error.rs b/components/ads-client/src/error.rs index 26ed558140..2b45bf3c38 100644 --- a/components/ads-client/src/error.rs +++ b/components/ads-client/src/error.rs @@ -14,6 +14,14 @@ pub enum AdsClientApiError { Other { reason: String }, } +impl From for AdsClientApiError { + fn from(err: context_id::ApiError) -> Self { + AdsClientApiError::Other { + reason: err.to_string(), + } + } +} + #[derive(Debug, thiserror::Error)] pub enum ComponentError { #[error("Error requesting ads: {0}")] @@ -53,6 +61,9 @@ pub enum RequestAdsError { #[derive(Debug, thiserror::Error)] pub enum BuildRequestError { + #[error(transparent)] + ContextId(#[from] context_id::ApiError), + #[error("Could not build request with empty placement configs")] EmptyConfig, diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs index e8f671c469..7b8eb503b8 100644 --- a/components/ads-client/src/lib.rs +++ b/components/ads-client/src/lib.rs @@ -88,7 +88,7 @@ impl MozAdsClient { pub fn cycle_context_id(&self) -> AdsClientApiResult { let mut inner = self.inner.lock(); - let previous_context_id = inner.cycle_context_id(); + let previous_context_id = inner.cycle_context_id()?; Ok(previous_context_id) } @@ -159,7 +159,7 @@ impl MozAdsClientInner { Ok(()) } - fn cycle_context_id(&mut self) -> String { + fn cycle_context_id(&mut self) -> context_id::ApiResult { self.client.cycle_context_id() } @@ -171,7 +171,7 @@ impl MozAdsClientInner { return Err(BuildRequestError::EmptyConfig); } - let context_id = self.client.get_context_id().to_string(); + let context_id = self.client.get_context_id()?; let mut request = AdRequest { placements: vec![], context_id, @@ -288,7 +288,7 @@ mod tests { fn test_build_ad_request_happy() { let mut mock = MockMARSClient::new(); mock.expect_get_context_id() - .return_const("mock-context-id".to_string()); + .returning(|| Ok("mock-context-id".to_string())); let inner_component = MozAdsClientInner { ads_cache: HashMap::new(), @@ -327,7 +327,7 @@ mod tests { let request = inner_component .build_request_from_placement_configs(&configs) .unwrap(); - let context_id = inner_component.client.get_context_id().to_string(); + let context_id = inner_component.client.get_context_id().unwrap(); let expected_request = AdRequest { context_id, @@ -366,7 +366,7 @@ mod tests { fn test_build_ad_request_fails_on_duplicate_placement_id() { let mut mock = MockMARSClient::new(); mock.expect_get_context_id() - .return_const("mock-context-id".to_string()); + .returning(|| Ok("mock-context-id".to_string())); let inner_component = MozAdsClientInner { ads_cache: HashMap::new(), @@ -411,7 +411,7 @@ mod tests { fn test_build_ad_request_fails_on_empty_configs() { let mut mock = MockMARSClient::new(); mock.expect_get_context_id() - .return_const("mock-context-id".to_string()); + .returning(|| Ok("mock-context-id".to_string())); let inner_component = MozAdsClientInner { ads_cache: HashMap::new(), @@ -428,7 +428,7 @@ mod tests { fn test_build_placements_happy() { let mut mock = MockMARSClient::new(); mock.expect_get_context_id() - .return_const("mock-context-id".to_string()); + .returning(|| Ok("mock-context-id".to_string())); let inner_component = MozAdsClientInner { ads_cache: HashMap::new(), @@ -449,7 +449,7 @@ mod tests { fn test_build_placements_with_empty_placement_in_response() { let mut mock = MockMARSClient::new(); mock.expect_get_context_id() - .return_const("mock-context-id".to_string()); + .returning(|| Ok("mock-context-id".to_string())); let inner_component = MozAdsClientInner { ads_cache: HashMap::new(), @@ -486,7 +486,7 @@ mod tests { fn test_request_ads_with_missing_callback_in_response() { let mut mock = MockMARSClient::new(); mock.expect_get_context_id() - .return_const("mock-context-id".to_string()); + .returning(|| Ok("mock-context-id".to_string())); let inner_component = MozAdsClientInner { ads_cache: HashMap::new(), @@ -518,7 +518,7 @@ mod tests { fn test_build_placements_fails_with_duplicate_placement() { let mut mock = MockMARSClient::new(); mock.expect_get_context_id() - .return_const("mock-context-id".to_string()); + .returning(|| Ok("mock-context-id".to_string())); let inner_component = MozAdsClientInner { ads_cache: HashMap::new(), @@ -572,7 +572,7 @@ mod tests { mock.expect_fetch_ads() .returning(|_req| Ok(get_example_happy_ad_response())); mock.expect_get_context_id() - .return_const("mock-context-id".to_string()); + .returning(|| Ok("mock-context-id".to_string())); mock.expect_get_mars_endpoint() .return_const("https://mock.endpoint/ads".to_string()); diff --git a/components/ads-client/src/mars.rs b/components/ads-client/src/mars.rs index a4860e4bad..a233c41eb1 100644 --- a/components/ads-client/src/mars.rs +++ b/components/ads-client/src/mars.rs @@ -10,8 +10,8 @@ use crate::{ }, models::{AdRequest, AdResponse}, }; +use context_id::{ContextIDComponent, DefaultContextIdCallback}; use url::Url; -use uuid::Uuid; use viaduct::Request; const DEFAULT_MARS_API_ENDPOINT: &str = "https://ads.mozilla.org/v1"; @@ -25,20 +25,25 @@ pub trait MARSClient: Sync + Send { ) -> Result<(), RecordImpressionError>; fn record_click(&self, url_callback_string: Option) -> Result<(), RecordClickError>; fn report_ad(&self, url_callback_string: Option) -> Result<(), ReportAdError>; - fn get_context_id(&self) -> &str; - fn cycle_context_id(&mut self) -> String; + fn get_context_id(&self) -> context_id::ApiResult; + fn cycle_context_id(&mut self) -> context_id::ApiResult; fn get_mars_endpoint(&self) -> &str; } pub struct DefaultMARSClient { - context_id: String, + context_id_component: ContextIDComponent, endpoint: String, } impl DefaultMARSClient { pub fn new(context_id: String) -> Self { Self { - context_id, + context_id_component: ContextIDComponent::new( + &context_id, + 0, + false, + Box::new(DefaultContextIdCallback), + ), endpoint: DEFAULT_MARS_API_ENDPOINT.to_string(), } } @@ -46,7 +51,12 @@ impl DefaultMARSClient { #[cfg(test)] pub fn new_with_endpoint(context_id: String, endpoint: String) -> Self { Self { - context_id, + context_id_component: ContextIDComponent::new( + &context_id, + 0, + false, + Box::new(DefaultContextIdCallback), + ), endpoint, } } @@ -60,20 +70,18 @@ impl DefaultMARSClient { } impl MARSClient for DefaultMARSClient { - fn get_context_id(&self) -> &str { - &self.context_id + fn cycle_context_id(&mut self) -> context_id::ApiResult { + let old_context_id = self.get_context_id()?; + self.context_id_component.force_rotation()?; + Ok(old_context_id) } - fn get_mars_endpoint(&self) -> &str { - &self.endpoint + fn get_context_id(&self) -> context_id::ApiResult { + self.context_id_component.request(0) } - /// Updates the client's context_id to the passed value and returns the previous context_id - /// TODO: Context_id functions should swap over to use the proper context_id component - fn cycle_context_id(&mut self) -> String { - let old_context_id = self.context_id.clone(); - self.context_id = Uuid::new_v4().to_string(); - old_context_id + fn get_mars_endpoint(&self) -> &str { + &self.endpoint } fn fetch_ads(&self, ad_request: &AdRequest) -> Result { @@ -136,15 +144,18 @@ mod tests { #[test] fn test_get_context_id() { let client = create_test_client(mockito::server_url()); - assert_eq!(client.get_context_id(), TEST_CONTEXT_ID.to_string()); + assert_eq!( + client.get_context_id().unwrap(), + TEST_CONTEXT_ID.to_string() + ); } #[test] fn test_cycle_context_id() { let mut client = create_test_client(mockito::server_url()); - let old_id = client.cycle_context_id(); + let old_id = client.cycle_context_id().unwrap(); assert_eq!(old_id, TEST_CONTEXT_ID); - assert_ne!(client.get_context_id(), TEST_CONTEXT_ID); + assert_ne!(client.get_context_id().unwrap(), TEST_CONTEXT_ID); } #[test] @@ -219,7 +230,7 @@ mod tests { let client = create_test_client(mockito::server_url()); let ad_request = AdRequest { - context_id: client.get_context_id().to_string(), + context_id: client.get_context_id().unwrap().to_string(), placements: vec![ AdPlacementRequest { placement: "example_placement_1".to_string(), diff --git a/components/ads-client/src/test_utils.rs b/components/ads-client/src/test_utils.rs index 447aa9c885..b3dd1fe920 100644 --- a/components/ads-client/src/test_utils.rs +++ b/components/ads-client/src/test_utils.rs @@ -11,7 +11,7 @@ use crate::{ IABContent, MozAdsPlacement, MozAdsPlacementConfig, }; -pub const TEST_CONTEXT_ID: &str = "test-context-id"; +pub const TEST_CONTEXT_ID: &str = "00000000-0000-4000-8000-000000000001"; pub fn get_example_happy_placement_config() -> Vec { vec![ diff --git a/components/context_id/src/lib.rs b/components/context_id/src/lib.rs index 5947529a02..d5d34daf2e 100644 --- a/components/context_id/src/lib.rs +++ b/components/context_id/src/lib.rs @@ -31,7 +31,6 @@ impl ContextIDComponent { /// /// If no creation timestamp is provided, the current time will be used. #[uniffi::constructor] - pub fn new( init_context_id: &str, creation_timestamp_s: i64,