Make stores only loadable with StoreRoot object

This patch changes the interfaces to no load the StoreRoot object
internally, but getting it from the caller.
The StoreRoot::new() function got more restrictive, to only be able to
load the StoreRoot object when pointing to an existing directory.

This makes is_dir() checks during the runtime unecessary, which reduces
runtime overall, because this test is only done once during
construction, not N times during usage of the store objects.

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

4 files changed, 26 insertions, 24 deletions

diff --git a/src/filestore/path.rs b/src/filestore/path.rs
index 25ffbd9..134c132 100644
--- a/src/filestore/path.rs
+++ b/src/filestore/path.rs
@@ -11,9 +11,13 @@ use anyhow::Context;
 pub struct StoreRoot(PathBuf);
 
 impl StoreRoot {
-    pub (in crate::filestore) fn new(root: PathBuf) -> Result<Self> {
+    pub fn new(root: PathBuf) -> Result<Self> {
         if root.is_absolute() {
-            Ok(StoreRoot(root))
+            if root.is_dir() {
+                Ok(StoreRoot(root))
+            } else {
+                Err(anyhow!("StoreRoot path does not point to directory: {}", root.display()))
+            }
         } else {
             Err(anyhow!("StoreRoot path is not absolute: {}", root.display()))
         }
@@ -37,6 +41,10 @@ impl StoreRoot {
         self.0.join(subpath).is_dir()
     }
 
+    pub (in crate::filestore) fn as_path(&self) -> &Path {
+        self.0.as_ref()
+    }
+
     pub fn display(&self) -> std::path::Display {
         self.0.display()
     }
diff --git a/src/filestore/release.rs b/src/filestore/release.rs
index fb0bac5..c05d7e8 100644
--- a/src/filestore/release.rs
+++ b/src/filestore/release.rs
@@ -1,10 +1,10 @@
 use std::fmt::Debug;
-use std::path::Path;
 
 use anyhow::Result;
 use indicatif::ProgressBar;
 
 use crate::filestore::util::FileStoreImpl;
+use crate::filestore::path::StoreRoot;
 
 // The implementation of this type must be available in the merged filestore.
 pub struct ReleaseStore(pub (in crate::filestore) FileStoreImpl);
@@ -17,7 +17,7 @@ impl Debug for ReleaseStore {
 
 
 impl ReleaseStore {
-    pub fn load(root: &Path, progress: ProgressBar) -> Result<Self> {
+    pub fn load(root: StoreRoot, progress: ProgressBar) -> Result<Self> {
         FileStoreImpl::load(root, progress).map(ReleaseStore)
     }
 }
diff --git a/src/filestore/staging.rs b/src/filestore/staging.rs
index 93c4047..d474d1a 100644
--- a/src/filestore/staging.rs
+++ b/src/filestore/staging.rs
@@ -25,7 +25,7 @@ impl Debug for StagingStore {
 }
 
 impl StagingStore {
-    pub fn load(root: &Path, progress: ProgressBar) -> Result<Self> {
+    pub fn load(root: StoreRoot, progress: ProgressBar) -> Result<Self> {
         FileStoreImpl::load(root, progress).map(StagingStore)
     }
 
diff --git a/src/filestore/util.rs b/src/filestore/util.rs
index 2dca4e2..75e3972 100644
--- a/src/filestore/util.rs
+++ b/src/filestore/util.rs
@@ -28,26 +28,20 @@ pub struct FileStoreImpl {
 
 impl FileStoreImpl {
     /// Loads the passed path recursively into a Path => Artifact mapping
-    pub fn load(path: &Path, progress: ProgressBar) -> Result<Self> {
-        if path.is_dir() {
-            let root = StoreRoot::new(path.to_path_buf())?;
+    pub fn load(root: StoreRoot, progress: ProgressBar) -> Result<Self> {
+        let store = WalkDir::new(root.as_path())
+            .follow_links(false)
+            .into_iter()
+            .filter_entry(|e| e.file_type().is_file())
+            .map_err(Error::from)
+            .and_then_ok(|f| {
+                progress.tick();
+                let p = root.stripped_from(f.path())?;
+                Artifact::load(&root, p.clone()).map(|a| (p, a))
+            })
+            .collect::<Result<BTreeMap<ArtifactPath, Artifact>>>()?;
 
-            let store = WalkDir::new(&path)
-                .follow_links(false)
-                .into_iter()
-                .filter_entry(|e| e.file_type().is_file())
-                .map_err(Error::from)
-                .and_then_ok(|f| {
-                    progress.tick();
-                    let p = root.stripped_from(f.path())?;
-                    Artifact::load(&root, p.clone()).map(|a| (p, a))
-                })
-                .collect::<Result<BTreeMap<ArtifactPath, Artifact>>>()?;
-
-            Ok(FileStoreImpl { root, store })
-        } else {
-            Err(anyhow!("File store cannot be loaded from non-directory: {}", path.display()))
-        }
+        Ok(FileStoreImpl { root, store })
     }
 
     pub fn root_path(&self) -> &StoreRoot {