Skip to content

Commit 8502484

Browse files
sypharsyphar
committed
migrate crate-details handler to axum
1 parent 86d4dad commit 8502484

File tree

3 files changed

+105
-71
lines changed

3 files changed

+105
-71
lines changed

‎src/web/crate_details.rs

Lines changed: 76 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
1-
use super::{markdown, match_version, redirect_base, MatchSemver, MetaData};
2-
use crate::utils::{get_correct_docsrs_style_file, report_error};
1+
use super::{markdown, match_version, MatchSemver, MetaData};
2+
use crate::utils::{get_correct_docsrs_style_file, report_error, spawn_blocking};
33
use crate::{
44
db::Pool,
5-
impl_webpage,
5+
impl_axum_webpage,
66
repositories::RepositoryStatsUpdater,
7-
web::{cache::CachePolicy, page::WebPage},
7+
web::{
8+
cache::CachePolicy,
9+
error::{AxumNope, AxumResult},
10+
},
11+
};
12+
use anyhow::{anyhow, Context as _};
13+
use axum::{
14+
extract::{Extension, Path},
15+
response::{IntoResponse, Response as AxumResponse},
816
};
9-
use anyhow::anyhow;
1017
use chrono::{DateTime, Utc};
11-
use iron::prelude::*;
12-
use iron::Url;
1318
use postgres::GenericClient;
14-
use router::Router;
19+
use serde::Deserialize;
1520
use serde::{ser::Serializer, Serialize};
1621
use serde_json::Value;
22+
use std::sync::Arc;
1723

1824
// TODO: Add target name and versions
1925

@@ -293,75 +299,87 @@ struct CrateDetailsPage {
293299
details: CrateDetails,
294300
}
295301

296-
impl_webpage! {
302+
impl_axum_webpage! {
297303
CrateDetailsPage = "crate/details.html",
298304
}
299305

300-
pub fn crate_details_handler(req: &mut Request) -> IronResult<Response> {
301-
let router = extension!(req, Router);
302-
// this handler must always called with a crate name
303-
let name = cexpect!(req, router.find("name"));
304-
let req_version = router.find("version");
306+
#[derive(Deserialize, Clone)]
307+
pub(crate) struct CrateDetailHandlerParams {
308+
name: String,
309+
version: Option<String>,
310+
}
305311

306-
if req_version.is_none() {
307-
let url = ctry!(
308-
req,
309-
Url::parse(&format!("{}/crate/{}/latest", redirect_base(req), name,)),
310-
);
311-
return Ok(super::cached_redirect(url, CachePolicy::ForeverInCdn));
312+
pub(crate) async fn crate_details_handler(
313+
Path(params): Path<CrateDetailHandlerParams>,
314+
Extension(pool): Extension<Pool>,
315+
Extension(repository_stats_updater): Extension<Arc<RepositoryStatsUpdater>>,
316+
) -> AxumResult<AxumResponse> {
317+
// this handler must always called with a crate name
318+
if params.version.is_none() {
319+
return Ok(super::axum_cached_redirect(
320+
&format!("/crate/{}/latest", params.name),
321+
CachePolicy::ForeverInCdn,
322+
)
323+
.into_response());
312324
}
313325

314-
let mut conn = extension!(req, Pool).get()?;
326+
let found_version = spawn_blocking({
327+
let pool = pool.clone();
328+
let params = params.clone();
329+
move || {
330+
let mut conn = pool.get()?;
331+
match_version(&mut conn, &params.name, params.version.as_deref())
332+
.and_then(|m| m.assume_exact())
333+
.context("error matching version")
334+
}
335+
})
336+
.await?;
315337

316-
let found_version =
317-
match_version(&mut conn, name, req_version).and_then(|m| m.assume_exact())?;
318338
let (version, version_or_latest, is_latest_url) = match found_version {
319339
MatchSemver::Exact((version, _)) => (version.clone(), version, false),
320340
MatchSemver::Latest((version, _)) => (version, "latest".to_string(), true),
321341
MatchSemver::Semver((version, _)) => {
322-
let url = ctry!(
323-
req,
324-
Url::parse(&format!(
325-
"{}/crate/{}/{}",
326-
redirect_base(req),
327-
name,
328-
version
329-
)),
330-
);
331-
332-
return Ok(super::cached_redirect(url, CachePolicy::ForeverInCdn));
342+
return Ok(super::axum_cached_redirect(
343+
&format!("/crate/{}/{}", &params.name, version),
344+
CachePolicy::ForeverInCdn,
345+
)
346+
.into_response());
333347
}
334348
};
335349

336-
let updater = extension!(req, RepositoryStatsUpdater);
337-
let details = cexpect!(
338-
req,
339-
ctry!(
340-
req,
341-
CrateDetails::new(
342-
&mut *conn,
343-
name,
344-
&version,
345-
&version_or_latest,
346-
Some(updater)
347-
)
348-
)
349-
);
350-
351-
let mut res = CrateDetailsPage { details }.into_response(req)?;
352-
res.extensions.insert::<CachePolicy>(if is_latest_url {
353-
CachePolicy::ForeverInCdn
354-
} else {
355-
CachePolicy::ForeverInCdnAndStaleInBrowser
356-
});
357-
Ok(res)
350+
let details = spawn_blocking(move || {
351+
let mut conn = pool.get()?;
352+
Ok(CrateDetails::new(
353+
&mut *conn,
354+
&params.name,
355+
&version,
356+
&version_or_latest,
357+
Some(&repository_stats_updater),
358+
)?
359+
.ok_or_else(|| {
360+
report_error(&anyhow!("could not find crate details"));
361+
AxumNope::VersionNotFound
362+
})?)
363+
})
364+
.await?;
365+
366+
let mut res = CrateDetailsPage { details }.into_response();
367+
res.extensions_mut()
368+
.insert::<CachePolicy>(if is_latest_url {
369+
CachePolicy::ForeverInCdn
370+
} else {
371+
CachePolicy::ForeverInCdnAndStaleInBrowser
372+
});
373+
Ok(res.into_response())
358374
}
359375

360376
#[cfg(test)]
361377
mod tests {
362378
use super::*;
363379
use crate::index::api::CrateOwner;
364-
use crate::test::{assert_cache_control, assert_redirect_cached, wrapper, TestDatabase};
380+
use crate::test::{
381+
assert_cache_control, assert_redirect, assert_redirect_cached, wrapper, TestDatabase,
382+
};
365383
use anyhow::{Context, Error};
366384
use kuchiki::traits::TendrilSink;
367385
use std::collections::HashMap;
@@ -1035,13 +1053,7 @@ mod tests {
10351053
assert!(body.contains("<a href=\"/crate/dummy/latest/source/\""));
10361054
assert!(body.contains("<a href=\"/crate/dummy/latest\""));
10371055

1038-
assert_redirect_cached(
1039-
"/crate/dummy/latest/",
1040-
"/crate/dummy/latest",
1041-
CachePolicy::NoCaching,
1042-
web,
1043-
&env.config(),
1044-
)?;
1056+
assert_redirect("/crate/dummy/latest/", "/crate/dummy/latest", web)?;
10451057
assert_redirect_cached(
10461058
"/crate/dummy",
10471059
"/crate/dummy/latest",

‎src/web/mod.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@ use anyhow::Error;
9797
use axum::{
9898
extract::Extension,
9999
http::{uri::Authority, StatusCode},
100-
middleware, Router as AxumRouter,
100+
middleware,
101+
response::IntoResponse,
102+
Router as AxumRouter,
101103
};
102104
use chrono::{DateTime, Utc};
103105
use csp::CspMiddleware;
@@ -327,7 +329,7 @@ fn match_version(
327329
WHERE normalize_crate_name(name) = normalize_crate_name($1)",
328330
&[&name],
329331
)
330-
.unwrap();
332+
.unwrap(); // FIXME: remove this unwrap when all handlers using it are migrated to axum
331333

332334
let row = rows.get(0).ok_or(Nope::CrateNotFound)?;
333335

@@ -443,6 +445,7 @@ pub(crate) fn build_axum_app(
443445
.layer(Extension(context.metrics()?))
444446
.layer(Extension(context.config()?))
445447
.layer(Extension(context.storage()?))
448+
.layer(Extension(context.repository_stats_updater()?))
446449
.layer(Extension(template_data))
447450
.layer(middleware::from_fn(csp::csp_middleware))
448451
.layer(middleware::from_fn(
@@ -530,12 +533,28 @@ fn redirect(url: Url) -> Response {
530533
resp
531534
}
532535

536+
fn axum_redirect(url: &str) -> impl IntoResponse {
537+
(
538+
StatusCode::FOUND,
539+
[(
540+
http::header::LOCATION,
541+
http::HeaderValue::try_from(url).expect("invalid url for redirect"),
542+
)],
543+
)
544+
}
545+
533546
fn cached_redirect(url: Url, cache_policy: cache::CachePolicy) -> Response {
534547
let mut resp = Response::with((status::Found, Redirect(url)));
535548
resp.extensions.insert::<cache::CachePolicy>(cache_policy);
536549
resp
537550
}
538551

552+
fn axum_cached_redirect(url: &str, cache_policy: cache::CachePolicy) -> impl IntoResponse {
553+
let mut resp = axum_redirect(url).into_response();
554+
resp.extensions_mut().insert(cache_policy);
555+
resp
556+
}
557+
539558
fn redirect_base(req: &Request) -> String {
540559
// Try to get the scheme from CloudFront first, and then from iron
541560
let scheme = req

‎src/web/routes.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,14 @@ pub(super) fn build_axum_routes() -> AxumRouter {
102102
"/releases/failures/:page",
103103
get_internal(super::releases::releases_failures_by_stars_handler),
104104
)
105+
.route(
106+
"/crate/:name",
107+
get_internal(super::crate_details::crate_details_handler),
108+
)
109+
.route(
110+
"/crate/:name/:version",
111+
get_internal(super::crate_details::crate_details_handler),
112+
)
105113
}
106114

107115
// REFACTOR: Break this into smaller initialization functions
@@ -149,11 +157,6 @@ pub(super) fn build_routes() -> Routes {
149157
routes.internal_page("/releases/search", super::releases::search_handler);
150158
routes.internal_page("/releases/queue", super::releases::build_queue_handler);
151159

152-
routes.internal_page("/crate/:name", super::crate_details::crate_details_handler);
153-
routes.internal_page(
154-
"/crate/:name/:version",
155-
super::crate_details::crate_details_handler,
156-
);
157160
routes.internal_page(
158161
"/crate/:name/:version/builds",
159162
super::builds::build_list_handler,

0 commit comments

Comments
 (0)