Merge pull request #1208 from matthiasbeyer/libimagdiary-bookmarkcollection-nonwrapping

libimagdiary: Do not wrap store types

4 files changed, 96 insertions, 113 deletions

diff --git a/bin/domain/imag-bookmark/src/main.rs b/bin/domain/imag-bookmark/src/main.rs
index 3529b0ef..7cfc8bb3 100644
--- a/bin/domain/imag-bookmark/src/main.rs
+++ b/bin/domain/imag-bookmark/src/main.rs
@@ -51,9 +51,10 @@ use toml_query::read::TomlValueReadExt;
 use libimagrt::runtime::Runtime;
 use libimagrt::setup::generate_runtime_setup;
 use libimagbookmark::collection::BookmarkCollection;
+use libimagbookmark::collection::BookmarkCollectionStore;
+use libimagbookmark::error::BookmarkError as BE;
 use libimagbookmark::link::Link as BookmarkLink;
 use libimagerror::trace::{MapErrTrace, trace_error, trace_error_exit};
-use libimagutil::info_result::*;
 
 mod ui;
 
@@ -85,16 +86,18 @@ fn add(rt: &Runtime) {
     let scmd = rt.cli().subcommand_matches("add").unwrap();
     let coll = get_collection_name(rt, "add", "collection");
 
-    BookmarkCollection::get(rt.store(), &coll)
-        .and_then(|mut collection| {
-            for url in scmd.values_of("urls").unwrap() { // unwrap saved by clap
-                let _ = collection.add_link(BookmarkLink::from(url))?;
-            }
-            Ok(())
-        })
-        .map_err_trace()
-        .map_info_str("Ready")
-        .ok();
+    let mut collection = BookmarkCollectionStore::get(rt.store(), &coll)
+        .map_err_trace_exit_unwrap(1)
+        .ok_or(BE::from(format!("No BookmarkcollectionStore '{}' found", coll)))
+        .map_err_trace_exit_unwrap(1);
+
+    for url in scmd.values_of("urls").unwrap() { // unwrap saved by clap
+        let _ = collection
+            .add_link(rt.store(), BookmarkLink::from(url))
+            .map_err_trace_exit_unwrap(1);
+    }
+
+    info!("Ready");
 }
 
 fn collection(rt: &Runtime) {
@@ -102,7 +105,7 @@ fn collection(rt: &Runtime) {
 
     if scmd.is_present("add") { // adding a new collection
         let name = scmd.value_of("add").unwrap();
-        if let Ok(_) = BookmarkCollection::new(rt.store(), &name) {
+        if let Ok(_) = BookmarkCollectionStore::new(rt.store(), &name) {
             info!("Created: {}", name);
         } else {
             warn!("Creating collection {} failed", name);
@@ -112,7 +115,7 @@ fn collection(rt: &Runtime) {
 
     if scmd.is_present("remove") { // remove a collection
         let name = scmd.value_of("remove").unwrap();
-        if let Ok(_) = BookmarkCollection::delete(rt.store(), &name) {
+        if let Ok(_) = BookmarkCollectionStore::delete(rt.store(), &name) {
             info!("Deleted: {}", name);
         } else {
             warn!("Deleting collection {} failed", name);
@@ -124,23 +127,25 @@ fn collection(rt: &Runtime) {
 fn list(rt: &Runtime) {
     let coll = get_collection_name(rt, "list", "collection");
 
-    BookmarkCollection::get(rt.store(), &coll)
-        .map(|collection| {
-            match collection.links() {
-                Ok(links) => {
-                    debug!("Listing...");
-                    for (i, link) in links.enumerate() {
-                        match link {
-                            Ok(link) => println!("{: >3}: {}", i, link),
-                            Err(e)   => trace_error(&e)
-                        }
-                    };
-                    debug!("... ready with listing");
-                },
-                Err(e) => trace_error_exit(&e, 1),
-            }
-        })
-        .ok();
+    let collection = BookmarkCollectionStore::get(rt.store(), &coll)
+        .map_err_trace_exit_unwrap(1)
+        .ok_or(BE::from(format!("No BookmarkcollectionStore '{}' found", coll)))
+        .map_err_trace_exit_unwrap(1);
+
+    match collection.links(rt.store()) {
+        Ok(links) => {
+            debug!("Listing...");
+            for (i, link) in links.enumerate() {
+                match link {
+                    Ok(link) => println!("{: >3}: {}", i, link),
+                    Err(e)   => trace_error(&e)
+                }
+            };
+            debug!("... ready with listing");
+        },
+        Err(e) => trace_error_exit(&e, 1),
+    }
+
     info!("Ready");
 }
 
@@ -148,13 +153,17 @@ fn remove(rt: &Runtime) {
     let scmd = rt.cli().subcommand_matches("remove").unwrap();
     let coll = get_collection_name(rt, "list", "collection");
 
-    BookmarkCollection::get(rt.store(), &coll)
-        .map(|mut collection| {
-            for url in scmd.values_of("urls").unwrap() { // enforced by clap
-                collection.remove_link(BookmarkLink::from(url)).map_err(|e| trace_error(&e)).ok();
-            }
-        })
-        .ok();
+    let mut collection = BookmarkCollectionStore::get(rt.store(), &coll)
+        .map_err_trace_exit_unwrap(1)
+        .ok_or(BE::from(format!("No BookmarkcollectionStore '{}' found", coll)))
+        .map_err_trace_exit_unwrap(1);
+
+    for url in scmd.values_of("urls").unwrap() { // enforced by clap
+        collection
+            .remove_link(rt.store(), BookmarkLink::from(url))
+            .map_err_trace_exit_unwrap(1);
+    }
+
     info!("Ready");
 }
 
diff --git a/doc/src/09020-changelog.md b/doc/src/09020-changelog.md
index cf1a1954..03bbb79b 100644
--- a/doc/src/09020-changelog.md
+++ b/doc/src/09020-changelog.md
@@ -32,6 +32,7 @@ This section contains the changelog from the last release to the next release.
 * Minor changes
     * Internals were refactored from `match`ing all the things into function
       chaining
+    * `libimagbookmark` does not longer wrap types from the store.
 * Bugfixes
 
 ## 0.5.0
diff --git a/lib/domain/libimagbookmark/src/collection.rs b/lib/domain/libimagbookmark/src/collection.rs
index 8842b022..b41be3af 100644
--- a/lib/domain/libimagbookmark/src/collection.rs
+++ b/lib/domain/libimagbookmark/src/collection.rs
@@ -23,20 +23,16 @@
 //! from the libimagentrylink::external::ExternalLinker trait on this to generate external links.
 //!
 //! The BookmarkCollection type offers helper functions to get all links or such things.
-use std::ops::Deref;
-use std::ops::DerefMut;
 
 use regex::Regex;
 
-use error::BookmarkErrorKind as BEK;
-use error::BookmarkError as BE;
-use error::ResultExt;
 use error::Result;
 use module_path::ModuleEntryPath;
 
 use libimagstore::store::Store;
-use libimagstore::storeid::IntoStoreId;
+use libimagstore::store::Entry;
 use libimagstore::store::FileLockEntry;
+use libimagstore::storeid::IntoStoreId;
 use libimagentrylink::external::ExternalLinker;
 use libimagentrylink::external::iter::UrlIter;
 use libimagentrylink::internal::InternalLinker;
@@ -46,99 +42,81 @@ use link::Link;
 
 use self::iter::LinksMatchingRegexIter;
 
-pub struct BookmarkCollection<'a> {
-    fle: FileLockEntry<'a>,
-    store: &'a Store,
-}
-
-/// {Internal, External}Linker is implemented as Deref is implemented
-impl<'a> Deref for BookmarkCollection<'a> {
-    type Target = FileLockEntry<'a>;
-
-    fn deref(&self) -> &FileLockEntry<'a> {
-        &self.fle
-    }
-
-}
-
-impl<'a> DerefMut for BookmarkCollection<'a> {
-
-    fn deref_mut(&mut self) -> &mut FileLockEntry<'a> {
-        &mut self.fle
-    }
-
+pub trait BookmarkCollectionStore<'a> {
+    fn new(&'a self, name: &str)                     -> Result<FileLockEntry<'a>>;
+    fn get(&'a self, name: &str)                     -> Result<Option<FileLockEntry<'a>>>;
+    fn delete(&'a self, name: &str)                     -> Result<()>;
 }
 
-impl<'a> BookmarkCollection<'a> {
+impl<'a> BookmarkCollectionStore<'a> for Store {
 
-    pub fn new(store: &'a Store, name: &str) -> Result<BookmarkCollection<'a>> {
+    fn new(&'a self, name: &str) -> Result<FileLockEntry<'a>> {
         ModuleEntryPath::new(name)
             .into_storeid()
-            .and_then(|id| store.create(id))
-            .map(|fle| {
-                BookmarkCollection {
-                    fle: fle,
-                    store: store,
-                }
-            })
-            .chain_err(|| BEK::StoreReadError)
+            .and_then(|id| self.create(id).map_err(From::from))
+            .map_err(From::from)
     }
 
-    pub fn get(store: &'a Store, name: &str) -> Result<BookmarkCollection<'a>> {
+    fn get(&'a self, name: &str) -> Result<Option<FileLockEntry<'a>>> {
         ModuleEntryPath::new(name)
             .into_storeid()
-            .and_then(|id| store.get(id))
-            .chain_err(|| BEK::StoreReadError)
-            .and_then(|fle| {
-                fle.ok_or(BE::from_kind(BEK::CollectionNotFound))
-                    .map(|e| BookmarkCollection { fle: e, store: store })
-            })
+            .and_then(|id| self.get(id).map_err(From::from))
+            .map_err(From::from)
     }
 
-    pub fn delete(store: &Store, name: &str) -> Result<()> {
+    fn delete(&'a self, name: &str) -> Result<()> {
         ModuleEntryPath::new(name)
             .into_storeid()
-            .and_then(|id| store.delete(id))
-            .chain_err(|| BEK::StoreReadError)
+            .and_then(|id| self.delete(id).map_err(From::from))
+            .map_err(From::from)
     }
 
-    pub fn links(&self) -> Result<UrlIter> {
-        self.fle.get_external_links(&self.store).chain_err(|| BEK::LinkError)
+}
+
+pub trait BookmarkCollection : Sized + InternalLinker + ExternalLinker {
+    fn links<'a>(&self, store: &'a Store)                        -> Result<UrlIter<'a>>;
+    fn link_entries(&self)                                   -> Result<Vec<StoreLink>>;
+    fn add_link(&mut self, store: &Store, l: Link)           -> Result<()>;
+    fn get_links_matching<'a>(&self, store: &'a Store, r: Regex) -> Result<LinksMatchingRegexIter<'a>>;
+    fn remove_link(&mut self, store: &Store, l: Link)        -> Result<()>;
+}
+
+impl BookmarkCollection for Entry {
+
+    fn links<'a>(&self, store: &'a Store) -> Result<UrlIter<'a>> {
+        self.get_external_links(store).map_err(From::from)
     }
 
-    pub fn link_entries(&self) -> Result<Vec<StoreLink>> {
+    fn link_entries(&self) -> Result<Vec<StoreLink>> {
         use libimagentrylink::external::is_external_link_storeid;
 
-        self.fle
-            .get_internal_links()
+        self.get_internal_links()
             .map(|v| v.filter(|id| is_external_link_storeid(id)).collect())
-            .chain_err(|| BEK::StoreReadError)
+            .map_err(From::from)
     }
 
-    pub fn add_link(&mut self, l: Link) -> Result<()> {
+    fn add_link(&mut self, store: &Store, l: Link) -> Result<()> {
         use link::IntoUrl;
 
         l.into_url()
-            .and_then(|url| self.add_external_link(self.store, url).chain_err(|| BEK::LinkingError))
-            .chain_err(|| BEK::LinkError)
+            .and_then(|url| self.add_external_link(store, url).map_err(From::from))
+            .map_err(From::from)
     }
 
-    pub fn get_links_matching(&self, r: Regex) -> Result<LinksMatchingRegexIter<'a>> {
+    fn get_links_matching<'a>(&self, store: &'a Store, r: Regex) -> Result<LinksMatchingRegexIter<'a>> {
         use self::iter::IntoLinksMatchingRegexIter;
 
-        self.get_external_links(self.store)
-            .chain_err(|| BEK::LinkError)
+        self.get_external_links(store)
             .map(|iter| iter.matching_regex(r))
+            .map_err(From::from)
     }
 
-    pub fn remove_link(&mut self, l: Link) -> Result<()> {
+    fn remove_link(&mut self, store: &Store, l: Link) -> Result<()> {
         use link::IntoUrl;
 
         l.into_url()
-            .and_then(|url| {
-                self.remove_external_link(self.store, url).chain_err(|| BEK::LinkingError)
-            })
-            .chain_err(|| BEK::LinkError)
+            .and_then(|url| self.remove_external_link(store, url).map_err(From::from))
+            .map_err(From::from)
     }
 
 }
@@ -146,7 +124,7 @@ impl<'a> BookmarkCollection<'a> {
 pub mod iter {
     use link::Link;
     use error::Result;
-    use error::{ResultExt, BookmarkErrorKind as BEK};
+    use error::BookmarkError as BE;
 
     pub struct LinkIter<I>(I)
         where I: Iterator<Item = Link>;
@@ -189,7 +167,7 @@ pub mod iter {
             loop {
                 let n = match self.0.next() {
                     Some(Ok(n))  => n,
-                    Some(Err(e)) => return Some(Err(e).chain_err(|| BEK::LinkError)),
+                    Some(Err(e)) => return Some(Err(BE::from(e))),
                     None         => return None,
                 };
 
diff --git a/lib/domain/libimagbookmark/src/error.rs b/lib/domain/libimagbookmark/src/error.rs
index 0001e76a..15c41435 100644
--- a/lib/domain/libimagbookmark/src/error.rs
+++ b/lib/domain/libimagbookmark/src/error.rs
@@ -22,17 +22,12 @@ error_chain! {
         BookmarkError, BookmarkErrorKind, ResultExt, Result;
     }
 
-    errors {
-        StoreReadError     {
-            description("Store read error")
-            display("Store read error")
-        }
-
-        LinkError          {
-            description("Link error")
-            display("Link error")
-        }
+    links {
+        StoreError(::libimagstore::error::StoreError, ::libimagstore::error::StoreErrorKind);
+        LinkError(::libimagentrylink::error::LinkError, ::libimagentrylink::error::LinkErrorKind);
+    }
 
+    errors {
         LinkParsingError   {
             description("Link parsing error")
             display("Link parsing error")