Made locations in cache optional

The locations of credentials, volume and audio are now stored
in three separate Optional<PathBuf>s.
Removed the clearing of the cache if an error occurs. This might
be added again later.

audio/src/fetch.rs

@@ -430,9 +430,7 @@ impl AudioFile {
                 .map(move |mut file| {
                     if let Some(cache) = session_.cache() {
                         debug!("File {} complete, saving to cache", file_id);
-                        if let Err(err) = cache.save_file(file_id, &mut file) {
+                        cache.save_file(file_id, &mut file);
-                            warn!("Cannot save file to cache: {}", err);
-                        }
                     } else {
                         debug!("File {} complete", file_id);
                     }

core/src/cache.rs

@@ -7,59 +7,58 @@ use crate::authentication::Credentials;
 use crate::spotify_id::FileId;
 use crate::volume::Volume;
 
+/// A cache for volume, credentials and audio files.
 #[derive(Clone)]
 pub struct Cache {
-    audio_root: PathBuf,
+    credentials_location: Option<PathBuf>,
-    system_root: PathBuf,
+    volume_location: Option<PathBuf>,
-    use_audio_cache: bool,
+    audio_location: Option<PathBuf>,
-}
-
-fn mkdir_existing(path: &Path) -> io::Result<()> {
-    fs::create_dir(path).or_else(|err| {
-        if err.kind() == io::ErrorKind::AlreadyExists {
-            Ok(())
-        } else {
-            Err(err)
-        }
-    })
 }
 
 impl Cache {
-    pub fn new(
+    pub fn new<P: AsRef<Path>>(
-        audio_location: PathBuf,
+        system_location: Option<P>,
-        system_location: PathBuf,
+        audio_location: Option<P>,
-        use_audio_cache: bool,
+    ) -> io::Result<Self> {
-    ) -> io::Result<Cache> {
+        if let Some(location) = &system_location {
-        if use_audio_cache {
+            fs::create_dir_all(location)?;
-            mkdir_existing(&audio_location)?;
-            mkdir_existing(&audio_location.join("files"))?;
         }
-        mkdir_existing(&system_location)?;
 
-        Ok(Cache {
+        if let Some(location) = &audio_location {
-            audio_root: audio_location,
+            fs::create_dir_all(location)?;
-            system_root: system_location,
+        }
-            use_audio_cache,
-        })
-    }
-}
 
-impl Cache {
+        let audio_location = audio_location.map(|p| p.as_ref().to_owned());
-    fn open_credentials_file(&self) -> io::Result<File> {
+        let volume_location = system_location.as_ref().map(|p| p.as_ref().join("volume"));
-        File::open(self.system_root.join("credentials.json"))
+        let credentials_location = system_location
-    }
+            .as_ref()
+            .map(|p| p.as_ref().join("credentials.json"));
 
-    fn read_credentials(&self) -> io::Result<Credentials> {
+        let cache = Cache {
-        let mut file = self.open_credentials_file()?;
+            credentials_location,
-        let mut contents = String::new();
+            volume_location,
-        file.read_to_string(&mut contents)?;
+            audio_location,
-        serde_json::from_str(&contents).map_err(|e| Error::new(ErrorKind::InvalidData, e))
+        };
+
+        Ok(cache)
     }
 
     pub fn credentials(&self) -> Option<Credentials> {
-        match self.read_credentials() {
+        let location = self.credentials_location.as_ref()?;
+
+        // This closure is just convencience to enable the question mark operator
+        let read = || {
+            let mut file = File::open(location)?;
+            let mut contents = String::new();
+            file.read_to_string(&mut contents)?;
+            serde_json::from_str(&contents).map_err(|e| Error::new(ErrorKind::InvalidData, e))
+        };
+
+        match read() {
             Ok(c) => Some(c),
             Err(e) => {
+                // If the file did not exist, the file was probably not written
+                // before. Otherwise, log the error.
                 if e.kind() != ErrorKind::NotFound {
                     warn!("Error reading credentials from cache: {}", e);
                 }
@@ -69,32 +68,31 @@ impl Cache {
     }
 
     pub fn save_credentials(&self, cred: &Credentials) {
-        let result = self
+        if let Some(location) = &self.credentials_location {
-            .open_credentials_file()
+            let result = File::create(location).and_then(|mut file| {
-            .and_then(|mut file| write!(file, "{}", serde_json::to_string(cred)?));
+                let data = serde_json::to_string(cred)?;
-        if let Err(e) = result {
+                write!(file, "{}", data)
-            warn!("Cannot save credentials to cache: {}", e);
+            });
+
+            if let Err(e) = result {
+                warn!("Cannot save credentials to cache: {}", e)
+            }
         }
     }
-}
-
-// cache volume to system_root/volume
-impl Cache {
-    fn open_volume_file(&self) -> io::Result<File> {
-        File::open(self.system_root.join("volume"))
-    }
-
-    fn read_volume(&self) -> io::Result<Volume> {
-        let mut file = self.open_volume_file()?;
-        let mut contents = String::new();
-        file.read_to_string(&mut contents)?;
-        contents
-            .parse()
-            .map_err(|e| Error::new(ErrorKind::InvalidData, e))
-    }
 
     pub fn volume(&self) -> Option<Volume> {
-        match self.read_volume() {
+        let location = self.volume_location.as_ref()?;
+
+        let read = || {
+            let mut file = File::open(location)?;
+            let mut contents = String::new();
+            file.read_to_string(&mut contents)?;
+            contents
+                .parse()
+                .map_err(|e| Error::new(ErrorKind::InvalidData, e))
+        };
+
+        match read() {
             Ok(v) => Some(v),
             Err(e) => {
                 if e.kind() != ErrorKind::NotFound {
@@ -106,26 +104,25 @@ impl Cache {
     }
 
     pub fn save_volume(&self, volume: Volume) {
-        let result = self
+        if let Some(ref location) = self.volume_location {
-            .open_volume_file()
+            let result = File::create(location).and_then(|mut file| write!(file, "{}", volume));
-            .and_then(|mut file| write!(file, "{}", volume));
+            if let Err(e) = result {
-        if let Err(e) = result {
+                warn!("Cannot save volume to cache: {}", e);
-            warn!("Cannot save volume to cache: {}", e);
+            }
         }
     }
-}
 
-impl Cache {
+    fn file_path(&self, file: FileId) -> Option<PathBuf> {
-    fn file_path(&self, file: FileId) -> PathBuf {
+        self.audio_location.as_ref().map(|location| {
-        let name = file.to_base16();
+            let name = file.to_base16();
-        self.audio_root
+            let mut path = location.join(&name[0..2]);
-            .join("files")
+            path.push(&name[2..]);
-            .join(&name[0..2])
+            path
-            .join(&name[2..])
+        })
     }
 
     pub fn file(&self, file: FileId) -> Option<File> {
-        File::open(self.file_path(file))
+        File::open(self.file_path(file)?)
             .map_err(|e| {
                 if e.kind() != ErrorKind::NotFound {
                     warn!("Error reading file from cache: {}", e)
@@ -134,30 +131,16 @@ impl Cache {
             .ok()
     }
 
-    pub fn save_file<F: Read>(&self, file: FileId, contents: &mut F) -> io::Result<()> {
+    pub fn save_file<F: Read>(&self, file: FileId, contents: &mut F) {
-        if self.use_audio_cache {
+        if let Some(path) = self.file_path(file) {
-            let path = self.file_path(file);
+            let parent = path.parent().unwrap();
-            mkdir_existing(path.parent().unwrap())?;
+            let result = fs::create_dir_all(parent)
+                .and_then(|_| File::create(path))
+                .and_then(|mut file| io::copy(contents, &mut file));
 
-            let mut cache_file = File::create(path).or_else(|_| {
+            if let Err(e) = result {
-                fs::remove_dir_all(&self.audio_root.join("files"))?;
+                warn!("Cannot save file to cache: {}", e)
-                mkdir_existing(&self.audio_root.join("files"))?;
+            }
-
-                let path = self.file_path(file);
-                mkdir_existing(path.parent().unwrap())?;
-                File::create(path)
-            })?;
-
-            io::copy(contents, &mut cache_file).or_else(|_| {
-                fs::remove_dir_all(&self.audio_root.join("files"))?;
-                mkdir_existing(&self.audio_root.join("files"))?;
-
-                let path = self.file_path(file);
-                mkdir_existing(path.parent().unwrap())?;
-                let mut file = File::create(path)?;
-                io::copy(contents, &mut file)
-            })?;
         }
-        Ok(())
     }
 }

src/main.rs

@@ -5,7 +5,7 @@ use sha1::{Digest, Sha1};
 use std::env;
 use std::io::{self, stderr, Write};
 use std::mem;
-use std::path::PathBuf;
+use std::path::Path;
 use std::process::exit;
 use std::str::FromStr;
 use std::time::Instant;
@@ -254,22 +254,32 @@ fn setup(args: &[String]) -> Setup {
         mapped_volume: !matches.opt_present("mixer-linear-volume"),
     };
 
-    let use_audio_cache = !matches.opt_present("disable-audio-cache");
+    let cache = {
+        let audio_dir;
+        let system_dir;
+        if matches.opt_present("disable-audio-cache") {
+            audio_dir = None;
+            system_dir = matches
+                .opt_str("system-cache")
+                .or_else(|| matches.opt_str("c"))
+                .map(|p| p.into());
+        } else {
+            let cache_dir = matches.opt_str("c");
+            audio_dir = cache_dir
+                .as_ref()
+                .map(|p| AsRef::<Path>::as_ref(p).join("files"));
+            system_dir = matches
+                .opt_str("system-cache")
+                .or_else(|| cache_dir)
+                .map(|p| p.into());
+        }
 
-    let cache_directory = matches.opt_str("c").unwrap_or(String::from(""));
+        match Cache::new(system_dir, audio_dir) {
-    let system_cache_directory = matches
+            Ok(cache) => Some(cache),
-        .opt_str("system-cache")
+            Err(e) => {
-        .unwrap_or(String::from(cache_directory.clone()));
+                warn!("Cannot create cache: {}", e);
-
+                None
-    let cache = match Cache::new(
+            }
-        PathBuf::from(cache_directory),
-        PathBuf::from(system_cache_directory),
-        use_audio_cache,
-    ) {
-        Ok(cache) => Some(cache),
-        Err(e) => {
-            warn!("Cannot create cache: {}", e);
-            None
         }
     };