Fix reverse_dependencies

migrations/20170308140537_create_to_semver_no_prerelease/down.sql

@@ -0,0 +1,3 @@
+DROP INDEX versions_to_semver_no_prerelease_idx;
+DROP FUNCTION to_semver_no_prerelease(text);
+DROP TYPE semver_triple;

migrations/20170308140537_create_to_semver_no_prerelease/up.sql

@@ -0,0 +1,17 @@
+CREATE TYPE semver_triple AS (
+  major int4,
+  minor int4,
+  teeny int4
+);
+
+CREATE FUNCTION to_semver_no_prerelease(text) RETURNS semver_triple IMMUTABLE AS $$
+  SELECT (
+    split_part($1, '.', 1)::int4,
+    split_part($1, '.', 2)::int4,
+    split_part(split_part($1, '+', 1), '.', 3)::int4
+  )::semver_triple
+  WHERE strpos($1, '-') = 0
+  $$ LANGUAGE SQL
+;
+
+CREATE INDEX versions_to_semver_no_prerelease_idx ON versions (crate_id, to_semver_no_prerelease(num) DESC NULLS LAST) WHERE NOT yanked;

src/krate.rs

@@ -432,32 +432,36 @@ impl Crate {
                                 -> CargoResult<(Vec<(Dependency, String, i32)>, i64)> {
         let select_sql = "
               FROM dependencies
-              INNER JOIN versions
+              INNER JOIN (
+                SELECT versions.*,
+                    row_number() OVER (PARTITION BY crate_id ORDER BY to_semver_no_prerelease(num) DESC NULLS LAST) rn
+                    FROM versions
+                    WHERE NOT yanked
+              ) versions
                 ON versions.id = dependencies.version_id
               INNER JOIN crates
                 ON crates.id = versions.crate_id
               WHERE dependencies.crate_id = $1
-                AND versions.num = $2
+                AND rn = 1
         ";
         let fetch_sql = format!("SELECT DISTINCT ON (crate_downloads, crate_name)
                                         dependencies.*,
                                         crates.downloads AS crate_downloads,
                                         crates.name AS crate_name
                                         {}
                                ORDER BY crate_downloads DESC
-                                 OFFSET $3
-                                  LIMIT $4",
+                                 OFFSET $2
+                                  LIMIT $3",
                                 select_sql);
         let count_sql = format!("SELECT COUNT(DISTINCT(crates.id)) {}", select_sql);
 
         let stmt = conn.prepare(&fetch_sql)?;
-        let max_version = self.max_version(conn)?.to_string();
-        let vec: Vec<_> = stmt.query(&[&self.id, &max_version, &offset, &limit])?
+        let vec: Vec<_> = stmt.query(&[&self.id, &offset, &limit])?
             .iter()
             .map(|r| (Model::from_row(&r), r.get("crate_name"), r.get("crate_downloads")))
             .collect();
         let stmt = conn.prepare(&count_sql)?;
-        let cnt: i64 = stmt.query(&[&self.id, &max_version])?.iter().next().unwrap().get(0);
+        let cnt: i64 = stmt.query(&[&self.id])?.iter().next().unwrap().get(0);
 
         Ok((vec, cnt))
     }

src/tests/krate.rs

@@ -1183,6 +1183,102 @@ fn reverse_dependencies() {
     assert_eq!(deps.meta.total, 0);
 }
 
+#[test]
+fn reverse_dependencies_when_old_version_doesnt_depend_but_new_does() {
+    let (_b, app, middle) = ::app();
+
+    let v100 = semver::Version::parse("1.0.0").unwrap();
+    let v110 = semver::Version::parse("1.1.0").unwrap();
+    let v200 = semver::Version::parse("2.0.0").unwrap();
+    let mut req = ::req(app, Method::Get,
+                        "/api/v1/crates/c1/reverse_dependencies");
+    ::mock_user(&mut req, ::user("foo"));
+    let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v110);
+    let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
+    let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
+
+    ::mock_dep(&mut req, &c2v2, &c1, None);
+
+    let mut response = ok_resp!(middle.call(&mut req));
+    let deps = ::json::<RevDeps>(&mut response);
+    assert_eq!(deps.dependencies.len(), 1);
+    assert_eq!(deps.meta.total, 1);
+    assert_eq!(deps.dependencies[0].crate_id, "c2");
+}
+
+#[test]
+fn reverse_dependencies_when_old_version_depended_but_new_doesnt() {
+    let (_b, app, middle) = ::app();
+
+    let v100 = semver::Version::parse("1.0.0").unwrap();
+    let v200 = semver::Version::parse("2.0.0").unwrap();
+    let mut req = ::req(app, Method::Get,
+                        "/api/v1/crates/c1/reverse_dependencies");
+    ::mock_user(&mut req, ::user("foo"));
+    let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
+    let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
+    let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
+
+    ::mock_dep(&mut req, &c2v1, &c1, None);
+
+    let mut response = ok_resp!(middle.call(&mut req));
+    let deps = ::json::<RevDeps>(&mut response);
+    assert_eq!(deps.dependencies.len(), 0);
+    assert_eq!(deps.meta.total, 0);
+}
+
+#[test]
+fn prerelease_versions_not_included_in_reverse_dependencies() {
+    let (_b, app, middle) = ::app();
+
+    let v100 = semver::Version::parse("1.0.0").unwrap();
+    let v110_pre = semver::Version::parse("1.1.0-pre").unwrap();
+    let mut req = ::req(app, Method::Get,
+                        "/api/v1/crates/c1/reverse_dependencies");
+    ::mock_user(&mut req, ::user("foo"));
+    let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
+    let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v110_pre);
+    let (_, c3v1) = ::mock_crate_vers(&mut req, ::krate("c3"), &v100);
+    let _ = ::mock_crate_vers(&mut req, ::krate("c3"), &v110_pre);
+
+    ::mock_dep(&mut req, &c3v1, &c1, None);
+
+    let mut response = ok_resp!(middle.call(&mut req));
+    let deps = ::json::<RevDeps>(&mut response);
+    assert_eq!(deps.dependencies.len(), 1);
+    assert_eq!(deps.meta.total, 1);
+    assert_eq!(deps.dependencies[0].crate_id, "c3");
+}
+
+#[test]
+fn yanked_versions_not_included_in_reverse_dependencies() {
+    let (_b, app, middle) = ::app();
+
+    let v100 = semver::Version::parse("1.0.0").unwrap();
+    let v200 = semver::Version::parse("2.0.0").unwrap();
+    let mut req = ::req(app, Method::Get,
+                        "/api/v1/crates/c1/reverse_dependencies");
+    ::mock_user(&mut req, ::user("foo"));
+    let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
+    let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
+    let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
+
+    ::mock_dep(&mut req, &c2v2, &c1, None);
+
+    let mut response = ok_resp!(middle.call(&mut req));
+    let deps = ::json::<RevDeps>(&mut response);
+    assert_eq!(deps.dependencies.len(), 1);
+    assert_eq!(deps.meta.total, 1);
+    assert_eq!(deps.dependencies[0].crate_id, "c2");
+
+    t!(c2v2.yank(req.tx().unwrap(), true));
+
+    let mut response = ok_resp!(middle.call(&mut req));
+    let deps = ::json::<RevDeps>(&mut response);
+    assert_eq!(deps.dependencies.len(), 0);
+    assert_eq!(deps.meta.total, 0);
+}
+
 #[test]
 fn author_license_and_description_required() {
     let (_b, app, middle) = ::app();