Skip to content

Add default_versions database table #8484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 25, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
drop table default_versions;
24 changes: 24 additions & 0 deletions migrations/2024-04-17-135931_default-versions-table/up.sql
Original file line number Diff line number Diff line change
@@ -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.';
48 changes: 29 additions & 19 deletions src/admin/delete_version.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::models::update_default_version;
use crate::schema::crates;
use crate::storage::Storage;
use crate::worker::jobs;
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions src/admin/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
34 changes: 34 additions & 0 deletions src/admin/update_default_versions.rs
Original file line number Diff line number Diff line change
@@ -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(())
}
4 changes: 4 additions & 0 deletions src/admin/yank_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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(())
}
5 changes: 4 additions & 1 deletion src/bin/crates-admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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<()> {
Expand Down Expand Up @@ -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),
}
}

Expand Down
20 changes: 19 additions & 1 deletion src/controllers/krate/publish.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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)?;

Expand Down Expand Up @@ -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)?;
Expand Down
4 changes: 4 additions & 0 deletions src/controllers/version/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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?
Expand Down
2 changes: 2 additions & 0 deletions src/models.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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;
Expand Down
Loading