Make FullArtifactPath a tuple of StoreRoot and ArtifactPath

This changes the implementation of FullArtifactPath from holding a
complete PathBuf object, to holding references to StoreRoot and
ArtifcatPath objects.

This not only reduces memory but more importantly is another step into
the right direction concerning strong path types.

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

3 files changed, 28 insertions, 37 deletions

diff --git a/src/filestore/path.rs b/src/filestore/path.rs
index 0eba9ea..ea06608 100644
--- a/src/filestore/path.rs
+++ b/src/filestore/path.rs
@@ -7,7 +7,7 @@ use anyhow::Error;
 use anyhow::Result;
 use anyhow::Context;
 
-#[derive(Debug)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct StoreRoot(PathBuf);
 
 impl StoreRoot {
@@ -21,9 +21,8 @@ impl StoreRoot {
             .map_err(Error::from)
     }
 
-    pub fn join(&self, ap: &ArtifactPath) -> FullArtifactPath {
-        let join = self.0.join(&ap.0);
-        FullArtifactPath(join)
+    pub fn join<'a>(&'a self, ap: &'a ArtifactPath) -> FullArtifactPath<'a> {
+        FullArtifactPath(&self, ap)
     }
 
     // Needed for FileStoreImpl::path_exists_in_store_root()
@@ -73,27 +72,23 @@ impl ArtifactPath {
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct FullArtifactPath(PathBuf);
-
-impl FullArtifactPath {
-    pub (in crate::filestore) fn is_dir(&self) -> bool {
-        self.0.is_dir()
-    }
+pub struct FullArtifactPath<'a>(&'a StoreRoot, &'a ArtifactPath);
 
-    pub (in crate::filestore) fn as_path(&self) -> &Path {
-        self.0.as_ref()
+impl<'a> FullArtifactPath<'a> {
+    fn joined(&self) -> PathBuf {
+        self.0.0.join(&self.1.0)
     }
 
     pub (in crate::filestore) fn is_file(&self) -> bool {
-        self.0.is_file()
+        self.joined().is_file()
     }
 
-    pub fn display(&self) -> std::path::Display {
-        self.0.display()
+    pub fn display(&self) -> FullArtifactPathDisplay<'a> {
+        FullArtifactPathDisplay(self.0, self.1)
     }
 
     pub async fn read(self) -> Result<Vec<u8>> {
-        tokio::fs::read(&self.0)
+        tokio::fs::read(self.joined())
             .await
             .map(Vec::from)
             .with_context(|| anyhow!("Reading artifact from path {}", self.0.display()))
@@ -101,3 +96,11 @@ impl FullArtifactPath {
     }
 }
 
+pub struct FullArtifactPathDisplay<'a>(&'a StoreRoot, &'a ArtifactPath);
+
+impl<'a> std::fmt::Display for FullArtifactPathDisplay<'a> {
+    fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(fmt, "{}/{}", self.0.display(), self.1.display())
+    }
+}
+
diff --git a/src/filestore/staging.rs b/src/filestore/staging.rs
index 0aca6da..992a612 100644
--- a/src/filestore/staging.rs
+++ b/src/filestore/staging.rs
@@ -65,16 +65,15 @@ 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() {
+                if self.0.root.join_path(&path).is_dir() {
                     None
                 } else {
                     Some({
-                        self.0.load_from_path(&fullpath)
-                            .inspect(|r| trace!("Loaded from path {} = {:?}", fullpath.display(), r))
-                            .with_context(|| anyhow!("Loading from path: {}", fullpath.display()))
+                        let path = ArtifactPath::new(path);
+                        self.0.load_from_path(&path)
+                            .inspect(|r| trace!("Loaded from path {} = {:?}", path.display(), r))
+                            .with_context(|| anyhow!("Loading from path: {}", path.display()))
                             .map_err(Error::from)
                             .map(|art| art.path().clone())
                     })
diff --git a/src/filestore/util.rs b/src/filestore/util.rs
index c32dd5d..1899c18 100644
--- a/src/filestore/util.rs
+++ b/src/filestore/util.rs
@@ -62,24 +62,13 @@ impl FileStoreImpl {
         self.store.values()
     }
 
-    pub (in crate::filestore) fn load_from_path(&mut self, pb: &FullArtifactPath) -> Result<&Artifact> {
-        if !self.is_sub_path(pb.as_path())? {
-            Err(anyhow!("Not a sub-path of {}: {}", self.root.display(), pb.display()))
+    pub (in crate::filestore) fn load_from_path(&mut self, artifact_path: &ArtifactPath) -> Result<&Artifact> {
+        if self.store.get(&artifact_path).is_some() {
+            Err(anyhow!("Entry exists: {}", artifact_path.display()))
         } else {
-            let art_path = self.root.stripped_from(pb.as_path())?;
-            if self.store.get(&art_path).is_some() {
-                Err(anyhow!("Entry exists: {}", pb.display()))
-            } else {
-                Ok(self.store.entry(art_path.clone()).or_insert(Artifact::load(&self.root, art_path)?))
-            }
+            Ok(self.store.entry(artifact_path.clone()).or_insert(Artifact::load(&self.root, artifact_path.clone())?))
         }
     }
 
-    fn is_sub_path(&self, p: &Path) -> Result<bool> {
-        p.canonicalize()
-            .map(|c| c.starts_with(self.root.as_ref()))
-            .map_err(Error::from)
-    }
-
 }