From 0916c92a18c79b0bcd9efc5f887c24ba4f787741 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 15 Feb 2019 18:26:57 +0100
Subject: [PATCH] Bugfix: Rebuild iterator when changing collection

This patch fixes a bug where the following code (here pseudocode) did
the wrong thing:

    store.entries().in_collection("foo").for_each(||...)

because the `.in_collection()` call for the underlying PathIterator
object did not actually re-build the iterator. It only changed the
contained `PathIterBuilder`, but did not call `.build_iter()` on it to
rebuild the iterator object.

The test added with this patch checks whether the iterator does the
right thing.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
---
 .../libimagstore/src/file_abstraction/iter.rs |  1 +
 lib/core/libimagstore/src/iter.rs             | 50 +++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/lib/core/libimagstore/src/file_abstraction/iter.rs b/lib/core/libimagstore/src/file_abstraction/iter.rs
index 072a708f..9df59a2f 100644
--- a/lib/core/libimagstore/src/file_abstraction/iter.rs
+++ b/lib/core/libimagstore/src/file_abstraction/iter.rs
@@ -67,6 +67,7 @@ impl PathIterator {
     pub fn in_collection(mut self, c: &str) -> Self {
         trace!("Generating iterator object for collection: {}", c);
         self.iter_builder.in_collection(c);
+        self.iter = self.iter_builder.build_iter();
         self
     }
 
diff --git a/lib/core/libimagstore/src/iter.rs b/lib/core/libimagstore/src/iter.rs
index dcf0838d..1639b02c 100644
--- a/lib/core/libimagstore/src/iter.rs
+++ b/lib/core/libimagstore/src/iter.rs
@@ -214,3 +214,53 @@ impl<'a> Iterator for Entries<'a> {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    extern crate env_logger;
+
+    use std::path::PathBuf;
+    use std::sync::Arc;
+
+    fn setup_logging() {
+        let _ = env_logger::try_init();
+    }
+
+    use store::Store;
+    use storeid::StoreId;
+    use file_abstraction::InMemoryFileAbstraction;
+    use libimagutil::variants::generate_variants;
+
+    pub fn get_store() -> Store {
+        let backend = Arc::new(InMemoryFileAbstraction::default());
+        Store::new_with_backend(PathBuf::from("/"), &None, backend).unwrap()
+    }
+
+    #[test]
+    fn test_entries_iterator_in_collection() {
+        setup_logging();
+        let store = get_store();
+
+        let ids = {
+            let base = String::from("entry");
+            let variants = vec!["coll_1", "coll_2", "coll_3"];
+            let modifier = |base: &String, v: &&str| {
+                StoreId::new(Some(store.path().clone()), PathBuf::from(format!("{}/{}", *v, base))).unwrap()
+            };
+
+            generate_variants(&base, variants.iter(), &modifier)
+        };
+
+        for id in ids {
+            let _ = store.retrieve(id).unwrap();
+        }
+
+        let succeeded = store.entries()
+            .unwrap()
+            .in_collection("coll_3")
+            .map(|id| { debug!("Processing id = {:?}", id); id })
+            .all(|id| id.unwrap().is_in_collection(&["coll_3"]));
+
+        assert!(succeeded, "not all entries in iterator are from coll_3 collection");
+    }
+}
+