From 8c37fb865abf30d8942c44182f95c172934a6fdd Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 2 Mar 2018 21:25:48 +0100 Subject: [PATCH 1/3] Rebuild DiaryEntryIterator to be based on StoreIdIterator This patch rebuilds DiaryEntryIterator to be a wrapper around StoreIdIterator and thus `Diary::entries()` to use `Store::entries` and not `Store::retrieve_for_module()`. The `Store::retrieve_for_module()` function is somehow buggy and loads contents of the files into memory and _somehow_ causes the entries to be rewritten without newlines. This bug is fixed by moving away from `Store::retrieve_for_module()`. --- lib/domain/libimagdiary/Cargo.toml | 1 + lib/domain/libimagdiary/src/diary.rs | 8 +-- lib/domain/libimagdiary/src/iter.rs | 89 +++++++++++++--------------- lib/domain/libimagdiary/src/lib.rs | 1 + 4 files changed, 47 insertions(+), 52 deletions(-) diff --git a/lib/domain/libimagdiary/Cargo.toml b/lib/domain/libimagdiary/Cargo.toml index d8f2e51a..1a07c17e 100644 --- a/lib/domain/libimagdiary/Cargo.toml +++ b/lib/domain/libimagdiary/Cargo.toml @@ -26,6 +26,7 @@ toml = "0.4" toml-query = "0.6" itertools = "0.7" error-chain = "0.11" +filters = "0.2" libimagstore = { version = "0.7.0", path = "../../../lib/core/libimagstore" } libimagerror = { version = "0.7.0", path = "../../../lib/core/libimagerror" } diff --git a/lib/domain/libimagdiary/src/diary.rs b/lib/domain/libimagdiary/src/diary.rs index 35d1adb4..292f0601 100644 --- a/lib/domain/libimagdiary/src/diary.rs +++ b/lib/domain/libimagdiary/src/diary.rs @@ -31,8 +31,8 @@ use chrono::naive::NaiveDateTime; use chrono::Timelike; use entry::IsDiaryEntry; -use entry::DiaryEntry; use diaryid::DiaryId; +use diaryid::FromStoreId; use error::DiaryErrorKind as DEK; use error::ResultExt; use error::Result; @@ -89,8 +89,8 @@ impl Diary for Store { // Get an iterator for iterating over all entries fn entries(&self, diary_name: &str) -> Result { debug!("Building iterator for module 'diary' with diary name = '{}'", diary_name); - self.retrieve_for_module("diary") - .map(|iter| DiaryEntryIterator::new(self, String::from(diary_name), iter)) + Store::entries(self) + .map(|iter| DiaryEntryIterator::new(String::from(diary_name), iter.without_store())) .chain_err(|| DEK::StoreReadError) } @@ -99,7 +99,7 @@ impl Diary for Store { Err(e) => Some(Err(e)), Ok(entries) => { entries - .map(|e| e.and_then(|e| e.diary_id())) + .map(|e| DiaryId::from_storeid(&e)) .sorted_by(|a, b| { match (a, b) { (&Ok(ref a), &Ok(ref b)) => { diff --git a/lib/domain/libimagdiary/src/iter.rs b/lib/domain/libimagdiary/src/iter.rs index 8480391d..b98746a0 100644 --- a/lib/domain/libimagdiary/src/iter.rs +++ b/lib/domain/libimagdiary/src/iter.rs @@ -20,13 +20,11 @@ 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 filters::filter::Filter; + +use libimagstore::storeid::StoreIdIterator; +use libimagstore::storeid::StoreId; -use diaryid::DiaryId; -use diaryid::FromStoreId; use is_in_diary::IsInDiary; use error::DiaryErrorKind as DEK; use error::DiaryError as DE; @@ -34,8 +32,7 @@ use error::ResultExt; use error::Result; /// A iterator for iterating over diary entries -pub struct DiaryEntryIterator<'a> { - store: &'a Store, +pub struct DiaryEntryIterator { name: String, iter: StoreIdIterator, @@ -44,7 +41,7 @@ pub struct DiaryEntryIterator<'a> { day: Option, } -impl<'a> Debug for DiaryEntryIterator<'a> { +impl Debug for DiaryEntryIterator { fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FmtError> { write!(fmt, "DiaryEntryIterator", @@ -53,11 +50,10 @@ impl<'a> Debug for DiaryEntryIterator<'a> { } -impl<'a> DiaryEntryIterator<'a> { +impl DiaryEntryIterator { - pub fn new(store: &'a Store, diaryname: String, iter: StoreIdIterator) -> DiaryEntryIterator<'a> { + pub fn new(diaryname: String, iter: StoreIdIterator) -> DiaryEntryIterator { DiaryEntryIterator { - store: store, name: diaryname, iter: iter, @@ -68,65 +64,62 @@ impl<'a> DiaryEntryIterator<'a> { } // Filter by year, get all diary entries for this year - pub fn year(mut self, year: i32) -> DiaryEntryIterator<'a> { + pub fn year(mut self, year: i32) -> DiaryEntryIterator { self.year = Some(year); self } // Filter by month, get all diary entries for this month (every year) - pub fn month(mut self, month: u32) -> DiaryEntryIterator<'a> { + pub fn month(mut self, month: u32) -> DiaryEntryIterator { self.month = Some(month); self } // Filter by day, get all diary entries for this day (every year, every year) - pub fn day(mut self, day: u32) -> DiaryEntryIterator<'a> { + pub fn day(mut self, day: u32) -> DiaryEntryIterator { self.day = Some(day); self } } -impl<'a> Iterator for DiaryEntryIterator<'a> { - type Item = Result>; +impl Filter for DiaryEntryIterator { + fn filter(&self, id: &StoreId) -> bool { + if id.is_in_diary(&self.name) { + match (self.year, self.month, self.day) { + (None , None , None) => true, + (Some(y) , None , None) => id.is_in_collection(&[&self.name, &y.to_string()]), + (Some(y) , Some(m) , None) => id.is_in_collection(&[&self.name, &y.to_string(), &m.to_string()]), + (Some(y) , Some(m) , Some(d)) => id.is_in_collection(&[&self.name, &y.to_string(), &m.to_string(), &d.to_string()]), + (None , Some(_) , Some(_)) => false /* invalid case */, + (None , None , Some(_)) => false /* invalid case */, + (None , Some(_) , None) => false /* invalid case */, + (Some(_) , None , Some(_)) => false /* invalid case */, + } + } else { + false + } + } +} + +impl Iterator for DiaryEntryIterator { + type Item = StoreId; fn next(&mut self) -> Option { loop { - let next = match self.iter.next() { - Some(s) => s, - None => return None, - }; - debug!("Next element: {:?}", next); - - if next.is_in_diary(&self.name) { - debug!("Seems to be in diary: {:?}", next); - let id = match DiaryId::from_storeid(&next) { - Ok(i) => i, - Err(e) => { - trace_error(&e); - debug!("Couldn't parse {:?} into DiaryId: {:?}", next, e); - continue; + match self.iter.next() { + None => return None, + Some(s) => { + debug!("Next element: {:?}", s); + if Filter::filter(self, &s) { + return Some(s) + } else { + continue } - }; - debug!("Success parsing id = {:?}", id); - - let y = match self.year { None => true, Some(y) => y == id.year() }; - let m = match self.month { None => true, Some(m) => m == id.month() }; - let d = match self.day { None => true, Some(d) => d == id.day() }; - - if y && m && d { - debug!("Return = {:?}", id); - return Some(self - .store - .retrieve(next) - .chain_err(|| DEK::StoreReadError)); - } - } else { - debug!("Not in the requested diary ({}): {:?}", self.name, next); + }, } } } - } diff --git a/lib/domain/libimagdiary/src/lib.rs b/lib/domain/libimagdiary/src/lib.rs index e0dba334..416d6e47 100644 --- a/lib/domain/libimagdiary/src/lib.rs +++ b/lib/domain/libimagdiary/src/lib.rs @@ -41,6 +41,7 @@ extern crate toml; extern crate toml_query; extern crate itertools; #[macro_use] extern crate error_chain; +extern crate filters; #[macro_use] extern crate libimagstore; #[macro_use] extern crate libimagentryutil; From affd15a890e233af193d92678b9c97ecfb897b0c Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 2 Mar 2018 21:28:40 +0100 Subject: [PATCH 2/3] Rewrite imag-diary for new Diary::entries() interface --- bin/domain/imag-diary/src/list.rs | 12 ++---------- bin/domain/imag-diary/src/view.rs | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/bin/domain/imag-diary/src/list.rs b/bin/domain/imag-diary/src/list.rs index 99409d3c..051fb028 100644 --- a/bin/domain/imag-diary/src/list.rs +++ b/bin/domain/imag-diary/src/list.rs @@ -38,16 +38,8 @@ pub fn list(rt: &Runtime) { Diary::entries(rt.store(), &diaryname) .map_dbg_str("Ok") .map_err_trace_exit_unwrap(1) - .filter_map(|entry| { - entry - .map_dbg(|e| format!("Filtering: {:?}", e)) - .map_err_trace() // error tracing here - .ok() // so we can ignore errors here - }) - .for_each(|e| { - writeln!(out, "{}", e - .get_location() - .clone() + .for_each(|id| { + writeln!(out, "{}", id .without_base() .to_str() .map_err_trace() diff --git a/bin/domain/imag-diary/src/view.rs b/bin/domain/imag-diary/src/view.rs index 7752c693..66a952c0 100644 --- a/bin/domain/imag-diary/src/view.rs +++ b/bin/domain/imag-diary/src/view.rs @@ -22,6 +22,7 @@ use libimagdiary::viewer::DiaryViewer as DV; use libimagrt::runtime::Runtime; use libimagerror::trace::MapErrTrace; use libimagutil::warn_exit::warn_exit; +use libimagstore::iter::get::StoreIdGetIteratorExtension; use util::get_diary_name; @@ -29,9 +30,16 @@ pub fn view(rt: &Runtime) { let diaryname = get_diary_name(rt).unwrap_or_else(|| warn_exit("No diary name", 1)); let hdr = rt.cli().subcommand_matches("view").unwrap().is_present("show-header"); - Diary::entries(rt.store(), &diaryname) - .and_then(|entries| DV::new(hdr).view_entries(entries.into_iter().filter_map(Result::ok))) - .map_err_trace() - .ok(); + let entries = Diary::entries(rt.store(), &diaryname) + .map_err_trace_exit_unwrap(1) + .into_get_iter(rt.store()) + .filter_map(Result::ok) + .map(|e| e.unwrap_or_else(|| { + error!("Failed to fetch entry"); + ::std::process::exit(1) + })); + + DV::new(hdr).view_entries(entries) + .map_err_trace_exit_unwrap(1); } From eccb52a85c5607b3bb46e870995f7abebe58c393 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 2 Mar 2018 23:27:20 +0100 Subject: [PATCH 3/3] Fix imag-log for new Diary::entries() interface --- bin/domain/imag-log/Cargo.toml | 1 + bin/domain/imag-log/src/main.rs | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/bin/domain/imag-log/Cargo.toml b/bin/domain/imag-log/Cargo.toml index 2eccb2db..02c24eb2 100644 --- a/bin/domain/imag-log/Cargo.toml +++ b/bin/domain/imag-log/Cargo.toml @@ -27,6 +27,7 @@ toml = "0.4" toml-query = "0.6" is-match = "0.1" +libimagstore = { version = "0.7.0", path = "../../../lib/core/libimagstore" } libimagrt = { version = "0.7.0", path = "../../../lib/core/libimagrt" } libimagerror = { version = "0.7.0", path = "../../../lib/core/libimagerror" } libimagdiary = { version = "0.7.0", path = "../../../lib/domain/libimagdiary" } diff --git a/bin/domain/imag-log/src/main.rs b/bin/domain/imag-log/src/main.rs index b6744831..9c0f199d 100644 --- a/bin/domain/imag-log/src/main.rs +++ b/bin/domain/imag-log/src/main.rs @@ -40,6 +40,7 @@ extern crate toml_query; extern crate libimaglog; #[macro_use] extern crate libimagrt; +extern crate libimagstore; extern crate libimagerror; extern crate libimagdiary; @@ -53,6 +54,7 @@ use libimagerror::exit::ExitUnwrap; use libimagdiary::diary::Diary; use libimaglog::log::Log; use libimaglog::error::LogError as LE; +use libimagstore::iter::get::StoreIdGetIteratorExtension; mod ui; use ui::build_ui; @@ -121,9 +123,15 @@ fn show(rt: &Runtime) { }; for iter in iters { - let _ = iter.map(|element| { + let _ = iter.into_get_iter(rt.store()).map(|element| { let e = element.map_err_trace_exit_unwrap(1); + if e.is_none() { + warn!("Failed to retrieve an entry from an existing store id"); + return Ok(()) + } + let e = e.unwrap(); // safe with above check + if !e.is_log().map_err_trace_exit_unwrap(1) { return Ok(()); }