Skip to content

Commit 8e179ae

Browse files
committed
Refactor ads-client to integrate context_id component
1 parent e8792ef commit 8e179ae

File tree

8 files changed

+63
-38
lines changed

8 files changed

+63
-38
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44

55
### Swift
66
- Added `@unchecked Sendable` to classes that conform to `FeatureManifestInterface`. ([#6963](https://github.com/mozilla/application-services/pull/6963)
7+
78
### Ads Client
89
- Added the Ads Client component to the Megazord.
910
- Updated the ApiError enum to AdsClientApiError to avoid naming collision.
11+
- The `context_id` is now generated and rotated via the existing eponym component crate.
1012

1113
# v144.0 (_2025-09-15_)
1214

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

components/ads-client/Cargo.toml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,20 @@ edition = "2021"
55
license = "MPL-2.0"
66

77
[dependencies]
8+
context_id = { path = "../context_id" }
9+
error-support = { path = "../support/error" }
810
parking_lot = "0.12"
911
serde = "1"
1012
serde_json = "1"
11-
url = "2"
13+
thiserror = "2"
1214
uniffi = { version = "0.29.0" }
15+
url = "2"
1316
uuid = { version = "1.3", features = ["v4"] }
1417
viaduct = { path = "../viaduct" }
15-
error-support = { path = "../support/error" }
16-
thiserror = "2"
1718

1819
[dev-dependencies]
19-
mockito = { version = "0.31", default-features = false}
2020
mockall = "0.12"
21+
mockito = { version = "0.31", default-features = false }
2122
viaduct-dev = { path = "../support/viaduct-dev" }
2223

2324
[build-dependencies]

components/ads-client/src/error.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ pub enum AdsClientApiError {
1414
Other { reason: String },
1515
}
1616

17+
impl From<context_id::ApiError> for AdsClientApiError {
18+
fn from(err: context_id::ApiError) -> Self {
19+
AdsClientApiError::Other {
20+
reason: err.to_string(),
21+
}
22+
}
23+
}
24+
1725
#[derive(Debug, thiserror::Error)]
1826
pub enum ComponentError {
1927
#[error("Error requesting ads: {0}")]
@@ -53,6 +61,9 @@ pub enum RequestAdsError {
5361

5462
#[derive(Debug, thiserror::Error)]
5563
pub enum BuildRequestError {
64+
#[error(transparent)]
65+
ContextId(#[from] context_id::ApiError),
66+
5667
#[error("Could not build request with empty placement configs")]
5768
EmptyConfig,
5869

components/ads-client/src/lib.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl MozAdsClient {
8888

8989
pub fn cycle_context_id(&self) -> AdsClientApiResult<String> {
9090
let mut inner = self.inner.lock();
91-
let previous_context_id = inner.cycle_context_id();
91+
let previous_context_id = inner.cycle_context_id()?;
9292
Ok(previous_context_id)
9393
}
9494

@@ -159,7 +159,7 @@ impl MozAdsClientInner {
159159
Ok(())
160160
}
161161

162-
fn cycle_context_id(&mut self) -> String {
162+
fn cycle_context_id(&mut self) -> context_id::ApiResult<String> {
163163
self.client.cycle_context_id()
164164
}
165165

@@ -171,7 +171,7 @@ impl MozAdsClientInner {
171171
return Err(BuildRequestError::EmptyConfig);
172172
}
173173

174-
let context_id = self.client.get_context_id().to_string();
174+
let context_id = self.client.get_context_id()?;
175175
let mut request = AdRequest {
176176
placements: vec![],
177177
context_id,
@@ -288,7 +288,7 @@ mod tests {
288288
fn test_build_ad_request_happy() {
289289
let mut mock = MockMARSClient::new();
290290
mock.expect_get_context_id()
291-
.return_const("mock-context-id".to_string());
291+
.returning(|| Ok("mock-context-id".to_string()));
292292

293293
let inner_component = MozAdsClientInner {
294294
ads_cache: HashMap::new(),
@@ -327,7 +327,7 @@ mod tests {
327327
let request = inner_component
328328
.build_request_from_placement_configs(&configs)
329329
.unwrap();
330-
let context_id = inner_component.client.get_context_id().to_string();
330+
let context_id = inner_component.client.get_context_id().unwrap();
331331

332332
let expected_request = AdRequest {
333333
context_id,
@@ -366,7 +366,7 @@ mod tests {
366366
fn test_build_ad_request_fails_on_duplicate_placement_id() {
367367
let mut mock = MockMARSClient::new();
368368
mock.expect_get_context_id()
369-
.return_const("mock-context-id".to_string());
369+
.returning(|| Ok("mock-context-id".to_string()));
370370

371371
let inner_component = MozAdsClientInner {
372372
ads_cache: HashMap::new(),
@@ -411,7 +411,7 @@ mod tests {
411411
fn test_build_ad_request_fails_on_empty_configs() {
412412
let mut mock = MockMARSClient::new();
413413
mock.expect_get_context_id()
414-
.return_const("mock-context-id".to_string());
414+
.returning(|| Ok("mock-context-id".to_string()));
415415

416416
let inner_component = MozAdsClientInner {
417417
ads_cache: HashMap::new(),
@@ -428,7 +428,7 @@ mod tests {
428428
fn test_build_placements_happy() {
429429
let mut mock = MockMARSClient::new();
430430
mock.expect_get_context_id()
431-
.return_const("mock-context-id".to_string());
431+
.returning(|| Ok("mock-context-id".to_string()));
432432

433433
let inner_component = MozAdsClientInner {
434434
ads_cache: HashMap::new(),
@@ -449,7 +449,7 @@ mod tests {
449449
fn test_build_placements_with_empty_placement_in_response() {
450450
let mut mock = MockMARSClient::new();
451451
mock.expect_get_context_id()
452-
.return_const("mock-context-id".to_string());
452+
.returning(|| Ok("mock-context-id".to_string()));
453453

454454
let inner_component = MozAdsClientInner {
455455
ads_cache: HashMap::new(),
@@ -486,7 +486,7 @@ mod tests {
486486
fn test_request_ads_with_missing_callback_in_response() {
487487
let mut mock = MockMARSClient::new();
488488
mock.expect_get_context_id()
489-
.return_const("mock-context-id".to_string());
489+
.returning(|| Ok("mock-context-id".to_string()));
490490

491491
let inner_component = MozAdsClientInner {
492492
ads_cache: HashMap::new(),
@@ -518,7 +518,7 @@ mod tests {
518518
fn test_build_placements_fails_with_duplicate_placement() {
519519
let mut mock = MockMARSClient::new();
520520
mock.expect_get_context_id()
521-
.return_const("mock-context-id".to_string());
521+
.returning(|| Ok("mock-context-id".to_string()));
522522

523523
let inner_component = MozAdsClientInner {
524524
ads_cache: HashMap::new(),
@@ -572,7 +572,7 @@ mod tests {
572572
mock.expect_fetch_ads()
573573
.returning(|_req| Ok(get_example_happy_ad_response()));
574574
mock.expect_get_context_id()
575-
.return_const("mock-context-id".to_string());
575+
.returning(|| Ok("mock-context-id".to_string()));
576576

577577
mock.expect_get_mars_endpoint()
578578
.return_const("https://mock.endpoint/ads".to_string());

components/ads-client/src/mars.rs

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use crate::{
1010
},
1111
models::{AdRequest, AdResponse},
1212
};
13+
use context_id::{ContextIDComponent, DefaultContextIdCallback};
1314
use url::Url;
14-
use uuid::Uuid;
1515
use viaduct::Request;
1616

1717
const DEFAULT_MARS_API_ENDPOINT: &str = "https://ads.mozilla.org/v1";
@@ -25,28 +25,38 @@ pub trait MARSClient: Sync + Send {
2525
) -> Result<(), RecordImpressionError>;
2626
fn record_click(&self, url_callback_string: Option<String>) -> Result<(), RecordClickError>;
2727
fn report_ad(&self, url_callback_string: Option<String>) -> Result<(), ReportAdError>;
28-
fn get_context_id(&self) -> &str;
29-
fn cycle_context_id(&mut self) -> String;
28+
fn get_context_id(&self) -> context_id::ApiResult<String>;
29+
fn cycle_context_id(&mut self) -> context_id::ApiResult<String>;
3030
fn get_mars_endpoint(&self) -> &str;
3131
}
3232

3333
pub struct DefaultMARSClient {
34-
context_id: String,
34+
context_id_component: ContextIDComponent,
3535
endpoint: String,
3636
}
3737

3838
impl DefaultMARSClient {
3939
pub fn new(context_id: String) -> Self {
4040
Self {
41-
context_id,
41+
context_id_component: ContextIDComponent::new(
42+
&context_id,
43+
0,
44+
false,
45+
Box::new(DefaultContextIdCallback),
46+
),
4247
endpoint: DEFAULT_MARS_API_ENDPOINT.to_string(),
4348
}
4449
}
4550

4651
#[cfg(test)]
4752
pub fn new_with_endpoint(context_id: String, endpoint: String) -> Self {
4853
Self {
49-
context_id,
54+
context_id_component: ContextIDComponent::new(
55+
&context_id,
56+
0,
57+
false,
58+
Box::new(DefaultContextIdCallback),
59+
),
5060
endpoint,
5161
}
5262
}
@@ -60,20 +70,18 @@ impl DefaultMARSClient {
6070
}
6171

6272
impl MARSClient for DefaultMARSClient {
63-
fn get_context_id(&self) -> &str {
64-
&self.context_id
73+
fn cycle_context_id(&mut self) -> context_id::ApiResult<String> {
74+
let old_context_id = self.get_context_id()?;
75+
self.context_id_component.force_rotation()?;
76+
Ok(old_context_id)
6577
}
6678

67-
fn get_mars_endpoint(&self) -> &str {
68-
&self.endpoint
79+
fn get_context_id(&self) -> context_id::ApiResult<String> {
80+
self.context_id_component.request(0)
6981
}
7082

71-
/// Updates the client's context_id to the passed value and returns the previous context_id
72-
/// TODO: Context_id functions should swap over to use the proper context_id component
73-
fn cycle_context_id(&mut self) -> String {
74-
let old_context_id = self.context_id.clone();
75-
self.context_id = Uuid::new_v4().to_string();
76-
old_context_id
83+
fn get_mars_endpoint(&self) -> &str {
84+
&self.endpoint
7785
}
7886

7987
fn fetch_ads(&self, ad_request: &AdRequest) -> Result<AdResponse, FetchAdsError> {
@@ -136,15 +144,18 @@ mod tests {
136144
#[test]
137145
fn test_get_context_id() {
138146
let client = create_test_client(mockito::server_url());
139-
assert_eq!(client.get_context_id(), TEST_CONTEXT_ID.to_string());
147+
assert_eq!(
148+
client.get_context_id().unwrap(),
149+
TEST_CONTEXT_ID.to_string()
150+
);
140151
}
141152

142153
#[test]
143154
fn test_cycle_context_id() {
144155
let mut client = create_test_client(mockito::server_url());
145-
let old_id = client.cycle_context_id();
156+
let old_id = client.cycle_context_id().unwrap();
146157
assert_eq!(old_id, TEST_CONTEXT_ID);
147-
assert_ne!(client.get_context_id(), TEST_CONTEXT_ID);
158+
assert_ne!(client.get_context_id().unwrap(), TEST_CONTEXT_ID);
148159
}
149160

150161
#[test]
@@ -219,7 +230,7 @@ mod tests {
219230
let client = create_test_client(mockito::server_url());
220231

221232
let ad_request = AdRequest {
222-
context_id: client.get_context_id().to_string(),
233+
context_id: client.get_context_id().unwrap().to_string(),
223234
placements: vec![
224235
AdPlacementRequest {
225236
placement: "example_placement_1".to_string(),

components/ads-client/src/test_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::{
1111
IABContent, MozAdsPlacement, MozAdsPlacementConfig,
1212
};
1313

14-
pub const TEST_CONTEXT_ID: &str = "test-context-id";
14+
pub const TEST_CONTEXT_ID: &str = "00000000-0000-4000-8000-000000000001";
1515

1616
pub fn get_example_happy_placement_config() -> Vec<MozAdsPlacementConfig> {
1717
vec![

components/context_id/src/lib.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ impl ContextIDComponent {
3131
///
3232
/// If no creation timestamp is provided, the current time will be used.
3333
#[uniffi::constructor]
34-
3534
pub fn new(
3635
init_context_id: &str,
3736
creation_timestamp_s: i64,

0 commit comments

Comments
 (0)