Skip to content
Closed
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
32 changes: 28 additions & 4 deletions src/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ pub struct CrateLinks {
pub reverse_dependencies: String,
}

#[derive(Insertable, AsChangeset, Default, Debug)]
#[derive(Insertable, AsChangeset, Debug)]
#[table_name = "crates"]
#[primary_key(name, max_upload_size)] // This is actually just to skip updating them
pub struct NewCrate<'a> {
Expand All @@ -147,9 +147,35 @@ pub struct NewCrate<'a> {
pub repository: Option<&'a str>,
pub max_upload_size: Option<i32>,
pub license: Option<&'a str>,
pub updated_at: Timespec,
}

impl<'a> Default for NewCrate<'a> {
fn default() -> Self {
NewCrate{
name: "",
description: None,
homepage: None,
documentation: None,
readme: None,
repository: None,
max_upload_size: None,
license: None,
// the `updated_at` column must be changed to be "now"
// even if none of the columns were actually changed.
updated_at: ::time::now_utc().to_timespec(),
}
}
}

impl<'a> NewCrate<'a> {
/// Creates this crate,
/// or updates the existing crate if it already exists under the same name.
///
/// This "create-or-update" has been made an atomic operation to avoid race conditions when
/// two authors try to publish crates with the same name. After this operation completes, a
/// CrateVersion should also be created, since it's valid to transiently have a crate with no
/// versions, but it doesn't make any sense.
pub fn create_or_update(
mut self,
conn: &PgConnection,
Expand All @@ -162,8 +188,6 @@ impl<'a> NewCrate<'a> {
self.ensure_name_not_reserved(conn)?;

conn.transaction(|| {
// To avoid race conditions, we try to insert
// first so we know whether to add an owner
if let Some(krate) = self.save_new_crate(conn, uploader)? {
return Ok(krate);
}
Expand Down Expand Up @@ -1136,7 +1160,7 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
readme: new_crate.readme.as_ref().map(|s| &**s),
repository: new_crate.repository.as_ref().map(|s| &**s),
license: new_crate.license.as_ref().map(|s| &**s),
max_upload_size: None,
.. NewCrate::default()
};

let license_file = new_crate.license_file.as_ref().map(|s| &**s);
Expand Down
22 changes: 21 additions & 1 deletion src/tests/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use cargo_registry::dependency::EncodableDependency;
use cargo_registry::download::EncodableVersionDownload;
use cargo_registry::git;
use cargo_registry::keyword::EncodableKeyword;
use cargo_registry::krate::{Crate, EncodableCrate, MAX_NAME_LENGTH};
use cargo_registry::krate::{Crate, EncodableCrate, MAX_NAME_LENGTH, NewCrate};

use cargo_registry::token::ApiToken;
use cargo_registry::owner::EncodableOwner;
Expand Down Expand Up @@ -2115,3 +2115,23 @@ fn block_blacklisted_documentation_url() {

assert_eq!(json.krate.documentation, None);
}

#[test]
fn updated_at_always_updated() {
let (_b, app, _middle) = ::app();
let conn = app.diesel_database.get().unwrap();
let u = ::new_user("foo").create_or_update(&conn).unwrap();
let create_crate = NewCrate{
name: "updated-crate",
.. NewCrate::default()
};
let created_crate = create_crate.create_or_update(&conn, None, u.id)
.expect("create");
let update_crate = NewCrate{
name: "updated-crate",
.. NewCrate::default()
};
let updated_crate = update_crate.create_or_update(&conn, None, u.id)
.expect("update");
assert!(created_crate.updated_at != updated_crate.updated_at);
}