From 96643ab20d1a58fb8cb2f30af2766aa3bc304e54 Mon Sep 17 00:00:00 2001 From: Hampus Date: Sun, 16 Aug 2026 23:19:51 +0200 Subject: [PATCH] fix(unfurl): key the cache on available provider credentials (#1688) --- fluxer_unfurl/src/router_impl.rs | 114 +++++++++++++++++++++++++++---- 1 file changed, 101 insertions(+), 13 deletions(-) diff --git a/fluxer_unfurl/src/router_impl.rs b/fluxer_unfurl/src/router_impl.rs index 7e475a0aa..b61739717 100644 --- a/fluxer_unfurl/src/router_impl.rs +++ b/fluxer_unfurl/src/router_impl.rs @@ -19,6 +19,14 @@ impl UnfurlRouter { .build(), } } + + fn invalidate_all_variants(&self, url: &str) { + for mode in [NsfwMode::Block, NsfwMode::Flag, NsfwMode::Allow] { + for capability in PROVIDER_CAPABILITIES { + self.l1.invalidate(&unfurl_cache_key(url, mode, capability)); + } + } + } } impl RouterService for UnfurlRouter { @@ -43,7 +51,8 @@ impl RouterService for UnfurlRouter { nsfw_mode, bypass_cache, cache_only, - .. + youtube_api_key, + klipy_api_key, } => { if *bypass_cache { return None; @@ -51,7 +60,11 @@ impl RouterService for UnfurlRouter { let mode = if *cache_only { "cache-only" } else { "full" }; Some(format!( "{mode}:{}", - unfurl_cache_key(url, nsfw_mode.unwrap_or_default()) + unfurl_cache_key( + url, + nsfw_mode.unwrap_or_default(), + provider_capability(youtube_api_key, klipy_api_key) + ) )) } UnfurlRequest::Invalidate { .. } => None, @@ -64,12 +77,18 @@ impl RouterService for UnfurlRouter { url, nsfw_mode, bypass_cache, + youtube_api_key, + klipy_api_key, .. } => { if *bypass_cache { None } else { - let key = unfurl_cache_key(url, nsfw_mode.unwrap_or_default()); + let key = unfurl_cache_key( + url, + nsfw_mode.unwrap_or_default(), + provider_capability(youtube_api_key, klipy_api_key), + ); self.l1.get(&key) } } @@ -79,38 +98,58 @@ impl RouterService for UnfurlRouter { fn l1_insert(&self, req: &UnfurlRequest, resp: &UnfurlResponse) { match req { - UnfurlRequest::Unfurl { url, nsfw_mode, .. } => { + UnfurlRequest::Unfurl { + url, + nsfw_mode, + youtube_api_key, + klipy_api_key, + .. + } => { if let UnfurlResponse::Resolved(result) = resp && !result.embeds.is_empty() { self.l1.insert( - unfurl_cache_key(url, nsfw_mode.unwrap_or_default()), + unfurl_cache_key( + url, + nsfw_mode.unwrap_or_default(), + provider_capability(youtube_api_key, klipy_api_key), + ), resp.clone(), ); } } UnfurlRequest::Invalidate { url } => { - self.l1.invalidate(&unfurl_cache_key(url, NsfwMode::Block)); - self.l1.invalidate(&unfurl_cache_key(url, NsfwMode::Flag)); - self.l1.invalidate(&unfurl_cache_key(url, NsfwMode::Allow)); + self.invalidate_all_variants(url); } } } fn l1_invalidate(&self, key: &str) { - self.l1.invalidate(&unfurl_cache_key(key, NsfwMode::Block)); - self.l1.invalidate(&unfurl_cache_key(key, NsfwMode::Flag)); - self.l1.invalidate(&unfurl_cache_key(key, NsfwMode::Allow)); + self.invalidate_all_variants(key); } } -fn unfurl_cache_key(url: &str, nsfw_mode: NsfwMode) -> String { +const PROVIDER_CAPABILITIES: &[&str] = &["none", "youtube", "klipy", "youtube+klipy"]; + +fn provider_capability( + youtube_api_key: &Option, + klipy_api_key: &Option, +) -> &'static str { + match (youtube_api_key.is_some(), klipy_api_key.is_some()) { + (true, true) => "youtube+klipy", + (true, false) => "youtube", + (false, true) => "klipy", + (false, false) => "none", + } +} + +fn unfurl_cache_key(url: &str, nsfw_mode: NsfwMode, capability: &str) -> String { let mode = match nsfw_mode { NsfwMode::Block => "block", NsfwMode::Flag => "flag", NsfwMode::Allow => "allow", }; - format!("{mode}:{url}") + format!("{mode}:{capability}:{url}") } #[cfg(test)] @@ -128,6 +167,55 @@ mod tests { } } + fn resolved_response() -> UnfurlResponse { + UnfurlResponse::Resolved(std::sync::Arc::new(crate::types::UnfurlResult { + embeds: vec![crate::types::MessageEmbed::new("link")], + cache_ttl_seconds: None, + })) + } + + fn unfurl(url: &str, youtube_api_key: Option<&str>) -> UnfurlRequest { + UnfurlRequest::Unfurl { + url: url.to_owned(), + nsfw_mode: Some(NsfwMode::Block), + bypass_cache: false, + cache_only: false, + youtube_api_key: youtube_api_key.map(str::to_owned), + klipy_api_key: None, + } + } + + #[test] + fn configuring_a_youtube_key_does_not_reuse_the_keyless_cache_entry() { + let router = UnfurlRouter::new(16); + let url = "https://www.youtube.com/watch?v=dQw4w9WgXcQ"; + router.l1_insert(&unfurl(url, None), &resolved_response()); + assert!( + router.l1_lookup(&unfurl(url, Some("configured"))).is_none(), + "a result resolved without a YouTube key must not be served once a key is configured" + ); + assert!( + router.l1_lookup(&unfurl(url, None)).is_some(), + "the keyless entry must still serve keyless requests" + ); + } + + #[test] + fn invalidate_clears_every_provider_capability_variant() { + let router = UnfurlRouter::new(16); + let url = "https://www.youtube.com/watch?v=dQw4w9WgXcQ"; + router.l1_insert(&unfurl(url, None), &resolved_response()); + router.l1_insert(&unfurl(url, Some("configured")), &resolved_response()); + router.l1_insert( + &UnfurlRequest::Invalidate { + url: url.to_owned(), + }, + &resolved_response(), + ); + assert!(router.l1_lookup(&unfurl(url, None)).is_none()); + assert!(router.l1_lookup(&unfurl(url, Some("configured"))).is_none()); + } + #[test] fn coalesce_key_separates_cache_only_from_full_unfurls() { assert_ne!(