diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e11857153..273c4b8417 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,9 @@ ## ✨ What's New ✨ +### Ads Client +* HTTP cache TTL is now resolved by priority — explicit per-request TTL (if any) wins, otherwise the response's `Cache-Control: max-age` is used, otherwise the configured `default_ttl`. Previously the layer took the minimum of all three, which effectively ignored the server's `max-age` signal. + ### Remote Settings * Add uptake telemetry support ([#7288](https://github.com/mozilla/application-services/pull/7288)) * Add v2 routes ([#7339](https://github.com/mozilla/application-services/pull/7339)) diff --git a/components/ads-client/docs/usage-javascript.md b/components/ads-client/docs/usage-javascript.md index aacca4b0c9..4467fb3f07 100644 --- a/components/ads-client/docs/usage-javascript.md +++ b/components/ads-client/docs/usage-javascript.md @@ -544,19 +544,15 @@ It reduces redundant network traffic and improves latency for repeated or identi ### Cache Lifecycle -Each network response can be stored in the cache with an associated effective TTL, computed as: +Each network response can be stored in the cache with an associated effective TTL, +resolved by priority (highest to lowest): -``` -effective_ttl = min(server_max_age, client_default_ttl, per_request_ttl) -``` - -where: - -- `server_max_age` comes from the HTTP `Cache-Control: max-age=N` header (if present), -- `client_default_ttl` is set in `MozAdsCacheConfig`, -- `per_request_ttl` is an optional override set in `MozAdsRequestCachePolicy`. +1. `per_request_ttl` — caller-provided override on `MozAdsRequestCachePolicy`. +2. `server_max_age` — value of the HTTP `Cache-Control: max-age=N` header on the response. +3. `client_default_ttl` — configured on `MozAdsCacheConfig`. -If the effective TTL resolves to 0 seconds, the response is not cached. +If the effective TTL resolves to 0 seconds (e.g. `Cache-Control: max-age=0`), +the response is not cached. ### Configuring The Cache diff --git a/components/ads-client/docs/usage-kotlin.md b/components/ads-client/docs/usage-kotlin.md index 9db60c6410..18ffec337e 100644 --- a/components/ads-client/docs/usage-kotlin.md +++ b/components/ads-client/docs/usage-kotlin.md @@ -501,19 +501,15 @@ It reduces redundant network traffic and improves latency for repeated or identi ### Cache Lifecycle -Each network response can be stored in the cache with an associated effective TTL, computed as: +Each network response can be stored in the cache with an associated effective TTL, +resolved by priority (highest to lowest): -``` -effective_ttl = min(server_max_age, client_default_ttl, per_request_ttl) -``` - -where: - -- `server_max_age` comes from the HTTP `Cache-Control: max-age=N` header (if present), -- `client_default_ttl` is set in `MozAdsCacheConfig`, -- `per_request_ttl` is an optional override set in `MozAdsRequestCachePolicy`. +1. `per_request_ttl` — caller-provided override on `MozAdsRequestCachePolicy`. +2. `server_max_age` — value of the HTTP `Cache-Control: max-age=N` header on the response. +3. `client_default_ttl` — configured on `MozAdsCacheConfig`. -If the effective TTL resolves to 0 seconds, the response is not cached. +If the effective TTL resolves to 0 seconds (e.g. `Cache-Control: max-age=0`), +the response is not cached. ### Configuring The Cache diff --git a/components/ads-client/docs/usage-swift.md b/components/ads-client/docs/usage-swift.md index d3ac5741dc..52cb21282b 100644 --- a/components/ads-client/docs/usage-swift.md +++ b/components/ads-client/docs/usage-swift.md @@ -501,19 +501,15 @@ It reduces redundant network traffic and improves latency for repeated or identi ### Cache Lifecycle -Each network response can be stored in the cache with an associated effective TTL, computed as: +Each network response can be stored in the cache with an associated effective TTL, +resolved by priority (highest to lowest): -``` -effective_ttl = min(server_max_age, client_default_ttl, per_request_ttl) -``` - -where: - -- `server_max_age` comes from the HTTP `Cache-Control: max-age=N` header (if present), -- `client_default_ttl` is set in `MozAdsCacheConfig`, -- `per_request_ttl` is an optional override set in `MozAdsRequestCachePolicy`. +1. `per_request_ttl` — caller-provided override on `MozAdsRequestCachePolicy`. +2. `server_max_age` — value of the HTTP `Cache-Control: max-age=N` header on the response. +3. `client_default_ttl` — configured on `MozAdsCacheConfig`. -If the effective TTL resolves to 0 seconds, the response is not cached. +If the effective TTL resolves to 0 seconds (e.g. `Cache-Control: max-age=0`), +the response is not cached. ### Configuring The Cache diff --git a/components/ads-client/src/http_cache.rs b/components/ads-client/src/http_cache.rs index f9d0af7704..2ae3d07812 100644 --- a/components/ads-client/src/http_cache.rs +++ b/components/ads-client/src/http_cache.rs @@ -11,6 +11,7 @@ mod outcome; mod request_hash; mod store; mod strategy; +mod ttl; use self::{ builder::HttpCacheBuilder, @@ -84,13 +85,15 @@ impl HttpCache { CachePolicy::CacheFirst { ttl } => CacheFirst { hash, request, - ttl: ttl.unwrap_or(self.default_ttl), + explicit_ttl: *ttl, + default_ttl: self.default_ttl, } .apply(client, &self.store), CachePolicy::NetworkFirst { ttl } => NetworkFirst { hash, request, - ttl: ttl.unwrap_or(self.default_ttl), + explicit_ttl: *ttl, + default_ttl: self.default_ttl, } .apply(client, &self.store), }?; @@ -321,13 +324,13 @@ mod tests { } #[test] - fn ttl_resolution_min_of_server_request_default() { + fn ttl_resolution_explicit_overrides_server_max_age() { viaduct_dev::init_backend_dev(); let _m = mockito::mock("POST", "/ads") .with_status(200) .with_header("content-type", "application/json") - .with_header("cache-control", "max-age=1") // Set max age to 1 second + .with_header("cache-control", "max-age=1") // Server says 1 second .with_body(r#"{"ok":true}"#) .expect(1) .create(); @@ -336,23 +339,61 @@ mod tests { let req = make_post_request(); let hash = RequestHash::new(&req); let policy = CachePolicy::CacheFirst { - ttl: Some(Duration::from_secs(20)), // 20 second ttl specified vs the cache's default of 300s + ttl: Some(Duration::from_secs(20)), // Caller asked for 20s }; let client = make_client(); - // Store ttl should resolve to 1s as specified by response headers + // Caller's explicit TTL wins over the server's max-age. let (_, outcomes) = cache.send_with_policy(&client, req, &policy).unwrap(); assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); - // After ~>1s, cleanup should remove it + // Past the server max-age (1s) but still under the explicit TTL (20s) — entry is still there. cache.store.get_clock().advance(2); cache.store.delete_expired_entries().unwrap(); + assert!(cache.store.lookup(&hash).unwrap().is_some()); + + // Past the explicit TTL — entry should be expired. + cache.store.get_clock().advance(20); + cache.store.delete_expired_entries().unwrap(); + assert!(cache.store.lookup(&hash).unwrap().is_none()); + } + + #[test] + fn ttl_resolution_uses_server_max_age_when_no_explicit_override() { + viaduct_dev::init_backend_dev(); + let _m = mockito::mock("POST", "/ads") + .with_status(200) + .with_header("content-type", "application/json") + .with_header("cache-control", "max-age=2") // Server says 2 seconds + .with_body(r#"{"ok":true}"#) + .expect(1) + .create(); + + // Configured default is 300s — should be ignored in favor of server max-age. + let cache = make_cache_with_ttl(300); + let req = make_post_request(); + let hash = RequestHash::new(&req); + + let client = make_client(); + let (_, outcomes) = cache + .send_with_policy(&client, req, &CachePolicy::default()) + .unwrap(); + assert!(matches!(outcomes.last().unwrap(), CacheOutcome::MissStored)); + + // Still cached at ~1s. + cache.store.get_clock().advance(1); + cache.store.delete_expired_entries().unwrap(); + assert!(cache.store.lookup(&hash).unwrap().is_some()); + + // Expired after exceeding server max-age. + cache.store.get_clock().advance(2); + cache.store.delete_expired_entries().unwrap(); assert!(cache.store.lookup(&hash).unwrap().is_none()); } #[test] - fn ttl_resolution_request_overrides_default_when_smaller() { + fn ttl_resolution_explicit_overrides_default() { viaduct_dev::init_backend_dev(); let _m = mockito::mock("POST", "/ads") diff --git a/components/ads-client/src/http_cache/cache_control.rs b/components/ads-client/src/http_cache/cache_control.rs index 526549dcaa..a469ffa312 100644 --- a/components/ads-client/src/http_cache/cache_control.rs +++ b/components/ads-client/src/http_cache/cache_control.rs @@ -51,11 +51,8 @@ impl CacheControl { !self.no_store } - pub fn effective_ttl(&self, requested_ttl: Duration) -> Duration { - match self.max_age { - Some(s) => std::cmp::min(requested_ttl, Duration::from_secs(s)), - None => requested_ttl, - } + pub fn max_age_duration(&self) -> Option { + self.max_age.map(Duration::from_secs) } } diff --git a/components/ads-client/src/http_cache/strategy.rs b/components/ads-client/src/http_cache/strategy.rs index 8368e9dc23..acf23c8234 100644 --- a/components/ads-client/src/http_cache/strategy.rs +++ b/components/ads-client/src/http_cache/strategy.rs @@ -6,6 +6,7 @@ use super::cache_control::CacheControl; use super::request_hash::RequestHash; use super::store::HttpCacheStore; +use super::ttl::EffectiveTtl; use super::{CacheOutcome, HttpCacheSendResult}; use std::time::Duration; use viaduct::{Client, Request}; @@ -13,7 +14,8 @@ use viaduct::{Client, Request}; pub struct CacheFirst { pub hash: RequestHash, pub request: Request, - pub ttl: Duration, + pub explicit_ttl: Option, + pub default_ttl: Duration, } impl CacheFirst { @@ -28,7 +30,8 @@ impl CacheFirst { let network = NetworkFirst { hash: self.hash, request: self.request, - ttl: self.ttl, + explicit_ttl: self.explicit_ttl, + default_ttl: self.default_ttl, }; let (response, mut network_outcomes) = network.apply(client, store)?; outcomes.append(&mut network_outcomes); @@ -39,7 +42,8 @@ impl CacheFirst { pub struct NetworkFirst { pub hash: RequestHash, pub request: Request, - pub ttl: Duration, + pub explicit_ttl: Option, + pub default_ttl: Duration, } impl NetworkFirst { @@ -47,7 +51,12 @@ impl NetworkFirst { let response = client.send_sync(self.request)?; let cache_control = CacheControl::from(&response); let outcome = if cache_control.should_cache() { - let ttl = cache_control.effective_ttl(self.ttl); + let ttl = EffectiveTtl { + explicit: self.explicit_ttl, + server_max_age: cache_control.max_age_duration(), + default: self.default_ttl, + } + .resolve(); if ttl.is_zero() { return Ok((response, vec![CacheOutcome::NoCache])); } diff --git a/components/ads-client/src/http_cache/ttl.rs b/components/ads-client/src/http_cache/ttl.rs new file mode 100644 index 0000000000..0183ddfbb8 --- /dev/null +++ b/components/ads-client/src/http_cache/ttl.rs @@ -0,0 +1,82 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public +* License, v. 2.0. If a copy of the MPL was not distributed with this +* file, You can obtain one at http://mozilla.org/MPL/2.0/. +*/ + +use std::time::Duration; + +/// The TTL to use when storing a response in the cache, computed from +/// three possible sources. +/// +/// `explicit` comes from the caller, `server_max_age` from the response's +/// `Cache-Control` header, and `default` from the cache's configuration. +pub struct EffectiveTtl { + /// Per-request override provided by the caller, if any. + pub explicit: Option, + /// `Cache-Control: max-age` from the server response, if present. + pub server_max_age: Option, + /// The cache's configured default TTL. + pub default: Duration, +} + +impl EffectiveTtl { + /// Resolve the TTL by priority (highest to lowest): + /// 1. `explicit` — caller-provided per-request override. + /// 2. `server_max_age` — value of `Cache-Control: max-age` on the response. + /// 3. `default` — the cache's configured default. + pub fn resolve(&self) -> Duration { + self.explicit + .or(self.server_max_age) + .unwrap_or(self.default) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn explicit_overrides_server_max_age_and_default() { + let ttl = EffectiveTtl { + explicit: Some(Duration::from_secs(60)), + server_max_age: Some(Duration::from_secs(3600)), + default: Duration::from_secs(300), + } + .resolve(); + assert_eq!(ttl, Duration::from_secs(60)); + } + + #[test] + fn falls_back_to_server_max_age_when_no_explicit() { + let ttl = EffectiveTtl { + explicit: None, + server_max_age: Some(Duration::from_secs(3600)), + default: Duration::from_secs(300), + } + .resolve(); + assert_eq!(ttl, Duration::from_secs(3600)); + } + + #[test] + fn falls_back_to_default_when_no_explicit_and_no_server_max_age() { + let ttl = EffectiveTtl { + explicit: None, + server_max_age: None, + default: Duration::from_secs(300), + } + .resolve(); + assert_eq!(ttl, Duration::from_secs(300)); + } + + #[test] + fn zero_server_max_age_yields_zero() { + // Lets the strategy emit NoCache without a network round-trip. + let ttl = EffectiveTtl { + explicit: None, + server_max_age: Some(Duration::ZERO), + default: Duration::from_secs(300), + } + .resolve(); + assert_eq!(ttl, Duration::ZERO); + } +}