From bcb29c97d94df7c4ae878c8885b0d66bf5667ea9 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Wed, 26 Nov 2025 13:40:15 +0100 Subject: [PATCH] switch testing to fastly, move error-log in cache middleware --- src/test/mod.rs | 12 +++++++++--- src/web/cache.rs | 15 +++++++-------- src/web/rustdoc.rs | 30 +++++++++++++++++++++--------- 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/test/mod.rs b/src/test/mod.rs index bac52363e..8a1230ff9 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -17,7 +17,7 @@ use crate::{ test::test_metrics::CollectedMetrics, web::{ build_axum_app, - cache::{self, TargetCdn}, + cache::{self, TargetCdn, X_RLNG_SOURCE_CDN}, headers::{IfNoneMatch, SURROGATE_CONTROL}, page::TemplateData, }, @@ -108,7 +108,7 @@ impl AxumResponseTestExt for axum::response::Response { // we emit the wrong cache policy in a handler. // // The fastly specifics are tested in web::cache unittests. - assert_cache_headers_eq(self, &cache_policy.render(config, TargetCdn::CloudFront)); + assert_cache_headers_eq(self, &cache_policy.render(config, TargetCdn::Fastly)); } fn error_for_status(self) -> Result @@ -248,7 +248,13 @@ impl AxumRouterTestExt for axum::Router { async fn get(&self, path: &str) -> Result { Ok(self .clone() - .oneshot(Request::builder().uri(path).body(Body::empty()).unwrap()) + .oneshot( + Request::builder() + .uri(path) + .header(X_RLNG_SOURCE_CDN, "fastly") + .body(Body::empty()) + .unwrap(), + ) .await?) } diff --git a/src/web/cache.rs b/src/web/cache.rs index eecddc87b..bfbd1e666 100644 --- a/src/web/cache.rs +++ b/src/web/cache.rs @@ -18,7 +18,7 @@ use serde::Deserialize; use std::{convert::Infallible, sync::Arc}; use tracing::error; -const X_RLNG_SOURCE_CDN: HeaderName = HeaderName::from_static("x-rlng-source-cdn"); +pub const X_RLNG_SOURCE_CDN: HeaderName = HeaderName::from_static("x-rlng-source-cdn"); #[derive(Debug, Clone, PartialEq)] pub struct ResponseCacheHeaders { @@ -305,7 +305,7 @@ pub(crate) async fn cache_middleware( // It's totally possible that this policy here then states NO_CACHING, // or FOREVER_IN_CDN_AND_BROWSER, where we wouln't need the surrogate key. &cache_headers - } else { + } else if cache_headers.needs_cdn_invalidation { // This theoretically shouldn't happen, all current crate names would be valid // for surrogate keys, and the `KrateName` validation matches the crates.io crate // publish validation. @@ -314,14 +314,13 @@ pub(crate) async fn cache_middleware( if resp_status.is_success() || resp_status.is_redirection() { error!( name = param.name, - "failed to create surrogate key for crate" + ?cache_headers, + "failed to create surrogate key for crate, falling back to NO_CACHING" ); } - if cache_headers.needs_cdn_invalidation { - &NO_CACHING - } else { - &cache_headers - } + &NO_CACHING + } else { + &cache_headers } } else { debug_assert!( diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index aef08b6f3..ac7ed83ea 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -2098,11 +2098,13 @@ mod test { .create() .await?; let web = env.web_app().await; - let resp = web - .assert_redirect("/dummy", "/dummy/latest/dummy/") - .await?; - assert_eq!(resp.status(), StatusCode::FOUND); - assert!(resp.headers().get("Cache-Control").is_none()); + web.assert_redirect_cached( + "/dummy", + "/dummy/latest/dummy/", + CachePolicy::ForeverInCdn, + env.config(), + ) + .await?; Ok(()) }) } @@ -3032,7 +3034,15 @@ mod test { #[test_case("search-1234.js")] #[test_case("settings-1234.js")] fn fallback_to_root_storage_for_some_js_assets(path: &str) { - // test workaround for https://github.com/rust-lang/docs.rs/issues/1979 + // tests for two separate things needed to serve old rustdoc content + // 1. `/{crate}/{version}/asset.js`, where we try to find the assets in the rustdoc archive + // 2. `/asset.js` where we try to find it in RUSTDOC_STATIC_STORAGE_PREFIX + // + // For 2), new builds use the assets from RUSTDOC_STATIC_STORAGE_PREFIX via + // `/-/rustdoc.static/asset.js`. + // + // For 1) I'm actually not sure, new builds don't seem to have these assets. + // ( the logic is special-cased to `search-` and `settings-` prefixes.) async_wrapper(|env| async move { env.fake_release() .await @@ -3042,13 +3052,15 @@ mod test { .create() .await?; + const ROOT_ASSET: &str = "normalize-20200403-1.44.0-nightly-74bd074ee.css"; + let storage = env.async_storage(); - storage.store_one("asset.js", *b"content").await?; + storage.store_one(ROOT_ASSET, *b"content").await?; storage.store_one(path, *b"more_content").await?; let web = env.web_app().await; - let response = web.get("/dummy/0.1.0/asset.js").await?; + let response = web.get(&format!("/dummy/0.1.0/{ROOT_ASSET}")).await?; assert_eq!( response.status(), StatusCode::NOT_FOUND, @@ -3056,7 +3068,7 @@ mod test { response.headers().get("Location"), ); - web.assert_success_and_conditional_get("/asset.js", "content") + web.assert_success_and_conditional_get(&format!("/{ROOT_ASSET}"), "content") .await?; web.assert_success_and_conditional_get(&format!("/dummy/0.1.0/{path}"), "more_content") .await?;