From c6e426cbacd261b31762b022060c3475506d493e Mon Sep 17 00:00:00 2001
From: Jordan Petridis <jordanpetridis@protonmail.com>
Date: Tue, 6 Feb 2018 20:14:03 +0200
Subject: [PATCH] Downloader: Change cache_image function to return Result<T,
 DownlaoderError> instead of Option<T>.

---
 hammond-downloader/src/downloader.rs | 40 +++++++++++++++-------------
 hammond-downloader/src/errors.rs     | 10 +++++--
 hammond-gtk/src/app.rs               |  4 +--
 hammond-gtk/src/utils.rs             |  3 ++-
 4 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/hammond-downloader/src/downloader.rs b/hammond-downloader/src/downloader.rs
index cb2a94a..4fa9bee 100644
--- a/hammond-downloader/src/downloader.rs
+++ b/hammond-downloader/src/downloader.rs
@@ -192,40 +192,39 @@ pub fn get_episode(
     Ok(())
 }
 
-pub fn cache_image(pd: &PodcastCoverQuery) -> Option<String> {
-    let url = pd.image_uri()?.to_owned();
+pub fn cache_image(pd: &PodcastCoverQuery) -> Result<String, DownloadError> {
+    let url = pd.image_uri()
+        .ok_or_else(|| DownloadError::NoImageLocation)?
+        .to_owned();
+
     if url == "" {
-        return None;
+        return Err(DownloadError::NoImageLocation);
     }
 
-    let cache_download_fold = format!("{}{}", HAMMOND_CACHE.to_str()?, pd.title().to_owned());
+    let cache_path = HAMMOND_CACHE
+        .to_str()
+        .ok_or_else(|| DownloadError::InvalidCacheLocation)?;
+    let cache_download_fold = format!("{}{}", cache_path, pd.title().to_owned());
 
     // Weird glob magic.
     if let Ok(mut foo) = glob(&format!("{}/cover.*", cache_download_fold)) {
         // For some reason there is no .first() method so nth(0) is used
         let path = foo.nth(0).and_then(|x| x.ok());
         if let Some(p) = path {
-            return Some(p.to_str()?.into());
+            return Ok(p.to_str()
+                .ok_or_else(|| DownloadError::InvalidCachedImageLocation)?
+                .into());
         }
     };
 
     // Create the folders if they don't exist.
     DirBuilder::new()
         .recursive(true)
-        .create(&cache_download_fold)
-        .ok()?;
+        .create(&cache_download_fold)?;
 
-    match download_into(&cache_download_fold, "cover", &url, None) {
-        Ok(path) => {
-            info!("Cached img into: {}", &path);
-            Some(path)
-        }
-        Err(err) => {
-            error!("Failed to get feed image.");
-            error!("Error: {}", err);
-            None
-        }
-    }
+    let path = download_into(&cache_download_fold, "cover", &url, None)?;
+    info!("Cached img into: {}", &path);
+    Ok(path)
 }
 
 #[cfg(test)]
@@ -235,6 +234,8 @@ mod tests {
     use hammond_data::dbqueries;
     use hammond_data::pipeline;
 
+    use std::fs;
+
     #[test]
     // This test inserts an rss feed to your `XDG_DATA/hammond/hammond.db` so we make it explicit
     // to run it.
@@ -257,6 +258,7 @@ mod tests {
             HAMMOND_CACHE.to_str().unwrap(),
             pd.title()
         );
-        assert_eq!(img_path, Some(foo_));
+        assert_eq!(img_path.unwrap(), foo_);
+        fs::remove_file(foo_).unwrap();
     }
 }
diff --git a/hammond-downloader/src/errors.rs b/hammond-downloader/src/errors.rs
index 7df3bd1..43d9400 100644
--- a/hammond-downloader/src/errors.rs
+++ b/hammond-downloader/src/errors.rs
@@ -10,10 +10,16 @@ pub enum DownloadError {
     DataError(#[cause] DataError),
     #[fail(display = "Io error: {}", _0)]
     IoError(#[cause] io::Error),
-    #[fail(display = "The Download was cancelled")]
-    DownloadCancelled,
     #[fail(display = "Unexpected server response: {}", _0)]
     UnexpectedResponse(reqwest::StatusCode),
+    #[fail(display = "The Download was cancelled.")]
+    DownloadCancelled,
+    #[fail(display = "Remote Image location not found.")]
+    NoImageLocation,
+    #[fail(display = "Failed to parse CacheLocation.")]
+    InvalidCacheLocation,
+    #[fail(display = "Failed to parse Cached Image Location.")]
+    InvalidCachedImageLocation,
 }
 
 impl From<reqwest::Error> for DownloadError {
diff --git a/hammond-gtk/src/app.rs b/hammond-gtk/src/app.rs
index 8bb95f5..fc48072 100644
--- a/hammond-gtk/src/app.rs
+++ b/hammond-gtk/src/app.rs
@@ -122,9 +122,9 @@ impl App {
             match receiver.recv_timeout(Duration::from_millis(10)) {
                 Ok(Action::UpdateSources(source)) => {
                     if let Some(s) = source {
-                        utils::refresh_feed(Some(vec![s]), sender.clone())
+                        utils::refresh_feed(Some(vec![s]), sender.clone());
                     } else {
-                        utils::refresh_feed(None, sender.clone())
+                        utils::refresh_feed(None, sender.clone());
                     }
                 }
                 Ok(Action::RefreshAllViews) => content.update(),
diff --git a/hammond-gtk/src/utils.rs b/hammond-gtk/src/utils.rs
index f300f18..aa896c7 100644
--- a/hammond-gtk/src/utils.rs
+++ b/hammond-gtk/src/utils.rs
@@ -1,5 +1,6 @@
 #![cfg_attr(feature = "cargo-clippy", allow(type_complexity))]
 
+use failure::Error;
 use gdk_pixbuf::Pixbuf;
 use send_cell::SendCell;
 
@@ -82,7 +83,7 @@ pub fn get_pixbuf_from_path(pd: &PodcastCoverQuery, size: u32) -> Option<Pixbuf>
         }
     }
 
-    let img_path = downloader::cache_image(pd)?;
+    let img_path = downloader::cache_image(pd).ok()?;
     let px = Pixbuf::new_from_file_at_scale(&img_path, size as i32, size as i32, true).ok();
     if let Some(px) = px {
         let mut hashmap = CACHED_PIXBUFS.write().unwrap();