diff --git a/migrations/2020-11-20-193109_add_version_upload_name/down.sql b/migrations/2020-11-20-193109_add_version_upload_name/down.sql new file mode 100644 index 00000000000..650aefc9ea1 --- /dev/null +++ b/migrations/2020-11-20-193109_add_version_upload_name/down.sql @@ -0,0 +1,3 @@ +ALTER TABLE versions +DROP COLUMN upload_name +; diff --git a/migrations/2020-11-20-193109_add_version_upload_name/up.sql b/migrations/2020-11-20-193109_add_version_upload_name/up.sql new file mode 100644 index 00000000000..903f4cd91b0 --- /dev/null +++ b/migrations/2020-11-20-193109_add_version_upload_name/up.sql @@ -0,0 +1,9 @@ +ALTER TABLE versions +ADD COLUMN upload_name VARCHAR NOT NULL +; + +UPDATE versions +SET upload_name = crates.name +FROM crates +WHERE versions.crate_id = crates.id +; diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 956e666890a..90eb5f53eac 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -93,7 +93,7 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult { }; let license_file = new_crate.license_file.as_deref(); - let krate = + let mut krate = persist.create_or_update(&conn, user.id, Some(&app.config.publish_rate_limit))?; let owners = krate.owners(&conn)?; @@ -107,12 +107,21 @@ pub fn publish(req: &mut dyn RequestExt) -> EndpointResult { } if krate.name != *name { - return Err(cargo_err(&format_args!( - "crate was previously named `{}`", - krate.name - ))); + // Case change is allowed for the name. However as the name isn't normally updated in + // the database, we'll need to do it manually here. + if krate.name.to_lowercase() == *name.to_lowercase() { + krate.rename(&conn, &name)?; + } else { + return Err(cargo_err(&format_args!( + "crate was previously named `{}`", + krate.name + ))); + } } + // No need for mutability anymore. + let krate = krate; + // Length of the .crate tarball, which appears after the metadata in the request body. // TODO: Not sure why we're using the total content length (metadata + .crate file length) // to compare against the max upload size... investigate that and perhaps change to use diff --git a/src/models/krate.rs b/src/models/krate.rs index 38bad45db3d..01740d4e827 100644 --- a/src/models/krate.rs +++ b/src/models/krate.rs @@ -543,6 +543,16 @@ impl Crate { Ok(rows.records_and_total()) } + + pub fn rename(&mut self, conn: &PgConnection, new_name: &str) -> AppResult<()> { + diesel::update(crates::table) + .filter(crates::id.eq(self.id)) + .set(crates::name.eq(new_name)) + .execute(conn)?; + self.name = new_name.to_string(); + + Ok(()) + } } use diesel::sql_types::{Date, Text}; diff --git a/src/schema.rs b/src/schema.rs index 1570beccb2b..51919bbfb32 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -1005,6 +1005,12 @@ table! { /// /// (Automatically generated by Diesel.) published_by -> Nullable, + /// The `upload_name` column of the `versions` table. + /// + /// Its SQL type is `Varchar`. + /// + /// (Automatically generated by Diesel.) + upload_name -> Varchar, } } diff --git a/src/tests/krate.rs b/src/tests/krate.rs index e839d541953..61f047b4be7 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -1109,7 +1109,7 @@ fn new_krate_duplicate_version() { #[test] fn new_crate_similar_name() { - let (app, _, user, token) = TestApp::init().with_token(); + let (app, _, user, token) = TestApp::full().with_token(); app.db(|conn| { CrateBuilder::new("Foo_similar", user.as_model().id) @@ -1118,15 +1118,10 @@ fn new_crate_similar_name() { }); let crate_to_publish = PublishBuilder::new("foo_similar").version("1.1.0"); - let json = token - .enqueue_publish(crate_to_publish) - .bad_with_status(StatusCode::OK); + let json = token.enqueue_publish(crate_to_publish).good(); - assert!( - json.errors[0].detail.contains("previously named"), - "{:?}", - json.errors - ); + assert_eq!(json.krate.name, "foo_similar"); + assert_eq!(json.krate.max_version, "1.1.0"); } #[test] @@ -1470,6 +1465,55 @@ fn download() { assert_dl_count("FOO_DOWNLOAD", Some(&query), 2); } +#[test] +fn download_renamed_crate() { + use chrono::{Duration, Utc}; + let (_, anon, _, token) = TestApp::full().with_token(); + + let crate_to_publish = PublishBuilder::new("FOO_RENAMED"); + token.enqueue_publish(crate_to_publish).good(); + + let crate_to_publish = PublishBuilder::new("foo_renamed").version("1.1.0"); + let json = token.enqueue_publish(crate_to_publish).good(); + + let assert_dl_count = |name_and_version: &str, query: Option<&str>, count: i32| { + let url = format!("/api/v1/crates/{}/downloads", name_and_version); + let downloads: Downloads = if let Some(query) = query { + anon.get_with_query(&url, query).good() + } else { + anon.get(&url).good() + }; + let total_downloads = downloads + .version_downloads + .iter() + .map(|vd| vd.downloads) + .sum::(); + assert_eq!(total_downloads, count); + }; + + let download = |name_and_version: &str| { + let url = format!("/api/v1/crates/{}/download", name_and_version); + anon.get::<()>(&url) + .assert_redirect_ends_with("foo_renamed-1.0.0.crate") + .assert_status(StatusCode::FOUND); + // TODO: test the with_json code path + }; + + // The important assert here is to ensure the download succeeds to ensure + // the old version blobs are still retrieved from the backend storage with + // their + + download("FOO_RENAMED/1.0.0"); + download("foo_renamed/1.0.0"); + assert_dl_count("FOO_RENAMED/1.0.0", None, 2); + assert_dl_count("FOO_RENAMED", None, 2); + + download("FOO_RENAMED/1.1.0"); + download("foo_renamed/1.1.0"); + assert_dl_count("foo_renamed/1.1.0", None, 2); + assert_dl_count("foo_renamed", None, 4); +} + #[test] fn download_nonexistent_version_of_existing_crate_404s() { let (app, anon, user) = TestApp::init().with_user(); @@ -1496,7 +1540,7 @@ fn download_noncanonical_crate_name() { // Request download for "foo-download" with a dash instead of an underscore, // and assert that the correct download link is returned. - anon.get::<()>("/api/v1/crates/foo-download/1.0.0/download") + anon.get::<()>("/api/v1/crates/Foo-Download/1.0.0/download") .assert_redirect_ends_with("/crates/foo_download/foo_download-1.0.0.crate"); }