From 5894c27e441394f0cb0c290e8ae271c2f9e3a02d Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 23 Sep 2017 15:03:29 +0200 Subject: [PATCH] Use error chain link functionality to remove link-conversion boilerplate --- lib/entry/libimagentrylink/src/error.rs | 28 ++++---------- lib/entry/libimagentrylink/src/external.rs | 29 +++++--------- lib/entry/libimagentrylink/src/internal.rs | 44 ++++++++++------------ 3 files changed, 36 insertions(+), 65 deletions(-) diff --git a/lib/entry/libimagentrylink/src/error.rs b/lib/entry/libimagentrylink/src/error.rs index 6f27f92a..b54c9d61 100644 --- a/lib/entry/libimagentrylink/src/error.rs +++ b/lib/entry/libimagentrylink/src/error.rs @@ -24,6 +24,14 @@ error_chain! { LinkError, LinkErrorKind, ResultExt, Result; } + links { + StoreError(::libimagstore::error::StoreError, ::libimagstore::error::StoreErrorKind); + } + + foreign_links { + TomlQueryError(::toml_query::error::Error); + } + errors { EntryHeaderReadError { description("Error while reading an entry header") @@ -70,21 +78,6 @@ error_chain! { display("URI is not valid") } - StoreReadError { - description("Store read error") - display("Store read error") - } - - StoreWriteError { - description("Store write error") - display("Store write error") - } - - StoreIdError { - description("StoreId handling error") - display("StoreId handling error") - } - DeadLink(from: StoreId, to: StoreId) { description("Dead link") display("Dead link from: {from} to: {to}", from = from, to = to) @@ -94,11 +87,6 @@ error_chain! { description("Error in link handling") display("Error in link handling") } - - StoreError { - description("Error while talking to the store") - display("Error while talking to the store") - } } } diff --git a/lib/entry/libimagentrylink/src/external.rs b/lib/entry/libimagentrylink/src/external.rs index ef111334..1211da3d 100644 --- a/lib/entry/libimagentrylink/src/external.rs +++ b/lib/entry/libimagentrylink/src/external.rs @@ -129,8 +129,6 @@ pub mod iter { use internal::Link; use internal::iter::LinkIter; - use error::LinkErrorKind as LEK; - use error::ResultExt; use error::Result; use url::Url; @@ -266,8 +264,8 @@ pub mod iter { debug!("Retrieving entry for id: '{:?}'", id); self.1 .retrieve(id.clone()) - .chain_err(|| LEK::StoreReadError) .map_dbg_err(|_| format!("Retrieving entry for id: '{:?}' failed", id)) + .map_err(From::from) .and_then(|f| { debug!("Store::retrieve({:?}) succeeded", id); debug!("getting external link from file now"); @@ -308,7 +306,6 @@ impl ExternalLinker for Entry { // /link/external/ -> load these files and get the external link from their headers, // put them into the return vector. self.get_internal_links() - .chain_err(|| LEK::StoreReadError) .map(|iter| { debug!("Getting external links"); iter.only_external_links().urls(store) @@ -330,7 +327,6 @@ impl ExternalLinker for Entry { }; let file_id = try!( ModuleEntryPath::new(format!("external/{}", hash)).into_storeid() - .chain_err(|| LEK::StoreWriteError) .map_dbg_err(|_| { format!("Failed to build StoreId for this hash '{:?}'", hash) }) @@ -344,7 +340,6 @@ impl ExternalLinker for Entry { // exist let mut file = try!(store .retrieve(file_id.clone()) - .chain_err(|| LEK::StoreWriteError) .map_dbg_err(|_| { format!("Failed to create or retrieve an file for this link '{:?}'", link) })); @@ -353,15 +348,14 @@ impl ExternalLinker for Entry { { let hdr = file.deref_mut().get_header_mut(); - let mut table = match hdr.read("links.external.content") { - Ok(Some(&Value::Table(ref table))) => table.clone(), - Ok(Some(_)) => { + let mut table = match try!(hdr.read("links.external.content")) { + Some(&Value::Table(ref table)) => table.clone(), + Some(_) => { warn!("There is a value at 'links.external.content' which is not a table."); warn!("Going to override this value"); BTreeMap::new() }, - Ok(None) => BTreeMap::new(), - Err(e) => return Err(e).chain_err(|| LEK::StoreWriteError), + None => BTreeMap::new(), }; let v = Value::String(link.into_string()); @@ -369,18 +363,13 @@ impl ExternalLinker for Entry { debug!("setting URL = '{:?}", v); table.insert(String::from("url"), v); - if let Err(e) = hdr.insert("links.external.content", Value::Table(table)) { - return Err(e).chain_err(|| LEK::StoreWriteError); - } else { - debug!("Setting URL worked"); - } + let _ = try!(hdr.insert("links.external.content", Value::Table(table))); + debug!("Setting URL worked"); } // then add an internal link to the new file or return an error if this fails - if let Err(e) = self.add_internal_link(file.deref_mut()) { - debug!("Error adding internal link"); - return Err(e).chain_err(|| LEK::StoreWriteError); - } + let _ = try!(self.add_internal_link(file.deref_mut())); + debug!("Error adding internal link"); } debug!("Ready iterating"); Ok(()) diff --git a/lib/entry/libimagentrylink/src/internal.rs b/lib/entry/libimagentrylink/src/internal.rs index 3fd02faf..e0b72792 100644 --- a/lib/entry/libimagentrylink/src/internal.rs +++ b/lib/entry/libimagentrylink/src/internal.rs @@ -51,7 +51,7 @@ impl Link { Link::Id { ref link } => link.exists(), Link::Annotated { ref link, .. } => link.exists(), } - .chain_err(|| LEK::StoreIdError) + .map_err(From::from) } pub fn to_str(&self) -> Result { @@ -59,7 +59,7 @@ impl Link { Link::Id { ref link } => link.to_str(), Link::Annotated { ref link, .. } => link.to_str(), } - .chain_err(|| LEK::StoreReadError) + .map_err(From::from) } @@ -288,10 +288,10 @@ pub mod iter { type Item = Result>; fn next(&mut self) -> Option { - self.0.next().and_then(|id| match self.1.get(id).chain_err(|| LEK::StoreReadError) { + self.0.next().and_then(|id| match self.1.get(id) { Ok(None) => None, Ok(Some(x)) => Some(Ok(x)), - Err(e) => Some(Err(e)), + Err(e) => Some(Err(e).map_err(From::from)), }) } @@ -318,8 +318,7 @@ pub mod iter { loop { match self.0.next() { Some(Ok(fle)) => { - let links = match fle.get_internal_links().chain_err(|| LEK::StoreReadError) - { + let links = match fle.get_internal_links() { Err(e) => return Some(Err(e)), Ok(links) => links.collect::>(), }; @@ -358,19 +357,14 @@ pub mod iter { loop { match self.0.next() { Some(Ok(fle)) => { - let links = match fle.get_internal_links().chain_err(|| LEK::StoreReadError) - { + let links = match fle.get_internal_links() { Err(e) => return Some(Err(e)), Ok(links) => links, }; if links.count() == 0 { - match self.0 - .store() - .delete(fle.get_location().clone()) - .chain_err(|| LEK::StoreWriteError) - { + match self.0.store().delete(fle.get_location().clone()) { Ok(x) => x, - Err(e) => return Some(Err(e)), + Err(e) => return Some(Err(e).map_err(From::from)), } } else { return Some(Ok(fle)); @@ -562,8 +556,8 @@ fn process_rw_result(links: Result>) -> Result { debug!("Matching the link: {:?}", link); match link { Value::String(s) => StoreId::new_baseless(PathBuf::from(s)) - .chain_err(|| LEK::StoreIdError) .map(|s| Link::Id { link: s }) + .map_err(From::from) , Value::Table(mut tab) => { debug!("Destructuring table"); @@ -582,7 +576,7 @@ fn process_rw_result(links: Result>) -> Result { match (link, anno) { (Value::String(link), Value::String(anno)) => { StoreId::new_baseless(PathBuf::from(link)) - .chain_err(|| LEK::StoreIdError) + .map_err(From::from) .map(|link| { Link::Annotated { link: link, @@ -642,24 +636,24 @@ pub mod store_check { /// The lambda returns an error if something fails let aggregate_link_network = |store: &Store| -> Result> { let iter = store - .entries() - .chain_err(|| LEK::StoreReadError)? + .entries()? .into_get_iter(store); let mut map = HashMap::new(); for element in iter { debug!("Checking element = {:?}", element); - let entry = match element { - Ok(Some(e)) => e, - Ok(None) => return Err(LE::from_kind(LEK::StoreIdError)), - Err(e) => return Err(e).chain_err(|| LE::from_kind(LEK::StoreReadError)), + let entry = match try!(element) { + Some(e) => e, + None => { + let e = String::from("TODO: Not yet handled"); + return Err(e).map_err(From::from); + }, }; debug!("Checking entry = {:?}", entry.get_location()); let internal_links = entry - .get_internal_links() - .chain_err(|| LEK::StoreError)? + .get_internal_links()? .into_getter(store); // get the FLEs from the Store let mut linking = Linking::default(); @@ -686,7 +680,7 @@ pub mod store_check { if is_match!(self.get(id.clone()), Ok(Some(_))) { debug!("Exists in store: {:?}", id); - if !try!(id.exists().chain_err(|| LEK::StoreReadError)) { + if !try!(id.exists()) { warn!("Does exist in store but not on FS: {:?}", id); Err(LE::from_kind(LEK::LinkTargetDoesNotExist)) } else {