Overhaul templating contrib library: use register callback.

This commit improves and changes the templating library in the following ways:

  * Templates are now registered/loaded at initialization.
  * No synchronization is required to read templates.
  * All templates are properly loaded (fixes #122).
  * Tera templates are given the proper name: `index`, not `index.html.tera`.
  * Rendering tests added for both templating engines.

There is one breaking change:

  * Tera templates are given the proper name: `index`, not `index.html.tera`.

contrib/Cargo.toml

@@ -32,4 +32,5 @@ serde_json = { version = "^0.8", optional = true }
 handlebars = { version = "^0.24", optional = true, features = ["serde_type"] }
 glob = { version = "^0.2", optional = true }
 lazy_static = { version = "^0.2", optional = true }
-tera = { version = "^0.6", optional = true }
+# tera = { version = "^0.6", optional = true }
+tera = { git = "https://github.com/SergioBenitez/tera", optional = true }

contrib/src/lib.rs

@@ -1,3 +1,5 @@
+#![feature(drop_types_in_const)]
+
 //! This crate contains officially sanctioned contributor libraries that provide
 //! functionality commonly used by Rocket applications.
 //!

contrib/src/templates/handlebars_templates.rs

@@ -1,31 +1,53 @@
 extern crate handlebars;
 
-use std::sync::RwLock;
-
 use super::serde::Serialize;
 use super::TemplateInfo;
 
 use self::handlebars::Handlebars;
 
-lazy_static! {
-    static ref HANDLEBARS: RwLock<Handlebars> = RwLock::new(Handlebars::new());
-}
+static mut HANDLEBARS: Option<Handlebars> = None;
 
 pub const EXT: &'static str = "hbs";
 
-pub fn render<T>(name: &str, info: &TemplateInfo, context: &T) -> Option<String>
+// This function must be called a SINGLE TIME from A SINGLE THREAD for safety to
+// hold here and in `render`.
+pub unsafe fn register(templates: &[(&str, &TemplateInfo)]) -> bool {
+    if HANDLEBARS.is_some() {
+        error_!("Internal error: reregistering handlebars!");
+        return false;
+    }
+
+    let mut hb = Handlebars::new();
+    let mut success = true;
+    for &(name, info) in templates {
+        let path = &info.full_path;
+        if let Err(e) = hb.register_template_file(name, path) {
+            error_!("Handlebars template '{}' failed registry: {:?}", name, e);
+            success = false;
+        }
+    }
+
+    HANDLEBARS = Some(hb);
+    success
+}
+
+pub fn render<T>(name: &str, _info: &TemplateInfo, context: &T) -> Option<String>
     where T: Serialize
 {
-    // FIXME: Expose a callback to register each template at launch => no lock.
-    if HANDLEBARS.read().unwrap().get_template(name).is_none() {
-        let p = &info.full_path;
-        if let Err(e) = HANDLEBARS.write().unwrap().register_template_file(name, p) {
-            error_!("Handlebars template '{}' failed registry: {:?}", name, e);
+    let hb = match unsafe { HANDLEBARS.as_ref() } {
+        Some(hb) => hb,
+        None => {
+            error_!("Internal error: `render` called before handlebars init.");
             return None;
         }
+    };
+
+    if hb.get_template(name).is_none() {
+        error_!("Handlebars template '{}' does not exist.", name);
+        return None;
     }
 
-    match HANDLEBARS.read().unwrap().render(name, context) {
+    match hb.render(name, context) {
         Ok(string) => Some(string),
         Err(e) => {
             error_!("Error rendering Handlebars template '{}': {}", name, e);

contrib/src/templates/macros.rs

@@ -2,19 +2,21 @@
 /// engines from the set of template engined passed in.
 macro_rules! engine_set {
     ($($feature:expr => $engine:ident),+,) => ({
-        use std::collections::HashSet;
-        let mut set = HashSet::new();
+        type RegisterFn = for<'a, 'b> unsafe fn(&'a [(&'b str, &TemplateInfo)]) -> bool;
+
+        let mut set = Vec::new();
         $(
             #[cfg(feature = $feature)]
-            fn $engine(set: &mut HashSet<String>) {
-                set.insert($engine::EXT.to_string());
+            fn $engine(set: &mut Vec<(&'static str, RegisterFn)>) {
+                set.push(($engine::EXT, $engine::register));
             }
 
             #[cfg(not(feature = $feature))]
-            fn $engine(_: &mut HashSet<String>) {  }
+            fn $engine(_: &mut Vec<(&'static str, RegisterFn)>) { }
 
             $engine(&mut set);
         )+
+
         set
     });
 }
@@ -25,25 +27,27 @@ macro_rules! engine_set {
 /// extension, and if so, calls the engine's `render` method. All of this only
 /// happens for engine's that have been enabled as features by the user.
 macro_rules! render_set {
-    ($name:expr, $info:expr, $ctxt:expr, $($feature:expr => $engine:ident),+,) => ({$(
-        #[cfg(feature = $feature)]
-        fn $engine<T: Serialize>(name: &str, info: &TemplateInfo, c: &T)
-                -> Option<Template> {
-            if info.extension == $engine::EXT {
-                let rendered = $engine::render(name, info, c);
-                return Some(Template(rendered, info.data_type.clone()));
-            }
+    ($name:expr, $info:expr, $ctxt:expr, $($feature:expr => $engine:ident),+,) => ({
+        $(
+            #[cfg(feature = $feature)]
+            fn $engine<T: Serialize>(name: &str, info: &TemplateInfo, c: &T)
+            -> Option<Template> {
+                if info.extension == $engine::EXT {
+                    let rendered = $engine::render(name, info, c);
+                    return Some(Template(rendered, info.data_type.clone()));
+                }
 
-            None
-        }
+                None
+            }
 
-        #[cfg(not(feature = $feature))]
-        fn $engine<T: Serialize>(_: &str, _: &TemplateInfo, _: &T)
-                -> Option<Template> { None }
+            #[cfg(not(feature = $feature))]
+            fn $engine<T: Serialize>(_: &str, _: &TemplateInfo, _: &T)
+            -> Option<Template> { None }
 
-        if let Some(template) = $engine($name, &$info, $ctxt) {
-            return template
-        }
-    )+});
+            if let Some(template) = $engine($name, &$info, $ctxt) {
+                return template
+            }
+         )+
+    });
 }
 

contrib/src/templates/mod.rs

@@ -17,7 +17,7 @@ use std::path::{Path, PathBuf};
 use std::collections::HashMap;
 use std::fmt;
 
-use rocket::config;
+use rocket::config::{self, ConfigError};
 use rocket::response::{self, Content, Responder};
 use rocket::http::{ContentType, Status};
 
@@ -104,17 +104,25 @@ const DEFAULT_TEMPLATE_DIR: &'static str = "templates";
 
 lazy_static! {
     static ref TEMPLATES: HashMap<String, TemplateInfo> = discover_templates();
-    static ref TEMPLATE_DIR: String = {
-        config::active().map(|config| {
-            let dir = config.get_str("template_dir").map_err(|e| {
+    static ref TEMPLATE_DIR: PathBuf = {
+        let default_dir_path = config::active().ok_or(ConfigError::NotFound)
+            .map(|config| config.root().join(DEFAULT_TEMPLATE_DIR))
+            .map_err(|_| {
+                warn_!("No configuration is active!");
+                warn_!("Using default template directory: {:?}", DEFAULT_TEMPLATE_DIR);
+            })
+            .unwrap_or(PathBuf::from(DEFAULT_TEMPLATE_DIR));
+
+        config::active().ok_or(ConfigError::NotFound)
+            .and_then(|config| config.get_str("template_dir"))
+            .map(|user_dir| PathBuf::from(user_dir))
+            .map_err(|e| {
                 if !e.is_not_found() {
                     e.pretty_print();
-                    warn_!("Using default directory '{}'", DEFAULT_TEMPLATE_DIR);
+                    warn_!("Using default directory '{:?}'", default_dir_path);
                 }
-            }).unwrap_or(DEFAULT_TEMPLATE_DIR);
-
-            config.root().join(dir).to_string_lossy().into_owned()
-        }).unwrap_or(DEFAULT_TEMPLATE_DIR.to_string())
+            })
+            .unwrap_or(default_dir_path)
     };
 }
 
@@ -144,11 +152,13 @@ impl Template {
             let names: Vec<_> = TEMPLATES.keys().map(|s| s.as_str()).collect();
             error_!("Template '{}' does not exist.", name);
             info_!("Known templates: {}", names.join(","));
-            info_!("Searched in '{}'.", *TEMPLATE_DIR);
+            info_!("Searched in '{:?}'.", *TEMPLATE_DIR);
             return Template(None, None);
         }
 
-        // Keep this set in-sync with the `engine_set` invocation.
+        // Keep this set in-sync with the `engine_set` invocation. The macro
+        // `return`s a `Template` if the extenion in `template` matches an
+        // engine in the set. Otherwise, control will fall through.
         render_set!(name, template.unwrap(), context,
             "tera_templates" => tera_templates,
             "handlebars_templates" => handlebars_templates,
@@ -219,6 +229,8 @@ fn split_path(path: &Path) -> (PathBuf, String, Option<String>) {
 /// Returns a HashMap of `TemplateInfo`'s for all of the templates in
 /// `TEMPLATE_DIR`. Templates are all files that match one of the extensions for
 /// engine's in `engine_set`.
+///
+/// **WARNING:** This function should be called ONCE from a SINGLE THREAD.
 fn discover_templates() -> HashMap<String, TemplateInfo> {
     // Keep this set in-sync with the `render_set` invocation.
     let engines = engine_set![
@@ -227,20 +239,31 @@ fn discover_templates() -> HashMap<String, TemplateInfo> {
     ];
 
     let mut templates = HashMap::new();
-    for ext in engines {
-        let mut glob_path: PathBuf = [&*TEMPLATE_DIR, "**", "*"].iter().collect();
+    for &(ext, _) in &engines {
+        let mut glob_path: PathBuf = TEMPLATE_DIR.join("**").join("*");
         glob_path.set_extension(ext);
         for path in glob(glob_path.to_str().unwrap()).unwrap().filter_map(Result::ok) {
             let (rel_path, name, data_type) = split_path(&path);
-            templates.insert(name, TemplateInfo {
+            let info = TemplateInfo {
                 full_path: path.to_path_buf(),
                 path: rel_path,
-                extension: path.extension().unwrap().to_string_lossy().into_owned(),
+                extension: ext.to_string(),
                 data_type: data_type,
-            });
+            };
+
+            templates.insert(name, info);
         }
     }
 
+    for &(ext, register_fn) in &engines {
+        let named_templates = templates.iter()
+            .filter(|&(_, i)| i.extension == ext)
+            .map(|(k, i)| (k.as_str(), i))
+            .collect::<Vec<_>>();
+
+        unsafe { register_fn(&*named_templates); }
+    };
+
     templates
 }
 

contrib/src/templates/tera_templates.rs

@@ -1,43 +1,59 @@
 extern crate tera;
 
-use std::path::PathBuf;
-
 use super::serde::Serialize;
-use super::{TemplateInfo, TEMPLATE_DIR};
-
-lazy_static! {
-    static ref TERA: Result<tera::Tera, String> = {
-        let path: PathBuf = [&*TEMPLATE_DIR, "**", "*.tera"].iter().collect();
-        let ext = [".html.tera", ".htm.tera", ".xml.tera", ".html", ".htm", ".xml"];
-        tera::Tera::new(path.to_str().unwrap())
-            .map(|mut tera| {
-                tera.autoescape_on(ext.to_vec());
-                tera
-            })
-            .map_err(|e| format!("{:?}", e))
-    };
-}
+use super::TemplateInfo;
+
+use self::tera::Tera;
+
+static mut TERA: Option<Tera> = None;
 
 pub const EXT: &'static str = "tera";
 
-pub fn render<T>(name: &str, info: &TemplateInfo, context: &T) -> Option<String>
+// This function must be called a SINGLE TIME from A SINGLE THREAD for safety to
+// hold here and in `render`.
+pub unsafe fn register(templates: &[(&str, &TemplateInfo)]) -> bool {
+    if TERA.is_some() {
+        error_!("Internal error: reregistering Tera!");
+        return false;
+    }
+
+    let mut tera = Tera::default();
+    let ext = [".html.tera", ".htm.tera", ".xml.tera", ".html", ".htm", ".xml"];
+    tera.autoescape_on(ext.to_vec());
+
+    // Collect into a tuple of (name, path) for Tera.
+    let tera_templates = templates.iter()
+        .map(|&(name, info)| (name, &info.full_path))
+        .collect::<Vec<_>>();
+
+    // Finally try to tell Tera about all of the templates.
+    let mut success = true;
+    if let Err(e) = tera.add_template_files(tera_templates) {
+        error_!("Failed to initialize Tera templates: {:?}", e);
+        success = false;
+    }
+
+    TERA = Some(tera);
+    success
+}
+
+pub fn render<T>(name: &str, _: &TemplateInfo, context: &T) -> Option<String>
     where T: Serialize
 {
-    let tera = match *TERA {
-        Ok(ref tera) => tera,
-        Err(ref e) => {
-            error_!("Tera failed to initialize: {}.", e);
+    let tera = match unsafe { TERA.as_ref() } {
+        Some(tera) => tera,
+        None => {
+            error_!("Internal error: `render` called before Tera init.");
             return None;
         }
     };
 
-    let template_name = &info.path.to_string_lossy();
-    if tera.get_template(template_name).is_err() {
-        error_!("Tera template '{}' does not exist.", template_name);
+    if tera.get_template(name).is_err() {
+        error_!("Tera template '{}' does not exist.", name);
         return None;
     };
 
-    match tera.value_render(template_name, &context) {
+    match tera.value_render(name, context) {
         Ok(string) => Some(string),
         Err(e) => {
             error_!("Error rendering Tera template '{}': {}", name, e);