From 3d8f75300df3b3675e6620ac0b4d3597f8d82f70 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:13:21 +0200 Subject: [PATCH 01/13] Remove unecessary error chaining --- lib/core/libimagstore/src/store.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 4fec02c1..dbb00182 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -146,7 +146,7 @@ impl StoreEntry { #[cfg(feature = "fs-lock")] { try!(open_file(pb.clone()) - .and_then(|f| f.lock_exclusive().chain_err(|| SEK::FileError)) + .and_then(|f| f.lock_exclusive()) .chain_err(|| SEK::IoError)); } @@ -180,8 +180,8 @@ impl StoreEntry { fn write_entry(&mut self, entry: &Entry) -> Result<()> { if self.is_borrowed() { assert_eq!(self.id, entry.location); - self.file.write_file_content(entry) - .chain_err(|| SEK::FileError) + self.file + .write_file_content(entry) .map(|_| ()) } else { Ok(()) @@ -194,9 +194,8 @@ impl Drop for StoreEntry { fn drop(self) { self.get_entry() - .and_then(|entry| open_file(entry.get_location().clone()).chain_err(|| SEK::IoError)) - .and_then(|f| f.unlock().chain_err(|| SEK::FileError)) - .chain_err(|| SEK::IoError) + .and_then(|entry| open_file(entry.get_location().clone())) + .and_then(|f| f.unlock()) } } From 21440d58aa19ba06d2d153da3a3ee6b774228b07 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:13:38 +0200 Subject: [PATCH 02/13] Remove outdated comment --- lib/core/libimagstore/src/store.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index dbb00182..fe9e2da1 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -243,12 +243,7 @@ impl Store { /// # Return values /// /// - On success: Store object - /// - On Failure: - /// - ConfigurationError if config is faulty - /// - IoError(FileError(CreateStoreDirDenied())) if store location does not exist and creating - /// is denied - /// - StorePathCreate(_) if creating the store directory failed - /// - StorePathExists() if location exists but is a file + /// pub fn new(location: PathBuf, store_config: Option) -> Result { let backend = Box::new(FSFileAbstraction::new()); Store::new_with_backend(location, store_config, backend) From b682e7f8db2bfb6b41004c625a3cc752981c7c95 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:14:08 +0200 Subject: [PATCH 03/13] Remove warning which is printed by the store --- lib/core/libimagstore/src/store.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index fe9e2da1..3413d905 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -264,9 +264,6 @@ impl Store { debug!("Building new Store object"); if !location.exists() { if !config_implicit_store_create_allowed(store_config.as_ref()) { - warn!("Implicitely creating store directory is denied"); - warn!(" -> Either because configuration does not allow it"); - warn!(" -> or because there is no configuration"); return Err(SE::from_kind(SEK::CreateStoreDirDenied)) .chain_err(|| SEK::FileError) .chain_err(|| SEK::IoError); From ca9123c740caa6d16d84956f4019e0e5fa649bb8 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:27:10 +0200 Subject: [PATCH 04/13] Add parameter to IdNotFound error --- lib/core/libimagstore/src/error.rs | 6 ++++-- lib/core/libimagstore/src/store.rs | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index 2f172e34..7ec504cf 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -17,6 +17,8 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // +use storeid::StoreId; + error_chain! { types { StoreError, StoreErrorKind, ResultExt, Result; @@ -69,9 +71,9 @@ error_chain! { display("ID locked") } - IdNotFound { + IdNotFound(sid: StoreId) { description("ID not found") - display("ID not found") + display("ID not found: {}", sid) } OutOfMemory { diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 3413d905..595bdb1e 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -554,7 +554,9 @@ impl Store { Ok(e) => e, }; - let se = try!(hsmap.get_mut(&entry.location).ok_or(SE::from_kind(SEK::IdNotFound))); + let se = try!(hsmap.get_mut(&entry.location).ok_or_else(|| { + SE::from_kind(SEK::IdNotFound(entry.location.clone())) + })); assert!(se.is_borrowed(), "Tried to update a non borrowed entry."); From 78db822917b735c174d38429259be6f6fb03e96c Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:27:51 +0200 Subject: [PATCH 05/13] Remove unused error type --- lib/core/libimagstore/src/error.rs | 30 ------------------------------ lib/core/libimagstore/src/store.rs | 2 +- 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index 7ec504cf..f2d338fc 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -76,11 +76,6 @@ error_chain! { display("ID not found: {}", sid) } - OutOfMemory { - description("Out of Memory") - display("Out of Memory") - } - FileNotFound { description("File corresponding to ID not found") display("File corresponding to ID not found") @@ -156,31 +151,11 @@ error_chain! { display("Entry has invalid formatting, missing header") } - HeaderPathSyntaxError { - description("Syntax error in accessor string") - display("Syntax error in accessor string") - } - - HeaderPathTypeFailure { - description("Header has wrong type for path") - display("Header has wrong type for path") - } - - HeaderKeyNotFound { - description("Header Key not found") - display("Header Key not found") - } - HeaderTypeFailure { description("Header type is wrong") display("Header type is wrong") } - StorePathLacksVersion { - description("The supplied store path has no version part") - display("The supplied store path has no version part") - } - GlobError { description("glob() error") display("glob() error") @@ -273,11 +248,6 @@ error_chain! { // Parser-related errors - TOMLParserErrors { - description("Several TOML-Parser-Errors") - display("Several TOML-Parser-Errors") - } - MissingMainSection { description("Missing main section") display("Missing main section") diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 595bdb1e..64219c46 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -1139,7 +1139,7 @@ impl Header for Value { use toml::de::from_str; from_str(s) - .chain_err(|| SEK::TOMLParserErrors) + .map_err(From::from) .and_then(verify_header_consistency) .map(Value::Table) } From ff8569809bd4013b9ec5b47c8f527365d70325bf Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:31:13 +0200 Subject: [PATCH 06/13] Add parameter to StorePathExists error --- lib/core/libimagstore/src/error.rs | 6 ++++-- lib/core/libimagstore/src/store.rs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index f2d338fc..8d0b180c 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -17,6 +17,8 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // +use std::path::PathBuf; + use storeid::StoreId; error_chain! { @@ -116,9 +118,9 @@ error_chain! { display("Directory/Directories could not be created") } - StorePathExists { + StorePathExists(pb: PathBuf) { description("Store path exists") - display("Store path exists") + display("Store path exists: {:?}", pb) } StorePathCreate { diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 64219c46..0945c3a4 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -274,7 +274,7 @@ impl Store { .map_dbg_err_str("Failed")); } else if location.is_file() { debug!("Store path exists as file"); - return Err(SE::from_kind(SEK::StorePathExists)); + return Err(SE::from_kind(SEK::StorePathExists(location))); } let store = Store { From a28613b889eeb7fe20751919df3be441e85f3532 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:34:06 +0200 Subject: [PATCH 07/13] Add param to StorePathCreate --- lib/core/libimagstore/src/error.rs | 4 ++-- lib/core/libimagstore/src/store.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index 8d0b180c..004e0b60 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -123,9 +123,9 @@ error_chain! { display("Store path exists: {:?}", pb) } - StorePathCreate { + StorePathCreate(pb: PathBuf) { description("Store path create") - display("Store path create") + display("Store path create: {:?}", pb) } LockError { diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 0945c3a4..bc9fa77d 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -270,7 +270,7 @@ impl Store { } try!(backend.create_dir_all(&location) - .chain_err(|| SEK::StorePathCreate) + .chain_err(|| SEK::StorePathCreate(location.clone())) .map_dbg_err_str("Failed")); } else if location.is_file() { debug!("Store path exists as file"); From 83f9350d984fac726ce459472e673a99a7a46e2e Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:36:35 +0200 Subject: [PATCH 08/13] Add param to EntryAlreadyBorrowed error --- lib/core/libimagstore/src/error.rs | 4 ++-- lib/core/libimagstore/src/store.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index 004e0b60..ce534833 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -138,9 +138,9 @@ error_chain! { display("The internal Store Lock has been poisoned") } - EntryAlreadyBorrowed { + EntryAlreadyBorrowed(id: StoreId) { description("Entry is already borrowed") - display("Entry is already borrowed") + display("Entry is already borrowed: {:?}", id) } EntryAlreadyExists { diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index bc9fa77d..e1ca17a1 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -173,7 +173,7 @@ impl StoreEntry { Err(err) }) } else { - Err(SE::from_kind(SEK::EntryAlreadyBorrowed)) + Err(SE::from_kind(SEK::EntryAlreadyBorrowed(self.id.clone()))) } } @@ -748,7 +748,7 @@ impl Store { // if we have one, but it is borrowed, we really should not rename it, as this might // lead to strange errors if hsmap.get(&old_id).map(|e| e.is_borrowed()).unwrap_or(false) { - return Err(SE::from_kind(SEK::EntryAlreadyBorrowed)); + return Err(SE::from_kind(SEK::EntryAlreadyBorrowed(old_id.clone()))); } let old_id_pb = try!(old_id.clone().with_base(self.path().clone()).into_pathbuf()); From 6aa695974cd3d08659bde7a3a80471715e81c4d7 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:38:59 +0200 Subject: [PATCH 09/13] Add param to EntryAlreadyExists error --- lib/core/libimagstore/src/error.rs | 4 ++-- lib/core/libimagstore/src/store.rs | 8 +++++--- lib/entry/libimagentrycategory/src/register.rs | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index ce534833..ca8df222 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -143,9 +143,9 @@ error_chain! { display("Entry is already borrowed: {:?}", id) } - EntryAlreadyExists { + EntryAlreadyExists(id: StoreId) { description("Entry already exists") - display("Entry already exists") + display("Entry already exists: {:?}", id) } MalformedEntry { diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index e1ca17a1..aa3b099f 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -404,7 +404,8 @@ impl Store { if hsmap.contains_key(&id) { debug!("Cannot create, internal cache already contains: '{}'", id); - return Err(SE::from_kind(SEK::EntryAlreadyExists)).chain_err(|| SEK::CreateCallError); + return Err(SE::from_kind(SEK::EntryAlreadyExists(id.clone()))) + .chain_err(|| SEK::CreateCallError); } hsmap.insert(id.clone(), { debug!("Creating: '{}'", id); @@ -675,7 +676,8 @@ impl Store { ); if hsmap.contains_key(&new_id) { - return Err(SE::from_kind(SEK::EntryAlreadyExists)).chain_err(|| SEK::MoveCallError) + return Err(SE::from_kind(SEK::EntryAlreadyExists(new_id.clone()))) + .chain_err(|| SEK::MoveCallError) } let old_id = entry.get_location().clone(); @@ -741,7 +743,7 @@ impl Store { }; if hsmap.contains_key(&new_id) { - return Err(SE::from_kind(SEK::EntryAlreadyExists)); + return Err(SE::from_kind(SEK::EntryAlreadyExists(new_id.clone()))); } // if we do not have an entry here, we fail in `FileAbstraction::rename()` below. diff --git a/lib/entry/libimagentrycategory/src/register.rs b/lib/entry/libimagentrycategory/src/register.rs index ad4d5b73..67cb6877 100644 --- a/lib/entry/libimagentrycategory/src/register.rs +++ b/lib/entry/libimagentrycategory/src/register.rs @@ -84,7 +84,7 @@ impl CategoryRegister for Store { .chain_err(|| CEK::HeaderWriteError) .chain_err(|| CEK::StoreWriteError) } - Err(store_error) => if is_match!(store_error.kind(), &SEK::EntryAlreadyExists) { + Err(store_error) => if is_match!(store_error.kind(), &SEK::EntryAlreadyExists(_)) { Ok(false) } else { Err(store_error).chain_err(|| CEK::StoreWriteError) From 2ce2ba54dabc30d30ba77b1b77120dcc424c139a Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:46:25 +0200 Subject: [PATCH 10/13] Replace GlobError with link to ::glob::PatternError --- lib/core/libimagstore/src/error.rs | 6 +----- lib/core/libimagstore/src/store.rs | 3 +-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index ca8df222..4fb3a44a 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -29,6 +29,7 @@ error_chain! { foreign_links { Io(::std::io::Error); TomlDeserError(::toml::de::Error); + GlobPatternError(::glob::PatternError); } errors { @@ -158,11 +159,6 @@ error_chain! { display("Header type is wrong") } - GlobError { - description("glob() error") - display("glob() error") - } - EncodingError { description("Encoding error") display("Encoding error") diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index aa3b099f..52426050 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -507,10 +507,9 @@ impl Store { .and_then(|path| { let path = [ path, "/**/*" ].join(""); debug!("glob()ing with '{}'", path); - glob(&path[..]).chain_err(|| SEK::GlobError) + glob(&path[..]).map_err(From::from) }) .map(|paths| GlobStoreIdIterator::new(paths, self.path().clone()).into()) - .chain_err(|| SEK::GlobError) .chain_err(|| SEK::RetrieveForModuleCallError) } From b772908697bbbdc0e79cc0f8238021df0dbe7a52 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:49:14 +0200 Subject: [PATCH 11/13] Add params to EntryRenameError --- lib/core/libimagstore/src/error.rs | 9 ++------- lib/core/libimagstore/src/store.rs | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index 4fb3a44a..02a765ea 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -164,14 +164,9 @@ error_chain! { display("Encoding error") } - StorePathError { - description("Store Path error") - display("Store Path error") - } - - EntryRenameError { + EntryRenameError(old: PathBuf, new: PathBuf) { description("Entry rename error") - display("Entry rename error") + display("Entry rename error: {:?} -> {:?}", old, new) } StoreIdHandlingError { diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 52426050..1c6acde6 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -756,7 +756,7 @@ impl Store { let new_id_pb = try!(new_id.clone().with_base(self.path().clone()).into_pathbuf()); match self.backend.rename(&old_id_pb, &new_id_pb) { - Err(e) => return Err(e).chain_err(|| SEK::EntryRenameError), + Err(e) => return Err(e).chain_err(|| SEK::EntryRenameError(old_id_pb, new_id_pb)), Ok(_) => { debug!("Rename worked on filesystem"); From a6701728807ff71f9aec7220967d64172c69dec6 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:50:37 +0200 Subject: [PATCH 12/13] Add param to StoreIdLocalPartAbsoluteError --- lib/core/libimagstore/src/error.rs | 4 ++-- lib/core/libimagstore/src/storeid.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index 02a765ea..3c01b7a6 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -174,9 +174,9 @@ error_chain! { display("StoreId handling error") } - StoreIdLocalPartAbsoluteError { + StoreIdLocalPartAbsoluteError(pb: PathBuf) { description("StoreId 'id' part is absolute (starts with '/') which is not allowed") - display("StoreId 'id' part is absolute (starts with '/') which is not allowed") + display("StoreId 'id' part is absolute (starts with '/') which is not allowed: {:?}", pb) } StoreIdBuildFromFullPathError { diff --git a/lib/core/libimagstore/src/storeid.rs b/lib/core/libimagstore/src/storeid.rs index 50dd0bc7..7c3ef517 100644 --- a/lib/core/libimagstore/src/storeid.rs +++ b/lib/core/libimagstore/src/storeid.rs @@ -68,7 +68,7 @@ impl StoreId { pub fn new_baseless(id: PathBuf) -> Result { debug!("Trying to get a new baseless id from: {:?}", id); if id.is_absolute() { - Err(SE::from_kind(SEK::StoreIdLocalPartAbsoluteError)) + Err(SE::from_kind(SEK::StoreIdLocalPartAbsoluteError(id))) } else { Ok(StoreId { base: None, From 785e17a4a35378af1794b58d91cee9fceca04874 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 9 Sep 2017 21:52:44 +0200 Subject: [PATCH 13/13] Add param to StoreIdHasNoBaseError --- lib/core/libimagstore/src/error.rs | 4 ++-- lib/core/libimagstore/src/storeid.rs | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/core/libimagstore/src/error.rs b/lib/core/libimagstore/src/error.rs index 3c01b7a6..fec24d9e 100644 --- a/lib/core/libimagstore/src/error.rs +++ b/lib/core/libimagstore/src/error.rs @@ -184,9 +184,9 @@ error_chain! { display("Building StoreId from full file path failed") } - StoreIdHasNoBaseError { + StoreIdHasNoBaseError(pb: PathBuf) { description("StoreId has no 'base' part") - display("StoreId has no 'base' part") + display("StoreId has no 'base' part: {:?}", pb) } CreateCallError { diff --git a/lib/core/libimagstore/src/storeid.rs b/lib/core/libimagstore/src/storeid.rs index 7c3ef517..94fa3433 100644 --- a/lib/core/libimagstore/src/storeid.rs +++ b/lib/core/libimagstore/src/storeid.rs @@ -89,8 +89,9 @@ impl StoreId { /// Transform the StoreId object into a PathBuf, error if the base of the StoreId is not /// specified. - pub fn into_pathbuf(self) -> Result { - let mut base = try!(self.base.ok_or(SEK::StoreIdHasNoBaseError)); + pub fn into_pathbuf(mut self) -> Result { + let base = self.base.take(); + let mut base = try!(base.ok_or_else(|| SEK::StoreIdHasNoBaseError(self.id.clone()))); base.push(self.id); Ok(base) } @@ -346,7 +347,7 @@ mod test { let pb = id.unwrap().into_pathbuf(); assert!(pb.is_err()); - assert!(is_match!(pb.unwrap_err().kind(), &SEK::StoreIdHasNoBaseError)); + assert!(is_match!(pb.unwrap_err().kind(), &SEK::StoreIdHasNoBaseError(_))); } #[test]