Skip to content

Commit

Permalink
Reuse the code to perform version yank update
Browse files Browse the repository at this point in the history 
Signed-off-by: Rustin170506 <[email protected]>
  • Loading branch information
@Rustin170506
Rustin170506 committed Sep 11, 2024
1 parent 58b48b0 commit 7dd8d49
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 101 deletions.
94 changes: 57 additions & 37 deletions src/controllers/version/metadata.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
use axum::extract::Path;
use axum::Json;
use crates_io_worker::BackgroundJob;
use diesel::{ExpressionMethods, RunQueryDsl};
use diesel::{ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel_async::async_connection_wrapper::AsyncConnectionWrapper;
use http::request::Parts;
use http::StatusCode;
Expand Down Expand Up @@ -142,14 +142,52 @@ fn apply_yank_update(
) -> AppResult<()> {
// Try to update the yank state first, to avoid unnecessary checks.
update_version_yank_state(version, update_data)?;
perform_version_yank_update(state, req, conn, version, krate)?;

// Add authentication check
Ok(())
}

fn update_version_yank_state(version: &mut Version, update_data: &VersionUpdate) -> AppResult<()> {
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;
}
(Some(false), Some(_)) => {
return Err(bad_request("Cannot set yank message when unyanking"));
}
(None, Some(message)) => {
if version.yanked {
version.yank_message = Some(message.clone());
} else {
return Err(bad_request(
"Cannot update yank message for a version that is not yanked",
));
}
}
// If both yanked and yank_message are None, do nothing.
// This function only cares about updating the yanked state and yank message.
(None, None) => {}
}
Ok(())
}

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)?;

// Add rate limiting check
state
.rate_limiter
.check_rate_limit(auth.user_id(), LimitedAction::YankUnyank, conn)?;
Expand All @@ -158,7 +196,6 @@ fn apply_yank_update(
let user = auth.user();
let owners = krate.owners(conn)?;

// Check user rights
if Handle::current().block_on(user.rights(state, &owners))? < Rights::Publish {
if user.is_admin {
warn!(
Expand All @@ -168,58 +205,41 @@ fn apply_yank_update(
} else {
return Err(custom(
StatusCode::FORBIDDEN,
"must already be an owner to update version",
"must already be an owner to yank or unyank",
));
}
}

diesel::update(&*version)
// Check if the yanked state or yank message has changed
let (yanked, yank_message) = crate::schema::versions::table
.find(version.id)
.select((
crate::schema::versions::yanked,
crate::schema::versions::yank_message,
))
.first::<(bool, Option<String>)>(conn)?;

if yanked == version.yanked && yank_message == version.yank_message {
// No changes, return early
return Ok(());
}

diesel::update(version)
.set((
crate::schema::versions::yanked.eq(version.yanked),
crate::schema::versions::yank_message.eq(&version.yank_message),
))
.execute(conn)?;

// Add version owner action
let action = if version.yanked {
VersionAction::Yank
} else {
VersionAction::Unyank
};
insert_version_owner_action(conn, version.id, user.id, api_token_id, action)?;

// Enqueue jobs
jobs::enqueue_sync_to_index(&krate.name, conn)?;
UpdateDefaultVersion::new(krate.id).enqueue(conn)?;

Ok(())
}

fn update_version_yank_state(version: &mut Version, update_data: &VersionUpdate) -> AppResult<()> {
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;
}
(Some(false), Some(_)) => {
return Err(bad_request("Cannot set yank message when unyanking"));
}
(None, Some(message)) => {
if version.yanked {
version.yank_message = Some(message.clone());
} else {
return Err(bad_request(
"Cannot update yank message for a version that is not yanked",
));
}
}
// If both yanked and yank_message are None, do nothing.
// This function only cares about updating the yanked state and yank message.
(None, None) => {}
}
Ok(())
}
69 changes: 5 additions & 64 deletions src/controllers/version/yank.rs
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7dd8d49

Please sign in to comment.