Merge pull request #681 from matthiasbeyer/rewrite-storeid-type-cleanup-libimagentrylink

libimagentrylink cleanup

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()
+    id.is_in_collection(&["link", "external"])
-        .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)
 }
 
 fn get_external_link_from_file(entry: &FileLockEntry) -> Result<Url> {

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 {
+                                    elem.map_err_into(LEK::InternalConversionError)
-                                        None => Err(LEK::InternalConversionError.into()),
+                                        .map(|e| {
-                                        Some(e) => {
                                             v.push(e);
-                                            Ok(v)
+                                            v
-                                        },
+                                        })
-                                    }
                                 })
                             }));
         process_rw_result(self.get_header_mut().set("imag.links", Value::Array(new_links)))
@@ -99,22 +97,16 @@ impl InternalLinker for Entry {
 
 }
 
-fn links_into_values(links: Vec<StoreId>) -> Vec<Option<Value>> {
+fn links_into_values(links: Vec<StoreId>) -> Vec<Result<Value>> {
-    use libimagutil::debug_result::InfoResult;
-
     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.without_base().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),
+                (&Ok(Value::String(ref a)), &Ok(Value::String(ref b))) => Ord::cmp(a, b),
-                (&None, _) | (_, &None) => Ordering::Equal,
+                (&Err(_), _) | (_, &Err(_)) => Ordering::Equal,
                 _ => unreachable!()
             }
         })
@@ -125,13 +117,11 @@ fn rewrite_links(header: &mut EntryHeader, links: Vec<StoreId>) -> Result<()> {
                      .into_iter()
                      .fold(Ok(vec![]), |acc, elem| {
                         acc.and_then(move |mut v| {
-                            match elem {
+                            elem.map_err_into(LEK::InternalConversionError)
-                                None => Err(LEK::InternalConversionError.into()),
+                                .map(|e| {
-                                Some(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 {
+                                    elem.map_err_into(LEK::InternalConversionError)
-                                        None => Err(LEK::InternalConversionError.into()),
+                                        .map(|e| {
-                                        Some(e) => {
                                             v.push(e);
-                                            Ok(v)
+                                            v
-                                        },
+                                        })
-                                    }
                                 })
                              }));
             process_rw_result(target.get_header_mut().set("imag.links", Value::Array(links)))

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<PathBuf> 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"]));
+    }
+
 }