Merge branch 'remove-link-annotation-support' into master

lib/entry/libimagentrylink/Cargo.toml

@@ -27,14 +27,20 @@ url = "1.5"
 sha-1 = "0.7"
 hex = "0.3"
 is-match = "0.1"
-toml-query = "0.9"
 failure        = "0.1"
 failure_derive = "0.1"
+serde = "1"
+serde_derive = "1"
 
 libimagstore = { version = "0.10.0", path = "../../../lib/core/libimagstore" }
 libimagerror = { version = "0.10.0", path = "../../../lib/core/libimagerror" }
 libimagutil  = { version = "0.10.0", path = "../../../lib/etc/libimagutil" }
 
+[dependencies.toml-query]
+version = "0.9"
+default-features = false
+features = [ "typed" ]
+
 [dev-dependencies]
 env_logger = "0.5"
 

lib/entry/libimagentrylink/src/iter.rs

@@ -54,6 +54,8 @@ impl Iterator for LinkIter {
     }
 }
 
+
+
 pub trait IntoValues {
     fn into_values(self) -> Vec<Result<Value>>;
 }

lib/entry/libimagentrylink/src/lib.rs

@@ -44,6 +44,8 @@ extern crate toml_query;
 extern crate url;
 extern crate sha1;
 extern crate hex;
+extern crate serde;
+#[macro_use] extern crate serde_derive;
 #[macro_use] extern crate failure;
 #[macro_use] extern crate is_match;
 

lib/entry/libimagentrylink/src/link.rs

@@ -23,7 +23,6 @@ use libimagstore::store::Store;
 use libimagerror::errors::ErrorMsg as EM;
 
 use toml::Value;
-use toml::map::Map;
 use failure::ResultExt;
 use failure::Fallible as Result;
 use failure::Error;
@@ -31,7 +30,6 @@ use failure::Error;
 #[derive(Eq, PartialOrd, Ord, Hash, Debug, Clone)]
 pub enum Link {
     Id          { link: StoreId },
-    Annotated   { link: StoreId, annotation: String },
 }
 
 impl Link {
@@ -39,7 +37,6 @@ impl Link {
     pub fn exists(&self, store: &Store) -> Result<bool> {
         match *self {
             Link::Id { ref link }             => store.exists(link.clone()),
-            Link::Annotated { ref link, .. }  => store.exists(link.clone()),
         }
         .map_err(From::from)
     }
@@ -47,16 +44,14 @@ impl Link {
     pub fn to_str(&self) -> Result<String> {
         match *self {
             Link::Id { ref link }             => link.to_str(),
-            Link::Annotated { ref link, .. }  => link.to_str(),
         }
         .map_err(From::from)
     }
 
-
+    #[cfg(test)]
     pub(crate) fn eq_store_id(&self, id: &StoreId) -> bool {
         match self {
             &Link::Id { link: ref s }             => s.eq(id),
-            &Link::Annotated { link: ref s, .. }  => s.eq(id),
         }
     }
 
@@ -64,30 +59,16 @@ impl Link {
     pub fn get_store_id(&self) -> &StoreId {
         match self {
             &Link::Id { link: ref s }             => s,
-            &Link::Annotated { link: ref s, .. }  => s,
         }
     }
 
     pub(crate) fn to_value(&self) -> Result<Value> {
         match self {
-            &Link::Id { link: ref s } =>
+            Link::Id { ref link } => link
-                s.to_str()
+                .to_str()
                 .map(Value::String)
                 .context(EM::ConversionError)
                 .map_err(Error::from),
-            &Link::Annotated { ref link, annotation: ref anno } => {
-                link.to_str()
-                    .map(Value::String)
-                    .context(EM::ConversionError)
-                    .map_err(Error::from)
-                    .map(|link| {
-                        let mut tab = Map::new();
-
-                        tab.insert("link".to_owned(),       link);
-                        tab.insert("annotation".to_owned(), Value::String(anno.clone()));
-                        Value::Table(tab)
-                    })
-            }
         }
     }
 
@@ -97,10 +78,6 @@ impl ::std::cmp::PartialEq for Link {
     fn eq(&self, other: &Self) -> bool {
         match (self, other) {
             (&Link::Id { link: ref a }, &Link::Id { link: ref b }) => a.eq(&b),
-            (&Link::Annotated { link: ref a, annotation: ref ann1 },
-             &Link::Annotated { link: ref b, annotation: ref ann2 }) =>
-                (a, ann1).eq(&(b, ann2)),
-            _ => false,
         }
     }
 }
@@ -116,7 +93,6 @@ impl Into<StoreId> for Link {
     fn into(self) -> StoreId {
         match self {
             Link::Id { link }            => link,
-            Link::Annotated { link, .. } => link,
         }
     }
 }
@@ -125,7 +101,6 @@ impl IntoStoreId for Link {
     fn into_storeid(self) -> Result<StoreId> {
         match self {
             Link::Id { link }            => Ok(link),
-            Link::Annotated { link, .. } => Ok(link),
         }
     }
 }
@@ -134,7 +109,6 @@ impl AsRef<StoreId> for Link {
     fn as_ref(&self) -> &StoreId {
         match self {
             &Link::Id { ref link }            => &link,
-            &Link::Annotated { ref link, .. } => &link,
         }
     }
 }

lib/entry/libimagentrylink/src/linkable.rs

@@ -17,27 +17,25 @@
 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
 //
 
+use std::path::PathBuf;
+
 use libimagstore::storeid::StoreId;
 use libimagstore::store::Entry;
 use libimagstore::store::Store;
-use libimagerror::errors::ErrorMsg as EM;
 
+use toml_query::read::Partial;
 use toml_query::read::TomlValueReadExt;
 use toml_query::insert::TomlValueInsertExt;
 use failure::ResultExt;
 use failure::Fallible as Result;
-use failure::Error;
 use failure::err_msg;
 
 use crate::iter::LinkIter;
-use crate::iter::IntoValues;
 use crate::link::Link;
 
-use toml::Value;
-
 pub trait Linkable {
 
-    /// Get the internal links from the implementor object
+    /// Get all links
     fn links(&self) -> Result<LinkIter>;
 
     /// Add an internal link to the implementor object
@@ -49,8 +47,24 @@ pub trait Linkable {
     /// Remove _all_ internal links
     fn unlink(&mut self, store: &Store) -> Result<()>;
 
-    /// Add internal annotated link
+}
-    fn add_annotated_link(&mut self, link: &mut Entry, annotation: String) -> Result<()>;
+
+#[derive(Serialize, Deserialize, Debug)]
+struct LinkPartial {
+    internal: Option<Vec<String>>,
+}
+
+impl Default for LinkPartial {
+    fn default() -> Self {
+        LinkPartial {
+            internal: None,
+        }
+    }
+}
+
+impl<'a> Partial<'a> for LinkPartial {
+    const LOCATION: &'static str = "links";
+    type Output = Self;
 }
 
 impl Linkable for Entry {
@@ -58,44 +72,71 @@ impl Linkable for Entry {
     fn links(&self) -> Result<LinkIter> {
         debug!("Getting internal links");
         trace!("Getting internal links from header of '{}' = {:?}", self.get_location(), self.get_header());
-        let res = self
+
+        let partial : LinkPartial = self
             .get_header()
-            .read("links.internal")
+            .read_partial::<LinkPartial>()?
-            .context(format_err!("Failed to read header 'links.internal' of '{}'", self.get_location()))
+            .unwrap_or_else(|| LinkPartial::default());
-            .context(EM::EntryHeaderReadError)
+
-            .context(EM::EntryHeaderError)
+        partial
-            .map_err(Error::from)
+            .internal
-            .map(|r| r.cloned());
+            .unwrap_or_else(|| vec![])
-        process_rw_result(res)
+            .into_iter()
+            .map(PathBuf::from)
+            .map(StoreId::new)
+            .map(|r| r.map(Link::from))
+            .collect::<Result<Vec<Link>>>()
+            .map(LinkIter::new)
     }
 
-    fn add_link(&mut self, link: &mut Entry) -> Result<()> {
+    fn add_link(&mut self, other: &mut Entry) -> Result<()> {
-        debug!("Adding internal link: {:?}", link);
+        debug!("Adding internal link: {:?}", other);
-        let location = link.get_location().clone().into();
+        let left_location  = self.get_location().to_str()?;
-        add_link_with_instance(self, link, location)
+        let right_location = other.get_location().to_str()?;
+
+        alter_linking(self, other, |mut left, mut right| {
+            let mut left_internal = left.internal.unwrap_or_else(|| vec![]);
+            left_internal.push(right_location);
+
+            left_internal.sort_unstable();
+            left_internal.dedup();
+
+            let mut right_internal = right.internal.unwrap_or_else(|| vec![]);
+            right_internal.push(left_location);
+
+            right_internal.sort_unstable();
+            right_internal.dedup();
+
+            left.internal = Some(left_internal);
+            right.internal = Some(right_internal);
+
+            Ok((left, right))
+        })
     }
 
-    fn remove_link(&mut self, link: &mut Entry) -> Result<()> {
+    fn remove_link(&mut self, other: &mut Entry) -> Result<()> {
-        debug!("Removing internal link: {:?}", link);
+        debug!("Remove internal link: {:?}", other);
+        let left_location  = self.get_location().to_str()?;
+        let right_location = other.get_location().to_str()?;
 
-        // Cloning because of borrowing
+        alter_linking(self, other, |mut left, mut right| {
-        let own_loc   = self.get_location().clone();
+            let mut left_internal = left.internal.unwrap_or_else(|| vec![]);
-        let other_loc = link.get_location().clone();
+            left_internal.retain(|l| *l != right_location);
 
-        debug!("Removing internal link from {:?} to {:?}", own_loc, other_loc);
+            left_internal.sort_unstable();
+            left_internal.dedup();
 
-        let links = link.links()?;
+            let mut right_internal = right.internal.unwrap_or_else(|| vec![]);
-        debug!("Rewriting own links for {:?}, without {:?}", other_loc, own_loc);
+            right_internal.retain(|l| *l != left_location);
 
-        let links = links.filter(|l| !l.eq_store_id(&own_loc));
+            right_internal.sort_unstable();
-        let _     = rewrite_links(link.get_header_mut(), links)?;
+            right_internal.dedup();
 
-        self.links()
+            left.internal = Some(left_internal);
-            .and_then(|links| {
+            right.internal = Some(right_internal);
-                debug!("Rewriting own links for {:?}, without {:?}", own_loc, other_loc);
+
-                let links = links.filter(|l| !l.eq_store_id(&other_loc));
+            Ok((left, right))
-                rewrite_links(self.get_header_mut(), links)
+        })
-            })
     }
 
     fn unlink(&mut self, store: &Store) -> Result<()> {
@@ -109,147 +150,33 @@ impl Linkable for Entry {
         Ok(())
     }
 
-    fn add_annotated_link(&mut self, link: &mut Entry, annotation: String) -> Result<()> {
-        let new_link = Link::Annotated {
-            link: link.get_location().clone(),
-            annotation: annotation,
-        };
-
-        add_link_with_instance(self, link, new_link)
-    }
-
 }
 
-fn add_link_with_instance(this: &mut Entry, link: &mut Entry, instance: Link) -> Result<()> {
+fn alter_linking<F>(left: &mut Entry, right: &mut Entry, f: F) -> Result<()>
-    debug!("Adding internal link from {:?} to {:?}", this.get_location(), instance);
+    where F: FnOnce(LinkPartial, LinkPartial) -> Result<(LinkPartial, LinkPartial)>
+{
+    debug!("Altering linkage of {:?} and {:?}", left, right);
 
-    add_foreign_link(link, this.get_location().clone())
+    let get_partial = |entry: &mut Entry| -> Result<LinkPartial> {
-        .and_then(|_| {
+        Ok(entry.get_header().read_partial::<LinkPartial>()?.unwrap_or_else(|| LinkPartial::default()))
-            this.links()
-                .and_then(|links| {
-                    let links = links.chain(LinkIter::new(vec![instance]));
-                    rewrite_links(this.get_header_mut(), links)
-                })
-        })
-}
-
-fn rewrite_links<I: Iterator<Item = Link>>(header: &mut Value, links: I) -> Result<()> {
-    let links = links.into_values()
-                     .into_iter()
-                     .fold(Ok(vec![]), |acc: Result<Vec<_>>, elem| {
-                        acc.and_then(move |mut v| {
-                            v.push(elem.context(EM::ConversionError)?);
-                            Ok(v)
-                        })
-                     })?;
-
-    debug!("Setting new link array: {:?}", links);
-    let process = header
-        .insert("links.internal", Value::Array(links))
-        .context(format_err!("Failed to insert header 'links.internal'"))
-        .context(EM::EntryHeaderReadError)
-        .map_err(Error::from);
-    process_rw_result(process).map(|_| ())
-}
-
-/// When Linking A -> B, the specification wants us to link back B -> A.
-/// This is a helper function which does this.
-fn add_foreign_link(target: &mut Entry, from: StoreId) -> Result<()> {
-    debug!("Linking back from {:?} to {:?}", target.get_location(), from);
-    target.links()
-        .and_then(|links| {
-            let links = links
-                             .chain(LinkIter::new(vec![from.into()]))
-                             .into_values()
-                             .into_iter()
-                             .fold(Ok(vec![]), |acc: Result<Vec<_>>, elem| {
-                                acc.and_then(move |mut v| {
-                                    v.push(elem.context(EM::ConversionError)?);
-                                    Ok(v)
-                                })
-                             })?;
-            debug!("Setting links in {:?}: {:?}", target.get_location(), links);
-
-            let res = target
-                .get_header_mut()
-                .insert("links.internal", Value::Array(links))
-                .context(format_err!("Failed to insert header 'links.internal'"))
-                .context(EM::EntryHeaderReadError)
-                .map_err(Error::from);
-
-            process_rw_result(res).map(|_| ())
-        })
-}
-
-fn process_rw_result(links: Result<Option<Value>>) -> Result<LinkIter> {
-    use std::path::PathBuf;
-
-    let links = match links {
-        Err(e) => {
-            debug!("RW action on store failed. Generating LinkError");
-            return Err(e).context(EM::EntryHeaderReadError).map_err(Error::from)
-        },
-        Ok(None) => {
-            debug!("We got no value from the header!");
-            return Ok(LinkIter::new(vec![]))
-        },
-        Ok(Some(Value::Array(l))) => l,
-        Ok(Some(_)) => {
-            debug!("We expected an Array for the links, but there was a non-Array!");
-            return Err(err_msg("Link type error"));
-        }
     };
 
-    if !links.iter().all(|l| is_match!(*l, Value::String(_)) || is_match!(*l, Value::Table(_))) {
+    let left_partial : LinkPartial = get_partial(left)?;
-        debug!("At least one of the Values which were expected in the Array of links is not a String or a Table!");
+    let right_partial : LinkPartial = get_partial(right)?;
-        debug!("Generating LinkError");
-        return Err(err_msg("Existing Link type error"));
-    }
 
-    let links : Vec<Link> = links.into_iter()
+    trace!("Partial left before: {:?}", left_partial);
-        .map(|link| {
+    trace!("Partial right before: {:?}", right_partial);
-            debug!("Matching the link: {:?}", link);
-            match link {
-                Value::String(s) => StoreId::new(PathBuf::from(s))
-                    .map(|s| Link::Id { link: s })
-                    .map_err(From::from)
-                    ,
-                Value::Table(mut tab) => {
-                    debug!("Destructuring table");
-                    if !tab.contains_key("link")
-                    || !tab.contains_key("annotation") {
-                        debug!("Things missing... returning Error instance");
-                        Err(err_msg("Link parser error"))
-                    } else {
-                        let link = tab.remove("link")
-                            .ok_or(err_msg("Link parser: field missing"))?;
 
-                        let anno = tab.remove("annotation")
+    let (left_partial, right_partial) = f(left_partial, right_partial)?;
-                            .ok_or(err_msg("Link parser: Field missing"))?;
 
-                        debug!("Ok, here we go with building a Link::Annotated");
+    trace!("Partial left after: {:?}", left_partial);
-                        match (link, anno) {
+    trace!("Partial right after: {:?}", right_partial);
-                            (Value::String(link), Value::String(anno)) => {
-                                StoreId::new(PathBuf::from(link))
-                                    .map_err(From::from)
-                                    .map(|link| {
-                                        Link::Annotated {
-                                            link: link,
-                                            annotation: anno,
-                                        }
-                                    })
-                            },
-                            _ => Err(err_msg("Link parser: Field type error")),
-                        }
-                    }
-                }
-                _ => unreachable!(),
-            }
-        })
-        .collect::<Result<Vec<Link>>>()?;
 
-    debug!("Ok, the RW action was successful, returning link vector now!");
+    left.get_header_mut().insert_serialized("links", left_partial)?;
-    Ok(LinkIter::new(links))
+    right.get_header_mut().insert_serialized("links", right_partial)?;
+
+    debug!("Finished altering linkage!");
+    Ok(())
 }
 
 #[cfg(test)]
@@ -259,7 +186,6 @@ mod test {
     use libimagstore::store::Store;
 
     use super::Linkable;
-    use super::Link;
 
     fn setup_logging() {
         let _ = ::env_logger::try_init();
@@ -291,7 +217,9 @@ mod test {
         assert!(e2.links().is_ok());
 
         {
-            assert!(e1.add_link(&mut e2).is_ok());
+            let res = e1.add_link(&mut e2);
+            debug!("Result = {:?}", res);
+            assert!(res.is_ok());
 
             let e1_links = e1.links().unwrap().collect::<Vec<_>>();
             let e2_links = e2.links().unwrap().collect::<Vec<_>>();
@@ -464,33 +392,4 @@ mod test {
         assert_eq!(e3.links().unwrap().collect::<Vec<_>>().len(), 0);
     }
 
-    #[test]
-    fn test_link_annotating() {
-        setup_logging();
-        let store      = get_store();
-        let mut entry1 = store.create(PathBuf::from("test_link_annotating-1")).unwrap();
-        let mut entry2 = store.create(PathBuf::from("test_link_annotating-2")).unwrap();
-
-        let res = entry1.add_annotated_link(&mut entry2, String::from("annotation"));
-        assert!(res.is_ok());
-
-        {
-            for link in entry1.links().unwrap() {
-                match link  {
-                    Link::Annotated {annotation, ..} => assert_eq!(annotation, "annotation"),
-                    _ => assert!(false, "Non-annotated link found"),
-                }
-            }
-        }
-
-        {
-            for link in entry2.links().unwrap() {
-                match link  {
-                    Link::Id {..}        => {},
-                    Link::Annotated {..} => assert!(false, "Annotated link found"),
-                }
-            }
-        }
-    }
-
 }

lib/entry/libimagentrymarkdown/src/processor.rs

@@ -438,7 +438,7 @@ mod tests {
         assert!(result.is_ok(), "Should be Ok(()): {:?}", result);
 
         // The hash of "http://example.com" processed in the `libimagentrylink` way.
-        let expected_link = "url/external/9c17e047f58f9220a7008d4f18152fee4d111d14";
+        let expected_link = "url/9c17e047f58f9220a7008d4f18152fee4d111d14";
         {
             let base_links = base.links();
             assert!(base_links.is_ok());

lib/entry/libimagentryurl/Cargo.toml

@@ -27,15 +27,21 @@ url = "1.5"
 sha-1 = "0.7"
 hex = "0.3"
 is-match = "0.1"
-toml-query = "0.9"
 failure        = "0.1"
 failure_derive = "0.1"
+serde = "1"
+serde_derive = "1"
 
 libimagstore     = { version = "0.10.0", path = "../../../lib/core/libimagstore" }
 libimagerror     = { version = "0.10.0", path = "../../../lib/core/libimagerror" }
 libimagutil      = { version = "0.10.0", path = "../../../lib/etc/libimagutil" }
 libimagentrylink = { version = "0.10.0", path = "../../../lib/entry/libimagentrylink" }
 
+[dependencies.toml-query]
+version = "0.9"
+default-features = false
+features = [ "typed" ]
+
 [dev-dependencies]
 env_logger = "0.5"
 

lib/entry/libimagentryurl/src/iter.rs

@@ -37,7 +37,7 @@ use libimagutil::debug_result::DebugResult;
 use failure::Fallible as Result;
 use url::Url;
 
-/// Helper for building `OnlyExternalIter` and `NoExternalIter`
+/// Helper for building `OnlyUrlIter` and `NoUrlIter`
 ///
 /// The boolean value defines, how to interpret the `is_external_link_storeid()` return value
 /// (here as "pred"):
@@ -55,9 +55,9 @@ use url::Url;
 /// false... and so on.
 ///
 /// As we can see, the operator between these two operants is `!(a ^ b)`.
-pub struct ExternalFilterIter(LinkIter, bool);
+pub struct UrlFilterIter(LinkIter, bool);
 
-impl Iterator for ExternalFilterIter {
+impl Iterator for UrlFilterIter {
     type Item = Link;
 
     fn next(&mut self) -> Option<Self::Item> {
@@ -78,24 +78,24 @@ impl Iterator for ExternalFilterIter {
 ///
 /// # See also
 ///
-/// Also see `OnlyExternalIter` and `NoExternalIter` and the helper traits/functions
+/// Also see `OnlyUrlIter` and `NoUrlIter` and the helper traits/functions
-/// `OnlyInteralLinks`/`only_links()` and `OnlyExternalLinks`/`only_urls()`.
+/// `OnlyInteralLinks`/`only_links()` and `OnlyUrlLinks`/`only_urls()`.
-pub trait SelectExternal {
+pub trait SelectUrl {
-    fn select_urls(self, b: bool) -> ExternalFilterIter;
+    fn select_urls(self, b: bool) -> UrlFilterIter;
 }
 
-impl SelectExternal for LinkIter {
+impl SelectUrl for LinkIter {
-    fn select_urls(self, b: bool) -> ExternalFilterIter {
+    fn select_urls(self, b: bool) -> UrlFilterIter {
-        ExternalFilterIter(self, b)
+        UrlFilterIter(self, b)
     }
 }
 
 
-pub struct OnlyExternalIter(ExternalFilterIter);
+pub struct OnlyUrlIter(UrlFilterIter);
 
-impl OnlyExternalIter {
+impl OnlyUrlIter {
-    pub fn new(li: LinkIter) -> OnlyExternalIter {
+    pub fn new(li: LinkIter) -> OnlyUrlIter {
-        OnlyExternalIter(ExternalFilterIter(li, true))
+        OnlyUrlIter(UrlFilterIter(li, true))
     }
 
     pub fn urls<'a>(self, store: &'a Store) -> UrlIter<'a> {
@@ -103,7 +103,7 @@ impl OnlyExternalIter {
     }
 }
 
-impl Iterator for OnlyExternalIter {
+impl Iterator for OnlyUrlIter {
     type Item = Link;
 
     fn next(&mut self) -> Option<Self::Item> {
@@ -111,15 +111,15 @@ impl Iterator for OnlyExternalIter {
     }
 }
 
-pub struct NoExternalIter(ExternalFilterIter);
+pub struct NoUrlIter(UrlFilterIter);
 
-impl NoExternalIter {
+impl NoUrlIter {
-    pub fn new(li: LinkIter) -> NoExternalIter {
+    pub fn new(li: LinkIter) -> NoUrlIter {
-        NoExternalIter(ExternalFilterIter(li, false))
+        NoUrlIter(UrlFilterIter(li, false))
     }
 }
 
-impl Iterator for NoExternalIter {
+impl Iterator for NoUrlIter {
     type Item = Link;
 
     fn next(&mut self) -> Option<Self::Item> {
@@ -127,35 +127,35 @@ impl Iterator for NoExternalIter {
     }
 }
 
-pub trait OnlyExternalLinks : Sized {
+pub trait OnlyUrlLinks : Sized {
-    fn only_urls(self) -> OnlyExternalIter ;
+    fn only_urls(self) -> OnlyUrlIter ;
 
-    fn no_links(self) -> OnlyExternalIter {
+    fn no_links(self) -> OnlyUrlIter {
         self.only_urls()
     }
 }
 
-impl OnlyExternalLinks for LinkIter {
+impl OnlyUrlLinks for LinkIter {
-    fn only_urls(self) -> OnlyExternalIter {
+    fn only_urls(self) -> OnlyUrlIter {
-        OnlyExternalIter::new(self)
+        OnlyUrlIter::new(self)
     }
 }
 
 pub trait OnlyInternalLinks : Sized {
-    fn only_links(self) -> NoExternalIter;
+    fn only_links(self) -> NoUrlIter;
 
-    fn no_urls(self) -> NoExternalIter {
+    fn no_urls(self) -> NoUrlIter {
         self.only_links()
     }
 }
 
 impl OnlyInternalLinks for LinkIter {
-    fn only_links(self) -> NoExternalIter {
+    fn only_links(self) -> NoUrlIter {
-        NoExternalIter::new(self)
+        NoUrlIter::new(self)
     }
 }
 
-pub struct UrlIter<'a>(OnlyExternalIter, &'a Store);
+pub struct UrlIter<'a>(OnlyUrlIter, &'a Store);
 
 impl<'a> Iterator for UrlIter<'a> {
     type Item = Result<Url>;
@@ -174,8 +174,8 @@ impl<'a> Iterator for UrlIter<'a> {
                         .map_err(From::from)
                         .and_then(|f| {
                             debug!("Store::retrieve({:?}) succeeded", id);
-                            debug!("getting external link from file now");
+                            debug!("getting uri link from file now");
-                            f.get_link_uri_from_filelockentry()
+                            f.get_url()
                                 .map_dbg_str("Error happened while getting link URI from FLE")
                                 .map_dbg_err(|e| format!("URL -> Err = {:?}", e))
                         })

lib/entry/libimagentryurl/src/lib.rs

@@ -55,6 +55,8 @@ extern crate toml_query;
 extern crate url;
 extern crate sha1;
 extern crate hex;
+extern crate serde;
+#[macro_use] extern crate serde_derive;
 #[macro_use] extern crate failure;
 
 #[cfg(test)]

lib/entry/libimagentryurl/src/link.rs

@@ -22,55 +22,122 @@ use failure::ResultExt;
 use failure::Fallible as Result;
 use failure::err_msg;
 use url::Url;
+use toml_query::read::Partial;
+use toml_query::insert::TomlValueInsertExt;
+use toml::Value;
 
 use libimagstore::store::Entry;
 use libimagerror::errors::ErrorMsg as EM;
 
-use toml_query::read::TomlValueReadTypeExt;
+use toml_query::read::TomlValueReadExt;
 
 pub trait Link {
-
-    fn get_link_uri_from_filelockentry(&self) -> Result<Option<Url>>;
-
     fn get_url(&self) -> Result<Option<Url>>;
-
+    fn set_url(&mut self, url: Url) -> Result<()>;
 }
 
+#[derive(Deserialize, Serialize, Debug)]
+pub(crate) struct UrlHeader {
+    pub uri: Option<String>,
+}
+
+impl Default for UrlHeader {
+    fn default() -> Self {
+        UrlHeader {
+            uri: None
+        }
+    }
+}
+
+impl<'a> Partial<'a> for UrlHeader {
+    const LOCATION: &'static str = "url";
+    type Output                  = Self;
+}
+
+
 impl Link for Entry {
 
-    fn get_link_uri_from_filelockentry(&self) -> Result<Option<Url>> {
+    /// Get the URL from entry Entry
-        self.get_header()
+    ///
-            .read_string("links.external.content.url")
+    /// # Notice
-            .context(format_err!("Error reading header 'links.external.content.url' from '{}'", self.get_location()))
+    ///
+    /// This actually returns the header field of the entry, parsed as URL
+    ///
+    ///
+    fn get_url(&self) -> Result<Option<Url>> {
+        let partial = self.get_header()
+            .read_partial::<UrlHeader>()
+            .context(format_err!("Error reading header 'url.uri' from '{}'", self.get_location()))
             .context(EM::EntryHeaderReadError)
+            .map_err(Error::from)?
+            .unwrap_or_else(|| Default::default());
+
+        debug!("Partial deserialized: {:?}", partial);
+
+        let url = match partial.uri {
+            Some(uri) => uri,
+            None      => return Ok(None),
+        };
+
+        debug!("Found url, parsing: {:?}", url);
+        Url::parse(&url)
             .map_err(Error::from)
-            .and_then(|opt| match opt {
+            .context(format_err!("Failed to parse URL: '{}'", url))
-                None        => Ok(None),
+            .context(err_msg("Invalid URI"))
-                Some(ref s) => {
+            .map_err(Error::from)
-                    debug!("Found url, parsing: {:?}", s);
+            .map(Some)
-                    Url::parse(&s[..])
-                        .map_err(Error::from)
-                        .context(format_err!("Failed to parse URL: '{}'", s))
-                        .context(err_msg("Invalid URI"))
-                        .map_err(Error::from)
-                        .map(Some)
-                },
-            })
             .context("Failed to get link URI from entry")
             .map_err(Error::from)
     }
 
-    fn get_url(&self) -> Result<Option<Url>> {
+    fn set_url(&mut self, url: Url) -> Result<()> {
-        match self.get_header().read_string("links.external.url")? {
+        let val = Value::String(url.to_string());
-            None        => Ok(None),
+        self.get_header_mut().insert_serialized("url.uri", val)?;
-            Some(ref s) => Url::parse(&s[..])
+
-                .context(format_err!("Failed to parse URL: '{}'", s))
+        debug!("Setting URL worked");
-                .map(Some)
+        Ok(())
-                .map_err(Error::from)
+    }
-                .context(EM::EntryHeaderReadError)
+}
-                .map_err(Error::from),
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::path::PathBuf;
+
+    use libimagstore::store::Store;
+
+    fn setup_logging() {
+        let _ = env_logger::try_init();
+    }
+
+    pub fn get_store() -> Store {
+        Store::new_inmemory(PathBuf::from("/"), &None).unwrap()
+    }
+
+    #[test]
+    fn test_header_set_correctly() {
+        setup_logging();
+        let store = get_store();
+        let mut e = store.retrieve(PathBuf::from("urlentry")).unwrap();
+        let url   = Url::parse("http://google.de").unwrap();
+
+        assert!(e.set_url(url).is_ok());
+        debug!("Fetch header: {:?}", e.get_header());
+
+        let url = e.get_header().read("url.uri");
+
+        debug!("Fetched header: {:?}", url);
+
+        assert!(url.is_ok());
+        let url = url.unwrap();
+
+        assert!(url.is_some());
+        let url = url.unwrap();
+
+        match url {
+            Value::String(ref s) => assert_eq!("http://google.de/", s),
+            _ => assert!(false),
         }
     }
 
 }
-

lib/entry/libimagentryurl/src/linker.rs

@@ -17,8 +17,6 @@
 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
 //
 
-use std::ops::DerefMut;
-
 use libimagstore::storeid::StoreId;
 use libimagstore::store::Store;
 use libimagstore::store::Entry;
@@ -26,77 +24,82 @@ use libimagutil::debug_result::DebugResult;
 use libimagentrylink::linkable::Linkable;
 
 use failure::Fallible as Result;
-use toml::Value;
-use toml::map::Map;
-use toml_query::read::TomlValueReadExt;
-use toml_query::insert::TomlValueInsertExt;
 use url::Url;
 use sha1::{Sha1, Digest};
 use hex;
 
+use crate::link::Link;
 use crate::iter::UrlIter;
 
 pub trait UrlLinker : Linkable {
 
-    /// Get the external links from the implementor object
     fn get_urls<'a>(&self, store: &'a Store) -> Result<UrlIter<'a>>;
 
-    /// Set the external links for the implementor object
     fn set_urls(&mut self, store: &Store, links: Vec<Url>) -> Result<Vec<StoreId>>;
 
-    /// Add an external link to the implementor object
     fn add_url(&mut self, store: &Store, link: Url) -> Result<Vec<StoreId>>;
 
-    /// Remove an external link from the implementor object
     fn remove_url(&mut self, store: &Store, link: Url) -> Result<Vec<StoreId>>;
 
 }
 
-/// Implement `ExternalLinker` for `Entry`, hiding the fact that there is no such thing as an external
-/// link in an entry, but internal links to other entries which serve as external links, as one
-/// entry in the store can only have one external link.
 impl UrlLinker for Entry {
 
-    /// Get the external links from the implementor object
+    /// Get URLs from the Entry
+    ///
+    ///
+    /// # Notice
+    ///
+    /// (Also see documentation of `UrlLinker::set_urls()`)
+    ///
+    /// This fetches all Links (as in `libimagentrylink` for the Entry and filters them by entries
+    /// which contain an URL.
+    ///
+    ///
+    /// # Return Value
+    ///
+    /// Iterator over URLs
+    ///
     fn get_urls<'a>(&self, store: &'a Store) -> Result<UrlIter<'a>> {
-        use crate::iter::OnlyExternalLinks;
+        use crate::iter::OnlyUrlLinks;
 
         // Iterate through all internal links and filter for FileLockEntries which live in
-        // /link/external/<SHA> -> load these files and get the external link from their headers,
+        // /url/<SHA> -> load these files and get the url from their headers,
         // put them into the return vector.
         self.links()
             .map(|iter| {
-                debug!("Getting external links");
+                debug!("Getting urls");
                 iter.only_urls().urls(store)
             })
     }
 
-    /// Set the external links for the implementor object
+    /// Set URLs for the Entry
+    ///
+    /// # Notice
+    ///
+    /// This does not actually add each URL in this entry, but retrieves (as in
+    /// `Store::retrieve()`) one entry for each URL and links (as in `libimagentrylink`) this entry
+    /// to the retrieved ones.
+    ///
     ///
     /// # Return Value
     ///
-    /// Returns the StoreIds which were newly created for the new external links, if there are more
+    /// Returns the StoreIds which were newly created for the new urls, if there are more
-    /// external links than before.
+    /// urls than before.
-    /// If there are less external links than before, an empty vec![] is returned.
+    /// If there are less urls than before, an empty vec![] is returned.
     ///
     fn set_urls(&mut self, store: &Store, links: Vec<Url>) -> Result<Vec<StoreId>> {
-        // Take all the links, generate a SHA sum out of each one, filter out the already existing
-        // store entries and store the other URIs in the header of one FileLockEntry each, in
-        // the path /link/external/<SHA of the URL>
-
         debug!("Iterating {} links = {:?}", links.len(), links);
         links.into_iter().map(|link| {
             let hash = hex::encode(Sha1::digest(&link.as_str().as_bytes()));
-            let file_id = crate::module_path::new_id(format!("external/{}", hash))
+            let file_id = crate::module_path::new_id(hash.clone())
-                .map_dbg_err(|_| {
+                .map_dbg_err(|_| format!("Failed to build StoreId for this hash '{:?}'", hash))?;
-                    format!("Failed to build StoreId for this hash '{:?}'", hash)
-                })?;
 
             debug!("Link    = '{:?}'", link);
             debug!("Hash    = '{:?}'", hash);
             debug!("StoreId = '{:?}'", file_id);
 
-            let link_already_exists = store.get(file_id.clone())?.is_some();
+            let link_already_exists = store.exists(file_id.clone())?;
 
             // retrieve the file from the store, which implicitely creates the entry if it does not
             // exist
@@ -107,31 +110,11 @@ impl UrlLinker for Entry {
                 })?;
 
             debug!("Generating header content!");
-            {
+            file.set_url(link)?;
-                let hdr = file.deref_mut().get_header_mut();
-
-                let mut table = match hdr.read("links.external.content")? {
-                    Some(&Value::Table(ref table)) => table.clone(),
-                    Some(_) => {
-                        warn!("There is a value at 'links.external.content' which is not a table.");
-                        warn!("Going to override this value");
-                        Map::new()
-                    },
-                    None => Map::new(),
-                };
-
-                let v = Value::String(link.into_string());
-
-                debug!("setting URL = '{:?}", v);
-                table.insert(String::from("url"), v);
-
-                let _ = hdr.insert("links.external.content", Value::Table(table))?;
-                debug!("Setting URL worked");
-            }
 
             // then add an internal link to the new file or return an error if this fails
-            let _ = self.add_link(file.deref_mut())?;
+            let _ = self.add_link(&mut file)?;
-            debug!("Added internal link");
+            debug!("Added linking: {:?} <-> {:?}", self.get_location(), file.get_location());
 
             Ok((link_already_exists, file_id))
         })
@@ -142,52 +125,40 @@ impl UrlLinker for Entry {
         .collect()
     }
 
-    /// Add an external link to the implementor object
+    /// Add an URL to the entry
+    ///
+    ///
+    /// # Notice
+    ///
+    /// (Also see documentation of `UrlLinker::set_urls()`)
+    ///
     ///
     /// # Return Value
     ///
-    /// (See ExternalLinker::set_urls())
+    /// (See UrlLinker::set_urls())
-    ///
-    /// Returns the StoreIds which were newly created for the new external links, if there are more
-    /// external links than before.
-    /// If there are less external links than before, an empty vec![] is returned.
     ///
     fn add_url(&mut self, store: &Store, link: Url) -> Result<Vec<StoreId>> {
-        // get external links, add this one, save them
+        let mut links = self.get_urls(store)?.collect::<Result<Vec<_>>>()?;
-        debug!("Getting links");
+        links.push(link);
-        self.get_urls(store)
+        self.set_urls(store, links)
-            .and_then(|links| {
-                let mut links = links.collect::<Result<Vec<_>>>()?;
-
-                debug!("Adding link = '{:?}' to links = {:?}", link, links);
-                links.push(link);
-
-                debug!("Setting {} links = {:?}", links.len(), links);
-                self.set_urls(store, links)
-            })
     }
 
-    /// Remove an external link from the implementor object
+    /// Remove an URL from the entry
+    ///
+    ///
+    /// # Notice
+    ///
+    /// (Also see documentation of `UrlLinker::set_urls()`)
+    ///
     ///
     /// # Return Value
     ///
-    /// (See ExternalLinker::set_urls())
+    /// (See UrlLinker::set_urls())
-    ///
-    /// Returns the StoreIds which were newly created for the new external links, if there are more
-    /// external links than before.
-    /// If there are less external links than before, an empty vec![] is returned.
     ///
     fn remove_url(&mut self, store: &Store, link: Url) -> Result<Vec<StoreId>> {
-        // get external links, remove this one, save them
+        let mut links = self.get_urls(store)?.collect::<Result<Vec<_>>>()?;
-        self.get_urls(store)
+        links.retain(|l| *l != link);
-            .and_then(|links| {
+        self.set_urls(store, links)
-                debug!("Removing link = '{:?}'", link);
-                let links = links
-                    .filter_map(Result::ok)
-                    .filter(|l| l.as_str() != link.as_str())
-                    .collect::<Vec<_>>();
-                self.set_urls(store, links)
-            })
     }
 
 }
@@ -207,6 +178,47 @@ mod tests {
         Store::new_inmemory(PathBuf::from("/"), &None).unwrap()
     }
 
+    #[test]
+    fn test_adding_url() {
+        use toml_query::read::TomlValueReadTypeExt;
+
+        setup_logging();
+        let store = get_store();
+        let mut e = store.retrieve(PathBuf::from("base-test_simple")).unwrap();
+        let url   = Url::parse("http://google.de").unwrap();
+
+        assert!(e.add_url(&store, url.clone()).is_ok());
+
+        debug!("{:?}", e);
+        debug!("Header: {:?}", e.get_header());
+
+        let link = e.links().unwrap().next();
+        assert!(link.is_some());
+        let link = link.unwrap();
+
+        debug!("link[0] = {:?}", link);
+        let id = link.get_store_id();
+
+        let link_entry = store.get(id.clone()).unwrap().unwrap();
+
+        debug!("Entry = {:?}", link_entry);
+        debug!("Header = {:?}", link_entry.get_header());
+
+        let link = match link_entry.get_header().read_string("url.uri") {
+            Ok(Some(s)) => s,
+            Ok(None) => {
+                assert!(false);
+                unreachable!()
+            },
+            Err(e) => {
+                error!("{:?}", e);
+                assert!(false);
+                unreachable!()
+            },
+        };
+
+        assert_eq!(link, "http://google.de/");
+    }
 
     #[test]
     fn test_simple() {
@@ -217,7 +229,22 @@ mod tests {
 
         assert!(e.add_url(&store, url.clone()).is_ok());
 
-        assert_eq!(1, e.get_urls(&store).unwrap().count());
+        debug!("{:?}", e);
+        debug!("Header: {:?}", e.get_header());
+
+        let urls = e.get_urls(&store);
+        let urls = match urls {
+            Err(e) => {
+                debug!("Error: {:?}", e);
+                assert!(false);
+                unreachable!()
+            },
+            Ok(urls) => urls.collect::<Vec<_>>(),
+        };
+
+        debug!("urls = {:?}", urls);
+
+        assert_eq!(1, urls.len());
         assert_eq!(url, e.get_urls(&store).unwrap().next().unwrap().unwrap());
     }
 

lib/entry/libimagentryurl/src/util.rs

@@ -21,8 +21,8 @@ use std::fmt::Debug;
 
 use libimagstore::storeid::StoreId;
 
-/// Check whether the StoreId starts with `/link/external/`
+/// Check whether the StoreId starts with `/url/`
 pub fn is_external_link_storeid<A: AsRef<StoreId> + Debug>(id: A) -> bool {
-    debug!("Checking whether this is a 'url/external/': '{:?}'", id);
+    debug!("Checking whether this is a 'url/': '{:?}'", id);
-    id.as_ref().is_in_collection(&["url", "external"])
+    id.as_ref().is_in_collection(&["url"])
 }