Skip to content

Commit c645ab9

Browse files
Turbo87Turbo87
authored
Merge pull request rust-lang#10047 from Turbo87/default-versions-cleanup
models/default_versions: Remove sync fns
2 parents 5c37bd0 + c888aff commit c645ab9

File tree

6 files changed

+78
-187
lines changed

6 files changed

+78
-187
lines changed

‎src/bin/crates-admin/default_versions.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use anyhow::Context;
2-
use crates_io::models::{async_update_default_version, async_verify_default_version};
2+
use crates_io::models::{update_default_version, verify_default_version};
33
use crates_io::{db, schema::crates};
44
use diesel::prelude::*;
55
use diesel_async::RunQueryDsl;
@@ -33,8 +33,8 @@ pub async fn run(command: Command) -> anyhow::Result<()> {
3333

3434
for crate_id in crate_ids.into_iter().progress_with(pb.clone()) {
3535
let result = match command {
36-
Command::Update => async_update_default_version(crate_id, &mut conn).await,
37-
Command::Verify => async_verify_default_version(crate_id, &mut conn).await,
36+
Command::Update => update_default_version(crate_id, &mut conn).await,
37+
Command::Verify => verify_default_version(crate_id, &mut conn).await,
3838
};
3939

4040
if let Err(error) = result {

‎src/bin/crates-admin/delete_version.rs

Lines changed: 43 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,12 @@ use anyhow::Context;
33
use crates_io::models::update_default_version;
44
use crates_io::schema::crates;
55
use crates_io::storage::Storage;
6-
use crates_io::tasks::spawn_blocking;
76
use crates_io::worker::jobs;
87
use crates_io::{db, schema::versions};
98
use crates_io_worker::BackgroundJob;
10-
use diesel::{Connection, ExpressionMethods, QueryDsl};
11-
use diesel_async::async_connection_wrapper::AsyncConnectionWrapper;
9+
use diesel::prelude::*;
10+
use diesel_async::scoped_futures::ScopedFutureExt;
11+
use diesel_async::{AsyncConnection, RunQueryDsl};
1212

1313
#[derive(clap::Parser, Debug)]
1414
#[command(
@@ -36,17 +36,12 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> {
3636

3737
let store = Storage::from_environment();
3838

39-
let crate_id: i32 = {
40-
use diesel_async::RunQueryDsl;
41-
42-
crates::table
43-
.select(crates::id)
44-
.filter(crates::name.eq(&opts.crate_name))
45-
.first(&mut conn)
46-
.await
47-
.context("Failed to look up crate id from the database")
48-
}?;
49-
39+
let crate_id: i32 = crates::table
40+
.select(crates::id)
41+
.filter(crates::name.eq(&opts.crate_name))
42+
.first(&mut conn)
43+
.await
44+
.context("Failed to look up crate id from the database")?;
5045
{
5146
let crate_name = &opts.crate_name;
5247

@@ -64,57 +59,51 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> {
6459
}
6560
}
6661

67-
let opts = spawn_blocking(move || {
68-
use diesel::RunQueryDsl;
69-
70-
let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into();
71-
62+
let opts = conn.transaction(|conn| async move {
7263
let crate_name = &opts.crate_name;
7364

74-
conn.transaction(|conn| {
75-
info!(%crate_name, %crate_id, versions = ?opts.versions, "Deleting versions from the database");
76-
let result = diesel::delete(
77-
versions::table
78-
.filter(versions::crate_id.eq(crate_id))
79-
.filter(versions::num.eq_any(&opts.versions)),
80-
)
81-
.execute(conn);
82-
83-
match result {
84-
Ok(num_deleted) if num_deleted == opts.versions.len() => {}
85-
Ok(num_deleted) => {
86-
warn!(
87-
%crate_name,
88-
"Deleted only {num_deleted} of {num_expected} versions from the database",
89-
num_expected = opts.versions.len()
90-
);
91-
}
92-
Err(error) => {
93-
warn!(%crate_name, ?error, "Failed to delete versions from the database")
94-
}
65+
info!(%crate_name, %crate_id, versions = ?opts.versions, "Deleting versions from the database");
66+
let result = diesel::delete(
67+
versions::table
68+
.filter(versions::crate_id.eq(crate_id))
69+
.filter(versions::num.eq_any(&opts.versions)),
70+
)
71+
.execute(conn).await;
72+
73+
match result {
74+
Ok(num_deleted) if num_deleted == opts.versions.len() => {}
75+
Ok(num_deleted) => {
76+
warn!(
77+
%crate_name,
78+
"Deleted only {num_deleted} of {num_expected} versions from the database",
79+
num_expected = opts.versions.len()
80+
);
9581
}
96-
97-
info!(%crate_name, %crate_id, "Updating default version in the database");
98-
if let Err(error) = update_default_version(crate_id, conn) {
99-
warn!(%crate_name, %crate_id, ?error, "Failed to update default version");
82+
Err(error) => {
83+
warn!(%crate_name, ?error, "Failed to delete versions from the database")
10084
}
101-
102-
Ok::<_, anyhow::Error>(())
103-
})?;
104-
105-
info!(%crate_name, "Enqueuing index sync jobs");
106-
if let Err(error) = jobs::SyncToGitIndex::new(crate_name).enqueue(conn) {
107-
warn!(%crate_name, ?error, "Failed to enqueue SyncToGitIndex job");
10885
}
109-
if let Err(error) = jobs::SyncToSparseIndex::new(crate_name).enqueue(conn) {
110-
warn!(%crate_name, ?error, "Failed to enqueue SyncToSparseIndex job");
86+
87+
info!(%crate_name, %crate_id, "Updating default version in the database");
88+
if let Err(error) = update_default_version(crate_id, conn).await {
89+
warn!(%crate_name, %crate_id, ?error, "Failed to update default version");
11190
}
11291

11392
Ok::<_, anyhow::Error>(opts)
114-
}).await??;
93+
}.scope_boxed()).await?;
11594

11695
let crate_name = &opts.crate_name;
11796

97+
info!(%crate_name, "Enqueuing index sync jobs");
98+
let job = jobs::SyncToGitIndex::new(crate_name);
99+
if let Err(error) = job.async_enqueue(&mut conn).await {
100+
warn!(%crate_name, ?error, "Failed to enqueue SyncToGitIndex job");
101+
}
102+
let job = jobs::SyncToSparseIndex::new(crate_name);
103+
if let Err(error) = job.async_enqueue(&mut conn).await {
104+
warn!(%crate_name, ?error, "Failed to enqueue SyncToSparseIndex job");
105+
}
106+
118107
for version in &opts.versions {
119108
debug!(%crate_name, %version, "Deleting crate file from S3");
120109
if let Err(error) = store.delete_crate_file(crate_name, version).await {

‎src/models.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
pub use self::action::{NewVersionOwnerAction, VersionAction, VersionOwnerAction};
22
pub use self::category::{Category, CrateCategory, NewCategory};
33
pub use self::crate_owner_invitation::{CrateOwnerInvitation, NewCrateOwnerInvitationOutcome};
4-
pub use self::default_versions::{
5-
async_update_default_version, async_verify_default_version, update_default_version,
6-
verify_default_version,
7-
};
4+
pub use self::default_versions::{update_default_version, verify_default_version};
85
pub use self::deleted_crate::NewDeletedCrate;
96
pub use self::dependency::{Dependency, DependencyKind, ReverseDependency};
107
pub use self::download::VersionDownload;

‎src/models/default_versions.rs

Lines changed: 25 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
use crate::schema::{default_versions, versions};
22
use crate::sql::SemverVersion;
3-
use crate::util::diesel::prelude::*;
4-
use crate::util::diesel::Conn;
5-
use diesel_async::AsyncPgConnection;
3+
use diesel::prelude::*;
4+
use diesel_async::{AsyncPgConnection, RunQueryDsl};
65

76
/// A subset of the columns of the `versions` table.
87
///
@@ -57,13 +56,11 @@ impl Ord for Version {
5756
///
5857
/// The default version is then written to the `default_versions` table.
5958
#[instrument(skip(conn))]
60-
pub async fn async_update_default_version(
59+
pub async fn update_default_version(
6160
crate_id: i32,
6261
conn: &mut AsyncPgConnection,
6362
) -> QueryResult<()> {
64-
use diesel_async::RunQueryDsl;
65-
66-
let default_version = async_calculate_default_version(crate_id, conn).await?;
63+
let default_version = calculate_default_version(crate_id, conn).await?;
6764

6865
debug!(
6966
"Updating default version to {} (id: {})…",
@@ -84,49 +81,13 @@ pub async fn async_update_default_version(
8481
Ok(())
8582
}
8683

87-
/// Updates the `default_versions` table entry for the specified crate.
88-
///
89-
/// This function first loads all versions of the crate from the database,
90-
/// then determines the default version based on the following criteria:
91-
///
92-
/// 1. The highest non-prerelease version that is not yanked.
93-
/// 2. The highest non-yanked version.
94-
/// 3. The highest version.
95-
///
96-
/// The default version is then written to the `default_versions` table.
97-
#[instrument(skip(conn))]
98-
pub fn update_default_version(crate_id: i32, conn: &mut impl Conn) -> QueryResult<()> {
99-
use diesel::RunQueryDsl;
100-
101-
let default_version = calculate_default_version(crate_id, conn)?;
102-
103-
debug!(
104-
"Updating default version to {} (id: {})…",
105-
default_version.num, default_version.id
106-
);
107-
108-
diesel::insert_into(default_versions::table)
109-
.values((
110-
default_versions::crate_id.eq(crate_id),
111-
default_versions::version_id.eq(default_version.id),
112-
))
113-
.on_conflict(default_versions::crate_id)
114-
.do_update()
115-
.set(default_versions::version_id.eq(default_version.id))
116-
.execute(conn)?;
117-
118-
Ok(())
119-
}
120-
12184
/// Verifies that the default version for the specified crate is up-to-date.
12285
#[instrument(skip(conn))]
123-
pub async fn async_verify_default_version(
86+
pub async fn verify_default_version(
12487
crate_id: i32,
12588
conn: &mut AsyncPgConnection,
12689
) -> QueryResult<()> {
127-
use diesel_async::RunQueryDsl;
128-
129-
let calculated = async_calculate_default_version(crate_id, conn).await?;
90+
let calculated = calculate_default_version(crate_id, conn).await?;
13091

13192
let saved = default_versions::table
13293
.select(default_versions::version_id)
@@ -154,44 +115,11 @@ pub async fn async_verify_default_version(
154115
Ok(())
155116
}
156117

157-
/// Verifies that the default version for the specified crate is up-to-date.
158-
#[instrument(skip(conn))]
159-
pub fn verify_default_version(crate_id: i32, conn: &mut impl Conn) -> QueryResult<()> {
160-
use diesel::RunQueryDsl;
161-
162-
let calculated = calculate_default_version(crate_id, conn)?;
163-
164-
let saved = default_versions::table
165-
.select(default_versions::version_id)
166-
.filter(default_versions::crate_id.eq(crate_id))
167-
.first::<i32>(conn)
168-
.optional()?;
169-
170-
if let Some(saved) = saved {
171-
if saved == calculated.id {
172-
debug!("Default version for crate {crate_id} is up to date");
173-
} else {
174-
warn!(
175-
"Default version for crate {crate_id} is outdated (expected: {saved}, actual: {})",
176-
calculated.id,
177-
);
178-
}
179-
} else {
180-
warn!(
181-
"Default version for crate {crate_id} is missing (expected: {})",
182-
calculated.id
183-
);
184-
}
185-
186-
Ok(())
187-
}
188-
189-
async fn async_calculate_default_version(
118+
async fn calculate_default_version(
190119
crate_id: i32,
191120
conn: &mut AsyncPgConnection,
192121
) -> QueryResult<Version> {
193122
use diesel::result::Error::NotFound;
194-
use diesel_async::RunQueryDsl;
195123

196124
debug!("Loading all versions for the crate…");
197125
let versions = versions::table
@@ -205,21 +133,6 @@ async fn async_calculate_default_version(
205133
versions.into_iter().max().ok_or(NotFound)
206134
}
207135

208-
fn calculate_default_version(crate_id: i32, conn: &mut impl Conn) -> QueryResult<Version> {
209-
use diesel::result::Error::NotFound;
210-
use diesel::RunQueryDsl;
211-
212-
debug!("Loading all versions for the crate…");
213-
let versions = versions::table
214-
.filter(versions::crate_id.eq(crate_id))
215-
.select(Version::as_returning())
216-
.load::<Version>(conn)?;
217-
218-
debug!("Found {} versions", versions.len());
219-
220-
versions.into_iter().max().ok_or(NotFound)
221-
}
222-
223136
#[cfg(test)]
224137
mod tests {
225138
use super::*;
@@ -331,19 +244,16 @@ mod tests {
331244
buf
332245
}
333246

334-
fn create_crate(name: &str, conn: &mut impl Conn) -> i32 {
335-
use diesel::RunQueryDsl;
336-
247+
async fn create_crate(name: &str, conn: &mut AsyncPgConnection) -> i32 {
337248
diesel::insert_into(crates::table)
338249
.values(crates::name.eq(name))
339250
.returning(crates::id)
340251
.get_result(conn)
252+
.await
341253
.unwrap()
342254
}
343255

344-
fn create_version(crate_id: i32, num: &str, conn: &mut impl Conn) {
345-
use diesel::RunQueryDsl;
346-
256+
async fn create_version(crate_id: i32, num: &str, conn: &mut AsyncPgConnection) {
347257
diesel::insert_into(versions::table)
348258
.values((
349259
versions::crate_id.eq(crate_id),
@@ -353,36 +263,36 @@ mod tests {
353263
versions::crate_size.eq(0),
354264
))
355265
.execute(conn)
266+
.await
356267
.unwrap();
357268
}
358269

359-
fn get_default_version(crate_id: i32, conn: &mut impl Conn) -> String {
360-
use diesel::RunQueryDsl;
361-
270+
async fn get_default_version(crate_id: i32, conn: &mut AsyncPgConnection) -> String {
362271
default_versions::table
363272
.inner_join(versions::table)
364273
.select(versions::num)
365274
.filter(default_versions::crate_id.eq(crate_id))
366275
.first(conn)
276+
.await
367277
.unwrap()
368278
}
369279

370-
#[test]
371-
fn test_update_default_version() {
280+
#[tokio::test]
281+
async fn test_update_default_version() {
372282
let test_db = TestDatabase::new();
373-
let conn = &mut test_db.connect();
283+
let conn = &mut test_db.async_connect().await;
374284

375-
let crate_id = create_crate("foo", conn);
376-
create_version(crate_id, "1.0.0", conn);
285+
let crate_id = create_crate("foo", conn).await;
286+
create_version(crate_id, "1.0.0", conn).await;
377287

378-
update_default_version(crate_id, conn).unwrap();
379-
assert_eq!(get_default_version(crate_id, conn), "1.0.0");
288+
update_default_version(crate_id, conn).await.unwrap();
289+
assert_eq!(get_default_version(crate_id, conn).await, "1.0.0");
380290

381-
create_version(crate_id, "1.1.0", conn);
382-
create_version(crate_id, "1.0.1", conn);
383-
assert_eq!(get_default_version(crate_id, conn), "1.0.0");
291+
create_version(crate_id, "1.1.0", conn).await;
292+
create_version(crate_id, "1.0.1", conn).await;
293+
assert_eq!(get_default_version(crate_id, conn).await, "1.0.0");
384294

385-
update_default_version(crate_id, conn).unwrap();
386-
assert_eq!(get_default_version(crate_id, conn), "1.1.0");
295+
update_default_version(crate_id, conn).await.unwrap();
296+
assert_eq!(get_default_version(crate_id, conn).await, "1.1.0");
387297
}
388298
}

0 commit comments

Comments
 (0)