Skip to content

Commit

Permalink
Move Repository::address to dedicated trait
Browse files Browse the repository at this point in the history
This makes it possible for other things to have a trait bound on
`Address` instead of all of `Repository`, which means other types can
implement `Address` without having to also implement all of
`Repository` (not shown in this commit).

Also change the method signature to return `Cow` since there are cases
where an existing `Url` value can be borrowed.

Signed-off-by: J Robert Ray <jrray@jrray.org>
  • Loading branch information
jrray committed Nov 27, 2024
1 parent 34f9a80 commit 80039ea
Show file tree
Hide file tree
Showing 14 changed files with 78 additions and 45 deletions.
2 changes: 1 addition & 1 deletion crates/spfs/src/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand Down
4 changes: 2 additions & 2 deletions crates/spfs/src/runtime/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand All @@ -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:?}");
Expand Down
17 changes: 17 additions & 0 deletions crates/spfs/src/storage/address.rs
Original file line number Diff line number Diff line change
@@ -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)
}
}
13 changes: 8 additions & 5 deletions crates/spfs/src/storage/fallback/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand Down
14 changes: 8 additions & 6 deletions crates/spfs/src/storage/fs/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
16 changes: 10 additions & 6 deletions crates/spfs/src/storage/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
}
Expand Down Expand Up @@ -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 })
Expand Down
2 changes: 2 additions & 0 deletions crates/spfs/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0
// https://github.com/spkenv/spk

mod address;
mod blob;
mod error;
mod layer;
Expand All @@ -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;
Expand Down
10 changes: 6 additions & 4 deletions crates/spfs/src/storage/pinned/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions crates/spfs/src/storage/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

pub use super::config::{FromConfig, FromUrl};
pub use super::{
Address,
BlobStorage,
LayerStorage,
ManifestStorage,
Expand Down
7 changes: 4 additions & 3 deletions crates/spfs/src/storage/proxy/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {}
10 changes: 2 additions & 8 deletions crates/spfs/src/storage/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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
}
Expand Down
10 changes: 7 additions & 3 deletions crates/spfs/src/storage/rpc/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {}
9 changes: 6 additions & 3 deletions crates/spfs/src/storage/tar/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
8 changes: 4 additions & 4 deletions crates/spk-storage/src/storage/spfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit 80039ea

Please sign in to comment.