Fix: Entries::in_collection() should be able to return error

This patch changes the Entries::in_collection() interface to return a
Result<()>. This is needed because the fs backend implementation should
be able to check whether a directory actually exists whenever we change
the iterator.

If the implementation detects that the directory does not exist, we can
fail early and error out.

All usages of the interface are adapted by the patch as well.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>

lib/core/libimagstore/src/file_abstraction/fs.rs

@@ -188,10 +188,16 @@ impl PathIterBuilder for WalkDirPathIterBuilder {
             }))
     }
 
-    fn in_collection(&mut self, c: &str) {
+    fn in_collection(&mut self, c: &str) -> Result<()> {
         debug!("Altering PathIterBuilder path with: {:?}", c);
         self.basepath.push(c);
         debug!(" -> path : {:?}", self.basepath);
+
+        if !self.basepath.exists() {
+            Err(format_err!("Does not exist: {}", self.basepath.display()))
+        } else {
+            Ok(())
+        }
     }
 }
 

lib/core/libimagstore/src/file_abstraction/inmemory.rs

@@ -211,10 +211,11 @@ impl PathIterBuilder for InMemPathIterBuilder {
         Box::new(self.0.clone().into_iter().map(Ok))
     }
 
-    fn in_collection(&mut self, c: &str) {
+    fn in_collection(&mut self, c: &str) -> Result<()> {
         debug!("Altering PathIterBuilder path with: {:?}", c);
         self.0.retain(|p| p.starts_with(c));
         debug!(" -> path : {:?}", self.0);
+        Ok(())
     }
 }
 

lib/core/libimagstore/src/file_abstraction/iter.rs

@@ -29,7 +29,7 @@ use crate::file_abstraction::FileAbstraction;
 /// See documentation for PathIterator
 pub(crate) trait PathIterBuilder : Debug {
     fn build_iter(&self) -> Box<Iterator<Item = Result<PathBuf>>>;
-    fn in_collection(&mut self, c: &str);
+    fn in_collection(&mut self, c: &str) -> Result<()>;
 }
 
 /// A wrapper for an iterator over `PathBuf`s
@@ -65,12 +65,12 @@ impl<'a> PathIterator<'a> {
         PathIterator { iter_builder, iter, storepath, backend }
     }
 
-    pub fn in_collection(mut self, c: &str) -> Self {
+    pub fn in_collection(mut self, c: &str) -> Result<Self> {
         trace!("Generating iterator object for collection: {}", c);
-        self.iter_builder.in_collection(c);
+        self.iter_builder.in_collection(c)?;
         self.iter = self.iter_builder.build_iter();
         trace!("Set new iterator");
-        self
+        Ok(self)
     }
 
     /// Turn iterator into its internals

lib/core/libimagstore/src/iter.rs

@@ -161,7 +161,7 @@ use failure::Fallible as Result;
 /// ```ignore
 ///     store
 ///         .entries()?
-///         .in_collection("foo")
+///         .in_collection("foo")?
 ///         .chain(store.entries()?.in_collection("bar"))
 /// ```
 ///
@@ -175,8 +175,8 @@ impl<'a> Entries<'a> {
         Entries(pi, store)
     }
 
-    pub fn in_collection(self, c: &str) -> Self {
+    pub fn in_collection(self, c: &str) -> Result<Self> {
-        Entries(self.0.in_collection(c), self.1)
+        Ok(Entries(self.0.in_collection(c)?, self.1))
     }
 
     /// Turn `Entries` iterator into generic `StoreIdIterator`
@@ -275,6 +275,7 @@ mod tests {
         let succeeded = store.entries()
             .unwrap()
             .in_collection("coll_3")
+            .unwrap()
             .map(|id| { debug!("Processing id = {:?}", id); id })
             .all(|id| id.unwrap().is_in_collection(&["coll_3"]));
 

lib/domain/libimagcontact/src/store.rs

@@ -144,7 +144,7 @@ impl<'a> ContactStore<'a> for Store {
     }
 
     fn all_contacts(&'a self) -> Result<Entries<'a>> {
-        self.entries().map(|ent| ent.in_collection("contact"))
+        self.entries()?.in_collection("contact")
     }
 
 }

lib/domain/libimagmail/src/store.rs

@@ -143,7 +143,7 @@ impl<'a> MailStore<'a> for Store {
     }
 
     fn all_mails(&'a self) -> Result<Entries<'a>> {
-        self.entries().map(|ent| ent.in_collection("mail"))
+        self.entries()?.in_collection("mail")
     }
 }
 

lib/domain/libimagtimetrack/src/timetrackingstore.rs

@@ -107,7 +107,7 @@ impl<'a> TimeTrackStore<'a> for Store {
     }
 
     fn get_timetrackings(&'a self) -> Result<TimeTrackingsGetIterator<'a>> {
-        Ok(TimeTrackingsGetIterator::new(self.entries()?.in_collection("timetrack"), self))
+        Ok(TimeTrackingsGetIterator::new(self.entries()?.in_collection("timetrack")?, self))
     }
 
 }

lib/domain/libimagwiki/src/wiki.rs

@@ -93,7 +93,7 @@ impl<'a, 'b> Wiki<'a, 'b> {
     }
 
     pub fn all_ids(&self) -> Result<Entries<'a>> {
-        self.0.entries().map(|iter| iter.in_collection("wiki"))
+        self.0.entries()?.in_collection("wiki")
     }
 
     pub fn delete_entry<EN: AsRef<str>>(&self, entry_name: EN) -> Result<()> {

lib/entry/libimagentryannotation/src/annotation_fetcher.rs

@@ -28,7 +28,7 @@ pub trait AnnotationFetcher<'a> {
 
 impl<'a> AnnotationFetcher<'a> for Store {
     fn all_annotations(&'a self) -> Result<Entries<'a>> {
-        self.entries().map(|iter| iter.in_collection("annotation"))
+        self.entries()?.in_collection("annotation")
     }
 }