matthiasbeyer/imag
1
0
Fork 0
Code Releases Activity

Merge pull request #1062 from matthiasbeyer/libimagstore/error-opt

Browse source 
libimagstore: Optimize errors
This commit is contained in:
Matthias Beyer 2017-09-10 11:45:59 +02:00 committed by GitHub
commit 53c7d602eb
4 changed files with 48 additions and 88 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

77
lib/core/libimagstore/src/error.rs
View file

@ -17,6 +17,10 @@
// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
//
use std::path::PathBuf;
use storeid::StoreId;
error_chain! {
types {
StoreError, StoreErrorKind, ResultExt, Result;
@ -25,6 +29,7 @@ error_chain! {
foreign_links {
Io(::std::io::Error);
TomlDeserError(::toml::de::Error);
GlobPatternError(::glob::PatternError);
}
errors {
@ -69,14 +74,9 @@ error_chain! {
display("ID locked")
}
IdNotFound {
IdNotFound(sid: StoreId) {
description("ID not found")
display("ID not found")
}
OutOfMemory {
description("Out of Memory")
display("Out of Memory")
display("ID not found: {}", sid)
}
FileNotFound {
@ -119,14 +119,14 @@ 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 {
StorePathCreate(pb: PathBuf) {
description("Store path create")
display("Store path create")
display("Store path create: {:?}", pb)
}
LockError {
@ -139,14 +139,14 @@ 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 {
EntryAlreadyExists(id: StoreId) {
description("Entry already exists")
display("Entry already exists")
display("Entry already exists: {:?}", id)
}
MalformedEntry {
@ -154,49 +154,19 @@ 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")
}
EncodingError {
description("Encoding error")
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 {
@ -204,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 {
@ -214,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 {
@ -271,11 +241,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")

48
lib/core/libimagstore/src/store.rs
View file

@ -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));
}
@ -173,15 +173,15 @@ impl StoreEntry {
Err(err)
})
} else {
Err(SE::from_kind(SEK::EntryAlreadyBorrowed))
Err(SE::from_kind(SEK::EntryAlreadyBorrowed(self.id.clone())))
}
}
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())
}
}
@ -244,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<Value>) -> Result<Store> {
let backend = Box::new(FSFileAbstraction::new());
Store::new_with_backend(location, store_config, backend)
@ -270,20 +264,17 @@ 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);
}
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");
return Err(SE::from_kind(SEK::StorePathExists));
return Err(SE::from_kind(SEK::StorePathExists(location)));
}
let store = Store {
@ -413,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);
@ -515,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)
}
@ -563,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.");
@ -682,7 +675,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();
@ -748,21 +742,21 @@ 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.
// 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());
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");
@ -1146,7 +1140,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)
}

9
lib/core/libimagstore/src/storeid.rs
View file

@ -68,7 +68,7 @@ impl StoreId {
pub fn new_baseless(id: PathBuf) -> Result<StoreId> {
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,
@ -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<PathBuf> {
let mut base = try!(self.base.ok_or(SEK::StoreIdHasNoBaseError));
pub fn into_pathbuf(mut self) -> Result<PathBuf> {
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]

2
lib/entry/libimagentrycategory/src/register.rs
View file

@ -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)