From 177f8c293dce28e30a619884babd6a5982a82e2c Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 12 Mar 2026 16:18:18 -0400 Subject: [PATCH 1/5] feat(ads-client): add reason parameter to report_ad per MARS /v1/t spec The /v1/t tracking endpoint requires a `reason` query parameter for report interactions. Add MozAdsReportReason enum with Inappropriate, NotInterested, and SeenTooManyTimes variants and thread it through the full stack, appending it to the callback URL before the request. --- CHANGELOG.md | 1 + components/ads-client/src/client.rs | 21 +++++++++++++++++++-- components/ads-client/src/ffi.rs | 18 ++++++++++++++++++ components/ads-client/src/lib.rs | 10 ++++++++-- components/ads-client/src/mars.rs | 13 +++++++++++-- 5 files changed, 57 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f99139514f..532338f83f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ ### Ads Client - Added `rotation_days` parameter to `MozAdsClientBuilder` to allow embedders to configure the context ID rotation period. ([#7262](https://github.com/mozilla/application-services/pull/7262)) +- Added `reason` parameter to `report_ad` to comply with the MARS `/v1/t` tracking endpoint spec. Accepted values: `inappropriate`, `not_interested`, `seen_too_many_times`. ### Logins - **BREAKING**: Removed `time_of_last_breach` field from `LoginMeta` and `Login`. This can be derived from Remote Settings during runtime instead. diff --git a/components/ads-client/src/client.rs b/components/ads-client/src/client.rs index 01503a22cc..5853c70fc5 100644 --- a/components/ads-client/src/client.rs +++ b/components/ads-client/src/client.rs @@ -9,6 +9,23 @@ use std::time::Duration; use crate::client::ad_response::{AdImage, AdResponse, AdResponseValue, AdSpoc, AdTile}; use crate::client::config::{AdsClientConfig, Environment}; use crate::error::{RecordClickError, RecordImpressionError, ReportAdError, RequestAdsError}; + +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum ReportReason { + Inappropriate, + NotInterested, + SeenTooManyTimes, +} + +impl ReportReason { + pub fn as_str(&self) -> &'static str { + match self { + ReportReason::Inappropriate => "inappropriate", + ReportReason::NotInterested => "not_interested", + ReportReason::SeenTooManyTimes => "seen_too_many_times", + } + } +} use crate::http_cache::{HttpCache, RequestCachePolicy}; use crate::mars::MARSClient; use crate::telemetry::Telemetry; @@ -199,9 +216,9 @@ where }) } - pub fn report_ad(&self, report_url: Url) -> Result<(), ReportAdError> { + pub fn report_ad(&self, report_url: Url, reason: ReportReason) -> Result<(), ReportAdError> { self.client - .report_ad(report_url) + .report_ad(report_url, reason) .inspect_err(|e| { self.telemetry.record(e); }) diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index 961b3c322c..168687cd41 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -11,6 +11,7 @@ use crate::client::ad_request::{AdContentCategory, AdPlacementRequest, IABConten use crate::client::ad_response::{ AdCallbacks, AdImage, AdSpoc, AdTile, SpocFrequencyCaps, SpocRanking, }; +use crate::client::ReportReason; use crate::client::config::{AdsCacheConfig, AdsClientConfig, Environment}; use crate::client::AdsClient; use crate::error::ComponentError; @@ -191,6 +192,23 @@ pub struct MozAdsRequestCachePolicy { pub ttl_seconds: Option, } +#[derive(Clone, Copy, Debug, uniffi::Enum)] +pub enum MozAdsReportReason { + Inappropriate, + NotInterested, + SeenTooManyTimes, +} + +impl From for ReportReason { + fn from(reason: MozAdsReportReason) -> Self { + match reason { + MozAdsReportReason::Inappropriate => ReportReason::Inappropriate, + MozAdsReportReason::NotInterested => ReportReason::NotInterested, + MozAdsReportReason::SeenTooManyTimes => ReportReason::SeenTooManyTimes, + } + } +} + #[derive(Clone, Copy, Debug, Default, uniffi::Enum)] pub enum MozAdsCacheMode { #[default] diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs index 2b7ba32498..5d94c5d38c 100644 --- a/components/ads-client/src/lib.rs +++ b/components/ads-client/src/lib.rs @@ -111,11 +111,17 @@ impl MozAdsClient { } #[handle_error(ComponentError)] - pub fn report_ad(&self, report_url: String) -> AdsClientApiResult<()> { + pub fn report_ad( + &self, + report_url: String, + reason: MozAdsReportReason, + ) -> AdsClientApiResult<()> { let url = AdsClientUrl::parse(&report_url) .map_err(|e| ComponentError::ReportAd(CallbackRequestError::InvalidUrl(e).into()))?; let inner = self.inner.lock(); - inner.report_ad(url).map_err(ComponentError::ReportAd) + inner + .report_ad(url, reason.into()) + .map_err(ComponentError::ReportAd) } pub fn clear_cache(&self) -> AdsClientApiResult<()> { diff --git a/components/ads-client/src/mars.rs b/components/ads-client/src/mars.rs index f9b25ab97a..d37a151259 100644 --- a/components/ads-client/src/mars.rs +++ b/components/ads-client/src/mars.rs @@ -7,6 +7,8 @@ use crate::{ client::{ ad_request::AdRequest, ad_response::{AdResponse, AdResponseValue}, + config::Environment, + ReportReason, }, error::{ check_http_status_for_error, CallbackRequestError, FetchAdsError, RecordClickError, @@ -90,7 +92,10 @@ where Ok(()) } - pub fn report_ad(&self, callback: Url) -> Result<(), ReportAdError> { + pub fn report_ad(&self, mut callback: Url, reason: ReportReason) -> Result<(), ReportAdError> { + callback + .query_pairs_mut() + .append_pair("reason", reason.as_str()); Ok(self.make_callback_request(callback)?) } @@ -142,6 +147,10 @@ mod tests { fn test_report_ad_with_valid_url_should_succeed() { viaduct_dev::init_backend_dev(); let _m = mock("GET", "/report_ad_callback_url") + .match_query(mockito::Matcher::UrlEncoded( + "reason".into(), + "not_interested".into(), + )) .with_status(200) .create(); @@ -151,7 +160,7 @@ mod tests { &mockito::server_url() )) .unwrap(); - let result = client.report_ad(url); + let result = client.report_ad(url, ReportReason::NotInterested); assert!(result.is_ok()); } From 2ccce93db78af9797bf4918b37c759326d0d4014 Mon Sep 17 00:00:00 2001 From: ahanot Date: Wed, 18 Mar 2026 19:02:52 -0400 Subject: [PATCH 2/5] fix(ads-client): remove unused import and fix formatting --- components/ads-client/src/ffi.rs | 2 +- components/ads-client/src/mars.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index 168687cd41..eb6e2fd8be 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -11,9 +11,9 @@ use crate::client::ad_request::{AdContentCategory, AdPlacementRequest, IABConten use crate::client::ad_response::{ AdCallbacks, AdImage, AdSpoc, AdTile, SpocFrequencyCaps, SpocRanking, }; -use crate::client::ReportReason; use crate::client::config::{AdsCacheConfig, AdsClientConfig, Environment}; use crate::client::AdsClient; +use crate::client::ReportReason; use crate::error::ComponentError; use crate::ffi::telemetry::MozAdsTelemetryWrapper; use crate::http_cache::{CacheMode, RequestCachePolicy}; diff --git a/components/ads-client/src/mars.rs b/components/ads-client/src/mars.rs index d37a151259..1b9fcd6fc0 100644 --- a/components/ads-client/src/mars.rs +++ b/components/ads-client/src/mars.rs @@ -7,7 +7,6 @@ use crate::{ client::{ ad_request::AdRequest, ad_response::{AdResponse, AdResponseValue}, - config::Environment, ReportReason, }, error::{ From 934eb44da8fd0d5fbdaefd20a206a6042e5d2490 Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 2 Apr 2026 13:56:25 -0400 Subject: [PATCH 3/5] test(ads-client): add record_impression, record_click, report_ad integration tests against prod Switch integration tests from staging to production. Add tests that request tile ads and use the returned callback URLs to verify record_impression, record_click, and report_ad each return 200. --- .../integration-tests/tests/mars.rs | 97 +++++++++++++++++-- 1 file changed, 88 insertions(+), 9 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index 24a12da4bc..a4fb8ac7a1 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -6,7 +6,8 @@ use std::sync::Arc; use ads_client::{ - MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount, + MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, + MozAdsPlacementRequestWithCount, MozAdsReportReason, }; fn init_backend() { @@ -14,18 +15,18 @@ fn init_backend() { let _ = viaduct_hyper::viaduct_init_backend_hyper(); } -fn staging_client() -> ads_client::MozAdsClient { +fn prod_client() -> ads_client::MozAdsClient { Arc::new(MozAdsClientBuilder::new()) - .environment(MozAdsEnvironment::Staging) + .environment(MozAdsEnvironment::Prod) .build() } #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_image_staging() { +fn test_contract_image_prod() { init_backend(); - let client = staging_client(); + let client = prod_client(); let result = client.request_image_ads( vec![MozAdsPlacementRequest { placement_id: "mock_billboard_1".to_string(), @@ -45,10 +46,10 @@ fn test_contract_image_staging() { #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_spoc_staging() { +fn test_contract_spoc_prod() { init_backend(); - let client = staging_client(); + let client = prod_client(); let result = client.request_spoc_ads( vec![MozAdsPlacementRequestWithCount { placement_id: "mock_spoc_1".to_string(), @@ -66,10 +67,10 @@ fn test_contract_spoc_staging() { #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_tile_staging() { +fn test_contract_tile_prod() { init_backend(); - let client = staging_client(); + let client = prod_client(); let result = client.request_tile_ads( vec![MozAdsPlacementRequest { placement_id: "mock_tile_1".to_string(), @@ -82,3 +83,81 @@ fn test_contract_tile_staging() { let placements = result.unwrap(); assert!(placements.contains_key("mock_tile_1")); } + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_record_impression() { + init_backend(); + + let client = prod_client(); + let placements = client + .request_tile_ads( + vec![MozAdsPlacementRequest { + placement_id: "mock_tile_1".to_string(), + iab_content: None, + }], + None, + ) + .expect("tile ad request should succeed"); + + let ad = placements + .get("mock_tile_1") + .expect("mock_tile_1 placement should be present"); + + let result = client.record_impression(ad.callbacks.impression.to_string()); + assert!(result.is_ok(), "record_impression failed: {:?}", result.err()); +} + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_record_click() { + init_backend(); + + let client = prod_client(); + let placements = client + .request_tile_ads( + vec![MozAdsPlacementRequest { + placement_id: "mock_tile_1".to_string(), + iab_content: None, + }], + None, + ) + .expect("tile ad request should succeed"); + + let ad = placements + .get("mock_tile_1") + .expect("mock_tile_1 placement should be present"); + + let result = client.record_click(ad.callbacks.click.to_string()); + assert!(result.is_ok(), "record_click failed: {:?}", result.err()); +} + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_report_ad() { + init_backend(); + + let client = prod_client(); + let placements = client + .request_tile_ads( + vec![MozAdsPlacementRequest { + placement_id: "mock_tile_1".to_string(), + iab_content: None, + }], + None, + ) + .expect("tile ad request should succeed"); + + let ad = placements + .get("mock_tile_1") + .expect("mock_tile_1 placement should be present"); + + let report_url = ad + .callbacks + .report + .as_ref() + .expect("mock_tile_1 should have a report URL"); + + let result = client.report_ad(report_url.to_string(), MozAdsReportReason::NotInterested); + assert!(result.is_ok(), "report_ad failed: {:?}", result.err()); +} From 3e232078f65d12c6b848f5b7064299cdc9abff4c Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 2 Apr 2026 14:55:33 -0400 Subject: [PATCH 4/5] feat(ads-client): bake placement_id and position into report callback URLs Add `add_placement_info_to_report_callbacks` on `AdResponse`, mirroring the existing `add_request_hash_to_callbacks` pattern. Called right after it in `request_ads`, it appends `placement_id` and 0-based `position` to each ad's report URL at parse time so `report_ad` sends all three params required by the MARS /v1/t spec without any API change. --- components/ads-client/src/client.rs | 1 + .../ads-client/src/client/ad_response.rs | 89 +++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/components/ads-client/src/client.rs b/components/ads-client/src/client.rs index 5853c70fc5..c9331c4dfb 100644 --- a/components/ads-client/src/client.rs +++ b/components/ads-client/src/client.rs @@ -132,6 +132,7 @@ where let cache_policy = options.unwrap_or_default(); let (mut response, request_hash) = self.client.fetch_ads::(ad_request, &cache_policy)?; response.add_request_hash_to_callbacks(&request_hash); + response.add_placement_info_to_report_callbacks(); Ok(response) } diff --git a/components/ads-client/src/client/ad_response.rs b/components/ads-client/src/client/ad_response.rs index 8cf72206fc..dc998d14ea 100644 --- a/components/ads-client/src/client/ad_response.rs +++ b/components/ads-client/src/client/ad_response.rs @@ -60,6 +60,19 @@ impl AdResponse { } } + pub fn add_placement_info_to_report_callbacks(&mut self) { + for (placement_id, ads) in self.data.iter_mut() { + for (position, ad) in ads.iter_mut().enumerate() { + if let Some(report_url) = ad.callbacks_mut().report.as_mut() { + report_url + .query_pairs_mut() + .append_pair("placement_id", placement_id) + .append_pair("position", &position.to_string()); + } + } + } + } + pub fn take_first(self) -> HashMap { self.data .into_iter() @@ -469,6 +482,82 @@ mod tests { .contains("request_hash=abc123def456")); } + #[test] + fn test_add_placement_info_to_report_callbacks() { + let mut response = AdResponse { + data: HashMap::from([( + "mock_tile_1".to_string(), + vec![ + AdImage { + alt_text: None, + block_key: "key1".into(), + callbacks: AdCallbacks { + click: Url::parse("https://example.com/click1").unwrap(), + impression: Url::parse("https://example.com/impression1").unwrap(), + report: Some(Url::parse("https://example.com/report").unwrap()), + }, + format: "billboard".to_string(), + image_url: "https://example.com/image1.png".to_string(), + url: "https://example.com/ad1".to_string(), + }, + AdImage { + alt_text: None, + block_key: "key2".into(), + callbacks: AdCallbacks { + click: Url::parse("https://example.com/click2").unwrap(), + impression: Url::parse("https://example.com/impression2").unwrap(), + report: Some(Url::parse("https://example.com/report").unwrap()), + }, + format: "billboard".to_string(), + image_url: "https://example.com/image2.png".to_string(), + url: "https://example.com/ad2".to_string(), + }, + ], + )]), + }; + + response.add_placement_info_to_report_callbacks(); + + let ads = &response.data["mock_tile_1"]; + + let report_0 = ads[0].callbacks.report.as_ref().unwrap().query().unwrap_or(""); + assert!(report_0.contains("placement_id=mock_tile_1")); + assert!(report_0.contains("position=0")); + + let report_1 = ads[1].callbacks.report.as_ref().unwrap().query().unwrap_or(""); + assert!(report_1.contains("placement_id=mock_tile_1")); + assert!(report_1.contains("position=1")); + } + + #[test] + fn test_add_placement_info_skips_ads_without_report_url() { + let mut response = AdResponse { + data: HashMap::from([( + "mock_tile_1".to_string(), + vec![AdImage { + alt_text: None, + block_key: "key1".into(), + callbacks: AdCallbacks { + click: Url::parse("https://example.com/click").unwrap(), + impression: Url::parse("https://example.com/impression").unwrap(), + report: None, + }, + format: "billboard".to_string(), + image_url: "https://example.com/image.png".to_string(), + url: "https://example.com/ad".to_string(), + }], + )]), + }; + + // Should not panic + response.add_placement_info_to_report_callbacks(); + + let ad = &response.data["mock_tile_1"][0]; + assert!(ad.callbacks.report.is_none()); + assert!(ad.callbacks.click.query().is_none()); + assert!(ad.callbacks.impression.query().is_none()); + } + #[test] fn test_pop_request_hash_from_url() { let mut url_with_hash = From 8f97c77563cda1ee849c8df7c4c80c938146c600 Mon Sep 17 00:00:00 2001 From: ahanot Date: Thu, 2 Apr 2026 15:09:52 -0400 Subject: [PATCH 5/5] chore(ads-client): cargo fmt --- .../ads-client/integration-tests/tests/mars.rs | 6 +++++- components/ads-client/src/client/ad_response.rs | 16 ++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index a4fb8ac7a1..9859a7e5a3 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -105,7 +105,11 @@ fn test_record_impression() { .expect("mock_tile_1 placement should be present"); let result = client.record_impression(ad.callbacks.impression.to_string()); - assert!(result.is_ok(), "record_impression failed: {:?}", result.err()); + assert!( + result.is_ok(), + "record_impression failed: {:?}", + result.err() + ); } #[test] diff --git a/components/ads-client/src/client/ad_response.rs b/components/ads-client/src/client/ad_response.rs index dc998d14ea..03f7f80c52 100644 --- a/components/ads-client/src/client/ad_response.rs +++ b/components/ads-client/src/client/ad_response.rs @@ -520,11 +520,23 @@ mod tests { let ads = &response.data["mock_tile_1"]; - let report_0 = ads[0].callbacks.report.as_ref().unwrap().query().unwrap_or(""); + let report_0 = ads[0] + .callbacks + .report + .as_ref() + .unwrap() + .query() + .unwrap_or(""); assert!(report_0.contains("placement_id=mock_tile_1")); assert!(report_0.contains("position=0")); - let report_1 = ads[1].callbacks.report.as_ref().unwrap().query().unwrap_or(""); + let report_1 = ads[1] + .callbacks + .report + .as_ref() + .unwrap() + .query() + .unwrap_or(""); assert!(report_1.contains("placement_id=mock_tile_1")); assert!(report_1.contains("position=1")); }