Rewrite internal linking to use StoreId instead of String as Link type

1 files changed, 69 insertions, 46 deletions

diff --git a/libimaglink/src/internal.rs b/libimaglink/src/internal.rs
index d9732758..4eee600d 100644
--- a/libimaglink/src/internal.rs
+++ b/libimaglink/src/internal.rs
@@ -1,6 +1,5 @@
 use libimagstore::storeid::StoreId;
 use libimagstore::store::Entry;
-use libimagstore::store::EntryHeader;
 use libimagstore::store::Result as StoreResult;
 
 use error::{LinkError, LinkErrorKind};
@@ -8,7 +7,7 @@ use result::Result;
 
 use toml::Value;
 
-pub type Link = String;
+pub type Link = StoreId;
 
 pub trait InternalLinker {
 
@@ -56,59 +55,77 @@ impl InternalLinker for Entry {
 
     fn add_internal_link(&mut self, link: &mut Entry) -> Result<()> {
         let new_link = link.get_location().clone();
-        let new_link = new_link.to_str();
-        if new_link.is_none() {
-            return Err(LinkError::new(LinkErrorKind::InternalConversionError, None));
-        }
-        let new_link = new_link.unwrap();
 
         add_foreign_link(link, self.get_location().clone())
             .and_then(|_| {
                 self.get_internal_links()
                     .and_then(|mut links| {
-                        links.push(String::from(new_link));
-                        let links = links.into_iter().map(|s| Value::String(s)).collect();
-                        let process = self.get_header_mut().set("imag.links", Value::Array(links));
-                        process_rw_result(process)
-                            .map(|_| ())
+                        links.push(new_link);
+
+                        // try to convert them to str so we can put them back into the header
+                        let links : Vec<Option<Value>> = links
+                            .into_iter()
+                            .map(|s| s.to_str().map(|s| Value::String(String::from(s))))
+                            .collect();
+
+                        if links.iter().any(|o| o.is_none()) {
+                            // if any type convert failed we fail as well
+                            Err(LinkError::new(LinkErrorKind::InternalConversionError, None))
+                        } else {
+                            // I know it is ugly
+                            let links = links.into_iter().map(|opt| opt.unwrap()).collect();
+                            let process = self.get_header_mut().set("imag.links", Value::Array(links));
+                            process_rw_result(process)
+                                .map(|_| ())
+                        }
                     })
             })
     }
 
     fn remove_internal_link(&mut self, link: &mut Entry) -> Result<()> {
-        let own_loc = link.get_location().clone();
-        let own_loc = own_loc.to_str();
-        if own_loc.is_none() {
-            return Err(LinkError::new(LinkErrorKind::InternalConversionError, None));
-        }
-        let own_loc = own_loc.unwrap();
-
+        let own_loc   = link.get_location().clone();
         let other_loc = link.get_location().clone();
-        let other_loc = other_loc.to_str();
-        if other_loc.is_none() {
-            return Err(LinkError::new(LinkErrorKind::InternalConversionError, None));
-        }
-        let other_loc = other_loc.unwrap();
 
         link.get_internal_links()
-            .and_then(|mut links| {
-                let links = links.into_iter()
+            .and_then(|links| {
+                let links : Vec<Option<Value>> = links.into_iter()
                     .filter(|l| l.clone() != own_loc)
-                    .map(|s| Value::String(s))
+                    .map(|s| {
+                        match s.to_str() {
+                            Some(s) => Some(Value::String(String::from(s))),
+                            _ => None
+                        }
+                    })
                     .collect();
-                process_rw_result(link.get_header_mut().set("imag.links", Value::Array(links)))
-                    .map(|_| ())
+
+                if links.iter().any(|o| o.is_none()) {
+                    Err(LinkError::new(LinkErrorKind::InternalConversionError, None))
+                } else {
+                    let links = links.into_iter().map(|opt| opt.unwrap()).collect();
+                    process_rw_result(self.get_header_mut().set("imag.links", Value::Array(links)))
+                        .map(|_| ())
+                }
             })
             .and_then(|_| {
                 self.get_internal_links()
-                    .and_then(|mut links| {
-                        let links = links
+                    .and_then(|links| {
+                        let links : Vec<Option<Value>> = links
                             .into_iter()
                             .filter(|l| l.clone() != other_loc)
-                            .map(|s| Value::String(s))
+                            .map(|s| {
+                                match s.to_str() {
+                                    Some(s) => Some(Value::String(String::from(s))),
+                                    _ => None
+                                }
+                            })
                             .collect();
-                        process_rw_result(self.get_header_mut().set("imag.links", Value::Array(links)))
-                            .map(|_| ())
+                        if links.iter().any(|o| o.is_none()) {
+                            Err(LinkError::new(LinkErrorKind::InternalConversionError, None))
+                        } else {
+                            let links = links.into_iter().map(|opt| opt.unwrap()).collect();
+                            process_rw_result(link.get_header_mut().set("imag.links", Value::Array(links)))
+                                .map(|_| ())
+                        }
                     })
             })
     }
@@ -118,19 +135,25 @@ impl InternalLinker for Entry {
 /// 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<()> {
-    let from = from.to_str();
-    if from.is_none() {
-        debug!("Cannot convert pathbuf '{:?}' to String", from);
-        return Err(LinkError::new(LinkErrorKind::InternalConversionError, None));
-    }
-    let from = from.unwrap();
-
     target.get_internal_links()
         .and_then(|mut links| {
-            links.push(String::from(from));
-            let links = links.into_iter().map(|s| Value::String(s)).collect();
-            process_rw_result(target.get_header_mut().set("imag.links", Value::Array(links)))
-                .map(|_| ())
+            links.push(from);
+            let links : Vec<Option<Value>> = links
+                .into_iter()
+                .map(|s| {
+                    match s.to_str() {
+                        Some(s) => Some(Value::String(String::from(s))),
+                        _ => None
+                    }
+                })
+                .collect();
+            if links.iter().any(|o| o.is_none()) {
+                Err(LinkError::new(LinkErrorKind::InternalConversionError, None))
+            } else {
+                let links = links.into_iter().map(|opt| opt.unwrap()).collect();
+                process_rw_result(target.get_header_mut().set("imag.links", Value::Array(links)))
+                    .map(|_| ())
+            }
         })
 }
 
@@ -149,7 +172,7 @@ fn process_rw_result(links: StoreResult<Option<Value>>) -> Result<Vec<Link>> {
     let links : Vec<Link> = links.into_iter()
         .map(|link| {
             match link {
-                Value::String(s) => String::from(s),
+                Value::String(s) => StoreId::from(s),
                 _ => unreachable!(),
             }
         })