Refactor navigation helpers (#465)

* Refactor navigation helpers

* Target::find: take previous_item by reference

This makes more sense for find as an interface, though it causes a
second clone in some cases. Maybe rustc is smart here?

* Test next and previous navigation helpers

* Add more next/previous tests

src/renderer/html_handlebars/helpers/navigation.rs

@@ -5,165 +5,230 @@ use serde_json;
 use handlebars::{Context, Handlebars, Helper, RenderContext, RenderError, Renderable};
 
 
-// Handlebars helper for navigation
+type StringMap = BTreeMap<String, String>;
 
-pub fn previous(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), RenderError> {
+/// Target for `find_chapter`.
-    debug!("[fn]: previous (handlebars helper)");
+enum Target {
-
+    Previous,
-    debug!("[*]: Get data from context");
+    Next,
-    let chapters = rc.evaluate_absolute("chapters").and_then(|c| {
-        serde_json::value::from_value::<Vec<BTreeMap<String, String>>>(c.clone())
-            .map_err(|_| RenderError::new("Could not decode the JSON data"))
-    })?;
-
-    let current = rc.evaluate_absolute("path")?
-                    .as_str()
-                    .ok_or_else(|| RenderError::new("Type error for `path`, string expected"))?
-                    .replace("\"", "");
-
-    let mut previous: Option<BTreeMap<String, String>> = None;
-
-    debug!("[*]: Search for current Chapter");
-    // Search for current chapter and return previous entry
-    for item in chapters {
-        match item.get("path") {
-            Some(path) if !path.is_empty() => {
-                if path == &current {
-                    debug!("[*]: Found current chapter");
-                    if let Some(previous) = previous {
-                        debug!("[*]: Creating BTreeMap to inject in context");
-                        // Create new BTreeMap to extend the context: 'title' and 'link'
-                        let mut previous_chapter = BTreeMap::new();
-
-                        // Chapter title
-                        previous.get("name")
-                                .ok_or_else(|| {
-                                                RenderError::new("No title found for chapter in \
-                                                                  JSON data")
-                                            })
-                                .and_then(|n| {
-                                              previous_chapter.insert("title".to_owned(), json!(n));
-                                              Ok(())
-                                          })?;
-
-
-                        // Chapter link
-                        previous.get("path")
-                                .ok_or_else(|| {
-                                                RenderError::new("No path found for chapter in \
-                                                                  JSON data")
-                                            })
-                                .and_then(|p| {
-                            Path::new(p).with_extension("html")
-                                        .to_str()
-                                        .ok_or_else(|| {
-                                                        RenderError::new("Link could not be \
-                                                                          converted to str")
-                                                    })
-                                        .and_then(|p| {
-                                previous_chapter
-                                            .insert("link".to_owned(), json!(p.replace("\\", "/")));
-                                Ok(())
-                            })
-                        })?;
-
-
-                        debug!("[*]: Render template");
-                        // Render template
-                        _h.template()
-                          .ok_or_else(|| RenderError::new("Error with the handlebars template"))
-                          .and_then(|t| {
-                            let mut local_rc = rc.with_context(Context::wraps(&previous_chapter)?);
-                            t.render(r, &mut local_rc)
-                        })?;
-                    }
-                    break;
-                } else {
-                    previous = Some(item.clone());
-                }
-            }
-            _ => continue,
-        }
-    }
-
-    Ok(())
 }
 
+impl Target {
+    /// Returns target if found.
+    fn find(&self,
+            base_path: &String,
+            current_path: &String,
+            current_item: &StringMap,
+            previous_item: &StringMap,
+            ) -> Result<Option<StringMap>, RenderError> {
+        match self {
+            &Target::Next => {
+                let previous_path = previous_item.get("path").ok_or_else(|| {
+                    RenderError::new("No path found for chapter in JSON data")
+                })?;
 
+                if previous_path == base_path {
+                    return Ok(Some(current_item.clone()));
+                }
+            },
 
+            &Target::Previous => {
+                if current_path == base_path {
+                    return Ok(Some(previous_item.clone()));
+                }
+            }
+        }
 
-pub fn next(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), RenderError> {
+        Ok(None)
-    debug!("[fn]: next (handlebars helper)");
+    }
+}
 
+fn find_chapter(
+    rc: &mut RenderContext,
+    target: Target
+) -> Result<Option<StringMap>, RenderError> {
     debug!("[*]: Get data from context");
+
     let chapters = rc.evaluate_absolute("chapters").and_then(|c| {
-        serde_json::value::from_value::<Vec<BTreeMap<String, String>>>(c.clone())
+        serde_json::value::from_value::<Vec<StringMap>>(c.clone())
             .map_err(|_| RenderError::new("Could not decode the JSON data"))
     })?;
-    let current = rc.evaluate_absolute("path")?
-                    .as_str()
-                    .ok_or_else(|| RenderError::new("Type error for `path`, string expected"))?
-                    .replace("\"", "");
 
-    let mut previous: Option<BTreeMap<String, String>> = None;
+    let base_path = rc.evaluate_absolute("path")?
+                      .as_str()
+                      .ok_or_else(|| RenderError::new("Type error for `path`, string expected"))?
+                      .replace("\"", "");
+
+    let mut previous: Option<StringMap> = None;
+
+    debug!("[*]: Search for chapter");
 
-    debug!("[*]: Search for current Chapter");
-    // Search for current chapter and return previous entry
     for item in chapters {
         match item.get("path") {
             Some(path) if !path.is_empty() => {
                 if let Some(previous) = previous {
-                    let previous_path = previous.get("path").ok_or_else(|| {
+                    if let Some(item) = target.find(&base_path, &path, &item, &previous)? {
-                        RenderError::new("No path found for chapter in JSON data")
+                        return Ok(Some(item));
-                    })?;
-
-                    if previous_path == &current {
-                        debug!("[*]: Found current chapter");
-                        debug!("[*]: Creating BTreeMap to inject in context");
-                        // Create new BTreeMap to extend the context: 'title' and 'link'
-                        let mut next_chapter = BTreeMap::new();
-
-                        item.get("name")
-                            .ok_or_else(|| {
-                                            RenderError::new("No title found for chapter in JSON \
-                                                              data")
-                                        })
-                            .and_then(|n| {
-                                          next_chapter.insert("title".to_owned(), json!(n));
-                                          Ok(())
-                                      })?;
-
-                        Path::new(path).with_extension("html")
-                                       .to_str()
-                                       .ok_or_else(|| {
-                                                       RenderError::new("Link could not converted \
-                                                                         to str")
-                                                   })
-                                       .and_then(|l| {
-                            debug!("[*]: Inserting link: {:?}", l);
-                            // Hack for windows who tends to use `\` as separator instead of `/`
-                            next_chapter.insert("link".to_owned(), json!(l.replace("\\", "/")));
-                            Ok(())
-                        })?;
-
-                        debug!("[*]: Render template");
-
-                        // Render template
-                        _h.template()
-                          .ok_or_else(|| RenderError::new("Error with the handlebars template"))
-                          .and_then(|t| {
-                            let mut local_rc = rc.with_context(Context::wraps(&next_chapter)?);
-                            t.render(r, &mut local_rc)
-                        })?;
-                        break;
                     }
                 }
 
                 previous = Some(item.clone());
             }
-
             _ => continue,
         }
     }
+
+   Ok(None)
+}
+
+fn render(
+    _h: &Helper,
+    r: &Handlebars,
+    rc: &mut RenderContext,
+    chapter: &StringMap,
+) -> Result<(), RenderError> {
+    debug!("[*]: Creating BTreeMap to inject in context");
+
+    let mut context = BTreeMap::new();
+
+    chapter.get("name")
+            .ok_or_else(|| RenderError::new("No title found for chapter in JSON data"))
+            .map(|name| context.insert("title".to_owned(), json!(name)))?;
+
+    chapter.get("path")
+            .ok_or_else(|| RenderError::new("No path found for chapter in JSON data"))
+            .and_then(|p| {
+                    Path::new(p).with_extension("html")
+                    .to_str()
+                    .ok_or_else(|| RenderError::new("Link could not be converted to str"))
+                    .map(|p| context.insert("link".to_owned(), json!(p.replace("\\", "/"))))
+            })?;
+
+    debug!("[*]: Render template");
+
+    _h.template()
+        .ok_or_else(|| RenderError::new("Error with the handlebars template"))
+        .and_then(|t| {
+            let mut local_rc = rc.with_context(Context::wraps(&context)?);
+            t.render(r, &mut local_rc)
+        })?;
+
     Ok(())
 }
+
+pub fn previous(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), RenderError> {
+    debug!("[fn]: previous (handlebars helper)");
+
+    if let Some(previous) = find_chapter(rc, Target::Previous)? {
+        render(_h, r, rc, &previous)?;
+    }
+
+    Ok(())
+}
+
+pub fn next(_h: &Helper, r: &Handlebars, rc: &mut RenderContext) -> Result<(), RenderError> {
+    debug!("[fn]: next (handlebars helper)");
+
+    if let Some(next) = find_chapter(rc, Target::Next)? {
+        render(_h, r, rc, &next)?;
+    }
+
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+   use super::*;
+
+   static TEMPLATE: &'static str =
+      "{{#previous}}{{title}}: {{link}}{{/previous}}|{{#next}}{{title}}: {{link}}{{/next}}";
+
+   #[test]
+   fn test_next_previous() {
+      let data = json!({
+         "name": "two",
+         "path": "two.path",
+         "chapters": [
+            {
+               "name": "one",
+               "path": "one.path"
+            },
+            {
+               "name": "two",
+               "path": "two.path",
+            },
+            {
+               "name": "three",
+               "path": "three.path"
+            }
+         ]
+      });
+
+      let mut h = Handlebars::new();
+      h.register_helper("previous", Box::new(previous));
+      h.register_helper("next", Box::new(next));
+
+      assert_eq!(
+         h.template_render(TEMPLATE, &data).unwrap(),
+         "one: one.html|three: three.html");
+   }
+
+   #[test]
+   fn test_first() {
+      let data = json!({
+         "name": "one",
+         "path": "one.path",
+         "chapters": [
+            {
+               "name": "one",
+               "path": "one.path"
+            },
+            {
+               "name": "two",
+               "path": "two.path",
+            },
+            {
+               "name": "three",
+               "path": "three.path"
+            }
+         ]
+      });
+
+      let mut h = Handlebars::new();
+      h.register_helper("previous", Box::new(previous));
+      h.register_helper("next", Box::new(next));
+
+      assert_eq!(
+         h.template_render(TEMPLATE, &data).unwrap(),
+         "|two: two.html");
+   }
+   #[test]
+   fn test_last() {
+      let data = json!({
+         "name": "three",
+         "path": "three.path",
+         "chapters": [
+            {
+               "name": "one",
+               "path": "one.path"
+            },
+            {
+               "name": "two",
+               "path": "two.path",
+            },
+            {
+               "name": "three",
+               "path": "three.path"
+            }
+         ]
+      });
+
+      let mut h = Handlebars::new();
+      h.register_helper("previous", Box::new(previous));
+      h.register_helper("next", Box::new(next));
+
+      assert_eq!(
+         h.template_render(TEMPLATE, &data).unwrap(),
+         "two: two.html|");
+   }
+}