From e023a856f11e957544bdde159aa100cebd438903 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 5 Sep 2016 15:01:14 +0200
Subject: [PATCH 01/36] Add Store testing

---
 libimagstore/src/store.rs | 93 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index ec37470b..e061e855 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2148,3 +2148,96 @@ Hai";
 
 }
 
+#[cfg(test)]
+mod store_tests {
+    use std::path::PathBuf;
+
+    use super::Store;
+
+    fn get_store() -> Store {
+        Store::new(PathBuf::from("/"), None).unwrap()
+    }
+
+    #[test]
+    fn test_store_instantiation() {
+        let store = get_store();
+
+        assert_eq!(store.location, PathBuf::from("/"));
+        assert!(store.entries.read().unwrap().is_empty());
+
+        assert!(store.store_unload_aspects.lock().unwrap().is_empty());
+
+        assert!(store.pre_create_aspects.lock().unwrap().is_empty());
+        assert!(store.post_create_aspects.lock().unwrap().is_empty());
+        assert!(store.pre_retrieve_aspects.lock().unwrap().is_empty());
+        assert!(store.post_retrieve_aspects.lock().unwrap().is_empty());
+        assert!(store.pre_update_aspects.lock().unwrap().is_empty());
+        assert!(store.post_update_aspects.lock().unwrap().is_empty());
+        assert!(store.pre_delete_aspects.lock().unwrap().is_empty());
+        assert!(store.post_delete_aspects.lock().unwrap().is_empty());
+        assert!(store.pre_move_aspects.lock().unwrap().is_empty());
+        assert!(store.post_move_aspects.lock().unwrap().is_empty());
+    }
+
+    #[test]
+    fn test_store_create() {
+        let store = get_store();
+
+        for n in 1..100 {
+            let s = format!("test-{}", n);
+            let entry = store.create(PathBuf::from(s.clone())).unwrap();
+            assert!(entry.verify().is_ok());
+            let loc = entry.get_location().clone().into_pathbuf().unwrap();
+            assert!(loc.starts_with("/"));
+            assert!(loc.ends_with(s));
+        }
+    }
+
+    #[test]
+    fn test_store_create_delete_get() {
+        let store = get_store();
+
+        for n in 1..100 {
+            let s = format!("test-{}", n);
+            let entry = store.create(PathBuf::from(s.clone())).unwrap();
+            assert!(entry.verify().is_ok());
+            let loc = entry.get_location().clone().into_pathbuf().unwrap();
+            assert!(loc.starts_with("/"));
+            assert!(loc.ends_with(s));
+        }
+
+        for n in 1..100 {
+            if n % 2 == 0 { continue; }
+            let s = format!("test-{}", n);
+            assert!(store.delete(PathBuf::from(s)).is_ok())
+        }
+
+        for n in 1..100 {
+            if n % 2 != 0 { continue; }
+            let s = format!("test-{}", n);
+            assert!(store.get(PathBuf::from(s)).is_ok())
+        }
+    }
+
+    #[test]
+    fn test_store_create_twice() {
+        use error::StoreErrorKind as SEK;
+
+        let store = get_store();
+
+        for n in 1..100 {
+            let s = format!("test-{}", n % 50);
+            store.create(PathBuf::from(s.clone()))
+                .map_err(|e| assert!(is_match!(e.err_type(), SEK::CreateCallError) && n >= 50))
+                .ok()
+                .map(|entry| {
+                    assert!(entry.verify().is_ok());
+                    let loc = entry.get_location().clone().into_pathbuf().unwrap();
+                    assert!(loc.starts_with("/"));
+                    assert!(loc.ends_with(s));
+                });
+        }
+    }
+
+}
+

From c9994c33b679528078bc43cfa790b4dbecec59a3 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 5 Sep 2016 15:20:55 +0200
Subject: [PATCH 02/36] Add test whether store-internal hashmap gets actually
 filled on Store::create()

---
 libimagstore/src/store.rs | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index e061e855..9465fc73 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2239,5 +2239,22 @@ mod store_tests {
         }
     }
 
+    #[test]
+    fn test_store_create_in_hm() {
+        use storeid::StoreId;
+
+        let store = get_store();
+
+        for n in 1..100 {
+            let pb = StoreId::new_baseless(PathBuf::from(format!("test-{}", n))).unwrap();
+
+            assert!(store.entries.read().unwrap().get(&pb).is_none());
+            assert!(store.create(pb.clone()).is_ok());
+
+            let pb = pb.with_base(store.path().clone());
+            assert!(store.entries.read().unwrap().get(&pb).is_some());
+        }
+    }
+
 }
 

From d5647069cbaf4fc155fd6020a7719a807a8c4543 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 5 Sep 2016 15:22:14 +0200
Subject: [PATCH 03/36] Add test that Store::retrieve() does ::create()
 underneath

---
 libimagstore/src/store.rs | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 9465fc73..e8efd29c 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2256,5 +2256,22 @@ mod store_tests {
         }
     }
 
+    #[test]
+    fn test_store_retrieve_in_hm() {
+        use storeid::StoreId;
+
+        let store = get_store();
+
+        for n in 1..100 {
+            let pb = StoreId::new_baseless(PathBuf::from(format!("test-{}", n))).unwrap();
+
+            assert!(store.entries.read().unwrap().get(&pb).is_none());
+            assert!(store.retrieve(pb.clone()).is_ok());
+
+            let pb = pb.with_base(store.path().clone());
+            assert!(store.entries.read().unwrap().get(&pb).is_some());
+        }
+    }
+
 }
 

From 0cdeeb4c55540c98d4b7b29663bf10da3a64e962 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 19 Sep 2016 10:33:23 +0200
Subject: [PATCH 04/36] Fix test backend implementation

We must return an error if the file is not found here.

This is because if we unwrap() here, we panic if the store action was
Store::retrieve() and there wasn't a file there. We then unwrap() on
None and panic because of this, causing all other tests to panic as
well because the Mutex gets corrupted.

The store handles FileNotFound errors on its own, so it is safe to
return the error here.
---
 libimagstore/src/file_abstraction.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libimagstore/src/file_abstraction.rs b/libimagstore/src/file_abstraction.rs
index a8eb2feb..90204998 100644
--- a/libimagstore/src/file_abstraction.rs
+++ b/libimagstore/src/file_abstraction.rs
@@ -7,9 +7,12 @@ pub use self::fs::FileAbstraction;
 #[cfg(test)]
 mod fs {
     use error::StoreError as SE;
+    use error::StoreErrorKind as SEK;
     use std::io::Cursor;
     use std::path::PathBuf;
 
+    use libimagerror::into::IntoError;
+
     use std::collections::HashMap;
     use std::sync::Mutex;
 
@@ -37,7 +40,7 @@ mod fs {
             match *self {
                 FileAbstraction::Absent(ref f) => {
                     let map = MAP.lock().unwrap();
-                    return Ok(map.get(f).unwrap().clone());
+                    return map.get(f).cloned().ok_or(SEK::FileNotFound.into_error());
                 },
             };
         }

From 85097554e9496ef3f6a0127f2aa5c89f1d7dff25 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Wed, 7 Sep 2016 12:47:52 +0200
Subject: [PATCH 05/36] Add hook implementation for succeeding hook tests

---
 libimagstore/src/store.rs | 106 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 105 insertions(+), 1 deletion(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index e8efd29c..9b90f89e 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2154,7 +2154,7 @@ mod store_tests {
 
     use super::Store;
 
-    fn get_store() -> Store {
+    pub fn get_store() -> Store {
         Store::new(PathBuf::from("/"), None).unwrap()
     }
 
@@ -2275,3 +2275,107 @@ mod store_tests {
 
 }
 
+#[cfg(test)]
+mod store_hook_tests {
+    use super::store_tests::get_store;
+
+    mod succeeding_hook {
+        use hook::Hook;
+        use hook::accessor::HookDataAccessor;
+        use hook::accessor::HookDataAccessorProvider;
+        use hook::position::HookPosition;
+
+        use self::accessor::SucceedingHookAccessor as DHA;
+
+        use toml::Value;
+
+        #[derive(Debug)]
+        pub struct SucceedingHook {
+            position: HookPosition,
+            accessor: DHA,
+        }
+
+        impl SucceedingHook {
+
+            pub fn new(pos: HookPosition) -> SucceedingHook {
+                SucceedingHook { position: pos.clone(), accessor: DHA::new(pos) }
+            }
+
+        }
+
+        impl Hook for SucceedingHook {
+            fn name(&self) -> &'static str { "testhook_succeeding" }
+            fn set_config(&mut self, _: &Value) { }
+        }
+
+        impl HookDataAccessorProvider for SucceedingHook {
+
+            fn accessor(&self) -> HookDataAccessor {
+                use hook::position::HookPosition as HP;
+                use hook::accessor::HookDataAccessor as HDA;
+
+                match self.position {
+                    HP::StoreUnload  |
+                    HP::PreCreate    |
+                    HP::PreRetrieve  |
+                    HP::PreDelete    |
+                    HP::PostDelete   => HDA::StoreIdAccess(&self.accessor),
+                    HP::PostCreate   |
+                    HP::PostRetrieve |
+                    HP::PreUpdate    |
+                    HP::PostUpdate   => HDA::MutableAccess(&self.accessor),
+                }
+            }
+
+        }
+
+        pub mod accessor {
+            use hook::result::HookResult;
+            use hook::accessor::MutableHookDataAccessor;
+            use hook::accessor::NonMutableHookDataAccessor;
+            use hook::accessor::StoreIdAccessor;
+            use hook::position::HookPosition;
+            use store::FileLockEntry;
+            use storeid::StoreId;
+
+            #[derive(Debug)]
+            pub struct SucceedingHookAccessor(HookPosition);
+
+            impl SucceedingHookAccessor {
+
+                pub fn new(position: HookPosition) -> SucceedingHookAccessor {
+                    SucceedingHookAccessor(position)
+                }
+
+            }
+
+            impl StoreIdAccessor for SucceedingHookAccessor {
+
+                fn access(&self, id: &StoreId) -> HookResult<()> {
+                    Ok(())
+                }
+
+            }
+
+            impl MutableHookDataAccessor for SucceedingHookAccessor {
+
+                fn access_mut(&self, fle: &mut FileLockEntry) -> HookResult<()> {
+                    Ok(())
+                }
+
+            }
+
+            impl NonMutableHookDataAccessor for SucceedingHookAccessor {
+
+                fn access(&self, fle: &FileLockEntry) -> HookResult<()> {
+                    Ok(())
+                }
+
+            }
+
+        }
+
+    }
+
+}
+

From 52b2a4589d99b26d22860806614b354534071b8d Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 19 Sep 2016 11:01:56 +0200
Subject: [PATCH 06/36] Add test to get non-existent entries

---
 libimagstore/src/store.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index e8efd29c..668e7d0d 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2273,5 +2273,16 @@ mod store_tests {
         }
     }
 
+    #[test]
+    fn test_get_none() {
+        let store = get_store();
+
+        for n in 1..100 {
+            match store.get(PathBuf::from(format!("test-{}", n))) {
+                Ok(None) => assert!(true),
+                _        => assert!(false),
+            }
+        }
+    }
 }
 

From 8345ff8248f20af20e129da56af664951612ff4f Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 19 Sep 2016 11:03:38 +0200
Subject: [PATCH 07/36] Add test to delete non-existent entries

---
 libimagstore/src/store.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 668e7d0d..284c9228 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2284,5 +2284,17 @@ mod store_tests {
             }
         }
     }
+
+    #[test]
+    fn test_delete_none() {
+        let store = get_store();
+
+        for n in 1..100 {
+            match store.delete(PathBuf::from(format!("test-{}", n))) {
+                Err(_) => assert!(true),
+                _      => assert!(false),
+            }
+        }
+    }
 }
 

From fd41fe59987fd7801d7c1bd4d9f05d7a35573617 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 19 Sep 2016 11:07:38 +0200
Subject: [PATCH 08/36] Fix Store::delete() for nonexistent IDs

If the ID does not exist, we should return an error instead of doing
nothing, shouldn't we?
---
 libimagstore/src/store.rs | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 284c9228..e1cd85b7 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -573,9 +573,13 @@ impl Store {
             };
 
             // if the entry is currently modified by the user, we cannot drop it
-            if entries.get(&id).map(|e| e.is_borrowed()).unwrap_or(false) {
-                return Err(SE::new(SEK::IdLocked, None))
-                    .map_err_into(SEK::DeleteCallError);
+            match entries.get(&id) {
+                None => {
+                    return Err(SEK::FileNotFound.into_error()).map_err_into(SEK::DeleteCallError)
+                },
+                Some(e) => if e.is_borrowed() {
+                    return Err(SE::new(SEK::IdLocked, None)).map_err_into(SEK::DeleteCallError)
+                }
             }
 
             // remove the entry first, then the file

From 4155924f85bf4af3a8fd3b0a66f46b521d589f23 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 19 Sep 2016 11:18:49 +0200
Subject: [PATCH 09/36] Add test for Store::retrieve_for_module()

---
 libimagstore/src/store.rs | 50 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index e1cd85b7..9e12a1eb 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2300,5 +2300,55 @@ mod store_tests {
             }
         }
     }
+
+    // Disabled because we cannot test this by now, as we rely on glob() in
+    // Store::retieve_for_module(), which accesses the filesystem and tests run in-memory, so there
+    // are no files on the filesystem in this test after Store::create().
+    //
+    // #[test]
+    // fn test_retrieve_for_module() {
+    //     let pathes = vec![
+    //         "foo/1", "foo/2", "foo/3", "foo/4", "foo/5",
+    //         "bar/1", "bar/2", "bar/3", "bar/4", "bar/5",
+    //         "bla/1", "bla/2", "bla/3", "bla/4", "bla/5",
+    //         "boo/1", "boo/2", "boo/3", "boo/4", "boo/5",
+    //         "glu/1", "glu/2", "glu/3", "glu/4", "glu/5",
+    //     ];
+
+    //     fn test(store: &Store, modulename: &str) {
+    //         use std::path::Component;
+    //         use storeid::StoreId;
+
+    //         let retrieved = store.retrieve_for_module(modulename);
+    //         assert!(retrieved.is_ok());
+    //         let v : Vec<StoreId> = retrieved.unwrap().collect();
+    //         println!("v = {:?}", v);
+    //         assert!(v.len() == 5);
+
+    //         let retrieved = store.retrieve_for_module(modulename);
+    //         assert!(retrieved.is_ok());
+
+    //         assert!(retrieved.unwrap().all(|e| {
+    //             let first = e.components().next();
+    //             assert!(first.is_some());
+    //             match first.unwrap() {
+    //                 Component::Normal(s) => s == modulename,
+    //                 _                    => false,
+    //             }
+    //         }))
+    //     }
+
+    //     let store = get_store();
+    //     for path in pathes {
+    //         assert!(store.create(PathBuf::from(path)).is_ok());
+    //     }
+
+    //     test(&store, "foo");
+    //     test(&store, "bar");
+    //     test(&store, "bla");
+    //     test(&store, "boo");
+    //     test(&store, "glu");
+    // }
+
 }
 

From 83ebe88022e848273b0ae2765cef854a9d8ed167 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Sun, 18 Sep 2016 19:49:50 +0200
Subject: [PATCH 10/36] Add PreCreate hook test

---
 libimagstore/src/store.rs | 53 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 9b90f89e..997f45b5 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2277,7 +2277,6 @@ mod store_tests {
 
 #[cfg(test)]
 mod store_hook_tests {
-    use super::store_tests::get_store;
 
     mod succeeding_hook {
         use hook::Hook;
@@ -2377,5 +2376,57 @@ mod store_hook_tests {
 
     }
 
+    use std::path::PathBuf;
+
+    use hook::position::HookPosition as HP;
+    use storeid::StoreId;
+    use store::Store;
+
+    use self::succeeding_hook::SucceedingHook;
+
+    fn get_store_with_config() -> Store {
+        use toml::Parser;
+
+        let cfg = Parser::new(mini_config()).parse().unwrap();
+        println!("Config parsed: {:?}", cfg);
+        Store::new(PathBuf::from("/"), Some(cfg.get("store").cloned().unwrap())).unwrap()
+    }
+
+    fn mini_config() -> &'static str {
+        r#"
+[store]
+store-unload-hook-aspects  = [ "test" ]
+pre-create-hook-aspects    = [ "test" ]
+post-create-hook-aspects   = [ "test" ]
+pre-move-hook-aspects      = [ "test" ]
+post-move-hook-aspects     = [ "test" ]
+pre-retrieve-hook-aspects  = [ "test" ]
+post-retrieve-hook-aspects = [ "test" ]
+pre-update-hook-aspects    = [ "test" ]
+post-update-hook-aspects   = [ "test" ]
+pre-delete-hook-aspects    = [ "test" ]
+post-delete-hook-aspects   = [ "test" ]
+
+[store.aspects.test]
+parallel = false
+mutable_hooks = true
+
+[store.hooks.testhook_succeeding]
+aspect = "test"
+        "#
+    }
+
+    #[test]
+    fn test_pre_create() {
+        let mut store = get_store_with_config();
+        let pos       = HP::PreCreate;
+        let hook      = SucceedingHook::new(pos.clone());
+
+        assert!(store.register_hook(pos, "test", Box::new(hook)).map_err(|e| println!("{:?}", e)).is_ok());
+
+        let pb = StoreId::new_baseless(PathBuf::from("test")).unwrap();
+        assert!(store.create(pb.clone()).is_ok());
+    }
+
 }
 

From 72a95ee5daeb6a19a49408a1c84df95dfcd90d8c Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 19 Sep 2016 21:00:45 +0200
Subject: [PATCH 11/36] Rewrite config validity checker to return Result<()>

And add more detailed error kinds for config errors
---
 libimagstore/src/configuration.rs | 139 +++++++++++++++++-------------
 libimagstore/src/error.rs         |  13 +++
 libimagstore/src/store.rs         |   4 +-
 3 files changed, 93 insertions(+), 63 deletions(-)

diff --git a/libimagstore/src/configuration.rs b/libimagstore/src/configuration.rs
index ff94a1a3..c450000c 100644
--- a/libimagstore/src/configuration.rs
+++ b/libimagstore/src/configuration.rs
@@ -1,5 +1,9 @@
 use toml::Value;
 
+use libimagerror::into::IntoError;
+
+use store::Result;
+
 /// Check whether the configuration is valid for the store
 ///
 /// The passed `Value` _must be_ the `[store]` sub-tree of the configuration. Otherwise this will
@@ -42,32 +46,42 @@ use toml::Value;
 /// You have been warned!
 ///
 ///
-pub fn config_is_valid(config: &Option<Value>) -> bool {
+pub fn config_is_valid(config: &Option<Value>) -> Result<()> {
     use std::collections::BTreeMap;
-    use std::io::Write;
-    use std::io::stderr;
+    use error::StoreErrorKind as SEK;
 
     if config.is_none() {
-        return true;
+        return Ok(());
     }
 
-    fn has_key_with_string_ary(v: &BTreeMap<String, Value>, key: &str) -> bool {
+    /// Check whether the config has a key with a string array.
+    /// The `key` is the key which is checked
+    /// The `kind` is the error kind which is used as `cause` if there is an error, so we can
+    /// indicate via error type which key is missing
+    fn has_key_with_string_ary(v: &BTreeMap<String, Value>, key: &str,
+                               kind: SEK) -> Result<()> {
         v.get(key)
-            .map_or_else(|| {
-                write!(stderr(), "Required key '{}' is not in store config", key).ok();
-                false
-            }, |t| match *t {
-                    Value::Array(ref a) => a.iter().all(|elem| {
-                        match *elem {
-                            Value::String(_) => true,
-                            _ => false,
-                        }
-                    }),
-                    _ => {
-                        write!(stderr(), "Key '{}' in store config should contain an array", key)
-                            .ok();
-                        false
-                    }
+            .ok_or_else(|| {
+                warn!("Required key '{}' is not in store config", key);
+                SEK::ConfigKeyMissingError.into_error_with_cause(Box::new(kind.into_error()))
+            })
+            .and_then(|t| match *t {
+                Value::Array(ref a) => {
+                    a.iter().fold(Ok(()), |acc, elem| {
+                        acc.and_then(|_| {
+                            if is_match!(*elem, Value::String(_)) {
+                                Ok(())
+                            } else {
+                                let cause = Box::new(kind.into_error());
+                                Err(SEK::ConfigTypeError.into_error_with_cause(cause))
+                            }
+                        })
+                    })
+                },
+                _ => {
+                    warn!("Key '{}' in store config should contain an array", key);
+                    Err(SEK::ConfigTypeError.into_error_with_cause(Box::new(kind.into_error())))
+                }
             })
     }
 
@@ -82,59 +96,64 @@ pub fn config_is_valid(config: &Option<Value>) -> bool {
                                              section: &str,
                                              key: &str,
                                              f: F)
-        -> bool
+        -> Result<()>
         where F: Fn(&Value) -> bool
     {
         store_config.get(section) // The store config has the section `section`
-            .map_or_else(|| {
-                write!(stderr(), "Store config expects section '{}' to be present, but isn't.",
-                        section).ok();
-                false
-            }, |section_table| {
-                match *section_table { // which is
-                    Value::Table(ref section_table) => // a table
-                        section_table
-                            .iter() // which has values,
-                            .all(|(inner_key, cfg)| { // and all of these values
-                                match *cfg {
-                                    Value::Table(ref hook_config) => { // are tables
-                                        hook_config.get(key) // with a key
-                                            // fullfilling this constraint
-                                            .map_or(false, |hook_aspect| f(&hook_aspect))
-                                    },
-                                    _ => {
-                                        write!(stderr(), "Store config expects '{}' to be in '{}.{}', but isn't.",
-                                                 key, section, inner_key).ok();
-                                        false
+            .ok_or_else(|| {
+                warn!("Store config expects section '{}' to be present, but isn't.", section);
+                SEK::ConfigKeyMissingError.into_error()
+            })
+            .and_then(|section_table| match *section_table { // which is
+                Value::Table(ref section_table) => // a table
+                    section_table.iter().fold(Ok(()), |acc, (inner_key, cfg)| {
+                        acc.and_then(|_| {
+                            match *cfg {
+                                Value::Table(ref hook_config) => { // are tables
+                                    // with a key
+                                    let hook_aspect_is_valid = try!(hook_config.get(key)
+                                        .map(|hook_aspect| f(&hook_aspect))
+                                        .ok_or(SEK::ConfigKeyMissingError.into_error())
+                                    );
+
+                                    if !hook_aspect_is_valid {
+                                        Err(SEK::ConfigTypeError.into_error())
+                                    } else {
+                                        Ok(())
                                     }
+                                },
+                                _ => {
+                                    warn!("Store config expects '{}' to be in '{}.{}', but isn't.",
+                                             key, section, inner_key);
+                                    Err(SEK::ConfigKeyMissingError.into_error())
                                 }
-                            }),
-                    _ => {
-                        write!(stderr(), "Store config expects '{}' to be a Table, but isn't.",
-                               section).ok();
-                        false
-                    }
+                            }
+                        })
+                    }),
+                _ => {
+                    warn!("Store config expects '{}' to be a Table, but isn't.", section);
+                    Err(SEK::ConfigTypeError.into_error())
                 }
             })
     }
 
     match *config {
         Some(Value::Table(ref t)) => {
-            has_key_with_string_ary(t, "store-unload-hook-aspects")    &&
+            try!(has_key_with_string_ary(t, "store-unload-hook-aspects", SEK::ConfigKeyUnloadAspectsError));
 
-            has_key_with_string_ary(t, "pre-create-hook-aspects")      &&
-            has_key_with_string_ary(t, "post-create-hook-aspects")     &&
-            has_key_with_string_ary(t, "pre-retrieve-hook-aspects")    &&
-            has_key_with_string_ary(t, "post-retrieve-hook-aspects")   &&
-            has_key_with_string_ary(t, "pre-update-hook-aspects")      &&
-            has_key_with_string_ary(t, "post-update-hook-aspects")     &&
-            has_key_with_string_ary(t, "pre-delete-hook-aspects")      &&
-            has_key_with_string_ary(t, "post-delete-hook-aspects")     &&
+            try!(has_key_with_string_ary(t, "pre-create-hook-aspects", SEK::ConfigKeyPreCreateAspectsError));
+            try!(has_key_with_string_ary(t, "post-create-hook-aspects", SEK::ConfigKeyPostCreateAspectsError));
+            try!(has_key_with_string_ary(t, "pre-retrieve-hook-aspects", SEK::ConfigKeyPreRetrieveAspectsError));
+            try!(has_key_with_string_ary(t, "post-retrieve-hook-aspects", SEK::ConfigKeyPostRetrieveAspectsError));
+            try!(has_key_with_string_ary(t, "pre-update-hook-aspects", SEK::ConfigKeyPreUpdateAspectsError));
+            try!(has_key_with_string_ary(t, "post-update-hook-aspects", SEK::ConfigKeyPostUpdateAspectsError));
+            try!(has_key_with_string_ary(t, "pre-delete-hook-aspects", SEK::ConfigKeyPreDeleteAspectsError));
+            try!(has_key_with_string_ary(t, "post-delete-hook-aspects", SEK::ConfigKeyPostDeleteAspectsError));
 
             // The section "hooks" has maps which have a key "aspect" which has a value of type
             // String
-            check_all_inner_maps_have_key_with(t, "hooks", "aspect",
-                                               |asp| is_match!(asp, &Value::String(_))) &&
+            try!(check_all_inner_maps_have_key_with(t, "hooks", "aspect",
+                                                    |asp| is_match!(asp, &Value::String(_))));
 
             // The section "aspects" has maps which have a key "parllel" which has a value of type
             // Boolean
@@ -142,8 +161,8 @@ pub fn config_is_valid(config: &Option<Value>) -> bool {
                                                |asp| is_match!(asp, &Value::Boolean(_)))
         }
         _ => {
-            write!(stderr(), "Store config is no table").ok();
-            false
+            warn!("Store config is no table");
+            Err(SEK::ConfigTypeError.into_error())
         },
     }
 }
diff --git a/libimagstore/src/error.rs b/libimagstore/src/error.rs
index fe0455c1..80c10a0c 100644
--- a/libimagstore/src/error.rs
+++ b/libimagstore/src/error.rs
@@ -6,6 +6,19 @@ pub struct CustomErrorData {}
 
 generate_custom_error_types!(StoreError, StoreErrorKind, CustomErrorData,
     ConfigurationError      => "Store Configuration Error",
+    ConfigTypeError         => "Store configuration type error",
+    ConfigKeyMissingError   => "Configuration Key missing",
+
+    ConfigKeyUnloadAspectsError       => "Config Key 'store-unload-hook-aspects' caused an error",
+    ConfigKeyPreCreateAspectsError    => "Config Key 'pre-create-hook-aspects' caused an error",
+    ConfigKeyPostCreateAspectsError   => "Config Key 'post-create-hook-aspects' caused an error",
+    ConfigKeyPreRetrieveAspectsError  => "Config Key 'pre-retrieve-hook-aspect' caused an error",
+    ConfigKeyPostRetrieveAspectsError => "Config Key 'post-retrieve-hook-aspec' caused an error",
+    ConfigKeyPreUpdateAspectsError    => "Config Key 'pre-update-hook-aspects' caused an error",
+    ConfigKeyPostUpdateAspectsError   => "Config Key 'post-update-hook-aspects' caused an error",
+    ConfigKeyPreDeleteAspectsError    => "Config Key 'pre-delete-hook-aspects' caused an error",
+    ConfigKeyPostDeleteAspectsError   => "Config Key 'post-delete-hook-aspects' caused an error",
+
     CreateStoreDirDenied    => "Creating store directory implicitely denied",
     FileError               => "File Error",
     IoError                 => "IO Error",
diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 997f45b5..c4a041e5 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -213,9 +213,7 @@ impl Store {
         use configuration::*;
 
         debug!("Validating Store configuration");
-        if !config_is_valid(&store_config) {
-            return Err(SE::new(SEK::ConfigurationError, None));
-        }
+        let _ = try!(config_is_valid(&store_config).map_err_into(SEK::ConfigurationError));
 
         debug!("Building new Store object");
         if !location.exists() {

From 4f2019a20aa9aede7bb966afb6d3234d10607412 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Tue, 20 Sep 2016 16:43:32 +0200
Subject: [PATCH 12/36] Extend test_store_create_delete_get()

to actually test:

1. get      -> Should return Ok(None)
2. create   -> Should return Ok(())
3. get      -> Should return Ok(Some(_))
4. delete   -> Should return Ok(())
5. get      -> Should return Ok(None)
---
 libimagstore/src/store.rs | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index aaebffa9..f19a5eff 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2196,28 +2196,38 @@ mod store_tests {
     }
 
     #[test]
-    fn test_store_create_delete_get() {
+    fn test_store_get_create_get_delete_get() {
         let store = get_store();
 
+        for n in 1..100 {
+            let res = store.get(PathBuf::from(format!("test-{}", n)));
+            assert!(match res { Ok(None) => true, _ => false, })
+        }
+
         for n in 1..100 {
             let s = format!("test-{}", n);
             let entry = store.create(PathBuf::from(s.clone())).unwrap();
+
             assert!(entry.verify().is_ok());
+
             let loc = entry.get_location().clone().into_pathbuf().unwrap();
+
             assert!(loc.starts_with("/"));
             assert!(loc.ends_with(s));
         }
 
         for n in 1..100 {
-            if n % 2 == 0 { continue; }
-            let s = format!("test-{}", n);
-            assert!(store.delete(PathBuf::from(s)).is_ok())
+            let res = store.get(PathBuf::from(format!("test-{}", n)));
+            assert!(match res { Ok(Some(_)) => true, _ => false, })
         }
 
         for n in 1..100 {
-            if n % 2 != 0 { continue; }
-            let s = format!("test-{}", n);
-            assert!(store.get(PathBuf::from(s)).is_ok())
+            assert!(store.delete(PathBuf::from(format!("test-{}", n))).is_ok())
+        }
+
+        for n in 1..100 {
+            let res = store.get(PathBuf::from(format!("test-{}", n)));
+            assert!(match res { Ok(None) => true, _ => false, })
         }
     }
 

From babf74e1e52469eb2ad41d3a40496d6db7016171 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Tue, 20 Sep 2016 16:52:14 +0200
Subject: [PATCH 13/36] Fix Store::get() to not check FS but internal hashmap

---
 libimagstore/src/store.rs | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index f19a5eff..b6a9c8ad 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -459,11 +459,20 @@ impl Store {
     ///
     /// This executes the {pre,post}_retrieve_aspects hooks.
     pub fn get<'a, S: IntoStoreId + Clone>(&'a self, id: S) -> Result<Option<FileLockEntry<'a>>> {
-        let id_copy = try!(id.clone().into_storeid()).with_base(self.path().clone());
-        if !id_copy.exists() {
-            debug!("Does not exist: {:?}", id_copy);
+        let id = try!(id.into_storeid()).with_base(self.path().clone());
+
+        let exists = try!(self.entries
+            .read()
+            .map(|map| map.contains_key(&id))
+            .map_err(|_| SE::new(SEK::LockPoisoned, None))
+            .map_err_into(SEK::GetCallError)
+        );
+
+        if !exists && !id.exists() {
+            debug!("Does not exist in internal cache or filesystem: {:?}", id);
             return Ok(None);
         }
+
         self.retrieve(id).map(Some).map_err_into(SEK::GetCallError)
     }
 

From 61201082c8d87439085085d32dab766f2535e354 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Wed, 21 Sep 2016 09:46:07 +0200
Subject: [PATCH 14/36] Fix: FileAbstraction::remove_file(): Add implementation

---
 libimagstore/src/file_abstraction.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libimagstore/src/file_abstraction.rs b/libimagstore/src/file_abstraction.rs
index 90204998..dc04d235 100644
--- a/libimagstore/src/file_abstraction.rs
+++ b/libimagstore/src/file_abstraction.rs
@@ -62,7 +62,8 @@ mod fs {
             };
         }
 
-        pub fn remove_file(_: &PathBuf) -> Result<(), SE> {
+        pub fn remove_file(path: &PathBuf) -> Result<(), SE> {
+            MAP.lock().unwrap().remove(path);
             Ok(())
         }
 

From d5a275fec05c0956a6897cd0e5ffef2ad22296ce Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Wed, 7 Sep 2016 12:36:03 +0200
Subject: [PATCH 15/36] Add test: Store::move_by_id()

---
 libimagstore/src/store.rs | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index b6a9c8ad..732a2490 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2367,6 +2367,40 @@ mod store_tests {
     //     test(&store, "glu");
     // }
 
+    #[test]
+    fn test_store_move_moves_in_hm() {
+        use storeid::StoreId;
+
+        let store = get_store();
+
+        for n in 1..100 {
+            if n % 2 == 0 { // every second
+                let id    = StoreId::new_baseless(PathBuf::from(format!("t-{}", n))).unwrap();
+                let id_mv = StoreId::new_baseless(PathBuf::from(format!("t-{}", n - 1))).unwrap();
+
+                {
+                    assert!(store.entries.read().unwrap().get(&id).is_none());
+                }
+
+                {
+                    assert!(store.create(id.clone()).is_ok());
+                }
+
+                {
+                    let id_with_base = id.clone().with_base(store.path().clone());
+                    assert!(store.entries.read().unwrap().get(&id_with_base).is_some());
+                }
+
+                let r = store.move_by_id(id.clone(), id_mv.clone());
+                assert!(r.map_err(|e| println!("ERROR: {:?}", e)).is_ok());
+
+                {
+                    assert!(store.entries.read().unwrap().get(&id_mv).is_some());
+                }
+            }
+        }
+    }
+
 }
 
 #[cfg(test)]

From caa214f1bdcdf29f20bd818ed997b3f4eada64db Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Thu, 22 Sep 2016 08:29:33 +0200
Subject: [PATCH 16/36] Fix negation error

We check whether the old key already exists.

If it does _not_ exist, the entry is borrowed, from my understanding.
I'm not sure, though.
---
 libimagstore/src/store.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 732a2490..68cf3d7a 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -667,7 +667,7 @@ impl Store {
                 Ok(m)  => m,
             };
 
-            if hsmap.contains_key(&old_id) {
+            if !hsmap.contains_key(&old_id) {
                 return Err(SE::new(SEK::EntryAlreadyBorrowed, None));
             } else {
                 let old_id_pb = try!(old_id.clone().with_base(self.path().clone()).into_pathbuf());

From 4f83b22b982ca3b5fe93704bf305a18f556e9b3d Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Thu, 22 Sep 2016 08:39:00 +0200
Subject: [PATCH 17/36] Fix Store::move_by_id(): Move in cache as well

---
 libimagstore/src/store.rs | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 68cf3d7a..5f3c15c8 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -662,11 +662,15 @@ impl Store {
         }
 
         {
-            let hsmap = match self.entries.write() {
+            let mut hsmap = match self.entries.write() {
                 Err(_) => return Err(SE::new(SEK::LockPoisoned, None)),
                 Ok(m)  => m,
             };
 
+            if hsmap.contains_key(&new_id) {
+                return Err(SEK::EntryAlreadyExists.into_error());
+            }
+
             if !hsmap.contains_key(&old_id) {
                 return Err(SE::new(SEK::EntryAlreadyBorrowed, None));
             } else {
@@ -675,7 +679,13 @@ impl Store {
                 match FileAbstraction::rename(&old_id_pb, &new_id_pb) {
                     Err(e) => return Err(SEK::EntryRenameError.into_error_with_cause(Box::new(e))),
                     Ok(_) => {
-                        debug!("Rename worked");
+                        debug!("Rename worked on filesystem");
+
+                        // assert enforced through check hsmap.contains_key(&new_id) above.
+                        // Should therefor never fail
+                        assert!(hsmap
+                                .remove(&old_id)
+                                .and_then(|entry| hsmap.insert(new_id.clone(), entry)).is_none())
                     }
                 }
             }
@@ -2395,7 +2405,8 @@ mod store_tests {
                 assert!(r.map_err(|e| println!("ERROR: {:?}", e)).is_ok());
 
                 {
-                    assert!(store.entries.read().unwrap().get(&id_mv).is_some());
+                    let id_mv_with_base = id_mv.clone().with_base(store.path().clone());
+                    assert!(store.entries.read().unwrap().get(&id_mv_with_base).is_some());
                 }
             }
         }

From 886eed3ff4b3f1d8fb0d7278e63e0e4f3b8c49da Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Thu, 22 Sep 2016 08:46:59 +0200
Subject: [PATCH 18/36] Rename: SucceedingHook -> TestHook

---
 libimagstore/src/store.rs | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index b6a9c8ad..7c8651f2 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2372,36 +2372,36 @@ mod store_tests {
 #[cfg(test)]
 mod store_hook_tests {
 
-    mod succeeding_hook {
+    mod test_hook {
         use hook::Hook;
         use hook::accessor::HookDataAccessor;
         use hook::accessor::HookDataAccessorProvider;
         use hook::position::HookPosition;
 
-        use self::accessor::SucceedingHookAccessor as DHA;
+        use self::accessor::TestHookAccessor as DHA;
 
         use toml::Value;
 
         #[derive(Debug)]
-        pub struct SucceedingHook {
+        pub struct TestHook {
             position: HookPosition,
             accessor: DHA,
         }
 
-        impl SucceedingHook {
+        impl TestHook {
 
-            pub fn new(pos: HookPosition) -> SucceedingHook {
-                SucceedingHook { position: pos.clone(), accessor: DHA::new(pos) }
+            pub fn new(pos: HookPosition) -> TestHook {
+                TestHook { position: pos.clone(), accessor: DHA::new(pos) }
             }
 
         }
 
-        impl Hook for SucceedingHook {
+        impl Hook for TestHook {
             fn name(&self) -> &'static str { "testhook_succeeding" }
             fn set_config(&mut self, _: &Value) { }
         }
 
-        impl HookDataAccessorProvider for SucceedingHook {
+        impl HookDataAccessorProvider for TestHook {
 
             fn accessor(&self) -> HookDataAccessor {
                 use hook::position::HookPosition as HP;
@@ -2432,17 +2432,17 @@ mod store_hook_tests {
             use storeid::StoreId;
 
             #[derive(Debug)]
-            pub struct SucceedingHookAccessor(HookPosition);
+            pub struct TestHookAccessor(HookPosition);
 
-            impl SucceedingHookAccessor {
+            impl TestHookAccessor {
 
-                pub fn new(position: HookPosition) -> SucceedingHookAccessor {
-                    SucceedingHookAccessor(position)
+                pub fn new(position: HookPosition) -> TestHookAccessor {
+                    TestHookAccessor(position)
                 }
 
             }
 
-            impl StoreIdAccessor for SucceedingHookAccessor {
+            impl StoreIdAccessor for TestHookAccessor {
 
                 fn access(&self, id: &StoreId) -> HookResult<()> {
                     Ok(())
@@ -2450,7 +2450,7 @@ mod store_hook_tests {
 
             }
 
-            impl MutableHookDataAccessor for SucceedingHookAccessor {
+            impl MutableHookDataAccessor for TestHookAccessor {
 
                 fn access_mut(&self, fle: &mut FileLockEntry) -> HookResult<()> {
                     Ok(())
@@ -2458,7 +2458,7 @@ mod store_hook_tests {
 
             }
 
-            impl NonMutableHookDataAccessor for SucceedingHookAccessor {
+            impl NonMutableHookDataAccessor for TestHookAccessor {
 
                 fn access(&self, fle: &FileLockEntry) -> HookResult<()> {
                     Ok(())
@@ -2476,7 +2476,7 @@ mod store_hook_tests {
     use storeid::StoreId;
     use store::Store;
 
-    use self::succeeding_hook::SucceedingHook;
+    use self::test_hook::TestHook;
 
     fn get_store_with_config() -> Store {
         use toml::Parser;
@@ -2514,7 +2514,7 @@ aspect = "test"
     fn test_pre_create() {
         let mut store = get_store_with_config();
         let pos       = HP::PreCreate;
-        let hook      = SucceedingHook::new(pos.clone());
+        let hook      = TestHook::new(pos.clone());
 
         assert!(store.register_hook(pos, "test", Box::new(hook)).map_err(|e| println!("{:?}", e)).is_ok());
 

From f16c09a98120004fcb6e371160c88300d427ad04 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 30 Sep 2016 12:50:59 +0200
Subject: [PATCH 19/36] Do not check whether old or new id exists/does not
 exist

---
 libimagstore/src/store.rs | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 5f3c15c8..d46aaebe 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -650,7 +650,6 @@ impl Store {
 
     /// Move an entry without loading
     pub fn move_by_id(&self, old_id: StoreId, new_id: StoreId) -> Result<()> {
-
         let new_id = new_id.with_base(self.path().clone());
         let old_id = old_id.with_base(self.path().clone());
 
@@ -671,22 +670,19 @@ impl Store {
                 return Err(SEK::EntryAlreadyExists.into_error());
             }
 
-            if !hsmap.contains_key(&old_id) {
-                return Err(SE::new(SEK::EntryAlreadyBorrowed, None));
-            } else {
-                let old_id_pb = try!(old_id.clone().with_base(self.path().clone()).into_pathbuf());
-                let new_id_pb = try!(new_id.clone().with_base(self.path().clone()).into_pathbuf());
-                match FileAbstraction::rename(&old_id_pb, &new_id_pb) {
-                    Err(e) => return Err(SEK::EntryRenameError.into_error_with_cause(Box::new(e))),
-                    Ok(_) => {
-                        debug!("Rename worked on filesystem");
+            let old_id_pb = try!(old_id.clone().with_base(self.path().clone()).into_pathbuf());
+            let new_id_pb = try!(new_id.clone().with_base(self.path().clone()).into_pathbuf());
 
-                        // assert enforced through check hsmap.contains_key(&new_id) above.
-                        // Should therefor never fail
-                        assert!(hsmap
-                                .remove(&old_id)
-                                .and_then(|entry| hsmap.insert(new_id.clone(), entry)).is_none())
-                    }
+            match FileAbstraction::rename(&old_id_pb, &new_id_pb) {
+                Err(e) => return Err(SEK::EntryRenameError.into_error_with_cause(Box::new(e))),
+                Ok(_) => {
+                    debug!("Rename worked on filesystem");
+
+                    // assert enforced through check hsmap.contains_key(&new_id) above.
+                    // Should therefor never fail
+                    assert!(hsmap
+                            .remove(&old_id)
+                            .and_then(|entry| hsmap.insert(new_id.clone(), entry)).is_none())
                 }
             }
 

From 1526c0b2dcdaae2057a894800d317f093161f066 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Thu, 22 Sep 2016 08:52:58 +0200
Subject: [PATCH 20/36] Make hook configurable whether it succeeds or not

---
 libimagstore/src/store.rs | 44 +++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 7c8651f2..4ac926a2 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2390,8 +2390,8 @@ mod store_hook_tests {
 
         impl TestHook {
 
-            pub fn new(pos: HookPosition) -> TestHook {
-                TestHook { position: pos.clone(), accessor: DHA::new(pos) }
+            pub fn new(pos: HookPosition, succeed: bool, error_aborting: bool) -> TestHook {
+                TestHook { position: pos.clone(), accessor: DHA::new(pos, succeed, error_aborting) }
             }
 
         }
@@ -2430,22 +2430,48 @@ mod store_hook_tests {
             use hook::position::HookPosition;
             use store::FileLockEntry;
             use storeid::StoreId;
+            use hook::error::HookErrorKind as HEK;
+            use hook::error::CustomData;
+            use libimagerror::into::IntoError;
 
             #[derive(Debug)]
-            pub struct TestHookAccessor(HookPosition);
+            pub struct TestHookAccessor {
+                pos: HookPosition,
+                succeed: bool,
+                error_aborting: bool
+            }
 
             impl TestHookAccessor {
 
-                pub fn new(position: HookPosition) -> TestHookAccessor {
-                    TestHookAccessor(position)
+                pub fn new(position: HookPosition, succeed: bool, error_aborting: bool)
+                    -> TestHookAccessor
+                {
+                    TestHookAccessor {
+                        pos: position,
+                        succeed: succeed,
+                        error_aborting: error_aborting,
+                    }
                 }
 
             }
 
+            fn get_result(succeed: bool, abort: bool) -> HookResult<()> {
+                if succeed {
+                    Ok(())
+                } else {
+                    if abort {
+                        Err(HEK::HookExecutionError.into_error())
+                    } else {
+                        let custom = CustomData::default().aborting(false);
+                        Err(HEK::HookExecutionError.into_error().with_custom_data(custom))
+                    }
+                }
+            }
+
             impl StoreIdAccessor for TestHookAccessor {
 
                 fn access(&self, id: &StoreId) -> HookResult<()> {
-                    Ok(())
+                    get_result(self.succeed, self.error_aborting)
                 }
 
             }
@@ -2453,7 +2479,7 @@ mod store_hook_tests {
             impl MutableHookDataAccessor for TestHookAccessor {
 
                 fn access_mut(&self, fle: &mut FileLockEntry) -> HookResult<()> {
-                    Ok(())
+                    get_result(self.succeed, self.error_aborting)
                 }
 
             }
@@ -2461,7 +2487,7 @@ mod store_hook_tests {
             impl NonMutableHookDataAccessor for TestHookAccessor {
 
                 fn access(&self, fle: &FileLockEntry) -> HookResult<()> {
-                    Ok(())
+                    get_result(self.succeed, self.error_aborting)
                 }
 
             }
@@ -2514,7 +2540,7 @@ aspect = "test"
     fn test_pre_create() {
         let mut store = get_store_with_config();
         let pos       = HP::PreCreate;
-        let hook      = TestHook::new(pos.clone());
+        let hook      = TestHook::new(pos.clone(), true, false);
 
         assert!(store.register_hook(pos, "test", Box::new(hook)).map_err(|e| println!("{:?}", e)).is_ok());
 

From 1244a6666f5ae3098e0fbdf76b1b2c9682a7e46d Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Mon, 3 Oct 2016 12:33:38 +0200
Subject: [PATCH 21/36] Add assert to Store::get() the new ID

---
 libimagstore/src/store.rs | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index d46aaebe..e3c8bad8 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2404,6 +2404,11 @@ mod store_tests {
                     let id_mv_with_base = id_mv.clone().with_base(store.path().clone());
                     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);
             }
         }
     }

From d9f4898a3afb4bb515d7d420421e31e02779034f Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Tue, 20 Sep 2016 16:30:19 +0200
Subject: [PATCH 22/36] Abstract testing of hook execution in helper function

---
 libimagstore/src/store.rs | 72 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 68 insertions(+), 4 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 07511e28..4a9a4524 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2582,16 +2582,80 @@ aspect = "test"
         "#
     }
 
-    #[test]
-    fn test_pre_create() {
+    fn test_hook_execution(hook_positions: &[HP]) {
         let mut store = get_store_with_config();
         let pos       = HP::PreCreate;
         let hook      = TestHook::new(pos.clone(), true, false);
 
-        assert!(store.register_hook(pos, "test", Box::new(hook)).map_err(|e| println!("{:?}", e)).is_ok());
+        println!("Registering hooks...");
+        for pos in hook_positions {
+            let hook = TestHook::new(pos.clone(), true, false);
+            println!("\tRegistering: {:?}", pos);
+            assert!(store.register_hook(pos.clone(), "test", Box::new(hook))
+                    .map_err(|e| println!("{:?}", e))
+                    .is_ok()
+            );
+        }
+        println!("... done.");
 
-        let pb = StoreId::new_baseless(PathBuf::from("test")).unwrap();
+        let pb       = StoreId::new_baseless(PathBuf::from("test")).unwrap();
+        let pb_moved = StoreId::new_baseless(PathBuf::from("test-moved")).unwrap();
+
+        println!("Creating {:?}", pb);
         assert!(store.create(pb.clone()).is_ok());
+
+        {
+            println!("Getting {:?} -> Some?", pb);
+            assert!(match store.get(pb.clone()) {
+                Ok(Some(_)) => true,
+                _           => false,
+            });
+        }
+
+        {
+            println!("Getting {:?} -> None?", pb_moved);
+            assert!(match store.get(pb_moved.clone()) {
+                Ok(None) => true,
+                _        => false,
+            });
+        }
+
+        {
+            println!("Moving {:?} -> {:?}", pb, pb_moved);
+            assert!(store.move_by_id(pb.clone(), pb_moved.clone()).is_ok());
+        }
+
+        {
+            println!("Getting {:?} -> None", pb);
+            assert!(match store.get(pb.clone()) {
+                Ok(None) => true,
+                _        => false,
+            });
+        }
+
+        {
+            println!("Getting {:?} -> Some", pb_moved);
+            assert!(match store.get(pb_moved.clone()) {
+                Ok(Some(_)) => true,
+                _           => false,
+            });
+        }
+
+        {
+            println!("Getting {:?} -> Some -> updating", pb_moved);
+            assert!(match store.get(pb_moved.clone()).map_err(|e| println!("ERROR GETTING: {:?}", e)) {
+                Ok(Some(fle)) => store.update(fle).map_err(|e| println!("ERROR UPDATING: {:?}", e)).is_ok(),
+                _             => false,
+            });
+        }
+
+        println!("Deleting {:?}", pb_moved);
+        assert!(store.delete(pb_moved).is_ok());
+    }
+
+    #[test]
+    fn test_pre_create() {
+        test_hook_execution(&[HP::PreCreate]);
     }
 
 }

From 1e83ad7bbd8834d555bb0e7387e824a2ea1fe503 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Wed, 21 Sep 2016 10:08:01 +0200
Subject: [PATCH 23/36] Add test for hook execution for each hook position

---
 libimagstore/src/store.rs | 57 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 4a9a4524..cbe7b54d 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2654,9 +2654,64 @@ aspect = "test"
     }
 
     #[test]
-    fn test_pre_create() {
+    fn test_storeunload() {
+        test_hook_execution(&[HP::StoreUnload]);
+    }
+
+    #[test]
+    fn test_precreate() {
         test_hook_execution(&[HP::PreCreate]);
     }
 
+    #[test]
+    fn test_postcreate() {
+        test_hook_execution(&[HP::PostCreate]);
+    }
+
+    #[test]
+    fn test_preretrieve() {
+        test_hook_execution(&[HP::PreRetrieve]);
+    }
+
+    #[test]
+    fn test_postretrieve() {
+        test_hook_execution(&[HP::PostRetrieve]);
+    }
+
+    #[test]
+    fn test_preupdate() {
+        test_hook_execution(&[HP::PreUpdate]);
+    }
+
+    #[test]
+    fn test_postupdate() {
+        test_hook_execution(&[HP::PostUpdate]);
+    }
+
+    #[test]
+    fn test_predelete() {
+        test_hook_execution(&[HP::PreDelete]);
+    }
+
+    #[test]
+    fn test_postdelete() {
+        test_hook_execution(&[HP::PostDelete]);
+    }
+
+    #[test]
+    fn test_multiple_same_position() {
+        let positions = [ HP::StoreUnload, HP::PreCreate, HP::PostCreate, HP::PreRetrieve,
+            HP::PostRetrieve, HP::PreUpdate, HP::PostUpdate, HP::PreDelete, HP::PostDelete ];
+
+        for position in positions.iter() {
+            for n in 2..10 {
+                let mut v = Vec::with_capacity(n);
+                for x in 0..n { v.push(position.clone()); }
+
+                test_hook_execution(&v);
+            }
+        }
+    }
+
 }
 

From b189bf7b8cead0bd33190755d46e75f09584cd58 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Tue, 4 Oct 2016 16:36:40 +0200
Subject: [PATCH 24/36] Add check if entry is present

If we try to rename an entry that is borrowed, we fail, as renaming an
borrowed entry might result in some _really_ ugly bugs.
---
 libimagstore/src/store.rs | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index cbe7b54d..1320c51d 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -670,6 +670,13 @@ impl Store {
                 return Err(SEK::EntryAlreadyExists.into_error());
             }
 
+            // if we do not have an entry here, we fail in `FileAbstraction::rename()` below.
+            // if we have one, but it is borrowed, we really should not rename it, as this might
+            // lead to strange errors
+            if hsmap.get(&old_id).map(|e| e.is_borrowed()).unwrap_or(false) {
+                return Err(SEK::EntryAlreadyBorrowed.into_error());
+            }
+
             let old_id_pb = try!(old_id.clone().with_base(self.path().clone()).into_pathbuf());
             let new_id_pb = try!(new_id.clone().with_base(self.path().clone()).into_pathbuf());
 

From c72291159e5da3bd0e19f294dd1fb79a6f17eff0 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 7 Oct 2016 17:47:49 +0200
Subject: [PATCH 25/36] Add comment/documentation for Store::move_by_id()

---
 libimagstore/src/store.rs | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 1320c51d..35e42d17 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -649,6 +649,41 @@ impl Store {
     }
 
     /// Move an entry without loading
+    ///
+    /// This function moves an entry from one path to another.
+    ///
+    /// Generally, this function shouldn't be used by library authors, if they "just" want to move
+    /// something around. A library for moving entries while caring about meta-data and links.
+    ///
+    /// # Errors
+    ///
+    /// This function returns an error in certain cases:
+    ///
+    /// * If pre-move-hooks error (if they return an error which indicates that the action should be
+    ///   aborted)
+    /// * If the about-to-be-moved entry is borrowed
+    /// * If the lock on the internal data structure cannot be aquired
+    /// * If the new path already exists
+    /// * If the about-to-be-moved entry does not exist
+    /// * If the FS-operation failed
+    /// * If the post-move-hooks error (though the operation has succeeded then).
+    ///
+    /// # Warnings
+    ///
+    /// This should be used with _great_ care, as moving an entry from `a` to `b` might result in
+    /// dangling links (see below).
+    ///
+    /// ## Moving linked entries
+    ///
+    /// If the entry which is moved is linked to another entry, these links get invalid (but we do
+    /// not detect this here). As links are always two-way-links, so `a` is not only linked to `b`,
+    /// but also the other way round, moving `b` to `c` results in the following scenario:
+    ///
+    /// * `a` links to `b`, which does not exist anymore.
+    /// * `c` links to `a`, which does exist.
+    ///
+    /// So the link is _partly dangling_, so to say.
+    ///
     pub fn move_by_id(&self, old_id: StoreId, new_id: StoreId) -> Result<()> {
         let new_id = new_id.with_base(self.path().clone());
         let old_id = old_id.with_base(self.path().clone());

From 485d2802367338892ae4f774b7a39038ec0068b7 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 7 Oct 2016 21:15:50 +0200
Subject: [PATCH 26/36] Bugfix: The StoreEntry should know the _new_ StoreId

When moving a entry in the store, we also should tell the StoreEntry
the new id.
---
 libimagstore/src/store.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 35e42d17..242fe10a 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -724,7 +724,10 @@ impl Store {
                     // Should therefor never fail
                     assert!(hsmap
                             .remove(&old_id)
-                            .and_then(|entry| hsmap.insert(new_id.clone(), entry)).is_none())
+                            .and_then(|mut entry| {
+                                entry.id = new_id.clone();
+                                hsmap.insert(new_id.clone(), entry)
+                            }).is_none())
                 }
             }
 

From cf50ddae3380c1736b17f3e31c529d1e8d3361b8 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 7 Oct 2016 18:46:50 +0200
Subject: [PATCH 27/36] [CHERRY-PICK] Add testing implementation for Drop for
 FileLockEntry

---
 libimagstore/src/store.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 242fe10a..5ce2bc6c 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -924,6 +924,7 @@ impl<'a> DerefMut for FileLockEntry<'a> {
     }
 }
 
+#[cfg(not(test))]
 impl<'a> Drop for FileLockEntry<'a> {
     /// This will silently ignore errors, use `Store::update` if you want to catch the errors
     fn drop(&mut self) {
@@ -931,6 +932,15 @@ impl<'a> Drop for FileLockEntry<'a> {
     }
 }
 
+#[cfg(test)]
+impl<'a> Drop for FileLockEntry<'a> {
+    /// This will not silently ignore errors but prints the result of the _update() call for testing
+    fn drop(&mut self) {
+        println!("Drop Result: {:?}", self.store._update(self));
+    }
+}
+
+
 /// `EntryContent` type
 pub type EntryContent = String;
 

From 095ae194162dd58612bd772b3f842e0fe61249d1 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 7 Oct 2016 19:20:34 +0200
Subject: [PATCH 28/36] [CHERRY-PICK] Add flag for _update() whether precense
 should be modified

This is a bugfix for an very particular issue.

Here's what happens:

If we create() an FileLockEntry and then update() it, we are running
into a problem because update() calls _update() which changes the
precense status of a FileLockEntry.

Because update() is _consuming_, the FileLockEntry gets drop()ed
afterwards. This _again_ causes _update() to be called, but with a new
presence status, which is not true in this moment (as the FileLockEntry
is still borrowed, but we already marked it as present).

This patch is a short-term fix. The real problem is, that our Store
interface is consuming. If the Store interface would be non-consuming,
this issue wouldn't happen, as the drop() call would not happen.

I'm rather sure that this patch will not be reverted in the process of
rewriting the Store interface to be non-consuming. But we never know.
---
 libimagstore/src/store.rs | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 5ce2bc6c..75ae0d37 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -507,7 +507,7 @@ impl Store {
                 .map_err_into(SEK::UpdateCallError);
         }
 
-        if let Err(e) = self._update(&entry) {
+        if let Err(e) = self._update(&entry, false) {
             return Err(e).map_err_into(SEK::UpdateCallError);
         }
 
@@ -522,7 +522,7 @@ impl Store {
     /// # Assumptions
     /// This method assumes that entry is dropped _right after_ the call, hence
     /// it is not public.
-    fn _update<'a>(&'a self, entry: &FileLockEntry<'a>) -> Result<()> {
+    fn _update<'a>(&'a self, entry: &FileLockEntry<'a>, modify_presence: bool) -> Result<()> {
         let mut hsmap = match self.entries.write() {
             Err(_) => return Err(SE::new(SEK::LockPoisoned, None)),
             Ok(e) => e,
@@ -537,7 +537,9 @@ impl Store {
 
         debug!("Writing Entry");
         try!(se.write_entry(&entry.entry));
-        se.status = StoreEntryStatus::Present;
+        if modify_presence {
+            se.status = StoreEntryStatus::Present;
+        }
 
         Ok(())
     }
@@ -928,7 +930,7 @@ impl<'a> DerefMut for FileLockEntry<'a> {
 impl<'a> Drop for FileLockEntry<'a> {
     /// This will silently ignore errors, use `Store::update` if you want to catch the errors
     fn drop(&mut self) {
-        let _ = self.store._update(self);
+        let _ = self.store._update(self, true);
     }
 }
 
@@ -936,7 +938,7 @@ impl<'a> Drop for FileLockEntry<'a> {
 impl<'a> Drop for FileLockEntry<'a> {
     /// This will not silently ignore errors but prints the result of the _update() call for testing
     fn drop(&mut self) {
-        println!("Drop Result: {:?}", self.store._update(self));
+        println!("Drop Result: {:?}", self.store._update(self, true));
     }
 }
 

From ae66b00f5f7f44ac06c13b14a5a1a42a1953497a Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 7 Oct 2016 18:10:37 +0200
Subject: [PATCH 29/36] store tests: Add erroring-hook-tests

---
 libimagstore/src/store.rs | 86 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 07511e28..919b6f50 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2594,5 +2594,91 @@ aspect = "test"
         assert!(store.create(pb.clone()).is_ok());
     }
 
+
+    fn get_store_with_aborting_hook_at_pos(pos: HP) -> Store {
+        let mut store = get_store_with_config();
+        let hook      = TestHook::new(pos.clone(), false, true);
+
+        assert!(store.register_hook(pos, "test", Box::new(hook)).map_err(|e| println!("{:?}", e)).is_ok());
+        store
+    }
+
+    fn default_test_id() -> StoreId {
+        StoreId::new_baseless(PathBuf::from("test")).unwrap()
+    }
+
+    #[test]
+    fn test_pre_create_error() {
+        let store   = get_store_with_aborting_hook_at_pos(HP::PreCreate);
+        assert!(store.create(default_test_id()).is_err());
+    }
+
+    #[test]
+    fn test_pre_retrieve_error() {
+        let store   = get_store_with_aborting_hook_at_pos(HP::PreRetrieve);
+        assert!(store.retrieve(default_test_id()).is_err());
+    }
+
+    #[test]
+    fn test_pre_delete_error() {
+        let store   = get_store_with_aborting_hook_at_pos(HP::PreDelete);
+        assert!(store.delete(default_test_id()).is_err());
+    }
+
+    #[test]
+    fn test_pre_update_error() {
+        let store   = get_store_with_aborting_hook_at_pos(HP::PreUpdate);
+        let fle     = store.create(default_test_id()).unwrap();
+
+        assert!(store.update(fle).is_err());
+    }
+
+    #[test]
+    fn test_post_create_error() {
+        let store   = get_store_with_aborting_hook_at_pos(HP::PostCreate);
+        let pb      = default_test_id();
+
+        assert!(store.create(pb.clone()).is_err());
+
+        // But the entry exists, as the hook fails post-create
+        assert!(store.entries.read().unwrap().get(&pb.with_base(store.path().clone())).is_some());
+    }
+
+    #[test]
+    fn test_post_retrieve_error() {
+        let store   = get_store_with_aborting_hook_at_pos(HP::PostRetrieve);
+        let pb      = default_test_id();
+
+        assert!(store.retrieve(pb.clone()).is_err());
+
+        // But the entry exists, as the hook fails post-retrieve
+        assert!(store.entries.read().unwrap().get(&pb.with_base(store.path().clone())).is_some());
+    }
+
+    #[test]
+    fn test_post_delete_error() {
+        let store   = get_store_with_aborting_hook_at_pos(HP::PostDelete);
+        let pb      = default_test_id();
+
+        assert!(store.create(pb.clone()).is_ok());
+        let pb = pb.with_base(store.path().clone());
+        assert!(store.entries.read().unwrap().get(&pb).is_some());
+
+        assert!(store.delete(pb.clone()).is_err());
+        // But the entry is removed, as we fail post-delete
+        assert!(store.entries.read().unwrap().get(&pb).is_none());
+    }
+
+    #[test]
+    fn test_post_update_error() {
+        let store   = get_store_with_aborting_hook_at_pos(HP::PostUpdate);
+        let pb      = default_test_id();
+        let fle     = store.create(pb.clone()).unwrap();
+        let pb      = pb.with_base(store.path().clone());
+
+        assert!(store.entries.read().unwrap().get(&pb).is_some());
+        assert!(store.update(fle).is_err());
+    }
+
 }
 

From d375a6d2c6f031e7c5b602975038992687040d85 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 7 Oct 2016 18:26:48 +0200
Subject: [PATCH 30/36] Add output to test helper, so we can see in the trace
 whats happening

---
 libimagstore/src/store.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 919b6f50..4172b4b2 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2502,12 +2502,16 @@ mod store_hook_tests {
             }
 
             fn get_result(succeed: bool, abort: bool) -> HookResult<()> {
+                println!("Generting result: succeed = {}, abort = {}", succeed, abort);
                 if succeed {
+                    println!("Generating result: Ok(())");
                     Ok(())
                 } else {
                     if abort {
+                        println!("Generating result: Err(_), aborting");
                         Err(HEK::HookExecutionError.into_error())
                     } else {
+                        println!("Generating result: Err(_), not aborting");
                         let custom = CustomData::default().aborting(false);
                         Err(HEK::HookExecutionError.into_error().with_custom_data(custom))
                     }

From 7b11e7dabbcaa61c1250451e91367ceacecc9ed4 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 7 Oct 2016 18:46:50 +0200
Subject: [PATCH 31/36] Add testing implementation for Drop for FileLockEntry

---
 libimagstore/src/store.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 4172b4b2..a423a059 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -879,6 +879,7 @@ impl<'a> DerefMut for FileLockEntry<'a> {
     }
 }
 
+#[cfg(not(test))]
 impl<'a> Drop for FileLockEntry<'a> {
     /// This will silently ignore errors, use `Store::update` if you want to catch the errors
     fn drop(&mut self) {
@@ -886,6 +887,15 @@ impl<'a> Drop for FileLockEntry<'a> {
     }
 }
 
+#[cfg(test)]
+impl<'a> Drop for FileLockEntry<'a> {
+    /// This will not silently ignore errors but prints the result of the _update() call for testing
+    fn drop(&mut self) {
+        println!("Drop Result: {:?}", self.store._update(self));
+    }
+}
+
+
 /// `EntryContent` type
 pub type EntryContent = String;
 

From 5202c5112a63bcfd4ca77081896f7cce40eab768 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Fri, 7 Oct 2016 19:20:34 +0200
Subject: [PATCH 32/36] Add flag for _update() whether precense should be
 modified

This is a bugfix for an very particular issue.

Here's what happens:

If we create() an FileLockEntry and then update() it, we are running
into a problem because update() calls _update() which changes the
precense status of a FileLockEntry.

Because update() is _consuming_, the FileLockEntry gets drop()ed
afterwards. This _again_ causes _update() to be called, but with a new
presence status, which is not true in this moment (as the FileLockEntry
is still borrowed, but we already marked it as present).

This patch is a short-term fix. The real problem is, that our Store
interface is consuming. If the Store interface would be non-consuming,
this issue wouldn't happen, as the drop() call would not happen.

I'm rather sure that this patch will not be reverted in the process of
rewriting the Store interface to be non-consuming. But we never know.
---
 libimagstore/src/store.rs | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index a423a059..f7b3e17f 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -507,7 +507,7 @@ impl Store {
                 .map_err_into(SEK::UpdateCallError);
         }
 
-        if let Err(e) = self._update(&entry) {
+        if let Err(e) = self._update(&entry, false) {
             return Err(e).map_err_into(SEK::UpdateCallError);
         }
 
@@ -522,7 +522,7 @@ impl Store {
     /// # Assumptions
     /// This method assumes that entry is dropped _right after_ the call, hence
     /// it is not public.
-    fn _update<'a>(&'a self, entry: &FileLockEntry<'a>) -> Result<()> {
+    fn _update<'a>(&'a self, entry: &FileLockEntry<'a>, modify_presence: bool) -> Result<()> {
         let mut hsmap = match self.entries.write() {
             Err(_) => return Err(SE::new(SEK::LockPoisoned, None)),
             Ok(e) => e,
@@ -537,7 +537,9 @@ impl Store {
 
         debug!("Writing Entry");
         try!(se.write_entry(&entry.entry));
-        se.status = StoreEntryStatus::Present;
+        if modify_presence {
+            se.status = StoreEntryStatus::Present;
+        }
 
         Ok(())
     }
@@ -883,7 +885,7 @@ impl<'a> DerefMut for FileLockEntry<'a> {
 impl<'a> Drop for FileLockEntry<'a> {
     /// This will silently ignore errors, use `Store::update` if you want to catch the errors
     fn drop(&mut self) {
-        let _ = self.store._update(self);
+        let _ = self.store._update(self, true);
     }
 }
 
@@ -891,7 +893,7 @@ impl<'a> Drop for FileLockEntry<'a> {
 impl<'a> Drop for FileLockEntry<'a> {
     /// This will not silently ignore errors but prints the result of the _update() call for testing
     fn drop(&mut self) {
-        println!("Drop Result: {:?}", self.store._update(self));
+        println!("Drop Result: {:?}", self.store._update(self, true));
     }
 }
 

From 9a8a2f1c29204c93ded0d74de449c4f1a83ae626 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Sun, 9 Oct 2016 13:36:06 +0200
Subject: [PATCH 33/36] Add hook tests for hooks that error with an error that
 does not abort the action

---
 libimagstore/src/store.rs | 81 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 81 insertions(+)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 4d173a07..49abf928 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2860,5 +2860,86 @@ aspect = "test"
         assert!(store.update(fle).is_err());
     }
 
+    fn get_store_with_allowed_error_hook_at_pos(pos: HP) -> Store {
+        let mut store = get_store_with_config();
+        let hook      = TestHook::new(pos.clone(), false, false);
+
+        assert!(store.register_hook(pos, "test", Box::new(hook)).map_err(|e| println!("{:?}", e)).is_ok());
+        store
+    }
+
+    #[test]
+    fn test_pre_create_allowed_error() {
+        let store   = get_store_with_allowed_error_hook_at_pos(HP::PreCreate);
+        assert!(store.create(default_test_id()).is_ok());
+    }
+
+    #[test]
+    fn test_pre_retrieve_allowed_error() {
+        let store   = get_store_with_allowed_error_hook_at_pos(HP::PreRetrieve);
+        assert!(store.retrieve(default_test_id()).is_ok());
+    }
+
+    #[test]
+    fn test_pre_delete_allowed_error() {
+        let store   = get_store_with_allowed_error_hook_at_pos(HP::PreDelete);
+        assert!(store.retrieve(default_test_id()).is_ok());
+        assert!(store.delete(default_test_id()).map_err(|e| println!("{:?}", e)).is_ok());
+    }
+
+    #[test]
+    fn test_pre_update_allowed_error() {
+        let store   = get_store_with_allowed_error_hook_at_pos(HP::PreUpdate);
+        let fle     = store.create(default_test_id()).unwrap();
+
+        assert!(store.update(fle).is_ok());
+    }
+
+    #[test]
+    fn test_post_create_allowed_error() {
+        let store   = get_store_with_allowed_error_hook_at_pos(HP::PostCreate);
+        let pb      = default_test_id();
+
+        assert!(store.create(pb.clone()).is_ok());
+
+        // But the entry exists, as the hook fails post-create
+        assert!(store.entries.read().unwrap().get(&pb.with_base(store.path().clone())).is_some());
+    }
+
+    #[test]
+    fn test_post_retrieve_allowed_error() {
+        let store   = get_store_with_allowed_error_hook_at_pos(HP::PostRetrieve);
+        let pb      = default_test_id();
+
+        assert!(store.retrieve(pb.clone()).is_ok());
+
+        // But the entry exists, as the hook fails post-retrieve
+        assert!(store.entries.read().unwrap().get(&pb.with_base(store.path().clone())).is_some());
+    }
+
+    #[test]
+    fn test_post_delete_allowed_error() {
+        let store   = get_store_with_allowed_error_hook_at_pos(HP::PostDelete);
+        let pb      = default_test_id();
+
+        assert!(store.create(pb.clone()).is_ok());
+        let pb = pb.with_base(store.path().clone());
+        assert!(store.entries.read().unwrap().get(&pb).is_some());
+
+        assert!(store.delete(pb.clone()).is_ok());
+        // But the entry is removed, as we fail post-delete
+        assert!(store.entries.read().unwrap().get(&pb).is_none());
+    }
+
+    #[test]
+    fn test_post_update_allowed_error() {
+        let store   = get_store_with_allowed_error_hook_at_pos(HP::PostUpdate);
+        let pb      = default_test_id();
+        let fle     = store.create(pb.clone()).unwrap();
+        let pb      = pb.with_base(store.path().clone());
+
+        assert!(store.entries.read().unwrap().get(&pb).is_some());
+        assert!(store.update(fle).is_ok());
+    }
 }
 

From 27e816fc69f978a26f502aca9f4d824d1e682977 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Tue, 11 Oct 2016 21:17:48 +0200
Subject: [PATCH 34/36] Do not simply unwrap, but return error in case of error

---
 libimagstore/src/file_abstraction.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libimagstore/src/file_abstraction.rs b/libimagstore/src/file_abstraction.rs
index dc04d235..1b9e6790 100644
--- a/libimagstore/src/file_abstraction.rs
+++ b/libimagstore/src/file_abstraction.rs
@@ -69,14 +69,14 @@ mod fs {
 
         pub fn copy(from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
             let mut map = MAP.lock().unwrap();
-            let a = map.get(from).unwrap().clone();
+            let a = try!(map.get(from).cloned().ok_or(SEK::FileNotFound.into_error()));
             map.insert(to.clone(), a);
             Ok(())
         }
 
         pub fn rename(from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
             let mut map = MAP.lock().unwrap();
-            let a = map.get(from).unwrap().clone();
+            let a = try!(map.get(from).cloned().ok_or(SEK::FileNotFound.into_error()));
             map.insert(to.clone(), a);
             Ok(())
         }

From 600059be82cb528a6b59e2003176d7c410548ade Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Tue, 11 Oct 2016 21:27:00 +0200
Subject: [PATCH 35/36] Add error output

---
 libimagstore/src/store.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 49abf928..0a690153 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2683,7 +2683,7 @@ aspect = "test"
 
         {
             println!("Moving {:?} -> {:?}", pb, pb_moved);
-            assert!(store.move_by_id(pb.clone(), pb_moved.clone()).is_ok());
+            assert!(store.move_by_id(pb.clone(), pb_moved.clone()).map_err(|e| println!("ERROR MOVING: {:?}", e)).is_ok());
         }
 
         {

From b4d2f5c1a6e9580fba9569761d1324bf57538443 Mon Sep 17 00:00:00 2001
From: Matthias Beyer <mail@beyermatthias.de>
Date: Tue, 11 Oct 2016 21:40:44 +0200
Subject: [PATCH 36/36] Each test should test with a unique test file name

---
 libimagstore/src/store.rs | 68 ++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 30 deletions(-)

diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs
index 0a690153..283938b2 100644
--- a/libimagstore/src/store.rs
+++ b/libimagstore/src/store.rs
@@ -2643,7 +2643,7 @@ aspect = "test"
         "#
     }
 
-    fn test_hook_execution(hook_positions: &[HP]) {
+    fn test_hook_execution(hook_positions: &[HP], storeid_name: &str) {
         let mut store = get_store_with_config();
         let pos       = HP::PreCreate;
         let hook      = TestHook::new(pos.clone(), true, false);
@@ -2659,8 +2659,8 @@ aspect = "test"
         }
         println!("... done.");
 
-        let pb       = StoreId::new_baseless(PathBuf::from("test")).unwrap();
-        let pb_moved = StoreId::new_baseless(PathBuf::from("test-moved")).unwrap();
+        let pb       = StoreId::new_baseless(PathBuf::from(storeid_name)).unwrap();
+        let pb_moved = StoreId::new_baseless(PathBuf::from(format!("{}-moved", storeid_name))).unwrap();
 
         println!("Creating {:?}", pb);
         assert!(store.create(pb.clone()).is_ok());
@@ -2716,47 +2716,47 @@ aspect = "test"
 
     #[test]
     fn test_storeunload() {
-        test_hook_execution(&[HP::StoreUnload]);
+        test_hook_execution(&[HP::StoreUnload], "test_storeunload");
     }
 
     #[test]
     fn test_precreate() {
-        test_hook_execution(&[HP::PreCreate]);
+        test_hook_execution(&[HP::PreCreate], "test_precreate");
     }
 
     #[test]
     fn test_postcreate() {
-        test_hook_execution(&[HP::PostCreate]);
+        test_hook_execution(&[HP::PostCreate], "test_postcreate");
     }
 
     #[test]
     fn test_preretrieve() {
-        test_hook_execution(&[HP::PreRetrieve]);
+        test_hook_execution(&[HP::PreRetrieve], "test_preretrieve");
     }
 
     #[test]
     fn test_postretrieve() {
-        test_hook_execution(&[HP::PostRetrieve]);
+        test_hook_execution(&[HP::PostRetrieve], "test_postretrieve");
     }
 
     #[test]
     fn test_preupdate() {
-        test_hook_execution(&[HP::PreUpdate]);
+        test_hook_execution(&[HP::PreUpdate], "test_preupdate");
     }
 
     #[test]
     fn test_postupdate() {
-        test_hook_execution(&[HP::PostUpdate]);
+        test_hook_execution(&[HP::PostUpdate], "test_postupdate");
     }
 
     #[test]
     fn test_predelete() {
-        test_hook_execution(&[HP::PreDelete]);
+        test_hook_execution(&[HP::PreDelete], "test_predelete");
     }
 
     #[test]
     fn test_postdelete() {
-        test_hook_execution(&[HP::PostDelete]);
+        test_hook_execution(&[HP::PostDelete], "test_postdelete");
     }
 
     #[test]
@@ -2769,7 +2769,7 @@ aspect = "test"
                 let mut v = Vec::with_capacity(n);
                 for x in 0..n { v.push(position.clone()); }
 
-                test_hook_execution(&v);
+                test_hook_execution(&v, "test_multiple_same_position");
             }
         }
     }
@@ -2789,26 +2789,30 @@ aspect = "test"
 
     #[test]
     fn test_pre_create_error() {
+        let storeid = StoreId::new_baseless(PathBuf::from("test_pre_create_error")).unwrap();
         let store   = get_store_with_aborting_hook_at_pos(HP::PreCreate);
-        assert!(store.create(default_test_id()).is_err());
+        assert!(store.create(storeid).is_err());
     }
 
     #[test]
     fn test_pre_retrieve_error() {
+        let storeid = StoreId::new_baseless(PathBuf::from("test_pre_retrieve_error")).unwrap();
         let store   = get_store_with_aborting_hook_at_pos(HP::PreRetrieve);
-        assert!(store.retrieve(default_test_id()).is_err());
+        assert!(store.retrieve(storeid).is_err());
     }
 
     #[test]
     fn test_pre_delete_error() {
+        let storeid = StoreId::new_baseless(PathBuf::from("test_pre_delete_error")).unwrap();
         let store   = get_store_with_aborting_hook_at_pos(HP::PreDelete);
-        assert!(store.delete(default_test_id()).is_err());
+        assert!(store.delete(storeid).is_err());
     }
 
     #[test]
     fn test_pre_update_error() {
+        let storeid = StoreId::new_baseless(PathBuf::from("test_pre_update_error")).unwrap();
         let store   = get_store_with_aborting_hook_at_pos(HP::PreUpdate);
-        let fle     = store.create(default_test_id()).unwrap();
+        let fle     = store.create(storeid).unwrap();
 
         assert!(store.update(fle).is_err());
     }
@@ -2816,7 +2820,7 @@ aspect = "test"
     #[test]
     fn test_post_create_error() {
         let store   = get_store_with_aborting_hook_at_pos(HP::PostCreate);
-        let pb      = default_test_id();
+        let pb      = StoreId::new_baseless(PathBuf::from("test_post_create_error")).unwrap();
 
         assert!(store.create(pb.clone()).is_err());
 
@@ -2827,7 +2831,7 @@ aspect = "test"
     #[test]
     fn test_post_retrieve_error() {
         let store   = get_store_with_aborting_hook_at_pos(HP::PostRetrieve);
-        let pb      = default_test_id();
+        let pb      = StoreId::new_baseless(PathBuf::from("test_post_retrieve_error")).unwrap();
 
         assert!(store.retrieve(pb.clone()).is_err());
 
@@ -2838,7 +2842,7 @@ aspect = "test"
     #[test]
     fn test_post_delete_error() {
         let store   = get_store_with_aborting_hook_at_pos(HP::PostDelete);
-        let pb      = default_test_id();
+        let pb      = StoreId::new_baseless(PathBuf::from("test_post_delete_error")).unwrap();
 
         assert!(store.create(pb.clone()).is_ok());
         let pb = pb.with_base(store.path().clone());
@@ -2852,7 +2856,7 @@ aspect = "test"
     #[test]
     fn test_post_update_error() {
         let store   = get_store_with_aborting_hook_at_pos(HP::PostUpdate);
-        let pb      = default_test_id();
+        let pb      = StoreId::new_baseless(PathBuf::from("test_post_update_error")).unwrap();
         let fle     = store.create(pb.clone()).unwrap();
         let pb      = pb.with_base(store.path().clone());
 
@@ -2870,27 +2874,31 @@ aspect = "test"
 
     #[test]
     fn test_pre_create_allowed_error() {
+        let storeid = StoreId::new_baseless(PathBuf::from("test_pre_create_allowed_error")).unwrap();
         let store   = get_store_with_allowed_error_hook_at_pos(HP::PreCreate);
-        assert!(store.create(default_test_id()).is_ok());
+        assert!(store.create(storeid).is_ok());
     }
 
     #[test]
     fn test_pre_retrieve_allowed_error() {
+        let storeid = StoreId::new_baseless(PathBuf::from("test_pre_retrieve_allowed_error")).unwrap();
         let store   = get_store_with_allowed_error_hook_at_pos(HP::PreRetrieve);
-        assert!(store.retrieve(default_test_id()).is_ok());
+        assert!(store.retrieve(storeid).is_ok());
     }
 
     #[test]
     fn test_pre_delete_allowed_error() {
+        let storeid = StoreId::new_baseless(PathBuf::from("test_pre_delete_allowed_error")).unwrap();
         let store   = get_store_with_allowed_error_hook_at_pos(HP::PreDelete);
-        assert!(store.retrieve(default_test_id()).is_ok());
-        assert!(store.delete(default_test_id()).map_err(|e| println!("{:?}", e)).is_ok());
+        assert!(store.retrieve(storeid.clone()).is_ok());
+        assert!(store.delete(storeid).map_err(|e| println!("{:?}", e)).is_ok());
     }
 
     #[test]
     fn test_pre_update_allowed_error() {
+        let storeid = StoreId::new_baseless(PathBuf::from("test_pre_update_allowed_error")).unwrap();
         let store   = get_store_with_allowed_error_hook_at_pos(HP::PreUpdate);
-        let fle     = store.create(default_test_id()).unwrap();
+        let fle     = store.create(storeid).unwrap();
 
         assert!(store.update(fle).is_ok());
     }
@@ -2898,7 +2906,7 @@ aspect = "test"
     #[test]
     fn test_post_create_allowed_error() {
         let store   = get_store_with_allowed_error_hook_at_pos(HP::PostCreate);
-        let pb      = default_test_id();
+        let pb      = StoreId::new_baseless(PathBuf::from("test_pre_create_allowed_error")).unwrap();
 
         assert!(store.create(pb.clone()).is_ok());
 
@@ -2909,7 +2917,7 @@ aspect = "test"
     #[test]
     fn test_post_retrieve_allowed_error() {
         let store   = get_store_with_allowed_error_hook_at_pos(HP::PostRetrieve);
-        let pb      = default_test_id();
+        let pb      = StoreId::new_baseless(PathBuf::from("test_pre_retrieve_allowed_error")).unwrap();
 
         assert!(store.retrieve(pb.clone()).is_ok());
 
@@ -2920,7 +2928,7 @@ aspect = "test"
     #[test]
     fn test_post_delete_allowed_error() {
         let store   = get_store_with_allowed_error_hook_at_pos(HP::PostDelete);
-        let pb      = default_test_id();
+        let pb      = StoreId::new_baseless(PathBuf::from("test_pre_delete_allowed_error")).unwrap();
 
         assert!(store.create(pb.clone()).is_ok());
         let pb = pb.with_base(store.path().clone());
@@ -2934,7 +2942,7 @@ aspect = "test"
     #[test]
     fn test_post_update_allowed_error() {
         let store   = get_store_with_allowed_error_hook_at_pos(HP::PostUpdate);
-        let pb      = default_test_id();
+        let pb      = StoreId::new_baseless(PathBuf::from("test_pre_update_allowed_error")).unwrap();
         let fle     = store.create(pb.clone()).unwrap();
         let pb      = pb.with_base(store.path().clone());