Skip to content

Commit 9c7b5e7

Browse files
authored
Merge pull request #9912 from Turbo87/deleted-crates-table
Add `deleted_crates` database table
2 parents 97e03ab + 31f08b3 commit 9c7b5e7

File tree

11 files changed

+201
-7
lines changed

11 files changed

+201
-7
lines changed

crates/crates_io_database/src/schema.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,26 @@ diesel::table! {
411411
}
412412
}
413413

414+
diesel::table! {
415+
/// Crates that have been deleted by users
416+
deleted_crates (id) {
417+
/// Unique identifier of the `deleted_crates` row
418+
id -> Int4,
419+
/// Name of the deleted crate (use `canon_crate_name()` for normalization, if needed)
420+
name -> Varchar,
421+
/// Date and time when the crate was created
422+
created_at -> Timestamptz,
423+
/// Date and time when the crate was deleted
424+
deleted_at -> Timestamptz,
425+
/// ID of the user who deleted the crate, or NULL if the user was deleted
426+
deleted_by -> Nullable<Int4>,
427+
/// Optional message left by the user who deleted the crate
428+
message -> Nullable<Varchar>,
429+
/// Date and time when users will be able to create a new crate with the same name
430+
available_at -> Timestamptz,
431+
}
432+
}
433+
414434
diesel::table! {
415435
/// Representation of the `dependencies` table.
416436
///
@@ -1026,6 +1046,7 @@ diesel::joinable!(crates_keywords -> crates (crate_id));
10261046
diesel::joinable!(crates_keywords -> keywords (keyword_id));
10271047
diesel::joinable!(default_versions -> crates (crate_id));
10281048
diesel::joinable!(default_versions -> versions (version_id));
1049+
diesel::joinable!(deleted_crates -> users (deleted_by));
10291050
diesel::joinable!(dependencies -> crates (crate_id));
10301051
diesel::joinable!(dependencies -> versions (version_id));
10311052
diesel::joinable!(emails -> users (user_id));
@@ -1054,6 +1075,7 @@ diesel::allow_tables_to_appear_in_same_query!(
10541075
crates_categories,
10551076
crates_keywords,
10561077
default_versions,
1078+
deleted_crates,
10571079
dependencies,
10581080
emails,
10591081
follows,

crates/crates_io_database_dump/src/dump-db.toml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,17 @@ dependencies = ["crates", "versions"]
106106
crate_id = "public"
107107
version_id = "public"
108108

109+
[deleted_crates]
110+
dependencies = ["users"]
111+
[deleted_crates.columns]
112+
id = "private"
113+
name = "private"
114+
created_at = "private"
115+
deleted_at = "private"
116+
deleted_by = "private"
117+
message = "private"
118+
available_at = "private"
119+
109120
[dependencies]
110121
dependencies = ["crates", "versions"]
111122
[dependencies.columns]
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
drop table deleted_crates;
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
create table deleted_crates
2+
(
3+
id serial primary key,
4+
name varchar not null,
5+
created_at timestamptz not null,
6+
deleted_at timestamptz not null,
7+
deleted_by integer
8+
constraint deleted_crates_users_id_fk
9+
references users
10+
on delete set null,
11+
message varchar,
12+
available_at timestamptz not null
13+
);
14+
15+
comment on table deleted_crates is 'Crates that have been deleted by users';
16+
comment on column deleted_crates.id is 'Unique identifier of the `deleted_crates` row';
17+
comment on column deleted_crates.name is 'Name of the deleted crate (use `canon_crate_name()` for normalization, if needed)';
18+
comment on column deleted_crates.created_at is 'Date and time when the crate was created';
19+
comment on column deleted_crates.deleted_at is 'Date and time when the crate was deleted';
20+
comment on column deleted_crates.deleted_by is 'ID of the user who deleted the crate, or NULL if the user was deleted';
21+
comment on column deleted_crates.message is 'Optional message left by the user who deleted the crate';
22+
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';
23+
24+
create index deleted_crates_canon_crate_name_index
25+
on deleted_crates (canon_crate_name(name));

src/bin/crates-admin/delete_crate.rs

Lines changed: 53 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
use crate::dialoguer;
22
use anyhow::Context;
3+
use chrono::{NaiveDateTime, Utc};
34
use colored::Colorize;
4-
use crates_io::schema::crate_downloads;
5+
use crates_io::models::{NewDeletedCrate, User};
6+
use crates_io::schema::{crate_downloads, deleted_crates};
57
use crates_io::worker::jobs;
68
use crates_io::{db, schema::crates};
79
use crates_io_worker::BackgroundJob;
810
use diesel::dsl::sql;
911
use diesel::expression::SqlLiteral;
1012
use diesel::prelude::*;
1113
use diesel::sql_types::{Array, BigInt, Text};
12-
use diesel_async::RunQueryDsl;
14+
use diesel_async::scoped_futures::ScopedFutureExt;
15+
use diesel_async::{AsyncConnection, AsyncPgConnection, RunQueryDsl};
1316
use std::fmt::Display;
1417

1518
#[derive(clap::Parser, Debug)]
@@ -26,6 +29,14 @@ pub struct Opts {
2629
/// Don't ask for confirmation: yes, we are sure. Best for scripting.
2730
#[arg(short, long)]
2831
yes: bool,
32+
33+
/// Your GitHub username.
34+
#[arg(long)]
35+
deleted_by: String,
36+
37+
/// An optional message explaining why the crate was deleted.
38+
#[arg(long)]
39+
message: Option<String>,
2940
}
3041

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

58+
let deleted_by = User::async_find_by_login(&mut conn, &opts.deleted_by)
59+
.await
60+
.context("Failed to look up `--deleted-by` user from the database")?;
61+
4762
println!("Deleting the following crates:");
4863
println!();
4964
for name in &crate_names {
@@ -58,17 +73,29 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> {
5873
return Ok(());
5974
}
6075

76+
let now = Utc::now();
77+
6178
for name in &crate_names {
6279
if let Some(crate_info) = existing_crates.iter().find(|info| info.name == *name) {
6380
let id = crate_info.id;
6481

82+
let created_at = crate_info.created_at.and_utc();
83+
let deleted_crate = NewDeletedCrate::builder(name)
84+
.created_at(&created_at)
85+
.deleted_at(&now)
86+
.deleted_by(deleted_by.id)
87+
.maybe_message(opts.message.as_deref())
88+
.available_at(&now)
89+
.build();
90+
6591
info!("{name}: Deleting crate from the database…");
66-
if let Err(error) = diesel::delete(crates::table.find(id))
67-
.execute(&mut conn)
68-
.await
69-
{
92+
let result = conn
93+
.transaction(|conn| delete_from_database(conn, id, deleted_crate).scope_boxed())
94+
.await;
95+
96+
if let Err(error) = result {
7097
warn!(%id, "{name}: Failed to delete crate from the database: {error}");
71-
}
98+
};
7299
} else {
73100
info!("{name}: Skipped missing crate");
74101
};
@@ -94,12 +121,31 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> {
94121
Ok(())
95122
}
96123

124+
async fn delete_from_database(
125+
conn: &mut AsyncPgConnection,
126+
crate_id: i32,
127+
deleted_crate: NewDeletedCrate<'_>,
128+
) -> anyhow::Result<()> {
129+
diesel::delete(crates::table.find(crate_id))
130+
.execute(conn)
131+
.await?;
132+
133+
diesel::insert_into(deleted_crates::table)
134+
.values(deleted_crate)
135+
.execute(conn)
136+
.await?;
137+
138+
Ok(())
139+
}
140+
97141
#[derive(Debug, Clone, Queryable, Selectable)]
98142
struct CrateInfo {
99143
#[diesel(select_expression = crates::columns::name)]
100144
name: String,
101145
#[diesel(select_expression = crates::columns::id)]
102146
id: i32,
147+
#[diesel(select_expression = crates::columns::created_at)]
148+
created_at: NaiveDateTime,
103149
#[diesel(select_expression = crate_downloads::columns::downloads)]
104150
downloads: i64,
105151
#[diesel(select_expression = owners_subquery())]

src/controllers/krate/publish.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use crate::worker::jobs::{
99
use axum::body::Bytes;
1010
use axum::Json;
1111
use cargo_manifest::{Dependency, DepsSet, TargetDepsSet};
12+
use chrono::{DateTime, SecondsFormat, Utc};
1213
use crates_io_tarball::{process_tarball, TarballError};
1314
use crates_io_worker::BackgroundJob;
1415
use diesel::connection::DefaultLoadingMode;
@@ -86,6 +87,22 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
8687
let (existing_crate, auth) = {
8788
use diesel_async::RunQueryDsl;
8889

90+
let deleted_crate: Option<(String, DateTime<Utc>)> = deleted_crates::table
91+
.filter(canon_crate_name(deleted_crates::name).eq(canon_crate_name(&metadata.name)))
92+
.filter(deleted_crates::available_at.gt(Utc::now()))
93+
.select((deleted_crates::name, deleted_crates::available_at))
94+
.first(&mut conn)
95+
.await
96+
.optional()?;
97+
98+
if let Some(deleted_crate) = deleted_crate {
99+
return Err(bad_request(format!(
100+
"A crate with the name `{}` was recently deleted. Reuse of this name will be available after {}.",
101+
deleted_crate.0,
102+
deleted_crate.1.to_rfc3339_opts(SecondsFormat::Secs, true)
103+
)));
104+
}
105+
89106
// this query should only be used for the endpoint scope calculation
90107
// since a race condition there would only cause `publish-new` instead of
91108
// `publish-update` to be used.

src/models.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ pub use self::action::{insert_version_owner_action, VersionAction, VersionOwnerA
22
pub use self::category::{Category, CrateCategory, NewCategory};
33
pub use self::crate_owner_invitation::{CrateOwnerInvitation, NewCrateOwnerInvitationOutcome};
44
pub use self::default_versions::{update_default_version, verify_default_version};
5+
pub use self::deleted_crate::NewDeletedCrate;
56
pub use self::dependency::{Dependency, DependencyKind, ReverseDependency};
67
pub use self::download::VersionDownload;
78
pub use self::email::{Email, NewEmail};
@@ -21,6 +22,7 @@ mod action;
2122
pub mod category;
2223
mod crate_owner_invitation;
2324
pub mod default_versions;
25+
mod deleted_crate;
2426
pub mod dependency;
2527
mod download;
2628
mod email;

src/models/deleted_crate.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
use bon::Builder;
2+
use chrono::{DateTime, Utc};
3+
use crates_io_database::schema::deleted_crates;
4+
5+
/// Struct used to `INSERT` a new `deleted_crates` record into the database.
6+
#[derive(Insertable, Debug, Builder)]
7+
#[diesel(table_name = deleted_crates, check_for_backend(diesel::pg::Pg))]
8+
pub struct NewDeletedCrate<'a> {
9+
#[builder(start_fn)]
10+
name: &'a str,
11+
created_at: &'a DateTime<Utc>,
12+
deleted_at: &'a DateTime<Utc>,
13+
deleted_by: Option<i32>,
14+
message: Option<&'a str>,
15+
available_at: &'a DateTime<Utc>,
16+
}

src/models/user.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,20 @@ impl User {
4646
.first(conn)
4747
}
4848

49+
pub async fn async_find_by_login(
50+
conn: &mut AsyncPgConnection,
51+
login: &str,
52+
) -> QueryResult<User> {
53+
use diesel_async::RunQueryDsl;
54+
55+
users::table
56+
.filter(lower(users::gh_login).eq(login.to_lowercase()))
57+
.filter(users::gh_id.ne(-1))
58+
.order(users::gh_id.desc())
59+
.first(conn)
60+
.await
61+
}
62+
4963
pub fn owning(krate: &Crate, conn: &mut impl Conn) -> QueryResult<Vec<Owner>> {
5064
use diesel::RunQueryDsl;
5165

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
use crate::models::NewDeletedCrate;
2+
use crate::tests::builders::PublishBuilder;
3+
use crate::tests::util::{RequestHelper, TestApp};
4+
use chrono::{Duration, Utc};
5+
use crates_io_database::schema::deleted_crates;
6+
use diesel_async::RunQueryDsl;
7+
use googletest::prelude::*;
8+
use http::StatusCode;
9+
use insta::assert_snapshot;
10+
11+
#[tokio::test(flavor = "multi_thread")]
12+
async fn test_recently_deleted_crate_with_same_name() -> anyhow::Result<()> {
13+
let (app, _, _, token) = TestApp::full().with_token();
14+
let mut conn = app.async_db_conn().await;
15+
16+
let now = Utc::now();
17+
let created_at = now - Duration::hours(24);
18+
let deleted_at = now - Duration::hours(1);
19+
let available_at = "2099-12-25T12:34:56Z".parse()?;
20+
21+
let deleted_crate = NewDeletedCrate::builder("actix_web")
22+
.created_at(&created_at)
23+
.deleted_at(&deleted_at)
24+
.available_at(&available_at)
25+
.build();
26+
27+
diesel::insert_into(deleted_crates::table)
28+
.values(deleted_crate)
29+
.execute(&mut conn)
30+
.await?;
31+
32+
let crate_to_publish = PublishBuilder::new("actix-web", "1.0.0");
33+
let response = token.publish_crate(crate_to_publish).await;
34+
assert_eq!(response.status(), StatusCode::BAD_REQUEST);
35+
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."}]}"#);
36+
assert_that!(app.stored_files().await, empty());
37+
38+
Ok(())
39+
}

0 commit comments

Comments
 (0)