Auto merge of #4175 - Turbo87:sync-yanked, r=JohnTitor

worker::git: Read `yanked` status from database

This PR implements one part of #4172. By no longer passing `yanked` into the `yank` (now `sync_yanked`) background worker task, and instead reading `yanked` from the database, we can avoid issues from tasks running out of order.

Author: bors

src/controllers/version/yank.rs

@@ -61,7 +61,7 @@ fn modify_yank(req: &mut dyn RequestExt, yanked: bool) -> EndpointResult {
 
     insert_version_owner_action(&conn, version.id, user.id, api_token_id, action)?;
 
-    worker::yank(krate.name, version, yanked).enqueue(&conn)?;
+    worker::sync_yanked(krate.name, version.num).enqueue(&conn)?;
 
     ok_true()
 }

src/worker/git.rs

@@ -1,7 +1,9 @@
 use crate::background_jobs::Environment;
 use crate::git::{Crate, Credentials};
-use crate::models::Version;
+use crate::schema;
+use anyhow::Context;
 use chrono::Utc;
+use diesel::prelude::*;
 use std::fs::{self, OpenOptions};
 use std::io::prelude::*;
 use swirl::PerformError;
@@ -29,12 +31,24 @@ pub fn add_crate(env: &Environment, krate: Crate) -> Result<(), PerformError> {
 /// `true` or `false`, write all the lines back out, and commit and
 /// push the changes.
 #[swirl::background_job]
-pub fn yank(
+pub fn sync_yanked(
     env: &Environment,
+    conn: &PgConnection,
     krate: String,
-    version: Version,
-    yanked: bool,
+    version_num: String,
 ) -> Result<(), PerformError> {
+    trace!(?krate, ?version_num, "Load yanked status from database");
+
+    let yanked: bool = schema::versions::table
+        .inner_join(schema::crates::table)
+        .filter(schema::crates::name.eq(&krate))
+        .filter(schema::versions::num.eq(&version_num))
+        .select(schema::versions::yanked)
+        .get_result(conn)
+        .context("Failed to load yanked status from database")?;
+
+    trace!(?krate, ?version_num, yanked);
+
     let repo = env.lock_index()?;
     let dst = repo.index_file(&krate);
 
@@ -44,24 +58,25 @@ pub fn yank(
         .map(|line| {
             let mut git_crate = serde_json::from_str::<Crate>(line)
                 .map_err(|_| format!("couldn't decode: `{}`", line))?;
-            if git_crate.name != krate || git_crate.vers != version.num {
+            if git_crate.name != krate || git_crate.vers != version_num {
                 return Ok(line.to_string());
             }
             git_crate.yanked = Some(yanked);
             Ok(serde_json::to_string(&git_crate)?)
         })
         .collect::<Result<Vec<_>, PerformError>>();
     let new = new?.join("\n") + "\n";
-    fs::write(&dst, new.as_bytes())?;
 
-    let message: String = format!(
-        "{} crate `{}#{}`",
-        if yanked { "Yanking" } else { "Unyanking" },
-        krate,
-        version.num
-    );
+    if new != prev {
+        fs::write(&dst, new.as_bytes())?;
 
-    repo.commit_and_push(&message, &dst)?;
+        let action = if yanked { "Yanking" } else { "Unyanking" };
+        let message = format!("{} crate `{}#{}`", action, krate, version_num);
+
+        repo.commit_and_push(&message, &dst)?;
+    } else {
+        debug!("Skipping `yanked` update because index is up-to-date");
+    }
 
     Ok(())
 }

src/worker/mod.rs

@@ -11,6 +11,6 @@ mod update_downloads;
 
 pub use daily_db_maintenance::daily_db_maintenance;
 pub use dump_db::dump_db;
-pub use git::{add_crate, squash_index, yank};
+pub use git::{add_crate, squash_index, sync_yanked};
 pub use readmes::render_and_upload_readme;
 pub use update_downloads::update_downloads;