Skip to content

Commit 9e5ccf0

Browse files
alexcrichtonalexcrichton
authored
Merge pull request #604 from sgrif/sg-fix-reverse-dependencies
Fix `reverse_dependencies`
2 parents 52777af + d397345 commit 9e5ccf0

File tree

4 files changed

+127
-7
lines changed

4 files changed

+127
-7
lines changed

‎migrations/20170308140537_create_to_semver_no_prerelease/down.sql

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
DROP INDEX versions_to_semver_no_prerelease_idx;
2+
DROP FUNCTION to_semver_no_prerelease(text);
3+
DROP TYPE semver_triple;

‎migrations/20170308140537_create_to_semver_no_prerelease/up.sql

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
CREATE TYPE semver_triple AS (
2+
major int4,
3+
minor int4,
4+
teeny int4
5+
);
6+
7+
CREATE FUNCTION to_semver_no_prerelease(text) RETURNS semver_triple IMMUTABLE AS $$
8+
SELECT (
9+
split_part($1, '.', 1)::int4,
10+
split_part($1, '.', 2)::int4,
11+
split_part(split_part($1, '+', 1), '.', 3)::int4
12+
)::semver_triple
13+
WHERE strpos($1, '-') = 0
14+
$$ LANGUAGE SQL
15+
;
16+
17+
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

+11-7
Original file line numberDiff line numberDiff line change
@@ -432,32 +432,36 @@ impl Crate {
432432
-> CargoResult<(Vec<(Dependency, String, i32)>, i64)> {
433433
let select_sql = "
434434
FROM dependencies
435-
INNER JOIN versions
435+
INNER JOIN (
436+
SELECT versions.*,
437+
row_number() OVER (PARTITION BY crate_id ORDER BY to_semver_no_prerelease(num) DESC NULLS LAST) rn
438+
FROM versions
439+
WHERE NOT yanked
440+
) versions
436441
ON versions.id = dependencies.version_id
437442
INNER JOIN crates
438443
ON crates.id = versions.crate_id
439444
WHERE dependencies.crate_id = $1
440-
AND versions.num = $2
445+
AND rn = 1
441446
";
442447
let fetch_sql = format!("SELECT DISTINCT ON (crate_downloads, crate_name)
443448
dependencies.*,
444449
crates.downloads AS crate_downloads,
445450
crates.name AS crate_name
446451
{}
447452
ORDER BY crate_downloads DESC
448-
OFFSET $3
449-
LIMIT $4",
453+
OFFSET $2
454+
LIMIT $3",
450455
select_sql);
451456
let count_sql = format!("SELECT COUNT(DISTINCT(crates.id)) {}", select_sql);
452457

453458
let stmt = conn.prepare(&fetch_sql)?;
454-
let max_version = self.max_version(conn)?.to_string();
455-
let vec: Vec<_> = stmt.query(&[&self.id, &max_version, &offset, &limit])?
459+
let vec: Vec<_> = stmt.query(&[&self.id, &offset, &limit])?
456460
.iter()
457461
.map(|r| (Model::from_row(&r), r.get("crate_name"), r.get("crate_downloads")))
458462
.collect();
459463
let stmt = conn.prepare(&count_sql)?;
460-
let cnt: i64 = stmt.query(&[&self.id, &max_version])?.iter().next().unwrap().get(0);
464+
let cnt: i64 = stmt.query(&[&self.id])?.iter().next().unwrap().get(0);
461465

462466
Ok((vec, cnt))
463467
}

‎src/tests/krate.rs

+96
Original file line numberDiff line numberDiff line change
@@ -1183,6 +1183,102 @@ fn reverse_dependencies() {
11831183
assert_eq!(deps.meta.total, 0);
11841184
}
11851185

1186+
#[test]
1187+
fn reverse_dependencies_when_old_version_doesnt_depend_but_new_does() {
1188+
let (_b, app, middle) = ::app();
1189+
1190+
let v100 = semver::Version::parse("1.0.0").unwrap();
1191+
let v110 = semver::Version::parse("1.1.0").unwrap();
1192+
let v200 = semver::Version::parse("2.0.0").unwrap();
1193+
let mut req = ::req(app, Method::Get,
1194+
"/api/v1/crates/c1/reverse_dependencies");
1195+
::mock_user(&mut req, ::user("foo"));
1196+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v110);
1197+
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
1198+
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
1199+
1200+
::mock_dep(&mut req, &c2v2, &c1, None);
1201+
1202+
let mut response = ok_resp!(middle.call(&mut req));
1203+
let deps = ::json::<RevDeps>(&mut response);
1204+
assert_eq!(deps.dependencies.len(), 1);
1205+
assert_eq!(deps.meta.total, 1);
1206+
assert_eq!(deps.dependencies[0].crate_id, "c2");
1207+
}
1208+
1209+
#[test]
1210+
fn reverse_dependencies_when_old_version_depended_but_new_doesnt() {
1211+
let (_b, app, middle) = ::app();
1212+
1213+
let v100 = semver::Version::parse("1.0.0").unwrap();
1214+
let v200 = semver::Version::parse("2.0.0").unwrap();
1215+
let mut req = ::req(app, Method::Get,
1216+
"/api/v1/crates/c1/reverse_dependencies");
1217+
::mock_user(&mut req, ::user("foo"));
1218+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
1219+
let (_, c2v1) = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
1220+
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
1221+
1222+
::mock_dep(&mut req, &c2v1, &c1, None);
1223+
1224+
let mut response = ok_resp!(middle.call(&mut req));
1225+
let deps = ::json::<RevDeps>(&mut response);
1226+
assert_eq!(deps.dependencies.len(), 0);
1227+
assert_eq!(deps.meta.total, 0);
1228+
}
1229+
1230+
#[test]
1231+
fn prerelease_versions_not_included_in_reverse_dependencies() {
1232+
let (_b, app, middle) = ::app();
1233+
1234+
let v100 = semver::Version::parse("1.0.0").unwrap();
1235+
let v110_pre = semver::Version::parse("1.1.0-pre").unwrap();
1236+
let mut req = ::req(app, Method::Get,
1237+
"/api/v1/crates/c1/reverse_dependencies");
1238+
::mock_user(&mut req, ::user("foo"));
1239+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
1240+
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v110_pre);
1241+
let (_, c3v1) = ::mock_crate_vers(&mut req, ::krate("c3"), &v100);
1242+
let _ = ::mock_crate_vers(&mut req, ::krate("c3"), &v110_pre);
1243+
1244+
::mock_dep(&mut req, &c3v1, &c1, None);
1245+
1246+
let mut response = ok_resp!(middle.call(&mut req));
1247+
let deps = ::json::<RevDeps>(&mut response);
1248+
assert_eq!(deps.dependencies.len(), 1);
1249+
assert_eq!(deps.meta.total, 1);
1250+
assert_eq!(deps.dependencies[0].crate_id, "c3");
1251+
}
1252+
1253+
#[test]
1254+
fn yanked_versions_not_included_in_reverse_dependencies() {
1255+
let (_b, app, middle) = ::app();
1256+
1257+
let v100 = semver::Version::parse("1.0.0").unwrap();
1258+
let v200 = semver::Version::parse("2.0.0").unwrap();
1259+
let mut req = ::req(app, Method::Get,
1260+
"/api/v1/crates/c1/reverse_dependencies");
1261+
::mock_user(&mut req, ::user("foo"));
1262+
let (c1, _) = ::mock_crate_vers(&mut req, ::krate("c1"), &v100);
1263+
let _ = ::mock_crate_vers(&mut req, ::krate("c2"), &v100);
1264+
let (_, c2v2) = ::mock_crate_vers(&mut req, ::krate("c2"), &v200);
1265+
1266+
::mock_dep(&mut req, &c2v2, &c1, None);
1267+
1268+
let mut response = ok_resp!(middle.call(&mut req));
1269+
let deps = ::json::<RevDeps>(&mut response);
1270+
assert_eq!(deps.dependencies.len(), 1);
1271+
assert_eq!(deps.meta.total, 1);
1272+
assert_eq!(deps.dependencies[0].crate_id, "c2");
1273+
1274+
t!(c2v2.yank(req.tx().unwrap(), true));
1275+
1276+
let mut response = ok_resp!(middle.call(&mut req));
1277+
let deps = ::json::<RevDeps>(&mut response);
1278+
assert_eq!(deps.dependencies.len(), 0);
1279+
assert_eq!(deps.meta.total, 0);
1280+
}
1281+
11861282
#[test]
11871283
fn author_license_and_description_required() {
11881284
let (_b, app, middle) = ::app();

0 commit comments

Comments
 (0)