From 1d629b3ea9eb78cc3711dfe3e027befd76f03846 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Thu, 15 Oct 2026 13:14:14 -0700 Subject: [PATCH 1/6] fix: adds request_spoc_or_image_ads --- .../integration-tests/tests/mars.rs | 49 +++++++++++++++++++ components/ads-client/src/ads.rs | 7 +++ components/ads-client/src/client.rs | 32 +++++++++++- components/ads-client/src/ffi.rs | 19 ++++++- components/ads-client/src/lib.rs | 23 +++++++++ components/ads-client/src/mars/ad_response.rs | 18 ++++++- 6 files changed, 145 insertions(+), 3 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index 2b0b109712..ef66facdcd 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -21,6 +21,12 @@ fn prod_client() -> ads_client::MozAdsClient { .build() } +fn staging_client() -> ads_client::MozAdsClient { + Arc::new(MozAdsClientBuilder::new()) + .environment(MozAdsEnvironment::Staging) + .build() +} + #[test] #[ignore = "integration test: run manually with -- --ignored"] fn test_contract_image_prod() { @@ -241,3 +247,46 @@ fn test_contract_tile_ohttp_prod() { "OHTTP response should contain mock_tile_1" ); } + +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_contract_newtab_stories_1_stage() { + init_backend(); + + // Use staging client, because this does not use a mock placement_id. + let client = staging_client(); + + // + let result = client.request_spoc_ads( + vec![MozAdsPlacementRequestWithCount { + count: 3, + iab_content: None, + placement_id: "newtab_stories_1".to_string(), + }], + None, + ); + + assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); + let placements = result.unwrap(); + assert!(placements.contains_key("newtab_stories_1")); + + // Note: this particular return is due to a bug with `newtab_stories_1`- it returns 3 ads, but only two of which are spoc. + // The image ad gets dropped, which is expected. + assert!(placements.get("newtab_stories_1").unwrap().len() == 2); + + // The existence of the function `request_spoc_or_image_ads` is a band-aid fix that allows all 3 ads to be returned. + let result = client.request_spoc_or_image_ads( + vec![MozAdsPlacementRequestWithCount { + count: 3, + iab_content: None, + placement_id: "newtab_stories_1".to_string(), + }], + None, + ); + + assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); + let placements = result.unwrap(); + assert!(placements.contains_key("newtab_stories_1")); + assert!(placements.get("newtab_stories_1").unwrap().len() == 3); + +} diff --git a/components/ads-client/src/ads.rs b/components/ads-client/src/ads.rs index 861061c5ac..efc7f4f5cd 100644 --- a/components/ads-client/src/ads.rs +++ b/components/ads-client/src/ads.rs @@ -88,6 +88,13 @@ pub struct AdTile { pub url: Url, } +#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[serde(untagged)] +pub enum AdImageOrSpoc { + Image(AdImage), + Spoc(AdSpoc), +} + #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct SpocFrequencyCaps { pub cap_key: String, diff --git a/components/ads-client/src/client.rs b/components/ads-client/src/client.rs index e777ff38bb..8b645c5c97 100644 --- a/components/ads-client/src/client.rs +++ b/components/ads-client/src/client.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::ads::{AdImage, AdSpoc, AdTile}; +use crate::ads::{AdImage, AdImageOrSpoc, AdSpoc, AdTile}; #[cfg(feature = "stateful")] use crate::ads_store::AdsStore; use crate::bytesize::ByteSize; @@ -232,6 +232,36 @@ impl AdsClient { }) } + // Sometimes, spoc array requests can return multiple types of ads for specific placements. + // This allows both types to be returned without error (as instances of the enum `AdImageOrSpoc`) + // TODO: This, as well as the other synchronous functions, are to be deprecated + // when the stateful client rework is complete. + pub fn request_spoc_or_image_ads( + &self, + ad_placement_requests: Vec, + flags: AdRequestFlags, + options: Option, + ohttp: bool, + blocks: Vec, + ) -> Result>, RequestAdsError> { + let result = self.request_ads::( + ad_placement_requests, + flags, + options, + ohttp, + blocks, + AdRequestRelevance::default(), + ); + result + .inspect_err(|e| { + telemetry::record_client_error(ClientOperation::RequestAds, e); + }) + .map(|response| { + telemetry::record_client_operation(ClientOperation::RequestAds); + response.data + }) + } + pub fn request_tile_ads( &self, ad_placement_requests: Vec, diff --git a/components/ads-client/src/ffi.rs b/components/ads-client/src/ffi.rs index b84c9cd602..de57508ecd 100644 --- a/components/ads-client/src/ffi.rs +++ b/components/ads-client/src/ffi.rs @@ -5,7 +5,9 @@ pub mod error; pub mod telemetry; -use crate::ads::{AdCallbacks, AdImage, AdSpoc, AdTile, SpocFrequencyCaps, SpocRanking}; +use crate::ads::{ + AdCallbacks, AdImage, AdImageOrSpoc, AdSpoc, AdTile, SpocFrequencyCaps, SpocRanking, +}; #[cfg(feature = "stateful")] use crate::client::config::AdsStoreConfig; use crate::client::config::{AdsCacheConfig, AdsClientConfig}; @@ -241,6 +243,12 @@ pub struct MozAdsSpoc { pub url: AdsClientUrl, } +#[derive(Debug, PartialEq, uniffi::Enum)] +pub enum MozAdsImageOrSpoc { + Image(MozAdsImage), + Spoc(MozAdsSpoc), +} + #[derive(Debug, PartialEq, uniffi::Record)] pub struct MozAdsSpocFrequencyCaps { pub cap_key: String, @@ -303,6 +311,15 @@ impl From for MozAdsSpocRanking { } } +impl From for MozAdsImageOrSpoc { + fn from(ad: AdImageOrSpoc) -> Self { + match ad { + AdImageOrSpoc::Image(ad) => MozAdsImageOrSpoc::Image(ad.into()), + AdImageOrSpoc::Spoc(ad) => MozAdsImageOrSpoc::Spoc(ad.into()), + } + } +} + impl From for MozAdsImage { fn from(img: AdImage) -> Self { Self { diff --git a/components/ads-client/src/lib.rs b/components/ads-client/src/lib.rs index 46a75d6a07..5399eb611f 100644 --- a/components/ads-client/src/lib.rs +++ b/components/ads-client/src/lib.rs @@ -144,6 +144,29 @@ impl MozAdsClient { .collect()) } + #[handle_error(ComponentError)] + #[uniffi::method(default(options = None))] + pub fn request_spoc_or_image_ads( + &self, + moz_ad_requests: Vec, + options: Option, + ) -> AdsClientApiResult>> { + let requests: Vec = moz_ad_requests.iter().map(|r| r.into()).collect(); + let options = options.unwrap_or_default(); + let flags = AdRequestFlags::from(&options); + let ohttp = options.ohttp; + let cache_policy = options.cache_policy.map(CachePolicy::from); + let blocks = options.blocks; + let response = self + .inner + .request_spoc_or_image_ads(requests, flags, cache_policy, ohttp, blocks) + .map_err(ComponentError::RequestAds)?; + Ok(response + .into_iter() + .map(|(k, v)| (k, v.into_iter().map(|spoc| spoc.into()).collect())) + .collect()) + } + #[handle_error(ComponentError)] #[uniffi::method(default(options = None))] pub fn request_tile_ads( diff --git a/components/ads-client/src/mars/ad_response.rs b/components/ads-client/src/mars/ad_response.rs index e6d8c804bf..a84ed749da 100644 --- a/components/ads-client/src/mars/ad_response.rs +++ b/components/ads-client/src/mars/ad_response.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::ads::{AdCallbacks, AdImage, AdSpoc, AdTile}; +use crate::ads::{AdCallbacks, AdImage, AdImageOrSpoc, AdSpoc, AdTile}; use crate::http_cache::RequestHash; use crate::telemetry; use serde::de::DeserializeOwned; @@ -102,6 +102,22 @@ impl AdResponseValue for AdSpoc { } } +impl AdResponseValue for AdImageOrSpoc { + fn callbacks_mut(&mut self) -> &mut AdCallbacks { + match self { + AdImageOrSpoc::Image(ad) => &mut ad.callbacks, + AdImageOrSpoc::Spoc(ad) => &mut ad.callbacks, + } + } + + fn cap_key(&self) -> Option { + match self { + AdImageOrSpoc::Image(_ad) => None, + AdImageOrSpoc::Spoc(ad) => Some(ad.caps.cap_key.clone()), + } + } +} + impl AdResponseValue for AdTile { fn callbacks_mut(&mut self) -> &mut AdCallbacks { &mut self.callbacks From 3c33dfacf402c80e4aba829550c7bb05a6ba602f Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Thu, 15 Oct 2026 13:27:02 -0700 Subject: [PATCH 2/6] fix: fmting --- components/ads-client/integration-tests/tests/mars.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index ef66facdcd..30210a2ef2 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -256,7 +256,6 @@ fn test_contract_newtab_stories_1_stage() { // Use staging client, because this does not use a mock placement_id. let client = staging_client(); - // let result = client.request_spoc_ads( vec![MozAdsPlacementRequestWithCount { count: 3, @@ -288,5 +287,4 @@ fn test_contract_newtab_stories_1_stage() { let placements = result.unwrap(); assert!(placements.contains_key("newtab_stories_1")); assert!(placements.get("newtab_stories_1").unwrap().len() == 3); - } From 550c3206112ec4cd44b397342ad0ca3e5cecccb1 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Thu, 15 Oct 2026 14:25:53 -0700 Subject: [PATCH 3/6] fix: removed potentially failing test --- .../ads-client/integration-tests/tests/mars.rs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index 30210a2ef2..b10de155a6 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -256,23 +256,8 @@ fn test_contract_newtab_stories_1_stage() { // Use staging client, because this does not use a mock placement_id. let client = staging_client(); - let result = client.request_spoc_ads( - vec![MozAdsPlacementRequestWithCount { - count: 3, - iab_content: None, - placement_id: "newtab_stories_1".to_string(), - }], - None, - ); - - assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); - let placements = result.unwrap(); - assert!(placements.contains_key("newtab_stories_1")); - // Note: this particular return is due to a bug with `newtab_stories_1`- it returns 3 ads, but only two of which are spoc. - // The image ad gets dropped, which is expected. - assert!(placements.get("newtab_stories_1").unwrap().len() == 2); - + // The image ad gets dropped with `request_spoc_ads`, which is expected. // The existence of the function `request_spoc_or_image_ads` is a band-aid fix that allows all 3 ads to be returned. let result = client.request_spoc_or_image_ads( vec![MozAdsPlacementRequestWithCount { From 096d64aea3c35dac0dd58dfd5f18d3eb8c5ddbb0 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Thu, 15 Oct 2026 14:27:21 -0700 Subject: [PATCH 4/6] fix: changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6123c01e51..27477cf98c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ ### Ads-Client - Telemetry is now a process-wide singleton internally; the binding API is unchanged, but the `MozAdsTelemetry` callback now receives metrics from every client in the process. +- `request_spoc_or_image_ads` is added to return ads that may ambiguously be spocs or images. ### Nimbus From dc66e60a0f12ad274d5fab28a00b91dfd91776d1 Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Fri, 16 Oct 2026 11:17:58 -0700 Subject: [PATCH 5/6] fix: removes and moves test --- .../integration-tests/tests/mars.rs | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index b10de155a6..3dcae9b386 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -21,12 +21,6 @@ fn prod_client() -> ads_client::MozAdsClient { .build() } -fn staging_client() -> ads_client::MozAdsClient { - Arc::new(MozAdsClientBuilder::new()) - .environment(MozAdsEnvironment::Staging) - .build() -} - #[test] #[ignore = "integration test: run manually with -- --ignored"] fn test_contract_image_prod() { @@ -104,6 +98,30 @@ fn test_contract_spoc_prod() { assert!(placements.get("mock_spoc_1").unwrap().len() == 3); } +// Note: this test does not actually test a mock placement that returns spoc *and* image ads, as no such placement currently exists, +// and direct staging calls may be stochastic (depends on inventory, etc.). +// TODO: When such a mock placement exists, add a test for it. +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_contract_spoc_or_image_prod() { + init_backend(); + + let client = prod_client(); + let result = client.request_spoc_or_image_ads( + vec![MozAdsPlacementRequestWithCount { + count: 3, + iab_content: None, + placement_id: "mock_spoc_1".to_string(), + }], + None, + ); + + assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); + let placements = result.unwrap(); + assert!(placements.contains_key("mock_spoc_1")); + assert!(placements.get("mock_spoc_1").unwrap().len() == 3); +} + #[test] #[ignore = "integration test: run manually with -- --ignored"] fn test_contract_tile_prod() { @@ -247,29 +265,3 @@ fn test_contract_tile_ohttp_prod() { "OHTTP response should contain mock_tile_1" ); } - -#[test] -#[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_newtab_stories_1_stage() { - init_backend(); - - // Use staging client, because this does not use a mock placement_id. - let client = staging_client(); - - // Note: this particular return is due to a bug with `newtab_stories_1`- it returns 3 ads, but only two of which are spoc. - // The image ad gets dropped with `request_spoc_ads`, which is expected. - // The existence of the function `request_spoc_or_image_ads` is a band-aid fix that allows all 3 ads to be returned. - let result = client.request_spoc_or_image_ads( - vec![MozAdsPlacementRequestWithCount { - count: 3, - iab_content: None, - placement_id: "newtab_stories_1".to_string(), - }], - None, - ); - - assert!(result.is_ok(), "Spoc ad request failed: {:?}", result.err()); - let placements = result.unwrap(); - assert!(placements.contains_key("newtab_stories_1")); - assert!(placements.get("newtab_stories_1").unwrap().len() == 3); -} From 797e17ef3c465c5e6a163672ecb6137acf96076b Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Fri, 16 Oct 2026 11:36:42 -0700 Subject: [PATCH 6/6] fix: adds one more test --- .../integration-tests/tests/mars.rs | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/components/ads-client/integration-tests/tests/mars.rs b/components/ads-client/integration-tests/tests/mars.rs index 3dcae9b386..77a01a09ba 100644 --- a/components/ads-client/integration-tests/tests/mars.rs +++ b/components/ads-client/integration-tests/tests/mars.rs @@ -103,7 +103,7 @@ fn test_contract_spoc_prod() { // TODO: When such a mock placement exists, add a test for it. #[test] #[ignore = "integration test: run manually with -- --ignored"] -fn test_contract_spoc_or_image_prod() { +fn test_contract_spoc_or_image_spocs_prod() { init_backend(); let client = prod_client(); @@ -122,6 +122,33 @@ fn test_contract_spoc_or_image_prod() { assert!(placements.get("mock_spoc_1").unwrap().len() == 3); } +// Note: this test does not actually test a mock placement that returns spoc *and* image ads, as no such placement currently exists, +// and direct staging calls may be stochastic (depends on inventory, etc.). +// TODO: When such a mock placement exists, add a test for it. +#[test] +#[ignore = "integration test: run manually with -- --ignored"] +fn test_contract_spoc_or_image_images_prod() { + init_backend(); + + let client = prod_client(); + let result = client.request_spoc_or_image_ads( + vec![MozAdsPlacementRequestWithCount { + count: 1, + iab_content: None, + placement_id: "mock_billboard_1".to_string(), + }], + None, + ); + + assert!( + result.is_ok(), + "Image ad request failed: {:?}", + result.err() + ); + let placements = result.unwrap(); + assert!(placements.contains_key("mock_billboard_1")); +} + #[test] #[ignore = "integration test: run manually with -- --ignored"] fn test_contract_tile_prod() {