Code review

Author: Kixiron

src/bin/cratesfyi.rs

@@ -54,8 +54,8 @@ enum CommandLine {
         socket_addr: String,
 
         /// Reload templates when they're changed
-        #[structopt(long = "reload")]
-        reload: bool,
+        #[structopt(long = "reload-templates")]
+        reload_templates: bool,
     },
 
     /// Starts cratesfyi daemon
@@ -84,9 +84,9 @@ impl CommandLine {
             Self::Build(build) => build.handle_args(),
             Self::StartWebServer {
                 socket_addr,
-                reload,
+                reload_templates,
             } => {
-                Server::start(Some(&socket_addr), reload);
+                Server::start(Some(&socket_addr), reload_templates);
             }
             Self::Daemon { foreground } => cratesfyi::utils::start_daemon(!foreground),
             Self::Database { subcommand } => subcommand.handle_args(),

src/web/mod.rs

@@ -363,7 +363,8 @@ pub struct Server {
 
 impl Server {
     pub fn start(addr: Option<&str>, reload_templates: bool) -> Self {
-        page::TEMPLATE_DATA.poke().expect("This returns Ok(())");
+        // Initialize templates
+        let _: &page::TemplateData = &*page::TEMPLATE_DATA;
         if reload_templates {
             page::TemplateData::start_template_reloading();
         }

src/web/page/handlebars.rs

@@ -0,0 +1,246 @@
+//! Generic page struct
+
+use handlebars_iron::Template;
+use iron::response::Response;
+use iron::{status, IronResult, Set};
+use serde::{
+    ser::{SerializeStruct, Serializer},
+    Serialize,
+};
+use serde_json::Value;
+use std::collections::BTreeMap;
+
+lazy_static::lazy_static! {
+    static ref RUSTC_RESOURCE_SUFFIX: String = load_rustc_resource_suffix()
+        .unwrap_or_else(|_| "???".into());
+}
+
+fn load_rustc_resource_suffix() -> Result<String, failure::Error> {
+    let conn = crate::db::connect_db()?;
+
+    let res = conn.query(
+        "SELECT value FROM config WHERE name = 'rustc_version';",
+        &[],
+    )?;
+    if res.is_empty() {
+        failure::bail!("missing rustc version");
+    }
+
+    if let Some(Ok(vers)) = res.get(0).get_opt::<_, Value>("value") {
+        if let Some(vers_str) = vers.as_str() {
+            return Ok(crate::utils::parse_rustc_version(vers_str)?);
+        }
+    }
+
+    failure::bail!("failed to parse the rustc version");
+}
+
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Page<T: Serialize> {
+    title: Option<String>,
+    content: T,
+    status: status::Status,
+    varss: BTreeMap<String, String>,
+    varsb: BTreeMap<String, bool>,
+    varsi: BTreeMap<String, i64>,
+    rustc_resource_suffix: &'static str,
+}
+
+impl<T: Serialize> Page<T> {
+    pub fn new(content: T) -> Page<T> {
+        Page {
+            title: None,
+            content,
+            status: status::Ok,
+            varss: BTreeMap::new(),
+            varsb: BTreeMap::new(),
+            varsi: BTreeMap::new(),
+            rustc_resource_suffix: &RUSTC_RESOURCE_SUFFIX,
+        }
+    }
+
+    /// Sets a string variable
+    pub fn set(mut self, var: &str, val: &str) -> Page<T> {
+        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<T> {
+        self.varsb.insert(var.to_owned(), val);
+        self
+    }
+
+    /// Sets a boolean variable to true
+    pub fn set_true(mut self, var: &str) -> Page<T> {
+        self.varsb.insert(var.to_owned(), true);
+        self
+    }
+
+    /// Sets an integer variable
+    pub fn set_int(mut self, var: &str, val: i64) -> Page<T> {
+        self.varsi.insert(var.to_owned(), val);
+        self
+    }
+
+    /// Sets title of page
+    pub fn title(mut self, title: &str) -> Page<T> {
+        self.title = Some(title.to_owned());
+        self
+    }
+
+    /// Sets status code for response
+    pub fn set_status(mut self, s: status::Status) -> Page<T> {
+        self.status = s;
+        self
+    }
+
+    #[allow(clippy::wrong_self_convention)]
+    pub fn to_resp(self, template: &str) -> IronResult<Response> {
+        let mut resp = Response::new();
+        let status = self.status;
+        let temp = Template::new(template, self);
+        resp.set_mut(temp).set_mut(status);
+
+        Ok(resp)
+    }
+}
+
+impl<T: Serialize> Serialize for Page<T> {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        // Make sure that the length parameter passed to serde is correct by
+        // adding the someness of the global alert to the total. `true`
+        // is 1 and `false` is 0, so it increments if the value is some (and therefore
+        // needs to be serialized)
+        let mut state = serializer.serialize_struct(
+            "Page",
+            8 + crate::GLOBAL_ALERT.is_some() as usize + self.title.is_some() as usize,
+        )?;
+
+        if let Some(ref title) = self.title {
+            state.serialize_field("title", title)?;
+        }
+
+        state.serialize_field("has_global_alert", &crate::GLOBAL_ALERT.is_some())?;
+        if let Some(ref global_alert) = crate::GLOBAL_ALERT {
+            state.serialize_field("global_alert", global_alert)?;
+        }
+
+        state.serialize_field("content", &self.content)?;
+        state.serialize_field("rustc_resource_suffix", self.rustc_resource_suffix)?;
+        state.serialize_field("cratesfyi_version", crate::BUILD_VERSION)?;
+        state.serialize_field(
+            "cratesfyi_version_safe",
+            &build_version_safe(crate::BUILD_VERSION),
+        )?;
+        state.serialize_field("varss", &self.varss)?;
+        state.serialize_field("varsb", &self.varsb)?;
+        state.serialize_field("varsi", &self.varsi)?;
+
+        state.end()
+    }
+}
+
+fn build_version_safe(version: &str) -> String {
+    version.replace(" ", "-").replace("(", "").replace(")", "")
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::web::releases::{self, Release};
+    use iron::Url;
+    use serde_json::json;
+
+    #[test]
+    fn serialize_page() {
+        let time = time::get_time();
+
+        let mut release = Release::default();
+        release.name = "lasso".into();
+        release.version = "0.1.0".into();
+        release.release_time = time.clone();
+
+        let mut varss = BTreeMap::new();
+        varss.insert("test".into(), "works".into());
+        let mut varsb = BTreeMap::new();
+        varsb.insert("test2".into(), true);
+        let mut varsi = BTreeMap::new();
+        varsi.insert("test3".into(), 1337);
+
+        let page = Page {
+            title: None,
+            content: vec![release.clone()],
+            status: status::Status::Ok,
+            varss,
+            varsb,
+            varsi,
+            rustc_resource_suffix: &*RUSTC_RESOURCE_SUFFIX,
+        };
+
+        let correct_json = json!({
+            "content": [{
+                "name": "lasso",
+                "version": "0.1.0",
+                "description": null,
+                "target_name": null,
+                "rustdoc_status": false,
+                "release_time": super::super::super::duration_to_str(time),
+                "release_time_rfc3339": time::at(time).rfc3339().to_string(),
+                "stars": 0
+            }],
+            "varss": { "test": "works" },
+            "varsb": { "test2": true },
+            "varsi": { "test3": 1337 },
+            "rustc_resource_suffix": &*RUSTC_RESOURCE_SUFFIX,
+            "cratesfyi_version": crate::BUILD_VERSION,
+            "cratesfyi_version_safe": build_version_safe(crate::BUILD_VERSION),
+            "has_global_alert": crate::GLOBAL_ALERT.is_some()
+        });
+
+        assert_eq!(correct_json, serde_json::to_value(&page).unwrap());
+    }
+
+    #[test]
+    fn load_page_from_releases() {
+        crate::test::wrapper(|env| {
+            let db = env.db();
+            db.fake_release().name("foo").version("0.1.0").create()?;
+            let packages = releases::get_releases(&db.conn(), 1, 1, releases::Order::ReleaseTime);
+
+            let mut varsb = BTreeMap::new();
+            varsb.insert("show_search_form".into(), true);
+            varsb.insert("hide_package_navigation".into(), true);
+
+            let correct_page = Page {
+                title: None,
+                content: packages.clone(),
+                status: status::Status::Ok,
+                varss: BTreeMap::new(),
+                varsb,
+                varsi: BTreeMap::new(),
+                rustc_resource_suffix: &RUSTC_RESOURCE_SUFFIX,
+            };
+
+            let page = Page::new(packages)
+                .set_true("show_search_form")
+                .set_true("hide_package_navigation");
+
+            assert_eq!(page, correct_page);
+
+            Ok(())
+        })
+    }
+
+    #[test]
+    fn build_version_url_safe() {
+        let safe = format!(
+            "https://docs.rs/builds/{}",
+            build_version_safe(crate::BUILD_VERSION)
+        );
+        assert!(Url::parse(&safe).is_ok());
+    }
+}