Merge pull request #8484 from Turbo87/default-versions-table

Add `default_versions` database table

migrations/2024-04-17-135931_default-versions-table/down.sql

@@ -0,0 +1 @@
+drop table default_versions;

migrations/2024-04-17-135931_default-versions-table/up.sql

@@ -0,0 +1,24 @@
+create table default_versions
+(
+    crate_id   integer not null
+        constraint default_versions_pk
+            primary key
+        constraint default_versions_crates_id_fk
+            references crates
+            on delete cascade,
+    version_id integer not null
+        constraint default_versions_versions_id_fk
+            references versions
+            on delete no action deferrable initially deferred
+);
+
+create unique index default_versions_version_id_uindex
+    on default_versions (version_id);
+
+comment on table default_versions is 'A mapping from crates to the versions that the frontend will display by default.';
+comment on column default_versions.crate_id is 'Reference to the crate in the `crates` table.';
+comment on column default_versions.version_id is 'Reference to the version in the `versions` table.';
+comment on constraint default_versions_crates_id_fk on default_versions is
+    'This is a `cascade` delete because we want to delete the row when the crate is deleted.';
+comment on constraint default_versions_versions_id_fk on default_versions is
+    'This is a `no action` delete because we want to fail the version deletion unless the default version is updated in the same transaction.';

src/admin/delete_version.rs

@@ -1,3 +1,4 @@
+use crate::models::update_default_version;
 use crate::schema::crates;
 use crate::storage::Storage;
 use crate::worker::jobs;
@@ -48,27 +49,36 @@ pub fn run(opts: Opts) -> anyhow::Result<()> {
         return Ok(());
     }
 
-    info!(%crate_name, %crate_id, versions = ?opts.versions, "Deleting versions from the database");
-    let result = diesel::delete(
-        versions::table
-            .filter(versions::crate_id.eq(crate_id))
-            .filter(versions::num.eq_any(&opts.versions)),
-    )
-    .execute(conn);
-
-    match result {
-        Ok(num_deleted) if num_deleted == opts.versions.len() => {}
-        Ok(num_deleted) => {
-            warn!(
-                %crate_name,
-                "Deleted only {num_deleted} of {num_expected} versions from the database",
-                num_expected = opts.versions.len()
-            );
+    conn.transaction(|conn| {
+        info!(%crate_name, %crate_id, versions = ?opts.versions, "Deleting versions from the database");
+        let result = diesel::delete(
+            versions::table
+                .filter(versions::crate_id.eq(crate_id))
+                .filter(versions::num.eq_any(&opts.versions)),
+        )
+        .execute(conn);
+
+        match result {
+            Ok(num_deleted) if num_deleted == opts.versions.len() => {}
+            Ok(num_deleted) => {
+                warn!(
+                    %crate_name,
+                    "Deleted only {num_deleted} of {num_expected} versions from the database",
+                    num_expected = opts.versions.len()
+                );
+            }
+            Err(error) => {
+                warn!(%crate_name, ?error, "Failed to delete versions from the database")
+            }
         }
-        Err(error) => {
-            warn!(%crate_name, ?error, "Failed to delete versions from the database")
+
+        info!(%crate_name, %crate_id, "Updating default version in the database");
+        if let Err(error) = update_default_version(crate_id, conn) {
+            warn!(%crate_name, %crate_id, ?error, "Failed to update default version");
         }
-    }
+
+        Ok::<_, anyhow::Error>(())
+    })?;
 
     info!(%crate_name, "Enqueuing index sync jobs");
     if let Err(error) = jobs::enqueue_sync_to_index(crate_name, conn) {

src/admin/mod.rs

@@ -9,6 +9,7 @@ pub mod populate;
 pub mod render_readmes;
 pub mod test_pagerduty;
 pub mod transfer_crates;
+pub mod update_default_versions;
 pub mod upload_index;
 pub mod verify_token;
 pub mod yank_version;

src/admin/update_default_versions.rs

@@ -0,0 +1,34 @@
+use crate::models::update_default_version;
+use crate::{db, schema::crates};
+use anyhow::Context;
+use diesel::prelude::*;
+use indicatif::{ProgressBar, ProgressIterator, ProgressStyle};
+
+#[derive(clap::Parser, Debug)]
+#[clap(
+    name = "update-default-versions",
+    about = "Iterates over every crate ever uploaded and updates the `default_versions` table."
+)]
+pub struct Opts;
+
+pub fn run(_opts: Opts) -> anyhow::Result<()> {
+    let mut conn = db::oneoff_connection().context("Failed to connect to the database")?;
+
+    let crate_ids: Vec<i32> = crates::table
+        .select(crates::id)
+        .load(&mut conn)
+        .context("Failed to load crates")?;
+
+    let pb = ProgressBar::new(crate_ids.len() as u64);
+    pb.set_style(ProgressStyle::with_template(
+        "{bar:60} ({pos}/{len}, ETA {eta})",
+    )?);
+
+    for crate_id in crate_ids.into_iter().progress_with(pb.clone()) {
+        if let Err(error) = update_default_version(crate_id, &mut conn) {
+            pb.suspend(|| warn!(%crate_id, %error, "Failed to update the default version"));
+        }
+    }
+
+    Ok(())
+}

src/admin/yank_version.rs

@@ -3,6 +3,8 @@ use crate::db;
 use crate::models::{Crate, Version};
 use crate::schema::versions;
 use crate::worker::jobs;
+use crate::worker::jobs::UpdateDefaultVersion;
+use crates_io_worker::BackgroundJob;
 use diesel::prelude::*;
 
 #[derive(clap::Parser, Debug)]
@@ -59,5 +61,7 @@ fn yank(opts: Opts, conn: &mut PgConnection) -> anyhow::Result<()> {
 
     jobs::enqueue_sync_to_index(&krate.name, conn)?;
 
+    UpdateDefaultVersion::new(krate.id).enqueue(conn)?;
+
     Ok(())
 }

src/bin/crates-admin.rs

@@ -3,7 +3,8 @@ extern crate tracing;
 
 use crates_io::admin::{
     delete_crate, delete_version, enqueue_job, git_import, migrate, populate, render_readmes,
-    test_pagerduty, transfer_crates, upload_index, verify_token, yank_version,
+    test_pagerduty, transfer_crates, update_default_versions, upload_index, verify_token,
+    yank_version,
 };
 
 #[derive(clap::Parser, Debug)]
@@ -22,6 +23,7 @@ enum Command {
     GitImport(git_import::Opts),
     #[clap(subcommand)]
     EnqueueJob(enqueue_job::Command),
+    UpdateDefaultVersions(update_default_versions::Opts),
 }
 
 fn main() -> anyhow::Result<()> {
@@ -49,6 +51,7 @@ fn main() -> anyhow::Result<()> {
         Command::YankVersion(opts) => yank_version::run(opts),
         Command::GitImport(opts) => git_import::run(opts),
         Command::EnqueueJob(command) => enqueue_job::run(command),
+        Command::UpdateDefaultVersions(opts) => update_default_versions::run(opts),
     }
 }
 

src/controllers/krate/publish.rs

@@ -1,7 +1,7 @@
 //! Functionality related to publishing a new crate or version of a crate.
 
 use crate::auth::AuthCheck;
-use crate::worker::jobs::{self, CheckTyposquat};
+use crate::worker::jobs::{self, CheckTyposquat, UpdateDefaultVersion};
 use axum::body::Bytes;
 use cargo_manifest::{Dependency, DepsSet, TargetDepsSet};
 use crates_io_tarball::{process_tarball, TarballError};
@@ -360,6 +360,18 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
             // Link this new version to all dependencies
             add_dependencies(conn, &deps, version.id)?;
 
+            // Insert the default version if it doesn't already exist. Compared
+            // to only using a background job, this prevents us from getting
+            // into a situation where a crate exists in the `crates` table but
+            // doesn't have a default version in the `default_versions` table.
+            let inserted_default_versions = diesel::insert_into(default_versions::table)
+                .values((
+                    default_versions::crate_id.eq(krate.id),
+                    default_versions::version_id.eq(version.id),
+                ))
+                .on_conflict_do_nothing()
+                .execute(conn)?;
+
             // Update all keywords for this crate
             Keyword::update_crate(conn, &krate, &keywords)?;
 
@@ -401,6 +413,12 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
 
             jobs::enqueue_sync_to_index(&krate.name, conn)?;
 
+            // If this is a new version for an existing crate it is sufficient
+            // to update the default version asynchronously in a background job.
+            if inserted_default_versions == 0 {
+                UpdateDefaultVersion::new(krate.id).enqueue(conn)?;
+            }
+
             // Experiment: check new crates for potential typosquatting.
             if existing_crate.is_none() {
                 CheckTyposquat::new(&krate.name).enqueue(conn)?;

src/controllers/version/yank.rs

@@ -10,6 +10,8 @@ use crate::rate_limiter::LimitedAction;
 use crate::schema::versions;
 use crate::util::errors::{custom, version_not_found};
 use crate::worker::jobs;
+use crate::worker::jobs::UpdateDefaultVersion;
+use crates_io_worker::BackgroundJob;
 use tokio::runtime::Handle;
 
 /// Handles the `DELETE /crates/:crate_id/:version/yank` route.
@@ -103,6 +105,8 @@ async fn modify_yank(
 
         jobs::enqueue_sync_to_index(&krate.name, conn)?;
 
+        UpdateDefaultVersion::new(krate.id).enqueue(conn)?;
+
         ok_true()
     })
     .await?

src/models.rs

@@ -1,6 +1,7 @@
 pub use self::action::{insert_version_owner_action, VersionAction, VersionOwnerAction};
 pub use self::category::{Category, CrateCategory, NewCategory};
 pub use self::crate_owner_invitation::{CrateOwnerInvitation, NewCrateOwnerInvitationOutcome};
+pub use self::default_versions::update_default_version;
 pub use self::dependency::{Dependency, DependencyKind, ReverseDependency};
 pub use self::download::VersionDownload;
 pub use self::email::{Email, NewEmail};
@@ -19,6 +20,7 @@ pub mod helpers;
 mod action;
 pub mod category;
 mod crate_owner_invitation;
+mod default_versions;
 pub mod dependency;
 mod download;
 mod email;