From b2806ab5929969c02aa054f3d941177c7de75393 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 28 Aug 2016 13:39:37 +0200 Subject: [PATCH 1/4] Add StoreId::is_in_collection() The concept of this function was introduced in 37076f028c6cde0924b820154f3464f53ef65268 but here added to StoreId as function. --- libimagstore/src/storeid.rs | 45 +++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/libimagstore/src/storeid.rs b/libimagstore/src/storeid.rs index e5efdeed..bcf6f8dd 100644 --- a/libimagstore/src/storeid.rs +++ b/libimagstore/src/storeid.rs @@ -86,6 +86,29 @@ impl StoreId { self.id.components() } + /// Check whether a StoreId points to an entry in a specific collection. + /// + /// A "collection" here is simply a directory. So `foo/bar/baz` is an entry which is in + /// collection ["foo", "bar"]. + /// + /// # Warning + /// + /// The collection specification _has_ to start with the module name. Otherwise this function + /// may return false negatives. + /// + pub fn is_in_collection(&self, colls: &[&str]) -> bool { + use std::path::Component; + + self.id + .components() + .zip(colls) + .map(|(component, pred_coll)| match component { + Component::Normal(ref s) => s.to_str().map(|ref s| s == pred_coll).unwrap_or(false), + _ => false + }) + .all(|x| x) && colls.last().map(|last| !self.id.ends_with(last)).unwrap_or(false) + } + } impl Into for StoreId { @@ -219,4 +242,26 @@ mod test { assert_eq!(p.into_storeid().unwrap().to_str().unwrap(), "test/test"); } + #[test] + fn storeid_in_collection() { + let p = module_path::ModuleEntryPath::new("1/2/3/4/5/6/7/8/9/0").into_storeid().unwrap(); + + assert!(p.is_in_collection(&["test", "1"])); + assert!(p.is_in_collection(&["test", "1", "2"])); + assert!(p.is_in_collection(&["test", "1", "2", "3"])); + assert!(p.is_in_collection(&["test", "1", "2", "3", "4"])); + assert!(p.is_in_collection(&["test", "1", "2", "3", "4", "5"])); + assert!(p.is_in_collection(&["test", "1", "2", "3", "4", "5", "6"])); + assert!(p.is_in_collection(&["test", "1", "2", "3", "4", "5", "6", "7"])); + assert!(p.is_in_collection(&["test", "1", "2", "3", "4", "5", "6", "7", "8"])); + assert!(p.is_in_collection(&["test", "1", "2", "3", "4", "5", "6", "7", "8", "9"])); + + // "0" is the filename, not a collection + assert!(!p.is_in_collection(&["test", "1", "2", "3", "4", "5", "6", "7", "8", "9", "0"])); + + assert!(!p.is_in_collection(&["test", "0", "2", "3", "4", "5", "6", "7", "8", "9", "0"])); + assert!(!p.is_in_collection(&["test", "1", "2", "3", "4", "5", "6", "8"])); + assert!(!p.is_in_collection(&["test", "1", "2", "3", "leet", "5", "6", "7"])); + } + } From d594b7c6a252f127fb6db0ae005fe61f4eb7a390 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 28 Aug 2016 13:41:28 +0200 Subject: [PATCH 2/4] Reimplement libimagentrylink::external::is_external_link_storeid() using StoreId::is_in_collection() --- libimagentrylink/src/external.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/libimagentrylink/src/external.rs b/libimagentrylink/src/external.rs index 11c524a0..171db965 100644 --- a/libimagentrylink/src/external.rs +++ b/libimagentrylink/src/external.rs @@ -91,16 +91,8 @@ pub trait ExternalLinker : InternalLinker { /// Check whether the StoreId starts with `/link/external/` pub fn is_external_link_storeid(id: &StoreId) -> bool { - use std::path::Component; - debug!("Checking whether this is a link/external/*: '{:?}'", id); - id.components() - .take(2) - .zip(&["link", "external"]) - .map(|(c, pred)| match c { - Component::Normal(ref s) => s.to_str().map(|ref s| s == pred).unwrap_or(false), - _ => false - }).all(|x| x) + id.is_in_collection(&["link", "external"]) } fn get_external_link_from_file(entry: &FileLockEntry) -> Result { From 3e4b0e4bb8939358dfa95b392ba247cf2ebb9280 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 28 Aug 2016 13:53:49 +0200 Subject: [PATCH 3/4] Rewrite internal link handling, returning Result<_> rather than Option<_> --- libimagentrylink/src/internal.rs | 46 ++++++++++++-------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index d77599ca..90344cf3 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -54,13 +54,11 @@ impl InternalLinker for Entry { .into_iter() .fold(Ok(vec![]), |acc, elem| { acc.and_then(move |mut v| { - match elem { - None => Err(LEK::InternalConversionError.into()), - Some(e) => { + elem.map_err_into(LEK::InternalConversionError) + .map(|e| { v.push(e); - Ok(v) - }, - } + v + }) }) })); process_rw_result(self.get_header_mut().set("imag.links", Value::Array(new_links))) @@ -99,23 +97,17 @@ impl InternalLinker for Entry { } -fn links_into_values(links: Vec) -> Vec> { - use libimagutil::debug_result::InfoResult; - +fn links_into_values(links: Vec) -> Vec> { links .into_iter() - .map(|s| { - s.to_str() - .map_dbg_err(|e| format!("Failed to translate StoreId to String: {:?}", e)) - .ok() - }) .unique() + .map(|s| s.to_str().map_err_into(LEK::InternalConversionError)) .map(|elem| elem.map(Value::String)) .sorted_by(|a, b| { match (a, b) { - (&Some(Value::String(ref a)), &Some(Value::String(ref b))) => Ord::cmp(a, b), - (&None, _) | (_, &None) => Ordering::Equal, - _ => unreachable!() + (&Ok(Value::String(ref a)), &Ok(Value::String(ref b))) => Ord::cmp(a, b), + (&Err(_), _) | (_, &Err(_)) => Ordering::Equal, + _ => unreachable!() } }) } @@ -125,13 +117,11 @@ fn rewrite_links(header: &mut EntryHeader, links: Vec) -> Result<()> { .into_iter() .fold(Ok(vec![]), |acc, elem| { acc.and_then(move |mut v| { - match elem { - None => Err(LEK::InternalConversionError.into()), - Some(e) => { + elem.map_err_into(LEK::InternalConversionError) + .map(|e| { v.push(e); - Ok(v) - }, - } + v + }) }) })); @@ -149,13 +139,11 @@ fn add_foreign_link(target: &mut Entry, from: StoreId) -> Result<()> { .into_iter() .fold(Ok(vec![]), |acc, elem| { acc.and_then(move |mut v| { - match elem { - None => Err(LEK::InternalConversionError.into()), - Some(e) => { + elem.map_err_into(LEK::InternalConversionError) + .map(|e| { v.push(e); - Ok(v) - }, - } + v + }) }) })); process_rw_result(target.get_header_mut().set("imag.links", Value::Array(links))) From 46bf387fc3a88a59de5cb5883b17ea35eae485d7 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 28 Aug 2016 18:39:22 +0200 Subject: [PATCH 4/4] Make internal linking only have store-relative links --- libimagentrylink/src/internal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libimagentrylink/src/internal.rs b/libimagentrylink/src/internal.rs index 90344cf3..e6dddb6f 100644 --- a/libimagentrylink/src/internal.rs +++ b/libimagentrylink/src/internal.rs @@ -101,7 +101,7 @@ fn links_into_values(links: Vec) -> Vec> { links .into_iter() .unique() - .map(|s| s.to_str().map_err_into(LEK::InternalConversionError)) + .map(|s| s.without_base().to_str().map_err_into(LEK::InternalConversionError)) .map(|elem| elem.map(Value::String)) .sorted_by(|a, b| { match (a, b) {