Add checks so that FullArtifactPath can only be constructed for existing files

In the process, FullArtifactPath::is_file() and FullArtifactPath::is_dir()
as well as ArtifactPath::file_stem() were removed.

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

2 files changed, 28 insertions, 34 deletions

diff --git a/src/filestore/artifact.rs b/src/filestore/artifact.rs
index 390bc7d..3636ab0 100644
--- a/src/filestore/artifact.rs
+++ b/src/filestore/artifact.rs
@@ -38,31 +38,23 @@ impl Ord for Artifact {
 
 impl Artifact {
     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,
-                name,
-                version
-            })
-        } else {
-            if root.join(&path).is_dir() {
-                Err(anyhow!("Cannot load non-file path: {}", path.display()))
-            } else {
-                Err(anyhow!("Path does not exist: {}", path.display()))
-            }
-        }
+        let joined_fullpath = root.join(&path)?;
+        let (name, version) = Self::parse_path(&joined_fullpath)
+            .with_context(|| anyhow!("Pathing artifact path: '{}'", joined_fullpath.display()))?;
+
+        Ok(Artifact {
+            path,
+            name,
+            version
+        })
     }
 
-    fn parse_path(root: &StoreRoot, path: &ArtifactPath) -> Result<(PackageName, PackageVersion)> {
+    fn parse_path(path: &FullArtifactPath) -> Result<(PackageName, PackageVersion)> {
         path.file_stem()
-            .ok_or_else(|| anyhow!("Cannot get filename from {}", (root.join(path)).display()))?
+            .ok_or_else(|| anyhow!("Cannot get filename from {}", path.display()))?
             .to_owned()
             .into_string()
-            .map_err(|_| anyhow!("Internal conversion of '{}' to UTF-8", (root.join(path)).display()))
+            .map_err(|_| anyhow!("Internal conversion of '{}' to UTF-8", path.display()))
             .and_then(|s| Self::parser().parse(s.as_bytes()).map_err(Error::from))
     }
 
diff --git a/src/filestore/path.rs b/src/filestore/path.rs
index b2f677b..c691cdb 100644
--- a/src/filestore/path.rs
+++ b/src/filestore/path.rs
@@ -25,8 +25,18 @@ impl StoreRoot {
         }
     }
 
-    pub fn join<'a>(&'a self, ap: &'a ArtifactPath) -> FullArtifactPath<'a> {
-        FullArtifactPath(&self, ap)
+    pub fn join<'a>(&'a self, ap: &'a ArtifactPath) -> Result<FullArtifactPath<'a>> {
+        let join = self.0.join(&ap.0);
+
+        if join.is_file() {
+            Ok(FullArtifactPath(&self, ap))
+        } else {
+            if join.is_dir() {
+                Err(anyhow!("Cannot load non-file path: {}", join.display()))
+            } else {
+                Err(anyhow!("Path does not exist: {}", join.display()))
+            }
+        }
     }
 
     pub fn is_file(&self, subpath: &Path) -> bool {
@@ -83,10 +93,6 @@ impl ArtifactPath {
     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()
-    }
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -97,18 +103,14 @@ impl<'a> FullArtifactPath<'a> {
         self.0.0.join(&self.1.0)
     }
 
-    pub (in crate::filestore) fn is_file(&self) -> bool {
-        self.joined().is_file()
-    }
-
-    pub (in crate::filestore) fn is_dir(&self) -> bool {
-        self.joined().is_dir()
-    }
-
     pub fn display(&self) -> FullArtifactPathDisplay<'a> {
         FullArtifactPathDisplay(self.0, self.1)
     }
 
+    pub (in crate::filestore) fn file_stem(&self) -> Option<&OsStr> {
+        self.1.0.file_stem()
+    }
+
     pub async fn read(self) -> Result<Vec<u8>> {
         tokio::fs::read(self.joined())
             .await