Skip to content

Commit 47fa0cf

Browse files
authored
feat(ads-client): add reason parameter to report_ad per MARS /v1/t spec (#7268)
* 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. * fix(ads-client): remove unused import and fix formatting * 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. * 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. * chore(ads-client): cargo fmt * test(ads-client): add integration test verifying report URL has exactly one placement_id and position * refactor(ads-client): move report URL query param assertions into test_report_ad
1 parent a98d072 commit 47fa0cf

7 files changed

Lines changed: 256 additions & 15 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
### Ads Client
3232
- 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))
33+
- 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`.
3334

3435
### Logins
3536
- **BREAKING**: Removed `time_of_last_breach` field from `LoginMeta` and `Login`. This can be derived from Remote Settings during runtime instead.

components/ads-client/integration-tests/tests/mars.rs

Lines changed: 98 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,26 +6,27 @@
66
use std::sync::Arc;
77

88
use ads_client::{
9-
MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest, MozAdsPlacementRequestWithCount,
9+
MozAdsClientBuilder, MozAdsEnvironment, MozAdsPlacementRequest,
10+
MozAdsPlacementRequestWithCount, MozAdsReportReason,
1011
};
1112

1213
fn init_backend() {
1314
// Err means the backend is already initialized.
1415
let _ = viaduct_hyper::viaduct_init_backend_hyper();
1516
}
1617

17-
fn staging_client() -> ads_client::MozAdsClient {
18+
fn prod_client() -> ads_client::MozAdsClient {
1819
Arc::new(MozAdsClientBuilder::new())
19-
.environment(MozAdsEnvironment::Staging)
20+
.environment(MozAdsEnvironment::Prod)
2021
.build()
2122
}
2223

2324
#[test]
2425
#[ignore = "integration test: run manually with -- --ignored"]
25-
fn test_contract_image_staging() {
26+
fn test_contract_image_prod() {
2627
init_backend();
2728

28-
let client = staging_client();
29+
let client = prod_client();
2930
let result = client.request_image_ads(
3031
vec![MozAdsPlacementRequest {
3132
placement_id: "mock_billboard_1".to_string(),
@@ -45,10 +46,10 @@ fn test_contract_image_staging() {
4546

4647
#[test]
4748
#[ignore = "integration test: run manually with -- --ignored"]
48-
fn test_contract_spoc_staging() {
49+
fn test_contract_spoc_prod() {
4950
init_backend();
5051

51-
let client = staging_client();
52+
let client = prod_client();
5253
let result = client.request_spoc_ads(
5354
vec![MozAdsPlacementRequestWithCount {
5455
placement_id: "mock_spoc_1".to_string(),
@@ -66,10 +67,10 @@ fn test_contract_spoc_staging() {
6667

6768
#[test]
6869
#[ignore = "integration test: run manually with -- --ignored"]
69-
fn test_contract_tile_staging() {
70+
fn test_contract_tile_prod() {
7071
init_backend();
7172

72-
let client = staging_client();
73+
let client = prod_client();
7374
let result = client.request_tile_ads(
7475
vec![MozAdsPlacementRequest {
7576
placement_id: "mock_tile_1".to_string(),
@@ -82,3 +83,91 @@ fn test_contract_tile_staging() {
8283
let placements = result.unwrap();
8384
assert!(placements.contains_key("mock_tile_1"));
8485
}
86+
87+
#[test]
88+
#[ignore = "integration test: run manually with -- --ignored"]
89+
fn test_record_impression() {
90+
init_backend();
91+
92+
let client = prod_client();
93+
let placements = client
94+
.request_tile_ads(
95+
vec![MozAdsPlacementRequest {
96+
placement_id: "mock_tile_1".to_string(),
97+
iab_content: None,
98+
}],
99+
None,
100+
)
101+
.expect("tile ad request should succeed");
102+
103+
let ad = placements
104+
.get("mock_tile_1")
105+
.expect("mock_tile_1 placement should be present");
106+
107+
let result = client.record_impression(ad.callbacks.impression.to_string());
108+
assert!(
109+
result.is_ok(),
110+
"record_impression failed: {:?}",
111+
result.err()
112+
);
113+
}
114+
115+
#[test]
116+
#[ignore = "integration test: run manually with -- --ignored"]
117+
fn test_record_click() {
118+
init_backend();
119+
120+
let client = prod_client();
121+
let placements = client
122+
.request_tile_ads(
123+
vec![MozAdsPlacementRequest {
124+
placement_id: "mock_tile_1".to_string(),
125+
iab_content: None,
126+
}],
127+
None,
128+
)
129+
.expect("tile ad request should succeed");
130+
131+
let ad = placements
132+
.get("mock_tile_1")
133+
.expect("mock_tile_1 placement should be present");
134+
135+
let result = client.record_click(ad.callbacks.click.to_string());
136+
assert!(result.is_ok(), "record_click failed: {:?}", result.err());
137+
}
138+
139+
#[test]
140+
#[ignore = "integration test: run manually with -- --ignored"]
141+
fn test_report_ad() {
142+
init_backend();
143+
144+
let client = prod_client();
145+
let placements = client
146+
.request_tile_ads(
147+
vec![MozAdsPlacementRequest {
148+
placement_id: "mock_tile_1".to_string(),
149+
iab_content: None,
150+
}],
151+
None,
152+
)
153+
.expect("tile ad request should succeed");
154+
155+
let ad = placements
156+
.get("mock_tile_1")
157+
.expect("mock_tile_1 placement should be present");
158+
159+
let report_url = ad
160+
.callbacks
161+
.report
162+
.as_ref()
163+
.expect("mock_tile_1 should have a report URL");
164+
165+
let pairs: Vec<(_, _)> = report_url.query_pairs().collect();
166+
let placement_id_count = pairs.iter().filter(|(k, _)| k == "placement_id").count();
167+
let position_count = pairs.iter().filter(|(k, _)| k == "position").count();
168+
assert_eq!(placement_id_count, 1, "expected exactly one placement_id");
169+
assert_eq!(position_count, 1, "expected exactly one position");
170+
171+
let result = client.report_ad(report_url.to_string(), MozAdsReportReason::NotInterested);
172+
assert!(result.is_ok(), "report_ad failed: {:?}", result.err());
173+
}

components/ads-client/src/client.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,23 @@ use std::time::Duration;
99
use crate::client::ad_response::{AdImage, AdResponse, AdResponseValue, AdSpoc, AdTile};
1010
use crate::client::config::{AdsClientConfig, Environment};
1111
use crate::error::{RecordClickError, RecordImpressionError, ReportAdError, RequestAdsError};
12+
13+
#[derive(Clone, Copy, Debug, PartialEq)]
14+
pub enum ReportReason {
15+
Inappropriate,
16+
NotInterested,
17+
SeenTooManyTimes,
18+
}
19+
20+
impl ReportReason {
21+
pub fn as_str(&self) -> &'static str {
22+
match self {
23+
ReportReason::Inappropriate => "inappropriate",
24+
ReportReason::NotInterested => "not_interested",
25+
ReportReason::SeenTooManyTimes => "seen_too_many_times",
26+
}
27+
}
28+
}
1229
use crate::http_cache::{HttpCache, RequestCachePolicy};
1330
use crate::mars::MARSClient;
1431
use crate::telemetry::Telemetry;
@@ -115,6 +132,7 @@ where
115132
let cache_policy = options.unwrap_or_default();
116133
let (mut response, request_hash) = self.client.fetch_ads::<A>(ad_request, &cache_policy)?;
117134
response.add_request_hash_to_callbacks(&request_hash);
135+
response.add_placement_info_to_report_callbacks();
118136
Ok(response)
119137
}
120138

@@ -199,9 +217,9 @@ where
199217
})
200218
}
201219

202-
pub fn report_ad(&self, report_url: Url) -> Result<(), ReportAdError> {
220+
pub fn report_ad(&self, report_url: Url, reason: ReportReason) -> Result<(), ReportAdError> {
203221
self.client
204-
.report_ad(report_url)
222+
.report_ad(report_url, reason)
205223
.inspect_err(|e| {
206224
self.telemetry.record(e);
207225
})

components/ads-client/src/client/ad_response.rs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,19 @@ impl<A: AdResponseValue> AdResponse<A> {
6060
}
6161
}
6262

63+
pub fn add_placement_info_to_report_callbacks(&mut self) {
64+
for (placement_id, ads) in self.data.iter_mut() {
65+
for (position, ad) in ads.iter_mut().enumerate() {
66+
if let Some(report_url) = ad.callbacks_mut().report.as_mut() {
67+
report_url
68+
.query_pairs_mut()
69+
.append_pair("placement_id", placement_id)
70+
.append_pair("position", &position.to_string());
71+
}
72+
}
73+
}
74+
}
75+
6376
pub fn take_first(self) -> HashMap<String, A> {
6477
self.data
6578
.into_iter()
@@ -469,6 +482,94 @@ mod tests {
469482
.contains("request_hash=abc123def456"));
470483
}
471484

485+
#[test]
486+
fn test_add_placement_info_to_report_callbacks() {
487+
let mut response = AdResponse {
488+
data: HashMap::from([(
489+
"mock_tile_1".to_string(),
490+
vec![
491+
AdImage {
492+
alt_text: None,
493+
block_key: "key1".into(),
494+
callbacks: AdCallbacks {
495+
click: Url::parse("https://example.com/click1").unwrap(),
496+
impression: Url::parse("https://example.com/impression1").unwrap(),
497+
report: Some(Url::parse("https://example.com/report").unwrap()),
498+
},
499+
format: "billboard".to_string(),
500+
image_url: "https://example.com/image1.png".to_string(),
501+
url: "https://example.com/ad1".to_string(),
502+
},
503+
AdImage {
504+
alt_text: None,
505+
block_key: "key2".into(),
506+
callbacks: AdCallbacks {
507+
click: Url::parse("https://example.com/click2").unwrap(),
508+
impression: Url::parse("https://example.com/impression2").unwrap(),
509+
report: Some(Url::parse("https://example.com/report").unwrap()),
510+
},
511+
format: "billboard".to_string(),
512+
image_url: "https://example.com/image2.png".to_string(),
513+
url: "https://example.com/ad2".to_string(),
514+
},
515+
],
516+
)]),
517+
};
518+
519+
response.add_placement_info_to_report_callbacks();
520+
521+
let ads = &response.data["mock_tile_1"];
522+
523+
let report_0 = ads[0]
524+
.callbacks
525+
.report
526+
.as_ref()
527+
.unwrap()
528+
.query()
529+
.unwrap_or("");
530+
assert!(report_0.contains("placement_id=mock_tile_1"));
531+
assert!(report_0.contains("position=0"));
532+
533+
let report_1 = ads[1]
534+
.callbacks
535+
.report
536+
.as_ref()
537+
.unwrap()
538+
.query()
539+
.unwrap_or("");
540+
assert!(report_1.contains("placement_id=mock_tile_1"));
541+
assert!(report_1.contains("position=1"));
542+
}
543+
544+
#[test]
545+
fn test_add_placement_info_skips_ads_without_report_url() {
546+
let mut response = AdResponse {
547+
data: HashMap::from([(
548+
"mock_tile_1".to_string(),
549+
vec![AdImage {
550+
alt_text: None,
551+
block_key: "key1".into(),
552+
callbacks: AdCallbacks {
553+
click: Url::parse("https://example.com/click").unwrap(),
554+
impression: Url::parse("https://example.com/impression").unwrap(),
555+
report: None,
556+
},
557+
format: "billboard".to_string(),
558+
image_url: "https://example.com/image.png".to_string(),
559+
url: "https://example.com/ad".to_string(),
560+
}],
561+
)]),
562+
};
563+
564+
// Should not panic
565+
response.add_placement_info_to_report_callbacks();
566+
567+
let ad = &response.data["mock_tile_1"][0];
568+
assert!(ad.callbacks.report.is_none());
569+
assert!(ad.callbacks.click.query().is_none());
570+
assert!(ad.callbacks.impression.query().is_none());
571+
}
572+
472573
#[test]
473574
fn test_pop_request_hash_from_url() {
474575
let mut url_with_hash =

components/ads-client/src/ffi.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use crate::client::ad_response::{
1313
};
1414
use crate::client::config::{AdsCacheConfig, AdsClientConfig, Environment};
1515
use crate::client::AdsClient;
16+
use crate::client::ReportReason;
1617
use crate::error::ComponentError;
1718
use crate::ffi::telemetry::MozAdsTelemetryWrapper;
1819
use crate::http_cache::{CacheMode, RequestCachePolicy};
@@ -191,6 +192,23 @@ pub struct MozAdsRequestCachePolicy {
191192
pub ttl_seconds: Option<u64>,
192193
}
193194

195+
#[derive(Clone, Copy, Debug, uniffi::Enum)]
196+
pub enum MozAdsReportReason {
197+
Inappropriate,
198+
NotInterested,
199+
SeenTooManyTimes,
200+
}
201+
202+
impl From<MozAdsReportReason> for ReportReason {
203+
fn from(reason: MozAdsReportReason) -> Self {
204+
match reason {
205+
MozAdsReportReason::Inappropriate => ReportReason::Inappropriate,
206+
MozAdsReportReason::NotInterested => ReportReason::NotInterested,
207+
MozAdsReportReason::SeenTooManyTimes => ReportReason::SeenTooManyTimes,
208+
}
209+
}
210+
}
211+
194212
#[derive(Clone, Copy, Debug, Default, uniffi::Enum)]
195213
pub enum MozAdsCacheMode {
196214
#[default]

components/ads-client/src/lib.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,17 @@ impl MozAdsClient {
111111
}
112112

113113
#[handle_error(ComponentError)]
114-
pub fn report_ad(&self, report_url: String) -> AdsClientApiResult<()> {
114+
pub fn report_ad(
115+
&self,
116+
report_url: String,
117+
reason: MozAdsReportReason,
118+
) -> AdsClientApiResult<()> {
115119
let url = AdsClientUrl::parse(&report_url)
116120
.map_err(|e| ComponentError::ReportAd(CallbackRequestError::InvalidUrl(e).into()))?;
117121
let inner = self.inner.lock();
118-
inner.report_ad(url).map_err(ComponentError::ReportAd)
122+
inner
123+
.report_ad(url, reason.into())
124+
.map_err(ComponentError::ReportAd)
119125
}
120126

121127
pub fn clear_cache(&self) -> AdsClientApiResult<()> {

0 commit comments

Comments
 (0)