Merge pull request #1153 from spkenv/repo-address-trait

Move Repository::address to dedicated trait

crates/spfs/src/bootstrap.rs

@@ -254,7 +254,7 @@ where
 
 pub(crate) fn build_spfs_remove_durable_command(
     runtime_name: String,
-    repo_address: Url,
+    repo_address: &Url,
 ) -> Result<Command> {
     let exe = match which_spfs("clean") {
         None => return Err(Error::MissingBinary("spfs-clean")),

crates/spfs/src/runtime/storage.rs

@@ -918,7 +918,7 @@ impl Storage {
     }
 
     /// The address of the underlying repository being used
-    pub fn address(&self) -> url::Url {
+    pub fn address(&self) -> Cow<'_, url::Url> {
         self.inner.address()
     }
 
@@ -935,7 +935,7 @@ impl Storage {
             // is run to do it.
             let mut cmd = bootstrap::build_spfs_remove_durable_command(
                 name.as_ref().to_string(),
-                self.inner.address(),
+                &self.inner.address(),
             )?
             .into_std();
             tracing::trace!("running: {cmd:?}");

crates/spfs/src/storage/address.rs

@@ -0,0 +1,17 @@
+// Copyright (c) Contributors to the SPK project.
+// SPDX-License-Identifier: Apache-2.0
+// https://github.com/spkenv/spk
+
+use std::borrow::Cow;
+
+/// The address of a repository.
+pub trait Address {
+    /// Return the address of this repository.
+    fn address(&self) -> Cow<'_, url::Url>;
+}
+
+impl<T: Address> Address for &T {
+    fn address(&self) -> Cow<'_, url::Url> {
+        T::address(&**self)
+    }
+}

crates/spfs/src/storage/fallback/repository.rs

@@ -366,8 +366,8 @@ impl BlobStorage for FallbackProxy {}
 impl ManifestStorage for FallbackProxy {}
 impl LayerStorage for FallbackProxy {}
 impl PlatformStorage for FallbackProxy {}
-impl Repository for FallbackProxy {
-    fn address(&self) -> url::Url {
+impl Address for FallbackProxy {
+    fn address(&self) -> Cow<'_, url::Url> {
         let config = Config {
             primary: self.primary.address().to_string(),
             secondary: self
@@ -376,11 +376,14 @@ impl Repository for FallbackProxy {
                 .map(|s| s.address().to_string())
                 .collect(),
         };
-        config
-            .to_address()
-            .expect("We should not fail to create a url")
+        Cow::Owned(
+            config
+                .to_address()
+                .expect("We should not fail to create a url"),
+        )
     }
 }
+impl Repository for FallbackProxy {}
 
 impl LocalRepository for FallbackProxy {
     #[inline]

crates/spfs/src/storage/fs/repository.rs

@@ -262,11 +262,12 @@ impl BlobStorage for FsRepository {}
 impl ManifestStorage for FsRepository {}
 impl LayerStorage for FsRepository {}
 impl PlatformStorage for FsRepository {}
-impl Repository for FsRepository {
-    fn address(&self) -> url::Url {
-        url::Url::from_directory_path(self.root()).unwrap()
+impl Address for FsRepository {
+    fn address(&self) -> Cow<'_, url::Url> {
+        Cow::Owned(url::Url::from_directory_path(self.root()).unwrap())
     }
 }
+impl Repository for FsRepository {}
 
 impl std::fmt::Debug for FsRepository {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -538,11 +539,12 @@ impl BlobStorage for OpenFsRepository {}
 impl ManifestStorage for OpenFsRepository {}
 impl LayerStorage for OpenFsRepository {}
 impl PlatformStorage for OpenFsRepository {}
-impl Repository for OpenFsRepository {
-    fn address(&self) -> url::Url {
-        url::Url::from_directory_path(self.root()).unwrap()
+impl Address for OpenFsRepository {
+    fn address(&self) -> Cow<'_, url::Url> {
+        Cow::Owned(url::Url::from_directory_path(self.root()).unwrap())
     }
 }
+impl Repository for OpenFsRepository {}
 
 impl std::fmt::Debug for OpenFsRepository {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {

crates/spfs/src/storage/handle.rs

@@ -36,12 +36,14 @@ macro_rules! each_variant {
     };
 }
 
-#[async_trait::async_trait]
-impl Repository for RepositoryHandle {
-    fn address(&self) -> url::Url {
+impl Address for RepositoryHandle {
+    fn address(&self) -> Cow<'_, url::Url> {
         each_variant!(self, repo, { repo.address() })
     }
+}
 
+#[async_trait::async_trait]
+impl Repository for RepositoryHandle {
     async fn has_ref(&self, reference: &str) -> bool {
         each_variant!(self, repo, { repo.has_ref(reference).await })
     }
@@ -247,13 +249,15 @@ impl Database for RepositoryHandle {
     }
 }
 
-#[async_trait::async_trait]
-impl Repository for Arc<RepositoryHandle> {
+impl Address for Arc<RepositoryHandle> {
     /// Return the address of this repository.
-    fn address(&self) -> url::Url {
+    fn address(&self) -> Cow<'_, url::Url> {
         each_variant!(&**self, repo, { repo.address() })
     }
+}
 
+#[async_trait::async_trait]
+impl Repository for Arc<RepositoryHandle> {
     /// Return true if this repository contains the given reference.
     async fn has_ref(&self, reference: &str) -> bool {
         each_variant!(&**self, repo, { repo.has_ref(reference).await })

crates/spfs/src/storage/mod.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 // https://github.com/spkenv/spk
 
+mod address;
 mod blob;
 mod error;
 mod layer;
@@ -24,6 +25,7 @@ pub mod tar;
 
 use std::sync::Arc;
 
+pub use address::Address;
 pub use blob::BlobStorage;
 use chrono::{DateTime, Utc};
 pub use error::OpenRepositoryError;

crates/spfs/src/storage/pinned/repository.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: Apache-2.0
 // https://github.com/spkenv/spk
 
+use std::borrow::Cow;
 use std::pin::Pin;
 use std::sync::Arc;
 
@@ -153,17 +154,18 @@ impl<T> BlobStorage for PinnedRepository<T> where T: BlobStorage + 'static {}
 impl<T> ManifestStorage for PinnedRepository<T> where T: ManifestStorage + 'static {}
 impl<T> LayerStorage for PinnedRepository<T> where T: LayerStorage + 'static {}
 impl<T> PlatformStorage for PinnedRepository<T> where T: PlatformStorage + 'static {}
-impl<T> Repository for PinnedRepository<T>
+impl<T> Address for PinnedRepository<T>
 where
     T: Repository + 'static,
 {
-    fn address(&self) -> url::Url {
-        let mut base = self.inner.address();
+    fn address(&self) -> Cow<'_, url::Url> {
+        let mut base = self.inner.address().into_owned();
         base.query_pairs_mut()
             .append_pair("when", &self.pin.to_string());
-        base
+        Cow::Owned(base)
     }
 }
+impl<T> Repository for PinnedRepository<T> where T: Repository + 'static {}
 
 impl<T> std::fmt::Debug for PinnedRepository<T>
 where

crates/spfs/src/storage/prelude.rs

@@ -4,6 +4,7 @@
 
 pub use super::config::{FromConfig, FromUrl};
 pub use super::{
+    Address,
     BlobStorage,
     LayerStorage,
     ManifestStorage,

crates/spfs/src/storage/proxy/repository.rs

@@ -335,8 +335,8 @@ impl BlobStorage for ProxyRepository {}
 impl ManifestStorage for ProxyRepository {}
 impl LayerStorage for ProxyRepository {}
 impl PlatformStorage for ProxyRepository {}
-impl Repository for ProxyRepository {
-    fn address(&self) -> url::Url {
+impl Address for ProxyRepository {
+    fn address(&self) -> Cow<'_, url::Url> {
         let config = Config {
             primary: self.primary.address().to_string(),
             secondary: self
@@ -345,6 +345,7 @@ impl Repository for ProxyRepository {
                 .map(|s| s.address().to_string())
                 .collect(),
         };
-        config.to_address().expect("config creates a valid url")
+        Cow::Owned(config.to_address().expect("config creates a valid url"))
     }
 }
+impl Repository for ProxyRepository {}

crates/spfs/src/storage/repository.rs

@@ -39,7 +39,8 @@ impl std::fmt::Display for Ref {
 /// Represents a storage location for spfs data.
 #[async_trait]
 pub trait Repository:
-    super::TagStorage
+    super::Address
+    + super::TagStorage
     + super::PayloadStorage
     + super::ManifestStorage
     + super::BlobStorage
@@ -51,9 +52,6 @@ pub trait Repository:
     + Send
     + Sync
 {
-    /// Return the address of this repository.
-    fn address(&self) -> url::Url;
-
     /// Return true if this repository contains the given reference.
     async fn has_ref(&self, reference: &str) -> bool {
         self.read_ref(reference).await.is_ok()
@@ -115,10 +113,6 @@ pub trait Repository:
 
 #[async_trait::async_trait]
 impl<T: Repository> Repository for &T {
-    fn address(&self) -> url::Url {
-        Repository::address(&**self)
-    }
-
     async fn has_ref(&self, reference: &str) -> bool {
         (**self).has_ref(reference).await
     }

crates/spfs/src/storage/rpc/repository.rs

@@ -2,6 +2,8 @@
 // SPDX-License-Identifier: Apache-2.0
 // https://github.com/spkenv/spk
 
+use std::borrow::Cow;
+
 use storage::FromUrl;
 
 use crate::config::ToAddress;
@@ -168,8 +170,10 @@ impl RpcRepository {
     }
 }
 
-impl storage::Repository for RpcRepository {
-    fn address(&self) -> url::Url {
-        self.address.clone()
+impl storage::Address for RpcRepository {
+    fn address(&self) -> Cow<'_, url::Url> {
+        Cow::Borrowed(&self.address)
     }
 }
+
+impl storage::Repository for RpcRepository {}

crates/spfs/src/storage/tar/repository.rs

@@ -410,8 +410,11 @@ impl BlobStorage for TarRepository {}
 impl ManifestStorage for TarRepository {}
 impl LayerStorage for TarRepository {}
 impl PlatformStorage for TarRepository {}
-impl Repository for TarRepository {
-    fn address(&self) -> url::Url {
-        url::Url::from_file_path(&self.repo_dir).expect("unexpected failure creating url")
+impl Address for TarRepository {
+    fn address(&self) -> Cow<'_, url::Url> {
+        Cow::Owned(
+            url::Url::from_file_path(&self.repo_dir).expect("unexpected failure creating url"),
+        )
     }
 }
+impl Repository for TarRepository {}

crates/spk-storage/src/storage/spfs.rs

@@ -159,7 +159,7 @@ where
         name_and_repo: NameAndRepositoryWithTagStrategy<S, T, TagStrategy>,
     ) -> Result<Self> {
         let inner = name_and_repo.repo.into();
-        let address = inner.address();
+        let address = inner.address().into_owned();
         Ok(Self {
             caches: CachesForAddress::new(&address),
             address,
@@ -175,7 +175,7 @@ where
 impl<S> SpfsRepository<S> {
     pub async fn new(name: &str, address: &str) -> Result<Self> {
         let inner = spfs::open_repository(address).await?;
-        let address = inner.address();
+        let address = inner.address().into_owned();
         Ok(Self {
             caches: CachesForAddress::new(&address),
             address,
@@ -1313,7 +1313,7 @@ pub async fn local_repository() -> Result<SpfsRepository<NormalizedTagStrategy>>
     let config = spfs::get_config()?;
     let repo = config.get_local_repository().await?;
     let inner: spfs::prelude::RepositoryHandle = repo.into();
-    let address = inner.address();
+    let address = inner.address().into_owned();
     Ok(SpfsRepository {
         caches: CachesForAddress::new(&address),
         address,
@@ -1333,7 +1333,7 @@ pub async fn remote_repository<S: AsRef<str>, TagStrategy>(
 ) -> Result<SpfsRepository<TagStrategy>> {
     let config = spfs::get_config()?;
     let inner = config.get_remote(&name).await?;
-    let address = inner.address();
+    let address = inner.address().into_owned();
     Ok(SpfsRepository {
         caches: CachesForAddress::new(&address),
         address,