Auto merge of #3650 - Turbo87:no-semver-columns, r=jtgeibel

models::Version: Change `num` field to a plain `String`

This PR prepares for `semver` v1.x no longer having a `diesel` feature, which would make it much more complicated for us to directly deserialize the SQL column into a `semver::Version` field. We can deserialize it to a `String` instead, and only parse it into a `semver::Version` on demand.

Author: bors

src/controllers/krate/downloads.rs

@@ -23,7 +23,7 @@ pub fn downloads(req: &mut dyn RequestExt) -> EndpointResult {
     let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
 
     let mut versions: Vec<Version> = krate.all_versions().load(&*conn)?;
-    versions.sort_by(|a, b| b.num.cmp(&a.num));
+    versions.sort_by_cached_key(|version| cmp::Reverse(semver::Version::parse(&version.num).ok()));
     let (latest_five, rest) = versions.split_at(cmp::min(5, versions.len()));
 
     let downloads = VersionDownload::belonging_to(latest_five)

src/controllers/krate/metadata.rs

@@ -4,6 +4,8 @@
 //! index or cached metadata which was extracted (client side) from the
 //! `Cargo.toml` file.
 
+use std::cmp::Reverse;
+
 use crate::controllers::frontend_prelude::*;
 use crate::controllers::helpers::pagination::PaginationOptions;
 
@@ -128,7 +130,10 @@ pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
         .left_outer_join(users::table)
         .select((versions::all_columns, users::all_columns.nullable()))
         .load(&*conn)?;
-    versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
+
+    versions_and_publishers
+        .sort_by_cached_key(|(version, _)| Reverse(semver::Version::parse(&version.num).ok()));
+
     let versions = versions_and_publishers
         .iter()
         .map(|(v, _)| v)
@@ -224,7 +229,10 @@ pub fn versions(req: &mut dyn RequestExt) -> EndpointResult {
         .left_outer_join(users::table)
         .select((versions::all_columns, users::all_columns.nullable()))
         .load(&*conn)?;
-    versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
+
+    versions_and_publishers
+        .sort_by_cached_key(|(version, _)| Reverse(semver::Version::parse(&version.num).ok()));
+
     let versions = versions_and_publishers
         .iter()
         .map(|(v, _)| v)

src/git.rs

@@ -328,13 +328,12 @@ pub fn yank(
         }
 
         let prev = fs::read_to_string(&dst)?;
-        let version_num = version.num.to_string();
         let new = prev
             .lines()
             .map(|line| {
                 let mut git_crate = serde_json::from_str::<Crate>(line)
                     .map_err(|_| format!("couldn't decode: `{}`", line))?;
-                if git_crate.name != krate || git_crate.vers != version_num {
+                if git_crate.name != krate || git_crate.vers != version.num {
                     return Ok(line.to_string());
                 }
                 git_crate.yanked = Some(yanked);

src/models/version.rs

@@ -14,7 +14,7 @@ use crate::schema::*;
 pub struct Version {
     pub id: i32,
     pub crate_id: i32,
-    pub num: semver::Version,
+    pub num: String,
     pub updated_at: NaiveDateTime,
     pub created_at: NaiveDateTime,
     pub downloads: i32,
@@ -59,16 +59,26 @@ impl TopVersions {
     /// highest version (in semver order) for a collection of date/version pairs.
     pub fn from_date_version_pairs<T>(pairs: T) -> Self
     where
-        T: Clone + IntoIterator<Item = (NaiveDateTime, semver::Version)>,
+        T: Clone + IntoIterator<Item = (NaiveDateTime, String)>,
     {
-        let newest = pairs.clone().into_iter().max().map(|(_, v)| v);
-        let highest = pairs.clone().into_iter().map(|(_, v)| v).max();
+        // filter out versions that we can't parse
+        let pairs: Vec<(NaiveDateTime, semver::Version)> = pairs
+            .into_iter()
+            .filter_map(|(date, version)| {
+                semver::Version::parse(&version)
+                    .ok()
+                    .map(|version| (date, version))
+            })
+            .collect();
 
+        let newest = pairs.iter().max().map(|(_, v)| v.clone());
+        let highest = pairs.iter().map(|(_, v)| v).max().map(|v| v.clone());
         let highest_stable = pairs
-            .into_iter()
+            .iter()
             .map(|(_, v)| v)
             .filter(|v| !v.is_prerelease())
-            .max();
+            .max()
+            .map(|v| v.clone());
 
         Self {
             highest,
@@ -217,7 +227,7 @@ mod tests {
 
     #[test]
     fn top_versions_single() {
-        let versions = vec![(date("2020-12-03T12:34:56"), version("1.0.0"))];
+        let versions = vec![(date("2020-12-03T12:34:56"), "1.0.0".into())];
         assert_eq!(
             TopVersions::from_date_version_pairs(versions),
             TopVersions {
@@ -230,7 +240,7 @@ mod tests {
 
     #[test]
     fn top_versions_prerelease() {
-        let versions = vec![(date("2020-12-03T12:34:56"), version("1.0.0-beta.5"))];
+        let versions = vec![(date("2020-12-03T12:34:56"), "1.0.0-beta.5".into())];
         assert_eq!(
             TopVersions::from_date_version_pairs(versions),
             TopVersions {
@@ -244,10 +254,11 @@ mod tests {
     #[test]
     fn top_versions_multiple() {
         let versions = vec![
-            (date("2018-12-03T12:34:56"), version("1.0.0")),
-            (date("2019-12-03T12:34:56"), version("2.0.0-alpha.1")),
-            (date("2020-12-03T12:34:56"), version("1.1.0")),
-            (date("2020-12-31T12:34:56"), version("1.0.4")),
+            (date("2018-12-03T12:34:56"), "1.0.0".into()),
+            (date("2019-12-03T12:34:56"), "2.0.0-alpha.1".into()),
+            (date("2020-12-01T12:34:56"), "everything is broken".into()),
+            (date("2020-12-03T12:34:56"), "1.1.0".into()),
+            (date("2020-12-31T12:34:56"), "1.0.4".into()),
         ];
         assert_eq!(
             TopVersions::from_date_version_pairs(versions),

src/views.rs

@@ -607,12 +607,15 @@ impl EncodableVersion {
             ..
         } = version;
 
-        let num = num.to_string();
+        let links = EncodableVersionLinks {
+            dependencies: format!("/api/v1/crates/{}/{}/dependencies", crate_name, num),
+            version_downloads: format!("/api/v1/crates/{}/{}/downloads", crate_name, num),
+        };
 
         Self {
             dl_path: format!("/api/v1/crates/{}/{}/download", crate_name, num),
             readme_path: format!("/api/v1/crates/{}/{}/readme", crate_name, num),
-            num: num.clone(),
+            num,
             id,
             krate: crate_name.to_string(),
             updated_at,
@@ -621,10 +624,7 @@ impl EncodableVersion {
             features,
             yanked,
             license,
-            links: EncodableVersionLinks {
-                dependencies: format!("/api/v1/crates/{}/{}/dependencies", crate_name, num),
-                version_downloads: format!("/api/v1/crates/{}/{}/downloads", crate_name, num),
-            },
+            links,
             crate_size,
             published_by: published_by.map(User::into),
             audit_actions: audit_actions