summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatthias Beyer <mail@beyermatthias.de>2018-07-20 01:35:20 +0200
committerGitHub <noreply@github.com>2018-07-20 01:35:20 +0200
commit2a62b6dffb9f82a464cc7d9e21a488aee04197ca (patch)
treec0ba3980a0f02cbfff72d35b07884bf69b10ae93
parent851db4abe4d8d0aebafe35ef41c5594213b1be6b (diff)
parentaff6ff105d18e1eea14fe89e9c629efa76fd020f (diff)
downloadimag-2a62b6dffb9f82a464cc7d9e21a488aee04197ca.zip
imag-2a62b6dffb9f82a464cc7d9e21a488aee04197ca.tar.gz
Merge pull request #1496 from matthiasbeyer/libimagstore/fix-use-backend
libimagstore: fix use backend
-rw-r--r--bin/core/imag-store/src/verify.rs5
-rw-r--r--lib/core/libimagstore/src/store.rs189
-rw-r--r--lib/core/libimagstore/src/util.rs3
3 files changed, 55 insertions, 142 deletions
diff --git a/bin/core/imag-store/src/verify.rs b/bin/core/imag-store/src/verify.rs
index 900fde0..8fa0098 100644
--- a/bin/core/imag-store/src/verify.rs
+++ b/bin/core/imag-store/src/verify.rs
@@ -23,7 +23,6 @@ use libimagrt::runtime::Runtime;
use libimagutil::warn_exit::warn_exit;
use libimagerror::trace::MapErrTrace;
use libimagerror::iter::TraceIterator;
-use libimagstore::store::Header;
/// Verify the store.
///
@@ -41,13 +40,13 @@ pub fn verify(rt: &Runtime) {
.all(|fle| {
let p = fle.get_location();
let content_len = fle.get_content().len();
- let (header, status) = if fle.get_header().verify().is_ok() {
+ let (verify, status) = if fle.verify().is_ok() {
("ok", true)
} else {
("broken", false)
};
- info!("{: >6} | {: >14} | {:?}", header, content_len, p.deref());
+ info!("{: >6} | {: >14} | {:?}", verify, content_len, p.deref());
status
});
diff --git a/lib/core/libimagstore/src/store.rs b/lib/core/libimagstore/src/store.rs
index e85b65f..8b19994 100644
--- a/lib/core/libimagstore/src/store.rs
+++ b/lib/core/libimagstore/src/store.rs
@@ -444,7 +444,7 @@ impl Store {
StoreEntry::new(id, &self.backend)?.get_entry()
}
- /// Delete an entry
+ /// Delete an entry and the corrosponding file on disk
///
/// # Return value
///
@@ -460,6 +460,12 @@ impl Store {
debug!("Deleting id: '{}'", 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 = id.clone().into_pathbuf()?;
+
{
let mut entries = self
.entries
@@ -467,44 +473,41 @@ impl Store {
.map_err(|_| SE::from_kind(SEK::LockPoisoned))
.chain_err(|| SEK::DeleteCallError(id.clone()))?;
- // if the entry is currently modified by the user, we cannot drop it
- match entries.get(&id) {
+ let do_remove = match entries.get(&id) {
+ Some(e) => if e.is_borrowed() { // entry is currently borrowed, we cannot delete it
+ return Err(SE::from_kind(SEK::IdLocked)).chain_err(|| SEK::DeleteCallError(id));
+ // false
+ } else { // Entry is in the cache
+ // Remove Entry from the cache
+ true
+ },
+
None => {
// 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 = id.clone().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 {
+ if !self.backend.exists(&pb)? {
debug!("Seems like {:?} is not even on the FS", pb);
return Err(SE::from_kind(SEK::FileNotFound))
.chain_err(|| SEK::DeleteCallError(id))
- }
+ } // else { continue }
+
+ false
},
- Some(e) => if e.is_borrowed() {
- return Err(SE::from_kind(SEK::IdLocked))
- .chain_err(|| SEK::DeleteCallError(id))
- }
- }
+ };
- // remove the entry first, then the file
- entries.remove(&id);
- let pb = id.clone().with_base(self.path().clone()).into_pathbuf()?;
- let _ = self
- .backend
- .remove_file(&pb)
- .chain_err(|| SEK::FileError)
- .chain_err(|| SEK::DeleteCallError(id))?;
+ if do_remove {
+ let _ = entries.remove(&id);
+ }
}
+ debug!("Seems like {:?} is on the FS", pb);
+ let _ = self
+ .backend
+ .remove_file(&pb)
+ .chain_err(|| SEK::FileError)
+ .chain_err(|| SEK::DeleteCallError(id))?;
+
debug!("Deleted");
Ok(())
}
@@ -662,19 +665,6 @@ impl Debug for Store {
}
-impl Drop for Store {
-
- ///
- /// Unlock all files on drop
- //
- /// TODO: Unlock them
- ///
- fn drop(&mut self) {
- debug!("Dropping store");
- }
-
-}
-
/// A struct that allows you to borrow an Entry
pub struct FileLockEntry<'a> {
store: &'a Store,
@@ -776,7 +766,18 @@ impl Entry {
/// This function should be used to get a new Header, as the default header may change. Via
/// this function, compatibility is ensured.
pub fn default_header() -> Value { // BTreeMap<String, Value>
- Value::default_header()
+ let mut m = BTreeMap::new();
+
+ m.insert(String::from("imag"), {
+ let mut imag_map = BTreeMap::<String, Value>::new();
+
+ imag_map.insert(String::from("version"),
+ Value::String(String::from(env!("CARGO_PKG_VERSION"))));
+
+ Value::Table(imag_map)
+ });
+
+ Value::Table(m)
}
/// See `Entry::from_str()`, as this function is used internally. This is just a wrapper for
@@ -863,37 +864,11 @@ impl Entry {
///
/// Currently, this only verifies the header. This might change in the future.
pub fn verify(&self) -> Result<()> {
- self.header.verify()
- }
-
-}
-
-impl PartialEq for Entry {
-
- fn eq(&self, other: &Entry) -> bool {
- self.location == other.location && // As the location only compares from the store root
- self.header == other.header && // and the other Entry could be from another store (not
- self.content == other.content // implemented by now, but we think ahead here)
- }
-
-}
-
-/// Extension trait for top-level toml::Value::Table, will only yield correct results on the
-/// top-level Value::Table, but not on intermediate tables.
-pub trait Header {
- fn verify(&self) -> Result<()>;
- fn parse(s: &str) -> Result<Value>;
- fn default_header() -> Value;
-}
-
-impl Header for Value {
-
- fn verify(&self) -> Result<()> {
- if !has_main_section(self)? {
+ if !has_main_section(&self.header)? {
Err(SE::from_kind(SEK::MissingMainSection))
- } else if !has_imag_version_in_main_section(self)? {
+ } else if !has_imag_version_in_main_section(&self.header)? {
Err(SE::from_kind(SEK::MissingVersionInfo))
- } else if !has_only_tables(self)? {
+ } else if !has_only_tables(&self.header)? {
debug!("Could not verify that it only has tables in its base table");
Err(SE::from_kind(SEK::NonTableInBaseTable))
} else {
@@ -901,27 +876,14 @@ impl Header for Value {
}
}
- fn parse(s: &str) -> Result<Value> {
- use toml::de::from_str;
-
- from_str(s)
- .map_err(From::from)
- .and_then(|h: Value| h.verify().map(|_| h))
- }
-
- fn default_header() -> Value {
- let mut m = BTreeMap::new();
-
- m.insert(String::from("imag"), {
- let mut imag_map = BTreeMap::<String, Value>::new();
-
- imag_map.insert(String::from("version"),
- Value::String(String::from(env!("CARGO_PKG_VERSION"))));
+}
- Value::Table(imag_map)
- });
+impl PartialEq for Entry {
- Value::Table(m)
+ fn eq(&self, other: &Entry) -> bool {
+ self.location == other.location && // As the location only compares from the store root
+ self.header == other.header && // and the other Entry could be from another store (not
+ self.content == other.content // implemented by now, but we think ahead here)
}
}
@@ -954,7 +916,6 @@ mod test {
use std::collections::BTreeMap;
use storeid::StoreId;
- use store::Header;
use store::has_main_section;
use store::has_imag_version_in_main_section;
@@ -1004,52 +965,6 @@ mod test {
assert!(has_imag_version_in_main_section(&Value::Table(map)).is_err());
}
- #[test]
- fn test_verification_good() {
- let mut header = BTreeMap::new();
- let sub = {
- let mut sub = BTreeMap::new();
- sub.insert("version".into(), Value::String(String::from("0.0.0")));
-
- Value::Table(sub)
- };
-
- header.insert("imag".into(), sub);
-
- assert!(Value::Table(header).verify().is_ok());
- }
-
- #[test]
- fn test_verification_invalid_versionstring() {
- let mut header = BTreeMap::new();
- let sub = {
- let mut sub = BTreeMap::new();
- sub.insert("version".into(), Value::String(String::from("000")));
-
- Value::Table(sub)
- };
-
- header.insert("imag".into(), sub);
-
- assert!(!Value::Table(header).verify().is_ok());
- }
-
-
- #[test]
- fn test_verification_current_version() {
- let mut header = BTreeMap::new();
- let sub = {
- let mut sub = BTreeMap::new();
- sub.insert("version".into(), Value::String(String::from(env!("CARGO_PKG_VERSION"))));
-
- Value::Table(sub)
- };
-
- header.insert("imag".into(), sub);
-
- assert!(Value::Table(header).verify().is_ok());
- }
-
static TEST_ENTRY : &'static str = "---
[imag]
version = '0.0.3'
diff --git a/lib/core/libimagstore/src/util.rs b/lib/core/libimagstore/src/util.rs
index 61b232a..6d7d478 100644
--- a/lib/core/libimagstore/src/util.rs
+++ b/lib/core/libimagstore/src/util.rs
@@ -22,7 +22,6 @@ use std::fmt::Write;
use toml::Value;
use store::Result;
-use store::Header;
#[cfg(feature = "early-panic")]
#[macro_export]
@@ -61,7 +60,7 @@ pub fn entry_buffer_to_header_content(buf: &str) -> Result<(Value, String)> {
}
}
- Ok((Value::parse(&header)?, content))
+ ::toml::de::from_str(&header).map_err(From::from).map(|h| (h, content))
}
#[cfg(test)]