WIP: Support changing crate name case

migrations/2020-11-20-193109_add_version_upload_name/down.sql

@@ -0,0 +1,3 @@
+ALTER TABLE versions
+DROP COLUMN upload_name
+;

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
+;

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

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};

src/schema.rs

@@ -1005,6 +1005,12 @@ table! {
         ///
         /// (Automatically generated by Diesel.)
         published_by -> Nullable<Int4>,
+        /// The `upload_name` column of the `versions` table.
+        ///
+        /// Its SQL type is `Varchar`.
+        ///
+        /// (Automatically generated by Diesel.)
+        upload_name -> Varchar,
     }
 }
 

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::<i32>();
+        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");
 }