Merge pull request #962 from matthiasbeyer/libimagstore/fs-memory-backend-as-dependency-injection

Libimagstore/fs memory backend as dependency injection
This commit is contained in:
Matthias Beyer 2017-06-10 21:01:57 +02:00 committed by GitHub
commit a9d2d7c354
2 changed files with 363 additions and 164 deletions

View file

@ -17,112 +17,200 @@
// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
//
pub use self::fs::FileAbstraction;
//! The filesystem abstraction code
//!
//! # Problem
//!
//! First, we had a compiletime backend for the store. This means that the actual filesystem
//! operations were compiled into the store either as real filesystem operations (in a normal debug
//! or release build) but as a in-memory variant in the 'test' case.
//! So tests did not hit the filesystem when running.
//! This gave us us the possibility to run tests concurrently with multiple
//! stores that did not interfere with eachother.
//!
//! This approach worked perfectly well until we started to test not the
//! store itself but crates that depend on the store implementation.
//! When running tests in a crate that depends on the store, the store
//! itself was compiled with the filesystem-hitting-backend.
//! This was problematic, as tests could not be implemented without hitting
//! the filesystem.
//!
//! Hence we implemented this.
//!
//! # Implementation
//!
//! The filesystem is abstracted via a trait `FileAbstraction` which
//! contains the essential functions for working with the filesystem.
//!
//! Two implementations are provided in the code:
//!
//! * FSFileAbstraction
//! * InMemoryFileAbstraction
//!
//! whereas the first actually works with the filesystem and the latter
//! works with an in-memory HashMap that is used as filesystem.
//!
//! Further, the trait `FileAbstractionInstance` was introduced for
//! functions which are executed on actual instances of content from the
//! filesystem, which was previousely tied into the general abstraction
//! mechanism.
//!
//! So, the `FileAbstraction` trait is for working with the filesystem, the
//! `FileAbstractionInstance` trait is for working with instances of content
//! from the filesystem (speak: actual Files).
//!
//! In case of the `FSFileAbstractionInstance`, which is the implementation
//! of the `FileAbstractionInstance` for the actual filesystem-hitting code,
//! the underlying resource is managed like with the old code before.
//! The `InMemoryFileAbstractionInstance` implementation is corrosponding to
//! the `InMemoryFileAbstraction` implementation - for the in-memory
//! "filesystem".
//!
//! The implementation of the `get_file_content()` function had to be
//! changed to return a `String` rather than a `&mut Read` because of
//! lifetime issues.
//! This change is store-internally and the API of the store itself was not
//! affected.
//!
use std::path::PathBuf;
use std::fmt::Debug;
use error::StoreError as SE;
pub use self::fs::FSFileAbstraction;
pub use self::fs::FSFileAbstractionInstance;
pub use self::inmemory::InMemoryFileAbstraction;
pub use self::inmemory::InMemoryFileAbstractionInstance;
// TODO:
// This whole thing can be written better with a trait based mechanism that is embedded into the
// store. However it would mean rewriting most things to be generic which can be a pain in the ass.
#[cfg(test)]
mod fs {
use error::StoreError as SE;
use error::StoreErrorKind as SEK;
use std::io::Cursor;
use std::path::PathBuf;
use std::collections::HashMap;
use std::sync::Mutex;
/// An abstraction trait over filesystem actions
pub trait FileAbstraction : Debug {
fn remove_file(&self, path: &PathBuf) -> Result<(), SE>;
fn copy(&self, from: &PathBuf, to: &PathBuf) -> Result<(), SE>;
fn rename(&self, from: &PathBuf, to: &PathBuf) -> Result<(), SE>;
fn create_dir_all(&self, _: &PathBuf) -> Result<(), SE>;
use libimagerror::into::IntoError;
use error::MapErrInto;
lazy_static! {
static ref MAP: Mutex<HashMap<PathBuf, Cursor<Vec<u8>>>> = {
Mutex::new(HashMap::new())
};
}
/// `FileAbstraction` type, this is the Test version!
///
/// A lazy file is either absent, but a path to it is available, or it is present.
#[derive(Debug)]
pub enum FileAbstraction {
Absent(PathBuf),
}
impl FileAbstraction {
/**
* Get the mutable file behind a FileAbstraction object
*/
pub fn get_file_content(&mut self) -> Result<Cursor<Vec<u8>>, SE> {
debug!("Getting lazy file: {:?}", self);
match *self {
FileAbstraction::Absent(ref f) => {
let map = try!(MAP.lock().map_err_into(SEK::LockPoisoned));
return map.get(f).cloned().ok_or(SEK::FileNotFound.into_error());
},
};
}
pub fn write_file_content(&mut self, buf: &[u8]) -> Result<(), SE> {
match *self {
FileAbstraction::Absent(ref f) => {
let mut map = try!(MAP.lock().map_err_into(SEK::LockPoisoned));
if let Some(ref mut cur) = map.get_mut(f) {
let mut vec = cur.get_mut();
vec.clear();
vec.extend_from_slice(buf);
return Ok(());
}
let vec = Vec::from(buf);
map.insert(f.clone(), Cursor::new(vec));
return Ok(());
},
};
}
pub fn remove_file(path: &PathBuf) -> Result<(), SE> {
try!(MAP.lock().map_err_into(SEK::LockPoisoned))
.remove(path)
.map(|_| ())
.ok_or(SEK::FileNotFound.into_error())
}
pub fn copy(from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
let mut map = try!(MAP.lock().map_err_into(SEK::LockPoisoned));
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 = try!(MAP.lock().map_err_into(SEK::LockPoisoned));
let a = try!(map.get(from).cloned().ok_or(SEK::FileNotFound.into_error()));
map.insert(to.clone(), a);
Ok(())
}
pub fn create_dir_all(_: &PathBuf) -> Result<(), SE> {
Ok(())
}
}
fn new_instance(&self, p: PathBuf) -> Box<FileAbstractionInstance>;
}
/// An abstraction trait over actions on files
pub trait FileAbstractionInstance : Debug {
fn get_file_content(&mut self) -> Result<String, SE>;
fn write_file_content(&mut self, buf: &[u8]) -> Result<(), SE>;
}
#[cfg(not(test))]
mod fs {
use error::{MapErrInto, StoreError as SE, StoreErrorKind as SEK};
use std::fs::{File, OpenOptions, create_dir_all, remove_file, copy, rename};
use std::io::{Seek, SeekFrom, Read};
use std::path::{Path, PathBuf};
use std::fs::{File, OpenOptions, create_dir_all, remove_file, copy, rename};
/// `FileAbstraction` type
use error::{MapErrInto, StoreError as SE, StoreErrorKind as SEK};
use super::FileAbstraction;
use super::FileAbstractionInstance;
#[derive(Debug)]
pub enum FSFileAbstractionInstance {
Absent(PathBuf),
File(File, PathBuf)
}
impl FileAbstractionInstance for FSFileAbstractionInstance {
/**
* Get the content behind this file
*/
fn get_file_content(&mut self) -> Result<String, SE> {
debug!("Getting lazy file: {:?}", self);
let (file, path) = match *self {
FSFileAbstractionInstance::File(ref mut f, _) => return {
// We seek to the beginning of the file since we expect each
// access to the file to be in a different context
try!(f.seek(SeekFrom::Start(0))
.map_err_into(SEK::FileNotSeeked));
let mut s = String::new();
f.read_to_string(&mut s)
.map_err_into(SEK::IoError)
.map(|_| s)
},
FSFileAbstractionInstance::Absent(ref p) =>
(try!(open_file(p).map_err_into(SEK::FileNotFound)), p.clone()),
};
*self = FSFileAbstractionInstance::File(file, path);
if let FSFileAbstractionInstance::File(ref mut f, _) = *self {
let mut s = String::new();
f.read_to_string(&mut s)
.map_err_into(SEK::IoError)
.map(|_| s)
} else {
unreachable!()
}
}
/**
* Write the content of this file
*/
fn write_file_content(&mut self, buf: &[u8]) -> Result<(), SE> {
use std::io::Write;
let (file, path) = match *self {
FSFileAbstractionInstance::File(ref mut f, _) => return {
// We seek to the beginning of the file since we expect each
// access to the file to be in a different context
try!(f.seek(SeekFrom::Start(0))
.map_err_into(SEK::FileNotCreated));
f.write_all(buf).map_err_into(SEK::FileNotWritten)
},
FSFileAbstractionInstance::Absent(ref p) =>
(try!(create_file(p).map_err_into(SEK::FileNotCreated)), p.clone()),
};
*self = FSFileAbstractionInstance::File(file, path);
if let FSFileAbstractionInstance::File(ref mut f, _) = *self {
return f.write_all(buf).map_err_into(SEK::FileNotWritten);
}
unreachable!();
}
}
/// `FSFileAbstraction` state type
///
/// A lazy file is either absent, but a path to it is available, or it is present.
#[derive(Debug)]
pub enum FileAbstraction {
Absent(PathBuf),
File(File, PathBuf)
pub struct FSFileAbstraction {
}
impl FSFileAbstraction {
pub fn new() -> FSFileAbstraction {
FSFileAbstraction { }
}
}
impl FileAbstraction for FSFileAbstraction {
fn remove_file(&self, path: &PathBuf) -> Result<(), SE> {
remove_file(path).map_err_into(SEK::FileNotRemoved)
}
fn copy(&self, from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
copy(from, to).map_err_into(SEK::FileNotCopied).map(|_| ())
}
fn rename(&self, from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
rename(from, to).map_err_into(SEK::FileNotRenamed)
}
fn create_dir_all(&self, path: &PathBuf) -> Result<(), SE> {
create_dir_all(path).map_err_into(SEK::DirNotCreated)
}
fn new_instance(&self, p: PathBuf) -> Box<FileAbstractionInstance> {
Box::new(FSFileAbstractionInstance::Absent(p))
}
}
fn open_file<A: AsRef<Path>>(p: A) -> ::std::io::Result<File> {
@ -139,87 +227,175 @@ mod fs {
OpenOptions::new().write(true).read(true).create(true).open(p)
}
impl FileAbstraction {
}
mod inmemory {
use error::StoreError as SE;
use error::StoreErrorKind as SEK;
use std::io::Read;
use std::io::Cursor;
use std::path::PathBuf;
use std::collections::HashMap;
use std::sync::Mutex;
use std::cell::RefCell;
use std::sync::Arc;
use libimagerror::into::IntoError;
use super::FileAbstraction;
use super::FileAbstractionInstance;
use error::MapErrInto;
type Backend = Arc<Mutex<RefCell<HashMap<PathBuf, Cursor<Vec<u8>>>>>>;
/// `FileAbstraction` type, this is the Test version!
///
/// A lazy file is either absent, but a path to it is available, or it is present.
#[derive(Debug)]
pub struct InMemoryFileAbstractionInstance {
fs_abstraction: Backend,
absent_path: PathBuf,
}
impl InMemoryFileAbstractionInstance {
pub fn new(fs: Backend, pb: PathBuf) -> InMemoryFileAbstractionInstance {
InMemoryFileAbstractionInstance {
fs_abstraction: fs,
absent_path: pb
}
}
}
impl FileAbstractionInstance for InMemoryFileAbstractionInstance {
/**
* Get the content behind this file
* Get the mutable file behind a InMemoryFileAbstraction object
*/
pub fn get_file_content(&mut self) -> Result<&mut Read, SE> {
fn get_file_content(&mut self) -> Result<String, SE> {
debug!("Getting lazy file: {:?}", self);
let (file, path) = match *self {
FileAbstraction::File(ref mut f, _) => return {
// We seek to the beginning of the file since we expect each
// access to the file to be in a different context
try!(f.seek(SeekFrom::Start(0))
.map_err_into(SEK::FileNotSeeked));
Ok(f)
let p = self.absent_path.clone();
match self.fs_abstraction.lock() {
Ok(mut mtx) => {
mtx.get_mut()
.get_mut(&p)
.ok_or(SEK::FileNotFound.into_error())
.and_then(|t| {
let mut s = String::new();
t.read_to_string(&mut s)
.map_err_into(SEK::IoError)
.map(|_| s)
})
}
Err(_) => Err(SEK::LockError.into_error())
}
}
fn write_file_content(&mut self, buf: &[u8]) -> Result<(), SE> {
match *self {
InMemoryFileAbstractionInstance { ref absent_path, .. } => {
let mut mtx = self.fs_abstraction.lock().expect("Locking Mutex failed");
let mut backend = mtx.get_mut();
if let Some(ref mut cur) = backend.get_mut(absent_path) {
let mut vec = cur.get_mut();
vec.clear();
vec.extend_from_slice(buf);
return Ok(());
}
let vec = Vec::from(buf);
backend.insert(absent_path.clone(), Cursor::new(vec));
return Ok(());
},
FileAbstraction::Absent(ref p) => (try!(open_file(p).map_err_into(SEK::FileNotFound)),
p.clone()),
};
*self = FileAbstraction::File(file, path);
if let FileAbstraction::File(ref mut f, _) = *self {
return Ok(f);
}
unreachable!()
}
/**
* Write the content of this file
*/
pub fn write_file_content(&mut self, buf: &[u8]) -> Result<(), SE> {
use std::io::Write;
let (file, path) = match *self {
FileAbstraction::File(ref mut f, _) => return {
// We seek to the beginning of the file since we expect each
// access to the file to be in a different context
try!(f.seek(SeekFrom::Start(0))
.map_err_into(SEK::FileNotCreated));
f.write_all(buf).map_err_into(SEK::FileNotWritten)
},
FileAbstraction::Absent(ref p) => (try!(create_file(p).map_err_into(SEK::FileNotCreated)),
p.clone()),
};
*self = FileAbstraction::File(file, path);
if let FileAbstraction::File(ref mut f, _) = *self {
return f.write_all(buf).map_err_into(SEK::FileNotWritten);
}
unreachable!();
#[derive(Debug)]
pub struct InMemoryFileAbstraction {
virtual_filesystem: Backend,
}
pub fn remove_file(path: &PathBuf) -> Result<(), SE> {
remove_file(path).map_err_into(SEK::FileNotRemoved)
impl InMemoryFileAbstraction {
pub fn new() -> InMemoryFileAbstraction {
InMemoryFileAbstraction {
virtual_filesystem: Arc::new(Mutex::new(RefCell::new(HashMap::new()))),
}
}
pub fn copy(from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
copy(from, to).map_err_into(SEK::FileNotCopied).map(|_| ())
pub fn backend(&self) -> &Backend {
&self.virtual_filesystem
}
pub fn rename(from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
rename(from, to).map_err_into(SEK::FileNotRenamed)
}
pub fn create_dir_all(path: &PathBuf) -> Result<(), SE> {
create_dir_all(path).map_err_into(SEK::DirNotCreated)
impl FileAbstraction for InMemoryFileAbstraction {
fn remove_file(&self, path: &PathBuf) -> Result<(), SE> {
debug!("Removing: {:?}", path);
self.backend()
.lock()
.expect("Locking Mutex failed")
.get_mut()
.remove(path)
.map(|_| ())
.ok_or(SEK::FileNotFound.into_error())
}
fn copy(&self, from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
debug!("Copying : {:?} -> {:?}", from, to);
let mut mtx = self.backend().lock().expect("Locking Mutex failed");
let mut backend = mtx.get_mut();
let a = try!(backend.get(from).cloned().ok_or(SEK::FileNotFound.into_error()));
backend.insert(to.clone(), a);
debug!("Copying: {:?} -> {:?} worked", from, to);
Ok(())
}
fn rename(&self, from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
debug!("Renaming: {:?} -> {:?}", from, to);
let mut mtx = self.backend().lock().expect("Locking Mutex failed");
let mut backend = mtx.get_mut();
let a = try!(backend.get(from).cloned().ok_or(SEK::FileNotFound.into_error()));
backend.insert(to.clone(), a);
debug!("Renaming: {:?} -> {:?} worked", from, to);
Ok(())
}
fn create_dir_all(&self, _: &PathBuf) -> Result<(), SE> {
Ok(())
}
fn new_instance(&self, p: PathBuf) -> Box<FileAbstractionInstance> {
Box::new(InMemoryFileAbstractionInstance::new(self.backend().clone(), p))
}
}
}
#[cfg(test)]
mod test {
use super::FileAbstraction;
use std::io::Read;
use super::FileAbstractionInstance;
use super::inmemory::InMemoryFileAbstraction;
use super::inmemory::InMemoryFileAbstractionInstance;
use std::path::PathBuf;
#[test]
fn lazy_file() {
let fs = InMemoryFileAbstraction::new();
let mut path = PathBuf::from("/tests");
path.set_file_name("test1");
let mut lf = FileAbstraction::Absent(path);
let mut lf = InMemoryFileAbstractionInstance::new(fs.backend().clone(), path);
lf.write_file_content(b"Hello World").unwrap();
let mut bah = Vec::new();
lf.get_file_content().unwrap().read_to_end(&mut bah).unwrap();
assert_eq!(bah, b"Hello World");
let bah = lf.get_file_content().unwrap();
assert_eq!(bah, "Hello World");
}
}

View file

@ -41,8 +41,12 @@ use walkdir::Iter as WalkDirIter;
use error::{StoreError as SE, StoreErrorKind as SEK};
use error::MapErrInto;
use storeid::{IntoStoreId, StoreId, StoreIdIterator};
use file_abstraction::FileAbstraction;
use file_abstraction::FileAbstractionInstance;
use toml_ext::*;
// We re-export the following things so tests can use them
pub use file_abstraction::FileAbstraction;
pub use file_abstraction::FSFileAbstraction;
pub use file_abstraction::InMemoryFileAbstraction;
use libimagerror::into::IntoError;
use libimagerror::trace::trace_error;
@ -65,7 +69,7 @@ enum StoreEntryStatus {
#[derive(Debug)]
struct StoreEntry {
id: StoreId,
file: FileAbstraction,
file: Box<FileAbstractionInstance>,
status: StoreEntryStatus,
}
@ -132,7 +136,7 @@ impl Iterator for Walk {
impl StoreEntry {
fn new(id: StoreId) -> Result<StoreEntry> {
fn new(id: StoreId, backend: &Box<FileAbstraction>) -> Result<StoreEntry> {
let pb = try!(id.clone().into_pathbuf());
#[cfg(feature = "fs-lock")]
@ -144,7 +148,7 @@ impl StoreEntry {
Ok(StoreEntry {
id: id,
file: FileAbstraction::Absent(pb),
file: backend.new_instance(pb),
status: StoreEntryStatus::Present,
})
}
@ -160,7 +164,7 @@ impl StoreEntry {
if !self.is_borrowed() {
self.file
.get_file_content()
.and_then(|mut file| Entry::from_reader(id.clone(), &mut file))
.and_then(|content| Entry::from_str(id.clone(), &content))
.or_else(|err| if err.err_type() == SEK::FileNotFound {
Ok(Entry::new(id.clone()))
} else {
@ -213,6 +217,11 @@ pub struct Store {
/// Could be optimized for a threadsafe HashMap
///
entries: Arc<RwLock<HashMap<StoreId, StoreEntry>>>,
/// The backend to use
///
/// This provides the filesystem-operation functions (or pretends to)
backend: Box<FileAbstraction>,
}
impl Store {
@ -240,6 +249,17 @@ impl Store {
/// - StorePathCreate(_) if creating the store directory failed
/// - StorePathExists() if location exists but is a file
pub fn new(location: PathBuf, store_config: Option<Value>) -> Result<Store> {
let backend = Box::new(FSFileAbstraction::new());
Store::new_with_backend(location, store_config, backend)
}
/// Create a Store object as descripbed in `Store::new()` documentation, but with an alternative
/// backend implementation.
///
/// Do not use directly, only for testing purposes.
pub fn new_with_backend(location: PathBuf,
store_config: Option<Value>,
backend: Box<FileAbstraction>) -> Result<Store> {
use configuration::*;
debug!("Validating Store configuration");
@ -256,7 +276,7 @@ impl Store {
.map_err_into(SEK::IoError);
}
try!(FileAbstraction::create_dir_all(&location)
try!(backend.create_dir_all(&location)
.map_err_into(SEK::StorePathCreate)
.map_dbg_err_str("Failed"));
} else if location.is_file() {
@ -268,6 +288,7 @@ impl Store {
location: location.clone(),
configuration: store_config,
entries: Arc::new(RwLock::new(HashMap::new())),
backend: backend,
};
debug!("Store building succeeded");
@ -367,7 +388,7 @@ impl Store {
}
hsmap.insert(id.clone(), {
debug!("Creating: '{}'", id);
let mut se = try!(StoreEntry::new(id.clone()));
let mut se = try!(StoreEntry::new(id.clone(), &self.backend));
se.status = StoreEntryStatus::Borrowed;
se
});
@ -400,7 +421,7 @@ impl Store {
.write()
.map_err(|_| SE::new(SEK::LockPoisoned, None))
.and_then(|mut es| {
let new_se = try!(StoreEntry::new(id.clone()));
let new_se = try!(StoreEntry::new(id.clone(), &self.backend));
let mut se = es.entry(id.clone()).or_insert(new_se);
let entry = se.get_entry();
se.status = StoreEntryStatus::Borrowed;
@ -557,7 +578,7 @@ impl Store {
return Err(SE::new(SEK::IdLocked, None)).map_err_into(SEK::RetrieveCopyCallError);
}
try!(StoreEntry::new(id)).get_entry()
try!(StoreEntry::new(id, &self.backend)).get_entry()
}
/// Delete an entry
@ -596,7 +617,7 @@ impl Store {
// remove the entry first, then the file
entries.remove(&id);
let pb = try!(id.clone().with_base(self.path().clone()).into_pathbuf());
if let Err(e) = FileAbstraction::remove_file(&pb) {
if let Err(e) = self.backend.remove_file(&pb) {
return Err(SEK::FileError.into_error_with_cause(Box::new(e)))
.map_err_into(SEK::DeleteCallError);
}
@ -638,11 +659,11 @@ impl Store {
let old_id_as_path = try!(old_id.clone().with_base(self.path().clone()).into_pathbuf());
let new_id_as_path = try!(new_id.clone().with_base(self.path().clone()).into_pathbuf());
FileAbstraction::copy(&old_id_as_path, &new_id_as_path)
self.backend.copy(&old_id_as_path, &new_id_as_path)
.and_then(|_| {
if remove_old {
debug!("Removing old '{:?}'", old_id_as_path);
FileAbstraction::remove_file(&old_id_as_path)
self.backend.remove_file(&old_id_as_path)
} else {
Ok(())
}
@ -710,7 +731,7 @@ impl Store {
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) {
match self.backend.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");
@ -1212,9 +1233,11 @@ mod store_tests {
use std::path::PathBuf;
use super::Store;
use file_abstraction::InMemoryFileAbstraction;
pub fn get_store() -> Store {
Store::new(PathBuf::from("/"), None).unwrap()
let backend = Box::new(InMemoryFileAbstraction::new());
Store::new_with_backend(PathBuf::from("/"), None, backend).unwrap()
}
#[test]