From e3c04b9b242a0ffd8dff8fc713a2dd9ee40c8dd2 Mon Sep 17 00:00:00 2001 From: Koenraad Verheyden Date: Fri, 10 Jan 2020 02:12:11 +0100 Subject: [PATCH 1/6] web::rustdoc: rename version to latest_version to avoid shadowing and dinstinguish its meaning --- src/web/rustdoc.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 34c6be660..17e0e37d5 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -262,9 +262,9 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { content.full = file_content; let crate_details = cexpect!(CrateDetails::new(&conn, &name, &version)); - let (path, version) = if let Some(version) = latest_version(&crate_details.versions(), &version) { - req_path[2] = &version; - (path_for_version(&req_path, &crate_details.target_name, &conn), version) + let (path, latest_version) = if let Some(latest_version) = latest_version(&crate_details.versions(), &version) { + req_path[2] = &latest_version; + (path_for_version(&req_path, &crate_details.target_name, &conn), latest_version) } else { Default::default() }; @@ -277,7 +277,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { .set_true("package_navigation_show_platforms_tab") .set_bool("is_latest_version", path.is_empty()) .set("path_in_latest", &path) - .set("latest_version", &version) + .set("latest_version", &latest_version) .to_resp("rustdoc") } From 309ea7a90f86fe2f1f54850b447867b2fe132cb8 Mon Sep 17 00:00:00 2001 From: Koenraad Verheyden Date: Fri, 10 Jan 2020 02:30:23 +0100 Subject: [PATCH 2/6] CrateDetails: add functions to work with latest version; refactor rustdoc This commit manages to remove _a lot_ of code from src::web::mod. The removed code was intended to determine the latest version of a list of versions. Its only caller, rustdoc, passed a list of versions directly from CrateDetails. This could be simplified by moving this logic into CrateDetails, which already has a sorted list of versions.struct. --- src/web/crate_details.rs | 37 +++++++++++++++++++++++++++++++- src/web/mod.rs | 46 ---------------------------------------- src/web/rustdoc.rs | 6 ++++-- 3 files changed, 40 insertions(+), 49 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 769a7782d..992dc25ac 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -261,8 +261,18 @@ impl CrateDetails { .map(|release| release.version.clone()) .collect() } -} + /// Returns whether this release is the latest release of this crate. + pub fn is_latest_version(&self) -> bool { + self.version == self.latest_version() + } + + /// Returns the version of the latest release of this crate. + pub fn latest_version(&self) -> &str { + // this is safe to unwrap: releases will always contain at least the version of this release + &self.releases.get(0).unwrap().version + } +} fn map_to_release(conn: &Connection, crate_id: i32, version: String) -> Release { let rows = conn.query( @@ -416,4 +426,29 @@ mod tests { Ok(()) }); } + + #[test] + fn test_latest_version() { + crate::test::wrapper(|env| { + let db = env.db(); + + db.fake_release().name("foo").version("0.0.1").create()?; + db.fake_release().name("foo").version("0.0.3").create()?; + db.fake_release().name("foo").version("0.0.2").create()?; + + let details = CrateDetails::new(&db.conn(), "foo", "0.0.1").unwrap(); + assert_eq!(details.is_latest_version(), false); + assert_eq!(details.latest_version(), "0.0.3"); + + let details = CrateDetails::new(&db.conn(), "foo", "0.0.2").unwrap(); + assert_eq!(details.is_latest_version(), false); + assert_eq!(details.latest_version(), "0.0.3"); + + let details = CrateDetails::new(&db.conn(), "foo", "0.0.3").unwrap(); + assert_eq!(details.is_latest_version(), true); + assert_eq!(details.latest_version(), "0.0.3"); + + Ok(()) + }) + } } diff --git a/src/web/mod.rs b/src/web/mod.rs index f1cf5f3da..3fe82f2e6 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -313,39 +313,6 @@ fn render_markdown(text: &str) -> String { -/// Returns latest version if required version is not the latest -/// req_version must be an exact version -fn latest_version(versions_json: &Vec, req_version: &str) -> Option { - let req_version = match Version::parse(req_version) { - Ok(v) => v, - Err(_) => return None, - }; - let versions = { - let mut versions: Vec = Vec::new(); - for version in versions_json { - let version = match Version::parse(&version) { - Ok(v) => v, - Err(_) => return None, - }; - versions.push(version); - } - - versions.sort(); - versions.reverse(); - versions - }; - - if req_version != versions[0] { - for i in 1..versions.len() { - if req_version == versions[i] { - return Some(format!("{}", versions[0])) - } - } - } - None -} - - pub struct Server { inner: Listening, } @@ -537,7 +504,6 @@ impl ToJson for MetaData { #[cfg(test)] mod test { extern crate env_logger; - use super::*; #[test] fn test_index_returns_success() { @@ -547,16 +513,4 @@ mod test { Ok(()) }); } - - #[test] - fn test_latest_version() { - let versions = vec!["1.0.0".to_string(), - "1.1.0".to_string(), - "0.9.0".to_string(), - "0.9.1".to_string()]; - assert_eq!(latest_version(&versions, "1.1.0"), None); - assert_eq!(latest_version(&versions, "1.0.0"), Some("1.1.0".to_owned())); - assert_eq!(latest_version(&versions, "0.9.0"), Some("1.1.0".to_owned())); - assert_eq!(latest_version(&versions, "invalidversion"), None); - } } diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 17e0e37d5..5b4c6bb9d 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -3,7 +3,7 @@ use super::pool::Pool; use super::file::File; -use super::{latest_version, redirect_base}; +use super::redirect_base; use super::crate_details::CrateDetails; use iron::prelude::*; use iron::{status, Url}; @@ -262,7 +262,9 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { content.full = file_content; let crate_details = cexpect!(CrateDetails::new(&conn, &name, &version)); - let (path, latest_version) = if let Some(latest_version) = latest_version(&crate_details.versions(), &version) { + + let (path, latest_version) = if !crate_details.is_latest_version() { + let latest_version = crate_details.latest_version().to_string(); req_path[2] = &latest_version; (path_for_version(&req_path, &crate_details.target_name, &conn), latest_version) } else { From 99875f0dd6ac487ed21a322826163cea82db5e93 Mon Sep 17 00:00:00 2001 From: Koenraad Verheyden Date: Fri, 10 Jan 2020 02:30:56 +0100 Subject: [PATCH 3/6] CrateDetails: remove dead code --- src/web/crate_details.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 992dc25ac..cc8ae10cd 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -255,13 +255,6 @@ impl CrateDetails { Some(crate_details) } - /// Returns all versions of this crate. - pub fn versions(&self) -> Vec { - self.releases.iter() - .map(|release| release.version.clone()) - .collect() - } - /// Returns whether this release is the latest release of this crate. pub fn is_latest_version(&self) -> bool { self.version == self.latest_version() @@ -403,7 +396,7 @@ mod tests { } #[test] - fn test_versions() { + fn test_releases_should_be_sorted() { crate::test::wrapper(|env| { let db = env.db(); @@ -414,8 +407,6 @@ mod tests { create_release(&db, "foo", "0.0.2", true)?; let details = CrateDetails::new(&db.conn(), "foo", "0.0.2").unwrap(); - - assert_eq!(details.versions(), vec!["1.0.0", "0.0.3", "0.0.2", "0.0.1"]); assert_eq!(details.releases, vec![ Release { version: "1.0.0".to_string(), build_status: true }, Release { version: "0.0.3".to_string(), build_status: false }, From 49f128361b39ef47245ff5800389788ba5c77c9e Mon Sep 17 00:00:00 2001 From: Koenraad Verheyden Date: Fri, 10 Jan 2020 12:20:16 +0100 Subject: [PATCH 4/6] CrateDetails: access Vec directly --- src/web/crate_details.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index cc8ae10cd..9d716f80f 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -262,8 +262,8 @@ impl CrateDetails { /// Returns the version of the latest release of this crate. pub fn latest_version(&self) -> &str { - // this is safe to unwrap: releases will always contain at least the version of this release - &self.releases.get(0).unwrap().version + // releases will always contain at least one element + &self.releases[0].version } } From 3685a812e6871afb9ed350c71573e1072f345c39 Mon Sep 17 00:00:00 2001 From: Koenraad Verheyden Date: Fri, 10 Jan 2020 12:29:59 +0100 Subject: [PATCH 5/6] refactor rustdoc to simplify how path to latest version is set --- src/web/crate_details.rs | 8 -------- src/web/rustdoc.rs | 10 ++++++---- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 9d716f80f..a270e3bee 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -255,11 +255,6 @@ impl CrateDetails { Some(crate_details) } - /// Returns whether this release is the latest release of this crate. - pub fn is_latest_version(&self) -> bool { - self.version == self.latest_version() - } - /// Returns the version of the latest release of this crate. pub fn latest_version(&self) -> &str { // releases will always contain at least one element @@ -428,15 +423,12 @@ mod tests { db.fake_release().name("foo").version("0.0.2").create()?; let details = CrateDetails::new(&db.conn(), "foo", "0.0.1").unwrap(); - assert_eq!(details.is_latest_version(), false); assert_eq!(details.latest_version(), "0.0.3"); let details = CrateDetails::new(&db.conn(), "foo", "0.0.2").unwrap(); - assert_eq!(details.is_latest_version(), false); assert_eq!(details.latest_version(), "0.0.3"); let details = CrateDetails::new(&db.conn(), "foo", "0.0.3").unwrap(); - assert_eq!(details.is_latest_version(), true); assert_eq!(details.latest_version(), "0.0.3"); Ok(()) diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 5b4c6bb9d..31d94ed44 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -263,10 +263,12 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { content.full = file_content; let crate_details = cexpect!(CrateDetails::new(&conn, &name, &version)); - let (path, latest_version) = if !crate_details.is_latest_version() { - let latest_version = crate_details.latest_version().to_string(); + let latest_version = crate_details.latest_version().to_owned(); + let is_latest_version = latest_version == version; + + let path = if !is_latest_version { req_path[2] = &latest_version; - (path_for_version(&req_path, &crate_details.target_name, &conn), latest_version) + path_for_version(&req_path, &crate_details.target_name, &conn) } else { Default::default() }; @@ -277,7 +279,7 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { .set_true("show_package_navigation") .set_true("package_navigation_documentation_tab") .set_true("package_navigation_show_platforms_tab") - .set_bool("is_latest_version", path.is_empty()) + .set_bool("is_latest_version", is_latest_version) .set("path_in_latest", &path) .set("latest_version", &latest_version) .to_resp("rustdoc") From 4c79bbbdaa5d667876f081b738a601fb0a96d142 Mon Sep 17 00:00:00 2001 From: Koenraad Verheyden Date: Fri, 10 Jan 2020 23:43:32 +0100 Subject: [PATCH 6/6] CrateDetails: extend test releases should be sorted with extra edge cases --- src/web/crate_details.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index a270e3bee..1d91820e4 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -396,17 +396,23 @@ mod tests { let db = env.db(); // Add new releases of 'foo' out-of-order since CrateDetails should sort them descending - create_release(&db, "foo", "0.0.1", true)?; - create_release(&db, "foo", "0.0.3", false)?; + create_release(&db, "foo", "0.1.0", true)?; + create_release(&db, "foo", "0.1.1", true)?; + create_release(&db, "foo", "0.3.0", false)?; create_release(&db, "foo", "1.0.0", true)?; - create_release(&db, "foo", "0.0.2", true)?; + create_release(&db, "foo", "0.12.0", true)?; + create_release(&db, "foo", "0.2.0", true)?; + create_release(&db, "foo", "0.2.0-alpha", true)?; - let details = CrateDetails::new(&db.conn(), "foo", "0.0.2").unwrap(); + let details = CrateDetails::new(&db.conn(), "foo", "0.2.0").unwrap(); assert_eq!(details.releases, vec![ Release { version: "1.0.0".to_string(), build_status: true }, - Release { version: "0.0.3".to_string(), build_status: false }, - Release { version: "0.0.2".to_string(), build_status: true }, - Release { version: "0.0.1".to_string(), build_status: true }, + Release { version: "0.12.0".to_string(), build_status: true }, + Release { version: "0.3.0".to_string(), build_status: false }, + Release { version: "0.2.0".to_string(), build_status: true }, + Release { version: "0.2.0-alpha".to_string(), build_status: true }, + Release { version: "0.1.1".to_string(), build_status: true }, + Release { version: "0.1.0".to_string(), build_status: true }, ]); Ok(())