Return glob errors instead of tracing internally

This changes the internal GlobStoreIdIterator to return Result<StoreId>,
which gives us the possibility to aggregate errors in the
Store::retrieve_for_module() function and return them instead of tracing
them from the store.

The changes the internals to actually fetch the whole list of storeids,
which is unfortunate of course, but changing the interface is not an
option here, in my opinion.

At least we're only aggregating pathes, so the memory usage is pretty
low here.
This commit is contained in:
Matthias Beyer 2017-09-20 22:42:11 +02:00
parent e2bf6c48ef
commit c6e94dfd3c

View file

@ -25,7 +25,6 @@ use std::result::Result as RResult;
use std::sync::Arc; use std::sync::Arc;
use std::sync::RwLock; use std::sync::RwLock;
use std::io::Read; use std::io::Read;
use std::convert::Into;
use std::ops::Deref; use std::ops::Deref;
use std::ops::DerefMut; use std::ops::DerefMut;
use std::fmt::Formatter; use std::fmt::Formatter;
@ -492,6 +491,7 @@ impl Store {
/// On success: An iterator over all entries in the module /// On success: An iterator over all entries in the module
/// ///
/// On failure: /// On failure:
/// - (if the glob or one of the intermediate fail)
/// - RetrieveForModuleCallError(GlobError(EncodingError())) if the path string cannot be /// - RetrieveForModuleCallError(GlobError(EncodingError())) if the path string cannot be
/// encoded /// encoded
/// - GRetrieveForModuleCallError(GlobError(lobError())) if the glob() failed. /// - GRetrieveForModuleCallError(GlobError(lobError())) if the glob() failed.
@ -509,7 +509,11 @@ impl Store {
debug!("glob()ing with '{}'", path); debug!("glob()ing with '{}'", path);
glob(&path[..]).map_err(From::from) glob(&path[..]).map_err(From::from)
}) })
.map(|paths| GlobStoreIdIterator::new(paths, self.path().clone()).into()) .and_then(|paths| {
GlobStoreIdIterator::new(paths, self.path().clone())
.collect::<Result<Vec<_>>>()
})
.map(|v| StoreIdIterator::new(Box::new(v.into_iter())))
.chain_err(|| SEK::RetrieveForModuleCallError) .chain_err(|| SEK::RetrieveForModuleCallError)
} }
@ -1039,24 +1043,16 @@ mod glob_store_iter {
use std::fmt::{Debug, Formatter}; use std::fmt::{Debug, Formatter};
use std::fmt::Error as FmtError; use std::fmt::Error as FmtError;
use std::path::PathBuf; use std::path::PathBuf;
use std::result::Result as RResult;
use glob::Paths; use glob::Paths;
use storeid::StoreId; use storeid::StoreId;
use storeid::StoreIdIterator; use error::Result;
use error::StoreErrorKind as SEK; use error::StoreErrorKind as SEK;
use error::ResultExt; use error::ResultExt;
use libimagerror::trace::trace_error; /// An iterator which is constructed from a `glob()` and returns valid `StoreId` objects or
/// errors
/// An iterator which is constructed from a `glob()` and returns valid `StoreId` objects
///
/// # Warning
///
/// On error, this iterator currently traces the error and return None (thus ending the
/// iteration). This is a known issue and will be resolved at some point.
///
/// TODO: See above.
///
pub struct GlobStoreIdIterator { pub struct GlobStoreIdIterator {
store_path: PathBuf, store_path: PathBuf,
paths: Paths, paths: Paths,
@ -1064,20 +1060,12 @@ mod glob_store_iter {
impl Debug for GlobStoreIdIterator { impl Debug for GlobStoreIdIterator {
fn fmt(&self, fmt: &mut Formatter) -> Result<(), FmtError> { fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FmtError> {
write!(fmt, "GlobStoreIdIterator") write!(fmt, "GlobStoreIdIterator")
} }
} }
impl Into<StoreIdIterator> for GlobStoreIdIterator {
fn into(self) -> StoreIdIterator {
StoreIdIterator::new(Box::new(self))
}
}
impl GlobStoreIdIterator { impl GlobStoreIdIterator {
pub fn new(paths: Paths, store_path: PathBuf) -> GlobStoreIdIterator { pub fn new(paths: Paths, store_path: PathBuf) -> GlobStoreIdIterator {
@ -1092,30 +1080,23 @@ mod glob_store_iter {
} }
impl Iterator for GlobStoreIdIterator { impl Iterator for GlobStoreIdIterator {
type Item = StoreId; type Item = Result<StoreId>;
fn next(&mut self) -> Option<StoreId> { fn next(&mut self) -> Option<Self::Item> {
while let Some(o) = self.paths.next() { while let Some(o) = self.paths.next() {
debug!("GlobStoreIdIterator::next() => {:?}", o); debug!("GlobStoreIdIterator::next() => {:?}", o);
match o.chain_err(|| SEK::StoreIdHandlingError) { match o.chain_err(|| SEK::StoreIdHandlingError) {
Err(e) => return Some(Err(e)),
Ok(path) => { Ok(path) => {
if path.exists() && path.is_file() { if path.exists() && path.is_file() {
return match StoreId::from_full_path(&self.store_path, path) { return match StoreId::from_full_path(&self.store_path, path) {
Ok(id) => Some(id), Ok(id) => Some(Ok(id)),
Err(e) => { Err(e) => Some(Err(e)),
trace_error(&e);
None
}
} }
/* } else { */ /* } else { */
/* continue */ /* continue */
} }
} }
Err(e) => {
debug!("GlobStoreIdIterator error: {:?}", e);
trace_error(&e);
return None
}
} }
} }