From 1e41d761d53ce79196ada11cb7afebe1323ee1e9 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 15 Dec 2024 12:43:07 +0100 Subject: [PATCH 1/7] axum: Extract `CratePath` and `CrateVersionPath` structs --- src/controllers/krate.rs | 9 ++++++++ src/controllers/krate/delete.rs | 14 ++++-------- src/controllers/krate/downloads.rs | 11 ++++----- src/controllers/krate/follow.rs | 22 ++++++------------ src/controllers/krate/metadata.rs | 30 ++++++++++-------------- src/controllers/krate/owners.rs | 34 ++++++++++++---------------- src/controllers/krate/versions.rs | 14 ++++-------- src/controllers/version.rs | 10 ++++++++ src/controllers/version/downloads.rs | 15 ++++++------ src/controllers/version/metadata.rs | 30 +++++++++++------------- src/controllers/version/yank.rs | 22 ++++++++---------- 11 files changed, 96 insertions(+), 115 deletions(-) diff --git a/src/controllers/krate.rs b/src/controllers/krate.rs index ca1cd37cddb..041e3fde67a 100644 --- a/src/controllers/krate.rs +++ b/src/controllers/krate.rs @@ -1,3 +1,5 @@ +use axum::extract::{FromRequestParts, Path}; + pub mod delete; pub mod downloads; pub mod follow; @@ -6,3 +8,10 @@ pub mod owners; pub mod publish; pub mod search; pub mod versions; + +#[derive(Deserialize, FromRequestParts)] +#[from_request(via(Path))] +pub struct CratePath { + /// Name of the crate + pub name: String, +} diff --git a/src/controllers/krate/delete.rs b/src/controllers/krate/delete.rs index 385a5ea27fe..5f99ec2bbd4 100644 --- a/src/controllers/krate/delete.rs +++ b/src/controllers/krate/delete.rs @@ -1,10 +1,10 @@ use crate::app::AppState; use crate::auth::AuthCheck; +use crate::controllers::krate::CratePath; use crate::models::{Crate, NewDeletedCrate, Rights}; use crate::schema::{crate_downloads, crates, dependencies}; use crate::util::errors::{crate_not_found, custom, AppResult, BoxedAppError}; use crate::worker::jobs; -use axum::extract::Path; use bigdecimal::ToPrimitive; use chrono::{TimeDelta, Utc}; use crates_io_database::schema::deleted_crates; @@ -33,19 +33,15 @@ const AVAILABLE_AFTER: TimeDelta = TimeDelta::hours(24); tag = "crates", responses((status = 200, description = "Successful Response")), )] -pub async fn delete_crate( - Path(name): Path, - parts: Parts, - app: AppState, -) -> AppResult { +pub async fn delete_crate(path: CratePath, parts: Parts, app: AppState) -> AppResult { let mut conn = app.db_write().await?; // Check that the user is authenticated let auth = AuthCheck::only_cookie().check(&parts, &mut conn).await?; // Check that the crate exists - let krate = find_crate(&mut conn, &name).await?; - let krate = krate.ok_or_else(|| crate_not_found(&name))?; + let krate = find_crate(&mut conn, &path.name).await?; + let krate = krate.ok_or_else(|| crate_not_found(&path.name))?; // Check that the user is an owner of the crate (team owners are not allowed to delete crates) let user = auth.user(); @@ -106,7 +102,7 @@ pub async fn delete_crate( let git_index_job = jobs::SyncToGitIndex::new(&krate.name); let sparse_index_job = jobs::SyncToSparseIndex::new(&krate.name); - let delete_from_storage_job = jobs::DeleteCrateFromStorage::new(name); + let delete_from_storage_job = jobs::DeleteCrateFromStorage::new(path.name); tokio::try_join!( git_index_job.enqueue(conn), diff --git a/src/controllers/krate/downloads.rs b/src/controllers/krate/downloads.rs index 4f156593fae..aa377bb007f 100644 --- a/src/controllers/krate/downloads.rs +++ b/src/controllers/krate/downloads.rs @@ -4,12 +4,12 @@ //! download counts are located in `version::downloads`. use crate::app::AppState; +use crate::controllers::krate::CratePath; use crate::models::{Crate, Version, VersionDownload}; use crate::schema::{crates, version_downloads, versions}; use crate::sql::to_char; use crate::util::errors::{crate_not_found, AppResult}; use crate::views::EncodableVersionDownload; -use axum::extract::Path; use axum_extra::json; use axum_extra::response::ErasedJson; use diesel::prelude::*; @@ -27,21 +27,18 @@ use std::cmp; responses((status = 200, description = "Successful Response")), )] -pub async fn get_crate_downloads( - state: AppState, - Path(crate_name): Path, -) -> AppResult { +pub async fn get_crate_downloads(state: AppState, path: CratePath) -> AppResult { let mut conn = state.db_read().await?; use diesel::dsl::*; use diesel::sql_types::BigInt; - let crate_id: i32 = Crate::by_name(&crate_name) + let crate_id: i32 = Crate::by_name(&path.name) .select(crates::id) .first(&mut conn) .await .optional()? - .ok_or_else(|| crate_not_found(&crate_name))?; + .ok_or_else(|| crate_not_found(&path.name))?; let mut versions: Vec = versions::table .filter(versions::crate_id.eq(crate_id)) diff --git a/src/controllers/krate/follow.rs b/src/controllers/krate/follow.rs index 0f94a2e45fa..a8509c7af55 100644 --- a/src/controllers/krate/follow.rs +++ b/src/controllers/krate/follow.rs @@ -3,10 +3,10 @@ use crate::app::AppState; use crate::auth::AuthCheck; use crate::controllers::helpers::ok_true; +use crate::controllers::krate::CratePath; use crate::models::{Crate, Follow}; use crate::schema::*; use crate::util::errors::{crate_not_found, AppResult}; -use axum::extract::Path; use axum::response::Response; use axum_extra::json; use axum_extra::response::ErasedJson; @@ -36,14 +36,10 @@ async fn follow_target( tag = "crates", responses((status = 200, description = "Successful Response")), )] -pub async fn follow_crate( - app: AppState, - Path(crate_name): Path, - req: Parts, -) -> AppResult { +pub async fn follow_crate(app: AppState, path: CratePath, req: Parts) -> AppResult { let mut conn = app.db_write().await?; let user_id = AuthCheck::default().check(&req, &mut conn).await?.user_id(); - let follow = follow_target(&crate_name, &mut conn, user_id).await?; + let follow = follow_target(&path.name, &mut conn, user_id).await?; diesel::insert_into(follows::table) .values(&follow) .on_conflict_do_nothing() @@ -60,14 +56,10 @@ pub async fn follow_crate( tag = "crates", responses((status = 200, description = "Successful Response")), )] -pub async fn unfollow_crate( - app: AppState, - Path(crate_name): Path, - req: Parts, -) -> AppResult { +pub async fn unfollow_crate(app: AppState, path: CratePath, req: Parts) -> AppResult { let mut conn = app.db_write().await?; let user_id = AuthCheck::default().check(&req, &mut conn).await?.user_id(); - let follow = follow_target(&crate_name, &mut conn, user_id).await?; + let follow = follow_target(&path.name, &mut conn, user_id).await?; diesel::delete(&follow).execute(&mut conn).await?; ok_true() @@ -82,7 +74,7 @@ pub async fn unfollow_crate( )] pub async fn get_following_crate( app: AppState, - Path(crate_name): Path, + path: CratePath, req: Parts, ) -> AppResult { use diesel::dsl::exists; @@ -93,7 +85,7 @@ pub async fn get_following_crate( .await? .user_id(); - let follow = follow_target(&crate_name, &mut conn, user_id).await?; + let follow = follow_target(&path.name, &mut conn, user_id).await?; let following = diesel::select(exists(follows::table.find(follow.id()))) .get_result::(&mut conn) .await?; diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 7478ce8e044..0ff06907bdb 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -6,6 +6,8 @@ use crate::app::AppState; use crate::controllers::helpers::pagination::PaginationOptions; +use crate::controllers::krate::CratePath; +use crate::controllers::version::CrateVersionPath; use crate::models::{ Category, Crate, CrateCategory, CrateKeyword, CrateName, Keyword, RecentCrateDownloads, User, Version, VersionOwnerAction, @@ -16,7 +18,6 @@ use crate::util::{redirect, RequestUtils}; use crate::views::{ EncodableCategory, EncodableCrate, EncodableDependency, EncodableKeyword, EncodableVersion, }; -use axum::extract::Path; use axum::response::{IntoResponse, Response}; use axum_extra::json; use axum_extra::response::ErasedJson; @@ -37,7 +38,8 @@ use std::str::FromStr; responses((status = 200, description = "Successful Response")), )] pub async fn find_new_crate(app: AppState, req: Parts) -> AppResult { - find_crate(app, Path("new".to_string()), req).await + let name = "new".to_string(); + find_crate(app, CratePath { name }, req).await } /// Get crate metadata. @@ -47,11 +49,7 @@ pub async fn find_new_crate(app: AppState, req: Parts) -> AppResult tag = "crates", responses((status = 200, description = "Successful Response")), )] -pub async fn find_crate( - app: AppState, - Path(name): Path, - req: Parts, -) -> AppResult { +pub async fn find_crate(app: AppState, path: CratePath, req: Parts) -> AppResult { let mut conn = app.db_read().await?; let include = req @@ -62,7 +60,7 @@ pub async fn find_crate( .unwrap_or_default(); let (krate, downloads, default_version, yanked): (Crate, i64, Option, Option) = - Crate::by_name(&name) + Crate::by_name(&path.name) .inner_join(crate_downloads::table) .left_join(default_versions::table) .left_join(versions::table.on(default_versions::version_id.eq(versions::id))) @@ -75,7 +73,7 @@ pub async fn find_crate( .first(&mut conn) .await .optional()? - .ok_or_else(|| crate_not_found(&name))?; + .ok_or_else(|| crate_not_found(&path.name))?; let versions_publishers_and_audit_actions = if include.versions { let mut versions_and_publishers: Vec<(Version, Option)> = @@ -253,12 +251,8 @@ impl FromStr for ShowIncludeMode { tag = "versions", responses((status = 200, description = "Successful Response")), )] -pub async fn get_version_readme( - app: AppState, - Path((crate_name, version)): Path<(String, String)>, - req: Parts, -) -> Response { - let redirect_url = app.storage.readme_location(&crate_name, &version); +pub async fn get_version_readme(app: AppState, path: CrateVersionPath, req: Parts) -> Response { + let redirect_url = app.storage.readme_location(&path.name, &path.version); if req.wants_json() { json!({ "url": redirect_url }).into_response() } else { @@ -275,18 +269,18 @@ pub async fn get_version_readme( )] pub async fn list_reverse_dependencies( app: AppState, - Path(name): Path, + path: CratePath, req: Parts, ) -> AppResult { let mut conn = app.db_read().await?; let pagination_options = PaginationOptions::builder().gather(&req)?; - let krate: Crate = Crate::by_name(&name) + let krate: Crate = Crate::by_name(&path.name) .first(&mut conn) .await .optional()? - .ok_or_else(|| crate_not_found(&name))?; + .ok_or_else(|| crate_not_found(&path.name))?; let (rev_deps, total) = krate .reverse_dependencies(&mut conn, pagination_options) diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index e44ac539061..4a1cc0abf71 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -1,12 +1,12 @@ //! All routes related to managing owners of a crate +use crate::controllers::krate::CratePath; use crate::models::{krate::NewOwnerInvite, token::EndpointScope}; use crate::models::{Crate, Owner, Rights, Team, User}; use crate::util::errors::{bad_request, crate_not_found, custom, AppResult}; use crate::views::EncodableOwner; use crate::{app::AppState, models::krate::OwnerAddError}; use crate::{auth::AuthCheck, email::Email}; -use axum::extract::Path; use axum::Json; use axum_extra::json; use axum_extra::response::ErasedJson; @@ -24,14 +24,14 @@ use secrecy::{ExposeSecret, SecretString}; tag = "owners", responses((status = 200, description = "Successful Response")), )] -pub async fn list_owners(state: AppState, Path(crate_name): Path) -> AppResult { +pub async fn list_owners(state: AppState, path: CratePath) -> AppResult { let mut conn = state.db_read().await?; - let krate: Crate = Crate::by_name(&crate_name) + let krate: Crate = Crate::by_name(&path.name) .first(&mut conn) .await .optional()? - .ok_or_else(|| crate_not_found(&crate_name))?; + .ok_or_else(|| crate_not_found(&path.name))?; let owners = krate .owners(&mut conn) @@ -50,16 +50,13 @@ pub async fn list_owners(state: AppState, Path(crate_name): Path) -> App tag = "owners", responses((status = 200, description = "Successful Response")), )] -pub async fn get_team_owners( - state: AppState, - Path(crate_name): Path, -) -> AppResult { +pub async fn get_team_owners(state: AppState, path: CratePath) -> AppResult { let mut conn = state.db_read().await?; - let krate: Crate = Crate::by_name(&crate_name) + let krate: Crate = Crate::by_name(&path.name) .first(&mut conn) .await .optional()? - .ok_or_else(|| crate_not_found(&crate_name))?; + .ok_or_else(|| crate_not_found(&path.name))?; let owners = Team::owning(&krate, &mut conn) .await? @@ -77,17 +74,14 @@ pub async fn get_team_owners( tag = "owners", responses((status = 200, description = "Successful Response")), )] -pub async fn get_user_owners( - state: AppState, - Path(crate_name): Path, -) -> AppResult { +pub async fn get_user_owners(state: AppState, path: CratePath) -> AppResult { let mut conn = state.db_read().await?; - let krate: Crate = Crate::by_name(&crate_name) + let krate: Crate = Crate::by_name(&path.name) .first(&mut conn) .await .optional()? - .ok_or_else(|| crate_not_found(&crate_name))?; + .ok_or_else(|| crate_not_found(&path.name))?; let owners = User::owning(&krate, &mut conn) .await? @@ -107,11 +101,11 @@ pub async fn get_user_owners( )] pub async fn add_owners( app: AppState, - Path(crate_name): Path, + path: CratePath, parts: Parts, Json(body): Json, ) -> AppResult { - modify_owners(app, crate_name, parts, body, true).await + modify_owners(app, path.name, parts, body, true).await } /// Remove crate owners. @@ -123,11 +117,11 @@ pub async fn add_owners( )] pub async fn remove_owners( app: AppState, - Path(crate_name): Path, + path: CratePath, parts: Parts, Json(body): Json, ) -> AppResult { - modify_owners(app, crate_name, parts, body, false).await + modify_owners(app, path.name, parts, body, false).await } #[derive(Deserialize)] diff --git a/src/controllers/krate/versions.rs b/src/controllers/krate/versions.rs index 75748a22c89..efc035e0e12 100644 --- a/src/controllers/krate/versions.rs +++ b/src/controllers/krate/versions.rs @@ -1,6 +1,5 @@ //! Endpoint for versions of a crate -use axum::extract::Path; use axum_extra::json; use axum_extra::response::ErasedJson; use diesel::dsl::not; @@ -14,6 +13,7 @@ use std::str::FromStr; use crate::app::AppState; use crate::controllers::helpers::pagination::{encode_seek, Page, PaginationOptions}; +use crate::controllers::krate::CratePath; use crate::models::{Crate, User, Version, VersionOwnerAction}; use crate::schema::{crates, users, versions}; use crate::util::errors::{bad_request, crate_not_found, AppResult, BoxedAppError}; @@ -27,19 +27,15 @@ use crate::views::EncodableVersion; tag = "versions", responses((status = 200, description = "Successful Response")), )] -pub async fn list_versions( - state: AppState, - Path(crate_name): Path, - req: Parts, -) -> AppResult { +pub async fn list_versions(state: AppState, path: CratePath, req: Parts) -> AppResult { let mut conn = state.db_read().await?; - let crate_id: i32 = Crate::by_name(&crate_name) + let crate_id: i32 = Crate::by_name(&path.name) .select(crates::id) .first(&mut conn) .await .optional()? - .ok_or_else(|| crate_not_found(&crate_name))?; + .ok_or_else(|| crate_not_found(&path.name))?; let mut pagination = None; let params = req.query(); @@ -78,7 +74,7 @@ pub async fn list_versions( .data .into_iter() .zip(actions) - .map(|((v, pb), aas)| EncodableVersion::from(v, &crate_name, pb, aas)) + .map(|((v, pb), aas)| EncodableVersion::from(v, &path.name, pb, aas)) .collect::>(); Ok(match pagination { diff --git a/src/controllers/version.rs b/src/controllers/version.rs index 093365be37d..3bd7b6314b8 100644 --- a/src/controllers/version.rs +++ b/src/controllers/version.rs @@ -2,11 +2,21 @@ pub mod downloads; pub mod metadata; pub mod yank; +use axum::extract::{FromRequestParts, Path}; use diesel_async::AsyncPgConnection; use crate::models::{Crate, Version}; use crate::util::errors::{crate_not_found, AppResult}; +#[derive(Deserialize, FromRequestParts)] +#[from_request(via(Path))] +pub struct CrateVersionPath { + /// Name of the crate + pub name: String, + /// Version number + pub version: String, +} + async fn version_and_crate( conn: &mut AsyncPgConnection, crate_name: &str, diff --git a/src/controllers/version/downloads.rs b/src/controllers/version/downloads.rs index d1e9382df55..f50e376d5de 100644 --- a/src/controllers/version/downloads.rs +++ b/src/controllers/version/downloads.rs @@ -2,14 +2,13 @@ //! //! Crate level functionality is located in `krate::downloads`. -use super::version_and_crate; +use super::{version_and_crate, CrateVersionPath}; use crate::app::AppState; use crate::models::VersionDownload; use crate::schema::*; use crate::util::errors::{version_not_found, AppResult}; use crate::util::{redirect, RequestUtils}; use crate::views::EncodableVersionDownload; -use axum::extract::Path; use axum::response::{IntoResponse, Response}; use axum_extra::json; use axum_extra::response::ErasedJson; @@ -29,11 +28,11 @@ use http::request::Parts; )] pub async fn download_version( app: AppState, - Path((crate_name, version)): Path<(String, String)>, + path: CrateVersionPath, req: Parts, ) -> AppResult { let wants_json = req.wants_json(); - let redirect_url = app.storage.crate_location(&crate_name, &version); + let redirect_url = app.storage.crate_location(&path.name, &path.version); if wants_json { Ok(json!({ "url": redirect_url }).into_response()) } else { @@ -52,15 +51,15 @@ pub async fn download_version( )] pub async fn get_version_downloads( app: AppState, - Path((crate_name, version)): Path<(String, String)>, + path: CrateVersionPath, req: Parts, ) -> AppResult { - if semver::Version::parse(&version).is_err() { - return Err(version_not_found(&crate_name, &version)); + if semver::Version::parse(&path.version).is_err() { + return Err(version_not_found(&path.name, &path.version)); } let mut conn = app.db_read().await?; - let (version, _) = version_and_crate(&mut conn, &crate_name, &version).await?; + let (version, _) = version_and_crate(&mut conn, &path.name, &path.version).await?; let cutoff_end_date = req .query() diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index 09169dfd53b..123689357e0 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -4,7 +4,6 @@ //! index or cached metadata which was extracted (client side) from the //! `Cargo.toml` file. -use axum::extract::Path; use axum::Json; use axum_extra::json; use axum_extra::response::ErasedJson; @@ -28,7 +27,7 @@ use crate::util::errors::{bad_request, custom, version_not_found, AppResult}; use crate::views::{EncodableDependency, EncodableVersion}; use crate::worker::jobs::{SyncToGitIndex, SyncToSparseIndex, UpdateDefaultVersion}; -use super::version_and_crate; +use super::{version_and_crate, CrateVersionPath}; #[derive(Deserialize)] pub struct VersionUpdate { @@ -55,14 +54,14 @@ pub struct VersionUpdateRequest { )] pub async fn get_version_dependencies( state: AppState, - Path((crate_name, version)): Path<(String, String)>, + path: CrateVersionPath, ) -> AppResult { - if semver::Version::parse(&version).is_err() { - return Err(version_not_found(&crate_name, &version)); + if semver::Version::parse(&path.version).is_err() { + return Err(version_not_found(&path.name, &path.version)); } let mut conn = state.db_read().await?; - let (version, _) = version_and_crate(&mut conn, &crate_name, &version).await?; + let (version, _) = version_and_crate(&mut conn, &path.name, &path.version).await?; let deps = Dependency::belonging_to(&version) .inner_join(crates::table) @@ -102,16 +101,13 @@ pub async fn get_version_authors() -> ErasedJson { tag = "versions", responses((status = 200, description = "Successful Response")), )] -pub async fn find_version( - state: AppState, - Path((crate_name, version)): Path<(String, String)>, -) -> AppResult { - if semver::Version::parse(&version).is_err() { - return Err(version_not_found(&crate_name, &version)); +pub async fn find_version(state: AppState, path: CrateVersionPath) -> AppResult { + if semver::Version::parse(&path.version).is_err() { + return Err(version_not_found(&path.name, &path.version)); } let mut conn = state.db_read().await?; - let (version, krate) = version_and_crate(&mut conn, &crate_name, &version).await?; + let (version, krate) = version_and_crate(&mut conn, &path.name, &path.version).await?; let published_by = version.published_by(&mut conn).await?; let actions = VersionOwnerAction::by_version(&mut conn, &version).await?; @@ -130,16 +126,16 @@ pub async fn find_version( )] pub async fn update_version( state: AppState, - Path((crate_name, version)): Path<(String, String)>, + path: CrateVersionPath, req: Parts, Json(update_request): Json, ) -> AppResult { - if semver::Version::parse(&version).is_err() { - return Err(version_not_found(&crate_name, &version)); + if semver::Version::parse(&path.version).is_err() { + return Err(version_not_found(&path.name, &path.version)); } let mut conn = state.db_write().await?; - let (mut version, krate) = version_and_crate(&mut conn, &crate_name, &version).await?; + let (mut version, krate) = version_and_crate(&mut conn, &path.name, &path.version).await?; validate_yank_update(&update_request.version, &version)?; let auth = authenticate(&req, &mut conn, &krate.name).await?; diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 1f1d92efdb8..86b98ebbf7c 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -1,12 +1,11 @@ //! Endpoints for yanking and unyanking specific versions of crates use super::metadata::{authenticate, perform_version_yank_update}; -use super::version_and_crate; +use super::{version_and_crate, CrateVersionPath}; use crate::app::AppState; use crate::controllers::helpers::ok_true; use crate::rate_limiter::LimitedAction; use crate::util::errors::{version_not_found, AppResult}; -use axum::extract::Path; use axum::response::Response; use http::request::Parts; @@ -29,10 +28,10 @@ use http::request::Parts; )] pub async fn yank_version( app: AppState, - Path((crate_name, version)): Path<(String, String)>, + path: CrateVersionPath, req: Parts, ) -> AppResult { - modify_yank(crate_name, version, app, req, true).await + modify_yank(path, app, req, true).await } /// Unyank a crate version. @@ -44,16 +43,15 @@ pub async fn yank_version( )] pub async fn unyank_version( app: AppState, - Path((crate_name, version)): Path<(String, String)>, + path: CrateVersionPath, req: Parts, ) -> AppResult { - modify_yank(crate_name, version, app, req, false).await + modify_yank(path, app, req, false).await } /// Changes `yanked` flag on a crate version record async fn modify_yank( - crate_name: String, - version: String, + path: CrateVersionPath, state: AppState, req: Parts, yanked: bool, @@ -61,13 +59,13 @@ async fn modify_yank( // FIXME: Should reject bad requests before authentication, but can't due to // lifetime issues with `req`. - if semver::Version::parse(&version).is_err() { - return Err(version_not_found(&crate_name, &version)); + if semver::Version::parse(&path.version).is_err() { + return Err(version_not_found(&path.name, &path.version)); } let mut conn = state.db_write().await?; - let (mut version, krate) = version_and_crate(&mut conn, &crate_name, &version).await?; - let auth = authenticate(&req, &mut conn, &crate_name).await?; + let (mut version, krate) = version_and_crate(&mut conn, &path.name, &path.version).await?; + let auth = authenticate(&req, &mut conn, &krate.name).await?; state .rate_limiter From 5b7ae939b045482a3810bb59867555bcec1918fd Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 15 Dec 2024 13:15:01 +0100 Subject: [PATCH 2/7] controllers/version: Extract `CrateVersionPath::load_version_and_crate()` fn --- src/controllers/version.rs | 9 +++++++++ src/controllers/version/downloads.rs | 4 ++-- src/controllers/version/metadata.rs | 8 ++++---- src/controllers/version/yank.rs | 4 ++-- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/controllers/version.rs b/src/controllers/version.rs index 3bd7b6314b8..597f07aff0b 100644 --- a/src/controllers/version.rs +++ b/src/controllers/version.rs @@ -17,6 +17,15 @@ pub struct CrateVersionPath { pub version: String, } +impl CrateVersionPath { + pub async fn load_version_and_crate( + &self, + conn: &mut AsyncPgConnection, + ) -> AppResult<(Version, Crate)> { + version_and_crate(conn, &self.name, &self.version).await + } +} + async fn version_and_crate( conn: &mut AsyncPgConnection, crate_name: &str, diff --git a/src/controllers/version/downloads.rs b/src/controllers/version/downloads.rs index f50e376d5de..65db440a41b 100644 --- a/src/controllers/version/downloads.rs +++ b/src/controllers/version/downloads.rs @@ -2,7 +2,7 @@ //! //! Crate level functionality is located in `krate::downloads`. -use super::{version_and_crate, CrateVersionPath}; +use super::CrateVersionPath; use crate::app::AppState; use crate::models::VersionDownload; use crate::schema::*; @@ -59,7 +59,7 @@ pub async fn get_version_downloads( } let mut conn = app.db_read().await?; - let (version, _) = version_and_crate(&mut conn, &path.name, &path.version).await?; + let (version, _) = path.load_version_and_crate(&mut conn).await?; let cutoff_end_date = req .query() diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index 123689357e0..4c2dbe4168a 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -27,7 +27,7 @@ use crate::util::errors::{bad_request, custom, version_not_found, AppResult}; use crate::views::{EncodableDependency, EncodableVersion}; use crate::worker::jobs::{SyncToGitIndex, SyncToSparseIndex, UpdateDefaultVersion}; -use super::{version_and_crate, CrateVersionPath}; +use super::CrateVersionPath; #[derive(Deserialize)] pub struct VersionUpdate { @@ -61,7 +61,7 @@ pub async fn get_version_dependencies( } let mut conn = state.db_read().await?; - let (version, _) = version_and_crate(&mut conn, &path.name, &path.version).await?; + let (version, _) = path.load_version_and_crate(&mut conn).await?; let deps = Dependency::belonging_to(&version) .inner_join(crates::table) @@ -107,7 +107,7 @@ pub async fn find_version(state: AppState, path: CrateVersionPath) -> AppResult< } let mut conn = state.db_read().await?; - let (version, krate) = version_and_crate(&mut conn, &path.name, &path.version).await?; + let (version, krate) = path.load_version_and_crate(&mut conn).await?; let published_by = version.published_by(&mut conn).await?; let actions = VersionOwnerAction::by_version(&mut conn, &version).await?; @@ -135,7 +135,7 @@ pub async fn update_version( } let mut conn = state.db_write().await?; - let (mut version, krate) = version_and_crate(&mut conn, &path.name, &path.version).await?; + let (mut version, krate) = path.load_version_and_crate(&mut conn).await?; validate_yank_update(&update_request.version, &version)?; let auth = authenticate(&req, &mut conn, &krate.name).await?; diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 86b98ebbf7c..4f3e634a4f8 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -1,7 +1,7 @@ //! Endpoints for yanking and unyanking specific versions of crates use super::metadata::{authenticate, perform_version_yank_update}; -use super::{version_and_crate, CrateVersionPath}; +use super::CrateVersionPath; use crate::app::AppState; use crate::controllers::helpers::ok_true; use crate::rate_limiter::LimitedAction; @@ -64,7 +64,7 @@ async fn modify_yank( } let mut conn = state.db_write().await?; - let (mut version, krate) = version_and_crate(&mut conn, &path.name, &path.version).await?; + let (mut version, krate) = path.load_version_and_crate(&mut conn).await?; let auth = authenticate(&req, &mut conn, &krate.name).await?; state From a1dde8b0cf1dea3900d19c539cb1509fb3625860 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 15 Dec 2024 13:16:59 +0100 Subject: [PATCH 3/7] controllers/version: Extract `CrateVersionPath::load_version()` fn --- src/controllers/version.rs | 4 ++++ src/controllers/version/downloads.rs | 2 +- src/controllers/version/metadata.rs | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/controllers/version.rs b/src/controllers/version.rs index 597f07aff0b..0c7a19f3d24 100644 --- a/src/controllers/version.rs +++ b/src/controllers/version.rs @@ -18,6 +18,10 @@ pub struct CrateVersionPath { } impl CrateVersionPath { + pub async fn load_version(&self, conn: &mut AsyncPgConnection) -> AppResult { + Ok(self.load_version_and_crate(conn).await?.0) + } + pub async fn load_version_and_crate( &self, conn: &mut AsyncPgConnection, diff --git a/src/controllers/version/downloads.rs b/src/controllers/version/downloads.rs index 65db440a41b..b5de37806b8 100644 --- a/src/controllers/version/downloads.rs +++ b/src/controllers/version/downloads.rs @@ -59,7 +59,7 @@ pub async fn get_version_downloads( } let mut conn = app.db_read().await?; - let (version, _) = path.load_version_and_crate(&mut conn).await?; + let version = path.load_version(&mut conn).await?; let cutoff_end_date = req .query() diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index 4c2dbe4168a..4874b935bbb 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -61,7 +61,7 @@ pub async fn get_version_dependencies( } let mut conn = state.db_read().await?; - let (version, _) = path.load_version_and_crate(&mut conn).await?; + let version = path.load_version(&mut conn).await?; let deps = Dependency::belonging_to(&version) .inner_join(crates::table) From 428dae5f7827ca88f87ba2c0ffcc75f8950e0695 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 15 Dec 2024 13:42:35 +0100 Subject: [PATCH 4/7] controllers/krate: Extract `CratePath::load_crate()` and `load_crate_id()` fns --- src/controllers/krate.rs | 32 ++++++++++++++++++++++++++++++ src/controllers/krate/delete.rs | 11 +++------- src/controllers/krate/downloads.rs | 13 ++++-------- src/controllers/krate/metadata.rs | 6 +----- src/controllers/krate/owners.rs | 18 +++-------------- src/controllers/krate/versions.rs | 13 ++++-------- 6 files changed, 47 insertions(+), 46 deletions(-) diff --git a/src/controllers/krate.rs b/src/controllers/krate.rs index 041e3fde67a..60ad2c23269 100644 --- a/src/controllers/krate.rs +++ b/src/controllers/krate.rs @@ -1,4 +1,9 @@ +use crate::models::Crate; +use crate::util::errors::{crate_not_found, AppResult}; use axum::extract::{FromRequestParts, Path}; +use crates_io_database::schema::crates; +use diesel::{OptionalExtension, QueryDsl}; +use diesel_async::{AsyncPgConnection, RunQueryDsl}; pub mod delete; pub mod downloads; @@ -15,3 +20,30 @@ pub struct CratePath { /// Name of the crate pub name: String, } + +impl CratePath { + pub async fn load_crate(&self, conn: &mut AsyncPgConnection) -> AppResult { + load_crate(conn, &self.name).await + } + + pub async fn load_crate_id(&self, conn: &mut AsyncPgConnection) -> AppResult { + load_crate_id(conn, &self.name).await + } +} + +pub async fn load_crate(conn: &mut AsyncPgConnection, name: &str) -> AppResult { + Crate::by_name(name) + .first(conn) + .await + .optional()? + .ok_or_else(|| crate_not_found(name)) +} + +pub async fn load_crate_id(conn: &mut AsyncPgConnection, name: &str) -> AppResult { + Crate::by_name(name) + .select(crates::id) + .first(conn) + .await + .optional()? + .ok_or_else(|| crate_not_found(name)) +} diff --git a/src/controllers/krate/delete.rs b/src/controllers/krate/delete.rs index 5f99ec2bbd4..7bec18bd98c 100644 --- a/src/controllers/krate/delete.rs +++ b/src/controllers/krate/delete.rs @@ -1,9 +1,9 @@ use crate::app::AppState; use crate::auth::AuthCheck; use crate::controllers::krate::CratePath; -use crate::models::{Crate, NewDeletedCrate, Rights}; +use crate::models::{NewDeletedCrate, Rights}; use crate::schema::{crate_downloads, crates, dependencies}; -use crate::util::errors::{crate_not_found, custom, AppResult, BoxedAppError}; +use crate::util::errors::{custom, AppResult, BoxedAppError}; use crate::worker::jobs; use bigdecimal::ToPrimitive; use chrono::{TimeDelta, Utc}; @@ -40,8 +40,7 @@ pub async fn delete_crate(path: CratePath, parts: Parts, app: AppState) -> AppRe let auth = AuthCheck::only_cookie().check(&parts, &mut conn).await?; // Check that the crate exists - let krate = find_crate(&mut conn, &path.name).await?; - let krate = krate.ok_or_else(|| crate_not_found(&path.name))?; + let krate = path.load_crate(&mut conn).await?; // Check that the user is an owner of the crate (team owners are not allowed to delete crates) let user = auth.user(); @@ -119,10 +118,6 @@ pub async fn delete_crate(path: CratePath, parts: Parts, app: AppState) -> AppRe Ok(StatusCode::NO_CONTENT) } -async fn find_crate(conn: &mut AsyncPgConnection, name: &str) -> QueryResult> { - Crate::by_name(name).first(conn).await.optional() -} - async fn get_crate_downloads(conn: &mut AsyncPgConnection, crate_id: i32) -> QueryResult { let downloads = crate_downloads::table .find(crate_id) diff --git a/src/controllers/krate/downloads.rs b/src/controllers/krate/downloads.rs index aa377bb007f..3516eea8385 100644 --- a/src/controllers/krate/downloads.rs +++ b/src/controllers/krate/downloads.rs @@ -5,10 +5,10 @@ use crate::app::AppState; use crate::controllers::krate::CratePath; -use crate::models::{Crate, Version, VersionDownload}; -use crate::schema::{crates, version_downloads, versions}; +use crate::models::{Version, VersionDownload}; +use crate::schema::{version_downloads, versions}; use crate::sql::to_char; -use crate::util::errors::{crate_not_found, AppResult}; +use crate::util::errors::AppResult; use crate::views::EncodableVersionDownload; use axum_extra::json; use axum_extra::response::ErasedJson; @@ -33,12 +33,7 @@ pub async fn get_crate_downloads(state: AppState, path: CratePath) -> AppResult< use diesel::dsl::*; use diesel::sql_types::BigInt; - let crate_id: i32 = Crate::by_name(&path.name) - .select(crates::id) - .first(&mut conn) - .await - .optional()? - .ok_or_else(|| crate_not_found(&path.name))?; + let crate_id: i32 = path.load_crate_id(&mut conn).await?; let mut versions: Vec = versions::table .filter(versions::crate_id.eq(crate_id)) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 0ff06907bdb..e58325cfe38 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -276,11 +276,7 @@ pub async fn list_reverse_dependencies( let pagination_options = PaginationOptions::builder().gather(&req)?; - let krate: Crate = Crate::by_name(&path.name) - .first(&mut conn) - .await - .optional()? - .ok_or_else(|| crate_not_found(&path.name))?; + let krate = path.load_crate(&mut conn).await?; let (rev_deps, total) = krate .reverse_dependencies(&mut conn, pagination_options) diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 4a1cc0abf71..62550dcaba8 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -27,11 +27,7 @@ use secrecy::{ExposeSecret, SecretString}; pub async fn list_owners(state: AppState, path: CratePath) -> AppResult { let mut conn = state.db_read().await?; - let krate: Crate = Crate::by_name(&path.name) - .first(&mut conn) - .await - .optional()? - .ok_or_else(|| crate_not_found(&path.name))?; + let krate = path.load_crate(&mut conn).await?; let owners = krate .owners(&mut conn) @@ -52,11 +48,7 @@ pub async fn list_owners(state: AppState, path: CratePath) -> AppResult AppResult { let mut conn = state.db_read().await?; - let krate: Crate = Crate::by_name(&path.name) - .first(&mut conn) - .await - .optional()? - .ok_or_else(|| crate_not_found(&path.name))?; + let krate = path.load_crate(&mut conn).await?; let owners = Team::owning(&krate, &mut conn) .await? @@ -77,11 +69,7 @@ pub async fn get_team_owners(state: AppState, path: CratePath) -> AppResult AppResult { let mut conn = state.db_read().await?; - let krate: Crate = Crate::by_name(&path.name) - .first(&mut conn) - .await - .optional()? - .ok_or_else(|| crate_not_found(&path.name))?; + let krate = path.load_crate(&mut conn).await?; let owners = User::owning(&krate, &mut conn) .await? diff --git a/src/controllers/krate/versions.rs b/src/controllers/krate/versions.rs index efc035e0e12..d02b744855b 100644 --- a/src/controllers/krate/versions.rs +++ b/src/controllers/krate/versions.rs @@ -14,9 +14,9 @@ use std::str::FromStr; use crate::app::AppState; use crate::controllers::helpers::pagination::{encode_seek, Page, PaginationOptions}; use crate::controllers::krate::CratePath; -use crate::models::{Crate, User, Version, VersionOwnerAction}; -use crate::schema::{crates, users, versions}; -use crate::util::errors::{bad_request, crate_not_found, AppResult, BoxedAppError}; +use crate::models::{User, Version, VersionOwnerAction}; +use crate::schema::{users, versions}; +use crate::util::errors::{bad_request, AppResult, BoxedAppError}; use crate::util::RequestUtils; use crate::views::EncodableVersion; @@ -30,12 +30,7 @@ use crate::views::EncodableVersion; pub async fn list_versions(state: AppState, path: CratePath, req: Parts) -> AppResult { let mut conn = state.db_read().await?; - let crate_id: i32 = Crate::by_name(&path.name) - .select(crates::id) - .first(&mut conn) - .await - .optional()? - .ok_or_else(|| crate_not_found(&path.name))?; + let crate_id = path.load_crate_id(&mut conn).await?; let mut pagination = None; let params = req.query(); From bb90b394f75213f87444ca4cb48e7cdb6fc25229 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 15 Dec 2024 12:45:04 +0100 Subject: [PATCH 5/7] utoipa: Use `CratePath` and `CrateVersionPath` as params descriptions --- src/controllers/krate.rs | 4 +- src/controllers/krate/delete.rs | 1 + src/controllers/krate/downloads.rs | 1 + src/controllers/krate/follow.rs | 3 + src/controllers/krate/metadata.rs | 3 + src/controllers/krate/owners.rs | 5 + src/controllers/krate/versions.rs | 1 + src/controllers/version.rs | 5 +- src/controllers/version/downloads.rs | 2 + src/controllers/version/metadata.rs | 4 + src/controllers/version/yank.rs | 2 + ..._io__openapi__tests__openapi_snapshot.snap | 332 ++++++++++++++++++ 12 files changed, 361 insertions(+), 2 deletions(-) diff --git a/src/controllers/krate.rs b/src/controllers/krate.rs index 60ad2c23269..cfdfadee18b 100644 --- a/src/controllers/krate.rs +++ b/src/controllers/krate.rs @@ -4,6 +4,7 @@ use axum::extract::{FromRequestParts, Path}; use crates_io_database::schema::crates; use diesel::{OptionalExtension, QueryDsl}; use diesel_async::{AsyncPgConnection, RunQueryDsl}; +use utoipa::IntoParams; pub mod delete; pub mod downloads; @@ -14,7 +15,8 @@ pub mod publish; pub mod search; pub mod versions; -#[derive(Deserialize, FromRequestParts)] +#[derive(Deserialize, FromRequestParts, IntoParams)] +#[into_params(parameter_in = Path)] #[from_request(via(Path))] pub struct CratePath { /// Name of the crate diff --git a/src/controllers/krate/delete.rs b/src/controllers/krate/delete.rs index 7bec18bd98c..994f8d4a146 100644 --- a/src/controllers/krate/delete.rs +++ b/src/controllers/krate/delete.rs @@ -30,6 +30,7 @@ const AVAILABLE_AFTER: TimeDelta = TimeDelta::hours(24); #[utoipa::path( delete, path = "/api/v1/crates/{name}", + params(CratePath), tag = "crates", responses((status = 200, description = "Successful Response")), )] diff --git a/src/controllers/krate/downloads.rs b/src/controllers/krate/downloads.rs index 3516eea8385..fdeea81d0e5 100644 --- a/src/controllers/krate/downloads.rs +++ b/src/controllers/krate/downloads.rs @@ -23,6 +23,7 @@ use std::cmp; #[utoipa::path( get, path = "/api/v1/crates/{name}/downloads", + params(CratePath), tag = "crates", responses((status = 200, description = "Successful Response")), )] diff --git a/src/controllers/krate/follow.rs b/src/controllers/krate/follow.rs index a8509c7af55..d1c91dfaada 100644 --- a/src/controllers/krate/follow.rs +++ b/src/controllers/krate/follow.rs @@ -33,6 +33,7 @@ async fn follow_target( #[utoipa::path( put, path = "/api/v1/crates/{name}/follow", + params(CratePath), tag = "crates", responses((status = 200, description = "Successful Response")), )] @@ -53,6 +54,7 @@ pub async fn follow_crate(app: AppState, path: CratePath, req: Parts) -> AppResu #[utoipa::path( delete, path = "/api/v1/crates/{name}/follow", + params(CratePath), tag = "crates", responses((status = 200, description = "Successful Response")), )] @@ -69,6 +71,7 @@ pub async fn unfollow_crate(app: AppState, path: CratePath, req: Parts) -> AppRe #[utoipa::path( get, path = "/api/v1/crates/{name}/following", + params(CratePath), tag = "crates", responses((status = 200, description = "Successful Response")), )] diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index e58325cfe38..6321b6e609d 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -46,6 +46,7 @@ pub async fn find_new_crate(app: AppState, req: Parts) -> AppResult #[utoipa::path( get, path = "/api/v1/crates/{name}", + params(CratePath), tag = "crates", responses((status = 200, description = "Successful Response")), )] @@ -248,6 +249,7 @@ impl FromStr for ShowIncludeMode { #[utoipa::path( get, path = "/api/v1/crates/{name}/{version}/readme", + params(CrateVersionPath), tag = "versions", responses((status = 200, description = "Successful Response")), )] @@ -264,6 +266,7 @@ pub async fn get_version_readme(app: AppState, path: CrateVersionPath, req: Part #[utoipa::path( get, path = "/api/v1/crates/{name}/reverse_dependencies", + params(CratePath), tag = "crates", responses((status = 200, description = "Successful Response")), )] diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs index 62550dcaba8..0dd02ef738b 100644 --- a/src/controllers/krate/owners.rs +++ b/src/controllers/krate/owners.rs @@ -21,6 +21,7 @@ use secrecy::{ExposeSecret, SecretString}; #[utoipa::path( get, path = "/api/v1/crates/{name}/owners", + params(CratePath), tag = "owners", responses((status = 200, description = "Successful Response")), )] @@ -43,6 +44,7 @@ pub async fn list_owners(state: AppState, path: CratePath) -> AppResult AppResult AppResult ErasedJson { #[utoipa::path( get, path = "/api/v1/crates/{name}/{version}", + params(CrateVersionPath), tag = "versions", responses((status = 200, description = "Successful Response")), )] @@ -121,6 +124,7 @@ pub async fn find_version(state: AppState, path: CrateVersionPath) -> AppResult< #[utoipa::path( patch, path = "/api/v1/crates/{name}/{version}", + params(CrateVersionPath), tag = "versions", responses((status = 200, description = "Successful Response")), )] diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 4f3e634a4f8..666a544693c 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -23,6 +23,7 @@ use http::request::Parts; #[utoipa::path( delete, path = "/api/v1/crates/{name}/{version}/yank", + params(CrateVersionPath), tag = "versions", responses((status = 200, description = "Successful Response")), )] @@ -38,6 +39,7 @@ pub async fn yank_version( #[utoipa::path( put, path = "/api/v1/crates/{name}/{version}/unyank", + params(CrateVersionPath), tag = "versions", responses((status = 200, description = "Successful Response")), )] diff --git a/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap b/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap index d78d2c940b4..a0898ec0232 100644 --- a/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap +++ b/src/snapshots/crates_io__openapi__tests__openapi_snapshot.snap @@ -181,6 +181,17 @@ snapshot_kind: text "delete": { "description": "The crate is immediately deleted from the database, and with a small delay\nfrom the git and sparse index, and the crate file storage.\n\nThe crate can only be deleted by the owner of the crate, and only if the\ncrate has been published for less than 72 hours, or if the crate has a\nsingle owner, has been downloaded less than 100 times for each month it has\nbeen published, and is not depended upon by any other crate on crates.io.", "operationId": "delete_crate", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -193,6 +204,17 @@ snapshot_kind: text }, "get": { "operationId": "find_crate", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -208,6 +230,17 @@ snapshot_kind: text "get": { "description": "This includes the per-day downloads for the last 90 days and for the\nlatest 5 versions plus the sum of the rest.", "operationId": "get_crate_downloads", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -222,6 +255,17 @@ snapshot_kind: text "/api/v1/crates/{name}/follow": { "delete": { "operationId": "unfollow_crate", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -234,6 +278,17 @@ snapshot_kind: text }, "put": { "operationId": "follow_crate", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -248,6 +303,17 @@ snapshot_kind: text "/api/v1/crates/{name}/following": { "get": { "operationId": "get_following_crate", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -262,6 +328,17 @@ snapshot_kind: text "/api/v1/crates/{name}/owner_team": { "get": { "operationId": "get_team_owners", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -276,6 +353,17 @@ snapshot_kind: text "/api/v1/crates/{name}/owner_user": { "get": { "operationId": "get_user_owners", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -290,6 +378,17 @@ snapshot_kind: text "/api/v1/crates/{name}/owners": { "delete": { "operationId": "remove_owners", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -302,6 +401,17 @@ snapshot_kind: text }, "get": { "operationId": "list_owners", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -314,6 +424,17 @@ snapshot_kind: text }, "put": { "operationId": "add_owners", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -328,6 +449,17 @@ snapshot_kind: text "/api/v1/crates/{name}/reverse_dependencies": { "get": { "operationId": "list_reverse_dependencies", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -342,6 +474,17 @@ snapshot_kind: text "/api/v1/crates/{name}/versions": { "get": { "operationId": "list_versions", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -356,6 +499,27 @@ snapshot_kind: text "/api/v1/crates/{name}/{version}": { "get": { "operationId": "find_version", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -369,6 +533,27 @@ snapshot_kind: text "patch": { "description": "This endpoint allows updating the `yanked` state of a version, including a yank message.", "operationId": "update_version", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -385,6 +570,27 @@ snapshot_kind: text "deprecated": true, "description": "This endpoint was deprecated by [RFC #3052](https://github.com/rust-lang/rfcs/pull/3052)\nand returns an empty list for backwards compatibility reasons.", "operationId": "get_version_authors", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -400,6 +606,27 @@ snapshot_kind: text "get": { "description": "This information can also be obtained directly from the index.\n\nIn addition to returning cached data from the index, this returns\nfields for `id`, `version_id`, and `downloads` (which appears to always\nbe 0)", "operationId": "get_version_dependencies", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -415,6 +642,27 @@ snapshot_kind: text "get": { "description": "This returns a URL to the location where the crate is stored.", "operationId": "download_version", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -430,6 +678,27 @@ snapshot_kind: text "get": { "description": "This includes the per-day downloads for the last 90 days.", "operationId": "get_version_downloads", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -444,6 +713,27 @@ snapshot_kind: text "/api/v1/crates/{name}/{version}/readme": { "get": { "operationId": "get_version_readme", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -458,6 +748,27 @@ snapshot_kind: text "/api/v1/crates/{name}/{version}/unyank": { "put": { "operationId": "unyank_version", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" @@ -473,6 +784,27 @@ snapshot_kind: text "delete": { "description": "This does not delete a crate version, it makes the crate\nversion accessible only to crates that already have a\n`Cargo.lock` containing this version.\n\nNotes:\n\nVersion deletion is not implemented to avoid breaking builds,\nand the goal of yanking a crate is to prevent crates\nbeginning to depend on the yanked crate version.", "operationId": "yank_version", + "parameters": [ + { + "description": "Name of the crate", + "in": "path", + "name": "name", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "Version number", + "example": "1.0.0", + "in": "path", + "name": "version", + "required": true, + "schema": { + "type": "string" + } + } + ], "responses": { "200": { "description": "Successful Response" From 1e6f1c01071270f74966fea6754228978c5b2b3b Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 15 Dec 2024 14:21:36 +0100 Subject: [PATCH 6/7] controllers/version: Use custom deserializer to validate `version` field --- src/controllers/version.rs | 9 +++++++++ src/controllers/version/downloads.rs | 6 +----- src/controllers/version/metadata.rs | 14 +------------- src/controllers/version/yank.rs | 6 +----- src/tests/routes/crates/downloads.rs | 4 ++-- 5 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/controllers/version.rs b/src/controllers/version.rs index 26c257651ed..e6b3d06b231 100644 --- a/src/controllers/version.rs +++ b/src/controllers/version.rs @@ -4,6 +4,8 @@ pub mod yank; use axum::extract::{FromRequestParts, Path}; use diesel_async::AsyncPgConnection; +use serde::de::Error; +use serde::{Deserialize, Deserializer}; use utoipa::IntoParams; use crate::models::{Crate, Version}; @@ -17,6 +19,7 @@ pub struct CrateVersionPath { pub name: String, /// Version number #[param(example = "1.0.0")] + #[serde(deserialize_with = "deserialize_version")] pub version: String, } @@ -51,3 +54,9 @@ async fn version_and_crate( Ok((version, krate)) } + +fn deserialize_version<'de, D: Deserializer<'de>>(deserializer: D) -> Result { + let s = String::deserialize(deserializer)?; + let _ = semver::Version::parse(&s).map_err(Error::custom)?; + Ok(s) +} diff --git a/src/controllers/version/downloads.rs b/src/controllers/version/downloads.rs index 0d075c5fa8e..cc2fab59407 100644 --- a/src/controllers/version/downloads.rs +++ b/src/controllers/version/downloads.rs @@ -6,7 +6,7 @@ use super::CrateVersionPath; use crate::app::AppState; use crate::models::VersionDownload; use crate::schema::*; -use crate::util::errors::{version_not_found, AppResult}; +use crate::util::errors::AppResult; use crate::util::{redirect, RequestUtils}; use crate::views::EncodableVersionDownload; use axum::response::{IntoResponse, Response}; @@ -56,10 +56,6 @@ pub async fn get_version_downloads( path: CrateVersionPath, req: Parts, ) -> AppResult { - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = app.db_read().await?; let version = path.load_version(&mut conn).await?; diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index f13b5df63a9..9cd541ec8a8 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -23,7 +23,7 @@ use crate::models::{ }; use crate::rate_limiter::LimitedAction; use crate::schema::versions; -use crate::util::errors::{bad_request, custom, version_not_found, AppResult}; +use crate::util::errors::{bad_request, custom, AppResult}; use crate::views::{EncodableDependency, EncodableVersion}; use crate::worker::jobs::{SyncToGitIndex, SyncToSparseIndex, UpdateDefaultVersion}; @@ -57,10 +57,6 @@ pub async fn get_version_dependencies( state: AppState, path: CrateVersionPath, ) -> AppResult { - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = state.db_read().await?; let version = path.load_version(&mut conn).await?; @@ -105,10 +101,6 @@ pub async fn get_version_authors() -> ErasedJson { responses((status = 200, description = "Successful Response")), )] pub async fn find_version(state: AppState, path: CrateVersionPath) -> AppResult { - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = state.db_read().await?; let (version, krate) = path.load_version_and_crate(&mut conn).await?; let published_by = version.published_by(&mut conn).await?; @@ -134,10 +126,6 @@ pub async fn update_version( req: Parts, Json(update_request): Json, ) -> AppResult { - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = state.db_write().await?; let (mut version, krate) = path.load_version_and_crate(&mut conn).await?; validate_yank_update(&update_request.version, &version)?; diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 666a544693c..4e69ef458bb 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -5,7 +5,7 @@ use super::CrateVersionPath; use crate::app::AppState; use crate::controllers::helpers::ok_true; use crate::rate_limiter::LimitedAction; -use crate::util::errors::{version_not_found, AppResult}; +use crate::util::errors::AppResult; use axum::response::Response; use http::request::Parts; @@ -61,10 +61,6 @@ async fn modify_yank( // FIXME: Should reject bad requests before authentication, but can't due to // lifetime issues with `req`. - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = state.db_write().await?; let (mut version, krate) = path.load_version_and_crate(&mut conn).await?; let auth = authenticate(&req, &mut conn, &krate.name).await?; diff --git a/src/tests/routes/crates/downloads.rs b/src/tests/routes/crates/downloads.rs index 057f551c675..b7b9bdc6a2a 100644 --- a/src/tests/routes/crates/downloads.rs +++ b/src/tests/routes/crates/downloads.rs @@ -209,9 +209,9 @@ async fn test_version_downloads() { let response = anon .get::<()>("/api/v1/crates/foo/invalid-version/downloads") .await; - assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!( response.text(), - @r#"{"errors":[{"detail":"crate `foo` does not have a version `invalid-version`"}]}"# + @r#"{"errors":[{"detail":"Invalid URL: unexpected character 'i' while parsing major version number"}]}"# ); } From 37ae5920b2c6427f1960e549139dd5cfaf26dad1 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 15 Dec 2024 14:29:32 +0100 Subject: [PATCH 7/7] controllers/version: Reuse `load_crate()` fn --- src/controllers/version.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/controllers/version.rs b/src/controllers/version.rs index e6b3d06b231..78921fb37b3 100644 --- a/src/controllers/version.rs +++ b/src/controllers/version.rs @@ -8,8 +8,9 @@ use serde::de::Error; use serde::{Deserialize, Deserializer}; use utoipa::IntoParams; +use crate::controllers::krate::load_crate; use crate::models::{Crate, Version}; -use crate::util::errors::{crate_not_found, AppResult}; +use crate::util::errors::AppResult; #[derive(Deserialize, FromRequestParts, IntoParams)] #[into_params(parameter_in = Path)] @@ -41,15 +42,7 @@ async fn version_and_crate( crate_name: &str, semver: &str, ) -> AppResult<(Version, Crate)> { - use diesel::prelude::*; - use diesel_async::RunQueryDsl; - - let krate: Crate = Crate::by_name(crate_name) - .first(conn) - .await - .optional()? - .ok_or_else(|| crate_not_found(crate_name))?; - + let krate = load_crate(conn, crate_name).await?; let version = krate.find_version(conn, semver).await?; Ok((version, krate))