From 7e43e4ebce408708dced5d5ad08985ac66483438 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Tue, 19 Nov 2024 12:00:42 +0100 Subject: [PATCH 1/4] models/action: Add `NewVersionOwnerAction::async_insert()` fn --- src/models/action.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/models/action.rs b/src/models/action.rs index 85d4ce4939e..e0a1372196f 100644 --- a/src/models/action.rs +++ b/src/models/action.rs @@ -114,4 +114,16 @@ impl NewVersionOwnerAction { .values(self) .get_result(conn) } + + pub async fn async_insert( + &self, + conn: &mut AsyncPgConnection, + ) -> QueryResult { + use diesel_async::RunQueryDsl; + + diesel::insert_into(version_owner_actions::table) + .values(self) + .get_result(conn) + .await + } } From 37b73ae859e849ae2092c6d585de018b7ce8a0aa Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Tue, 19 Nov 2024 12:03:58 +0100 Subject: [PATCH 2/4] controllers/version/metadata: Migrate `perform_version_yank_update()` to async/await --- src/controllers/version/metadata.rs | 49 ++++++++++++++++------------- src/controllers/version/yank.rs | 28 +++++++---------- 2 files changed, 39 insertions(+), 38 deletions(-) diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index 3ba9d1165a6..47355f97559 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -15,7 +15,6 @@ use diesel_async::AsyncPgConnection; use http::request::Parts; use http::StatusCode; use serde::Deserialize; -use tokio::runtime::Handle; use crate::app::AppState; use crate::auth::{AuthCheck, Authentication}; @@ -27,7 +26,6 @@ use crate::rate_limiter::LimitedAction; use crate::schema::versions; use crate::tasks::spawn_blocking; use crate::util::diesel::prelude::*; -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::{SyncToGitIndex, SyncToSparseIndex, UpdateDefaultVersion}; @@ -137,19 +135,20 @@ pub async fn update( .check_rate_limit(auth.user_id(), LimitedAction::YankUnyank, &mut conn) .await?; + perform_version_yank_update( + &state, + &mut conn, + &mut version, + &krate, + &auth, + update_request.version.yanked, + update_request.version.yank_message, + ) + .await?; + spawn_blocking(move || { let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - perform_version_yank_update( - &state, - conn, - &mut version, - &krate, - &auth, - 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); @@ -186,24 +185,24 @@ pub async fn authenticate( .await } -pub fn perform_version_yank_update( +pub async fn perform_version_yank_update( state: &AppState, - conn: &mut impl Conn, + conn: &mut AsyncPgConnection, version: &mut Version, krate: &Crate, auth: &Authentication, yanked: Option, yank_message: Option, ) -> AppResult<()> { - use diesel::RunQueryDsl; + use diesel_async::RunQueryDsl; let api_token_id = auth.api_token_id(); let user = auth.user(); - let owners = krate.owners(conn)?; + let owners = krate.async_owners(conn).await?; let yanked = yanked.unwrap_or(version.yanked); - if Handle::current().block_on(user.rights(state, &owners))? < Rights::Publish { + if user.rights(state, &owners).await? < Rights::Publish { if user.is_admin { let action = if yanked { "yanking" } else { "unyanking" }; warn!( @@ -230,7 +229,8 @@ pub fn perform_version_yank_update( versions::yanked.eq(yanked), versions::yank_message.eq(&yank_message), )) - .execute(conn)?; + .execute(conn) + .await?; // If no rows were updated, return early if updated_cnt == 0 { @@ -252,11 +252,16 @@ pub fn perform_version_yank_update( .maybe_api_token_id(api_token_id) .action(action) .build() - .insert(conn)?; + .async_insert(conn) + .await?; - SyncToGitIndex::new(&krate.name).enqueue(conn)?; - SyncToSparseIndex::new(&krate.name).enqueue(conn)?; - UpdateDefaultVersion::new(krate.id).enqueue(conn)?; + SyncToGitIndex::new(&krate.name).async_enqueue(conn).await?; + SyncToSparseIndex::new(&krate.name) + .async_enqueue(conn) + .await?; + UpdateDefaultVersion::new(krate.id) + .async_enqueue(conn) + .await?; Ok(()) } diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 122a0517c08..c9e18f65074 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -5,11 +5,9 @@ use super::version_and_crate; use crate::app::AppState; use crate::controllers::helpers::ok_true; use crate::rate_limiter::LimitedAction; -use crate::tasks::spawn_blocking; use crate::util::errors::{version_not_found, AppResult}; use axum::extract::Path; use axum::response::Response; -use diesel_async::async_connection_wrapper::AsyncConnectionWrapper; use http::request::Parts; /// Handles the `DELETE /crates/:crate_id/:version/yank` route. @@ -62,18 +60,16 @@ async fn modify_yank( .check_rate_limit(auth.user_id(), LimitedAction::YankUnyank, &mut conn) .await?; - spawn_blocking(move || { - let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - perform_version_yank_update( - &state, - conn, - &mut version, - &krate, - &auth, - Some(yanked), - None, - )?; - ok_true() - }) - .await + perform_version_yank_update( + &state, + &mut conn, + &mut version, + &krate, + &auth, + Some(yanked), + None, + ) + .await?; + + ok_true() } From 33b36c39b04b5c6631ec8b7c8ae8d0f205bd3a9a Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Tue, 19 Nov 2024 12:09:38 +0100 Subject: [PATCH 3/4] models/version: Migrate `Version::published_by()` to async/await --- src/controllers/version/metadata.rs | 6 ++++-- src/models/version.rs | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index 47355f97559..94c0c0bbfc3 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -100,10 +100,11 @@ pub async fn show( let mut conn = state.db_read().await?; let (version, krate) = version_and_crate(&mut conn, &crate_name, &version).await?; + let published_by = version.published_by(&mut conn).await?; + spawn_blocking(move || { let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - let published_by = version.published_by(conn)?; let actions = VersionOwnerAction::by_version(conn, &version)?; let version = EncodableVersion::from(version, &krate.name, published_by, actions); @@ -146,10 +147,11 @@ pub async fn update( ) .await?; + let published_by = version.published_by(&mut conn).await?; + spawn_blocking(move || { let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - 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!({ "version": updated_version })) diff --git a/src/models/version.rs b/src/models/version.rs index 5800f216960..66db5a3fb28 100644 --- a/src/models/version.rs +++ b/src/models/version.rs @@ -55,11 +55,11 @@ impl Version { /// Gets the User who ran `cargo publish` for this version, if recorded. /// Not for use when you have a group of versions you need the publishers for. - pub fn published_by(&self, conn: &mut impl Conn) -> QueryResult> { - use diesel::RunQueryDsl; + pub async fn published_by(&self, conn: &mut AsyncPgConnection) -> QueryResult> { + use diesel_async::RunQueryDsl; match self.published_by { - Some(pb) => users::table.find(pb).first(conn).optional(), + Some(pb) => users::table.find(pb).first(conn).await.optional(), None => Ok(None), } } From 80e44a8304a3b5a17123946ef4ecd413a82e0020 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Tue, 19 Nov 2024 12:11:12 +0100 Subject: [PATCH 4/4] models/action: Migrate `VersionOwnerAction::by_version()` to async/await --- src/controllers/version/metadata.rs | 26 ++++++-------------------- src/models/action.rs | 8 ++++++-- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index 94c0c0bbfc3..1bed5a3aff3 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -10,7 +10,6 @@ use axum_extra::json; use axum_extra::response::ErasedJson; use crates_io_database::schema::{crates, dependencies}; use crates_io_worker::BackgroundJob; -use diesel_async::async_connection_wrapper::AsyncConnectionWrapper; use diesel_async::AsyncPgConnection; use http::request::Parts; use http::StatusCode; @@ -24,7 +23,6 @@ use crate::models::{ }; use crate::rate_limiter::LimitedAction; use crate::schema::versions; -use crate::tasks::spawn_blocking; use crate::util::diesel::prelude::*; use crate::util::errors::{bad_request, custom, version_not_found, AppResult}; use crate::views::{EncodableDependency, EncodableVersion}; @@ -101,16 +99,10 @@ pub async fn show( let mut conn = state.db_read().await?; let (version, krate) = version_and_crate(&mut conn, &crate_name, &version).await?; let published_by = version.published_by(&mut conn).await?; + let actions = VersionOwnerAction::by_version(&mut conn, &version).await?; - spawn_blocking(move || { - let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - - let actions = VersionOwnerAction::by_version(conn, &version)?; - - let version = EncodableVersion::from(version, &krate.name, published_by, actions); - Ok(json!({ "version": version })) - }) - .await + let version = EncodableVersion::from(version, &krate.name, published_by, actions); + Ok(json!({ "version": version })) } /// Handles the `PATCH /crates/:crate/:version` route. @@ -148,15 +140,9 @@ pub async fn update( .await?; let published_by = version.published_by(&mut conn).await?; - - spawn_blocking(move || { - let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - - let actions = VersionOwnerAction::by_version(conn, &version)?; - let updated_version = EncodableVersion::from(version, &krate.name, published_by, actions); - Ok(json!({ "version": updated_version })) - }) - .await + let actions = VersionOwnerAction::by_version(&mut conn, &version).await?; + let updated_version = EncodableVersion::from(version, &krate.name, published_by, actions); + Ok(json!({ "version": updated_version })) } fn validate_yank_update(update_data: &VersionUpdate, version: &Version) -> AppResult<()> { diff --git a/src/models/action.rs b/src/models/action.rs index e0a1372196f..a4a2a3f27bc 100644 --- a/src/models/action.rs +++ b/src/models/action.rs @@ -57,8 +57,11 @@ impl VersionOwnerAction { version_owner_actions::table.load(conn) } - pub fn by_version(conn: &mut impl Conn, version: &Version) -> QueryResult> { - use diesel::RunQueryDsl; + pub async fn by_version( + conn: &mut AsyncPgConnection, + version: &Version, + ) -> QueryResult> { + use diesel_async::RunQueryDsl; use version_owner_actions::dsl::version_id; version_owner_actions::table @@ -66,6 +69,7 @@ impl VersionOwnerAction { .inner_join(users::table) .order(version_owner_actions::dsl::id) .load(conn) + .await } pub fn for_versions(