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 @@ -37,6 +37,8 @@
* Enable staging environment support for all platforms (previously feature-gated)
* Temporarily disable cache invalidation on click and impression recording (will be re-enabled behind Nimbus experiment)
* Enable automatic context_id rotation every 3 days
* **BREAKING**: Removed `cycle_context_id()` API method - context_id rotation is now automatic
* Modified HTTP cache to ignore `context_id` field in request bodies when generating cache keys, preventing unnecessary cache invalidation on rotation

### Android
* Upgraded Kotlin compiler from 2.2.21 to 2.3.0 ([#7183](https://github.com/mozilla/application-services/pull/7183))
Expand Down
1 change: 0 additions & 1 deletion components/ads-client/docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ let client = MozAdsClientBuilder::new()
| Method | Return Type | Description |
| ----------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `clear_cache(&self)` | `AdsClientApiResult<()>` | Clears the client's HTTP cache. Returns an error if clearing fails. |
| `cycle_context_id(&self)` | `AdsClientApiResult<String>` | Rotates the client's context ID and returns the **previous** ID. |
| `record_click(&self, click_url: String)` | `AdsClientApiResult<()>` | Records a click using the provided callback URL (typically from `ad.callbacks.click`). |
| `record_impression(&self, impression_url: String)` | `AdsClientApiResult<()>` | Records an impression using the provided callback URL (typically from `ad.callbacks.impression`). |
| `report_ad(&self, report_url: String)` | `AdsClientApiResult<()>` | Reports an ad using the provided callback URL (typically from `ad.callbacks.report`). |
Expand Down
22 changes: 0 additions & 22 deletions components/ads-client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,13 +205,6 @@ where
self.context_id_component.request(3)
}

pub fn cycle_context_id(&mut self) -> context_id::ApiResult<String> {
let old_context_id = self.get_context_id()?;
self.context_id_component.force_rotation()?;
let _ = self.client.clear_cache();
Ok(old_context_id)
}

pub fn clear_cache(&self) -> Result<(), HttpCacheError> {
self.client.clear_cache()
}
Expand Down Expand Up @@ -267,21 +260,6 @@ mod tests {
assert!(!context_id.is_empty());
}

#[test]
fn test_cycle_context_id() {
let config = AdsClientConfig {
environment: Environment::Test,
cache_config: None,
telemetry: MozAdsTelemetryWrapper::noop(),
};
let mut client = AdsClient::new(config);
let old_id = client.get_context_id().unwrap();
let previous_id = client.cycle_context_id().unwrap();
assert_eq!(previous_id, old_id);
let new_id = client.get_context_id().unwrap();
assert_ne!(new_id, old_id);
}

#[test]
fn test_request_image_ads_happy() {
viaduct_dev::init_backend_dev();
Expand Down
77 changes: 76 additions & 1 deletion components/ads-client/src/http_cache/request_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,22 @@ impl From<&Request> for RequestHash {
header.value.hash(&mut hasher);
}

request.body.hash(&mut hasher);
// Strip context_id before hashing — it rotates on client re-instantiation
// and should not invalidate cached responses.
// NOTE: This couples ads-client domain logic to the cache. When the cache
// is extracted into its own module, this should move to the caller side.
let body_for_hash = request.body.as_deref().and_then(|bytes| {
serde_json::from_slice::<serde_json::Value>(bytes)
.ok()
.map(|mut value| {
if let Some(obj) = value.as_object_mut() {
obj.remove("context_id");
}
serde_json::to_vec(&value).unwrap_or_else(|_| bytes.to_vec())
})
});
body_for_hash.hash(&mut hasher);

RequestHash(format!("{:x}", hasher.finish()))
}
}
Expand Down Expand Up @@ -118,6 +133,66 @@ mod tests {
assert_eq!(h_req1.to_string(), h_req3.to_string());
}

#[test]
fn test_context_id_ignored_in_hash() {
let url = "https://example.com/ads";
let body1 = serde_json::to_vec(&serde_json::json!({
"context_id": "aaaa-bbbb-cccc",
"placements": [{"placement": "tile_1", "count": 1}]
}))
.unwrap();
let body2 = serde_json::to_vec(&serde_json::json!({
"context_id": "dddd-eeee-ffff",
"placements": [{"placement": "tile_1", "count": 1}]
}))
.unwrap();

let req1 = Request {
method: Method::Post,
url: url.parse().unwrap(),
headers: Headers::new(),
body: Some(body1),
};
let req2 = Request {
method: Method::Post,
url: url.parse().unwrap(),
headers: Headers::new(),
body: Some(body2),
};

assert_eq!(RequestHash::from(&req1), RequestHash::from(&req2));
}

#[test]
fn test_different_placements_produce_different_hash() {
let url = "https://example.com/ads";
let body1 = serde_json::to_vec(&serde_json::json!({
"context_id": "same-id",
"placements": [{"placement": "tile_1", "count": 1}]
}))
.unwrap();
let body2 = serde_json::to_vec(&serde_json::json!({
"context_id": "same-id",
"placements": [{"placement": "tile_2", "count": 3}]
}))
.unwrap();

let req1 = Request {
method: Method::Post,
url: url.parse().unwrap(),
headers: Headers::new(),
body: Some(body1),
};
let req2 = Request {
method: Method::Post,
url: url.parse().unwrap(),
headers: Headers::new(),
body: Some(body2),
};

assert_ne!(RequestHash::from(&req1), RequestHash::from(&req2));
}

#[test]
fn test_request_hash_from_string() {
let hash_str = "abc123def456";
Expand Down
6 changes: 0 additions & 6 deletions components/ads-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,6 @@ impl MozAdsClient {
inner.report_ad(url).map_err(ComponentError::ReportAd)
}

pub fn cycle_context_id(&self) -> AdsClientApiResult<String> {
let mut inner = self.inner.lock();
let previous_context_id = inner.cycle_context_id()?;
Ok(previous_context_id)
}

pub fn clear_cache(&self) -> AdsClientApiResult<()> {
let inner = self.inner.lock();
inner
Expand Down