Merge pull request #837 from Turbo87/reverse-deps

Fix reverse dependencies query

Author: carols10cents

app/serializers/dependency.js

@@ -0,0 +1,7 @@
+import DS from 'ember-data';
+
+export default DS.RESTSerializer.extend({
+    attrs: {
+        version: 'version_id',
+    },
+});

app/templates/crate/reverse-dependencies.hbs

@@ -17,7 +17,7 @@
     {{#each model as |dependency|}}
         <div class='crate row'>
             <div>
-            {{#link-to 'crate' dependency.crate_id}}{{dependency.crate_id}}{{/link-to}} requires {{dependency.req}}
+            {{#link-to 'crate' dependency.version.crateName}}{{dependency.version.crateName}}{{/link-to}} requires {{dependency.req}}
             </div>
             <div class='stats downloads'>
                 {{svg-jar "download-clear-back"}}

mirage/config.js

@@ -142,15 +142,13 @@ export default function() {
         let dependencies = allDependencies.slice(start, end);
         let total = allDependencies.length;
 
-        let serialized = this.serialize(dependencies);
+        let versions = schema.versions.find(dependencies.models.map(it => it.version_id));
 
-        // TODO https://github.com/rust-lang/crates.io/pull/810
-        serialized.dependencies.forEach(dep => {
-            let version = schema.versions.find(dep.version_id);
-            dep.crate_id = version.crate;
-        });
-
-        return withMeta(serialized, { total });
+        return {
+            ...this.serialize(dependencies),
+            ...this.serialize(versions),
+            meta: { total },
+        };
     });
 
     this.get('/crates/:crate_id/downloads', function(schema, request) {

src/dependency.rs

@@ -30,7 +30,6 @@ pub struct Dependency {
 
 pub struct ReverseDependency {
     dependency: Dependency,
-    crate_name: String,
     crate_downloads: i32,
 }
 
@@ -125,9 +124,9 @@ impl Dependency {
 }
 
 impl ReverseDependency {
-    pub fn encodable(self) -> EncodableDependency {
+    pub fn encodable(self, crate_name: &str) -> EncodableDependency {
         self.dependency.encodable(
-            &self.crate_name,
+            crate_name,
             Some(self.crate_downloads),
         )
     }
@@ -218,11 +217,10 @@ impl Queryable<dependencies::SqlType, Pg> for Dependency {
 impl Queryable<(dependencies::SqlType, Integer, Text), Pg> for ReverseDependency {
     type Row = (<Dependency as Queryable<dependencies::SqlType, Pg>>::Row, i32, String);
 
-    fn build((dep_row, downloads, name): Self::Row) -> Self {
+    fn build((dep_row, downloads, _name): Self::Row) -> Self {
         ReverseDependency {
             dependency: Dependency::build(dep_row),
             crate_downloads: downloads,
-            crate_name: name,
         }
     }
 }

src/krate.rs

@@ -1543,19 +1543,33 @@ fn modify_owners(req: &mut Request, add: bool) -> CargoResult<Response> {
 
 /// Handles the `GET /crates/:crate_id/reverse_dependencies` route.
 pub fn reverse_dependencies(req: &mut Request) -> CargoResult<Response> {
+    use diesel::expression::dsl::any;
+
     let name = &req.params()["crate_id"];
     let conn = req.db_conn()?;
     let krate = Crate::by_name(name).first::<Crate>(&*conn)?;
     let (offset, limit) = req.pagination(10, 100)?;
     let (rev_deps, total) = krate.reverse_dependencies(&*conn, offset, limit)?;
-    let rev_deps = rev_deps
+    let rev_deps: Vec<_> = rev_deps
+        .into_iter()
+        .map(|dep| dep.encodable(&krate.name))
+        .collect();
+
+    let version_ids: Vec<i32> = rev_deps.iter().map(|dep| dep.version_id).collect();
+
+    let versions = versions::table
+        .filter(versions::id.eq(any(version_ids)))
+        .inner_join(crates::table)
+        .select((versions::all_columns, crates::name))
+        .load::<(Version, String)>(&*conn)?
         .into_iter()
-        .map(ReverseDependency::encodable)
+        .map(|(version, krate_name)| version.encodable(&krate_name))
         .collect();
 
     #[derive(RustcEncodable)]
     struct R {
         dependencies: Vec<EncodableDependency>,
+        versions: Vec<EncodableVersion>,
         meta: Meta,
     }
     #[derive(RustcEncodable)]
@@ -1564,6 +1578,7 @@ pub fn reverse_dependencies(req: &mut Request) -> CargoResult<Response> {
     }
     Ok(req.json(&R {
         dependencies: rev_deps,
+        versions,
         meta: Meta { total: total },
     }))
 }

src/tests/krate.rs

@@ -60,6 +60,7 @@ struct Deps {
 #[derive(RustcDecodable)]
 struct RevDeps {
     dependencies: Vec<EncodableDependency>,
+    versions: Vec<EncodableVersion>,
     meta: CrateMeta,
 }
 #[derive(RustcDecodable)]
@@ -1635,7 +1636,10 @@ fn reverse_dependencies() {
     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");
+    assert_eq!(deps.dependencies[0].crate_id, "c1");
+    assert_eq!(deps.versions.len(), 1);
+    assert_eq!(deps.versions[0].krate, "c2");
+    assert_eq!(deps.versions[0].num, "1.1.0");
 
     // c1 has no dependent crates.
     req.with_path("/api/v1/crates/c2/reverse_dependencies");
@@ -1670,7 +1674,7 @@ fn reverse_dependencies_when_old_version_doesnt_depend_but_new_does() {
     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");
+    assert_eq!(deps.dependencies[0].crate_id, "c1");
 }
 
 #[test]
@@ -1728,7 +1732,7 @@ fn prerelease_versions_not_included_in_reverse_dependencies() {
     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");
+    assert_eq!(deps.dependencies[0].crate_id, "c1");
 }
 
 #[test]
@@ -1756,7 +1760,7 @@ fn yanked_versions_not_included_in_reverse_dependencies() {
     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");
+    assert_eq!(deps.dependencies[0].crate_id, "c1");
 
     // TODO: have this test call `version.yank()` once the yank method is converted to diesel
     diesel::update(versions::table.filter(versions::num.eq("2.0.0")))