Merge pull request #7191 from Turbo87/create-update

models/krate: Split `create_or_update()` fn into `create()` and `update()`

Author: Turbo87

src/controllers/krate/publish.rs

@@ -154,7 +154,12 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
                 return Err(cargo_err("cannot upload a crate with a reserved name"));
             }
 
-            let krate = persist.create_or_update(conn, user.id)?;
+            // To avoid race conditions, we try to insert
+            // first so we know whether to add an owner
+            let krate = match persist.create(conn, user.id).optional()? {
+                Some(krate) => krate,
+                None => persist.update(conn)?,
+            };
 
             let owners = krate.owners(conn)?;
             if user.rights(&app, &owners)? < Rights::Publish {

src/downloads_counter.rs

@@ -439,7 +439,7 @@ mod tests {
                 name: "foo",
                 ..NewCrate::default()
             }
-            .create_or_update(conn, user.id)
+            .create(conn, user.id)
             .expect("failed to create crate");
 
             Self {

src/models/krate.rs

@@ -101,50 +101,39 @@ pub struct NewCrate<'a> {
 }
 
 impl<'a> NewCrate<'a> {
-    pub fn create_or_update(self, conn: &mut PgConnection, uploader: i32) -> AppResult<Crate> {
+    pub fn update(&self, conn: &mut PgConnection) -> QueryResult<Crate> {
         use diesel::update;
 
-        conn.transaction(|conn| {
-            // To avoid race conditions, we try to insert
-            // first so we know whether to add an owner
-            if let Some(krate) = self.save_new_crate(conn, uploader)? {
-                return Ok(krate);
-            }
-
-            update(crates::table)
-                .filter(canon_crate_name(crates::name).eq(canon_crate_name(self.name)))
-                .set(&self)
-                .returning(Crate::as_returning())
-                .get_result(conn)
-                .map_err(Into::into)
-        })
+        update(crates::table)
+            .filter(canon_crate_name(crates::name).eq(canon_crate_name(self.name)))
+            .set(self)
+            .returning(Crate::as_returning())
+            .get_result(conn)
     }
 
-    fn save_new_crate(&self, conn: &mut PgConnection, user_id: i32) -> QueryResult<Option<Crate>> {
+    pub fn create(&self, conn: &mut PgConnection, user_id: i32) -> QueryResult<Crate> {
         use crate::schema::crates::dsl::*;
 
         conn.transaction(|conn| {
-            let maybe_inserted: Option<Crate> = diesel::insert_into(crates)
+            let krate: Crate = diesel::insert_into(crates)
                 .values(self)
                 .on_conflict_do_nothing()
                 .returning(Crate::as_returning())
-                .get_result(conn)
-                .optional()?;
-
-            if let Some(ref krate) = maybe_inserted {
-                let owner = CrateOwner {
-                    crate_id: krate.id,
-                    owner_id: user_id,
-                    created_by: user_id,
-                    owner_kind: OwnerKind::User as i32,
-                    email_notifications: true,
-                };
-                diesel::insert_into(crate_owners::table)
-                    .values(&owner)
-                    .execute(conn)?;
-            }
+                .get_result(conn)?;
+
+            let owner = CrateOwner {
+                crate_id: krate.id,
+                owner_id: user_id,
+                created_by: user_id,
+                owner_kind: OwnerKind::User as i32,
+                email_notifications: true,
+            };
+
+            diesel::insert_into(crate_owners::table)
+                .values(&owner)
+                .execute(conn)?;
 
-            Ok(maybe_inserted)
+            Ok(krate)
         })
     }
 }

src/tests/builders/krate.rs

@@ -114,7 +114,7 @@ impl<'a> CrateBuilder<'a> {
     pub fn build(mut self, connection: &mut PgConnection) -> AppResult<Crate> {
         use diesel::{insert_into, select, update};
 
-        let mut krate = self.krate.create_or_update(connection, self.owner_id)?;
+        let mut krate = self.krate.create(connection, self.owner_id)?;
 
         // Since we are using `NewCrate`, we can't set all the
         // crate properties in a single DB call.

src/worker/update_downloads.rs

@@ -98,7 +98,7 @@ mod test {
             name: "foo",
             ..Default::default()
         }
-        .create_or_update(conn, user_id)
+        .create(conn, user_id)
         .unwrap();
         let version = NewVersion::new(
             krate.id,