From 8bbaeeef451905b565cf9666ab3b28c2535ae732 Mon Sep 17 00:00:00 2001 From: Matthias Beyer Date: Wed, 20 Sep 2017 22:22:50 +0200 Subject: [PATCH] Fix: Store::delete() should check FS as well This patch fixes a problem where the Store::delete() function only checked the store-internal cache whether an entry exists, but not the Filesystem. After this patch is applied, the Store::delete() function also checks the filesystem whether the entry exists. --- doc/src/09020-changelog.md | 2 ++ lib/core/libimagstore/src/store.rs | 19 ++++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/doc/src/09020-changelog.md b/doc/src/09020-changelog.md index 00bc1da8..bfc8eda0 100644 --- a/doc/src/09020-changelog.md +++ b/doc/src/09020-changelog.md @@ -44,6 +44,8 @@ This section contains the changelog from the last release to the next release. entries. This is not allowed because this namespace is reserved for the store itself. This bug was fixed, links are now located in the `links` namespace in the header of an entry. + * `Store::delete()` did only check the store-internal cache whether an entry + exists, but not the filesystem. This was fixed. * Minor changes * If building from a `nix-shell`, the mozilla rust overlay is expected to be present diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs index 76d792d8..fb651070 100644 --- a/lib/core/libimagstore/src/store.rs +++ b/lib/core/libimagstore/src/store.rs @@ -630,7 +630,24 @@ impl Store { // if the entry is currently modified by the user, we cannot drop it match entries.get(&id) { None => { - return Err(SE::from_kind(SEK::FileNotFound)).chain_err(|| SEK::DeleteCallError) + // The entry is not in the internal cache. But maybe on the filesystem? + debug!("Seems like {:?} is not in the internal cache", id); + + // Small optimization: We need the pathbuf for deleting, but when calling + // StoreId::exists(), a PathBuf object gets allocated. So we simply get a + // PathBuf here, check whether it is there and if it is, we can re-use it to + // delete the filesystem file. + let pb = try!(id.into_pathbuf()); + + if pb.exists() { + // looks like we're deleting a not-loaded file from the store. + debug!("Seems like {:?} is on the FS", pb); + return self.backend.remove_file(&pb) + } else { + debug!("Seems like {:?} is not even on the FS", pb); + return Err(SE::from_kind(SEK::FileNotFound)) + .chain_err(|| SEK::DeleteCallError) + } }, Some(e) => if e.is_borrowed() { return Err(SE::from_kind(SEK::IdLocked)).chain_err(|| SEK::DeleteCallError)