migrate crate-details handler to axum

src/web/crate_details.rs

@@ -1,19 +1,25 @@
-use super::{markdown, match_version, redirect_base, MatchSemver, MetaData};
-use crate::utils::{get_correct_docsrs_style_file, report_error};
+use super::{markdown, match_version, MatchSemver, MetaData};
+use crate::utils::{get_correct_docsrs_style_file, report_error, spawn_blocking};
 use crate::{
     db::Pool,
-    impl_webpage,
+    impl_axum_webpage,
     repositories::RepositoryStatsUpdater,
-    web::{cache::CachePolicy, page::WebPage},
+    web::{
+        cache::CachePolicy,
+        error::{AxumNope, AxumResult},
+    },
 };
 use anyhow::anyhow;
+use axum::{
+    extract::{Extension, Path},
+    response::{IntoResponse, Response as AxumResponse},
+};
 use chrono::{DateTime, Utc};
-use iron::prelude::*;
-use iron::Url;
 use postgres::GenericClient;
-use router::Router;
+use serde::Deserialize;
 use serde::{ser::Serializer, Serialize};
 use serde_json::Value;
+use std::sync::Arc;
 
 // TODO: Add target name and versions
 
@@ -293,75 +299,87 @@ struct CrateDetailsPage {
     details: CrateDetails,
 }
 
-impl_webpage! {
+impl_axum_webpage! {
     CrateDetailsPage = "crate/details.html",
+    cpu_intensive_rendering = true,
 }
 
-pub fn crate_details_handler(req: &mut Request) -> IronResult<Response> {
-    let router = extension!(req, Router);
-    // this handler must always called with a crate name
-    let name = cexpect!(req, router.find("name"));
-    let req_version = router.find("version");
+#[derive(Deserialize, Clone, Debug)]
+pub(crate) struct CrateDetailHandlerParams {
+    name: String,
+    version: Option<String>,
+}
 
-    if req_version.is_none() {
-        let url = ctry!(
-            req,
-            Url::parse(&format!("{}/crate/{}/latest", redirect_base(req), name,)),
-        );
-        return Ok(super::cached_redirect(url, CachePolicy::ForeverInCdn));
+#[tracing::instrument]
+pub(crate) async fn crate_details_handler(
+    Path(params): Path<CrateDetailHandlerParams>,
+    Extension(pool): Extension<Pool>,
+    Extension(repository_stats_updater): Extension<Arc<RepositoryStatsUpdater>>,
+) -> AxumResult<AxumResponse> {
+    // this handler must always called with a crate name
+    if params.version.is_none() {
+        return Ok(super::axum_cached_redirect(
+            &format!("/crate/{}/latest", params.name),
+            CachePolicy::ForeverInCdn,
+        )?
+        .into_response());
     }
 
-    let mut conn = extension!(req, Pool).get()?;
+    let found_version = spawn_blocking({
+        let pool = pool.clone();
+        let params = params.clone();
+        move || {
+            let mut conn = pool.get()?;
+            Ok(
+                match_version(&mut conn, &params.name, params.version.as_deref())
+                    .and_then(|m| m.assume_exact())?,
+            )
+        }
+    })
+    .await?;
 
-    let found_version =
-        match_version(&mut conn, name, req_version).and_then(|m| m.assume_exact())?;
     let (version, version_or_latest, is_latest_url) = match found_version {
         MatchSemver::Exact((version, _)) => (version.clone(), version, false),
         MatchSemver::Latest((version, _)) => (version, "latest".to_string(), true),
         MatchSemver::Semver((version, _)) => {
-            let url = ctry!(
-                req,
-                Url::parse(&format!(
-                    "{}/crate/{}/{}",
-                    redirect_base(req),
-                    name,
-                    version
-                )),
-            );
-
-            return Ok(super::cached_redirect(url, CachePolicy::ForeverInCdn));
+            return Ok(super::axum_cached_redirect(
+                &format!("/crate/{}/{}", &params.name, version),
+                CachePolicy::ForeverInCdn,
+            )?
+            .into_response());
         }
     };
 
-    let updater = extension!(req, RepositoryStatsUpdater);
-    let details = cexpect!(
-        req,
-        ctry!(
-            req,
-            CrateDetails::new(
-                &mut *conn,
-                name,
-                &version,
-                &version_or_latest,
-                Some(updater)
-            )
+    let details = spawn_blocking(move || {
+        let mut conn = pool.get()?;
+        CrateDetails::new(
+            &mut *conn,
+            &params.name,
+            &version,
+            &version_or_latest,
+            Some(&repository_stats_updater),
         )
-    );
-
-    let mut res = CrateDetailsPage { details }.into_response(req)?;
-    res.extensions.insert::<CachePolicy>(if is_latest_url {
-        CachePolicy::ForeverInCdn
-    } else {
-        CachePolicy::ForeverInCdnAndStaleInBrowser
-    });
-    Ok(res)
+    })
+    .await?
+    .ok_or(AxumNope::VersionNotFound)?;
+
+    let mut res = CrateDetailsPage { details }.into_response();
+    res.extensions_mut()
+        .insert::<CachePolicy>(if is_latest_url {
+            CachePolicy::ForeverInCdn
+        } else {
+            CachePolicy::ForeverInCdnAndStaleInBrowser
+        });
+    Ok(res.into_response())
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::index::api::CrateOwner;
-    use crate::test::{assert_cache_control, assert_redirect_cached, wrapper, TestDatabase};
+    use crate::test::{
+        assert_cache_control, assert_redirect, assert_redirect_cached, wrapper, TestDatabase,
+    };
     use anyhow::{Context, Error};
     use kuchiki::traits::TendrilSink;
     use std::collections::HashMap;
@@ -1035,13 +1053,7 @@ mod tests {
             assert!(body.contains("<a href=\"/crate/dummy/latest/source/\""));
             assert!(body.contains("<a href=\"/crate/dummy/latest\""));
 
-            assert_redirect_cached(
-                "/crate/dummy/latest/",
-                "/crate/dummy/latest",
-                CachePolicy::NoCaching,
-                web,
-                &env.config(),
-            )?;
+            assert_redirect("/crate/dummy/latest/", "/crate/dummy/latest", web)?;
             assert_redirect_cached(
                 "/crate/dummy",
                 "/crate/dummy/latest",

src/web/error.rs

@@ -156,7 +156,7 @@ pub enum AxumNope {
     #[error("Internal server error")]
     InternalServerError,
     #[error("internal error")]
-    InternalError(#[from] anyhow::Error),
+    InternalError(anyhow::Error),
 }
 
 impl IntoResponse for AxumNope {
@@ -234,6 +234,32 @@ impl IntoResponse for AxumNope {
     }
 }
 
+impl From<anyhow::Error> for AxumNope {
+    fn from(err: anyhow::Error) -> Self {
+        match err.downcast::<AxumNope>() {
+            Ok(axum_nope) => axum_nope,
+            Err(err) => match err.downcast::<Nope>() {
+                Ok(iron_nope) => AxumNope::from(iron_nope),
+                Err(err) => AxumNope::InternalError(err),
+            },
+        }
+    }
+}
+
+impl From<Nope> for AxumNope {
+    fn from(err: Nope) -> Self {
+        match err {
+            Nope::ResourceNotFound => AxumNope::ResourceNotFound,
+            Nope::BuildNotFound => AxumNope::BuildNotFound,
+            Nope::CrateNotFound => AxumNope::CrateNotFound,
+            Nope::OwnerNotFound => AxumNope::OwnerNotFound,
+            Nope::VersionNotFound => AxumNope::VersionNotFound,
+            Nope::NoResults => todo!(),
+            Nope::InternalServerError => AxumNope::InternalServerError,
+        }
+    }
+}
+
 pub(crate) type AxumResult<T> = Result<T, AxumNope>;
 
 #[cfg(test)]

src/web/mod.rs

@@ -77,7 +77,7 @@ mod builds;
 pub(crate) mod cache;
 pub(crate) mod crate_details;
 mod csp;
-mod error;
+pub(crate) mod error;
 mod extensions;
 mod features;
 mod file;
@@ -97,7 +97,9 @@ use anyhow::Error;
 use axum::{
     extract::Extension,
     http::{uri::Authority, StatusCode},
-    middleware, Router as AxumRouter,
+    middleware,
+    response::IntoResponse,
+    Router as AxumRouter,
 };
 use chrono::{DateTime, Utc};
 use csp::CspMiddleware;
@@ -327,7 +329,7 @@ fn match_version(
                  WHERE normalize_crate_name(name) = normalize_crate_name($1)",
                 &[&name],
             )
-            .unwrap();
+            .unwrap(); // FIXME: remove this unwrap when all handlers using it are migrated to axum
 
         let row = rows.get(0).ok_or(Nope::CrateNotFound)?;
 
@@ -443,6 +445,7 @@ pub(crate) fn build_axum_app(
             .layer(Extension(context.metrics()?))
             .layer(Extension(context.config()?))
             .layer(Extension(context.storage()?))
+            .layer(Extension(context.repository_stats_updater()?))
             .layer(Extension(template_data))
             .layer(middleware::from_fn(csp::csp_middleware))
             .layer(middleware::from_fn(
@@ -530,12 +533,34 @@ fn redirect(url: Url) -> Response {
     resp
 }
 
+fn axum_redirect(url: &str) -> Result<impl IntoResponse, Error> {
+    if !url.starts_with('/') || url.starts_with("//") {
+        return Err(anyhow!("invalid redirect URL: {}", url));
+    }
+    Ok((
+        StatusCode::FOUND,
+        [(
+            http::header::LOCATION,
+            http::HeaderValue::try_from(url).expect("invalid url for redirect"),
+        )],
+    ))
+}
+
 fn cached_redirect(url: Url, cache_policy: cache::CachePolicy) -> Response {
     let mut resp = Response::with((status::Found, Redirect(url)));
     resp.extensions.insert::<cache::CachePolicy>(cache_policy);
     resp
 }
 
+fn axum_cached_redirect(
+    url: &str,
+    cache_policy: cache::CachePolicy,
+) -> Result<impl IntoResponse, Error> {
+    let mut resp = axum_redirect(url)?.into_response();
+    resp.extensions_mut().insert(cache_policy);
+    Ok(resp)
+}
+
 fn redirect_base(req: &Request) -> String {
     // Try to get the scheme from CloudFront first, and then from iron
     let scheme = req
@@ -682,8 +707,10 @@ impl_axum_webpage! {
 mod test {
     use super::*;
     use crate::{docbuilder::DocCoverage, test::*, web::match_version};
+    use axum::http::StatusCode;
     use kuchiki::traits::TendrilSink;
     use serde_json::json;
+    use test_case::test_case;
 
     fn release(version: &str, env: &TestEnvironment) -> i32 {
         env.fake_release()
@@ -1133,4 +1160,42 @@ mod test {
             Ok(())
         });
     }
+
+    #[test]
+    fn test_axum_redirect() {
+        let response = axum_redirect("/something").unwrap().into_response();
+        assert_eq!(response.status(), StatusCode::FOUND);
+        assert_eq!(
+            response.headers().get(http::header::LOCATION).unwrap(),
+            "/something"
+        );
+        assert!(response
+            .headers()
+            .get(http::header::CACHE_CONTROL)
+            .is_none());
+        assert!(response.extensions().get::<cache::CachePolicy>().is_none());
+    }
+
+    #[test]
+    fn test_axum_redirect_cached() {
+        let response = axum_cached_redirect("/something", cache::CachePolicy::NoCaching)
+            .unwrap()
+            .into_response();
+        assert_eq!(response.status(), StatusCode::FOUND);
+        assert_eq!(
+            response.headers().get(http::header::LOCATION).unwrap(),
+            "/something"
+        );
+        assert!(matches!(
+            response.extensions().get::<cache::CachePolicy>().unwrap(),
+            cache::CachePolicy::NoCaching,
+        ))
+    }
+
+    #[test_case("without_leading_slash")]
+    #[test_case("//with_double_leading_slash")]
+    fn test_axum_redirect_failure(path: &str) {
+        assert!(axum_redirect(path).is_err());
+        assert!(axum_cached_redirect(path, cache::CachePolicy::NoCaching).is_err());
+    }
 }