Skip to content

Commit 5a8684c

Browse files
Almajualexcottner
authored andcommitted
feat(ads-client): ignore context_id in HTTP cache key generation (#7214)
Remove cycle_context_id() API and modify cache to ignore context_id field in request bodies when generating cache keys. This prevents unnecessary cache invalidation when context_id rotates automatically. - Remove cycle_context_id() public API (breaking change) - Strip context_id from request body before hashing for cache key - Add tests for context_id-agnostic caching behavior - Update documentation to remove cycle_context_id() method
1 parent 8e6987b commit 5a8684c

File tree

5 files changed

+78
-30
lines changed

5 files changed

+78
-30
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@
3737
* Enable staging environment support for all platforms (previously feature-gated)
3838
* Temporarily disable cache invalidation on click and impression recording (will be re-enabled behind Nimbus experiment)
3939
* Enable automatic context_id rotation every 3 days
40+
* **BREAKING**: Removed `cycle_context_id()` API method - context_id rotation is now automatic
41+
* Modified HTTP cache to ignore `context_id` field in request bodies when generating cache keys, preventing unnecessary cache invalidation on rotation
4042

4143
### Android
4244
* Upgraded Kotlin compiler from 2.2.21 to 2.3.0 ([#7183](https://github.com/mozilla/application-services/pull/7183))

components/ads-client/docs/usage.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ let client = MozAdsClientBuilder::new()
5252
| Method | Return Type | Description |
5353
| ----------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
5454
| `clear_cache(&self)` | `AdsClientApiResult<()>` | Clears the client's HTTP cache. Returns an error if clearing fails. |
55-
| `cycle_context_id(&self)` | `AdsClientApiResult<String>` | Rotates the client's context ID and returns the **previous** ID. |
5655
| `record_click(&self, click_url: String)` | `AdsClientApiResult<()>` | Records a click using the provided callback URL (typically from `ad.callbacks.click`). |
5756
| `record_impression(&self, impression_url: String)` | `AdsClientApiResult<()>` | Records an impression using the provided callback URL (typically from `ad.callbacks.impression`). |
5857
| `report_ad(&self, report_url: String)` | `AdsClientApiResult<()>` | Reports an ad using the provided callback URL (typically from `ad.callbacks.report`). |

components/ads-client/src/client.rs

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,6 @@ where
205205
self.context_id_component.request(3)
206206
}
207207

208-
pub fn cycle_context_id(&mut self) -> context_id::ApiResult<String> {
209-
let old_context_id = self.get_context_id()?;
210-
self.context_id_component.force_rotation()?;
211-
let _ = self.client.clear_cache();
212-
Ok(old_context_id)
213-
}
214-
215208
pub fn clear_cache(&self) -> Result<(), HttpCacheError> {
216209
self.client.clear_cache()
217210
}
@@ -267,21 +260,6 @@ mod tests {
267260
assert!(!context_id.is_empty());
268261
}
269262

270-
#[test]
271-
fn test_cycle_context_id() {
272-
let config = AdsClientConfig {
273-
environment: Environment::Test,
274-
cache_config: None,
275-
telemetry: MozAdsTelemetryWrapper::noop(),
276-
};
277-
let mut client = AdsClient::new(config);
278-
let old_id = client.get_context_id().unwrap();
279-
let previous_id = client.cycle_context_id().unwrap();
280-
assert_eq!(previous_id, old_id);
281-
let new_id = client.get_context_id().unwrap();
282-
assert_ne!(new_id, old_id);
283-
}
284-
285263
#[test]
286264
fn test_request_image_ads_happy() {
287265
viaduct_dev::init_backend_dev();

components/ads-client/src/http_cache/request_hash.rs

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,22 @@ impl From<&Request> for RequestHash {
2222
header.value.hash(&mut hasher);
2323
}
2424

25-
request.body.hash(&mut hasher);
25+
// Strip context_id before hashing — it rotates on client re-instantiation
26+
// and should not invalidate cached responses.
27+
// NOTE: This couples ads-client domain logic to the cache. When the cache
28+
// is extracted into its own module, this should move to the caller side.
29+
let body_for_hash = request.body.as_deref().and_then(|bytes| {
30+
serde_json::from_slice::<serde_json::Value>(bytes)
31+
.ok()
32+
.map(|mut value| {
33+
if let Some(obj) = value.as_object_mut() {
34+
obj.remove("context_id");
35+
}
36+
serde_json::to_vec(&value).unwrap_or_else(|_| bytes.to_vec())
37+
})
38+
});
39+
body_for_hash.hash(&mut hasher);
40+
2641
RequestHash(format!("{:x}", hasher.finish()))
2742
}
2843
}
@@ -118,6 +133,66 @@ mod tests {
118133
assert_eq!(h_req1.to_string(), h_req3.to_string());
119134
}
120135

136+
#[test]
137+
fn test_context_id_ignored_in_hash() {
138+
let url = "https://example.com/ads";
139+
let body1 = serde_json::to_vec(&serde_json::json!({
140+
"context_id": "aaaa-bbbb-cccc",
141+
"placements": [{"placement": "tile_1", "count": 1}]
142+
}))
143+
.unwrap();
144+
let body2 = serde_json::to_vec(&serde_json::json!({
145+
"context_id": "dddd-eeee-ffff",
146+
"placements": [{"placement": "tile_1", "count": 1}]
147+
}))
148+
.unwrap();
149+
150+
let req1 = Request {
151+
method: Method::Post,
152+
url: url.parse().unwrap(),
153+
headers: Headers::new(),
154+
body: Some(body1),
155+
};
156+
let req2 = Request {
157+
method: Method::Post,
158+
url: url.parse().unwrap(),
159+
headers: Headers::new(),
160+
body: Some(body2),
161+
};
162+
163+
assert_eq!(RequestHash::from(&req1), RequestHash::from(&req2));
164+
}
165+
166+
#[test]
167+
fn test_different_placements_produce_different_hash() {
168+
let url = "https://example.com/ads";
169+
let body1 = serde_json::to_vec(&serde_json::json!({
170+
"context_id": "same-id",
171+
"placements": [{"placement": "tile_1", "count": 1}]
172+
}))
173+
.unwrap();
174+
let body2 = serde_json::to_vec(&serde_json::json!({
175+
"context_id": "same-id",
176+
"placements": [{"placement": "tile_2", "count": 3}]
177+
}))
178+
.unwrap();
179+
180+
let req1 = Request {
181+
method: Method::Post,
182+
url: url.parse().unwrap(),
183+
headers: Headers::new(),
184+
body: Some(body1),
185+
};
186+
let req2 = Request {
187+
method: Method::Post,
188+
url: url.parse().unwrap(),
189+
headers: Headers::new(),
190+
body: Some(body2),
191+
};
192+
193+
assert_ne!(RequestHash::from(&req1), RequestHash::from(&req2));
194+
}
195+
121196
#[test]
122197
fn test_request_hash_from_string() {
123198
let hash_str = "abc123def456";

components/ads-client/src/lib.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,6 @@ impl MozAdsClient {
118118
inner.report_ad(url).map_err(ComponentError::ReportAd)
119119
}
120120

121-
pub fn cycle_context_id(&self) -> AdsClientApiResult<String> {
122-
let mut inner = self.inner.lock();
123-
let previous_context_id = inner.cycle_context_id()?;
124-
Ok(previous_context_id)
125-
}
126-
127121
pub fn clear_cache(&self) -> AdsClientApiResult<()> {
128122
let inner = self.inner.lock();
129123
inner

0 commit comments

Comments
 (0)