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()`.
This commit is contained in:
Matthias Beyer 2018-03-02 21:25:48 +01:00
parent ef3b60eea5
commit 8c37fb865a
4 changed files with 47 additions and 52 deletions

View file

@ -26,6 +26,7 @@ toml = "0.4"
toml-query = "0.6" toml-query = "0.6"
itertools = "0.7" itertools = "0.7"
error-chain = "0.11" error-chain = "0.11"
filters = "0.2"
libimagstore = { version = "0.7.0", path = "../../../lib/core/libimagstore" } libimagstore = { version = "0.7.0", path = "../../../lib/core/libimagstore" }
libimagerror = { version = "0.7.0", path = "../../../lib/core/libimagerror" } libimagerror = { version = "0.7.0", path = "../../../lib/core/libimagerror" }

View file

@ -31,8 +31,8 @@ use chrono::naive::NaiveDateTime;
use chrono::Timelike; use chrono::Timelike;
use entry::IsDiaryEntry; use entry::IsDiaryEntry;
use entry::DiaryEntry;
use diaryid::DiaryId; use diaryid::DiaryId;
use diaryid::FromStoreId;
use error::DiaryErrorKind as DEK; use error::DiaryErrorKind as DEK;
use error::ResultExt; use error::ResultExt;
use error::Result; use error::Result;
@ -89,8 +89,8 @@ impl Diary for Store {
// Get an iterator for iterating over all entries // Get an iterator for iterating over all entries
fn entries(&self, diary_name: &str) -> Result<DiaryEntryIterator> { fn entries(&self, diary_name: &str) -> Result<DiaryEntryIterator> {
debug!("Building iterator for module 'diary' with diary name = '{}'", diary_name); debug!("Building iterator for module 'diary' with diary name = '{}'", diary_name);
self.retrieve_for_module("diary") Store::entries(self)
.map(|iter| DiaryEntryIterator::new(self, String::from(diary_name), iter)) .map(|iter| DiaryEntryIterator::new(String::from(diary_name), iter.without_store()))
.chain_err(|| DEK::StoreReadError) .chain_err(|| DEK::StoreReadError)
} }
@ -99,7 +99,7 @@ impl Diary for Store {
Err(e) => Some(Err(e)), Err(e) => Some(Err(e)),
Ok(entries) => { Ok(entries) => {
entries entries
.map(|e| e.and_then(|e| e.diary_id())) .map(|e| DiaryId::from_storeid(&e))
.sorted_by(|a, b| { .sorted_by(|a, b| {
match (a, b) { match (a, b) {
(&Ok(ref a), &Ok(ref b)) => { (&Ok(ref a), &Ok(ref b)) => {

View file

@ -20,13 +20,11 @@
use std::fmt::{Debug, Formatter, Error as FmtError}; use std::fmt::{Debug, Formatter, Error as FmtError};
use std::result::Result as RResult; use std::result::Result as RResult;
use libimagstore::store::Store; use filters::filter::Filter;
use libimagstore::store::FileLockEntry;
use libimagstore::storeid::StoreIdIterator; use libimagstore::storeid::StoreIdIterator;
use libimagerror::trace::trace_error; use libimagstore::storeid::StoreId;
use diaryid::DiaryId;
use diaryid::FromStoreId;
use is_in_diary::IsInDiary; use is_in_diary::IsInDiary;
use error::DiaryErrorKind as DEK; use error::DiaryErrorKind as DEK;
use error::DiaryError as DE; use error::DiaryError as DE;
@ -34,8 +32,7 @@ use error::ResultExt;
use error::Result; use error::Result;
/// A iterator for iterating over diary entries /// A iterator for iterating over diary entries
pub struct DiaryEntryIterator<'a> { pub struct DiaryEntryIterator {
store: &'a Store,
name: String, name: String,
iter: StoreIdIterator, iter: StoreIdIterator,
@ -44,7 +41,7 @@ pub struct DiaryEntryIterator<'a> {
day: Option<u32>, day: Option<u32>,
} }
impl<'a> Debug for DiaryEntryIterator<'a> { impl Debug for DiaryEntryIterator {
fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FmtError> { fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FmtError> {
write!(fmt, "DiaryEntryIterator<name = {}, year = {:?}, month = {:?}, day = {:?}>", write!(fmt, "DiaryEntryIterator<name = {}, year = {:?}, month = {:?}, day = {:?}>",
@ -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 { DiaryEntryIterator {
store: store,
name: diaryname, name: diaryname,
iter: iter, iter: iter,
@ -68,65 +64,62 @@ impl<'a> DiaryEntryIterator<'a> {
} }
// Filter by year, get all diary entries for this year // 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.year = Some(year);
self self
} }
// Filter by month, get all diary entries for this month (every year) // 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.month = Some(month);
self self
} }
// Filter by day, get all diary entries for this day (every year, every year) // 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.day = Some(day);
self self
} }
} }
impl<'a> Iterator for DiaryEntryIterator<'a> { impl Filter<StoreId> for DiaryEntryIterator {
type Item = Result<FileLockEntry<'a>>; 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<Self::Item> { fn next(&mut self) -> Option<Self::Item> {
loop { loop {
let next = match self.iter.next() { match self.iter.next() {
Some(s) => s, None => return None,
None => return None, Some(s) => {
}; debug!("Next element: {:?}", s);
debug!("Next element: {:?}", next); if Filter::filter(self, &s) {
return Some(s)
if next.is_in_diary(&self.name) { } else {
debug!("Seems to be in diary: {:?}", next); continue
let id = match DiaryId::from_storeid(&next) {
Ok(i) => i,
Err(e) => {
trace_error(&e);
debug!("Couldn't parse {:?} into DiaryId: {:?}", next, e);
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);
} }
} }
} }
} }

View file

@ -41,6 +41,7 @@ extern crate toml;
extern crate toml_query; extern crate toml_query;
extern crate itertools; extern crate itertools;
#[macro_use] extern crate error_chain; #[macro_use] extern crate error_chain;
extern crate filters;
#[macro_use] extern crate libimagstore; #[macro_use] extern crate libimagstore;
#[macro_use] extern crate libimagentryutil; #[macro_use] extern crate libimagentryutil;