From b166f045d3eb97b923f5e1d7f0581bf044be6eae Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sun, 8 Feb 2026 23:26:52 +0100 Subject: [PATCH] migrate remaining places to KrateName NewType --- Cargo.lock | 1 + crates/bin/docs_rs_admin/src/main.rs | 2 +- .../src/docbuilder/rustwide_builder.rs | 119 ++++++++------- crates/bin/docs_rs_watcher/src/db/delete.rs | 23 +-- .../bin/docs_rs_watcher/src/index_watcher.rs | 12 +- crates/bin/docs_rs_watcher/src/main.rs | 4 +- crates/bin/docs_rs_watcher/src/rebuilds.rs | 4 +- .../bin/docs_rs_web/src/extractors/rustdoc.rs | 2 +- .../docs_rs_web/src/handlers/build_details.rs | 2 +- crates/bin/docs_rs_web/src/handlers/builds.rs | 10 +- .../docs_rs_web/src/handlers/crate_details.rs | 17 ++- .../bin/docs_rs_web/src/handlers/features.rs | 4 +- .../bin/docs_rs_web/src/handlers/releases.rs | 11 +- .../bin/docs_rs_web/src/handlers/rustdoc.rs | 2 +- crates/bin/docs_rs_web/src/handlers/source.rs | 8 +- crates/bin/docs_rs_web/src/match_release.rs | 14 +- crates/bin/docs_rs_web/src/metadata.rs | 15 +- .../lib/docs_rs_build_limits/src/blacklist.rs | 6 +- .../lib/docs_rs_build_limits/src/overrides.rs | 15 +- crates/lib/docs_rs_build_queue/Cargo.toml | 1 + .../lib/docs_rs_build_queue/src/priority.rs | 143 +++++++----------- crates/lib/docs_rs_database/src/releases.rs | 46 +++--- crates/lib/docs_rs_registry_api/src/api.rs | 18 ++- .../docs_rs_storage/src/storage/blocking.rs | 6 +- .../src/storage/non_blocking.rs | 10 +- .../docs_rs_storage/src/utils/storage_path.rs | 8 +- crates/lib/docs_rs_test_fakes/src/legacy.rs | 39 +++-- crates/lib/docs_rs_types/src/krate_name.rs | 26 +++- 28 files changed, 286 insertions(+), 282 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8320b07f7..202cb431c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1977,6 +1977,7 @@ dependencies = [ "opentelemetry", "pretty_assertions", "sqlx", + "test-case", "tokio", "tracing", ] diff --git a/crates/bin/docs_rs_admin/src/main.rs b/crates/bin/docs_rs_admin/src/main.rs index 316467817..c746badbd 100644 --- a/crates/bin/docs_rs_admin/src/main.rs +++ b/crates/bin/docs_rs_admin/src/main.rs @@ -306,7 +306,7 @@ enum DatabaseSubcommand { /// Updates info for a crate from the registry's API UpdateCrateRegistryFields { #[arg(name = "CRATE")] - name: String, + name: KrateName, }, /// Blacklist operations diff --git a/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs b/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs index e7281d07a..cbc750012 100644 --- a/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs +++ b/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs @@ -42,14 +42,14 @@ use std::{ fs::{self, File}, io::BufReader, path::Path, - sync::Arc, + sync::{Arc, LazyLock}, time::Instant, }; use tracing::{debug, error, info, info_span, instrument, warn}; const USER_AGENT: &str = "docs.rs builder (https://github.com/rust-lang/docs.rs)"; const COMPONENTS: &[&str] = &["llvm-tools-preview", "rustc-dev", "rustfmt"]; -const DUMMY_CRATE_NAME: &str = "empty-library"; +static DUMMY_CRATE_NAME: LazyLock = LazyLock::new(|| "empty-library".parse().unwrap()); const DUMMY_CRATE_VERSION: Version = Version::new(1, 0, 0); async fn get_configured_toolchain(conn: &mut sqlx::PgConnection) -> Result { @@ -384,14 +384,13 @@ impl RustwideBuilder { } #[instrument(skip(self))] - fn get_limits(&self, krate: &str) -> Result { - let krate: KrateName = krate.parse()?; + fn get_limits(&self, krate: &KrateName) -> Result { self.runtime.block_on({ let db = self.db.clone(); let config = self.config.clone(); async move { let mut conn = db.get_async().await?; - Limits::for_crate(&config.build_limits, &mut conn, &krate).await + Limits::for_crate(&config.build_limits, &mut conn, krate).await } }) } @@ -402,7 +401,7 @@ impl RustwideBuilder { info!("building a dummy crate to get essential files"); - let limits = self.get_limits(DUMMY_CRATE_NAME)?; + let limits = self.get_limits(&DUMMY_CRATE_NAME)?; // FIXME: for now, purge all build dirs before each build. // Currently we have some error situations where the build directory wouldn't be deleted @@ -420,7 +419,7 @@ impl RustwideBuilder { .build_dir(&format!("essential-files-{parsed_rustc_version}")); // This is an empty library crate that is supposed to always build. - let krate = Crate::crates_io(DUMMY_CRATE_NAME, &DUMMY_CRATE_VERSION.to_string()); + let krate = Crate::crates_io(DUMMY_CRATE_NAME.as_str(), &DUMMY_CRATE_VERSION.to_string()); krate.fetch(&self.workspace)?; build_dir @@ -430,7 +429,7 @@ impl RustwideBuilder { let res = self.execute_build( BuildId(0), - DUMMY_CRATE_NAME, + &DUMMY_CRATE_NAME, &DUMMY_CRATE_VERSION, HOST_TARGET, true, @@ -484,7 +483,10 @@ impl RustwideBuilder { })?; let package = metadata.root(); self.build_package( - &package.name, + &package + .name + .parse() + .context("invalid crate name in package")?, &package.version, PackageKind::Local(path), false, @@ -494,7 +496,7 @@ impl RustwideBuilder { #[instrument(skip(self))] pub fn build_package( &mut self, - name: &str, + name: &KrateName, version: &Version, kind: PackageKind<'_>, collect_metrics: bool, @@ -541,7 +543,7 @@ impl RustwideBuilder { #[allow(clippy::too_many_arguments)] fn build_package_inner( &mut self, - name: &str, + name: &KrateName, version: &Version, kind: PackageKind<'_>, crate_id: CrateId, @@ -554,8 +556,7 @@ impl RustwideBuilder { let is_blacklisted = self.runtime.block_on(async { let mut conn = self.db.get_async().await?; - let name: KrateName = name.parse()?; - let is_blacklisted = is_blacklisted(&mut conn, &name).await?; + let is_blacklisted = is_blacklisted(&mut conn, name).await?; Ok::<_, Error>(is_blacklisted) })?; @@ -609,9 +610,9 @@ impl RustwideBuilder { let version = version.to_string(); let krate = match kind { PackageKind::Local(path) => Crate::local(path), - PackageKind::CratesIo => Crate::crates_io(name, &version), + PackageKind::CratesIo => Crate::crates_io(name.as_str(), &version), PackageKind::Registry(registry) => { - Crate::registry(AlternativeRegistry::new(registry), name, &version) + Crate::registry(AlternativeRegistry::new(registry), name.as_str(), &version) } }; krate.fetch(&self.workspace)?; @@ -893,7 +894,7 @@ impl RustwideBuilder { fn build_target( &self, build_id: BuildId, - name: &str, + name: &KrateName, version: &Version, target: &str, build: &Build, @@ -938,7 +939,7 @@ impl RustwideBuilder { fn execute_json_build( &self, build_id: BuildId, - name: &str, + name: &KrateName, version: &Version, target: &str, is_default_target: bool, @@ -1082,7 +1083,7 @@ impl RustwideBuilder { fn execute_build( &self, build_id: BuildId, - name: &str, + name: &KrateName, version: &Version, target: &str, is_default_target: bool, @@ -1356,12 +1357,12 @@ mod tests { BuildStatus, CompressionAlgorithm, Feature, ReleaseId, Version, testing::V0_1, }; use pretty_assertions::assert_eq; - use std::{collections::BTreeMap, io, iter}; + use std::{collections::BTreeMap, io, iter, path::PathBuf}; use test_case::test_case; fn get_features( env: &TestEnvironment, - name: &str, + name: &KrateName, version: &Version, ) -> Result>, anyhow::Error> { block_on_async_with_conn!(env, |mut conn| async { @@ -1371,7 +1372,7 @@ mod tests { FROM releases INNER JOIN crates ON crates.id = releases.crate_id WHERE crates.name = $1 AND releases.version = $2"#, - name, + name as _, version as _, ) .fetch_one(&mut *conn) @@ -1414,8 +1415,8 @@ mod tests { fn test_build_crate() -> Result<()> { let env = TestEnvironment::new()?; - let crate_ = DUMMY_CRATE_NAME; - let crate_path = crate_.replace('-', "_"); + let crate_ = &*DUMMY_CRATE_NAME; + let crate_path = crate_.as_str().replace('-', "_"); let version = DUMMY_CRATE_VERSION; let default_target = "x86_64-unknown-linux-gnu"; @@ -1456,7 +1457,7 @@ mod tests { WHERE c.name = $1 AND r.version = $2"#, - crate_, + crate_ as _, version as _, ) .fetch_one(&mut *conn) @@ -1606,7 +1607,7 @@ mod tests { config.include_default_targets = false; let env = TestEnvironment::builder().config(config).build()?; - let crate_ = DUMMY_CRATE_NAME; + let crate_ = &*DUMMY_CRATE_NAME; let version = DUMMY_CRATE_VERSION; let mut builder = env.build_builder()?; @@ -1635,7 +1636,7 @@ mod tests { let env = TestEnvironment::new()?; // some binary crate - let crate_ = "heater"; + let crate_ = KrateName::from_static("heater"); let version = Version::new(0, 2, 3); let storage = env.blocking_storage()?; @@ -1648,7 +1649,7 @@ mod tests { builder.update_toolchain()?; assert!( !builder - .build_package(crate_, &version, PackageKind::CratesIo, false)? + .build_package(&crate_, &version, PackageKind::CratesIo, false)? .successful ); @@ -1665,7 +1666,7 @@ mod tests { WHERE c.name = $1 AND r.version = $2", - crate_, + crate_ as _, version as _ ) .fetch_one(&mut *conn) @@ -1677,11 +1678,11 @@ mod tests { assert_eq!(row.is_library, Some(false)); // doc archive exists - let doc_archive = rustdoc_archive_path(crate_, &version); + let doc_archive = rustdoc_archive_path(&crate_, &version); assert!(!storage.exists(&doc_archive)?); // source archive exists - let source_archive = source_archive_path(crate_, &version); + let source_archive = source_archive_path(&crate_, &version); assert!(storage.exists(&source_archive)?); // old rustdoc & source files still exist @@ -1698,12 +1699,12 @@ mod tests { // rand 0.8.5 fails to build with recent nightly versions // https://github.com/rust-lang/docs.rs/issues/26750 - let crate_ = "rand"; + let crate_ = KrateName::from_static("rand"); let version = Version::new(0, 8, 5); // create a successful release & build in the database let release_id = block_on_async_with_conn!(env, |mut conn| async { - let crate_id = initialize_crate(&mut *conn, crate_).await?; + let crate_id = initialize_crate(&mut *conn, &crate_).await?; let release_id = initialize_release(&mut *conn, crate_id, &version).await?; let build_id = initialize_build(&mut *conn, release_id).await?; finish_build( @@ -1721,7 +1722,7 @@ mod tests { crate_id, release_id, &MetadataPackage { - name: crate_.into(), + name: crate_.as_str().into(), version: version.clone(), id: "".into(), license: None, @@ -1778,7 +1779,7 @@ mod tests { assert!( // not successful build !builder - .build_package(crate_, &version, PackageKind::CratesIo, false)? + .build_package(&crate_, &version, PackageKind::CratesIo, false)? .successful ); @@ -1790,25 +1791,27 @@ mod tests { #[test_case("scsys-derive", Version::new(0, 2, 6))] #[test_case("thiserror-impl", Version::new(1, 0, 26))] #[ignore] - fn test_proc_macro(crate_: &str, version: Version) -> Result<()> { + fn test_proc_macro(crate_: &'static str, version: Version) -> Result<()> { let env = TestEnvironment::new()?; + let crate_ = KrateName::from_static(crate_); + let mut builder = env.build_builder()?; builder.update_toolchain()?; assert!( builder - .build_package(crate_, &version, PackageKind::CratesIo, false)? + .build_package(&crate_, &version, PackageKind::CratesIo, false)? .successful ); let storage = env.blocking_storage()?; // doc archive exists - let doc_archive = rustdoc_archive_path(crate_, &version); + let doc_archive = rustdoc_archive_path(&crate_, &version); assert!(storage.exists(&doc_archive)?); // source archive exists - let source_archive = source_archive_path(crate_, &version); + let source_archive = source_archive_path(&crate_, &version); assert!(storage.exists(&source_archive)?); Ok(()) @@ -1819,7 +1822,7 @@ mod tests { fn test_cross_compile_non_host_default() -> Result<()> { let env = TestEnvironment::new()?; - let crate_ = "windows-win"; + let crate_ = KrateName::from_static("windows-win"); let version = Version::new(2, 4, 1); let mut builder = env.build_builder()?; builder.update_toolchain()?; @@ -1828,22 +1831,22 @@ mod tests { } assert!( builder - .build_package(crate_, &version, PackageKind::CratesIo, false)? + .build_package(&crate_, &version, PackageKind::CratesIo, false)? .successful ); let storage = env.blocking_storage()?; // doc archive exists - let doc_archive = rustdoc_archive_path(crate_, &version); + let doc_archive = rustdoc_archive_path(&crate_, &version); assert!(storage.exists(&doc_archive)?, "{}", doc_archive); // source archive exists - let source_archive = source_archive_path(crate_, &version); + let source_archive = source_archive_path(&crate_, &version); assert!(storage.exists(&source_archive)?, "{}", source_archive); let target = "x86_64-unknown-linux-gnu"; - let crate_path = crate_.replace('-', "_"); + let crate_path = crate_.as_str().replace('-', "_"); let target_docs_present = storage.exists_in_archive( &doc_archive, None, @@ -1920,13 +1923,13 @@ mod tests { fn test_rustflags_are_passed_to_build_script() -> Result<()> { let env = TestEnvironment::new()?; - let crate_ = "proc-macro2"; + let crate_ = KrateName::from_static("proc-macro2"); let version = Version::new(1, 0, 95); let mut builder = env.build_builder()?; builder.update_toolchain()?; assert!( builder - .build_package(crate_, &version, PackageKind::CratesIo, false)? + .build_package(&crate_, &version, PackageKind::CratesIo, false)? .successful ); Ok(()) @@ -1941,7 +1944,7 @@ mod tests { // package with invalid cargo metadata. // Will succeed in the crate fetch step, so sources are // added. Will fail when we try to build. - let crate_ = "simple-build-failure"; + let crate_ = KrateName::from_static("simple-build-failure"); let version = V0_1; let test_crate = Path::new("tests/crates/simple-build-failure/"); @@ -1952,7 +1955,7 @@ mod tests { assert!(!builder.build_local_package(test_crate)?.successful); // source archive exists - let source_archive = source_archive_path(crate_, &version); + let source_archive = source_archive_path(&crate_, &version); let storage = env.blocking_storage()?; assert!( @@ -1961,7 +1964,7 @@ mod tests { ); assert!( storage - .fetch_source_file(crate_, &version, None, "src/main.rs", true) + .fetch_source_file(&crate_, &version, None, "src/main.rs", true) .is_ok() ); @@ -1975,13 +1978,13 @@ mod tests { // https://github.com/rust-lang/docs.rs/issues/2491 // package without Cargo.toml, so fails directly in the fetch stage. - let crate_ = "emheap"; + let crate_ = KrateName::from_static("emheap"); let version = Version::new(0, 1, 0); let mut builder = env.build_builder()?; builder.update_toolchain()?; // `Result` is `Ok`, but the build-result is `false` - let summary = builder.build_package(crate_, &version, PackageKind::CratesIo, false)?; + let summary = builder.build_package(&crate_, &version, PackageKind::CratesIo, false)?; assert!(!summary.successful); assert!(summary.should_reattempt); @@ -1998,7 +2001,7 @@ mod tests { INNER JOIN releases as r on c.id = r.crate_id INNER JOIN builds as b on b.rid = r.id WHERE c.name = $1 and r.version = $2"#, - crate_, + crate_ as _, version as _, ) .fetch_one(&mut *conn) @@ -2019,18 +2022,18 @@ mod tests { fn test_implicit_features_for_optional_dependencies() -> Result<()> { let env = TestEnvironment::new()?; - let crate_ = "serde"; + let crate_ = KrateName::from_static("serde"); let version = Version::new(1, 0, 152); let mut builder = env.build_builder()?; builder.update_toolchain()?; assert!( builder - .build_package(crate_, &version, PackageKind::CratesIo, false)? + .build_package(&crate_, &version, PackageKind::CratesIo, false)? .successful ); assert!( - get_features(&env, crate_, &version)? + get_features(&env, &crate_, &version)? .unwrap() .iter() .any(|f| f.name == "serde_derive") @@ -2044,15 +2047,17 @@ mod tests { fn test_no_implicit_features_for_optional_dependencies_with_dep_syntax() -> Result<()> { let env = TestEnvironment::new()?; + let krate = KrateName::from_static("optional-dep"); + let mut builder = env.build_builder()?; builder.update_toolchain()?; assert!( builder - .build_local_package(Path::new("tests/crates/optional-dep"))? + .build_local_package(&PathBuf::from(&format!("tests/crates/{krate}")))? .successful ); - let mut features = get_features(&env, "optional-dep", &Version::new(0, 0, 1))? + let mut features = get_features(&env, &krate, &Version::new(0, 0, 1))? .unwrap() .iter() .map(|f| f.name.to_owned()) @@ -2143,7 +2148,7 @@ mod tests { assert!( builder .build_package( - DUMMY_CRATE_NAME, + &DUMMY_CRATE_NAME, &DUMMY_CRATE_VERSION, PackageKind::CratesIo, false diff --git a/crates/bin/docs_rs_watcher/src/db/delete.rs b/crates/bin/docs_rs_watcher/src/db/delete.rs index 1d5a1b26c..5979bd9d1 100644 --- a/crates/bin/docs_rs_watcher/src/db/delete.rs +++ b/crates/bin/docs_rs_watcher/src/db/delete.rs @@ -107,7 +107,7 @@ async fn get_id(conn: &mut sqlx::PgConnection, name: &KrateName) -> Result Result { let mut transaction = conn.begin().await?; - sqlx::query!("DELETE FROM sandbox_overrides WHERE crate_name = $1", name,) - .execute(&mut *transaction) - .await?; + sqlx::query!( + "DELETE FROM sandbox_overrides WHERE crate_name = $1", + name as _ + ) + .execute(&mut *transaction) + .await?; for &(table, column) in METADATA { sqlx::query( @@ -211,11 +214,13 @@ mod tests { }; use test_case::test_case; - async fn crate_exists(conn: &mut sqlx::PgConnection, name: &str) -> Result { - Ok(sqlx::query!("SELECT id FROM crates WHERE name = $1;", name) - .fetch_optional(conn) - .await? - .is_some()) + async fn crate_exists(conn: &mut sqlx::PgConnection, name: &KrateName) -> Result { + Ok( + sqlx::query!("SELECT id FROM crates WHERE name = $1;", name as _) + .fetch_optional(conn) + .await? + .is_some(), + ) } async fn release_exists(conn: &mut sqlx::PgConnection, id: ReleaseId) -> Result { diff --git a/crates/bin/docs_rs_watcher/src/index_watcher.rs b/crates/bin/docs_rs_watcher/src/index_watcher.rs index 45de7db60..fbeb57164 100644 --- a/crates/bin/docs_rs_watcher/src/index_watcher.rs +++ b/crates/bin/docs_rs_watcher/src/index_watcher.rs @@ -260,8 +260,7 @@ async fn process_crate_deleted(context: &Context, krate: &KrateName) -> Result<( ); queue_crate_invalidation(krate, context.cdn.as_deref()).await; - let name: KrateName = krate.parse()?; - context.build_queue()?.remove_crate_from_queue(&name).await + context.build_queue()?.remove_crate_from_queue(krate).await } pub(crate) async fn set_yanked( @@ -283,7 +282,7 @@ pub(crate) async fn set_yanked( AND version = $2 RETURNING crates.id as "id: CrateId" "#, - name, + name as _, version as _, yanked, ) @@ -298,12 +297,7 @@ pub(crate) async fn set_yanked( ); update_latest_version_id(&mut conn, crate_id).await?; } else { - let name: KrateName = name.parse()?; - match context - .build_queue()? - .has_build_queued(&name, version) - .await - { + match context.build_queue()?.has_build_queued(name, version).await { Ok(false) => { error!( %name, diff --git a/crates/bin/docs_rs_watcher/src/main.rs b/crates/bin/docs_rs_watcher/src/main.rs index beb11516b..088932ceb 100644 --- a/crates/bin/docs_rs_watcher/src/main.rs +++ b/crates/bin/docs_rs_watcher/src/main.rs @@ -153,7 +153,7 @@ enum DatabaseSubcommand { /// Updates info for a crate from the registry's API UpdateCrateRegistryFields { #[arg(name = "CRATE")] - name: String, + name: KrateName, }, /// Remove documentation from the database @@ -214,7 +214,7 @@ enum DeleteSubcommand { Version { /// Name of the crate to delete #[arg(name = "CRATE_NAME")] - name: docs_rs_types::KrateName, + name: KrateName, /// The version of the crate to delete #[arg(name = "VERSION")] diff --git a/crates/bin/docs_rs_watcher/src/rebuilds.rs b/crates/bin/docs_rs_watcher/src/rebuilds.rs index 0ca5ff6a2..9a3015f1f 100644 --- a/crates/bin/docs_rs_watcher/src/rebuilds.rs +++ b/crates/bin/docs_rs_watcher/src/rebuilds.rs @@ -162,10 +162,10 @@ mod tests { let build_queue = env.build_queue()?; build_queue - .add_crate(&FOO.parse().unwrap(), &V1, PRIORITY_CONTINUOUS, None) + .add_crate(&FOO, &V1, PRIORITY_CONTINUOUS, None) .await?; build_queue - .add_crate(&BAR.parse().unwrap(), &V1, PRIORITY_CONTINUOUS, None) + .add_crate(&BAR, &V1, PRIORITY_CONTINUOUS, None) .await?; env.fake_release() diff --git a/crates/bin/docs_rs_web/src/extractors/rustdoc.rs b/crates/bin/docs_rs_web/src/extractors/rustdoc.rs index 78f7581a8..5e5f5e5cb 100644 --- a/crates/bin/docs_rs_web/src/extractors/rustdoc.rs +++ b/crates/bin/docs_rs_web/src/extractors/rustdoc.rs @@ -751,7 +751,7 @@ fn get_file_extension(path: &str) -> Option<&str> { }) } -fn generate_rustdoc_url(name: &str, version: &ReqVersion, path: &str) -> EscapedURI { +fn generate_rustdoc_url(name: &KrateName, version: &ReqVersion, path: &str) -> EscapedURI { EscapedURI::from_path(format!("/{}/{}/{}", name, version, path)) } diff --git a/crates/bin/docs_rs_web/src/handlers/build_details.rs b/crates/bin/docs_rs_web/src/handlers/build_details.rs index 86c235d4a..881161752 100644 --- a/crates/bin/docs_rs_web/src/handlers/build_details.rs +++ b/crates/bin/docs_rs_web/src/handlers/build_details.rs @@ -96,7 +96,7 @@ pub(crate) async fn build_details_handler( INNER JOIN crates ON releases.crate_id = crates.id WHERE builds.id = $1 AND crates.name = $2 AND releases.version = $3"#, id.0, - params.name(), + params.name() as _, version as _ ) .fetch_optional(&mut *conn) diff --git a/crates/bin/docs_rs_web/src/handlers/builds.rs b/crates/bin/docs_rs_web/src/handlers/builds.rs index a9679fb14..e92ffa651 100644 --- a/crates/bin/docs_rs_web/src/handlers/builds.rs +++ b/crates/bin/docs_rs_web/src/handlers/builds.rs @@ -112,8 +112,8 @@ async fn crate_version_exists( INNER JOIN crates ON crates.id = releases.crate_id WHERE crates.name = $1 AND releases.version = $2 LIMIT 1"#, - name, - version.to_string(), + name as _, + version as _, ) .fetch_optional(&mut *conn) .await?; @@ -185,7 +185,7 @@ pub(crate) async fn build_trigger_rebuild_handler( async fn get_builds( conn: &mut sqlx::PgConnection, - name: &str, + name: &KrateName, version: &Version, ) -> Result> { Ok(sqlx::query_as!( @@ -218,8 +218,8 @@ async fn get_builds( crates.name = $1 AND releases.version = $2 ORDER BY builds.id DESC"#, - name, - version.to_string(), + name as _, + version as _, ) .fetch_all(&mut *conn) .await?) diff --git a/crates/bin/docs_rs_web/src/handlers/crate_details.rs b/crates/bin/docs_rs_web/src/handlers/crate_details.rs index 7f809f522..da9429883 100644 --- a/crates/bin/docs_rs_web/src/handlers/crate_details.rs +++ b/crates/bin/docs_rs_web/src/handlers/crate_details.rs @@ -98,7 +98,7 @@ impl CrateDetails { async fn new( conn: &mut sqlx::PgConnection, - name: &str, + name: &KrateName, version: &Version, req_version: Option, prefetched_releases: Vec, @@ -161,8 +161,8 @@ impl CrateDetails { DESC LIMIT 1 ) AS builds ON true WHERE crates.name = $1 AND releases.version = $2;"#, - name, - version.to_string(), + name as _, + version as _, ) .fetch_optional(&mut *conn) .await? @@ -743,21 +743,24 @@ mod tests { status } - async fn crate_details( + async fn crate_details( conn: &mut sqlx::PgConnection, - name: &str, + name: K, version: V, req_version: Option, ) -> CrateDetails where + K: TryInto, + K::Error: std::error::Error + Send + Sync + 'static, V: TryInto, V::Error: std::error::Error + Send + Sync + 'static, { + let name = name.try_into().expect("invalid crate name"); let version = version.try_into().expect("invalid version"); let crate_id = sqlx::query_scalar!( r#"SELECT id as "id: CrateId" FROM crates WHERE name = $1"#, - name + name as _ ) .fetch_one(&mut *conn) .await @@ -765,7 +768,7 @@ mod tests { let releases = releases_for_crate(&mut *conn, crate_id).await.unwrap(); - CrateDetails::new(&mut *conn, name, &version, req_version, releases) + CrateDetails::new(&mut *conn, &name, &version, req_version, releases) .await .unwrap() .unwrap() diff --git a/crates/bin/docs_rs_web/src/handlers/features.rs b/crates/bin/docs_rs_web/src/handlers/features.rs index 255fe4c73..4f7227e07 100644 --- a/crates/bin/docs_rs_web/src/handlers/features.rs +++ b/crates/bin/docs_rs_web/src/handlers/features.rs @@ -181,8 +181,8 @@ pub(crate) async fn build_features_handler( FROM releases INNER JOIN crates ON crates.id = releases.crate_id WHERE crates.name = $1 AND releases.version = $2"#, - params.name(), - version.to_string(), + params.name() as _, + version as _, ) .fetch_optional(&mut *conn) .await? diff --git a/crates/bin/docs_rs_web/src/handlers/releases.rs b/crates/bin/docs_rs_web/src/handlers/releases.rs index ef6fd633a..a55f2c06c 100644 --- a/crates/bin/docs_rs_web/src/handlers/releases.rs +++ b/crates/bin/docs_rs_web/src/handlers/releases.rs @@ -552,9 +552,10 @@ pub(crate) async fn search_handler( // since we never pass a version into `match_version` here, we'll never get // `MatchVersion::Exact`, so the distinction between `Exact` and `Semver` doesn't // matter - if let Ok(matchver) = match_version(&mut conn, krate, &ReqVersion::Latest) - .await - .map(|matched_release| matched_release.into_exactly_named()) + if let Ok(krate) = krate.parse::() + && let Ok(matchver) = match_version(&mut conn, &krate, &ReqVersion::Latest) + .await + .map(|matched_release| matched_release.into_exactly_named()) { query_params.remove("query"); queries.extend(query_params); @@ -563,7 +564,7 @@ pub(crate) async fn search_handler( let params = RustdocParams::from_matched_release(&matchver); trace!( - krate, + %krate, ?params, "redirecting I'm feeling lucky search to crate page" ); @@ -809,7 +810,7 @@ mod tests { async_wrapper(|env| async move { let mut conn = env.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "foo").await?; + let crate_id = initialize_crate(&mut conn, &FOO).await?; let release_id = initialize_release(&mut conn, crate_id, &V1).await?; let build_id = initialize_build(&mut conn, release_id).await?; diff --git a/crates/bin/docs_rs_web/src/handlers/rustdoc.rs b/crates/bin/docs_rs_web/src/handlers/rustdoc.rs index a36099073..83cb33d8f 100644 --- a/crates/bin/docs_rs_web/src/handlers/rustdoc.rs +++ b/crates/bin/docs_rs_web/src/handlers/rustdoc.rs @@ -3559,7 +3559,7 @@ mod test { ) -> Result<()> { let env = TestEnvironment::new().await?; - const NAME: &str = "dummy"; + const NAME: &KrateName = &KrateName::from_static("dummy"); const VERSION: Version = Version::new(0, 1, 0); const TARGET: &str = "x86_64-unknown-linux-gnu"; const FORMAT_VERSION: RustdocJsonFormatVersion = RustdocJsonFormatVersion::Latest; diff --git a/crates/bin/docs_rs_web/src/handlers/source.rs b/crates/bin/docs_rs_web/src/handlers/source.rs index eaf7a45ab..a8beb67ff 100644 --- a/crates/bin/docs_rs_web/src/handlers/source.rs +++ b/crates/bin/docs_rs_web/src/handlers/source.rs @@ -19,7 +19,7 @@ use axum::{Extension, response::IntoResponse}; use axum_extra::{TypedHeader, headers::HeaderMapExt}; use docs_rs_headers::{CanonicalUrl, IfNoneMatch}; use docs_rs_storage::{AsyncStorage, PathNotFoundError}; -use docs_rs_types::{BuildId, ReqVersion, Version}; +use docs_rs_types::{BuildId, KrateName, ReqVersion, Version}; use mime::Mime; use std::{cmp::Ordering, sync::Arc}; use tracing::instrument; @@ -73,7 +73,7 @@ impl FileList { #[instrument(skip(conn))] async fn from_path( conn: &mut sqlx::PgConnection, - name: &str, + name: &KrateName, version: &Version, folder: &str, ) -> Result> { @@ -82,7 +82,7 @@ impl FileList { FROM releases INNER JOIN crates ON crates.id = releases.crate_id WHERE crates.name = $1 AND releases.version = $2", - name, + name as _, version as _, ) .fetch_optional(&mut *conn) @@ -238,7 +238,7 @@ pub(crate) async fn source_browser_handler( WHERE name = $1 AND version = $2"#, - params.name(), + params.name() as _, version as _, ) .fetch_one(&mut *conn) diff --git a/crates/bin/docs_rs_web/src/match_release.rs b/crates/bin/docs_rs_web/src/match_release.rs index 984055fdb..a28e8445f 100644 --- a/crates/bin/docs_rs_web/src/match_release.rs +++ b/crates/bin/docs_rs_web/src/match_release.rs @@ -147,7 +147,7 @@ fn semver_match<'a, F: Fn(&Release) -> bool>( #[instrument(skip(conn))] pub(crate) async fn match_version( conn: &mut sqlx::PgConnection, - name: &str, + name: &KrateName, input_version: &ReqVersion, ) -> Result { let (crate_id, name, corrected_name) = { @@ -158,17 +158,13 @@ pub(crate) async fn match_version( name as "name: KrateName" FROM crates WHERE normalize_crate_name(name) = normalize_crate_name($1)"#, - name, + name as _, ) .fetch_optional(&mut *conn) .await .context("error fetching crate")? .ok_or(AxumNope::CrateNotFound)?; - let name: KrateName = name - .parse() - .expect("here we know it's valid, because we found it after normalizing"); - if row.name != name { (row.id, name, Some(row.name)) } else { @@ -193,7 +189,7 @@ pub(crate) async fn match_version( .find(|release| &release.version == parsed_req_version) { return Ok(MatchedRelease { - name, + name: name.clone(), corrected_name, req_version: input_version.clone(), release: release.clone(), @@ -254,7 +250,7 @@ mod tests { use crate::testing::{TestEnvironment, async_wrapper}; use docs_rs_database::Pool; use docs_rs_test_fakes::FakeBuild; - use docs_rs_types::ReleaseId; + use docs_rs_types::{ReleaseId, testing::FOO}; use std::str::FromStr as _; async fn release(version: &str, env: &TestEnvironment) -> ReleaseId { @@ -272,7 +268,7 @@ mod tests { let mut conn = pool.get_async().await.unwrap(); let version = match_version( &mut conn, - "foo", + &FOO, &ReqVersion::from_str(v.unwrap_or_default()).unwrap(), ) .await diff --git a/crates/bin/docs_rs_web/src/metadata.rs b/crates/bin/docs_rs_web/src/metadata.rs index 85ec82057..1c3ad57bc 100644 --- a/crates/bin/docs_rs_web/src/metadata.rs +++ b/crates/bin/docs_rs_web/src/metadata.rs @@ -29,7 +29,7 @@ pub(crate) struct MetaData { impl MetaData { pub(crate) async fn from_crate( conn: &mut sqlx::PgConnection, - name: &str, + name: &KrateName, version: &Version, req_version: Option, ) -> Result { @@ -53,8 +53,8 @@ impl MetaData { DESC LIMIT 1 ) AS builds ON true WHERE crates.name = $1 AND releases.version = $2"#, - name, - version.to_string(), + name as _, + version as _, ) .fetch_one(&mut *conn) .await @@ -84,6 +84,7 @@ mod tests { use crate::testing::TestEnvironment; use super::*; + use docs_rs_types::testing::{FOO, V0_1}; use serde_json::json; #[test] @@ -173,13 +174,7 @@ mod tests { .await?; let mut conn = env.async_conn().await?; - let metadata = MetaData::from_crate( - &mut conn, - "foo", - &"0.1.0".parse().unwrap(), - Some(ReqVersion::Latest), - ) - .await; + let metadata = MetaData::from_crate(&mut conn, &FOO, &V0_1, Some(ReqVersion::Latest)).await; assert_eq!( metadata.unwrap(), MetaData { diff --git a/crates/lib/docs_rs_build_limits/src/blacklist.rs b/crates/lib/docs_rs_build_limits/src/blacklist.rs index 445e8021b..8b17559fd 100644 --- a/crates/lib/docs_rs_build_limits/src/blacklist.rs +++ b/crates/lib/docs_rs_build_limits/src/blacklist.rs @@ -19,7 +19,7 @@ pub enum BlacklistError { pub async fn is_blacklisted(conn: &mut sqlx::PgConnection, name: &KrateName) -> Result { Ok(sqlx::query_scalar!( r#"SELECT 1 FROM blacklisted_crates WHERE crate_name = $1;"#, - name + name as _ ) .fetch_optional(conn) .await? @@ -50,7 +50,7 @@ pub async fn add_crate(conn: &mut sqlx::PgConnection, name: &KrateName) -> Resul sqlx::query!( "INSERT INTO blacklisted_crates (crate_name) VALUES ($1);", - name + name as _ ) .execute(conn) .await?; @@ -66,7 +66,7 @@ pub async fn remove_crate(conn: &mut sqlx::PgConnection, name: &KrateName) -> Re sqlx::query!( "DELETE FROM blacklisted_crates WHERE crate_name = $1;", - name + name as _ ) .execute(conn) .await?; diff --git a/crates/lib/docs_rs_build_limits/src/overrides.rs b/crates/lib/docs_rs_build_limits/src/overrides.rs index 661550706..6bee2e960 100644 --- a/crates/lib/docs_rs_build_limits/src/overrides.rs +++ b/crates/lib/docs_rs_build_limits/src/overrides.rs @@ -45,7 +45,7 @@ impl Overrides { ) -> Result> { Ok(sqlx::query!( "SELECT * FROM sandbox_overrides WHERE crate_name = $1", - krate + krate as _ ) .fetch_optional(conn) .await? @@ -65,7 +65,7 @@ impl Overrides { ); } - if sqlx::query_scalar!("SELECT id FROM crates WHERE crates.name = $1", krate) + if sqlx::query_scalar!("SELECT id FROM crates WHERE crates.name = $1", krate as _) .fetch_optional(&mut *conn) .await? .is_none() @@ -85,7 +85,7 @@ impl Overrides { max_targets = $3, timeout_seconds = $4 ", - krate, + krate as _, overrides.memory.map(|i| i as i64), overrides.targets.map(|i| i as i32), overrides.timeout.map(|d| d.as_secs() as i32), @@ -96,9 +96,12 @@ impl Overrides { } pub async fn remove(conn: &mut sqlx::PgConnection, krate: &KrateName) -> Result<()> { - sqlx::query!("DELETE FROM sandbox_overrides WHERE crate_name = $1", krate) - .execute(conn) - .await?; + sqlx::query!( + "DELETE FROM sandbox_overrides WHERE crate_name = $1", + krate as _ + ) + .execute(conn) + .await?; Ok(()) } } diff --git a/crates/lib/docs_rs_build_queue/Cargo.toml b/crates/lib/docs_rs_build_queue/Cargo.toml index 9e6e0bc63..a5a141019 100644 --- a/crates/lib/docs_rs_build_queue/Cargo.toml +++ b/crates/lib/docs_rs_build_queue/Cargo.toml @@ -33,3 +33,4 @@ docs_rs_opentelemetry = { path = "../docs_rs_opentelemetry", features = ["testin docs_rs_types = { path = "../docs_rs_types", features = ["testing"] } docs_rs_utils = { path = "../../lib/docs_rs_utils", features = ["testing"] } pretty_assertions = { workspace = true } +test-case = { workspace = true } diff --git a/crates/lib/docs_rs_build_queue/src/priority.rs b/crates/lib/docs_rs_build_queue/src/priority.rs index ab26c11d2..413d9b257 100644 --- a/crates/lib/docs_rs_build_queue/src/priority.rs +++ b/crates/lib/docs_rs_build_queue/src/priority.rs @@ -1,5 +1,6 @@ use crate::PRIORITY_DEFAULT; use anyhow::Result; +use docs_rs_types::KrateName; use futures_util::stream::TryStreamExt; /// Get the build queue priority for a crate, returns the matching pattern too @@ -16,12 +17,12 @@ pub async fn list_crate_priorities(conn: &mut sqlx::PgConnection) -> Result Result> { // Search the `priority` table for a priority where the crate name matches the stored pattern Ok(sqlx::query!( "SELECT pattern, priority FROM crate_priorities WHERE $1 LIKE pattern LIMIT 1", - name + name as _ ) .fetch_optional(&mut *conn) .await? @@ -29,7 +30,7 @@ pub async fn get_crate_pattern_and_priority( } /// Get the build queue priority for a crate -pub async fn get_crate_priority(conn: &mut sqlx::PgConnection, name: &str) -> Result { +pub async fn get_crate_priority(conn: &mut sqlx::PgConnection, name: &KrateName) -> Result { Ok(get_crate_pattern_and_priority(conn, name) .await? .map_or(PRIORITY_DEFAULT, |(_, priority)| priority)) @@ -76,44 +77,51 @@ mod tests { use docs_rs_config::AppConfig as _; use docs_rs_database::{Config, testing::TestDatabase}; use docs_rs_opentelemetry::testing::TestMetrics; - + use test_case::test_case; + + #[test_case( + "docsrs-%", + &["docsrs-database", "docsrs-", "docsrs-s3", "docsrs-webserver"], + &["docsrs"] + )] + #[test_case( + "_c_", + &["rcc"], + &["rc"] + )] + #[test_case( + "hexponent", + &["hexponent"], + &["hexponents", "floathexponent"] + )] #[tokio::test(flavor = "multi_thread")] - async fn set_priority() -> Result<()> { + async fn set_priority( + pattern: &str, + should_match: &[&str], + should_not_match: &[&str], + ) -> Result<()> { let test_metrics = TestMetrics::new(); let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; + const PRIO: i32 = -100; + let mut conn = db.async_conn().await?; - set_crate_priority(&mut conn, "docsrs-%", -100).await?; - assert_eq!( - get_crate_priority(&mut conn, "docsrs-database").await?, - -100 - ); - assert_eq!(get_crate_priority(&mut conn, "docsrs-").await?, -100); - assert_eq!(get_crate_priority(&mut conn, "docsrs-s3").await?, -100); - assert_eq!( - get_crate_priority(&mut conn, "docsrs-webserver").await?, - -100 - ); - assert_eq!( - get_crate_priority(&mut conn, "docsrs").await?, - PRIORITY_DEFAULT - ); + set_crate_priority(&mut conn, pattern, PRIO).await?; - set_crate_priority(&mut conn, "_c_", 100).await?; - assert_eq!(get_crate_priority(&mut conn, "rcc").await?, 100); - assert_eq!(get_crate_priority(&mut conn, "rc").await?, PRIORITY_DEFAULT); + for name in should_match { + assert_eq!( + get_crate_priority(&mut conn, &name.parse().unwrap()).await?, + PRIO + ); + } - set_crate_priority(&mut conn, "hexponent", 10).await?; - assert_eq!(get_crate_priority(&mut conn, "hexponent").await?, 10); - assert_eq!( - get_crate_priority(&mut conn, "hexponents").await?, - PRIORITY_DEFAULT - ); - assert_eq!( - get_crate_priority(&mut conn, "floathexponent").await?, - PRIORITY_DEFAULT - ); + for name in should_not_match { + assert_eq!( + get_crate_priority(&mut conn, &name.parse().unwrap()).await?, + PRIORITY_DEFAULT + ); + } Ok(()) } @@ -124,43 +132,16 @@ mod tests { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; + let pattern = "docsrs-%"; + let krate = KrateName::from_static("docsrs-"); + const PRIO: i32 = -100; - set_crate_priority(&mut conn, "docsrs-%", -100).await?; - assert_eq!(get_crate_priority(&mut conn, "docsrs-").await?, -100); + set_crate_priority(&mut conn, pattern, PRIO).await?; + assert_eq!(get_crate_priority(&mut conn, &krate).await?, PRIO); + assert_eq!(remove_crate_priority(&mut conn, pattern).await?, Some(PRIO)); assert_eq!( - remove_crate_priority(&mut conn, "docsrs-%").await?, - Some(-100) - ); - assert_eq!( - get_crate_priority(&mut conn, "docsrs-").await?, - PRIORITY_DEFAULT - ); - - Ok(()) - } - - #[tokio::test(flavor = "multi_thread")] - async fn get_priority() -> Result<()> { - let test_metrics = TestMetrics::new(); - let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; - - let mut conn = db.async_conn().await?; - - set_crate_priority(&mut conn, "docsrs-%", -100).await?; - - assert_eq!( - get_crate_priority(&mut conn, "docsrs-database").await?, - -100 - ); - assert_eq!(get_crate_priority(&mut conn, "docsrs-").await?, -100); - assert_eq!(get_crate_priority(&mut conn, "docsrs-s3").await?, -100); - assert_eq!( - get_crate_priority(&mut conn, "docsrs-webserver").await?, - -100 - ); - assert_eq!( - get_crate_priority(&mut conn, "unrelated").await?, + get_crate_priority(&mut conn, &krate).await?, PRIORITY_DEFAULT ); @@ -174,26 +155,14 @@ mod tests { let mut conn = db.async_conn().await?; - assert_eq!( - get_crate_priority(&mut conn, "docsrs").await?, - PRIORITY_DEFAULT - ); - assert_eq!( - get_crate_priority(&mut conn, "rcc").await?, - PRIORITY_DEFAULT - ); - assert_eq!( - get_crate_priority(&mut conn, "lasso").await?, - PRIORITY_DEFAULT - ); - assert_eq!( - get_crate_priority(&mut conn, "hexponent").await?, - PRIORITY_DEFAULT - ); - assert_eq!( - get_crate_priority(&mut conn, "rust4lyfe").await?, - PRIORITY_DEFAULT - ); + for name in &["docsrs", "rcc", "lasso", "hexponent", "rust4lyfe"] { + let krate = KrateName::from_static(name); + + assert_eq!( + get_crate_priority(&mut conn, &krate).await?, + PRIORITY_DEFAULT + ); + } Ok(()) } diff --git a/crates/lib/docs_rs_database/src/releases.rs b/crates/lib/docs_rs_database/src/releases.rs index 0fd23d182..169cec2cf 100644 --- a/crates/lib/docs_rs_database/src/releases.rs +++ b/crates/lib/docs_rs_database/src/releases.rs @@ -3,7 +3,8 @@ use anyhow::{Context, Result, anyhow}; use docs_rs_cargo_metadata::{MetadataPackage, ReleaseDependencyList}; use docs_rs_registry_api::{CrateData, CrateOwner, ReleaseData}; use docs_rs_types::{ - BuildId, BuildStatus, CompressionAlgorithm, CrateId, DocCoverage, Feature, ReleaseId, Version, + BuildId, BuildStatus, CompressionAlgorithm, CrateId, DocCoverage, Feature, KrateName, + ReleaseId, Version, }; use docs_rs_utils::rustc_version::parse_rustc_date; use futures_util::stream::TryStreamExt; @@ -303,7 +304,7 @@ pub async fn update_build_with_error( Ok(build_id) } -pub async fn initialize_crate(conn: &mut sqlx::PgConnection, name: &str) -> Result { +pub async fn initialize_crate(conn: &mut sqlx::PgConnection, name: &KrateName) -> Result { sqlx::query_scalar!( "INSERT INTO crates (name) VALUES ($1) @@ -311,7 +312,7 @@ pub async fn initialize_crate(conn: &mut sqlx::PgConnection, name: &str) -> Resu SET -- this `SET` is needed so the id is always returned. name = EXCLUDED.name RETURNING id", - name + name as _ ) .fetch_one(&mut *conn) .await @@ -502,13 +503,13 @@ async fn add_keywords_into_database( #[instrument(skip(conn))] pub async fn update_crate_data_in_database( conn: &mut sqlx::PgConnection, - name: &str, + name: &KrateName, registry_data: &CrateData, ) -> Result<()> { info!("Updating crate data for {}", name); let crate_id = sqlx::query_scalar!( r#"SELECT id as "id: CrateId" FROM crates WHERE crates.name = $1"#, - name + name as _ ) .fetch_one(&mut *conn) .await?; @@ -672,7 +673,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let release_id = initialize_release(&mut conn, crate_id, &V0_1).await?; let build_id = initialize_build(&mut conn, release_id).await?; @@ -707,7 +708,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let release_id = initialize_release(&mut conn, crate_id, &V0_1).await?; let build_id = initialize_build(&mut conn, release_id).await?; @@ -757,7 +758,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let release_id = initialize_release(&mut conn, crate_id, &V0_1).await?; let build_id = initialize_build(&mut conn, release_id).await?; @@ -803,7 +804,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let release_id = initialize_release(&mut conn, crate_id, &V0_1).await?; let build_id = initialize_build(&mut conn, release_id).await?; @@ -953,7 +954,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let owner1 = CrateOwner { avatar: "avatar".repeat(100), @@ -994,7 +995,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let owner1 = CrateOwner { avatar: "avatar".into(), @@ -1035,7 +1036,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; // set initial owner details update_owners_in_database( @@ -1085,7 +1086,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; // set initial owner details update_owners_in_database( @@ -1204,19 +1205,18 @@ mod test { let mut conn = db.async_conn().await?; - let name = "krate"; - let crate_id = initialize_crate(&mut conn, name).await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let id = sqlx::query_scalar!( r#"SELECT id as "id: CrateId" FROM crates WHERE name = $1"#, - name + KRATE as _ ) .fetch_one(&mut *conn) .await?; assert_eq!(crate_id, id); - let same_crate_id = initialize_crate(&mut conn, name).await?; + let same_crate_id = initialize_crate(&mut conn, &KRATE).await?; assert_eq!(crate_id, same_crate_id); Ok(()) @@ -1228,8 +1228,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let name = "krate"; - let crate_id = initialize_crate(&mut conn, name).await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let release_id = initialize_release(&mut conn, crate_id, &V1).await?; @@ -1255,8 +1254,7 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - let name = "krate"; - let crate_id = initialize_crate(&mut conn, name).await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let release_id = initialize_release(&mut conn, crate_id, &V1).await?; let build_id = initialize_build(&mut conn, release_id).await?; @@ -1283,14 +1281,14 @@ mod test { let mut conn = db.async_conn().await?; - let name: String = "krate".repeat(100); + let name: KrateName = "krate".repeat(100)[..64].parse()?; let crate_id = initialize_crate(&mut conn, &name).await?; let db_name = sqlx::query_scalar!("SELECT name FROM crates WHERE id = $1", crate_id.0) .fetch_one(&mut *conn) .await?; - assert_eq!(db_name, name); + assert_eq!(db_name, name.as_str()); Ok(()) } @@ -1302,7 +1300,7 @@ mod test { let mut conn = db.async_conn().await?; - let crate_id = initialize_crate(&mut conn, "krate").await?; + let crate_id = initialize_crate(&mut conn, &KRATE).await?; let version = Version::parse(&format!( "1.2.3-{}+{}", "prerelease".repeat(100), diff --git a/crates/lib/docs_rs_registry_api/src/api.rs b/crates/lib/docs_rs_registry_api/src/api.rs index 0525c5b9a..1a32a8737 100644 --- a/crates/lib/docs_rs_registry_api/src/api.rs +++ b/crates/lib/docs_rs_registry_api/src/api.rs @@ -4,7 +4,7 @@ use crate::{ }; use anyhow::{Context, Result, anyhow, bail}; use chrono::{DateTime, Utc}; -use docs_rs_types::Version; +use docs_rs_types::{KrateName, Version}; use docs_rs_utils::{APP_USER_AGENT, retry_async}; use reqwest::header::{ACCEPT, HeaderValue, USER_AGENT}; use serde::Deserialize; @@ -46,7 +46,7 @@ impl RegistryApi { } #[instrument(skip(self))] - pub async fn get_crate_data(&self, name: &str) -> Result { + pub async fn get_crate_data(&self, name: &KrateName) -> Result { let owners = self .get_owners(name) .await @@ -56,7 +56,11 @@ impl RegistryApi { } #[instrument(skip(self))] - pub async fn get_release_data(&self, name: &str, version: &Version) -> Result { + pub async fn get_release_data( + &self, + name: &KrateName, + version: &Version, + ) -> Result { let (release_time, yanked, downloads) = self .get_release_time_yanked_downloads(name, version) .await @@ -72,14 +76,14 @@ impl RegistryApi { /// Get release_time, yanked and downloads from the registry's API async fn get_release_time_yanked_downloads( &self, - name: &str, + name: &KrateName, version: &Version, ) -> Result<(DateTime, bool, i32)> { let url = { let mut url = self.api_base.clone(); url.path_segments_mut() .map_err(|()| anyhow!("Invalid API url"))? - .extend(&["api", "v1", "crates", name, "versions"]); + .extend(&["api", "v1", "crates", name.as_str(), "versions"]); url }; @@ -124,12 +128,12 @@ impl RegistryApi { } /// Fetch owners from the registry's API - async fn get_owners(&self, name: &str) -> Result> { + async fn get_owners(&self, name: &KrateName) -> Result> { let url = { let mut url = self.api_base.clone(); url.path_segments_mut() .map_err(|()| anyhow!("Invalid API url"))? - .extend(&["api", "v1", "crates", name, "owners"]); + .extend(&["api", "v1", "crates", name.as_str(), "owners"]); url }; diff --git a/crates/lib/docs_rs_storage/src/storage/blocking.rs b/crates/lib/docs_rs_storage/src/storage/blocking.rs index 4f6cce847..bd4c5a152 100644 --- a/crates/lib/docs_rs_storage/src/storage/blocking.rs +++ b/crates/lib/docs_rs_storage/src/storage/blocking.rs @@ -1,6 +1,6 @@ use crate::{blob::Blob, file::FileEntry, storage::non_blocking::AsyncStorage, types::FileRange}; use anyhow::Result; -use docs_rs_types::{BuildId, CompressionAlgorithm, Version}; +use docs_rs_types::{BuildId, CompressionAlgorithm, KrateName, Version}; use std::{fmt, path::Path, sync::Arc}; use tokio::runtime; @@ -22,7 +22,7 @@ impl Storage { pub fn fetch_source_file( &self, - name: &str, + name: &KrateName, version: &Version, latest_build_id: Option, path: &str, @@ -39,7 +39,7 @@ impl Storage { pub fn rustdoc_file_exists( &self, - name: &str, + name: &KrateName, version: &Version, latest_build_id: Option, path: &str, diff --git a/crates/lib/docs_rs_storage/src/storage/non_blocking.rs b/crates/lib/docs_rs_storage/src/storage/non_blocking.rs index 9731740d1..5d4cf6920 100644 --- a/crates/lib/docs_rs_storage/src/storage/non_blocking.rs +++ b/crates/lib/docs_rs_storage/src/storage/non_blocking.rs @@ -19,7 +19,7 @@ use anyhow::Result; use dashmap::DashMap; use docs_rs_mimes::{self as mimes, detect_mime}; use docs_rs_opentelemetry::AnyMeterProvider; -use docs_rs_types::{BuildId, CompressionAlgorithm, Version}; +use docs_rs_types::{BuildId, CompressionAlgorithm, KrateName, Version}; use docs_rs_utils::spawn_blocking; use futures_util::stream::BoxStream; use std::{ @@ -75,7 +75,7 @@ impl AsyncStorage { #[instrument] pub async fn stream_rustdoc_file( &self, - name: &str, + name: &KrateName, version: &Version, latest_build_id: Option, path: &str, @@ -94,7 +94,7 @@ impl AsyncStorage { pub async fn fetch_source_file( &self, - name: &str, + name: &KrateName, version: &Version, latest_build_id: Option, path: &str, @@ -109,7 +109,7 @@ impl AsyncStorage { #[instrument] pub async fn stream_source_file( &self, - name: &str, + name: &KrateName, version: &Version, latest_build_id: Option, path: &str, @@ -128,7 +128,7 @@ impl AsyncStorage { #[instrument] pub async fn rustdoc_file_exists( &self, - name: &str, + name: &KrateName, version: &Version, latest_build_id: Option, path: &str, diff --git a/crates/lib/docs_rs_storage/src/utils/storage_path.rs b/crates/lib/docs_rs_storage/src/utils/storage_path.rs index 48ba13c5c..123b675de 100644 --- a/crates/lib/docs_rs_storage/src/utils/storage_path.rs +++ b/crates/lib/docs_rs_storage/src/utils/storage_path.rs @@ -1,12 +1,12 @@ use docs_rs_rustdoc_json::RustdocJsonFormatVersion; -use docs_rs_types::{CompressionAlgorithm, Version}; +use docs_rs_types::{CompressionAlgorithm, KrateName, Version}; -pub fn rustdoc_archive_path(name: &str, version: &Version) -> String { +pub fn rustdoc_archive_path(name: &KrateName, version: &Version) -> String { format!("rustdoc/{name}/{version}.zip") } pub fn rustdoc_json_path( - name: &str, + name: &KrateName, version: &Version, target: &str, format_version: RustdocJsonFormatVersion, @@ -24,6 +24,6 @@ pub fn rustdoc_json_path( path } -pub fn source_archive_path(name: &str, version: &Version) -> String { +pub fn source_archive_path(name: &KrateName, version: &Version) -> String { format!("sources/{name}/{version}.zip") } diff --git a/crates/lib/docs_rs_test_fakes/src/legacy.rs b/crates/lib/docs_rs_test_fakes/src/legacy.rs index cab64b3ff..fcfd45146 100644 --- a/crates/lib/docs_rs_test_fakes/src/legacy.rs +++ b/crates/lib/docs_rs_test_fakes/src/legacy.rs @@ -13,7 +13,8 @@ use docs_rs_storage::{ source_archive_path, }; use docs_rs_types::{ - BuildId, BuildStatus, CompressionAlgorithm, DocCoverage, ReleaseId, Version, VersionReq, + BuildId, BuildStatus, CompressionAlgorithm, DocCoverage, KrateName, ReleaseId, Version, + VersionReq, }; use std::{ collections::{BTreeMap, HashMap}, @@ -25,18 +26,21 @@ use tracing::debug; /// Create a fake release in the database that failed before the build. /// This is a temporary small factory function only until we refactored the /// `FakeRelease` and `FakeBuild` factories to be more flexible. -pub async fn fake_release_that_failed_before_build( +pub async fn fake_release_that_failed_before_build( conn: &mut sqlx::PgConnection, - name: &str, + name: K, version: V, errors: &str, ) -> Result<(ReleaseId, BuildId)> where + K: TryInto, + K::Error: std::error::Error + Send + Sync + 'static, V: TryInto, V::Error: std::error::Error + Send + Sync + 'static, { + let name = name.try_into()?; let version = version.try_into()?; - let crate_id = initialize_crate(&mut *conn, name).await?; + let crate_id = initialize_crate(&mut *conn, &name).await?; let release_id = initialize_release(&mut *conn, crate_id, &version).await?; let build_id = initialize_build(&mut *conn, release_id).await?; @@ -163,10 +167,16 @@ impl<'a> FakeRelease<'a> { self } - pub fn name(mut self, new: &str) -> Self { - self.package.name = new.into(); + pub fn name(mut self, new: K) -> Self + where + K: TryInto, + K::Error: fmt::Debug, + { + let new = new.try_into().expect("invalid crate name").to_string(); + + self.package.name = new.clone(); self.package.id = format!("{new}-id"); - self.package.targets[0].name = new.into(); + self.package.targets[0].name = new; self } @@ -406,9 +416,12 @@ impl<'a> FakeRelease<'a> { source_directory.display() ); if archive_storage { + // NOTE: should we migrate MetadataPackage? + let krate_name: KrateName = package.name.parse()?; + let archive = match kind { - FileKind::Rustdoc => rustdoc_archive_path(&package.name, &package.version), - FileKind::Sources => source_archive_path(&package.name, &package.version), + FileKind::Rustdoc => rustdoc_archive_path(&krate_name, &package.version), + FileKind::Sources => source_archive_path(&krate_name, &package.version), }; debug!("store in archive: {:?}", archive); Ok(storage @@ -514,6 +527,8 @@ impl<'a> FakeRelease<'a> { self.doc_targets.insert(0, default_target.to_owned()); } + let krate_name: KrateName = package.name.parse()?; + for target in &self.doc_targets { let dummy_rustdoc_json_content = serde_json::to_vec(&serde_json::json!({ "format_version": 42 @@ -529,7 +544,7 @@ impl<'a> FakeRelease<'a> { storage .store_one_uncompressed( &rustdoc_json_path( - &package.name, + &krate_name, &package.version, target, format_version, @@ -546,7 +561,7 @@ impl<'a> FakeRelease<'a> { // be set to docsrs_metadata::HOST_TARGET, because then tests fail on all // non-linux platforms. let mut async_conn = pool.get_async().await?; - let crate_id = initialize_crate(&mut async_conn, &package.name).await?; + let crate_id = initialize_crate(&mut async_conn, &krate_name).await?; let release_id = initialize_release(&mut async_conn, crate_id, &package.version).await?; docs_rs_database::releases::finish_release( @@ -569,7 +584,7 @@ impl<'a> FakeRelease<'a> { .await?; docs_rs_database::releases::update_crate_data_in_database( &mut async_conn, - &package.name, + &krate_name, &self.registry_crate_data, ) .await?; diff --git a/crates/lib/docs_rs_types/src/krate_name.rs b/crates/lib/docs_rs_types/src/krate_name.rs index 4288d6c69..a68a62c84 100644 --- a/crates/lib/docs_rs_types/src/krate_name.rs +++ b/crates/lib/docs_rs_types/src/krate_name.rs @@ -8,14 +8,14 @@ use sqlx::{ postgres::{PgArgumentBuffer, PgHasArrayType, PgTypeInfo, PgValueRef}, prelude::*, }; -use std::{borrow::Cow, fmt, io::Write, ops::Deref, str::FromStr}; +use std::{borrow::Cow, fmt, io::Write, str::FromStr}; /// validated crate name /// /// Right now only used in web::cache, but we'll probably also use it /// to match our routes later. /// -#[derive(Debug, Clone, PartialEq, Eq, Hash, DeserializeFromStr, SerializeDisplay)] +#[derive(Debug, Clone, Eq, PartialOrd, Ord, Hash, DeserializeFromStr, SerializeDisplay)] pub struct KrateName(Cow<'static, str>); impl KrateName { @@ -23,12 +23,18 @@ impl KrateName { pub const fn from_static(s: &'static str) -> Self { KrateName(Cow::Borrowed(s)) } -} -impl Deref for KrateName { - type Target = str; + pub fn as_str(&self) -> &str { + &self.0 + } + + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } +} - fn deref(&self) -> &Self::Target { +impl AsRef for KrateName { + fn as_ref(&self) -> &str { &self.0 } } @@ -60,6 +66,14 @@ impl FromStr for KrateName { } } +impl TryFrom<&str> for KrateName { + type Error = InvalidCrateName; + + fn try_from(value: &str) -> Result { + value.parse() + } +} + impl Type for KrateName { fn type_info() -> PgTypeInfo { >::type_info()