Skip to content

Add deleted_crates database table #9912

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 6 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions crates/crates_io_database/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,26 @@ diesel::table! {
}
}

diesel::table! {
/// Crates that have been deleted by users
deleted_crates (id) {
/// Unique identifier of the `deleted_crates` row
id -> Int4,
/// Name of the deleted crate (use `canon_crate_name()` for normalization, if needed)
name -> Varchar,
/// Date and time when the crate was created
created_at -> Timestamptz,
/// Date and time when the crate was deleted
deleted_at -> Timestamptz,
/// ID of the user who deleted the crate, or NULL if the user was deleted
deleted_by -> Nullable<Int4>,
/// Optional message left by the user who deleted the crate
message -> Nullable<Varchar>,
/// Date and time when users will be able to create a new crate with the same name
available_at -> Timestamptz,
}
}

diesel::table! {
/// Representation of the `dependencies` table.
///
Expand Down Expand Up @@ -1026,6 +1046,7 @@ diesel::joinable!(crates_keywords -> crates (crate_id));
diesel::joinable!(crates_keywords -> keywords (keyword_id));
diesel::joinable!(default_versions -> crates (crate_id));
diesel::joinable!(default_versions -> versions (version_id));
diesel::joinable!(deleted_crates -> users (deleted_by));
diesel::joinable!(dependencies -> crates (crate_id));
diesel::joinable!(dependencies -> versions (version_id));
diesel::joinable!(emails -> users (user_id));
Expand Down Expand Up @@ -1054,6 +1075,7 @@ diesel::allow_tables_to_appear_in_same_query!(
crates_categories,
crates_keywords,
default_versions,
deleted_crates,
dependencies,
emails,
follows,
Expand Down
11 changes: 11 additions & 0 deletions crates/crates_io_database_dump/src/dump-db.toml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,17 @@ dependencies = ["crates", "versions"]
crate_id = "public"
version_id = "public"

[deleted_crates]
dependencies = ["users"]
[deleted_crates.columns]
id = "private"
name = "private"
created_at = "private"
deleted_at = "private"
deleted_by = "private"
message = "private"
available_at = "private"

[dependencies]
dependencies = ["crates", "versions"]
[dependencies.columns]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
drop table deleted_crates;
25 changes: 25 additions & 0 deletions migrations/2024-11-12-125605_create-deleted-crates-table/up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
create table deleted_crates
(
id serial primary key,
name varchar not null,
created_at timestamptz not null,
deleted_at timestamptz not null,
deleted_by integer
constraint deleted_crates_users_id_fk
references users
on delete set null,
message varchar,
available_at timestamptz not null
);

comment on table deleted_crates is 'Crates that have been deleted by users';
comment on column deleted_crates.id is 'Unique identifier of the `deleted_crates` row';
comment on column deleted_crates.name is 'Name of the deleted crate (use `canon_crate_name()` for normalization, if needed)';
comment on column deleted_crates.created_at is 'Date and time when the crate was created';
comment on column deleted_crates.deleted_at is 'Date and time when the crate was deleted';
comment on column deleted_crates.deleted_by is 'ID of the user who deleted the crate, or NULL if the user was deleted';
comment on column deleted_crates.message is 'Optional message left by the user who deleted the crate';
comment on column deleted_crates.available_at is 'Date and time when users will be able to create a new crate with the same name';

create index deleted_crates_canon_crate_name_index
on deleted_crates (canon_crate_name(name));
60 changes: 53 additions & 7 deletions src/bin/crates-admin/delete_crate.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
use crate::dialoguer;
use anyhow::Context;
use chrono::{NaiveDateTime, Utc};
use colored::Colorize;
use crates_io::schema::crate_downloads;
use crates_io::models::{NewDeletedCrate, User};
use crates_io::schema::{crate_downloads, deleted_crates};
use crates_io::worker::jobs;
use crates_io::{db, schema::crates};
use crates_io_worker::BackgroundJob;
use diesel::dsl::sql;
use diesel::expression::SqlLiteral;
use diesel::prelude::*;
use diesel::sql_types::{Array, BigInt, Text};
use diesel_async::RunQueryDsl;
use diesel_async::scoped_futures::ScopedFutureExt;
use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
use std::fmt::Display;

#[derive(clap::Parser, Debug)]
Expand All @@ -26,6 +29,14 @@
/// Don't ask for confirmation: yes, we are sure. Best for scripting.
#[arg(short, long)]
yes: bool,

/// Your GitHub username.
#[arg(long)]
deleted_by: String,

Check warning on line 35 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L35

Added line #L35 was not covered by tests

/// An optional message explaining why the crate was deleted.
#[arg(long)]
message: Option<String>,
}

pub async fn run(opts: Opts) -> anyhow::Result<()> {
Expand All @@ -44,6 +55,10 @@
.await
.context("Failed to look up crate name from the database")?;

let deleted_by = User::async_find_by_login(&mut conn, &opts.deleted_by)
.await
.context("Failed to look up `--deleted-by` user from the database")?;

Check warning on line 60 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L58-L60

Added lines #L58 - L60 were not covered by tests

println!("Deleting the following crates:");
println!();
for name in &crate_names {
Expand All @@ -58,17 +73,29 @@
return Ok(());
}

let now = Utc::now();

Check warning on line 76 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L76

Added line #L76 was not covered by tests

for name in &crate_names {
if let Some(crate_info) = existing_crates.iter().find(|info| info.name == *name) {
let id = crate_info.id;

let created_at = crate_info.created_at.and_utc();
let deleted_crate = NewDeletedCrate::builder(name)
.created_at(&created_at)
.deleted_at(&now)
.deleted_by(deleted_by.id)
.maybe_message(opts.message.as_deref())
.available_at(&now)
.build();

Check warning on line 90 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L82-L90

Added lines #L82 - L90 were not covered by tests
info!("{name}: Deleting crate from the database…");
if let Err(error) = diesel::delete(crates::table.find(id))
.execute(&mut conn)
.await
{
let result = conn
.transaction(|conn| delete_from_database(conn, id, deleted_crate).scope_boxed())
.await;

Check warning on line 94 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L92-L94

Added lines #L92 - L94 were not covered by tests

if let Err(error) = result {

Check warning on line 96 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L96

Added line #L96 was not covered by tests
warn!(%id, "{name}: Failed to delete crate from the database: {error}");
}
};

Check warning on line 98 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L98

Added line #L98 was not covered by tests
} else {
info!("{name}: Skipped missing crate");
};
Expand All @@ -94,12 +121,31 @@
Ok(())
}

async fn delete_from_database(
conn: &mut AsyncPgConnection,
crate_id: i32,
deleted_crate: NewDeletedCrate<'_>,
) -> anyhow::Result<()> {
diesel::delete(crates::table.find(crate_id))
.execute(conn)
.await?;

Check warning on line 131 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L124-L131

Added lines #L124 - L131 were not covered by tests

diesel::insert_into(deleted_crates::table)
.values(deleted_crate)
.execute(conn)
.await?;

Check warning on line 136 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L133-L136

Added lines #L133 - L136 were not covered by tests

Ok(())
}

Check warning on line 139 in src/bin/crates-admin/delete_crate.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/crates-admin/delete_crate.rs#L138-L139

Added lines #L138 - L139 were not covered by tests

#[derive(Debug, Clone, Queryable, Selectable)]
struct CrateInfo {
#[diesel(select_expression = crates::columns::name)]
name: String,
#[diesel(select_expression = crates::columns::id)]
id: i32,
#[diesel(select_expression = crates::columns::created_at)]
created_at: NaiveDateTime,
#[diesel(select_expression = crate_downloads::columns::downloads)]
downloads: i64,
#[diesel(select_expression = owners_subquery())]
Expand Down
17 changes: 17 additions & 0 deletions src/controllers/krate/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::worker::jobs::{
use axum::body::Bytes;
use axum::Json;
use cargo_manifest::{Dependency, DepsSet, TargetDepsSet};
use chrono::{DateTime, SecondsFormat, Utc};
use crates_io_tarball::{process_tarball, TarballError};
use crates_io_worker::BackgroundJob;
use diesel::connection::DefaultLoadingMode;
Expand Down Expand Up @@ -86,6 +87,22 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
let (existing_crate, auth) = {
use diesel_async::RunQueryDsl;

let deleted_crate: Option<(String, DateTime<Utc>)> = deleted_crates::table
.filter(canon_crate_name(deleted_crates::name).eq(canon_crate_name(&metadata.name)))
.filter(deleted_crates::available_at.gt(Utc::now()))
.select((deleted_crates::name, deleted_crates::available_at))
.first(&mut conn)
.await
.optional()?;

if let Some(deleted_crate) = deleted_crate {
return Err(bad_request(format!(
"A crate with the name `{}` was recently deleted. Reuse of this name will be available after {}.",
deleted_crate.0,
deleted_crate.1.to_rfc3339_opts(SecondsFormat::Secs, true)
)));
}

// this query should only be used for the endpoint scope calculation
// since a race condition there would only cause `publish-new` instead of
// `publish-update` to be used.
Expand Down
2 changes: 2 additions & 0 deletions src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ pub use self::action::{insert_version_owner_action, VersionAction, VersionOwnerA
pub use self::category::{Category, CrateCategory, NewCategory};
pub use self::crate_owner_invitation::{CrateOwnerInvitation, NewCrateOwnerInvitationOutcome};
pub use self::default_versions::{update_default_version, verify_default_version};
pub use self::deleted_crate::NewDeletedCrate;
pub use self::dependency::{Dependency, DependencyKind, ReverseDependency};
pub use self::download::VersionDownload;
pub use self::email::{Email, NewEmail};
Expand All @@ -21,6 +22,7 @@ mod action;
pub mod category;
mod crate_owner_invitation;
pub mod default_versions;
mod deleted_crate;
pub mod dependency;
mod download;
mod email;
Expand Down
16 changes: 16 additions & 0 deletions src/models/deleted_crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
use bon::Builder;
use chrono::{DateTime, Utc};
use crates_io_database::schema::deleted_crates;

/// Struct used to `INSERT` a new `deleted_crates` record into the database.
#[derive(Insertable, Debug, Builder)]
#[diesel(table_name = deleted_crates, check_for_backend(diesel::pg::Pg))]
pub struct NewDeletedCrate<'a> {
#[builder(start_fn)]
name: &'a str,
created_at: &'a DateTime<Utc>,
deleted_at: &'a DateTime<Utc>,
deleted_by: Option<i32>,
message: Option<&'a str>,
available_at: &'a DateTime<Utc>,
}
14 changes: 14 additions & 0 deletions src/models/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,20 @@
.first(conn)
}

pub async fn async_find_by_login(
conn: &mut AsyncPgConnection,
login: &str,
) -> QueryResult<User> {

Check warning on line 57 in src/models/user.rs

View check run for this annotation

Codecov / codecov/patch

src/models/user.rs#L54-L57

Added lines #L54 - L57 were not covered by tests
use diesel_async::RunQueryDsl;

users::table
.filter(lower(users::gh_login).eq(login.to_lowercase()))
.filter(users::gh_id.ne(-1))
.order(users::gh_id.desc())
.first(conn)
.await
}

Check warning on line 66 in src/models/user.rs

View check run for this annotation

Codecov / codecov/patch

src/models/user.rs#L60-L66

Added lines #L60 - L66 were not covered by tests

pub fn owning(krate: &Crate, conn: &mut impl Conn) -> QueryResult<Vec<Owner>> {
use diesel::RunQueryDsl;

Expand Down
39 changes: 39 additions & 0 deletions src/tests/krate/publish/deleted_crates.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use crate::models::NewDeletedCrate;
use crate::tests::builders::PublishBuilder;
use crate::tests::util::{RequestHelper, TestApp};
use chrono::{Duration, Utc};
use crates_io_database::schema::deleted_crates;
use diesel_async::RunQueryDsl;
use googletest::prelude::*;
use http::StatusCode;
use insta::assert_snapshot;

#[tokio::test(flavor = "multi_thread")]
async fn test_recently_deleted_crate_with_same_name() -> anyhow::Result<()> {
let (app, _, _, token) = TestApp::full().with_token();
let mut conn = app.async_db_conn().await;

let now = Utc::now();
let created_at = now - Duration::hours(24);
let deleted_at = now - Duration::hours(1);
let available_at = "2099-12-25T12:34:56Z".parse()?;

let deleted_crate = NewDeletedCrate::builder("actix_web")
.created_at(&created_at)
.deleted_at(&deleted_at)
.available_at(&available_at)
.build();

diesel::insert_into(deleted_crates::table)
.values(deleted_crate)
.execute(&mut conn)
.await?;

let crate_to_publish = PublishBuilder::new("actix-web", "1.0.0");
let response = token.publish_crate(crate_to_publish).await;
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"A crate with the name `actix_web` was recently deleted. Reuse of this name will be available after 2099-12-25T12:34:56Z."}]}"#);
assert_that!(app.stored_files().await, empty());

Ok(())
}
1 change: 1 addition & 0 deletions src/tests/krate/publish/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ mod auth;
mod basics;
mod build_metadata;
mod categories;
mod deleted_crates;
mod dependencies;
mod emails;
mod features;
Expand Down