Make sure only the relative pathes are written to database

This patch is a bit of a mess.

It makes sure that an crate::filestore::Artifact only knows its
_relative_ path to the store it is put into.  First of all, this is what
it should be like. Secondly, we only want to track relative pathes in
the database (to reduce database size, but more
importantly because it is just duplicated data).  The full path of an
Artifact can always be reconstructed from the submit id, the configured
pathes of the staging/release store and the artifact
information (relative path) tracked in the database.

This patch would not be necessary if we would introduce strong typing
for the different kinds of pathes.
This is most certainly on our todo list here.

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

3 files changed, 31 insertions, 21 deletions

diff --git a/src/filestore/artifact.rs b/src/filestore/artifact.rs
index 92efd3d..9e02de6 100644
--- a/src/filestore/artifact.rs
+++ b/src/filestore/artifact.rs
@@ -40,10 +40,11 @@ impl Ord for Artifact {
 
 
 impl Artifact {
-    pub fn load(path: &Path) -> Result<Self> {
-        if path.is_file() {
-            let (name, version) = Self::parse_path(path)
-                .with_context(|| anyhow!("Pathing artifact path: '{}'", path.display()))?;
+    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()))?;
 
             Ok(Artifact {
                 path: path.to_path_buf(),
@@ -60,12 +61,12 @@ impl Artifact {
         }
     }
 
-    fn parse_path(path: &Path) -> Result<(PackageName, PackageVersion)> {
+    fn parse_path(root: &Path, path: &Path) -> Result<(PackageName, PackageVersion)> {
         path.file_stem()
-            .ok_or_else(|| anyhow!("Cannot get filename from {}", path.display()))?
+            .ok_or_else(|| anyhow!("Cannot get filename from {}", (root.join(path)).display()))?
             .to_owned()
             .into_string()
-            .map_err(|_| anyhow!("Internal conversion of '{}' to UTF-8", path.display()))
+            .map_err(|_| anyhow!("Internal conversion of '{}' to UTF-8", (root.join(path)).display()))
             .and_then(|s| Self::parser().parse(s.as_bytes()).map_err(Error::from))
     }
 
@@ -108,7 +109,8 @@ mod tests {
     #[test]
     fn test_parser_one_letter_name() {
         let p = PathBuf::from("a-1.ext");
-        let r = Artifact::parse_path(&p);
+        let root = PathBuf::from("/");
+        let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
         let (name, version) = r.unwrap();
@@ -120,7 +122,8 @@ mod tests {
     #[test]
     fn test_parser_multi_letter_name() {
         let p = PathBuf::from("foo-1.ext");
-        let r = Artifact::parse_path(&p);
+        let root = PathBuf::from("/");
+        let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
         let (name, version) = r.unwrap();
@@ -132,7 +135,8 @@ mod tests {
     #[test]
     fn test_parser_multi_char_version() {
         let p = PathBuf::from("foo-1123.ext");
-        let r = Artifact::parse_path(&p);
+        let root = PathBuf::from("/");
+        let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
         let (name, version) = r.unwrap();
@@ -144,7 +148,8 @@ mod tests {
     #[test]
     fn test_parser_multi_char_version_dashed() {
         let p = PathBuf::from("foo-1-1-2-3.ext");
-        let r = Artifact::parse_path(&p);
+        let root = PathBuf::from("/");
+        let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
         let (name, version) = r.unwrap();
@@ -156,7 +161,8 @@ mod tests {
     #[test]
     fn test_parser_multi_char_version_dashed_and_dotted() {
         let p = PathBuf::from("foo-1-1.2-3.ext");
-        let r = Artifact::parse_path(&p);
+        let root = PathBuf::from("/");
+        let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
         let (name, version) = r.unwrap();
@@ -168,7 +174,8 @@ mod tests {
     #[test]
     fn test_parser_alnum_version() {
         let p = PathBuf::from("foo-1-1.2a3.ext");
-        let r = Artifact::parse_path(&p);
+        let root = PathBuf::from("/");
+        let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
         let (name, version) = r.unwrap();
@@ -180,7 +187,8 @@ mod tests {
     #[test]
     fn test_parser_package_name_with_number() {
         let p = PathBuf::from("foo2-1-1.2a3.ext");
-        let r = Artifact::parse_path(&p);
+        let root = PathBuf::from("/");
+        let r = Artifact::parse_path(&root, &p);
 
         assert!(r.is_ok(), "Expected to be Ok(_): {:?}", r);
         let (name, version) = r.unwrap();
diff --git a/src/filestore/staging.rs b/src/filestore/staging.rs
index 91e71d2..bd97f6c 100644
--- a/src/filestore/staging.rs
+++ b/src/filestore/staging.rs
@@ -51,7 +51,6 @@ impl StagingStore {
                         let p = ent?.path().context("Getting path of TAR entry")?.into_owned();
                         Ok(p)
                     })
-                    .map_ok(|path| dest.join(path))
                     .inspect(|p| trace!("Path in tar archive: {:?}", p))
                     .collect::<Result<Vec<_>>>()
                     .context("Collecting outputs of TAR archive")?;
@@ -67,13 +66,14 @@ impl StagingStore {
             .into_iter()
             .inspect(|p| trace!("Trying to load into staging store: {}", p.display()))
             .filter_map(|path| {
-                if path.is_dir() {
+                let fullpath = self.0.root.join(&path);
+                if fullpath.is_dir() {
                     None
                 } else {
                     Some({
-                        self.0.load_from_path(&path)
-                            .inspect(|r| trace!("Loading from path = {:?}", r))
-                            .with_context(|| anyhow!("Loading from path: {}", path.display()))
+                        self.0.load_from_path(&fullpath)
+                            .inspect(|r| trace!("Loaded from path {} = {:?}", fullpath.display(), r))
+                            .with_context(|| anyhow!("Loading from path: {}", fullpath.display()))
                             .map_err(Error::from)
                             .map(|art| art.path().clone())
                     })
diff --git a/src/filestore/util.rs b/src/filestore/util.rs
index c898420..f6d775f 100644
--- a/src/filestore/util.rs
+++ b/src/filestore/util.rs
@@ -40,7 +40,8 @@ impl FileStoreImpl {
                 .map_ok(|f| f.path().to_path_buf())
                 .and_then_ok(|pb| {
                     progress.tick();
-                    Artifact::load(&pb).map(|a| (pb, a))
+                    let p = pb.strip_prefix(root)?;
+                    Artifact::load(root, &p).map(|a| (pb, a))
                 })
                 .collect::<Result<BTreeMap<PathBuf, Artifact>>>()?;
 
@@ -69,7 +70,8 @@ impl FileStoreImpl {
             if self.store.get(pb).is_some() {
                 Err(anyhow!("Entry exists: {}", pb.display()))
             } else {
-                Ok(self.store.entry(pb.to_path_buf()).or_insert(Artifact::load(pb)?))
+                let p = pb.strip_prefix(&self.root)?;
+                Ok(self.store.entry(pb.to_path_buf()).or_insert(Artifact::load(&self.root, p)?))
             }
         }
     }