diff --git a/lib/core/libimagstore/src/file_abstraction/fs.rs b/lib/core/libimagstore/src/file_abstraction/fs.rs index 574caceb..1e47e0e7 100644 --- a/lib/core/libimagstore/src/file_abstraction/fs.rs +++ b/lib/core/libimagstore/src/file_abstraction/fs.rs @@ -20,6 +20,7 @@ use std::fs::{File, OpenOptions, create_dir_all, remove_file, copy, rename}; use std::io::{Seek, SeekFrom, Read}; use std::path::{Path, PathBuf}; +use std::sync::Arc; use error::{StoreError as SE, StoreErrorKind as SEK}; use error::ResultExt; @@ -30,6 +31,9 @@ use super::Drain; use store::Entry; use storeid::StoreId; use file_abstraction::iter::PathIterator; +use file_abstraction::iter::PathIterBuilder; + +use walkdir::WalkDir; #[derive(Debug)] pub struct FSFileAbstractionInstance(PathBuf); @@ -133,18 +137,34 @@ impl FileAbstraction for FSFileAbstraction { }) } - fn pathes_recursively(&self, basepath: PathBuf) -> Result { - use walkdir::WalkDir; + fn pathes_recursively(&self, + basepath: PathBuf, + storepath: PathBuf, + backend: Arc) + -> Result + { + trace!("Building PathIterator object"); + Ok(PathIterator::new(Box::new(WalkDirPathIterBuilder { basepath }), storepath, backend)) + } +} - let i = WalkDir::new(basepath) +pub(crate) struct WalkDirPathIterBuilder { + basepath: PathBuf +} + +impl PathIterBuilder for WalkDirPathIterBuilder { + fn build_iter(&self) -> Box>> { + Box::new(WalkDir::new(self.basepath.clone()) .min_depth(1) .max_open(100) .into_iter() .map(|r| { r.map(|e| PathBuf::from(e.path())).chain_err(|| SE::from_kind(SEK::FileError)) - }); + })) + } - Ok(PathIterator::new(Box::new(i))) + fn in_collection(&mut self, c: &str) { + self.basepath.push(c); } } diff --git a/lib/core/libimagstore/src/file_abstraction/inmemory.rs b/lib/core/libimagstore/src/file_abstraction/inmemory.rs index 495286fd..303f89d7 100644 --- a/lib/core/libimagstore/src/file_abstraction/inmemory.rs +++ b/lib/core/libimagstore/src/file_abstraction/inmemory.rs @@ -33,6 +33,7 @@ use super::Drain; use store::Entry; use storeid::StoreId; use file_abstraction::iter::PathIterator; +use file_abstraction::iter::PathIterBuilder; type Backend = Arc>>>; @@ -181,9 +182,9 @@ impl FileAbstraction for InMemoryFileAbstraction { Ok(()) } - fn pathes_recursively(&self, _basepath: PathBuf) -> Result { - debug!("Getting all pathes"); - let keys : Vec> = self + fn pathes_recursively(&self, _basepath: PathBuf, storepath: PathBuf, backend: Arc) -> Result { + trace!("Building PathIterator object (inmemory implementation)"); + let keys : Vec = self .backend() .lock() .map_err(|_| SE::from_kind(SEK::FileError))? @@ -191,9 +192,21 @@ impl FileAbstraction for InMemoryFileAbstraction { .keys() .map(PathBuf::from) .map(Ok) - .collect(); // we have to collect() because of the lock() above. + .collect::>()?; // we have to collect() because of the lock() above. - Ok(PathIterator::new(Box::new(keys.into_iter()))) + Ok(PathIterator::new(Box::new(InMemPathIterBuilder(keys)), storepath, backend)) + } +} + +pub(crate) struct InMemPathIterBuilder(Vec); + +impl PathIterBuilder for InMemPathIterBuilder { + fn build_iter(&self) -> Box>> { + Box::new(self.0.clone().into_iter().map(Ok)) + } + + fn in_collection(&mut self, c: &str) { + self.0.retain(|p| p.starts_with(c)); } } diff --git a/lib/core/libimagstore/src/file_abstraction/iter.rs b/lib/core/libimagstore/src/file_abstraction/iter.rs index 2cf479b7..4c5dcd4e 100644 --- a/lib/core/libimagstore/src/file_abstraction/iter.rs +++ b/lib/core/libimagstore/src/file_abstraction/iter.rs @@ -24,48 +24,63 @@ use error::Result; use storeid::StoreId; use file_abstraction::FileAbstraction; +/// See documentation for PathIterator +pub trait PathIterBuilder { + fn build_iter(&self) -> Box>>; + fn in_collection(&mut self, c: &str); +} + /// A wrapper for an iterator over `PathBuf`s -pub struct PathIterator(Box>>); +/// +/// As the backend defines how "iterating over all entries" is implemented, this type holds a +/// `PathIterBuilder` internally. This type is used to create new iterator instances every time the +/// "settings" for how the iterator behaves are changed. This basically means: If the PathIterator +/// is requested to not iterate over a directory "a" but rather its subdirectory "a/b", the +/// implementation asks the `PathIterBuilder` to create a new iterator for that. +/// +/// The `PathIterBuilder` can then yield a new iterator instance which is optimized for the new +/// requirements (which basically means: Construct a new WalkDir object which does traverse the +/// subdirectory instead of the parent). +/// +/// This means quite a few allocations down the road, as the PathIterator itself is not generic, but +/// this seems to be the best way to implement this. +pub struct PathIterator { + iter_builder: Box, + iter: Box>>, + storepath: PathBuf, + backend: Arc, +} impl PathIterator { - pub fn new(iter: Box>>) -> PathIterator { - PathIterator(iter) + pub fn new(iter_builder: Box, + storepath: PathBuf, + backend: Arc) + -> PathIterator + { + trace!("Generating iterator object with PathIterBuilder"); + let iter = iter_builder.build_iter(); + PathIterator { iter_builder, iter, storepath, backend } } - pub fn store_id_constructing(self, storepath: PathBuf, backend: Arc) - -> StoreIdConstructingIterator - { - StoreIdConstructingIterator(self, storepath, backend) + pub fn in_collection(mut self, c: &str) -> Self { + trace!("Generating iterator object for collection: {}", c); + self.iter_builder.in_collection(c); + self } } impl Iterator for PathIterator { - type Item = Result; - - fn next(&mut self) -> Option { - self.0.next() - } - -} - - -/// Helper type for constructing StoreIds from a PathIterator. -/// -/// Automatically ignores non-files. -pub struct StoreIdConstructingIterator(PathIterator, PathBuf, Arc); - -impl Iterator for StoreIdConstructingIterator { type Item = Result; fn next(&mut self) -> Option { - while let Some(next) = self.0.next() { + while let Some(next) = self.iter.next() { match next { - Err(e) => return Some(Err(e)), - Ok(next) => match self.2.is_file(&next) { + Err(e) => return Some(Err(e)), + Ok(next) => match self.backend.is_file(&next) { Err(e) => return Some(Err(e)), - Ok(true) => return Some(StoreId::from_full_path(&self.1, next)), + Ok(true) => return Some(StoreId::from_full_path(&self.storepath, next)), Ok(false) => { continue }, } } diff --git a/lib/core/libimagstore/src/file_abstraction/mod.rs b/lib/core/libimagstore/src/file_abstraction/mod.rs index a3172735..5e50db55 100644 --- a/lib/core/libimagstore/src/file_abstraction/mod.rs +++ b/lib/core/libimagstore/src/file_abstraction/mod.rs @@ -20,6 +20,7 @@ use std::path::PathBuf; use std::fmt::Debug; use std::collections::HashMap; +use std::sync::Arc; use error::StoreError as SE; use store::Entry; @@ -50,7 +51,7 @@ pub trait FileAbstraction : Debug { fn drain(&self) -> Result; fn fill<'a>(&'a mut self, d: Drain) -> Result<(), SE>; - fn pathes_recursively(&self, basepath: PathBuf) -> Result; + fn pathes_recursively(&self, basepath: PathBuf, storepath: PathBuf, backend: Arc) -> Result; } /// An abstraction trait over actions on files diff --git a/lib/core/libimagstore/src/iter.rs b/lib/core/libimagstore/src/iter.rs index 0e066b09..d2fd1a16 100644 --- a/lib/core/libimagstore/src/iter.rs +++ b/lib/core/libimagstore/src/iter.rs @@ -144,3 +144,81 @@ mod compile_test { } } +use storeid::StoreId; +use storeid::StoreIdIterator; +use self::delete::StoreDeleteIterator; +use self::get::StoreGetIterator; +use self::retrieve::StoreRetrieveIterator; +use file_abstraction::iter::PathIterator; +use store::Store; +use error::StoreError; +use error::Result; + +/// Iterator for iterating over all (or a subset of all) entries +/// +/// The iterator now has functionality to optimize the iteration, if only a subdirectory of the +/// store is required, for example `$STORE/foo`. +/// +/// This is done via functionality where the underlying iterator gets +/// altered. +/// +/// As the (for the filesystem backend underlying) `walkdir::WalkDir` type is not as nice as it +/// could be, iterating over two subdirectories with one iterator is not possible. Thus, iterators +/// for two collections in the store should be build like this (untested): +/// +/// ```ignore +/// store +/// .entries()? +/// .in_collection("foo") +/// .chain(store.entries()?.in_collection("bar")) +/// ``` +/// +/// Functionality to exclude subdirectories is not possible with the current implementation and has +/// to be done during iteration, with filtering (as usual). +pub struct Entries<'a>(PathIterator, &'a Store); + +impl<'a> Entries<'a> { + + pub(crate) fn new(pi: PathIterator, store: &'a Store) -> Self { + Entries(pi, store) + } + + pub fn in_collection(self, c: &str) -> Self { + Entries(self.0.in_collection(c), self.1) + } + + pub fn without_store(self) -> StoreIdIterator { + StoreIdIterator::new(Box::new(self.0)) + } + + /// Transform the iterator into a StoreDeleteIterator + /// + /// This immitates the API from `libimagstore::iter`. + pub fn into_delete_iter(self) -> StoreDeleteIterator<'a, StoreError> { + StoreDeleteIterator::new(Box::new(self.0), self.1) + } + + /// Transform the iterator into a StoreGetIterator + /// + /// This immitates the API from `libimagstore::iter`. + pub fn into_get_iter(self) -> StoreGetIterator<'a, StoreError> { + StoreGetIterator::new(Box::new(self.0), self.1) + } + + /// Transform the iterator into a StoreRetrieveIterator + /// + /// This immitates the API from `libimagstore::iter`. + pub fn into_retrieve_iter(self) -> StoreRetrieveIterator<'a, StoreError> { + StoreRetrieveIterator::new(Box::new(self.0), self.1) + } + +} + +impl<'a> Iterator for Entries<'a> { + type Item = Result; + + fn next(&mut self) -> Option { + self.0.next() + } +} + diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 8b199946..79236655 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -37,7 +37,8 @@ use toml_query::read::TomlValueReadTypeExt; use error::{StoreError as SE, StoreErrorKind as SEK}; use error::ResultExt; -use storeid::{IntoStoreId, StoreId, StoreIdIteratorWithStore}; +use storeid::{IntoStoreId, StoreId}; +use iter::Entries; use file_abstraction::FileAbstractionInstance; // We re-export the following things so tests can use them @@ -642,12 +643,11 @@ impl Store { } /// Get _all_ entries in the store (by id as iterator) - pub fn entries(&self) -> Result { + pub fn entries<'a>(&'a self) -> Result> { + trace!("Building 'Entries' iterator"); self.backend - .pathes_recursively(self.path().clone()) - .map(|i| i.store_id_constructing(self.path().clone(), self.backend.clone())) - .map(Box::new) - .map(|it| StoreIdIteratorWithStore::new(it, self)) + .pathes_recursively(self.path().clone(), self.path().clone(), self.backend.clone()) + .map(|i| Entries::new(i, self)) } /// Gets the path where this store is on the disk diff --git a/lib/core/libimagstore/src/storeid.rs b/lib/core/libimagstore/src/storeid.rs index ddb1443e..2f2bf723 100644 --- a/lib/core/libimagstore/src/storeid.rs +++ b/lib/core/libimagstore/src/storeid.rs @@ -266,6 +266,10 @@ impl StoreIdIterator { StoreIdIterator { iter } } + pub fn with_store<'a>(self, store: &'a Store) -> StoreIdIteratorWithStore<'a> { + StoreIdIteratorWithStore(self, store) + } + } impl Iterator for StoreIdIterator { diff --git a/lib/domain/libimaghabit/src/store.rs b/lib/domain/libimaghabit/src/store.rs index f8d1a8c2..b4f89bfd 100644 --- a/lib/domain/libimaghabit/src/store.rs +++ b/lib/domain/libimaghabit/src/store.rs @@ -48,11 +48,11 @@ pub trait HabitStore { impl HabitStore for Store { /// Get an iterator over all habits fn all_habit_templates(&self) -> Result { - self.entries().map(HabitTemplateStoreIdIterator::from).map_err(From::from) + Ok(HabitTemplateStoreIdIterator::from(self.entries()?.without_store())) } fn all_habit_instances(&self) -> Result { - self.entries().map(HabitInstanceStoreIdIterator::from).map_err(From::from) + Ok(HabitInstanceStoreIdIterator::from(self.entries()?.without_store())) } } diff --git a/lib/domain/libimagtimetrack/src/iter/get.rs b/lib/domain/libimagtimetrack/src/iter/get.rs index fdb55c20..202a87d9 100644 --- a/lib/domain/libimagtimetrack/src/iter/get.rs +++ b/lib/domain/libimagtimetrack/src/iter/get.rs @@ -17,18 +17,18 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // -use libimagstore::storeid::StoreIdIteratorWithStore; +use libimagstore::iter::Entries; use libimagstore::store::Store; use libimagstore::store::Result as StoreResult; use libimagstore::store::FileLockEntry; use constants::*; -pub struct TimeTrackingsGetIterator<'a>(StoreIdIteratorWithStore<'a>, &'a Store); +pub struct TimeTrackingsGetIterator<'a>(Entries<'a>, &'a Store); impl<'a> TimeTrackingsGetIterator<'a> { - pub fn new(sit: StoreIdIteratorWithStore<'a>, store: &'a Store) -> Self { - TimeTrackingsGetIterator(sit, store) + pub fn new(entries: Entries<'a>, store: &'a Store) -> Self { + TimeTrackingsGetIterator(entries, store) } } diff --git a/lib/domain/libimagwiki/src/wiki.rs b/lib/domain/libimagwiki/src/wiki.rs index 0612bec8..da194fd4 100644 --- a/lib/domain/libimagwiki/src/wiki.rs +++ b/lib/domain/libimagwiki/src/wiki.rs @@ -95,7 +95,7 @@ impl<'a, 'b> Wiki<'a, 'b> { pub fn all_ids(&self) -> Result { let filter = IdIsInWikiFilter(self.1); - Ok(WikiIdIterator(self.0.entries()?, filter)) + Ok(WikiIdIterator(self.0.entries()?.without_store().with_store(self.0), filter)) } pub fn delete_entry>(&self, entry_name: EN) -> Result<()> {