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/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index 24a12da4bc..9859a7e5a3 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,85 @@ 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()); +} diff --git a/components/ads-client/src/client.rs b/components/ads-client/src/client.rs index 01503a22cc..c9331c4dfb 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; @@ -115,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) } @@ -199,9 +217,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/client/ad_response.rs b/components/ads-client/src/client/ad_response.rs index 8cf72206fc..03f7f80c52 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,94 @@ 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 = diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index 961b3c322c..eb6e2fd8be 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -13,6 +13,7 @@ use crate::client::ad_response::{ }; 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}; @@ -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..1b9fcd6fc0 100644 --- a/components/ads-client/src/mars.rs +++ b/components/ads-client/src/mars.rs @@ -7,6 +7,7 @@ use crate::{ client::{ ad_request::AdRequest, ad_response::{AdResponse, AdResponseValue}, + ReportReason, }, error::{ check_http_status_for_error, CallbackRequestError, FetchAdsError, RecordClickError, @@ -90,7 +91,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 +146,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 +159,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()); }