template reload tweaks and documentation

contrib/lib/src/templates/fairing.rs

@@ -20,7 +20,7 @@ mod context {
             ContextManager(ctxt)
         }
 
-        pub fn get<'a>(&'a self) -> impl Deref<Target=Context> + 'a {
+        pub fn context<'a>(&'a self) -> impl Deref<Target=Context> + 'a {
             &self.0
         }
     }
@@ -55,13 +55,13 @@ mod context {
                 if watcher.watch(ctxt.root.clone(), RecursiveMode::Recursive).is_ok() {
                     Some((watcher, Mutex::new(rx)))
                 } else {
-                    warn!("Could not monitor the templates directory for changes.");
-                    warn!("Live template reload will be unavailable");
+                    warn_!("Could not monitor the templates directory for changes.");
+                    warn_!("Live template reload will be unavailable");
                     None
                 }
             } else {
-                warn!("Could not instantiate a filesystem watcher.");
-                warn!("Live template reload will be unavailable");
+                warn_!("Could not instantiate a filesystem watcher.");
+                warn_!("Live template reload will be unavailable");
                 None
             };
 
@@ -71,14 +71,18 @@ mod context {
             }
         }
 
-        pub fn get<'a>(&'a self) -> impl Deref<Target=Context> + 'a {
+        pub fn context<'a>(&'a self) -> impl Deref<Target=Context> + 'a {
             self.context.read().unwrap()
         }
 
-        fn get_mut<'a>(&'a self) -> impl DerefMut<Target=Context> + 'a {
+        fn context_mut<'a>(&'a self) -> impl DerefMut<Target=Context> + 'a {
             self.context.write().unwrap()
         }
 
+        /// Checks whether any template files have changed on disk.  If
+        /// there have been changes since the last reload, all templates
+        /// are reinitialized from disk and the user's customization
+        /// callback is run again.
         pub fn reload_if_needed<F: Fn(&mut Engines)>(&self, custom_callback: F) {
             self.watcher.as_ref().map(|w| {
                 let rx = w.1.lock().expect("receive queue");
@@ -88,14 +92,14 @@ mod context {
                 }
 
                 if changed {
-                    warn!("Change detected, reloading templates");
-                    let mut ctxt = self.get_mut();
+                    info!("Change detected, reloading templates");
+                    let mut ctxt = self.context_mut();
                     if let Some(mut new_ctxt) = Context::initialize(ctxt.root.clone()) {
                         custom_callback(&mut new_ctxt.engines);
                         *ctxt = new_ctxt;
                     } else {
-                        warn!("An error occurred while reloading templates.");
-                        warn!("The previous templates will remain active.");
+                        warn_!("An error occurred while reloading templates.");
+                        warn_!("The previous templates will remain active.");
                     };
                 }
             });
@@ -105,18 +109,18 @@ mod context {
 
 pub use self::context::ContextManager;
 
+/// The TemplateFairing initializes the template system on attach,
+/// running custom_callback after templates have been loaded.
+/// In debug mode, the fairing checks for modifications to templates
+/// before every request and reloads them if necessary.
 pub struct TemplateFairing {
-    custom_callback: Box<Fn(&mut Engines) + Send + Sync + 'static>,
-}
-
-impl TemplateFairing {
-    pub fn new(custom_callback: Box<Fn(&mut Engines) + Send + Sync + 'static>) -> TemplateFairing {
-        TemplateFairing { custom_callback }
-    }
+    pub(crate) custom_callback: Box<Fn(&mut Engines) + Send + Sync + 'static>,
 }
 
 impl Fairing for TemplateFairing {
     fn info(&self) -> Info {
+        // The on_request part of this fairing only applies in debug
+        // mode, so only register it in debug mode for better performance.
         Info {
             name: "Templates",
             #[cfg(debug_assertions)]
@@ -126,6 +130,11 @@ impl Fairing for TemplateFairing {
         }
     }
 
+    /// Initializes the template context. Templates will be searched for in the
+    /// `template_dir` config variable or the default ([DEFAULT_TEMPLATE_DIR]).
+    /// The user's callback, if any was supplied, is called to customize the
+    /// template engines. In debug mode, the `ContextManager::new` method
+    /// initializes a directory watcher for auto-reloading of templates.
     fn on_attach(&self, rocket: Rocket) -> Result<Rocket, Rocket> {
         let mut template_root = rocket.config().root_relative(DEFAULT_TEMPLATE_DIR);
         match rocket.config().get_str("template_dir") {
@@ -148,7 +157,8 @@ impl Fairing for TemplateFairing {
 
     #[cfg(debug_assertions)]
     fn on_request(&self, req: &mut ::rocket::Request, _data: &::rocket::Data) {
-        let cm = req.guard::<::rocket::State<ContextManager>>().unwrap();
+        let cm = req.guard::<::rocket::State<ContextManager>>()
+            .expect("Template ContextManager registered in on_attach");
         cm.reload_if_needed(&*self.custom_callback);
     }
 }

contrib/lib/src/templates/metadata.rs

@@ -65,7 +65,7 @@ impl<'a> TemplateMetadata<'a> {
     /// }
     /// ```
     pub fn contains_template(&self, name: &str) -> bool {
-        self.0.get().templates.contains_key(name)
+        self.0.context().templates.contains_key(name)
     }
 }
 

contrib/lib/src/templates/mod.rs

@@ -210,7 +210,7 @@ impl Template {
     pub fn custom<F>(f: F) -> impl Fairing
         where F: Fn(&mut Engines) + Send + Sync + 'static
     {
-        TemplateFairing::new(Box::new(f))
+        TemplateFairing { custom_callback: Box::new(f) }
     }
 
     /// Render the template named `name` with the context `context`. The
@@ -275,7 +275,7 @@ impl Template {
     pub fn show<S, C>(rocket: &Rocket, name: S, context: C) -> Option<String>
         where S: Into<Cow<'static, str>>, C: Serialize
     {
-        let ctxt = rocket.state::<ContextManager>().map(ContextManager::get).or_else(|| {
+        let ctxt = rocket.state::<ContextManager>().map(ContextManager::context).or_else(|| {
             warn!("Uninitialized template context: missing fairing.");
             info!("To use templates, you must attach `Template::fairing()`.");
             info!("See the `Template` documentation for more information.");
@@ -315,13 +315,12 @@ impl Template {
 /// rendering fails, an `Err` of `Status::InternalServerError` is returned.
 impl Responder<'static> for Template {
     fn respond_to(self, req: &Request) -> response::Result<'static> {
-        let cm = req.guard::<State<ContextManager>>().succeeded().ok_or_else(|| {
+        let ctxt = req.guard::<State<ContextManager>>().succeeded().ok_or_else(|| {
             error_!("Uninitialized template context: missing fairing.");
             info_!("To use templates, you must attach `Template::fairing()`.");
             info_!("See the `Template` documentation for more information.");
             Status::InternalServerError
-        })?;
-        let ctxt = cm.get();
+        })?.inner().context();
 
         let (render, content_type) = self.finalize(&ctxt)?;
         Content(content_type, render).respond_to(req)

contrib/lib/tests/templates.rs

@@ -117,31 +117,34 @@ mod templates_tests {
             assert_eq!(response.status(), Status::NotFound);
         }
 
-        #[cfg(debug_assertions)]
         #[test]
+        #[cfg(debug_assertions)]
         fn test_template_reload() {
-            use rocket::local::Client;
             use std::fs::File;
             use std::io::Write;
             use std::path::Path;
             use std::thread;
             use std::time::Duration;
 
+            use rocket::local::Client;
+
+            const RELOAD_TEMPLATE: &str = "hbs/reload";
+            const INITIAL_TEXT: &str = "initial";
+            const NEW_TEXT: &str = "reload";
+
             fn write_file(path: &Path, text: &str) {
                 let mut file = File::create(path).expect("open file");
                 file.write_all(text.as_bytes()).expect("write file");
                 file.sync_all().expect("sync file");
             }
 
-            const RELOAD_TEMPLATE: &'static str = "hbs/reload";
-            const INITIAL_TEXT: &'static str = "initial";
-            const NEW_TEXT: &'static str = "reload";
-
             let reload_path = Path::join(
                 Path::new(env!("CARGO_MANIFEST_DIR")),
                 "tests/templates/hbs/reload.txt.hbs"
             );
 
+            // set up the template before initializing the Rocket instance so
+            // that it will be picked up in the initial loading of templates.
             write_file(&reload_path, INITIAL_TEXT);
 
             let client = Client::new(rocket()).unwrap();
@@ -160,6 +163,8 @@ mod templates_tests {
                 // if the new content is correct, we are done
                 let new_rendered = Template::show(client.rocket(), RELOAD_TEMPLATE, ());
                 if new_rendered == Some(NEW_TEXT.into()) {
+                    // TODO: deleting the file can break concurrent tests,
+                    // but not deleting the file will leave it as an untracked file in git
                     return;
                 }