Skip to content

WIP: Fix reverse dependencies query #810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ pub struct Dependency {

pub struct ReverseDependency {
dependency: Dependency,
crate_name: String,
crate_downloads: i32,
}

Expand Down Expand Up @@ -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),
)
}
Expand Down Expand Up @@ -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,
}
}
}
Expand Down
19 changes: 17 additions & 2 deletions src/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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 },
}))
}
Expand Down
12 changes: 8 additions & 4 deletions src/tests/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ struct Deps {
#[derive(RustcDecodable)]
struct RevDeps {
dependencies: Vec<EncodableDependency>,
versions: Vec<EncodableVersion>,
meta: CrateMeta,
}
#[derive(RustcDecodable)]
Expand Down Expand Up @@ -1616,7 +1617,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");
Expand Down Expand Up @@ -1651,7 +1655,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]
Expand Down Expand Up @@ -1709,7 +1713,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]
Expand Down Expand Up @@ -1737,7 +1741,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")))
Expand Down