Rewrite link_checker to use a Result internally (#928)

components/link_checker/src/lib.rs

@@ -3,50 +3,33 @@ use reqwest::header::{HeaderMap, ACCEPT};
 use reqwest::{blocking::Client, StatusCode};
 
 use config::LinkChecker;
-use errors::Result;
 
 use std::collections::HashMap;
+use std::result;
 use std::sync::{Arc, RwLock};
 
-#[derive(Clone, Debug, PartialEq)]
+pub type Result = result::Result<StatusCode, String>;
-pub struct LinkResult {
+
-    pub code: Option<StatusCode>,
+pub fn is_valid(res: &Result) -> bool {
-    /// Whether the HTTP request didn't make it to getting a HTTP code
+    match res {
-    pub error: Option<String>,
+        Ok(ref code) => code.is_success() || *code == StatusCode::NOT_MODIFIED,
+        Err(_) => false,
+    }
 }
 
-impl LinkResult {
+pub fn message(res: &Result) -> String {
-    pub fn is_valid(&self) -> bool {
+    match res {
-        if self.error.is_some() {
+        Ok(ref code) => format!("{}", code),
-            return false;
+        Err(ref error) => error.clone(),
-        }
-
-        if let Some(c) = self.code {
-            return c.is_success() || c == StatusCode::NOT_MODIFIED;
-        }
-
-        true
-    }
-
-    pub fn message(&self) -> String {
-        if let Some(ref e) = self.error {
-            return e.clone();
-        }
-
-        if let Some(c) = self.code {
-            return format!("{}", c);
-        }
-
-        "Unknown error".to_string()
     }
 }
 
 lazy_static! {
     // Keep history of link checks so a rebuild doesn't have to check again
-    static ref LINKS: Arc<RwLock<HashMap<String, LinkResult>>> = Arc::new(RwLock::new(HashMap::new()));
+    static ref LINKS: Arc<RwLock<HashMap<String, Result>>> = Arc::new(RwLock::new(HashMap::new()));
 }
 
-pub fn check_url(url: &str, config: &LinkChecker) -> LinkResult {
+pub fn check_url(url: &str, config: &LinkChecker) -> Result {
     {
         let guard = LINKS.read().unwrap();
         if let Some(res) = guard.get(url) {
@@ -75,13 +58,13 @@ pub fn check_url(url: &str, config: &LinkChecker) -> LinkResult {
             };
 
             match check_page_for_anchor(url, body) {
-                Ok(_) => LinkResult { code: Some(response.status()), error: None },
+                Ok(_) => Ok(response.status()),
-                Err(e) => LinkResult { code: None, error: Some(e.to_string()) },
+                Err(e) => Err(e.to_string()),
             }
         }
         Ok(response) => {
             if response.status().is_success() || response.status() == StatusCode::NOT_MODIFIED {
-                LinkResult { code: Some(response.status()), error: None }
+                Ok(response.status())
             } else {
                 let error_string = if response.status().is_informational() {
                     format!("Informational status code ({}) received", response.status())
@@ -95,10 +78,10 @@ pub fn check_url(url: &str, config: &LinkChecker) -> LinkResult {
                     format!("Non-success status code ({}) received", response.status())
                 };
 
-                LinkResult { code: None, error: Some(error_string) }
+                Err(error_string)
             }
         }
-        Err(e) => LinkResult { code: None, error: Some(e.to_string()) },
+        Err(e) => Err(e.to_string()),
     };
 
     LINKS.write().unwrap().insert(url.to_string(), res.clone());
@@ -115,7 +98,7 @@ fn has_anchor(url: &str) -> bool {
     }
 }
 
-fn check_page_for_anchor(url: &str, body: String) -> Result<()> {
+fn check_page_for_anchor(url: &str, body: String) -> errors::Result<()> {
     let index = url.find('#').unwrap();
     let anchor = url.get(index + 1..).unwrap();
     let checks: [String; 8] = [
@@ -138,7 +121,9 @@ fn check_page_for_anchor(url: &str, body: String) -> Result<()> {
 
 #[cfg(test)]
 mod tests {
-    use super::{check_page_for_anchor, check_url, has_anchor, LinkChecker, LINKS};
+    use super::{
+        check_page_for_anchor, check_url, has_anchor, is_valid, message, LinkChecker, LINKS,
+    };
     use mockito::mock;
 
     // NOTE: HTTP mock paths below are randomly generated to avoid name
@@ -167,7 +152,8 @@ mod tests {
             .create();
 
         let res = check_url(&url, &LinkChecker::default());
-        assert!(res.is_valid());
+        assert!(is_valid(&res));
+        assert_eq!(message(&res), "200 OK");
         assert!(LINKS.read().unwrap().get(&url).is_some());
     }
 
@@ -187,9 +173,9 @@ mod tests {
 
         let url = format!("{}{}", mockito::server_url(), "/c7qrtrv3zz");
         let res = check_url(&url, &LinkChecker::default());
-        assert!(res.is_valid());
+        assert!(is_valid(&res));
-        assert!(res.code.is_some());
+        assert!(res.is_ok());
-        assert!(res.error.is_none());
+        assert_eq!(message(&res), "200 OK");
     }
 
     #[test]
@@ -225,9 +211,9 @@ mod tests {
 
         let url = format!("{}{}", mockito::server_url(), "/cav9vibhsc");
         let res = check_url(&url, &LinkChecker::default());
-        assert_eq!(res.is_valid(), false);
+        assert!(!is_valid(&res));
-        assert!(res.code.is_none());
+        assert!(res.is_err());
-        assert!(res.error.is_some());
+        assert_eq!(message(&res), "Client error status code (404 Not Found) received");
     }
 
     #[test]
@@ -240,9 +226,9 @@ mod tests {
 
         let url = format!("{}{}", mockito::server_url(), "/nlhab9c1vc");
         let res = check_url(&url, &LinkChecker::default());
-        assert_eq!(res.is_valid(), false);
+        assert!(!is_valid(&res));
-        assert!(res.code.is_none());
+        assert!(res.is_err());
-        assert!(res.error.is_some());
+        assert_eq!(message(&res), "Client error status code (404 Not Found) received");
     }
 
     #[test]
@@ -255,17 +241,18 @@ mod tests {
 
         let url = format!("{}{}", mockito::server_url(), "/qdbrssazes");
         let res = check_url(&url, &LinkChecker::default());
-        assert_eq!(res.is_valid(), false);
+        assert!(!is_valid(&res));
-        assert!(res.code.is_none());
+        assert!(res.is_err());
-        assert!(res.error.is_some());
+        assert_eq!(message(&res), "Server error status code (500 Internal Server Error) received");
     }
 
     #[test]
     fn can_fail_unresolved_links() {
         let res = check_url("https://t6l5cn9lpm.lxizfnzckd", &LinkChecker::default());
-        assert_eq!(res.is_valid(), false);
+        assert!(!is_valid(&res));
-        assert!(res.code.is_none());
+        assert!(res.is_err());
-        assert!(res.error.is_some());
+        assert!(message(&res)
+            .starts_with("error sending request for url (https://t6l5cn9lpm.lxizfnzckd/)"));
     }
 
     #[test]
@@ -312,29 +299,25 @@ mod tests {
     #[test]
     fn can_check_url_for_anchor() {
         let url = "https://doc.rust-lang.org/std/index.html#the-rust-standard-library";
-        let res = has_anchor(url);
+        assert!(has_anchor(url));
-        assert_eq!(res, true);
     }
 
     #[test]
     fn will_return_false_when_no_anchor() {
         let url = "https://doc.rust-lang.org/std/index.html";
-        let res = has_anchor(url);
+        assert!(!has_anchor(url));
-        assert_eq!(res, false);
     }
 
     #[test]
     fn will_return_false_when_has_router_url() {
         let url = "https://doc.rust-lang.org/#/std";
-        let res = has_anchor(url);
+        assert!(!has_anchor(url));
-        assert_eq!(res, false);
     }
 
     #[test]
     fn will_return_false_when_has_router_url_alt() {
         let url = "https://doc.rust-lang.org/#!/std";
-        let res = has_anchor(url);
+        assert!(!has_anchor(url));
-        assert_eq!(res, false);
     }
 
     #[test]
@@ -360,7 +343,7 @@ mod tests {
 
         // anchor check is ignored because the url matches the prefix
         let ignore = format!("{}{}", mockito::server_url(), "/ignore/i30hobj1cy#nonexistent");
-        assert!(check_url(&ignore, &config).is_valid());
+        assert!(is_valid(&check_url(&ignore, &config)));
 
         let _m2 = mock("GET", "/guvqcqwmth")
             .with_header("Content-Type", "text/html")
@@ -380,9 +363,9 @@ mod tests {
 
         // other anchors are checked
         let existent = format!("{}{}", mockito::server_url(), "/guvqcqwmth#existent");
-        assert!(check_url(&existent, &config).is_valid());
+        assert!(is_valid(&check_url(&existent, &config)));
 
         let nonexistent = format!("{}{}", mockito::server_url(), "/guvqcqwmth#nonexistent");
-        assert_eq!(check_url(&nonexistent, &config).is_valid(), false);
+        assert!(!is_valid(&check_url(&nonexistent, &config)));
     }
 }

components/site/src/lib.rs

@@ -18,7 +18,6 @@ use library::{
     find_taxonomies, sort_actual_pages_by_date, Library, Page, Paginator, Section, Taxonomy,
     TaxonomyItem,
 };
-use link_checker::check_url;
 use templates::{global_fns, render_redirect_template, ZOLA_TERA};
 use utils::fs::{copy_directory, create_directory, create_file, ensure_directory_exists};
 use utils::net::get_available_port;
@@ -415,8 +414,8 @@ impl Site {
                     {
                         return None;
                     }
-                    let res = check_url(&link, &self.config.link_checker);
+                    let res = link_checker::check_url(&link, &self.config.link_checker);
-                    if res.is_valid() {
+                    if link_checker::is_valid(&res) {
                         None
                     } else {
                         Some((page_path, link, res))
@@ -442,7 +441,7 @@ impl Site {
                     "Dead link in {} to {}: {}",
                     page_path.to_string_lossy(),
                     link,
-                    check_res.message()
+                    link_checker::message(&check_res)
                 )
             })
             .collect::<Vec<_>>()