models/default_version: Implement Ord for Version

This allows us to simplify the `find_default_version()` version fn, which now only needs to iterate the slice once instead of up to three times, and it makes it possible to directly compare two `Version` instances too.

Author: Turbo87

src/models/default_versions.rs

@@ -8,7 +8,7 @@ use diesel::prelude::*;
 /// This struct is used to load all versions of a crate from the database,
 /// without loading all the additional data unnecessary for default version
 /// resolution.
-#[derive(Clone, Debug, Queryable, Selectable)]
+#[derive(Clone, Debug, PartialEq, Eq, Queryable, Selectable)]
 #[diesel(table_name = versions)]
 #[diesel(check_for_backend(diesel::pg::Pg))]
 struct Version {
@@ -23,6 +23,22 @@ impl Version {
     fn is_prerelease(&self) -> bool {
         !self.num.pre.is_empty()
     }
+
+    fn ord_tuple(&self) -> (bool, bool, &semver::Version, i32) {
+        (!self.yanked, !self.is_prerelease(), &self.num, self.id)
+    }
+}
+
+impl PartialOrd for Version {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for Version {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.ord_tuple().cmp(&other.ord_tuple())
+    }
 }
 
 /// Updates the `default_versions` table entry for the specified crate.
@@ -102,20 +118,16 @@ fn calculate_default_version(crate_id: i32, conn: &mut impl Conn) -> QueryResult
 }
 
 fn find_default_version(versions: &[Version]) -> Option<&Version> {
-    highest(versions, |v| !v.is_prerelease() && !v.yanked)
-        .or_else(|| highest(versions, |v| !v.yanked))
-        .or_else(|| highest(versions, |_| true))
-}
-
-fn highest(versions: &[Version], filter: impl FnMut(&&Version) -> bool) -> Option<&Version> {
-    versions.iter().filter(filter).max_by_key(|v| &v.num)
+    versions.iter().max()
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::schema::crates;
     use crate::test_util::test_db_connection;
+    use insta::assert_snapshot;
+    use std::fmt::Write;
 
     fn v(num: &str, yanked: bool) -> Version {
         let num = semver::Version::parse(num).unwrap();
@@ -179,6 +191,51 @@ mod tests {
         check(&versions, "1.0.0-beta.3");
     }
 
+    #[test]
+    fn test_ord() {
+        let mut versions = vec![
+            v("1.0.0", false),
+            v("1.0.0-beta.1", false),
+            v("1.0.0-beta.2", false),
+            v("1.0.0-beta.3", false),
+            v("1.0.1", true),
+            v("1.0.2", false),
+            v("1.1.0", false),
+            v("1.1.1-beta.1", true),
+            v("1.1.1", true),
+            v("1.0.3", false),
+            v("2.0.0-beta.1", false),
+        ];
+
+        versions.sort();
+
+        assert_snapshot!(format_versions(&versions), @r#"
+        1.1.1-beta.1 (yanked)
+        1.0.1 (yanked)
+        1.1.1 (yanked)
+        1.0.0-beta.1
+        1.0.0-beta.2
+        1.0.0-beta.3
+        2.0.0-beta.1
+        1.0.0
+        1.0.2
+        1.0.3
+        1.1.0
+        "#);
+    }
+
+    fn format_versions(versions: &[Version]) -> String {
+        let mut buf = String::with_capacity(versions.len() * 20);
+        for v in versions {
+            write!(buf, "{}", v.num).unwrap();
+            if v.yanked {
+                buf.push_str(" (yanked)");
+            }
+            buf.push('\n');
+        }
+        buf
+    }
+
     fn create_crate(name: &str, conn: &mut impl Conn) -> i32 {
         diesel::insert_into(crates::table)
             .values(crates::name.eq(name))