From 4b42b3328d0db1f19cf24a1933f82c4b4e33a142 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 28 Aug 2017 10:31:13 +0200 Subject: [PATCH 01/25] Rewrite libimagentrylink::external::Link to be a trait --- lib/entry/libimagentrylink/src/external.rs | 83 +++++++++++----------- 1 file changed, 41 insertions(+), 42 deletions(-) diff --git a/lib/entry/libimagentrylink/src/external.rs b/lib/entry/libimagentrylink/src/external.rs index daed7a12..b908974a 100644 --- a/lib/entry/libimagentrylink/src/external.rs +++ b/lib/entry/libimagentrylink/src/external.rs @@ -35,12 +35,12 @@ use std::collections::BTreeMap; use std::fmt::Debug; use libimagstore::store::Entry; -use libimagstore::store::FileLockEntry; use libimagstore::store::Store; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagstore::toml_ext::TomlValueExt; use libimagutil::debug_result::*; +use libimagerror::into::IntoError; use error::LinkError as LE; use error::LinkErrorKind as LEK; @@ -56,37 +56,32 @@ use url::Url; use crypto::sha1::Sha1; use crypto::digest::Digest; -/// "Link" Type, just an abstraction over `FileLockEntry` to have some convenience internally. -pub struct Link<'a> { - link: FileLockEntry<'a> +pub trait Link { + + fn get_link_uri_from_filelockentry(&self) -> Result>; + + fn get_url(&self) -> Result>; + } -impl<'a> Link<'a> { +impl Link for Entry { - pub fn new(fle: FileLockEntry<'a>) -> Link<'a> { - Link { link: fle } - } - - /// Get a link Url object from a `FileLockEntry`, ignore errors. - fn get_link_uri_from_filelockentry(file: &FileLockEntry<'a>) -> Option { - file.get_header() + fn get_link_uri_from_filelockentry(&self) -> Result> { + self.get_header() .read("imag.content.url") - .ok() + .map_err_into(LEK::EntryHeaderReadError) .and_then(|opt| match opt { Some(Value::String(s)) => { debug!("Found url, parsing: {:?}", s); - Url::parse(&s[..]).ok() + Url::parse(&s[..]).map_err_into(LEK::InvalidUri).map(Some) }, - _ => None + Some(_) => Err(LEK::LinkParserFieldTypeError.into_error()), + None => Ok(None), }) } - pub fn get_url(&self) -> Result> { - let opt = self.link - .get_header() - .read("imag.content.url"); - - match opt { + fn get_url(&self) -> Result> { + match self.get_header().read("imag.content.url") { Ok(Some(Value::String(s))) => { Url::parse(&s[..]) .map(Some) @@ -259,23 +254,32 @@ pub mod iter { type Item = Result; fn next(&mut self) -> Option { - use super::get_external_link_from_file; + use external::Link; - self.0 - .next() - .map(|id| { - debug!("Retrieving entry for id: '{:?}'", id); - self.1 - .retrieve(id.clone()) - .map_err_into(LEK::StoreReadError) - .map_dbg_err(|_| format!("Retrieving entry for id: '{:?}' failed", id)) - .and_then(|f| { - debug!("Store::retrieve({:?}) succeeded", id); - debug!("getting external link from file now"); - get_external_link_from_file(&f) - .map_dbg_err(|e| format!("URL -> Err = {:?}", e)) - }) - }) + loop { + let next = self.0 + .next() + .map(|id| { + debug!("Retrieving entry for id: '{:?}'", id); + self.1 + .retrieve(id.clone()) + .map_err_into(LEK::StoreReadError) + .map_dbg_err(|_| format!("Retrieving entry for id: '{:?}' failed", id)) + .and_then(|f| { + debug!("Store::retrieve({:?}) succeeded", id); + debug!("getting external link from file now"); + f.get_link_uri_from_filelockentry() + .map_dbg_err(|e| format!("URL -> Err = {:?}", e)) + }) + }); + + match next { + Some(Ok(Some(link))) => return Some(Ok(link)), + Some(Ok(None)) => continue, + Some(Err(e)) => return Some(Err(e)), + None => return None + } + } } } @@ -289,11 +293,6 @@ pub fn is_external_link_storeid + Debug>(id: A) -> bool { id.as_ref().local().starts_with("links/external") } -fn get_external_link_from_file(entry: &FileLockEntry) -> Result { - Link::get_link_uri_from_filelockentry(entry) // TODO: Do not hide error by using this function - .ok_or(LE::new(LEK::StoreReadError, None)) -} - /// Implement `ExternalLinker` for `Entry`, hiding the fact that there is no such thing as an external /// link in an entry, but internal links to other entries which serve as external links, as one /// entry in the store can only have one external link. From cf19e0563c3aa2e07263640643da18d65cd49c3f Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 27 Aug 2017 20:38:26 +0200 Subject: [PATCH 02/25] Reorganize functionality in traits --- lib/entry/libimagentryref/Cargo.toml | 1 + lib/entry/libimagentryref/src/lib.rs | 3 + lib/entry/libimagentryref/src/lister.rs | 116 ++---- lib/entry/libimagentryref/src/reference.rs | 460 ++++++--------------- lib/entry/libimagentryref/src/refstore.rs | 286 +++++++++++++ lib/entry/libimagentryref/src/util.rs | 56 +++ 6 files changed, 510 insertions(+), 412 deletions(-) create mode 100644 lib/entry/libimagentryref/src/refstore.rs create mode 100644 lib/entry/libimagentryref/src/util.rs diff --git a/lib/entry/libimagentryref/Cargo.toml b/lib/entry/libimagentryref/Cargo.toml index 7b7763e8..6bc8f38d 100644 --- a/lib/entry/libimagentryref/Cargo.toml +++ b/lib/entry/libimagentryref/Cargo.toml @@ -19,6 +19,7 @@ log = "0.3" rust-crypto = "0.2" semver = "0.5" toml = "^0.4" +toml-query = "0.3.0" version = "2.0.1" walkdir = "1.0.*" diff --git a/lib/entry/libimagentryref/src/lib.rs b/lib/entry/libimagentryref/src/lib.rs index 86c187d0..dc4c10a1 100644 --- a/lib/entry/libimagentryref/src/lib.rs +++ b/lib/entry/libimagentryref/src/lib.rs @@ -36,6 +36,7 @@ extern crate crypto; extern crate itertools; extern crate semver; extern crate toml; +extern crate toml_query; extern crate version; extern crate walkdir; @@ -52,4 +53,6 @@ pub mod hasher; pub mod hashers; pub mod lister; pub mod reference; +pub mod refstore; pub mod result; +mod util; diff --git a/lib/entry/libimagentryref/src/lister.rs b/lib/entry/libimagentryref/src/lister.rs index 662ea9f6..af843327 100644 --- a/lib/entry/libimagentryref/src/lister.rs +++ b/lib/entry/libimagentryref/src/lister.rs @@ -20,13 +20,14 @@ use std::default::Default; use std::io::stdout; use std::io::Write; +use std::ops::Deref; use libimagentrylist::lister::Lister; use libimagentrylist::result::Result; use libimagerror::trace::trace_error; use libimagstore::store::FileLockEntry; -use libimagerror::into::IntoError; use libimagentrylist::error::ListErrorKind as LEK; +use libimagentrylist::error as lerror; use reference::Ref; use error::MapErrInto; @@ -88,15 +89,45 @@ impl Lister for RefLister { debug!("fold({:?}, {:?})", accu, entry); let r = accu.and_then(|_| { debug!("Listing Entry: {:?}", entry); - lister_fn(entry, - self.check_dead, - self.check_changed, - self.check_changed_content, - self.check_changed_permiss) + { + let is_dead = if self.check_dead { + if try!(entry.fs_link_exists()) { + "dead" + } else { + "alive" + } + } else { + "not checked" + }; + + let is_changed = if self.check_changed { + if check_changed(entry.deref()) { "changed" } else { "unchanged" } + } else { + "not checked" + }; + + let is_changed_content = if self.check_changed_content { + if check_changed_content(entry.deref()) { "changed" } else { "unchanged" } + } else { + "not checked" + }; + + let is_changed_permiss = if self.check_changed_permiss { + if check_changed_permiss(entry.deref()) { "changed" } else { "unchanged" } + } else { + "not checked" + }; + + Ok(format!("{} | {} | {} | {} | {} | {}", + is_dead, + is_changed, + is_changed_content, + is_changed_permiss, + entry.get_path_hash().unwrap_or_else(|_| String::from("Cannot get hash")), + entry.get_location())) + } .and_then(|s| { - write!(stdout(), "{}\n", s) - .map_err(Box::new) - .map_err(|e| LEK::FormatError.into_error_with_cause(e)) + lerror::MapErrInto::map_err_into(write!(stdout(), "{}\n", s), LEK::FormatError) }) .map_err_into(REK::RefToDisplayError) }) @@ -104,73 +135,16 @@ impl Lister for RefLister { (r, i + 1) }); debug!("Iterated over {} entries", n); - r.map_err(|e| LEK::FormatError.into_error_with_cause(Box::new(e))) + lerror::MapErrInto::map_err_into(r, LEK::FormatError) } } -fn lister_fn(fle: FileLockEntry, - do_check_dead: bool, - do_check_changed: bool, - do_check_changed_content: bool, - do_check_changed_permiss: bool) -> Result -{ - Ref::from_filelockentry(fle) - .map(|r| { - let is_dead = if do_check_dead { - if check_dead(&r) { "dead" } else { "alive" } - } else { - "not checked" - }; - - let is_changed = if do_check_changed { - if check_changed(&r) { "changed" } else { "unchanged" } - } else { - "not checked" - }; - - let is_changed_content = if do_check_changed_content { - if check_changed_content(&r) { "changed" } else { "unchanged" } - } else { - "not checked" - }; - - let is_changed_permiss = if do_check_changed_permiss { - if check_changed_permiss(&r) { "changed" } else { "unchanged" } - } else { - "not checked" - }; - - format!("{} | {} | {} | {} | {} | {}", - is_dead, - is_changed, - is_changed_content, - is_changed_permiss, - r.get_path_hash().unwrap_or_else(|_| String::from("Cannot get hash")), - r.get_location()) - }) - .map_err(|e| LEK::FormatError.into_error_with_cause(Box::new(e))) -} - -fn check_dead(r: &Ref) -> bool { - match r.fs_link_exists() { - Ok(b) => b, - Err(e) => { - warn!("Could not check whether the ref {} exists on the FS:", r); - trace_error(&e); - - // We continue here and tell the callee that this reference is dead, what is kind of - // true actually, as we might not have access to it right now - true - }, - } -} - -fn check_changed(r: &Ref) -> bool { +fn check_changed(r: &R) -> bool { check_changed_content(r) && check_changed_permiss(r) } -fn check_changed_content(r: &Ref) -> bool { +fn check_changed_content(r: &R) -> bool { let eq = r.get_current_hash() .and_then(|hash| r.get_stored_hash().map(|stored| (hash, stored))) .map(|(hash, stored)| hash == stored); @@ -178,7 +152,7 @@ fn check_changed_content(r: &Ref) -> bool { match eq { Ok(eq) => eq, Err(e) => { - warn!("Could not check whether the ref {} changed on the FS:", r); + warn!("Could not check whether the ref changed on the FS"); trace_error(&e); // We continue here and tell the callee that this reference is unchanged @@ -187,7 +161,7 @@ fn check_changed_content(r: &Ref) -> bool { } } -fn check_changed_permiss(_: &Ref) -> bool { +fn check_changed_permiss(_: &R) -> bool { warn!("Permission changes tracking not supported yet."); false } diff --git a/lib/entry/libimagentryref/src/reference.rs b/lib/entry/libimagentryref/src/reference.rs index e860452e..5263c765 100644 --- a/lib/entry/libimagentryref/src/reference.rs +++ b/lib/entry/libimagentryref/src/reference.rs @@ -21,18 +21,10 @@ //! files outside of the imag store. use std::path::PathBuf; -use std::ops::Deref; -use std::ops::DerefMut; -use std::collections::BTreeMap; use std::fs::File; -use std::fmt::{Display, Error as FmtError, Formatter}; use std::fs::Permissions; -use std::result::Result as RResult; -use libimagstore::store::FileLockEntry; -use libimagstore::storeid::StoreId; -use libimagstore::storeid::IntoStoreId; -use libimagstore::store::Store; +use libimagstore::store::Entry; use libimagstore::toml_ext::TomlValueExt; use libimagerror::into::IntoError; @@ -40,218 +32,92 @@ use toml::Value; use error::RefErrorKind as REK; use error::MapErrInto; -use flags::RefFlags; use result::Result; use hasher::*; -use module_path::ModuleEntryPath; -#[derive(Debug)] -pub struct Ref<'a>(FileLockEntry<'a>); - -impl<'a> Ref<'a> { - - /// Try to build a Ref object based on an existing FileLockEntry object - pub fn from_filelockentry(fle: FileLockEntry<'a>) -> Result> { - Ref::read_reference(&fle).map(|_| Ref(fle)) - } - - /// Try to get `si` as Ref object from the store - pub fn get(store: &'a Store, si: StoreId) -> Result> { - match store.get(si) { - Err(e) => return Err(REK::StoreReadError.into_error_with_cause(Box::new(e))), - Ok(None) => return Err(REK::RefNotInStore.into_error()), - Ok(Some(fle)) => Ref::from_filelockentry(fle), - } - } - - /// Get a Ref object from the store by hash. - /// - /// Returns None if the hash cannot be found. - pub fn get_by_hash(store: &'a Store, hash: String) -> Result>> { - ModuleEntryPath::new(hash) - .into_storeid() - .and_then(|id| store.get(id)) - .map(|opt_fle| opt_fle.map(|fle| Ref(fle))) - .map_err(Box::new) - .map_err(|e| REK::StoreReadError.into_error_with_cause(e)) - } - - /// Delete a ref by hash - /// - /// If the returned Result contains an error, the ref might not be deleted. - pub fn delete_by_hash(store: &'a Store, hash: String) -> Result<()> { - ModuleEntryPath::new(hash) - .into_storeid() - .and_then(|id| store.delete(id)) - .map_err(Box::new) - .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) - } - - fn read_reference(fle: &FileLockEntry<'a>) -> Result { - match fle.get_header().read("ref.path") { - Ok(Some(Value::String(s))) => Ok(PathBuf::from(s)), - Ok(Some(_)) => Err(REK::HeaderTypeError.into_error()), - Ok(None) => Err(REK::HeaderFieldMissingError.into_error()), - Err(e) => Err(REK::StoreReadError.into_error_with_cause(Box::new(e))), - } - } - - pub fn create_with_hasher(store: &'a Store, pb: PathBuf, flags: RefFlags, mut h: H) - -> Result> - { - if !pb.exists() { - return Err(REK::RefTargetDoesNotExist.into_error()); - } - if flags.get_content_hashing() && pb.is_dir() { - return Err(REK::RefTargetCannotBeHashed.into_error()); - } - - let (mut fle, content_hash, permissions, canonical_path) = { // scope to be able to fold - try!(File::open(pb.clone()) - .map_err(Box::new) - .map_err(|e| REK::RefTargetFileCannotBeOpened.into_error_with_cause(e)) - - // If we were able to open this file, - // we hash the contents of the file and return (file, hash) - .and_then(|mut file| { - let opt_contenthash = if flags.get_content_hashing() { - Some(try!(h.create_hash(&pb, &mut file))) - } else { - None - }; - - Ok((file, opt_contenthash)) - }) - - // and then we get the permissions if we have to - // and return (file, content hash, permissions) - .and_then(|(file, opt_contenthash)| { - let opt_permissions = if flags.get_permission_tracking() { - Some(try!(file - .metadata() - .map(|md| md.permissions()) - .map_err(Box::new) - .map_err(|e| REK::RefTargetCannotReadPermissions.into_error_with_cause(e)) - )) - } else { - None - }; - - Ok((opt_contenthash, opt_permissions)) - }) - - // and then we try to canonicalize the PathBuf, because we want to store a - // canonicalized path - // and return (file, content hash, permissions, canonicalized path) - .and_then(|(opt_contenthash, opt_permissions)| { - pb.canonicalize() - .map(|can| (opt_contenthash, opt_permissions, can)) - // if PathBuf::canonicalize() failed, build an error from the return value - .map_err(|e| REK::PathCanonicalizationError.into_error_with_cause(Box::new(e))) - }) - - // and then we hash the canonicalized path - // and return (file, content hash, permissions, canonicalized path, path hash) - .and_then(|(opt_contenthash, opt_permissions, can)| { - let path_hash = try!(Ref::hash_path(&can) - .map_err(Box::new) - .map_err(|e| REK::PathHashingError.into_error_with_cause(e)) - ); - - Ok((opt_contenthash, opt_permissions, can, path_hash)) - }) - - // and then we convert the PathBuf of the canonicalized path to a String to be able - // to save it in the Ref FileLockEntry obj - // and return - // (file, content hash, permissions, canonicalized path as String, path hash) - .and_then(|(opt_conhash, opt_perm, can, path_hash)| { - match can.to_str().map(String::from) { - // UTF convert error in PathBuf::to_str(), - None => Err(REK::PathUTF8Error.into_error()), - Some(can) => Ok((opt_conhash, opt_perm, can, path_hash)) - } - }) - - // and then we create the FileLockEntry in the Store - // and return (filelockentry, content hash, permissions, canonicalized path) - .and_then(|(opt_conhash, opt_perm, can, path_hash)| { - let fle = try!(store - .create(ModuleEntryPath::new(path_hash)) - .map_err(Box::new) - .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) - ); - - Ok((fle, opt_conhash, opt_perm, can)) - }) - ) - }; - - for tpl in [ - Some((String::from("ref"), Value::Table(BTreeMap::new()))), - Some((String::from("ref.permissions"), Value::Table(BTreeMap::new()))), - Some((String::from("ref.path"), Value::String(canonical_path))), - Some((String::from("ref.content_hash"), Value::Table(BTreeMap::new()))), - - content_hash.map(|hash| { - (format!("ref.content_hash.{}", h.hash_name()), Value::String(hash)) - }), - permissions.map(|p| { - (String::from("ref.permissions.ro"), Value::Boolean(p.readonly())) - }), - ].into_iter() - { - match tpl { - &Some((ref s, ref v)) => { - match fle.get_header_mut().insert(s, v.clone()) { - Ok(false) => { - let e = REK::HeaderFieldAlreadyExistsError.into_error(); - let e = Box::new(e); - let e = REK::HeaderFieldWriteError.into_error_with_cause(e); - return Err(e); - }, - Err(e) => { - let e = Box::new(e); - let e = REK::HeaderFieldWriteError.into_error_with_cause(e); - return Err(e); - }, - _ => (), - } - } - &None => { - debug!("Not going to insert."); - } - } - } - - Ok(Ref(fle)) - } - - /// Create a Ref object which refers to `pb` - pub fn create(store: &'a Store, pb: PathBuf, flags: RefFlags) -> Result> { - Ref::create_with_hasher(store, pb, flags, DefaultHasher::new()) - } - - /// Creates a Hash from a PathBuf by making the PathBuf absolute and then running a hash - /// algorithm on it - fn hash_path(pb: &PathBuf) -> Result { - use crypto::sha1::Sha1; - use crypto::digest::Digest; - - match pb.to_str() { - Some(s) => { - let mut hasher = Sha1::new(); - hasher.input_str(s); - Ok(hasher.result_str()) - }, - None => return Err(REK::PathUTF8Error.into_error()), - } - } +pub trait Ref { /// Get the hash from the path of the ref - pub fn get_path_hash(&self) -> Result { - self.0 - .get_location() + fn get_path_hash(&self) -> Result; + + /// Get the hash of the link target which is stored in the ref object + fn get_stored_hash(&self) -> Result; + + /// Get the hahs of the link target which is stored in the ref object, which is hashed with a + /// custom Hasher instance. + fn get_stored_hash_with_hasher(&self, h: &H) -> Result; + + /// Get the hash of the link target by reading the link target and hashing the contents + fn get_current_hash(&self) -> Result; + + /// Get the hash of the link target by reading the link target and hashing the contents with the + /// custom hasher + fn get_current_hash_with_hasher(&self, h: H) -> Result; + + /// check whether the pointer the Ref represents still points to a file which exists + fn fs_link_exists(&self) -> Result; + + /// Alias for `r.fs_link_exists() && r.deref().is_file()` + fn is_ref_to_file(&self) -> Result; + + /// Alias for `r.fs_link_exists() && r.deref().is_dir()` + fn is_ref_to_dir(&self) -> Result; + + /// Alias for `!Ref::fs_link_exists()` + fn is_dangling(&self) -> Result; + + /// check whether the pointer the Ref represents is valid + /// This includes: + /// - Hashsum of the file is still the same as stored in the Ref + /// - file permissions are still valid + fn fs_link_valid(&self) -> Result; + + /// Check whether the file permissions of the referenced file are equal to the stored + /// permissions + fn fs_link_valid_permissions(&self) -> Result; + + /// Check whether the Hashsum of the referenced file is equal to the stored hashsum + fn fs_link_valid_hash(&self) -> Result; + + /// Update the Ref by re-checking the file from FS + /// This errors if the file is not present or cannot be read() + fn update_ref(&mut self) -> Result<()>; + + /// Update the Ref by re-checking the file from FS using the passed Hasher instance + /// This errors if the file is not present or cannot be read() + fn update_ref_with_hasher(&mut self, h: &H) -> Result<()>; + + /// Get the path of the file which is reffered to by this Ref + fn fs_file(&self) -> Result; + + /// Re-find a referenced file + /// + /// This function tries to re-find a ref by searching all directories in `search_roots` recursively + /// for a file which matches the hash of the Ref. + /// + /// If `search_roots` is `None`, it starts at the filesystem root `/`. + /// + /// If the target cannot be found, this yields a RefTargetDoesNotExist error kind. + /// + /// # Warning + /// + /// This option causes heavy I/O as it recursively searches the Filesystem. + fn refind(&self, search_roots: Option>) -> Result; + + /// See documentation of `Ref::refind()` + fn refind_with_hasher(&self, search_roots: Option>, h: H) + -> Result; + + /// Get the permissions of the file which are present + fn get_current_permissions(&self) -> Result; +} + + +impl Ref for Entry { + + /// Get the hash from the path of the ref + fn get_path_hash(&self) -> Result { + self.get_location() .clone() .into_pathbuf() .map_err_into(REK::StoreIdError) @@ -265,14 +131,14 @@ impl<'a> Ref<'a> { } /// Get the hash of the link target which is stored in the ref object - pub fn get_stored_hash(&self) -> Result { + fn get_stored_hash(&self) -> Result { self.get_stored_hash_with_hasher(&DefaultHasher::new()) } /// Get the hahs of the link target which is stored in the ref object, which is hashed with a /// custom Hasher instance. - pub fn get_stored_hash_with_hasher(&self, h: &H) -> Result { - match self.0.get_header().read(&format!("ref.content_hash.{}", h.hash_name())[..]) { + fn get_stored_hash_with_hasher(&self, h: &H) -> Result { + match self.get_header().read(&format!("ref.content_hash.{}", h.hash_name())[..]) { // content hash stored... Ok(Some(Value::String(s))) => Ok(s), @@ -288,13 +154,13 @@ impl<'a> Ref<'a> { } /// Get the hash of the link target by reading the link target and hashing the contents - pub fn get_current_hash(&self) -> Result { + fn get_current_hash(&self) -> Result { self.get_current_hash_with_hasher(DefaultHasher::new()) } /// Get the hash of the link target by reading the link target and hashing the contents with the /// custom hasher - pub fn get_current_hash_with_hasher(&self, mut h: H) -> Result { + fn get_current_hash_with_hasher(&self, mut h: H) -> Result { self.fs_file() .and_then(|pb| { File::open(pb.clone()) @@ -305,40 +171,23 @@ impl<'a> Ref<'a> { .and_then(|(path, mut file)| h.create_hash(&path, &mut file)) } - /// Get the permissions of the file which are present - fn get_current_permissions(&self) -> Result { - self.fs_file() - .and_then(|pb| { - File::open(pb) - .map_err(Box::new) - .map_err(|e| REK::HeaderFieldReadError.into_error_with_cause(e)) - }) - .and_then(|file| { - file - .metadata() - .map(|md| md.permissions()) - .map_err(Box::new) - .map_err(|e| REK::RefTargetCannotReadPermissions.into_error_with_cause(e)) - }) - } - /// check whether the pointer the Ref represents still points to a file which exists - pub fn fs_link_exists(&self) -> Result { + fn fs_link_exists(&self) -> Result { self.fs_file().map(|pathbuf| pathbuf.exists()) } /// Alias for `r.fs_link_exists() && r.deref().is_file()` - pub fn is_ref_to_file(&self) -> Result { + fn is_ref_to_file(&self) -> Result { self.fs_file().map(|pathbuf| pathbuf.is_file()) } /// Alias for `r.fs_link_exists() && r.deref().is_dir()` - pub fn is_ref_to_dir(&self) -> Result { + fn is_ref_to_dir(&self) -> Result { self.fs_file().map(|pathbuf| pathbuf.is_dir()) } /// Alias for `!Ref::fs_link_exists()` - pub fn is_dangling(&self) -> Result { + fn is_dangling(&self) -> Result { self.fs_link_exists().map(|b| !b) } @@ -346,7 +195,7 @@ impl<'a> Ref<'a> { /// This includes: /// - Hashsum of the file is still the same as stored in the Ref /// - file permissions are still valid - pub fn fs_link_valid(&self) -> Result { + fn fs_link_valid(&self) -> Result { match (self.fs_link_valid_permissions(), self.fs_link_valid_hash()) { (Ok(true) , Ok(true)) => Ok(true), (Ok(_) , Ok(_)) => Ok(false), @@ -357,8 +206,8 @@ impl<'a> Ref<'a> { /// Check whether the file permissions of the referenced file are equal to the stored /// permissions - pub fn fs_link_valid_permissions(&self) -> Result { - self.0 + fn fs_link_valid_permissions(&self) -> Result { + self .get_header() .read("ref.permissions.ro") .map_err(Box::new) @@ -376,7 +225,7 @@ impl<'a> Ref<'a> { } /// Check whether the Hashsum of the referenced file is equal to the stored hashsum - pub fn fs_link_valid_hash(&self) -> Result { + fn fs_link_valid_hash(&self) -> Result { let stored_hash = try!(self.get_stored_hash()); let current_hash = try!(self.get_current_hash()); Ok(stored_hash == current_hash) @@ -384,24 +233,24 @@ impl<'a> Ref<'a> { /// Update the Ref by re-checking the file from FS /// This errors if the file is not present or cannot be read() - pub fn update_ref(&mut self) -> Result<()> { + fn update_ref(&mut self) -> Result<()> { self.update_ref_with_hasher(&DefaultHasher::new()) } /// Update the Ref by re-checking the file from FS using the passed Hasher instance /// This errors if the file is not present or cannot be read() - pub fn update_ref_with_hasher(&mut self, h: &H) -> Result<()> { + fn update_ref_with_hasher(&mut self, h: &H) -> Result<()> { let current_hash = try!(self.get_current_hash()); // uses the default hasher let current_perm = try!(self.get_current_permissions()); - try!(self.0 + try!(self .get_header_mut() .set("ref.permissions.ro", Value::Boolean(current_perm.readonly())) .map_err(Box::new) .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) ); - try!(self.0 + try!(self .get_header_mut() .set(&format!("ref.content_hash.{}", h.hash_name())[..], Value::String(current_hash)) .map_err(Box::new) @@ -412,8 +261,8 @@ impl<'a> Ref<'a> { } /// Get the path of the file which is reffered to by this Ref - pub fn fs_file(&self) -> Result { - match self.0.get_header().read("ref.path") { + fn fs_file(&self) -> Result { + match self.get_header().read("ref.path") { Ok(Some(Value::String(ref s))) => Ok(PathBuf::from(s)), Ok(Some(_)) => Err(REK::HeaderTypeError.into_error()), Ok(None) => Err(REK::HeaderFieldMissingError.into_error()), @@ -421,56 +270,6 @@ impl<'a> Ref<'a> { } } - /// Check whether there is a reference to the file at `pb` - pub fn exists(store: &Store, pb: PathBuf) -> Result { - pb.canonicalize() - .map_err(Box::new) - .map_err(|e| REK::PathCanonicalizationError.into_error_with_cause(e)) - .and_then(|can| { - Ref::hash_path(&can) - .map_err(Box::new) - .map_err(|e| REK::PathHashingError.into_error_with_cause(e)) - }) - .and_then(|hash| { - store.retrieve_for_module("ref").map(|iter| (hash, iter)) - .map_err(Box::new) - .map_err(|e| REK::StoreReadError.into_error_with_cause(e)) - }) - .and_then(|(hash, possible_refs)| { - // This is kind of a manual Iterator::filter() call what we do here, but with the - // actual ::filter method we cannot return the error in a nice way, so we do it - // manually here. If you can come up with a better version of this, feel free to - // take this note as a todo. - for r in possible_refs { - let contains_hash = try!(r.to_str() - .map_err_into(REK::TypeConversionError) - .map(|s| s.contains(&hash[..]))); - - if !contains_hash { - continue; - } - - match store.get(r) { - Ok(Some(fle)) => { - if Ref::read_reference(&fle).map(|path| path == pb).unwrap_or(false) { - return Ok(true) - } - }, - - Ok(None) => { // Something weird just happened - return Err(REK::StoreReadError.into_error()); - }, - - Err(e) => { - return Err(REK::StoreReadError.into_error_with_cause(Box::new(e))); - }, - } - } - - Ok(false) - }) - } - /// Re-find a referenced file /// /// This function tries to re-find a ref by searching all directories in `search_roots` recursively @@ -483,11 +282,12 @@ impl<'a> Ref<'a> { /// # Warning /// /// This option causes heavy I/O as it recursively searches the Filesystem. - pub fn refind(&self, search_roots: Option>) -> Result { + fn refind(&self, search_roots: Option>) -> Result { self.refind_with_hasher(search_roots, DefaultHasher::new()) } - pub fn refind_with_hasher(&self, search_roots: Option>, mut h: H) + /// See documentation of `Ref::refind()` + fn refind_with_hasher(&self, search_roots: Option>, mut h: H) -> Result { use itertools::Itertools; @@ -534,43 +334,21 @@ impl<'a> Ref<'a> { }) } -} - -impl<'a> Deref for Ref<'a> { - type Target = FileLockEntry<'a>; - - fn deref(&self) -> &FileLockEntry<'a> { - &self.0 - } - -} - -impl<'a> DerefMut for Ref<'a> { - - fn deref_mut(&mut self) -> &mut FileLockEntry<'a> { - &mut self.0 - } - -} - -impl<'a> Display for Ref<'a> { - - fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FmtError> { - let path = self.fs_file() - .map(|pb| String::from(pb.to_str().unwrap_or(""))) - .unwrap_or(String::from("Could not read Path from reference object")); - - let hash = self.get_stored_hash().unwrap_or(String::from("")); - - write!(fmt, "Ref({} -> {})", hash, path) - } - -} - -impl<'a> Into> for Ref<'a> { - - fn into(self) -> FileLockEntry<'a> { - self.0 + /// Get the permissions of the file which are present + fn get_current_permissions(&self) -> Result { + self.fs_file() + .and_then(|pb| { + File::open(pb) + .map_err(Box::new) + .map_err(|e| REK::HeaderFieldReadError.into_error_with_cause(e)) + }) + .and_then(|file| { + file + .metadata() + .map(|md| md.permissions()) + .map_err(Box::new) + .map_err(|e| REK::RefTargetCannotReadPermissions.into_error_with_cause(e)) + }) } } diff --git a/lib/entry/libimagentryref/src/refstore.rs b/lib/entry/libimagentryref/src/refstore.rs new file mode 100644 index 00000000..5d4e1243 --- /dev/null +++ b/lib/entry/libimagentryref/src/refstore.rs @@ -0,0 +1,286 @@ +// +// imag - the personal information management suite for the commandline +// Copyright (C) 2015, 2016 Matthias Beyer and contributors +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; version +// 2.1 of the License. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +use std::path::PathBuf; +use std::collections::BTreeMap; +use std::fs::File; + +use libimagstore::store::FileLockEntry; +use libimagstore::storeid::StoreId; +use libimagstore::storeid::IntoStoreId; +use libimagstore::store::Store; +use libimagstore::toml_ext::TomlValueExt; +use libimagerror::into::IntoError; + +use toml::Value; + +use error::RefErrorKind as REK; +use error::MapErrInto; +use flags::RefFlags; +use result::Result; +use hasher::*; +use module_path::ModuleEntryPath; +use util::*; + +pub trait RefStore { + + /// Check whether there is a reference to the file at `pb` + fn exists(&self, pb: PathBuf) -> Result; + + /// Try to get `si` as Ref object from the store + fn get<'a>(&'a self, si: StoreId) -> Result>; + + /// Get a Ref object from the store by hash. + /// + /// Returns None if the hash cannot be found. + fn get_by_hash<'a>(&'a self, hash: String) -> Result>>; + + /// Delete a ref by hash + /// + /// If the returned Result contains an error, the ref might not be deleted. + fn delete_by_hash(&self, hash: String) -> Result<()>; + + /// Create a Ref object which refers to `pb` + fn create<'a>(&'a self, pb: PathBuf, flags: RefFlags) -> Result>; + + fn create_with_hasher<'a, H: Hasher>(&'a self, pb: PathBuf, flags: RefFlags, h: H) + -> Result>; + +} + +impl RefStore for Store { + + /// Check whether there is a reference to the file at `pb` + fn exists(&self, pb: PathBuf) -> Result { + pb.canonicalize() + .map_err(Box::new) + .map_err(|e| REK::PathCanonicalizationError.into_error_with_cause(e)) + .and_then(|can| { + hash_path(&can) + .map_err(Box::new) + .map_err(|e| REK::PathHashingError.into_error_with_cause(e)) + }) + .and_then(|hash| { + self.retrieve_for_module("ref").map(|iter| (hash, iter)) + .map_err(Box::new) + .map_err(|e| REK::StoreReadError.into_error_with_cause(e)) + }) + .and_then(|(hash, possible_refs)| { + // This is kind of a manual Iterator::filter() call what we do here, but with the + // actual ::filter method we cannot return the error in a nice way, so we do it + // manually here. If you can come up with a better version of this, feel free to + // take this note as a todo. + for r in possible_refs { + let contains_hash = try!(r.to_str() + .map_err_into(REK::TypeConversionError) + .map(|s| s.contains(&hash[..]))); + + if !contains_hash { + continue; + } + + match self.get(r) { + Ok(Some(fle)) => { + if read_reference(&fle).map(|path| path == pb).unwrap_or(false) { + return Ok(true) + } + }, + + Ok(None) => { // Something weird just happened + return Err(REK::StoreReadError.into_error()); + }, + + Err(e) => { + return Err(REK::StoreReadError.into_error_with_cause(Box::new(e))); + }, + } + } + + Ok(false) + }) + } + + /// Try to get `si` as Ref object from the store + fn get<'a>(&'a self, si: StoreId) -> Result> { + match self.get(si) { + Err(e) => return Err(REK::StoreReadError.into_error_with_cause(Box::new(e))), + Ok(None) => return Err(REK::RefNotInStore.into_error()), + Ok(Some(fle)) => Ok(fle), + } + } + + /// Get a Ref object from the store by hash. + /// + /// Returns None if the hash cannot be found. + fn get_by_hash<'a>(&'a self, hash: String) -> Result>> { + ModuleEntryPath::new(hash) + .into_storeid() + .and_then(|id| self.get(id)) + .map_err(Box::new) + .map_err(|e| REK::StoreReadError.into_error_with_cause(e)) + } + + /// Delete a ref by hash + /// + /// If the returned Result contains an error, the ref might not be deleted. + fn delete_by_hash(&self, hash: String) -> Result<()> { + ModuleEntryPath::new(hash) + .into_storeid() + .and_then(|id| self.delete(id)) + .map_err(Box::new) + .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) + } + + /// Create a Ref object which refers to `pb` + fn create<'a>(&'a self, pb: PathBuf, flags: RefFlags) -> Result> { + self.create_with_hasher(pb, flags, DefaultHasher::new()) + } + + fn create_with_hasher<'a, H: Hasher>(&'a self, pb: PathBuf, flags: RefFlags, mut h: H) + -> Result> + { + if !pb.exists() { + return Err(REK::RefTargetDoesNotExist.into_error()); + } + if flags.get_content_hashing() && pb.is_dir() { + return Err(REK::RefTargetCannotBeHashed.into_error()); + } + + let (mut fle, content_hash, permissions, canonical_path) = { // scope to be able to fold + try!(File::open(pb.clone()) + .map_err(Box::new) + .map_err(|e| REK::RefTargetFileCannotBeOpened.into_error_with_cause(e)) + + // If we were able to open this file, + // we hash the contents of the file and return (file, hash) + .and_then(|mut file| { + let opt_contenthash = if flags.get_content_hashing() { + Some(try!(h.create_hash(&pb, &mut file))) + } else { + None + }; + + Ok((file, opt_contenthash)) + }) + + // and then we get the permissions if we have to + // and return (file, content hash, permissions) + .and_then(|(file, opt_contenthash)| { + let opt_permissions = if flags.get_permission_tracking() { + Some(try!(file + .metadata() + .map(|md| md.permissions()) + .map_err(Box::new) + .map_err(|e| REK::RefTargetCannotReadPermissions.into_error_with_cause(e)) + )) + } else { + None + }; + + Ok((opt_contenthash, opt_permissions)) + }) + + // and then we try to canonicalize the PathBuf, because we want to store a + // canonicalized path + // and return (file, content hash, permissions, canonicalized path) + .and_then(|(opt_contenthash, opt_permissions)| { + pb.canonicalize() + .map(|can| (opt_contenthash, opt_permissions, can)) + // if PathBuf::canonicalize() failed, build an error from the return value + .map_err(|e| REK::PathCanonicalizationError.into_error_with_cause(Box::new(e))) + }) + + // and then we hash the canonicalized path + // and return (file, content hash, permissions, canonicalized path, path hash) + .and_then(|(opt_contenthash, opt_permissions, can)| { + let path_hash = try!(hash_path(&can) + .map_err(Box::new) + .map_err(|e| REK::PathHashingError.into_error_with_cause(e)) + ); + + Ok((opt_contenthash, opt_permissions, can, path_hash)) + }) + + // and then we convert the PathBuf of the canonicalized path to a String to be able + // to save it in the Ref FileLockEntry obj + // and return + // (file, content hash, permissions, canonicalized path as String, path hash) + .and_then(|(opt_conhash, opt_perm, can, path_hash)| { + match can.to_str().map(String::from) { + // UTF convert error in PathBuf::to_str(), + None => Err(REK::PathUTF8Error.into_error()), + Some(can) => Ok((opt_conhash, opt_perm, can, path_hash)) + } + }) + + // and then we create the FileLockEntry in the Store + // and return (filelockentry, content hash, permissions, canonicalized path) + .and_then(|(opt_conhash, opt_perm, can, path_hash)| { + let fle = try!(self + .create(ModuleEntryPath::new(path_hash)) + .map_err(Box::new) + .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) + ); + + Ok((fle, opt_conhash, opt_perm, can)) + }) + ) + }; + + for tpl in [ + Some((String::from("ref"), Value::Table(BTreeMap::new()))), + Some((String::from("ref.permissions"), Value::Table(BTreeMap::new()))), + Some((String::from("ref.path"), Value::String(canonical_path))), + Some((String::from("ref.content_hash"), Value::Table(BTreeMap::new()))), + + content_hash.map(|hash| { + (format!("ref.content_hash.{}", h.hash_name()), Value::String(hash)) + }), + permissions.map(|p| { + (String::from("ref.permissions.ro"), Value::Boolean(p.readonly())) + }), + ].into_iter() + { + match tpl { + &Some((ref s, ref v)) => { + match fle.get_header_mut().insert(s, v.clone()) { + Ok(false) => { + let e = REK::HeaderFieldAlreadyExistsError.into_error(); + let e = Box::new(e); + let e = REK::HeaderFieldWriteError.into_error_with_cause(e); + return Err(e); + }, + Err(e) => { + let e = Box::new(e); + let e = REK::HeaderFieldWriteError.into_error_with_cause(e); + return Err(e); + }, + _ => (), + } + } + &None => { + debug!("Not going to insert."); + } + } + } + + Ok(fle) + } + +} diff --git a/lib/entry/libimagentryref/src/util.rs b/lib/entry/libimagentryref/src/util.rs new file mode 100644 index 00000000..7f9a4730 --- /dev/null +++ b/lib/entry/libimagentryref/src/util.rs @@ -0,0 +1,56 @@ +// +// imag - the personal information management suite for the commandline +// Copyright (C) 2015, 2016 Matthias Beyer and contributors +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; version +// 2.1 of the License. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +use std::path::PathBuf; + +use error::RefErrorKind as REK; +use result::Result; + +use libimagstore::store::Entry; +use libimagerror::into::IntoError; + +use toml::Value; +use toml_query::read::TomlValueReadExt; + +/// Creates a Hash from a PathBuf by making the PathBuf absolute and then running a hash +/// algorithm on it +pub fn hash_path(pb: &PathBuf) -> Result { + use crypto::sha1::Sha1; + use crypto::digest::Digest; + + match pb.to_str() { + Some(s) => { + let mut hasher = Sha1::new(); + hasher.input_str(s); + Ok(hasher.result_str()) + }, + None => return Err(REK::PathUTF8Error.into_error()), + } +} + +/// Read the reference from a file +pub fn read_reference(refentry: &Entry) -> Result { + match refentry.get_header().read("ref.path") { + Ok(Some(&Value::String(ref s))) => Ok(PathBuf::from(s)), + Ok(Some(_)) => Err(REK::HeaderTypeError.into_error()), + Ok(None) => Err(REK::HeaderFieldMissingError.into_error()), + Err(e) => Err(REK::StoreReadError.into_error_with_cause(Box::new(e))), + } +} + From d58b97fdf1cf979720a01f8d23f3ce0c59c4eafc Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sun, 27 Aug 2017 21:08:23 +0200 Subject: [PATCH 03/25] Light refactoring Use .map_err_into() instead of manual wrapping, some boilerplate minimized. --- .../libimagentryref/src/hashers/nbytes.rs | 11 +--- lib/entry/libimagentryref/src/reference.rs | 37 +++-------- lib/entry/libimagentryref/src/refstore.rs | 63 +++++++------------ 3 files changed, 34 insertions(+), 77 deletions(-) diff --git a/lib/entry/libimagentryref/src/hashers/nbytes.rs b/lib/entry/libimagentryref/src/hashers/nbytes.rs index 995e3dd8..5d190840 100644 --- a/lib/entry/libimagentryref/src/hashers/nbytes.rs +++ b/lib/entry/libimagentryref/src/hashers/nbytes.rs @@ -24,8 +24,6 @@ use std::result::Result as RResult; use crypto::sha1::Sha1; use crypto::digest::Digest; -use libimagerror::into::IntoError; - use hasher::Hasher; use result::Result; use error::RefErrorKind as REK; @@ -54,16 +52,13 @@ impl Hasher for NBytesHasher { } fn create_hash(&mut self, _: &PathBuf, contents: &mut R) -> Result { - let s = contents + let s = try!(contents .bytes() .take(self.n) .collect::, _>>() .map_err_into(REK::IOError) - .and_then(|v| String::from_utf8(v).map_err_into(REK::IOError)) - .map_err(Box::new) - .map_err(|e| REK::UTF8Error.into_error_with_cause(e)) - .map_err_into(REK::IOError); - self.hasher.input_str(&try!(s)[..]); + .and_then(|v| String::from_utf8(v).map_err_into(REK::UTF8Error))); + self.hasher.input_str(&s[..]); Ok(self.hasher.result_str()) } diff --git a/lib/entry/libimagentryref/src/reference.rs b/lib/entry/libimagentryref/src/reference.rs index 5263c765..0af07a2a 100644 --- a/lib/entry/libimagentryref/src/reference.rs +++ b/lib/entry/libimagentryref/src/reference.rs @@ -162,12 +162,7 @@ impl Ref for Entry { /// custom hasher fn get_current_hash_with_hasher(&self, mut h: H) -> Result { self.fs_file() - .and_then(|pb| { - File::open(pb.clone()) - .map(|f| (pb, f)) - .map_err(Box::new) - .map_err(|e| REK::IOError.into_error_with_cause(e)) - }) + .and_then(|pb| File::open(pb.clone()).map(|f| (pb, f)).map_err_into(REK::IOError)) .and_then(|(path, mut file)| h.create_hash(&path, &mut file)) } @@ -210,8 +205,7 @@ impl Ref for Entry { self .get_header() .read("ref.permissions.ro") - .map_err(Box::new) - .map_err(|e| REK::HeaderFieldReadError.into_error_with_cause(e)) + .map_err_into(REK::HeaderFieldReadError) .and_then(|ro| { match ro { Some(Value::Boolean(b)) => Ok(b), @@ -220,8 +214,7 @@ impl Ref for Entry { } }) .and_then(|ro| self.get_current_permissions().map(|perm| ro == perm.readonly())) - .map_err(Box::new) - .map_err(|e| REK::RefTargetCannotReadPermissions.into_error_with_cause(e)) + .map_err_into(REK::RefTargetCannotReadPermissions) } /// Check whether the Hashsum of the referenced file is equal to the stored hashsum @@ -246,15 +239,13 @@ impl Ref for Entry { try!(self .get_header_mut() .set("ref.permissions.ro", Value::Boolean(current_perm.readonly())) - .map_err(Box::new) - .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) + .map_err_into(REK::StoreWriteError) ); try!(self .get_header_mut() .set(&format!("ref.content_hash.{}", h.hash_name())[..], Value::String(current_hash)) - .map_err(Box::new) - .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) + .map_err_into(REK::StoreWriteError) ); Ok(()) @@ -304,13 +295,11 @@ impl Ref for Entry { .into_iter() .map(|entry| { entry - .map_err(Box::new) - .map_err(|e| REK::IOError.into_error_with_cause(e)) + .map_err_into(REK::IOError) .and_then(|entry| { let pb = PathBuf::from(entry.path()); File::open(entry.path()) - .map_err(Box::new) - .map_err(|e| REK::IOError.into_error_with_cause(e)) + .map_err_into(REK::IOError) .map(|f| (pb, f)) }) .and_then(|(p, mut f)| h.create_hash(&p, &mut f).map(|h| (p, h))) @@ -321,8 +310,7 @@ impl Ref for Entry { None } }) - .map_err(Box::new) - .map_err(|e| REK::IOError.into_error_with_cause(e)) + .map_err_into(REK::IOError) }) .filter_map(|e| e.ok()) .filter_map(|e| e) @@ -337,17 +325,12 @@ impl Ref for Entry { /// Get the permissions of the file which are present fn get_current_permissions(&self) -> Result { self.fs_file() - .and_then(|pb| { - File::open(pb) - .map_err(Box::new) - .map_err(|e| REK::HeaderFieldReadError.into_error_with_cause(e)) - }) + .and_then(|pb| File::open(pb).map_err_into(REK::HeaderFieldReadError)) .and_then(|file| { file .metadata() .map(|md| md.permissions()) - .map_err(Box::new) - .map_err(|e| REK::RefTargetCannotReadPermissions.into_error_with_cause(e)) + .map_err_into(REK::RefTargetCannotReadPermissions) }) } diff --git a/lib/entry/libimagentryref/src/refstore.rs b/lib/entry/libimagentryref/src/refstore.rs index 5d4e1243..e486d67a 100644 --- a/lib/entry/libimagentryref/src/refstore.rs +++ b/lib/entry/libimagentryref/src/refstore.rs @@ -69,17 +69,13 @@ impl RefStore for Store { /// Check whether there is a reference to the file at `pb` fn exists(&self, pb: PathBuf) -> Result { pb.canonicalize() - .map_err(Box::new) - .map_err(|e| REK::PathCanonicalizationError.into_error_with_cause(e)) - .and_then(|can| { - hash_path(&can) - .map_err(Box::new) - .map_err(|e| REK::PathHashingError.into_error_with_cause(e)) - }) + .map_err_into(REK::PathCanonicalizationError) + .and_then(|c| hash_path(&c)) + .map_err_into(REK::PathHashingError) .and_then(|hash| { - self.retrieve_for_module("ref").map(|iter| (hash, iter)) - .map_err(Box::new) - .map_err(|e| REK::StoreReadError.into_error_with_cause(e)) + self.retrieve_for_module("ref") + .map(|iter| (hash, iter)) + .map_err_into(REK::StoreReadError) }) .and_then(|(hash, possible_refs)| { // This is kind of a manual Iterator::filter() call what we do here, but with the @@ -89,7 +85,8 @@ impl RefStore for Store { for r in possible_refs { let contains_hash = try!(r.to_str() .map_err_into(REK::TypeConversionError) - .map(|s| s.contains(&hash[..]))); + .map(|s| s.contains(&hash[..])) + ); if !contains_hash { continue; @@ -102,13 +99,8 @@ impl RefStore for Store { } }, - Ok(None) => { // Something weird just happened - return Err(REK::StoreReadError.into_error()); - }, - - Err(e) => { - return Err(REK::StoreReadError.into_error_with_cause(Box::new(e))); - }, + Ok(None) => return Err(REK::StoreReadError.into_error()), + Err(e) => return Err(e).map_err_into(REK::StoreReadError) } } @@ -119,7 +111,7 @@ impl RefStore for Store { /// Try to get `si` as Ref object from the store fn get<'a>(&'a self, si: StoreId) -> Result> { match self.get(si) { - Err(e) => return Err(REK::StoreReadError.into_error_with_cause(Box::new(e))), + Err(e) => return Err(e).map_err_into(REK::StoreReadError), Ok(None) => return Err(REK::RefNotInStore.into_error()), Ok(Some(fle)) => Ok(fle), } @@ -132,8 +124,7 @@ impl RefStore for Store { ModuleEntryPath::new(hash) .into_storeid() .and_then(|id| self.get(id)) - .map_err(Box::new) - .map_err(|e| REK::StoreReadError.into_error_with_cause(e)) + .map_err_into(REK::StoreReadError) } /// Delete a ref by hash @@ -143,8 +134,7 @@ impl RefStore for Store { ModuleEntryPath::new(hash) .into_storeid() .and_then(|id| self.delete(id)) - .map_err(Box::new) - .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) + .map_err_into(REK::StoreWriteError) } /// Create a Ref object which refers to `pb` @@ -164,8 +154,7 @@ impl RefStore for Store { let (mut fle, content_hash, permissions, canonical_path) = { // scope to be able to fold try!(File::open(pb.clone()) - .map_err(Box::new) - .map_err(|e| REK::RefTargetFileCannotBeOpened.into_error_with_cause(e)) + .map_err_into(REK::RefTargetFileCannotBeOpened) // If we were able to open this file, // we hash the contents of the file and return (file, hash) @@ -186,8 +175,7 @@ impl RefStore for Store { Some(try!(file .metadata() .map(|md| md.permissions()) - .map_err(Box::new) - .map_err(|e| REK::RefTargetCannotReadPermissions.into_error_with_cause(e)) + .map_err_into(REK::RefTargetCannotReadPermissions) )) } else { None @@ -203,16 +191,13 @@ impl RefStore for Store { pb.canonicalize() .map(|can| (opt_contenthash, opt_permissions, can)) // if PathBuf::canonicalize() failed, build an error from the return value - .map_err(|e| REK::PathCanonicalizationError.into_error_with_cause(Box::new(e))) + .map_err_into(REK::PathCanonicalizationError) }) // and then we hash the canonicalized path // and return (file, content hash, permissions, canonicalized path, path hash) .and_then(|(opt_contenthash, opt_permissions, can)| { - let path_hash = try!(hash_path(&can) - .map_err(Box::new) - .map_err(|e| REK::PathHashingError.into_error_with_cause(e)) - ); + let path_hash = try!(hash_path(&can).map_err_into(REK::PathHashingError)); Ok((opt_contenthash, opt_permissions, can, path_hash)) }) @@ -234,8 +219,7 @@ impl RefStore for Store { .and_then(|(opt_conhash, opt_perm, can, path_hash)| { let fle = try!(self .create(ModuleEntryPath::new(path_hash)) - .map_err(Box::new) - .map_err(|e| REK::StoreWriteError.into_error_with_cause(e)) + .map_err_into(REK::StoreWriteError) ); Ok((fle, opt_conhash, opt_perm, can)) @@ -262,15 +246,9 @@ impl RefStore for Store { match fle.get_header_mut().insert(s, v.clone()) { Ok(false) => { let e = REK::HeaderFieldAlreadyExistsError.into_error(); - let e = Box::new(e); - let e = REK::HeaderFieldWriteError.into_error_with_cause(e); - return Err(e); - }, - Err(e) => { - let e = Box::new(e); - let e = REK::HeaderFieldWriteError.into_error_with_cause(e); - return Err(e); + return Err(e).map_err_into(REK::HeaderFieldWriteError); }, + Err(e) => return Err(e).map_err_into(REK::HeaderFieldWriteError), _ => (), } } @@ -284,3 +262,4 @@ impl RefStore for Store { } } + From 6d40797a0755ada383261896605cab75ac2c3ecc Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 28 Aug 2017 09:54:25 +0200 Subject: [PATCH 04/25] Fix libimagmail to use new libimagentryref interface --- lib/domain/libimagmail/src/iter.rs | 14 +++++++------- lib/domain/libimagmail/src/mail.rs | 16 +++++++++------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/lib/domain/libimagmail/src/iter.rs b/lib/domain/libimagmail/src/iter.rs index fe1e72a4..6fee9e64 100644 --- a/lib/domain/libimagmail/src/iter.rs +++ b/lib/domain/libimagmail/src/iter.rs @@ -27,16 +27,17 @@ use mail::Mail; use result::Result; +use libimagstore::store::FileLockEntry; use libimagentryref::reference::Ref; use std::marker::PhantomData; -pub struct MailIter<'a, I: 'a + Iterator>> { - _marker: PhantomData<&'a I>, +pub struct MailIter<'a, I: Iterator>> { + _marker: PhantomData, i: I, } -impl<'a, I: Iterator>> MailIter<'a, I> { +impl<'a, I: Iterator>> MailIter<'a, I> { pub fn new(i: I) -> MailIter<'a, I> { MailIter { _marker: PhantomData, i: i } @@ -44,12 +45,11 @@ impl<'a, I: Iterator>> MailIter<'a, I> { } -impl<'a, I: Iterator>> Iterator for MailIter<'a, I> { - +impl<'a, I: Iterator>> Iterator for MailIter<'a, I> { type Item = Result>; - fn next(&mut self) -> Option>> { - self.i.next().map(Mail::from_ref) + fn next(&mut self) -> Option { + self.i.next().map(Mail::from_fle) } } diff --git a/lib/domain/libimagmail/src/mail.rs b/lib/domain/libimagmail/src/mail.rs index 11b57398..d5f02cdf 100644 --- a/lib/domain/libimagmail/src/mail.rs +++ b/lib/domain/libimagmail/src/mail.rs @@ -23,8 +23,10 @@ use std::fs::File; use std::io::Read; use libimagstore::store::Store; +use libimagstore::store::FileLockEntry; use libimagentryref::reference::Ref; use libimagentryref::flags::RefFlags; +use libimagentryref::refstore::RefStore; use email::MimeMessage; use email::results::ParsingResult as EmailParsingResult; @@ -47,7 +49,7 @@ impl From for Buffer { } } -pub struct Mail<'a>(Ref<'a>, Buffer); +pub struct Mail<'a>(FileLockEntry<'a>, Buffer); impl<'a> Mail<'a> { @@ -57,7 +59,7 @@ impl<'a> Mail<'a> { let f = RefFlags::default().with_content_hashing(true).with_permission_tracking(false); let p = PathBuf::from(p.as_ref()); - Ref::create_with_hasher(store, p, f, h) + store.create_with_hasher(p, f, h) .map_err_into(MEK::RefCreationError) .and_then(|reference| { reference.fs_file() @@ -76,19 +78,19 @@ impl<'a> Mail<'a> { /// Opens a mail by the passed hash pub fn open>(store: &Store, hash: S) -> Result> { - Ref::get_by_hash(store, String::from(hash.as_ref())) + store.get_by_hash(String::from(hash.as_ref())) .map_err_into(MEK::FetchByHashError) .map_err_into(MEK::FetchError) .and_then(|o| match o { - Some(r) => Mail::from_ref(r).map(Some), + Some(r) => Mail::from_fle(r).map(Some), None => Ok(None), }) } /// Implement me as TryFrom as soon as it is stable - pub fn from_ref(r: Ref<'a>) -> Result { - r.fs_file() + pub fn from_fle(fle: FileLockEntry<'a>) -> Result> { + fle.fs_file() .map_err_into(MEK::RefHandlingError) .and_then(|path| File::open(path).map_err_into(MEK::IOError)) .and_then(|mut file| { @@ -98,7 +100,7 @@ impl<'a> Mail<'a> { .map_err_into(MEK::IOError) }) .map(Buffer::from) - .map(|buffer| Mail(r, buffer)) + .map(|buffer| Mail(fle, buffer)) } pub fn get_field(&self, field: &str) -> Result> { From 7ff3985eaf6f2fddf1bd5651a4c77afdfd91a02b Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 28 Aug 2017 10:43:51 +0200 Subject: [PATCH 05/25] Fix imag-mail to use new Ref interface --- bin/domain/imag-mail/src/main.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/bin/domain/imag-mail/src/main.rs b/bin/domain/imag-mail/src/main.rs index 1eeb52bf..3091fbf2 100644 --- a/bin/domain/imag-mail/src/main.rs +++ b/bin/domain/imag-mail/src/main.rs @@ -33,6 +33,7 @@ extern crate libimagentryref; use libimagerror::trace::{MapErrTrace, trace_error, trace_error_exit}; use libimagmail::mail::Mail; use libimagentryref::reference::Ref; +use libimagentryref::refstore::RefStore; use libimagrt::runtime::Runtime; use libimagrt::setup::generate_runtime_setup; use libimagutil::debug_result::*; @@ -83,11 +84,11 @@ fn list(rt: &Runtime) { let iter = match store.retrieve_for_module("ref") { Ok(iter) => iter.filter_map(|id| { - Ref::get(store, id) - .map_err_into(MEK::RefHandlingError) - .and_then(|rf| Mail::from_ref(rf)) - .map_err_trace() - .ok() + match store.get(id).map_err_into(MEK::RefHandlingError).map_err_trace() { + Ok(Some(fle)) => Mail::from_fle(fle).map_err_trace().ok(), + Ok(None) => None, + Err(e) => trace_error_exit(&e, 1), + } }), Err(e) => trace_error_exit(&e, 1), }; From 0e6599f1922a972d5f8083b9aaf099c798d33915 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 28 Aug 2017 14:55:33 +0200 Subject: [PATCH 06/25] Fix imag-ref --- bin/core/imag-ref/src/main.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/core/imag-ref/src/main.rs b/bin/core/imag-ref/src/main.rs index aa2f7d08..98d88f7b 100644 --- a/bin/core/imag-ref/src/main.rs +++ b/bin/core/imag-ref/src/main.rs @@ -50,7 +50,7 @@ use ui::build_ui; use std::path::PathBuf; -use libimagentryref::reference::Ref; +use libimagentryref::refstore::RefStore; use libimagentryref::flags::RefFlags; use libimagerror::trace::trace_error; use libimagrt::setup::generate_runtime_setup; @@ -84,7 +84,7 @@ fn add(rt: &Runtime) { .with_content_hashing(cmd.is_present("track-content")) .with_permission_tracking(cmd.is_present("track-permissions")); - match Ref::create(rt.store(), path, flags) { + match RefStore::create(rt.store(), path, flags) { Ok(r) => { debug!("Reference created: {:?}", r); info!("Ok"); @@ -104,7 +104,7 @@ fn remove(rt: &Runtime) { let yes = cmd.is_present("yes"); if yes || ask_bool(&format!("Delete Ref with hash '{}'", hash)[..], None) { - match Ref::delete_by_hash(rt.store(), hash) { + match rt.store().delete_by_hash(hash) { Err(e) => trace_error(&e), Ok(_) => info!("Ok"), } @@ -128,7 +128,7 @@ fn list(rt: &Runtime) { let iter = match rt.store().retrieve_for_module("ref") { Ok(iter) => iter.filter_map(|id| { - match Ref::get(rt.store(), id) { + match rt.store().get(id) { Ok(r) => Some(r), Err(e) => { trace_error(&e); @@ -147,7 +147,7 @@ fn list(rt: &Runtime) { .check_changed(do_check_changed) .check_changed_content(do_check_changed_content) .check_changed_permiss(do_check_changed_permiss) - .list(iter.map(|e| e.into())) + .list(iter.filter_map(Into::into)) .ok(); } From e338fef98edf8d4c22c4a8b38adaaac2b6e43705 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 31 Aug 2017 13:11:38 +0200 Subject: [PATCH 07/25] Remove duplicated key --- lib/entry/libimagentryref/Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/entry/libimagentryref/Cargo.toml b/lib/entry/libimagentryref/Cargo.toml index 9c3bb9c2..d183b5a8 100644 --- a/lib/entry/libimagentryref/Cargo.toml +++ b/lib/entry/libimagentryref/Cargo.toml @@ -20,7 +20,6 @@ rust-crypto = "0.2" toml = "^0.4" toml-query = "0.3.0" walkdir = "1.0.*" -toml-query = "0.3.0" libimagstore = { version = "0.4.0", path = "../../../lib/core/libimagstore" } libimagerror = { version = "0.4.0", path = "../../../lib/core/libimagerror" } From 74ec12d5c2111a7510c45d842967cc22c3197a0f Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 31 Aug 2017 18:59:56 +0200 Subject: [PATCH 08/25] Remove artifacts I don't even know how this could re-appear in the codebase as we removed the toml_ext thing a while ago... strange! --- lib/entry/libimagentryref/src/reference.rs | 1 - lib/entry/libimagentryref/src/refstore.rs | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/entry/libimagentryref/src/reference.rs b/lib/entry/libimagentryref/src/reference.rs index d0042517..07ba742e 100644 --- a/lib/entry/libimagentryref/src/reference.rs +++ b/lib/entry/libimagentryref/src/reference.rs @@ -25,7 +25,6 @@ use std::fs::File; use std::fs::Permissions; use libimagstore::store::Entry; -use libimagstore::toml_ext::TomlValueExt; use libimagstore::store::FileLockEntry; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; diff --git a/lib/entry/libimagentryref/src/refstore.rs b/lib/entry/libimagentryref/src/refstore.rs index fed7ba13..8c4e1134 100644 --- a/lib/entry/libimagentryref/src/refstore.rs +++ b/lib/entry/libimagentryref/src/refstore.rs @@ -25,7 +25,6 @@ use libimagstore::store::FileLockEntry; use libimagstore::storeid::StoreId; use libimagstore::storeid::IntoStoreId; use libimagstore::store::Store; -use libimagstore::toml_ext::TomlValueExt; use libimagerror::into::IntoError; use toml::Value; @@ -145,6 +144,8 @@ impl RefStore for Store { fn create_with_hasher<'a, H: Hasher>(&'a self, pb: PathBuf, flags: RefFlags, mut h: H) -> Result> { + use toml_query::insert::TomlValueInsertExt; + if !pb.exists() { return Err(REK::RefTargetDoesNotExist.into_error()); } From ec639be3e13d302edd7c24340ac3cb3c7f612ad7 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 31 Aug 2017 19:26:33 +0200 Subject: [PATCH 09/25] Fix artifacts from libimagstore::toml_ext times --- lib/entry/libimagentryref/src/refstore.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/entry/libimagentryref/src/refstore.rs b/lib/entry/libimagentryref/src/refstore.rs index 8c4e1134..e8ec1edd 100644 --- a/lib/entry/libimagentryref/src/refstore.rs +++ b/lib/entry/libimagentryref/src/refstore.rs @@ -249,6 +249,9 @@ impl RefStore for Store { let e = REK::HeaderFieldAlreadyExistsError.into_error(); return Err(e).map_err_into(REK::HeaderFieldWriteError); }, + Ok(None) => { + // Okay, we just inserted a new header value... + }, Err(e) => return Err(e).map_err_into(REK::HeaderFieldWriteError), _ => (), } From 6a8af5ef01282d65648c350c2c9ef339cbe7dc1a Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 31 Aug 2017 19:28:28 +0200 Subject: [PATCH 10/25] Remove unused imports --- lib/entry/libimagentryref/src/reference.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/entry/libimagentryref/src/reference.rs b/lib/entry/libimagentryref/src/reference.rs index 07ba742e..3dd196ba 100644 --- a/lib/entry/libimagentryref/src/reference.rs +++ b/lib/entry/libimagentryref/src/reference.rs @@ -25,16 +25,11 @@ use std::fs::File; use std::fs::Permissions; use libimagstore::store::Entry; -use libimagstore::store::FileLockEntry; -use libimagstore::storeid::StoreId; -use libimagstore::storeid::IntoStoreId; -use libimagstore::store::Store; use libimagerror::into::IntoError; use toml::Value; use toml_query::read::TomlValueReadExt; use toml_query::set::TomlValueSetExt; -use toml_query::insert::TomlValueInsertExt; use error::RefErrorKind as REK; use error::MapErrInto; From 7e4275c420fcad1686a22077996041dd7ef3f2e7 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Thu, 31 Aug 2017 19:34:26 +0200 Subject: [PATCH 11/25] Remove unused crate import --- bin/domain/imag-mail/Cargo.toml | 1 - bin/domain/imag-mail/src/main.rs | 3 --- lib/domain/libimagmail/src/iter.rs | 1 - 3 files changed, 5 deletions(-) diff --git a/bin/domain/imag-mail/Cargo.toml b/bin/domain/imag-mail/Cargo.toml index ac69c58c..5c6a7bae 100644 --- a/bin/domain/imag-mail/Cargo.toml +++ b/bin/domain/imag-mail/Cargo.toml @@ -17,5 +17,4 @@ version = "2.0.1" libimagrt = { version = "0.4.0", path = "../../../lib/core/libimagrt" } libimagerror = { version = "0.4.0", path = "../../../lib/core/libimagerror" } libimagmail = { version = "0.4.0", path = "../../../lib/domain/libimagmail" } -libimagentryref = { version = "0.4.0", path = "../../../lib/entry/libimagentryref" } libimagutil = { version = "0.4.0", path = "../../../lib/etc/libimagutil" } diff --git a/bin/domain/imag-mail/src/main.rs b/bin/domain/imag-mail/src/main.rs index a8524bb5..8596d77b 100644 --- a/bin/domain/imag-mail/src/main.rs +++ b/bin/domain/imag-mail/src/main.rs @@ -25,12 +25,9 @@ extern crate libimagrt; extern crate libimagmail; extern crate libimagerror; extern crate libimagutil; -extern crate libimagentryref; use libimagerror::trace::{MapErrTrace, trace_error, trace_error_exit}; use libimagmail::mail::Mail; -use libimagentryref::reference::Ref; -use libimagentryref::refstore::RefStore; use libimagrt::runtime::Runtime; use libimagrt::setup::generate_runtime_setup; use libimagutil::info_result::*; diff --git a/lib/domain/libimagmail/src/iter.rs b/lib/domain/libimagmail/src/iter.rs index 6fee9e64..ee418765 100644 --- a/lib/domain/libimagmail/src/iter.rs +++ b/lib/domain/libimagmail/src/iter.rs @@ -28,7 +28,6 @@ use mail::Mail; use result::Result; use libimagstore::store::FileLockEntry; -use libimagentryref::reference::Ref; use std::marker::PhantomData; From 9c69645b69052fd1170484e390cbea363f784f9a Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 28 Aug 2017 21:58:43 +0200 Subject: [PATCH 12/25] Implement Diary as trait --- lib/domain/libimagdiary/src/diary.rs | 171 ++++++++++++++++---------- lib/domain/libimagdiary/src/entry.rs | 5 +- lib/domain/libimagdiary/src/error.rs | 3 +- lib/domain/libimagdiary/src/iter.rs | 45 ++++++- lib/domain/libimagdiary/src/viewer.rs | 7 +- 5 files changed, 154 insertions(+), 77 deletions(-) diff --git a/lib/domain/libimagdiary/src/diary.rs b/lib/domain/libimagdiary/src/diary.rs index 431b6320..be768fed 100644 --- a/lib/domain/libimagdiary/src/diary.rs +++ b/lib/domain/libimagdiary/src/diary.rs @@ -19,6 +19,7 @@ use std::cmp::Ordering; +use libimagstore::store::FileLockEntry; use libimagstore::store::Store; use libimagstore::storeid::IntoStoreId; use libimagerror::trace::trace_error; @@ -27,102 +28,136 @@ use chrono::offset::Local; use chrono::Datelike; use itertools::Itertools; use chrono::naive::NaiveDateTime; +use chrono::Timelike; use entry::Entry; use diaryid::DiaryId; -use error::DiaryError as DE; use error::DiaryErrorKind as DEK; +use error::MapErrInto; use result::Result; use iter::DiaryEntryIterator; -use is_in_diary::IsInDiary; +use iter::DiaryNameIterator; -#[derive(Debug)] -pub struct Diary<'a> { - store: &'a Store, - name: &'a str, -} +trait Diary { -impl<'a> Diary<'a> { + /// Wrapper around Store::get for DiaryId + fn get(&self, id: DiaryId) -> Result>; - pub fn open(store: &'a Store, name: &'a str) -> Diary<'a> { - Diary { - store: store, - name: name, - } - } + /// Wrapper around Store::retrieve for DiaryId + fn retrieve(&self, id: DiaryId) -> Result; + + /// Wrapper around Store::delete for DiaryId + fn delete(&self, entry: Entry) -> Result<()>; // create or get a new entry for today - pub fn new_entry_today(&self) -> Result { - let dt = Local::now(); - let ndt = dt.naive_local(); - let id = DiaryId::new(String::from(self.name), ndt.year(), ndt.month(), ndt.day(), 0, 0); - self.new_entry_by_id(id) + fn new_entry_today(&self, diary_name: &str) -> Result; + + // create or get a new entry for now + fn new_entry_now(&self, diary_name: &str) -> Result; + + // Get an iterator for iterating over all entries of a Diary + fn entries(&self, diary_name: &str) -> Result; + + fn get_youngest_entry_id(&self, diary_name: &str) -> Option>; + + /// Get all diary names + fn diary_names(&self) -> Result; + +} + +impl Diary for Store { + + + /// Wrapper around Store::get for DiaryId + fn get(&self, id: DiaryId) -> Result> { + id.into_storeid().and_then(|id| self.get(id)).map_err_into(DEK::StoreWriteError) } - pub fn new_entry_by_id(&self, id: DiaryId) -> Result { - self.retrieve(id.with_diary_name(String::from(self.name))) + /// Wrapper around Store::retrieve for DiaryId + fn retrieve(&self, id: DiaryId) -> Result { + id.into_storeid().and_then(|id| self.retrieve(id)).map_err_into(DEK::StoreWriteError) } - pub fn retrieve(&self, id: DiaryId) -> Result { - id.into_storeid() - .and_then(|id| self.store.retrieve(id)) - .map(|fle| Entry::new(fle)) - .map_err(|e| DE::new(DEK::StoreWriteError, Some(Box::new(e)))) - } - - // Get an iterator for iterating over all entries - pub fn entries(&self) -> Result> { - self.store - .retrieve_for_module("diary") - .map(|iter| DiaryEntryIterator::new(self.name, self.store, iter)) - .map_err(|e| DE::new(DEK::StoreReadError, Some(Box::new(e)))) - } - - pub fn delete_entry(&self, entry: Entry) -> Result<()> { - if !entry.is_in_diary(self.name) { - return Err(DE::new(DEK::EntryNotInDiary, None)); - } + /// Wrapper around Store::delete for DiaryId + fn delete(&self, entry: Entry) -> Result<()> { let id = entry.get_location().clone(); drop(entry); - self.store.delete(id) - .map_err(|e| DE::new(DEK::StoreWriteError, Some(Box::new(e)))) + self.delete(id).map_err_into(DEK::StoreWriteError) } - pub fn get_youngest_entry(&self) -> Option> { - match self.entries() { + // create or get a new entry for today + fn new_entry_today(&self, diary_name: &str) -> Result { + let dt = Local::now(); + let ndt = dt.naive_local(); + let id = DiaryId::new(String::from(diary_name), ndt.year(), ndt.month(), ndt.day(), 0, 0); + Diary::retrieve(self, id) + } + + // create or get a new entry for today + fn new_entry_now(&self, diary_name: &str) -> Result { + let dt = Local::now(); + let ndt = dt.naive_local(); + let id = DiaryId::new(String::from(diary_name), + ndt.year(), + ndt.month(), + ndt.day(), + ndt.minute(), + ndt.second()); + + Diary::retrieve(self, id) + } + + // Get an iterator for iterating over all entries + fn entries(&self, diary_name: &str) -> Result { + self.retrieve_for_module("diary") + .map(|iter| DiaryEntryIterator::new(self, String::from(diary_name), iter)) + .map_err_into(DEK::StoreReadError) + } + + fn get_youngest_entry_id(&self, diary_name: &str) -> Option> { + match Diary::entries(self, diary_name) { Err(e) => Some(Err(e)), Ok(entries) => { - entries.sorted_by(|a, b| { - match (a, b) { - (&Ok(ref a), &Ok(ref b)) => { - let a : NaiveDateTime = a.diary_id().into(); - let b : NaiveDateTime = b.diary_id().into(); + entries + .map(|e| e.and_then(|e| e.diary_id())) + .sorted_by(|a, b| { + match (a, b) { + (&Ok(ref a), &Ok(ref b)) => { + let a : NaiveDateTime = a.clone().into(); + let b : NaiveDateTime = b.clone().into(); - a.cmp(&b) - }, + a.cmp(&b) + }, - (&Ok(_), &Err(ref e)) => { - trace_error(e); - Ordering::Less - }, - (&Err(ref e), &Ok(_)) => { - trace_error(e); - Ordering::Greater - }, - (&Err(ref e1), &Err(ref e2)) => { - trace_error(e1); - trace_error(e2); - Ordering::Equal - }, - } - }).into_iter().next() + (&Ok(_), &Err(ref e)) => { + trace_error(e); + Ordering::Less + }, + (&Err(ref e), &Ok(_)) => { + trace_error(e); + Ordering::Greater + }, + (&Err(ref e1), &Err(ref e2)) => { + trace_error(e1); + trace_error(e2); + Ordering::Equal + }, + } + }) + .into_iter() + //.map(|sidres| sidres.map(|sid| DiaryId::from_storeid(&sid))) + .next() } } } - pub fn name(&self) -> &'a str { - &self.name + /// Get all diary names + fn diary_names(&self) -> Result { + self.retrieve_for_module("diary") + .map_err_into(DEK::StoreReadError) + .map(DiaryNameIterator::new) } + } diff --git a/lib/domain/libimagdiary/src/entry.rs b/lib/domain/libimagdiary/src/entry.rs index 0148b59d..d0fb2e53 100644 --- a/lib/domain/libimagdiary/src/entry.rs +++ b/lib/domain/libimagdiary/src/entry.rs @@ -27,6 +27,7 @@ use libimagrt::runtime::Runtime; use diaryid::DiaryId; use diaryid::FromStoreId; +use result::Result; #[derive(Debug)] pub struct Entry<'a>(FileLockEntry<'a>); @@ -57,8 +58,8 @@ impl<'a> Entry<'a> { /// Get the diary id for this entry. /// /// TODO: calls Option::unwrap() as it assumes that an existing Entry has an ID that is parsable - pub fn diary_id(&self) -> DiaryId { - DiaryId::from_storeid(&self.0.get_location().clone()).unwrap() + pub fn diary_id(&self) -> Result { + DiaryId::from_storeid(&self.0.get_location().clone()) } } diff --git a/lib/domain/libimagdiary/src/error.rs b/lib/domain/libimagdiary/src/error.rs index b5406b12..1164f806 100644 --- a/lib/domain/libimagdiary/src/error.rs +++ b/lib/domain/libimagdiary/src/error.rs @@ -28,7 +28,8 @@ generate_error_module!( EntryNotInDiary => "Entry not in Diary", IOError => "IO Error", ViewError => "Error viewing diary entry", - IdParseError => "Error while parsing ID" + IdParseError => "Error while parsing ID", + DiaryNameFindingError => "Error while finding a diary name" ); ); diff --git a/lib/domain/libimagdiary/src/iter.rs b/lib/domain/libimagdiary/src/iter.rs index de849872..2ed40ece 100644 --- a/lib/domain/libimagdiary/src/iter.rs +++ b/lib/domain/libimagdiary/src/iter.rs @@ -22,6 +22,8 @@ use std::result::Result as RResult; use libimagstore::store::Store; use libimagstore::storeid::StoreIdIterator; +use libimagerror::trace::trace_error; +use libimagerror::into::IntoError; use diaryid::DiaryId; use diaryid::FromStoreId; @@ -29,13 +31,14 @@ use is_in_diary::IsInDiary; use entry::Entry as DiaryEntry; use error::DiaryError as DE; use error::DiaryErrorKind as DEK; +use error::MapErrInto; use result::Result; -use libimagerror::trace::trace_error; +use is_in_diary::IsInDiary; /// A iterator for iterating over diary entries pub struct DiaryEntryIterator<'a> { store: &'a Store, - name: &'a str, + name: String, iter: StoreIdIterator, year: Option, @@ -54,7 +57,7 @@ impl<'a> Debug for DiaryEntryIterator<'a> { impl<'a> DiaryEntryIterator<'a> { - pub fn new(diaryname: &'a str, store: &'a Store, iter: StoreIdIterator) -> DiaryEntryIterator<'a> { + pub fn new(store: &'a Store, diaryname: String, iter: StoreIdIterator) -> DiaryEntryIterator<'a> { DiaryEntryIterator { store: store, name: diaryname, @@ -97,7 +100,7 @@ impl<'a> Iterator for DiaryEntryIterator<'a> { }; debug!("Next element: {:?}", next); - if next.is_in_diary(self.name) { + if next.is_in_diary(&self.name) { debug!("Seems to be in diary: {:?}", next); let id = match DiaryId::from_storeid(&next) { Ok(i) => i, @@ -130,3 +133,37 @@ impl<'a> Iterator for DiaryEntryIterator<'a> { } + +/// Get diary names. +/// +/// # Warning +/// +/// Does _not_ run a `unique` on the iterator! +pub struct DiaryNameIterator(StoreIdIterator); + +impl DiaryNameIterator { + pub fn new(s: StoreIdIterator) -> DiaryNameIterator { + DiaryNameIterator(s) + } +} + +impl Iterator for DiaryNameIterator { + type Item = Result; + + fn next(&mut self) -> Option { + self.0 + .next() + .map(|s| { + s.to_str() + .map_err_into(DEK::DiaryNameFindingError) + .and_then(|s| { + s.split("diary/") + .nth(1) + .and_then(|n| n.split("/").nth(0).map(String::from)) + .ok_or(DEK::DiaryNameFindingError.into_error()) + }) + }) + } + +} + diff --git a/lib/domain/libimagdiary/src/viewer.rs b/lib/domain/libimagdiary/src/viewer.rs index 93b155a7..38c6e179 100644 --- a/lib/domain/libimagdiary/src/viewer.rs +++ b/lib/domain/libimagdiary/src/viewer.rs @@ -26,6 +26,7 @@ use result::Result; use libimagentryview::viewer::Viewer; use libimagentryview::builtin::plain::PlainViewer; +use libimagerror::trace::trace_error; /// This viewer does _not_ implement libimagentryview::viewer::Viewer because we need to be able to /// call some diary-type specific functions on the entries passed to this. @@ -48,8 +49,10 @@ impl DiaryViewer { /// error. pub fn view_entries<'a, I: Iterator>>(&self, entries: I) -> Result<()> { for entry in entries { - let id = entry.diary_id(); - println!("{} :\n", id); + match entry.diary_id() { + Ok(id) => println!("{} :\n", id), + Err(e) => trace_error(&e), + } let _ = try!(self.0 .view_entry(&entry) .map_err_into(DEK::ViewError) From 95b7da1ed21e4af5142b84bb49768a9c40b8dd68 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 29 Aug 2017 10:52:10 +0200 Subject: [PATCH 13/25] Impl Entry (now DiaryEntry) as trait --- lib/domain/libimagdiary/src/diary.rs | 36 ++------------- lib/domain/libimagdiary/src/entry.rs | 63 +++------------------------ lib/domain/libimagdiary/src/iter.rs | 8 ++-- lib/domain/libimagdiary/src/viewer.rs | 5 ++- 4 files changed, 16 insertions(+), 96 deletions(-) diff --git a/lib/domain/libimagdiary/src/diary.rs b/lib/domain/libimagdiary/src/diary.rs index be768fed..cf738e33 100644 --- a/lib/domain/libimagdiary/src/diary.rs +++ b/lib/domain/libimagdiary/src/diary.rs @@ -21,7 +21,6 @@ use std::cmp::Ordering; use libimagstore::store::FileLockEntry; use libimagstore::store::Store; -use libimagstore::storeid::IntoStoreId; use libimagerror::trace::trace_error; use chrono::offset::Local; @@ -30,7 +29,7 @@ use itertools::Itertools; use chrono::naive::NaiveDateTime; use chrono::Timelike; -use entry::Entry; +use entry::DiaryEntry; use diaryid::DiaryId; use error::DiaryErrorKind as DEK; use error::MapErrInto; @@ -40,15 +39,6 @@ use iter::DiaryNameIterator; trait Diary { - /// Wrapper around Store::get for DiaryId - fn get(&self, id: DiaryId) -> Result>; - - /// Wrapper around Store::retrieve for DiaryId - fn retrieve(&self, id: DiaryId) -> Result; - - /// Wrapper around Store::delete for DiaryId - fn delete(&self, entry: Entry) -> Result<()>; - // create or get a new entry for today fn new_entry_today(&self, diary_name: &str) -> Result; @@ -67,31 +57,13 @@ trait Diary { impl Diary for Store { - - /// Wrapper around Store::get for DiaryId - fn get(&self, id: DiaryId) -> Result> { - id.into_storeid().and_then(|id| self.get(id)).map_err_into(DEK::StoreWriteError) - } - - /// Wrapper around Store::retrieve for DiaryId - fn retrieve(&self, id: DiaryId) -> Result { - id.into_storeid().and_then(|id| self.retrieve(id)).map_err_into(DEK::StoreWriteError) - } - - /// Wrapper around Store::delete for DiaryId - fn delete(&self, entry: Entry) -> Result<()> { - let id = entry.get_location().clone(); - drop(entry); - - self.delete(id).map_err_into(DEK::StoreWriteError) - } - // create or get a new entry for today fn new_entry_today(&self, diary_name: &str) -> Result { let dt = Local::now(); let ndt = dt.naive_local(); let id = DiaryId::new(String::from(diary_name), ndt.year(), ndt.month(), ndt.day(), 0, 0); - Diary::retrieve(self, id) + + self.retrieve(id).map_err_into(DEK::StoreReadError) } // create or get a new entry for today @@ -105,7 +77,7 @@ impl Diary for Store { ndt.minute(), ndt.second()); - Diary::retrieve(self, id) + self.retrieve(id).map_err_into(DEK::StoreReadError) } // Get an iterator for iterating over all entries diff --git a/lib/domain/libimagdiary/src/entry.rs b/lib/domain/libimagdiary/src/entry.rs index d0fb2e53..2da96199 100644 --- a/lib/domain/libimagdiary/src/entry.rs +++ b/lib/domain/libimagdiary/src/entry.rs @@ -17,75 +17,24 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // -use std::ops::Deref; -use std::ops::DerefMut; - -use libimagstore::store::FileLockEntry; -use libimagentryedit::edit::Edit; -use libimagentryedit::result::Result as EditResult; -use libimagrt::runtime::Runtime; +use libimagstore::store::Entry; use diaryid::DiaryId; use diaryid::FromStoreId; use result::Result; -#[derive(Debug)] -pub struct Entry<'a>(FileLockEntry<'a>); - -impl<'a> Deref for Entry<'a> { - type Target = FileLockEntry<'a>; - - fn deref(&self) -> &FileLockEntry<'a> { - &self.0 - } - +pub trait DiaryEntry { + fn diary_id(&self) -> Result; } -impl<'a> DerefMut for Entry<'a> { - - fn deref_mut(&mut self) -> &mut FileLockEntry<'a> { - &mut self.0 - } - -} - -impl<'a> Entry<'a> { - - pub fn new(fle: FileLockEntry<'a>) -> Entry<'a> { - Entry(fle) - } +impl DiaryEntry for Entry { /// Get the diary id for this entry. /// /// TODO: calls Option::unwrap() as it assumes that an existing Entry has an ID that is parsable - pub fn diary_id(&self) -> Result { - DiaryId::from_storeid(&self.0.get_location().clone()) + fn diary_id(&self) -> Result { + DiaryId::from_storeid(&self.get_location().clone()) } } -impl<'a> Into> for Entry<'a> { - - fn into(self) -> FileLockEntry<'a> { - self.0 - } - -} - -impl<'a> From> for Entry<'a> { - - fn from(fle: FileLockEntry<'a>) -> Entry<'a> { - Entry::new(fle) - } - -} - -impl<'a> Edit for Entry<'a> { - - fn edit_content(&mut self, rt: &Runtime) -> EditResult<()> { - self.0.edit_content(rt) - } - -} - - diff --git a/lib/domain/libimagdiary/src/iter.rs b/lib/domain/libimagdiary/src/iter.rs index 2ed40ece..32fe407c 100644 --- a/lib/domain/libimagdiary/src/iter.rs +++ b/lib/domain/libimagdiary/src/iter.rs @@ -21,14 +21,13 @@ use std::fmt::{Debug, Formatter, Error as FmtError}; use std::result::Result as RResult; use libimagstore::store::Store; +use libimagstore::store::FileLockEntry; use libimagstore::storeid::StoreIdIterator; use libimagerror::trace::trace_error; use libimagerror::into::IntoError; use diaryid::DiaryId; use diaryid::FromStoreId; -use is_in_diary::IsInDiary; -use entry::Entry as DiaryEntry; use error::DiaryError as DE; use error::DiaryErrorKind as DEK; use error::MapErrInto; @@ -90,9 +89,9 @@ impl<'a> DiaryEntryIterator<'a> { } impl<'a> Iterator for DiaryEntryIterator<'a> { - type Item = Result>; + type Item = Result>; - fn next(&mut self) -> Option>> { + fn next(&mut self) -> Option { loop { let next = match self.iter.next() { Some(s) => s, @@ -121,7 +120,6 @@ impl<'a> Iterator for DiaryEntryIterator<'a> { return Some(self .store .retrieve(next) - .map(|fle| DiaryEntry::new(fle)) .map_err(|e| DE::new(DEK::StoreReadError, Some(Box::new(e)))) ); } diff --git a/lib/domain/libimagdiary/src/viewer.rs b/lib/domain/libimagdiary/src/viewer.rs index 38c6e179..c0c03fc6 100644 --- a/lib/domain/libimagdiary/src/viewer.rs +++ b/lib/domain/libimagdiary/src/viewer.rs @@ -19,11 +19,12 @@ //! A diary viewer built on libimagentryview. -use entry::Entry; +use entry::DiaryEntry; use error::DiaryErrorKind as DEK; use error::MapErrInto; use result::Result; +use libimagstore::store::FileLockEntry; use libimagentryview::viewer::Viewer; use libimagentryview::builtin::plain::PlainViewer; use libimagerror::trace::trace_error; @@ -47,7 +48,7 @@ impl DiaryViewer { /// View all entries from the iterator, or stop immediately if an error occurs, returning that /// error. - pub fn view_entries<'a, I: Iterator>>(&self, entries: I) -> Result<()> { + pub fn view_entries<'a, I: Iterator>>(&self, entries: I) -> Result<()> { for entry in entries { match entry.diary_id() { Ok(id) => println!("{} :\n", id), From 31fa07d43acf2684c9ea43a4e65a4a8e44af4ee0 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 29 Aug 2017 10:52:22 +0200 Subject: [PATCH 14/25] Add another small helper --- lib/domain/libimagdiary/src/is_in_diary.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/domain/libimagdiary/src/is_in_diary.rs b/lib/domain/libimagdiary/src/is_in_diary.rs index 09228072..8b48e1cc 100644 --- a/lib/domain/libimagdiary/src/is_in_diary.rs +++ b/lib/domain/libimagdiary/src/is_in_diary.rs @@ -24,6 +24,8 @@ pub trait IsInDiary { fn is_in_diary(&self, name: &str) -> bool; + fn is_a_diary_entry(&self) -> bool; + } impl IsInDiary for Entry { @@ -32,6 +34,10 @@ impl IsInDiary for Entry { self.get_location().clone().is_in_diary(name) } + fn is_a_diary_entry(&self) -> bool { + self.get_location().clone().is_a_diary_entry() + } + } impl IsInDiary for StoreId { @@ -40,5 +46,9 @@ impl IsInDiary for StoreId { self.local().starts_with(format!("diary/{}", name)) } + fn is_a_diary_entry(&self) -> bool { + self.local().starts_with("diary") + } + } From 21c15ca2070b35ea91b52208e8a1217404fd8458 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 29 Aug 2017 16:04:11 +0200 Subject: [PATCH 15/25] Adapt imag-diary to compile with the new libimagdiary API --- bin/domain/imag-diary/src/create.rs | 45 ++++++++++++++++------------ bin/domain/imag-diary/src/delete.rs | 27 ++++++++++++----- bin/domain/imag-diary/src/edit.rs | 35 ++++++++++++++++------ bin/domain/imag-diary/src/list.rs | 4 +-- bin/domain/imag-diary/src/view.rs | 3 +- lib/domain/libimagdiary/src/diary.rs | 2 +- 6 files changed, 74 insertions(+), 42 deletions(-) diff --git a/bin/domain/imag-diary/src/create.rs b/bin/domain/imag-diary/src/create.rs index 78f679a7..61b892b6 100644 --- a/bin/domain/imag-diary/src/create.rs +++ b/bin/domain/imag-diary/src/create.rs @@ -26,9 +26,10 @@ use libimagdiary::error::MapErrInto; use libimagentryedit::edit::Edit; use libimagrt::runtime::Runtime; use libimagerror::trace::trace_error; -use libimagdiary::entry::Entry; -use libimagdiary::result::Result; +use libimagerror::trace::trace_error_exit; use libimagutil::warn_exit::warn_exit; +use libimagstore::store::FileLockEntry; +use libimagstore::store::Store; use util::get_diary_name; @@ -38,18 +39,18 @@ pub fn create(rt: &Runtime) { let prevent_edit = rt.cli().subcommand_matches("create").unwrap().is_present("no-edit"); - fn create_entry<'a>(diary: &'a Diary, rt: &Runtime) -> Result> { + fn create_entry<'a>(diary: &'a Store, diaryname: &str, rt: &Runtime) -> FileLockEntry<'a> { use std::str::FromStr; let create = rt.cli().subcommand_matches("create").unwrap(); - if !create.is_present("timed") { + let entry = if !create.is_present("timed") { debug!("Creating non-timed entry"); - diary.new_entry_today() + diary.new_entry_today(diaryname) } else { let id = match create.value_of("timed") { Some("h") | Some("hourly") => { debug!("Creating hourly-timed entry"); - let time = DiaryId::now(String::from(diary.name())); + let time = DiaryId::now(String::from(diaryname)); let hr = create .value_of("hour") .map(|v| { debug!("Creating hourly entry with hour = {:?}", v); v }) @@ -65,7 +66,7 @@ pub fn create(rt: &Runtime) { Some("m") | Some("minutely") => { debug!("Creating minutely-timed entry"); - let time = DiaryId::now(String::from(diary.name())); + let time = DiaryId::now(String::from(diaryname)); let hr = create .value_of("hour") .map(|h| { debug!("hour = {:?}", h); h }) @@ -98,21 +99,27 @@ pub fn create(rt: &Runtime) { None => warn_exit("Unexpected error, cannot continue", 1) }; - diary.new_entry_by_id(id) + diary.retrieve(id).map_err_into(DEK::StoreReadError) + }; + + match entry { + Err(e) => trace_error_exit(&e, 1), + Ok(e) => { + debug!("Created: {}", e.get_location()); + e + } } } - let diary = Diary::open(rt.store(), &diaryname[..]); - let res = create_entry(&diary, rt) - .and_then(|mut entry| { - if prevent_edit { - debug!("Not editing new diary entry"); - Ok(()) - } else { - debug!("Editing new diary entry"); - entry.edit_content(rt).map_err_into(DEK::DiaryEditError) - } - }); + let mut entry = create_entry(rt.store(), &diaryname, rt); + + let res = if prevent_edit { + debug!("Not editing new diary entry"); + Ok(()) + } else { + debug!("Editing new diary entry"); + entry.edit_content(rt).map_err_into(DEK::DiaryEditError) + }; if let Err(e) = res { trace_error(&e); diff --git a/bin/domain/imag-diary/src/delete.rs b/bin/domain/imag-diary/src/delete.rs index da44bc92..514700f7 100644 --- a/bin/domain/imag-diary/src/delete.rs +++ b/bin/domain/imag-diary/src/delete.rs @@ -17,15 +17,17 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // +use std::process::exit; + use chrono::naive::NaiveDateTime; -use libimagdiary::diary::Diary; use libimagdiary::diaryid::DiaryId; use libimagrt::runtime::Runtime; use libimagerror::trace::trace_error_exit; use libimagtimeui::datetime::DateTime; use libimagtimeui::parse::Parse; use libimagutil::warn_exit::warn_exit; +use libimagstore::storeid::IntoStoreId; use util::get_diary_name; @@ -35,9 +37,6 @@ pub fn delete(rt: &Runtime) { let diaryname = get_diary_name(rt) .unwrap_or_else(|| warn_exit("No diary selected. Use either the configuration file or the commandline option", 1)); - let diary = Diary::open(rt.store(), &diaryname[..]); - debug!("Diary opened: {:?}", diary); - let datetime : Option = rt .cli() .subcommand_matches("delete") @@ -48,8 +47,17 @@ pub fn delete(rt: &Runtime) { .map(|dt| dt.into()); let to_del = match datetime { - Some(dt) => Some(diary.retrieve(DiaryId::from_datetime(diaryname.clone(), dt))), - None => diary.get_youngest_entry(), + Some(dt) => { + let id = match DiaryId::from_datetime(diaryname.clone(), dt).into_storeid() { + Ok(id) => id, + Err(e) => trace_error_exit(&e, 1), + }; + Some(rt.store().retrieve(id)) + }, + None => { + warn!("Not deleting entries, because missing date/time specification"); + exit(1); + }, }; let to_del = match to_del { @@ -59,12 +67,15 @@ pub fn delete(rt: &Runtime) { None => warn_exit("No entry", 1) }; - if !ask_bool(&format!("Deleting {:?}", to_del.get_location())[..], Some(true)) { + let location = to_del.get_location().clone(); + drop(to_del); + + if !ask_bool(&format!("Deleting {:?}", location), Some(true)) { info!("Aborting delete action"); return; } - if let Err(e) = diary.delete_entry(to_del) { + if let Err(e) = rt.store().delete(location) { trace_error_exit(&e, 1) } diff --git a/bin/domain/imag-diary/src/edit.rs b/bin/domain/imag-diary/src/edit.rs index 1cd5f9e5..3f9e59cf 100644 --- a/bin/domain/imag-diary/src/edit.rs +++ b/bin/domain/imag-diary/src/edit.rs @@ -17,6 +17,8 @@ // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA // +use std::process::exit; + use chrono::naive::NaiveDateTime; use libimagdiary::diary::Diary; @@ -30,12 +32,13 @@ use libimagerror::into::IntoError; use libimagtimeui::datetime::DateTime; use libimagtimeui::parse::Parse; use libimagutil::warn_exit::warn_exit; +use libimagstore::storeid::IntoStoreId; +use libimagerror::trace::trace_error_exit; use util::get_diary_name; pub fn edit(rt: &Runtime) { let diaryname = get_diary_name(rt).unwrap_or_else(|| warn_exit("No diary name", 1)); - let diary = Diary::open(rt.store(), &diaryname[..]); let datetime : Option = rt .cli() @@ -46,16 +49,30 @@ pub fn edit(rt: &Runtime) { .map(|dt| dt.into()); let to_edit = match datetime { - Some(dt) => Some(diary.retrieve(DiaryId::from_datetime(diaryname.clone(), dt))), - None => diary.get_youngest_entry(), + Some(dt) => { + let id = match DiaryId::from_datetime(diaryname.clone(), dt).into_storeid() { + Ok(id) => id, + Err(e) => trace_error_exit(&e, 1), + }; + rt.store().get(id) + }, + None => match rt.store().get_youngest_entry_id(&diaryname) { + Some(Ok(id)) => match id.into_storeid() { + Ok(sid) => rt.store().get(sid), + Err(e) => trace_error_exit(&e, 1), + }, + Some(Err(e)) => trace_error_exit(&e, 1), + None => { + error!("No entries in diary. Aborting"); + exit(1) + } + } }; - match to_edit { - Some(Ok(mut e)) => e.edit_content(rt).map_err_into(DEK::IOError), - - Some(Err(e)) => Err(e), - None => Err(DEK::EntryNotInDiary.into_error()), - } + to_edit.map(|opte| match opte { + Some(mut e) => e.edit_content(rt).map_err_into(DEK::IOError), + None => Err(DEK::EntryNotInDiary.into_error()), + }) .map_err_trace().ok(); } diff --git a/bin/domain/imag-diary/src/list.rs b/bin/domain/imag-diary/src/list.rs index 4ed3123c..90a8dd96 100644 --- a/bin/domain/imag-diary/src/list.rs +++ b/bin/domain/imag-diary/src/list.rs @@ -42,9 +42,7 @@ pub fn list(rt: &Runtime) { .unwrap_or(String::from("<>")) } - let diary = Diary::open(rt.store(), &diaryname[..]); - debug!("Diary opened: {:?}", diary); - diary.entries() + Diary::entries(rt.store(), &diaryname) .and_then(|es| { debug!("Iterator for listing: {:?}", es); diff --git a/bin/domain/imag-diary/src/view.rs b/bin/domain/imag-diary/src/view.rs index 041a1fe0..1e60383b 100644 --- a/bin/domain/imag-diary/src/view.rs +++ b/bin/domain/imag-diary/src/view.rs @@ -27,10 +27,9 @@ use util::get_diary_name; pub fn view(rt: &Runtime) { let diaryname = get_diary_name(rt).unwrap_or_else(|| warn_exit("No diary name", 1)); - let diary = Diary::open(rt.store(), &diaryname[..]); let hdr = rt.cli().subcommand_matches("view").unwrap().is_present("show-header"); - diary.entries() + Diary::entries(rt.store(), &diaryname) .and_then(|entries| DV::new(hdr).view_entries(entries.into_iter().filter_map(Result::ok))) .map_err_trace() .ok(); diff --git a/lib/domain/libimagdiary/src/diary.rs b/lib/domain/libimagdiary/src/diary.rs index cf738e33..c67467cc 100644 --- a/lib/domain/libimagdiary/src/diary.rs +++ b/lib/domain/libimagdiary/src/diary.rs @@ -37,7 +37,7 @@ use result::Result; use iter::DiaryEntryIterator; use iter::DiaryNameIterator; -trait Diary { +pub trait Diary { // create or get a new entry for today fn new_entry_today(&self, diary_name: &str) -> Result; From 7e7cf8ecf83fef1dff15150bcca003e1eb9d89bf Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 29 Aug 2017 16:10:09 +0200 Subject: [PATCH 16/25] Refactor, minify delete() impl --- bin/domain/imag-diary/src/delete.rs | 41 +++++++++++------------------ 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/bin/domain/imag-diary/src/delete.rs b/bin/domain/imag-diary/src/delete.rs index 514700f7..17f24a91 100644 --- a/bin/domain/imag-diary/src/delete.rs +++ b/bin/domain/imag-diary/src/delete.rs @@ -37,45 +37,34 @@ pub fn delete(rt: &Runtime) { let diaryname = get_diary_name(rt) .unwrap_or_else(|| warn_exit("No diary selected. Use either the configuration file or the commandline option", 1)); - let datetime : Option = rt + let to_del_location = rt .cli() .subcommand_matches("delete") .unwrap() .value_of("datetime") .map(|dt| { debug!("DateTime = {:?}", dt); dt }) .and_then(DateTime::parse) - .map(|dt| dt.into()); - - let to_del = match datetime { - Some(dt) => { - let id = match DiaryId::from_datetime(diaryname.clone(), dt).into_storeid() { - Ok(id) => id, - Err(e) => trace_error_exit(&e, 1), - }; - Some(rt.store().retrieve(id)) - }, - None => { + .map(|dt| dt.into()) + .ok_or_else(|| { warn!("Not deleting entries, because missing date/time specification"); exit(1); - }, - }; + }) + .and_then(|dt: NaiveDateTime| { + DiaryId::from_datetime(diaryname.clone(), dt) + .into_storeid() + .map(|id| rt.store().retrieve(id)) + .unwrap_or_else(|e| trace_error_exit(&e, 1)) + }) + .unwrap_or_else(|e| trace_error_exit(&e, 1)) + .get_location() + .clone(); - let to_del = match to_del { - Some(Ok(e)) => e, - - Some(Err(e)) => trace_error_exit(&e, 1), - None => warn_exit("No entry", 1) - }; - - let location = to_del.get_location().clone(); - drop(to_del); - - if !ask_bool(&format!("Deleting {:?}", location), Some(true)) { + if !ask_bool(&format!("Deleting {:?}", to_del_location), Some(true)) { info!("Aborting delete action"); return; } - if let Err(e) = rt.store().delete(location) { + if let Err(e) = rt.store().delete(to_del_location) { trace_error_exit(&e, 1) } From 356c86fd511d3e4f683620bc0feb3fbff4144e35 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 29 Aug 2017 16:26:57 +0200 Subject: [PATCH 17/25] Refactor, minify create() impl --- bin/domain/imag-diary/src/create.rs | 153 ++++++++++++++-------------- 1 file changed, 75 insertions(+), 78 deletions(-) diff --git a/bin/domain/imag-diary/src/create.rs b/bin/domain/imag-diary/src/create.rs index 61b892b6..b93c2805 100644 --- a/bin/domain/imag-diary/src/create.rs +++ b/bin/domain/imag-diary/src/create.rs @@ -19,13 +19,14 @@ use std::process::exit; +use clap::ArgMatches; + use libimagdiary::diary::Diary; use libimagdiary::diaryid::DiaryId; use libimagdiary::error::DiaryErrorKind as DEK; use libimagdiary::error::MapErrInto; use libimagentryedit::edit::Edit; use libimagrt::runtime::Runtime; -use libimagerror::trace::trace_error; use libimagerror::trace::trace_error_exit; use libimagutil::warn_exit::warn_exit; use libimagstore::store::FileLockEntry; @@ -37,94 +38,90 @@ pub fn create(rt: &Runtime) { let diaryname = get_diary_name(rt) .unwrap_or_else( || warn_exit("No diary selected. Use either the configuration file or the commandline option", 1)); - let prevent_edit = rt.cli().subcommand_matches("create").unwrap().is_present("no-edit"); - - fn create_entry<'a>(diary: &'a Store, diaryname: &str, rt: &Runtime) -> FileLockEntry<'a> { - use std::str::FromStr; - - let create = rt.cli().subcommand_matches("create").unwrap(); - let entry = if !create.is_present("timed") { - debug!("Creating non-timed entry"); - diary.new_entry_today(diaryname) - } else { - let id = match create.value_of("timed") { - Some("h") | Some("hourly") => { - debug!("Creating hourly-timed entry"); - let time = DiaryId::now(String::from(diaryname)); - let hr = create - .value_of("hour") - .map(|v| { debug!("Creating hourly entry with hour = {:?}", v); v }) - .and_then(|s| { - FromStr::from_str(s) - .map_err(|_| warn!("Could not parse hour: '{}'", s)) - .ok() - }) - .unwrap_or(time.hour()); - - time.with_hour(hr).with_minute(0) - }, - - Some("m") | Some("minutely") => { - debug!("Creating minutely-timed entry"); - let time = DiaryId::now(String::from(diaryname)); - let hr = create - .value_of("hour") - .map(|h| { debug!("hour = {:?}", h); h }) - .and_then(|s| { - FromStr::from_str(s) - .map_err(|_| warn!("Could not parse hour: '{}'", s)) - .ok() - }) - .unwrap_or(time.hour()); - - let min = create - .value_of("minute") - .map(|m| { debug!("minute = {:?}", m); m }) - .and_then(|s| { - FromStr::from_str(s) - .map_err(|_| warn!("Could not parse minute: '{}'", s)) - .ok() - }) - .unwrap_or(time.minute()); - - time.with_hour(hr).with_minute(min) - }, - - Some(_) => { - warn!("Timed creation failed: Unknown spec '{}'", - create.value_of("timed").unwrap()); - exit(1); - }, - - None => warn_exit("Unexpected error, cannot continue", 1) - }; - - diary.retrieve(id).map_err_into(DEK::StoreReadError) - }; - - match entry { - Err(e) => trace_error_exit(&e, 1), - Ok(e) => { - debug!("Created: {}", e.get_location()); - e - } - } - } - let mut entry = create_entry(rt.store(), &diaryname, rt); - let res = if prevent_edit { + let res = if rt.cli().subcommand_matches("create").unwrap().is_present("no-edit") { debug!("Not editing new diary entry"); Ok(()) } else { debug!("Editing new diary entry"); - entry.edit_content(rt).map_err_into(DEK::DiaryEditError) + entry.edit_content(rt) + .map_err_into(DEK::DiaryEditError) }; if let Err(e) = res { - trace_error(&e); + trace_error_exit(&e, 1); } else { info!("Ok!"); } } +fn create_entry<'a>(diary: &'a Store, diaryname: &str, rt: &Runtime) -> FileLockEntry<'a> { + let create = rt.cli().subcommand_matches("create").unwrap(); + let entry = if !create.is_present("timed") { + debug!("Creating non-timed entry"); + diary.new_entry_today(diaryname) + } else { + let id = create_id_from_clispec(&create, &diaryname); + diary.retrieve(id).map_err_into(DEK::StoreReadError) + }; + + match entry { + Err(e) => trace_error_exit(&e, 1), + Ok(e) => { + debug!("Created: {}", e.get_location()); + e + } + } +} + + +fn create_id_from_clispec(create: &ArgMatches, diaryname: &str) -> DiaryId { + use std::str::FromStr; + + let get_hourly_id = |create: &ArgMatches| -> DiaryId { + let time = DiaryId::now(String::from(diaryname)); + let hr = create + .value_of("hour") + .map(|v| { debug!("Creating hourly entry with hour = {:?}", v); v }) + .and_then(|s| { + FromStr::from_str(s) + .map_err(|_| warn!("Could not parse hour: '{}'", s)) + .ok() + }) + .unwrap_or(time.hour()); + + time.with_hour(hr) + }; + + match create.value_of("timed") { + Some("h") | Some("hourly") => { + debug!("Creating hourly-timed entry"); + get_hourly_id(create) + }, + + Some("m") | Some("minutely") => { + let time = get_hourly_id(create); + let min = create + .value_of("minute") + .map(|m| { debug!("minute = {:?}", m); m }) + .and_then(|s| { + FromStr::from_str(s) + .map_err(|_| warn!("Could not parse minute: '{}'", s)) + .ok() + }) + .unwrap_or(time.minute()); + + time.with_minute(min) + }, + + Some(_) => { + warn!("Timed creation failed: Unknown spec '{}'", + create.value_of("timed").unwrap()); + exit(1); + }, + + None => warn_exit("Unexpected error, cannot continue", 1) + } +} + From 8071c4c72159e731c805770c64a8ef72f7651d94 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Tue, 29 Aug 2017 16:50:29 +0200 Subject: [PATCH 18/25] Refactor, minify edit() impl --- bin/domain/imag-diary/src/edit.rs | 53 +++++++++++++------------------ 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/bin/domain/imag-diary/src/edit.rs b/bin/domain/imag-diary/src/edit.rs index 3f9e59cf..c623e1fa 100644 --- a/bin/domain/imag-diary/src/edit.rs +++ b/bin/domain/imag-diary/src/edit.rs @@ -32,7 +32,6 @@ use libimagerror::into::IntoError; use libimagtimeui::datetime::DateTime; use libimagtimeui::parse::Parse; use libimagutil::warn_exit::warn_exit; -use libimagstore::storeid::IntoStoreId; use libimagerror::trace::trace_error_exit; use util::get_diary_name; @@ -40,40 +39,32 @@ use util::get_diary_name; pub fn edit(rt: &Runtime) { let diaryname = get_diary_name(rt).unwrap_or_else(|| warn_exit("No diary name", 1)); - let datetime : Option = rt - .cli() + rt.cli() .subcommand_matches("edit") .unwrap() .value_of("datetime") .and_then(DateTime::parse) - .map(|dt| dt.into()); - - let to_edit = match datetime { - Some(dt) => { - let id = match DiaryId::from_datetime(diaryname.clone(), dt).into_storeid() { - Ok(id) => id, - Err(e) => trace_error_exit(&e, 1), - }; - rt.store().get(id) - }, - None => match rt.store().get_youngest_entry_id(&diaryname) { - Some(Ok(id)) => match id.into_storeid() { - Ok(sid) => rt.store().get(sid), - Err(e) => trace_error_exit(&e, 1), - }, - Some(Err(e)) => trace_error_exit(&e, 1), - None => { - error!("No entries in diary. Aborting"); - exit(1) - } - } - }; - - to_edit.map(|opte| match opte { - Some(mut e) => e.edit_content(rt).map_err_into(DEK::IOError), - None => Err(DEK::EntryNotInDiary.into_error()), - }) - .map_err_trace().ok(); + .map(|dt| dt.into()) + .map(|dt: NaiveDateTime| DiaryId::from_datetime(diaryname.clone(), dt)) + .or_else(|| { + rt.store() + .get_youngest_entry_id(&diaryname) + .map(|optid| match optid { + Ok(id) => id, + Err(e) => trace_error_exit(&e, 1), + }) + }) + .ok_or_else(|| { + error!("No entries in diary. Aborting"); + exit(1) + }) + .and_then(|id| rt.store().get(id)) + .map(|opte| match opte { + Some(mut e) => e.edit_content(rt).map_err_into(DEK::IOError), + None => Err(DEK::EntryNotInDiary.into_error()), + }) + .map_err_trace() + .ok(); } From f3bb6d02d0a04e8897cf59253a2e7e7fb59401bc Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Sat, 2 Sep 2017 12:23:29 +0200 Subject: [PATCH 19/25] Change the Task(FileLockEntry) type to a trait --- lib/domain/libimagtodo/src/task.rs | 158 ++++++++--------------------- 1 file changed, 42 insertions(+), 116 deletions(-) diff --git a/lib/domain/libimagtodo/src/task.rs b/lib/domain/libimagtodo/src/task.rs index 3dbbd04c..260d1a3a 100644 --- a/lib/domain/libimagtodo/src/task.rs +++ b/lib/domain/libimagtodo/src/task.rs @@ -18,8 +18,8 @@ // use std::collections::BTreeMap; -use std::ops::{Deref, DerefMut}; use std::io::BufRead; +use std::marker::Sized; use std::result::Result as RResult; use toml::Value; @@ -29,31 +29,43 @@ use task_hookrs::task::Task as TTask; use task_hookrs::import::{import_task, import_tasks}; use libimagstore::store::{FileLockEntry, Store}; -use libimagstore::storeid::{IntoStoreId, StoreIdIterator, StoreId}; +use libimagstore::storeid::{IntoStoreId, StoreIdIterator}; use module_path::ModuleEntryPath; use error::{TodoErrorKind as TEK, MapErrInto}; use result::Result; /// Task struct containing a `FileLockEntry` -#[derive(Debug)] -pub struct Task<'a>(FileLockEntry<'a>); +pub trait Task<'a> { + fn import_task_from_reader(store: &'a Store, r: R) -> Result<(Self, String, Uuid)> + where Self: Sized; + fn get_task_from_import(store: &'a Store, r: R) -> Result> + where Self: Sized; + fn get_task_from_string(store: &'a Store, s: String) -> Result> + where Self: Sized; + fn get_task_from_uuid(store: &'a Store, uuid: Uuid) -> Result> + where Self: Sized; + fn retrieve_task_from_import(store: &'a Store, r: R) -> Result + where Self: Sized; + fn retrieve_task_from_string(store: &'a Store, s: String) -> Result + where Self: Sized; + fn delete_tasks_by_imports(store: &Store, r: R) -> Result<()>; + fn delete_task_by_uuid(store: &Store, uuid: Uuid) -> Result<()>; + fn all_tasks(store: &Store) -> Result; + fn new_from_twtask(store: &'a Store, task: TTask) -> Result + where Self: Sized; +} -impl<'a> Task<'a> { +impl<'a> Task<'a> for FileLockEntry<'a> { - /// Concstructs a new `Task` with a `FileLockEntry` - pub fn new(fle: FileLockEntry<'a>) -> Task<'a> { - Task(fle) - } - - pub fn import(store: &'a Store, mut r: R) -> Result<(Task<'a>, String, Uuid)> { + fn import_task_from_reader(store: &'a Store, mut r: R) -> Result<(FileLockEntry<'a>, String, Uuid)> { let mut line = String::new(); try!(r.read_line(&mut line).map_err_into(TEK::UTF8Error)); import_task(&line.as_str()) .map_err_into(TEK::ImportError) .and_then(|t| { let uuid = t.uuid().clone(); - t.into_task(store).map(|t| (t, line, uuid)) + Self::new_from_twtask(store, t).map(|t| (t, line, uuid)) }) } @@ -66,23 +78,21 @@ impl<'a> Task<'a> { /// * Ok(Err(String)) - where the String is the String read from the `r` parameter /// * Err(_) - where the error is an error that happened during evaluation /// - pub fn get_from_import(store: &'a Store, mut r: R) -> Result, String>> - where R: BufRead - { + fn get_task_from_import(store: &'a Store, mut r: R) -> Result, String>> { let mut line = String::new(); try!(r.read_line(&mut line).map_err_into(TEK::UTF8Error)); - Task::get_from_string(store, line) + Self::get_task_from_string(store, line) } /// Get a task from a String. The String is expected to contain the JSON-representation of the /// Task to get from the store (only the UUID really matters in this case) /// /// For an explanation on the return values see `Task::get_from_import()`. - pub fn get_from_string(store: &'a Store, s: String) -> Result, String>> { + fn get_task_from_string(store: &'a Store, s: String) -> Result, String>> { import_task(s.as_str()) .map_err_into(TEK::ImportError) .map(|t| t.uuid().clone()) - .and_then(|uuid| Task::get_from_uuid(store, uuid)) + .and_then(|uuid| Self::get_task_from_uuid(store, uuid)) .and_then(|o| match o { None => Ok(Err(s)), Some(t) => Ok(Ok(t)), @@ -92,35 +102,34 @@ impl<'a> Task<'a> { /// Get a task from an UUID. /// /// If there is no task with this UUID, this returns `Ok(None)`. - pub fn get_from_uuid(store: &'a Store, uuid: Uuid) -> Result>> { + fn get_task_from_uuid(store: &'a Store, uuid: Uuid) -> Result>> { ModuleEntryPath::new(format!("taskwarrior/{}", uuid)) .into_storeid() .and_then(|store_id| store.get(store_id)) - .map(|o| o.map(Task::new)) .map_err_into(TEK::StoreError) } /// Same as Task::get_from_import() but uses Store::retrieve() rather than Store::get(), to /// implicitely create the task if it does not exist. - pub fn retrieve_from_import(store: &'a Store, mut r: R) -> Result> { + fn retrieve_task_from_import(store: &'a Store, mut r: R) -> Result> { let mut line = String::new(); try!(r.read_line(&mut line).map_err_into(TEK::UTF8Error)); - Task::retrieve_from_string(store, line) + Self::retrieve_task_from_string(store, line) } /// Retrieve a task from a String. The String is expected to contain the JSON-representation of /// the Task to retrieve from the store (only the UUID really matters in this case) - pub fn retrieve_from_string(store: &'a Store, s: String) -> Result> { - Task::get_from_string(store, s) + fn retrieve_task_from_string(store: &'a Store, s: String) -> Result> { + Self::get_task_from_string(store, s) .and_then(|opt| match opt { Ok(task) => Ok(task), Err(string) => import_task(string.as_str()) .map_err_into(TEK::ImportError) - .and_then(|t| t.into_task(store)), + .and_then(|t| Self::new_from_twtask(store, t)), }) } - pub fn delete_by_imports(store: &Store, r: R) -> Result<()> { + fn delete_tasks_by_imports(store: &Store, r: R) -> Result<()> { use serde_json::ser::to_string as serde_to_string; use task_hookrs::status::TaskStatus; @@ -144,7 +153,7 @@ impl<'a> Task<'a> { // Here we check if the status of a task is deleted and if yes, we delete it // from the store. if *ttask.status() == TaskStatus::Deleted { - match Task::delete_by_uuid(store, *ttask.uuid()) { + match Self::delete_task_by_uuid(store, *ttask.uuid()) { Ok(_) => info!("Deleted task {}", *ttask.uuid()), Err(e) => return Err(e), } @@ -157,70 +166,23 @@ impl<'a> Task<'a> { Ok(()) } - pub fn delete_by_uuid(store: &Store, uuid: Uuid) -> Result<()> { + fn delete_task_by_uuid(store: &Store, uuid: Uuid) -> Result<()> { ModuleEntryPath::new(format!("taskwarrior/{}", uuid)) .into_storeid() .and_then(|id| store.delete(id)) .map_err_into(TEK::StoreError) } - pub fn all_as_ids(store: &Store) -> Result { + fn all_tasks(store: &Store) -> Result { store.retrieve_for_module("todo/taskwarrior") .map_err_into(TEK::StoreError) } - pub fn all(store: &Store) -> Result { - Task::all_as_ids(store) - .map(|iter| TaskIterator::new(store, iter)) - } - -} - -impl<'a> Deref for Task<'a> { - type Target = FileLockEntry<'a>; - - fn deref(&self) -> &FileLockEntry<'a> { - &self.0 - } - -} - -impl<'a> DerefMut for Task<'a> { - - fn deref_mut(&mut self) -> &mut FileLockEntry<'a> { - &mut self.0 - } - -} - -/// A trait to get a `libimagtodo::task::Task` out of the implementing object. -pub trait IntoTask<'a> { - - /// # Usage - /// ```ignore - /// use std::io::stdin; - /// - /// use task_hookrs::task::Task; - /// use task_hookrs::import::import; - /// use libimagstore::store::{Store, FileLockEntry}; - /// - /// if let Ok(task_hookrs_task) = import(stdin()) { - /// // Store is given at runtime - /// let task = task_hookrs_task.into_filelockentry(store); - /// println!("Task with uuid: {}", task.flentry.get_header().get("todo.uuid")); - /// } - /// ``` - fn into_task(self, store : &'a Store) -> Result>; - -} - -impl<'a> IntoTask<'a> for TTask { - - fn into_task(self, store : &'a Store) -> Result> { + fn new_from_twtask(store: &'a Store, task: TTask) -> Result> { use toml_query::read::TomlValueReadExt; use toml_query::set::TomlValueSetExt; - let uuid = self.uuid(); + let uuid = task.uuid(); ModuleEntryPath::new(format!("taskwarrior/{}", uuid)) .into_storeid() .map_err_into(TEK::StoreIdError) @@ -241,47 +203,11 @@ impl<'a> IntoTask<'a> for TTask { } // If none of the errors above have returned the function, everything is fine - Ok(Task::new(fle)) + Ok(fle) }) }) + } } -trait FromStoreId { - fn from_storeid<'a>(&'a Store, StoreId) -> Result>; -} - -impl<'a> FromStoreId for Task<'a> { - - fn from_storeid<'b>(store: &'b Store, id: StoreId) -> Result> { - store.retrieve(id) - .map_err_into(TEK::StoreError) - .map(Task::new) - } -} - -pub struct TaskIterator<'a> { - store: &'a Store, - iditer: StoreIdIterator, -} - -impl<'a> TaskIterator<'a> { - - pub fn new(store: &'a Store, iditer: StoreIdIterator) -> TaskIterator<'a> { - TaskIterator { - store: store, - iditer: iditer, - } - } - -} - -impl<'a> Iterator for TaskIterator<'a> { - type Item = Result>; - - fn next(&mut self) -> Option>> { - self.iditer.next().map(|id| Task::from_storeid(self.store, id)) - } -} - From ef92acb1b08a4d4d47779721c18b10fbb8ff82db Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Sat, 2 Sep 2017 13:29:49 +0200 Subject: [PATCH 20/25] Refactor imag-todo to work with the changes in libimagtodo --- bin/domain/imag-todo/Cargo.toml | 1 + bin/domain/imag-todo/src/main.rs | 50 ++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/bin/domain/imag-todo/Cargo.toml b/bin/domain/imag-todo/Cargo.toml index a2c1503d..fd77cb99 100644 --- a/bin/domain/imag-todo/Cargo.toml +++ b/bin/domain/imag-todo/Cargo.toml @@ -22,5 +22,6 @@ is-match = "0.1.*" version = "2.0.1" libimagrt = { version = "0.4.0", path = "../../../lib/core/libimagrt" } +libimagstore = { version = "0.4.0", path = "../../../lib/core/libimagstore" } libimagerror = { version = "0.4.0", path = "../../../lib/core/libimagerror" } libimagtodo = { version = "0.4.0", path = "../../../lib/domain/libimagtodo" } diff --git a/bin/domain/imag-todo/src/main.rs b/bin/domain/imag-todo/src/main.rs index 43545d2a..3254f5c5 100644 --- a/bin/domain/imag-todo/src/main.rs +++ b/bin/domain/imag-todo/src/main.rs @@ -25,6 +25,7 @@ extern crate toml_query; #[macro_use] extern crate version; extern crate libimagrt; +extern crate libimagstore; extern crate libimagerror; extern crate libimagtodo; @@ -35,6 +36,7 @@ use toml::Value; use libimagrt::runtime::Runtime; use libimagrt::setup::generate_runtime_setup; +use libimagstore::store::FileLockEntry; use libimagtodo::task::Task; use libimagerror::trace::{MapErrTrace, trace_error, trace_error_exit}; @@ -61,9 +63,9 @@ fn tw_hook(rt: &Runtime) { let subcmd = rt.cli().subcommand_matches("tw-hook").unwrap(); if subcmd.is_present("add") { let stdin = stdin(); - let stdin = stdin.lock(); // implements BufRead which is required for `Task::import()` + let stdin = stdin.lock(); // implements BufRead which is required for `FileLockEntry::import_task_from_reader()` - match Task::import(rt.store(), stdin) { + match FileLockEntry::import_task_from_reader(rt.store(), stdin) { Ok((_, line, uuid)) => println!("{}\nTask {} stored in imag", line, uuid), Err(e) => trace_error_exit(&e, 1), } @@ -71,7 +73,7 @@ fn tw_hook(rt: &Runtime) { // The used hook is "on-modify". This hook gives two json-objects // per usage und wants one (the second one) back. let stdin = stdin(); - Task::delete_by_imports(rt.store(), stdin.lock()).map_err_trace().ok(); + FileLockEntry::delete_tasks_by_imports(rt.store(), stdin.lock()).map_err_trace().ok(); } else { // Should not be possible, as one argument is required via // ArgGroup @@ -92,30 +94,34 @@ fn list(rt: &Runtime) { is_match!(e.kind(), &::toml_query::error::ErrorKind::IdentifierNotFoundInDocument(_)) }; - let res = Task::all(rt.store()) // get all tasks + let res = FileLockEntry::all_tasks(rt.store()) // get all tasks .map(|iter| { // and if this succeeded // filter out the ones were we can read the uuid - let uuids : Vec<_> = iter.filter_map(|t| match t { - Ok(v) => match v.get_header().read(&String::from("todo.uuid")) { - Ok(Some(&Value::String(ref u))) => Some(u.clone()), - Ok(Some(_)) => { - warn!("Header type error"); - None - }, - Ok(None) => { - warn!("Header missing field"); - None + let uuids : Vec<_> = iter.filter_map(|storeid| { + match rt.store().retrieve(storeid) { + Ok(fle) => { + match fle.get_header().read(&String::from("todo.uuid")) { + Ok(Some(&Value::String(ref u))) => Some(u.clone()), + Ok(Some(_)) => { + warn!("Header type error"); + None + }, + Ok(None) => { + warn!("Header missing field"); + None + }, + Err(e) => { + if !no_identifier(&e) { + trace_error(&e); + } + None + } + } }, Err(e) => { - if !no_identifier(&e) { - trace_error(&e); - } + trace_error(&e); None - } - }, - Err(e) => { - trace_error(&e); - None + }, } }) .collect(); From 58a2729da010b81b7f9894013b867c325f922272 Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Sat, 2 Sep 2017 14:30:27 +0200 Subject: [PATCH 21/25] Changed Task trait to TaskStore trait, implemented for the Store instead of the FileLockEntry --- lib/domain/libimagtodo/src/task.rs | 74 +++++++++++++----------------- 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/lib/domain/libimagtodo/src/task.rs b/lib/domain/libimagtodo/src/task.rs index 260d1a3a..8e8cd9a5 100644 --- a/lib/domain/libimagtodo/src/task.rs +++ b/lib/domain/libimagtodo/src/task.rs @@ -19,7 +19,6 @@ use std::collections::BTreeMap; use std::io::BufRead; -use std::marker::Sized; use std::result::Result as RResult; use toml::Value; @@ -36,36 +35,29 @@ use error::{TodoErrorKind as TEK, MapErrInto}; use result::Result; /// Task struct containing a `FileLockEntry` -pub trait Task<'a> { - fn import_task_from_reader(store: &'a Store, r: R) -> Result<(Self, String, Uuid)> - where Self: Sized; - fn get_task_from_import(store: &'a Store, r: R) -> Result> - where Self: Sized; - fn get_task_from_string(store: &'a Store, s: String) -> Result> - where Self: Sized; - fn get_task_from_uuid(store: &'a Store, uuid: Uuid) -> Result> - where Self: Sized; - fn retrieve_task_from_import(store: &'a Store, r: R) -> Result - where Self: Sized; - fn retrieve_task_from_string(store: &'a Store, s: String) -> Result - where Self: Sized; - fn delete_tasks_by_imports(store: &Store, r: R) -> Result<()>; - fn delete_task_by_uuid(store: &Store, uuid: Uuid) -> Result<()>; - fn all_tasks(store: &Store) -> Result; - fn new_from_twtask(store: &'a Store, task: TTask) -> Result - where Self: Sized; +pub trait TaskStore<'a> { + fn import_task_from_reader(&'a self, r: R) -> Result<(FileLockEntry<'a>, String, Uuid)>; + fn get_task_from_import(&'a self, r: R) -> Result, String>>; + fn get_task_from_string(&'a self, s: String) -> Result, String>>; + fn get_task_from_uuid(&'a self, uuid: Uuid) -> Result>>; + fn retrieve_task_from_import(&'a self, r: R) -> Result>; + fn retrieve_task_from_string(&'a self, s: String) -> Result>; + fn delete_tasks_by_imports(&self, r: R) -> Result<()>; + fn delete_task_by_uuid(&self, uuid: Uuid) -> Result<()>; + fn all_tasks(&self) -> Result; + fn new_from_twtask(&'a self, task: TTask) -> Result>; } -impl<'a> Task<'a> for FileLockEntry<'a> { +impl<'a> TaskStore<'a> for Store { - fn import_task_from_reader(store: &'a Store, mut r: R) -> Result<(FileLockEntry<'a>, String, Uuid)> { + fn import_task_from_reader(&'a self, mut r: R) -> Result<(FileLockEntry<'a>, String, Uuid)> { let mut line = String::new(); try!(r.read_line(&mut line).map_err_into(TEK::UTF8Error)); import_task(&line.as_str()) .map_err_into(TEK::ImportError) .and_then(|t| { let uuid = t.uuid().clone(); - Self::new_from_twtask(store, t).map(|t| (t, line, uuid)) + self.new_from_twtask(t).map(|t| (t, line, uuid)) }) } @@ -78,21 +70,21 @@ impl<'a> Task<'a> for FileLockEntry<'a> { /// * Ok(Err(String)) - where the String is the String read from the `r` parameter /// * Err(_) - where the error is an error that happened during evaluation /// - fn get_task_from_import(store: &'a Store, mut r: R) -> Result, String>> { + fn get_task_from_import(&'a self, mut r: R) -> Result, String>> { let mut line = String::new(); try!(r.read_line(&mut line).map_err_into(TEK::UTF8Error)); - Self::get_task_from_string(store, line) + self.get_task_from_string(line) } /// Get a task from a String. The String is expected to contain the JSON-representation of the /// Task to get from the store (only the UUID really matters in this case) /// /// For an explanation on the return values see `Task::get_from_import()`. - fn get_task_from_string(store: &'a Store, s: String) -> Result, String>> { + fn get_task_from_string(&'a self, s: String) -> Result, String>> { import_task(s.as_str()) .map_err_into(TEK::ImportError) .map(|t| t.uuid().clone()) - .and_then(|uuid| Self::get_task_from_uuid(store, uuid)) + .and_then(|uuid| self.get_task_from_uuid(uuid)) .and_then(|o| match o { None => Ok(Err(s)), Some(t) => Ok(Ok(t)), @@ -102,34 +94,34 @@ impl<'a> Task<'a> for FileLockEntry<'a> { /// Get a task from an UUID. /// /// If there is no task with this UUID, this returns `Ok(None)`. - fn get_task_from_uuid(store: &'a Store, uuid: Uuid) -> Result>> { + fn get_task_from_uuid(&'a self, uuid: Uuid) -> Result>> { ModuleEntryPath::new(format!("taskwarrior/{}", uuid)) .into_storeid() - .and_then(|store_id| store.get(store_id)) + .and_then(|store_id| self.get(store_id)) .map_err_into(TEK::StoreError) } /// Same as Task::get_from_import() but uses Store::retrieve() rather than Store::get(), to /// implicitely create the task if it does not exist. - fn retrieve_task_from_import(store: &'a Store, mut r: R) -> Result> { + fn retrieve_task_from_import(&'a self, mut r: R) -> Result> { let mut line = String::new(); try!(r.read_line(&mut line).map_err_into(TEK::UTF8Error)); - Self::retrieve_task_from_string(store, line) + self.retrieve_task_from_string(line) } /// Retrieve a task from a String. The String is expected to contain the JSON-representation of /// the Task to retrieve from the store (only the UUID really matters in this case) - fn retrieve_task_from_string(store: &'a Store, s: String) -> Result> { - Self::get_task_from_string(store, s) + fn retrieve_task_from_string(&'a self, s: String) -> Result> { + self.get_task_from_string(s) .and_then(|opt| match opt { Ok(task) => Ok(task), Err(string) => import_task(string.as_str()) .map_err_into(TEK::ImportError) - .and_then(|t| Self::new_from_twtask(store, t)), + .and_then(|t| self.new_from_twtask(t)), }) } - fn delete_tasks_by_imports(store: &Store, r: R) -> Result<()> { + fn delete_tasks_by_imports(&self, r: R) -> Result<()> { use serde_json::ser::to_string as serde_to_string; use task_hookrs::status::TaskStatus; @@ -153,7 +145,7 @@ impl<'a> Task<'a> for FileLockEntry<'a> { // Here we check if the status of a task is deleted and if yes, we delete it // from the store. if *ttask.status() == TaskStatus::Deleted { - match Self::delete_task_by_uuid(store, *ttask.uuid()) { + match self.delete_task_by_uuid(*ttask.uuid()) { Ok(_) => info!("Deleted task {}", *ttask.uuid()), Err(e) => return Err(e), } @@ -166,19 +158,19 @@ impl<'a> Task<'a> for FileLockEntry<'a> { Ok(()) } - fn delete_task_by_uuid(store: &Store, uuid: Uuid) -> Result<()> { + fn delete_task_by_uuid(&self, uuid: Uuid) -> Result<()> { ModuleEntryPath::new(format!("taskwarrior/{}", uuid)) .into_storeid() - .and_then(|id| store.delete(id)) + .and_then(|id| self.delete(id)) .map_err_into(TEK::StoreError) } - fn all_tasks(store: &Store) -> Result { - store.retrieve_for_module("todo/taskwarrior") + fn all_tasks(&self) -> Result { + self.retrieve_for_module("todo/taskwarrior") .map_err_into(TEK::StoreError) } - fn new_from_twtask(store: &'a Store, task: TTask) -> Result> { + fn new_from_twtask(&'a self, task: TTask) -> Result> { use toml_query::read::TomlValueReadExt; use toml_query::set::TomlValueSetExt; @@ -187,7 +179,7 @@ impl<'a> Task<'a> for FileLockEntry<'a> { .into_storeid() .map_err_into(TEK::StoreIdError) .and_then(|id| { - store.retrieve(id) + self.retrieve(id) .map_err_into(TEK::StoreError) .and_then(|mut fle| { { From 0f317858e274d92469589feaafc8aabe3250705b Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Sat, 2 Sep 2017 14:34:03 +0200 Subject: [PATCH 22/25] Refactor imag-todo to work with the TaskStore trait instead of Task --- bin/domain/imag-todo/Cargo.toml | 1 - bin/domain/imag-todo/src/main.rs | 10 ++++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/bin/domain/imag-todo/Cargo.toml b/bin/domain/imag-todo/Cargo.toml index fd77cb99..a2c1503d 100644 --- a/bin/domain/imag-todo/Cargo.toml +++ b/bin/domain/imag-todo/Cargo.toml @@ -22,6 +22,5 @@ is-match = "0.1.*" version = "2.0.1" libimagrt = { version = "0.4.0", path = "../../../lib/core/libimagrt" } -libimagstore = { version = "0.4.0", path = "../../../lib/core/libimagstore" } libimagerror = { version = "0.4.0", path = "../../../lib/core/libimagerror" } libimagtodo = { version = "0.4.0", path = "../../../lib/domain/libimagtodo" } diff --git a/bin/domain/imag-todo/src/main.rs b/bin/domain/imag-todo/src/main.rs index 3254f5c5..dd226a3f 100644 --- a/bin/domain/imag-todo/src/main.rs +++ b/bin/domain/imag-todo/src/main.rs @@ -25,7 +25,6 @@ extern crate toml_query; #[macro_use] extern crate version; extern crate libimagrt; -extern crate libimagstore; extern crate libimagerror; extern crate libimagtodo; @@ -36,8 +35,7 @@ use toml::Value; use libimagrt::runtime::Runtime; use libimagrt::setup::generate_runtime_setup; -use libimagstore::store::FileLockEntry; -use libimagtodo::task::Task; +use libimagtodo::task::TaskStore; use libimagerror::trace::{MapErrTrace, trace_error, trace_error_exit}; mod ui; @@ -65,7 +63,7 @@ fn tw_hook(rt: &Runtime) { let stdin = stdin(); let stdin = stdin.lock(); // implements BufRead which is required for `FileLockEntry::import_task_from_reader()` - match FileLockEntry::import_task_from_reader(rt.store(), stdin) { + match rt.store().import_task_from_reader(stdin) { Ok((_, line, uuid)) => println!("{}\nTask {} stored in imag", line, uuid), Err(e) => trace_error_exit(&e, 1), } @@ -73,7 +71,7 @@ fn tw_hook(rt: &Runtime) { // The used hook is "on-modify". This hook gives two json-objects // per usage und wants one (the second one) back. let stdin = stdin(); - FileLockEntry::delete_tasks_by_imports(rt.store(), stdin.lock()).map_err_trace().ok(); + rt.store().delete_tasks_by_imports(stdin.lock()).map_err_trace().ok(); } else { // Should not be possible, as one argument is required via // ArgGroup @@ -94,7 +92,7 @@ fn list(rt: &Runtime) { is_match!(e.kind(), &::toml_query::error::ErrorKind::IdentifierNotFoundInDocument(_)) }; - let res = FileLockEntry::all_tasks(rt.store()) // get all tasks + let res = rt.store().all_tasks() // get all tasks .map(|iter| { // and if this succeeded // filter out the ones were we can read the uuid let uuids : Vec<_> = iter.filter_map(|storeid| { From 72ea21ee1f85451cd84245746606822279ad19c8 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 2 Sep 2017 21:48:31 +0200 Subject: [PATCH 23/25] Rename module task -> taskstore --- bin/domain/imag-todo/src/main.rs | 2 +- lib/domain/libimagtodo/src/lib.rs | 2 +- lib/domain/libimagtodo/src/{task.rs => taskstore.rs} | 0 3 files changed, 2 insertions(+), 2 deletions(-) rename lib/domain/libimagtodo/src/{task.rs => taskstore.rs} (100%) diff --git a/bin/domain/imag-todo/src/main.rs b/bin/domain/imag-todo/src/main.rs index dd226a3f..223926c6 100644 --- a/bin/domain/imag-todo/src/main.rs +++ b/bin/domain/imag-todo/src/main.rs @@ -35,7 +35,7 @@ use toml::Value; use libimagrt::runtime::Runtime; use libimagrt::setup::generate_runtime_setup; -use libimagtodo::task::TaskStore; +use libimagtodo::taskstore::TaskStore; use libimagerror::trace::{MapErrTrace, trace_error, trace_error_exit}; mod ui; diff --git a/lib/domain/libimagtodo/src/lib.rs b/lib/domain/libimagtodo/src/lib.rs index 7961a7a1..ee85dab5 100644 --- a/lib/domain/libimagtodo/src/lib.rs +++ b/lib/domain/libimagtodo/src/lib.rs @@ -47,5 +47,5 @@ module_entry_path_mod!("todo"); pub mod error; pub mod result; -pub mod task; +pub mod taskstore; diff --git a/lib/domain/libimagtodo/src/task.rs b/lib/domain/libimagtodo/src/taskstore.rs similarity index 100% rename from lib/domain/libimagtodo/src/task.rs rename to lib/domain/libimagtodo/src/taskstore.rs From 39dd9e8d7c7a2bb86c5ca1c17ca3cd99ce642d7c Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 2 Sep 2017 22:03:12 +0200 Subject: [PATCH 24/25] Add Task trait for getting UUID from task --- lib/domain/libimagtodo/src/error.rs | 5 ++- lib/domain/libimagtodo/src/lib.rs | 1 + lib/domain/libimagtodo/src/task.rs | 47 +++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 lib/domain/libimagtodo/src/task.rs diff --git a/lib/domain/libimagtodo/src/error.rs b/lib/domain/libimagtodo/src/error.rs index 621d3249..de39049f 100644 --- a/lib/domain/libimagtodo/src/error.rs +++ b/lib/domain/libimagtodo/src/error.rs @@ -23,7 +23,10 @@ generate_error_module!( StoreError => "Store Error", StoreIdError => "Store Id handling error", ImportError => "Error importing", - UTF8Error => "Encountered non-UTF8 characters while reading input" + UTF8Error => "Encountered non-UTF8 characters while reading input", + HeaderFieldMissing => "Header field missing", + HeaderTypeError => "Header field type error", + UuidParserError => "Uuid parser error" ); ); diff --git a/lib/domain/libimagtodo/src/lib.rs b/lib/domain/libimagtodo/src/lib.rs index ee85dab5..6c0c482e 100644 --- a/lib/domain/libimagtodo/src/lib.rs +++ b/lib/domain/libimagtodo/src/lib.rs @@ -47,5 +47,6 @@ module_entry_path_mod!("todo"); pub mod error; pub mod result; +pub mod task; pub mod taskstore; diff --git a/lib/domain/libimagtodo/src/task.rs b/lib/domain/libimagtodo/src/task.rs new file mode 100644 index 00000000..b2b70696 --- /dev/null +++ b/lib/domain/libimagtodo/src/task.rs @@ -0,0 +1,47 @@ +// +// imag - the personal information management suite for the commandline +// Copyright (C) 2015, 2016 Matthias Beyer and contributors +// +// This library is free software; you can redistribute it and/or +// modify it under the terms of the GNU Lesser General Public +// License as published by the Free Software Foundation; version +// 2.1 of the License. +// +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public +// License along with this library; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +// + +use error::TodoErrorKind as TEK; +use error::MapErrInto; +use result::Result; + +use libimagstore::store::Entry; +use libimagerror::into::IntoError; + +use uuid::Uuid; +use toml::Value; +use toml_query::read::TomlValueReadExt; + +pub trait Task { + fn get_uuid(&self) -> Result; +} + +impl Task for Entry { + fn get_uuid(&self) -> Result { + match self.get_header().read("todo.uuid") { + Ok(Some(&Value::String(ref uuid))) => { + Uuid::parse_str(uuid).map_err_into(TEK::UuidParserError) + }, + Ok(Some(_)) => Err(TEK::HeaderTypeError.into_error()), + Ok(None) => Err(TEK::HeaderFieldMissing.into_error()), + Err(e) => Err(e).map_err_into(TEK::StoreError), + } + } +} + From 51650bf043cfeb56acca1dda52517033af23a900 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Sep 2017 16:24:50 +0200 Subject: [PATCH 25/25] Small embellishments Fix false documentation, make warn!() into error!() and provide more information in the logged message. --- bin/domain/imag-todo/src/main.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/bin/domain/imag-todo/src/main.rs b/bin/domain/imag-todo/src/main.rs index 223926c6..20ad60e4 100644 --- a/bin/domain/imag-todo/src/main.rs +++ b/bin/domain/imag-todo/src/main.rs @@ -61,7 +61,9 @@ fn tw_hook(rt: &Runtime) { let subcmd = rt.cli().subcommand_matches("tw-hook").unwrap(); if subcmd.is_present("add") { let stdin = stdin(); - let stdin = stdin.lock(); // implements BufRead which is required for `FileLockEntry::import_task_from_reader()` + + // implements BufRead which is required for `Store::import_task_from_reader()` + let stdin = stdin.lock(); match rt.store().import_task_from_reader(stdin) { Ok((_, line, uuid)) => println!("{}\nTask {} stored in imag", line, uuid), @@ -101,11 +103,12 @@ fn list(rt: &Runtime) { match fle.get_header().read(&String::from("todo.uuid")) { Ok(Some(&Value::String(ref u))) => Some(u.clone()), Ok(Some(_)) => { - warn!("Header type error"); + error!("Header type error, expected String at 'todo.uuid' in {}", + fle.get_location()); None }, Ok(None) => { - warn!("Header missing field"); + error!("Header missing field in {}", fle.get_location()); None }, Err(e) => {