Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PATCH /crates/:crate/:version route #9423

Merged
merged 5 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 157 additions & 2 deletions src/controllers/version/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,43 @@

use axum::extract::Path;
use axum::Json;
use crates_io_worker::BackgroundJob;
use diesel::{
BoolExpressionMethods, ExpressionMethods, PgExpressionMethods, 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<bool>,
yank_message: Option<String>,
}
Rustin170506 marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Deserialize)]
pub struct VersionUpdateRequest {
version: VersionUpdate,
}

/// Handles the `GET /crates/:crate_id/:version/dependencies` route.
///
/// This information can be obtained directly from the index.
Expand Down Expand Up @@ -84,3 +110,132 @@
})
.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_request): Json<VersionUpdateRequest>,
) -> AppResult<Json<Value>> {
if semver::Version::parse(&version).is_err() {
return Err(version_not_found(&crate_name, &version));

Check warning on line 124 in src/controllers/version/metadata.rs

View check run for this annotation

Codecov / codecov/patch

src/controllers/version/metadata.rs#L124

Added line #L124 was not covered by tests
}

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_request.version, &version)?;
perform_version_yank_update(
&state,
&req,
conn,
&mut version,
&krate,
update_request.version.yanked,
update_request.version.yank_message,
)?;

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(())
}

pub fn perform_version_yank_update(
state: &AppState,
req: &Parts,
conn: &mut impl Conn,
version: &mut Version,
krate: &Crate,
yanked: Option<bool>,
yank_message: Option<String>,
) -> 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)?;
Turbo87 marked this conversation as resolved.
Show resolved Hide resolved

let api_token_id = auth.api_token_id();
let user = auth.user();
let owners = krate.owners(conn)?;

let yanked = yanked.unwrap_or(version.yanked);

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} {}@{}",

Check warning on line 196 in src/controllers/version/metadata.rs

View check run for this annotation

Codecov / codecov/patch

src/controllers/version/metadata.rs#L196

Added line #L196 was not covered by tests
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 and update if necessary
let updated_cnt = diesel::update(
versions::table.find(version.id).filter(
versions::yanked
.is_distinct_from(yanked)
.or(versions::yank_message.is_distinct_from(&yank_message)),
),
)
.set((
versions::yanked.eq(yanked),
versions::yank_message.eq(&yank_message),
))
.execute(conn)?;

// If no rows were updated, return early
if updated_cnt == 0 {
return Ok(());

Check warning on line 223 in src/controllers/version/metadata.rs

View check run for this annotation

Codecov / codecov/patch

src/controllers/version/metadata.rs#L223

Added line #L223 was not covered by tests
}

// Apply the update to the version
version.yanked = yanked;
version.yank_message = yank_message;

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

Ok(())
}
68 changes: 4 additions & 64 deletions src/controllers/version/yank.rs
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,8 @@ 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)?;
perform_version_yank_update(&state, &req, conn, &mut version, &krate, Some(yanked), None)?;
ok_true()
})
.await
Expand Down
2 changes: 1 addition & 1 deletion src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
---
source: src/tests/krate/yanking.rs
expression: json
---
{
"version": {
"id": 1,
"crate": "patchable",
"num": "1.0.0",
"dl_path": "/api/v1/crates/patchable/1.0.0/download",
"readme_path": "/api/v1/crates/patchable/1.0.0/readme",
"updated_at": "[datetime]",
"created_at": "[datetime]",
"downloads": 0,
"features": {},
"yanked": true,
"yank_message": "Yanking reason",
"lib_links": null,
"license": "MIT",
"links": {
"dependencies": "/api/v1/crates/patchable/1.0.0/dependencies",
"version_downloads": "/api/v1/crates/patchable/1.0.0/downloads",
"authors": "/api/v1/crates/patchable/1.0.0/authors"
},
"crate_size": 151,
"published_by": {
"id": 1,
"login": "foo",
"name": null,
"avatar": null,
"url": "https://github.com/foo"
},
"audit_actions": [
{
"action": "publish",
"user": {
"id": 1,
"login": "foo",
"name": null,
"avatar": null,
"url": "https://github.com/foo"
},
"time": "[datetime]"
},
{
"action": "yank",
"user": {
"id": 1,
"login": "foo",
"name": null,
"avatar": null,
"url": "https://github.com/foo"
},
"time": "[datetime]"
}
],
"checksum": "ddfc395ab340f413ee1d1ed0afce51a7c9df1c99c551fed5aef76edd4abe4048",
"rust_version": null,
"has_lib": false,
"bin_names": []
}
}
Loading