refactor & centralize rustdoc parameter handling & url generation

Cargo.toml

@@ -59,7 +59,7 @@ bzip2 = "0.6.0"
 getrandom = "0.3.1"
 itertools = { version = "0.14.0" }
 hex = "0.4.3"
-derive_more = { version = "2.0.0", features = ["display"] }
+derive_more = { version = "2.0.0", features = ["display", "deref"] }
 sysinfo = { version = "0.37.2", default-features = false, features = ["system"] }
 derive_builder = "0.20.2"
 

src/db/add_package.rs

@@ -4,13 +4,13 @@ use crate::{
     error::Result,
     registry_api::{CrateData, CrateOwner, ReleaseData},
     storage::CompressionAlgorithm,
-    utils::{MetadataPackage, rustc_version::parse_rustc_date},
+    utils::{Dependency, MetadataPackage, rustc_version::parse_rustc_date},
     web::crate_details::{latest_release, releases_for_crate},
 };
 use anyhow::{Context, anyhow};
-use derive_more::Display;
+use derive_more::{Deref, Display};
 use futures_util::stream::TryStreamExt;
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use slug::slugify;
 use std::{
@@ -33,6 +33,44 @@ pub struct ReleaseId(pub i32);
 #[sqlx(transparent)]
 pub struct BuildId(pub i32);
 
+type DepOut = (String, String, String, bool);
+type DepIn = (String, String, Option<String>, Option<bool>);
+
+/// A crate dependency in our internal representation for releases.dependencies json.
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Deref)]
+#[serde(from = "DepIn", into = "DepOut")]
+pub(crate) struct ReleaseDependency(Dependency);
+
+impl ReleaseDependency {
+    pub(crate) fn into_inner(self) -> Dependency {
+        self.0
+    }
+}
+
+impl From<DepIn> for ReleaseDependency {
+    fn from((name, req, kind, optional): DepIn) -> Self {
+        ReleaseDependency(Dependency {
+            name,
+            req,
+            kind,
+            optional: optional.unwrap_or(false),
+            rename: None,
+        })
+    }
+}
+
+impl From<ReleaseDependency> for DepOut {
+    fn from(rd: ReleaseDependency) -> Self {
+        let d = rd.0;
+        (
+            d.name,
+            d.req.to_string(),
+            d.kind.unwrap_or_else(|| "normal".into()),
+            d.optional,
+        )
+    }
+}
+
 /// Adds a package into database.
 ///
 /// Package must be built first.
@@ -59,7 +97,7 @@ pub(crate) async fn finish_release(
     source_size: u64,
 ) -> Result<()> {
     debug!("updating release data");
-    let dependencies = convert_dependencies(metadata_pkg);
+    let dependencies = convert_dependencies(metadata_pkg)?;
     let rustdoc = get_rustdoc(metadata_pkg, source_dir).unwrap_or(None);
     let readme = get_readme(metadata_pkg, source_dir).unwrap_or(None);
     let features = get_features(metadata_pkg);
@@ -94,7 +132,7 @@ pub(crate) async fn finish_release(
            WHERE id = $1"#,
         release_id.0,
         registry_data.release_time,
-        serde_json::to_value(dependencies)?,
+        dependencies,
         metadata_pkg.package_name(),
         registry_data.yanked,
         has_docs,
@@ -393,20 +431,14 @@ pub(crate) async fn initialize_build(
     Ok(build_id)
 }
 
-/// Convert dependencies into Vec<(String, String, String, bool)>
-fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String, bool)> {
-    pkg.dependencies
+/// Convert dependencies into our own internal JSON representation
+fn convert_dependencies(pkg: &MetadataPackage) -> Result<serde_json::Value> {
+    let dependencies: Vec<_> = pkg
+        .dependencies
         .iter()
-        .map(|dependency| {
-            let name = dependency.name.clone();
-            let version = dependency.req.clone();
-            let kind = dependency
-                .kind
-                .clone()
-                .unwrap_or_else(|| "normal".to_string());
-            (name, version, kind, dependency.optional)
-        })
-        .collect()
+        .map(|dependency| ReleaseDependency(dependency.clone()))
+        .collect::<Vec<_>>();
+    Ok(serde_json::to_value(dependencies)?)
 }
 
 /// Reads features and converts them to Vec<Feature> with default being first

src/db/mod.rs

@@ -4,8 +4,8 @@ use sqlx::migrate::{Migrate, Migrator};
 
 pub use self::add_package::update_latest_version_id;
 pub(crate) use self::add_package::{
-    add_doc_coverage, finish_build, finish_release, initialize_build, initialize_crate,
-    initialize_release, update_build_with_error,
+    ReleaseDependency, add_doc_coverage, finish_build, finish_release, initialize_build,
+    initialize_crate, initialize_release, update_build_with_error,
 };
 pub use self::{
     add_package::{

src/docbuilder/rustwide_builder.rs

@@ -1450,11 +1450,10 @@ mod tests {
             &format!("{default_target}/{crate_path}/index.html"),
         )?);
 
-        let default_target_url =
-            format!("/{crate_}/{version}/{default_target}/{crate_path}/index.html");
+        let default_target_url = format!("/{crate_}/{version}/{default_target}/{crate_path}/");
         runtime.block_on(web.assert_redirect(
             &default_target_url,
-            &format!("/{crate_}/{version}/{crate_path}/index.html"),
+            &format!("/{crate_}/{version}/{crate_path}/"),
         ))?;
 
         // Non-dist toolchains only have a single target, and of course
@@ -1498,7 +1497,6 @@ mod tests {
                     json_files.sort();
                     dbg!(&json_files);
                     assert!(json_files[0].starts_with(&format!("empty-library_1.0.0_{target}_")));
-
                     assert!(json_files[0].ends_with(&format!(".json.{ext}")));
                     assert_eq!(
                         json_files[1],

src/test/fakes.rs

@@ -12,7 +12,7 @@ use crate::storage::{
     AsyncStorage, CompressionAlgorithm, RustdocJsonFormatVersion, compress, rustdoc_archive_path,
     rustdoc_json_path, source_archive_path,
 };
-use crate::utils::{Dependency, MetadataPackage, Target};
+use crate::utils::{Dependency, MetadataPackage, cargo_metadata::Target};
 use anyhow::{Context, bail};
 use base64::{Engine, engine::general_purpose::STANDARD as b64};
 use chrono::{DateTime, Utc};
@@ -330,7 +330,7 @@ impl<'a> FakeRelease<'a> {
     }
 
     /// Returns the release_id
-    pub(crate) async fn create(self) -> Result<ReleaseId> {
+    pub(crate) async fn create(mut self) -> Result<ReleaseId> {
         use std::fs;
         use std::path::Path;
 
@@ -514,37 +514,34 @@ impl<'a> FakeRelease<'a> {
         store_files_into(&self.source_files, crate_dir)?;
 
         let default_target = self.default_target.unwrap_or("x86_64-unknown-linux-gnu");
+        if !self.doc_targets.iter().any(|t| t == default_target) {
+            self.doc_targets.insert(0, default_target.to_owned());
+        }
 
-        {
-            let mut targets = self.doc_targets.clone();
-            if !targets.contains(&default_target.to_owned()) {
-                targets.push(default_target.to_owned());
-            }
-            for target in &targets {
-                let dummy_rustdoc_json_content = serde_json::to_vec(&serde_json::json!({
-                    "format_version": 42
-                }))?;
-
-                for alg in RUSTDOC_JSON_COMPRESSION_ALGORITHMS {
-                    let compressed_json: Vec<u8> = compress(&*dummy_rustdoc_json_content, *alg)?;
-
-                    for format_version in [
-                        RustdocJsonFormatVersion::Version(42),
-                        RustdocJsonFormatVersion::Latest,
-                    ] {
-                        storage
-                            .store_one_uncompressed(
-                                &rustdoc_json_path(
-                                    &package.name,
-                                    &package.version,
-                                    target,
-                                    format_version,
-                                    Some(*alg),
-                                ),
-                                compressed_json.clone(),
-                            )
-                            .await?;
-                    }
+        for target in &self.doc_targets {
+            let dummy_rustdoc_json_content = serde_json::to_vec(&serde_json::json!({
+                "format_version": 42
+            }))?;
+
+            for alg in RUSTDOC_JSON_COMPRESSION_ALGORITHMS {
+                let compressed_json: Vec<u8> = compress(&*dummy_rustdoc_json_content, *alg)?;
+
+                for format_version in [
+                    RustdocJsonFormatVersion::Version(42),
+                    RustdocJsonFormatVersion::Latest,
+                ] {
+                    storage
+                        .store_one_uncompressed(
+                            &rustdoc_json_path(
+                                &package.name,
+                                &package.version,
+                                target,
+                                format_version,
+                                Some(*alg),
+                            ),
+                            compressed_json.clone(),
+                        )
+                        .await?;
                 }
             }
         }
@@ -555,6 +552,7 @@ impl<'a> FakeRelease<'a> {
         let mut async_conn = db.async_conn().await;
         let crate_id = initialize_crate(&mut async_conn, &package.name).await?;
         let release_id = initialize_release(&mut async_conn, crate_id, &package.version).await?;
+
         crate::db::finish_release(
             &mut async_conn,
             crate_id,

src/test/mod.rs

@@ -72,7 +72,7 @@ impl AxumResponseTestExt for axum::response::Response {
                 expected_directives.to_str().unwrap(),
             );
         } else {
-            assert!(cache_control.is_none());
+            assert!(cache_control.is_none(), "{:?}", cache_control);
         }
     }
 

src/utils/cargo_metadata.rs

@@ -125,7 +125,7 @@ impl Target {
     }
 }
 
-#[derive(Debug, Deserialize, Serialize)]
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
 pub(crate) struct Dependency {
     pub(crate) name: String,
     pub(crate) req: String,

src/utils/mod.rs

@@ -1,35 +1,34 @@
 //! Various utilities for docs.rs
 
-pub(crate) use self::cargo_metadata::{CargoMetadata, Package as MetadataPackage};
-pub(crate) use self::copy::copy_dir_all;
-pub use self::daemon::{start_daemon, watch_registry};
-pub(crate) use self::html::rewrite_rustdoc_html_stream;
-pub use self::queue::{
-    get_crate_pattern_and_priority, get_crate_priority, list_crate_priorities,
-    remove_crate_priority, set_crate_priority,
+pub(crate) use self::{
+    cargo_metadata::{CargoMetadata, Dependency, Package as MetadataPackage},
+    copy::copy_dir_all,
+    html::rewrite_rustdoc_html_stream,
+    rustc_version::{get_correct_docsrs_style_file, parse_rustc_version},
+};
+pub use self::{
+    daemon::{start_daemon, watch_registry},
+    queue::{
+        get_crate_pattern_and_priority, get_crate_priority, list_crate_priorities,
+        remove_crate_priority, set_crate_priority,
+    },
+    queue_builder::queue_builder,
 };
-pub use self::queue_builder::queue_builder;
-pub(crate) use self::rustc_version::{get_correct_docsrs_style_file, parse_rustc_version};
-
-#[cfg(test)]
-pub(crate) use self::cargo_metadata::{Dependency, Target};
 
-mod cargo_metadata;
+pub(crate) mod cargo_metadata;
 pub mod consistency;
 mod copy;
 pub mod daemon;
 mod html;
 mod queue;
 pub(crate) mod queue_builder;
 pub(crate) mod rustc_version;
-use anyhow::{Context as _, Result};
-use serde::Serialize;
-use serde::de::DeserializeOwned;
-use std::{fmt, panic};
-use tracing::{Span, error, warn};
 pub(crate) mod sized_buffer;
 
-use std::{future::Future, thread, time::Duration};
+use anyhow::{Context as _, Result};
+use serde::{Serialize, de::DeserializeOwned};
+use std::{fmt, future::Future, panic, thread, time::Duration};
+use tracing::{Span, error, warn};
 
 pub(crate) fn report_error(err: &anyhow::Error) {
     // Debug-format for anyhow errors includes context & backtrace