fix wrong target-redifect links in topbar release list partial

Author: syphar

src/web/crate_details.rs

@@ -584,8 +584,9 @@ pub(crate) async fn get_all_releases(
     Path(params): Path<RustdocHtmlParams>,
     mut conn: DbConnection,
 ) -> AxumResult<AxumResponse> {
-    let req_path: String = params.path.clone().unwrap_or_default();
-    let req_path: Vec<&str> = req_path.split('/').collect();
+    // NOTE: we're getting RustDocHtmlParams here, where both target and path are optional.
+    // Due to how this handler is used in the `releases_list` macro, we always get both values.
+    // both values (when used in the topbar).
 
     let matched_release = match_version(&mut conn, &params.name, &params.version)
         .await?
@@ -599,61 +600,26 @@ pub(crate) async fn get_all_releases(
         return Err(AxumNope::CrateNotFound);
     }
 
-    let doc_targets = sqlx::query_scalar!(
-        "SELECT
-            releases.doc_targets
-         FROM releases
-         WHERE releases.id = $1;",
-        matched_release.id().0,
-    )
-    .fetch_optional(&mut *conn)
-    .await?
-    .ok_or(AxumNope::CrateNotFound)?
-    .map(MetaData::parse_doc_targets)
-    .ok_or_else(|| anyhow!("empty doc targets for successful release"))?;
-
-    let inner;
-    let (target, inner_path) = {
-        let mut inner_path = req_path.clone();
-
-        let target = if inner_path.len() > 1
-            && doc_targets
-                .iter()
-                .any(|s| Some(s) == params.target.as_ref())
-        {
-            inner_path.remove(0);
-            params.target.as_ref().unwrap()
-        } else {
-            ""
-        };
-
-        inner = inner_path.join("/");
-        (target, inner.trim_end_matches('/'))
-    };
-
-    let target_name = matched_release
-        .target_name()
-        .ok_or_else(|| anyhow!("empty target name for succesful release"))?;
-
-    let inner_path = if inner_path.is_empty() {
-        format!("{target_name}/index.html")
+    // NOTE: we don't check if the target exists here.
+    // If the target doesn't exist, the target-redirect will think
+    // it's part of the `inner_path`, don't find the file in storage,
+    // and redirect to a search.
+    let target = if let Some(req_target) = params.target {
+        format!("{req_target}/")
     } else {
-        format!("{target_name}/{inner_path}")
-    };
-
-    let target = if target.is_empty() {
         String::new()
-    } else {
-        format!("{target}/")
     };
 
-    let res = ReleaseList {
+    let inner_path = params.path.unwrap_or_default();
+    let inner_path = inner_path.trim_end_matches('/');
+
+    Ok(ReleaseList {
         releases: matched_release.all_releases,
         target,
-        inner_path,
+        inner_path: inner_path.to_string(),
         crate_name: params.name,
-    };
-    Ok(res.into_response())
+    }
+    .into_response())
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -2034,6 +2000,17 @@ mod tests {
                 .create()
                 .await?;
 
+            check_links(
+                // https://github.com/rust-lang/docs.rs/issues/2922
+                &env,
+                "/crate/dummy-ba/0.5.0/menus/releases/x86_64-unknown-linux-gnu/src/dummy_ba/de.rs.html",
+                vec![
+                    "/crate/dummy-ba/0.5.0/target-redirect/x86_64-unknown-linux-gnu/src/dummy_ba/de.rs.html".to_string(),
+                    "/crate/dummy-ba/0.4.0/target-redirect/x86_64-unknown-linux-gnu/src/dummy_ba/de.rs.html".to_string(),
+                ],
+            )
+            .await;
+
             check_links(
                 &env,
                 "/crate/dummy-ba/latest/menus/releases/dummy_ba/index.html",

src/web/routes.rs

@@ -269,22 +269,10 @@ pub(super) fn build_axum_routes() -> AxumRouter {
             "/crate/{name}/{version}/menus/platforms",
             get_internal(super::crate_details::get_all_platforms_root),
         )
-        .route(
-            "/crate/{name}/{version}/menus/releases/{target}",
-            get_internal(super::crate_details::get_all_releases),
-        )
         .route(
             "/crate/{name}/{version}/menus/releases/{target}/{*path}",
             get_internal(super::crate_details::get_all_releases),
         )
-        .route(
-            "/crate/{name}/{version}/menus/releases",
-            get_internal(super::crate_details::get_all_releases),
-        )
-        .route(
-            "/crate/{name}/{version}/menus/releases/{target}/",
-            get_internal(super::crate_details::get_all_releases),
-        )
         .route(
             "/-/rustdoc.static/{*path}",
             get_internal(super::rustdoc::static_asset_handler),

src/web/rustdoc.rs

@@ -1105,6 +1105,7 @@ mod test {
     use anyhow::Context;
     use chrono::{NaiveDate, Utc};
     use kuchikiki::traits::TendrilSink;
+    use pretty_assertions::assert_eq;
     use reqwest::StatusCode;
     use std::collections::BTreeMap;
     use test_case::test_case;
@@ -2687,15 +2688,15 @@ mod test {
             let releases_response = env
                 .web_app()
                 .await
-                .get("/crate/hexponent/0.3.1/menus/releases")
+                .get("/crate/hexponent/0.3.1/menus/releases/x86_64-unknown-linux-gnu/hexponent/index.html")
                 .await?;
             assert!(releases_response.status().is_success());
             releases_response.assert_cache_control(CachePolicy::ForeverInCdn, &env.config());
             assert_eq!(
                 parse_release_links_from_menu(&releases_response.text().await?),
                 vec![
-                    "/crate/hexponent/0.3.1/target-redirect/hexponent/index.html".to_owned(),
-                    "/crate/hexponent/0.3.0/target-redirect/hexponent/index.html".to_owned(),
+                    "/crate/hexponent/0.3.1/target-redirect/x86_64-unknown-linux-gnu/hexponent/index.html".to_owned(),
+                    "/crate/hexponent/0.3.0/target-redirect/x86_64-unknown-linux-gnu/hexponent/index.html".to_owned(),
                 ]
             );