From 0f7b2f16f9272d728f8912bff58eb4ad4d34c7e6 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 16 Apr 2016 15:54:25 +0200 Subject: [PATCH 1/4] Add dependency: itertool = 0.4.7 --- libimaglink/Cargo.toml | 1 + libimaglink/src/lib.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/libimaglink/Cargo.toml b/libimaglink/Cargo.toml index 54c3b15a..01640d27 100644 --- a/libimaglink/Cargo.toml +++ b/libimaglink/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" authors = ["Matthias Beyer "] [dependencies] +itertools = "0.4.7" log = "0.3.4" toml = "0.1.27" url = "0.5.5" diff --git a/libimaglink/src/lib.rs b/libimaglink/src/lib.rs index abb3a7ab..401b624a 100644 --- a/libimaglink/src/lib.rs +++ b/libimaglink/src/lib.rs @@ -1,3 +1,4 @@ +extern crate itertools; #[macro_use] extern crate log; extern crate toml; extern crate url; From b63e2cb7b21f01e02c2668e7bedec57e70a70de8 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 16 Apr 2016 15:54:38 +0200 Subject: [PATCH 2/4] Make links unique before writing --- libimaglink/src/internal.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/libimaglink/src/internal.rs b/libimaglink/src/internal.rs index 74a7fcee..244da33c 100644 --- a/libimaglink/src/internal.rs +++ b/libimaglink/src/internal.rs @@ -7,6 +7,7 @@ use error::{LinkError, LinkErrorKind}; use result::Result; use toml::Value; +use itertools::Itertools; pub type Link = StoreId; @@ -48,9 +49,10 @@ impl InternalLinker for Entry { } let link = link.unwrap(); - new_links.push(Value::String(String::from(link))); + new_links.push(String::from(link)); } + let new_links = new_links.into_iter().unique().map(|s| Value::String(s)).collect(); process_rw_result(self.get_header_mut().set("imag.links", Value::Array(new_links))) } @@ -90,7 +92,9 @@ impl InternalLinker for Entry { fn rewrite_links(header: &mut EntryHeader, links: Vec) -> Result<()> { let links : Vec> = links .into_iter() - .map(|s| s.to_str().map(|s| Value::String(String::from(s)))) + .map(|s| s.to_str().map(|s| String::from(s))) + .unique() + .map(|elem| elem.map(|s| Value::String(s))) .collect(); if links.iter().any(|o| o.is_none()) { @@ -114,10 +118,12 @@ fn add_foreign_link(target: &mut Entry, from: StoreId) -> Result<()> { .into_iter() .map(|s| { match s.to_str() { - Some(s) => Some(Value::String(String::from(s))), + Some(s) => Some(String::from(s)), _ => None } }) + .unique() + .map(|elem| elem.map(|s| Value::String(s))) .collect(); if links.iter().any(|o| o.is_none()) { Err(LinkError::new(LinkErrorKind::InternalConversionError, None)) From 915e711c3ed0eb9f7a8b7b094e2a11a0d4310772 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 16 Apr 2016 16:09:00 +0200 Subject: [PATCH 3/4] Sort links alphabetically before writing --- libimaglink/src/internal.rs | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/libimaglink/src/internal.rs b/libimaglink/src/internal.rs index 244da33c..f6539014 100644 --- a/libimaglink/src/internal.rs +++ b/libimaglink/src/internal.rs @@ -1,3 +1,5 @@ +use std::cmp::Ordering; + use libimagstore::storeid::StoreId; use libimagstore::store::Entry; use libimagstore::store::EntryHeader; @@ -52,7 +54,16 @@ impl InternalLinker for Entry { new_links.push(String::from(link)); } - let new_links = new_links.into_iter().unique().map(|s| Value::String(s)).collect(); + let new_links = new_links + .into_iter() + .unique() + .map(|s| Value::String(s)) + .sorted_by(|a, b| { + match (a, b) { + (&Value::String(ref a), &Value::String(ref b)) => Ord::cmp(a, b), + _ => unreachable!() + } + }); process_rw_result(self.get_header_mut().set("imag.links", Value::Array(new_links))) } @@ -95,7 +106,13 @@ fn rewrite_links(header: &mut EntryHeader, links: Vec) -> Result<()> { .map(|s| s.to_str().map(|s| String::from(s))) .unique() .map(|elem| elem.map(|s| Value::String(s))) - .collect(); + .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!() + } + }); if links.iter().any(|o| o.is_none()) { // if any type convert failed we fail as well @@ -124,7 +141,13 @@ fn add_foreign_link(target: &mut Entry, from: StoreId) -> Result<()> { }) .unique() .map(|elem| elem.map(|s| Value::String(s))) - .collect(); + .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!() + } + }); if links.iter().any(|o| o.is_none()) { Err(LinkError::new(LinkErrorKind::InternalConversionError, None)) } else { From 59071c9948cc3a81987150912cc1ec3959d5edb6 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 16 Apr 2016 16:09:34 +0200 Subject: [PATCH 4/4] Refactor: Use helper for type conversions instead of doing everything three times --- libimaglink/src/internal.rs | 51 ++++++++++--------------------------- 1 file changed, 14 insertions(+), 37 deletions(-) diff --git a/libimaglink/src/internal.rs b/libimaglink/src/internal.rs index f6539014..e22214d1 100644 --- a/libimaglink/src/internal.rs +++ b/libimaglink/src/internal.rs @@ -45,25 +45,14 @@ impl InternalLinker for Entry { return Err(e); } let link = link.get_location().clone(); - let link = link.to_str(); - if link.is_none() { - return Err(LinkError::new(LinkErrorKind::InternalConversionError, None)); - } - let link = link.unwrap(); - - new_links.push(String::from(link)); + new_links.push(link); } - let new_links = new_links - .into_iter() - .unique() - .map(|s| Value::String(s)) - .sorted_by(|a, b| { - match (a, b) { - (&Value::String(ref a), &Value::String(ref b)) => Ord::cmp(a, b), - _ => unreachable!() - } - }); + let new_links = links_into_values(new_links); + if new_links.iter().any(|o| o.is_none()) { + return Err(LinkError::new(LinkErrorKind::InternalConversionError, None)); + } + let new_links = new_links.into_iter().map(|o| o.unwrap()).collect(); process_rw_result(self.get_header_mut().set("imag.links", Value::Array(new_links))) } @@ -100,8 +89,8 @@ impl InternalLinker for Entry { } -fn rewrite_links(header: &mut EntryHeader, links: Vec) -> Result<()> { - let links : Vec> = links +fn links_into_values(links: Vec) -> Vec> { + links .into_iter() .map(|s| s.to_str().map(|s| String::from(s))) .unique() @@ -112,7 +101,11 @@ fn rewrite_links(header: &mut EntryHeader, links: Vec) -> Result<()> { (&None, _) | (_, &None) => Ordering::Equal, _ => unreachable!() } - }); + }) +} + +fn rewrite_links(header: &mut EntryHeader, links: Vec) -> Result<()> { + let links = links_into_values(links); if links.iter().any(|o| o.is_none()) { // if any type convert failed we fail as well @@ -131,23 +124,7 @@ fn add_foreign_link(target: &mut Entry, from: StoreId) -> Result<()> { target.get_internal_links() .and_then(|mut links| { links.push(from); - let links : Vec> = links - .into_iter() - .map(|s| { - match s.to_str() { - Some(s) => Some(String::from(s)), - _ => None - } - }) - .unique() - .map(|elem| elem.map(|s| Value::String(s))) - .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!() - } - }); + let links = links_into_values(links); if links.iter().any(|o| o.is_none()) { Err(LinkError::new(LinkErrorKind::InternalConversionError, None)) } else {