From cea3de0da802a067a25194fa0f1372a6b3cde409 Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Sat, 27 Nov 2021 20:41:32 -0800 Subject: [PATCH] Serve /crate/latest/crate rather than redirect --- src/web/build_details.rs | 2 +- src/web/builds.rs | 21 ++- src/web/crate_details.rs | 85 ++++++++++-- src/web/features.rs | 18 ++- src/web/mod.rs | 58 +++++++- src/web/rustdoc.rs | 162 ++++++++++++++++++++--- src/web/source.rs | 46 ++++++- templates/header/package_navigation.html | 2 +- templates/rustdoc/topbar.html | 14 +- 9 files changed, 349 insertions(+), 59 deletions(-) diff --git a/src/web/build_details.rs b/src/web/build_details.rs index dbc80f200..112886618 100644 --- a/src/web/build_details.rs +++ b/src/web/build_details.rs @@ -81,7 +81,7 @@ pub fn build_details_handler(req: &mut Request) -> IronResult { }; BuildDetailsPage { - metadata: cexpect!(req, MetaData::from_crate(&mut conn, name, version)), + metadata: cexpect!(req, MetaData::from_crate(&mut conn, name, version, version)), build_details, } .into_response(req) diff --git a/src/web/builds.rs b/src/web/builds.rs index e3aca7cf4..a11b73735 100644 --- a/src/web/builds.rs +++ b/src/web/builds.rs @@ -49,9 +49,10 @@ pub fn build_list_handler(req: &mut Request) -> IronResult { .last() .map_or(false, |segment| segment.ends_with(".json")); - let version = + let (version, version_or_latest) = match match_version(&mut conn, name, req_version).and_then(|m| m.assume_exact())? { - MatchSemver::Exact((version, _)) => version, + MatchSemver::Exact((version, _)) => (version.clone(), version), + MatchSemver::Latest((version, _)) => (version, "latest".to_string()), MatchSemver::Semver((version, _)) => { let ext = if is_json { ".json" } else { "" }; @@ -117,7 +118,10 @@ pub fn build_list_handler(req: &mut Request) -> IronResult { Ok(resp) } else { BuildsPage { - metadata: cexpect!(req, MetaData::from_crate(&mut conn, name, &version)), + metadata: cexpect!( + req, + MetaData::from_crate(&mut conn, name, &version, &version_or_latest) + ), builds, limits, } @@ -307,7 +311,7 @@ mod tests { } #[test] - fn latest_redirect() { + fn latest_200() { wrapper(|env| { env.fake_release() .name("aquarelle") @@ -332,7 +336,12 @@ mod tests { assert!(resp .url() .as_str() - .ends_with("/crate/aquarelle/0.2.0/builds")); + .ends_with("/crate/aquarelle/latest/builds")); + let body = String::from_utf8(resp.bytes().unwrap().to_vec()).unwrap(); + assert!(body.contains(" Option { // get all stuff, I love you rustfmt @@ -150,6 +151,7 @@ impl CrateDetails { let metadata = MetaData { name: krate.get("name"), version: krate.get("version"), + version_or_latest: version_or_latest.to_string(), description: krate.get("description"), rustdoc_status: krate.get("rustdoc_status"), target_name: krate.get("target_name"), @@ -281,16 +283,21 @@ pub fn crate_details_handler(req: &mut Request) -> IronResult { let name = cexpect!(req, router.find("name")); let req_version = router.find("version"); - let mut conn = extension!(req, Pool).get()?; - - match match_version(&mut conn, name, req_version).and_then(|m| m.assume_exact())? { - MatchSemver::Exact((version, _)) => { - let updater = extension!(req, RepositoryStatsUpdater); - let details = cexpect!(req, CrateDetails::new(&mut conn, name, &version, updater)); + if req_version == None { + let url = ctry!( + req, + Url::parse(&format!("{}/crate/{}/latest", redirect_base(req), name,)), + ); + return Ok(super::redirect(url)); + } - CrateDetailsPage { details }.into_response(req) - } + let mut conn = extension!(req, Pool).get()?; + let found_version = + match_version(&mut conn, name, req_version).and_then(|m| m.assume_exact())?; + let (version, version_or_latest) = match found_version { + MatchSemver::Exact((version, _)) => (version.clone(), version), + MatchSemver::Latest((version, _)) => (version, "latest".to_string()), MatchSemver::Semver((version, _)) => { let url = ctry!( req, @@ -302,16 +309,24 @@ pub fn crate_details_handler(req: &mut Request) -> IronResult { )), ); - Ok(super::redirect(url)) + return Ok(super::redirect(url)); } - } + }; + + let updater = extension!(req, RepositoryStatsUpdater); + let details = cexpect!( + req, + CrateDetails::new(&mut conn, name, &version, &version_or_latest, updater) + ); + + CrateDetailsPage { details }.into_response(req) } #[cfg(test)] mod tests { use super::*; use crate::index::api::CrateOwner; - use crate::test::{wrapper, TestDatabase}; + use crate::test::{assert_redirect, wrapper, TestDatabase}; use anyhow::{Context, Error}; use kuchiki::traits::TendrilSink; use std::collections::HashMap; @@ -326,6 +341,7 @@ mod tests { &mut db.conn(), package, version, + version, db.repository_stats_updater(), ) .with_context(|| anyhow::anyhow!("could not fetch crate details"))?; @@ -459,6 +475,7 @@ mod tests { &mut db.conn(), "foo", "0.2.0", + "0.2.0", db.repository_stats_updater(), ) .unwrap(); @@ -534,6 +551,7 @@ mod tests { &mut db.conn(), "foo", version, + version, db.repository_stats_updater(), ) .unwrap(); @@ -564,6 +582,7 @@ mod tests { &mut db.conn(), "foo", version, + version, db.repository_stats_updater(), ) .unwrap(); @@ -595,6 +614,7 @@ mod tests { &mut db.conn(), "foo", version, + version, db.repository_stats_updater(), ) .unwrap(); @@ -634,6 +654,7 @@ mod tests { &mut db.conn(), "foo", version, + version, db.repository_stats_updater(), ) .unwrap(); @@ -696,6 +717,7 @@ mod tests { &mut db.conn(), "foo", "0.0.1", + "0.0.1", db.repository_stats_updater(), ) .unwrap(); @@ -726,6 +748,7 @@ mod tests { &mut db.conn(), "foo", "0.0.1", + "0.0.1", db.repository_stats_updater(), ) .unwrap(); @@ -755,6 +778,7 @@ mod tests { &mut db.conn(), "foo", "0.0.1", + "0.0.1", db.repository_stats_updater(), ) .unwrap(); @@ -779,6 +803,7 @@ mod tests { &mut db.conn(), "foo", "0.0.1", + "0.0.1", db.repository_stats_updater(), ) .unwrap(); @@ -956,4 +981,42 @@ mod tests { Ok(()) }); } + + #[test] + fn latest_url() { + wrapper(|env| { + env.fake_release() + .name("dummy") + .version("0.4.0") + .rustdoc_file("dummy/index.html") + .rustdoc_file("x86_64-pc-windows-msvc/dummy/index.html") + .default_target("x86_64-unknown-linux-gnu") + .add_target("x86_64-pc-windows-msvc") + .create()?; + let web = env.frontend(); + + let resp = env.frontend().get("/crate/dummy/latest").send()?; + assert!(resp.status().is_success()); + assert!(resp.url().as_str().ends_with("/crate/dummy/latest")); + let body = String::from_utf8(resp.bytes().unwrap().to_vec()).unwrap(); + assert!(body.contains(" IronResult { let req_version = router.find("version"); let mut conn = extension!(req, Pool).get()?; - let version = + let (version, version_or_latest) = match match_version(&mut conn, name, req_version).and_then(|m| m.assume_exact())? { - MatchSemver::Exact((version, _)) => version, + MatchSemver::Exact((version, _)) => (version.clone(), version), + MatchSemver::Latest((version, _)) => (version, "latest".to_string()), MatchSemver::Semver((version, _)) => { let url = ctry!( @@ -69,7 +70,10 @@ pub fn build_features_handler(req: &mut Request) -> IronResult { } FeaturesPage { - metadata: cexpect!(req, MetaData::from_crate(&mut conn, name, &version)), + metadata: cexpect!( + req, + MetaData::from_crate(&mut conn, name, &version, &version_or_latest) + ), features, default_len, } @@ -244,7 +248,7 @@ mod tests { } #[test] - fn latest_redirect() { + fn latest_200() { wrapper(|env| { env.fake_release() .name("foo") @@ -259,7 +263,11 @@ mod tests { .create()?; let resp = env.frontend().get("/crate/foo/latest/features").send()?; - assert!(resp.url().as_str().ends_with("/crate/foo/0.2.0/features")); + assert!(resp.url().as_str().ends_with("/crate/foo/latest/features")); + let body = String::from_utf8(resp.bytes().unwrap().to_vec()).unwrap(); + assert!(body.contains(" (String, i32) { match self { - MatchSemver::Exact((v, i)) | MatchSemver::Semver((v, i)) => (v, i), + MatchSemver::Exact((v, i)) + | MatchSemver::Semver((v, i)) + | MatchSemver::Latest((v, i)) => (v, i), } } } @@ -268,12 +272,12 @@ impl MatchSemver { fn match_version( conn: &mut Client, name: &str, - version: Option<&str>, + input_version: Option<&str>, ) -> Result { // version is an Option<&str> from router::Router::get, need to decode first use iron::url::percent_encoding::percent_decode; - let req_version = version + let req_version = input_version .and_then(|v| percent_decode(v.as_bytes()).decode_utf8().ok()) .map(|v| { if v == "newest" || v == "latest" { @@ -358,7 +362,11 @@ fn match_version( { return Ok(MatchVersion { corrected_name, - version: MatchSemver::Semver((version.to_string(), *id)), + version: if input_version == Some("latest") { + MatchSemver::Latest((version.to_string(), *id)) + } else { + MatchSemver::Semver((version.to_string(), *id)) + }, rustdoc_status: *rustdoc_status, }); } @@ -515,6 +523,10 @@ fn redirect_base(req: &Request) -> String { #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub(crate) struct MetaData { pub(crate) name: String, + // If we're on a page with /latest/ in the URL, the string "latest". + // Otherwise, the version as a string. + pub(crate) version_or_latest: String, + // The exact version of the crate being shown. Never contains "latest". pub(crate) version: String, pub(crate) description: Option, pub(crate) target_name: Option, @@ -525,7 +537,12 @@ pub(crate) struct MetaData { } impl MetaData { - fn from_crate(conn: &mut Client, name: &str, version: &str) -> Option { + fn from_crate( + conn: &mut Client, + name: &str, + version: &str, + version_or_latest: &str, + ) -> Option { let rows = conn .query( "SELECT crates.name, @@ -548,6 +565,7 @@ impl MetaData { Some(MetaData { name: row.get(0), version: row.get(1), + version_or_latest: version_or_latest.to_string(), description: row.get(2), target_name: row.get(3), rustdoc_status: row.get(4), @@ -752,7 +770,7 @@ mod test { .create() .unwrap(); let web = env.frontend(); - assert_redirect("/bat//", "/bat/0.2.0/bat/", web)?; + assert_redirect("/bat//", "/bat/latest/bat/", web)?; Ok(()) }) } @@ -899,6 +917,7 @@ mod test { let mut metadata = MetaData { name: "serde".to_string(), version: "1.0.0".to_string(), + version_or_latest: "1.0.0".to_string(), description: Some("serde does stuff".to_string()), target_name: None, rustdoc_status: true, @@ -913,6 +932,7 @@ mod test { let correct_json = json!({ "name": "serde", "version": "1.0.0", + "version_or_latest": "1.0.0", "description": "serde does stuff", "target_name": null, "rustdoc_status": true, @@ -930,6 +950,7 @@ mod test { let correct_json = json!({ "name": "serde", "version": "1.0.0", + "version_or_latest": "1.0.0", "description": "serde does stuff", "target_name": "serde_lib_name", "rustdoc_status": true, @@ -947,6 +968,7 @@ mod test { let correct_json = json!({ "name": "serde", "version": "1.0.0", + "version_or_latest": "1.0.0", "description": null, "target_name": "serde_lib_name", "rustdoc_status": true, @@ -961,6 +983,30 @@ mod test { assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap()); } + #[test] + fn metadata_from_crate() { + wrapper(|env| { + release("0.1.0", env); + let mut conn = env.db().conn(); + let metadata = MetaData::from_crate(&mut conn, "foo", "0.1.0", "latest"); + assert_eq!( + metadata.unwrap(), + MetaData { + name: "foo".to_string(), + version_or_latest: "latest".to_string(), + version: "0.1.0".to_string(), + description: Some("Fake package".to_string()), + target_name: Some("foo".to_string()), + rustdoc_status: true, + default_target: "x86_64-unknown-linux-gnu".to_string(), + doc_targets: vec![], + yanked: false, + }, + ); + Ok(()) + }) + } + #[test] fn test_tabindex_is_present_on_topbar_crate_search_input() { wrapper(|env| { diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index c0d682050..600f333ef 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -156,7 +156,11 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { // use that instead crate_name = new_name; } - let (version, id) = v.version.into_parts(); + let (mut version, id) = v.version.into_parts(); + + if req_version == None || req_version == Some("latest") { + version = "latest".to_string() + } // get target name and whether it has docs // FIXME: This is a bit inefficient but allowing us to use less code in general @@ -303,14 +307,23 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { // * If there is a semver (but not exact) match, redirect to the exact version. let release_found = match_version(&mut conn, &name, url_version)?; - let version = match release_found.version { + let (version, version_or_latest) = match release_found.version { MatchSemver::Exact((version, _)) => { // Redirect when the requested crate name isn't correct if let Some(name) = release_found.corrected_name { return redirect(&name, &version, &req_path); } - version + (version.clone(), version) + } + + MatchSemver::Latest((version, _)) => { + // Redirect when the requested crate name isn't correct + if let Some(name) = release_found.corrected_name { + return redirect(&name, "latest", &req_path); + } + + (version, "latest".to_string()) } // Redirect when the requested version isn't correct @@ -328,12 +341,15 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { // Get the crate's details from the database // NOTE: we know this crate must exist because we just checked it above (or else `match_version` is buggy) - let krate = cexpect!(req, CrateDetails::new(&mut conn, &name, &version, updater)); + let krate = cexpect!( + req, + CrateDetails::new(&mut conn, &name, &version, &version_or_latest, updater) + ); // if visiting the full path to the default target, remove the target from the path // expects a req_path that looks like `[/:target]/.*` if req_path.get(0).copied() == Some(&krate.metadata.default_target) { - return redirect(&name, &version, &req_path[1..]); + return redirect(&name, &version_or_latest, &req_path[1..]); } // Create the path to access the file from @@ -368,7 +384,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { req, storage.rustdoc_file_exists(&name, &version, &path, krate.archive_storage) ) { - redirect(&name, &version, &req_path) + redirect(&name, &version_or_latest, &req_path) } else if req_path.get(0).map_or(false, |p| p.contains('-')) { // This is a target, not a module; it may not have been built. // Redirect to the default target and show a search page instead of a hard 404. @@ -431,11 +447,8 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { (target, inner_path.join("/")) }; - // If the requested crate version is the most recent, use it to build the url - let mut latest_path = if is_latest_version { - format!("/{}/{}", name, latest_version) - // If the requested version is not the latest, then find the path of the latest version for the `Go to latest` link - } else if latest_release.build_status { + // Find the path of the latest version for the `Go to latest` and `Permalink` links + let mut latest_path = if latest_release.build_status { let target = if target.is_empty() { &krate.metadata.default_target } else { @@ -543,11 +556,21 @@ pub fn target_redirect_handler(req: &mut Request) -> IronResult { let base = redirect_base(req); let updater = extension!(req, RepositoryStatsUpdater); - let crate_details = match CrateDetails::new(&mut conn, name, version, updater) { - Some(krate) => krate, - None => return Err(Nope::VersionNotFound.into()), + let release_found = match_version(&mut conn, name, Some(version))?; + + let (version, version_or_latest) = match release_found.version { + MatchSemver::Exact((version, _)) => (version.clone(), version), + MatchSemver::Latest((version, _)) => (version, "latest".to_string()), + // semver matching not supported here + MatchSemver::Semver(_) => return Err(Nope::VersionNotFound.into()), }; + let crate_details = + match CrateDetails::new(&mut conn, name, &version, &version_or_latest, updater) { + Some(krate) => krate, + None => return Err(Nope::VersionNotFound.into()), + }; + // [crate, :name, :version, target-redirect, :target, *path] // is transformed to // [:target?, *path] @@ -570,7 +593,7 @@ pub fn target_redirect_handler(req: &mut Request) -> IronResult { req, storage.rustdoc_file_exists( name, - version, + &version, &file_path.join("/"), crate_details.archive_storage ) @@ -582,10 +605,10 @@ pub fn target_redirect_handler(req: &mut Request) -> IronResult { }; let url = format!( - "{base}/{name}/{version}/{path}", + "{base}/{name}/{version_or_latest}/{path}", base = base, name = name, - version = version, + version_or_latest = version_or_latest, path = path ); @@ -731,6 +754,8 @@ mod test { assert_success(base, web)?; assert_redirect("/dummy/0.1.0/x86_64-unknown-linux-gnu/dummy/", base, web)?; + assert_success("/dummy/latest/dummy/", web)?; + // set an explicit target that requires cross-compile let target = "x86_64-pc-windows-msvc"; env.fake_release() @@ -769,6 +794,26 @@ mod test { }); } + #[test] + fn latest_url() { + wrapper(|env| { + env.fake_release() + .name("dummy") + .version("0.1.0") + .archive_storage(true) + .rustdoc_file("dummy/index.html") + .create()?; + + let resp = env.frontend().get("/dummy/latest/dummy/").send()?; + assert!(resp.url().as_str().ends_with("/dummy/latest/dummy/")); + let body = String::from_utf8(resp.bytes().unwrap().to_vec()).unwrap(); + assert!(body.contains(" Option { + fn from_path( + conn: &mut Client, + name: &str, + version: &str, + version_or_latest: &str, + req_path: &str, + ) -> Option { let rows = conn .query( "SELECT crates.name, @@ -134,6 +140,7 @@ impl FileList { metadata: MetaData { name: rows[0].get(0), version: rows[0].get(1), + version_or_latest: version_or_latest.to_string(), description: rows[0].get(2), target_name: rows[0].get(3), rustdoc_status: rows[0].get(4), @@ -178,8 +185,9 @@ pub fn source_browser_handler(req: &mut Request) -> IronResult { // use that instead crate_name = new_name; } - let version = match v.version { - MatchSemver::Exact((version, _)) => version, + let (version, version_or_latest) = match v.version { + MatchSemver::Latest((version, _)) => (version, "latest".to_string()), + MatchSemver::Exact((version, _)) => (version.clone(), version), MatchSemver::Semver((version, _)) => { let url = ctry!( req, @@ -270,8 +278,14 @@ pub fn source_browser_handler(req: &mut Request) -> IronResult { (None, false) }; - let file_list = FileList::from_path(&mut conn, crate_name, &version, &req_path) - .ok_or(Nope::ResourceNotFound)?; + let file_list = FileList::from_path( + &mut conn, + crate_name, + &version, + &version_or_latest, + &req_path, + ) + .ok_or(Nope::ResourceNotFound)?; SourcePage { file_list, @@ -325,6 +339,28 @@ mod tests { }); } + #[test] + fn latest_contains_links_to_latest() { + wrapper(|env| { + env.fake_release() + .archive_storage(true) + .name("fake") + .version("0.1.0") + .source_file(".cargo-ok", b"ok") + .source_file("README.md", b"hello") + .create()?; + let resp = env.frontend().get("/crate/fake/latest/source/").send()?; + assert!(resp.url().as_str().ends_with("/crate/fake/latest/source/")); + let body = String::from_utf8(resp.bytes().unwrap().to_vec()).unwrap(); + assert!(body.contains("
{# The partial path of the crate, `:name/:release` #} - {%- set crate_path = metadata.name ~ "/" ~ metadata.version -%} + {%- set crate_path = metadata.name ~ "/" ~ metadata.version_or_latest -%} {# If docs are built, show a button for them #} diff --git a/templates/rustdoc/topbar.html b/templates/rustdoc/topbar.html index 61bef4565..5eb8fabda 100644 --- a/templates/rustdoc/topbar.html +++ b/templates/rustdoc/topbar.html @@ -1,7 +1,7 @@ {%- import "macros.html" as macros -%} {# The url of the current release, `/crate/:name/:version` #} -{%- set crate_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.version -%} +{%- set crate_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.version_or_latest -%} {%- include "header/topbar_begin.html" -%}{# extra whitespace unremovable, need to use html tags unaffacted by whitespace T_T @@ -22,6 +22,14 @@ {%- include "clipboard.svg" -%} + {%- if metadata.version_or_latest == "latest" -%} +
  • + + {{ "link" | fas }} Permalink + +
    • + {%- endif -%} +
  • {{ "cube" | fas(fw=true) }} Docs.rs crate page @@ -211,10 +219,10 @@ because the documentation root page is guaranteed to exist for all targets. #} {%- if use_direct_platform_links -%} - {%- set target_url = "/" ~ metadata.name ~ "/" ~ metadata.version ~ "/" ~ target ~ "/" ~ inner_path -%} + {%- set target_url = "/" ~ metadata.name ~ "/" ~ metadata.version_or_latest ~ "/" ~ target ~ "/" ~ inner_path -%} {%- set target_no_follow = "" -%} {%- else -%} - {%- set target_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.version ~ "/target-redirect/" ~ target ~ "/" ~ inner_path -%} + {%- set target_url = "/crate/" ~ metadata.name ~ "/" ~ metadata.version_or_latest ~ "/target-redirect/" ~ target ~ "/" ~ inner_path -%} {%- set target_no_follow = "nofollow" -%} {%- endif -%}