matthiasbeyer/imag
1
0
Fork 0
Code Releases Activity

Merge branch 'minor'

Browse source 
This merges some important fixes:

* libimagstore: Remove calls to filesystem-accessing functions but use
  abstractions instead.
* Make Debug for FileLockEntry more verbose
* Fix In-Memory test backend bug where the backend did not remove the
  old entry on "move"

as well as some nice formatting stuff and refactorings to simplify code
and similar improvements.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
This commit is contained in:
Matthias Beyer 2018-11-06 20:25:15 +01:00
commit 7fd3350146
13 changed files with 166 additions and 198 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
bin/domain/imag-contact/src/main.rs
View file

@ -119,14 +119,11 @@ fn list(rt: &Runtime) {
.all_contacts()
.map_err_trace_exit_unwrap(1)
.into_get_iter(rt.store())
.map(|fle| {
let fle = fle
.map_err_trace_exit_unwrap(1)
.ok_or_else(|| Error::from(err_msg("StoreId not found".to_owned())))
.map_err_trace_exit_unwrap(1);
fle.deser().map_err_trace_exit_unwrap(1)
})
.trace_unwrap_exit(1)
.map(|fle| fle.ok_or_else(|| Error::from(err_msg("StoreId not found".to_owned()))))
.trace_unwrap_exit(1)
.map(|e| e.deser())
.trace_unwrap_exit(1)
.enumerate();
if scmd.is_present("json") {
@ -140,21 +137,19 @@ fn list(rt: &Runtime) {
}
}
} else {
iterator
.map(|(i, deservcard)| {
let data = build_data_object_for_handlebars(i, &deservcard);
let rendered = iterator
.map(|(i, dvcard)| build_data_object_for_handlebars(i, &dvcard))
.map(|data| list_format.render("format", &data).map_err(Error::from))
.trace_unwrap_exit(1)
.collect::<Vec<String>>();
// collect, so that we can have rendered all the things and printing is faster.
list_format.render("format", &data)
.map_err(Error::from)
.map_err_trace_exit_unwrap(1)
})
let output = rt.stdout();
let mut output = output.lock();
// collect, so that we can have rendered all the things and printing is faster.
.collect::<Vec<String>>()
.into_iter()
.for_each(|s| {
writeln!(rt.stdout(), "{}", s).to_exit_code().unwrap_or_exit()
});
rendered.into_iter().for_each(|s| {
writeln!(output, "{}", s).to_exit_code().unwrap_or_exit()
});
}
}

17
bin/domain/imag-diary/src/delete.rs
View file

@ -19,7 +19,7 @@
use std::process::exit;
use chrono::naive::NaiveDateTime;
use chrono::naive::NaiveDateTime as NDT;
use libimagdiary::diaryid::DiaryId;
use libimagrt::runtime::Runtime;
@ -44,17 +44,10 @@ pub fn delete(rt: &Runtime) {
.value_of("datetime")
.map(|dt| { debug!("DateTime = {:?}", dt); dt })
.and_then(DateTime::parse)
.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))
.map_err_trace_exit_unwrap(1)
})
.map(Into::into)
.ok_or_else(|| warn_exit("Not deleting entries: missing date/time specification", 1))
.and_then(|dt: NDT| DiaryId::from_datetime(diaryname.clone(), dt).into_storeid())
.and_then(|id| rt.store().retrieve(id))
.map_err_trace_exit_unwrap(1)
.get_location()
.clone();

9
bin/domain/imag-diary/src/list.rs
View file

@ -51,10 +51,9 @@ pub fn list(rt: &Runtime) {
[id.year() as u32, id.month(), id.day(), id.hour(), id.minute(), id.second()]
});
for id in ids.into_iter().map(|id| id.into_storeid().map_err_trace_exit_unwrap(1)) {
writeln!(rt.stdout(), "{}", id)
.to_exit_code()
.unwrap_or_exit();
}
ids.into_iter()
.map(IntoStoreId::into_storeid)
.trace_unwrap_exit(1)
.for_each(|id| writeln!(rt.stdout(), "{}", id).to_exit_code().unwrap_or_exit());
}

25
bin/domain/imag-habit/src/main.rs
View file

@ -69,6 +69,7 @@ use libimagstore::store::FileLockEntry;
use libimagstore::store::Store;
use libimagstore::storeid::StoreId;
use libimaginteraction::ask::ask_bool;
use libimagutil::debug_result::DebugResult;
mod ui;
@ -115,20 +116,16 @@ fn create(rt: &Runtime) {
let date = scmd.value_of("create-date").unwrap(); // safe by clap
let parsedate = |d, pname| match kairos_parse(d).map_err_trace_exit_unwrap(1) {
Parsed::TimeType(tt) => match tt.calculate() {
Ok(tt) => match tt.get_moment() {
Some(mom) => mom.date(),
None => {
debug!("TimeType yielded: '{:?}'", tt);
error!("Error: '{}' parameter does not yield a point in time", pname);
exit(1);
},
},
Err(e) => {
error!("Error: '{:?}'", e);
exit(1);
}
},
Parsed::TimeType(tt) => tt.calculate()
.map_dbg(|y| format!("TimeType yielded: '{:?}'", y))
.map_err_trace_exit_unwrap(1)
.get_moment()
.ok_or_else(|| {
error!("Error: '{}' parameter does not yield a point in time", pname);
exit(1)
})
.unwrap() // safe by above
.date(),
_ => {
error!("Error: '{}' parameter does not yield a point in time", pname);
exit(1);

10
lib/core/libimagrt/src/io.rs
View file

@ -68,13 +68,9 @@ impl Debug for OutputProxy {
impl OutputProxy {
pub fn lock(&self) -> LockedOutputProxy {
match *self {
OutputProxy::Out(ref r) => {
LockedOutputProxy::Out(r.lock())
},
OutputProxy::Err(ref r) => {
LockedOutputProxy::Err(r.lock())
},
OutputProxy::Sink => LockedOutputProxy::Sink,
OutputProxy::Out(ref r) => LockedOutputProxy::Out(r.lock()),
OutputProxy::Err(ref r) => LockedOutputProxy::Err(r.lock()),
OutputProxy::Sink => LockedOutputProxy::Sink,
}
}
}

146
lib/core/libimagrt/src/logger.rs
View file

@ -88,34 +88,12 @@ impl ImagLogger {
::libimaginteraction::format::register_all_format_helpers(&mut handlebars);
{
let fmt = aggregate_global_format_trace(config)?;
handlebars.register_template_string("TRACE", fmt)
.map_err(Error::from)
.context(err_msg("Handlebars template error"))?; // name must be uppercase
}
{
let fmt = aggregate_global_format_debug(config)?;
handlebars.register_template_string("DEBUG", fmt)
.map_err(Error::from)
.context(err_msg("Handlebars template error"))?; // name must be uppercase
}
{
let fmt = aggregate_global_format_info(config)?;
handlebars.register_template_string("INFO", fmt)
.map_err(Error::from)
.context(err_msg("Handlebars template error"))?; // name must be uppercase
}
{
let fmt = aggregate_global_format_warn(config)?;
handlebars.register_template_string("WARN", fmt)
.map_err(Error::from)
.context(err_msg("Handlebars template error"))?; // name must be uppercase
}
{
let fmt = aggregate_global_format_error(config)?;
handlebars.register_template_string("ERROR", fmt)
.map_err(Error::from)
.context(err_msg("Handlebars template error"))?; // name must be uppercase
use self::log_lvl_aggregate::*;
let _ = aggregate::<Trace>(&mut handlebars, config, "TRACE")?;
let _ = aggregate::<Debug>(&mut handlebars, config, "DEBUG")?;
let _ = aggregate::<Info>(&mut handlebars, config, "INFO")?;
let _ = aggregate::<Warn>(&mut handlebars, config, "WARN")?;
let _ = aggregate::<Error>(&mut handlebars, config, "ERROR")?;
}
Ok(ImagLogger {
@ -289,9 +267,8 @@ fn translate_destinations(raw: &Vec<Value>) -> Result<Vec<LogDestination>> {
acc.and_then(|mut v| {
let dest = val.as_str()
.ok_or_else(|| {
let path = "imag.logging.modules.<mod>.destinations";
let ty = "Array<String>";
Error::from(format_err!("Type error at {}, expected {}", path, ty))
let msg = "Type error at 'imag.logging.modules.<mod>.destinations', expected Array<String>";
Error::from(err_msg(msg))
})
.and_then(translate_destination)?;
v.push(dest);
@ -312,9 +289,8 @@ fn aggregate_global_destinations(matches: &ArgMatches, config: Option<&Value>)
.ok_or_else(|| err_msg("Global log destination config missing"))?
.as_array()
.ok_or_else(|| {
let path = "imag.logging.destinations";
let ty = "Array";
Error::from(format_err!("Type error at {}, expected {}", path, ty))
let msg = "Type error at 'imag.logging.destinations', expected 'Array'";
Error::from(err_msg(msg))
})
.and_then(translate_destinations),
None => {
@ -335,54 +311,55 @@ fn aggregate_global_destinations(matches: &ArgMatches, config: Option<&Value>)
}
}
macro_rules! aggregate_global_format {
($read_str:expr, $error_msg_if_missing:expr, $config:expr) => {
try!($config.ok_or_else(|| Error::from(err_msg($error_msg_if_missing))))
.read_string($read_str)
.map_err(Error::from)
.context(EM::TomlQueryError)?
.ok_or_else(|| Error::from(err_msg($error_msg_if_missing)))
};
}
mod log_lvl_aggregate {
use failure::Fallible as Result;
use failure::Error as E;
use failure::ResultExt;
use failure::err_msg;
use toml::Value;
use toml_query::read::TomlValueReadTypeExt;
use handlebars::Handlebars;
fn aggregate_global_format_trace(config: Option<&Value>)
-> Result<String>
{
aggregate_global_format!("imag.logging.format.trace",
"Config missing: Logging format: Trace",
config)
}
use libimagerror::errors::ErrorMsg as EM;
fn aggregate_global_format_debug(config: Option<&Value>)
-> Result<String>
{
aggregate_global_format!("imag.logging.format.debug",
"Config missing: Logging format: Debug",
config)
}
macro_rules! aggregate_global_format_with {
($t:ident, $read_str:expr) => {
pub struct $t;
impl LogLevelAggregator for $t {
fn aggregate(config: Option<&Value>) -> Result<String> {
config.ok_or_else(|| {
E::from(err_msg(concat!("Config missing: Logging format: ", stringify!($t))))
})?
.read_string($read_str)
.map_err(E::from)
.context(EM::TomlQueryError)?
.ok_or_else(|| {
E::from(err_msg(concat!("Config missing: Logging format: ", stringify!($t))))
})
}
}
};
}
fn aggregate_global_format_info(config: Option<&Value>)
-> Result<String>
{
aggregate_global_format!("imag.logging.format.info",
"Config missing: Logging format: Info",
config)
}
pub trait LogLevelAggregator {
fn aggregate(config: Option<&Value>) -> Result<String>;
}
fn aggregate_global_format_warn(config: Option<&Value>)
-> Result<String>
{
aggregate_global_format!("imag.logging.format.warn",
"Config missing: Logging format: Warn",
config)
}
pub fn aggregate<T: LogLevelAggregator>(hb: &mut Handlebars, config: Option<&Value>, lvlstr: &str)
-> Result<()>
{
hb.register_template_string(lvlstr, T::aggregate(config)?)
.map_err(E::from)
.context(err_msg("Handlebars template error"))
.map_err(E::from)
}
aggregate_global_format_with!(Trace, "imag.logging.format.trace");
aggregate_global_format_with!(Debug, "imag.logging.format.debug");
aggregate_global_format_with!(Info, "imag.logging.format.info");
aggregate_global_format_with!(Warn, "imag.logging.format.warn");
aggregate_global_format_with!(Error, "imag.logging.format.error");
fn aggregate_global_format_error(config: Option<&Value>)
-> Result<String>
{
aggregate_global_format!("imag.logging.format.error",
"Config missing: Logging format: Error",
config)
}
fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>)
@ -414,9 +391,8 @@ fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>)
.map(|val| {
val.as_array()
.ok_or_else(|| {
let path = "imag.logging.modules.<mod>.destinations";
let ty = "Array";
Error::from(format_err!("Type error at {}, expected {}", path, ty))
let msg = "Type error at 'imag.logging.modules.<mod>.destinations', expected 'Array'";
Error::from(err_msg(msg))
})
.and_then(translate_destinations)
})
@ -434,9 +410,8 @@ fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>)
.context(EM::TomlQueryError)?
.map(|v| v.as_bool().unwrap_or(false))
.ok_or_else(|| {
let path = "imag.logging.modules.<mod>.enabled";
let ty = "Boolean";
Error::from(format_err!("Type error at {}, expected {}", path, ty))
let msg = "Type error at 'imag.logging.modules.<mod>.enabled', expected 'Boolean'";
Error::from(err_msg(msg))
})?;
let module_settings = ModuleSettings {
@ -452,9 +427,8 @@ fn aggregate_module_settings(_matches: &ArgMatches, config: Option<&Value>)
Ok(settings)
},
Ok(Some(_)) => {
let path = "imag.logging.modules";
let ty = "Table";
Err(Error::from(format_err!("Type error at {}, expected {}", path, ty)))
let msg = "Type error at 'imag.logging.modules', expected 'Table'";
Err(Error::from(err_msg(msg)))
},
Ok(None) => {
// No modules configured. This is okay!

26
lib/core/libimagrt/src/runtime.rs
View file

@ -137,13 +137,11 @@ impl<'a> Runtime<'a> {
Store::new(storepath, &config)
};
store_result.map(|store| {
Runtime {
cli_matches: matches,
configuration: config,
rtp: rtp,
store: store,
}
store_result.map(|store| Runtime {
cli_matches: matches,
configuration: config,
rtp: rtp,
store: store,
})
.context(err_msg("Cannot instantiate runtime"))
.map_err(Error::from)
@ -390,15 +388,17 @@ impl<'a> Runtime<'a> {
self.cli()
.value_of("editor")
.map(String::from)
.or_else(|| {
.ok_or_else(|| {
self.config()
.and_then(|v| match v.read("rt.editor") {
Ok(Some(&Value::String(ref s))) => Some(s.clone()),
_ => None, // FIXME silently ignore errors in config is bad
.ok_or_else(|| Error::from(err_msg("No Configuration!")))
.and_then(|v| match v.read("rt.editor")? {
Some(&Value::String(ref s)) => Ok(Some(s.clone())),
Some(_) => Err(Error::from(err_msg("Type error at 'rt.editor', expected 'String'"))),
None => Ok(None),
})
})
.or(env::var("EDITOR").ok())
.ok_or_else(|| Error::from(EM::IO))
.or(env::var("EDITOR"))
.map_err(|_| Error::from(EM::IO))
.map_dbg(|s| format!("Editing with '{}'", s))
.and_then(|s| {
let mut split = s.split_whitespace();

34
lib/core/libimagstore/src/file_abstraction/fs.rs
View file

@ -105,15 +105,14 @@ impl FileAbstraction for FSFileAbstraction {
}
fn rename(&self, from: &PathBuf, to: &PathBuf) -> Result<()> {
match to.parent() {
Some(p) => if !p.exists() {
if let Some(p) = to.parent() {
if !p.exists() {
debug!("Creating: {:?}", p);
let _ = create_dir_all(&PathBuf::from(p)).context(EM::DirNotCreated)?;
},
None => {
debug!("Failed to find parent. This looks like it will fail now");
//nothing
},
let _ = create_dir_all(&p).context(EM::DirNotCreated)?;
}
} else {
debug!("Failed to find parent. This looks like it will fail now");
//nothing
}
debug!("Renaming {:?} to {:?}", from, to);
@ -149,10 +148,9 @@ impl FileAbstraction for FSFileAbstraction {
/// FileAbstraction::fill implementation that consumes the Drain and writes everything to the
/// filesystem
fn fill(&mut self, mut d: Drain) -> Result<()> {
d.iter()
.fold(Ok(()), |acc, (path, element)| {
acc.and_then(|_| self.new_instance(path).write_file_content(&element))
})
d.iter().fold(Ok(()), |acc, (path, element)| {
acc.and_then(|_| self.new_instance(path).write_file_content(&element))
})
}
fn pathes_recursively(&self,
@ -190,11 +188,9 @@ impl PathIterBuilder for WalkDirPathIterBuilder {
fn open_file<A: AsRef<Path>>(p: A) -> ::std::io::Result<Option<File>> {
match OpenOptions::new().write(true).read(true).open(p) {
Err(e) => {
match e.kind() {
::std::io::ErrorKind::NotFound => return Ok(None),
_ => return Err(e),
}
Err(e) => match e.kind() {
::std::io::ErrorKind::NotFound => return Ok(None),
_ => return Err(e),
},
Ok(file) => Ok(Some(file))
}
@ -205,9 +201,7 @@ fn create_file<A: AsRef<Path>>(p: A) -> ::std::io::Result<File> {
trace!("'{}' is directory = {}", parent.display(), parent.is_dir());
if !parent.is_dir() {
trace!("Implicitely creating directory: {:?}", parent);
if let Err(e) = create_dir_all(parent) {
return Err(e);
}
let _ = create_dir_all(parent)?;
}
}
OpenOptions::new().write(true).read(true).create(true).open(p)

2
lib/core/libimagstore/src/file_abstraction/inmemory.rs
View file

@ -140,7 +140,7 @@ impl FileAbstraction for InMemoryFileAbstraction {
let mut mtx = self.backend().lock().expect("Locking Mutex failed");
let backend = mtx.get_mut();
let a = backend.get(from).cloned().ok_or_else(|| EM::FileNotFound)?;
let a = backend.remove(from).ok_or_else(|| EM::FileNotFound)?;
backend.insert(to.clone(), a);
debug!("Renaming: {:?} -> {:?} worked", from, to);
Ok(())

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

@ -222,11 +222,13 @@ impl Store {
debug!("Creating id: '{}'", id);
let exists = id.exists()? || self.entries
.read()
.map(|map| map.contains_key(&id))
.map_err(|_| Error::from(EM::LockError))
.context(format_err!("CreateCallError: {}", id))?;
let exists =
self.entries
.read()
.map(|map| map.contains_key(&id))
.map_err(|_| Error::from(EM::LockError))
.context(format_err!("CreateCallError: {}", id))? ||
self.backend.exists(&id.clone().into_pathbuf()?)?;
if exists {
debug!("Entry exists: {:?}", id);
@ -304,11 +306,14 @@ impl Store {
debug!("Getting id: '{}'", id);
let exists = id.exists()? || self.entries
.read()
.map(|map| map.contains_key(&id))
.map_err(|_| Error::from(EM::LockError))
.context(format_err!("GetCallError: {}", id))?;
let exists =
self.entries
.read()
.map(|map| map.contains_key(&id))
.map_err(|_| Error::from(EM::LockError))
.context(format_err!("CreateCallError: {}", id))? ||
self.backend.exists(&id.clone().into_pathbuf()?)?;
if !exists {
debug!("Does not exist in internal cache or filesystem: {:?}", id);
@ -676,8 +681,10 @@ impl<'a> FileLockEntry<'a, > {
impl<'a> Debug for FileLockEntry<'a> {
fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FMTError> {
write!(fmt, "FileLockEntry(Store = {})", self.store.location.to_str()
.unwrap_or("Unknown Path"))
write!(fmt,
"FileLockEntry(Store = {store}, location = {location:?})",
store = self.store.location.to_str().unwrap_or("Unknown Path"),
location = self.entry.get_location())
}
}
@ -1214,10 +1221,13 @@ mod store_tests {
assert!(store.entries.read().unwrap().get(&id_mv_with_base).is_some());
}
assert!(match store.get(id.clone()) { Ok(None) => true, _ => false },
"Moved id ({:?}) is still there", id);
assert!(match store.get(id_mv.clone()) { Ok(Some(_)) => true, _ => false },
"New id ({:?}) is not in store...", id_mv);
let res = store.get(id.clone());
assert!(match res { Ok(None) => true, _ => false },
"Moved id ({:?}) is still there: {:?}", id, res);
let res = store.get(id_mv.clone());
assert!(match res { Ok(Some(_)) => true, _ => false },
"New id ({:?}) is not in store: {:?}", id_mv, res);
}
}
}

11
lib/core/libimagstore/src/util.rs
View file

@ -69,8 +69,14 @@ pub fn entry_buffer_to_header_content(buf: &str) -> Result<(Value, String)> {
#[cfg(test)]
mod test {
extern crate env_logger;
use super::entry_buffer_to_header_content;
fn setup_logging() {
let _ = env_logger::try_init();
}
fn mkfile(content: &str) -> String {
format!(r#"---
[imag]
@ -97,16 +103,17 @@ version = '{version}'
#[test]
fn test_entry_buffer_to_header_content_2() {
setup_logging();
let content = r#"Hai
"#;
let file = mkfile(&content);
eprintln!("FILE: <<<{}>>>", file);
debug!("FILE: <<<{}>>>", file);
let res = entry_buffer_to_header_content(&file);
assert!(res.is_ok());
let (_, res_content) = res.unwrap();
eprintln!("CONTENT: <<<{}>>>", res_content);
debug!("CONTENT: <<<{}>>>", res_content);
assert_eq!(res_content, content)
}

4
lib/domain/libimagmail/src/mail.rs
View file

@ -194,4 +194,8 @@ impl<'a> Mail<'a> {
self.get_field("In-Reply-To")
}
pub fn fle(&self) -> &FileLockEntry<'a> {
&self.0
}
}

1
lib/domain/libimagtodo/src/taskstore.rs
View file

@ -140,7 +140,6 @@ impl<'a> TaskStore<'a> for Store {
// the change. The (maybe modified) second one is
// expected by taskwarrior.
let val = serde_to_string(&ttask).context(err_msg("Import error"))?;
println!("{}", val);
// Taskwarrior does not have the concept of deleted tasks, but only modified
// ones.