Skip to content

Commit ca8176c

Browse files
sypharJoshua Nelson
syphar
authored and
Joshua Nelson
committed
fix performance regression in all releases-views, restructure tests for these views
1 parent 4e07c92 commit ca8176c

File tree

2 files changed

+189
-61
lines changed

2 files changed

+189
-61
lines changed

‎src/db/migrate.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,22 @@ pub fn migrate(version: Option<Version>, conn: &mut Client) -> CratesfyiResult<(
602602
ALTER release_time TYPE timestamp USING release_time AT TIME ZONE 'UTC';
603603
",
604604
),
605+
migration!(
606+
context,
607+
26,
608+
"create indexes for crates, github_repos and releases",
609+
// upgrade
610+
"
611+
CREATE INDEX crates_latest_version_idx ON crates (latest_version_id);
612+
CREATE INDEX releases_github_repo_idx ON releases (github_repo);
613+
CREATE INDEX github_repos_stars_idx ON github_repos(stars DESC);
614+
",
615+
"
616+
DROP INDEX crates_latest_version_idx;
617+
DROP INDEX releases_github_repo_idx;
618+
DROP INDEX github_repos_stars_idx;
619+
",
620+
),
605621
];
606622

607623
for migration in migrations {

‎src/web/releases.rs

Lines changed: 173 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,13 @@ pub(crate) fn get_releases(conn: &mut Client, page: i64, limit: i64, order: Orde
8484
releases.rustdoc_status,
8585
github_repos.stars
8686
FROM crates
87-
INNER JOIN releases ON crates.id = releases.crate_id
87+
INNER JOIN releases ON crates.latest_version_id = releases.id
8888
LEFT JOIN github_repos ON releases.github_repo = github_repos.id
8989
WHERE
90-
((NOT $3) OR (releases.build_status = FALSE AND releases.is_library = TRUE))
91-
AND crates.latest_version_id = releases.id
92-
ORDER BY {} DESC NULLS LAST
90+
((NOT $3) OR (releases.build_status = FALSE AND releases.is_library = TRUE))
91+
AND {0} IS NOT NULL
92+
93+
ORDER BY {0} DESC
9394
LIMIT $1 OFFSET $2",
9495
ordering,
9596
);
@@ -692,14 +693,14 @@ pub fn build_queue_handler(req: &mut Request) -> IronResult<Response> {
692693
#[cfg(test)]
693694
mod tests {
694695
use super::*;
695-
use crate::test::{assert_success, wrapper, TestEnvironment};
696+
use crate::test::{assert_success, wrapper, TestFrontend};
696697
use chrono::TimeZone;
697698
use failure::Error;
698699
use kuchiki::traits::TendrilSink;
699700
use std::collections::HashSet;
700701

701702
#[test]
702-
fn releases_by_stars() {
703+
fn get_releases_by_stars() {
703704
wrapper(|env| {
704705
let db = env.db();
705706

@@ -713,14 +714,14 @@ mod tests {
713714
.version("1.0.0")
714715
.github_stats("ghost/bar", 20, 20, 20)
715716
.create()?;
717+
// release without stars will not be shown
716718
env.fake_release().name("baz").version("1.0.0").create()?;
717719

718720
let releases = get_releases(&mut db.conn(), 1, 10, Order::GithubStars);
719721
assert_eq!(
720722
vec![
721723
"bar", // 20 stars
722724
"foo", // 10 stars
723-
"baz", // no stars (still included at the bottom)
724725
],
725726
releases
726727
.iter()
@@ -1063,49 +1064,6 @@ mod tests {
10631064
})
10641065
}
10651066

1066-
fn releases_link_test(path: &str, env: &TestEnvironment) -> Result<(), Error> {
1067-
env.fake_release()
1068-
.name("crate_that_succeeded")
1069-
.version("0.1.0")
1070-
.create()?;
1071-
// make sure that crates get at most one release shown, so they don't crowd the page
1072-
env.fake_release()
1073-
.name("crate_that_succeeded")
1074-
.version("0.2.0")
1075-
.create()?;
1076-
env.fake_release()
1077-
.name("crate_that_failed")
1078-
.version("0.1.0")
1079-
.build_result_failed()
1080-
.create()?;
1081-
let page = kuchiki::parse_html().one(env.frontend().get(path).send()?.text()?);
1082-
let releases: Vec<_> = page.select("a.release").expect("missing heading").collect();
1083-
if path.contains("failures") {
1084-
assert_eq!(
1085-
1,
1086-
releases.len(),
1087-
"expected one failed release for path {}",
1088-
path
1089-
);
1090-
} else {
1091-
assert_eq!(2, releases.len(), "expected 2 releases for path {}", path);
1092-
}
1093-
for release in releases {
1094-
let attributes = release.attributes.borrow();
1095-
let url = attributes.get("href").unwrap();
1096-
if url.contains("crate_that_succeeded") {
1097-
assert_eq!(
1098-
url, "/crate_that_succeeded/0.2.0/crate_that_succeeded",
1099-
"for path {}",
1100-
path
1101-
);
1102-
} else {
1103-
assert_eq!(url, "/crate/crate_that_failed/0.1.0", "for path {}", path);
1104-
}
1105-
}
1106-
Ok(())
1107-
}
1108-
11091067
#[test]
11101068
fn search() {
11111069
wrapper(|env| {
@@ -1115,20 +1073,174 @@ mod tests {
11151073
})
11161074
}
11171075

1076+
fn get_release_links(path: &str, web: &TestFrontend) -> Result<Vec<String>, Error> {
1077+
let response = web.get(path).send()?;
1078+
assert!(response.status().is_success());
1079+
1080+
let page = kuchiki::parse_html().one(response.text()?);
1081+
1082+
Ok(page
1083+
.select("a.release")
1084+
.expect("missing heading")
1085+
.map(|el| {
1086+
let attributes = el.attributes.borrow();
1087+
attributes.get("href").unwrap().to_string()
1088+
})
1089+
.collect())
1090+
}
1091+
11181092
#[test]
1119-
fn releases() {
1093+
fn releases_by_stars() {
11201094
wrapper(|env| {
1121-
let web = env.frontend();
1122-
for page in &[
1123-
"/",
1124-
"/releases",
1125-
"/releases/stars",
1126-
"/releases/recent-failures",
1127-
"/releases/failures",
1128-
] {
1129-
assert_success(page, web)?;
1130-
releases_link_test(page, env)?;
1095+
env.fake_release()
1096+
.name("crate_that_succeeded_with_github")
1097+
.version("0.1.0")
1098+
.github_stats("some/repo", 66, 22, 11)
1099+
.release_time(Utc.ymd(2020, 4, 16).and_hms(4, 33, 50))
1100+
.create()?;
1101+
1102+
env.fake_release()
1103+
.name("crate_that_succeeded_with_github")
1104+
.version("0.2.0")
1105+
.github_stats("some/repo", 66, 22, 11)
1106+
.release_time(Utc.ymd(2020, 4, 20).and_hms(4, 33, 50))
1107+
.create()?;
1108+
1109+
env.fake_release()
1110+
.name("crate_that_succeeded_without_github")
1111+
.release_time(Utc.ymd(2020, 5, 16).and_hms(4, 33, 50))
1112+
.version("0.2.0")
1113+
.create()?;
1114+
1115+
env.fake_release()
1116+
.name("crate_that_failed_with_github")
1117+
.version("0.1.0")
1118+
.github_stats("some/repo", 33, 22, 11)
1119+
.release_time(Utc.ymd(2020, 6, 16).and_hms(4, 33, 50))
1120+
.build_result_failed()
1121+
.create()?;
1122+
1123+
let links = get_release_links("/releases/stars", env.frontend())?;
1124+
1125+
// output is sorted by stars, not release-time
1126+
assert_eq!(links.len(), 2);
1127+
assert_eq!(
1128+
links[0],
1129+
"/crate_that_succeeded_with_github/0.2.0/crate_that_succeeded_with_github"
1130+
);
1131+
assert_eq!(links[1], "/crate/crate_that_failed_with_github/0.1.0");
1132+
1133+
Ok(())
1134+
})
1135+
}
1136+
1137+
#[test]
1138+
fn failures_by_stars() {
1139+
wrapper(|env| {
1140+
env.fake_release()
1141+
.name("crate_that_succeeded_with_github")
1142+
.version("0.1.0")
1143+
.github_stats("some/repo", 66, 22, 11)
1144+
.release_time(Utc.ymd(2020, 4, 16).and_hms(4, 33, 50))
1145+
.create()?;
1146+
1147+
env.fake_release()
1148+
.name("crate_that_succeeded_with_github")
1149+
.version("0.2.0")
1150+
.github_stats("some/repo", 66, 22, 11)
1151+
.release_time(Utc.ymd(2020, 4, 20).and_hms(4, 33, 50))
1152+
.create()?;
1153+
1154+
env.fake_release()
1155+
.name("crate_that_succeeded_without_github")
1156+
.release_time(Utc.ymd(2020, 5, 16).and_hms(4, 33, 50))
1157+
.version("0.2.0")
1158+
.create()?;
1159+
1160+
env.fake_release()
1161+
.name("crate_that_failed_with_github")
1162+
.version("0.1.0")
1163+
.github_stats("some/repo", 33, 22, 11)
1164+
.release_time(Utc.ymd(2020, 6, 16).and_hms(4, 33, 50))
1165+
.build_result_failed()
1166+
.create()?;
1167+
1168+
let links = get_release_links("/releases/failures", env.frontend())?;
1169+
1170+
// output is sorted by stars, not release-time
1171+
assert_eq!(links.len(), 1);
1172+
assert_eq!(links[0], "/crate/crate_that_failed_with_github/0.1.0");
1173+
1174+
Ok(())
1175+
})
1176+
}
1177+
1178+
#[test]
1179+
fn releases_failed_by_time() {
1180+
wrapper(|env| {
1181+
env.fake_release()
1182+
.name("crate_that_succeeded_with_github")
1183+
.version("0.1.0")
1184+
.github_stats("some/repo", 33, 22, 11)
1185+
.release_time(Utc.ymd(2020, 4, 16).and_hms(4, 33, 50))
1186+
.create()?;
1187+
// make sure that crates get at most one release shown, so they don't crowd the page
1188+
env.fake_release()
1189+
.name("crate_that_succeeded_with_github")
1190+
.github_stats("some/repo", 33, 22, 11)
1191+
.release_time(Utc.ymd(2020, 5, 16).and_hms(4, 33, 50))
1192+
.version("0.2.0")
1193+
.create()?;
1194+
env.fake_release()
1195+
.name("crate_that_failed")
1196+
.version("0.1.0")
1197+
.release_time(Utc.ymd(2020, 6, 16).and_hms(4, 33, 50))
1198+
.build_result_failed()
1199+
.create()?;
1200+
1201+
let links = get_release_links("/releases/recent-failures", env.frontend())?;
1202+
1203+
assert_eq!(links.len(), 1);
1204+
assert_eq!(links[0], "/crate/crate_that_failed/0.1.0");
1205+
1206+
Ok(())
1207+
})
1208+
}
1209+
1210+
#[test]
1211+
fn releases_homepage_and_recent() {
1212+
wrapper(|env| {
1213+
env.fake_release()
1214+
.name("crate_that_succeeded_with_github")
1215+
.version("0.1.0")
1216+
.github_stats("some/repo", 33, 22, 11)
1217+
.release_time(Utc.ymd(2020, 4, 16).and_hms(4, 33, 50))
1218+
.create()?;
1219+
// make sure that crates get at most one release shown, so they don't crowd the page
1220+
env.fake_release()
1221+
.name("crate_that_succeeded_with_github")
1222+
.github_stats("some/repo", 33, 22, 11)
1223+
.release_time(Utc.ymd(2020, 5, 16).and_hms(4, 33, 50))
1224+
.version("0.2.0")
1225+
.create()?;
1226+
env.fake_release()
1227+
.name("crate_that_failed")
1228+
.version("0.1.0")
1229+
.release_time(Utc.ymd(2020, 6, 16).and_hms(4, 33, 50))
1230+
.build_result_failed()
1231+
.create()?;
1232+
1233+
for page in &["/", "/releases"] {
1234+
let links = get_release_links(page, env.frontend())?;
1235+
1236+
assert_eq!(links.len(), 2);
1237+
assert_eq!(links[0], "/crate/crate_that_failed/0.1.0");
1238+
assert_eq!(
1239+
links[1],
1240+
"/crate_that_succeeded_with_github/0.2.0/crate_that_succeeded_with_github"
1241+
);
11311242
}
1243+
11321244
Ok(())
11331245
})
11341246
}

0 commit comments

Comments
 (0)