Merge pull request #11290 from Turbo87/version-save

Simplify `NewVersion::save()` fn

Author: Turbo87

crates/crates_io_database/src/models/mod.rs

@@ -36,3 +36,4 @@ pub mod token;
 pub mod trustpub;
 pub mod user;
 pub mod version;
+pub mod versions_published_by;

crates/crates_io_database/src/models/version.rs

@@ -4,12 +4,11 @@ use bon::Builder;
 use chrono::{DateTime, Utc};
 use crates_io_index::features::FeaturesMap;
 use diesel::prelude::*;
-use diesel_async::scoped_futures::ScopedFutureExt;
-use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
+use diesel_async::{AsyncPgConnection, RunQueryDsl};
 use serde::Deserialize;
 
 use crate::models::{Crate, User};
-use crate::schema::*;
+use crate::schema::{readme_renderings, users, versions};
 
 // Queryable has a custom implementation below
 #[derive(Clone, Identifiable, Associations, Debug, Queryable, Selectable)]
@@ -107,34 +106,12 @@ pub struct NewVersion<'a> {
 }
 
 impl NewVersion<'_> {
-    pub async fn save(
-        &self,
-        conn: &mut AsyncPgConnection,
-        published_by_email: &str,
-    ) -> QueryResult<Version> {
-        use diesel::insert_into;
-
-        conn.transaction(|conn| {
-            async move {
-                let version: Version = insert_into(versions::table)
-                    .values(self)
-                    .returning(Version::as_returning())
-                    .get_result(conn)
-                    .await?;
-
-                insert_into(versions_published_by::table)
-                    .values((
-                        versions_published_by::version_id.eq(version.id),
-                        versions_published_by::email.eq(published_by_email),
-                    ))
-                    .execute(conn)
-                    .await?;
-
-                Ok(version)
-            }
-            .scope_boxed()
-        })
-        .await
+    pub async fn save(&self, conn: &mut AsyncPgConnection) -> QueryResult<Version> {
+        diesel::insert_into(versions::table)
+            .values(self)
+            .returning(Version::as_returning())
+            .get_result(conn)
+            .await
     }
 }
 

crates/crates_io_database/src/models/versions_published_by.rs

@@ -0,0 +1,17 @@
+use crate::schema::versions_published_by;
+use diesel::prelude::*;
+use diesel_async::{AsyncPgConnection, RunQueryDsl};
+
+pub async fn insert(
+    version_id: i32,
+    email: &str,
+    conn: &mut AsyncPgConnection,
+) -> QueryResult<usize> {
+    diesel::insert_into(versions_published_by::table)
+        .values((
+            versions_published_by::version_id.eq(version_id),
+            versions_published_by::email.eq(email),
+        ))
+        .execute(conn)
+        .await
+}

src/controllers/krate/publish.rs

@@ -42,6 +42,7 @@ use crate::util::errors::{AppResult, BoxedAppError, bad_request, custom, interna
 use crate::views::{
     EncodableCrate, EncodableCrateDependency, GoodCrate, PublishMetadata, PublishWarnings,
 };
+use crates_io_database::models::versions_published_by;
 use crates_io_diesel_helpers::canon_crate_name;
 
 const MISSING_RIGHTS_ERROR_MESSAGE: &str = "this crate exists but you don't seem to be an owner. \
@@ -432,7 +433,7 @@ pub async fn publish(app: AppState, req: Parts, body: Body) -> AppResult<Json<Go
             .keywords(&keywords)
             .build();
 
-        let version = new_version.save(conn, &verified_email_address).await.map_err(|error| {
+        let version = new_version.save(conn).await.map_err(|error| {
             use diesel::result::{Error, DatabaseErrorKind};
             match error {
                 Error::DatabaseError(DatabaseErrorKind::UniqueViolation, _) =>
@@ -441,6 +442,8 @@ pub async fn publish(app: AppState, req: Parts, body: Body) -> AppResult<Json<Go
             }
         })?;
 
+        versions_published_by::insert(version.id, &verified_email_address, conn).await?;
+
         NewVersionOwnerAction::builder()
             .version_id(version.id)
             .user_id(user.id)

src/tests/builders/version.rs

@@ -3,6 +3,7 @@ use crate::schema::dependencies;
 use std::collections::BTreeMap;
 
 use chrono::{DateTime, Utc};
+use crates_io_database::models::versions_published_by;
 use diesel::prelude::*;
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
 
@@ -110,7 +111,8 @@ impl VersionBuilder {
             .maybe_created_at(self.created_at.as_ref())
             .build();
 
-        let vers = new_version.save(connection, "[email protected]").await?;
+        let vers = new_version.save(connection).await?;
+        versions_published_by::insert(vers.id, "[email protected]", connection).await?;
 
         let new_deps = self
             .dependencies

src/worker/jobs/downloads/update_metadata.rs

@@ -136,7 +136,8 @@ mod tests {
             .checksum("0000000000000000000000000000000000000000000000000000000000000000")
             .build();
 
-        let version = version.save(conn, "[email protected]").await.unwrap();
+        let version = version.save(conn).await.unwrap();
+
         (krate, version)
     }