From 87ee509dc05306002bd13d7c7ba60612b4860582 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Wed, 22 Feb 2017 10:53:27 +0100 Subject: [PATCH 1/8] Prepare for annotations implementation This patch changes the `libimagentrylink` internals to use a enum { Id(StoreId), Annotated(StoreId, String) } as Link type. In this patch, changes were made to be able to compile the code, but tests might fail and other crates might fail as well. --- libimagentrylink/src/external.rs | 5 +- libimagentrylink/src/internal.rs | 141 +++++++++++++++++++++++++------ 2 files changed, 117 insertions(+), 29 deletions(-) diff --git a/libimagentrylink/src/external.rs b/libimagentrylink/src/external.rs index 0d6aab08..daed7a12 100644 --- a/libimagentrylink/src/external.rs +++ b/libimagentrylink/src/external.rs @@ -32,6 +32,7 @@ use std::ops::DerefMut; use std::collections::BTreeMap; +use std::fmt::Debug; use libimagstore::store::Entry; use libimagstore::store::FileLockEntry; @@ -283,9 +284,9 @@ pub mod iter { /// Check whether the StoreId starts with `/link/external/` -pub fn is_external_link_storeid(id: &StoreId) -> bool { +pub fn is_external_link_storeid + Debug>(id: A) -> bool { debug!("Checking whether this is a 'links/external/': '{:?}'", id); - id.local().starts_with("links/external") + id.as_ref().local().starts_with("links/external") } fn get_external_link_from_file(entry: &FileLockEntry) -> Result { diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index 83add039..3b922ea7 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -17,7 +17,10 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // +use std::collections::BTreeMap; + use libimagstore::storeid::StoreId; +use libimagstore::storeid::IntoStoreId; use libimagstore::store::Entry; use libimagstore::store::Result as StoreResult; use libimagstore::toml_ext::TomlValueExt; @@ -31,7 +34,96 @@ use self::iter::IntoValues; use toml::Value; -pub type Link = StoreId; +#[derive(Eq, PartialOrd, Ord, Hash, Debug, Clone)] +pub enum Link { + Id { link: StoreId }, + Annotated { link: StoreId, annotation: String }, +} + +impl Link { + + 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), + } + } + + /// Helper wrapper around Link for StoreId + fn without_base(self) -> Link { + match self { + Link::Id { link: s } => Link::Id { link: s.without_base() }, + Link::Annotated { link: s, annotation: ann } => + Link::Annotated { link: s.without_base(), annotation: ann }, + } + } + + fn to_value(&self) -> Result { + match self { + &Link::Id { link: ref s } => + s.to_str().map(Value::String).map_err_into(LEK::InternalConversionError), + &Link::Annotated { ref link, annotation: ref anno } => { + link.to_str() + .map(Value::String) + .map_err_into(LEK::InternalConversionError) + .map(|link| { + let mut tab = BTreeMap::new(); + + tab.insert("link".to_owned(), link); + tab.insert("annotation".to_owned(), Value::String(anno.clone())); + Value::Table(tab) + }) + } + } + } + +} + +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, + } + } +} + +impl From for Link { + + fn from(s: StoreId) -> Link { + Link::Id { link: s } + } +} + +impl Into for Link { + fn into(self) -> StoreId { + match self { + Link::Id { link } => link, + Link::Annotated { link, .. } => link, + } + } +} + +impl IntoStoreId for Link { + fn into_storeid(self) -> StoreResult { + match self { + Link::Id { link } => Ok(link), + Link::Annotated { link, .. } => Ok(link), + } + } +} + +impl AsRef for Link { + fn as_ref(&self) -> &StoreId { + match self { + &Link::Id { ref link } => &link, + &Link::Annotated { ref link, .. } => &link, + } + } +} pub trait InternalLinker { @@ -51,7 +143,6 @@ pub trait InternalLinker { pub mod iter { use std::vec::IntoIter; - use std::cmp::Ordering; use super::Link; use error::LinkErrorKind as LEK; @@ -87,27 +178,17 @@ pub mod iter { } pub trait IntoValues { - fn into_values(self) -> IntoIter>; + fn into_values(self) -> Vec>; } impl> IntoValues for I { - fn into_values(self) -> IntoIter> { - self.map(|s| s.without_base().to_str().map_err_into(LEK::InternalConversionError)) - .unique_by(|entry| { - match entry { - &Ok(ref e) => Some(e.clone()), - &Err(_) => None, - } - }) - .map(|elem| elem.map(Value::String)) - .sorted_by(|a, b| { - match (a, b) { - (&Ok(Value::String(ref a)), &Ok(Value::String(ref b))) => Ord::cmp(a, b), - (&Err(_), _) | (_, &Err(_)) => Ordering::Equal, - _ => unreachable!() - } - }) - .into_iter() + fn into_values(self) -> Vec> { + self.map(|s| s.without_base()) + .unique() + .sorted() + .into_iter() // Cannot sort toml::Value, hence uglyness here + .map(|link| link.to_value().map_err_into(LEK::InternalConversionError)) + .collect() } } @@ -282,12 +363,12 @@ impl InternalLinker for Entry { if let Err(e) = add_foreign_link(link, self_location.clone()) { return Err(e); } - let link = link.get_location().clone(); - new_links.push(link); + new_links.push(link.get_location().clone().into()); } let new_links = try!(LinkIter::new(new_links) .into_values() + .into_iter() .fold(Ok(vec![]), |acc, elem| { acc.and_then(move |mut v| { elem.map_err_into(LEK::InternalConversionError) @@ -301,7 +382,7 @@ impl InternalLinker for Entry { } fn add_internal_link(&mut self, link: &mut Entry) -> Result<()> { - let new_link = link.get_location().clone(); + let new_link = link.get_location().clone().into(); debug!("Adding internal link from {:?} to {:?}", self.get_location(), new_link); @@ -324,13 +405,15 @@ impl InternalLinker for Entry { link.get_internal_links() .and_then(|links| { debug!("Rewriting own links for {:?}, without {:?}", other_loc, own_loc); - rewrite_links(link.get_header_mut(), links.filter(|l| *l != own_loc)) + let links = links.filter(|l| !l.eq_store_id(&own_loc)); + rewrite_links(link.get_header_mut(), links) }) .and_then(|_| { self.get_internal_links() .and_then(|links| { debug!("Rewriting own links for {:?}, without {:?}", own_loc, other_loc); - rewrite_links(self.get_header_mut(), links.filter(|l| *l != other_loc)) + let links = links.filter(|l| !l.eq_store_id(&other_loc)); + rewrite_links(self.get_header_mut(), links) }) }) } @@ -339,6 +422,7 @@ impl InternalLinker for Entry { fn rewrite_links>(header: &mut Value, links: I) -> Result<()> { let links = try!(links.into_values() + .into_iter() .fold(Ok(vec![]), |acc, elem| { acc.and_then(move |mut v| { elem.map_err_into(LEK::InternalConversionError) @@ -361,8 +445,9 @@ fn add_foreign_link(target: &mut Entry, from: StoreId) -> Result<()> { target.get_internal_links() .and_then(|links| { let links = try!(links - .chain(LinkIter::new(vec![from])) + .chain(LinkIter::new(vec![from.into()])) .into_values() + .into_iter() .fold(Ok(vec![]), |acc, elem| { acc.and_then(move |mut v| { elem.map_err_into(LEK::InternalConversionError) @@ -407,7 +492,9 @@ fn process_rw_result(links: StoreResult>) -> Result { .map(|link| { match link { Value::String(s) => StoreId::new_baseless(PathBuf::from(s)) - .map_err_into(LEK::StoreIdError), + .map_err_into(LEK::StoreIdError) + .map(|s| Link::Id { link: s }) + , _ => unreachable!(), } }) From 21d758d63565f278e0d5fb8ab5c9cf1c8871c668 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Wed, 22 Feb 2017 11:04:22 +0100 Subject: [PATCH 2/8] Add with_base() wrapper for tests --- libimagentrylink/src/internal.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index 3b922ea7..5bca4ed9 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -18,6 +18,8 @@ // use std::collections::BTreeMap; +#[cfg(test)] +use std::path::PathBuf; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; @@ -58,6 +60,16 @@ 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 } => @@ -554,8 +566,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()) == *e2.get_location()).unwrap_or(false)); - assert!(e2_links.first().map(|l| l.clone().with_base(store.path().clone()) == *e1.get_location()).unwrap_or(false)); + 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)); } { From 8de5bf93bef1f8c04ccfa7f204097b606403ad3c Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Wed, 22 Feb 2017 17:20:23 +0100 Subject: [PATCH 3/8] Add support for new link type in return-value parsing --- libimagentrylink/src/error.rs | 3 +++ libimagentrylink/src/internal.rs | 34 ++++++++++++++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/libimagentrylink/src/error.rs b/libimagentrylink/src/error.rs index 7665da4d..10d83f94 100644 --- a/libimagentrylink/src/error.rs +++ b/libimagentrylink/src/error.rs @@ -23,6 +23,9 @@ generate_error_module!( EntryHeaderWriteError => "Error while writing an entry header", ExistingLinkTypeWrong => "Existing link entry has wrong type", LinkTargetDoesNotExist => "Link target does not exist in the store", + LinkParserError => "Link cannot be parsed", + LinkParserFieldMissingError => "Link cannot be parsed: Field missing", + LinkParserFieldTypeError => "Link cannot be parsed: Field type wrong", InternalConversionError => "Error while converting values internally", InvalidUri => "URI is not valid", StoreReadError => "Store read error", diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index 5bca4ed9..7c845a9a 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -494,19 +494,49 @@ fn process_rw_result(links: StoreResult>) -> Result { } }; - if !links.iter().all(|l| is_match!(*l, Value::String(_))) { - debug!("At least one of the Values which were expected in the Array of links is a non-String!"); + if !links.iter().all(|l| is_match!(*l, Value::String(_)) || is_match!(*l, Value::Table(_))) { + debug!("At least one of the Values which were expected in the Array of links is not a String or a Table!"); debug!("Generating LinkError"); return Err(LEK::ExistingLinkTypeWrong.into()); } let links : Vec = try!(links.into_iter() .map(|link| { + debug!("Matching the link: {:?}", link); match link { Value::String(s) => StoreId::new_baseless(PathBuf::from(s)) .map_err_into(LEK::StoreIdError) .map(|s| Link::Id { link: s }) , + Value::Table(mut tab) => { + debug!("Destructuring table"); + if !tab.contains_key("link") + || !tab.contains_key("annotation") { + debug!("Things missing... returning Error instance"); + Err(LEK::LinkParserError.into_error()) + } else { + let link = try!(tab.remove("link") + .ok_or(LEK::LinkParserFieldMissingError.into_error())); + + let anno = try!(tab.remove("annotation") + .ok_or(LEK::LinkParserFieldMissingError.into_error())); + + 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)) + .map_err_into(LEK::StoreIdError) + .map(|link| { + Link::Annotated { + link: link, + annotation: anno, + } + }) + }, + _ => Err(LEK::LinkParserFieldTypeError.into_error()), + } + } + } _ => unreachable!(), } }) From 633f999d54187bf69140cd3264138592c37af0aa Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Wed, 22 Feb 2017 17:40:28 +0100 Subject: [PATCH 4/8] Add new trait function to add annotated links With this new trait function, one can add annotated links to the entries. There's no support yet for removing these links. --- libimagentrylink/src/internal.rs | 38 ++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index 7c845a9a..633c1f53 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -151,6 +151,8 @@ pub trait InternalLinker { /// Remove an internal link from the implementor object fn remove_internal_link(&mut self, link: &mut Entry) -> Result<()>; + /// Add internal annotated link + fn add_internal_annotated_link(&mut self, link: &mut Entry, annotation: String) -> Result<()>; } pub mod iter { @@ -394,18 +396,8 @@ impl InternalLinker for Entry { } fn add_internal_link(&mut self, link: &mut Entry) -> Result<()> { - let new_link = link.get_location().clone().into(); - - debug!("Adding internal link from {:?} to {:?}", self.get_location(), new_link); - - add_foreign_link(link, self.get_location().clone()) - .and_then(|_| { - self.get_internal_links() - .and_then(|links| { - let links = links.chain(LinkIter::new(vec![new_link])); - rewrite_links(self.get_header_mut(), links) - }) - }) + let location = link.get_location().clone().into(); + add_internal_link_with_instance(self, link, location) } fn remove_internal_link(&mut self, link: &mut Entry) -> Result<()> { @@ -430,6 +422,28 @@ impl InternalLinker for Entry { }) } + fn add_internal_annotated_link(&mut self, link: &mut Entry, annotation: String) -> Result<()> { + let new_link = Link::Annotated { + link: link.get_location().clone(), + annotation: annotation, + }; + + add_internal_link_with_instance(self, link, new_link) + } + +} + +fn add_internal_link_with_instance(this: &mut Entry, link: &mut Entry, instance: Link) -> Result<()> { + debug!("Adding internal link from {:?} to {:?}", this.get_location(), instance); + + add_foreign_link(link, this.get_location().clone()) + .and_then(|_| { + this.get_internal_links() + .and_then(|links| { + let links = links.chain(LinkIter::new(vec![instance])); + rewrite_links(this.get_header_mut(), links) + }) + }) } fn rewrite_links>(header: &mut Value, links: I) -> Result<()> { From c550325711611cdab17cfd188d9e2b6314f85860 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 23 Feb 2017 14:40:33 +0100 Subject: [PATCH 5/8] Add Link::exists() -> StoreId::exists() wrapper --- libimagentrylink/src/internal.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index 633c1f53..299da00d 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -44,6 +44,13 @@ pub enum Link { impl Link { + pub fn exists(&self) -> bool { + match *self { + Link::Id { ref link } => link.exists(), + Link::Annotated { ref link, .. } => link.exists(), + } + } + fn eq_store_id(&self, id: &StoreId) -> bool { match self { &Link::Id { link: ref s } => s.eq(id), From 6b821f7b28d4c2787e837683ea2347d7d63883fb Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 23 Feb 2017 15:25:32 +0100 Subject: [PATCH 6/8] Add wrapper for Link::to_str() -> StoreId::to_str() --- libimagentrylink/src/internal.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index 299da00d..1ab7c234 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -51,6 +51,15 @@ impl Link { } } + pub fn to_str(&self) -> Result { + match *self { + Link::Id { ref link } => link.to_str(), + Link::Annotated { ref link, .. } => link.to_str(), + } + .map_err_into(LEK::StoreReadError) + } + + fn eq_store_id(&self, id: &StoreId) -> bool { match self { &Link::Id { link: ref s } => s.eq(id), From f225aec9dadf450da09a9ce4bf67eabe0eddd21f Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 21 Apr 2017 17:53:50 +0200 Subject: [PATCH 7/8] Add Link::get_store_id() helper function --- libimagentrylink/src/internal.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index 1ab7c234..1c170265 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -67,6 +67,14 @@ impl Link { } } + /// Get the StoreId inside the Link, which is always present + pub fn get_store_id(&self) -> &StoreId { + match self { + &Link::Id { link: ref s } => s, + &Link::Annotated { link: ref s, .. } => s, + } + } + /// Helper wrapper around Link for StoreId fn without_base(self) -> Link { match self { From cecdd09d51de83fe645ab1efc72353022b9b6a80 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 21 Apr 2017 17:54:03 +0200 Subject: [PATCH 8/8] Pass StoreId iterator here --- libimagannotation/src/annotation_fetcher.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libimagannotation/src/annotation_fetcher.rs b/libimagannotation/src/annotation_fetcher.rs index 6c5ac298..18c4a8be 100644 --- a/libimagannotation/src/annotation_fetcher.rs +++ b/libimagannotation/src/annotation_fetcher.rs @@ -58,8 +58,7 @@ impl<'a> AnnotationFetcher<'a> for Store { fn annotations_for_entry(&'a self, entry: &Entry) -> Result> { entry.get_internal_links() .map_err_into(AEK::StoreReadError) - .map(Box::new) - .map(|iter| StoreIdIterator::new(iter)) + .map(|iter| StoreIdIterator::new(Box::new(iter.map(|e| e.get_store_id().clone())))) .map(|iter| NoteIterator::new(self, iter)) .map(|iter| AnnotationIter::new(iter)) }