From 04a8c5f721e7ad9c04fe46f3488c2fa2bef7d0cf Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Wed, 28 Nov 2018 02:14:39 +0100 Subject: Move to simpler implementation of Storeid This changes the implementation of the StoreId type to be less complex. Before we always had to re-attach the path of the store itself each time a StoreId object was passed to the store. Now we do not do this anymore, hopefully we can move to a implementation of the Store API which takes references of StoreId objects in the future. Signed-off-by: Matthias Beyer --- lib/core/libimagstore/src/file_abstraction/fs.rs | 10 +- .../libimagstore/src/file_abstraction/inmemory.rs | 6 +- lib/core/libimagstore/src/file_abstraction/iter.rs | 18 +- lib/core/libimagstore/src/file_abstraction/mod.rs | 19 +- lib/core/libimagstore/src/iter.rs | 22 +- lib/core/libimagstore/src/store.rs | 76 ++++--- lib/core/libimagstore/src/storeid.rs | 222 ++++++++++----------- 7 files changed, 175 insertions(+), 198 deletions(-) diff --git a/lib/core/libimagstore/src/file_abstraction/fs.rs b/lib/core/libimagstore/src/file_abstraction/fs.rs index f68d2aa1..05f48b89 100644 --- a/lib/core/libimagstore/src/file_abstraction/fs.rs +++ b/lib/core/libimagstore/src/file_abstraction/fs.rs @@ -28,7 +28,7 @@ use super::FileAbstraction; use super::FileAbstractionInstance; use super::Drain; use store::Entry; -use storeid::StoreId; +use storeid::StoreIdWithBase; use file_abstraction::iter::PathIterator; use file_abstraction::iter::PathIterBuilder; @@ -45,7 +45,7 @@ impl FileAbstractionInstance for FSFileAbstractionInstance { /** * Get the content behind this file */ - fn get_file_content(&mut self, id: StoreId) -> Result> { + fn get_file_content<'a>(&mut self, id: StoreIdWithBase<'a>) -> Result> { debug!("Getting lazy file: {:?}", self); let mut file = match open_file(&self.0) { @@ -153,11 +153,11 @@ impl FileAbstraction for FSFileAbstraction { }) } - fn pathes_recursively(&self, + fn pathes_recursively<'a>(&self, basepath: PathBuf, - storepath: PathBuf, + storepath: &'a PathBuf, backend: Arc) - -> Result + -> Result> { trace!("Building PathIterator object"); Ok(PathIterator::new(Box::new(WalkDirPathIterBuilder { basepath }), storepath, backend)) diff --git a/lib/core/libimagstore/src/file_abstraction/inmemory.rs b/lib/core/libimagstore/src/file_abstraction/inmemory.rs index 292aea15..49f7c56d 100644 --- a/lib/core/libimagstore/src/file_abstraction/inmemory.rs +++ b/lib/core/libimagstore/src/file_abstraction/inmemory.rs @@ -33,7 +33,7 @@ use super::FileAbstraction; use super::FileAbstractionInstance; use super::Drain; use store::Entry; -use storeid::StoreId; +use storeid::StoreIdWithBase; use file_abstraction::iter::PathIterator; use file_abstraction::iter::PathIterBuilder; @@ -64,7 +64,7 @@ impl FileAbstractionInstance for InMemoryFileAbstractionInstance { /** * Get the mutable file behind a InMemoryFileAbstraction object */ - fn get_file_content(&mut self, _: StoreId) -> Result> { + fn get_file_content(&mut self, _: StoreIdWithBase<'_>) -> Result> { debug!("Getting lazy file: {:?}", self); self.fs_abstraction @@ -187,7 +187,7 @@ impl FileAbstraction for InMemoryFileAbstraction { Ok(()) } - fn pathes_recursively(&self, _basepath: PathBuf, storepath: PathBuf, backend: Arc) -> Result { + fn pathes_recursively<'a>(&self, _basepath: PathBuf, storepath: &'a PathBuf, backend: Arc) -> Result> { trace!("Building PathIterator object (inmemory implementation)"); let keys : Vec = self .backend() diff --git a/lib/core/libimagstore/src/file_abstraction/iter.rs b/lib/core/libimagstore/src/file_abstraction/iter.rs index 9df59a2f..7a5a5c8f 100644 --- a/lib/core/libimagstore/src/file_abstraction/iter.rs +++ b/lib/core/libimagstore/src/file_abstraction/iter.rs @@ -22,7 +22,7 @@ use std::sync::Arc; use failure::Fallible as Result; -use storeid::StoreId; +use storeid::StoreIdWithBase; use file_abstraction::FileAbstraction; /// See documentation for PathIterator @@ -45,19 +45,19 @@ pub trait PathIterBuilder { /// /// 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 { +pub(crate) struct PathIterator<'a> { iter_builder: Box, iter: Box>>, - storepath: PathBuf, + storepath: &'a PathBuf, backend: Arc, } -impl PathIterator { +impl<'a> PathIterator<'a> { pub fn new(iter_builder: Box, - storepath: PathBuf, + storepath: &'a PathBuf, backend: Arc) - -> PathIterator + -> PathIterator<'a> { trace!("Generating iterator object with PathIterBuilder"); let iter = iter_builder.build_iter(); @@ -73,8 +73,8 @@ impl PathIterator { } -impl Iterator for PathIterator { - type Item = Result; +impl<'a> Iterator for PathIterator<'a> { + type Item = Result>; fn next(&mut self) -> Option { while let Some(next) = self.iter.next() { @@ -82,7 +82,7 @@ impl Iterator for PathIterator { 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.storepath, next)), + Ok(true) => return Some(StoreIdWithBase::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 d7716a56..13fd727a 100644 --- a/lib/core/libimagstore/src/file_abstraction/mod.rs +++ b/lib/core/libimagstore/src/file_abstraction/mod.rs @@ -25,7 +25,7 @@ use std::sync::Arc; use failure::Fallible as Result; use store::Entry; -use storeid::StoreId; +use storeid::StoreIdWithBase; mod fs; mod inmemory; @@ -52,7 +52,7 @@ pub trait FileAbstraction : Debug { fn drain(&self) -> Result; fn fill<'a>(&'a mut self, d: Drain) -> Result<()>; - fn pathes_recursively(&self, basepath: PathBuf, storepath: PathBuf, backend: Arc) -> Result; + fn pathes_recursively<'a>(&self, basepath: PathBuf, storepath: &'a PathBuf, backend: Arc) -> Result>; } /// An abstraction trait over actions on files @@ -60,9 +60,9 @@ pub trait FileAbstractionInstance : Debug { /// Get the contents of the FileAbstractionInstance, as Entry object. /// - /// The `StoreId` is passed because the backend does not know where the Entry lives, but the + /// The `StoreIdWithBase` is passed because the backend does not know where the Entry lives, but the /// Entry type itself must be constructed with the id. - fn get_file_content(&mut self, id: StoreId) -> Result>; + fn get_file_content<'a>(&mut self, id: StoreIdWithBase<'a>) -> Result>; fn write_file_content(&mut self, buf: &Entry) -> Result<()>; } @@ -101,18 +101,19 @@ mod test { use super::FileAbstractionInstance; use super::inmemory::InMemoryFileAbstraction; use super::inmemory::InMemoryFileAbstractionInstance; - use storeid::StoreId; + use storeid::StoreIdWithBase; use store::Entry; #[test] fn lazy_file() { + let store_path = PathBuf::from("/"); let fs = InMemoryFileAbstraction::default(); let mut path = PathBuf::from("tests"); path.set_file_name("test1"); let mut lf = InMemoryFileAbstractionInstance::new(fs.backend().clone(), path.clone()); - let loca = StoreId::new_baseless(path).unwrap(); + let loca = StoreIdWithBase::new(&store_path, path); let file = Entry::from_str(loca.clone(), &format!(r#"--- [imag] version = "{}" @@ -126,13 +127,14 @@ Hello World"#, env!("CARGO_PKG_VERSION"))).unwrap(); #[test] fn lazy_file_multiline() { + let store_path = PathBuf::from("/"); let fs = InMemoryFileAbstraction::default(); let mut path = PathBuf::from("tests"); path.set_file_name("test1"); let mut lf = InMemoryFileAbstractionInstance::new(fs.backend().clone(), path.clone()); - let loca = StoreId::new_baseless(path).unwrap(); + let loca = StoreIdWithBase::new(&store_path, path); let file = Entry::from_str(loca.clone(), &format!(r#"--- [imag] version = "{}" @@ -147,13 +149,14 @@ baz"#, env!("CARGO_PKG_VERSION"))).unwrap(); #[test] fn lazy_file_multiline_trailing_newlines() { + let store_path = PathBuf::from("/"); let fs = InMemoryFileAbstraction::default(); let mut path = PathBuf::from("tests"); path.set_file_name("test1"); let mut lf = InMemoryFileAbstractionInstance::new(fs.backend().clone(), path.clone()); - let loca = StoreId::new_baseless(path).unwrap(); + let loca = StoreIdWithBase::new(&store_path, path); let file = Entry::from_str(loca.clone(), &format!(r#"--- [imag] version = "{}" diff --git a/lib/core/libimagstore/src/iter.rs b/lib/core/libimagstore/src/iter.rs index 1639b02c..e236d984 100644 --- a/lib/core/libimagstore/src/iter.rs +++ b/lib/core/libimagstore/src/iter.rs @@ -138,7 +138,6 @@ mod compile_test { } use storeid::StoreId; -use storeid::StoreIdIterator; use self::delete::StoreDeleteIterator; use self::get::StoreGetIterator; use self::retrieve::StoreRetrieveIterator; @@ -167,11 +166,11 @@ use failure::Fallible as Result; /// /// 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); +pub struct Entries<'a>(PathIterator<'a>, &'a Store); impl<'a> Entries<'a> { - pub(crate) fn new(pi: PathIterator, store: &'a Store) -> Self { + pub(crate) fn new(pi: PathIterator<'a>, store: &'a Store) -> Self { Entries(pi, store) } @@ -179,29 +178,30 @@ impl<'a> Entries<'a> { Entries(self.0.in_collection(c), self.1) } - pub fn without_store(self) -> StoreIdIterator { - StoreIdIterator::new(Box::new(self.0)) - } + // TODO: Remove or fix me + //pub fn without_store(self) -> StoreIdIterator { + // StoreIdIterator::new(Box::new(self.0.map(|r| r.map(|id| id.without_base())))) + //} /// Transform the iterator into a StoreDeleteIterator /// /// This immitates the API from `libimagstore::iter`. pub fn into_delete_iter(self) -> StoreDeleteIterator<'a> { - StoreDeleteIterator::new(Box::new(self.0), self.1) + StoreDeleteIterator::new(Box::new(self.0.map(|r| r.map(|id| id.without_base()))), self.1) } /// Transform the iterator into a StoreGetIterator /// /// This immitates the API from `libimagstore::iter`. pub fn into_get_iter(self) -> StoreGetIterator<'a> { - StoreGetIterator::new(Box::new(self.0), self.1) + StoreGetIterator::new(Box::new(self.0.map(|r| r.map(|id| id.without_base()))), self.1) } /// Transform the iterator into a StoreRetrieveIterator /// /// This immitates the API from `libimagstore::iter`. pub fn into_retrieve_iter(self) -> StoreRetrieveIterator<'a> { - StoreRetrieveIterator::new(Box::new(self.0), self.1) + StoreRetrieveIterator::new(Box::new(self.0.map(|r| r.map(|id| id.without_base()))), self.1) } } @@ -210,7 +210,7 @@ impl<'a> Iterator for Entries<'a> { type Item = Result; fn next(&mut self) -> Option { - self.0.next() + self.0.next().map(|r| r.map(|id| id.without_base())) } } @@ -244,7 +244,7 @@ mod tests { let base = String::from("entry"); let variants = vec!["coll_1", "coll_2", "coll_3"]; let modifier = |base: &String, v: &&str| { - StoreId::new(Some(store.path().clone()), PathBuf::from(format!("{}/{}", *v, base))).unwrap() + StoreId::new(PathBuf::from(format!("{}/{}", *v, base))).unwrap() }; generate_variants(&base, variants.iter(), &modifier) diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index aa3ae4ba..cb85177f 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -64,14 +64,15 @@ enum StoreEntryStatus { #[derive(Debug)] struct StoreEntry { id: StoreId, + store_base: PathBuf, // small sacrefice over lifetimes on the Store type file: Box, status: StoreEntryStatus, } impl StoreEntry { - fn new(id: StoreId, backend: &Arc) -> Result { - let pb = id.clone().into_pathbuf()?; + fn new(store_base: PathBuf, id: StoreId, backend: &Arc) -> Result { + let pb = id.clone().with_base(&store_base).into_pathbuf()?; #[cfg(feature = "fs-lock")] { @@ -82,6 +83,7 @@ impl StoreEntry { Ok(StoreEntry { id, + store_base, file: backend.new_instance(pb), status: StoreEntryStatus::Present, }) @@ -95,7 +97,7 @@ impl StoreEntry { fn get_entry(&mut self) -> Result { if !self.is_borrowed() { - match self.file.get_file_content(self.id.clone())? { + match self.file.get_file_content(self.id.clone().with_base(&self.store_base))? { Some(file) => Ok(file), None => Ok(Entry::new(self.id.clone())) } @@ -218,7 +220,7 @@ impl Store { /// On success: FileLockEntry /// pub fn create<'a, S: IntoStoreId>(&'a self, id: S) -> Result> { - let id = id.into_storeid()?.with_base(self.path().clone()); + let id = id.into_storeid()?; debug!("Creating id: '{}'", id); @@ -244,7 +246,7 @@ impl Store { } hsmap.insert(id.clone(), { debug!("Creating: '{}'", id); - let mut se = StoreEntry::new(id.clone(), &self.backend)?; + let mut se = StoreEntry::new(self.path().clone(), id.clone(), &self.backend)?; se.status = StoreEntryStatus::Borrowed; se }); @@ -266,14 +268,14 @@ impl Store { /// On success: FileLockEntry /// pub fn retrieve<'a, S: IntoStoreId>(&'a self, id: S) -> Result> { - let id = id.into_storeid()?.with_base(self.path().clone()); + let id = id.into_storeid()?; debug!("Retrieving id: '{}'", id); let entry = self .entries .write() .map_err(|_| Error::from(EM::LockError)) .and_then(|mut es| { - let new_se = StoreEntry::new(id.clone(), &self.backend)?; + let new_se = StoreEntry::new(self.path().clone(), id.clone(), &self.backend)?; let se = es.entry(id.clone()).or_insert(new_se); let entry = se.get_entry(); se.status = StoreEntryStatus::Borrowed; @@ -296,7 +298,7 @@ impl Store { /// - Errors Store::retrieve() might return /// pub fn get<'a, S: IntoStoreId + Clone>(&'a self, id: S) -> Result>> { - let id = id.into_storeid()?.with_base(self.path().clone()); + let id = id.into_storeid()?; debug!("Getting id: '{}'", id); @@ -409,7 +411,7 @@ impl Store { /// On success: Entry /// pub fn get_copy(&self, id: S) -> Result { - let id = id.into_storeid()?.with_base(self.path().clone()); + let id = id.into_storeid()?; debug!("Retrieving copy of '{}'", id); let entries = self.entries.write() .map_err(|_| Error::from(EM::LockError)) @@ -422,7 +424,7 @@ impl Store { .map_err(Error::from) } - StoreEntry::new(id, &self.backend)?.get_entry() + StoreEntry::new(self.path().clone(), id, &self.backend)?.get_entry() } /// Delete an entry and the corrosponding file on disk @@ -432,7 +434,7 @@ impl Store { /// On success: () /// pub fn delete(&self, id: S) -> Result<()> { - let id = id.into_storeid()?.with_base(self.path().clone()); + let id = id.into_storeid()?; debug!("Deleting id: '{}'", id); @@ -440,7 +442,7 @@ impl Store { // StoreId::exists(), a PathBuf object gets allocated. So we simply get a // PathBuf here, check whether it is there and if it is, we can re-use it to // delete the filesystem file. - let pb = id.clone().into_pathbuf()?; + let pb = id.clone().with_base(self.path()).into_pathbuf()?; { let mut entries = self @@ -507,7 +509,6 @@ impl Store { fn save_to_other_location(&self, entry: &FileLockEntry, new_id: StoreId, remove_old: bool) -> Result<()> { - let new_id = new_id.with_base(self.path().clone()); let hsmap = self .entries .write() @@ -522,8 +523,8 @@ impl Store { let old_id = entry.get_location().clone(); - let old_id_as_path = old_id.clone().with_base(self.path().clone()).into_pathbuf()?; - let new_id_as_path = new_id.clone().with_base(self.path().clone()).into_pathbuf()?; + let old_id_as_path = old_id.clone().with_base(self.path()).into_pathbuf()?; + let new_id_as_path = new_id.clone().with_base(self.path()).into_pathbuf()?; self.backend .copy(&old_id_as_path, &new_id_as_path) .and_then(|_| if remove_old { @@ -571,9 +572,6 @@ impl Store { /// So the link is _partly dangling_, so to say. /// pub fn move_by_id(&self, old_id: StoreId, new_id: StoreId) -> Result<()> { - let new_id = new_id.with_base(self.path().clone()); - let old_id = old_id.with_base(self.path().clone()); - debug!("Moving '{}' to '{}'", old_id, new_id); { @@ -594,8 +592,8 @@ impl Store { debug!("Old id is not yet borrowed"); - let old_id_pb = old_id.clone().with_base(self.path().clone()).into_pathbuf()?; - let new_id_pb = new_id.clone().with_base(self.path().clone()).into_pathbuf()?; + let old_id_pb = old_id.clone().with_base(self.path()).into_pathbuf()?; + let new_id_pb = new_id.clone().with_base(self.path()).into_pathbuf()?; if self.backend.exists(&new_id_pb)? { return Err(format_err!("Entry already exists: {}", new_id)); @@ -618,8 +616,8 @@ impl Store { assert!(hsmap .remove(&old_id) .and_then(|mut entry| { - entry.id = new_id.clone(); - hsmap.insert(new_id.clone(), entry) + entry.id = new_id.clone().into(); + hsmap.insert(new_id.clone().into(), entry) }).is_none()) } @@ -631,7 +629,7 @@ impl Store { pub fn entries<'a>(&'a self) -> Result> { trace!("Building 'Entries' iterator"); self.backend - .pathes_recursively(self.path().clone(), self.path().clone(), self.backend.clone()) + .pathes_recursively(self.path().clone(), self.path(), self.backend.clone()) .map(|i| Entries::new(i, self)) } @@ -645,7 +643,7 @@ impl Store { .context(format_err!("CreateCallError: {}", id)); let backend_has_entry = |id: StoreId| - self.backend.exists(&id.with_base(self.path().to_path_buf()).into_pathbuf()?); + self.backend.exists(&id.with_base(self.path()).into_pathbuf()?); Ok(cache_has_entry(&id)? || backend_has_entry(id)?) } @@ -1000,7 +998,7 @@ Hai setup_logging(); debug!("{}", TEST_ENTRY); - let entry = Entry::from_str(StoreId::new_baseless(PathBuf::from("test/foo~1.3")).unwrap(), + let entry = Entry::from_str(StoreId::new(PathBuf::from("test/foo~1.3")).unwrap(), TEST_ENTRY).unwrap(); assert_eq!(entry.content, "Hai"); @@ -1014,7 +1012,7 @@ Hai setup_logging(); debug!("{}", TEST_ENTRY); - let entry = Entry::from_str(StoreId::new_baseless(PathBuf::from("test/foo~1.3")).unwrap(), + let entry = Entry::from_str(StoreId::new(PathBuf::from("test/foo~1.3")).unwrap(), TEST_ENTRY).unwrap(); let string = entry.to_str().unwrap(); @@ -1029,7 +1027,7 @@ Hai setup_logging(); debug!("{}", TEST_ENTRY_TNL); - let entry = Entry::from_str(StoreId::new_baseless(PathBuf::from("test/foo~1.3")).unwrap(), + let entry = Entry::from_str(StoreId::new(PathBuf::from("test/foo~1.3")).unwrap(), TEST_ENTRY_TNL).unwrap(); let string = entry.to_str().unwrap(); @@ -1072,7 +1070,7 @@ mod store_tests { let s = format!("test-{}", n); let entry = store.create(PathBuf::from(s.clone())).unwrap(); assert!(entry.verify().is_ok()); - let loc = entry.get_location().clone().into_pathbuf().unwrap(); + let loc = entry.get_location().clone().with_base(store.path()).into_pathbuf().unwrap(); assert!(loc.starts_with("/")); assert!(loc.ends_with(s)); } @@ -1093,7 +1091,7 @@ mod store_tests { assert!(entry.verify().is_ok()); - let loc = entry.get_location().clone().into_pathbuf().unwrap(); + let loc = entry.get_location().clone().with_base(store.path()).into_pathbuf().unwrap(); assert!(loc.starts_with("/")); assert!(loc.ends_with(s)); @@ -1125,7 +1123,7 @@ mod store_tests { .ok() .map(|entry| { assert!(entry.verify().is_ok()); - let loc = entry.get_location().clone().into_pathbuf().unwrap(); + let loc = entry.get_location().clone().with_base(store.path()).into_pathbuf().unwrap(); assert!(loc.starts_with("/")); assert!(loc.ends_with(s)); }); @@ -1139,12 +1137,10 @@ mod store_tests { let store = get_store(); for n in 1..100 { - let pb = StoreId::new_baseless(PathBuf::from(format!("test-{}", n))).unwrap(); + let pb = StoreId::new(PathBuf::from(format!("test-{}", n))).unwrap(); assert!(store.entries.read().unwrap().get(&pb).is_none()); assert!(store.create(pb.clone()).is_ok()); - - let pb = pb.with_base(store.path().clone()); assert!(store.entries.read().unwrap().get(&pb).is_some()); } } @@ -1156,12 +1152,10 @@ mod store_tests { let store = get_store(); for n in 1..100 { - let pb = StoreId::new_baseless(PathBuf::from(format!("test-{}", n))).unwrap(); + let pb = StoreId::new(PathBuf::from(format!("test-{}", n))).unwrap(); assert!(store.entries.read().unwrap().get(&pb).is_none()); assert!(store.retrieve(pb.clone()).is_ok()); - - let pb = pb.with_base(store.path().clone()); assert!(store.entries.read().unwrap().get(&pb).is_some()); } } @@ -1199,8 +1193,8 @@ mod store_tests { for n in 1..100 { if n % 2 == 0 { // every second - let id = StoreId::new_baseless(PathBuf::from(format!("t-{}", n))).unwrap(); - let id_mv = StoreId::new_baseless(PathBuf::from(format!("t-{}", n - 1))).unwrap(); + let id = StoreId::new(PathBuf::from(format!("t-{}", n))).unwrap(); + let id_mv = StoreId::new(PathBuf::from(format!("t-{}", n - 1))).unwrap(); { assert!(store.entries.read().unwrap().get(&id).is_none()); @@ -1211,16 +1205,14 @@ mod store_tests { } { - let id_with_base = id.clone().with_base(store.path().clone()); - assert!(store.entries.read().unwrap().get(&id_with_base).is_some()); + assert!(store.entries.read().unwrap().get(&id).is_some()); } let r = store.move_by_id(id.clone(), id_mv.clone()); assert!(r.map_err(|e| debug!("ERROR: {:?}", e)).is_ok()); { - let id_mv_with_base = id_mv.clone().with_base(store.path().clone()); - assert!(store.entries.read().unwrap().get(&id_mv_with_base).is_some()); + assert!(store.entries.read().unwrap().get(&id_mv).is_some()); } let res = store.get(id.clone()); diff --git a/lib/core/libimagstore/src/storeid.rs b/lib/core/libimagstore/src/storeid.rs index 17d9b67d..cc7b2881 100644 --- a/lib/core/libimagstore/src/storeid.rs +++ b/lib/core/libimagstore/src/storeid.rs @@ -43,87 +43,28 @@ use iter::retrieve::StoreRetrieveIterator; /// A StoreId object is a unique identifier for one entry in the store which might be present or /// not. /// -#[derive(Debug, Clone, Hash, Eq, PartialOrd, Ord)] -pub struct StoreId { - base: Option, - id: PathBuf, -} - -impl PartialEq for StoreId { - fn eq(&self, other: &StoreId) -> bool { - self.id == other.id - } -} +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub struct StoreId(PathBuf); impl StoreId { - pub fn new(base: Option, id: PathBuf) -> Result { - StoreId::new_baseless(id).map(|mut sid| { sid.base = base; sid }) - } - - /// Try to create a StoreId object from a filesystem-absolute path. - /// - /// Automatically creates a StoreId object which has a `base` set to `store_part` if stripping - /// the `store_part` from the `full_path` succeeded. - pub fn from_full_path(store_part: &PathBuf, full_path: D) -> Result - where D: Deref - { - let p = full_path - .strip_prefix(store_part) - .map_err(Error::from) - .context(err_msg("Error building Store Id from full path"))?; - StoreId::new(Some(store_part.clone()), PathBuf::from(p)) - } - - pub fn new_baseless(id: PathBuf) -> Result { + pub fn new(id: PathBuf) -> Result { debug!("Trying to get a new baseless id from: {:?}", id); if id.is_absolute() { debug!("Error: Id is absolute!"); Err(format_err!("Store Id local part is absolute: {}", id.display())) } else { debug!("Building Storeid object baseless"); - Ok(StoreId { - base: None, - id - }) + Ok(StoreId(id)) } } - pub fn without_base(mut self) -> StoreId { - self.base = None; - self - } - - pub fn with_base(mut self, base: PathBuf) -> Self { - self.base = Some(base); - self - } - - /// Transform the StoreId object into a PathBuf, error if the base of the StoreId is not - /// specified. - pub fn into_pathbuf(mut self) -> Result { - let base = self.base.take(); - let mut base = base.ok_or_else(|| { - format_err!("Store Id has no base: {:?}", self.id.display().to_string()) - })?; - base.push(self.id); - Ok(base) - } - - /// Check whether the StoreId exists (as in whether the file exists) - /// - /// # Warning - /// - /// Should be considered deprecated - pub fn exists(&self) -> Result { - self.clone().into_pathbuf().map(|pb| pb.exists()) + pub fn with_base<'a>(self, base: &'a PathBuf) -> StoreIdWithBase<'a> { + StoreIdWithBase(base, self.0) } pub fn to_str(&self) -> Result { - match self.base.as_ref() { - None => Ok(self.id.display().to_string()), - Some(ref base) => Ok(format!("{}/{}", base.display(), self.id.display())), - } + Ok(self.0.display().to_string()) } /// Helper function for creating a displayable String from StoreId @@ -145,12 +86,12 @@ impl StoreId { /// Can be used to check whether a StoreId points to an entry in a specific collection of /// StoreIds. pub fn components(&self) -> Components { - self.id.components() + self.0.components() } /// Get the _local_ part of a StoreId object, as in "the part from the store root to the entry". pub fn local(&self) -> &PathBuf { - &self.id + &self.0 } /// Check whether a StoreId points to an entry in a specific collection. @@ -166,7 +107,7 @@ impl StoreId { pub fn is_in_collection, V: AsRef<[S]>>(&self, colls: &V) -> bool { use std::path::Component; - self.id + self.0 .components() .zip(colls.as_ref().iter()) .all(|(component, pred_coll)| match component { @@ -179,7 +120,7 @@ impl StoreId { } pub fn local_push>(&mut self, path: P) { - self.id.push(path) + self.0.push(path) } } @@ -187,7 +128,7 @@ impl StoreId { impl Display for StoreId { fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FmtError> { - write!(fmt, "{}", self.id.display()) + write!(fmt, "{}", self.0.display()) } } @@ -206,10 +147,75 @@ impl IntoStoreId for StoreId { impl IntoStoreId for PathBuf { fn into_storeid(self) -> Result { - StoreId::new_baseless(self) + StoreId::new(self) } } +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) struct StoreIdWithBase<'a>(&'a PathBuf, PathBuf); + +impl<'a> StoreIdWithBase<'a> { + pub(crate) fn new(base: &'a PathBuf, path: PathBuf) -> Self { + StoreIdWithBase(base, path) + } + + pub(crate) fn without_base(self) -> StoreId { + StoreId(self.1) + } + + /// Transform the StoreId object into a PathBuf, error if the base of the StoreId is not + /// specified. + pub(crate) fn into_pathbuf(self) -> Result { + let mut base = self.0.clone(); + base.push(self.1); + Ok(base) + } + + /// Check whether the StoreId exists (as in whether the file exists) + pub fn exists(&self) -> Result { + self.clone().into_pathbuf().map(|pb| pb.exists()) + } + + pub fn to_str(&self) -> Result { + let mut base = self.0.clone(); + base.push(self.1.clone()); + Ok(base.display().to_string()) + } + + /// Try to create a StoreId object from a filesystem-absolute path. + /// + /// Automatically creates a StoreId object which has a `base` set to `store_part` if stripping + /// the `store_part` from the `full_path` succeeded. + pub(crate) fn from_full_path(store_part: &'a PathBuf, full_path: D) -> Result> + where D: Deref + { + let p = full_path + .strip_prefix(store_part) + .map_err(Error::from) + .context(err_msg("Error building Store Id from full path"))?; + Ok(StoreIdWithBase(store_part, PathBuf::from(p))) + } +} + +impl<'a> IntoStoreId for StoreIdWithBase<'a> { + fn into_storeid(self) -> Result { + Ok(StoreId(self.1)) + } +} + +impl<'a> Into for StoreIdWithBase<'a> { + fn into(self) -> StoreId { + StoreId(self.1) + } +} + +impl<'a> Display for StoreIdWithBase<'a> { + fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FmtError> { + write!(fmt, "{}/{}", self.0.display(), self.1.display()) + } +} + + #[macro_export] macro_rules! module_entry_path_mod { ($name:expr) => ( @@ -249,7 +255,7 @@ macro_rules! module_entry_path_mod { impl $crate::storeid::IntoStoreId for ModuleEntryPath { fn into_storeid(self) -> Result<$crate::storeid::StoreId> { - StoreId::new(None, self.0) + StoreId::new(self.0) } } } @@ -355,6 +361,7 @@ mod test { use std::path::PathBuf; use storeid::StoreId; + use storeid::StoreIdWithBase; use storeid::IntoStoreId; module_entry_path_mod!("test"); @@ -368,88 +375,63 @@ mod test { #[test] fn test_baseless_path() { - let id = StoreId::new_baseless(PathBuf::from("test")); + let id = StoreId::new(PathBuf::from("test")); assert!(id.is_ok()); - assert_eq!(id.unwrap(), StoreId { - base: None, - id: PathBuf::from("test") - }); + assert_eq!(id.unwrap(), StoreId(PathBuf::from("test"))); } #[test] fn test_base_path() { - let id = StoreId::from_full_path(&PathBuf::from("/tmp/"), PathBuf::from("/tmp/test")); + let id = StoreId::new(PathBuf::from("test")); assert!(id.is_ok()); - assert_eq!(id.unwrap(), StoreId { - base: Some(PathBuf::from("/tmp/")), - id: PathBuf::from("test") - }); + assert_eq!(id.unwrap(), StoreId(PathBuf::from("test"))); } #[test] fn test_adding_base_to_baseless_path() { - let id = StoreId::new_baseless(PathBuf::from("test")); + let id = StoreId::new(PathBuf::from("test")); assert!(id.is_ok()); let id = id.unwrap(); - assert_eq!(id, StoreId { base: None, id: PathBuf::from("test") }); + assert_eq!(id, StoreId(PathBuf::from("test"))); - let id = id.with_base(PathBuf::from("/tmp/")); - assert_eq!(id, StoreId { - base: Some(PathBuf::from("/tmp/")), - id: PathBuf::from("test") - }); + let storebase = PathBuf::from("/tmp/"); + let id = id.with_base(&storebase); + assert_eq!(id, StoreIdWithBase(&PathBuf::from("/tmp/"), PathBuf::from("test"))); } #[test] fn test_removing_base_from_base_path() { - let id = StoreId::from_full_path(&PathBuf::from("/tmp/"), PathBuf::from("/tmp/test")); + let id = StoreId::new(PathBuf::from("/tmp/test")); assert!(id.is_ok()); - let id = id.unwrap(); + let storebase = PathBuf::from("/tmp/"); + let id = id.unwrap().with_base(&storebase); - assert_eq!(id, StoreId { - base: Some(PathBuf::from("/tmp/")), - id: PathBuf::from("test") - }); + assert_eq!(id, StoreIdWithBase(&PathBuf::from("/tmp/"), PathBuf::from("test"))); let id = id.without_base(); - assert_eq!(id, StoreId { - base: None, - id: PathBuf::from("test") - }); - } - - #[test] - fn test_baseless_into_pathbuf_is_err() { - let id = StoreId::new_baseless(PathBuf::from("test")); - assert!(id.is_ok()); - assert!(id.unwrap().into_pathbuf().is_err()); - } - - #[test] - fn test_baseless_into_pathbuf_is_storeidhasnobaseerror() { - let id = StoreId::new_baseless(PathBuf::from("test")); - assert!(id.is_ok()); - - let pb = id.unwrap().into_pathbuf(); - assert!(pb.is_err()); + assert_eq!(id, StoreId(PathBuf::from("test"))); } #[test] fn test_basefull_into_pathbuf_is_ok() { - let id = StoreId::from_full_path(&PathBuf::from("/tmp/"), PathBuf::from("/tmp/test")); + let id = StoreId::new(PathBuf::from("/tmp/test")); assert!(id.is_ok()); - assert!(id.unwrap().into_pathbuf().is_ok()); + + let storebase = PathBuf::from("/tmp/"); + let id = id.unwrap().with_base(&storebase); + assert!(id.into_pathbuf().is_ok()); } #[test] fn test_basefull_into_pathbuf_is_correct() { - let id = StoreId::from_full_path(&PathBuf::from("/tmp/"), PathBuf::from("/tmp/test")); + let id = StoreId::new(PathBuf::from("/tmp/test")); assert!(id.is_ok()); - let pb = id.unwrap().into_pathbuf(); + let storebase = PathBuf::from("/tmp/"); + let pb = id.unwrap().with_base(&storebase).into_pathbuf(); assert!(pb.is_ok()); assert_eq!(pb.unwrap(), PathBuf::from("/tmp/test")); -- cgit v1.2.3 From 35a500f91c6dd1c792f5ea43d87f9b81da3f5146 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 2 Dec 2018 23:58:45 +0100 Subject: Delete unused code Signed-off-by: Matthias Beyer --- lib/core/libimagstore/src/storeid.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/core/libimagstore/src/storeid.rs b/lib/core/libimagstore/src/storeid.rs index cc7b2881..3842057e 100644 --- a/lib/core/libimagstore/src/storeid.rs +++ b/lib/core/libimagstore/src/storeid.rs @@ -171,17 +171,6 @@ impl<'a> StoreIdWithBase<'a> { Ok(base) } - /// Check whether the StoreId exists (as in whether the file exists) - pub fn exists(&self) -> Result { - self.clone().into_pathbuf().map(|pb| pb.exists()) - } - - pub fn to_str(&self) -> Result { - let mut base = self.0.clone(); - base.push(self.1.clone()); - Ok(base.display().to_string()) - } - /// Try to create a StoreId object from a filesystem-absolute path. /// /// Automatically creates a StoreId object which has a `base` set to `store_part` if stripping -- cgit v1.2.3 From b66371f79b22fd017faacbc7120306b916bd2776 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 2 Dec 2018 23:52:14 +0100 Subject: Rewrite Store::new* API: libimagstore does not export backend types With this change applied, libimagstore does not export backend representing types anymore. This change is necessar because when we want to switch the `StoreId` implementation from "complex and stateful" to "Stateless and Cow<'_, &str>", we need to be able to have a type which represents a "`StoreId` plus the path of the Store itself". This "the path of the Store itself" is passed around as reference, to minimize runtime impact. Because such a type should not be exported by the libimagstore crate, we make it `pub(crate)` internally. But because the backend APIs also have to use this type, we would export the (private) type in the APIs of the backend. Because of that we make the backend API also non-visible to crate users, which also decreases the surface of the libimagstore API itself. Besides: Remove function `Link::with_base()` which is not needed anymore (was used in tests only). Signed-off-by: Matthias Beyer --- lib/core/libimagrt/src/runtime.rs | 6 +---- lib/core/libimagstore/src/file_abstraction/fs.rs | 2 +- .../libimagstore/src/file_abstraction/inmemory.rs | 2 +- lib/core/libimagstore/src/file_abstraction/iter.rs | 2 +- lib/core/libimagstore/src/file_abstraction/mod.rs | 16 +++++-------- lib/core/libimagstore/src/lib.rs | 2 +- lib/core/libimagstore/src/store.rs | 27 ++++++++++++++-------- lib/core/libimagstore/src/storeid.rs | 3 ++- lib/domain/libimagtimetrack/src/iter/mod.rs | 7 +----- lib/entry/libimagentrycategory/src/store.rs | 4 +--- lib/entry/libimagentrydatetime/src/datetime.rs | 5 +--- lib/entry/libimagentrygps/src/entry.rs | 5 +--- lib/entry/libimagentrylink/src/external.rs | 5 +--- lib/entry/libimagentrylink/src/internal.rs | 21 +++-------------- lib/entry/libimagentrymarkdown/src/processor.rs | 4 +--- 15 files changed, 40 insertions(+), 71 deletions(-) diff --git a/lib/core/libimagrt/src/runtime.rs b/lib/core/libimagrt/src/runtime.rs index 8fb59022..e5fd4041 100644 --- a/lib/core/libimagrt/src/runtime.rs +++ b/lib/core/libimagrt/src/runtime.rs @@ -22,7 +22,6 @@ use std::process::Command; use std::env; use std::process::exit; use std::io::Stdin; -use std::sync::Arc; use std::io::StdoutLock; use std::borrow::Borrow; use std::result::Result as RResult; @@ -48,7 +47,6 @@ use libimagerror::trace::*; use libimagerror::io::ToExitCode; use libimagstore::store::Store; use libimagstore::storeid::StoreId; -use libimagstore::file_abstraction::InMemoryFileAbstraction; use libimagutil::debug_result::DebugResult; use spec::CliSpec; use atty; @@ -143,9 +141,7 @@ impl<'a> Runtime<'a> { trace!("Config = {:#?}", config); let store_result = if cli_app.use_inmemory_fs() { - Store::new_with_backend(storepath, - &config, - Arc::new(InMemoryFileAbstraction::default())) + Store::new_inmemory(storepath, &config) } else { Store::new(storepath, &config) }; diff --git a/lib/core/libimagstore/src/file_abstraction/fs.rs b/lib/core/libimagstore/src/file_abstraction/fs.rs index 05f48b89..f6264337 100644 --- a/lib/core/libimagstore/src/file_abstraction/fs.rs +++ b/lib/core/libimagstore/src/file_abstraction/fs.rs @@ -164,7 +164,7 @@ impl FileAbstraction for FSFileAbstraction { } } -pub(crate) struct WalkDirPathIterBuilder { +pub struct WalkDirPathIterBuilder { basepath: PathBuf } diff --git a/lib/core/libimagstore/src/file_abstraction/inmemory.rs b/lib/core/libimagstore/src/file_abstraction/inmemory.rs index 49f7c56d..43dddb0c 100644 --- a/lib/core/libimagstore/src/file_abstraction/inmemory.rs +++ b/lib/core/libimagstore/src/file_abstraction/inmemory.rs @@ -203,7 +203,7 @@ impl FileAbstraction for InMemoryFileAbstraction { } } -pub(crate) struct InMemPathIterBuilder(Vec); +pub struct InMemPathIterBuilder(Vec); impl PathIterBuilder for InMemPathIterBuilder { fn build_iter(&self) -> Box>> { diff --git a/lib/core/libimagstore/src/file_abstraction/iter.rs b/lib/core/libimagstore/src/file_abstraction/iter.rs index 7a5a5c8f..5ef5d386 100644 --- a/lib/core/libimagstore/src/file_abstraction/iter.rs +++ b/lib/core/libimagstore/src/file_abstraction/iter.rs @@ -26,7 +26,7 @@ use storeid::StoreIdWithBase; use file_abstraction::FileAbstraction; /// See documentation for PathIterator -pub trait PathIterBuilder { +pub(crate) trait PathIterBuilder { fn build_iter(&self) -> Box>>; fn in_collection(&mut self, c: &str); } diff --git a/lib/core/libimagstore/src/file_abstraction/mod.rs b/lib/core/libimagstore/src/file_abstraction/mod.rs index 13fd727a..9a80293e 100644 --- a/lib/core/libimagstore/src/file_abstraction/mod.rs +++ b/lib/core/libimagstore/src/file_abstraction/mod.rs @@ -27,18 +27,14 @@ use failure::Fallible as Result; use store::Entry; use storeid::StoreIdWithBase; -mod fs; -mod inmemory; -pub(crate) mod iter; - -pub use self::fs::FSFileAbstraction; -pub use self::fs::FSFileAbstractionInstance; -pub use self::inmemory::InMemoryFileAbstraction; -pub use self::inmemory::InMemoryFileAbstractionInstance; +pub mod fs; +pub mod inmemory; +pub mod iter; + use self::iter::PathIterator; /// An abstraction trait over filesystem actions -pub trait FileAbstraction : Debug { +pub(crate) trait FileAbstraction : Debug { fn remove_file(&self, path: &PathBuf) -> Result<()>; fn copy(&self, from: &PathBuf, to: &PathBuf) -> Result<()>; fn rename(&self, from: &PathBuf, to: &PathBuf) -> Result<()>; @@ -56,7 +52,7 @@ pub trait FileAbstraction : Debug { } /// An abstraction trait over actions on files -pub trait FileAbstractionInstance : Debug { +pub(crate) trait FileAbstractionInstance : Debug { /// Get the contents of the FileAbstractionInstance, as Entry object. /// diff --git a/lib/core/libimagstore/src/lib.rs b/lib/core/libimagstore/src/lib.rs index fe721daa..c3274f6b 100644 --- a/lib/core/libimagstore/src/lib.rs +++ b/lib/core/libimagstore/src/lib.rs @@ -58,5 +58,5 @@ pub mod storeid; pub mod iter; pub mod store; mod configuration; -pub mod file_abstraction; +mod file_abstraction; diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index cb85177f..3cd0bfeb 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -43,12 +43,10 @@ use failure::Error; use storeid::{IntoStoreId, StoreId}; use iter::Entries; +use file_abstraction::FileAbstraction; use file_abstraction::FileAbstractionInstance; - -// We re-export the following things so tests can use them -pub use file_abstraction::FileAbstraction; -pub use file_abstraction::FSFileAbstraction; -pub use file_abstraction::InMemoryFileAbstraction; +use file_abstraction::fs::FSFileAbstraction; +use file_abstraction::inmemory::InMemoryFileAbstraction; use libimagutil::debug_result::*; @@ -172,13 +170,24 @@ impl Store { Store::new_with_backend(location, store_config, backend) } + /// Create the store with an in-memory filesystem + /// + /// # Usage + /// + /// this is for testing purposes only + #[inline] + pub fn new_inmemory(location: PathBuf, store_config: &Option) -> Result { + let backend = Arc::new(InMemoryFileAbstraction::default()); + Self::new_with_backend(location, store_config, backend) + } + /// Create a Store object as descripbed in `Store::new()` documentation, but with an alternative /// backend implementation. /// /// Do not use directly, only for testing purposes. - pub fn new_with_backend(location: PathBuf, - store_config: &Option, - backend: Arc) -> Result { + pub(crate) fn new_with_backend(location: PathBuf, + store_config: &Option, + backend: Arc) -> Result { use configuration::*; debug!("Building new Store object"); @@ -1047,9 +1056,9 @@ mod store_tests { } use super::Store; - use file_abstraction::InMemoryFileAbstraction; pub fn get_store() -> Store { + use file_abstraction::inmemory::InMemoryFileAbstraction; let backend = Arc::new(InMemoryFileAbstraction::default()); Store::new_with_backend(PathBuf::from("/"), &None, backend).unwrap() } diff --git a/lib/core/libimagstore/src/storeid.rs b/lib/core/libimagstore/src/storeid.rs index 3842057e..477ae550 100644 --- a/lib/core/libimagstore/src/storeid.rs +++ b/lib/core/libimagstore/src/storeid.rs @@ -59,7 +59,7 @@ impl StoreId { } } - pub fn with_base<'a>(self, base: &'a PathBuf) -> StoreIdWithBase<'a> { + pub(crate) fn with_base<'a>(self, base: &'a PathBuf) -> StoreIdWithBase<'a> { StoreIdWithBase(base, self.0) } @@ -155,6 +155,7 @@ impl IntoStoreId for PathBuf { pub(crate) struct StoreIdWithBase<'a>(&'a PathBuf, PathBuf); impl<'a> StoreIdWithBase<'a> { + #[cfg(test)] pub(crate) fn new(base: &'a PathBuf, path: PathBuf) -> Self { StoreIdWithBase(base, path) } diff --git a/lib/domain/libimagtimetrack/src/iter/mod.rs b/lib/domain/libimagtimetrack/src/iter/mod.rs index 9bdda525..0cead41c 100644 --- a/lib/domain/libimagtimetrack/src/iter/mod.rs +++ b/lib/domain/libimagtimetrack/src/iter/mod.rs @@ -26,8 +26,6 @@ pub mod tag; #[cfg(test)] mod test { - use std::sync::Arc; - use chrono::naive::NaiveDate; use libimagstore::store::Store; @@ -37,10 +35,7 @@ mod test { fn get_store() -> Store { use std::path::PathBuf; - use libimagstore::file_abstraction::InMemoryFileAbstraction; - - let backend = Arc::new(InMemoryFileAbstraction::default()); - Store::new_with_backend(PathBuf::from("/"), &None, backend).unwrap() + Store::new_inmemory(PathBuf::from("/"), &None).unwrap() } #[test] diff --git a/lib/entry/libimagentrycategory/src/store.rs b/lib/entry/libimagentrycategory/src/store.rs index e05e8230..4c8dbbed 100644 --- a/lib/entry/libimagentrycategory/src/store.rs +++ b/lib/entry/libimagentrycategory/src/store.rs @@ -137,9 +137,7 @@ mod tests { use libimagstore::store::Store; pub fn get_store() -> Store { - use libimagstore::store::InMemoryFileAbstraction; - let backend = Arc::new(InMemoryFileAbstraction::default()); - Store::new_with_backend(PathBuf::from("/"), &None, backend).unwrap() + Store::new_inmemory(PathBuf::from("/"), &None).unwrap() } #[test] diff --git a/lib/entry/libimagentrydatetime/src/datetime.rs b/lib/entry/libimagentrydatetime/src/datetime.rs index 745f0807..acfa26c5 100644 --- a/lib/entry/libimagentrydatetime/src/datetime.rs +++ b/lib/entry/libimagentrydatetime/src/datetime.rs @@ -209,7 +209,6 @@ fn val_to_ndt(v: &Value) -> Result { #[cfg(test)] mod tests { use std::path::PathBuf; - use std::sync::Arc; use super::*; @@ -221,9 +220,7 @@ mod tests { use toml_query::read::TomlValueReadExt; pub fn get_store() -> Store { - use libimagstore::store::InMemoryFileAbstraction; - let backend = Arc::new(InMemoryFileAbstraction::default()); - Store::new_with_backend(PathBuf::from("/"), &None, backend).unwrap() + Store::new_inmemory(PathBuf::from("/"), &None).unwrap() } #[test] diff --git a/lib/entry/libimagentrygps/src/entry.rs b/lib/entry/libimagentrygps/src/entry.rs index 410ad2cd..6c6177eb 100644 --- a/lib/entry/libimagentrygps/src/entry.rs +++ b/lib/entry/libimagentrygps/src/entry.rs @@ -104,7 +104,6 @@ impl GPSEntry for Entry { #[cfg(test)] mod tests { use std::path::PathBuf; - use std::sync::Arc; use libimagstore::store::Store; @@ -115,9 +114,7 @@ mod tests { } fn get_store() -> Store { - use libimagstore::file_abstraction::InMemoryFileAbstraction; - let backend = Arc::new(InMemoryFileAbstraction::default()); - Store::new_with_backend(PathBuf::from("/"), &None, backend).unwrap() + Store::new_inmemory(PathBuf::from("/"), &None).unwrap() } #[test] diff --git a/lib/entry/libimagentrylink/src/external.rs b/lib/entry/libimagentrylink/src/external.rs index 486b0612..539bbe20 100644 --- a/lib/entry/libimagentrylink/src/external.rs +++ b/lib/entry/libimagentrylink/src/external.rs @@ -447,7 +447,6 @@ impl ExternalLinker for Entry { mod tests { use super::*; use std::path::PathBuf; - use std::sync::Arc; use libimagstore::store::Store; @@ -457,9 +456,7 @@ mod tests { } pub fn get_store() -> Store { - use libimagstore::file_abstraction::InMemoryFileAbstraction; - let backend = Arc::new(InMemoryFileAbstraction::default()); - Store::new_with_backend(PathBuf::from("/"), &None, backend).unwrap() + Store::new_inmemory(PathBuf::from("/"), &None).unwrap() } diff --git a/lib/entry/libimagentrylink/src/internal.rs b/lib/entry/libimagentrylink/src/internal.rs index 8323d656..751f0b1e 100644 --- a/lib/entry/libimagentrylink/src/internal.rs +++ b/lib/entry/libimagentrylink/src/internal.rs @@ -18,8 +18,6 @@ // use std::collections::BTreeMap; -#[cfg(test)] -use std::path::PathBuf; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; @@ -88,16 +86,6 @@ impl Link { } } - /// Helper wrapper around Link for StoreId - #[cfg(test)] - fn with_base(self, pb: PathBuf) -> Link { - match self { - Link::Id { link: s } => Link::Id { link: s.with_base(pb) }, - Link::Annotated { link: s, annotation: ann } => - Link::Annotated { link: s.with_base(pb), annotation: ann }, - } - } - fn to_value(&self) -> Result { match self { &Link::Id { link: ref s } => @@ -783,7 +771,6 @@ pub mod store_check { #[cfg(test)] mod test { use std::path::PathBuf; - use std::sync::Arc; use libimagstore::store::Store; @@ -795,9 +782,7 @@ mod test { } pub fn get_store() -> Store { - use libimagstore::file_abstraction::InMemoryFileAbstraction; - let backend = Arc::new(InMemoryFileAbstraction::default()); - Store::new_with_backend(PathBuf::from("/"), &None, backend).unwrap() + Store::new_inmemory(PathBuf::from("/"), &None).unwrap() } #[test] @@ -833,8 +818,8 @@ mod test { assert_eq!(e1_links.len(), 1); assert_eq!(e2_links.len(), 1); - assert!(e1_links.first().map(|l| l.clone().with_base(store.path().clone()).eq_store_id(e2.get_location())).unwrap_or(false)); - assert!(e2_links.first().map(|l| l.clone().with_base(store.path().clone()).eq_store_id(e1.get_location())).unwrap_or(false)); + assert!(e1_links.first().map(|l| l.clone().eq_store_id(e2.get_location())).unwrap_or(false)); + assert!(e2_links.first().map(|l| l.clone().eq_store_id(e1.get_location())).unwrap_or(false)); } { diff --git a/lib/entry/libimagentrymarkdown/src/processor.rs b/lib/entry/libimagentrymarkdown/src/processor.rs index 1adc90e0..cd3a9a6f 100644 --- a/lib/entry/libimagentrymarkdown/src/processor.rs +++ b/lib/entry/libimagentrymarkdown/src/processor.rs @@ -242,9 +242,7 @@ mod tests { } pub fn get_store() -> Store { - use libimagstore::file_abstraction::InMemoryFileAbstraction; - let fs = InMemoryFileAbstraction::default(); - Store::new_with_backend(PathBuf::from("/"), &None, Arc::new(fs)).unwrap() + Store::new_inmemory(PathBuf::from("/"), &None).unwrap() } #[test] -- cgit v1.2.3 From 642702b724e365ae67974bf4a0387dd54948f6e9 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Wed, 5 Dec 2018 19:01:42 +0100 Subject: Rewrite: StoreId::new_baseless() -> StoreId::new() Signed-off-by: Matthias Beyer --- bin/core/imag-link/src/main.rs | 8 ++++---- bin/core/imag-mv/src/main.rs | 4 ++-- bin/core/imag-tag/src/main.rs | 12 ++++++------ lib/core/libimagrt/src/runtime.rs | 2 +- lib/domain/libimagtimetrack/src/tag.rs | 2 +- lib/entry/libimagentrydatetime/src/datepath/compiler.rs | 2 +- lib/entry/libimagentrylink/src/internal.rs | 4 ++-- lib/entry/libimagentryref/src/refstore.rs | 4 ++-- lib/etc/libimaginteraction/src/ui.rs | 4 ++-- 9 files changed, 21 insertions(+), 21 deletions(-) diff --git a/bin/core/imag-link/src/main.rs b/bin/core/imag-link/src/main.rs index dc9be715..8f899d10 100644 --- a/bin/core/imag-link/src/main.rs +++ b/bin/core/imag-link/src/main.rs @@ -168,8 +168,8 @@ fn link_from_to<'a, I>(rt: &'a Runtime, from: &'a str, to: I) } else { debug!("Linking internally: {:?} -> {:?}", from, entry); - let from_id = StoreId::new_baseless(PathBuf::from(from)).map_err_trace_exit_unwrap(); - let entr_id = StoreId::new_baseless(PathBuf::from(entry)).map_err_trace_exit_unwrap(); + let from_id = StoreId::new(PathBuf::from(from)).map_err_trace_exit_unwrap(); + let entr_id = StoreId::new(PathBuf::from(entry)).map_err_trace_exit_unwrap(); if from_id == entr_id { error!("Cannot link entry with itself. Exiting"); @@ -366,13 +366,13 @@ mod tests { let mut path = PathBuf::new(); path.set_file_name(name); - let default_entry = Entry::new(StoreId::new_baseless(PathBuf::from("")).unwrap()) + let default_entry = Entry::new(StoreId::new(PathBuf::from("")).unwrap()) .to_str() .unwrap(); debug!("Default entry constructed"); - let id = StoreId::new_baseless(path)?; + let id = StoreId::new(path)?; debug!("StoreId constructed: {:?}", id); let mut entry = rt.store().create(id.clone())?; diff --git a/bin/core/imag-mv/src/main.rs b/bin/core/imag-mv/src/main.rs index ff267067..f5d4d341 100644 --- a/bin/core/imag-mv/src/main.rs +++ b/bin/core/imag-mv/src/main.rs @@ -72,7 +72,7 @@ fn main() { .cli() .value_of("source") .map(PathBuf::from) - .map(StoreId::new_baseless) + .map(StoreId::new) .unwrap() // unwrap safe by clap .map_err_trace_exit_unwrap(); @@ -80,7 +80,7 @@ fn main() { .cli() .value_of("dest") .map(PathBuf::from) - .map(StoreId::new_baseless) + .map(StoreId::new) .unwrap() // unwrap safe by clap .map_err_trace_exit_unwrap(); diff --git a/bin/core/imag-tag/src/main.rs b/bin/core/imag-tag/src/main.rs index ddf3fa70..e3ba7f20 100644 --- a/bin/core/imag-tag/src/main.rs +++ b/bin/core/imag-tag/src/main.rs @@ -264,11 +264,11 @@ mod tests { let mut path = PathBuf::new(); path.set_file_name(name); - let default_entry = Entry::new(StoreId::new_baseless(PathBuf::from("")).unwrap()) + let default_entry = Entry::new(StoreId::new(PathBuf::from("")).unwrap()) .to_str() .unwrap(); - let id = StoreId::new_baseless(path)?; + let id = StoreId::new(path)?; let mut entry = rt.store().create(id.clone())?; entry.get_content_mut().push_str(&default_entry); @@ -303,7 +303,7 @@ mod tests { debug!("Add-tags: {:?}", add); debug!("Altering things"); - alter(&rt, StoreId::new_baseless(id.clone()).unwrap(), add, None); + alter(&rt, StoreId::new(id.clone()).unwrap(), add, None); debug!("Altered"); let test_entry = rt.store().get(id).unwrap().unwrap(); @@ -338,7 +338,7 @@ mod tests { debug!("Rem-tags: {:?}", rem); debug!("Altering things"); - alter(&rt, StoreId::new_baseless(id.clone()).unwrap(), add, rem); + alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem); debug!("Altered"); let test_entry = rt.store().get(id).unwrap().unwrap(); @@ -366,7 +366,7 @@ mod tests { debug!("Rem-tags: {:?}", rem); debug!("Altering things"); - alter(&rt, StoreId::new_baseless(id.clone()).unwrap(), add, rem); + alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem); debug!("Altered"); let test_entry = rt.store().get(id).unwrap().unwrap(); @@ -394,7 +394,7 @@ mod tests { debug!("Rem-tags: {:?}", rem); debug!("Altering things"); - alter(&rt, StoreId::new_baseless(id.clone()).unwrap(), add, rem); + alter(&rt, StoreId::new(id.clone()).unwrap(), add, rem); debug!("Altered"); let test_entry = rt.store().get(id).unwrap().unwrap(); diff --git a/lib/core/libimagrt/src/runtime.rs b/lib/core/libimagrt/src/runtime.rs index e5fd4041..4f2a485f 100644 --- a/lib/core/libimagrt/src/runtime.rs +++ b/lib/core/libimagrt/src/runtime.rs @@ -414,7 +414,7 @@ impl<'a> Runtime<'a> { trace!("Got IDs = {}", buf); buf.lines() .map(PathBuf::from) - .map(|id| StoreId::new_baseless(id).map_err(Error::from)) + .map(|id| StoreId::new(id).map_err(Error::from)) .collect() }) } else { diff --git a/lib/domain/libimagtimetrack/src/tag.rs b/lib/domain/libimagtimetrack/src/tag.rs index 2f099878..0bc96c42 100644 --- a/lib/domain/libimagtimetrack/src/tag.rs +++ b/lib/domain/libimagtimetrack/src/tag.rs @@ -65,7 +65,7 @@ impl<'a> From<&'a String> for TimeTrackingTag { impl IntoStoreId for TimeTrackingTag { fn into_storeid(self) -> Result { - StoreId::new_baseless(PathBuf::from(self.0)) + StoreId::new(PathBuf::from(self.0)) } } diff --git a/lib/entry/libimagentrydatetime/src/datepath/compiler.rs b/lib/entry/libimagentrydatetime/src/datepath/compiler.rs index 037803dc..d7515d16 100644 --- a/lib/entry/libimagentrydatetime/src/datepath/compiler.rs +++ b/lib/entry/libimagentrydatetime/src/datepath/compiler.rs @@ -119,7 +119,7 @@ impl DatePathCompiler { } } - StoreId::new_baseless(PathBuf::from(s)) + StoreId::new(PathBuf::from(s)) } } diff --git a/lib/entry/libimagentrylink/src/internal.rs b/lib/entry/libimagentrylink/src/internal.rs index 751f0b1e..7b092d24 100644 --- a/lib/entry/libimagentrylink/src/internal.rs +++ b/lib/entry/libimagentrylink/src/internal.rs @@ -568,7 +568,7 @@ fn process_rw_result(links: Result>) -> Result { .map(|link| { debug!("Matching the link: {:?}", link); match link { - Value::String(s) => StoreId::new_baseless(PathBuf::from(s)) + Value::String(s) => StoreId::new(PathBuf::from(s)) .map(|s| Link::Id { link: s }) .map_err(From::from) , @@ -588,7 +588,7 @@ fn process_rw_result(links: Result>) -> Result { debug!("Ok, here we go with building a Link::Annotated"); match (link, anno) { (Value::String(link), Value::String(anno)) => { - StoreId::new_baseless(PathBuf::from(link)) + StoreId::new(PathBuf::from(link)) .map_err(From::from) .map(|link| { Link::Annotated { diff --git a/lib/entry/libimagentryref/src/refstore.rs b/lib/entry/libimagentryref/src/refstore.rs index 58f05b7b..c378d6fd 100644 --- a/lib/entry/libimagentryref/src/refstore.rs +++ b/lib/entry/libimagentryref/src/refstore.rs @@ -91,7 +91,7 @@ impl<'a> RefStore<'a> for Store { fn get_ref>(&'a self, hash: H) -> Result>> { - let sid = StoreId::new_baseless(PathBuf::from(format!("{}/{}", RPG::collection(), hash.as_ref()))) + let sid = StoreId::new(PathBuf::from(format!("{}/{}", RPG::collection(), hash.as_ref()))) .map_err(Error::from)?; debug!("Getting: {:?}", sid); @@ -104,7 +104,7 @@ impl<'a> RefStore<'a> for Store { { let hash = RPG::unique_hash(&path)?; let pathbuf = PathBuf::from(format!("{}/{}", RPG::collection(), hash)); - let sid = StoreId::new_baseless(pathbuf.clone())?; + let sid = StoreId::new(pathbuf.clone())?; debug!("Creating: {:?}", sid); self.create(sid) diff --git a/lib/etc/libimaginteraction/src/ui.rs b/lib/etc/libimaginteraction/src/ui.rs index 09395d54..0ad099a7 100644 --- a/lib/etc/libimaginteraction/src/ui.rs +++ b/lib/etc/libimaginteraction/src/ui.rs @@ -55,7 +55,7 @@ pub fn get_id(matches: &ArgMatches) -> Result> { vals.into_iter() .fold(Ok(vec![]), |acc, elem| { acc.and_then(|mut v| { - let elem = StoreId::new_baseless(PathBuf::from(String::from(elem)))?; + let elem = StoreId::new(PathBuf::from(String::from(elem)))?; v.push(elem); Ok(v) }) @@ -70,7 +70,7 @@ pub fn get_or_select_id(matches: &ArgMatches, store_path: &PathBuf) -> Result Date: Thu, 13 Dec 2018 01:28:01 +0100 Subject: Fix libimagentrylink for new StoreId API Signed-off-by: Matthias Beyer --- lib/entry/libimagentrylink/src/internal.rs | 34 +++++++++++++++--------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/entry/libimagentrylink/src/internal.rs b/lib/entry/libimagentrylink/src/internal.rs index 7b092d24..65277095 100644 --- a/lib/entry/libimagentrylink/src/internal.rs +++ b/lib/entry/libimagentrylink/src/