Add ArtifactPath, StoreRoot

This is the first step towards strong path typing for distinction
between artifact pathes.
It adds a type for Store root pathes and a type for artifact pathes.

Signed-off-by: Matthias Beyer <mail@beyermatthias.de>

5 files changed, 146 insertions, 46 deletions

diff --git a/src/filestore/artifact.rs b/src/filestore/artifact.rs
index e411b06..d97aceb 100644
--- a/src/filestore/artifact.rs
+++ b/src/filestore/artifact.rs
@@ -1,8 +1,4 @@
-use std::cmp::Ord;
 use std::cmp::Ordering;
-use std::cmp::PartialOrd;
-use std::path::Path;
-use std::path::PathBuf;
 
 use anyhow::Context;
 use anyhow::Error;
@@ -13,11 +9,12 @@ use pom::parser::Parser as PomParser;
 
 use crate::package::PackageName;
 use crate::package::PackageVersion;
+use crate::filestore::path::*;
 
 #[derive(Clone, PartialEq, Eq, Debug, Getters)]
 pub struct Artifact {
     #[getset(get = "pub")]
-    path: PathBuf,
+    path: ArtifactPath,
 
     #[getset(get = "pub")]
     name: PackageName,
@@ -40,15 +37,14 @@ impl Ord for Artifact {
 
 
 impl Artifact {
-    pub fn load(root: &Path, path: &Path) -> Result<Self> {
-        let joined = root.join(path);
-        if joined.is_file() {
-            let (name, version) = Self::parse_path(root, path)
-                .with_context(|| anyhow!("Pathing artifact path: '{}'", joined.display()))?;
+    pub fn load(root: &StoreRoot, path: ArtifactPath) -> Result<Self> {
+        let joined_fullpath = root.join(&path);
+        if joined_fullpath.is_file() {
+            let (name, version) = Self::parse_path(root, &path)
+                .with_context(|| anyhow!("Pathing artifact path: '{}'", joined_fullpath.display()))?;
 
             Ok(Artifact {
-                path: path.to_path_buf(),
-
+                path,
                 name,
                 version
             })
@@ -61,7 +57,7 @@ impl Artifact {
         }
     }
 
-    fn parse_path(root: &Path, path: &Path) -> Result<(PackageName, PackageVersion)> {
+    fn parse_path(root: &StoreRoot, path: &ArtifactPath) -> Result<(PackageName, PackageVersion)> {
         path.file_stem()
             .ok_or_else(|| anyhow!("Cannot get filename from {}", (root.join(path)).display()))?
             .to_owned()
@@ -83,11 +79,14 @@ mod tests {
     use super::*;
     use crate::package::tests::pname;
     use crate::package::tests::pversion;
+    use crate::filestore::path::StoreRoot;
+    use crate::filestore::path::ArtifactPath;
+    use std::path::PathBuf;
 
     #[test]
     fn test_parser_one_letter_name() {
-        let p = PathBuf::from("a-1.ext");
-        let root = PathBuf::from("/");
+        let p = ArtifactPath::new(PathBuf::from("a-1.ext"));
+        let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
@@ -99,8 +98,8 @@ mod tests {
 
     #[test]
     fn test_parser_multi_letter_name() {
-        let p = PathBuf::from("foo-1.ext");
-        let root = PathBuf::from("/");
+        let p = ArtifactPath::new(PathBuf::from("foo-1.ext"));
+        let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
@@ -112,8 +111,8 @@ mod tests {
 
     #[test]
     fn test_parser_multi_char_version() {
-        let p = PathBuf::from("foo-1123.ext");
-        let root = PathBuf::from("/");
+        let p = ArtifactPath::new(PathBuf::from("foo-1123.ext"));
+        let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
@@ -125,8 +124,8 @@ mod tests {
 
     #[test]
     fn test_parser_multi_char_version_dashed() {
-        let p = PathBuf::from("foo-1-1-2-3.ext");
-        let root = PathBuf::from("/");
+        let p = ArtifactPath::new(PathBuf::from("foo-1-1-2-3.ext"));
+        let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
@@ -138,8 +137,8 @@ mod tests {
 
     #[test]
     fn test_parser_multi_char_version_dashed_and_dotted() {
-        let p = PathBuf::from("foo-1-1.2-3.ext");
-        let root = PathBuf::from("/");
+        let p = ArtifactPath::new(PathBuf::from("foo-1-1.2-3.ext"));
+        let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
@@ -151,8 +150,8 @@ mod tests {
 
     #[test]
     fn test_parser_alnum_version() {
-        let p = PathBuf::from("foo-1-1.2a3.ext");
-        let root = PathBuf::from("/");
+        let p = ArtifactPath::new(PathBuf::from("foo-1-1.2a3.ext"));
+        let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
@@ -164,8 +163,8 @@ mod tests {
 
     #[test]
     fn test_parser_package_name_with_number() {
-        let p = PathBuf::from("foo2-1-1.2a3.ext");
-        let root = PathBuf::from("/");
+        let p = ArtifactPath::new(PathBuf::from("foo2-1-1.2a3.ext"));
+        let root = StoreRoot::new(PathBuf::from("/"));
         let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
diff --git a/src/filestore/mod.rs b/src/filestore/mod.rs
index 6f313a4..65e0609 100644
--- a/src/filestore/mod.rs
+++ b/src/filestore/mod.rs
@@ -10,4 +10,6 @@ pub use staging::*;
 mod merged;
 pub use merged::*;
 
+pub mod path;
+
 mod util;
diff --git a/src/filestore/path.rs b/src/filestore/path.rs
new file mode 100644
index 0000000..31e4bb0
--- /dev/null
+++ b/src/filestore/path.rs
@@ -0,0 +1,95 @@
+use std::path::Path;
+use std::path::PathBuf;
+use std::ffi::OsStr;
+
+use anyhow::Error;
+use anyhow::Result;
+
+#[derive(Debug)]
+pub struct StoreRoot(PathBuf);
+
+impl StoreRoot {
+    pub (in crate::filestore) fn new(root: PathBuf) -> Self {
+        StoreRoot(root)
+    }
+
+    pub (in crate::filestore) fn stripped_from(&self, pb: &Path) -> Result<ArtifactPath> {
+        pb.strip_prefix(&self.0)
+            .map(|p| ArtifactPath::new(p.to_path_buf()))
+            .map_err(Error::from)
+    }
+
+    pub fn join(&self, ap: &ArtifactPath) -> FullArtifactPath {
+        let join = self.0.join(&ap.0);
+        FullArtifactPath(join)
+    }
+
+    // Needed for FileStoreImpl::path_exists_in_store_root()
+    pub (in crate::filestore) fn join_path(&self, p: &Path) -> PathBuf {
+        self.0.join(p)
+    }
+
+    pub fn display(&self) -> std::path::Display {
+        self.0.display()
+    }
+}
+
+impl AsRef<Path> for StoreRoot {
+    fn as_ref(&self) -> &Path {
+        &self.0
+    }
+}
+
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct ArtifactPath(PathBuf);
+
+impl ArtifactPath {
+    pub (in crate::filestore) fn new(p: PathBuf) -> Self {
+        ArtifactPath(p)
+    }
+
+    pub fn display(&self) -> std::path::Display {
+        self.0.display()
+    }
+
+    pub fn file_name(&self) -> Option<&OsStr> {
+        self.0.file_name()
+    }
+
+    pub fn to_str(&self) -> Option<&str> {
+        self.0.to_str()
+    }
+
+    pub (in crate::filestore) fn file_stem(&self) -> Option<&OsStr> {
+        self.0.file_stem()
+    }
+
+    pub (in crate::filestore) fn is_dir(&self) -> bool {
+        self.0.is_dir()
+    }
+}
+
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct FullArtifactPath(PathBuf);
+
+impl AsRef<Path> for FullArtifactPath {
+    fn as_ref(&self) -> &Path {
+        self.0.as_ref()
+    }
+}
+
+impl FullArtifactPath {
+    pub (in crate::filestore) fn is_dir(&self) -> bool {
+        self.0.is_dir()
+    }
+
+    pub (in crate::filestore) fn is_file(&self) -> bool {
+        self.0.is_file()
+    }
+
+    pub fn display(&self) -> std::path::Display {
+        self.0.display()
+    }
+}
+
diff --git a/src/filestore/staging.rs b/src/filestore/staging.rs
index cbc8732..86e22ae 100644
--- a/src/filestore/staging.rs
+++ b/src/filestore/staging.rs
@@ -1,6 +1,5 @@
 use std::fmt::Debug;
 use std::path::Path;
-use std::path::PathBuf;
 
 use anyhow::Context;
 use anyhow::Error;
@@ -12,6 +11,8 @@ use log::trace;
 use result_inspect::ResultInspect;
 use tar;
 
+use crate::filestore::path::ArtifactPath;
+use crate::filestore::path::StoreRoot;
 use crate::filestore::util::FileStoreImpl;
 
 // The implementation of this type must be available in the merged filestore.
@@ -33,7 +34,7 @@ impl StagingStore {
     /// # Returns
     ///
     /// Returns a list of Artifacts that were written from the stream
-    pub async fn write_files_from_tar_stream<S>(&mut self, stream: S) -> Result<Vec<PathBuf>>
+    pub async fn write_files_from_tar_stream<S>(&mut self, stream: S) -> Result<Vec<ArtifactPath>>
         where S: Stream<Item = Result<Vec<u8>>>
     {
         use futures::stream::TryStreamExt;
@@ -64,13 +65,14 @@ impl StagingStore {
             .context("Concatenating the output bytestream")?
             .into_iter()
             .inspect(|p| trace!("Trying to load into staging store: {}", p.display()))
+            .map(ArtifactPath::new)
             .filter_map(|path| {
                 let fullpath = self.0.root.join(&path);
                 if fullpath.is_dir() {
                     None
                 } else {
                     Some({
-                        self.0.load_from_path(&fullpath)
+                        self.0.load_from_path(fullpath.as_ref())
                             .inspect(|r| trace!("Loaded from path {} = {:?}", fullpath.display(), r))
                             .with_context(|| anyhow!("Loading from path: {}", fullpath.display()))
                             .map_err(Error::from)
@@ -81,7 +83,7 @@ impl StagingStore {
             .collect()
     }
 
-    pub fn root_path(&self) -> &Path {
+    pub fn root_path(&self) -> &StoreRoot {
         self.0.root_path()
     }
 
diff --git a/src/filestore/util.rs b/src/filestore/util.rs
index 359b8db..4e0b85c 100644
--- a/src/filestore/util.rs
+++ b/src/filestore/util.rs
@@ -14,6 +14,7 @@ use resiter::Map;
 use walkdir::WalkDir;
 
 use crate::filestore::Artifact;
+use crate::filestore::path::*;
 
 /// The actual filestore implementation
 ///
@@ -22,53 +23,54 @@ use crate::filestore::Artifact;
 ///
 /// It can then be wrapped into the actual interface of this module with specialized functionality.
 pub struct FileStoreImpl {
-    pub(in crate::filestore) root: PathBuf,
+    pub(in crate::filestore) root: StoreRoot,
     store: BTreeMap<PathBuf, Artifact>,
 }
 
 impl FileStoreImpl {
     /// Loads the passed path recursively into a Path => Artifact mapping
-    pub fn load(root: &Path, progress: ProgressBar) -> Result<Self> {
-        if root.is_dir() {
-            let store = WalkDir::new(root)
+    pub fn load(path: &Path, progress: ProgressBar) -> Result<Self> {
+        if path.is_dir() {
+            let root = StoreRoot::new(path.to_path_buf());
+
+            let store = WalkDir::new(&path)
                 .follow_links(false)
                 .into_iter()
                 .filter_entry(|e| e.file_type().is_file())
                 .map_err(Error::from)
-                .map_ok(|f| f.path().to_path_buf())
-                .and_then_ok(|pb| {
+                .and_then_ok(|f| {
                     progress.tick();
-                    let p = pb.strip_prefix(root)?;
-                    Artifact::load(root, &p).map(|a| (pb, a))
+                    let p = root.stripped_from(f.path())?;
+                    Artifact::load(&root, p).map(|a| (f.path().to_path_buf(), a))
                 })
                 .collect::<Result<BTreeMap<PathBuf, Artifact>>>()?;
 
-            Ok(FileStoreImpl { root: root.to_path_buf(), store })
+            Ok(FileStoreImpl { root, store })
         } else {
-            Err(anyhow!("File store cannot be loaded from non-directory: {}", root.display()))
+            Err(anyhow!("File store cannot be loaded from non-directory: {}", path.display()))
         }
     }
 
-    pub fn root_path(&self) -> &Path {
+    pub fn root_path(&self) -> &StoreRoot {
         &self.root
     }
 
     pub fn path_exists_in_store_root(&self, p: &Path) -> bool {
-        self.root.join(p).is_file()
+        self.root.join_path(p).is_file()
     }
 
     pub (in crate::filestore) fn values(&self) -> impl Iterator<Item = &Artifact> {
         self.store.values()
     }
 
-    pub (in crate::filestore) fn load_from_path(&mut self, pb: &PathBuf) -> Result<&Artifact> {
+    pub (in crate::filestore) fn load_from_path(&mut self, pb: &Path) -> Result<&Artifact> {
         if !self.is_sub_path(pb)? {
             Err(anyhow!("Not a sub-path of {}: {}", self.root.display(), pb.display()))
         } else {
             if self.store.get(pb).is_some() {
                 Err(anyhow!("Entry exists: {}", pb.display()))
             } else {
-                let p = pb.strip_prefix(&self.root)?;
+                let p = self.root.stripped_from(pb)?;
                 Ok(self.store.entry(pb.to_path_buf()).or_insert(Artifact::load(&self.root, p)?))
             }
         }
@@ -76,7 +78,7 @@ impl FileStoreImpl {
 
     fn is_sub_path(&self, p: &Path) -> Result<bool> {
         p.canonicalize()
-            .map(|c| c.starts_with(&self.root))
+            .map(|c| c.starts_with(self.root.as_ref()))
             .map_err(Error::from)
     }