From b66371f79b22fd017faacbc7120306b916bd2776 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 2 Dec 2018 23:52:14 +0100 Subject: [PATCH] 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 +---- .../libimagstore/src/file_abstraction/fs.rs | 2 +- .../src/file_abstraction/inmemory.rs | 2 +- .../libimagstore/src/file_abstraction/iter.rs | 2 +- .../libimagstore/src/file_abstraction/mod.rs | 14 ++++------ 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 +-- .../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 +++------------ .../libimagentrymarkdown/src/processor.rs | 4 +-- 15 files changed, 39 insertions(+), 70 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 mod fs; +pub mod inmemory; +pub mod iter; -pub use self::fs::FSFileAbstraction; -pub use self::fs::FSFileAbstractionInstance; -pub use self::inmemory::InMemoryFileAbstraction; -pub use self::inmemory::InMemoryFileAbstractionInstance; 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]