From 12b8f8f33105ca340700fd63ff004a3773b0fb47 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 2 Jul 2016 17:52:12 +0200 Subject: [PATCH 1/7] Move storification of StoreId object to new StoreId::storified() func --- libimagstore/src/store.rs | 24 ++++++++---------------- libimagstore/src/storeid.rs | 12 ++++++++++++ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs index 82dd1a2e..1d7a3931 100644 --- a/libimagstore/src/store.rs +++ b/libimagstore/src/store.rs @@ -317,17 +317,9 @@ impl Store { self.configuration.as_ref() } - fn storify_id(&self, id: StoreId) -> StoreId { - debug!("Create new store id out of: {:?} and {:?}", self.location, id); - let mut new_id = self.location.clone(); - new_id.push(id); - debug!("Created: '{:?}'", new_id); - StoreId::from(new_id) - } - /// Creates the Entry at the given location (inside the entry) pub fn create<'a, S: IntoStoreId>(&'a self, id: S) -> Result> { - let id = self.storify_id(id.into_storeid()); + let id = id.into_storeid().storified(self); if let Err(e) = self.execute_hooks_for_id(self.pre_create_aspects.clone(), &id) { if e.is_aborting() { return Err(e) @@ -365,7 +357,7 @@ impl Store { /// Implicitely creates a entry in the store if there is no entry with the id `id`. For a /// non-implicitely-create look at `Store::get`. pub fn retrieve<'a, S: IntoStoreId>(&'a self, id: S) -> Result> { - let id = self.storify_id(id.into_storeid()); + let id = id.into_storeid().storified(self); if let Err(e) = self.execute_hooks_for_id(self.pre_retrieve_aspects.clone(), &id) { if e.is_aborting() { return Err(e) @@ -398,7 +390,7 @@ impl Store { /// /// This executes the {pre,post}_retrieve_aspects hooks. pub fn get<'a, S: IntoStoreId + Clone>(&'a self, id: S) -> Result>> { - if !self.storify_id(id.clone().into_storeid()).exists() { + if !id.clone().into_storeid().storified(self).exists() { debug!("Does not exist: {:?}", id.clone().into_storeid()); return Ok(None); } @@ -520,7 +512,7 @@ impl Store { /// Retrieve a copy of a given entry, this cannot be used to mutate /// the one on disk pub fn retrieve_copy(&self, id: S) -> Result { - let id = self.storify_id(id.into_storeid()); + let id = id.into_storeid().storified(self); let entries = match self.entries.write() { Err(_) => { return Err(SE::new(SEK::LockPoisoned, None)) @@ -539,7 +531,7 @@ impl Store { /// Delete an entry pub fn delete(&self, id: S) -> Result<()> { - let id = self.storify_id(id.into_storeid()); + let id = id.into_storeid().storified(self); if let Err(e) = self.execute_hooks_for_id(self.pre_delete_aspects.clone(), &id) { if e.is_aborting() { return Err(e) @@ -593,7 +585,7 @@ impl Store { use std::fs::copy; use std::fs::remove_file; - let new_id = self.storify_id(new_id); + let new_id = new_id.storified(self); let hsmap = self.entries.write(); if hsmap.is_err() { return Err(SE::new(SEK::LockPoisoned, None)).map_err_into(SEK::MoveCallError) @@ -622,8 +614,8 @@ impl Store { pub fn move_by_id(&self, old_id: StoreId, new_id: StoreId) -> Result<()> { use std::fs::rename; - let new_id = self.storify_id(new_id); - let old_id = self.storify_id(old_id); + let new_id = new_id.storified(self); + let old_id = old_id.storified(self); if let Err(e) = self.execute_hooks_for_id(self.pre_move_aspects.clone(), &old_id) { if e.is_aborting() { diff --git a/libimagstore/src/storeid.rs b/libimagstore/src/storeid.rs index 147b80cb..48f8b834 100644 --- a/libimagstore/src/storeid.rs +++ b/libimagstore/src/storeid.rs @@ -16,6 +16,18 @@ use store::Store; #[derive(Debug, Clone, PartialEq, Hash, Eq, PartialOrd, Ord)] pub struct StoreId(PathBuf); +impl StoreId { + + pub fn storified(self, store: &Store) -> StoreId { + debug!("Create new store id out of: {:?} and {:?}", store.path(), self); + let mut new_id = store.path().clone(); + new_id.push(self); + debug!("Created: '{:?}'", new_id); + StoreId::from(new_id) + } + +} + impl Into for StoreId { fn into(self) -> PathBuf { From 37380c84b9a4ddb8dc59beab1f961a54eb31b643 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 2 Jul 2016 17:54:35 +0200 Subject: [PATCH 2/7] Add StoreId::unstorified() --- libimagstore/src/storeid.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libimagstore/src/storeid.rs b/libimagstore/src/storeid.rs index 48f8b834..93c19dd7 100644 --- a/libimagstore/src/storeid.rs +++ b/libimagstore/src/storeid.rs @@ -9,6 +9,7 @@ use std::fmt::Error as FmtError; use std::result::Result as RResult; use error::StoreErrorKind as SEK; +use error::MapErrInto; use store::Result; use store::Store; @@ -26,6 +27,13 @@ impl StoreId { StoreId::from(new_id) } + pub fn unstorified(self, store: &Store) -> Result { + self.strip_prefix(store.path()) + .map(PathBuf::from) + .map(StoreId::from) + .map_err_into(SEK::StoreIdHandlingError) + } + } impl Into for StoreId { From 9f213ddc314691f9a15b33aaee5328b7b345cc49 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 2 Jul 2016 23:26:20 +0200 Subject: [PATCH 3/7] Add error kind for storeid handling errors --- libimagstore/src/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/libimagstore/src/error.rs b/libimagstore/src/error.rs index a3fb7d5c..d9a7fd99 100644 --- a/libimagstore/src/error.rs +++ b/libimagstore/src/error.rs @@ -34,6 +34,7 @@ generate_custom_error_types!(StoreError, StoreErrorKind, CustomErrorData, EncodingError => "Encoding error", StorePathError => "Store Path error", EntryRenameError => "Entry rename error", + StoreIdHandlingError => "StoreId handling error", CreateCallError => "Error when calling create()", RetrieveCallError => "Error when calling retrieve()", From 41d7d1c213dc98a7962fe92bba1539706040309b Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Sat, 2 Jul 2016 23:26:43 +0200 Subject: [PATCH 4/7] Store::create() Make outgoing storeid object unstorified --- libimagstore/src/store.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs index 1d7a3931..b992ee6c 100644 --- a/libimagstore/src/store.rs +++ b/libimagstore/src/store.rs @@ -344,10 +344,13 @@ impl Store { se }); - let mut fle = FileLockEntry::new(self, Entry::new(id)); - self.execute_hooks_for_mut_file(self.post_create_aspects.clone(), &mut fle) - .map_err_into(SEK::PostHookExecuteError) - .map(|_| fle) + id.unstorified(self) + .and_then(|id| { + let mut fle = FileLockEntry::new(self, Entry::new(id)); + self.execute_hooks_for_mut_file(self.post_create_aspects.clone(), &mut fle) + .map_err_into(SEK::PostHookExecuteError) + .map(|_| fle) + }) .map_err_into(SEK::CreateCallError) } From 31d5dac63dcf8063a87205b684cf6ba81593f5be Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Jul 2016 12:45:17 +0200 Subject: [PATCH 5/7] Revert "Add StoreId::unstorified()" This reverts commit 37380c84b9a4ddb8dc59beab1f961a54eb31b643. We do not want such a feature in the store, actually. StoreId objects are either storified or not, but you cannot unstorify them. --- libimagstore/src/storeid.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/libimagstore/src/storeid.rs b/libimagstore/src/storeid.rs index 93c19dd7..48f8b834 100644 --- a/libimagstore/src/storeid.rs +++ b/libimagstore/src/storeid.rs @@ -9,7 +9,6 @@ use std::fmt::Error as FmtError; use std::result::Result as RResult; use error::StoreErrorKind as SEK; -use error::MapErrInto; use store::Result; use store::Store; @@ -27,13 +26,6 @@ impl StoreId { StoreId::from(new_id) } - pub fn unstorified(self, store: &Store) -> Result { - self.strip_prefix(store.path()) - .map(PathBuf::from) - .map(StoreId::from) - .map_err_into(SEK::StoreIdHandlingError) - } - } impl Into for StoreId { From a706680fd59e7e5ef925a9ba5b7126906b796fd7 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Jul 2016 12:46:06 +0200 Subject: [PATCH 6/7] Revert "Store::create() Make outgoing storeid object unstorified" This reverts commit 41d7d1c213dc98a7962fe92bba1539706040309b. --- libimagstore/src/store.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/libimagstore/src/store.rs b/libimagstore/src/store.rs index b992ee6c..1d7a3931 100644 --- a/libimagstore/src/store.rs +++ b/libimagstore/src/store.rs @@ -344,13 +344,10 @@ impl Store { se }); - id.unstorified(self) - .and_then(|id| { - let mut fle = FileLockEntry::new(self, Entry::new(id)); - self.execute_hooks_for_mut_file(self.post_create_aspects.clone(), &mut fle) - .map_err_into(SEK::PostHookExecuteError) - .map(|_| fle) - }) + let mut fle = FileLockEntry::new(self, Entry::new(id)); + self.execute_hooks_for_mut_file(self.post_create_aspects.clone(), &mut fle) + .map_err_into(SEK::PostHookExecuteError) + .map(|_| fle) .map_err_into(SEK::CreateCallError) } From 9605d6daa636b391f0f88f8833b0fb7763ed32ad Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Mon, 4 Jul 2016 12:49:12 +0200 Subject: [PATCH 7/7] Ensure that StoreId::storified() does not alter already storified StoreId objects --- libimagstore/src/storeid.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/libimagstore/src/storeid.rs b/libimagstore/src/storeid.rs index 48f8b834..a1be9fa4 100644 --- a/libimagstore/src/storeid.rs +++ b/libimagstore/src/storeid.rs @@ -19,11 +19,16 @@ pub struct StoreId(PathBuf); impl StoreId { pub fn storified(self, store: &Store) -> StoreId { - debug!("Create new store id out of: {:?} and {:?}", store.path(), self); - let mut new_id = store.path().clone(); - new_id.push(self); - debug!("Created: '{:?}'", new_id); - StoreId::from(new_id) + if self.starts_with(store.path()) { + debug!("Not storifying {:?}, because it is already.", self); + self + } else { + debug!("Create new store id out of: {:?} and {:?}", store.path(), self); + let mut new_id = store.path().clone(); + new_id.push(self); + debug!("Created: '{:?}'", new_id); + StoreId::from(new_id) + } } }