Skip to content

Commit 338ff27

Browse files
committed
controllers/krate/publish: Always upsert the default_value
The value is determined by comparing the existing `default_value` to the published version. This could potentially result in upserting an outdated version. A subsequent background job is necessary to ensure correctness and eventual consistency.
1 parent ae79a10 commit 338ff27

File tree

1 file changed

+38
-14
lines changed

1 file changed

+38
-14
lines changed

src/controllers/krate/publish.rs

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use tokio::runtime::Handle;
2323
use url::Url;
2424

2525
use crate::models::{
26+
default_versions::{find_default_version, Version as DefaultVersion},
2627
insert_version_owner_action, Category, Crate, DependencyKind, Keyword, NewCrate, NewVersion,
2728
Rights, VersionAction,
2829
};
@@ -64,7 +65,7 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
6465

6566
Crate::validate_crate_name("crate", &metadata.name).map_err(bad_request)?;
6667

67-
let version = match semver::Version::parse(&metadata.vers) {
68+
let semver = match semver::Version::parse(&metadata.vers) {
6869
Ok(parsed) => parsed,
6970
Err(_) => {
7071
return Err(bad_request(format_args!(
@@ -75,7 +76,7 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
7576
};
7677

7778
// Convert the version back to a string to deal with any inconsistencies
78-
let version_string = version.to_string();
79+
let version_string = semver.to_string();
7980

8081
let request_log = req.request_log();
8182
request_log.add("crate_name", &*metadata.name);
@@ -385,16 +386,41 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
385386
// Link this new version to all dependencies
386387
add_dependencies(conn, &deps, version.id)?;
387388

388-
// Insert the default version if it doesn't already exist. Compared
389-
// to only using a background job, this prevents us from getting
390-
// into a situation where a crate exists in the `crates` table but
391-
// doesn't have a default version in the `default_versions` table.
392-
let inserted_default_versions = diesel::insert_into(default_versions::table)
389+
let existing_default_version = default_versions::table
390+
.inner_join(versions::table)
391+
.filter(default_versions::crate_id.eq(krate.id))
392+
.select(DefaultVersion::as_select())
393+
.first(conn)
394+
.optional()?;
395+
396+
let published_default_version = DefaultVersion {
397+
id: version.id,
398+
num: semver,
399+
yanked: false,
400+
};
401+
402+
// Upsert the `default_value` determined by the existing `default_value` and the
403+
// published version. Note that this could potentially write an outdated version
404+
// (although this should not happen regularly), as we might be comparing to an
405+
// outdated value.
406+
//
407+
// Compared to only using a background job, this prevents us from getting into a
408+
// situation where a crate exists in the `crates` table but doesn't have a default
409+
// version in the `default_versions` table.
410+
let default_version = if let Some(default_version) = existing_default_version {
411+
find_default_version(&[default_version, published_default_version]).cloned()
412+
.expect("at least published version should be chosen")
413+
} else {
414+
published_default_version
415+
};
416+
diesel::insert_into(default_versions::table)
393417
.values((
394418
default_versions::crate_id.eq(krate.id),
395-
default_versions::version_id.eq(version.id),
419+
default_versions::version_id.eq(default_version.id),
396420
))
397-
.on_conflict_do_nothing()
421+
.on_conflict(default_versions::crate_id)
422+
.do_update()
423+
.set(default_versions::version_id.eq(default_version.id))
398424
.execute(conn)?;
399425

400426
// Update all keywords for this crate
@@ -446,11 +472,9 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
446472

447473
SendPublishNotificationsJob::new(version.id).enqueue(conn)?;
448474

449-
// If this is a new version for an existing crate it is sufficient
450-
// to update the default version asynchronously in a background job.
451-
if inserted_default_versions == 0 {
452-
UpdateDefaultVersion::new(krate.id).enqueue(conn)?;
453-
}
475+
// Update the default version asynchronously in a background job
476+
// to ensure correctness and eventual consistency.
477+
UpdateDefaultVersion::new(krate.id).enqueue(conn)?;
454478

455479
// Experiment: check new crates for potential typosquatting.
456480
if existing_crate.is_none() {

0 commit comments

Comments
 (0)