Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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_)

Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

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

9 changes: 5 additions & 4 deletions components/ads-client/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
11 changes: 11 additions & 0 deletions components/ads-client/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ pub enum AdsClientApiError {
Other { reason: String },
}

impl From<context_id::ApiError> 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}")]
Expand Down Expand Up @@ -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,

Expand Down
24 changes: 12 additions & 12 deletions components/ads-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl MozAdsClient {

pub fn cycle_context_id(&self) -> AdsClientApiResult<String> {
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)
}

Expand Down Expand Up @@ -159,7 +159,7 @@ impl MozAdsClientInner {
Ok(())
}

fn cycle_context_id(&mut self) -> String {
fn cycle_context_id(&mut self) -> context_id::ApiResult<String> {
self.client.cycle_context_id()
}

Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand All @@ -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(),
Expand All @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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());
Expand Down
51 changes: 31 additions & 20 deletions components/ads-client/src/mars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -25,28 +25,38 @@ pub trait MARSClient: Sync + Send {
) -> Result<(), RecordImpressionError>;
fn record_click(&self, url_callback_string: Option<String>) -> Result<(), RecordClickError>;
fn report_ad(&self, url_callback_string: Option<String>) -> Result<(), ReportAdError>;
fn get_context_id(&self) -> &str;
fn cycle_context_id(&mut self) -> String;
fn get_context_id(&self) -> context_id::ApiResult<String>;
fn cycle_context_id(&mut self) -> context_id::ApiResult<String>;
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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should eventually figure out how we want to handle time here, but approving as this is something we can look into more later and don't want to block this merge.

false,
Box::new(DefaultContextIdCallback),
),
endpoint: DEFAULT_MARS_API_ENDPOINT.to_string(),
}
}

#[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,
}
}
Expand All @@ -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<String> {
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<String> {
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<AdResponse, FetchAdsError> {
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion components/ads-client/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<MozAdsPlacementConfig> {
vec![
Expand Down
1 change: 0 additions & 1 deletion components/context_id/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down