Merge pull request #569 from TheNeikos/replace-lazy_file

Replace lazy file

libimagstore/src/error.rs

@@ -14,6 +14,12 @@ generate_custom_error_types!(StoreError, StoreErrorKind, CustomErrorData,
     OutOfMemory             => "Out of Memory",
     FileNotFound            => "File corresponding to ID not found",
     FileNotCreated          => "File corresponding to ID could not be created",
+    FileNotWritten          => "File corresponding to ID could not be written to",
+    FileNotSeeked           => "File corresponding to ID could not be seeked",
+    FileNotRemoved          => "File corresponding to ID could not be removed",
+    FileNotRenamed          => "File corresponding to ID could not be renamed",
+    FileNotCopied           => "File could not be copied",
+    DirNotCreated           => "Directory/Directories could not be created",
     StorePathExists         => "Store path exists",
     StorePathCreate         => "Store path create",
     LockError               => "Error locking datastructure",

libimagstore/src/file_abstraction.rs

@@ -0,0 +1,199 @@
+pub use self::fs::FileAbstraction;
+
+// 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 std::io::Cursor;
+    use std::path::PathBuf;
+
+    use std::collections::HashMap;
+    use std::sync::Mutex;
+
+    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 = MAP.lock().unwrap();
+                    return Ok(map.get(f).unwrap().clone());
+                },
+            };
+        }
+
+        pub fn write_file_content(&mut self, buf: &[u8]) -> Result<(), SE> {
+            match *self {
+                FileAbstraction::Absent(ref f) => {
+                    let mut map = MAP.lock().unwrap();
+                    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(_: &PathBuf) -> Result<(), SE> {
+            Ok(())
+        }
+
+        pub fn copy(from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
+            let mut map = MAP.lock().unwrap();
+            let a = map.get(from).unwrap().clone();
+            map.insert(to.clone(), a);
+            Ok(())
+        }
+
+        pub fn rename(from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
+            let mut map = MAP.lock().unwrap();
+            let a = map.get(from).unwrap().clone();
+            map.insert(to.clone(), a);
+            Ok(())
+        }
+
+        pub fn create_dir_all(_: &PathBuf) -> Result<(), SE> {
+            Ok(())
+        }
+    }
+}
+
+#[cfg(not(test))]
+mod fs {
+    use error::{MapErrInto, StoreError as SE, StoreErrorKind as SEK};
+    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
+    ///
+    /// 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)
+    }
+
+    fn open_file<A: AsRef<Path>>(p: A) -> ::std::io::Result<File> {
+        OpenOptions::new().write(true).read(true).open(p)
+    }
+
+    fn create_file<A: AsRef<Path>>(p: A) -> ::std::io::Result<File> {
+        if let Some(parent) = p.as_ref().parent() {
+            debug!("Implicitely creating directory: {:?}", parent);
+            if let Err(e) = create_dir_all(parent) {
+                return Err(e);
+            }
+        }
+        OpenOptions::new().write(true).read(true).create(true).open(p)
+    }
+
+    impl FileAbstraction {
+
+        /**
+         * Get the content behind this file
+         */
+        pub fn get_file_content(&mut self) -> Result<&mut Read, 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)
+                },
+                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!();
+        }
+
+        pub fn remove_file(path: &PathBuf) -> Result<(), SE> {
+            remove_file(path).map_err_into(SEK::FileNotRemoved)
+        }
+
+        pub fn copy(from: &PathBuf, to: &PathBuf) -> Result<(), SE> {
+            copy(from, to).map_err_into(SEK::FileNotCopied).map(|_| ())
+        }
+
+        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)
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::FileAbstraction;
+    use std::io::Read;
+    use std::path::PathBuf;
+
+    #[test]
+    fn lazy_file() {
+        let mut path = PathBuf::from("/tests");
+        path.set_file_name("test1");
+        let mut lf = FileAbstraction::Absent(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");
+    }
+
+}

libimagstore/src/lazyfile.rs

@@ -1,115 +0,0 @@
-use error::{MapErrInto, StoreError as SE, StoreErrorKind as SEK};
-use std::io::{Seek, SeekFrom};
-use std::path::{Path, PathBuf};
-use std::fs::{File, OpenOptions, create_dir_all};
-
-/// `LazyFile` type
-///
-/// A lazy file is either absent, but a path to it is available, or it is present.
-#[derive(Debug)]
-pub enum LazyFile {
-    Absent(PathBuf),
-    File(File)
-}
-
-fn open_file<A: AsRef<Path>>(p: A) -> ::std::io::Result<File> {
-    OpenOptions::new().write(true).read(true).open(p)
-}
-
-fn create_file<A: AsRef<Path>>(p: A) -> ::std::io::Result<File> {
-    if let Some(parent) = p.as_ref().parent() {
-        debug!("Implicitely creating directory: {:?}", parent);
-        if let Err(e) = create_dir_all(parent) {
-            return Err(e);
-        }
-    }
-    OpenOptions::new().write(true).read(true).create(true).open(p)
-}
-
-impl LazyFile {
-
-    /**
-     * Get the mutable file behind a LazyFile object
-     */
-    pub fn get_file_mut(&mut self) -> Result<&mut File, SE> {
-        debug!("Getting lazy file: {:?}", self);
-        let file = match *self {
-            LazyFile::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
-                f.seek(SeekFrom::Start(0))
-                    .map_err_into(SEK::FileNotCreated)
-                    .map(|_| f)
-            },
-            LazyFile::Absent(ref p) => try!(open_file(p).map_err_into(SEK::FileNotFound)),
-        };
-        *self = LazyFile::File(file);
-        if let LazyFile::File(ref mut f) = *self {
-            return Ok(f);
-        }
-        unreachable!()
-    }
-
-    /**
-     * Create a file out of this LazyFile object
-     */
-    pub fn create_file(&mut self) -> Result<&mut File, SE> {
-        debug!("Creating lazy file: {:?}", self);
-        let file = match *self {
-            LazyFile::File(ref mut f) => return Ok(f),
-            LazyFile::Absent(ref p) => try!(create_file(p).map_err_into(SEK::FileNotFound)),
-        };
-        *self = LazyFile::File(file);
-        if let LazyFile::File(ref mut f) = *self {
-            return Ok(f);
-        }
-        unreachable!()
-    }
-}
-
-#[cfg(test)]
-mod test {
-    use super::LazyFile;
-    use std::io::{Read, Write};
-    use std::path::PathBuf;
-    use tempdir::TempDir;
-
-    fn get_dir() -> TempDir {
-        TempDir::new("test-image").unwrap()
-    }
-
-    #[test]
-    fn lazy_file() {
-        let dir = get_dir();
-        let mut path = PathBuf::from(dir.path());
-        path.set_file_name("test1");
-        let mut lf = LazyFile::Absent(path);
-
-        write!(lf.create_file().unwrap(), "Hello World").unwrap();
-        dir.close().unwrap();
-    }
-
-    #[test]
-    fn lazy_file_with_file() {
-        let dir = get_dir();
-        let mut path = PathBuf::from(dir.path());
-        path.set_file_name("test2");
-        let mut lf = LazyFile::Absent(path.clone());
-
-        {
-            let mut file = lf.create_file().unwrap();
-
-            file.write(b"Hello World").unwrap();
-            file.sync_all().unwrap();
-        }
-
-        {
-            let mut file = lf.get_file_mut().unwrap();
-            let mut s = Vec::new();
-            file.read_to_end(&mut s).unwrap();
-            assert_eq!(s, "Hello World".to_string().into_bytes());
-        }
-
-        dir.close().unwrap();
-    }
-}

libimagstore/src/lib.rs

@@ -32,5 +32,5 @@ pub mod error;
 pub mod hook;
 pub mod store;
 mod configuration;
-mod lazyfile;
+mod file_abstraction;
 

libimagstore/src/store.rs

@@ -1,12 +1,11 @@
 use std::collections::HashMap;
-use std::fs::{File, remove_file};
 use std::ops::Drop;
 use std::path::PathBuf;
 use std::result::Result as RResult;
 use std::sync::Arc;
 use std::sync::RwLock;
 use std::collections::BTreeMap;
-use std::io::{Seek, SeekFrom};
+use std::io::Read;
 use std::convert::From;
 use std::convert::Into;
 use std::sync::Mutex;
@@ -26,7 +25,7 @@ use error::{ParserErrorKind, ParserError};
 use error::{StoreError as SE, StoreErrorKind as SEK};
 use error::MapErrInto;
 use storeid::{IntoStoreId, StoreId, StoreIdIterator};
-use lazyfile::LazyFile;
+use file_abstraction::FileAbstraction;
 
 use hook::aspect::Aspect;
 use hook::error::HookErrorKind;
@@ -56,7 +55,7 @@ enum StoreEntryStatus {
 #[derive(Debug)]
 struct StoreEntry {
     id: StoreId,
-    file: LazyFile,
+    file: FileAbstraction,
     status: StoreEntryStatus,
 }
 
@@ -116,7 +115,7 @@ impl StoreEntry {
     fn new(id: StoreId) -> StoreEntry {
         StoreEntry {
             id: id.clone(),
-            file: LazyFile::Absent(id.into()),
+            file: FileAbstraction::Absent(id.into()),
             status: StoreEntryStatus::Present,
         }
     }
@@ -129,7 +128,7 @@ impl StoreEntry {
 
     fn get_entry(&mut self) -> Result<Entry> {
         if !self.is_borrowed() {
-            let file = self.file.get_file_mut();
+            let file = self.file.get_file_content();
             if let Err(err) = file {
                 if err.err_type() == SEK::FileNotFound {
                     Ok(Entry::new(self.id.clone()))
@@ -138,9 +137,7 @@ impl StoreEntry {
                 }
             } else {
                 // TODO:
-                let mut file = file.unwrap();
+                let entry = Entry::from_reader(self.id.clone(), &mut file.unwrap());
-                let entry = Entry::from_file(self.id.clone(), &mut file);
-                file.seek(SeekFrom::Start(0)).ok();
                 entry
             }
         } else {
@@ -150,12 +147,10 @@ impl StoreEntry {
 
     fn write_entry(&mut self, entry: &Entry) -> Result<()> {
         if self.is_borrowed() {
-            use std::io::Write;
-            let file = try!(self.file.create_file());
-
             assert_eq!(self.id, entry.location);
-            try!(file.set_len(0).map_err_into(SEK::FileError));
+            self.file.write_file_content(entry.to_str().as_bytes())
-            file.write_all(entry.to_str().as_bytes()).map_err_into(SEK::FileError)
+                .map_err_into(SEK::FileError)
+                .map(|_| ())
         } else {
             Ok(())
         }
@@ -202,7 +197,6 @@ impl Store {
 
     /// Create a new Store object
     pub fn new(location: PathBuf, store_config: Option<Value>) -> Result<Store> {
-        use std::fs::create_dir_all;
         use configuration::*;
 
         debug!("Validating Store configuration");
@@ -222,7 +216,7 @@ impl Store {
             }
 
             debug!("Creating store path");
-            let c = create_dir_all(location.clone());
+            let c = FileAbstraction::create_dir_all(&location);
             if c.is_err() {
                 debug!("Failed");
                 return Err(SEK::StorePathCreate.into_error_with_cause(Box::new(c.unwrap_err())));
@@ -615,7 +609,7 @@ impl Store {
 
             // remove the entry first, then the file
             entries.remove(&id);
-            if let Err(e) = remove_file(&id) {
+            if let Err(e) = FileAbstraction::remove_file(&id) {
                 return Err(SEK::FileError.into_error_with_cause(Box::new(e)))
                     .map_err_into(SEK::DeleteCallError);
             }
@@ -643,9 +637,6 @@ impl Store {
     fn save_to_other_location(&self, entry: &FileLockEntry, new_id: StoreId, remove_old: bool)
         -> Result<()>
     {
-        use std::fs::copy;
-        use std::fs::remove_file;
-
         let new_id = new_id.storified(self);
         let hsmap = self.entries.write();
         if hsmap.is_err() {
@@ -657,10 +648,10 @@ impl Store {
 
         let old_id = entry.get_location().clone();
 
-        copy(old_id.clone(), new_id.clone())
+        FileAbstraction::copy(&old_id.clone().into(), &new_id.clone().into())
             .and_then(|_| {
                 if remove_old {
-                    remove_file(old_id)
+                    FileAbstraction::remove_file(&old_id.clone().into())
                 } else {
                     Ok(())
                 }
@@ -674,7 +665,6 @@ impl Store {
 
     /// Move an entry without loading
     pub fn move_by_id(&self, old_id: StoreId, new_id: StoreId) -> Result<()> {
-        use std::fs::rename;
 
         let new_id = new_id.storified(self);
         let old_id = old_id.storified(self);
@@ -691,16 +681,27 @@ impl Store {
             if hsmap.is_err() {
                 return Err(SE::new(SEK::LockPoisoned, None))
             }
-            if hsmap.unwrap().contains_key(&old_id) {
+            let hsmap = hsmap.unwrap();
+            if hsmap.contains_key(&old_id) {
                 return Err(SE::new(SEK::EntryAlreadyBorrowed, None));
             } else {
-                match rename(old_id, new_id.clone()) {
+                match FileAbstraction::rename(&old_id.clone(), &new_id) {
                     Err(e) => return Err(SEK::EntryRenameError.into_error_with_cause(Box::new(e))),
                     _ => {
                         debug!("Rename worked");
                     },
                 }
-            }
+                if hsmap.contains_key(&old_id) {
+                    return Err(SE::new(SEK::EntryAlreadyBorrowed, None));
+                } else {
+                    match FileAbstraction::rename(&old_id, &new_id) {
+                        Err(e) => return Err(SEK::EntryRenameError.into_error_with_cause(Box::new(e))),
+                        _ => {
+                            debug!("Rename worked");
+                        },
+                    }
+                }
+        }
 
         }
 
@@ -1424,9 +1425,8 @@ impl Entry {
         }
     }
 
-    pub fn from_file<S: IntoStoreId>(loc: S, file: &mut File) -> Result<Entry> {
+    pub fn from_reader<S: IntoStoreId>(loc: S, file: &mut Read) -> Result<Entry> {
         let text = {
-            use std::io::Read;
             let mut s = String::new();
             try!(file.read_to_string(&mut s));
             s