matthiasbeyer/imag
1
0
Fork 0
Code Releases Activity

Merge pull request #1496 from matthiasbeyer/libimagstore/fix-use-backend

Browse source 
libimagstore: fix use backend
This commit is contained in:
Matthias Beyer 2018-07-20 01:35:20 +02:00 committed by GitHub
commit 2a62b6dffb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 57 additions and 144 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
bin/core/imag-store/src/verify.rs
View file

@ -23,7 +23,6 @@ use libimagrt::runtime::Runtime;
use libimagutil::warn_exit::warn_exit; use libimagutil::warn_exit::warn_exit;
use libimagerror::trace::MapErrTrace; use libimagerror::trace::MapErrTrace;
use libimagerror::iter::TraceIterator; use libimagerror::iter::TraceIterator;
use libimagstore::store::Header;
/// Verify the store. /// Verify the store.
/// ///
@ -41,13 +40,13 @@ pub fn verify(rt: &Runtime) {
.all(|fle| { .all(|fle| {
let p = fle.get_location(); let p = fle.get_location();
let content_len = fle.get_content().len(); 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) ("ok", true)
} else { } else {
("broken", false) ("broken", false)
}; };
info!("{: >6} | {: >14} | {:?}", header, content_len, p.deref()); info!("{: >6} | {: >14} | {:?}", verify, content_len, p.deref());
status status
}); });

193
lib/core/libimagstore/src/store.rs
View file

@ -444,7 +444,7 @@ impl Store {
StoreEntry::new(id, &self.backend)?.get_entry() StoreEntry::new(id, &self.backend)?.get_entry()
} }
/// Delete an entry /// Delete an entry and the corrosponding file on disk
/// ///
/// # Return value /// # Return value
/// ///
@ -460,6 +460,12 @@ impl Store {
debug!("Deleting id: '{}'", id); 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 let mut entries = self
.entries .entries
@ -467,44 +473,41 @@ impl Store {
.map_err(|_| SE::from_kind(SEK::LockPoisoned)) .map_err(|_| SE::from_kind(SEK::LockPoisoned))
.chain_err(|| SEK::DeleteCallError(id.clone()))?; .chain_err(|| SEK::DeleteCallError(id.clone()))?;
// if the entry is currently modified by the user, we cannot drop it let do_remove = match entries.get(&id) {
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 => { None => {
// The entry is not in the internal cache. But maybe on the filesystem? // The entry is not in the internal cache. But maybe on the filesystem?
debug!("Seems like {:?} is not in the internal cache", id); debug!("Seems like {:?} is not in the internal cache", id);
// Small optimization: We need the pathbuf for deleting, but when calling if !self.backend.exists(&pb)? {
// 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 {
debug!("Seems like {:?} is not even on the FS", pb); debug!("Seems like {:?} is not even on the FS", pb);
return Err(SE::from_kind(SEK::FileNotFound)) return Err(SE::from_kind(SEK::FileNotFound))
.chain_err(|| SEK::DeleteCallError(id)) .chain_err(|| SEK::DeleteCallError(id))
} } // else { continue }
},
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 false
entries.remove(&id); },
let pb = id.clone().with_base(self.path().clone()).into_pathbuf()?; };
let _ = self
.backend if do_remove {
.remove_file(&pb) let _ = entries.remove(&id);
.chain_err(|| SEK::FileError) }
.chain_err(|| SEK::DeleteCallError(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"); debug!("Deleted");
Ok(()) 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 /// A struct that allows you to borrow an Entry
pub struct FileLockEntry<'a> { pub struct FileLockEntry<'a> {
store: &'a Store, 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 should be used to get a new Header, as the default header may change. Via
/// this function, compatibility is ensured. /// this function, compatibility is ensured.
pub fn default_header() -> Value { // BTreeMap<String, Value> 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 /// See `Entry::from_str()`, as this function is used internally. This is just a wrapper for
@ -863,7 +864,16 @@ impl Entry {
/// ///
/// Currently, this only verifies the header. This might change in the future. /// Currently, this only verifies the header. This might change in the future.
pub fn verify(&self) -> Result<()> { pub fn verify(&self) -> Result<()> {
self.header.verify() if !has_main_section(&self.header)? {
Err(SE::from_kind(SEK::MissingMainSection))
} else if !has_imag_version_in_main_section(&self.header)? {
Err(SE::from_kind(SEK::MissingVersionInfo))
} 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 {
Ok(())
}
} }
} }
@ -878,54 +888,6 @@ impl PartialEq for Entry {
} }
/// 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)? {
Err(SE::from_kind(SEK::MissingMainSection))
} else if !has_imag_version_in_main_section(self)? {
Err(SE::from_kind(SEK::MissingVersionInfo))
} else if !has_only_tables(self)? {
debug!("Could not verify that it only has tables in its base table");
Err(SE::from_kind(SEK::NonTableInBaseTable))
} else {
Ok(())
}
}
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)
});
Value::Table(m)
}
}
fn has_only_tables(t: &Value) -> Result<bool> { fn has_only_tables(t: &Value) -> Result<bool> {
debug!("Verifying that table has only tables"); debug!("Verifying that table has only tables");
match *t { match *t {
@ -954,7 +916,6 @@ mod test {
use std::collections::BTreeMap; use std::collections::BTreeMap;
use storeid::StoreId; use storeid::StoreId;
use store::Header;
use store::has_main_section; use store::has_main_section;
use store::has_imag_version_in_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()); 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 = "--- static TEST_ENTRY : &'static str = "---
[imag] [imag]
version = '0.0.3' version = '0.0.3'

3
lib/core/libimagstore/src/util.rs
View file

@ -22,7 +22,6 @@ use std::fmt::Write;
use toml::Value; use toml::Value;
use store::Result; use store::Result;
use store::Header;
#[cfg(feature = "early-panic")] #[cfg(feature = "early-panic")]
#[macro_export] #[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)] #[cfg(test)]