From c7075899cede95dc5121ff432c4334cdf2bc76d3 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Thu, 3 Sep 2026 02:09:28 +0200 Subject: [PATCH 1/2] refactor registry-api, switch to using sparse index for fetching release data --- ...9e244e8a9b001751dac4874a5d03f0b4be759.json | 16 + ...51ada808cd686be2c5adc2470b4b5fec6da8a.json | 42 + ...43808aa79e9416f755d3d47e8946c5a053652.json | 37 + ...8035db0e485f9b3cd388b7d811d8623f2cea1.json | 30 - ...0ddd58b40bc7410a666a629fd006cffc79b0c.json | 28 + ...f8b7eca4b4ad20d8174bd6f74de699d5fd870.json | 35 - Cargo.lock | 18 +- Cargo.toml | 3 +- crates/bin/cratesfyi/src/main.rs | 3 +- crates/bin/docs_rs_admin/src/main.rs | 3 +- .../src/docbuilder/rustwide_builder.rs | 9 +- crates/bin/docs_rs_builder/src/main.rs | 3 +- .../docs_rs_import_release/src/crates_io.rs | 7 +- .../bin/docs_rs_import_release/src/import.rs | 2 +- crates/bin/docs_rs_import_release/src/main.rs | 3 +- crates/bin/docs_rs_watcher/Cargo.toml | 7 +- .../docs_rs_watcher/src/consistency/data.rs | 2 + .../bin/docs_rs_watcher/src/consistency/db.rs | 32 +- .../docs_rs_watcher/src/consistency/diff.rs | 85 ++ .../docs_rs_watcher/src/consistency/index.rs | 88 +- .../docs_rs_watcher/src/consistency/mod.rs | 75 +- crates/bin/docs_rs_web/Cargo.toml | 3 +- crates/bin/docs_rs_web/src/context.rs | 3 +- .../bin/docs_rs_web/src/handlers/releases.rs | 449 ++++------ .../templates/releases/search_results.html | 10 +- crates/lib/docs_rs_context/Cargo.toml | 1 + crates/lib/docs_rs_context/src/context.rs | 4 +- .../src/testing/test_env/blocking.rs | 7 +- .../src/testing/test_env/non_blocking.rs | 23 +- crates/lib/docs_rs_database/src/releases.rs | 109 ++- crates/lib/docs_rs_registry_api/Cargo.toml | 15 +- crates/lib/docs_rs_registry_api/src/api.rs | 807 +++++++++++++----- crates/lib/docs_rs_registry_api/src/config.rs | 17 +- crates/lib/docs_rs_registry_api/src/error.rs | 21 +- crates/lib/docs_rs_registry_api/src/lib.rs | 9 +- crates/lib/docs_rs_registry_api/src/models.rs | 517 ++++++++++- .../docs_rs_registry_api/src/testing/mod.rs | 3 + .../src/testing/test_env.rs | 412 +++++++++ crates/lib/docs_rs_test_fakes/src/legacy.rs | 7 +- 39 files changed, 2260 insertions(+), 685 deletions(-) create mode 100644 .sqlx/query-1eadbdf3aea6c6a21c40c8d2af79e244e8a9b001751dac4874a5d03f0b4be759.json create mode 100644 .sqlx/query-7e2d11c856c7e152b714d67008251ada808cd686be2c5adc2470b4b5fec6da8a.json create mode 100644 .sqlx/query-a077028273b117da2b7feca00ec43808aa79e9416f755d3d47e8946c5a053652.json delete mode 100644 .sqlx/query-a5f6e3ae754baadd1e7f66331048035db0e485f9b3cd388b7d811d8623f2cea1.json create mode 100644 .sqlx/query-c7f88808e3ff4e622770238c6cc0ddd58b40bc7410a666a629fd006cffc79b0c.json delete mode 100644 .sqlx/query-cf716c7aab15930129024f26b40f8b7eca4b4ad20d8174bd6f74de699d5fd870.json create mode 100644 crates/lib/docs_rs_registry_api/src/testing/mod.rs create mode 100644 crates/lib/docs_rs_registry_api/src/testing/test_env.rs diff --git a/.sqlx/query-1eadbdf3aea6c6a21c40c8d2af79e244e8a9b001751dac4874a5d03f0b4be759.json b/.sqlx/query-1eadbdf3aea6c6a21c40c8d2af79e244e8a9b001751dac4874a5d03f0b4be759.json new file mode 100644 index 0000000000..54cab6cf18 --- /dev/null +++ b/.sqlx/query-1eadbdf3aea6c6a21c40c8d2af79e244e8a9b001751dac4874a5d03f0b4be759.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "UPDATE releases\n SET release_time = $3\n FROM crates\n WHERE crates.id = releases.crate_id\n AND crates.name = $1\n AND releases.version = $2", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Text", + "Text", + "Timestamptz" + ] + }, + "nullable": [] + }, + "hash": "1eadbdf3aea6c6a21c40c8d2af79e244e8a9b001751dac4874a5d03f0b4be759" +} diff --git a/.sqlx/query-7e2d11c856c7e152b714d67008251ada808cd686be2c5adc2470b4b5fec6da8a.json b/.sqlx/query-7e2d11c856c7e152b714d67008251ada808cd686be2c5adc2470b4b5fec6da8a.json new file mode 100644 index 0000000000..2fa2273872 --- /dev/null +++ b/.sqlx/query-7e2d11c856c7e152b714d67008251ada808cd686be2c5adc2470b4b5fec6da8a.json @@ -0,0 +1,42 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT\n name as \"name!: KrateName\",\n version as \"version!: Version\",\n yanked,\n release_time\n FROM (\n SELECT\n crates.name,\n releases.version,\n releases.yanked,\n releases.release_time\n FROM crates\n INNER JOIN releases ON releases.crate_id = crates.id\n UNION ALL\n -- crates & releases that are already queued\n -- don't have to be requeued.\n SELECT\n queue.name,\n queue.version,\n NULL as yanked,\n NULL as release_time\n FROM queue\n LEFT OUTER JOIN crates ON crates.name = queue.name\n LEFT OUTER JOIN releases ON (\n releases.crate_id = crates.id AND\n releases.version = queue.version\n )\n WHERE (\n crates.id IS NULL OR\n releases.id IS NULL\n )\n ) AS inp\n ORDER BY name", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "name!: KrateName", + "type_info": "Text", + "origin": "Expression" + }, + { + "ordinal": 1, + "name": "version!: Version", + "type_info": "Text", + "origin": "Expression" + }, + { + "ordinal": 2, + "name": "yanked", + "type_info": "Bool", + "origin": "Expression" + }, + { + "ordinal": 3, + "name": "release_time", + "type_info": "Timestamptz", + "origin": "Expression" + } + ], + "parameters": { + "Left": [] + }, + "nullable": [ + null, + null, + null, + null + ] + }, + "hash": "7e2d11c856c7e152b714d67008251ada808cd686be2c5adc2470b4b5fec6da8a" +} diff --git a/.sqlx/query-a077028273b117da2b7feca00ec43808aa79e9416f755d3d47e8946c5a053652.json b/.sqlx/query-a077028273b117da2b7feca00ec43808aa79e9416f755d3d47e8946c5a053652.json new file mode 100644 index 0000000000..9c9d78ce58 --- /dev/null +++ b/.sqlx/query-a077028273b117da2b7feca00ec43808aa79e9416f755d3d47e8946c5a053652.json @@ -0,0 +1,37 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT version as \"version!: Version\", yanked, release_time\n FROM (\n SELECT releases.version, releases.yanked, releases.release_time\n FROM crates\n INNER JOIN releases ON releases.crate_id = crates.id\n WHERE crates.name = $1\n UNION ALL\n SELECT queue.version, NULL as yanked, NULL as release_time\n FROM queue\n LEFT OUTER JOIN crates ON crates.name = queue.name\n LEFT OUTER JOIN releases ON (\n releases.crate_id = crates.id AND\n releases.version = queue.version\n )\n WHERE queue.name = $1\n AND (crates.id IS NULL OR releases.id IS NULL)\n ) AS inp", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "version!: Version", + "type_info": "Text", + "origin": "Expression" + }, + { + "ordinal": 1, + "name": "yanked", + "type_info": "Bool", + "origin": "Expression" + }, + { + "ordinal": 2, + "name": "release_time", + "type_info": "Timestamptz", + "origin": "Expression" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + null, + null, + null + ] + }, + "hash": "a077028273b117da2b7feca00ec43808aa79e9416f755d3d47e8946c5a053652" +} diff --git a/.sqlx/query-a5f6e3ae754baadd1e7f66331048035db0e485f9b3cd388b7d811d8623f2cea1.json b/.sqlx/query-a5f6e3ae754baadd1e7f66331048035db0e485f9b3cd388b7d811d8623f2cea1.json deleted file mode 100644 index 8d4b1c0df2..0000000000 --- a/.sqlx/query-a5f6e3ae754baadd1e7f66331048035db0e485f9b3cd388b7d811d8623f2cea1.json +++ /dev/null @@ -1,30 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT version as \"version!: Version\", yanked\n FROM (\n SELECT releases.version, releases.yanked\n FROM crates\n INNER JOIN releases ON releases.crate_id = crates.id\n WHERE crates.name = $1\n UNION ALL\n SELECT queue.version, NULL as yanked\n FROM queue\n LEFT OUTER JOIN crates ON crates.name = queue.name\n LEFT OUTER JOIN releases ON (\n releases.crate_id = crates.id AND\n releases.version = queue.version\n )\n WHERE queue.name = $1\n AND (crates.id IS NULL OR releases.id IS NULL)\n ) AS inp", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "version!: Version", - "type_info": "Text", - "origin": "Expression" - }, - { - "ordinal": 1, - "name": "yanked", - "type_info": "Bool", - "origin": "Expression" - } - ], - "parameters": { - "Left": [ - "Text" - ] - }, - "nullable": [ - null, - null - ] - }, - "hash": "a5f6e3ae754baadd1e7f66331048035db0e485f9b3cd388b7d811d8623f2cea1" -} diff --git a/.sqlx/query-c7f88808e3ff4e622770238c6cc0ddd58b40bc7410a666a629fd006cffc79b0c.json b/.sqlx/query-c7f88808e3ff4e622770238c6cc0ddd58b40bc7410a666a629fd006cffc79b0c.json new file mode 100644 index 0000000000..f178336fdf --- /dev/null +++ b/.sqlx/query-c7f88808e3ff4e622770238c6cc0ddd58b40bc7410a666a629fd006cffc79b0c.json @@ -0,0 +1,28 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT release_time FROM releases WHERE id = $1", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "release_time", + "type_info": "Timestamptz", + "origin": { + "Table": { + "table": "releases", + "name": "release_time" + } + } + } + ], + "parameters": { + "Left": [ + "Int4" + ] + }, + "nullable": [ + true + ] + }, + "hash": "c7f88808e3ff4e622770238c6cc0ddd58b40bc7410a666a629fd006cffc79b0c" +} diff --git a/.sqlx/query-cf716c7aab15930129024f26b40f8b7eca4b4ad20d8174bd6f74de699d5fd870.json b/.sqlx/query-cf716c7aab15930129024f26b40f8b7eca4b4ad20d8174bd6f74de699d5fd870.json deleted file mode 100644 index 049d249748..0000000000 --- a/.sqlx/query-cf716c7aab15930129024f26b40f8b7eca4b4ad20d8174bd6f74de699d5fd870.json +++ /dev/null @@ -1,35 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT\n name as \"name!: KrateName\",\n version as \"version!: Version\",\n yanked\n FROM (\n SELECT\n crates.name,\n releases.version,\n releases.yanked\n FROM crates\n INNER JOIN releases ON releases.crate_id = crates.id\n UNION ALL\n -- crates & releases that are already queued\n -- don't have to be requeued.\n SELECT\n queue.name,\n queue.version,\n NULL as yanked\n FROM queue\n LEFT OUTER JOIN crates ON crates.name = queue.name\n LEFT OUTER JOIN releases ON (\n releases.crate_id = crates.id AND\n releases.version = queue.version\n )\n WHERE (\n crates.id IS NULL OR\n releases.id IS NULL\n )\n ) AS inp\n ORDER BY name", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "name!: KrateName", - "type_info": "Text", - "origin": "Expression" - }, - { - "ordinal": 1, - "name": "version!: Version", - "type_info": "Text", - "origin": "Expression" - }, - { - "ordinal": 2, - "name": "yanked", - "type_info": "Bool", - "origin": "Expression" - } - ], - "parameters": { - "Left": [] - }, - "nullable": [ - null, - null, - null - ] - }, - "hash": "cf716c7aab15930129024f26b40f8b7eca4b4ad20d8174bd6f74de699d5fd870" -} diff --git a/Cargo.lock b/Cargo.lock index 3da2ba40e5..1032c2aa43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1520,13 +1520,14 @@ dependencies = [ [[package]] name = "crates-index" -version = "3.14.0" +version = "3.14.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed2cb0f9440838694b31ff3787148d329e02f03c16ff96c28790261563e82174" +checksum = "8759756200ee0abc6f62374c5205af1528d3b0a851c83667a69f76de1e19836f" dependencies = [ "gix 0.83.0", "hex", "home", + "http 1.5.0", "memchr", "rayon", "rustc-hash", @@ -2349,10 +2350,12 @@ dependencies = [ "anyhow", "bon", "chrono", + "crates-index", "docs_rs_config", "docs_rs_env_vars", "docs_rs_types", "docs_rs_utils", + "http 1.5.0", "mime", "mockito", "reqwest", @@ -2360,7 +2363,11 @@ dependencies = [ "reqwest-retry", "serde", "serde_json", + "serde_urlencoded", + "serde_with", "sqlx", + "strum", + "tempfile", "test-case", "thiserror", "tokio", @@ -2523,6 +2530,7 @@ name = "docs_rs_watcher" version = "0.6.0" dependencies = [ "anyhow", + "chrono", "clap", "crates-index", "crates-index-diff", @@ -2535,21 +2543,19 @@ dependencies = [ "docs_rs_fastly", "docs_rs_logging", "docs_rs_opentelemetry", + "docs_rs_registry_api", "docs_rs_repository_stats", "docs_rs_rustdoc_json", "docs_rs_storage", "docs_rs_test_fakes", "docs_rs_types", - "docs_rs_uri", "docs_rs_utils", "futures-util", "itertools 0.15.0", "opentelemetry", "pretty_assertions", "rayon", - "reqwest", "sqlx", - "test-case", "tokio", "tracing", ] @@ -2598,7 +2604,6 @@ dependencies = [ "lol_html", "md5", "mime", - "mockito", "num_cpus", "opentelemetry", "opentelemetry_sdk", @@ -2612,6 +2617,7 @@ dependencies = [ "serde_json", "slug", "sqlx", + "strum", "syntect", "tempfile", "test-case", diff --git a/Cargo.toml b/Cargo.toml index e812548ec2..923cabc16a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ bon = { version = "3.8.1", features = ["experimental-overwritable"] } bytes = "1.11.0" chrono = { version = "0.4.11", default-features = false, features = ["clock", "serde"] } clap = { version = "4.0.22", features = ["derive"] } +crates-index = { version = "3.14.1", default-features = false } futures-util = "0.3.5" http = "1.0.0" itertools = "0.15.0" @@ -51,7 +52,7 @@ postcard = { version = "1.1.3", default-features = false, features = ["use-std"] pretty_assertions = "1.4.0" rand = "0.10" regex = "1" -reqwest = { version = "0.13", features = ["gzip", "json", "stream"] } +reqwest = { version = "0.13", features = ["gzip", "http2", "json", "stream"] } reqwest-middleware = "0.5" reqwest-retry = "0.9" sentry = { version = "0.49.0", features = ["backtrace", "panic", "tower-http", "tracing"] } diff --git a/crates/bin/cratesfyi/src/main.rs b/crates/bin/cratesfyi/src/main.rs index ccdb6c767c..0e5d062e60 100644 --- a/crates/bin/cratesfyi/src/main.rs +++ b/crates/bin/cratesfyi/src/main.rs @@ -47,7 +47,8 @@ impl CommandLine { .await? .with_maybe_cdn()? .with_build_queue()? - .with_registry_api()? + .with_registry_api() + .await? .with_repository_stats()? .with_build_limits()? .build() diff --git a/crates/bin/docs_rs_admin/src/main.rs b/crates/bin/docs_rs_admin/src/main.rs index 931d403de8..e1eec17063 100644 --- a/crates/bin/docs_rs_admin/src/main.rs +++ b/crates/bin/docs_rs_admin/src/main.rs @@ -80,7 +80,8 @@ impl CommandLine { .await? .with_build_queue()? .with_repository_stats()? - .with_registry_api()? + .with_registry_api() + .await? .with_maybe_cdn()? .build()?; 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 359b8f42ba..fe0114fb97 100644 --- a/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs +++ b/crates/bin/docs_rs_builder/src/docbuilder/rustwide_builder.rs @@ -23,6 +23,7 @@ use docs_rs_database::{ service_config::{ConfigName, get_config, set_config}, }; use docs_rs_registry_api::RegistryApi; +use docs_rs_registry_api::ReleaseData; use docs_rs_repository_stats::{RepositoryStatsUpdater, workspaces}; use docs_rs_rustdoc_json::{ RUSTDOC_JSON_COMPRESSION_ALGORITHMS, RustdocJsonFormatVersion, @@ -811,7 +812,7 @@ impl RustwideBuilder { } else { None } - .unwrap_or_default(); + .unwrap_or_else(ReleaseData::dummy); let cargo_metadata = res.cargo_metadata.root(); let repository = self.get_repo(cargo_metadata)?; @@ -1423,13 +1424,11 @@ mod tests { testing::{TestEnvironment, TestEnvironmentExt as _}, }; use docs_rs_config::AppConfig as _; - use docs_rs_utils::block_on_async_with_conn; - // use crate::test::{AxumRouterTestExt, TestEnvironment}; - use docs_rs_registry_api::ReleaseData; use docs_rs_types::{ BuildStatus, CompressionAlgorithm, Feature, ReleaseId, SimpleBuildError, Version, testing::V0_1, }; + use docs_rs_utils::block_on_async_with_conn; use pretty_assertions::assert_eq; use std::{collections::BTreeMap, io, iter, path::PathBuf}; use test_case::test_case; @@ -1840,7 +1839,7 @@ mod tests { "x86_64-pc-windows-msvc".into(), "x86_64-unknown-linux-gnu".into(), ], - &ReleaseData::default(), + &ReleaseData::dummy(), true, false, iter::once(CompressionAlgorithm::Deflate), diff --git a/crates/bin/docs_rs_builder/src/main.rs b/crates/bin/docs_rs_builder/src/main.rs index 1a328b6c46..b78b65dd95 100644 --- a/crates/bin/docs_rs_builder/src/main.rs +++ b/crates/bin/docs_rs_builder/src/main.rs @@ -54,7 +54,8 @@ impl CommandLine { .await? .with_maybe_cdn()? .with_build_queue()? - .with_registry_api()? + .with_registry_api() + .await? .with_repository_stats()? .with_build_limits()? .build() diff --git a/crates/bin/docs_rs_import_release/src/crates_io.rs b/crates/bin/docs_rs_import_release/src/crates_io.rs index f1b2ba2aba..242f19bdb1 100644 --- a/crates/bin/docs_rs_import_release/src/crates_io.rs +++ b/crates/bin/docs_rs_import_release/src/crates_io.rs @@ -1,6 +1,7 @@ use crate::common::download_to_temp_file; use anyhow::{Result, bail}; use async_tar::Archive; +use docs_rs_registry_api::RegistryApi; use docs_rs_storage::compression::wrap_reader_for_decompression; use docs_rs_types::{CompressionAlgorithm, KrateName, Version}; use docs_rs_utils::spawn_blocking; @@ -21,14 +22,12 @@ impl AsRef for SourceDir { } pub(crate) async fn download_and_extract_source( + registry: &RegistryApi, name: &KrateName, version: &Version, ) -> Result { debug!("downloading source"); - let crate_archive = download_to_temp_file(format!( - "https://static.crates.io/crates/{name}/{name}-{version}.crate" - )) - .await?; + let crate_archive = download_to_temp_file(registry.download_url(name, version)?).await?; let temp_dir = spawn_blocking(|| Ok(tempfile::tempdir()?)).await?; diff --git a/crates/bin/docs_rs_import_release/src/import.rs b/crates/bin/docs_rs_import_release/src/import.rs index 7308921799..920d766ef1 100644 --- a/crates/bin/docs_rs_import_release/src/import.rs +++ b/crates/bin/docs_rs_import_release/src/import.rs @@ -99,7 +99,7 @@ async fn import_test_release_inner( build_id: BuildId, ) -> Result<()> { info!("download & inspect source from crates.io..."); - let source_dir = download_and_extract_source(name, version).await?; + let source_dir = download_and_extract_source(registry_api, name, version).await?; let cargo_metadata = spawn_blocking({ let source_dir = source_dir.source_path.clone(); diff --git a/crates/bin/docs_rs_import_release/src/main.rs b/crates/bin/docs_rs_import_release/src/main.rs index 5b67d51e24..302a6d28aa 100644 --- a/crates/bin/docs_rs_import_release/src/main.rs +++ b/crates/bin/docs_rs_import_release/src/main.rs @@ -46,7 +46,8 @@ impl CommandLine { .await? .with_storage() .await? - .with_registry_api()? + .with_registry_api() + .await? .with_repository_stats()? .build()?; diff --git a/crates/bin/docs_rs_watcher/Cargo.toml b/crates/bin/docs_rs_watcher/Cargo.toml index fba68787bd..65d6657ce0 100644 --- a/crates/bin/docs_rs_watcher/Cargo.toml +++ b/crates/bin/docs_rs_watcher/Cargo.toml @@ -8,9 +8,10 @@ edition.workspace = true [dependencies] anyhow = { workspace = true } +chrono = { workspace = true } clap = { workspace = true } # NOTE: on the new infra, switch back from `git-https-reqwest` to `git-https` (curl) once the curl version is new enough -crates-index = { version = "3.0.0", default-features = false, features = ["git", "git-https-reqwest", "git-performance", "parallel"] } +crates-index = { workspace = true, default-features = false, features = ["git", "git-https-reqwest", "git-performance", "parallel", "sparse"] } # NOTE: on the new infra, switch back from `http-reqwest` to `http-curl` once the curl version is new enough crates-index-diff = { version = "31.0.0", default-features = false, features = ["http-reqwest", "max-performance", "semver"] } docs_rs_build_queue = { path = "../../lib/docs_rs_build_queue" } @@ -22,16 +23,15 @@ docs_rs_env_vars = { path = "../../lib/docs_rs_env_vars" } docs_rs_fastly = { path = "../../lib/docs_rs_fastly" } docs_rs_logging = { path = "../../lib/docs_rs_logging" } docs_rs_opentelemetry = { path = "../../lib/docs_rs_opentelemetry" } +docs_rs_registry_api = { path = "../../lib/docs_rs_registry_api" } docs_rs_repository_stats = { path = "../../lib/docs_rs_repository_stats" } docs_rs_storage = { path = "../../lib/docs_rs_storage" } docs_rs_types = { path = "../../lib/docs_rs_types" } -docs_rs_uri = { path = "../../lib/docs_rs_uri" } docs_rs_utils = { path = "../../lib/docs_rs_utils" } futures-util = { workspace = true } itertools = { workspace = true } opentelemetry = { workspace = true } rayon = "1.6.1" -reqwest = { workspace = true } sqlx = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } @@ -46,7 +46,6 @@ docs_rs_storage = { path = "../../lib/docs_rs_storage", features = ["testing"] } docs_rs_test_fakes = { path = "../../lib/docs_rs_test_fakes" } docs_rs_types = { path = "../../lib/docs_rs_types", features = ["testing"] } pretty_assertions = { workspace = true } -test-case = { workspace = true } [lints] workspace = true diff --git a/crates/bin/docs_rs_watcher/src/consistency/data.rs b/crates/bin/docs_rs_watcher/src/consistency/data.rs index feedb1e390..d138471d4a 100644 --- a/crates/bin/docs_rs_watcher/src/consistency/data.rs +++ b/crates/bin/docs_rs_watcher/src/consistency/data.rs @@ -1,3 +1,4 @@ +use chrono::{DateTime, Utc}; use docs_rs_types::{KrateName, Version}; #[derive(Clone, PartialEq, Debug)] @@ -14,4 +15,5 @@ pub(super) type Releases = Vec; pub(super) struct Release { pub(super) version: Version, pub(super) yanked: Option, + pub(super) release_time: Option>, } diff --git a/crates/bin/docs_rs_watcher/src/consistency/db.rs b/crates/bin/docs_rs_watcher/src/consistency/db.rs index 2d01de5475..4479cd7f99 100644 --- a/crates/bin/docs_rs_watcher/src/consistency/db.rs +++ b/crates/bin/docs_rs_watcher/src/consistency/db.rs @@ -8,12 +8,14 @@ pub(super) async fn load(conn: &mut sqlx::PgConnection) -> Result { r#"SELECT name as "name!: KrateName", version as "version!: Version", - yanked + yanked, + release_time FROM ( SELECT crates.name, releases.version, - releases.yanked + releases.yanked, + releases.release_time FROM crates INNER JOIN releases ON releases.crate_id = crates.id UNION ALL @@ -22,7 +24,8 @@ pub(super) async fn load(conn: &mut sqlx::PgConnection) -> Result { SELECT queue.name, queue.version, - NULL as yanked + NULL as yanked, + NULL as release_time FROM queue LEFT OUTER JOIN crates ON crates.name = queue.name LEFT OUTER JOIN releases ON ( @@ -46,6 +49,7 @@ pub(super) async fn load(conn: &mut sqlx::PgConnection) -> Result { .map(|row| Release { version: row.version.clone(), yanked: row.yanked, + release_time: row.release_time, }) .collect(); @@ -65,14 +69,14 @@ pub(super) async fn load_single( name: &KrateName, ) -> Result> { let rows = sqlx::query!( - r#"SELECT version as "version!: Version", yanked + r#"SELECT version as "version!: Version", yanked, release_time FROM ( - SELECT releases.version, releases.yanked + SELECT releases.version, releases.yanked, releases.release_time FROM crates INNER JOIN releases ON releases.crate_id = crates.id WHERE crates.name = $1 UNION ALL - SELECT queue.version, NULL as yanked + SELECT queue.version, NULL as yanked, NULL as release_time FROM queue LEFT OUTER JOIN crates ON crates.name = queue.name LEFT OUTER JOIN releases ON ( @@ -96,6 +100,7 @@ pub(super) async fn load_single( .map(|row| Release { version: row.version, yanked: row.yanked, + release_time: row.release_time, }) .collect(); releases.sort_by(|lhs, rhs| lhs.version.cmp(&rhs.version)); @@ -111,6 +116,7 @@ mod tests { use crate::testing::TestEnvironment; use super::*; + use chrono::{DateTime, Utc}; use docs_rs_types::{ KrateName, testing::{KRATE, V1, V2, V3}, @@ -122,12 +128,14 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn test_load() -> Result<()> { let env = TestEnvironment::new().await?; + let release_time = "2024-01-01T00:00:00Z".parse::>()?; env.build_queue()?.add_crate(&QUEUED, &V1, 0).await?; env.fake_release() .await .name("krate") .version(V2) + .release_time(release_time) .create() .await?; env.fake_release() @@ -135,6 +143,7 @@ mod tests { .name("krate") .version(V3) .yanked(true) + .release_time(release_time) .create() .await?; @@ -157,6 +166,7 @@ mod tests { .name("krate") .version(V0_9_3) .yanked(false) + .release_time(release_time) .create() .await?; env.fake_release() @@ -164,6 +174,7 @@ mod tests { .name("krate") .version(V0_10_3) .yanked(false) + .release_time(release_time) .create() .await?; @@ -179,18 +190,22 @@ mod tests { Release { version: V0_9_3, yanked: Some(false), + release_time: Some(release_time), }, Release { version: V0_10_3, yanked: Some(false), + release_time: Some(release_time), }, Release { version: V2, yanked: Some(false), + release_time: Some(release_time), }, Release { version: V3, yanked: Some(true), + release_time: Some(release_time), } ] }, @@ -199,6 +214,7 @@ mod tests { releases: vec![Release { version: V1, yanked: None, + release_time: None, }] }, ] @@ -209,11 +225,13 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn test_load_single() -> Result<()> { let env = TestEnvironment::new().await?; + let release_time = "2024-01-01T00:00:00Z".parse::>()?; env.fake_release() .await .name(KRATE) .version(V1) + .release_time(release_time) .create() .await?; env.fake_release() @@ -233,10 +251,12 @@ mod tests { Release { version: V1, yanked: Some(false), + release_time: Some(release_time), }, Release { version: V2, yanked: None, + release_time: None, }, ], } diff --git a/crates/bin/docs_rs_watcher/src/consistency/diff.rs b/crates/bin/docs_rs_watcher/src/consistency/diff.rs index fd1b4aefb5..1ad2d27914 100644 --- a/crates/bin/docs_rs_watcher/src/consistency/diff.rs +++ b/crates/bin/docs_rs_watcher/src/consistency/diff.rs @@ -1,4 +1,5 @@ use super::data::Crate; +use chrono::{DateTime, Utc}; use docs_rs_types::{KrateName, Version}; use itertools::{ EitherOrBoth::{Both, Left, Right}, @@ -13,6 +14,7 @@ pub(super) enum Difference { ReleaseNotInIndex(KrateName, Version), ReleaseNotInDb(KrateName, Version), ReleaseYank(KrateName, Version, bool), + ReleaseTime(KrateName, Version, DateTime), } impl Display for Difference { @@ -36,6 +38,12 @@ impl Display for Difference { "release yanked difference, index yanked:{yanked}, release: {name} {version}", )?; } + Difference::ReleaseTime(name, version, release_time) => { + write!( + f, + "release time difference, index release time: {release_time}, release: {name} {version}", + )?; + } } Ok(()) } @@ -74,6 +82,26 @@ where index_yanked, )); } + + // NOTE: `yanked` and `release_time` come both from the + // crates.io sparse index, or historically from the crates.io API. + // We might have releases were both fields are empty because of an + // error, or because the release build is still in progress. + // So there might be cases where `release_time` was empty because + // it was empty on the index (unlikely), or we might have cases + // where the releases is still in progress. + // Since `yanked` is mandatory on the sparse index, we can use that + // as an indicator that `release_time` can be overwritten. + if db_release.yanked.is_some() + && let Some(index_release_time) = index_release.release_time + && db_release.release_time != Some(index_release_time) + { + result.push(Difference::ReleaseTime( + db_crate.name.clone(), + db_release.version.clone(), + index_release_time, + )); + } } Left(db_release) => result.push(Difference::ReleaseNotInIndex( db_crate.name.clone(), @@ -105,6 +133,7 @@ where mod tests { use super::super::data::Release; use super::*; + use chrono::DateTime; use docs_rs_types::testing::{KRATE, V2, V3}; use std::iter; @@ -134,10 +163,12 @@ mod tests { Release { version: V2, yanked: Some(false), + release_time: None, }, Release { version: V3, yanked: Some(true), + release_time: None, }, ], }]; @@ -156,10 +187,12 @@ mod tests { Release { version: V2, yanked: Some(true), + release_time: None, }, Release { version: V3, yanked: Some(true), + release_time: None, }, ], }]; @@ -169,10 +202,12 @@ mod tests { Release { version: V2, yanked: Some(false), + release_time: None, }, Release { version: V3, yanked: Some(true), + release_time: None, }, ], }]; @@ -190,6 +225,55 @@ mod tests { releases: vec![Release { version: V2, yanked: None, + release_time: None, + }], + }]; + let index_releases = [Crate { + name: KRATE, + releases: vec![Release { + version: V2, + yanked: Some(false), + release_time: Some("2024-01-01T00:00:00Z".parse::>().unwrap()), + }], + }]; + + assert!(calculate_diff(db_releases.iter(), index_releases.iter()).is_empty()); + } + + #[test] + fn test_release_time_diff() { + let db_releases = [Crate { + name: KRATE, + releases: vec![Release { + version: V2, + yanked: Some(false), + release_time: Some("2024-01-01T00:00:00Z".parse::>().unwrap()), + }], + }]; + let expected = "2024-01-02T00:00:00Z".parse::>().unwrap(); + let index_releases = [Crate { + name: KRATE, + releases: vec![Release { + version: V2, + yanked: Some(false), + release_time: Some(expected), + }], + }]; + + assert_eq!( + calculate_diff(db_releases.iter(), index_releases.iter()), + vec![Difference::ReleaseTime(KRATE, V2, expected)] + ); + } + + #[test] + fn test_missing_index_release_time_does_not_clear_database_value() { + let db_releases = [Crate { + name: KRATE, + releases: vec![Release { + version: V2, + yanked: Some(false), + release_time: Some("2024-01-01T00:00:00Z".parse::>().unwrap()), }], }]; let index_releases = [Crate { @@ -197,6 +281,7 @@ mod tests { releases: vec![Release { version: V2, yanked: Some(false), + release_time: None, }], }]; diff --git a/crates/bin/docs_rs_watcher/src/consistency/index.rs b/crates/bin/docs_rs_watcher/src/consistency/index.rs index ce48a5cdbb..dc06c357e9 100644 --- a/crates/bin/docs_rs_watcher/src/consistency/index.rs +++ b/crates/bin/docs_rs_watcher/src/consistency/index.rs @@ -1,9 +1,10 @@ use super::data::{Crate, Crates, Release, Releases}; use crate::Config; use anyhow::Result; +use chrono::{DateTime, Utc}; +use docs_rs_registry_api::RegistryApi; use docs_rs_types::{KrateName, Version}; -use docs_rs_uri::EscapedURI; -use docs_rs_utils::{APP_USER_AGENT, run_blocking}; +use docs_rs_utils::run_blocking; use rayon::iter::ParallelIterator; use tracing::debug; @@ -29,19 +30,7 @@ pub(super) async fn load(config: &Config) -> Result { .crates_parallel() .map(|krate| { krate.map(|krate| { - let mut releases: Releases = - krate - .versions() - .iter() - .filter_map(|version| { - version.version().parse::().ok().map(|semversion| { - Release { - version: semversion, - yanked: Some(version.is_yanked()), - } - }) - }) - .collect(); + let mut releases = releases_from_index(&krate); releases.sort_by(|lhs, rhs| lhs.version.cmp(&rhs.version)); @@ -63,21 +52,23 @@ pub(super) async fn load(config: &Config) -> Result { .await } -pub(super) async fn load_single(name: &KrateName) -> Result> { - let url = sparse_index_url(name); - let response = reqwest::Client::builder() - .user_agent(APP_USER_AGENT) - .build()? - .get(url.to_string()) - .send() - .await?; - - if response.status() == reqwest::StatusCode::NOT_FOUND { +pub(super) async fn load_single( + registry_api: &RegistryApi, + name: &KrateName, +) -> Result> { + let Some(krate) = registry_api.get_crate_from_index(name).await? else { return Ok(None); - } + }; + + let releases = releases_from_index(&krate); + + Ok(Some(Crate { + name: name.clone(), + releases, + })) +} - let bytes = response.error_for_status()?.bytes().await?; - let krate = crates_index::Crate::from_slice(&bytes)?; +fn releases_from_index(krate: &crates_index::Crate) -> Releases { let mut releases: Releases = krate .versions() .iter() @@ -89,40 +80,31 @@ pub(super) async fn load_single(name: &KrateName) -> Result> { .map(|version| Release { version, yanked: Some(index_version.is_yanked()), + release_time: index_version + .pubtime() + .and_then(|time| time.parse::>().ok()), }) }) .collect(); releases.sort_by(|lhs, rhs| lhs.version.cmp(&rhs.version)); - - Ok(Some(Crate { - name: name.clone(), - releases, - })) -} - -fn sparse_index_url(name: &KrateName) -> EscapedURI { - let name = name.as_str().to_ascii_lowercase(); - let path = match name.len() { - 1 => format!("1/{name}"), - 2 => format!("2/{name}"), - 3 => format!("3/{}/{name}", &name[..1]), - _ => format!("{}/{}/{name}", &name[..2], &name[2..4]), - }; - format!("https://index.crates.io/{path}") - .parse() - .expect("the sparse index URL is valid") + releases } #[cfg(test)] mod tests { use super::*; - use test_case::test_case; + use chrono::{DateTime, Utc}; + + #[test] + fn releases_include_index_pubtime() { + let krate = crates_index::Crate::from_slice( + br#"{"name":"krate","vers":"1.0.0","deps":[],"cksum":"0000000000000000000000000000000000000000000000000000000000000000","features":{},"yanked":false,"pubtime":"2024-01-02T03:04:05Z"}"#, + ) + .unwrap(); - #[test_case("a", "https://index.crates.io/1/a")] - #[test_case("ab", "https://index.crates.io/2/ab")] - #[test_case("abc", "https://index.crates.io/3/a/abc")] - #[test_case("Serde", "https://index.crates.io/se/rd/serde")] - fn sparse_index_urls(name: &str, expected: &str) { - assert_eq!(sparse_index_url(&name.parse().unwrap()), expected); + assert_eq!( + releases_from_index(&krate)[0].release_time, + Some("2024-01-02T03:04:05Z".parse::>().unwrap()) + ); } } diff --git a/crates/bin/docs_rs_watcher/src/consistency/mod.rs b/crates/bin/docs_rs_watcher/src/consistency/mod.rs index b8357591b5..df6b4dd210 100644 --- a/crates/bin/docs_rs_watcher/src/consistency/mod.rs +++ b/crates/bin/docs_rs_watcher/src/consistency/mod.rs @@ -1,8 +1,9 @@ use crate::{Config, db::delete, index_watcher::set_yanked}; use anyhow::{Context as _, Result}; +use chrono::{DateTime, Utc}; use docs_rs_build_queue::PRIORITY_CONSISTENCY_CHECK; use docs_rs_context::Context; -use docs_rs_types::KrateName; +use docs_rs_types::{KrateName, Version}; use itertools::Itertools; use tracing::{info, warn}; @@ -57,8 +58,10 @@ pub async fn run_single_check( .await .context("Loading crate data from database for consistency check")?; + let registry_api = ctx.registry_api()?; + info!(%name, "Loading crate data from sparse index..."); - let index_data = index::load_single(name) + let index_data = index::load_single(registry_api, name) .await .context("Loading crate data from sparse index for consistency check")?; @@ -75,6 +78,7 @@ struct HandleResult { crates_deleted: u32, releases_deleted: u32, yanks_corrected: u32, + release_times_corrected: u32, } fn print_summary(diff: &[diff::Difference], result: &HandleResult, dry_run: bool) { @@ -88,6 +92,7 @@ fn print_summary(diff: &[diff::Difference], result: &HandleResult, dry_run: bool diff::Difference::ReleaseNotInIndex(_, _) => "ReleaseNotInIndex", diff::Difference::ReleaseNotInDb(_, _) => "ReleaseNotInDb", diff::Difference::ReleaseYank(_, _, _) => "ReleaseYank", + diff::Difference::ReleaseTime(_, _, _) => "ReleaseTime", }) { println!("{key:17} => {count:4}"); } @@ -102,6 +107,10 @@ fn print_summary(diff: &[diff::Difference], result: &HandleResult, dry_run: bool println!("crates deleted: {:4}", result.crates_deleted); println!("releases deleted: {:4}", result.releases_deleted); println!("yanks corrected: {:4}", result.yanks_corrected); + println!( + "release times corrected: {:4}", + result.release_times_corrected + ); } async fn handle_diff<'a, I>( @@ -170,17 +179,49 @@ where } result.yanks_corrected += 1; } + diff::Difference::ReleaseTime(name, version, release_time) => { + if !dry_run + && let Err(err) = + set_release_time(&mut conn, name, version, *release_time).await + { + warn!(?difference, ?err, "error handling ReleaseTime"); + } + result.release_times_corrected += 1; + } } } Ok(result) } +async fn set_release_time( + conn: &mut sqlx::PgConnection, + name: &KrateName, + version: &Version, + release_time: DateTime, +) -> Result<()> { + sqlx::query!( + r#"UPDATE releases + SET release_time = $3 + FROM crates + WHERE crates.id = releases.crate_id + AND crates.name = $1 + AND releases.version = $2"#, + name as _, + version as _, + release_time, + ) + .execute(conn) + .await?; + Ok(()) +} + #[cfg(test)] mod tests { use super::diff::Difference; use super::*; use crate::testing::TestEnvironment; + use chrono::DateTime; use docs_rs_types::{ Version, testing::{KRATE, V1, V2}, @@ -307,6 +348,36 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "multi_thread")] + async fn test_wrong_release_time() -> Result<()> { + let env = TestEnvironment::new().await?; + let original = "2024-01-01T00:00:00Z".parse::>()?; + let expected = "2024-01-02T00:00:00Z".parse::>()?; + env.fake_release() + .await + .name("krate") + .version(V1) + .release_time(original) + .create() + .await?; + + let diff = [Difference::ReleaseTime(KRATE, V1, expected)]; + + handle_diff(env.config(), &env, diff.iter(), true).await?; + assert_eq!( + single_row::>(&env, "SELECT release_time FROM releases").await?, + vec![original] + ); + + handle_diff(env.config(), &env, diff.iter(), false).await?; + assert_eq!( + single_row::>(&env, "SELECT release_time FROM releases").await?, + vec![expected] + ); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread")] async fn test_missing_release_in_db() -> Result<()> { let env = TestEnvironment::new().await?; diff --git a/crates/bin/docs_rs_web/Cargo.toml b/crates/bin/docs_rs_web/Cargo.toml index 6b9be90e9a..f64568ddcf 100644 --- a/crates/bin/docs_rs_web/Cargo.toml +++ b/crates/bin/docs_rs_web/Cargo.toml @@ -59,6 +59,7 @@ serde = { workspace = true } serde_json = { workspace = true } slug = { workspace = true } sqlx = { workspace = true } +strum = { workspace = true } syntect = { version = "5.0.0", default-features = false, features = ["dump-load", "html", "parsing", "regex-onig"] } thiserror = { workspace = true } tokio = { workspace = true } @@ -82,13 +83,13 @@ docs_rs_config = { path = "../../lib/docs_rs_config", features = ["testing"] } docs_rs_context = { path = "../../lib/docs_rs_context", features = ["testing"] } docs_rs_database = { path = "../../lib/docs_rs_database", features = ["testing"] } docs_rs_headers = { path = "../../lib/docs_rs_headers", features = ["testing"] } +docs_rs_registry_api = { path = "../../lib/docs_rs_registry_api", features = ["testing"] } docs_rs_storage = { path = "../../lib/docs_rs_storage", features = ["testing"] } docs_rs_test_fakes = { path = "../../lib/docs_rs_test_fakes" } docs_rs_types = { path = "../../lib/docs_rs_types", features = ["testing"] } http-body-util = "0.1.0" indoc = "2.0.0" kuchikiki = "0.8" -mockito = { workspace = true } opentelemetry_sdk = { workspace = true } pretty_assertions = { workspace = true } reqwest = { workspace = true } diff --git a/crates/bin/docs_rs_web/src/context.rs b/crates/bin/docs_rs_web/src/context.rs index d4d739b73e..172d9c979f 100644 --- a/crates/bin/docs_rs_web/src/context.rs +++ b/crates/bin/docs_rs_web/src/context.rs @@ -13,7 +13,8 @@ pub async fn build_context() -> Result> { .with_build_queue()? .with_storage() .await? - .with_registry_api()? + .with_registry_api() + .await? .with_build_limits()? .build()?, )) diff --git a/crates/bin/docs_rs_web/src/handlers/releases.rs b/crates/bin/docs_rs_web/src/handlers/releases.rs index 33a7749ec4..11356006d8 100644 --- a/crates/bin/docs_rs_web/src/handlers/releases.rs +++ b/crates/bin/docs_rs_web/src/handlers/releases.rs @@ -32,8 +32,8 @@ use std::{ str, sync::Arc, }; +use strum::IntoEnumIterator; use tracing::{error, trace, warn}; -use url::form_urlencoded; /// Number of release in home page const RELEASES_IN_HOME: i64 = 15; @@ -144,8 +144,8 @@ pub(crate) enum ReleaseStatus { struct SearchResult { pub results: Vec, - pub prev_page: Option, - pub next_page: Option, + pub prev_page: Option, + pub next_page: Option, } /// Get the search results for a crate search query @@ -154,10 +154,9 @@ struct SearchResult { async fn get_search_results( conn: &mut sqlx::PgConnection, registry: &RegistryApi, - query_params: &str, - query: &str, + cursor: registry_api::SearchCursor, ) -> Result { - let registry_api::Search { crates, meta } = registry.search(query_params).await?; + let registry_api::Search { crates, meta } = registry.search(&cursor).await?; let names = Arc::new( crates @@ -229,7 +228,8 @@ async fn get_search_results( // extend with the release/build information from docs.rs // Crates that are not on docs.rs yet will not be returned. let mut results = Vec::new(); - if let Ok(krate) = query.parse::() + if let Some(query) = cursor.query() + && let Ok(krate) = query.parse::() && let Some(desc) = super::rustdoc::DOC_RUST_LANG_ORG_REDIRECTS.get(&krate) { results.push(ReleaseStatus::External(desc)); @@ -247,8 +247,8 @@ async fn get_search_results( Ok(SearchResult { results, - prev_page: meta.prev_page, - next_page: meta.next_page, + prev_page: meta.prev_page().cloned(), + next_page: meta.next_page().cloned(), }) } @@ -435,7 +435,7 @@ pub(crate) struct Search { pub(crate) message: Option, pub(crate) releases: Vec, pub(crate) search_query: Option, - pub(crate) search_sort_by: Option, + pub(crate) search_sort_by: Option, pub(crate) previous_page_link: Option, pub(crate) next_page_link: Option, /// This should always be `ReleaseType::Search` @@ -529,10 +529,18 @@ pub(crate) async fn search_handler( .get("query") .map(|q| q.to_string()) .unwrap_or_else(|| "".to_string()); + let mut sort_by = query_params .get("sort") - .map(|q| q.to_string()) - .unwrap_or_else(|| "relevance".to_string()); + .and_then(|sort| { + sort.parse() + .inspect_err(|err| { + warn!(%sort, ?err, "invalid search sort from user"); + }) + .ok() + }) + .unwrap_or_default(); + // check if I am feeling lucky button pressed and redirect user to crate page // if there is a match. Also check for paths to items within crates. if query_params.remove("i-am-feeling-lucky").is_some() || query.contains("::") { @@ -584,39 +592,57 @@ pub(crate) async fn search_handler( } let search_result = if let Some(paginate) = query_params.get("paginate") { - let decoded = b64.decode(paginate.as_bytes()).map_err(|e| { - warn!("error when decoding pagination base64 string \"{paginate}\": {e:?}"); + let decoded = b64.decode(paginate.as_bytes()).map_err(|err| { + warn!( + paginate, + ?err, + "error when decoding pagination base64 string" + ); AxumNope::NoResults })?; let query_params = String::from_utf8_lossy(&decoded); - let query_params = query_params.strip_prefix('?').ok_or_else(|| { + let search_cursor: registry_api::SearchCursor = query_params.parse().map_err(|err| { // sometimes we see plain bytes being passed to `paginate`. // In these cases we just return `NoResults` and don't call // the crates.io API. // The whole point of the `paginate` design is that we don't // know anything about the pagination args and crates.io can // change them as they wish, so we cannot do any more checks here. - warn!("didn't get query args in `paginate` arguments for search: \"{query_params}\""); + warn!( + %query_params, + ?err, + "didn't get query args in `paginate` arguments for search" + ); AxumNope::NoResults })?; - for (k, v) in form_urlencoded::parse(query_params.as_bytes()) { - match &*k { - "q" => query = v.to_string(), - "sort" => sort_by = v.to_string(), - _ => {} - } + if let Some(new_query) = search_cursor.query() { + query = new_query.into(); } - get_search_results(&mut conn, ®istry, query_params, "").await + if let Some(new_sort_by) = search_cursor + .sort_by() + .inspect_err(|err| { + error!( + cursor = %search_cursor.as_params(), + ?err, + "unknown search-ordering from crates.io" + ) + }) + .ok() + .flatten() + { + sort_by = new_sort_by; + } + + get_search_results(&mut conn, ®istry, search_cursor).await } else if !query.is_empty() { - let query_params: String = form_urlencoded::Serializer::new(String::new()) - .append_pair("q", &query) - .append_pair("sort", &sort_by) - .append_pair("per_page", &RELEASES_IN_RELEASES.to_string()) - .finish(); + let search_query = registry_api::SearchQuery::builder(&query) + .sort_by(sort_by) + .per_page(RELEASES_IN_RELEASES as u32) + .build(); - get_search_results(&mut conn, ®istry, &query_params, &query).await + get_search_results(&mut conn, ®istry, search_query.into()).await } else { return Err(AxumNope::NoResults); }; @@ -634,12 +660,18 @@ pub(crate) async fn search_handler( releases: search_result.results, search_query: Some(query), search_sort_by: Some(sort_by), - next_page_link: search_result - .next_page - .map(|params| format!("/releases/search?paginate={}", b64.encode(params))), - previous_page_link: search_result - .prev_page - .map(|params| format!("/releases/search?paginate={}", b64.encode(params))), + next_page_link: search_result.next_page.map(|params| { + format!( + "/releases/search?paginate={}", + b64.encode(params.as_params()) + ) + }), + previous_page_link: search_result.prev_page.map(|params| { + format!( + "/releases/search?paginate={}", + b64.encode(params.as_params()) + ) + }), ..Default::default() } .into_response()) @@ -837,16 +869,14 @@ mod tests { use docs_rs_database::releases::{ finish_build, initialize_build, initialize_crate, initialize_release, }; - use docs_rs_registry_api::{CrateOwner, OwnerKind}; + use docs_rs_registry_api::{CrateOwner, OwnerKind, SearchQuery, testing::TestRegistry}; use docs_rs_test_fakes::{FakeBuild, fake_release_that_failed_before_build}; use docs_rs_types::{ BuildStatus, SimpleBuildError, testing::{BAR, BAZ, FOO, V0_1, V1, V2, V3}, }; use kuchikiki::traits::TendrilSink; - use mockito::Matcher; use reqwest::StatusCode; - use serde_json::json; use std::collections::HashSet; use std::str::FromStr; use test_case::test_case; @@ -1088,16 +1118,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn search_result_can_retrieve_sort_by_from_pagination() -> Result<()> { - let mut crates_io = mockito::Server::new_async().await; - - let env = TestEnvironment::builder() - .registry_api_config( - docs_rs_registry_api::Config::builder() - .registry_api_host(crates_io.url().parse().unwrap()) - .build(), - ) - .build() - .await?; + let env = TestEnvironment::new().await?; let web = env.web_app().await; env.fake_release() @@ -1106,35 +1127,28 @@ mod tests { .create() .await?; - let _m = crates_io - .mock("GET", "/api/v1/crates") - .match_query(Matcher::AllOf(vec![ - Matcher::UrlEncoded("q".into(), "some_random_crate".into()), - Matcher::UrlEncoded("per_page".into(), "30".into()), - Matcher::UrlEncoded("page".into(), "2".into()), - Matcher::UrlEncoded("sort".into(), "recent-updates".into()), - ])) - .with_status(200) - .with_header("content-type", "application/json") - .with_body( - json!({ - "crates": [ - { "name": "some_random_crate" }, - ], - "meta": { - "next_page": "?q=some_random_crate&sort=recent-updates&per_page=30&page=2", - "prev_page": "?q=some_random_crate&sort=recent-updates&per_page=30&page=1", - } - }) - .to_string(), - ) - .create_async() + let cursor = registry_api::SearchCursor::builder() + .query("some_random_crate") + .per_page(30) + .page(2) + .sort_by(registry_api::SearchSort::RecentUpdates) + .build(); + + let next_page_cursor = cursor.clone().adapt().page(2).build(); + let prev_page_cursor = cursor.clone().adapt().page(1).build(); + + env.test_registry() + .mock_search(cursor.clone()) + .crate_names(["some_random_crate"]) + .next_page(next_page_cursor.clone()) + .prev_page(prev_page_cursor) + .create() .await; // click the "Next Page" Button, the "Sort by" SelectBox should keep the same option. let next_page_url = format!( "/releases/search?paginate={}", - b64.encode("?q=some_random_crate&sort=recent-updates&per_page=30&page=2"), + b64.encode(next_page_cursor.as_params()), ); let response = web.get(&next_page_url).await?; assert!(response.status().is_success()); @@ -1155,16 +1169,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn search_result_passes_cratesio_pagination_links() -> Result<()> { - let mut crates_io = mockito::Server::new_async().await; - - let env = TestEnvironment::builder() - .registry_api_config( - docs_rs_registry_api::Config::builder() - .registry_api_host(crates_io.url().parse().unwrap()) - .build(), - ) - .build() - .await?; + let env = TestEnvironment::new().await?; let web = env.web_app().await; env.fake_release() @@ -1173,27 +1178,28 @@ mod tests { .create() .await?; - let _m = crates_io - .mock("GET", "/api/v1/crates") - .match_query(Matcher::AllOf(vec![ - Matcher::UrlEncoded("q".into(), "some_random_crate".into()), - Matcher::UrlEncoded("per_page".into(), "30".into()), - ])) - .with_status(200) - .with_header("content-type", "application/json") - .with_body( - json!({ - "crates": [ - { "name": "some_random_crate" }, - ], - "meta": { - "next_page": "?some=parameters&that=cratesio&might=return", - "prev_page": "?and=the¶meters=for&the=previouspage", - } - }) - .to_string(), - ) - .create_async() + let search_cursor = registry_api::SearchCursor::builder() + .query("some_random_crate") + .per_page(30) + .build(); + let next_page = registry_api::SearchCursor::builder() + .custom_arg("some", "parameters") + .custom_arg("that", "cratesio") + .custom_arg("might", "return") + .build(); + + let prev_page = registry_api::SearchCursor::builder() + .custom_arg("and", "the") + .custom_arg("parameters", "for") + .custom_arg("the", "previouspage") + .build(); + + env.test_registry() + .mock_search(search_cursor) + .crate_names(["some_random_crate"]) + .next_page(next_page.clone()) + .prev_page(prev_page.clone()) + .create() .await; let response = web.get("/releases/search?query=some_random_crate").await?; @@ -1216,14 +1222,14 @@ mod tests { other_search_links[0], format!( "/releases/search?paginate={}", - b64.encode("?and=the¶meters=for&the=previouspage"), + b64.encode(prev_page.as_params()), ) ); assert_eq!( other_search_links[1], format!( "/releases/search?paginate={}", - b64.encode("?some=parameters&that=cratesio&might=return") + b64.encode(next_page.as_params()), ) ); @@ -1253,33 +1259,36 @@ mod tests { async fn crates_io_errors_are_correctly_returned_and_we_dont_try_parsing( status: StatusCode, ) -> Result<()> { - let mut crates_io = mockito::Server::new_async().await; + let env = TestEnvironment::new().await?; - let env = TestEnvironment::builder() - .registry_api_config( - docs_rs_registry_api::Config::builder() - .registry_api_host(crates_io.url().parse().unwrap()) - .crates_io_api_call_retries(0) - .build(), - ) - .build() - .await?; + let query = registry_api::SearchQuery::builder("doesnt_matter_here") + .per_page(30) + .build(); - let _m = crates_io - .mock("GET", "/api/v1/crates") - .match_query(Matcher::AllOf(vec![ - Matcher::UrlEncoded("q".into(), "doesnt_matter_here".into()), - Matcher::UrlEncoded("per_page".into(), "30".into()), - ])) - .with_status(status.as_u16() as usize) - .create_async() - .await; + if status.is_client_error() { + env.test_registry() + .mock_search_error(query) + .client_error(status) + .api_error_messages(["error 1", "error 2"]) + .create() + .await; + } else if status.is_server_error() { + env.test_registry() + .mock_search_error(query) + .server_error(status) + .error_text("some server error") + .create() + .await; + } else { + panic!("not an error"); + } let response = env .web_app() .await .get("/releases/search?query=doesnt_matter_here") .await?; + assert_eq!(response.status(), status); assert!(response.text().await?.contains(&format!("{status}"))); @@ -1288,16 +1297,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn search_encoded_pagination_passed_to_cratesio() -> Result<()> { - let mut crates_io = mockito::Server::new_async().await; - - let env = TestEnvironment::builder() - .registry_api_config( - docs_rs_registry_api::Config::builder() - .registry_api_host(crates_io.url().parse().unwrap()) - .build(), - ) - .build() - .await?; + let env = TestEnvironment::new().await?; let web = env.web_app().await; env.fake_release() @@ -1306,33 +1306,21 @@ mod tests { .create() .await?; - let _m = crates_io - .mock("GET", "/api/v1/crates") - .match_query(Matcher::AllOf(vec![ - Matcher::UrlEncoded("some".into(), "dummy".into()), - Matcher::UrlEncoded("pagination".into(), "parameters".into()), - ])) - .with_status(200) - .with_header("content-type", "application/json") - .with_body( - json!({ - "crates": [ - { "name": "some_random_crate" }, - ], - "meta": { - "next_page": null, - "prev_page": null, - } - }) - .to_string(), - ) - .create_async() + let search_cursor = registry_api::SearchCursor::builder() + .custom_arg("some", "dummy") + .custom_arg("pagination", "parameters") + .build(); + + env.test_registry() + .mock_search(search_cursor.clone()) + .crate_names(["some_random_crate"]) + .create() .await; let links = get_release_links( &format!( "/releases/search?paginate={}", - b64.encode("?some=dummy&pagination=parameters") + b64.encode(search_cursor.as_params()) ), &web, ) @@ -1345,16 +1333,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn search_lucky_with_unknown_crate() -> Result<()> { - let mut crates_io = mockito::Server::new_async().await; - - let env = TestEnvironment::builder() - .registry_api_config( - docs_rs_registry_api::Config::builder() - .registry_api_host(crates_io.url().parse().unwrap()) - .build(), - ) - .build() - .await?; + let env = TestEnvironment::new().await?; let web = env.web_app().await; env.fake_release() @@ -1363,28 +1342,15 @@ mod tests { .create() .await?; - let _m = crates_io - .mock("GET", "/api/v1/crates") - .match_query(Matcher::AllOf(vec![ - Matcher::UrlEncoded("q".into(), "some_random_".into()), - Matcher::UrlEncoded("per_page".into(), "30".into()), - ])) - .with_status(200) - .with_header("content-type", "application/json") - .with_body( - json!({ - "crates": [ - { "name": "some_random_crate" }, - { "name": "some_other_crate" }, - ], - "meta": { - "next_page": null, - "prev_page": null, - } - }) - .to_string(), + env.test_registry() + .mock_search( + registry_api::SearchCursor::builder() + .query("some_random_") + .per_page(30) + .build(), ) - .create_async() + .crate_names(["some_random_crate", "some_other_crate"]) + .create() .await; // when clicking "I'm feeling lucky" and the query doesn't match any crate, @@ -1402,16 +1368,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn search() -> Result<()> { - let mut crates_io = mockito::Server::new_async().await; - - let env = TestEnvironment::builder() - .registry_api_config( - docs_rs_registry_api::Config::builder() - .registry_api_host(crates_io.url().parse().unwrap()) - .build(), - ) - .build() - .await?; + let env = TestEnvironment::new().await?; let web = env.web_app().await; env.fake_release() @@ -1466,32 +1423,17 @@ mod tests { ) .await?; - let _m = crates_io - .mock("GET", "/api/v1/crates") - .match_query(Matcher::AllOf(vec![ - Matcher::UrlEncoded("q".into(), "some_random_crate".into()), - Matcher::UrlEncoded("per_page".into(), "30".into()), - ])) - .with_status(200) - .with_header("content-type", "application/json") - .with_body( - json!({ - "crates": [ - { "name": "some_random_crate" }, - { "name": "some_other_crate" }, - { "name": "and_another_one" }, - { "name": "yet_another_crate" }, - { "name": "in_progress" }, - { "name": "failed_hard" } - ], - "meta": { - "next_page": null, - "prev_page": null, - } - }) - .to_string(), - ) - .create_async() + env.test_registry() + .mock_search(registry_api::SearchQuery::from("some_random_crate")) + .crate_names([ + "some_random_crate", + "some_other_crate", + "and_another_one", + "yet_another_crate", + "in_progress", + "failed_hard", + ]) + .create() .await; let links = get_release_links("/releases/search?query=some_random_crate", &web).await?; @@ -2188,16 +2130,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn crates_not_on_docsrs() -> Result<()> { - let mut crates_io = mockito::Server::new_async().await; - - let env = TestEnvironment::builder() - .registry_api_config( - docs_rs_registry_api::Config::builder() - .registry_api_host(crates_io.url().parse().unwrap()) - .build(), - ) - .build() - .await?; + let env = TestEnvironment::new().await?; let web = env.web_app().await; env.fake_release() @@ -2206,29 +2139,14 @@ mod tests { .create() .await?; - let _m = crates_io - .mock("GET", "/api/v1/crates") - .match_query(Matcher::AllOf(vec![ - Matcher::UrlEncoded("q".into(), "some_random_crate".into()), - Matcher::UrlEncoded("per_page".into(), "30".into()), - ])) - .with_status(200) - .with_header("content-type", "application/json") - .with_body( - json!({ - "crates": [ - { "name": "some_random_crate" }, - { "name": "some_random_crate2" }, - { "name": "some_random_crate3" }, - ], - "meta": { - "next_page": "null", - "prev_page": "null", - } - }) - .to_string(), - ) - .create_async() + env.test_registry() + .mock_search(registry_api::SearchQuery::from("some_random_crate")) + .crate_names([ + "some_random_crate", + "some_random_crate2", + "some_random_crate3", + ]) + .create() .await; let response = web.get("/releases/search?query=some_random_crate").await?; @@ -2314,10 +2232,21 @@ mod tests { fn test_search_std() { async_wrapper(|env| async move { let web = env.web_app().await; + let registry = env.test_registry(); + + async fn inner( + registry: &TestRegistry, + web: &axum::Router, + krate: &str, + ) -> Result<(), anyhow::Error> { + registry + .mock_search(SearchQuery::from(krate)) + .crate_names(["other"]) + .create() + .await; - async fn inner(web: &axum::Router, krate: &str) -> Result<(), anyhow::Error> { let full = kuchikiki::parse_html().one( - web.get(&format!("/releases/search?query={krate}")) + web.assert_success(&format!("/releases/search?query={krate}")) .await? .text() .await?, @@ -2354,8 +2283,8 @@ mod tests { Ok(()) } - inner(&web, "std").await?; - inner(&web, "libstd").await?; + inner(registry, &web, "std").await?; + inner(registry, &web, "libstd").await?; Ok(()) }); diff --git a/crates/bin/docs_rs_web/templates/releases/search_results.html b/crates/bin/docs_rs_web/templates/releases/search_results.html index ed27dd607d..1f47e5ca74 100644 --- a/crates/bin/docs_rs_web/templates/releases/search_results.html +++ b/crates/bin/docs_rs_web/templates/releases/search_results.html @@ -20,13 +20,11 @@ - {% set search_sort_by_val = search_sort_by.as_deref().unwrap_or_default() %} + {% set search_sort_by_val = search_sort_by.unwrap_or_default().to_string() %} diff --git a/crates/lib/docs_rs_context/Cargo.toml b/crates/lib/docs_rs_context/Cargo.toml index 5702d7dee3..cdb9298b35 100644 --- a/crates/lib/docs_rs_context/Cargo.toml +++ b/crates/lib/docs_rs_context/Cargo.toml @@ -11,6 +11,7 @@ testing = [ "docs_rs_database/testing", "docs_rs_fastly/testing", "docs_rs_logging/testing", + "docs_rs_registry_api/testing", "docs_rs_storage/testing", ] diff --git a/crates/lib/docs_rs_context/src/context.rs b/crates/lib/docs_rs_context/src/context.rs index e2d7d6c51d..2a3584c44f 100644 --- a/crates/lib/docs_rs_context/src/context.rs +++ b/crates/lib/docs_rs_context/src/context.rs @@ -240,12 +240,12 @@ impl ContextBuilder { self.registry_api_internal(registry_api) } - pub fn with_registry_api(self) -> Result>> + pub async fn with_registry_api(self) -> Result>> where S::RegistryApi: IsUnset, { let config = docs_rs_registry_api::Config::from_environment()?; - let api = RegistryApi::from_config(&config)?; + let api = RegistryApi::from_config(&config).await?; Ok(self.registry_api(config.into(), api.into())) } diff --git a/crates/lib/docs_rs_context/src/testing/test_env/blocking.rs b/crates/lib/docs_rs_context/src/testing/test_env/blocking.rs index ac1a80672b..42085f9106 100644 --- a/crates/lib/docs_rs_context/src/testing/test_env/blocking.rs +++ b/crates/lib/docs_rs_context/src/testing/test_env/blocking.rs @@ -30,11 +30,7 @@ impl BlockingTestEnvironment { } #[builder(finish_fn = build)] - pub fn builder( - config: Option, - registry_api_config: Option, - storage_config: Option, - ) -> Result { + pub fn builder(config: Option, storage_config: Option) -> Result { let runtime = tokio::runtime::Builder::new_multi_thread() .enable_all() .build() @@ -44,7 +40,6 @@ impl BlockingTestEnvironment { inner: runtime.block_on( TestEnvironment::builder() .maybe_config(config) - .maybe_registry_api_config(registry_api_config) .maybe_storage_config(storage_config) .build(), )?, diff --git a/crates/lib/docs_rs_context/src/testing/test_env/non_blocking.rs b/crates/lib/docs_rs_context/src/testing/test_env/non_blocking.rs index 98583d1f5b..8c0119a7fd 100644 --- a/crates/lib/docs_rs_context/src/testing/test_env/non_blocking.rs +++ b/crates/lib/docs_rs_context/src/testing/test_env/non_blocking.rs @@ -6,7 +6,7 @@ use docs_rs_config::AppConfig; use docs_rs_database::{AsyncPoolClient, Config as DatabaseConfig, testing::TestDatabase}; use docs_rs_fastly::Cdn; use docs_rs_opentelemetry::testing::{CollectedMetrics, TestMetrics}; -use docs_rs_registry_api::RegistryApi; +use docs_rs_registry_api::testing::TestRegistry; use docs_rs_storage::{Config as StorageConfig, testing::TestStorage}; use docs_rs_test_fakes::FakeRelease; use std::{ops::Deref, sync::Arc}; @@ -20,6 +20,7 @@ pub struct TestEnvironment { storage: TestStorage, #[allow(dead_code)] // we need to keep the storage so it can be cleaned up. db: TestDatabase, + registry: TestRegistry, } impl Deref for TestEnvironment { @@ -42,7 +43,6 @@ impl TestEnvironment { #[builder(finish_fn = build)] pub async fn builder( config: Option, - registry_api_config: Option, storage_config: Option, build_queue_config: Option, ) -> Result { @@ -54,14 +54,7 @@ impl TestEnvironment { C::test_config()? }); - let registry_api_config = - Arc::new(if let Some(registry_api_config) = registry_api_config { - registry_api_config - } else { - docs_rs_registry_api::Config::from_environment()? - }); - - let registry_api = RegistryApi::from_config(®istry_api_config)?; + let test_registry = TestRegistry::new().await?; let metrics = TestMetrics::new(); @@ -98,7 +91,10 @@ impl TestEnvironment { .pool(db_config.into(), db.pool().clone()) .storage(storage_config.clone(), test_storage.storage()) .build_queue(build_queue_config, build_queue) - .registry_api(registry_api_config, registry_api.into()) + .registry_api( + test_registry.test_config().clone(), + test_registry.api().clone(), + ) .with_repository_stats()? .maybe_cdn( Arc::new(docs_rs_fastly::Config::test_config()?), @@ -109,6 +105,7 @@ impl TestEnvironment { .into(), db, storage: test_storage, + registry: test_registry, metrics, }) } @@ -127,6 +124,10 @@ impl TestEnvironment { .expect("we always have a CDN in test environments") } + pub fn test_registry(&self) -> &TestRegistry { + &self.registry + } + pub async fn async_conn(&self) -> Result { self.context.pool()?.get_async().await.map_err(Into::into) } diff --git a/crates/lib/docs_rs_database/src/releases.rs b/crates/lib/docs_rs_database/src/releases.rs index a40afc8648..8fca44ca3e 100644 --- a/crates/lib/docs_rs_database/src/releases.rs +++ b/crates/lib/docs_rs_database/src/releases.rs @@ -50,6 +50,10 @@ pub async fn finish_release( let features = get_features(metadata_pkg); let is_library = metadata_pkg.is_library(); + // NOTE: we're still inserting dummy data in case of empty release-data fields + // or missing release data. + let registry_data = registry_data.clone().for_database(); + let result = sqlx::query!( r#"UPDATE releases SET release_time = $2, @@ -656,6 +660,7 @@ mod test { name: &KrateName, version: &Version, keywords: KL, + registry_data: &ReleaseData, ) -> Result where K: Into, @@ -689,7 +694,7 @@ mod test { tempdir.path(), DEFAULT_TARGET, vec![DEFAULT_TARGET.to_string()], - &ReleaseData::default(), + registry_data, true, false, iter::empty(), @@ -701,6 +706,59 @@ mod test { Ok(release_id) } + #[tokio::test(flavor = "multi_thread")] + async fn test_finish_release_uses_dummy_release_time() -> Result<()> { + let test_metrics = TestMetrics::new(); + let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; + let mut conn = db.async_conn().await?; + + let release_id = fake_release_with_keywords( + &mut conn, + &KRATE, + &V0_1, + iter::empty::(), + &ReleaseData::dummy(), + ) + .await?; + let release_time = sqlx::query_scalar!( + "SELECT release_time FROM releases WHERE id = $1", + release_id.0, + ) + .fetch_one(&mut *conn) + .await?; + + assert!(release_time.is_some()); + Ok(()) + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_finish_release_uses_dummy_release_time_when_missing() -> Result<()> { + let test_metrics = TestMetrics::new(); + let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; + let mut conn = db.async_conn().await?; + + let release_id = fake_release_with_keywords( + &mut conn, + &KRATE, + &V0_1, + iter::empty::(), + &ReleaseData { + release_time: None, + yanked: false, + }, + ) + .await?; + let release_time = sqlx::query_scalar!( + "SELECT release_time FROM releases WHERE id = $1", + release_id.0, + ) + .fetch_one(&mut *conn) + .await?; + + assert!(release_time.is_some()); + Ok(()) + } + #[tokio::test(flavor = "multi_thread")] async fn test_set_build_to_error() -> Result<()> { let test_metrics = TestMetrics::new(); @@ -901,8 +959,14 @@ mod test { let mut conn = db.async_conn().await?; - let release_id = - fake_release_with_keywords(&mut conn, &KRATE, &V0_1, ["kw 1", "kw 2"]).await?; + let release_id = fake_release_with_keywords( + &mut conn, + &KRATE, + &V0_1, + ["kw 1", "kw 2"], + &ReleaseData::dummy(), + ) + .await?; let kw_r = sqlx::query!( r#"SELECT @@ -941,10 +1005,24 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - fake_release_with_keywords(&mut conn, &KRATE, &V0_1, ["kw 3", "kw 4"]).await?; + fake_release_with_keywords( + &mut conn, + &KRATE, + &V0_1, + ["kw 3", "kw 4"], + &ReleaseData::dummy(), + ) + .await?; // same version so we have the same release - fake_release_with_keywords(&mut conn, &KRATE, &V0_1, ["kw 3", "kw 4"]).await?; + fake_release_with_keywords( + &mut conn, + &KRATE, + &V0_1, + ["kw 3", "kw 4"], + &ReleaseData::dummy(), + ) + .await?; Ok(()) } @@ -955,10 +1033,23 @@ mod test { let db = TestDatabase::new(&Config::test_config()?, test_metrics.provider()).await?; let mut conn = db.async_conn().await?; - fake_release_with_keywords(&mut conn, &KRATE, &V1, ["kw 3", "kw 4"]).await?; + fake_release_with_keywords( + &mut conn, + &KRATE, + &V1, + ["kw 3", "kw 4"], + &ReleaseData::dummy(), + ) + .await?; - let release_id = - fake_release_with_keywords(&mut conn, &KRATE, &V1, ["kw 1", "kw 2"]).await?; + let release_id = fake_release_with_keywords( + &mut conn, + &KRATE, + &V1, + ["kw 1", "kw 2"], + &ReleaseData::dummy(), + ) + .await?; let mut conn = db.async_conn().await?; let kw_r = sqlx::query!( @@ -1462,7 +1553,7 @@ mod test { tempdir.path(), DEFAULT_TARGET, vec![DEFAULT_TARGET.to_string()], - &ReleaseData::default(), + &ReleaseData::dummy(), true, false, iter::empty(), diff --git a/crates/lib/docs_rs_registry_api/Cargo.toml b/crates/lib/docs_rs_registry_api/Cargo.toml index f1267ff6fb..9f1a9cab15 100644 --- a/crates/lib/docs_rs_registry_api/Cargo.toml +++ b/crates/lib/docs_rs_registry_api/Cargo.toml @@ -5,28 +5,41 @@ license = "MIT" repository = "https://github.com/rust-lang/docs.rs" edition = "2024" +[features] +testing = ["dep:mockito", "dep:tempfile", "dep:tokio"] + [dependencies] anyhow = { workspace = true } bon = { workspace = true } chrono = { workspace = true } +crates-index = { workspace = true, default-features = false, features = ["sparse"] } docs_rs_config = { path = "../docs_rs_config" } docs_rs_env_vars = { path = "../docs_rs_env_vars" } docs_rs_types = { path = "../docs_rs_types" } docs_rs_utils = { path = "../docs_rs_utils" } +http = { workspace = true } +mime = { workspace = true } +mockito = { workspace = true, optional = true } reqwest = { workspace = true } reqwest-middleware = { workspace = true } reqwest-retry = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +serde_urlencoded = "0.7.1" +serde_with = { workspace = true } sqlx = { workspace = true } +strum = { workspace = true } +tempfile = { workspace = true, optional = true } thiserror = { workspace = true } +tokio = { workspace = true, optional = true } tracing = { workspace = true } url = { workspace = true } [dev-dependencies] +docs_rs_config = { path = "../docs_rs_config", features = ["testing"] } docs_rs_types = { path = "../docs_rs_types", features = ["testing"] } -mime = { workspace = true } mockito = { workspace = true } +tempfile = { workspace = true } test-case = { workspace = true } tokio = { workspace = true } diff --git a/crates/lib/docs_rs_registry_api/src/api.rs b/crates/lib/docs_rs_registry_api/src/api.rs index 1760fea58a..4632aa4e4b 100644 --- a/crates/lib/docs_rs_registry_api/src/api.rs +++ b/crates/lib/docs_rs_registry_api/src/api.rs @@ -1,55 +1,212 @@ use crate::{ Config, error::{Error, Result}, - models::{ApiErrors, CrateData, CrateOwner, OwnerKind, ReleaseData, Search, SearchResponse}, + models::{ + ApiErrors, CrateData, CrateOwner, OwnerKind, ReleaseData, Search, SearchCursor, + SearchResponse, + }, }; -use chrono::{DateTime, Utc}; +use anyhow::Context as _; use docs_rs_types::{KrateName, Version}; use docs_rs_utils::APP_USER_AGENT; -use reqwest::{ - StatusCode, - header::{ACCEPT, HeaderValue, USER_AGENT}, -}; +use reqwest::header::ACCEPT; use reqwest_middleware::{ClientBuilder, ClientWithMiddleware}; use reqwest_retry::{RetryTransientMiddleware, policies::ExponentialBackoff}; use serde::{Deserialize, de::DeserializeOwned}; +use std::{fmt, io, path::Path}; use tracing::instrument; use url::Url; +/// Send a `crates_index` request via reqwest. +/// +/// This is needed since the `crates_index` API expects `http::Request` and `http::Response` +/// objects and we're using async `reqwest` to do the actual requests. +/// +/// See +/// https://github.com/frewsxcv/rust-crates-index/blob/master/examples/sparse_http_reqwest.rs +#[instrument(skip_all)] +async fn send_sparse_request( + client: &ClientWithMiddleware, + request: http::Request<()>, +) -> Result>> { + let (mut parts, _) = request.into_parts(); + + // NOTE: oddly, when testing, hyper / reqwest connected via HTTP/1 to + // the sparse index. + // The prepared requests from `crates_index` try to force HTTP/2, and then + // fail. + // For now we just use HTTP/1. + parts.version = http::Version::HTTP_11; + + let request: reqwest::Request = http::Request::from_parts(parts, Vec::new()).try_into()?; + + let response = client.execute(request).await?; + + let mut builder = http::Response::builder() + .status(response.status()) + .version(response.version()); + + builder.headers_mut().unwrap().extend( + response + .headers() + .iter() + .map(|(key, value)| (key.clone(), value.clone())), + ); + + Ok(builder.body(response.bytes().await?.to_vec())?) +} + +async fn fetch_index_config( + index: &crates_index::SparseIndex, + client: &ClientWithMiddleware, +) -> Result { + match index.index_config() { + // Local `config.json` exists: use it without a request. + Ok(config) => Ok(config), + + // It is absent: fetch the live config and save it locally. + Err(crates_index::Error::Io(error)) if error.kind() == io::ErrorKind::NotFound => { + let response = + send_sparse_request(client, index.make_config_request()?.body(())?).await?; + + // `true` writes config.json into Cargo's sparse-index directory. + Ok(index.parse_config_response(response, true)?) + } + + // Do not hide corrupt JSON or permission errors by fetching anew. + Err(error) => Err(error.into()), + } +} + +/// Client for registry data. +/// +/// Release metadata is read from the sparse index, while endpoints not represented in the index, +/// such as owners and search, are requested from the API URL advertised by the index. #[derive(Debug)] pub struct RegistryApi { + index_config: crates_index::IndexConfig, api_base: Url, + pub(crate) sparse_index: crates_index::SparseIndex, client: ClientWithMiddleware, } impl RegistryApi { - pub fn from_config(config: &Config) -> Result { + /// Create a client using the configured sparse-index URL and retry policy. + /// + /// The index's `config.json` determines the API and download URLs. + pub async fn from_config(config: &Config) -> Result { Self::new( - config.registry_api_host.clone(), + config.sparse_index_host.clone(), config.crates_io_api_call_retries, + None, ) + .await } - pub fn new(api_base: Url, max_retries: u32) -> Result { - let headers = [ - (USER_AGENT, HeaderValue::from_static(APP_USER_AGENT)), - (ACCEPT, HeaderValue::from_static("application/json")), - ] - .into_iter() - .collect(); - + /// Create a client from a sparse-index URL. + /// + /// The index's `config.json` determines the API and download URLs. When `cargo_home` is + /// provided, it is used as the sparse-index cache location; otherwise Cargo's normal cache + /// location is used. + pub(crate) async fn new( + sparse_base: Url, + max_retries: u32, + cargo_home: Option<&Path>, + ) -> Result { let client = ClientBuilder::new( reqwest::Client::builder() - .default_headers(headers) - .build() - .map_err(reqwest_middleware::Error::Reqwest)?, + .user_agent(APP_USER_AGENT) + .gzip(true) + .build()?, ) .with(RetryTransientMiddleware::new_with_policy( ExponentialBackoff::builder().build_with_max_retries(max_retries), )) .build(); - Ok(Self { api_base, client }) + let sparse_index = if let Some(cargo_home) = cargo_home { + crates_index::SparseIndex::with_path(cargo_home, sparse_base.as_str())? + } else { + // uses default cargo home on the system. + crates_index::SparseIndex::from_url(sparse_base.as_str())? + }; + let index_config = fetch_index_config(&sparse_index, &client).await?; + + Ok(Self { + api_base: index_config + .api + .as_deref() + // we only care about package indexes with api. + .ok_or(Error::InvalidApiUrl)? + .parse() + .map_err(|_| Error::InvalidApiUrl)?, + index_config, + sparse_index, + client, + }) + } + + /// Return the download URL for a crate version according to the index configuration. + pub fn download_url(&self, name: &KrateName, version: &Version) -> Result { + self.index_config + .download_url(name.as_str(), &version.to_string()) + .ok_or_else(|| { + crates_index::Error::Url(format!( + "can't create download URL for {} {}", + name, version + )) + })? + .parse::() + .map_err(|err| { + crates_index::Error::Url(format!( + "invalid download URL for {} {}: {:?}", + name, version, err + )) + }) + .map_err(Into::into) + } + + /// Fetch all published versions of a crate from the sparse index. + /// + /// Returns `None` when the index has no entry for `name`. + #[instrument(skip(self))] + pub async fn get_crate_from_index( + &self, + name: &KrateName, + ) -> Result> { + let response = send_sparse_request( + &self.client, + self.sparse_index + .make_cache_request(name.as_str())? + .body(())?, + ) + .await?; + + Ok(self + .sparse_index + .parse_cache_response(name.as_str(), response, true)?) + } + + /// Fetch a specific crate version from the sparse index. + /// + /// Returns `None` when either the crate or the requested version is absent. + #[instrument(skip(self))] + pub async fn get_version_from_index( + &self, + name: &KrateName, + version: &Version, + ) -> Result> { + let Some(krate) = self.get_crate_from_index(name).await? else { + return Ok(None); + }; + + let version = version.to_string(); + + Ok(krate + .versions() + .iter() + .find(|v| v.version() == version) + .cloned()) } /// Make a request to crates.io, parse the response as JSON. @@ -65,18 +222,20 @@ impl RegistryApi { /// /// We treat 5xx errors just as text, not knowing where they were raised. /// For 4xx errors we try to parse the the JSON error description. - async fn request(&self, url: &Url) -> Result + async fn api_request(&self, url: impl reqwest::IntoUrl) -> Result where T: DeserializeOwned, { - let response = self.client.get(url.clone()).send().await?; + let response = self + .client + .get(url) + .header(ACCEPT, mime::APPLICATION_JSON.as_ref()) + .send() + .await?; let status = response.status(); if status.is_success() { - Ok(response - .json() - .await - .map_err(reqwest_middleware::Error::Reqwest)?) + Ok(response.json().await?) } else if status.is_server_error() { // this just to let reqwest generate us its "standard" error let err = response.error_for_status_ref().unwrap_err(); @@ -93,6 +252,9 @@ impl RegistryApi { } } + /// Fetch crate-level metadata that is not present in the sparse index. + /// + /// At present, this contains its owners from the registry API. #[instrument(skip(self))] pub async fn get_crate_data(&self, name: &KrateName) -> Result { Ok(CrateData { @@ -100,55 +262,32 @@ impl RegistryApi { }) } + /// Fetch release metadata from the sparse index. + /// + /// Returns `None` when either the crate or the requested version is absent. #[instrument(skip(self))] pub async fn get_release_data( &self, name: &KrateName, version: &Version, ) -> Result> { - let url = { - let mut url = self.api_base.clone(); - url.path_segments_mut() - .map_err(|_| Error::InvalidApiUrl)? - .extend(&["api", "v1", "crates", name.as_str(), "versions"]); - url - }; - - #[derive(Deserialize)] - struct Response { - versions: Vec, - } - - #[derive(Deserialize)] - struct VersionData { - num: Version, - #[serde(default = "Utc::now")] - created_at: DateTime, - #[serde(default)] - yanked: bool, - } - - let response: Response = match self.request(&url).await { - Ok(response) => response, - Err(err) if err.status() == Some(StatusCode::NOT_FOUND) => return Ok(None), - Err(err) => return Err(err), - }; - - let Some(version) = response - .versions - .into_iter() - .find(|data| data.num == *version) - else { + let Some(version) = self.get_version_from_index(name, version).await? else { return Ok(None); }; Ok(Some(ReleaseData { - release_time: version.created_at, - yanked: version.yanked, + release_time: version + .pubtime() + .map(|pt| { + pt.parse() + .context("invalid datetime format in package index") + }) + .transpose()?, + yanked: version.is_yanked(), })) } - /// Fetch owners from the registry's API + /// Fetch owners from the registry API. async fn get_owners(&self, name: &KrateName) -> Result> { let url = { let mut url = self.api_base.clone(); @@ -173,18 +312,12 @@ impl RegistryApi { kind: Option, } - let response: Response = self.request(&url).await?; + let response: Response = self.api_request(url).await?; let result = response .users .into_iter() - .filter(|data| { - !data - .login - .as_ref() - .map(|login| login.is_empty()) - .unwrap_or_default() - }) + .filter(|data| data.login.as_ref().is_some_and(|login| !login.is_empty())) .map(|data| CrateOwner { avatar: data.avatar.unwrap_or_default(), login: data.login.unwrap_or_default(), @@ -195,8 +328,16 @@ impl RegistryApi { Ok(result) } - /// Fetch crates from the registry's API. - pub async fn search(&self, query_params: &str) -> Result { + /// Run a search with a crates.io generated cursor for fetching next/previous pages. + #[instrument(skip(self))] + pub async fn search(&self, cursor: C) -> Result + where + C: Into + fmt::Debug, + { + let cursor = cursor.into(); + + let query_params = cursor.query_for_url(); + let url = { let mut url = self.api_base.clone(); url.path_segments_mut() @@ -206,7 +347,7 @@ impl RegistryApi { url }; - let response: SearchResponse = self.request(&url).await?; + let response: SearchResponse = self.api_request(url).await?; Ok(Search { crates: response.crates.ok_or(Error::MissingReleases)?, @@ -218,109 +359,196 @@ impl RegistryApi { #[cfg(test)] mod tests { use super::*; - use crate::models::{ApiError, SearchCrate, SearchMeta}; - use anyhow::anyhow; + use crate::{ + models::{ApiError, SearchCrate, SearchMeta, SearchQuery, SearchSort}, + testing::TestRegistry, + }; + use chrono::{DateTime, Utc}; + use crates_index::IndexConfig; use docs_rs_types::testing::{KRATE, V1, V2}; use reqwest::{StatusCode, header::CONTENT_TYPE}; use serde::Serialize; use test_case::test_case; - async fn test_search(status: StatusCode, body: impl Serialize) -> Result { - let mut crates_io_api = mockito::Server::new_async().await; + const CHECKSUM: &str = "0000000000000000000000000000000000000000000000000000000000000000"; + + fn sparse_entry(version: &Version, pubtime: Option<&str>, yanked: bool) -> serde_json::Value { + serde_json::json!({ + "name": KRATE.as_str(), + "vers": version.to_string(), + "pubtime": pubtime, + "deps": [], + "cksum": CHECKSUM, + "features": {}, + "yanked": yanked, + }) + } - let _m = crates_io_api - .mock("GET", "/api/v1/crates?q=foo") - .with_status(status.as_u16().into()) - .with_header(CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) - .with_body(serde_json::to_vec(&body).unwrap()) - .create_async() - .await; + async fn test_search(body: impl Serialize) -> Result { + let env = TestRegistry::new().await?; - let api = RegistryApi::new(crates_io_api.url().parse().unwrap(), 0)?; - api.search("q=foo").await + env.create_api_mock("/api/v1/crates?q=foo", move |mock| { + mock.with_status(StatusCode::OK.as_u16().into()) + .with_header(CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) + .with_body(serde_json::to_vec(&body).unwrap()) + }) + .await; + + env.api().search(&SearchQuery::from("foo")).await } - async fn test_get_release( - status: StatusCode, - body: impl Serialize, + async fn test_get_release( + payload: impl IntoIterator, version: &Version, - ) -> Result> { - let mut crates_io_api = mockito::Server::new_async().await; - - let _m = crates_io_api - .mock("GET", "/api/v1/crates/krate/versions") - .with_status(status.as_u16().into()) - .with_header(CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) - .with_body(serde_json::to_vec(&body).unwrap()) - .create_async() + ) -> Result> + where + L: Serialize, + { + let env = TestRegistry::new().await?; + env.mock_index_response(&KRATE, payload).await; + + env.api().get_release_data(&KRATE, version).await + } + + #[tokio::test] + async fn test_search_ok() -> Result<()> { + let env = TestRegistry::new().await?; + let next_page = SearchCursor::builder().custom_arg("next", "").build(); + let prev_page = SearchCursor::builder().custom_arg("prev", "").build(); + + let query = SearchQuery::from("foo"); + + env.mock_search(query.clone()) + .crate_names(["foo", "bar"]) + .next_page(next_page.clone()) + .prev_page(prev_page.clone()) + .create() .await; - let api = RegistryApi::new(crates_io_api.url().parse().unwrap(), 0)?; - api.get_release_data(&KRATE, version).await + let result = env.api().search(&query).await?; + + assert_eq!( + result.crates, + vec![ + SearchCrate { name: "foo".into() }, + SearchCrate { name: "bar".into() }, + ] + ); + assert_eq!( + result.meta, + SearchMeta { + next_page: Some(next_page), + prev_page: Some(prev_page), + } + ); + + Ok(()) } - #[test] - fn test_error_without_status() { - for err in [ - Error::InvalidApiUrl, - Error::MissingReleases, - Error::MissingMetadata, - Error::Other(anyhow!("some error")), - ] { - assert!(err.status().is_none()); - } + #[tokio::test] + async fn test_search_passes_query_params_and_returns_pagination() -> Result<()> { + let env = TestRegistry::new().await?; + let this_cursor = SearchCursor::builder() + .query("some_random_crate") + .page(2) + .per_page(30) + .sort_by(SearchSort::RecentUpdates) + .build(); + let crates = vec![SearchCrate { + name: "some_random_crate".into(), + }]; + let meta = SearchMeta { + next_page: Some(this_cursor.clone().adapt().page(3).build()), + prev_page: Some(this_cursor.clone().adapt().page(1).build()), + }; + + env.mock_search(&this_cursor) + .crate_names(["some_random_crate"]) + .maybe_next_page(meta.next_page().cloned()) + .maybe_prev_page(meta.prev_page().cloned()) + .create() + .await; + + let result = env.api().search(&this_cursor).await?; + + assert_eq!(result.crates, crates); + assert_eq!(result.meta, meta); + + Ok(()) } - #[test] - fn test_error_with_included_status() { - let status = StatusCode::INTERNAL_SERVER_ERROR; + #[tokio::test] + async fn test_search_encodes_initial_search_params() -> Result<()> { + let env = TestRegistry::new().await?; + let query = SearchQuery::builder("some random crate") + .sort_by(SearchSort::RecentUpdates) + .per_page(30) + .build(); + + env.mock_search(query.clone()) + .crate_names(["some_random_crate"]) + .create() + .await; - assert!(Error::CrateIoApiError(status, ApiErrors::default()).status() == Some(status)); + let result = env.api().search(&query).await?; + assert_eq!(result.crates[0].name, "some_random_crate"); - assert!(Error::CrateIoError(status, "".into()).status() == Some(status)); + Ok(()) } #[tokio::test] - async fn test_error_reqwest_error_status() -> Result<()> { - let status = StatusCode::INTERNAL_SERVER_ERROR; - - let mut srv = mockito::Server::new_async().await; - let _m = srv - .mock("GET", "/") - .with_status(status.as_u16().into()) - .create_async() + async fn test_search_follows_pagination() -> Result<()> { + let env = TestRegistry::new().await?; + let query = SearchQuery::builder("some random crate") + .sort_by(SearchSort::RecentUpdates) + .per_page(30) + .build(); + + let next_page: SearchCursor = query.clone().into(); + let next_page = next_page.adapt().page(2).build(); + + env.mock_search(query.clone()) + .crate_names(["first_page"]) + .next_page(next_page.clone()) + .create() .await; - let err = reqwest::get(&srv.url()) - .await - .map_err(reqwest_middleware::Error::Reqwest)? - .error_for_status() - .unwrap_err(); + let first_page = env.api().search(&query).await?; + let cursor = first_page.meta.next_page().expect("next page cursor"); - assert_eq!(err.status(), Some(status)); + env.mock_search(next_page) + .crate_names(["second_page"]) + .create() + .await; + + let second_page = env.api().search(cursor).await?; + assert_eq!(second_page.crates[0].name, "second_page"); Ok(()) } #[tokio::test] - async fn test_search_ok() -> Result<()> { - let crates = vec![ - SearchCrate { name: "foo".into() }, - SearchCrate { name: "bar".into() }, - ]; + async fn test_search_passes_opaque_pagination_params() -> Result<()> { + let env = TestRegistry::new().await?; + let crates = vec![SearchCrate { + name: "some_random_crate".into(), + }]; let meta = SearchMeta { - next_page: Some("next".into()), - prev_page: Some("prev".into()), + next_page: None, + prev_page: None, }; - let result = test_search( - StatusCode::OK, - SearchResponse { - crates: Some(crates.clone()), - meta: Some(meta.clone()), - }, - ) - .await?; + let cursor = SearchCursor::builder() + .custom_arg("some", "dummy") + .custom_arg("pagination", "parameters") + .build(); + + env.mock_search(&cursor) + .crate_names(["some_random_crate"]) + .create() + .await; + + let result = env.api().search(&cursor).await?; assert_eq!(result.crates, crates); assert_eq!(result.meta, meta); @@ -331,18 +559,15 @@ mod tests { #[tokio::test] async fn test_search_crates_missing() -> Result<()> { let meta = SearchMeta { - next_page: Some("next".into()), - prev_page: Some("prev".into()), + next_page: Some("?next".parse().unwrap()), + prev_page: Some("?prev".parse().unwrap()), }; assert!(matches!( - test_search( - StatusCode::OK, - SearchResponse { - crates: None, - meta: Some(meta.clone()), - } - ) + test_search(SearchResponse { + crates: None, + meta: Some(meta.clone()), + }) .await .unwrap_err(), Error::MissingReleases @@ -359,13 +584,10 @@ mod tests { ]; assert!(matches!( - test_search( - StatusCode::OK, - SearchResponse { - crates: Some(crates.clone()), - meta: None, - } - ) + test_search(SearchResponse { + crates: Some(crates.clone()), + meta: None, + }) .await .unwrap_err(), Error::MissingMetadata @@ -378,6 +600,8 @@ mod tests { #[test_case(StatusCode::BAD_REQUEST)] #[test_case(StatusCode::UNAUTHORIZED)] async fn test_search_new_style_api_errors(status: StatusCode) -> Result<()> { + let env = TestRegistry::new().await?; + let query = SearchQuery::from("foo"); let response = ApiErrors { errors: vec![ ApiError { @@ -389,21 +613,53 @@ mod tests { ], }; + env.mock_search_error(query.clone()) + .client_error(status) + .api_errors(response.clone()) + .create() + .await; + assert!(matches!( - test_search(status, response.clone()).await.unwrap_err(), + env.api().search(&query).await.unwrap_err(), Error::CrateIoApiError(_status, errors) if errors == response )); Ok(()) } + #[tokio::test] + async fn test_search_not_found_is_a_plain_api_error() -> Result<()> { + let env = TestRegistry::new().await?; + let query = SearchQuery::from("foo"); + + env.mock_search_error(query.clone()) + .client_error(StatusCode::NOT_FOUND) + .create() + .await; + + assert!(matches!( + env.api().search(&query).await.unwrap_err(), + Error::CrateIoError(status, _) if status == StatusCode::NOT_FOUND + )); + + Ok(()) + } + #[tokio::test] #[test_case(StatusCode::INTERNAL_SERVER_ERROR)] #[test_case(StatusCode::BAD_GATEWAY)] async fn test_search_server_errors(status: StatusCode) -> Result<()> { + let env = TestRegistry::new().await?; + let query = SearchQuery::from("foo"); let msg = "some error message"; - let err = test_search(status, msg).await.unwrap_err(); + env.mock_search_error(query.clone()) + .server_error(status) + .error_text(msg) + .create() + .await; + + let err = env.api().search(&query).await.unwrap_err(); assert!(err.to_string().contains(msg)); assert_eq!(err.status(), Some(status)); @@ -417,24 +673,33 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_search_retries_server_errors() -> Result<()> { + const RETRIES: u32 = 2; + + let env = TestRegistry::builder().retries(RETRIES).build().await?; + env.create_api_mock("/api/v1/crates?q=foo", |mock| { + mock.with_status(StatusCode::INTERNAL_SERVER_ERROR.as_u16().into()) + .expect((RETRIES + 1) as usize) + }) + .await; + + assert!(matches!( + env.api().search(&SearchQuery::from("foo")).await.unwrap_err(), + Error::HttpError(error, _) if error.status() == Some(StatusCode::INTERNAL_SERVER_ERROR) + )); + env.assert_mocks().await; + + Ok(()) + } + #[tokio::test] async fn test_get_release_ok() -> Result<()> { let release_data = test_get_release( - StatusCode::OK, - serde_json::json!({ - "versions": [ - { - "num": V1.to_string(), - "created_at": "2024-01-01T00:00:00Z", - "yanked": false, - }, - { - "num": V2.to_string(), - "created_at": "2024-01-02T00:00:00Z", - "yanked": true, - } - ] - }), + [ + sparse_entry(&V1, Some("2024-01-01T00:00:00Z"), false), + sparse_entry(&V2, Some("2024-01-02T00:00:00Z"), true), + ], &V1, ) .await? @@ -443,9 +708,11 @@ mod tests { assert_eq!( release_data, ReleaseData { - release_time: DateTime::parse_from_rfc3339("2024-01-01T00:00:00Z") - .unwrap() - .with_timezone(&Utc), + release_time: Some( + DateTime::parse_from_rfc3339("2024-01-01T00:00:00Z") + .unwrap() + .with_timezone(&Utc) + ), yanked: false, } ); @@ -454,17 +721,23 @@ mod tests { } #[tokio::test] - async fn test_get_release_not_found_empty_result() -> Result<()> { - assert!( - test_get_release( - StatusCode::OK, - serde_json::json!({ - "versions": [] - }), - &V1, - ) - .await? - .is_none() + async fn test_get_release_without_pubtime() -> Result<()> { + let release_data = test_get_release( + [ + sparse_entry(&V1, None, false), + sparse_entry(&V2, Some("2024-01-02T00:00:00Z"), true), + ], + &V1, + ) + .await? + .expect("found version"); + + assert_eq!( + release_data, + ReleaseData { + release_time: None, + yanked: false, + } ); Ok(()) @@ -474,16 +747,7 @@ mod tests { async fn test_get_release_not_found_other_version() -> Result<()> { assert!( test_get_release( - StatusCode::OK, - serde_json::json!({ - "versions": [ - { - "num": V1.to_string(), - "created_at": "2024-01-01T00:00:00Z", - "yanked": false, - } - ] - }), + [sparse_entry(&V1, Some("2024-01-01T00:00:00Z"), false)], &V2, ) .await? @@ -494,12 +758,149 @@ mod tests { } #[tokio::test] - async fn test_get_release_not_found_404() -> Result<()> { + async fn test_get_release_not_found_defaults_to_404() -> Result<()> { + let env = TestRegistry::new().await?; + + assert!(env.api().get_release_data(&KRATE, &V1).await?.is_none()); + + Ok(()) + } + + #[tokio::test] + async fn test_get_crate_and_version() -> Result<()> { + let env = TestRegistry::new().await?; + env.mock_index_response( + &KRATE, + [sparse_entry(&V1, Some("2024-01-01T00:00:00Z"), false)], + ) + .await; + + assert_eq!( + env.api() + .get_crate_from_index(&KRATE) + .await? + .unwrap() + .versions() + .len(), + 1 + ); + assert_eq!( + env.api() + .get_version_from_index(&KRATE, &V1) + .await? + .unwrap() + .version(), + V1.to_string() + ); assert!( - test_get_release(StatusCode::NOT_FOUND, "", &V1) + env.api() + .get_version_from_index(&KRATE, &V2) .await? .is_none() ); + Ok(()) + } + + #[tokio::test] + async fn test_get_crate_data_normalizes_owners() -> Result<()> { + let env = TestRegistry::new().await?; + env.create_api_mock("/api/v1/crates/krate/owners", move |mock| { + mock.with_status(StatusCode::OK.as_u16().into()) + .with_header(CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) + .with_body( + serde_json::to_vec(&serde_json::json!({ + "users": [ + {"login": "team", "avatar": "avatar", "kind": "team"}, + {"login": "user"}, + {"login": ""}, + {} + ] + })) + .unwrap(), + ) + }) + .await; + + let data = env.api().get_crate_data(&KRATE).await?; + assert_eq!(data.owners.len(), 2); + assert_eq!(data.owners[0].login, "team"); + assert_eq!(data.owners[0].kind, OwnerKind::Team); + assert_eq!(data.owners[1].avatar, ""); + assert_eq!(data.owners[1].kind, OwnerKind::User); + Ok(()) + } + + #[tokio::test] + async fn test_get_release_yanked_and_invalid_pubtime() -> Result<()> { + let data = test_get_release([sparse_entry(&V1, Some("invalid"), true)], &V1).await; + assert!(matches!(data.unwrap_err(), Error::Other(_))); + + let data = test_get_release([sparse_entry(&V1, Some("2024-01-01T00:00:00Z"), true)], &V1) + .await? + .unwrap(); + assert!(data.yanked); + Ok(()) + } + + #[tokio::test] + async fn test_invalid_index_config_is_an_error() -> Result<()> { + for config in [ + IndexConfig { + dl: "http://127.0.0.1:1".into(), + api: None, + }, + IndexConfig { + dl: "http://127.0.0.1:1".into(), + api: Some("not a url".into()), + }, + ] { + let err = TestRegistry::builder() + .index_config(config) + .build() + .await + .err() + .expect("invalid config"); + assert!(matches!( + err.downcast_ref::(), + Some(Error::InvalidApiUrl) + )); + } + Ok(()) + } + + #[tokio::test] + async fn test_invalid_sparse_url_and_download_url_are_errors() -> Result<()> { + let cargo_home = tempfile::tempdir().unwrap(); + let err = RegistryApi::new( + "https://index.example".parse().unwrap(), + 0, + Some(cargo_home.path()), + ) + .await + .expect_err("invalid sparse URL"); + assert!(matches!(err, Error::SparseIndexError(_))); + + let env = TestRegistry::builder() + .index_config(IndexConfig { + dl: "not a URL".into(), + api: Some("http://127.0.0.1:1".into()), + }) + .build() + .await?; + assert!(env.api().download_url(&KRATE, &V1).is_err()); + Ok(()) + } + + #[tokio::test] + async fn test_download_url() -> Result<()> { + let env = TestRegistry::new().await?; + + assert!( + env.api() + .download_url(&KRATE, &V1)? + .to_string() + .ends_with("/crates/krate/1.0.0/download") + ); Ok(()) } diff --git a/crates/lib/docs_rs_registry_api/src/config.rs b/crates/lib/docs_rs_registry_api/src/config.rs index ef5f8b1188..e482adbbdf 100644 --- a/crates/lib/docs_rs_registry_api/src/config.rs +++ b/crates/lib/docs_rs_registry_api/src/config.rs @@ -1,23 +1,32 @@ use anyhow::Result; use docs_rs_config::AppConfig; use docs_rs_env_vars::maybe_env; +use tracing::warn; use url::Url; #[derive(Debug, bon::Builder)] +/// Configuration for [`crate::RegistryApi`]. pub struct Config { - #[builder(default = "https://crates.io".parse().unwrap())] - pub registry_api_host: Url, + /// Base URL of the sparse registry index, including its `sparse+` scheme. + /// + /// Defaults to the crates.io sparse index. + #[builder(default = crates_index::sparse::URL.parse().unwrap())] + pub sparse_index_host: Url, - // amount of retries for external API calls, mostly crates.io + /// Maximum number of retries for transient registry HTTP failures. #[builder(default = 3)] pub crates_io_api_call_retries: u32, } impl AppConfig for Config { fn from_environment() -> Result { + if maybe_env::("DOCSRS_REGISTRY_API_HOST")?.is_some() { + warn!("legacy config for registry api host. will be ignored.") + } + Ok(Self::builder() .maybe_crates_io_api_call_retries(maybe_env("DOCSRS_CRATESIO_API_CALL_RETRIES")?) - .maybe_registry_api_host(maybe_env("DOCSRS_REGISTRY_API_HOST")?) + .maybe_sparse_index_host(maybe_env("DOCSRS_SPARSE_INDEX_HOST")?) .build()) } } diff --git a/crates/lib/docs_rs_registry_api/src/error.rs b/crates/lib/docs_rs_registry_api/src/error.rs index a140196f2f..7af441a37d 100644 --- a/crates/lib/docs_rs_registry_api/src/error.rs +++ b/crates/lib/docs_rs_registry_api/src/error.rs @@ -4,6 +4,7 @@ use reqwest::StatusCode; pub(crate) type Result = std::result::Result; #[derive(Debug, thiserror::Error)] +/// Errors returned while reading registry data. pub enum Error { #[error("Invalid API url")] InvalidApiUrl, @@ -11,6 +12,12 @@ pub enum Error { CrateIoApiError(StatusCode, ApiErrors), #[error("Error from crates.io: {0}\n{1}")] CrateIoError(StatusCode, String), + #[error("Invalid search cursor: {0}")] + InvalidSearchCursor(String), + #[error(transparent)] + SparseIndexError(#[from] Box), + #[error(transparent)] + HttpLibError(#[from] http::Error), #[error("missing releases in crates.io response")] MissingReleases, #[error("missing metadata in crates.io response")] @@ -22,7 +29,7 @@ pub enum Error { } impl Error { - /// return the HTTP status code of any error inside, if there is any. + /// Return the associated HTTP status code, when the error contains one. pub fn status(&self) -> Option { match self { Self::CrateIoError(status, _) | Self::CrateIoApiError(status, _) => Some(*status), @@ -32,6 +39,18 @@ impl Error { } } +impl From for Error { + fn from(err: reqwest::Error) -> Self { + Self::HttpError(reqwest_middleware::Error::Reqwest(err), String::new()) + } +} + +impl From for Error { + fn from(err: crates_index::Error) -> Self { + Box::new(err).into() + } +} + impl From for Error { fn from(err: reqwest_middleware::Error) -> Self { Self::HttpError(err, String::new()) diff --git a/crates/lib/docs_rs_registry_api/src/lib.rs b/crates/lib/docs_rs_registry_api/src/lib.rs index 1763fd6b22..9083d41481 100644 --- a/crates/lib/docs_rs_registry_api/src/lib.rs +++ b/crates/lib/docs_rs_registry_api/src/lib.rs @@ -1,9 +1,16 @@ +//! Access to crate metadata split between the sparse registry index and its HTTP API. + mod api; mod config; mod error; mod models; +#[cfg(any(test, feature = "testing"))] +/// Test utilities for supplying a local, fully mocked registry. +pub mod testing; pub use api::RegistryApi; pub use config::Config; pub use error::Error; -pub use models::{CrateData, CrateOwner, OwnerKind, ReleaseData, Search}; +pub use models::{ + CrateData, CrateOwner, OwnerKind, ReleaseData, Search, SearchCursor, SearchQuery, SearchSort, +}; diff --git a/crates/lib/docs_rs_registry_api/src/models.rs b/crates/lib/docs_rs_registry_api/src/models.rs index 9bf0e63af6..3418594f2b 100644 --- a/crates/lib/docs_rs_registry_api/src/models.rs +++ b/crates/lib/docs_rs_registry_api/src/models.rs @@ -1,82 +1,399 @@ +use crate::error::{Error, Result}; use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; -use std::fmt; +use serde_with::{DeserializeFromStr, SerializeDisplay}; +use std::{borrow::Cow, fmt, str::FromStr}; +const SEARCH_ARG_QUERY: &str = "q"; +const SEARCH_ARG_SORT: &str = "sort"; +const SEARCH_ARG_PER_PAGE: &str = "per_page"; +const SEARCH_ARG_PAGE: &str = "page"; + +/// Crate-level metadata returned by [`crate::RegistryApi`]. #[derive(Debug)] pub struct CrateData { + /// Owners reported by the registry API. pub owners: Vec, } -#[derive(Debug)] +/// Metadata for one published crate version. +#[derive(Debug, Clone)] #[cfg_attr(test, derive(PartialEq))] pub struct ReleaseData { - pub release_time: DateTime, + /// Time at which the version was published. + pub release_time: Option>, + /// Whether the version has been yanked. pub yanked: bool, } -impl Default for ReleaseData { - fn default() -> ReleaseData { +/// The `ReleaseData` fields represent what we get from the registry / index. +/// +/// But: in many places all over the codebase, we might expect data in +/// `releases.yanked` or `releases.release_time` when a build is finished. +/// +/// So for now we explicitly generate dummy data and insert it. +impl ReleaseData { + pub fn dummy() -> Self { ReleaseData { - release_time: Utc::now(), + release_time: Some(Utc::now()), yanked: false, } } + + /// `ReleaseData` with dummy values inserted if needed. + pub fn for_database(mut self) -> Self { + if self.release_time.is_none() { + self.release_time = Self::dummy().release_time; + } + + self + } } +/// An owner of a crate as reported by the registry API. #[derive(Debug, Clone)] pub struct CrateOwner { + /// URL of the owner's avatar, if the API supplied one. pub avatar: String, + /// The owner's registry login. pub login: String, + /// Whether this owner is a user or a team. pub kind: OwnerKind, } +/// Kind of a crate owner. #[derive( - Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, sqlx::Type, + Debug, + Clone, + Copy, + PartialEq, + Eq, + PartialOrd, + Ord, + Serialize, + Deserialize, + sqlx::Type, + strum::Display, )] #[sqlx(type_name = "owner_kind", rename_all = "lowercase")] #[serde(rename_all = "lowercase")] +#[strum(serialize_all = "lowercase")] pub enum OwnerKind { + /// An individual registry user. User, + /// A registry team. Team, } -impl fmt::Display for OwnerKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::User => f.write_str("user"), - Self::Team => f.write_str("team"), - } - } -} - #[derive(Deserialize, Debug, Default)] -#[cfg_attr(test, derive(Serialize))] +#[cfg_attr(any(test, feature = "testing"), derive(Serialize))] pub(crate) struct SearchResponse { pub(crate) crates: Option>, pub(crate) meta: Option, } +/// A crate returned by a registry search. #[derive(Deserialize, Debug)] -#[cfg_attr(test, derive(Serialize, PartialEq, Clone))] +#[cfg_attr(any(test, feature = "testing"), derive(Serialize))] +#[cfg_attr(test, derive(PartialEq, Clone))] pub struct SearchCrate { + /// Name of the matching crate. pub name: String, } +/// Pagination cursors returned by a registry search. #[derive(Deserialize, Debug)] -#[cfg_attr(test, derive(Serialize, PartialEq, Clone))] +#[cfg_attr(any(test, feature = "testing"), derive(Serialize))] +#[cfg_attr(test, derive(PartialEq, Clone))] pub struct SearchMeta { - pub next_page: Option, - pub prev_page: Option, + /// Cursor for the next result page, if one exists. + pub(crate) next_page: Option, + /// Cursor for the previous result page, if one exists. + pub(crate) prev_page: Option, +} + +impl SearchMeta { + /// Return the next-page cursor as a typed value. + pub fn next_page(&self) -> Option<&SearchCursor> { + self.next_page.as_ref() + } + + /// Return the previous-page cursor as a typed value. + pub fn prev_page(&self) -> Option<&SearchCursor> { + self.prev_page.as_ref() + } +} + +/// An opaque cursor returned by the registry search API for a subsequent result page. +/// +/// The registry owns the cursor's query parameters. Callers can inspect the search query and +/// sort order needed for display, but can't change it. +#[derive(Debug, Clone, PartialEq, Eq, SerializeDisplay, DeserializeFromStr)] +pub struct SearchCursor(String); + +impl<'a> SearchCursor { + pub fn parameters(&'a self) -> impl Iterator, Cow<'a, str>)> { + url::form_urlencoded::parse(self.0.trim_start_matches('?').as_bytes()) + } + + fn parameter(&'a self, name: &str) -> Option> { + self.parameters() + .find_map(|(key, value)| (key == name).then_some(value)) + } + + fn parse_parameter(&'a self, name: &str) -> Result> + where + T: FromStr, + T::Err: fmt::Debug, + { + let Some(value) = self.parameter(name) else { + return Ok(None); + }; + + Ok(Some(value.parse().map_err(|err| { + Error::InvalidSearchCursor(format!( + "unknown valid in \"{}\" argument: {}\n{:?}", + name, value, err + )) + })?)) + } + + /// Return the original query parameters supplied by the registry. + pub fn as_params(&self) -> &str { + &self.0 + } + + /// Return the search term embedded in this cursor, if one is present. + pub fn query(&'a self) -> Option> { + self.parameter(SEARCH_ARG_QUERY) + } + + /// Return the `page` number + pub fn page(&'a self) -> Result> { + self.parse_parameter(SEARCH_ARG_PAGE) + } + + /// Return the `per_page` number + pub fn per_page(&'a self) -> Result> { + self.parse_parameter(SEARCH_ARG_PER_PAGE) + } + + /// Return the sort order embedded in this cursor, if one is present. + pub fn sort_by(&self) -> Result> { + self.parse_parameter(SEARCH_ARG_SORT) + } + + /// `url::Url` needs the query without the leading `?` + pub(crate) fn query_for_url(&self) -> &str { + self.0.trim_start_matches('?') + } +} + +/// `SearchCursor::builder`, only for tests. +/// +/// In prod code, the `SearchCursor` object is always created from a +/// crates.io API response, via `FromStr`. +/// +/// Ensures that there are no duplicates, but keeps the intersion order. +/// This is helpful when using `.adapt()`. +#[cfg(any(test, feature = "testing"))] +pub struct SearchCursorBuilder { + args: Vec<(String, String)>, +} + +#[cfg(any(test, feature = "testing"))] +impl SearchCursorBuilder { + pub fn custom_arg(mut self, name: impl Into, value: impl Into) -> Self { + let name = name.into(); + let value = value.into(); + + // if we already have the key in our cursor, update the value. + if let Some((_, current_value)) = self.args.iter_mut().find(|(key, _)| key == &name) { + *current_value = value; + } else { + self.args.push((name, value)); + } + + self + } + + pub fn query(self, query: impl Into) -> Self { + self.custom_arg(SEARCH_ARG_QUERY, query) + } + + pub fn sort_by(self, sort_by: SearchSort) -> Self { + self.custom_arg(SEARCH_ARG_SORT, sort_by.id()) + } + + pub fn per_page(self, per_page: u32) -> Self { + self.custom_arg(SEARCH_ARG_PER_PAGE, per_page.to_string()) + } + + pub fn page(self, page: u32) -> Self { + self.custom_arg(SEARCH_ARG_PAGE, page.to_string()) + } + + pub fn build(self) -> SearchCursor { + SearchCursor(format!( + "?{}", + url::form_urlencoded::Serializer::new(String::new()) + .extend_pairs(self.args) + .finish() + )) + } +} + +#[cfg(any(test, feature = "testing"))] +impl SearchCursor { + /// create a builder to create a new `SearchCursor` for tests. + pub fn builder() -> SearchCursorBuilder { + SearchCursorBuilder { args: Vec::new() } + } + + /// creates a pre-filled builder from an existing `SearchCursor`. + /// + /// Useful when you have an initial `SearchCursor` and then want + /// to create the `next_page` and `prev_page` cursors just with + /// one attribute changed, while the rest stays the same. + pub fn adapt(self) -> SearchCursorBuilder { + SearchCursorBuilder { + args: self + .parameters() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), + } + } +} + +impl fmt::Display for SearchCursor { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.0) + } } +impl AsRef for SearchCursor { + fn as_ref(&self) -> &str { + self.as_params() + } +} + +impl FromStr for SearchCursor { + type Err = Error; + + fn from_str(params: &str) -> Result { + if params.starts_with('?') { + Ok(Self(params.into())) + } else { + Err(Error::InvalidSearchCursor( + "registry pagination cursor must start with '?'".into(), + )) + } + } +} + +impl From<&SearchQuery> for SearchCursor { + fn from(query: &SearchQuery) -> Self { + query.clone().into() + } +} + +impl From<&SearchCursor> for SearchCursor { + fn from(cursor: &SearchCursor) -> Self { + cursor.clone() + } +} + +impl From for SearchCursor { + fn from(query: SearchQuery) -> Self { + format!( + "?{}", + serde_urlencoded::to_string(query).expect("always succeeds") + ) + .parse() + .expect("SearchQuery always produces a valid cursor") + } +} + +/// The sorting options the crates.io search API offers. +#[derive( + Debug, + PartialEq, + Eq, + Clone, + Copy, + Default, + strum::EnumString, + strum::Display, + strum::EnumIter, + strum::IntoStaticStr, + strum::AsRefStr, + SerializeDisplay, + DeserializeFromStr, +)] +#[strum(serialize_all = "kebab-case")] +pub enum SearchSort { + #[default] + Relevance, + Downloads, + RecentDownloads, + RecentUpdates, + New, +} + +impl SearchSort { + pub fn id(&self) -> &'static str { + self.into() + } + + /// Return the human-readable label used by the UI. + pub fn label(&self) -> &'static str { + match *self { + Self::Relevance => "Relevance", + Self::Downloads => "All-Time Downloads", + Self::RecentDownloads => "Recent Downloads", + Self::RecentUpdates => "Recent Updates", + Self::New => "Newly Added", + } + } +} + +/// Parameters for an crates.io crate search. +#[derive(Debug, Clone, PartialEq, Eq, bon::Builder, Serialize, Deserialize)] +pub struct SearchQuery { + #[builder(start_fn, into)] + #[serde(rename = "q")] + query: String, + + #[serde(rename = "sort", skip_serializing_if = "Option::is_none")] + sort_by: Option, + + #[serde(skip_serializing_if = "Option::is_none")] + per_page: Option, +} + +impl From for SearchQuery { + fn from(query: String) -> Self { + Self::builder(query).build() + } +} + +impl From<&str> for SearchQuery { + fn from(query: &str) -> Self { + Self::builder(query).build() + } +} + +/// Results returned by [`crate::RegistryApi::search`]. #[derive(Deserialize, Debug)] pub struct Search { + /// Crates matching the requested query. pub crates: Vec, + /// Pagination cursors for the result set. pub meta: SearchMeta, } #[derive(Deserialize, Debug, Clone, PartialEq, Eq, Default)] -#[cfg_attr(test, derive(Serialize))] +#[cfg_attr(any(test, feature = "testing"), derive(Serialize))] pub struct ApiErrors { pub errors: Vec, } @@ -91,7 +408,7 @@ impl fmt::Display for ApiErrors { } #[derive(Deserialize, Debug, Clone, PartialEq, Eq)] -#[cfg_attr(test, derive(Serialize))] +#[cfg_attr(any(test, feature = "testing"), derive(Serialize))] pub struct ApiError { pub detail: Option, } @@ -105,3 +422,157 @@ impl fmt::Display for ApiError { ) } } + +#[cfg(test)] +mod tests { + use super::*; + use test_case::test_case; + + #[test] + fn search_cursor_parses_registry_pagination_params() { + let cursor = SearchCursor::builder() + .query("some crate") + .sort_by(SearchSort::RecentUpdates) + .page(2) + .custom_arg("opaque", "value/with escapes") + .build(); + + assert_eq!( + cursor.as_params(), + "?q=some+crate&sort=recent-updates&page=2&opaque=value%2Fwith+escapes" + ); + assert_eq!(cursor.query().as_deref(), Some("some crate")); + assert_eq!(cursor.sort_by().unwrap(), Some(SearchSort::RecentUpdates)); + assert_eq!( + cursor + .parameters() + .map(|(key, value)| (key.into_owned(), value.into_owned())) + .collect::>(), + vec![ + (SEARCH_ARG_QUERY.into(), "some crate".into()), + (SEARCH_ARG_SORT.into(), "recent-updates".into()), + (SEARCH_ARG_PAGE.into(), "2".into()), + ("opaque".into(), "value/with escapes".into()), + ] + ); + let serialized = serde_json::to_string(&cursor).unwrap(); + assert_eq!(serialized, format!("\"{cursor}\"")); + let deserialized: SearchCursor = serde_json::from_str(&serialized).unwrap(); + assert_eq!(deserialized.as_params(), cursor.as_params()); + } + + #[test] + fn search_cursor_rejects_invalid_values() { + let error = "q=crate".parse::().unwrap_err(); + assert_eq!( + error.to_string(), + "Invalid search cursor: registry pagination cursor must start with '?'" + ); + + let cursor: SearchCursor = "?sort=not-a-sort".parse().unwrap(); + assert!(cursor.sort_by().is_err()); + } + + #[test] + fn search_cursor_builder_keeps_argument_order() { + let cursor = SearchCursor::builder() + .custom_arg("zoo", "0") + .custom_arg("first", "1") + .custom_arg("second", "2") + .custom_arg("third", "3") + .build(); + + assert_eq!(cursor.as_params(), "?zoo=0&first=1&second=2&third=3"); + } + + #[test] + fn search_cursor_builder_overwrites_an_existing_argument_in_place() { + let cursor = SearchCursor::builder() + .custom_arg("first", "old") + .custom_arg("second", "2") + .custom_arg("first", "new") + .build(); + + assert_eq!(cursor.as_params(), "?first=new&second=2"); + } + + #[test_case(SearchSort::Relevance, "relevance", "Relevance")] + #[test_case(SearchSort::Downloads, "downloads", "All-Time Downloads")] + #[test_case(SearchSort::RecentDownloads, "recent-downloads", "Recent Downloads")] + #[test_case(SearchSort::RecentUpdates, "recent-updates", "Recent Updates")] + #[test_case(SearchSort::New, "new", "Newly Added")] + fn search_sort_has_stable_identifiers_and_labels(sort: SearchSort, id: &str, label: &str) { + assert_eq!(sort.id(), id); + assert_eq!(sort.to_string(), id); + assert_eq!(id.parse::().unwrap(), sort); + assert_eq!(sort.label(), label); + assert_eq!(serde_json::to_string(&sort).unwrap(), format!("\"{id}\"")); + } + + #[test] + fn search_sort_rejects_unknown_identifier() { + assert!("oldest".parse::().is_err()); + } + + #[test] + fn search_query_serializes_with_defaults_and_custom_values() { + fn parse(params: &str) -> Vec<(String, String)> { + url::form_urlencoded::parse(params.as_bytes()) + .into_iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect() + } + + let default_query = SearchQuery::from("some crate"); + let default_params = serde_urlencoded::to_string(&default_query).unwrap(); + assert_eq!( + parse(&default_params), + vec![(SEARCH_ARG_QUERY.into(), "some crate".into())] + ); + + let custom_query = SearchQuery::builder("some crate") + .sort_by(SearchSort::RecentUpdates) + .per_page(50) + .build(); + let custom_params = serde_urlencoded::to_string(&custom_query).unwrap(); + assert_eq!( + parse(&custom_params), + vec![ + (SEARCH_ARG_QUERY.into(), "some crate".into()), + (SEARCH_ARG_SORT.into(), "recent-updates".into()), + (SEARCH_ARG_PER_PAGE.into(), "50".into()), + ] + ); + assert_eq!( + SearchCursor::from(custom_query).as_params(), + format!("?{custom_params}") + ); + } + + #[test_case(OwnerKind::User, "user")] + #[test_case(OwnerKind::Team, "team")] + fn owner_kind_display_and_serde_round_trip(kind: OwnerKind, value: &str) { + assert_eq!(kind.to_string(), value); + assert_eq!( + serde_json::to_string(&kind).unwrap(), + format!("\"{value}\"") + ); + assert_eq!( + serde_json::from_str::(&format!("\"{value}\"")).unwrap(), + kind + ); + } + + #[test] + fn search_cursor_requires_a_query_string() { + let error = "page=2".parse::().unwrap_err(); + assert_eq!( + error.to_string(), + "Invalid search cursor: registry pagination cursor must start with '?'" + ); + assert_eq!( + "?page=2".parse::().unwrap().as_params(), + "?page=2" + ); + } +} diff --git a/crates/lib/docs_rs_registry_api/src/testing/mod.rs b/crates/lib/docs_rs_registry_api/src/testing/mod.rs new file mode 100644 index 0000000000..2cf47776b8 --- /dev/null +++ b/crates/lib/docs_rs_registry_api/src/testing/mod.rs @@ -0,0 +1,3 @@ +mod test_env; + +pub use test_env::TestRegistry; diff --git a/crates/lib/docs_rs_registry_api/src/testing/test_env.rs b/crates/lib/docs_rs_registry_api/src/testing/test_env.rs new file mode 100644 index 0000000000..c28ab3991c --- /dev/null +++ b/crates/lib/docs_rs_registry_api/src/testing/test_env.rs @@ -0,0 +1,412 @@ +use crate::{ + Config, RegistryApi, SearchCursor, + models::{ApiError, ApiErrors, SearchCrate, SearchMeta, SearchResponse}, +}; +use anyhow::Result; +use bon::bon; +use docs_rs_types::KrateName; +use docs_rs_utils::spawn_blocking; +use mockito::Matcher; +use reqwest::{StatusCode, header::CONTENT_TYPE}; +use serde::Serialize; +use std::sync::Arc; +use tokio::sync::Mutex; +use url::Url; + +struct TestRegistryInner { + api_server: mockito::ServerGuard, + index_server: mockito::ServerGuard, + #[allow(dead_code)] + download_server: mockito::ServerGuard, + mocks: Vec, +} + +/// A local registry fixture backed by isolated mock HTTP servers. +/// +/// It provides a [`RegistryApi`] configured to use a temporary sparse-index cache, API server, +/// and download server. Add only the responses required by the test; unknown sparse-index crate +/// entries behave as not found. +pub struct TestRegistry { + #[allow(dead_code)] + cargo_home: tempfile::TempDir, + inner: Mutex, + api: Arc, + config: Arc, +} + +#[bon] +impl TestRegistry { + pub async fn new() -> Result { + Self::builder().build().await + } + + #[builder( + finish_fn(name = build), + on(_, into) + )] + pub(crate) async fn builder( + /// set http retries in case of errors. + /// Only needed for testing the retry behaviour in this crate. + /// The workspace-wide `TestRegistry` doesn't retry. + #[builder(default)] + retries: u32, + + /// custom index config, in case we want to test broken index config + /// error handling. + /// By default, we'll generate a correct `IndexConfig`, that fits + /// to the local http mocks we set up. + index_config: Option, + ) -> Result { + let cargo_home = spawn_blocking(|| Ok(tempfile::tempdir()?)).await?; + let api_server = mockito::Server::new_async().await; + let mut index_server = mockito::Server::new_async().await; + let download_server = mockito::Server::new_async().await; + + let index_config = index_config.unwrap_or_else(|| crates_index::IndexConfig { + dl: format!("{}/crates", download_server.url()), + api: Some(api_server.url()), + }); + + // Mockito chooses matching mocks that still expect hits first, then the last match. An + // optional fallback registered first therefore yields to the config and crate mocks below. + let index_object_missing_mock = index_server + .mock("GET", Matcher::Any) + .expect_at_least(0) + .with_status(StatusCode::NOT_FOUND.as_u16().into()) + .create_async() + .await; + + let config_mock = index_server + .mock("GET", "/config.json") + .with_header(CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) + .with_body(serde_json::to_vec(&serde_json::json!({ + "dl": index_config.dl, + "api": index_config.api, + }))?) + .create_async() + .await; + + // NOTE: cargo expects the `sparse+` schema prefix so it can differentiate + // the sparse index url from a git-http URL. + let index_url: Url = format!("sparse+{}", index_server.url()).parse()?; + + let api = RegistryApi::new(index_url.clone(), retries, Some(cargo_home.path())).await?; + + let config = Arc::new( + Config::builder() + .crates_io_api_call_retries(retries) + .sparse_index_host(index_url) + .build(), + ); + + Ok(Self { + cargo_home, + inner: Mutex::new(TestRegistryInner { + api_server, + index_server, + download_server, + mocks: vec![config_mock, index_object_missing_mock], + }), + api: Arc::new(api), + config, + }) + } + + /// dummy config for the global context. + /// + /// Matches the setup of the `TestEnvironment`. + pub fn test_config(&self) -> &Arc { + &self.config + } + + /// Mock the sparse-index response for `krate`. + /// + /// Each item in `versions` is one JSON line from a sparse-index crate entry. + pub async fn mock_index_response( + &self, + krate: &KrateName, + versions: impl IntoIterator, + ) where + L: Serialize, + { + let crate_url: Url = self + .api + .sparse_index + .crate_url(krate.as_str()) + .unwrap() + .parse() + .unwrap(); + + let payload = versions + .into_iter() + .map(|v| serde_json::to_string(&v).unwrap()) + .collect::>() + .join("\n"); + + let mut inner = self.inner.lock().await; + + let index_mock = inner + .index_server + .mock("GET", crate_url.path()) + .with_status(StatusCode::OK.as_u16().into()) + .with_body(payload) + .create_async() + .await; + + inner.mocks.push(index_mock); + } + + /// Create a custom mock for a registry API `GET` request. + /// + /// `path` may include a query string, whose URL-encoded pairs are matched independently of + /// their order. The closure can configure status, headers, and response body on the + /// underlying Mockito mock. + pub(crate) async fn create_api_mock(&self, path: impl AsRef, mut f: F) + where + F: FnMut(mockito::Mock) -> mockito::Mock, + { + let url = Url::parse("http://registry.test") + .unwrap() + .join(path.as_ref()) + .unwrap(); + let query_matchers: Vec<_> = url + .query_pairs() + .map(|(name, value)| Matcher::UrlEncoded(name.into_owned(), value.into_owned())) + .collect(); + + let mut inner = self.inner.lock().await; + let mut mock = inner.api_server.mock("GET", url.path()); + if !query_matchers.is_empty() { + mock = mock.match_query(Matcher::AllOf(query_matchers)); + } + let mock = f(mock).create_async().await; + inner.mocks.push(mock); + } + + /// Mock the registry search API with an error response. + /// + /// first, call either `client_error` or `server_error`. + /// + /// Calling either disables the other. + /// + /// Then, you can call `api_errors`, `api_error_messages` + /// or `error_text` (for client error / server error) for + /// the body of the response. + /// + /// You can also just call `.create`, without defining client/server error, then + /// we'll mock a INTERNAL_SERVER_ERROR. + #[builder( + builder_type = SearchErrorMockBuilder, + state_mod = search_error_mock_builder, + finish_fn = create + )] + pub async fn mock_search_error( + &self, + #[builder(start_fn)] cursor: C, + + #[builder( + setters(vis = "", name = client_status_internal) + )] + client_status: Option, + + #[builder( + into, + setters(vis = "", name = api_errors_internal) + )] + api_errors: Option, + + #[builder( + default = StatusCode::INTERNAL_SERVER_ERROR, + setters(vis = "", name = server_status_internal) + )] + server_status: StatusCode, + + #[builder( + default, + into, + setters(vis = "", name = error_text_internal) + )] + error_text: String, + ) where + C: Into, + { + let cursor = cursor.into(); + + self.create_api_mock(format!("/api/v1/crates{cursor}"), move |mock| { + // NOTE: the builder ensures at compile or build-time that + // * either client status or server status are set + // * any status that is set, is a correct client/server status. + if let Some(status) = client_status { + let mock = mock.with_status(status.as_u16().into()); + + if let Some(api_errors) = api_errors.as_ref() { + mock.with_header(CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) + .with_body(serde_json::to_vec(&api_errors).unwrap()) + } else { + // without error messages, the body should by empty + mock + } + } else { + mock.with_status(server_status.as_u16().into()) + .with_header(CONTENT_TYPE, mime::TEXT_PLAIN.as_ref()) + .with_body(&error_text) + } + }) + .await; + } + + /// Mock the registry search API with domain data instead of a raw HTTP response. + /// + /// The cursor's URL-encoded pairs are matched independently of their order. + #[builder( + builder_type = SearchMockBuilder, + state_mod = search_mock_builder, + finish_fn = create + )] + pub async fn mock_search( + &self, + #[builder(start_fn)] cursor: C, + search_result: Vec, + next_page: Option, + prev_page: Option, + ) where + C: Into, + { + let cursor = cursor.into(); + + let response = SearchResponse { + crates: Some(search_result), + meta: Some(SearchMeta { + next_page, + prev_page, + }), + }; + + self.create_api_mock(format!("/api/v1/crates{cursor}"), move |mock| { + mock.with_status(StatusCode::OK.as_u16().into()) + .with_header(CONTENT_TYPE, mime::APPLICATION_JSON.as_ref()) + .with_body(serde_json::to_vec(&response).unwrap()) + }) + .await; + } + + /// Assert that every required mock received its expected requests. + pub async fn assert_mocks(&self) { + let inner = self.inner.lock().await; + + for mock in &inner.mocks { + mock.assert_async().await; + } + } + + /// Return the registry client configured for this fixture. + pub fn api(&self) -> &Arc { + &self.api + } +} + +use search_mock_builder::{IsUnset, SetSearchResult, State}; + +/// Extends the builder that is returned from `mock_search`. +impl<'a, C, S> SearchMockBuilder<'a, C, S> +where + C: Into, + S: State, +{ + pub fn crate_names(self, names: I) -> SearchMockBuilder<'a, C, SetSearchResult> + where + N: Into, + I: IntoIterator, + S::SearchResult: IsUnset, + { + let crates: Vec<_> = names + .into_iter() + .map(|name| SearchCrate { name: name.into() }) + .collect(); + + self.search_result(crates) + } +} + +use search_error_mock_builder::{ + IsSet as EIsSet, IsUnset as EIsUnset, SetApiErrors, SetClientStatus, SetErrorText, + SetServerStatus, State as EState, +}; + +/// Extends the builder that is returned from `mock_search_error`. +impl<'a, C, S> SearchErrorMockBuilder<'a, C, S> +where + C: Into, + S: EState, +{ + pub fn client_error( + self, + status: StatusCode, + ) -> SearchErrorMockBuilder<'a, C, SetClientStatus> + where + S::ClientStatus: EIsUnset, + S::ApiErrors: EIsUnset, + S::ServerStatus: EIsUnset, + S::ErrorText: EIsUnset, + { + assert!(status.is_client_error()); + self.client_status_internal(status) + } + + pub fn api_errors(self, api_errors: ApiErrors) -> SearchErrorMockBuilder<'a, C, SetApiErrors> + where + S::ClientStatus: EIsSet, + S::ApiErrors: EIsUnset, + S::ServerStatus: EIsUnset, + S::ErrorText: EIsUnset, + { + self.api_errors_internal(api_errors) + } + + pub fn api_error_messages( + self, + api_error_messages: I, + ) -> SearchErrorMockBuilder<'a, C, SetApiErrors> + where + E: Into, + I: IntoIterator, + S::ClientStatus: EIsSet, + S::ApiErrors: EIsUnset, + S::ServerStatus: EIsUnset, + S::ErrorText: EIsUnset, + { + let errors: Vec<_> = api_error_messages + .into_iter() + .map(|e| ApiError { + detail: Some(e.into()), + }) + .collect(); + + self.api_errors(ApiErrors { errors }) + } + + pub fn server_error( + self, + status: StatusCode, + ) -> SearchErrorMockBuilder<'a, C, SetServerStatus> + where + S::ClientStatus: EIsUnset, + S::ApiErrors: EIsUnset, + S::ServerStatus: EIsUnset, + S::ErrorText: EIsUnset, + { + assert!(status.is_server_error()); + self.server_status_internal(status) + } + + pub fn error_text(self, error_text: E) -> SearchErrorMockBuilder<'a, C, SetErrorText> + where + E: Into, + S::ClientStatus: EIsUnset, + S::ApiErrors: EIsUnset, + S::ServerStatus: EIsSet, + S::ErrorText: EIsUnset, + { + self.error_text_internal(error_text.into()) + } +} diff --git a/crates/lib/docs_rs_test_fakes/src/legacy.rs b/crates/lib/docs_rs_test_fakes/src/legacy.rs index fad629cf82..4fbd806514 100644 --- a/crates/lib/docs_rs_test_fakes/src/legacy.rs +++ b/crates/lib/docs_rs_test_fakes/src/legacy.rs @@ -146,10 +146,7 @@ impl<'a> FakeRelease<'a> { doc_targets: Vec::new(), default_target: None, registry_crate_data: CrateData { owners: Vec::new() }, - registry_release_data: ReleaseData { - release_time: Utc::now(), - yanked: false, - }, + registry_release_data: ReleaseData::dummy(), has_docs: true, has_examples: false, readme: None, @@ -171,7 +168,7 @@ impl<'a> FakeRelease<'a> { } pub fn release_time(mut self, new: DateTime) -> Self { - self.registry_release_data.release_time = new; + self.registry_release_data.release_time = Some(new); self } From 0b9f1471a5fdd26d0474921bc5000bf311056136 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Thu, 3 Sep 2026 14:20:07 +0200 Subject: [PATCH 2/2] fix typo in comment Co-authored-by: Guillaume Gomez --- crates/bin/docs_rs_watcher/src/consistency/diff.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bin/docs_rs_watcher/src/consistency/diff.rs b/crates/bin/docs_rs_watcher/src/consistency/diff.rs index 1ad2d27914..b9c730483a 100644 --- a/crates/bin/docs_rs_watcher/src/consistency/diff.rs +++ b/crates/bin/docs_rs_watcher/src/consistency/diff.rs @@ -85,7 +85,7 @@ where // NOTE: `yanked` and `release_time` come both from the // crates.io sparse index, or historically from the crates.io API. - // We might have releases were both fields are empty because of an + // We might have releases where both fields are empty because of an // error, or because the release build is still in progress. // So there might be cases where `release_time` was empty because // it was empty on the index (unlikely), or we might have cases