From 23f4d7948fe0623324c312247212ac805f70fc20 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 13 Oct 2017 11:58:21 +0200 Subject: [PATCH] Fix Store::entries() to not yield directories Before the iterator did also yield storeids for directories, which was a bug. This change introduces a new if_file() function in the store-internal backend, which is needed to check whether a path actually points to a File, be it inmemory or on the real filesystem. That's because tests might fail if they check via PathBuf::is_file() as in tests, the entries only exist inmemory. --- doc/src/09020-changelog.md | 4 ++++ .../libimagstore/src/file_abstraction/fs.rs | 4 ++++ .../src/file_abstraction/inmemory.rs | 7 ++++++ .../libimagstore/src/file_abstraction/mod.rs | 1 + .../src/file_abstraction/stdio/mod.rs | 4 ++++ .../src/file_abstraction/stdio/out.rs | 4 ++++ lib/core/libimagstore/src/store.rs | 24 +++++++++++-------- 7 files changed, 38 insertions(+), 10 deletions(-) diff --git a/doc/src/09020-changelog.md b/doc/src/09020-changelog.md index 7f4aace2..85cdd770 100644 --- a/doc/src/09020-changelog.md +++ b/doc/src/09020-changelog.md @@ -34,6 +34,10 @@ This section contains the changelog from the last release to the next release. anymore. This is minor because `libimagentryanntation` is not yet used by any other crate. +* Bugfixes + * `Store::entries()` does not yield StoreIds which point to directories + anymore, only StoreIds pointing to files. + ## 0.4.0 * Major changes diff --git a/lib/core/libimagstore/src/file_abstraction/fs.rs b/lib/core/libimagstore/src/file_abstraction/fs.rs index de292925..0773a178 100644 --- a/lib/core/libimagstore/src/file_abstraction/fs.rs +++ b/lib/core/libimagstore/src/file_abstraction/fs.rs @@ -150,6 +150,10 @@ impl FileAbstraction for FSFileAbstraction { Ok(path.exists()) } + fn is_file(&self, path: &PathBuf) -> Result { + Ok(path.is_file()) + } + fn new_instance(&self, p: PathBuf) -> Box { Box::new(FSFileAbstractionInstance::Absent(p)) } diff --git a/lib/core/libimagstore/src/file_abstraction/inmemory.rs b/lib/core/libimagstore/src/file_abstraction/inmemory.rs index 909aad9d..9a9adf67 100644 --- a/lib/core/libimagstore/src/file_abstraction/inmemory.rs +++ b/lib/core/libimagstore/src/file_abstraction/inmemory.rs @@ -159,6 +159,13 @@ impl FileAbstraction for InMemoryFileAbstraction { Ok(backend.contains_key(pb)) } + fn is_file(&self, pb: &PathBuf) -> Result { + // Because we only store Entries in the memory-internal backend, we only have to check for + // existance here, as if a path exists in the inmemory storage, it is always mapped to an + // entry. hence it is always a path to a file + self.exists(pb) + } + fn new_instance(&self, p: PathBuf) -> Box { Box::new(InMemoryFileAbstractionInstance::new(self.backend().clone(), p)) } diff --git a/lib/core/libimagstore/src/file_abstraction/mod.rs b/lib/core/libimagstore/src/file_abstraction/mod.rs index 4aad4dfb..6c23f347 100644 --- a/lib/core/libimagstore/src/file_abstraction/mod.rs +++ b/lib/core/libimagstore/src/file_abstraction/mod.rs @@ -44,6 +44,7 @@ pub trait FileAbstraction : Debug { fn create_dir_all(&self, _: &PathBuf) -> Result<(), SE>; fn exists(&self, &PathBuf) -> Result; + fn is_file(&self, &PathBuf) -> Result; fn new_instance(&self, p: PathBuf) -> Box; diff --git a/lib/core/libimagstore/src/file_abstraction/stdio/mod.rs b/lib/core/libimagstore/src/file_abstraction/stdio/mod.rs index a9606c91..7bf4f425 100644 --- a/lib/core/libimagstore/src/file_abstraction/stdio/mod.rs +++ b/lib/core/libimagstore/src/file_abstraction/stdio/mod.rs @@ -119,6 +119,10 @@ impl FileAbstraction for StdIoFileAbstraction { self.0.exists(p) } + fn is_file(&self, p: &PathBuf) -> Result { + self.0.is_file(p) + } + fn drain(&self) -> Result { self.0.drain() } diff --git a/lib/core/libimagstore/src/file_abstraction/stdio/out.rs b/lib/core/libimagstore/src/file_abstraction/stdio/out.rs index 456fe9d4..c8540b33 100644 --- a/lib/core/libimagstore/src/file_abstraction/stdio/out.rs +++ b/lib/core/libimagstore/src/file_abstraction/stdio/out.rs @@ -140,6 +140,10 @@ impl FileAbstraction for StdoutFileAbstraction { self.mem.exists(p) } + fn is_file(&self, p: &PathBuf) -> Result { + self.mem.is_file(p) + } + fn drain(&self) -> Result { self.backend_cloned().map(Drain::new) } diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index c376ecce..0e0c57da 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -812,17 +812,21 @@ impl Store { self.backend .pathes_recursively(self.path().clone()) .and_then(|iter| { - let iter : Result> = iter - .map(|path| StoreId::from_full_path(self.path(), path)) - .fold(Ok(vec![]), |acc, elem| { - acc.and_then(move |mut a| { - a.push(try!(elem)); - Ok(a) - }) - }); + let mut elems = vec![]; + for element in iter { + let is_file = { + let mut base = self.path().clone(); + base.push(element.clone()); + println!("Checking: {:?}", base); + try!(self.backend.is_file(&base)) + }; - let iter = try!(iter); - Ok(StoreIdIterator::new(Box::new(iter.into_iter()))) + if is_file { + let sid = try!(StoreId::from_full_path(self.path(), element)); + elems.push(sid); + } + } + Ok(StoreIdIterator::new(Box::new(elems.into_iter()))) }) }