From 20cd74ae54f71fd36e033910142e16c43434bb72 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Wed, 1 Apr 2020 21:12:05 -0500 Subject: [PATCH 01/14] Refactors! Rebased --- src/db/add_package.rs | 12 +- src/docbuilder/options.rs | 9 +- src/docbuilder/rustwide_builder.rs | 2 +- src/utils/daemon.rs | 128 +++++++++++- src/utils/github_updater.rs | 3 +- src/web/mod.rs | 46 ++--- src/web/releases.rs | 14 +- src/web/rustdoc.rs | 304 +++++++++++++++++------------ 8 files changed, 347 insertions(+), 171 deletions(-) diff --git a/src/db/add_package.rs b/src/db/add_package.rs index 7ec1b7049..0914f150d 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -356,7 +356,11 @@ fn get_release_time_yanked_downloads( } } - Ok((release_time.unwrap_or(time::get_time()), yanked.unwrap_or(false), downloads.unwrap_or(0))) + Ok(( + release_time.unwrap_or_else(time::get_time), + yanked.unwrap_or(false), + downloads.unwrap_or(0), + )) } @@ -376,8 +380,7 @@ fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_ } }; // add releationship - let _ = conn.query("INSERT INTO keyword_rels (rid, kid) VALUES ($1, $2)", - &[release_id, &keyword_id]); + let _ = conn.query("INSERT INTO keyword_rels (rid, kid) VALUES ($1, $2)", &[&release_id, &keyword_id]); } Ok(()) @@ -496,8 +499,7 @@ fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id: }; // add relationship - let _ = conn.query("INSERT INTO owner_rels (cid, oid) VALUES ($1, $2)", - &[crate_id, &owner_id]); + let _ = conn.query("INSERT INTO owner_rels (cid, oid) VALUES ($1, $2)", &[&crate_id, &owner_id]); } Ok(()) } diff --git a/src/docbuilder/options.rs b/src/docbuilder/options.rs index 70dc0c604..b7cd509de 100644 --- a/src/docbuilder/options.rs +++ b/src/docbuilder/options.rs @@ -60,12 +60,11 @@ impl fmt::Debug for DocBuilderOptions { impl DocBuilderOptions { /// Creates new DocBuilderOptions from prefix pub fn from_prefix(prefix: PathBuf) -> DocBuilderOptions { - let (prefix, crates_io_index_path) = - generate_paths(prefix); + let (prefix, crates_io_index_path) = generate_paths(prefix); + DocBuilderOptions { - prefix: prefix, - crates_io_index_path: crates_io_index_path, - + prefix, + crates_io_index_path, ..Default::default() } } diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index d1175dfb7..d4ecf5132 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -215,7 +215,7 @@ impl RustwideBuilder { let file_name = if versioned { format!("{}-{}.{}", segments[1], rustc_version, segments[0]) } else { - file.to_string() + (*file).to_string() }; let source_path = source.join(&file_name); let dest_path = dest.path().join(&file_name); diff --git a/src/utils/daemon.rs b/src/utils/daemon.rs index 65a1f3c32..0763ab219 100644 --- a/src/utils/daemon.rs +++ b/src/utils/daemon.rs @@ -210,7 +210,6 @@ pub fn start_daemon(background: bool) { } }).unwrap(); - // update release activity everyday at 23:55 thread::Builder::new().name("release activity updater".to_string()).spawn(move || { loop { @@ -255,7 +254,134 @@ pub fn start_daemon(background: bool) { crate::Server::start(None); } +/// Builds crates every minute +fn queue_reader_runtime() { + let opts = opts(); + let mut doc_builder = DocBuilder::new(opts); + + /// Represents the current state of the builder thread. + enum BuilderState { + /// The builder thread has just started, and hasn't built any crates yet. + Fresh, + /// The builder has just seen an empty build queue. + EmptyQueue, + /// The builder has just seen the lock file. + Locked, + /// The builder has just finished building a crate. The enclosed count is the number of + /// crates built since the caches have been refreshed. + QueueInProgress(usize), + } + + let mut builder = RustwideBuilder::init().unwrap(); + + let mut status = BuilderState::Fresh; + + loop { + if !status.is_in_progress() { + thread::sleep(Duration::from_secs(60)); + } + + // check lock file + if doc_builder.is_locked() { + warn!("Lock file exits, skipping building new crates"); + status = BuilderState::Locked; + continue; + } + + if status.count() >= 10 { + // periodically, we need to flush our caches and ping the hubs + debug!("10 builds in a row; flushing caches"); + status = BuilderState::QueueInProgress(0); + + match pubsubhubbub::ping_hubs() { + Err(e) => error!("Failed to ping hub: {}", e), + Ok(n) => debug!("Succesfully pinged {} hubs", n) + } + + if let Err(e) = doc_builder.load_cache() { + error!("Failed to load cache: {}", e); + } + + if let Err(e) = doc_builder.save_cache() { + error!("Failed to save cache: {}", e); + } + } + + // Only build crates if there are any to build + debug!("Checking build queue"); + match doc_builder.get_queue_count() { + Err(e) => { + error!("Failed to read the number of crates in the queue: {}", e); + continue; + } + Ok(0) => { + if status.count() > 0 { + // ping the hubs before continuing + match pubsubhubbub::ping_hubs() { + Err(e) => error!("Failed to ping hub: {}", e), + Ok(n) => debug!("Succesfully pinged {} hubs", n) + } + + if let Err(e) = doc_builder.save_cache() { + error!("Failed to save cache: {}", e); + } + } + debug!("Queue is empty, going back to sleep"); + status = BuilderState::EmptyQueue; + continue; + } + Ok(queue_count) => { + info!("Starting build with {} crates in queue (currently on a {} crate streak)", + queue_count, status.count()); + } + } + + // if we're starting a new batch, reload our caches and sources + if !status.is_in_progress() { + if let Err(e) = doc_builder.load_cache() { + error!("Failed to load cache: {}", e); + continue; + } + } + + // Run build_packages_queue under `catch_unwind` to catch panics + // This only panicked twice in the last 6 months but its just a better + // idea to do this. + let res = catch_unwind(AssertUnwindSafe(|| { + match doc_builder.build_next_queue_package(&mut builder) { + Err(e) => error!("Failed to build crate from queue: {}", e), + Ok(crate_built) => if crate_built { + status.increment(); + } + + } + })); + + if let Err(e) = res { + error!("GRAVE ERROR Building new crates panicked: {:?}", e); + } + } + + impl BuilderState { + fn count(&self) -> usize { + match *self { + BuilderState::QueueInProgress(n) => n, + _ => 0, + } + } + + fn is_in_progress(&self) -> bool { + match *self { + BuilderState::QueueInProgress(_) => true, + _ => false, + } + } + fn increment(&mut self) { + *self = BuilderState::QueueInProgress(self.count() + 1); + } + } +} fn opts() -> DocBuilderOptions { let prefix = PathBuf::from(env::var("CRATESFYI_PREFIX") diff --git a/src/utils/github_updater.rs b/src/utils/github_updater.rs index c672c93f6..a1de0cf97 100644 --- a/src/utils/github_updater.rs +++ b/src/utils/github_updater.rs @@ -83,8 +83,7 @@ fn get_github_fields(path: &str) -> Result { .basic_auth( env::var("CRATESFYI_GITHUB_USERNAME") .ok() - .and_then(|u| Some(u.to_string())) - .unwrap_or("".to_string()), + .unwrap_or_default(), env::var("CRATESFYI_GITHUB_ACCESSTOKEN").ok(), ) .send()?; diff --git a/src/web/mod.rs b/src/web/mod.rs index 5bd6e5945..31fcbf0d1 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -475,29 +475,29 @@ pub(crate) struct MetaData { impl MetaData { fn from_crate(conn: &Connection, name: &str, version: &str) -> Option { - for row in &conn.query("SELECT crates.name, - releases.version, - releases.description, - releases.target_name, - releases.rustdoc_status, - releases.default_target - FROM releases - INNER JOIN crates ON crates.id = releases.crate_id - WHERE crates.name = $1 AND releases.version = $2", - &[&name, &version]) - .unwrap() { - - return Some(MetaData { - name: row.get(0), - version: row.get(1), - description: row.get(2), - target_name: row.get(3), - rustdoc_status: row.get(4), - default_target: row.get(5), - }); - } - - None + let rows = conn.query( + "SELECT crates.name, + releases.version, + releases.description, + releases.target_name, + releases.rustdoc_status, + releases.default_target + FROM releases + INNER JOIN crates ON crates.id = releases.crate_id + WHERE crates.name = $1 AND releases.version = $2", + &[&name, &version] + ).unwrap(); + + let row = rows.iter().next()?; + + Some(MetaData { + name: row.get(0), + version: row.get(1), + description: row.get(2), + target_name: row.get(3), + rustdoc_status: row.get(4), + default_target: row.get(5), + }) } } diff --git a/src/web/releases.rs b/src/web/releases.rs index 53bf13cc7..7d8124355 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -400,12 +400,14 @@ pub fn author_handler(req: &mut Request) -> IronResult { let author = ctry!(router.find("author") .ok_or(IronError::new(Nope::CrateNotFound, status::NotFound))); - let (author_name, packages) = if author.starts_with("@") { - let mut author = author.clone().split("@"); - get_releases_by_owner(&conn, - page_number, - RELEASES_IN_RELEASES, - cexpect!(author.nth(1))) + let (author_name, packages) = if author.starts_with('@') { + let mut author = author.clone().split('@'); + get_releases_by_owner( + &conn, + page_number, + RELEASES_IN_RELEASES, + cexpect!(author.nth(1)), + ) } else { get_releases_by_author(&conn, page_number, RELEASES_IN_RELEASES, author) }; diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index de7637e22..9ca504427 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -1,25 +1,23 @@ //! rustdoc handler - -use super::pool::Pool; +use super::crate_details::CrateDetails; +use super::error::Nope; use super::file::File; +use super::page::Page; +use super::pool::Pool; use super::redirect_base; -use super::crate_details::CrateDetails; +use super::{match_version, MatchVersion}; +use crate::utils; +use iron::headers::{CacheControl, CacheDirective, Expires, HttpDate}; +use iron::modifiers::Redirect; use iron::prelude::*; +use iron::Handler; use iron::{status, Url}; -use iron::modifiers::Redirect; +use postgres::Connection; use router::Router; -use super::{match_version, MatchVersion}; -use super::error::Nope; -use super::page::Page; use rustc_serialize::json::{Json, ToJson}; use std::collections::BTreeMap; -use iron::headers::{Expires, HttpDate, CacheControl, CacheDirective}; -use postgres::Connection; use time; -use iron::Handler; -use crate::utils; - #[derive(Debug)] struct RustdocPage { @@ -33,7 +31,6 @@ struct RustdocPage { crate_details: Option, } - impl Default for RustdocPage { fn default() -> RustdocPage { RustdocPage { @@ -49,7 +46,6 @@ impl Default for RustdocPage { } } - impl ToJson for RustdocPage { fn to_json(&self) -> Json { let mut m: BTreeMap = BTreeMap::new(); @@ -77,8 +73,7 @@ impl RustLangRedirector { .expect("failed to parse rust-lang.org base URL") .join(target) .expect("failed to append crate name to rust-lang.org base URL"); - let url = Url::from_generic_url(url) - .expect("failed to convert url::Url to iron::Url"); + let url = Url::from_generic_url(url).expect("failed to convert url::Url to iron::Url"); Self { url } } } @@ -92,19 +87,13 @@ impl iron::Handler for RustLangRedirector { /// Handler called for `/:crate` and `/:crate/:version` URLs. Automatically redirects to the docs /// or crate details page based on whether the given crate version was successfully built. pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { - - fn redirect_to_doc(req: &Request, - name: &str, - vers: &str, - target_name: &str) - -> IronResult { - let mut url_str = format!( - "{}/{}/{}/{}/", - redirect_base(req), - name, - vers, - target_name, - ); + fn redirect_to_doc( + req: &Request, + name: &str, + vers: &str, + target_name: &str, + ) -> IronResult { + let mut url_str = format!("{}/{}/{}/{}/", redirect_base(req), name, vers, target_name,); if let Some(query) = req.url.query() { url_str.push('?'); url_str.push_str(query); @@ -116,14 +105,10 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { Ok(resp) } - fn redirect_to_crate(req: &Request, - name: &str, - vers: &str) - -> IronResult { - let url = ctry!(Url::parse(&format!("{}/crate/{}/{}", - redirect_base(req), - name, - vers)[..])); + fn redirect_to_crate(req: &Request, name: &str, vers: &str) -> IronResult { + let url = ctry!(Url::parse( + &format!("{}/crate/{}/{}", redirect_base(req), name, vers)[..] + )); let mut resp = Response::with((status::Found, Redirect(url))); resp.headers.set(Expires(HttpDate(time::now()))); @@ -134,7 +119,14 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { // this unwrap is safe because iron urls are always able to use `path_segments` // i'm using this instead of `req.url.path()` to avoid allocating the Vec, and also to avoid // keeping the borrow alive into the return statement - if req.url.as_ref().path_segments().unwrap().last().map_or(false, |s| s.ends_with(".js")) { + if req + .url + .as_ref() + .path_segments() + .unwrap() + .last() + .map_or(false, |s| s.ends_with(".js")) + { // javascript files should be handled by the file server instead of erroneously // redirecting to the crate root page if req.url.as_ref().path_segments().unwrap().count() > 2 { @@ -149,7 +141,14 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { None => return Err(IronError::new(Nope::ResourceNotFound, status::NotFound)), } } - } else if req.url.as_ref().path_segments().unwrap().last().map_or(false, |s| s.ends_with(".ico")) { + } else if req + .url + .as_ref() + .path_segments() + .unwrap() + .last() + .map_or(false, |s| s.ends_with(".ico")) + { // route .ico files into their dedicated handler so that docs.rs's favicon is always // displayed return super::ico_handler(req); @@ -172,10 +171,12 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { // get target name and whether it has docs // FIXME: This is a bit inefficient but allowing us to use less code in general let (target_name, has_docs): (String, bool) = { - let rows = ctry!(conn.query("SELECT target_name, rustdoc_status + let rows = ctry!(conn.query( + "SELECT target_name, rustdoc_status FROM releases WHERE releases.id = $1", - &[&id])); + &[&id] + )); (rows.get(0).get(0), rows.get(0).get(1)) }; @@ -187,13 +188,11 @@ pub fn rustdoc_redirector_handler(req: &mut Request) -> IronResult { } } - /// Serves documentation generated by rustdoc. /// /// This includes all HTML files for an individual crate, as well as the `search-index.js`, which is /// also crate-specific. pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { - let router = extension!(req, Router); let name = router.find("crate").unwrap_or("").to_string(); let url_version = router.find("version"); @@ -214,11 +213,9 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { // to prevent cloudfront caching the wrong artifacts on URLs with loose semver // versions, redirect the browser to the returned version instead of loading it // immediately - let url = ctry!(Url::parse(&format!("{}/{}/{}/{}", - base, - name, - v, - req_path.join("/"))[..])); + let url = ctry!(Url::parse( + &format!("{}/{}/{}/{}", base, name, v, req_path.join("/"))[..] + )); return Ok(super::redirect(url)); } MatchVersion::None => return Err(IronError::new(Nope::ResourceNotFound, status::NotFound)), @@ -235,11 +232,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { // expects a req_path that looks like `/rustdoc/:crate/:version[/:target]/.*` let crate_details = cexpect!(CrateDetails::new(&conn, &name, &version)); if req_path[3] == crate_details.metadata.default_target { - let path = [ - base, - req_path[1..3].join("/"), - req_path[4..].join("/") - ].join("/"); + let path = [base, req_path[1..3].join("/"), req_path[4..].join("/")].join("/"); let canonical = Url::parse(&path).expect("got an invalid URL to start"); return Ok(super::redirect(canonical)); } @@ -264,7 +257,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { Some(f) => f, None => return Err(IronError::new(Nope::ResourceNotFound, status::NotFound)), } - }, + } }; // serve file directly if it's not html @@ -334,7 +327,11 @@ fn path_for_version(req_path: &[&str], target_name: &str, conn: &Connection) -> req_path[req_path.len() - 2] } else { // this is an item - req_path.last().unwrap().split('.').nth(1) + req_path + .last() + .unwrap() + .split('.') + .nth(1) .expect("paths should be of the form ..html") }; // check if req_path[3] is the platform choice or the name of the crate @@ -353,9 +350,9 @@ fn path_for_version(req_path: &[&str], target_name: &str, conn: &Connection) -> } pub fn badge_handler(req: &mut Request) -> IronResult { + use badge::{Badge, BadgeOptions}; use iron::headers::ContentType; use params::{Params, Value}; - use badge::{Badge, BadgeOptions}; let version = { let params = ctry!(req.get_ref::()); @@ -370,11 +367,13 @@ pub fn badge_handler(req: &mut Request) -> IronResult { let options = match match_version(&conn, &name, Some(&version)) { MatchVersion::Exact((version, id)) => { - let rows = ctry!(conn.query("SELECT rustdoc_status - FROM releases - WHERE releases.id = $1", - &[&id])); - if rows.len() > 0 && rows.get(0).get(0) { + let rows = ctry!(conn.query( + "SELECT rustdoc_status + FROM releases + WHERE releases.id = $1", + &[&id] + )); + if !rows.is_empty() && rows.get(0).get(0) { BadgeOptions { subject: "docs".to_owned(), status: version, @@ -389,28 +388,33 @@ pub fn badge_handler(req: &mut Request) -> IronResult { } } MatchVersion::Semver((version, _)) => { - let url = ctry!(Url::parse(&format!("{}/{}/badge.svg?version={}", - redirect_base(req), - name, - version)[..])); + let url = ctry!(Url::parse( + &format!( + "{}/{}/badge.svg?version={}", + redirect_base(req), + name, + version + )[..] + )); return Ok(super::redirect(url)); } - MatchVersion::None => { - BadgeOptions { - subject: "docs".to_owned(), - status: "no builds".to_owned(), - color: "#e05d44".to_owned(), - } - } + MatchVersion::None => BadgeOptions { + subject: "docs".to_owned(), + status: "no builds".to_owned(), + color: "#e05d44".to_owned(), + }, }; let mut resp = Response::with((status::Ok, ctry!(Badge::new(options)).to_svg())); - resp.headers.set(ContentType("image/svg+xml".parse().unwrap())); + resp.headers + .set(ContentType("image/svg+xml".parse().unwrap())); resp.headers.set(Expires(HttpDate(time::now()))); - resp.headers.set(CacheControl(vec![CacheDirective::NoCache, - CacheDirective::NoStore, - CacheDirective::MustRevalidate])); + resp.headers.set(CacheControl(vec![ + CacheDirective::NoCache, + CacheDirective::NoStore, + CacheDirective::MustRevalidate, + ])); Ok(resp) } @@ -424,8 +428,8 @@ pub struct SharedResourceHandler; impl Handler for SharedResourceHandler { fn handle(&self, req: &mut Request) -> IronResult { let path = req.url.path(); - let filename = path.last().unwrap(); // unwrap is fine: vector is non-empty - let suffix = filename.split('.').last().unwrap(); // unwrap is fine: split always works + let filename = path.last().unwrap(); // unwrap is fine: vector is non-empty + let suffix = filename.split('.').last().unwrap(); // unwrap is fine: split always works if ["js", "css", "woff", "svg"].contains(&suffix) { let conn = extension!(req, Pool).get(); @@ -464,19 +468,21 @@ mod test { let db = env.db(); // first release works, second fails db.fake_release() - .name("buggy").version("0.1.0") - .build_result_successful(true) - .rustdoc_file("settings.html", b"some data") - .rustdoc_file("directory_1/index.html", b"some data 1") - .rustdoc_file("directory_2.html/index.html", b"some data 1") - .rustdoc_file("all.html", b"some data 2") - .rustdoc_file("directory_3/.gitignore", b"*.ext") - .rustdoc_file("directory_4/empty_file_no_ext", b"") - .create()?; + .name("buggy") + .version("0.1.0") + .build_result_successful(true) + .rustdoc_file("settings.html", b"some data") + .rustdoc_file("directory_1/index.html", b"some data 1") + .rustdoc_file("directory_2.html/index.html", b"some data 1") + .rustdoc_file("all.html", b"some data 2") + .rustdoc_file("directory_3/.gitignore", b"*.ext") + .rustdoc_file("directory_4/empty_file_no_ext", b"") + .create()?; db.fake_release() - .name("buggy").version("0.2.0") - .build_result_successful(false) - .create()?; + .name("buggy") + .version("0.2.0") + .build_result_successful(false) + .create()?; let web = env.frontend(); assert_success("/", web)?; assert_success("/crate/buggy/0.1.0/", web)?; @@ -494,9 +500,10 @@ mod test { wrapper(|env| { let db = env.db(); db.fake_release() - .name("dummy").version("0.1.0") - .rustdoc_file("dummy/index.html", b"some content") - .create()?; + .name("dummy") + .version("0.1.0") + .rustdoc_file("dummy/index.html", b"some content") + .create()?; let web = env.frontend(); // no explicit default-target @@ -506,9 +513,12 @@ mod test { // set an explicit target that requires cross-compile let target = "x86_64-pc-windows-msvc"; - db.fake_release().name("dummy").version("0.2.0") - .rustdoc_file("dummy/index.html", b"some content") - .default_target(target).create()?; + db.fake_release() + .name("dummy") + .version("0.2.0") + .rustdoc_file("dummy/index.html", b"some content") + .default_target(target) + .create()?; let base = "/dummy/0.2.0/dummy/"; assert_success(base, web)?; assert_redirect("/dummy/0.2.0/x86_64-pc-windows-msvc/dummy/", base, web)?; @@ -516,16 +526,23 @@ mod test { // set an explicit target without cross-compile // also check that /:crate/:version/:platform/all.html doesn't panic let target = "x86_64-unknown-linux-gnu"; - db.fake_release().name("dummy").version("0.3.0") - .rustdoc_file("dummy/index.html", b"some content") - .rustdoc_file("all.html", b"html") - .default_target(target).create()?; + db.fake_release() + .name("dummy") + .version("0.3.0") + .rustdoc_file("dummy/index.html", b"some content") + .rustdoc_file("all.html", b"html") + .default_target(target) + .create()?; let base = "/dummy/0.3.0/dummy/"; assert_success(base, web)?; assert_redirect("/dummy/0.3.0/x86_64-unknown-linux-gnu/dummy/", base, web)?; - assert_redirect("/dummy/0.3.0/x86_64-unknown-linux-gnu/all.html", "/dummy/0.3.0/all.html", web)?; + assert_redirect( + "/dummy/0.3.0/x86_64-unknown-linux-gnu/all.html", + "/dummy/0.3.0/all.html", + web, + )?; assert_redirect("/dummy/0.3.0/", base, web)?; - assert_redirect("/dummy/0.3.0/index.html",base, web)?; + assert_redirect("/dummy/0.3.0/index.html", base, web)?; Ok(()) }); } @@ -533,15 +550,19 @@ mod test { fn go_to_latest_version() { wrapper(|env| { let db = env.db(); - db.fake_release().name("dummy").version("0.1.0") - .rustdoc_file("dummy/blah/index.html", b"lah") - .rustdoc_file("dummy/blah/blah.html", b"lah") - .rustdoc_file("dummy/struct.will-be-deleted.html", b"lah") - .create()?; - db.fake_release().name("dummy").version("0.2.0") - .rustdoc_file("dummy/blah/index.html", b"lah") - .rustdoc_file("dummy/blah/blah.html", b"lah") - .create()?; + db.fake_release() + .name("dummy") + .version("0.1.0") + .rustdoc_file("dummy/blah/index.html", b"lah") + .rustdoc_file("dummy/blah/blah.html", b"lah") + .rustdoc_file("dummy/struct.will-be-deleted.html", b"lah") + .create()?; + db.fake_release() + .name("dummy") + .version("0.2.0") + .rustdoc_file("dummy/blah/index.html", b"lah") + .rustdoc_file("dummy/blah/blah.html", b"lah") + .create()?; let web = env.frontend(); @@ -556,9 +577,15 @@ mod test { assert_eq!(redirect, "/dummy/0.2.0/dummy/blah/blah.html"); // check it searches for removed pages - let redirect = latest_version_redirect("/dummy/0.1.0/dummy/struct.will-be-deleted.html", &web)?; + let redirect = + latest_version_redirect("/dummy/0.1.0/dummy/struct.will-be-deleted.html", &web)?; assert_eq!(redirect, "/dummy/0.2.0/dummy?search=will-be-deleted"); - assert_redirect("/dummy/0.2.0/dummy?search=will-be-deleted", "/dummy/0.2.0/dummy/?search=will-be-deleted", &web).unwrap(); + assert_redirect( + "/dummy/0.2.0/dummy?search=will-be-deleted", + "/dummy/0.2.0/dummy/?search=will-be-deleted", + &web, + ) + .unwrap(); Ok(()) }) @@ -568,20 +595,34 @@ mod test { fn go_to_latest_version_keeps_platform() { wrapper(|env| { let db = env.db(); - db.fake_release().name("dummy").version("0.1.0") - .add_platform("x86_64-pc-windows-msvc") - .create().unwrap(); - db.fake_release().name("dummy").version("0.2.0") - .add_platform("x86_64-pc-windows-msvc") - .create().unwrap(); + db.fake_release() + .name("dummy") + .version("0.1.0") + .add_platform("x86_64-pc-windows-msvc") + .create() + .unwrap(); + db.fake_release() + .name("dummy") + .version("0.2.0") + .add_platform("x86_64-pc-windows-msvc") + .create() + .unwrap(); let web = env.frontend(); - let redirect = latest_version_redirect("/dummy/0.1.0/x86_64-pc-windows-msvc/dummy", web)?; - assert_eq!(redirect, "/dummy/0.2.0/x86_64-pc-windows-msvc/dummy/index.html"); + let redirect = + latest_version_redirect("/dummy/0.1.0/x86_64-pc-windows-msvc/dummy", web)?; + assert_eq!( + redirect, + "/dummy/0.2.0/x86_64-pc-windows-msvc/dummy/index.html" + ); - let redirect = latest_version_redirect("/dummy/0.1.0/x86_64-pc-windows-msvc/dummy/", web)?; - assert_eq!(redirect, "/dummy/0.2.0/x86_64-pc-windows-msvc/dummy/index.html"); + let redirect = + latest_version_redirect("/dummy/0.1.0/x86_64-pc-windows-msvc/dummy/", web)?; + assert_eq!( + redirect, + "/dummy/0.2.0/x86_64-pc-windows-msvc/dummy/index.html" + ); Ok(()) }) @@ -590,11 +631,18 @@ mod test { fn redirect_latest_goes_to_crate_if_build_failed() { wrapper(|env| { let db = env.db(); - db.fake_release().name("dummy").version("0.1.0") - .rustdoc_file("dummy/index.html", b"lah") - .create().unwrap(); - db.fake_release().name("dummy").version("0.2.0") - .build_result_successful(false).create().unwrap(); + db.fake_release() + .name("dummy") + .version("0.1.0") + .rustdoc_file("dummy/index.html", b"lah") + .create() + .unwrap(); + db.fake_release() + .name("dummy") + .version("0.2.0") + .build_result_successful(false) + .create() + .unwrap(); let web = env.frontend(); let redirect = latest_version_redirect("/dummy/0.1.0/dummy/", web)?; From 4b094874dd37a046cc6468af09d25763de79781f Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 12:06:33 -0500 Subject: [PATCH 02/14] Even more refactors --- build.rs | 2 +- src/bin/cratesfyi.rs | 353 ++++++++++++++------------ src/db/add_package.rs | 186 ++++++++------ src/db/file.rs | 4 +- src/docbuilder/crates.rs | 22 +- src/docbuilder/limits.rs | 2 +- src/docbuilder/metadata.rs | 214 +++++++++++----- src/docbuilder/mod.rs | 56 ++-- src/docbuilder/options.rs | 45 ++-- src/docbuilder/rustwide_builder.rs | 8 +- src/lib.rs | 21 +- src/test/fakes.rs | 2 +- src/utils/daemon.rs | 156 ++---------- src/utils/github_updater.rs | 22 +- src/utils/html.rs | 28 +- src/utils/release_activity_updater.rs | 78 +++--- src/utils/rustc_version.rs | 32 ++- src/web/builds.rs | 4 +- src/web/crate_details.rs | 14 +- src/web/mod.rs | 191 ++++++++------ src/web/page.rs | 21 +- src/web/releases.rs | 4 +- src/web/routes.rs | 7 +- src/web/source.rs | 55 ++-- 24 files changed, 822 insertions(+), 705 deletions(-) diff --git a/build.rs b/build.rs index 1b07f5913..abe993788 100644 --- a/build.rs +++ b/build.rs @@ -13,7 +13,7 @@ fn main() { fn write_git_version() { - let git_hash = get_git_hash().unwrap_or("???????".to_owned()); + let git_hash = get_git_hash().unwrap_or_else(|| "???????".to_owned()); let build_date = time::strftime("%Y-%m-%d", &time::now_utc()).unwrap(); let dest_path = Path::new(&env::var("OUT_DIR").unwrap()).join("git_version"); let mut file = File::create(&dest_path).unwrap(); diff --git a/src/bin/cratesfyi.rs b/src/bin/cratesfyi.rs index df186b179..c26498840 100644 --- a/src/bin/cratesfyi.rs +++ b/src/bin/cratesfyi.rs @@ -1,17 +1,204 @@ use std::env; use std::path::{Path, PathBuf}; -use clap::{Arg, App, AppSettings, SubCommand}; -use cratesfyi::{DocBuilder, RustwideBuilder, DocBuilderOptions, db}; +use clap::{App, AppSettings, Arg, SubCommand}; +use cratesfyi::db::{add_path_into_database, connect_db}; use cratesfyi::utils::add_crate_to_queue; use cratesfyi::Server; -use cratesfyi::db::{add_path_into_database, connect_db}; - +use cratesfyi::{db, DocBuilder, DocBuilderOptions, RustwideBuilder}; pub fn main() { logger_init(); - let matches = App::new("cratesfyi") + let matches = build_cli().get_matches(); + + // REFACTOR: Break this monster up unto smaller functions, and maybe document it a bit + if let Some(matches) = matches.subcommand_matches("build") { + let docbuilder_opts = { + let mut docbuilder_opts = if let Some(prefix) = matches.value_of("PREFIX") { + DocBuilderOptions::from_prefix(PathBuf::from(prefix)) + } else if let Ok(prefix) = env::var("CRATESFYI_PREFIX") { + DocBuilderOptions::from_prefix(PathBuf::from(prefix)) + } else { + DocBuilderOptions::default() + }; + + if let Some(crates_io_index_path) = matches.value_of("CRATES_IO_INDEX_PATH") { + docbuilder_opts.crates_io_index_path = PathBuf::from(crates_io_index_path); + } + + docbuilder_opts.skip_if_exists = matches.is_present("SKIP_IF_EXISTS"); + docbuilder_opts.skip_if_log_exists = matches.is_present("SKIP_IF_LOG_EXISTS"); + docbuilder_opts.keep_build_directory = matches.is_present("KEEP_BUILD_DIRECTORY"); + + docbuilder_opts.check_paths().unwrap(); + + docbuilder_opts + }; + + let mut docbuilder = DocBuilder::new(docbuilder_opts); + + if matches.subcommand_matches("world").is_some() { + docbuilder.load_cache().expect("Failed to load cache"); + let mut builder = RustwideBuilder::init().unwrap(); + + builder + .build_world(&mut docbuilder) + .expect("Failed to build world"); + docbuilder.save_cache().expect("Failed to save cache"); + } else if let Some(matches) = matches.subcommand_matches("crate") { + docbuilder.load_cache().expect("Failed to load cache"); + let mut builder = RustwideBuilder::init().expect("failed to initialize rustwide"); + + match matches.value_of("LOCAL") { + Some(path) => builder.build_local_package(&mut docbuilder, Path::new(path)), + None => builder.build_package( + &mut docbuilder, + matches.value_of("CRATE_NAME").unwrap(), + matches.value_of("CRATE_VERSION").unwrap(), + None, + ), + } + .expect("Building documentation failed"); + + docbuilder.save_cache().expect("Failed to save cache"); + } else if let Some(m) = matches.subcommand_matches("update-toolchain") { + if m.is_present("ONLY_FIRST_TIME") { + let conn = db::connect_db().unwrap(); + let res = conn + .query("SELECT * FROM config WHERE name = 'rustc_version';", &[]) + .unwrap(); + if !res.is_empty() { + println!("update-toolchain was already called in the past, exiting"); + return; + } + } + + let mut builder = RustwideBuilder::init().unwrap(); + builder + .update_toolchain() + .expect("failed to update toolchain"); + } else if matches.subcommand_matches("add-essential-files").is_some() { + let mut builder = RustwideBuilder::init().unwrap(); + builder + .add_essential_files() + .expect("failed to add essential files"); + } else if matches.subcommand_matches("lock").is_some() { + docbuilder.lock().expect("Failed to lock"); + } else if matches.subcommand_matches("unlock").is_some() { + docbuilder.unlock().expect("Failed to unlock"); + } else if matches.subcommand_matches("print-options").is_some() { + println!("{:?}", docbuilder.options()); + } + } else if let Some(matches) = matches.subcommand_matches("database") { + if let Some(matches) = matches.subcommand_matches("migrate") { + let version = matches + .value_of("VERSION") + .map(|v| v.parse::().expect("Version should be an integer")); + db::migrate( + version, + &connect_db().expect("failed to connect to the database"), + ) + .expect("Failed to run database migrations"); + } else if matches.subcommand_matches("update-github-fields").is_some() { + cratesfyi::utils::github_updater().expect("Failed to update github fields"); + } else if let Some(matches) = matches.subcommand_matches("add-directory") { + add_path_into_database( + &db::connect_db().unwrap(), + matches.value_of("PREFIX").unwrap_or(""), + matches.value_of("DIRECTORY").unwrap(), + ) + .expect("Failed to add directory into database"); + } else if matches + .subcommand_matches("update-release-activity") + .is_some() + { + // FIXME: This is actually util command not database + cratesfyi::utils::update_release_activity().expect("Failed to update release activity"); + } else if matches.subcommand_matches("update-search-index").is_some() { + let conn = db::connect_db().unwrap(); + db::update_search_index(&conn).expect("Failed to update search index"); + } else if matches.subcommand_matches("move-to-s3").is_some() { + let conn = db::connect_db().unwrap(); + let mut count = 1; + let mut total = 0; + while count != 0 { + count = db::move_to_s3(&conn, 5_000).expect("Failed to upload batch to S3"); + total += count; + eprintln!( + "moved {} rows to s3 in this batch, total moved so far: {}", + count, total + ); + } + } else if let Some(matches) = matches.subcommand_matches("delete-crate") { + let name = matches.value_of("CRATE_NAME").expect("missing crate name"); + let conn = db::connect_db().expect("failed to connect to the database"); + db::delete_crate(&conn, &name).expect("failed to delete the crate"); + } else if let Some(matches) = matches.subcommand_matches("blacklist") { + let conn = db::connect_db().expect("failed to connect to the database"); + + if matches.subcommand_matches("list").is_some() { + let crates = + db::blacklist::list_crates(&conn).expect("failed to list crates on blacklist"); + println!("{}", crates.join("\n")); + } else if let Some(matches) = matches.subcommand_matches("add") { + let crate_name = matches.value_of("CRATE_NAME").expect("verified by clap"); + db::blacklist::add_crate(&conn, crate_name) + .expect("failed to add crate to blacklist"); + } else if let Some(matches) = matches.subcommand_matches("remove") { + let crate_name = matches.value_of("CRATE_NAME").expect("verified by clap"); + db::blacklist::remove_crate(&conn, crate_name) + .expect("failed to remove crate from blacklist"); + } + } + } else if let Some(matches) = matches.subcommand_matches("start-web-server") { + Server::start(Some( + matches.value_of("SOCKET_ADDR").unwrap_or("0.0.0.0:3000"), + )); + } else if matches.subcommand_matches("daemon").is_some() { + let foreground = matches + .subcommand_matches("daemon") + .map_or(false, |opts| opts.is_present("FOREGROUND")); + cratesfyi::utils::start_daemon(!foreground); + } else if let Some(matches) = matches.subcommand_matches("queue") { + if let Some(matches) = matches.subcommand_matches("add") { + let priority = matches.value_of("BUILD_PRIORITY").unwrap_or("5"); + let priority: i32 = priority.parse().expect("--priority was not a number"); + let conn = connect_db().expect("Could not connect to database"); + + add_crate_to_queue( + &conn, + matches.value_of("CRATE_NAME").unwrap(), + matches.value_of("CRATE_VERSION").unwrap(), + priority, + ) + .expect("Could not add crate to queue"); + } + } +} + +fn logger_init() { + use std::io::Write; + + let mut builder = env_logger::Builder::new(); + builder.format(|buf, record| { + writeln!( + buf, + "{} [{}] {}: {}", + time::now().strftime("%Y/%m/%d %H:%M:%S").unwrap(), + record.level(), + record.target(), + record.args() + ) + }); + builder.parse_filters(&env::var("RUST_LOG").unwrap_or_else(|_| "cratesfyi=info".to_owned())); + + rustwide::logging::init_with(builder.build()); +} + +/// Creates the command line interface to cratesfyi +fn build_cli<'a, 'b>() -> App<'a, 'b> { + App::new("cratesfyi") .version(cratesfyi::BUILD_VERSION) .about(env!("CARGO_PKG_DESCRIPTION")) .setting(AppSettings::ArgRequiredElseHelp) @@ -140,160 +327,4 @@ pub fn main() { .long("priority") .help("Priority of build (default: 5) (new crate builds get priority 0)") .takes_value(true)))) - .get_matches(); - - - - if let Some(matches) = matches.subcommand_matches("build") { - let docbuilder_opts = { - let mut docbuilder_opts = if let Some(prefix) = matches.value_of("PREFIX") { - DocBuilderOptions::from_prefix(PathBuf::from(prefix)) - } else if let Ok(prefix) = env::var("CRATESFYI_PREFIX") { - DocBuilderOptions::from_prefix(PathBuf::from(prefix)) - } else { - DocBuilderOptions::default() - }; - - if let Some(crates_io_index_path) = matches.value_of("CRATES_IO_INDEX_PATH") { - docbuilder_opts.crates_io_index_path = PathBuf::from(crates_io_index_path); - } - - docbuilder_opts.skip_if_exists = matches.is_present("SKIP_IF_EXISTS"); - docbuilder_opts.skip_if_log_exists = matches.is_present("SKIP_IF_LOG_EXISTS"); - docbuilder_opts.keep_build_directory = matches.is_present("KEEP_BUILD_DIRECTORY"); - - docbuilder_opts.check_paths().unwrap(); - - docbuilder_opts - }; - - let mut docbuilder = DocBuilder::new(docbuilder_opts); - - if let Some(_) = matches.subcommand_matches("world") { - docbuilder.load_cache().expect("Failed to load cache"); - let mut builder = RustwideBuilder::init().unwrap(); - builder.build_world(&mut docbuilder).expect("Failed to build world"); - docbuilder.save_cache().expect("Failed to save cache"); - } else if let Some(matches) = matches.subcommand_matches("crate") { - docbuilder.load_cache().expect("Failed to load cache"); - let mut builder = RustwideBuilder::init().expect("failed to initialize rustwide"); - match matches.value_of("LOCAL") { - Some(path) => builder.build_local_package(&mut docbuilder, Path::new(path)), - None => builder.build_package(&mut docbuilder, - matches.value_of("CRATE_NAME").unwrap(), matches.value_of("CRATE_VERSION").unwrap(), None), - }.expect("Building documentation failed"); - docbuilder.save_cache().expect("Failed to save cache"); - } else if let Some(m) = matches.subcommand_matches("update-toolchain") { - if m.is_present("ONLY_FIRST_TIME") { - let conn = db::connect_db().unwrap(); - let res = conn.query("SELECT * FROM config WHERE name = 'rustc_version';", &[]).unwrap(); - if !res.is_empty() { - println!("update-toolchain was already called in the past, exiting"); - return; - } - } - let mut builder = RustwideBuilder::init().unwrap(); - builder.update_toolchain().expect("failed to update toolchain"); - } else if matches.subcommand_matches("add-essential-files").is_some() { - let mut builder = RustwideBuilder::init().unwrap(); - builder.add_essential_files().expect("failed to add essential files"); - } else if let Some(_) = matches.subcommand_matches("lock") { - docbuilder.lock().expect("Failed to lock"); - } else if let Some(_) = matches.subcommand_matches("unlock") { - docbuilder.unlock().expect("Failed to unlock"); - } else if let Some(_) = matches.subcommand_matches("print-options") { - println!("{:?}", docbuilder.options()); - } - - } else if let Some(matches) = matches.subcommand_matches("database") { - if let Some(matches) = matches.subcommand_matches("migrate") { - let version = matches.value_of("VERSION").map(|v| v.parse::() - .expect("Version should be an integer")); - db::migrate(version, &connect_db().expect("failed to connect to the database")) - .expect("Failed to run database migrations"); - } else if let Some(_) = matches.subcommand_matches("update-github-fields") { - cratesfyi::utils::github_updater().expect("Failed to update github fields"); - } else if let Some(matches) = matches.subcommand_matches("add-directory") { - add_path_into_database(&db::connect_db().unwrap(), - matches.value_of("PREFIX").unwrap_or(""), - matches.value_of("DIRECTORY").unwrap()) - .expect("Failed to add directory into database"); - } else if let Some(_) = matches.subcommand_matches("update-release-activity") { - // FIXME: This is actually util command not database - cratesfyi::utils::update_release_activity().expect("Failed to update release activity"); - } else if let Some(_) = matches.subcommand_matches("update-search-index") { - let conn = db::connect_db().unwrap(); - db::update_search_index(&conn).expect("Failed to update search index"); - } else if let Some(_) = matches.subcommand_matches("move-to-s3") { - let conn = db::connect_db().unwrap(); - let mut count = 1; - let mut total = 0; - while count != 0 { - count = db::move_to_s3(&conn, 5_000).expect("Failed to upload batch to S3"); - total += count; - eprintln!( - "moved {} rows to s3 in this batch, total moved so far: {}", - count, total - ); - } - } else if let Some(matches) = matches.subcommand_matches("delete-crate") { - let name = matches.value_of("CRATE_NAME").expect("missing crate name"); - let conn = db::connect_db().expect("failed to connect to the database"); - db::delete_crate(&conn, &name).expect("failed to delete the crate"); - } else if let Some(matches) = matches.subcommand_matches("blacklist") { - let conn = db::connect_db().expect("failed to connect to the database"); - - if let Some(_) = matches.subcommand_matches("list") { - let crates = db::blacklist::list_crates(&conn).expect("failed to list crates on blacklist"); - println!("{}", crates.join("\n")); - - } else if let Some(matches) = matches.subcommand_matches("add") { - let crate_name = matches.value_of("CRATE_NAME").expect("verified by clap"); - db::blacklist::add_crate(&conn, crate_name).expect("failed to add crate to blacklist"); - - } else if let Some(matches) = matches.subcommand_matches("remove") { - let crate_name = matches.value_of("CRATE_NAME").expect("verified by clap"); - db::blacklist::remove_crate(&conn, crate_name).expect("failed to remove crate from blacklist"); - } - } - - } else if let Some(matches) = matches.subcommand_matches("start-web-server") { - Server::start(Some(matches.value_of("SOCKET_ADDR").unwrap_or("0.0.0.0:3000"))); - - } else if let Some(_) = matches.subcommand_matches("daemon") { - let foreground = matches.subcommand_matches("daemon") - .map_or(false, |opts| opts.is_present("FOREGROUND")); - cratesfyi::utils::start_daemon(!foreground); - - } else if let Some(matches) = matches.subcommand_matches("queue") { - if let Some(matches) = matches.subcommand_matches("add") { - let priority = matches.value_of("BUILD_PRIORITY").unwrap_or("5"); - let priority: i32 = priority.parse().expect("--priority was not a number"); - let conn = connect_db().expect("Could not connect to database"); - - add_crate_to_queue(&conn, - matches.value_of("CRATE_NAME").unwrap(), - matches.value_of("CRATE_VERSION").unwrap(), - priority).expect("Could not add crate to queue"); - } - } -} - - - -fn logger_init() { - use std::io::Write; - - let mut builder = env_logger::Builder::new(); - builder.format(|buf, record| { - writeln!(buf, "{} [{}] {}: {}", - time::now().strftime("%Y/%m/%d %H:%M:%S").unwrap(), - record.level(), - record.target(), - record.args()) - }); - builder.parse_filters(&env::var("RUST_LOG") - .unwrap_or("cratesfyi=info".to_owned())); - - rustwide::logging::init_with(builder.build()); } diff --git a/src/db/add_package.rs b/src/db/add_package.rs index 0914f150d..7e26e2815 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -25,17 +25,19 @@ use failure::err_msg; /// /// NOTE: `source_files` refers to the files originally in the crate, /// not the files generated by rustdoc. -pub(crate) fn add_package_into_database(conn: &Connection, - metadata_pkg: &MetadataPackage, - source_dir: &Path, - res: &BuildResult, - default_target: &str, - source_files: Option, - doc_targets: Vec, - cratesio_data: &CratesIoData, - has_docs: bool, - has_examples: bool) - -> Result { +// REFACTOR: This should be multiple smaller functions +pub(crate) fn add_package_into_database( + conn: &Connection, + metadata_pkg: &MetadataPackage, + source_dir: &Path, + res: &BuildResult, + default_target: &str, + source_files: Option, + doc_targets: Vec, + cratesio_data: &CratesIoData, + has_docs: bool, + has_examples: bool +) -> Result { debug!("Adding package into database"); let crate_id = initialize_package_in_database(&conn, metadata_pkg)?; let dependencies = convert_dependencies(metadata_pkg); @@ -45,9 +47,10 @@ pub(crate) fn add_package_into_database(conn: &Connection, let release_id: i32 = { let rows = conn.query("SELECT id FROM releases WHERE crate_id = $1 AND version = $2", - &[&crate_id, &format!("{}", metadata_pkg.version)])?; + &[&crate_id, &metadata_pkg.version])?; - if rows.len() == 0 { + if rows.is_empty() { + // REFACTOR: These are absurdly large let rows = conn.query("INSERT INTO releases ( crate_id, version, release_time, dependencies, target_name, yanked, build_status, @@ -91,6 +94,7 @@ pub(crate) fn add_package_into_database(conn: &Connection, rows.get(0).get(0) } else { + // REFACTOR: These are absurdly large conn.query("UPDATE releases SET release_time = $3, dependencies = $4, @@ -117,7 +121,7 @@ pub(crate) fn add_package_into_database(conn: &Connection, default_target = $25 WHERE crate_id = $1 AND version = $2", &[&crate_id, - &format!("{}", metadata_pkg.version), + &metadata_pkg.version, &cratesio_data.release_time, &dependencies.to_json(), &metadata_pkg.package_name(), @@ -145,33 +149,40 @@ pub(crate) fn add_package_into_database(conn: &Connection, } }; - - add_keywords_into_database(&conn, &metadata_pkg, &release_id)?; - add_authors_into_database(&conn, &metadata_pkg, &release_id)?; - add_owners_into_database(&conn, &cratesio_data.owners, &crate_id)?; - + add_keywords_into_database(&conn, &metadata_pkg, release_id)?; + add_authors_into_database(&conn, &metadata_pkg, release_id)?; + add_owners_into_database(&conn, &cratesio_data.owners, crate_id)?; // Update versions { let metadata_version = Version::parse(&metadata_pkg.version)?; - let mut versions: Json = conn.query("SELECT versions FROM crates WHERE id = $1", - &[&crate_id])? - .get(0) - .get(0); + let mut versions: Json = conn.query( + "SELECT versions FROM crates WHERE id = $1", + &[&crate_id] + )? + .get(0) + .get(0); + if let Some(versions_array) = versions.as_array_mut() { let mut found = false; + for version in versions_array.clone() { let version = Version::parse(version.as_string().unwrap())?; + if version == metadata_version { found = true; } } + if !found { - versions_array.push(format!("{}", &metadata_pkg.version).to_json()); + versions_array.push(metadata_pkg.version.to_json()); } } - let _ = conn.query("UPDATE crates SET versions = $1 WHERE id = $2", - &[&versions, &crate_id]); + + let _ = conn.query( + "UPDATE crates SET versions = $1 WHERE id = $2", + &[&versions, &crate_id] + ); } Ok(release_id) @@ -179,37 +190,45 @@ pub(crate) fn add_package_into_database(conn: &Connection, /// Adds a build into database -pub(crate) fn add_build_into_database(conn: &Connection, - release_id: &i32, - res: &BuildResult) - -> Result { +pub(crate) fn add_build_into_database( + conn: &Connection, + release_id: i32, + res: &BuildResult +) -> Result { debug!("Adding build into database"); - let rows = conn.query("INSERT INTO builds (rid, rustc_version, - cratesfyi_version, - build_status, output) - VALUES ($1, $2, $3, $4, $5) - RETURNING id", - &[release_id, - &res.rustc_version, - &res.docsrs_version, - &res.successful, - &res.build_log])?; + + let rows = conn.query( + "INSERT INTO builds (rid, rustc_version, + cratesfyi_version, + build_status, output) + VALUES ($1, $2, $3, $4, $5) + RETURNING id", + &[ + &release_id, + &res.rustc_version, + &res.docsrs_version, + &res.successful, + &res.build_log, + ], + )?; + Ok(rows.get(0).get(0)) } - fn initialize_package_in_database(conn: &Connection, pkg: &MetadataPackage) -> Result { let mut rows = conn.query("SELECT id FROM crates WHERE name = $1", &[&pkg.name])?; + // insert crate into database if it is not exists - if rows.len() == 0 { - rows = conn.query("INSERT INTO crates (name) VALUES ($1) RETURNING id", - &[&pkg.name])?; + if rows.is_empty() { + rows = conn.query( + "INSERT INTO crates (name) VALUES ($1) RETURNING id", + &[&pkg.name] + )?; } + Ok(rows.get(0).get(0)) } - - /// 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(); @@ -219,19 +238,19 @@ fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)> let kind = dependency.kind.clone().unwrap_or_else(|| "normal".into()); dependencies.push((name, version, kind.to_string())); } + dependencies } - /// Reads readme if there is any read defined in Cargo.toml of a Package fn get_readme(pkg: &MetadataPackage, source_dir: &Path) -> Result> { - let readme_path = source_dir.join(pkg.readme.clone().unwrap_or("README.md".to_owned())); + let readme_path = source_dir.join(pkg.readme.clone().unwrap_or_else(|| "README.md".to_owned())); if !readme_path.exists() { return Ok(None); } - let mut reader = fs::File::open(readme_path).map(|f| BufReader::new(f))?; + let mut reader = fs::File::open(readme_path).map(BufReader::new)?; let mut readme = String::new(); reader.read_to_string(&mut readme)?; @@ -262,17 +281,20 @@ fn get_rustdoc(pkg: &MetadataPackage, source_dir: &Path) -> Result Result> { - let reader = fs::File::open(file_path).map(|f| BufReader::new(f))?; + let reader = fs::File::open(file_path).map(BufReader::new)?; let mut rustdoc = String::new(); for line in reader.lines() { let line = line?; + if line.starts_with("//!") { // some lines may or may not have a space between the `//!` and the start of the text let line = line.trim_start_matches("//!").trim_start(); + if !line.is_empty() { rustdoc.push_str(line); } + rustdoc.push('\n'); } } @@ -286,7 +308,6 @@ fn read_rust_doc(file_path: &Path) -> Result> { } } - pub(crate) struct CratesIoData { pub(crate) release_time: Timespec, pub(crate) yanked: bool, @@ -308,21 +329,25 @@ impl CratesIoData { } } - /// Get release_time, yanked and downloads from crates.io fn get_release_time_yanked_downloads( pkg: &MetadataPackage, ) -> Result<(time::Timespec, bool, i32)> { let url = format!("https://crates.io/api/v1/crates/{}/versions", pkg.name); + // FIXME: There is probably better way to do this // and so many unwraps... let client = Client::new(); - let mut res = client.get(&url[..]) + + let mut res = client + .get(&url[..]) .header(ACCEPT, "application/json") .send()?; + let mut body = String::new(); res.read_to_string(&mut body).unwrap(); let json = Json::from_str(&body[..]).unwrap(); + let versions = json.as_object() .and_then(|o| o.get("versions")) .and_then(|v| v.as_array()) @@ -331,7 +356,9 @@ fn get_release_time_yanked_downloads( let (mut release_time, mut yanked, mut downloads) = (None, None, None); for version in versions { - let version = version.as_object().ok_or_else(|| err_msg("Not a JSON object"))?; + let version = version.as_object() + .ok_or_else(|| err_msg("Not a JSON object"))?; + let version_num = version.get("num") .and_then(|v| v.as_string()) .ok_or_else(|| err_msg("Not a JSON object"))?; @@ -340,6 +367,7 @@ fn get_release_time_yanked_downloads( let release_time_raw = version.get("created_at") .and_then(|c| c.as_string()) .ok_or_else(|| err_msg("Not a JSON object"))?; + release_time = Some(time::strptime(release_time_raw, "%Y-%m-%dT%H:%M:%S") .unwrap() .to_timespec()); @@ -365,20 +393,24 @@ fn get_release_time_yanked_downloads( /// Adds keywords into database -fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: &i32) -> Result<()> { +fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: i32) -> Result<()> { for keyword in &pkg.keywords { let slug = slugify(&keyword); let keyword_id: i32 = { let rows = conn.query("SELECT id FROM keywords WHERE slug = $1", &[&slug])?; - if rows.len() > 0 { + + if !rows.is_empty() { rows.get(0).get(0) } else { - conn.query("INSERT INTO keywords (name, slug) VALUES ($1, $2) RETURNING id", - &[&keyword, &slug])? - .get(0) - .get(0) + conn.query( + "INSERT INTO keywords (name, slug) VALUES ($1, $2) RETURNING id", + &[&keyword, &slug] + )? + .get(0) + .get(0) } }; + // add releationship let _ = conn.query("INSERT INTO keyword_rels (rid, kid) VALUES ($1, $2)", &[&release_id, &keyword_id]); } @@ -389,18 +421,25 @@ fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_ /// Adds authors into database -fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: &i32) -> Result<()> { - +fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: i32) -> Result<()> { let author_capture_re = Regex::new("^([^><]+)<*(.*?)>*$").unwrap(); for author in &pkg.authors { if let Some(author_captures) = author_capture_re.captures(&author[..]) { - let author = author_captures.get(1).map(|m| m.as_str()).unwrap_or("").trim(); - let email = author_captures.get(2).map(|m| m.as_str()).unwrap_or("").trim(); + let author = author_captures + .get(1) + .map(|m| m.as_str()) + .unwrap_or("") + .trim(); + let email = author_captures + .get(2) + .map(|m| m.as_str()) + .unwrap_or("") + .trim(); let slug = slugify(&author); let author_id: i32 = { let rows = conn.query("SELECT id FROM authors WHERE slug = $1", &[&slug])?; - if rows.len() > 0 { + if rows.is_empty() { rows.get(0).get(0) } else { conn.query("INSERT INTO authors (name, email, slug) VALUES ($1, $2, $3) @@ -412,15 +451,16 @@ fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_i }; // add relationship - let _ = conn.query("INSERT INTO author_rels (rid, aid) VALUES ($1, $2)", - &[release_id, &author_id]); + let _ = conn.query( + "INSERT INTO author_rels (rid, aid) VALUES ($1, $2)", + &[&release_id, &author_id] + ); } } Ok(()) } - pub(crate) struct CrateOwner { pub(crate) avatar: String, pub(crate) email: String, @@ -445,21 +485,25 @@ fn get_owners(pkg: &MetadataPackage) -> Result> { let mut result = Vec::new(); if let Some(owners) = json.as_object() .and_then(|j| j.get("users")) - .and_then(|j| j.as_array()) { + .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_string()) .unwrap_or(""); + let email = owner.as_object() .and_then(|o| o.get("email")) .and_then(|o| o.as_string()) .unwrap_or(""); + let login = owner.as_object() .and_then(|o| o.get("login")) .and_then(|o| o.as_string()) .unwrap_or(""); + let name = owner.as_object() .and_then(|o| o.get("name")) .and_then(|o| o.as_string()) @@ -482,11 +526,11 @@ fn get_owners(pkg: &MetadataPackage) -> Result> { } /// Adds owners into database -fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id: &i32) -> Result<()> { +fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id: i32) -> Result<()> { for owner in owners { let owner_id: i32 = { let rows = conn.query("SELECT id FROM owners WHERE login = $1", &[&owner.login])?; - if rows.len() > 0 { + if rows.is_empty() { rows.get(0).get(0) } else { conn.query("INSERT INTO owners (login, avatar, name, email) diff --git a/src/db/file.rs b/src/db/file.rs index 4052fc023..ed1df727a 100644 --- a/src/db/file.rs +++ b/src/db/file.rs @@ -103,7 +103,7 @@ pub fn get_path(conn: &Connection, path: &str) -> Option { FROM files WHERE path = $1", &[&path]).unwrap(); - if rows.len() == 0 { + if rows.is_empty() { None } else { let row = rows.get(0); @@ -124,6 +124,7 @@ pub(super) fn s3_client() -> Option { if std::env::var_os("AWS_ACCESS_KEY_ID").is_none() && std::env::var_os("FORCE_S3").is_none() { return None; } + let creds = match DefaultCredentialsProvider::new() { Ok(creds) => creds, Err(err) => { @@ -131,6 +132,7 @@ pub(super) fn s3_client() -> Option { return None; } }; + Some(S3Client::new_with( rusoto_core::request::HttpClient::new().unwrap(), creds, diff --git a/src/docbuilder/crates.rs b/src/docbuilder/crates.rs index 1f67839f9..ffcf51bbe 100644 --- a/src/docbuilder/crates.rs +++ b/src/docbuilder/crates.rs @@ -8,10 +8,9 @@ use crate::error::Result; use failure::err_msg; fn crates_from_file(path: &PathBuf, func: &mut F) -> Result<()> - where F: FnMut(&str, &str) -> () + where F: FnMut(&str, &str) -> (), { - - let reader = fs::File::open(path).map(|f| BufReader::new(f))?; + let reader = fs::File::open(path).map(BufReader::new)?; let mut name = String::new(); let mut versions = Vec::new(); @@ -19,13 +18,16 @@ fn crates_from_file(path: &PathBuf, func: &mut F) -> Result<()> for line in reader.lines() { // some crates have invalid UTF-8 (nanny-sys-0.0.7) // skip them - let line = match line { - Ok(l) => l, - Err(_) => continue, + let line = if let Ok(line) = line { + line + } else { + continue; }; - let data = match Json::from_str(line.trim()) { - Ok(d) => d, - Err(_) => continue, + + let data = if let Ok(data) = Json::from_str(line.trim()) { + data + } else { + continue; }; let obj = data.as_object().ok_or_else(|| err_msg("Not a JSON object"))?; @@ -43,7 +45,7 @@ fn crates_from_file(path: &PathBuf, func: &mut F) -> Result<()> name.clear(); name.push_str(crate_name); - versions.push(format!("{}", vers)); + versions.push(vers.to_string()); } if !name.is_empty() { diff --git a/src/docbuilder/limits.rs b/src/docbuilder/limits.rs index c83673711..43db1f641 100644 --- a/src/docbuilder/limits.rs +++ b/src/docbuilder/limits.rs @@ -96,7 +96,7 @@ fn scale(value: usize, interval: usize, labels: &[&str]) -> String { for label in &labels[1..] { if value / interval >= 1.0 { chosen_label = label; - value = value / interval; + value /= interval; } else { break; } diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index 181168676..e42123346 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -1,8 +1,8 @@ -use std::collections::HashSet; -use std::path::Path; -use toml::Value; use crate::error::Result; use failure::err_msg; +use std::collections::HashSet; +use std::path::Path; +use toml::{map::Map, Value}; /// Metadata for custom builds /// @@ -83,23 +83,26 @@ impl Metadata { return Ok(Metadata::from_manifest(manifest_path)); } } + Err(err_msg("Manifest not found")) } fn from_manifest>(path: P) -> Metadata { - use std::fs::File; - use std::io::Read; - let mut f = match File::open(path) { - Ok(f) => f, - Err(_) => return Metadata::default(), + use std::{fs::File, io::Read}; + + let mut file = if let Ok(file) = File::open(path) { + file + } else { + return Metadata::default(); }; - let mut s = String::new(); - if let Err(_) = f.read_to_string(&mut s) { + + let mut meta = String::new(); + if file.read_to_string(&mut meta).is_err() { return Metadata::default(); } - Metadata::from_str(&s) - } + Metadata::from_str(&meta) + } // This is similar to Default trait but it's private fn default() -> Metadata { @@ -114,56 +117,98 @@ impl Metadata { } } - fn from_str(manifest: &str) -> Metadata { let mut metadata = Metadata::default(); - let manifest = match manifest.parse::() { - Ok(m) => m, - Err(_) => return metadata, + let manifest = if let Ok(manifest) = manifest.parse::() { + manifest + } else { + return metadata; }; - if let Some(table) = manifest.get("package").and_then(|p| p.as_table()) - .and_then(|p| p.get("metadata")).and_then(|p| p.as_table()) - .and_then(|p| p.get("docs")).and_then(|p| p.as_table()) - .and_then(|p| p.get("rs")).and_then(|p| p.as_table()) { - metadata.features = table.get("features").and_then(|f| f.as_array()) - .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); - metadata.no_default_features = table.get("no-default-features") - .and_then(|v| v.as_bool()).unwrap_or(metadata.no_default_features); - metadata.all_features = table.get("all-features") - .and_then(|v| v.as_bool()).unwrap_or(metadata.all_features); - metadata.default_target = table.get("default-target") - .and_then(|v| v.as_str()).map(|v| v.to_owned()); - metadata.targets = table.get("targets").and_then(|f| f.as_array()) - .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); - metadata.rustc_args = table.get("rustc-args").and_then(|f| f.as_array()) - .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); - metadata.rustdoc_args = table.get("rustdoc-args").and_then(|f| f.as_array()) - .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); - } + if let Some(table) = fetch_manifest_tables(&manifest) { + let collect_into_array = + |f: &Vec| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect(); + + metadata.features = table + .get("features") + .and_then(|f| f.as_array()) + .and_then(collect_into_array); + + metadata.no_default_features = table + .get("no-default-features") + .and_then(|v| v.as_bool()) + .unwrap_or(metadata.no_default_features); + + metadata.all_features = table + .get("all-features") + .and_then(|v| v.as_bool()) + .unwrap_or(metadata.all_features); + + metadata.default_target = table + .get("default-target") + .and_then(|v| v.as_str()) + .map(|v| v.to_owned()); + + metadata.targets = table + .get("targets") + .and_then(|f| f.as_array()) + .and_then(collect_into_array); + + metadata.rustc_args = table + .get("rustc-args") + .and_then(|f| f.as_array()) + .and_then(collect_into_array); + + metadata.rustdoc_args = table + .get("rustdoc-args") + .and_then(|f| f.as_array()) + .and_then(collect_into_array); + } metadata } + pub(super) fn targets(&self) -> BuildTargets<'_> { use super::rustwide_builder::{HOST_TARGET, TARGETS}; - let default_target = self.default_target.as_deref() + let default_target = self + .default_target + .as_deref() // Use the first element of `targets` if `default_target` is unset and `targets` is non-empty - .or_else(|| self.targets.as_ref().and_then(|targets| targets.iter().next().map(String::as_str))) + .or_else(|| { + self.targets + .as_ref() + .and_then(|targets| targets.iter().next().map(String::as_str)) + }) .unwrap_or(HOST_TARGET); // Let people opt-in to only having specific targets - let mut targets: HashSet<_> = self.targets.as_ref() + let mut targets: HashSet<_> = self + .targets + .as_ref() .map(|targets| targets.iter().map(String::as_str).collect()) .unwrap_or_else(|| TARGETS.iter().copied().collect()); targets.remove(&default_target); - BuildTargets { default_target, other_targets: targets } + BuildTargets { + default_target, + other_targets: targets, + } } } - +fn fetch_manifest_tables<'a>(manifest: &'a Value) -> Option<&'a Map> { + manifest + .get("package")? + .as_table()? + .get("metadata")? + .as_table()? + .get("docs")? + .as_table()? + .get("rs")? + .as_table() +} #[cfg(test)] mod test { @@ -199,7 +244,10 @@ mod test { assert_eq!(features[0], "feature1".to_owned()); assert_eq!(features[1], "feature2".to_owned()); - assert_eq!(metadata.default_target.unwrap(), "x86_64-unknown-linux-gnu".to_owned()); + assert_eq!( + metadata.default_target.unwrap(), + "x86_64-unknown-linux-gnu".to_owned() + ); let targets = metadata.targets.expect("should have explicit target"); assert_eq!(targets.len(), 2); @@ -229,32 +277,42 @@ mod test { assert!(metadata.targets.is_none()); // no package.metadata.docs.rs section - let metadata = Metadata::from_str(r#" + let metadata = Metadata::from_str( + r#" [package] name = "test" - "#); + "#, + ); assert!(metadata.targets.is_none()); // targets explicitly set to empty array - let metadata = Metadata::from_str(r#" + let metadata = Metadata::from_str( + r#" [package.metadata.docs.rs] targets = [] - "#); + "#, + ); assert!(metadata.targets.unwrap().is_empty()); } #[test] fn test_select_targets() { - use crate::docbuilder::rustwide_builder::{HOST_TARGET, TARGETS}; use super::BuildTargets; + use crate::docbuilder::rustwide_builder::{HOST_TARGET, TARGETS}; let mut metadata = Metadata::default(); + // unchanged default_target, targets not specified - let BuildTargets { default_target: default, other_targets: tier_one } = metadata.targets(); + let BuildTargets { + default_target: default, + other_targets: tier_one, + } = metadata.targets(); assert_eq!(default, HOST_TARGET); + // should be equal to TARGETS \ {HOST_TARGET} for actual in &tier_one { assert!(TARGETS.contains(actual)); } + for expected in TARGETS { if *expected == HOST_TARGET { assert!(!tier_one.contains(&HOST_TARGET)); @@ -265,47 +323,89 @@ mod test { // unchanged default_target, targets specified to be empty metadata.targets = Some(Vec::new()); - let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); + + let BuildTargets { + default_target: default, + other_targets: others, + } = metadata.targets(); + assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // unchanged default_target, targets non-empty - metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); - let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); + metadata.targets = Some(vec![ + "i686-pc-windows-msvc".into(), + "i686-apple-darwin".into(), + ]); + + let BuildTargets { + default_target: default, + other_targets: others, + } = metadata.targets(); + assert_eq!(default, "i686-pc-windows-msvc"); assert_eq!(others.len(), 1); assert!(others.contains(&"i686-apple-darwin")); // make sure that default_target is not built twice metadata.targets = Some(vec![HOST_TARGET.into()]); - let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); + let BuildTargets { + default_target: default, + other_targets: others, + } = metadata.targets(); + assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // make sure that duplicates are removed - metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-pc-windows-msvc".into()]); - let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); + metadata.targets = Some(vec![ + "i686-pc-windows-msvc".into(), + "i686-pc-windows-msvc".into(), + ]); + + let BuildTargets { + default_target: default, + other_targets: others, + } = metadata.targets(); + assert_eq!(default, "i686-pc-windows-msvc"); assert!(others.is_empty()); // make sure that `default_target` always takes priority over `targets` metadata.default_target = Some("i686-apple-darwin".into()); - let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); + let BuildTargets { + default_target: default, + other_targets: others, + } = metadata.targets(); + assert_eq!(default, "i686-apple-darwin"); assert_eq!(others.len(), 1); assert!(others.contains(&"i686-pc-windows-msvc")); // make sure that `default_target` takes priority over `HOST_TARGET` metadata.targets = Some(vec![]); - let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); + let BuildTargets { + default_target: default, + other_targets: others, + } = metadata.targets(); + assert_eq!(default, "i686-apple-darwin"); assert!(others.is_empty()); // and if `targets` is unset, it should still be set to `TARGETS` metadata.targets = None; - let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); + let BuildTargets { + default_target: default, + other_targets: others, + } = metadata.targets(); + assert_eq!(default, "i686-apple-darwin"); - let tier_one_targets_no_default = TARGETS.iter().filter(|&&t| t != "i686-apple-darwin").copied().collect(); + let tier_one_targets_no_default = TARGETS + .iter() + .filter(|&&t| t != "i686-apple-darwin") + .copied() + .collect(); + assert_eq!(others, tier_one_targets_no_default); } } diff --git a/src/docbuilder/mod.rs b/src/docbuilder/mod.rs index 90b94526e..d5a1f4da2 100644 --- a/src/docbuilder/mod.rs +++ b/src/docbuilder/mod.rs @@ -1,25 +1,22 @@ - -pub(crate) mod options; -mod metadata; -mod limits; -mod rustwide_builder; mod crates; +mod limits; +mod metadata; +pub(crate) mod options; mod queue; +mod rustwide_builder; -pub use self::rustwide_builder::RustwideBuilder; -pub(crate) use self::rustwide_builder::BuildResult; pub(crate) use self::limits::Limits; pub(self) use self::metadata::Metadata; +pub(crate) use self::rustwide_builder::BuildResult; +pub use self::rustwide_builder::RustwideBuilder; - +use crate::error::Result; +use crate::DocBuilderOptions; +use std::collections::BTreeSet; use std::fs; use std::io::prelude::*; use std::io::BufReader; use std::path::PathBuf; -use std::collections::BTreeSet; -use crate::DocBuilderOptions; -use crate::error::Result; - /// chroot based documentation builder pub struct DocBuilder { @@ -28,22 +25,21 @@ pub struct DocBuilder { db_cache: BTreeSet, } - impl DocBuilder { pub fn new(options: DocBuilderOptions) -> DocBuilder { DocBuilder { - options: options, + options, cache: BTreeSet::new(), db_cache: BTreeSet::new(), } } - /// Loads build cache pub fn load_cache(&mut self) -> Result<()> { debug!("Loading cache"); + let path = PathBuf::from(&self.options.prefix).join("cache"); - let reader = fs::File::open(path).map(|f| BufReader::new(f)); + let reader = fs::File::open(path).map(BufReader::new); if let Ok(reader) = reader { for line in reader.lines() { @@ -56,40 +52,43 @@ impl DocBuilder { Ok(()) } - fn load_database_cache(&mut self) -> Result<()> { debug!("Loading database cache"); + use crate::db::connect_db; let conn = connect_db()?; - for row in &conn.query("SELECT name, version FROM crates, releases \ - WHERE crates.id = releases.crate_id", - &[]) - .unwrap() { + for row in &conn + .query( + "SELECT name, version FROM crates, releases \ + WHERE crates.id = releases.crate_id", + &[], + ) + .unwrap() + { let name: String = row.get(0); let version: String = row.get(1); + self.db_cache.insert(format!("{}-{}", name, version)); } Ok(()) } - /// Saves build cache pub fn save_cache(&self) -> Result<()> { debug!("Saving cache"); + let path = PathBuf::from(&self.options.prefix).join("cache"); - let mut file = fs::OpenOptions::new() - .write(true) - .create(true) - .open(path)?; + let mut file = fs::OpenOptions::new().write(true).create(true).open(path)?; + for krate in &self.cache { writeln!(file, "{}", krate)?; } + Ok(()) } - fn lock_path(&self) -> PathBuf { self.options.prefix.join("cratesfyi.lock") } @@ -100,6 +99,7 @@ impl DocBuilder { if !path.exists() { fs::OpenOptions::new().write(true).create(true).open(path)?; } + Ok(()) } @@ -109,6 +109,7 @@ impl DocBuilder { if path.exists() { fs::remove_file(path)?; } + Ok(()) } @@ -130,6 +131,7 @@ impl DocBuilder { let name = format!("{}-{}", name, version); let local = self.options.skip_if_log_exists && self.cache.contains(&name); let db = self.options.skip_if_exists && self.db_cache.contains(&name); + !(local || db) } } diff --git a/src/docbuilder/options.rs b/src/docbuilder/options.rs index b7cd509de..668a2f20d 100644 --- a/src/docbuilder/options.rs +++ b/src/docbuilder/options.rs @@ -1,8 +1,6 @@ - - -use std::{env, fmt}; -use std::path::PathBuf; use crate::error::Result; +use std::path::PathBuf; +use std::{env, fmt}; #[derive(Clone)] pub struct DocBuilderOptions { @@ -16,19 +14,15 @@ pub struct DocBuilderOptions { pub debug: bool, } - - impl Default for DocBuilderOptions { fn default() -> DocBuilderOptions { - let cwd = env::current_dir().unwrap(); - let (prefix, crates_io_index_path) = - generate_paths(cwd); + let (prefix, crates_io_index_path) = generate_paths(cwd); DocBuilderOptions { - prefix: prefix, - crates_io_index_path: crates_io_index_path, + prefix, + crates_io_index_path, keep_build_directory: false, skip_if_exists: false, @@ -40,28 +34,28 @@ impl Default for DocBuilderOptions { } } - impl fmt::Debug for DocBuilderOptions { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, - "DocBuilderOptions {{ \ + write!( + f, + "DocBuilderOptions {{ \ crates_io_index_path: {:?}, \ keep_build_directory: {:?}, skip_if_exists: {:?}, \ skip_if_log_exists: {:?}, debug: {:?} }}", - self.crates_io_index_path, - self.keep_build_directory, - self.skip_if_exists, - self.skip_if_log_exists, - self.debug) + self.crates_io_index_path, + self.keep_build_directory, + self.skip_if_exists, + self.skip_if_log_exists, + self.debug + ) } } - impl DocBuilderOptions { /// Creates new DocBuilderOptions from prefix pub fn from_prefix(prefix: PathBuf) -> DocBuilderOptions { let (prefix, crates_io_index_path) = generate_paths(prefix); - + DocBuilderOptions { prefix, crates_io_index_path, @@ -69,17 +63,18 @@ impl DocBuilderOptions { } } - pub fn check_paths(&self) -> Result<()> { if !self.crates_io_index_path.exists() { - failure::bail!("crates.io-index path '{}' does not exist", self.crates_io_index_path.display()); + failure::bail!( + "crates.io-index path '{}' does not exist", + self.crates_io_index_path.display() + ); } + Ok(()) } } - - fn generate_paths(prefix: PathBuf) -> (PathBuf, PathBuf) { let crates_io_index_path = PathBuf::from(&prefix).join("crates.io-index"); diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index d4ecf5132..f5d38f339 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -90,7 +90,7 @@ impl RustwideBuilder { workspace.purge_all_build_dirs()?; let toolchain_name = std::env::var("CRATESFYI_TOOLCHAIN") - .map(|t| Cow::Owned(t)) + .map(Cow::Owned) .unwrap_or_else(|_| Cow::Borrowed("nightly")); let toolchain = Toolchain::dist(&toolchain_name); @@ -108,7 +108,7 @@ impl RustwideBuilder { let mut targets_to_install = TARGETS .iter() - .map(|t| t.to_string()) + .map(|t| (*t).to_string()) .collect::>(); let installed_targets = match self.toolchain.installed_targets(&self.workspace) { Ok(targets) => targets, @@ -146,7 +146,7 @@ impl RustwideBuilder { } self.rustc_version = self.detect_rustc_version()?; - if old_version.as_ref().map(|s| s.as_str()) != Some(&self.rustc_version) { + if old_version.as_deref() != Some(&self.rustc_version) { self.add_essential_files()?; } @@ -389,7 +389,7 @@ impl RustwideBuilder { has_docs, has_examples, )?; - add_build_into_database(&conn, &release_id, &res.result)?; + add_build_into_database(&conn, release_id, &res.result)?; doc_builder.add_to_cache(name, version); Ok(res) diff --git a/src/lib.rs b/src/lib.rs index 3af8e597f..9a103d297 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,22 +4,21 @@ #[macro_use] extern crate log; -pub use self::docbuilder::RustwideBuilder; -pub use self::docbuilder::DocBuilder; pub use self::docbuilder::options::DocBuilderOptions; +pub use self::docbuilder::DocBuilder; +pub use self::docbuilder::RustwideBuilder; pub use self::web::Server; -mod error; pub mod db; -pub mod utils; mod docbuilder; -mod web; +mod error; #[cfg(test)] mod test; +pub mod utils; +mod web; use web::page::GlobalAlert; - // Warning message shown in the navigation bar of every page. Set to `None` to hide it. pub(crate) static GLOBAL_ALERT: Option = None; /* @@ -31,10 +30,10 @@ pub(crate) static GLOBAL_ALERT: Option = Some(GlobalAlert { }); */ - /// Version string generated at build time contains last git /// commit hash and build date -pub const BUILD_VERSION: &'static str = concat!(env!("CARGO_PKG_VERSION"), - " ", - include_str!(concat!(env!("OUT_DIR"), - "/git_version"))); +pub const BUILD_VERSION: &str = concat!( + env!("CARGO_PKG_VERSION"), + " ", + include_str!(concat!(env!("OUT_DIR"), "/git_version")) +); diff --git a/src/test/fakes.rs b/src/test/fakes.rs index 998f96958..0894b5b24 100644 --- a/src/test/fakes.rs +++ b/src/test/fakes.rs @@ -182,7 +182,7 @@ impl<'a> FakeRelease<'a> { self.has_docs, self.has_examples, )?; - crate::db::add_build_into_database(&db.conn(), &release_id, &self.build_result)?; + crate::db::add_build_into_database(&db.conn(), release_id, &self.build_result)?; Ok(release_id) } diff --git a/src/utils/daemon.rs b/src/utils/daemon.rs index 0763ab219..f9d4b1ff4 100644 --- a/src/utils/daemon.rs +++ b/src/utils/daemon.rs @@ -25,13 +25,16 @@ use ::{ }; pub fn start_daemon(background: bool) { + const CRATE_VARIABLES: [&str; 4] = [ + "CRATESFYI_PREFIX", + "CRATESFYI_PREFIX", + "CRATESFYI_GITHUB_USERNAME", + "CRATESFYI_GITHUB_ACCESSTOKEN" + ]; + // first check required environment variables - for v in ["CRATESFYI_PREFIX", - "CRATESFYI_PREFIX", - "CRATESFYI_GITHUB_USERNAME", - "CRATESFYI_GITHUB_ACCESSTOKEN"] - .iter() { - env::var(v).expect(&format!("Environment variable {} not found", v)); + for v in CRATE_VARIABLES.iter() { + env::var(v).unwrap_or_else(|_| panic!("Environment variable {} not found", v)); } let dbopts = opts(); @@ -44,6 +47,7 @@ pub fn start_daemon(background: bool) { { panic!("running in background not supported on windows"); } + #[cfg(not(target_os = "windows"))] { // fork the process @@ -82,6 +86,7 @@ pub fn start_daemon(background: bool) { }).unwrap(); // build new crates every minute + // REFACTOR: Break this into smaller functions thread::Builder::new().name("build queue reader".to_string()).spawn(move || { let opts = opts(); let mut doc_builder = DocBuilder::new(opts); @@ -141,6 +146,7 @@ pub fn start_daemon(background: bool) { error!("Failed to read the number of crates in the queue: {}", e); continue; } + Ok(0) => { if status.count() > 0 { // ping the hubs before continuing @@ -157,6 +163,7 @@ pub fn start_daemon(background: bool) { status = BuilderState::EmptyQueue; continue; } + Ok(queue_count) => { info!("Starting build with {} crates in queue (currently on a {} crate streak)", queue_count, status.count()); @@ -180,7 +187,6 @@ pub fn start_daemon(background: bool) { Ok(crate_built) => if crate_built { status.increment(); } - } })); @@ -251,140 +257,14 @@ pub fn start_daemon(background: bool) { // at least start web server info!("Starting web server"); + crate::Server::start(None); } -/// Builds crates every minute -fn queue_reader_runtime() { - let opts = opts(); - let mut doc_builder = DocBuilder::new(opts); - - /// Represents the current state of the builder thread. - enum BuilderState { - /// The builder thread has just started, and hasn't built any crates yet. - Fresh, - /// The builder has just seen an empty build queue. - EmptyQueue, - /// The builder has just seen the lock file. - Locked, - /// The builder has just finished building a crate. The enclosed count is the number of - /// crates built since the caches have been refreshed. - QueueInProgress(usize), - } - - let mut builder = RustwideBuilder::init().unwrap(); - - let mut status = BuilderState::Fresh; - - loop { - if !status.is_in_progress() { - thread::sleep(Duration::from_secs(60)); - } - - // check lock file - if doc_builder.is_locked() { - warn!("Lock file exits, skipping building new crates"); - status = BuilderState::Locked; - continue; - } - - if status.count() >= 10 { - // periodically, we need to flush our caches and ping the hubs - debug!("10 builds in a row; flushing caches"); - status = BuilderState::QueueInProgress(0); - - match pubsubhubbub::ping_hubs() { - Err(e) => error!("Failed to ping hub: {}", e), - Ok(n) => debug!("Succesfully pinged {} hubs", n) - } - - if let Err(e) = doc_builder.load_cache() { - error!("Failed to load cache: {}", e); - } - - if let Err(e) = doc_builder.save_cache() { - error!("Failed to save cache: {}", e); - } - } - - // Only build crates if there are any to build - debug!("Checking build queue"); - match doc_builder.get_queue_count() { - Err(e) => { - error!("Failed to read the number of crates in the queue: {}", e); - continue; - } - Ok(0) => { - if status.count() > 0 { - // ping the hubs before continuing - match pubsubhubbub::ping_hubs() { - Err(e) => error!("Failed to ping hub: {}", e), - Ok(n) => debug!("Succesfully pinged {} hubs", n) - } - - if let Err(e) = doc_builder.save_cache() { - error!("Failed to save cache: {}", e); - } - } - debug!("Queue is empty, going back to sleep"); - status = BuilderState::EmptyQueue; - continue; - } - Ok(queue_count) => { - info!("Starting build with {} crates in queue (currently on a {} crate streak)", - queue_count, status.count()); - } - } - - // if we're starting a new batch, reload our caches and sources - if !status.is_in_progress() { - if let Err(e) = doc_builder.load_cache() { - error!("Failed to load cache: {}", e); - continue; - } - } - - // Run build_packages_queue under `catch_unwind` to catch panics - // This only panicked twice in the last 6 months but its just a better - // idea to do this. - let res = catch_unwind(AssertUnwindSafe(|| { - match doc_builder.build_next_queue_package(&mut builder) { - Err(e) => error!("Failed to build crate from queue: {}", e), - Ok(crate_built) => if crate_built { - status.increment(); - } - - } - })); - - if let Err(e) = res { - error!("GRAVE ERROR Building new crates panicked: {:?}", e); - } - } - - impl BuilderState { - fn count(&self) -> usize { - match *self { - BuilderState::QueueInProgress(n) => n, - _ => 0, - } - } - - fn is_in_progress(&self) -> bool { - match *self { - BuilderState::QueueInProgress(_) => true, - _ => false, - } - } - - fn increment(&mut self) { - *self = BuilderState::QueueInProgress(self.count() + 1); - } - } -} - fn opts() -> DocBuilderOptions { - let prefix = PathBuf::from(env::var("CRATESFYI_PREFIX") - .expect("CRATESFYI_PREFIX environment variable not found")); + let prefix = PathBuf::from( + env::var("CRATESFYI_PREFIX").expect("CRATESFYI_PREFIX environment variable not found") + ); + DocBuilderOptions::from_prefix(prefix) } diff --git a/src/utils/github_updater.rs b/src/utils/github_updater.rs index a1de0cf97..2acce7bca 100644 --- a/src/utils/github_updater.rs +++ b/src/utils/github_updater.rs @@ -108,27 +108,29 @@ fn get_github_fields(path: &str) -> Result { .and_then(|d| d.as_string()) .unwrap_or(""), "%Y-%m-%dT%H:%M:%S") - .unwrap_or(time::now()) + .unwrap_or_else(|_| time::now()) .to_timespec(), }) } - - fn get_github_path(url: &str) -> Option { let re = Regex::new(r"https?://github\.com/([\w\._-]+)/([\w\._-]+)").unwrap(); match re.captures(url) { Some(cap) => { let username = cap.get(1).unwrap().as_str(); let reponame = cap.get(2).unwrap().as_str(); - Some(format!("{}/{}", - username, - if reponame.ends_with(".git") { - reponame.split(".git").nth(0).unwrap() - } else { - reponame - })) + + Some(format!( + "{}/{}", + username, + if reponame.ends_with(".git") { + reponame.split(".git").next().unwrap() + } else { + reponame + } + )) } + None => None, } } diff --git a/src/utils/html.rs b/src/utils/html.rs index f0671e423..a217b7069 100644 --- a/src/utils/html.rs +++ b/src/utils/html.rs @@ -1,9 +1,9 @@ use crate::error::Result; use failure::err_msg; -use html5ever::serialize::{serialize, SerializeOpts}; -use html5ever::rcdom::{RcDom, NodeData, Handle}; use html5ever::driver::{parse_document, ParseOpts}; +use html5ever::rcdom::{Handle, NodeData, RcDom}; +use html5ever::serialize::{serialize, SerializeOpts}; use html5ever::tendril::TendrilSink; /// Extracts the contents of the `` and `` tags from an HTML document, as well as the @@ -23,8 +23,8 @@ fn extract_from_rcdom(dom: &RcDom) -> Result<(Handle, Handle)> { let (mut head, mut body) = (None, None); while let Some(handle) = worklist.pop() { - match handle.data { - NodeData::Element { ref name, .. } => match name.local.as_ref() { + if let NodeData::Element { ref name, .. } = handle.data { + match name.local.as_ref() { "head" => { if head.is_some() { return Err(err_msg("duplicate tag")); @@ -32,6 +32,7 @@ fn extract_from_rcdom(dom: &RcDom) -> Result<(Handle, Handle)> { head = Some(handle.clone()); } } + "body" => { if body.is_some() { return Err(err_msg("duplicate tag")); @@ -39,9 +40,9 @@ fn extract_from_rcdom(dom: &RcDom) -> Result<(Handle, Handle)> { body = Some(handle.clone()); } } - _ => {} // do nothing + + _ => {} // do nothing } - _ => {} // do nothing } worklist.extend(handle.children.borrow().iter().cloned()); @@ -54,8 +55,7 @@ fn extract_from_rcdom(dom: &RcDom) -> Result<(Handle, Handle)> { fn stringify(node: Handle) -> String { let mut vec = Vec::new(); - serialize(&mut vec, &node, SerializeOpts::default()) - .expect("serializing into buffer failed"); + serialize(&mut vec, &node, SerializeOpts::default()).expect("serializing into buffer failed"); String::from_utf8(vec).expect("html5ever returned non-utf8 data") } @@ -65,11 +65,13 @@ fn extract_class(node: &Handle) -> String { NodeData::Element { ref attrs, .. } => { let attrs = attrs.borrow(); - attrs.iter() - .find(|a| &a.name.local == "class") - .map_or(String::new(), |a| a.value.to_string()) + attrs + .iter() + .find(|a| &a.name.local == "class") + .map_or(String::new(), |a| a.value.to_string()) } - _ => String::new() + + _ => String::new(), } } @@ -80,6 +82,7 @@ mod test { let (head, body, class) = super::extract_head_and_body( r#"

hello

"# ).unwrap(); + assert_eq!(head, r#""#); assert_eq!(body, "

hello

"); assert_eq!(class, "rustdoc struct"); @@ -92,6 +95,7 @@ mod test { let expected_head = std::fs::read_to_string("tests/regex/head.html").unwrap(); let expected_body = std::fs::read_to_string("tests/regex/body.html").unwrap(); let (head, body, class) = super::extract_head_and_body(&original).unwrap(); + assert_eq!(head, expected_head.trim()); assert_eq!(&body, &expected_body.trim()); assert_eq!(class, "rustdoc struct"); diff --git a/src/utils/release_activity_updater.rs b/src/utils/release_activity_updater.rs index ed0ea0c22..c11b769c5 100644 --- a/src/utils/release_activity_updater.rs +++ b/src/utils/release_activity_updater.rs @@ -1,42 +1,50 @@ - use crate::db::connect_db; -use time::{now, Duration}; -use std::collections::BTreeMap; -use rustc_serialize::json::ToJson; use crate::error::Result; - +use rustc_serialize::json::ToJson; +use std::collections::BTreeMap; +use time::{now, Duration}; pub fn update_release_activity() -> Result<()> { - let conn = connect_db()?; - let mut dates = Vec::new(); - let mut crate_counts = Vec::new(); - let mut failure_counts = Vec::new(); + let mut dates = Vec::with_capacity(30); + let mut crate_counts = Vec::with_capacity(30); + let mut failure_counts = Vec::with_capacity(30); for day in 0..30 { - let rows = conn.query(&format!("SELECT COUNT(*) - FROM releases - WHERE release_time < NOW() - INTERVAL '{} day' AND - release_time > NOW() - INTERVAL '{} day'", - day, - day + 1), - &[])?; + let rows = conn.query( + &format!( + "SELECT COUNT(*) + FROM releases + WHERE release_time < NOW() - INTERVAL '{} day' AND + release_time > NOW() - INTERVAL '{} day'", + day, + day + 1 + ), + &[], + )?; + let failures_count_rows = conn.query( - &format!("SELECT COUNT(*) - FROM releases - WHERE is_library = TRUE AND - build_status = FALSE AND - release_time < NOW() - INTERVAL '{} day' AND - release_time > NOW() - INTERVAL '{} day'", - day, - day + 1), - &[])?; + &format!( + "SELECT COUNT(*) + FROM releases + WHERE is_library = TRUE AND + build_status = FALSE AND + release_time < NOW() - INTERVAL '{} day' AND + release_time > NOW() - INTERVAL '{} day'", + day, + day + 1 + ), + &[], + )?; + let release_count: i64 = rows.get(0).get(0); let failure_count: i64 = failures_count_rows.get(0).get(0); let now = now(); let date = now - Duration::days(day); + + // unwrap is fine here, as our date format is always valid dates.push(format!("{}", date.strftime("%d %b").unwrap())); - // unwrap is fine here, ~~~~~~~~~~~~^ our date format is always valid + crate_counts.push(release_count); failure_counts.push(failure_count); } @@ -50,20 +58,24 @@ pub fn update_release_activity() -> Result<()> { map.insert("dates".to_owned(), dates.to_json()); map.insert("counts".to_owned(), crate_counts.to_json()); map.insert("failures".to_owned(), failure_counts.to_json()); + map.to_json() }; - conn.query("INSERT INTO config (name, value) VALUES ('release_activity', $1)", - &[&map]) - .or_else(|_| { - conn.query("UPDATE config SET value = $1 WHERE name = 'release_activity'", - &[&map]) - })?; + conn.query( + "INSERT INTO config (name, value) VALUES ('release_activity', $1)", + &[&map], + ) + .or_else(|_| { + conn.query( + "UPDATE config SET value = $1 WHERE name = 'release_activity'", + &[&map], + ) + })?; Ok(()) } - #[cfg(test)] mod test { use super::update_release_activity; diff --git a/src/utils/rustc_version.rs b/src/utils/rustc_version.rs index 8613b3d45..09ca56038 100644 --- a/src/utils/rustc_version.rs +++ b/src/utils/rustc_version.rs @@ -1,26 +1,32 @@ - -use regex::Regex; use crate::error::Result; use failure::err_msg; +use regex::Regex; /// Parses rustc commit hash from rustc version string pub fn parse_rustc_version>(version: S) -> Result { let version_regex = Regex::new(r" ([\w.-]+) \((\w+) (\d+)-(\d+)-(\d+)\)")?; - let captures = version_regex.captures(version.as_ref()) + let captures = version_regex + .captures(version.as_ref()) .ok_or_else(|| err_msg("Failed to parse rustc version"))?; - Ok(format!("{}{}{}-{}-{}", - captures.get(3).unwrap().as_str(), - captures.get(4).unwrap().as_str(), - captures.get(5).unwrap().as_str(), - captures.get(1).unwrap().as_str(), - captures.get(2).unwrap().as_str())) + Ok(format!( + "{}{}{}-{}-{}", + captures.get(3).unwrap().as_str(), + captures.get(4).unwrap().as_str(), + captures.get(5).unwrap().as_str(), + captures.get(1).unwrap().as_str(), + captures.get(2).unwrap().as_str() + )) } #[test] fn test_parse_rustc_version() { - assert_eq!(parse_rustc_version("rustc 1.10.0-nightly (57ef01513 2016-05-23)").unwrap(), - "20160523-1.10.0-nightly-57ef01513"); - assert_eq!(parse_rustc_version("cratesfyi 0.2.0 (ba9ae23 2016-05-26)").unwrap(), - "20160526-0.2.0-ba9ae23"); + assert_eq!( + parse_rustc_version("rustc 1.10.0-nightly (57ef01513 2016-05-23)").unwrap(), + "20160523-1.10.0-nightly-57ef01513" + ); + assert_eq!( + parse_rustc_version("cratesfyi 0.2.0 (ba9ae23 2016-05-26)").unwrap(), + "20160526-0.2.0-ba9ae23" + ); } diff --git a/src/web/builds.rs b/src/web/builds.rs index 80ee92ad9..b0ce00b1c 100644 --- a/src/web/builds.rs +++ b/src/web/builds.rs @@ -97,7 +97,7 @@ pub fn build_list_handler(req: &mut Request) -> IronResult { let id: i32 = row.get(5); let build = Build { - id: id, + id, rustc_version: row.get(6), cratesfyi_version: row.get(7), build_status: row.get(8), @@ -134,7 +134,7 @@ pub fn build_list_handler(req: &mut Request) -> IronResult { let builds_page = BuildsPage { metadata: MetaData::from_crate(&conn, &name, &version), builds: build_list, - build_details: build_details, + build_details, limits, }; Page::new(builds_page) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index e4e4d9517..16986ae54 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -146,7 +146,7 @@ impl CrateDetails { let rows = conn.query(query, &[&name, &version]).unwrap(); - if rows.len() == 0 { + if rows.is_empty() { return None; } @@ -158,15 +158,15 @@ impl CrateDetails { let mut versions: Vec = Vec::new(); let versions_from_db: Json = rows.get(0).get(17); - versions_from_db.as_array().map(|vers| { + if let Some(vers) = versions_from_db.as_array() { for version in vers { - version.as_string().map(|version| { + if let Some(version) = version.as_string() { if let Ok(sem_ver) = semver::Version::parse(&version) { versions.push(sem_ver); - }; - }); + } + } } - }); + } versions.sort(); versions.reverse(); @@ -206,7 +206,7 @@ impl CrateDetails { github_stars: rows.get(0).get(18), github_forks: rows.get(0).get(19), github_issues: rows.get(0).get(20), - metadata: metadata, + metadata, is_library: rows.get(0).get(21), doc_targets: rows.get(0).get(22), license: rows.get(0).get(23), diff --git a/src/web/mod.rs b/src/web/mod.rs index 31fcbf0d1..d35eb687e 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -1,6 +1,5 @@ //! Web interface of cratesfyi - pub(crate) mod page; /// ctry! (cratesfyitry) is extremely similar to try! and itry! @@ -9,10 +8,12 @@ macro_rules! ctry { ($result:expr) => (match $result { Ok(v) => v, Err(e) => { - return $crate::web::page::Page::new(format!("{:?}", e)).title("An error has occured") - .set_status(::iron::status::BadRequest).to_resp("resp"); + return $crate::web::page::Page::new(format!("{:?}", e)) + .title("An error has occured") + .set_status(::iron::status::BadRequest) + .to_resp("resp"); } - }) + }); } /// cexpect will check an option and if it's not Some @@ -23,16 +24,17 @@ macro_rules! cexpect { None => { return $crate::web::page::Page::new("Resource not found".to_owned()) .title("An error has occured") - .set_status(::iron::status::BadRequest).to_resp("resp"); + .set_status(::iron::status::BadRequest) + .to_resp("resp"); } - }) + }); } /// Gets an extension from Request macro_rules! extension { ($req:expr, $ext:ty) => ( cexpect!($req.extensions.get::<$ext>()) - ) + ); } mod rustdoc; @@ -70,10 +72,10 @@ use std::sync::{Arc, Mutex}; /// Duration of static files for staticfile and DatabaseFileHandler (in seconds) const STATIC_FILE_CACHE_DURATION: u64 = 60 * 60 * 24 * 30 * 12; // 12 months -const STYLE_CSS: &'static str = include_str!(concat!(env!("OUT_DIR"), "/style.css")); -const MENU_JS: &'static str = include_str!(concat!(env!("OUT_DIR"), "/menu.js")); -const INDEX_JS: &'static str = include_str!(concat!(env!("OUT_DIR"), "/index.js")); -const OPENSEARCH_XML: &'static [u8] = include_bytes!("opensearch.xml"); +const STYLE_CSS: &str = include_str!(concat!(env!("OUT_DIR"), "/style.css")); +const MENU_JS: &str = include_str!(concat!(env!("OUT_DIR"), "/menu.js")); +const INDEX_JS: &str = include_str!(concat!(env!("OUT_DIR"), "/index.js")); +const OPENSEARCH_XML: & [u8] = include_bytes!("opensearch.xml"); const DEFAULT_BIND: &str = "0.0.0.0:3000"; @@ -88,7 +90,6 @@ struct CratesfyiHandler { pool_factory: PoolFactory, } - impl CratesfyiHandler { fn chain(pool_factory: &PoolFactoryFn, base: H) -> Chain { // TODO: Use DocBuilderOptions for paths @@ -112,7 +113,11 @@ impl CratesfyiHandler { let shared_resources = Self::chain(&pool_factory, rustdoc::SharedResourceHandler); let router_chain = Self::chain(&pool_factory, routes.iron_router()); - let prefix = PathBuf::from(env::var("CRATESFYI_PREFIX").expect("the CRATESFYI_PREFIX environment variable is not set")).join("public_html"); + let prefix = PathBuf::from( + env::var("CRATESFYI_PREFIX") + .expect("the CRATESFYI_PREFIX environment variable is not set"), + ) + .join("public_html"); let static_handler = Static::new(prefix) .cache(Duration::from_secs(STATIC_FILE_CACHE_DURATION)); @@ -220,55 +225,61 @@ impl MatchVersion { /// `version` may be an exact version number or loose semver version requirement. The return value /// will indicate whether the given version exactly matched a version number from the database. fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> MatchVersion { + use std::borrow::Cow; // version is an Option<&str> from router::Router::get // need to decode first use url::percent_encoding::percent_decode; - let req_version = version.and_then(|v| { - match percent_decode(v.as_bytes()).decode_utf8() { - Ok(p) => Some(p.into_owned()), - Err(_) => None, - } + let req_version = version + .and_then(|v| { + percent_decode(v.as_bytes()) + .decode_utf8() + .ok() + .map(Cow::into_owned) }) .map(|v| if v == "newest" || v == "latest" { "*".to_owned() } else { v }) - .unwrap_or("*".to_string()); + .unwrap_or_else(|| "*".to_string()); let versions: Vec<(String, i32)> = { let query = "SELECT version, releases.id FROM releases INNER JOIN crates ON releases.crate_id = crates.id WHERE name = $1 AND yanked = false"; let rows = conn.query(query, &[&name]).unwrap(); - if rows.len() == 0 { + + if rows.is_empty() { return MatchVersion::None; } + rows.iter().map(|row| (row.get(0), row.get(1))).collect() }; // first check for exact match // we can't expect users to use semver in query - for version in &versions { - if version.0 == req_version { - return MatchVersion::Exact(version.clone()); - } + if let Some(exact) = versions.iter().find(|(vers, _)| vers == &req_version) { + // REFACTOR: Get around the .clone() + return MatchVersion::Exact(exact.clone()); } // Now try to match with semver - let req_sem_ver = match VersionReq::parse(&req_version) { - Ok(v) => v, - Err(_) => return MatchVersion::None, + let req_sem_ver = if let Ok(version) = VersionReq::parse(&req_version) { + version + } else { + return MatchVersion::None; }; // 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 { + for (vers, id) in &versions { // in theory a crate must always have a semver compatible version // but check result just in case - let version = match Version::parse(&version.0) { - Ok(v) => (v, version.1), - Err(_) => return MatchVersion::None, + let version = if let Ok(v) = Version::parse(&vers) { + (v, *id) + } else { + return MatchVersion::None; }; + versions_sem.push(version); } @@ -277,10 +288,8 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> MatchV versions_sem }; - for version in &versions_sem { - if req_sem_ver.matches(&version.0) { - return MatchVersion::Semver((version.0.to_string(), version.1)); - } + if let Some((version, id)) = versions_sem.iter().find(|(vers, _)| req_sem_ver.matches(vers)) { + return MatchVersion::Semver((version.to_string(), *id)); } // semver is acting weird for '*' (any) range if a crate only have pre-release versions @@ -292,10 +301,6 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> MatchV MatchVersion::None } - - - - /// Wrapper around the Markdown parser and renderer to render markdown fn render_markdown(text: &str) -> String { use comrak::{markdown_to_html, ComrakOptions}; @@ -314,15 +319,13 @@ fn render_markdown(text: &str) -> String { markdown_to_html(text, &options) } - - pub struct Server { inner: Listening, } impl Server { pub fn start(addr: Option<&str>) -> Self { - let server = Self::start_inner(addr.unwrap_or(DEFAULT_BIND), Box::new(|| Pool::new())); + let server = Self::start_inner(addr.unwrap_or(DEFAULT_BIND), Box::new(Pool::new)); info!("Running docs.rs web server on http://{}", server.addr()); server } @@ -364,33 +367,31 @@ impl Server { } } - /// Converts Timespec to nice readable relative time string fn duration_to_str(ts: time::Timespec) -> String { - let tm = time::at(ts); let delta = time::now() - tm; + let delta = ( + delta.num_days(), + delta.num_hours(), + delta.num_minutes(), + delta.num_seconds(), + ); - if delta.num_days() > 5 { - format!("{}", tm.strftime("%b %d, %Y").unwrap()) - } else if delta.num_days() > 1 { - format!("{} days ago", delta.num_days()) - } else if delta.num_days() == 1 { - "one day ago".to_string() - } else if delta.num_hours() > 1 { - format!("{} hours ago", delta.num_hours()) - } else if delta.num_hours() == 1 { - "an hour ago".to_string() - } else if delta.num_minutes() > 1 { - format!("{} minutes ago", delta.num_minutes()) - } else if delta.num_minutes() == 1 { - "one minute ago".to_string() - } else if delta.num_seconds() > 0 { - format!("{} seconds ago", delta.num_seconds()) - } else { - "just now".to_string() - } + match delta { + (days, _, _, _) if days > 5 => format!("{}", tm.strftime("%b %d, %Y").unwrap()), + (days, _, _, _) if days > 1 => format!("{} days ago", days), + (days, _, _, _) if days == 1 => "one day ago".to_string(), + + (_, hours, _, _) if hours > 1 => format!("{} hours ago", hours), + (_, hours, _, _) if hours == 1 => "an hour ago".to_string(), + (_, _, minutes, _) if minutes > 1 => format!("{} minutes ago", minutes), + (_, _, minutes, _) if minutes == 1 => "one minute ago".to_string(), + + (_, _, _, seconds) if seconds > 0 => format!("{} seconds ago", seconds), + _ => "just now".to_string(), + } } /// Creates a `Response` which redirects to the given path on the scheme/host/port from the given @@ -422,28 +423,40 @@ fn redirect_base(req: &Request) -> String { fn style_css_handler(_: &mut Request) -> IronResult { let mut response = Response::with((status::Ok, STYLE_CSS)); - let cache = vec![CacheDirective::Public, - CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32)]; + let cache = vec![ + CacheDirective::Public, + CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32), + ]; + response.headers.set(ContentType("text/css".parse().unwrap())); response.headers.set(CacheControl(cache)); + Ok(response) } fn load_js(file_path_str: &'static str) -> IronResult { let mut response = Response::with((status::Ok, file_path_str)); - let cache = vec![CacheDirective::Public, - CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32)]; + let cache = vec![ + CacheDirective::Public, + CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32), + ]; + response.headers.set(ContentType("application/javascript".parse().unwrap())); response.headers.set(CacheControl(cache)); + Ok(response) } fn opensearch_xml_handler(_: &mut Request) -> IronResult { let mut response = Response::with((status::Ok, OPENSEARCH_XML)); - let cache = vec![CacheDirective::Public, - CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32)]; + let cache = vec![ + CacheDirective::Public, + CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32) + ]; + response.headers.set(ContentType("application/opensearchdescription+xml".parse().unwrap())); response.headers.set(CacheControl(cache)); + Ok(response) } @@ -472,7 +485,6 @@ pub(crate) struct MetaData { pub default_target: String, } - impl MetaData { fn from_crate(conn: &Connection, name: &str, version: &str) -> Option { let rows = conn.query( @@ -501,7 +513,6 @@ impl MetaData { } } - impl ToJson for MetaData { fn to_json(&self) -> Json { let mut m: BTreeMap = BTreeMap::new(); @@ -511,6 +522,7 @@ impl ToJson for MetaData { m.insert("target_name".to_owned(), self.target_name.to_json()); m.insert("rustdoc_status".to_owned(), self.rustdoc_status.to_json()); m.insert("default_target".to_owned(), self.default_target.to_json()); + m.to_json() } } @@ -527,6 +539,7 @@ mod test { fn release(version: &str, db: &TestDatabase) -> i32 { db.fake_release().name("foo").version(version).create().unwrap() } + fn version(v: Option<&str>, db: &TestDatabase) -> Option { match_version(&db.conn(), "foo", v).strip_id() } @@ -564,15 +577,18 @@ mod test { .source_file("test.rs", &[]) .create() .unwrap(); + let web = env.frontend(); assert!(clipboard_is_present_for_path( "/crate/fake_crate/0.0.1", web )); + assert!(clipboard_is_present_for_path( "/crate/fake_crate/0.0.1/source/", web )); + Ok(()) }); } @@ -586,7 +602,9 @@ mod test { .version("0.0.1") .create() .unwrap(); + let web = env.frontend(); + assert!(!clipboard_is_present_for_path("/about", web)); assert!(!clipboard_is_present_for_path("/releases", web)); assert!(!clipboard_is_present_for_path("/", web)); @@ -594,7 +612,9 @@ mod test { "/fake_crate/0.0.1/fake_crate", web )); + assert!(!clipboard_is_present_for_path("/not/a/real/path", web)); + Ok(()) }); } @@ -605,6 +625,7 @@ mod test { let web = env.frontend(); for krate in &["std", "alloc", "core", "proc_macro", "test"] { let target = format!("https://doc.rust-lang.org/stable/{}/", krate); + // with or without slash assert_redirect(&format!("/{}", krate), &target, web)?; assert_redirect(&format!("/{}/", krate), &target, web)?; @@ -617,15 +638,23 @@ mod test { fn binary_docs_redirect_to_crate() { wrapper(|env| { let db = env.db(); - db.fake_release().name("bat").version("0.2.0").binary(true) - .create().unwrap(); + db.fake_release() + .name("bat") + .version("0.2.0") + .binary(true) + .create() + .unwrap(); + let web = env.frontend(); + assert_redirect("/bat/0.2.0", "/crate/bat/0.2.0", web)?; assert_redirect("/bat/0.2.0/i686-unknown-linux-gnu", "/crate/bat/0.2.0", web)?; + /* TODO: this should work (https://github.com/rust-lang/docs.rs/issues/603) assert_redirect("/bat/0.2.0/i686-unknown-linux-gnu/bat", "/crate/bat/0.2.0", web)?; assert_redirect("/bat/0.2.0/i686-unknown-linux-gnu/bat/", "/crate/bat/0.2.0/", web)?; */ + Ok(()) }) } @@ -634,14 +663,19 @@ mod test { fn can_view_source() { wrapper(|env| { let db = env.db(); - db.fake_release().name("regex").version("0.3.0") - .source_file("src/main.rs", br#"println!("definitely valid rust")"#) - .create().unwrap(); + db.fake_release() + .name("regex") + .version("0.3.0") + .source_file("src/main.rs", br#"println!("definitely valid rust")"#) + .create() + .unwrap(); + let web = env.frontend(); assert_success("/crate/regex/0.3.0/source/src/main.rs", web)?; assert_success("/crate/regex/0.3.0/source", web)?; assert_success("/crate/regex/0.3.0/source/src", web)?; assert_success("/regex/0.3.0/src/regex/main.rs", web)?; + Ok(()) }) } @@ -663,8 +697,10 @@ mod test { release("0.3.0"); let three = semver("0.3.0"); assert_eq!(version(None), three); + // same thing but with "*" assert_eq!(version(Some("*")), three); + // make sure exact matches still work assert_eq!(version(Some("0.3.0")), exact("0.3.0")); @@ -680,6 +716,7 @@ mod test { let release_id = release("0.3.0", db); let query = "UPDATE releases SET yanked = true WHERE id = $1 AND version = '0.3.0'"; + db.conn().query(query, &[&release_id]).unwrap(); assert_eq!(version(None, db), None); assert_eq!(version(Some("0.3"), db), None); diff --git a/src/web/page.rs b/src/web/page.rs index 983d96c46..f0b515eac 100644 --- a/src/web/page.rs +++ b/src/web/page.rs @@ -62,7 +62,7 @@ impl Page { pub fn new(content: T) -> Page { Page { title: None, - content: content, + content, status: status::Ok, varss: BTreeMap::new(), varsb: BTreeMap::new(), @@ -73,28 +73,28 @@ impl Page { /// Sets a string variable pub fn set(mut self, var: &str, val: &str) -> Page { - &self.varss.insert(var.to_owned(), val.to_owned()); + self.varss.insert(var.to_owned(), val.to_owned()); self } /// Sets a boolean variable pub fn set_bool(mut self, var: &str, val: bool) -> Page { - &self.varsb.insert(var.to_owned(), val); + self.varsb.insert(var.to_owned(), val); self } /// Sets a boolean variable to true pub fn set_true(mut self, var: &str) -> Page { - &self.varsb.insert(var.to_owned(), true); + self.varsb.insert(var.to_owned(), true); self } /// Sets an integer variable pub fn set_int(mut self, var: &str, val: i64) -> Page { - &self.varsi.insert(var.to_owned(), val); + self.varsi.insert(var.to_owned(), val); self } @@ -118,6 +118,7 @@ impl Page { let status = self.status; let temp = Template::new(template, self); resp.set_mut(temp).set_mut(status); + Ok(resp) } } @@ -139,8 +140,14 @@ impl ToJson for Page { tree.insert("content".to_owned(), self.content.to_json()); tree.insert("rustc_resource_suffix".to_owned(), self.rustc_resource_suffix.to_json()); tree.insert("cratesfyi_version".to_owned(), crate::BUILD_VERSION.to_json()); - tree.insert("cratesfyi_version_safe".to_owned(), - crate::BUILD_VERSION.replace(" ", "-").replace("(", "").replace(")", "").to_json()); + tree.insert( + "cratesfyi_version_safe".to_owned(), + crate::BUILD_VERSION + .replace(" ", "-") + .replace("(", "") + .replace(")", "") + .to_json() + ); tree.insert("varss".to_owned(), self.varss.to_json()); tree.insert("varsb".to_owned(), self.varsb.to_json()); tree.insert("varsi".to_owned(), self.varsi.to_json()); diff --git a/src/web/releases.rs b/src/web/releases.rs index 7d8124355..71323271c 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -398,7 +398,7 @@ pub fn author_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool).get(); let author = ctry!(router.find("author") - .ok_or(IronError::new(Nope::CrateNotFound, status::NotFound))); + .ok_or_else(|| IronError::new(Nope::CrateNotFound, status::NotFound))); let (author_name, packages) = if author.starts_with('@') { let mut author = author.clone().split('@'); @@ -522,7 +522,7 @@ pub fn search_handler(req: &mut Request) -> IronResult { let search_query = query.replace(" ", " & "); get_search_results(&conn, &search_query, 1, RELEASES_IN_RELEASES) - .ok_or(IronError::new(Nope::NoResults, status::NotFound)) + .ok_or_else(|| IronError::new(Nope::NoResults, status::NotFound)) .and_then(|(_, results)| { // FIXME: There is no pagination Page::new(results) diff --git a/src/web/routes.rs b/src/web/routes.rs index b3dec0d49..25727ab2f 100644 --- a/src/web/routes.rs +++ b/src/web/routes.rs @@ -1,11 +1,12 @@ +use crate::web::{INDEX_JS, MENU_JS}; use iron::middleware::Handler; +use iron::Request; use router::Router; use std::collections::HashSet; -use crate::web::{MENU_JS, INDEX_JS}; -use iron::Request; const DOC_RUST_LANG_ORG_REDIRECTS: &[&str] = &["alloc", "core", "proc_macro", "std", "test"]; +// REFACTOR: Break this into smaller initialization functions pub(super) fn build_routes() -> Routes { let mut routes = Routes::new(); @@ -197,7 +198,7 @@ impl Routes { if !pattern.ends_with('/') { let pattern = format!("{}/", pattern); self.get.push(( - pattern.to_string(), + pattern, Box::new(SimpleRedirect::new(|url| { url.set_path(&url.path().trim_end_matches('/').to_string()) })), diff --git a/src/web/source.rs b/src/web/source.rs index 44eaf13d2..47df9f4df 100644 --- a/src/web/source.rs +++ b/src/web/source.rs @@ -1,6 +1,5 @@ //! Source code browser - use std::collections::BTreeMap; use std::cmp::Ordering; use super::MetaData; @@ -12,7 +11,6 @@ use router::Router; use rustc_serialize::json::{Json, ToJson}; use postgres::Connection; - #[derive(PartialEq, PartialOrd)] enum FileType { Dir, @@ -21,20 +19,17 @@ enum FileType { RustSource, } - #[derive(PartialEq, PartialOrd)] struct File { name: String, file_type: FileType, } - struct FileList { metadata: MetaData, files: Vec, } - impl ToJson for FileList { fn to_json(&self) -> Json { let mut m: BTreeMap = BTreeMap::new(); @@ -47,13 +42,14 @@ impl ToJson for FileList { let mut file_m: BTreeMap = BTreeMap::new(); file_m.insert("name".to_string(), file.name.to_json()); - file_m.insert(match file.file_type { - FileType::Dir => "file_type_dir".to_string(), - FileType::Text => "file_type_text".to_string(), - FileType::Binary => "file_type_binary".to_string(), - FileType::RustSource => "file_type_rust_source".to_string(), - }, - true.to_json()); + let file_type = match file.file_type { + FileType::Dir => "file_type_dir", + FileType::Text => "file_type_text", + FileType::Binary => "file_type_binary", + FileType::RustSource => "file_type_rust_source", + }; + + file_m.insert(file_type.to_string(), true.to_json()); file_vec.push(file_m.to_json()); } @@ -63,7 +59,6 @@ impl ToJson for FileList { } } - impl FileList { /// Gets FileList from a request path /// @@ -82,12 +77,12 @@ impl FileList { /// This function is only returning FileList for requested directory. If is empty, /// it will return list of files (and dirs) for root directory. req_path must be a /// directory or empty for root directory. - fn from_path(conn: &Connection, - name: &str, - version: &str, - req_path: &str) - -> Option { - + fn from_path( + conn: &Connection, + name: &str, + version: &str, + req_path: &str + ) -> Option { let rows = conn.query("SELECT crates.name, releases.version, releases.description, @@ -101,7 +96,7 @@ impl FileList { &[&name, &version]) .unwrap(); - if rows.len() == 0 { + if rows.is_empty() { return None; } @@ -109,22 +104,22 @@ impl FileList { let mut file_list: Vec = Vec::new(); - files.as_array().map(|files| { + if let Some(files) = files.as_array() { for file in files { - file.as_array().map(|file| { + if let Some(file) = file.as_array() { let mime = file[0].as_string().unwrap(); let path = file[1].as_string().unwrap(); // skip .cargo-ok generated by cargo if path == ".cargo-ok" { - return; + continue; } // 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(); + let path_splited: Vec<&str> = path.split('/').collect(); // if path have '/' it is a directory let ftype = if path_splited.len() > 1 { @@ -148,9 +143,9 @@ impl FileList { file_list.push(file); } } - }); + } } - }); + } if file_list.is_empty() { return None; @@ -198,11 +193,11 @@ pub fn source_browser_handler(req: &mut Request) -> IronResult { // FileList::from_path is only working for directories // remove file name if it's not a directory - req_path.last_mut().map(|last| { + if let Some(last) = req_path.last_mut() { if !last.is_empty() { *last = ""; } - }); + } // remove crate name and version from req_path let path = req_path.join("/").replace(&format!("{}/{}/", name, version), ""); @@ -210,18 +205,16 @@ pub fn source_browser_handler(req: &mut Request) -> IronResult { (path, file_path) }; - let conn = extension!(req, Pool).get(); // try to get actual file first // skip if request is a directory - let file = if !file_path.ends_with("/") { + let file = if !file_path.ends_with('/') { DbFile::from_path(&conn, &file_path) } else { None }; - let (content, is_rust_source) = if let Some(file) = file { // serve the file with DatabaseFileHandler if file isn't text and not empty if !file.0.mime.starts_with("text") && !file.is_empty() { From 22b0336e52f9c0c17e834c17225b1f1d00b987a5 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 12:20:59 -0500 Subject: [PATCH 03/14] Revert Josh's heart attack --- src/bin/cratesfyi.rs | 353 +++++++++++++++++-------------------- src/docbuilder/metadata.rs | 214 ++++++---------------- 2 files changed, 218 insertions(+), 349 deletions(-) diff --git a/src/bin/cratesfyi.rs b/src/bin/cratesfyi.rs index c26498840..df186b179 100644 --- a/src/bin/cratesfyi.rs +++ b/src/bin/cratesfyi.rs @@ -1,204 +1,17 @@ use std::env; use std::path::{Path, PathBuf}; -use clap::{App, AppSettings, Arg, SubCommand}; -use cratesfyi::db::{add_path_into_database, connect_db}; +use clap::{Arg, App, AppSettings, SubCommand}; +use cratesfyi::{DocBuilder, RustwideBuilder, DocBuilderOptions, db}; use cratesfyi::utils::add_crate_to_queue; use cratesfyi::Server; -use cratesfyi::{db, DocBuilder, DocBuilderOptions, RustwideBuilder}; +use cratesfyi::db::{add_path_into_database, connect_db}; + pub fn main() { logger_init(); - let matches = build_cli().get_matches(); - - // REFACTOR: Break this monster up unto smaller functions, and maybe document it a bit - if let Some(matches) = matches.subcommand_matches("build") { - let docbuilder_opts = { - let mut docbuilder_opts = if let Some(prefix) = matches.value_of("PREFIX") { - DocBuilderOptions::from_prefix(PathBuf::from(prefix)) - } else if let Ok(prefix) = env::var("CRATESFYI_PREFIX") { - DocBuilderOptions::from_prefix(PathBuf::from(prefix)) - } else { - DocBuilderOptions::default() - }; - - if let Some(crates_io_index_path) = matches.value_of("CRATES_IO_INDEX_PATH") { - docbuilder_opts.crates_io_index_path = PathBuf::from(crates_io_index_path); - } - - docbuilder_opts.skip_if_exists = matches.is_present("SKIP_IF_EXISTS"); - docbuilder_opts.skip_if_log_exists = matches.is_present("SKIP_IF_LOG_EXISTS"); - docbuilder_opts.keep_build_directory = matches.is_present("KEEP_BUILD_DIRECTORY"); - - docbuilder_opts.check_paths().unwrap(); - - docbuilder_opts - }; - - let mut docbuilder = DocBuilder::new(docbuilder_opts); - - if matches.subcommand_matches("world").is_some() { - docbuilder.load_cache().expect("Failed to load cache"); - let mut builder = RustwideBuilder::init().unwrap(); - - builder - .build_world(&mut docbuilder) - .expect("Failed to build world"); - docbuilder.save_cache().expect("Failed to save cache"); - } else if let Some(matches) = matches.subcommand_matches("crate") { - docbuilder.load_cache().expect("Failed to load cache"); - let mut builder = RustwideBuilder::init().expect("failed to initialize rustwide"); - - match matches.value_of("LOCAL") { - Some(path) => builder.build_local_package(&mut docbuilder, Path::new(path)), - None => builder.build_package( - &mut docbuilder, - matches.value_of("CRATE_NAME").unwrap(), - matches.value_of("CRATE_VERSION").unwrap(), - None, - ), - } - .expect("Building documentation failed"); - - docbuilder.save_cache().expect("Failed to save cache"); - } else if let Some(m) = matches.subcommand_matches("update-toolchain") { - if m.is_present("ONLY_FIRST_TIME") { - let conn = db::connect_db().unwrap(); - let res = conn - .query("SELECT * FROM config WHERE name = 'rustc_version';", &[]) - .unwrap(); - if !res.is_empty() { - println!("update-toolchain was already called in the past, exiting"); - return; - } - } - - let mut builder = RustwideBuilder::init().unwrap(); - builder - .update_toolchain() - .expect("failed to update toolchain"); - } else if matches.subcommand_matches("add-essential-files").is_some() { - let mut builder = RustwideBuilder::init().unwrap(); - builder - .add_essential_files() - .expect("failed to add essential files"); - } else if matches.subcommand_matches("lock").is_some() { - docbuilder.lock().expect("Failed to lock"); - } else if matches.subcommand_matches("unlock").is_some() { - docbuilder.unlock().expect("Failed to unlock"); - } else if matches.subcommand_matches("print-options").is_some() { - println!("{:?}", docbuilder.options()); - } - } else if let Some(matches) = matches.subcommand_matches("database") { - if let Some(matches) = matches.subcommand_matches("migrate") { - let version = matches - .value_of("VERSION") - .map(|v| v.parse::().expect("Version should be an integer")); - db::migrate( - version, - &connect_db().expect("failed to connect to the database"), - ) - .expect("Failed to run database migrations"); - } else if matches.subcommand_matches("update-github-fields").is_some() { - cratesfyi::utils::github_updater().expect("Failed to update github fields"); - } else if let Some(matches) = matches.subcommand_matches("add-directory") { - add_path_into_database( - &db::connect_db().unwrap(), - matches.value_of("PREFIX").unwrap_or(""), - matches.value_of("DIRECTORY").unwrap(), - ) - .expect("Failed to add directory into database"); - } else if matches - .subcommand_matches("update-release-activity") - .is_some() - { - // FIXME: This is actually util command not database - cratesfyi::utils::update_release_activity().expect("Failed to update release activity"); - } else if matches.subcommand_matches("update-search-index").is_some() { - let conn = db::connect_db().unwrap(); - db::update_search_index(&conn).expect("Failed to update search index"); - } else if matches.subcommand_matches("move-to-s3").is_some() { - let conn = db::connect_db().unwrap(); - let mut count = 1; - let mut total = 0; - while count != 0 { - count = db::move_to_s3(&conn, 5_000).expect("Failed to upload batch to S3"); - total += count; - eprintln!( - "moved {} rows to s3 in this batch, total moved so far: {}", - count, total - ); - } - } else if let Some(matches) = matches.subcommand_matches("delete-crate") { - let name = matches.value_of("CRATE_NAME").expect("missing crate name"); - let conn = db::connect_db().expect("failed to connect to the database"); - db::delete_crate(&conn, &name).expect("failed to delete the crate"); - } else if let Some(matches) = matches.subcommand_matches("blacklist") { - let conn = db::connect_db().expect("failed to connect to the database"); - - if matches.subcommand_matches("list").is_some() { - let crates = - db::blacklist::list_crates(&conn).expect("failed to list crates on blacklist"); - println!("{}", crates.join("\n")); - } else if let Some(matches) = matches.subcommand_matches("add") { - let crate_name = matches.value_of("CRATE_NAME").expect("verified by clap"); - db::blacklist::add_crate(&conn, crate_name) - .expect("failed to add crate to blacklist"); - } else if let Some(matches) = matches.subcommand_matches("remove") { - let crate_name = matches.value_of("CRATE_NAME").expect("verified by clap"); - db::blacklist::remove_crate(&conn, crate_name) - .expect("failed to remove crate from blacklist"); - } - } - } else if let Some(matches) = matches.subcommand_matches("start-web-server") { - Server::start(Some( - matches.value_of("SOCKET_ADDR").unwrap_or("0.0.0.0:3000"), - )); - } else if matches.subcommand_matches("daemon").is_some() { - let foreground = matches - .subcommand_matches("daemon") - .map_or(false, |opts| opts.is_present("FOREGROUND")); - cratesfyi::utils::start_daemon(!foreground); - } else if let Some(matches) = matches.subcommand_matches("queue") { - if let Some(matches) = matches.subcommand_matches("add") { - let priority = matches.value_of("BUILD_PRIORITY").unwrap_or("5"); - let priority: i32 = priority.parse().expect("--priority was not a number"); - let conn = connect_db().expect("Could not connect to database"); - - add_crate_to_queue( - &conn, - matches.value_of("CRATE_NAME").unwrap(), - matches.value_of("CRATE_VERSION").unwrap(), - priority, - ) - .expect("Could not add crate to queue"); - } - } -} - -fn logger_init() { - use std::io::Write; - - let mut builder = env_logger::Builder::new(); - builder.format(|buf, record| { - writeln!( - buf, - "{} [{}] {}: {}", - time::now().strftime("%Y/%m/%d %H:%M:%S").unwrap(), - record.level(), - record.target(), - record.args() - ) - }); - builder.parse_filters(&env::var("RUST_LOG").unwrap_or_else(|_| "cratesfyi=info".to_owned())); - - rustwide::logging::init_with(builder.build()); -} - -/// Creates the command line interface to cratesfyi -fn build_cli<'a, 'b>() -> App<'a, 'b> { - App::new("cratesfyi") + let matches = App::new("cratesfyi") .version(cratesfyi::BUILD_VERSION) .about(env!("CARGO_PKG_DESCRIPTION")) .setting(AppSettings::ArgRequiredElseHelp) @@ -327,4 +140,160 @@ fn build_cli<'a, 'b>() -> App<'a, 'b> { .long("priority") .help("Priority of build (default: 5) (new crate builds get priority 0)") .takes_value(true)))) + .get_matches(); + + + + if let Some(matches) = matches.subcommand_matches("build") { + let docbuilder_opts = { + let mut docbuilder_opts = if let Some(prefix) = matches.value_of("PREFIX") { + DocBuilderOptions::from_prefix(PathBuf::from(prefix)) + } else if let Ok(prefix) = env::var("CRATESFYI_PREFIX") { + DocBuilderOptions::from_prefix(PathBuf::from(prefix)) + } else { + DocBuilderOptions::default() + }; + + if let Some(crates_io_index_path) = matches.value_of("CRATES_IO_INDEX_PATH") { + docbuilder_opts.crates_io_index_path = PathBuf::from(crates_io_index_path); + } + + docbuilder_opts.skip_if_exists = matches.is_present("SKIP_IF_EXISTS"); + docbuilder_opts.skip_if_log_exists = matches.is_present("SKIP_IF_LOG_EXISTS"); + docbuilder_opts.keep_build_directory = matches.is_present("KEEP_BUILD_DIRECTORY"); + + docbuilder_opts.check_paths().unwrap(); + + docbuilder_opts + }; + + let mut docbuilder = DocBuilder::new(docbuilder_opts); + + if let Some(_) = matches.subcommand_matches("world") { + docbuilder.load_cache().expect("Failed to load cache"); + let mut builder = RustwideBuilder::init().unwrap(); + builder.build_world(&mut docbuilder).expect("Failed to build world"); + docbuilder.save_cache().expect("Failed to save cache"); + } else if let Some(matches) = matches.subcommand_matches("crate") { + docbuilder.load_cache().expect("Failed to load cache"); + let mut builder = RustwideBuilder::init().expect("failed to initialize rustwide"); + match matches.value_of("LOCAL") { + Some(path) => builder.build_local_package(&mut docbuilder, Path::new(path)), + None => builder.build_package(&mut docbuilder, + matches.value_of("CRATE_NAME").unwrap(), matches.value_of("CRATE_VERSION").unwrap(), None), + }.expect("Building documentation failed"); + docbuilder.save_cache().expect("Failed to save cache"); + } else if let Some(m) = matches.subcommand_matches("update-toolchain") { + if m.is_present("ONLY_FIRST_TIME") { + let conn = db::connect_db().unwrap(); + let res = conn.query("SELECT * FROM config WHERE name = 'rustc_version';", &[]).unwrap(); + if !res.is_empty() { + println!("update-toolchain was already called in the past, exiting"); + return; + } + } + let mut builder = RustwideBuilder::init().unwrap(); + builder.update_toolchain().expect("failed to update toolchain"); + } else if matches.subcommand_matches("add-essential-files").is_some() { + let mut builder = RustwideBuilder::init().unwrap(); + builder.add_essential_files().expect("failed to add essential files"); + } else if let Some(_) = matches.subcommand_matches("lock") { + docbuilder.lock().expect("Failed to lock"); + } else if let Some(_) = matches.subcommand_matches("unlock") { + docbuilder.unlock().expect("Failed to unlock"); + } else if let Some(_) = matches.subcommand_matches("print-options") { + println!("{:?}", docbuilder.options()); + } + + } else if let Some(matches) = matches.subcommand_matches("database") { + if let Some(matches) = matches.subcommand_matches("migrate") { + let version = matches.value_of("VERSION").map(|v| v.parse::() + .expect("Version should be an integer")); + db::migrate(version, &connect_db().expect("failed to connect to the database")) + .expect("Failed to run database migrations"); + } else if let Some(_) = matches.subcommand_matches("update-github-fields") { + cratesfyi::utils::github_updater().expect("Failed to update github fields"); + } else if let Some(matches) = matches.subcommand_matches("add-directory") { + add_path_into_database(&db::connect_db().unwrap(), + matches.value_of("PREFIX").unwrap_or(""), + matches.value_of("DIRECTORY").unwrap()) + .expect("Failed to add directory into database"); + } else if let Some(_) = matches.subcommand_matches("update-release-activity") { + // FIXME: This is actually util command not database + cratesfyi::utils::update_release_activity().expect("Failed to update release activity"); + } else if let Some(_) = matches.subcommand_matches("update-search-index") { + let conn = db::connect_db().unwrap(); + db::update_search_index(&conn).expect("Failed to update search index"); + } else if let Some(_) = matches.subcommand_matches("move-to-s3") { + let conn = db::connect_db().unwrap(); + let mut count = 1; + let mut total = 0; + while count != 0 { + count = db::move_to_s3(&conn, 5_000).expect("Failed to upload batch to S3"); + total += count; + eprintln!( + "moved {} rows to s3 in this batch, total moved so far: {}", + count, total + ); + } + } else if let Some(matches) = matches.subcommand_matches("delete-crate") { + let name = matches.value_of("CRATE_NAME").expect("missing crate name"); + let conn = db::connect_db().expect("failed to connect to the database"); + db::delete_crate(&conn, &name).expect("failed to delete the crate"); + } else if let Some(matches) = matches.subcommand_matches("blacklist") { + let conn = db::connect_db().expect("failed to connect to the database"); + + if let Some(_) = matches.subcommand_matches("list") { + let crates = db::blacklist::list_crates(&conn).expect("failed to list crates on blacklist"); + println!("{}", crates.join("\n")); + + } else if let Some(matches) = matches.subcommand_matches("add") { + let crate_name = matches.value_of("CRATE_NAME").expect("verified by clap"); + db::blacklist::add_crate(&conn, crate_name).expect("failed to add crate to blacklist"); + + } else if let Some(matches) = matches.subcommand_matches("remove") { + let crate_name = matches.value_of("CRATE_NAME").expect("verified by clap"); + db::blacklist::remove_crate(&conn, crate_name).expect("failed to remove crate from blacklist"); + } + } + + } else if let Some(matches) = matches.subcommand_matches("start-web-server") { + Server::start(Some(matches.value_of("SOCKET_ADDR").unwrap_or("0.0.0.0:3000"))); + + } else if let Some(_) = matches.subcommand_matches("daemon") { + let foreground = matches.subcommand_matches("daemon") + .map_or(false, |opts| opts.is_present("FOREGROUND")); + cratesfyi::utils::start_daemon(!foreground); + + } else if let Some(matches) = matches.subcommand_matches("queue") { + if let Some(matches) = matches.subcommand_matches("add") { + let priority = matches.value_of("BUILD_PRIORITY").unwrap_or("5"); + let priority: i32 = priority.parse().expect("--priority was not a number"); + let conn = connect_db().expect("Could not connect to database"); + + add_crate_to_queue(&conn, + matches.value_of("CRATE_NAME").unwrap(), + matches.value_of("CRATE_VERSION").unwrap(), + priority).expect("Could not add crate to queue"); + } + } +} + + + +fn logger_init() { + use std::io::Write; + + let mut builder = env_logger::Builder::new(); + builder.format(|buf, record| { + writeln!(buf, "{} [{}] {}: {}", + time::now().strftime("%Y/%m/%d %H:%M:%S").unwrap(), + record.level(), + record.target(), + record.args()) + }); + builder.parse_filters(&env::var("RUST_LOG") + .unwrap_or("cratesfyi=info".to_owned())); + + rustwide::logging::init_with(builder.build()); } diff --git a/src/docbuilder/metadata.rs b/src/docbuilder/metadata.rs index e42123346..181168676 100644 --- a/src/docbuilder/metadata.rs +++ b/src/docbuilder/metadata.rs @@ -1,8 +1,8 @@ -use crate::error::Result; -use failure::err_msg; use std::collections::HashSet; use std::path::Path; -use toml::{map::Map, Value}; +use toml::Value; +use crate::error::Result; +use failure::err_msg; /// Metadata for custom builds /// @@ -83,27 +83,24 @@ impl Metadata { return Ok(Metadata::from_manifest(manifest_path)); } } - Err(err_msg("Manifest not found")) } fn from_manifest>(path: P) -> Metadata { - use std::{fs::File, io::Read}; - - let mut file = if let Ok(file) = File::open(path) { - file - } else { - return Metadata::default(); + use std::fs::File; + use std::io::Read; + let mut f = match File::open(path) { + Ok(f) => f, + Err(_) => return Metadata::default(), }; - - let mut meta = String::new(); - if file.read_to_string(&mut meta).is_err() { + let mut s = String::new(); + if let Err(_) = f.read_to_string(&mut s) { return Metadata::default(); } - - Metadata::from_str(&meta) + Metadata::from_str(&s) } + // This is similar to Default trait but it's private fn default() -> Metadata { Metadata { @@ -117,98 +114,56 @@ impl Metadata { } } + fn from_str(manifest: &str) -> Metadata { let mut metadata = Metadata::default(); - let manifest = if let Ok(manifest) = manifest.parse::() { - manifest - } else { - return metadata; + let manifest = match manifest.parse::() { + Ok(m) => m, + Err(_) => return metadata, }; - if let Some(table) = fetch_manifest_tables(&manifest) { - let collect_into_array = - |f: &Vec| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect(); - - metadata.features = table - .get("features") - .and_then(|f| f.as_array()) - .and_then(collect_into_array); - - metadata.no_default_features = table - .get("no-default-features") - .and_then(|v| v.as_bool()) - .unwrap_or(metadata.no_default_features); - - metadata.all_features = table - .get("all-features") - .and_then(|v| v.as_bool()) - .unwrap_or(metadata.all_features); - - metadata.default_target = table - .get("default-target") - .and_then(|v| v.as_str()) - .map(|v| v.to_owned()); - - metadata.targets = table - .get("targets") - .and_then(|f| f.as_array()) - .and_then(collect_into_array); - - metadata.rustc_args = table - .get("rustc-args") - .and_then(|f| f.as_array()) - .and_then(collect_into_array); - - metadata.rustdoc_args = table - .get("rustdoc-args") - .and_then(|f| f.as_array()) - .and_then(collect_into_array); - } + if let Some(table) = manifest.get("package").and_then(|p| p.as_table()) + .and_then(|p| p.get("metadata")).and_then(|p| p.as_table()) + .and_then(|p| p.get("docs")).and_then(|p| p.as_table()) + .and_then(|p| p.get("rs")).and_then(|p| p.as_table()) { + metadata.features = table.get("features").and_then(|f| f.as_array()) + .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); + metadata.no_default_features = table.get("no-default-features") + .and_then(|v| v.as_bool()).unwrap_or(metadata.no_default_features); + metadata.all_features = table.get("all-features") + .and_then(|v| v.as_bool()).unwrap_or(metadata.all_features); + metadata.default_target = table.get("default-target") + .and_then(|v| v.as_str()).map(|v| v.to_owned()); + metadata.targets = table.get("targets").and_then(|f| f.as_array()) + .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); + metadata.rustc_args = table.get("rustc-args").and_then(|f| f.as_array()) + .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); + metadata.rustdoc_args = table.get("rustdoc-args").and_then(|f| f.as_array()) + .and_then(|f| f.iter().map(|v| v.as_str().map(|v| v.to_owned())).collect()); + } metadata } - pub(super) fn targets(&self) -> BuildTargets<'_> { use super::rustwide_builder::{HOST_TARGET, TARGETS}; - let default_target = self - .default_target - .as_deref() + let default_target = self.default_target.as_deref() // Use the first element of `targets` if `default_target` is unset and `targets` is non-empty - .or_else(|| { - self.targets - .as_ref() - .and_then(|targets| targets.iter().next().map(String::as_str)) - }) + .or_else(|| self.targets.as_ref().and_then(|targets| targets.iter().next().map(String::as_str))) .unwrap_or(HOST_TARGET); // Let people opt-in to only having specific targets - let mut targets: HashSet<_> = self - .targets - .as_ref() + let mut targets: HashSet<_> = self.targets.as_ref() .map(|targets| targets.iter().map(String::as_str).collect()) .unwrap_or_else(|| TARGETS.iter().copied().collect()); targets.remove(&default_target); - BuildTargets { - default_target, - other_targets: targets, - } + BuildTargets { default_target, other_targets: targets } } } -fn fetch_manifest_tables<'a>(manifest: &'a Value) -> Option<&'a Map> { - manifest - .get("package")? - .as_table()? - .get("metadata")? - .as_table()? - .get("docs")? - .as_table()? - .get("rs")? - .as_table() -} + #[cfg(test)] mod test { @@ -244,10 +199,7 @@ mod test { assert_eq!(features[0], "feature1".to_owned()); assert_eq!(features[1], "feature2".to_owned()); - assert_eq!( - metadata.default_target.unwrap(), - "x86_64-unknown-linux-gnu".to_owned() - ); + assert_eq!(metadata.default_target.unwrap(), "x86_64-unknown-linux-gnu".to_owned()); let targets = metadata.targets.expect("should have explicit target"); assert_eq!(targets.len(), 2); @@ -277,42 +229,32 @@ mod test { assert!(metadata.targets.is_none()); // no package.metadata.docs.rs section - let metadata = Metadata::from_str( - r#" + let metadata = Metadata::from_str(r#" [package] name = "test" - "#, - ); + "#); assert!(metadata.targets.is_none()); // targets explicitly set to empty array - let metadata = Metadata::from_str( - r#" + let metadata = Metadata::from_str(r#" [package.metadata.docs.rs] targets = [] - "#, - ); + "#); assert!(metadata.targets.unwrap().is_empty()); } #[test] fn test_select_targets() { - use super::BuildTargets; use crate::docbuilder::rustwide_builder::{HOST_TARGET, TARGETS}; + use super::BuildTargets; let mut metadata = Metadata::default(); - // unchanged default_target, targets not specified - let BuildTargets { - default_target: default, - other_targets: tier_one, - } = metadata.targets(); + let BuildTargets { default_target: default, other_targets: tier_one } = metadata.targets(); assert_eq!(default, HOST_TARGET); - // should be equal to TARGETS \ {HOST_TARGET} for actual in &tier_one { assert!(TARGETS.contains(actual)); } - for expected in TARGETS { if *expected == HOST_TARGET { assert!(!tier_one.contains(&HOST_TARGET)); @@ -323,89 +265,47 @@ mod test { // unchanged default_target, targets specified to be empty metadata.targets = Some(Vec::new()); - - let BuildTargets { - default_target: default, - other_targets: others, - } = metadata.targets(); - + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // unchanged default_target, targets non-empty - metadata.targets = Some(vec![ - "i686-pc-windows-msvc".into(), - "i686-apple-darwin".into(), - ]); - - let BuildTargets { - default_target: default, - other_targets: others, - } = metadata.targets(); - + metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-apple-darwin".into()]); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert_eq!(others.len(), 1); assert!(others.contains(&"i686-apple-darwin")); // make sure that default_target is not built twice metadata.targets = Some(vec![HOST_TARGET.into()]); - let BuildTargets { - default_target: default, - other_targets: others, - } = metadata.targets(); - + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, HOST_TARGET); assert!(others.is_empty()); // make sure that duplicates are removed - metadata.targets = Some(vec![ - "i686-pc-windows-msvc".into(), - "i686-pc-windows-msvc".into(), - ]); - - let BuildTargets { - default_target: default, - other_targets: others, - } = metadata.targets(); - + metadata.targets = Some(vec!["i686-pc-windows-msvc".into(), "i686-pc-windows-msvc".into()]); + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-pc-windows-msvc"); assert!(others.is_empty()); // make sure that `default_target` always takes priority over `targets` metadata.default_target = Some("i686-apple-darwin".into()); - let BuildTargets { - default_target: default, - other_targets: others, - } = metadata.targets(); - + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-apple-darwin"); assert_eq!(others.len(), 1); assert!(others.contains(&"i686-pc-windows-msvc")); // make sure that `default_target` takes priority over `HOST_TARGET` metadata.targets = Some(vec![]); - let BuildTargets { - default_target: default, - other_targets: others, - } = metadata.targets(); - + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-apple-darwin"); assert!(others.is_empty()); // and if `targets` is unset, it should still be set to `TARGETS` metadata.targets = None; - let BuildTargets { - default_target: default, - other_targets: others, - } = metadata.targets(); - + let BuildTargets { default_target: default, other_targets: others } = metadata.targets(); assert_eq!(default, "i686-apple-darwin"); - let tier_one_targets_no_default = TARGETS - .iter() - .filter(|&&t| t != "i686-apple-darwin") - .copied() - .collect(); - + let tier_one_targets_no_default = TARGETS.iter().filter(|&&t| t != "i686-apple-darwin").copied().collect(); assert_eq!(others, tier_one_targets_no_default); } } From 73af62f9e43c49f2185cd40e0ec55aebc35a9840 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 12:24:05 -0500 Subject: [PATCH 04/14] Revert the rest of Josh's heart attack --- src/web/mod.rs | 191 ++++++++++++++++++++----------------------------- 1 file changed, 77 insertions(+), 114 deletions(-) diff --git a/src/web/mod.rs b/src/web/mod.rs index d35eb687e..31fcbf0d1 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -1,5 +1,6 @@ //! Web interface of cratesfyi + pub(crate) mod page; /// ctry! (cratesfyitry) is extremely similar to try! and itry! @@ -8,12 +9,10 @@ macro_rules! ctry { ($result:expr) => (match $result { Ok(v) => v, Err(e) => { - return $crate::web::page::Page::new(format!("{:?}", e)) - .title("An error has occured") - .set_status(::iron::status::BadRequest) - .to_resp("resp"); + return $crate::web::page::Page::new(format!("{:?}", e)).title("An error has occured") + .set_status(::iron::status::BadRequest).to_resp("resp"); } - }); + }) } /// cexpect will check an option and if it's not Some @@ -24,17 +23,16 @@ macro_rules! cexpect { None => { return $crate::web::page::Page::new("Resource not found".to_owned()) .title("An error has occured") - .set_status(::iron::status::BadRequest) - .to_resp("resp"); + .set_status(::iron::status::BadRequest).to_resp("resp"); } - }); + }) } /// Gets an extension from Request macro_rules! extension { ($req:expr, $ext:ty) => ( cexpect!($req.extensions.get::<$ext>()) - ); + ) } mod rustdoc; @@ -72,10 +70,10 @@ use std::sync::{Arc, Mutex}; /// Duration of static files for staticfile and DatabaseFileHandler (in seconds) const STATIC_FILE_CACHE_DURATION: u64 = 60 * 60 * 24 * 30 * 12; // 12 months -const STYLE_CSS: &str = include_str!(concat!(env!("OUT_DIR"), "/style.css")); -const MENU_JS: &str = include_str!(concat!(env!("OUT_DIR"), "/menu.js")); -const INDEX_JS: &str = include_str!(concat!(env!("OUT_DIR"), "/index.js")); -const OPENSEARCH_XML: & [u8] = include_bytes!("opensearch.xml"); +const STYLE_CSS: &'static str = include_str!(concat!(env!("OUT_DIR"), "/style.css")); +const MENU_JS: &'static str = include_str!(concat!(env!("OUT_DIR"), "/menu.js")); +const INDEX_JS: &'static str = include_str!(concat!(env!("OUT_DIR"), "/index.js")); +const OPENSEARCH_XML: &'static [u8] = include_bytes!("opensearch.xml"); const DEFAULT_BIND: &str = "0.0.0.0:3000"; @@ -90,6 +88,7 @@ struct CratesfyiHandler { pool_factory: PoolFactory, } + impl CratesfyiHandler { fn chain(pool_factory: &PoolFactoryFn, base: H) -> Chain { // TODO: Use DocBuilderOptions for paths @@ -113,11 +112,7 @@ impl CratesfyiHandler { let shared_resources = Self::chain(&pool_factory, rustdoc::SharedResourceHandler); let router_chain = Self::chain(&pool_factory, routes.iron_router()); - let prefix = PathBuf::from( - env::var("CRATESFYI_PREFIX") - .expect("the CRATESFYI_PREFIX environment variable is not set"), - ) - .join("public_html"); + let prefix = PathBuf::from(env::var("CRATESFYI_PREFIX").expect("the CRATESFYI_PREFIX environment variable is not set")).join("public_html"); let static_handler = Static::new(prefix) .cache(Duration::from_secs(STATIC_FILE_CACHE_DURATION)); @@ -225,61 +220,55 @@ impl MatchVersion { /// `version` may be an exact version number or loose semver version requirement. The return value /// will indicate whether the given version exactly matched a version number from the database. fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> MatchVersion { - use std::borrow::Cow; // version is an Option<&str> from router::Router::get // need to decode first use url::percent_encoding::percent_decode; - let req_version = version - .and_then(|v| { - percent_decode(v.as_bytes()) - .decode_utf8() - .ok() - .map(Cow::into_owned) + let req_version = version.and_then(|v| { + match percent_decode(v.as_bytes()).decode_utf8() { + Ok(p) => Some(p.into_owned()), + Err(_) => None, + } }) .map(|v| if v == "newest" || v == "latest" { "*".to_owned() } else { v }) - .unwrap_or_else(|| "*".to_string()); + .unwrap_or("*".to_string()); let versions: Vec<(String, i32)> = { let query = "SELECT version, releases.id FROM releases INNER JOIN crates ON releases.crate_id = crates.id WHERE name = $1 AND yanked = false"; let rows = conn.query(query, &[&name]).unwrap(); - - if rows.is_empty() { + if rows.len() == 0 { return MatchVersion::None; } - rows.iter().map(|row| (row.get(0), row.get(1))).collect() }; // first check for exact match // we can't expect users to use semver in query - if let Some(exact) = versions.iter().find(|(vers, _)| vers == &req_version) { - // REFACTOR: Get around the .clone() - return MatchVersion::Exact(exact.clone()); + for version in &versions { + if version.0 == req_version { + return MatchVersion::Exact(version.clone()); + } } // Now try to match with semver - let req_sem_ver = if let Ok(version) = VersionReq::parse(&req_version) { - version - } else { - return MatchVersion::None; + let req_sem_ver = match VersionReq::parse(&req_version) { + Ok(v) => v, + Err(_) => return MatchVersion::None, }; // we need to sort versions first let versions_sem = { - let mut versions_sem: Vec<(Version, i32)> = Vec::with_capacity(versions.len()); + let mut versions_sem: Vec<(Version, i32)> = Vec::new(); - for (vers, id) in &versions { + for version in &versions { // in theory a crate must always have a semver compatible version // but check result just in case - let version = if let Ok(v) = Version::parse(&vers) { - (v, *id) - } else { - return MatchVersion::None; + let version = match Version::parse(&version.0) { + Ok(v) => (v, version.1), + Err(_) => return MatchVersion::None, }; - versions_sem.push(version); } @@ -288,8 +277,10 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> MatchV versions_sem }; - if let Some((version, id)) = versions_sem.iter().find(|(vers, _)| req_sem_ver.matches(vers)) { - return MatchVersion::Semver((version.to_string(), *id)); + for version in &versions_sem { + if req_sem_ver.matches(&version.0) { + return MatchVersion::Semver((version.0.to_string(), version.1)); + } } // semver is acting weird for '*' (any) range if a crate only have pre-release versions @@ -301,6 +292,10 @@ fn match_version(conn: &Connection, name: &str, version: Option<&str>) -> MatchV MatchVersion::None } + + + + /// Wrapper around the Markdown parser and renderer to render markdown fn render_markdown(text: &str) -> String { use comrak::{markdown_to_html, ComrakOptions}; @@ -319,13 +314,15 @@ fn render_markdown(text: &str) -> String { markdown_to_html(text, &options) } + + pub struct Server { inner: Listening, } impl Server { pub fn start(addr: Option<&str>) -> Self { - let server = Self::start_inner(addr.unwrap_or(DEFAULT_BIND), Box::new(Pool::new)); + let server = Self::start_inner(addr.unwrap_or(DEFAULT_BIND), Box::new(|| Pool::new())); info!("Running docs.rs web server on http://{}", server.addr()); server } @@ -367,31 +364,33 @@ impl Server { } } + /// Converts Timespec to nice readable relative time string fn duration_to_str(ts: time::Timespec) -> String { + let tm = time::at(ts); let delta = time::now() - tm; - let delta = ( - delta.num_days(), - delta.num_hours(), - delta.num_minutes(), - delta.num_seconds(), - ); - - match delta { - (days, _, _, _) if days > 5 => format!("{}", tm.strftime("%b %d, %Y").unwrap()), - (days, _, _, _) if days > 1 => format!("{} days ago", days), - (days, _, _, _) if days == 1 => "one day ago".to_string(), - (_, hours, _, _) if hours > 1 => format!("{} hours ago", hours), - (_, hours, _, _) if hours == 1 => "an hour ago".to_string(), - - (_, _, minutes, _) if minutes > 1 => format!("{} minutes ago", minutes), - (_, _, minutes, _) if minutes == 1 => "one minute ago".to_string(), - - (_, _, _, seconds) if seconds > 0 => format!("{} seconds ago", seconds), - _ => "just now".to_string(), + if delta.num_days() > 5 { + format!("{}", tm.strftime("%b %d, %Y").unwrap()) + } else if delta.num_days() > 1 { + format!("{} days ago", delta.num_days()) + } else if delta.num_days() == 1 { + "one day ago".to_string() + } else if delta.num_hours() > 1 { + format!("{} hours ago", delta.num_hours()) + } else if delta.num_hours() == 1 { + "an hour ago".to_string() + } else if delta.num_minutes() > 1 { + format!("{} minutes ago", delta.num_minutes()) + } else if delta.num_minutes() == 1 { + "one minute ago".to_string() + } else if delta.num_seconds() > 0 { + format!("{} seconds ago", delta.num_seconds()) + } else { + "just now".to_string() } + } /// Creates a `Response` which redirects to the given path on the scheme/host/port from the given @@ -423,40 +422,28 @@ fn redirect_base(req: &Request) -> String { fn style_css_handler(_: &mut Request) -> IronResult { let mut response = Response::with((status::Ok, STYLE_CSS)); - let cache = vec![ - CacheDirective::Public, - CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32), - ]; - + let cache = vec![CacheDirective::Public, + CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32)]; response.headers.set(ContentType("text/css".parse().unwrap())); response.headers.set(CacheControl(cache)); - Ok(response) } fn load_js(file_path_str: &'static str) -> IronResult { let mut response = Response::with((status::Ok, file_path_str)); - let cache = vec![ - CacheDirective::Public, - CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32), - ]; - + let cache = vec![CacheDirective::Public, + CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32)]; response.headers.set(ContentType("application/javascript".parse().unwrap())); response.headers.set(CacheControl(cache)); - Ok(response) } fn opensearch_xml_handler(_: &mut Request) -> IronResult { let mut response = Response::with((status::Ok, OPENSEARCH_XML)); - let cache = vec![ - CacheDirective::Public, - CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32) - ]; - + let cache = vec![CacheDirective::Public, + CacheDirective::MaxAge(STATIC_FILE_CACHE_DURATION as u32)]; response.headers.set(ContentType("application/opensearchdescription+xml".parse().unwrap())); response.headers.set(CacheControl(cache)); - Ok(response) } @@ -485,6 +472,7 @@ pub(crate) struct MetaData { pub default_target: String, } + impl MetaData { fn from_crate(conn: &Connection, name: &str, version: &str) -> Option { let rows = conn.query( @@ -513,6 +501,7 @@ impl MetaData { } } + impl ToJson for MetaData { fn to_json(&self) -> Json { let mut m: BTreeMap = BTreeMap::new(); @@ -522,7 +511,6 @@ impl ToJson for MetaData { m.insert("target_name".to_owned(), self.target_name.to_json()); m.insert("rustdoc_status".to_owned(), self.rustdoc_status.to_json()); m.insert("default_target".to_owned(), self.default_target.to_json()); - m.to_json() } } @@ -539,7 +527,6 @@ mod test { fn release(version: &str, db: &TestDatabase) -> i32 { db.fake_release().name("foo").version(version).create().unwrap() } - fn version(v: Option<&str>, db: &TestDatabase) -> Option { match_version(&db.conn(), "foo", v).strip_id() } @@ -577,18 +564,15 @@ mod test { .source_file("test.rs", &[]) .create() .unwrap(); - let web = env.frontend(); assert!(clipboard_is_present_for_path( "/crate/fake_crate/0.0.1", web )); - assert!(clipboard_is_present_for_path( "/crate/fake_crate/0.0.1/source/", web )); - Ok(()) }); } @@ -602,9 +586,7 @@ mod test { .version("0.0.1") .create() .unwrap(); - let web = env.frontend(); - assert!(!clipboard_is_present_for_path("/about", web)); assert!(!clipboard_is_present_for_path("/releases", web)); assert!(!clipboard_is_present_for_path("/", web)); @@ -612,9 +594,7 @@ mod test { "/fake_crate/0.0.1/fake_crate", web )); - assert!(!clipboard_is_present_for_path("/not/a/real/path", web)); - Ok(()) }); } @@ -625,7 +605,6 @@ mod test { let web = env.frontend(); for krate in &["std", "alloc", "core", "proc_macro", "test"] { let target = format!("https://doc.rust-lang.org/stable/{}/", krate); - // with or without slash assert_redirect(&format!("/{}", krate), &target, web)?; assert_redirect(&format!("/{}/", krate), &target, web)?; @@ -638,23 +617,15 @@ mod test { fn binary_docs_redirect_to_crate() { wrapper(|env| { let db = env.db(); - db.fake_release() - .name("bat") - .version("0.2.0") - .binary(true) - .create() - .unwrap(); - + db.fake_release().name("bat").version("0.2.0").binary(true) + .create().unwrap(); let web = env.frontend(); - assert_redirect("/bat/0.2.0", "/crate/bat/0.2.0", web)?; assert_redirect("/bat/0.2.0/i686-unknown-linux-gnu", "/crate/bat/0.2.0", web)?; - /* TODO: this should work (https://github.com/rust-lang/docs.rs/issues/603) assert_redirect("/bat/0.2.0/i686-unknown-linux-gnu/bat", "/crate/bat/0.2.0", web)?; assert_redirect("/bat/0.2.0/i686-unknown-linux-gnu/bat/", "/crate/bat/0.2.0/", web)?; */ - Ok(()) }) } @@ -663,19 +634,14 @@ mod test { fn can_view_source() { wrapper(|env| { let db = env.db(); - db.fake_release() - .name("regex") - .version("0.3.0") - .source_file("src/main.rs", br#"println!("definitely valid rust")"#) - .create() - .unwrap(); - + db.fake_release().name("regex").version("0.3.0") + .source_file("src/main.rs", br#"println!("definitely valid rust")"#) + .create().unwrap(); let web = env.frontend(); assert_success("/crate/regex/0.3.0/source/src/main.rs", web)?; assert_success("/crate/regex/0.3.0/source", web)?; assert_success("/crate/regex/0.3.0/source/src", web)?; assert_success("/regex/0.3.0/src/regex/main.rs", web)?; - Ok(()) }) } @@ -697,10 +663,8 @@ mod test { release("0.3.0"); let three = semver("0.3.0"); assert_eq!(version(None), three); - // same thing but with "*" assert_eq!(version(Some("*")), three); - // make sure exact matches still work assert_eq!(version(Some("0.3.0")), exact("0.3.0")); @@ -716,7 +680,6 @@ mod test { let release_id = release("0.3.0", db); let query = "UPDATE releases SET yanked = true WHERE id = $1 AND version = '0.3.0'"; - db.conn().query(query, &[&release_id]).unwrap(); assert_eq!(version(None, db), None); assert_eq!(version(Some("0.3"), db), None); From 3a08105e0dbad850b53d09038f8e605ab30fb6e1 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 12:25:59 -0500 Subject: [PATCH 05/14] Add package is a monster --- src/db/add_package.rs | 186 +++++++++++------------------ src/docbuilder/rustwide_builder.rs | 2 +- src/test/fakes.rs | 2 +- 3 files changed, 73 insertions(+), 117 deletions(-) diff --git a/src/db/add_package.rs b/src/db/add_package.rs index 7e26e2815..0914f150d 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -25,19 +25,17 @@ use failure::err_msg; /// /// NOTE: `source_files` refers to the files originally in the crate, /// not the files generated by rustdoc. -// REFACTOR: This should be multiple smaller functions -pub(crate) fn add_package_into_database( - conn: &Connection, - metadata_pkg: &MetadataPackage, - source_dir: &Path, - res: &BuildResult, - default_target: &str, - source_files: Option, - doc_targets: Vec, - cratesio_data: &CratesIoData, - has_docs: bool, - has_examples: bool -) -> Result { +pub(crate) fn add_package_into_database(conn: &Connection, + metadata_pkg: &MetadataPackage, + source_dir: &Path, + res: &BuildResult, + default_target: &str, + source_files: Option, + doc_targets: Vec, + cratesio_data: &CratesIoData, + has_docs: bool, + has_examples: bool) + -> Result { debug!("Adding package into database"); let crate_id = initialize_package_in_database(&conn, metadata_pkg)?; let dependencies = convert_dependencies(metadata_pkg); @@ -47,10 +45,9 @@ pub(crate) fn add_package_into_database( let release_id: i32 = { let rows = conn.query("SELECT id FROM releases WHERE crate_id = $1 AND version = $2", - &[&crate_id, &metadata_pkg.version])?; + &[&crate_id, &format!("{}", metadata_pkg.version)])?; - if rows.is_empty() { - // REFACTOR: These are absurdly large + if rows.len() == 0 { let rows = conn.query("INSERT INTO releases ( crate_id, version, release_time, dependencies, target_name, yanked, build_status, @@ -94,7 +91,6 @@ pub(crate) fn add_package_into_database( rows.get(0).get(0) } else { - // REFACTOR: These are absurdly large conn.query("UPDATE releases SET release_time = $3, dependencies = $4, @@ -121,7 +117,7 @@ pub(crate) fn add_package_into_database( default_target = $25 WHERE crate_id = $1 AND version = $2", &[&crate_id, - &metadata_pkg.version, + &format!("{}", metadata_pkg.version), &cratesio_data.release_time, &dependencies.to_json(), &metadata_pkg.package_name(), @@ -149,40 +145,33 @@ pub(crate) fn add_package_into_database( } }; - add_keywords_into_database(&conn, &metadata_pkg, release_id)?; - add_authors_into_database(&conn, &metadata_pkg, release_id)?; - add_owners_into_database(&conn, &cratesio_data.owners, crate_id)?; + + add_keywords_into_database(&conn, &metadata_pkg, &release_id)?; + add_authors_into_database(&conn, &metadata_pkg, &release_id)?; + add_owners_into_database(&conn, &cratesio_data.owners, &crate_id)?; + // Update versions { let metadata_version = Version::parse(&metadata_pkg.version)?; - let mut versions: Json = conn.query( - "SELECT versions FROM crates WHERE id = $1", - &[&crate_id] - )? - .get(0) - .get(0); - + let mut versions: Json = conn.query("SELECT versions FROM crates WHERE id = $1", + &[&crate_id])? + .get(0) + .get(0); if let Some(versions_array) = versions.as_array_mut() { let mut found = false; - for version in versions_array.clone() { let version = Version::parse(version.as_string().unwrap())?; - if version == metadata_version { found = true; } } - if !found { - versions_array.push(metadata_pkg.version.to_json()); + versions_array.push(format!("{}", &metadata_pkg.version).to_json()); } } - - let _ = conn.query( - "UPDATE crates SET versions = $1 WHERE id = $2", - &[&versions, &crate_id] - ); + let _ = conn.query("UPDATE crates SET versions = $1 WHERE id = $2", + &[&versions, &crate_id]); } Ok(release_id) @@ -190,45 +179,37 @@ pub(crate) fn add_package_into_database( /// Adds a build into database -pub(crate) fn add_build_into_database( - conn: &Connection, - release_id: i32, - res: &BuildResult -) -> Result { +pub(crate) fn add_build_into_database(conn: &Connection, + release_id: &i32, + res: &BuildResult) + -> Result { debug!("Adding build into database"); - - let rows = conn.query( - "INSERT INTO builds (rid, rustc_version, - cratesfyi_version, - build_status, output) - VALUES ($1, $2, $3, $4, $5) - RETURNING id", - &[ - &release_id, - &res.rustc_version, - &res.docsrs_version, - &res.successful, - &res.build_log, - ], - )?; - + let rows = conn.query("INSERT INTO builds (rid, rustc_version, + cratesfyi_version, + build_status, output) + VALUES ($1, $2, $3, $4, $5) + RETURNING id", + &[release_id, + &res.rustc_version, + &res.docsrs_version, + &res.successful, + &res.build_log])?; Ok(rows.get(0).get(0)) } + fn initialize_package_in_database(conn: &Connection, pkg: &MetadataPackage) -> Result { let mut rows = conn.query("SELECT id FROM crates WHERE name = $1", &[&pkg.name])?; - // insert crate into database if it is not exists - if rows.is_empty() { - rows = conn.query( - "INSERT INTO crates (name) VALUES ($1) RETURNING id", - &[&pkg.name] - )?; + if rows.len() == 0 { + rows = conn.query("INSERT INTO crates (name) VALUES ($1) RETURNING id", + &[&pkg.name])?; } - Ok(rows.get(0).get(0)) } + + /// 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(); @@ -238,19 +219,19 @@ fn convert_dependencies(pkg: &MetadataPackage) -> Vec<(String, String, String)> let kind = dependency.kind.clone().unwrap_or_else(|| "normal".into()); dependencies.push((name, version, kind.to_string())); } - dependencies } + /// Reads readme if there is any read defined in Cargo.toml of a Package fn get_readme(pkg: &MetadataPackage, source_dir: &Path) -> Result> { - let readme_path = source_dir.join(pkg.readme.clone().unwrap_or_else(|| "README.md".to_owned())); + let readme_path = source_dir.join(pkg.readme.clone().unwrap_or("README.md".to_owned())); if !readme_path.exists() { return Ok(None); } - let mut reader = fs::File::open(readme_path).map(BufReader::new)?; + let mut reader = fs::File::open(readme_path).map(|f| BufReader::new(f))?; let mut readme = String::new(); reader.read_to_string(&mut readme)?; @@ -281,20 +262,17 @@ fn get_rustdoc(pkg: &MetadataPackage, source_dir: &Path) -> Result Result> { - let reader = fs::File::open(file_path).map(BufReader::new)?; + let reader = fs::File::open(file_path).map(|f| BufReader::new(f))?; let mut rustdoc = String::new(); for line in reader.lines() { let line = line?; - if line.starts_with("//!") { // some lines may or may not have a space between the `//!` and the start of the text let line = line.trim_start_matches("//!").trim_start(); - if !line.is_empty() { rustdoc.push_str(line); } - rustdoc.push('\n'); } } @@ -308,6 +286,7 @@ fn read_rust_doc(file_path: &Path) -> Result> { } } + pub(crate) struct CratesIoData { pub(crate) release_time: Timespec, pub(crate) yanked: bool, @@ -329,25 +308,21 @@ impl CratesIoData { } } + /// Get release_time, yanked and downloads from crates.io fn get_release_time_yanked_downloads( pkg: &MetadataPackage, ) -> Result<(time::Timespec, bool, i32)> { let url = format!("https://crates.io/api/v1/crates/{}/versions", pkg.name); - // FIXME: There is probably better way to do this // and so many unwraps... let client = Client::new(); - - let mut res = client - .get(&url[..]) + let mut res = client.get(&url[..]) .header(ACCEPT, "application/json") .send()?; - let mut body = String::new(); res.read_to_string(&mut body).unwrap(); let json = Json::from_str(&body[..]).unwrap(); - let versions = json.as_object() .and_then(|o| o.get("versions")) .and_then(|v| v.as_array()) @@ -356,9 +331,7 @@ fn get_release_time_yanked_downloads( let (mut release_time, mut yanked, mut downloads) = (None, None, None); for version in versions { - let version = version.as_object() - .ok_or_else(|| err_msg("Not a JSON object"))?; - + let version = version.as_object().ok_or_else(|| err_msg("Not a JSON object"))?; let version_num = version.get("num") .and_then(|v| v.as_string()) .ok_or_else(|| err_msg("Not a JSON object"))?; @@ -367,7 +340,6 @@ fn get_release_time_yanked_downloads( let release_time_raw = version.get("created_at") .and_then(|c| c.as_string()) .ok_or_else(|| err_msg("Not a JSON object"))?; - release_time = Some(time::strptime(release_time_raw, "%Y-%m-%dT%H:%M:%S") .unwrap() .to_timespec()); @@ -393,24 +365,20 @@ fn get_release_time_yanked_downloads( /// Adds keywords into database -fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: i32) -> Result<()> { +fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: &i32) -> Result<()> { for keyword in &pkg.keywords { let slug = slugify(&keyword); let keyword_id: i32 = { let rows = conn.query("SELECT id FROM keywords WHERE slug = $1", &[&slug])?; - - if !rows.is_empty() { + if rows.len() > 0 { rows.get(0).get(0) } else { - conn.query( - "INSERT INTO keywords (name, slug) VALUES ($1, $2) RETURNING id", - &[&keyword, &slug] - )? - .get(0) - .get(0) + conn.query("INSERT INTO keywords (name, slug) VALUES ($1, $2) RETURNING id", + &[&keyword, &slug])? + .get(0) + .get(0) } }; - // add releationship let _ = conn.query("INSERT INTO keyword_rels (rid, kid) VALUES ($1, $2)", &[&release_id, &keyword_id]); } @@ -421,25 +389,18 @@ fn add_keywords_into_database(conn: &Connection, pkg: &MetadataPackage, release_ /// Adds authors into database -fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: i32) -> Result<()> { +fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_id: &i32) -> Result<()> { + let author_capture_re = Regex::new("^([^><]+)<*(.*?)>*$").unwrap(); for author in &pkg.authors { if let Some(author_captures) = author_capture_re.captures(&author[..]) { - let author = author_captures - .get(1) - .map(|m| m.as_str()) - .unwrap_or("") - .trim(); - let email = author_captures - .get(2) - .map(|m| m.as_str()) - .unwrap_or("") - .trim(); + let author = author_captures.get(1).map(|m| m.as_str()).unwrap_or("").trim(); + let email = author_captures.get(2).map(|m| m.as_str()).unwrap_or("").trim(); let slug = slugify(&author); let author_id: i32 = { let rows = conn.query("SELECT id FROM authors WHERE slug = $1", &[&slug])?; - if rows.is_empty() { + if rows.len() > 0 { rows.get(0).get(0) } else { conn.query("INSERT INTO authors (name, email, slug) VALUES ($1, $2, $3) @@ -451,16 +412,15 @@ fn add_authors_into_database(conn: &Connection, pkg: &MetadataPackage, release_i }; // add relationship - let _ = conn.query( - "INSERT INTO author_rels (rid, aid) VALUES ($1, $2)", - &[&release_id, &author_id] - ); + let _ = conn.query("INSERT INTO author_rels (rid, aid) VALUES ($1, $2)", + &[release_id, &author_id]); } } Ok(()) } + pub(crate) struct CrateOwner { pub(crate) avatar: String, pub(crate) email: String, @@ -485,25 +445,21 @@ fn get_owners(pkg: &MetadataPackage) -> Result> { let mut result = Vec::new(); if let Some(owners) = json.as_object() .and_then(|j| j.get("users")) - .and_then(|j| j.as_array()) - { + .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_string()) .unwrap_or(""); - let email = owner.as_object() .and_then(|o| o.get("email")) .and_then(|o| o.as_string()) .unwrap_or(""); - let login = owner.as_object() .and_then(|o| o.get("login")) .and_then(|o| o.as_string()) .unwrap_or(""); - let name = owner.as_object() .and_then(|o| o.get("name")) .and_then(|o| o.as_string()) @@ -526,11 +482,11 @@ fn get_owners(pkg: &MetadataPackage) -> Result> { } /// Adds owners into database -fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id: i32) -> Result<()> { +fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id: &i32) -> Result<()> { for owner in owners { let owner_id: i32 = { let rows = conn.query("SELECT id FROM owners WHERE login = $1", &[&owner.login])?; - if rows.is_empty() { + if rows.len() > 0 { rows.get(0).get(0) } else { conn.query("INSERT INTO owners (login, avatar, name, email) diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index f5d38f339..b36e27e31 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -389,7 +389,7 @@ impl RustwideBuilder { has_docs, has_examples, )?; - add_build_into_database(&conn, release_id, &res.result)?; + add_build_into_database(&conn, &release_id, &res.result)?; doc_builder.add_to_cache(name, version); Ok(res) diff --git a/src/test/fakes.rs b/src/test/fakes.rs index 0894b5b24..998f96958 100644 --- a/src/test/fakes.rs +++ b/src/test/fakes.rs @@ -182,7 +182,7 @@ impl<'a> FakeRelease<'a> { self.has_docs, self.has_examples, )?; - crate::db::add_build_into_database(&db.conn(), release_id, &self.build_result)?; + crate::db::add_build_into_database(&db.conn(), &release_id, &self.build_result)?; Ok(release_id) } From 4af55fe34751791d3b55c41ff68c9133715be7e1 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 13:00:08 -0500 Subject: [PATCH 06/14] Removed duplicate CRATESFYI_PREFIX variable Co-Authored-By: Joshua Nelson --- src/utils/daemon.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/utils/daemon.rs b/src/utils/daemon.rs index f9d4b1ff4..7add0f997 100644 --- a/src/utils/daemon.rs +++ b/src/utils/daemon.rs @@ -25,8 +25,7 @@ use ::{ }; pub fn start_daemon(background: bool) { - const CRATE_VARIABLES: [&str; 4] = [ - "CRATESFYI_PREFIX", + const CRATE_VARIABLES: [&str; 3] = [ "CRATESFYI_PREFIX", "CRATESFYI_GITHUB_USERNAME", "CRATESFYI_GITHUB_ACCESSTOKEN" From 21e83e3f807710270eabf269a2ede3d6ad4b1e96 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 13:03:37 -0500 Subject: [PATCH 07/14] Split reponame into a separate variable --- src/utils/github_updater.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/utils/github_updater.rs b/src/utils/github_updater.rs index 2acce7bca..3d1e5b4e9 100644 --- a/src/utils/github_updater.rs +++ b/src/utils/github_updater.rs @@ -119,16 +119,13 @@ fn get_github_path(url: &str) -> Option { Some(cap) => { let username = cap.get(1).unwrap().as_str(); let reponame = cap.get(2).unwrap().as_str(); + let reponame = if reponame.ends_with(".git") { + reponame.split(".git").next().unwrap() + } else { + reponame + }; - Some(format!( - "{}/{}", - username, - if reponame.ends_with(".git") { - reponame.split(".git").next().unwrap() - } else { - reponame - } - )) + Some(format!("{}/{}", username, reponame)) } None => None, From ea1ed62b4eb413790e6d9163d0d801dc2c7f392d Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 13:09:23 -0500 Subject: [PATCH 08/14] Return query error --- src/docbuilder/mod.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/docbuilder/mod.rs b/src/docbuilder/mod.rs index d5a1f4da2..8fb38c0e0 100644 --- a/src/docbuilder/mod.rs +++ b/src/docbuilder/mod.rs @@ -58,14 +58,11 @@ impl DocBuilder { use crate::db::connect_db; let conn = connect_db()?; - for row in &conn - .query( - "SELECT name, version FROM crates, releases \ - WHERE crates.id = releases.crate_id", - &[], - ) - .unwrap() - { + for row in &conn.query( + "SELECT name, version FROM crates, releases \ + WHERE crates.id = releases.crate_id", + &[], + )? { let name: String = row.get(0); let version: String = row.get(1); From d6b4065570c43132ac0b765cdba50c43b1c3ce5f Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 13:12:42 -0500 Subject: [PATCH 09/14] Changed ok_or_else to ok_or --- src/web/releases.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/web/releases.rs b/src/web/releases.rs index 71323271c..675dced54 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -398,7 +398,7 @@ pub fn author_handler(req: &mut Request) -> IronResult { let conn = extension!(req, Pool).get(); let author = ctry!(router.find("author") - .ok_or_else(|| IronError::new(Nope::CrateNotFound, status::NotFound))); + .ok_or(IronError::new(Nope::CrateNotFound, status::NotFound))); let (author_name, packages) = if author.starts_with('@') { let mut author = author.clone().split('@'); From d68dac7e4630e8d264de17a5fa6e8e5b202837e7 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Thu, 2 Apr 2020 13:16:13 -0500 Subject: [PATCH 10/14] Annotated &&str dereferences with reasoning --- src/docbuilder/rustwide_builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index b36e27e31..ee517fc44 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -108,7 +108,7 @@ impl RustwideBuilder { let mut targets_to_install = TARGETS .iter() - .map(|t| (*t).to_string()) + .map(|t| (*t).to_string()) // &str has a specialized ToString impl, while &&str goes through Display .collect::>(); let installed_targets = match self.toolchain.installed_targets(&self.workspace) { Ok(targets) => targets, @@ -215,7 +215,7 @@ impl RustwideBuilder { let file_name = if versioned { format!("{}-{}.{}", segments[1], rustc_version, segments[0]) } else { - (*file).to_string() + (*file).to_string() // &str has a specialized ToString impl, while &&str goes through Display }; let source_path = source.join(&file_name); let dest_path = dest.path().join(&file_name); From 846d03d0a8c25cc109a53191c6fe61e6964470bd Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Tue, 14 Apr 2020 11:41:21 -0500 Subject: [PATCH 11/14] Rustfmt --- build.rs | 2 +- src/db/file.rs | 2 +- src/docbuilder/rustwide_builder.rs | 4 ++-- src/utils/github_updater.rs | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/build.rs b/build.rs index f9bde9dbe..907df3ff0 100644 --- a/build.rs +++ b/build.rs @@ -19,7 +19,7 @@ fn main() { fn write_git_version() { let maybe_hash = get_git_hash(); let git_hash = maybe_hash.as_deref().unwrap_or("???????"); - + let build_date = time::strftime("%Y-%m-%d", &time::now_utc()).unwrap(); let dest_path = Path::new(&env::var("OUT_DIR").unwrap()).join("git_version"); diff --git a/src/db/file.rs b/src/db/file.rs index 983534b40..ccd1916cf 100644 --- a/src/db/file.rs +++ b/src/db/file.rs @@ -134,7 +134,7 @@ pub(super) fn s3_client() -> Option { return None; } }; - + Some(S3Client::new_with( rusoto_core::request::HttpClient::new().unwrap(), creds, diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 14193a942..5f0010bfe 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -124,7 +124,7 @@ impl RustwideBuilder { .iter() .map(|t| (*t).to_string()) // &str has a specialized ToString impl, while &&str goes through Display .collect::>(); - + let installed_targets = match self.toolchain.installed_targets(&self.workspace) { Ok(targets) => targets, Err(err) => { @@ -226,7 +226,7 @@ impl RustwideBuilder { let file_name = if versioned { format!("{}-{}.{}", segments[1], rustc_version, segments[0]) } else { - (*file).to_string() // &str has a specialized ToString impl, while &&str goes through Display + (*file).to_string() // &str has a specialized ToString impl, while &&str goes through Display }; let source_path = source.join(&file_name); let dest_path = dest.path().join(&file_name); diff --git a/src/utils/github_updater.rs b/src/utils/github_updater.rs index 08cafbe85..0a80ba808 100644 --- a/src/utils/github_updater.rs +++ b/src/utils/github_updater.rs @@ -136,7 +136,7 @@ fn get_github_path(url: &str) -> Option { Some(cap) => { let username = cap.get(1).unwrap().as_str(); let reponame = cap.get(2).unwrap().as_str(); - + let reponame = if reponame.ends_with(".git") { reponame.split(".git").next().unwrap() } else { From 7950bc26f9dcf3b7166ed985b61fb63f0a0bb4b8 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Tue, 14 Apr 2020 18:57:55 -0500 Subject: [PATCH 12/14] Update src/utils/release_activity_updater.rs Co-Authored-By: Joshua Nelson --- src/utils/release_activity_updater.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/utils/release_activity_updater.rs b/src/utils/release_activity_updater.rs index b0c203b3a..1d1b9ac84 100644 --- a/src/utils/release_activity_updater.rs +++ b/src/utils/release_activity_updater.rs @@ -43,7 +43,6 @@ pub fn update_release_activity() -> Result<()> { // unwrap is fine here, as our date format is always valid dates.push(format!("{}", date.strftime("%d %b").unwrap())); - crate_counts.push(release_count); failure_counts.push(failure_count); } From b8730c5ebb249de5cd22e750260fe2caa797cdb5 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Wed, 15 Apr 2020 08:44:11 -0500 Subject: [PATCH 13/14] References --- src/docbuilder/rustwide_builder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 5f0010bfe..845e5aa60 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -122,7 +122,7 @@ impl RustwideBuilder { let mut targets_to_install = TARGETS .iter() - .map(|t| (*t).to_string()) // &str has a specialized ToString impl, while &&str goes through Display + .map(|&t| t.to_string()) // &str has a specialized ToString impl, while &&str goes through Display .collect::>(); let installed_targets = match self.toolchain.installed_targets(&self.workspace) { @@ -226,7 +226,7 @@ impl RustwideBuilder { let file_name = if versioned { format!("{}-{}.{}", segments[1], rustc_version, segments[0]) } else { - (*file).to_string() // &str has a specialized ToString impl, while &&str goes through Display + file.to_string() }; let source_path = source.join(&file_name); let dest_path = dest.path().join(&file_name); From e99463e09c4a8ff8ddd71757583e23acd7340fb8 Mon Sep 17 00:00:00 2001 From: Chase Wilson Date: Wed, 15 Apr 2020 09:53:20 -0500 Subject: [PATCH 14/14] If > unwrap_or_else --- src/utils/daemon.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/utils/daemon.rs b/src/utils/daemon.rs index 364680748..48bfc653a 100644 --- a/src/utils/daemon.rs +++ b/src/utils/daemon.rs @@ -26,7 +26,9 @@ pub fn start_daemon(background: bool) { // first check required environment variables for v in CRATE_VARIABLES.iter() { - env::var(v).unwrap_or_else(|_| panic!("Environment variable {} not found", v)); + if env::var(v).is_err() { + panic!("Environment variable {} not found", v) + } } let dbopts = opts();