From 406616d4f5f5e242898d46298a1df8a081f59d8a Mon Sep 17 00:00:00 2001 From: Rustin170506 <29879298+Rustin170506@users.noreply.github.com> Date: Fri, 6 Sep 2024 23:05:05 +0800 Subject: [PATCH] Add `PATCH /crates/:crate/:version` route Signed-off-by: Rustin170506 <29879298+Rustin170506@users.noreply.github.com> --- src/controllers/version/metadata.rs | 159 +++++++++++++++++++++++++++- src/controllers/version/yank.rs | 69 +----------- src/router.rs | 2 +- 3 files changed, 163 insertions(+), 67 deletions(-) diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index 1588648b978..42698774996 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -6,17 +6,37 @@ use axum::extract::Path; use axum::Json; +use crates_io_worker::BackgroundJob; +use diesel::{ExpressionMethods, QueryDsl, RunQueryDsl}; use diesel_async::async_connection_wrapper::AsyncConnectionWrapper; +use http::request::Parts; +use http::StatusCode; +use serde::Deserialize; use serde_json::Value; +use tokio::runtime::Handle; use crate::app::AppState; -use crate::models::VersionOwnerAction; +use crate::auth::AuthCheck; +use crate::models::token::EndpointScope; +use crate::models::{ + insert_version_owner_action, Crate, Rights, Version, VersionAction, VersionOwnerAction, +}; +use crate::rate_limiter::LimitedAction; +use crate::schema::versions; use crate::tasks::spawn_blocking; -use crate::util::errors::{version_not_found, AppResult}; +use crate::util::diesel::Conn; +use crate::util::errors::{bad_request, custom, version_not_found, AppResult}; use crate::views::{EncodableDependency, EncodableVersion}; +use crate::worker::jobs::{self, UpdateDefaultVersion}; use super::version_and_crate; +#[derive(Deserialize)] +pub struct VersionUpdate { + yanked: Option, + yank_message: Option, +} + /// Handles the `GET /crates/:crate_id/:version/dependencies` route. /// /// This information can be obtained directly from the index. @@ -84,3 +104,138 @@ pub async fn show( }) .await } + +/// Handles the `PATCH /crates/:crate/:version` route. +/// +/// This endpoint allows updating the yanked state of a version, including a yank message. +pub async fn update( + state: AppState, + Path((crate_name, version)): Path<(String, String)>, + req: Parts, + Json(update_data): Json, +) -> AppResult> { + if semver::Version::parse(&version).is_err() { + return Err(version_not_found(&crate_name, &version)); + } + + let conn = state.db_write().await?; + spawn_blocking(move || { + let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); + let (mut version, krate) = version_and_crate(conn, &crate_name, &version)?; + + validate_yank_update(&update_data, &version)?; + apply_yank_update(&mut version, &update_data); + perform_version_yank_update(&state, &req, conn, &version, &krate)?; + + let published_by = version.published_by(conn); + let actions = VersionOwnerAction::by_version(conn, &version)?; + let updated_version = EncodableVersion::from(version, &krate.name, published_by, actions); + Ok(Json(json!({ "version": updated_version }))) + }) + .await +} + +fn validate_yank_update(update_data: &VersionUpdate, version: &Version) -> AppResult<()> { + match (update_data.yanked, &update_data.yank_message) { + (Some(false), Some(_)) => { + return Err(bad_request("Cannot set yank message when unyanking")); + } + (None, Some(_)) => { + if !version.yanked { + return Err(bad_request( + "Cannot update yank message for a version that is not yanked", + )); + } + } + _ => {} + } + Ok(()) +} + +fn apply_yank_update(version: &mut Version, update_data: &VersionUpdate) { + match (update_data.yanked, &update_data.yank_message) { + (Some(true), Some(message)) => { + version.yanked = true; + version.yank_message = Some(message.clone()); + } + (Some(yanked), None) => { + version.yanked = yanked; + version.yank_message = None; + } + (None, Some(message)) => { + version.yank_message = Some(message.clone()); + } + // If both yanked and yank_message are None, do nothing. + _ => {} + } +} + +pub fn perform_version_yank_update( + state: &AppState, + req: &Parts, + conn: &mut impl Conn, + version: &Version, + krate: &Crate, +) -> AppResult<()> { + let auth = AuthCheck::default() + .with_endpoint_scope(EndpointScope::Yank) + .for_crate(&krate.name) + .check(req, conn)?; + + state + .rate_limiter + .check_rate_limit(auth.user_id(), LimitedAction::YankUnyank, conn)?; + + let api_token_id = auth.api_token_id(); + let user = auth.user(); + let owners = krate.owners(conn)?; + + if Handle::current().block_on(user.rights(state, &owners))? < Rights::Publish { + if user.is_admin { + let action = if version.yanked { + "yanking" + } else { + "unyanking" + }; + warn!( + "Admin {} is {action} {}@{}", + user.gh_login, krate.name, version.num + ); + } else { + return Err(custom( + StatusCode::FORBIDDEN, + "must already be an owner to yank or unyank", + )); + } + } + + // Check if the yanked state or yank message has changed + let (yanked, yank_message) = versions::table + .find(version.id) + .select((versions::yanked, versions::yank_message)) + .first::<(bool, Option)>(conn)?; + + if yanked == version.yanked && yank_message == version.yank_message { + // No changes, return early + return Ok(()); + } + + diesel::update(version) + .set(( + versions::yanked.eq(version.yanked), + versions::yank_message.eq(&version.yank_message), + )) + .execute(conn)?; + + let action = if version.yanked { + VersionAction::Yank + } else { + VersionAction::Unyank + }; + insert_version_owner_action(conn, version.id, user.id, api_token_id, action)?; + + jobs::enqueue_sync_to_index(&krate.name, conn)?; + UpdateDefaultVersion::new(krate.id).enqueue(conn)?; + + Ok(()) +} diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 59226522d20..9ed44910fd1 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -1,26 +1,15 @@ //! Endpoints for yanking and unyanking specific versions of crates +use super::metadata::perform_version_yank_update; use super::version_and_crate; use crate::app::AppState; -use crate::auth::AuthCheck; use crate::controllers::helpers::ok_true; -use crate::models::token::EndpointScope; -use crate::models::Rights; -use crate::models::{insert_version_owner_action, VersionAction}; -use crate::rate_limiter::LimitedAction; -use crate::schema::versions; use crate::tasks::spawn_blocking; -use crate::util::errors::{custom, version_not_found, AppResult}; -use crate::worker::jobs; -use crate::worker::jobs::UpdateDefaultVersion; +use crate::util::errors::{version_not_found, AppResult}; use axum::extract::Path; use axum::response::Response; -use crates_io_worker::BackgroundJob; -use diesel::prelude::*; use diesel_async::async_connection_wrapper::AsyncConnectionWrapper; use http::request::Parts; -use http::StatusCode; -use tokio::runtime::Handle; /// Handles the `DELETE /crates/:crate_id/:version/yank` route. /// This does not delete a crate version, it makes the crate @@ -66,57 +55,9 @@ async fn modify_yank( let conn = state.db_write().await?; spawn_blocking(move || { let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - - let auth = AuthCheck::default() - .with_endpoint_scope(EndpointScope::Yank) - .for_crate(&crate_name) - .check(&req, conn)?; - - state - .rate_limiter - .check_rate_limit(auth.user_id(), LimitedAction::YankUnyank, conn)?; - - let (version, krate) = version_and_crate(conn, &crate_name, &version)?; - let api_token_id = auth.api_token_id(); - let user = auth.user(); - let owners = krate.owners(conn)?; - - if Handle::current().block_on(user.rights(&state, &owners))? < Rights::Publish { - if user.is_admin { - let action = if yanked { "yanking" } else { "unyanking" }; - warn!( - "Admin {} is {action} {}@{}", - user.gh_login, krate.name, version.num - ); - } else { - return Err(custom( - StatusCode::FORBIDDEN, - "must already be an owner to yank or unyank", - )); - } - } - - if version.yanked == yanked { - // The crate is already in the state requested, nothing to do - return ok_true(); - } - - diesel::update(&version) - .set(versions::yanked.eq(yanked)) - .execute(conn)?; - - let action = if yanked { - VersionAction::Yank - } else { - VersionAction::Unyank - }; - - insert_version_owner_action(conn, version.id, user.id, api_token_id, action)?; - - jobs::enqueue_sync_to_index(&krate.name, conn)?; - - UpdateDefaultVersion::new(krate.id).enqueue(conn)?; - + let (mut version, krate) = version_and_crate(conn, &crate_name, &version)?; + version.yanked = yanked; + perform_version_yank_update(&state, &req, conn, &version, &krate)?; ok_true() }) .await diff --git a/src/router.rs b/src/router.rs index 397e68f8e22..b74cce82a69 100644 --- a/src/router.rs +++ b/src/router.rs @@ -45,7 +45,7 @@ pub fn build_axum_router(state: AppState) -> Router<()> { .route("/api/v1/crates/:crate_id", get(krate::metadata::show)) .route( "/api/v1/crates/:crate_id/:version", - get(version::metadata::show), + get(version::metadata::show).patch(version::metadata::update), ) .route( "/api/v1/crates/:crate_id/:version/readme",