From 8c37fb865abf30d8942c44182f95c172934a6fdd Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Fri, 2 Mar 2018 21:25:48 +0100 Subject: [PATCH] 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;