From 7ef5e988b98bc22e95098aa643e2f4dba10dd485 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Sun, 5 Apr 2020 10:47:18 -0500 Subject: [PATCH 1/6] Rebased with master and followed a few clippy lints --- src/db/add_package.rs | 5 ++- src/db/blacklist.rs | 6 +-- src/db/file.rs | 24 +++++------ src/docbuilder/crates.rs | 2 +- src/docbuilder/limits.rs | 40 +++++++++-------- src/docbuilder/metadata.rs | 4 +- src/utils/github_updater.rs | 2 +- src/utils/pubsubhubbub.rs | 2 +- src/web/builds.rs | 14 +++--- src/web/crate_details.rs | 60 +++++++++++++++----------- src/web/mod.rs | 2 +- src/web/releases.rs | 30 +++++++------ src/web/rustdoc.rs | 1 + src/web/sitemap.rs | 19 ++++---- src/web/source.rs | 86 ++++++++++++++++++++----------------- 15 files changed, 162 insertions(+), 135 deletions(-) diff --git a/src/db/add_package.rs b/src/db/add_package.rs index 2c30f751b..b7117a703 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -185,13 +185,16 @@ fn initialize_package_in_database(conn: &Connection, pkg: &MetadataPackage) -> R /// Convert dependencies into Vec<(String, String, String)> fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)> { - let mut dependencies: Vec<(String, String, String)> = Vec::new(); + let mut dependencies: Vec<(String, String, String)> = + Vec::with_capacity(pkg.dependencies.len()); + for dependency in &pkg.dependencies { let name = dependency.name.clone(); let version = dependency.req.clone(); let kind = dependency.kind.clone().unwrap_or_else(|| "normal".into()); dependencies.push((name, version, kind.to_string())); } + dependencies } diff --git a/src/db/blacklist.rs b/src/db/blacklist.rs index 298e0570c..22686f503 100644 --- a/src/db/blacklist.rs +++ b/src/db/blacklist.rs @@ -83,11 +83,11 @@ mod tests { crate::test::wrapper(|env| { let db = env.db(); - assert!(is_blacklisted(&db.conn(), "crate foo")? == false); + assert!(!is_blacklisted(&db.conn(), "crate foo")?); add_crate(&db.conn(), "crate foo")?; - assert!(is_blacklisted(&db.conn(), "crate foo")? == true); + assert!(is_blacklisted(&db.conn(), "crate foo")?); remove_crate(&db.conn(), "crate foo")?; - assert!(is_blacklisted(&db.conn(), "crate foo")? == false); + assert!(!is_blacklisted(&db.conn(), "crate foo")?); Ok(()) }); } diff --git a/src/db/file.rs b/src/db/file.rs index 3280d1e40..d0c46402e 100644 --- a/src/db/file.rs +++ b/src/db/file.rs @@ -37,17 +37,15 @@ pub fn add_path_into_database>( } fn file_list_to_json(file_list: Vec<(PathBuf, String)>) -> Result { - let mut file_list_json: Vec = Vec::new(); - - for file in file_list { - let mut v = Vec::with_capacity(2); - v.push(Value::String(file.1)); - v.push(Value::String( - file.0.into_os_string().into_string().unwrap(), - )); - - file_list_json.push(Value::Array(v)); - } - - Ok(Value::Array(file_list_json)) + let file_list: Vec<_> = file_list + .into_iter() + .map(|(path, name)| { + Value::Array(vec![ + Value::String(name), + Value::String(path.into_os_string().into_string().unwrap()), + ]) + }) + .collect(); + + Ok(Value::Array(file_list)) } diff --git a/src/docbuilder/crates.rs b/src/docbuilder/crates.rs index 08de1d100..6368f7325 100644 --- a/src/docbuilder/crates.rs +++ b/src/docbuilder/crates.rs @@ -12,7 +12,7 @@ where let reader = fs::File::open(path).map(BufReader::new)?; let mut name = String::new(); - let mut versions = Vec::new(); + let mut versions = Vec::new(); // TODO: Find a way to pre-allocate the versions vector for line in reader.lines() { // some crates have invalid UTF-8 (nanny-sys-0.0.7) diff --git a/src/docbuilder/limits.rs b/src/docbuilder/limits.rs index 3db3c45fc..fe58aa2a8 100644 --- a/src/docbuilder/limits.rs +++ b/src/docbuilder/limits.rs @@ -119,6 +119,7 @@ fn scale(value: usize, interval: usize, labels: &[&str]) -> String { mod test { use super::*; use crate::test::*; + #[test] fn retrieve_limits() { wrapper(|env| { @@ -151,40 +152,41 @@ mod test { targets: 1, ..Limits::default() }; - db.conn().query("INSERT INTO sandbox_overrides (crate_name, max_memory_bytes, timeout_seconds, max_targets) - VALUES ($1, $2, $3, $4)", - &[&krate, &(limits.memory as i64), &(limits.timeout.as_secs() as i32), &(limits.targets as i32)])?; + db.conn().query( + "INSERT INTO sandbox_overrides (crate_name, max_memory_bytes, timeout_seconds, max_targets) + VALUES ($1, $2, $3, $4)", + &[&krate, &(limits.memory as i64), &(limits.timeout.as_secs() as i32), &(limits.targets as i32)] + )?; assert_eq!(limits, Limits::for_crate(&db.conn(), krate)?); Ok(()) }); } + #[test] fn display_limits() { let limits = Limits { - memory: 102400, + memory: 102_400, timeout: Duration::from_secs(300), targets: 1, ..Limits::default() }; let display = limits.for_website(); + assert_eq!(display.get("Network access"), Some(&"blocked".into())); assert_eq!( - display.get("Network access".into()), - Some(&"blocked".into()) - ); - assert_eq!( - display.get("Maximum size of a build log".into()), + display.get("Maximum size of a build log"), Some(&"100 KB".into()) ); assert_eq!( - display.get("Maximum number of build targets".into()), + display.get("Maximum number of build targets"), Some(&limits.targets.to_string()) ); assert_eq!( - display.get("Maximum rustdoc execution time".into()), + display.get("Maximum rustdoc execution time"), Some(&"5 minutes".into()) ); - assert_eq!(display.get("Available RAM".into()), Some(&"100 KB".into())); + assert_eq!(display.get("Available RAM"), Some(&"100 KB".into())); } + #[test] fn scale_limits() { // time @@ -197,18 +199,18 @@ mod test { assert_eq!(SIZE_SCALE(100), "100 bytes"); assert_eq!(SIZE_SCALE(1024), "1 KB"); assert_eq!(SIZE_SCALE(10240), "10 KB"); - assert_eq!(SIZE_SCALE(1048576), "1 MB"); - assert_eq!(SIZE_SCALE(10485760), "10 MB"); - assert_eq!(SIZE_SCALE(1073741824), "1 GB"); - assert_eq!(SIZE_SCALE(10737418240), "10 GB"); + assert_eq!(SIZE_SCALE(1_048_576), "1 MB"); + assert_eq!(SIZE_SCALE(10_485_760), "10 MB"); + assert_eq!(SIZE_SCALE(1_073_741_824), "1 GB"); + assert_eq!(SIZE_SCALE(10_737_418_240), "10 GB"); assert_eq!(SIZE_SCALE(std::u32::MAX as usize), "4 GB"); // fractional sizes assert_eq!(TIME_SCALE(90), "1.5 minutes"); assert_eq!(TIME_SCALE(5400), "1.5 hours"); - assert_eq!(SIZE_SCALE(1288490189), "1.2 GB"); - assert_eq!(SIZE_SCALE(3758096384), "3.5 GB"); - assert_eq!(SIZE_SCALE(1048051712), "999.5 MB"); + assert_eq!(SIZE_SCALE(1_288_490_189), "1.2 GB"); + assert_eq!(SIZE_SCALE(3_758_096_384), "3.5 GB"); + assert_eq!(SIZE_SCALE(1_048_051_712), "999.5 MB"); } } diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 7581324aa..eb7d607a7 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -234,8 +234,8 @@ mod test { let metadata = Metadata::from_str(manifest); assert!(metadata.features.is_some()); - assert!(metadata.all_features == true); - assert!(metadata.no_default_features == true); + assert!(metadata.all_features); + assert!(metadata.no_default_features); assert!(metadata.default_target.is_some()); assert!(metadata.rustdoc_args.is_some()); diff --git a/src/utils/github_updater.rs b/src/utils/github_updater.rs index 557343f42..16283376e 100644 --- a/src/utils/github_updater.rs +++ b/src/utils/github_updater.rs @@ -186,7 +186,7 @@ mod test { assert!(fields.is_ok()); let fields = fields.unwrap(); - assert!(fields.description != "".to_string()); + assert!(fields.description != ""); assert!(fields.stars >= 0); assert!(fields.forks >= 0); assert!(fields.issues >= 0); diff --git a/src/utils/pubsubhubbub.rs b/src/utils/pubsubhubbub.rs index c8d4e4ddb..e286d9882 100644 --- a/src/utils/pubsubhubbub.rs +++ b/src/utils/pubsubhubbub.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use reqwest::*; fn ping_hub(url: &str) -> Result { - let mut params = HashMap::new(); + let mut params = HashMap::with_capacity(2); params.insert("hub.mode", "publish"); params.insert("hub.url", "https://docs.rs/releases/feed"); let client = Client::new(); diff --git a/src/web/builds.rs b/src/web/builds.rs index b223a8c51..49aec1787 100644 --- a/src/web/builds.rs +++ b/src/web/builds.rs @@ -70,11 +70,7 @@ pub fn build_list_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool).get()?; let limits = ctry!(Limits::for_crate(&conn, name)); - let mut build_list: Vec = Vec::new(); - let mut build_details = None; - - // FIXME: getting builds.output may cause performance issues when release have tons of builds - for row in &ctry!(conn.query( + let query = ctry!(conn.query( "SELECT crates.name, releases.version, releases.description, @@ -92,7 +88,13 @@ pub fn build_list_handler(req: &mut Request) -> IronResult { WHERE crates.name = $1 AND releases.version = $2 ORDER BY id DESC", &[&name, &version] - )) { + )); + + let mut build_list: Vec = Vec::with_capacity(query.len()); + let mut build_details = None; + + // FIXME: getting builds.output may cause performance issues when release have tons of builds + for row in query.iter() { let id: i32 = row.get(5); let build = Build { diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index c646a1cc0..71b85c84b 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -157,25 +157,31 @@ impl CrateDetails { // sort versions with semver let releases = { - let mut versions: Vec = Vec::new(); let versions_from_db: Value = rows.get(0).get(17); - if let Some(vers) = versions_from_db.as_array() { - for version in vers { - if let Some(version) = version.as_str() { - if let Ok(sem_ver) = semver::Version::parse(&version) { - versions.push(sem_ver); + if let Some(versions_from_db) = versions_from_db.as_array() { + let mut versions: Vec = versions_from_db + .iter() + .filter_map(|version| { + if let Some(version) = version.as_str() { + if let Ok(sem_ver) = semver::Version::parse(&version) { + return Some(sem_ver); + } } - } - } - } - versions.sort(); - versions.reverse(); - versions - .iter() - .map(|version| map_to_release(&conn, crate_id, version.to_string())) - .collect() + None + }) + .collect(); + + versions.sort(); + versions.reverse(); + versions + .iter() + .map(|version| map_to_release(&conn, crate_id, version.to_string())) + .collect() + } else { + Vec::new() + } }; let metadata = MetaData { @@ -237,7 +243,7 @@ impl CrateDetails { } // get authors - for row in &conn + let authors = conn .query( "SELECT name, slug FROM authors @@ -245,22 +251,26 @@ impl CrateDetails { WHERE rid = $1", &[&release_id], ) - .unwrap() - { + .unwrap(); + crate_details.authors.reserve(authors.len()); + + for row in authors.iter() { crate_details.authors.push((row.get(0), row.get(1))); } // get owners - for row in &conn + let owners = conn .query( "SELECT login, avatar - FROM owners - INNER JOIN owner_rels ON owner_rels.oid = owners.id - WHERE cid = $1", + FROM owners + INNER JOIN owner_rels ON owner_rels.oid = owners.id + WHERE cid = $1", &[&crate_id], ) - .unwrap() - { + .unwrap(); + crate_details.owners.reserve(owners.len()); + + for row in owners.iter() { crate_details.owners.push((row.get(0), row.get(1))); } @@ -394,7 +404,7 @@ mod tests { expected_last_successful_build: Option<&str>, ) -> Result<(), Error> { let details = CrateDetails::new(&db.conn(), package, version) - .ok_or(failure::err_msg("could not fetch crate details"))?; + .ok_or_else(|| failure::err_msg("could not fetch crate details"))?; assert_eq!( details.last_successful_build, diff --git a/src/web/mod.rs b/src/web/mod.rs index 88dc7e980..930e93e51 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -289,7 +289,7 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> Option // we need to sort versions first let versions_sem = { - let mut versions_sem: Vec<(Version, i32)> = Vec::new(); + let mut versions_sem: Vec<(Version, i32)> = Vec::with_capacity(versions.len()); for version in versions.iter().filter(|(_, _, yanked)| !yanked) { // in theory a crate must always have a semver compatible version, but check result just in case diff --git a/src/web/releases.rs b/src/web/releases.rs index d79d4509a..8f568097f 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -140,9 +140,10 @@ pub(crate) fn get_releases(conn: &Connection, page: i64, limit: i64, order: Orde LIMIT $1 OFFSET $2" } }; + let query = conn.query(&query, &[&limit, &offset]).unwrap(); - let mut packages = Vec::new(); - for row in &conn.query(&query, &[&limit, &offset]).unwrap() { + let mut packages = Vec::with_capacity(query.len()); + for row in query.iter() { let package = Release { name: row.get(0), version: row.get(1), @@ -182,10 +183,11 @@ fn get_releases_by_author( WHERE authors.slug = $1 ORDER BY crates.github_stars DESC LIMIT $2 OFFSET $3"; + let query = conn.query(&query, &[&author, &limit, &offset]).unwrap(); let mut author_name = String::new(); - let mut packages = Vec::new(); - for row in &conn.query(&query, &[&author, &limit, &offset]).unwrap() { + let mut packages = Vec::with_capacity(query.len()); + for row in query.iter() { let package = Release { name: row.get(0), version: row.get(1), @@ -227,10 +229,11 @@ fn get_releases_by_owner( WHERE owners.login = $1 ORDER BY crates.github_stars DESC LIMIT $2 OFFSET $3"; + let query = conn.query(&query, &[&author, &limit, &offset]).unwrap(); let mut author_name = String::new(); - let mut packages = Vec::new(); - for row in &conn.query(&query, &[&author, &limit, &offset]).unwrap() { + let mut packages = Vec::with_capacity(query.len()); + for row in query.iter() { let package = Release { name: row.get(0), version: row.get(1), @@ -637,17 +640,18 @@ pub fn activity_handler(req: &mut Request) -> IronResult { pub fn build_queue_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool).get()?; - let mut crates: Vec<(String, String, i32)> = Vec::new(); - for krate in &conn + let query = conn .query( "SELECT name, version, priority - FROM queue - WHERE attempt < 5 - ORDER BY priority ASC, attempt ASC, id ASC", + FROM queue + WHERE attempt < 5 + ORDER BY priority ASC, attempt ASC, id ASC", &[], ) - .unwrap() - { + .unwrap(); + + let mut crates: Vec<(String, String, i32)> = Vec::with_capacity(query.len()); + for krate in query.iter() { crates.push(( krate.get("name"), krate.get("version"), diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 3888b882a..893992529 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -579,6 +579,7 @@ mod test { let data = web.get(path).send()?.text()?; println!("{}", data); let dom = kuchiki::parse_html().one(data); + if let Some(elem) = dom .select("form > ul > li > a.warn") .expect("invalid selector") diff --git a/src/web/sitemap.rs b/src/web/sitemap.rs index fb1ffe7cc..570d46b8d 100644 --- a/src/web/sitemap.rs +++ b/src/web/sitemap.rs @@ -7,19 +7,20 @@ use std::collections::BTreeMap; pub fn sitemap_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool).get()?; - let mut releases: Vec<(String, String)> = Vec::new(); - for row in &conn + let query = conn .query( "SELECT DISTINCT ON (crates.name) - crates.name, - releases.release_time - FROM crates - INNER JOIN releases ON releases.crate_id = crates.id - WHERE rustdoc_status = true", + crates.name, + releases.release_time + FROM crates + INNER JOIN releases ON releases.crate_id = crates.id + WHERE rustdoc_status = true", &[], ) - .unwrap() - { + .unwrap(); + + let mut releases: Vec<(String, String)> = Vec::with_capacity(query.len()); + for row in query.iter() { releases.push((row.get(0), format!("{}", time::at(row.get(1)).rfc3339()))); } let mut resp = ctry!(Page::new(releases).to_resp("sitemap")); diff --git a/src/web/source.rs b/src/web/source.rs index a1fd60a65..7fe211ceb 100644 --- a/src/web/source.rs +++ b/src/web/source.rs @@ -104,49 +104,55 @@ impl FileList { let files: Value = rows.get(0).get_opt(5).unwrap().ok()?; - let mut file_list: Vec = Vec::new(); - - if let Some(files) = files.as_array() { - for file in files { - if let Some(file) = file.as_array() { - let mime = file[0].as_str().unwrap(); - let path = file[1].as_str().unwrap(); - - // skip .cargo-ok generated by cargo - if path == ".cargo-ok" { - continue; - } + let mut file_list: Vec = if let Some(files) = files.as_array() { + let mut file_list = files + .iter() + .filter_map(|file| { + if let Some(file) = file.as_array() { + let mime = file[0].as_str().unwrap(); + let path = file[1].as_str().unwrap(); + + // skip .cargo-ok generated by cargo + if path == ".cargo-ok" { + return None; + } - // look only files for req_path - if path.starts_with(&req_path) { - // remove req_path from path to reach files in this directory - let path = path.replace(&req_path, ""); - let path_splited: Vec<&str> = path.split('/').collect(); - - // if path have '/' it is a directory - let ftype = if path_splited.len() > 1 { - FileType::Dir - } else if mime.starts_with("text") && path_splited[0].ends_with(".rs") { - FileType::RustSource - } else if mime.starts_with("text") { - FileType::Text - } else { - FileType::Binary - }; - - let file = File { - name: path_splited[0].to_owned(), - file_type: ftype, - }; - - // avoid adding duplicates, a directory may occur more than once - if !file_list.contains(&file) { - file_list.push(file); + // look only files for req_path + if path.starts_with(&req_path) { + // remove req_path from path to reach files in this directory + let path = path.replace(&req_path, ""); + let path_splited: Vec<&str> = path.split('/').collect(); + + // if path have '/' it is a directory + let ftype = if path_splited.len() > 1 { + FileType::Dir + } else if mime.starts_with("text") && path_splited[0].ends_with(".rs") { + FileType::RustSource + } else if mime.starts_with("text") { + FileType::Text + } else { + FileType::Binary + }; + + let file = File { + name: path_splited[0].to_owned(), + file_type: ftype, + }; + + // avoid adding duplicates, a directory may occur more than once + return Some(file); } } - } - } - } + + None + }) + .collect::>(); + + file_list.dedup(); + file_list + } else { + Vec::new() + }; if file_list.is_empty() { return None; From 53d407566683ce5f4a3939c7051941c3a8451475 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 18:51:51 -0500 Subject: [PATCH 2/6] Converted convert_dependencies into a map --- src/db/add_package.rs | 19 ++++--- src/index/api.rs | 74 +++++++++++++------------- src/web/builds.rs | 41 +++++++-------- src/web/releases.rs | 117 ++++++++++++++++++++++-------------------- src/web/sitemap.rs | 12 +++-- 5 files changed, 134 insertions(+), 129 deletions(-) diff --git a/src/db/add_package.rs b/src/db/add_package.rs index b7117a703..6a9517655 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -185,17 +185,16 @@ fn initialize_package_in_database(conn: &Connection, pkg: &MetadataPackage) -> R /// Convert dependencies into Vec<(String, String, String)> fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)> { - let mut dependencies: Vec<(String, String, String)> = - Vec::with_capacity(pkg.dependencies.len()); + pkg.dependencies + .iter() + .map(|dependency| { + let name = dependency.name.clone(); + let version = dependency.req.clone(); + let kind = dependency.kind.clone().unwrap_or_else(|| "normal".into()); - for dependency in &pkg.dependencies { - let name = dependency.name.clone(); - let version = dependency.req.clone(); - let kind = dependency.kind.clone().unwrap_or_else(|| "normal".into()); - dependencies.push((name, version, kind.to_string())); - } - - dependencies + (name, version, kind) + }) + .collect() } /// Reads readme if there is any read defined in Cargo.toml of a Package diff --git a/src/index/api.rs b/src/index/api.rs index 3dd480d34..f645ea61e 100644 --- a/src/index/api.rs +++ b/src/index/api.rs @@ -116,47 +116,43 @@ fn get_owners(pkg: &MetadataPackage) -> Result> { res.read_to_string(&mut body).unwrap(); let json: Value = serde_json::from_str(&body[..])?; - let mut result = Vec::new(); - if let Some(owners) = json + let owners = json .as_object() .and_then(|j| j.get("users")) - .and_then(|j| j.as_array()) - { - for owner in owners { - // FIXME: I know there is a better way to do this - let avatar = owner - .as_object() - .and_then(|o| o.get("avatar")) - .and_then(|o| o.as_str()) - .unwrap_or(""); - let email = owner - .as_object() - .and_then(|o| o.get("email")) - .and_then(|o| o.as_str()) - .unwrap_or(""); - let login = owner - .as_object() - .and_then(|o| o.get("login")) - .and_then(|o| o.as_str()) - .unwrap_or(""); - let name = owner - .as_object() - .and_then(|o| o.get("name")) - .and_then(|o| o.as_str()) - .unwrap_or(""); - - if login.is_empty() { - continue; - } - - result.push(CrateOwner { - avatar: avatar.to_string(), - email: email.to_string(), - login: login.to_string(), - name: name.to_string(), - }); - } - } + .and_then(|j| j.as_array()); + + let result = if let Some(owners) = owners { + owners + .iter() + .filter_map(|owner| { + fn extract<'a>(owner: &'a Value, field: &str) -> &'a str { + owner + .as_object() + .and_then(|o| o.get(field)) + .and_then(|o| o.as_str()) + .unwrap_or_default() + } + + let avatar = extract(owner, "avatar"); + let email = extract(owner, "email"); + let login = extract(owner, "login"); + let name = extract(owner, "name"); + + if login.is_empty() { + return None; + } + + Some(CrateOwner { + avatar: avatar.to_string(), + email: email.to_string(), + login: login.to_string(), + name: name.to_string(), + }) + }) + .collect() + } else { + Vec::new() + }; Ok(result) } diff --git a/src/web/builds.rs b/src/web/builds.rs index 49aec1787..9c3223003 100644 --- a/src/web/builds.rs +++ b/src/web/builds.rs @@ -90,28 +90,29 @@ pub fn build_list_handler(req: &mut Request) -> IronResult { &[&name, &version] )); - let mut build_list: Vec = Vec::with_capacity(query.len()); let mut build_details = None; - // FIXME: getting builds.output may cause performance issues when release have tons of builds - for row in query.iter() { - let id: i32 = row.get(5); - - let build = Build { - id, - rustc_version: row.get(6), - cratesfyi_version: row.get(7), - build_status: row.get(8), - build_time: row.get(9), - output: row.get(10), - }; - - if id == req_build_id { - build_details = Some(build.clone()); - } - - build_list.push(build); - } + let mut build_list = query + .into_iter() + .map(|row| { + let id: i32 = row.get(5); + + let build = Build { + id, + rustc_version: row.get(6), + cratesfyi_version: row.get(7), + build_status: row.get(8), + build_time: row.get(9), + output: row.get(10), + }; + + if id == req_build_id { + build_details = Some(build.clone()); + } + + build + }) + .collect::>(); if req.url.path().join("/").ends_with(".json") { use iron::headers::{ diff --git a/src/web/releases.rs b/src/web/releases.rs index 8f568097f..a5c09c153 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -142,9 +142,9 @@ pub(crate) fn get_releases(conn: &Connection, page: i64, limit: i64, order: Orde }; let query = conn.query(&query, &[&limit, &offset]).unwrap(); - let mut packages = Vec::with_capacity(query.len()); - for row in query.iter() { - let package = Release { + query + .into_iter() + .map(|row| Release { name: row.get(0), version: row.get(1), description: row.get(2), @@ -152,12 +152,8 @@ pub(crate) fn get_releases(conn: &Connection, page: i64, limit: i64, order: Orde release_time: row.get(4), rustdoc_status: row.get(5), stars: row.get(6), - }; - - packages.push(package); - } - - packages + }) + .collect() } fn get_releases_by_author( @@ -185,24 +181,27 @@ fn get_releases_by_author( LIMIT $2 OFFSET $3"; let query = conn.query(&query, &[&author, &limit, &offset]).unwrap(); - let mut author_name = String::new(); - let mut packages = Vec::with_capacity(query.len()); - for row in query.iter() { - let package = Release { - name: row.get(0), - version: row.get(1), - description: row.get(2), - target_name: row.get(3), - release_time: row.get(4), - rustdoc_status: row.get(5), - stars: row.get(6), - }; + let mut author_name = None; + let packages = query + .into_iter() + .map(|row| { + if author_name.is_none() { + author_name = Some(row.get(7)); + } - author_name = row.get(7); - packages.push(package); - } + Release { + name: row.get(0), + version: row.get(1), + description: row.get(2), + target_name: row.get(3), + release_time: row.get(4), + rustdoc_status: row.get(5), + stars: row.get(6), + } + }) + .collect(); - (author_name, packages) + (author_name.unwrap_or_default(), packages) } fn get_releases_by_owner( @@ -231,28 +230,31 @@ fn get_releases_by_owner( LIMIT $2 OFFSET $3"; let query = conn.query(&query, &[&author, &limit, &offset]).unwrap(); - let mut author_name = String::new(); - let mut packages = Vec::with_capacity(query.len()); - for row in query.iter() { - let package = Release { - name: row.get(0), - version: row.get(1), - description: row.get(2), - target_name: row.get(3), - release_time: row.get(4), - rustdoc_status: row.get(5), - stars: row.get(6), - }; + let mut author_name = None; + let packages = query + .into_iter() + .map(|row| { + if author_name.is_none() { + author_name = Some(if !row.get::(7).is_empty() { + row.get(7) + } else { + row.get(8) + }); + } - author_name = if !row.get::(7).is_empty() { - row.get(7) - } else { - row.get(8) - }; - packages.push(package); - } + Release { + name: row.get(0), + version: row.get(1), + description: row.get(2), + target_name: row.get(3), + release_time: row.get(4), + rustdoc_status: row.get(5), + stars: row.get(6), + } + }) + .collect(); - (author_name, packages) + (author_name.unwrap_or_default(), packages) } /// Get the search results for a crate search query @@ -650,17 +652,20 @@ pub fn build_queue_handler(req: &mut Request) -> IronResult { ) .unwrap(); - let mut crates: Vec<(String, String, i32)> = Vec::with_capacity(query.len()); - for krate in query.iter() { - crates.push(( - krate.get("name"), - krate.get("version"), - // The priority here is inverted: in the database if a crate has a higher priority it - // will be built after everything else, which is counter-intuitive for people not - // familiar with docs.rs's inner workings. - -krate.get::<_, i32>("priority"), - )); - } + let crates = query + .into_iter() + .map(|krate| { + ( + krate.get("name"), + krate.get("version"), + // The priority here is inverted: in the database if a crate has a higher priority it + // will be built after everything else, which is counter-intuitive for people not + // familiar with docs.rs's inner workings. + -krate.get::<_, i32>("priority"), + ) + }) + .collect::>(); + let is_empty = crates.is_empty(); Page::new(crates) .title("Build queue") diff --git a/src/web/sitemap.rs b/src/web/sitemap.rs index 570d46b8d..74587698f 100644 --- a/src/web/sitemap.rs +++ b/src/web/sitemap.rs @@ -19,10 +19,14 @@ pub fn sitemap_handler(req: &mut Request) -> IronResult { ) .unwrap(); - let mut releases: Vec<(String, String)> = Vec::with_capacity(query.len()); - for row in query.iter() { - releases.push((row.get(0), format!("{}", time::at(row.get(1)).rfc3339()))); - } + let releases = query.into_iter() + .map(|row| { + let time = format!("{}", time::at(row.get(1)).rfc3339()); + + (row.get(0), time) + }) + .collect::>(); + let mut resp = ctry!(Page::new(releases).to_resp("sitemap")); resp.headers .set(ContentType("application/xml".parse().unwrap())); From 0589c192d1b6bb147d926bbe2cd2fa330b5092c9 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 22:56:57 -0500 Subject: [PATCH 3/6] Greasin' the wheels --- src/db/add_package.rs | 2 +- src/docbuilder/crates.rs | 2 +- src/web/crate_details.rs | 4 ++-- src/web/releases.rs | 10 +++++----- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/db/add_package.rs b/src/db/add_package.rs index 6a9517655..a2e02d31c 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -190,7 +190,7 @@ fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)> .map(|dependency| { let name = dependency.name.clone(); let version = dependency.req.clone(); - let kind = dependency.kind.clone().unwrap_or_else(|| "normal".into()); + let kind = dependency.kind.clone().unwrap_or_default(); (name, version, kind) }) diff --git a/src/docbuilder/crates.rs b/src/docbuilder/crates.rs index 6368f7325..08de1d100 100644 --- a/src/docbuilder/crates.rs +++ b/src/docbuilder/crates.rs @@ -12,7 +12,7 @@ where let reader = fs::File::open(path).map(BufReader::new)?; let mut name = String::new(); - let mut versions = Vec::new(); // TODO: Find a way to pre-allocate the versions vector + let mut versions = Vec::new(); for line in reader.lines() { // some crates have invalid UTF-8 (nanny-sys-0.0.7) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 71b85c84b..4dfeeaff8 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -254,7 +254,7 @@ impl CrateDetails { .unwrap(); crate_details.authors.reserve(authors.len()); - for row in authors.iter() { + for row in &authors { crate_details.authors.push((row.get(0), row.get(1))); } @@ -270,7 +270,7 @@ impl CrateDetails { .unwrap(); crate_details.owners.reserve(owners.len()); - for row in owners.iter() { + for row in &owners { crate_details.owners.push((row.get(0), row.get(1))); } diff --git a/src/web/releases.rs b/src/web/releases.rs index a5c09c153..bcdc1411c 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -645,14 +645,14 @@ pub fn build_queue_handler(req: &mut Request) -> IronResult { let query = conn .query( "SELECT name, version, priority - FROM queue - WHERE attempt < 5 - ORDER BY priority ASC, attempt ASC, id ASC", + FROM queue + WHERE attempt < 5 + ORDER BY priority ASC, attempt ASC, id ASC", &[], ) .unwrap(); - let crates = query + let crates: Vec<(String, String, i32)> = query .into_iter() .map(|krate| { ( @@ -664,7 +664,7 @@ pub fn build_queue_handler(req: &mut Request) -> IronResult { -krate.get::<_, i32>("priority"), ) }) - .collect::>(); + .collect(); let is_empty = crates.is_empty(); Page::new(crates) From 18743a11fddc5459c6a7449fdd5153b3d479b4f4 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 23:04:20 -0500 Subject: [PATCH 4/6] Put it on a slip-n-slide --- src/db/add_package.rs | 5 ++++- src/web/crate_details.rs | 28 ++++++++++++++-------------- src/web/sitemap.rs | 3 ++- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/db/add_package.rs b/src/db/add_package.rs index a2e02d31c..0e85cbd6e 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -190,7 +190,10 @@ fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)> .map(|dependency| { let name = dependency.name.clone(); let version = dependency.req.clone(); - let kind = dependency.kind.clone().unwrap_or_default(); + let kind = dependency + .kind + .clone() + .unwrap_or_else(|| "normal".to_string()); (name, version, kind) }) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 4dfeeaff8..37fdf3a25 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -246,33 +246,33 @@ impl CrateDetails { let authors = conn .query( "SELECT name, slug - FROM authors - INNER JOIN author_rels ON author_rels.aid = authors.id - WHERE rid = $1", + FROM authors + INNER JOIN author_rels ON author_rels.aid = authors.id + WHERE rid = $1", &[&release_id], ) .unwrap(); - crate_details.authors.reserve(authors.len()); - for row in &authors { - crate_details.authors.push((row.get(0), row.get(1))); - } + crate_details.authors = authors + .into_iter() + .map(|row| (row.get(0), row.get(1))) + .collect(); // get owners let owners = conn .query( "SELECT login, avatar - FROM owners - INNER JOIN owner_rels ON owner_rels.oid = owners.id - WHERE cid = $1", + FROM owners + INNER JOIN owner_rels ON owner_rels.oid = owners.id + WHERE cid = $1", &[&crate_id], ) .unwrap(); - crate_details.owners.reserve(owners.len()); - for row in &owners { - crate_details.owners.push((row.get(0), row.get(1))); - } + crate_details.owners = owners + .into_iter() + .map(|row| (row.get(0), row.get(1))) + .collect(); if !crate_details.build_status { crate_details.last_successful_build = crate_details diff --git a/src/web/sitemap.rs b/src/web/sitemap.rs index 74587698f..2fb3eebed 100644 --- a/src/web/sitemap.rs +++ b/src/web/sitemap.rs @@ -19,7 +19,8 @@ pub fn sitemap_handler(req: &mut Request) -> IronResult { ) .unwrap(); - let releases = query.into_iter() + let releases = query + .into_iter() .map(|row| { let time = format!("{}", time::at(row.get(1)).rfc3339()); From 59285c3bff9887b8a53581ba2162f662b12d4877 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Sun, 5 Apr 2020 15:09:23 -0500 Subject: [PATCH 5/6] Dedup yourself --- src/web/releases.rs | 51 ++++++++++++++++++------------------ src/web/source.rs | 63 ++++++++++++++++++++++----------------------- 2 files changed, 57 insertions(+), 57 deletions(-) diff --git a/src/web/releases.rs b/src/web/releases.rs index bcdc1411c..72b142eda 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -164,21 +164,22 @@ fn get_releases_by_author( ) -> (String, Vec) { let offset = (page - 1) * limit; - let query = "SELECT crates.name, - releases.version, - releases.description, - releases.target_name, - releases.release_time, - releases.rustdoc_status, - crates.github_stars, - authors.name - FROM crates - INNER JOIN releases ON releases.id = crates.latest_version_id - INNER JOIN author_rels ON releases.id = author_rels.rid - INNER JOIN authors ON authors.id = author_rels.aid - WHERE authors.slug = $1 - ORDER BY crates.github_stars DESC - LIMIT $2 OFFSET $3"; + let query = " + SELECT crates.name, + releases.version, + releases.description, + releases.target_name, + releases.release_time, + releases.rustdoc_status, + crates.github_stars, + authors.name + FROM crates + INNER JOIN releases ON releases.id = crates.latest_version_id + INNER JOIN author_rels ON releases.id = author_rels.rid + INNER JOIN authors ON authors.id = author_rels.aid + WHERE authors.slug = $1 + ORDER BY crates.github_stars DESC + LIMIT $2 OFFSET $3"; let query = conn.query(&query, &[&author, &limit, &offset]).unwrap(); let mut author_name = None; @@ -537,13 +538,13 @@ pub fn search_handler(req: &mut Request) -> IronResult { if query.is_empty() { let rows = ctry!(conn.query( "SELECT crates.name, - releases.version, - releases.target_name - FROM crates - INNER JOIN releases - ON crates.latest_version_id = releases.id - WHERE github_stars >= 100 AND rustdoc_status = true - OFFSET FLOOR(RANDOM() * 280) LIMIT 1", + releases.version, + releases.target_name + FROM crates + INNER JOIN releases + ON crates.latest_version_id = releases.id + WHERE github_stars >= 100 AND rustdoc_status = true + OFFSET FLOOR(RANDOM() * 280) LIMIT 1", &[] )); // ~~~~~~^ @@ -645,9 +646,9 @@ pub fn build_queue_handler(req: &mut Request) -> IronResult { let query = conn .query( "SELECT name, version, priority - FROM queue - WHERE attempt < 5 - ORDER BY priority ASC, attempt ASC, id ASC", + FROM queue + WHERE attempt < 5 + ORDER BY priority ASC, attempt ASC, id ASC", &[], ) .unwrap(); diff --git a/src/web/source.rs b/src/web/source.rs index 7fe211ceb..e8808f008 100644 --- a/src/web/source.rs +++ b/src/web/source.rs @@ -104,8 +104,8 @@ impl FileList { let files: Value = rows.get(0).get_opt(5).unwrap().ok()?; - let mut file_list: Vec = if let Some(files) = files.as_array() { - let mut file_list = files + if let Some(files) = files.as_array() { + let mut file_list: Vec = files .iter() .filter_map(|file| { if let Some(file) = file.as_array() { @@ -139,7 +139,6 @@ impl FileList { file_type: ftype, }; - // avoid adding duplicates, a directory may occur more than once return Some(file); } } @@ -148,38 +147,38 @@ impl FileList { }) .collect::>(); + if file_list.is_empty() { + return None; + } + + file_list.sort_by(|a, b| { + // directories must be listed first + if a.file_type == FileType::Dir && b.file_type != FileType::Dir { + Ordering::Less + } else if a.file_type != FileType::Dir && b.file_type == FileType::Dir { + Ordering::Greater + } else { + a.name.to_lowercase().cmp(&b.name.to_lowercase()) + } + }); + + // avoid adding duplicates, a directory may occur more than once file_list.dedup(); - file_list - } else { - Vec::new() - }; - if file_list.is_empty() { - return None; + Some(FileList { + metadata: MetaData { + name: rows.get(0).get(0), + version: rows.get(0).get(1), + description: rows.get(0).get(2), + target_name: rows.get(0).get(3), + rustdoc_status: rows.get(0).get(4), + default_target: rows.get(0).get(6), + }, + files: file_list, + }) + } else { + None } - - file_list.sort_by(|a, b| { - // directories must be listed first - if a.file_type == FileType::Dir && b.file_type != FileType::Dir { - Ordering::Less - } else if a.file_type != FileType::Dir && b.file_type == FileType::Dir { - Ordering::Greater - } else { - a.name.to_lowercase().cmp(&b.name.to_lowercase()) - } - }); - - Some(FileList { - metadata: MetaData { - name: rows.get(0).get(0), - version: rows.get(0).get(1), - description: rows.get(0).get(2), - target_name: rows.get(0).get(3), - rustdoc_status: rows.get(0).get(4), - default_target: rows.get(0).get(6), - }, - files: file_list, - }) } } From a4ee7c5ac85c2140be0f7f299bf0c3e7b72684b0 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 7 May 2020 12:10:13 -0500 Subject: [PATCH 6/6] Reverted file sorting --- src/web/source.rs | 80 +++++++++++++++++++++++------------------------ 1 file changed, 39 insertions(+), 41 deletions(-) diff --git a/src/web/source.rs b/src/web/source.rs index e8808f008..4851be467 100644 --- a/src/web/source.rs +++ b/src/web/source.rs @@ -104,48 +104,49 @@ impl FileList { let files: Value = rows.get(0).get_opt(5).unwrap().ok()?; + let mut file_list = Vec::new(); if let Some(files) = files.as_array() { - let mut file_list: Vec = files - .iter() - .filter_map(|file| { - if let Some(file) = file.as_array() { - let mime = file[0].as_str().unwrap(); - let path = file[1].as_str().unwrap(); - - // skip .cargo-ok generated by cargo - if path == ".cargo-ok" { - return None; - } + file_list.reserve(files.len()); - // look only files for req_path - if path.starts_with(&req_path) { - // remove req_path from path to reach files in this directory - let path = path.replace(&req_path, ""); - let path_splited: Vec<&str> = path.split('/').collect(); - - // if path have '/' it is a directory - let ftype = if path_splited.len() > 1 { - FileType::Dir - } else if mime.starts_with("text") && path_splited[0].ends_with(".rs") { - FileType::RustSource - } else if mime.starts_with("text") { - FileType::Text - } else { - FileType::Binary - }; - - let file = File { - name: path_splited[0].to_owned(), - file_type: ftype, - }; - - return Some(file); - } + for file in files { + if let Some(file) = file.as_array() { + let mime = file[0].as_str().unwrap(); + let path = file[1].as_str().unwrap(); + + // skip .cargo-ok generated by cargo + if path == ".cargo-ok" { + return None; } - None - }) - .collect::>(); + // look only files for req_path + if path.starts_with(&req_path) { + // remove req_path from path to reach files in this directory + let path = path.replace(&req_path, ""); + let path_splited: Vec<&str> = path.split('/').collect(); + + // if path have '/' it is a directory + let ftype = if path_splited.len() > 1 { + FileType::Dir + } else if mime.starts_with("text") && path_splited[0].ends_with(".rs") { + FileType::RustSource + } else if mime.starts_with("text") { + FileType::Text + } else { + FileType::Binary + }; + + let file = File { + name: path_splited[0].to_owned(), + file_type: ftype, + }; + + // avoid adding duplicates, a directory may occur more than once + if !file_list.contains(&file) { + file_list.push(file); + } + } + } + } if file_list.is_empty() { return None; @@ -162,9 +163,6 @@ impl FileList { } }); - // avoid adding duplicates, a directory may occur more than once - file_list.dedup(); - Some(FileList { metadata: MetaData { name: rows.get(0).get(0),