Create newtype for tag namespace

As part of transitioning to use RelativePath instead of Path, create wrapper types to partially hide the underlying type from the public API. Signed-off-by: J Robert Ray <jrray@jrray.org>
spkenv · Dec 12, 2023 · 4d57ce5 · 4d57ce5
1 parent 0b9f2c9
commit 4d57ce5
Show file tree

Hide file tree

Showing 17 changed files with 298 additions and 193 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/spfs/Cargo.toml b/crates/spfs/Cargo.toml
@@ -52,7 +52,7 @@ pin-project-lite = { workspace = true }
 progress_bar_derive_macro = { workspace = true }
 prost = { workspace = true }
 rand = "0.8.5"
-relative-path = "1.3"
+relative-path = { version = "1.3", features = ["serde"] }
 ring = "0.16.15"
 semver = "1.0"
 sentry = { workspace = true, optional = true }

diff --git a/crates/spfs/src/config.rs b/crates/spfs/src/config.rs
@@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize};
 use storage::{FromConfig, FromUrl};
 use tokio_stream::StreamExt;
 
+use crate::storage::TagNamespaceBuf;
 use crate::{runtime, storage, tracking, Error, Result};
 
 #[cfg(test)]
@@ -79,7 +80,7 @@ pub struct Storage {
     /// is owned by a different user than the current user. Only applies to
     /// payloads readable by "other".
     pub allow_payload_sharing_between_users: bool,
-    pub tag_namespace: Option<PathBuf>,
+    pub tag_namespace: Option<TagNamespaceBuf>,
 }
 
 impl Storage {

diff --git a/crates/spfs/src/fixtures.rs b/crates/spfs/src/fixtures.rs
@@ -42,7 +42,9 @@ impl TempRepo {
                 let mut repo = spfs::storage::fs::FsRepository::open(tempdir.path().join("repo"))
                     .await
                     .unwrap();
-                repo.set_tag_namespace(Some(namespace.as_ref().into()));
+                repo.set_tag_namespace(Some(spfs::storage::TagNamespaceBuf::new(
+                    namespace.as_ref(),
+                )));
                 TempRepo::FS(Arc::new(repo.into()), Arc::clone(tempdir))
             }
             _ => panic!("only TempRepo::FS type supports setting tag namespaces"),

diff --git a/crates/spfs/src/server/tag.rs b/crates/spfs/src/server/tag.rs
@@ -3,23 +3,23 @@
 // https://github.com/imageworks/spk
 
 use std::convert::TryInto;
-use std::path::Path;
 use std::sync::Arc;
 
 use futures::TryStreamExt;
+use relative_path::RelativePath;
 use tokio_stream::StreamExt;
 use tonic::{Request, Response, Status};
 
 use crate::prelude::*;
 use crate::proto::tag_service_server::TagServiceServer;
 use crate::proto::{self, convert_digest, RpcResult};
-use crate::storage;
+use crate::storage::{self, TagNamespace};
 
-fn string_to_namespace(namespace: &String) -> Option<&Path> {
+fn string_to_namespace(namespace: &String) -> Option<&TagNamespace> {
     if namespace.is_empty() {
         None
     } else {
-        Some(Path::new(namespace))
+        Some(TagNamespace::new(RelativePath::new(namespace)))
     }
 }
 

diff --git a/crates/spfs/src/storage/fallback/repository.rs b/crates/spfs/src/storage/fallback/repository.rs
@@ -3,7 +3,6 @@
 // https://github.com/imageworks/spk
 
 use std::borrow::Cow;
-use std::path::{Path, PathBuf};
 use std::pin::Pin;
 use std::sync::Arc;
 
@@ -15,7 +14,7 @@ use crate::config::ToAddress;
 use crate::prelude::*;
 use crate::storage::fs::{FsHashStore, ManifestRenderPath, OpenFsRepository, RenderStore};
 use crate::storage::tag::TagSpecAndTagStream;
-use crate::storage::{EntryType, LocalRepository, TagStorageMut};
+use crate::storage::{EntryType, LocalRepository, TagNamespace, TagNamespaceBuf, TagStorageMut};
 use crate::tracking::BlobRead;
 use crate::{encoding, graph, storage, tracking, Error, Result};
 
@@ -287,44 +286,44 @@ impl PayloadStorage for FallbackProxy {
 #[async_trait::async_trait]
 impl TagStorage for FallbackProxy {
     #[inline]
-    fn get_tag_namespace(&self) -> Option<Cow<'_, Path>> {
+    fn get_tag_namespace(&self) -> Option<Cow<'_, TagNamespace>> {
         self.primary.get_tag_namespace()
     }
 
     fn ls_tags_in_namespace(
         &self,
-        namespace: Option<&Path>,
+        namespace: Option<&TagNamespace>,
         path: &RelativePath,
     ) -> Pin<Box<dyn Stream<Item = Result<EntryType>> + Send>> {
         self.primary.ls_tags_in_namespace(namespace, path)
     }
 
     fn find_tags_in_namespace(
         &self,
-        namespace: Option<&Path>,
+        namespace: Option<&TagNamespace>,
         digest: &encoding::Digest,
     ) -> Pin<Box<dyn Stream<Item = Result<tracking::TagSpec>> + Send>> {
         self.primary.find_tags_in_namespace(namespace, digest)
     }
 
     fn iter_tag_streams_in_namespace(
         &self,
-        namespace: Option<&Path>,
+        namespace: Option<&TagNamespace>,
     ) -> Pin<Box<dyn Stream<Item = Result<TagSpecAndTagStream>> + Send>> {
         self.primary.iter_tag_streams_in_namespace(namespace)
     }
 
     async fn read_tag_in_namespace(
         &self,
-        namespace: Option<&Path>,
+        namespace: Option<&TagNamespace>,
         tag: &tracking::TagSpec,
     ) -> Result<Pin<Box<dyn Stream<Item = Result<tracking::Tag>> + Send>>> {
         self.primary.read_tag_in_namespace(namespace, tag).await
     }
 
     async fn insert_tag_in_namespace(
         &self,
-        namespace: Option<&Path>,
+        namespace: Option<&TagNamespace>,
         tag: &tracking::Tag,
     ) -> Result<()> {
         self.primary.insert_tag_in_namespace(namespace, tag).await?;
@@ -333,7 +332,7 @@ impl TagStorage for FallbackProxy {
 
     async fn remove_tag_stream_in_namespace(
         &self,
-        namespace: Option<&Path>,
+        namespace: Option<&TagNamespace>,
         tag: &tracking::TagSpec,
     ) -> Result<()> {
         self.primary
@@ -344,7 +343,7 @@ impl TagStorage for FallbackProxy {
 
     async fn remove_tag_in_namespace(
         &self,
-        namespace: Option<&Path>,
+        namespace: Option<&TagNamespace>,
         tag: &tracking::Tag,
     ) -> Result<()> {
         self.primary.remove_tag_in_namespace(namespace, tag).await?;
@@ -353,7 +352,10 @@ impl TagStorage for FallbackProxy {
 }
 
 impl TagStorageMut for FallbackProxy {
-    fn try_set_tag_namespace(&mut self, tag_namespace: Option<PathBuf>) -> Result<Option<PathBuf>> {
+    fn try_set_tag_namespace(
+        &mut self,
+        tag_namespace: Option<TagNamespaceBuf>,
+    ) -> Result<Option<TagNamespaceBuf>> {
         Ok(Arc::make_mut(&mut self.primary).set_tag_namespace(tag_namespace))
     }
 }

diff --git a/crates/spfs/src/storage/fs/repository.rs b/crates/spfs/src/storage/fs/repository.rs
@@ -19,7 +19,13 @@ use super::FsHashStore;
 use crate::config::ToAddress;
 use crate::runtime::makedirs_with_perms;
 use crate::storage::prelude::*;
-use crate::storage::{LocalRepository, OpenRepositoryError, OpenRepositoryResult};
+use crate::storage::{
+    LocalRepository,
+    OpenRepositoryError,
+    OpenRepositoryResult,
+    TagNamespace,
+    TagNamespaceBuf,
+};
 use crate::{Error, Result};
 
 /// The directory name within the repo where durable runtimes keep
@@ -55,7 +61,7 @@ pub struct Params {
     pub create: bool,
     #[serde(default)]
     pub lazy: bool,
-    pub tag_namespace: Option<PathBuf>,
+    pub tag_namespace: Option<TagNamespaceBuf>,
 }
 
 #[async_trait::async_trait]
@@ -214,7 +220,7 @@ impl FsRepository {
         }
     }
 
-    pub fn get_tag_namespace(&self) -> Option<Cow<'_, Path>> {
+    pub fn get_tag_namespace(&self) -> Option<Cow<'_, TagNamespace>> {
         match &**self.0.load() {
             InnerFsRepository::Open(repo) => repo
                 .get_tag_namespace()
@@ -229,7 +235,10 @@ impl FsRepository {
         }
     }
 
-    pub fn set_tag_namespace(&mut self, tag_namespace: Option<PathBuf>) -> Option<PathBuf> {
+    pub fn set_tag_namespace(
+        &mut self,
+        tag_namespace: Option<TagNamespaceBuf>,
+    ) -> Option<TagNamespaceBuf> {
         let mut old_namespace = None;
         self.0.rcu(|inner| match &**inner {
             InnerFsRepository::Open(repo) => {
@@ -269,7 +278,7 @@ pub struct OpenFsRepository {
     root: PathBuf,
     /// the namespace to use for tag resolution. If set, then this is treated
     /// as "chroot" of the real tag root.
-    tag_namespace: Option<PathBuf>,
+    tag_namespace: Option<TagNamespaceBuf>,
     /// stores the actual file data/payloads of this repo
     pub payloads: FsHashStore,
     /// stores all digraph object data for this repo
@@ -333,7 +342,8 @@ impl OpenFsRepository {
                 tag_namespace: self.tag_namespace.clone(),
             },
         }
-        .to_address().expect("repository address is valid")
+        .to_address()
+        .expect("repository address is valid")
     }
 
     /// The filesystem root path of this repository
@@ -381,13 +391,16 @@ impl OpenFsRepository {
 
     /// Return the configured tag namespace, if any.
     #[inline]
-    pub fn get_tag_namespace(&self) -> Option<Cow<'_, Path>> {
+    pub fn get_tag_namespace(&self) -> Option<Cow<'_, TagNamespace>> {
         self.tag_namespace.as_deref().map(Cow::Borrowed)
     }
 
     /// Set the configured tag namespace, returning the old tag namespace,
     /// if there was one.
-    pub fn set_tag_namespace(&mut self, tag_namespace: Option<PathBuf>) -> Option<PathBuf> {
+    pub fn set_tag_namespace(
+        &mut self,
+        tag_namespace: Option<TagNamespaceBuf>,
+    ) -> Option<TagNamespaceBuf> {
         std::mem::replace(&mut self.tag_namespace, tag_namespace)
     }