switch testing to fastly, move error-log in cache middleware

Author: syphar

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<Self>
@@ -248,7 +248,13 @@ impl AxumRouterTestExt for axum::Router {
     async fn get(&self, path: &str) -> Result<AxumResponse> {
         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?)
     }
 

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!(

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,21 +3052,23 @@ 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,
                 "{:?}",
                 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?;