From 5c8af4460e14a63a8d7fd9aaf0dcb5edb0e19e4a Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 18 May 2019 13:37:46 +0200 Subject: [PATCH] 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 --- lib/core/libimagstore/src/file_abstraction/fs.rs | 8 +++++++- lib/core/libimagstore/src/file_abstraction/inmemory.rs | 3 ++- lib/core/libimagstore/src/file_abstraction/iter.rs | 8 ++++---- lib/core/libimagstore/src/iter.rs | 7 ++++--- lib/domain/libimagcontact/src/store.rs | 2 +- lib/domain/libimagmail/src/store.rs | 2 +- lib/domain/libimagtimetrack/src/timetrackingstore.rs | 2 +- lib/domain/libimagwiki/src/wiki.rs | 2 +- .../libimagentryannotation/src/annotation_fetcher.rs | 2 +- 9 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/core/libimagstore/src/file_abstraction/fs.rs b/lib/core/libimagstore/src/file_abstraction/fs.rs index 5d960bd0..a14628cb 100644 --- a/lib/core/libimagstore/src/file_abstraction/fs.rs +++ b/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(()) + } } } diff --git a/lib/core/libimagstore/src/file_abstraction/inmemory.rs b/lib/core/libimagstore/src/file_abstraction/inmemory.rs index 13428721..d1a47acb 100644 --- a/lib/core/libimagstore/src/file_abstraction/inmemory.rs +++ b/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(()) } } diff --git a/lib/core/libimagstore/src/file_abstraction/iter.rs b/lib/core/libimagstore/src/file_abstraction/iter.rs index c15949df..0fb232bc 100644 --- a/lib/core/libimagstore/src/file_abstraction/iter.rs +++ b/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>>; - 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 { 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 diff --git a/lib/core/libimagstore/src/iter.rs b/lib/core/libimagstore/src/iter.rs index 4e66fb6e..6594579f 100644 --- a/lib/core/libimagstore/src/iter.rs +++ b/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 { - Entries(self.0.in_collection(c), self.1) + pub fn in_collection(self, c: &str) -> Result { + 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"])); diff --git a/lib/domain/libimagcontact/src/store.rs b/lib/domain/libimagcontact/src/store.rs index 66c76469..bb02367f 100644 --- a/lib/domain/libimagcontact/src/store.rs +++ b/lib/domain/libimagcontact/src/store.rs @@ -144,7 +144,7 @@ impl<'a> ContactStore<'a> for Store { } fn all_contacts(&'a self) -> Result> { - self.entries().map(|ent| ent.in_collection("contact")) + self.entries()?.in_collection("contact") } } diff --git a/lib/domain/libimagmail/src/store.rs b/lib/domain/libimagmail/src/store.rs index 851c4d4f..d87ac8ef 100644 --- a/lib/domain/libimagmail/src/store.rs +++ b/lib/domain/libimagmail/src/store.rs @@ -143,7 +143,7 @@ impl<'a> MailStore<'a> for Store { } fn all_mails(&'a self) -> Result> { - self.entries().map(|ent| ent.in_collection("mail")) + self.entries()?.in_collection("mail") } } diff --git a/lib/domain/libimagtimetrack/src/timetrackingstore.rs b/lib/domain/libimagtimetrack/src/timetrackingstore.rs index 967c8958..bf25a4df 100644 --- a/lib/domain/libimagtimetrack/src/timetrackingstore.rs +++ b/lib/domain/libimagtimetrack/src/timetrackingstore.rs @@ -107,7 +107,7 @@ impl<'a> TimeTrackStore<'a> for Store { } fn get_timetrackings(&'a self) -> Result> { - Ok(TimeTrackingsGetIterator::new(self.entries()?.in_collection("timetrack"), self)) + Ok(TimeTrackingsGetIterator::new(self.entries()?.in_collection("timetrack")?, self)) } } diff --git a/lib/domain/libimagwiki/src/wiki.rs b/lib/domain/libimagwiki/src/wiki.rs index 4024e29a..c61a6962 100644 --- a/lib/domain/libimagwiki/src/wiki.rs +++ b/lib/domain/libimagwiki/src/wiki.rs @@ -93,7 +93,7 @@ impl<'a, 'b> Wiki<'a, 'b> { } pub fn all_ids(&self) -> Result> { - self.0.entries().map(|iter| iter.in_collection("wiki")) + self.0.entries()?.in_collection("wiki") } pub fn delete_entry>(&self, entry_name: EN) -> Result<()> { diff --git a/lib/entry/libimagentryannotation/src/annotation_fetcher.rs b/lib/entry/libimagentryannotation/src/annotation_fetcher.rs index 6fbd0078..02e9be36 100644 --- a/lib/entry/libimagentryannotation/src/annotation_fetcher.rs +++ b/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> { - self.entries().map(|iter| iter.in_collection("annotation")) + self.entries()?.in_collection("annotation") } }