From ff2b568e5f2734c9340bdad8a1e4c255f21fa41f 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 1/5] Add `PATCH /crates/:crate/:version` route
Signed-off-by: Rustin170506 <29879298+Rustin170506@users.noreply.github.com>
---
src/controllers/version/metadata.rs | 162 +++++++++++++++++++++++++++-
src/controllers/version/yank.rs | 68 +-----------
src/router.rs | 2 +-
3 files changed, 165 insertions(+), 67 deletions(-)
diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs
index 1588648b978..9f18c6712f5 100644
--- a/src/controllers/version/metadata.rs
+++ b/src/controllers/version/metadata.rs
@@ -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>,
+}
+#[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.
@@ -84,3 +110,135 @@ 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_request): Json<VersionUpdateRequest>,
+) -> AppResult<Json<Value>> {
+ 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_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)?;
+
+ 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",
+ ));
+ }
+ }
+
+ let yanked = yanked.unwrap_or(version.yanked);
+ // 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(());
+ }
+
+ // Apply the update to the version
+ version.yanked = yanked;
+ version.yank_message = yank_message;
+
+ 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..eb85695d6bf 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,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
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",
From 5790f16950285b5f0457719d587fe0ee8bd863c5 Mon Sep 17 00:00:00 2001
From: Rustin170506 <29879298+Rustin170506@users.noreply.github.com>
Date: Tue, 10 Sep 2024 23:36:22 +0800
Subject: [PATCH 2/5] Add unit tests for the patch API
---
..._yanking__patch_version_yank_unyank-2.snap | 62 ++++++++++++++
..._yanking__patch_version_yank_unyank-3.snap | 73 ++++++++++++++++
..._yanking__patch_version_yank_unyank-4.snap | 73 ++++++++++++++++
..._yanking__patch_version_yank_unyank-5.snap | 84 +++++++++++++++++++
..._yanking__patch_version_yank_unyank-6.snap | 84 +++++++++++++++++++
...e__yanking__patch_version_yank_unyank.snap | 62 ++++++++++++++
src/tests/krate/yanking.rs | 67 +++++++++++++++
src/tests/mod.rs | 2 +-
.../routes/crates/versions/yank_unyank.rs | 34 +++++++-
src/tests/util.rs | 14 ++++
10 files changed, 553 insertions(+), 2 deletions(-)
create mode 100644 src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-2.snap
create mode 100644 src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-3.snap
create mode 100644 src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-4.snap
create mode 100644 src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-5.snap
create mode 100644 src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-6.snap
create mode 100644 src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank.snap
diff --git a/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-2.snap b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-2.snap
new file mode 100644
index 00000000000..af1afffa98b
--- /dev/null
+++ b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-2.snap
@@ -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": []
+ }
+}
diff --git a/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-3.snap b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-3.snap
new file mode 100644
index 00000000000..455b8364ca7
--- /dev/null
+++ b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-3.snap
@@ -0,0 +1,73 @@
+---
+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": "Updated 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]"
+ },
+ {
+ "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": []
+ }
+}
diff --git a/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-4.snap b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-4.snap
new file mode 100644
index 00000000000..455b8364ca7
--- /dev/null
+++ b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-4.snap
@@ -0,0 +1,73 @@
+---
+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": "Updated 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]"
+ },
+ {
+ "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": []
+ }
+}
diff --git a/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-5.snap b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-5.snap
new file mode 100644
index 00000000000..e441d6b90d1
--- /dev/null
+++ b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-5.snap
@@ -0,0 +1,84 @@
+---
+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": false,
+ "yank_message": null,
+ "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]"
+ },
+ {
+ "action": "yank",
+ "user": {
+ "id": 1,
+ "login": "foo",
+ "name": null,
+ "avatar": null,
+ "url": "https://github.com/foo"
+ },
+ "time": "[datetime]"
+ },
+ {
+ "action": "unyank",
+ "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": []
+ }
+}
diff --git a/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-6.snap b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-6.snap
new file mode 100644
index 00000000000..e441d6b90d1
--- /dev/null
+++ b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank-6.snap
@@ -0,0 +1,84 @@
+---
+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": false,
+ "yank_message": null,
+ "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]"
+ },
+ {
+ "action": "yank",
+ "user": {
+ "id": 1,
+ "login": "foo",
+ "name": null,
+ "avatar": null,
+ "url": "https://github.com/foo"
+ },
+ "time": "[datetime]"
+ },
+ {
+ "action": "unyank",
+ "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": []
+ }
+}
diff --git a/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank.snap b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank.snap
new file mode 100644
index 00000000000..af1afffa98b
--- /dev/null
+++ b/src/tests/krate/snapshots/crates_io__tests__krate__yanking__patch_version_yank_unyank.snap
@@ -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": []
+ }
+}
diff --git a/src/tests/krate/yanking.rs b/src/tests/krate/yanking.rs
index c8216355cda..1dc3aa83695 100644
--- a/src/tests/krate/yanking.rs
+++ b/src/tests/krate/yanking.rs
@@ -3,9 +3,11 @@ use crate::schema::publish_limit_buckets;
use crate::tests::builders::PublishBuilder;
use crate::tests::routes::crates::versions::yank_unyank::YankRequestHelper;
use crate::tests::util::{RequestHelper, TestApp};
+use crate::tests::VersionResponse;
use chrono::Utc;
use diesel::{ExpressionMethods, RunQueryDsl};
use googletest::prelude::*;
+use insta::assert_json_snapshot;
use std::time::Duration;
#[tokio::test(flavor = "multi_thread")]
@@ -220,3 +222,68 @@ async fn publish_after_yank_max_version() {
let json = anon.show_crate("fyk_max").await;
assert_eq!(json.krate.max_version, "2.0.0");
}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn patch_version_yank_unyank() {
+ let (_, anon, _, token) = TestApp::full().with_token();
+
+ // Upload a new crate
+ let crate_to_publish = PublishBuilder::new("patchable", "1.0.0");
+ token.publish_crate(crate_to_publish).await.good();
+
+ // Check initial state
+ let json = anon.show_version("patchable", "1.0.0").await;
+ assert!(!json.version.yanked);
+ assert_eq!(json.version.yank_message, None);
+
+ let assert_json_helper = |json: VersionResponse| {
+ assert_json_snapshot!(json, {
+ ".version.created_at" => "[datetime]",
+ ".version.updated_at" => "[datetime]",
+ ".version.audit_actions[].time" => "[datetime]",
+ });
+ };
+
+ // Yank with message
+ let response = token
+ .update_yank_status("patchable", "1.0.0", Some(true), Some("Yanking reason"))
+ .await
+ .good();
+ assert_json_helper(response);
+
+ let json = anon.show_version("patchable", "1.0.0").await;
+ assert_json_helper(json);
+
+ // Update yank message
+ let response = token
+ .update_yank_status("patchable", "1.0.0", None, Some("Updated reason"))
+ .await
+ .good();
+ assert_json_helper(response);
+
+ let json = anon.show_version("patchable", "1.0.0").await;
+ assert_json_helper(json);
+
+ // Unyank
+ let response = token
+ .update_yank_status("patchable", "1.0.0", Some(false), None)
+ .await
+ .good();
+ assert_json_helper(response);
+
+ let json = anon.show_version("patchable", "1.0.0").await;
+ assert_json_helper(json);
+
+ // Attempt to set yank message on unyanked version (should fail)
+ token
+ .update_yank_status("patchable", "1.0.0", None, Some("Invalid message"))
+ .await
+ .status()
+ .is_client_error();
+ // Attempt to unyank with message (should fail)
+ token
+ .update_yank_status("patchable", "1.0.0", Some(false), Some("Invalid message"))
+ .await
+ .status()
+ .is_client_error();
+}
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
index 7b7abe2c8ac..1457059fad5 100644
--- a/src/tests/mod.rs
+++ b/src/tests/mod.rs
@@ -56,7 +56,7 @@ pub struct CrateResponse {
versions: Option<Vec<EncodableVersion>>,
keywords: Option<Vec<EncodableKeyword>>,
}
-#[derive(Deserialize)]
+#[derive(Serialize, Deserialize)]
pub struct VersionResponse {
version: EncodableVersion,
}
diff --git a/src/tests/routes/crates/versions/yank_unyank.rs b/src/tests/routes/crates/versions/yank_unyank.rs
index f802ddb6528..26011389291 100644
--- a/src/tests/routes/crates/versions/yank_unyank.rs
+++ b/src/tests/routes/crates/versions/yank_unyank.rs
@@ -1,8 +1,9 @@
use crate::tests::builders::{CrateBuilder, PublishBuilder};
use crate::tests::util::{RequestHelper, Response, TestApp};
-use crate::tests::OkBool;
+use crate::tests::{OkBool, VersionResponse};
use http::StatusCode;
use insta::assert_snapshot;
+use serde_json::json;
pub trait YankRequestHelper {
/// Yank the specified version of the specified crate and run all pending background jobs
@@ -10,6 +11,15 @@ pub trait YankRequestHelper {
/// Unyank the specified version of the specified crate and run all pending background jobs
async fn unyank(&self, krate_name: &str, version: &str) -> Response<OkBool>;
+
+ /// Update the yank status of the specified version of the specified crate with a patch request and run all pending background jobs
+ async fn update_yank_status(
+ &self,
+ krate_name: &str,
+ version: &str,
+ yanked: Option<bool>,
+ yank_message: Option<&str>,
+ ) -> Response<VersionResponse>;
}
impl<T: RequestHelper> YankRequestHelper for T {
@@ -26,6 +36,28 @@ impl<T: RequestHelper> YankRequestHelper for T {
self.app().run_pending_background_jobs().await;
response
}
+
+ async fn update_yank_status(
+ &self,
+ krate_name: &str,
+ version: &str,
+ yanked: Option<bool>,
+ yank_message: Option<&str>,
+ ) -> Response<VersionResponse> {
+ let url = format!("/api/v1/crates/{krate_name}/{version}");
+
+ let json_body = json!({
+ "version": {
+ "yanked": yanked,
+ "yank_message": yank_message
+ }
+ });
+ let body = serde_json::to_string(&json_body).expect("Failed to serialize JSON body");
+
+ let response = self.patch(&url, body).await;
+ self.app().run_pending_background_jobs().await;
+ response
+ }
}
#[tokio::test(flavor = "multi_thread")]
diff --git a/src/tests/util.rs b/src/tests/util.rs
index b6b58432d84..aa096506dd5 100644
--- a/src/tests/util.rs
+++ b/src/tests/util.rs
@@ -145,6 +145,20 @@ pub trait RequestHelper {
self.run(request).await
}
+ /// Issue a PATCH request
+ async fn patch<T>(&self, path: &str, body: impl Into<Bytes>) -> Response<T> {
+ let body = body.into();
+ let is_json = body.starts_with(b"{") && body.ends_with(b"}");
+
+ let mut request = self.request_builder(Method::PATCH, path);
+ *request.body_mut() = body;
+ if is_json {
+ request.header(header::CONTENT_TYPE, "application/json");
+ }
+
+ self.run(request).await
+ }
+
/// Issue a DELETE request
async fn delete<T>(&self, path: &str) -> Response<T> {
let request = self.request_builder(Method::DELETE, path);
From 3739c88899ab7cd362451c544b45c5376999afe6 Mon Sep 17 00:00:00 2001
From: Tobias Bieniek <tobias@bieniek.cloud>
Date: Mon, 30 Sep 2024 12:39:38 +0200
Subject: [PATCH 3/5] tests/krate/yanking: Fix status code assertions
---
src/tests/krate/yanking.rs | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/tests/krate/yanking.rs b/src/tests/krate/yanking.rs
index 1dc3aa83695..f88de43cea4 100644
--- a/src/tests/krate/yanking.rs
+++ b/src/tests/krate/yanking.rs
@@ -7,6 +7,7 @@ use crate::tests::VersionResponse;
use chrono::Utc;
use diesel::{ExpressionMethods, RunQueryDsl};
use googletest::prelude::*;
+use http::StatusCode;
use insta::assert_json_snapshot;
use std::time::Duration;
@@ -275,15 +276,14 @@ async fn patch_version_yank_unyank() {
assert_json_helper(json);
// Attempt to set yank message on unyanked version (should fail)
- token
+ let response = token
.update_yank_status("patchable", "1.0.0", None, Some("Invalid message"))
- .await
- .status()
- .is_client_error();
+ .await;
+ assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+
// Attempt to unyank with message (should fail)
- token
+ let response = token
.update_yank_status("patchable", "1.0.0", Some(false), Some("Invalid message"))
- .await
- .status()
- .is_client_error();
+ .await;
+ assert_eq!(response.status(), StatusCode::BAD_REQUEST);
}
From cccdd7451ceef8f8685f6e7f5691fa562742045c Mon Sep 17 00:00:00 2001
From: Tobias Bieniek <tobias@bieniek.cloud>
Date: Mon, 30 Sep 2024 12:46:38 +0200
Subject: [PATCH 4/5] tests/krate/yanking: Add error response body assertions
---
src/tests/krate/yanking.rs | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/tests/krate/yanking.rs b/src/tests/krate/yanking.rs
index f88de43cea4..854f2bb0b3d 100644
--- a/src/tests/krate/yanking.rs
+++ b/src/tests/krate/yanking.rs
@@ -8,7 +8,7 @@ use chrono::Utc;
use diesel::{ExpressionMethods, RunQueryDsl};
use googletest::prelude::*;
use http::StatusCode;
-use insta::assert_json_snapshot;
+use insta::{assert_json_snapshot, assert_snapshot};
use std::time::Duration;
#[tokio::test(flavor = "multi_thread")]
@@ -280,10 +280,12 @@ async fn patch_version_yank_unyank() {
.update_yank_status("patchable", "1.0.0", None, Some("Invalid message"))
.await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+ assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Cannot update yank message for a version that is not yanked"}]}"#);
// Attempt to unyank with message (should fail)
let response = token
.update_yank_status("patchable", "1.0.0", Some(false), Some("Invalid message"))
.await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
+ assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Cannot set yank message when unyanking"}]}"#);
}
From 45ec3fb3a48423334226be69ce96ae04b0bf9cb2 Mon Sep 17 00:00:00 2001
From: Tobias Bieniek <tobias@bieniek.cloud>
Date: Mon, 30 Sep 2024 13:18:32 +0200
Subject: [PATCH 5/5] controllers/version/metadata: Fix `yanked` usage
This was using the previous `yanked` state for the admin log message, instead of the new state.
---
src/controllers/version/metadata.rs | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs
index 9f18c6712f5..41e24edd06e 100644
--- a/src/controllers/version/metadata.rs
+++ b/src/controllers/version/metadata.rs
@@ -187,13 +187,11 @@ pub fn perform_version_yank_update(
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 version.yanked {
- "yanking"
- } else {
- "unyanking"
- };
+ let action = if yanked { "yanking" } else { "unyanking" };
warn!(
"Admin {} is {action} {}@{}",
user.gh_login, krate.name, version.num
@@ -206,7 +204,6 @@ pub fn perform_version_yank_update(
}
}
- let yanked = yanked.unwrap_or(version.yanked);
// 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(
@@ -230,7 +227,7 @@ pub fn perform_version_yank_update(
version.yanked = yanked;
version.yank_message = yank_message;
- let action = if version.yanked {
+ let action = if yanked {
VersionAction::Yank
} else {
VersionAction::Unyank