Merge pull request #706 from matthiasbeyer/libimagstore/remove-storeid-into-pathbuf

Libimagstore/remove storeid into pathbuf

libimagentrylist/src/listers/path.rs

@@ -3,6 +3,7 @@ use std::io::Write;
 
 use lister::Lister;
 use result::Result;
+use error::MapErrInto;
 
 use libimagstore::store::FileLockEntry;
 use libimagutil::iter::FoldResult;
@@ -26,12 +27,11 @@ impl Lister for PathLister {
     fn list<'a, I: Iterator<Item = FileLockEntry<'a>>>(&self, entries: I) -> Result<()> {
         use error::ListError as LE;
         use error::ListErrorKind as LEK;
-        use std::path::PathBuf;
 
         entries.fold_defresult(|entry| {
             Ok(entry.get_location().clone())
+                .and_then(|pb| pb.into_pathbuf().map_err_into(LEK::FormatError))
                 .and_then(|pb| {
-                    let pb : PathBuf = pb.into();
                     if self.absolute {
                         pb.canonicalize().map_err(|e| LE::new(LEK::FormatError, Some(Box::new(e))))
                     } else {

libimagerror/src/error_gen.rs

@@ -74,10 +74,12 @@ macro_rules! generate_custom_error_types {
                 }
             }
 
+            #[allow(dead_code)]
             pub fn err_type(&self) -> $kindname {
                 self.err_type
             }
 
+            #[allow(dead_code)]
             pub fn with_custom_data(mut self, custom: $customMemberTypeName) -> $name {
                 self.custom_data = Some(custom);
                 self

libimagref/src/reference.rs

@@ -15,6 +15,7 @@ use libimagstore::storeid::StoreId;
 use libimagstore::storeid::IntoStoreId;
 use libimagstore::store::Store;
 use libimagerror::into::IntoError;
+use libimagerror::trace::MapErrTrace;
 
 use toml::Value;
 
@@ -230,11 +231,18 @@ impl<'a> Ref<'a> {
 
     /// Get the hash from the path of the ref
     pub fn get_path_hash(&self) -> Option<String> {
-        let pb : PathBuf = self.0.get_location().clone().into();
+        self.0
+            .get_location()
+            .clone()
+            .into_pathbuf()
+            .map_err_trace()
+            .ok() // TODO: Hiding the error here is not so nice
+            .and_then(|pb| {
                 pb.file_name()
                     .and_then(|osstr| osstr.to_str())
                     .and_then(|s| s.split("~").next())
                     .map(String::from)
+            })
     }
 
     /// Get the hash of the link target which is stored in the ref object

libimagstore/src/error.rs

@@ -44,6 +44,7 @@ generate_custom_error_types!(StoreError, StoreErrorKind, CustomErrorData,
     StoreIdHandlingError    => "StoreId handling error",
     StoreIdLocalPartAbsoluteError => "StoreId 'id' part is absolute (starts with '/') which is not allowed",
     StoreIdBuildFromFullPathError => "Building StoreId from full file path failed",
+    StoreIdHasNoBaseError   => "StoreId has no 'base' part",
 
     CreateCallError            => "Error when calling create()",
     RetrieveCallError          => "Error when calling retrieve()",

libimagstore/src/hook/error.rs

@@ -8,6 +8,8 @@ generate_custom_error_types!(HookError, HookErrorKind, CustomData,
     MutableHooksNotAllowed => "Mutable Hooks are denied"
 );
 
+generate_result_helper!(HookError, HookErrorKind);
+
 #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Copy)]
 pub struct CustomData {
     aborting: bool,

libimagstore/src/store.rs

@@ -124,12 +124,13 @@ impl Iterator for Walk {
 
 impl StoreEntry {
 
-    fn new(id: StoreId) -> StoreEntry {
+    fn new(id: StoreId) -> Result<StoreEntry> {
-        StoreEntry {
+        let pb = try!(id.clone().into_pathbuf());
-            id: id.clone(),
+        Ok(StoreEntry {
-            file: FileAbstraction::Absent(id.into()),
+            id: id,
+            file: FileAbstraction::Absent(pb),
             status: StoreEntryStatus::Present,
-        }
+        })
     }
 
     /// The entry is currently borrowed, meaning that some thread is currently
@@ -406,7 +407,7 @@ impl Store {
                 return Err(SEK::EntryAlreadyExists.into_error()).map_err_into(SEK::CreateCallError);
             }
             hsmap.insert(id.clone(), {
-                let mut se = StoreEntry::new(id.clone());
+                let mut se = try!(StoreEntry::new(id.clone()));
                 se.status = StoreEntryStatus::Borrowed;
                 se
             });
@@ -439,7 +440,8 @@ impl Store {
                 .write()
                 .map_err(|_| SE::new(SEK::LockPoisoned, None))
                 .and_then(|mut es| {
-                    let mut se = es.entry(id.clone()).or_insert_with(|| StoreEntry::new(id.clone()));
+                    let new_se = try!(StoreEntry::new(id.clone()));
+                    let mut se = es.entry(id.clone()).or_insert(new_se);
                     let entry = se.get_entry();
                     se.status = StoreEntryStatus::Borrowed;
                     entry
@@ -550,7 +552,7 @@ impl Store {
             return Err(SE::new(SEK::IdLocked, None)).map_err_into(SEK::RetrieveCopyCallError);
         }
 
-        StoreEntry::new(id).get_entry()
+        try!(StoreEntry::new(id)).get_entry()
     }
 
     /// Delete an entry
@@ -578,7 +580,8 @@ impl Store {
 
             // remove the entry first, then the file
             entries.remove(&id);
-            if let Err(e) = FileAbstraction::remove_file(&id.clone().into()) {
+            let pb = try!(id.clone().with_base(self.path().clone()).into_pathbuf());
+            if let Err(e) = FileAbstraction::remove_file(&pb) {
                 return Err(SEK::FileError.into_error_with_cause(Box::new(e)))
                     .map_err_into(SEK::DeleteCallError);
             }
@@ -617,10 +620,12 @@ impl Store {
 
         let old_id = entry.get_location().clone();
 
-        FileAbstraction::copy(&old_id.clone().into(), &new_id.clone().into())
+        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)
             .and_then(|_| {
                 if remove_old {
-                    FileAbstraction::remove_file(&old_id.clone().into())
+                    FileAbstraction::remove_file(&old_id_as_path)
                 } else {
                     Ok(())
                 }
@@ -654,8 +659,8 @@ impl Store {
             if hsmap.contains_key(&old_id) {
                 return Err(SE::new(SEK::EntryAlreadyBorrowed, None));
             } else {
-                let old_id_pb = old_id.clone().into();
+                let old_id_pb = try!(old_id.clone().with_base(self.path().clone()).into_pathbuf());
-                let new_id_pb = new_id.clone().into();
+                let new_id_pb = try!(new_id.clone().with_base(self.path().clone()).into_pathbuf());
                 match FileAbstraction::rename(&old_id_pb, &new_id_pb) {
                     Err(e) => return Err(SEK::EntryRenameError.into_error_with_cause(Box::new(e))),
                     Ok(_) => {

libimagstore/src/storeid.rs

@@ -62,9 +62,17 @@ impl StoreId {
         self
     }
 
+    /// Transform the StoreId object into a PathBuf, error if the base of the StoreId is not
+    /// specified.
+    pub fn into_pathbuf(self) -> Result<PathBuf> {
+        let mut base = try!(self.base.ok_or(SEK::StoreIdHasNoBaseError.into_error()));
+        base.push(self.id);
+        Ok(base)
+    }
+
     pub fn exists(&self) -> bool {
-        let pb : PathBuf = self.clone().into();
+        // TODO: hiding error here.
-        pb.exists()
+        self.clone().into_pathbuf().map(|pb| pb.exists()).unwrap_or(false)
     }
 
     pub fn is_file(&self) -> bool {
@@ -103,16 +111,6 @@ impl StoreId {
 
 }
 
-impl Into<PathBuf> for StoreId {
-
-    fn into(self) -> PathBuf {
-        let mut base = self.base.unwrap_or(PathBuf::from("/"));
-        base.push(self.id);
-        base
-    }
-
-}
-
 impl Display for StoreId {
 
     fn fmt(&self, fmt: &mut Formatter) -> RResult<(), FmtError> {

libimagstorestdhook/src/flock.rs

@@ -1,9 +1,7 @@
-use std::io::Result as IoResult;
-use std::path::PathBuf;
-
 use toml::Value;
 
 use fs2::FileExt;
+use std::fs::File;
 
 use libimagstore::hook::Hook;
 use libimagstore::hook::accessor::HookDataAccessor as HDA;
@@ -17,25 +15,54 @@ use libimagstore::storeid::StoreId;
 use libimagstore::store::FileLockEntry;
 use libimagstore::store::Entry;
 
+mod error {
+    generate_error_imports!();
+    generate_error_types!(FlockError, FlockErrorKind,
+        IOError                    => "IO Error",
+        StoreIdPathBufConvertError => "Error while converting StoreId to PathBuf",
+        FileOpenError              => "Error on File::open()",
+        LockError                  => "Error while lock()ing",
+        UnlockError                => "Error while unlock()ing"
+    );
+}
+use self::error::FlockError as FE;
+use self::error::FlockErrorKind as FEK;
+use self::error::MapErrInto;
+
 trait EntryFlock {
-    fn lock(&self) -> IoResult<()>;
+    fn lock(&self) -> Result<(), FE>;
-    fn unlock(&self) -> IoResult<()>;
+    fn unlock(&self) -> Result<(), FE>;
+}
+
+fn open_file(id: StoreId) -> Result<File, FE> {
+    id.into_pathbuf()
+        .map_err_into(FEK::StoreIdPathBufConvertError)
+        .and_then(|loc| {
+            File::open(loc)
+                .map_err_into(FEK::FileOpenError)
+                .map_err_into(FEK::IOError)
+        })
 }
 
 impl EntryFlock for Entry {
 
-    fn lock(&self) -> IoResult<()> {
+    fn lock(&self) -> Result<(), FE> {
-        use std::fs::File;
+        open_file(self.get_location().clone())
-
+            .and_then(|file| {
-        let location : PathBuf = self.get_location().clone().into();
+                file.lock_exclusive()
-        File::open(location).and_then(|file| file.lock_exclusive())
+                    .map_err_into(FEK::LockError)
+                    .map_err_into(FEK::IOError)
+            })
     }
 
-    fn unlock(&self) -> IoResult<()> {
+    fn unlock(&self) -> Result<(), FE> {
-        use std::fs::File;
+        open_file(self.get_location().clone())
-
+            .and_then(|file| {
-        let location : PathBuf = self.get_location().clone().into();
+                file.unlock()
-        File::open(location).and_then(|file| file.unlock())
+                    .map_err_into(FEK::UnlockError)
+                    .map_err_into(FEK::LockError)
+                    .map_err_into(FEK::IOError)
+            })
     }
 
 }

libimagstorestdhook/src/lib.rs

@@ -20,7 +20,7 @@ extern crate fs2;
 
 extern crate libimagstore;
 extern crate libimagentrylink;
-extern crate libimagerror;
+#[macro_use] extern crate libimagerror;
 
 pub mod debug;
 pub mod flock;