From 3561a233d7904da29c4454fb74c98e382ea276c6 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Wed, 27 Nov 2024 10:45:55 -0800 Subject: [PATCH 1/2] Relocate RepositoryHandle to the handle.rs file This type's implementation was awkwardly split across mod.rs and handle.rs. Signed-off-by: J Robert Ray --- crates/spfs/src/storage/handle.rs | 111 +++++++++++++++++++++++++++- crates/spfs/src/storage/mod.rs | 115 +----------------------------- 2 files changed, 111 insertions(+), 115 deletions(-) diff --git a/crates/spfs/src/storage/handle.rs b/crates/spfs/src/storage/handle.rs index f588f1adb..0ce5d8a37 100644 --- a/crates/spfs/src/storage/handle.rs +++ b/crates/spfs/src/storage/handle.rs @@ -13,11 +13,120 @@ use spfs_encoding as encoding; use super::prelude::*; use super::tag::TagSpecAndTagStream; -use super::{RepositoryHandle, TagNamespace, TagNamespaceBuf, TagStorageMut}; +use super::{TagNamespace, TagNamespaceBuf, TagStorageMut}; use crate::graph::ObjectProto; use crate::tracking::{self, BlobRead}; use crate::{graph, Error, Result}; +#[derive(Debug)] +#[allow(clippy::large_enum_variant)] +pub enum RepositoryHandle { + FS(super::fs::FsRepository), + Tar(super::tar::TarRepository), + Rpc(super::rpc::RpcRepository), + FallbackProxy(Box), + Proxy(Box), + Pinned(Box>), +} + +impl RepositoryHandle { + /// Pin this repository to a specific date time, limiting + /// all results to that instant and before. + /// + /// If this repository is already pinned, this function + /// CAN move the pin farther into the future than it was + /// before. In other words, pinned repositories are never + /// nested via this function call. + pub fn into_pinned(self, time: DateTime) -> Self { + match self { + RepositoryHandle::Pinned(pinned) => Self::Pinned(Box::new( + super::pinned::PinnedRepository::new(Arc::clone(pinned.inner()), time), + )), + _ => Self::Pinned(Box::new(super::pinned::PinnedRepository::new( + Arc::new(self), + time, + ))), + } + } + + /// Make a pinned version of this repository at a specific date time, + /// limiting all results to that instant and before. + /// + /// If this repository is already pinned, this function + /// CAN move the pin farther into the future than it was + /// before. In other words, pinned repositories are never + /// nested via this function call. + pub fn to_pinned(self: &Arc, time: DateTime) -> Self { + match &**self { + RepositoryHandle::Pinned(pinned) => Self::Pinned(Box::new( + super::pinned::PinnedRepository::new(Arc::clone(pinned.inner()), time), + )), + _ => Self::Pinned(Box::new(super::pinned::PinnedRepository::new( + Arc::clone(self), + time, + ))), + } + } + + pub fn try_as_tag_mut(&mut self) -> Result<&mut dyn TagStorageMut> { + match self { + RepositoryHandle::FS(repo) => Ok(repo), + RepositoryHandle::Tar(repo) => Ok(repo), + RepositoryHandle::Rpc(repo) => Ok(repo), + RepositoryHandle::FallbackProxy(repo) => Ok(&mut **repo), + RepositoryHandle::Proxy(repo) => Ok(&mut **repo), + RepositoryHandle::Pinned(_) => Err(Error::RepositoryIsPinned), + } + } +} + +impl From for RepositoryHandle { + fn from(repo: super::fs::FsRepository) -> Self { + RepositoryHandle::FS(repo) + } +} + +impl From for RepositoryHandle { + fn from(repo: super::fs::OpenFsRepository) -> Self { + RepositoryHandle::FS(repo.into()) + } +} + +impl From> for RepositoryHandle { + fn from(repo: Arc) -> Self { + RepositoryHandle::FS(repo.into()) + } +} + +impl From for RepositoryHandle { + fn from(repo: super::tar::TarRepository) -> Self { + RepositoryHandle::Tar(repo) + } +} + +impl From for RepositoryHandle { + fn from(repo: super::rpc::RpcRepository) -> Self { + RepositoryHandle::Rpc(repo) + } +} + +impl From for RepositoryHandle { + fn from(repo: super::fallback::FallbackProxy) -> Self { + RepositoryHandle::FallbackProxy(Box::new(repo)) + } +} + +impl From for RepositoryHandle { + fn from(repo: super::proxy::ProxyRepository) -> Self { + RepositoryHandle::Proxy(Box::new(repo)) + } +} + +impl From>> for RepositoryHandle { + fn from(repo: Box>) -> Self { + RepositoryHandle::Pinned(repo) + } +} /// Runs a code block on each variant of the handle, /// easily allowing the use of storage code without using /// a dyn reference diff --git a/crates/spfs/src/storage/mod.rs b/crates/spfs/src/storage/mod.rs index 673ad6d35..c346aa4f1 100644 --- a/crates/spfs/src/storage/mod.rs +++ b/crates/spfs/src/storage/mod.rs @@ -23,12 +23,10 @@ pub mod proxy; pub mod rpc; pub mod tar; -use std::sync::Arc; - pub use address::Address; pub use blob::BlobStorage; -use chrono::{DateTime, Utc}; pub use error::OpenRepositoryError; +pub use handle::RepositoryHandle; pub use layer::LayerStorage; pub use manifest::ManifestStorage; pub use payload::PayloadStorage; @@ -39,114 +37,3 @@ pub use tag::{EntryType, TagStorage, TagStorageMut}; pub use tag_namespace::{TagNamespace, TagNamespaceBuf, TAG_NAMESPACE_MARKER}; pub use self::config::{FromConfig, FromUrl, OpenRepositoryResult}; -use crate::{Error, Result}; - -#[derive(Debug)] -#[allow(clippy::large_enum_variant)] -pub enum RepositoryHandle { - FS(fs::FsRepository), - Tar(tar::TarRepository), - Rpc(rpc::RpcRepository), - FallbackProxy(Box), - Proxy(Box), - Pinned(Box>), -} - -impl RepositoryHandle { - /// Pin this repository to a specific date time, limiting - /// all results to that instant and before. - /// - /// If this repository is already pinned, this function - /// CAN move the pin farther into the future than it was - /// before. In other words, pinned repositories are never - /// nested via this function call. - pub fn into_pinned(self, time: DateTime) -> Self { - match self { - RepositoryHandle::Pinned(pinned) => Self::Pinned(Box::new( - pinned::PinnedRepository::new(Arc::clone(pinned.inner()), time), - )), - _ => Self::Pinned(Box::new(pinned::PinnedRepository::new( - Arc::new(self), - time, - ))), - } - } - - /// Make a pinned version of this repository at a specific date time, - /// limiting all results to that instant and before. - /// - /// If this repository is already pinned, this function - /// CAN move the pin farther into the future than it was - /// before. In other words, pinned repositories are never - /// nested via this function call. - pub fn to_pinned(self: &Arc, time: DateTime) -> Self { - match &**self { - RepositoryHandle::Pinned(pinned) => Self::Pinned(Box::new( - pinned::PinnedRepository::new(Arc::clone(pinned.inner()), time), - )), - _ => Self::Pinned(Box::new(pinned::PinnedRepository::new( - Arc::clone(self), - time, - ))), - } - } - - pub fn try_as_tag_mut(&mut self) -> Result<&mut dyn TagStorageMut> { - match self { - RepositoryHandle::FS(repo) => Ok(repo), - RepositoryHandle::Tar(repo) => Ok(repo), - RepositoryHandle::Rpc(repo) => Ok(repo), - RepositoryHandle::FallbackProxy(repo) => Ok(&mut **repo), - RepositoryHandle::Proxy(repo) => Ok(&mut **repo), - RepositoryHandle::Pinned(_) => Err(Error::RepositoryIsPinned), - } - } -} - -impl From for RepositoryHandle { - fn from(repo: fs::FsRepository) -> Self { - RepositoryHandle::FS(repo) - } -} - -impl From for RepositoryHandle { - fn from(repo: fs::OpenFsRepository) -> Self { - RepositoryHandle::FS(repo.into()) - } -} - -impl From> for RepositoryHandle { - fn from(repo: Arc) -> Self { - RepositoryHandle::FS(repo.into()) - } -} - -impl From for RepositoryHandle { - fn from(repo: tar::TarRepository) -> Self { - RepositoryHandle::Tar(repo) - } -} - -impl From for RepositoryHandle { - fn from(repo: rpc::RpcRepository) -> Self { - RepositoryHandle::Rpc(repo) - } -} - -impl From for RepositoryHandle { - fn from(repo: fallback::FallbackProxy) -> Self { - RepositoryHandle::FallbackProxy(Box::new(repo)) - } -} - -impl From for RepositoryHandle { - fn from(repo: proxy::ProxyRepository) -> Self { - RepositoryHandle::Proxy(Box::new(repo)) - } -} - -impl From>> for RepositoryHandle { - fn from(repo: Box>) -> Self { - RepositoryHandle::Pinned(repo) - } -} From 9cf3488ee118f83fe599995e911dd228cd6262e5 Mon Sep 17 00:00:00 2001 From: J Robert Ray Date: Wed, 27 Nov 2024 12:35:10 -0800 Subject: [PATCH 2/2] Make Repository object safe This involves moving the method that has generics into a separate trait, and then by extension traits that needed that method into their own separate traits. Now it is possible to use `dyn Repository` which may become useful. Signed-off-by: J Robert Ray --- crates/spfs/src/graph/database.rs | 20 ++- crates/spfs/src/graph/mod.rs | 1 + crates/spfs/src/resolve.rs | 2 +- crates/spfs/src/runtime/storage_test.rs | 2 +- crates/spfs/src/storage/blob.rs | 8 +- .../spfs/src/storage/fallback/repository.rs | 13 +- crates/spfs/src/storage/fs/database.rs | 150 +++++++++--------- crates/spfs/src/storage/fs/renderer_test.rs | 2 +- crates/spfs/src/storage/handle.rs | 23 ++- crates/spfs/src/storage/layer.rs | 8 +- crates/spfs/src/storage/mod.rs | 8 +- crates/spfs/src/storage/payload.rs | 2 +- crates/spfs/src/storage/pinned/repository.rs | 22 ++- crates/spfs/src/storage/platform.rs | 8 +- crates/spfs/src/storage/prelude.rs | 6 +- crates/spfs/src/storage/proxy/repository.rs | 13 +- crates/spfs/src/storage/repository.rs | 26 +-- crates/spfs/src/storage/rpc/database.rs | 29 ++-- crates/spfs/src/storage/tar/repository.rs | 15 +- crates/spk-storage/src/storage/spfs.rs | 4 +- 20 files changed, 214 insertions(+), 148 deletions(-) diff --git a/crates/spfs/src/graph/database.rs b/crates/spfs/src/graph/database.rs index cadad604c..cbab5b625 100644 --- a/crates/spfs/src/graph/database.rs +++ b/crates/spfs/src/graph/database.rs @@ -250,9 +250,6 @@ impl DatabaseView for &T { #[async_trait::async_trait] pub trait Database: DatabaseView { - /// Write an object to the database, for later retrieval. - async fn write_object(&self, obj: &FlatObject) -> Result<()>; - /// Remove an object from the database. async fn remove_object(&self, digest: encoding::Digest) -> Result<()>; @@ -269,10 +266,6 @@ pub trait Database: DatabaseView { #[async_trait::async_trait] impl Database for &T { - async fn write_object(&self, obj: &FlatObject) -> Result<()> { - Database::write_object(&**self, obj).await - } - async fn remove_object(&self, digest: encoding::Digest) -> Result<()> { Database::remove_object(&**self, digest).await } @@ -285,3 +278,16 @@ impl Database for &T { Database::remove_object_if_older_than(&**self, older_than, digest).await } } + +#[async_trait::async_trait] +pub trait DatabaseExt: Send + Sync { + /// Write an object to the database, for later retrieval. + async fn write_object(&self, obj: &FlatObject) -> Result<()>; +} + +#[async_trait::async_trait] +impl DatabaseExt for &T { + async fn write_object(&self, obj: &FlatObject) -> Result<()> { + DatabaseExt::write_object(&**self, obj).await + } +} diff --git a/crates/spfs/src/graph/mod.rs b/crates/spfs/src/graph/mod.rs index 8a26800e6..ee64fb823 100644 --- a/crates/spfs/src/graph/mod.rs +++ b/crates/spfs/src/graph/mod.rs @@ -27,6 +27,7 @@ pub use annotation::{ pub use blob::Blob; pub use database::{ Database, + DatabaseExt, DatabaseIterator, DatabaseView, DatabaseWalker, diff --git a/crates/spfs/src/resolve.rs b/crates/spfs/src/resolve.rs index 011a6b794..92eebba70 100644 --- a/crates/spfs/src/resolve.rs +++ b/crates/spfs/src/resolve.rs @@ -211,7 +211,7 @@ pub(crate) async fn resolve_overlay_dirs( skip_runtime_save: bool, ) -> Result> where - R: Repository + ManifestRenderPath, + R: Repository + graph::DatabaseExt + ManifestRenderPath, { enum ResolvedManifest { Existing { diff --git a/crates/spfs/src/runtime/storage_test.rs b/crates/spfs/src/runtime/storage_test.rs index 7571808a1..2693950fe 100644 --- a/crates/spfs/src/runtime/storage_test.rs +++ b/crates/spfs/src/runtime/storage_test.rs @@ -13,7 +13,7 @@ use crate::fixtures::*; use crate::graph::object::{DigestStrategy, EncodingFormat}; use crate::graph::{AnnotationValue, Layer, Platform}; use crate::runtime::{BindMount, KeyValuePair, LiveLayer, LiveLayerContents, SpecApiVersion}; -use crate::storage::prelude::Database; +use crate::storage::prelude::DatabaseExt; use crate::{encoding, Config}; #[rstest] diff --git a/crates/spfs/src/storage/blob.rs b/crates/spfs/src/storage/blob.rs index 6dc5722ca..7a9a2ad58 100644 --- a/crates/spfs/src/storage/blob.rs +++ b/crates/spfs/src/storage/blob.rs @@ -33,7 +33,13 @@ pub trait BlobStorage: graph::Database + Sync + Send { }), } } +} + +/// Blanket implementation. +impl BlobStorage for T where T: graph::Database + Sync + Send {} +#[async_trait::async_trait] +pub trait BlobStorageExt: graph::DatabaseExt { /// Store the given blob async fn write_blob(&self, blob: graph::Blob) -> Result<()> { self.write_object(&blob).await @@ -41,4 +47,4 @@ pub trait BlobStorage: graph::Database + Sync + Send { } /// Blanket implementation. -impl BlobStorage for T where T: graph::Database + Sync + Send {} +impl BlobStorageExt for T where T: graph::DatabaseExt + Sync + Send {} diff --git a/crates/spfs/src/storage/fallback/repository.rs b/crates/spfs/src/storage/fallback/repository.rs index 7413b8375..8523161de 100644 --- a/crates/spfs/src/storage/fallback/repository.rs +++ b/crates/spfs/src/storage/fallback/repository.rs @@ -157,11 +157,6 @@ impl graph::DatabaseView for FallbackProxy { #[async_trait::async_trait] impl graph::Database for FallbackProxy { - async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { - self.primary.write_object(obj).await?; - Ok(()) - } - async fn remove_object(&self, digest: encoding::Digest) -> Result<()> { self.primary.remove_object(digest).await?; Ok(()) @@ -179,6 +174,14 @@ impl graph::Database for FallbackProxy { } } +#[async_trait::async_trait] +impl graph::DatabaseExt for FallbackProxy { + async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { + self.primary.write_object(obj).await?; + Ok(()) + } +} + #[async_trait::async_trait] impl PayloadStorage for FallbackProxy { async fn has_payload(&self, digest: encoding::Digest) -> bool { diff --git a/crates/spfs/src/storage/fs/database.rs b/crates/spfs/src/storage/fs/database.rs index 36de80eaa..b45f86f2f 100644 --- a/crates/spfs/src/storage/fs/database.rs +++ b/crates/spfs/src/storage/fs/database.rs @@ -56,10 +56,6 @@ impl DatabaseView for super::FsRepository { #[async_trait::async_trait] impl graph::Database for super::FsRepository { - async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { - self.opened().await?.write_object(obj).await - } - async fn remove_object(&self, digest: encoding::Digest) -> crate::Result<()> { self.opened().await?.remove_object(digest).await } @@ -76,6 +72,13 @@ impl graph::Database for super::FsRepository { } } +#[async_trait::async_trait] +impl graph::DatabaseExt for super::FsRepository { + async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { + self.opened().await?.write_object(obj).await + } +} + #[async_trait::async_trait] impl DatabaseView for super::OpenFsRepository { async fn has_object(&self, digest: encoding::Digest) -> bool { @@ -127,6 +130,77 @@ impl DatabaseView for super::OpenFsRepository { #[async_trait::async_trait] impl graph::Database for super::OpenFsRepository { + async fn remove_object(&self, digest: encoding::Digest) -> crate::Result<()> { + let filepath = self.objects.build_digest_path(&digest); + + // this might fail but we don't consider that fatal just yet + #[cfg(unix)] + let _ = tokio::fs::set_permissions(&filepath, std::fs::Permissions::from_mode(0o777)).await; + + if let Err(err) = tokio::fs::remove_file(&filepath).await { + return match err.kind() { + std::io::ErrorKind::NotFound => Ok(()), + _ => Err(Error::StorageWriteError( + "remove_file on object file in remove_object", + filepath, + err, + )), + }; + } + tracing::trace!(%digest, "removed object from db"); + Ok(()) + } + + async fn remove_object_if_older_than( + &self, + older_than: DateTime, + digest: encoding::Digest, + ) -> crate::Result { + let filepath = self.objects.build_digest_path(&digest); + + // this might fail but we don't consider that fatal just yet + #[cfg(unix)] + let _ = tokio::fs::set_permissions(&filepath, std::fs::Permissions::from_mode(0o777)).await; + + let metadata = tokio::fs::symlink_metadata(&filepath) + .await + .map_err(|err| match err.kind() { + std::io::ErrorKind::NotFound => Error::UnknownObject(digest), + _ => Error::StorageReadError( + "symlink_metadata on digest path", + filepath.clone(), + err, + ), + })?; + + let mtime = metadata.modified().map_err(|err| { + Error::StorageReadError( + "modified on symlink metadata of digest path", + filepath.clone(), + err, + ) + })?; + + if DateTime::::from(mtime) >= older_than { + return Ok(false); + } + + if let Err(err) = tokio::fs::remove_file(&filepath).await { + return match err.kind() { + std::io::ErrorKind::NotFound => Ok(true), + _ => Err(Error::StorageWriteError( + "remove_file on object file in remove_object_if_older_than", + filepath, + err, + )), + }; + } + Ok(true) + } +} + +#[async_trait::async_trait] +impl graph::DatabaseExt for super::OpenFsRepository { async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { let digest = obj.digest()?; let filepath = self.objects.build_digest_path(&digest); @@ -207,72 +281,4 @@ impl graph::Database for super::OpenFsRepository { } } } - - async fn remove_object(&self, digest: encoding::Digest) -> crate::Result<()> { - let filepath = self.objects.build_digest_path(&digest); - - // this might fail but we don't consider that fatal just yet - #[cfg(unix)] - let _ = tokio::fs::set_permissions(&filepath, std::fs::Permissions::from_mode(0o777)).await; - - if let Err(err) = tokio::fs::remove_file(&filepath).await { - return match err.kind() { - std::io::ErrorKind::NotFound => Ok(()), - _ => Err(Error::StorageWriteError( - "remove_file on object file in remove_object", - filepath, - err, - )), - }; - } - tracing::trace!(%digest, "removed object from db"); - Ok(()) - } - - async fn remove_object_if_older_than( - &self, - older_than: DateTime, - digest: encoding::Digest, - ) -> crate::Result { - let filepath = self.objects.build_digest_path(&digest); - - // this might fail but we don't consider that fatal just yet - #[cfg(unix)] - let _ = tokio::fs::set_permissions(&filepath, std::fs::Permissions::from_mode(0o777)).await; - - let metadata = tokio::fs::symlink_metadata(&filepath) - .await - .map_err(|err| match err.kind() { - std::io::ErrorKind::NotFound => Error::UnknownObject(digest), - _ => Error::StorageReadError( - "symlink_metadata on digest path", - filepath.clone(), - err, - ), - })?; - - let mtime = metadata.modified().map_err(|err| { - Error::StorageReadError( - "modified on symlink metadata of digest path", - filepath.clone(), - err, - ) - })?; - - if DateTime::::from(mtime) >= older_than { - return Ok(false); - } - - if let Err(err) = tokio::fs::remove_file(&filepath).await { - return match err.kind() { - std::io::ErrorKind::NotFound => Ok(true), - _ => Err(Error::StorageWriteError( - "remove_file on object file in remove_object_if_older_than", - filepath, - err, - )), - }; - } - Ok(true) - } } diff --git a/crates/spfs/src/storage/fs/renderer_test.rs b/crates/spfs/src/storage/fs/renderer_test.rs index 0691e35ba..446ed5eca 100644 --- a/crates/spfs/src/storage/fs/renderer_test.rs +++ b/crates/spfs/src/storage/fs/renderer_test.rs @@ -11,7 +11,7 @@ use crate::encoding::prelude::*; use crate::fixtures::*; use crate::graph::object::{DigestStrategy, EncodingFormat}; use crate::storage::fs::{FsRepository, OpenFsRepository}; -use crate::storage::{Repository, RepositoryHandle}; +use crate::storage::{RepositoryExt, RepositoryHandle}; use crate::{tracking, Config}; #[rstest( diff --git a/crates/spfs/src/storage/handle.rs b/crates/spfs/src/storage/handle.rs index 0ce5d8a37..597ba0b4a 100644 --- a/crates/spfs/src/storage/handle.rs +++ b/crates/spfs/src/storage/handle.rs @@ -127,6 +127,7 @@ impl From>> for Repository RepositoryHandle::Pinned(repo) } } + /// Runs a code block on each variant of the handle, /// easily allowing the use of storage code without using /// a dyn reference @@ -309,10 +310,6 @@ impl DatabaseView for RepositoryHandle { #[async_trait::async_trait] impl Database for RepositoryHandle { - async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { - each_variant!(self, repo, { repo.write_object(obj).await }) - } - async fn remove_object(&self, digest: encoding::Digest) -> Result<()> { each_variant!(self, repo, { repo.remove_object(digest).await }) } @@ -328,6 +325,13 @@ impl Database for RepositoryHandle { } } +#[async_trait::async_trait] +impl DatabaseExt for RepositoryHandle { + async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { + each_variant!(self, repo, { repo.write_object(obj).await }) + } +} + impl Address for Arc { /// Return the address of this repository. fn address(&self) -> Cow<'_, url::Url> { @@ -481,10 +485,6 @@ impl DatabaseView for Arc { #[async_trait::async_trait] impl Database for Arc { - async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { - each_variant!(&**self, repo, { repo.write_object(obj).await }) - } - async fn remove_object(&self, digest: encoding::Digest) -> Result<()> { each_variant!(&**self, repo, { repo.remove_object(digest).await }) } @@ -499,3 +499,10 @@ impl Database for Arc { }) } } + +#[async_trait::async_trait] +impl DatabaseExt for Arc { + async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { + each_variant!(&**self, repo, { repo.write_object(obj).await }) + } +} diff --git a/crates/spfs/src/storage/layer.rs b/crates/spfs/src/storage/layer.rs index 8103c5177..d4082f8d3 100644 --- a/crates/spfs/src/storage/layer.rs +++ b/crates/spfs/src/storage/layer.rs @@ -38,7 +38,13 @@ pub trait LayerStorage: graph::Database + Sync + Send { }), } } +} + +/// Blanket implementation. +impl LayerStorage for T where T: graph::Database + Sync + Send {} +#[async_trait::async_trait] +pub trait LayerStorageExt: graph::DatabaseExt { /// Create and storage a new layer for the given layer. async fn create_layer(&self, manifest: &graph::Manifest) -> Result { let layer = graph::Layer::new(manifest.digest()?); @@ -58,4 +64,4 @@ pub trait LayerStorage: graph::Database + Sync + Send { } /// Blanket implementation. -impl LayerStorage for T where T: graph::Database + Sync + Send {} +impl LayerStorageExt for T where T: graph::DatabaseExt + Sync + Send {} diff --git a/crates/spfs/src/storage/mod.rs b/crates/spfs/src/storage/mod.rs index c346aa4f1..aaeeb3e25 100644 --- a/crates/spfs/src/storage/mod.rs +++ b/crates/spfs/src/storage/mod.rs @@ -24,15 +24,15 @@ pub mod rpc; pub mod tar; pub use address::Address; -pub use blob::BlobStorage; +pub use blob::{BlobStorage, BlobStorageExt}; pub use error::OpenRepositoryError; pub use handle::RepositoryHandle; -pub use layer::LayerStorage; +pub use layer::{LayerStorage, LayerStorageExt}; pub use manifest::ManifestStorage; pub use payload::PayloadStorage; -pub use platform::PlatformStorage; +pub use platform::{PlatformStorage, PlatformStorageExt}; pub use proxy::{Config, ProxyRepository}; -pub use repository::{LocalRepository, Repository}; +pub use repository::{LocalRepository, Repository, RepositoryExt}; pub use tag::{EntryType, TagStorage, TagStorageMut}; pub use tag_namespace::{TagNamespace, TagNamespaceBuf, TAG_NAMESPACE_MARKER}; diff --git a/crates/spfs/src/storage/payload.rs b/crates/spfs/src/storage/payload.rs index f6f726605..1e32b5b89 100644 --- a/crates/spfs/src/storage/payload.rs +++ b/crates/spfs/src/storage/payload.rs @@ -28,7 +28,7 @@ pub trait PayloadStorage: Sync + Send { /// /// It is unsafe to write payload data without also creating a blob /// to track that payload in the database. Usually, its better to - /// call [`super::Repository::commit_blob`] instead. + /// call [`super::RepositoryExt::commit_blob`] instead. async unsafe fn write_data( &self, reader: Pin>, diff --git a/crates/spfs/src/storage/pinned/repository.rs b/crates/spfs/src/storage/pinned/repository.rs index 69747dac1..a7d390f0c 100644 --- a/crates/spfs/src/storage/pinned/repository.rs +++ b/crates/spfs/src/storage/pinned/repository.rs @@ -91,14 +91,6 @@ impl graph::Database for super::PinnedRepository where T: graph::Database + 'static, { - async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { - // objects are stored by digest, not time, and so can still - // be safely written to a past repository view. In practice, - // this allows some recovery and sync operations to still function - // on pinned repositories - self.inner.write_object(obj).await - } - async fn remove_object(&self, _digest: encoding::Digest) -> crate::Result<()> { Err(Error::RepositoryIsPinned) } @@ -112,6 +104,20 @@ where } } +#[async_trait::async_trait] +impl graph::DatabaseExt for super::PinnedRepository +where + T: graph::DatabaseExt + 'static, +{ + async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { + // objects are stored by digest, not time, and so can still + // be safely written to a past repository view. In practice, + // this allows some recovery and sync operations to still function + // on pinned repositories + self.inner.write_object(obj).await + } +} + #[async_trait::async_trait] impl PayloadStorage for PinnedRepository where diff --git a/crates/spfs/src/storage/platform.rs b/crates/spfs/src/storage/platform.rs index 06d5eb9a0..f2eee744e 100644 --- a/crates/spfs/src/storage/platform.rs +++ b/crates/spfs/src/storage/platform.rs @@ -37,7 +37,13 @@ pub trait PlatformStorage: graph::Database + Sync + Send { }), } } +} + +/// Blanket implementation. +impl PlatformStorage for T where T: graph::Database + Sync + Send {} +#[async_trait::async_trait] +pub trait PlatformStorageExt: graph::DatabaseExt { /// Create and storage a new platform for the given platform. /// Layers are ordered bottom to top. async fn create_platform(&self, layers: graph::Stack) -> Result { @@ -48,4 +54,4 @@ pub trait PlatformStorage: graph::Database + Sync + Send { } /// Blanket implementation. -impl PlatformStorage for T where T: graph::Database + Sync + Send {} +impl PlatformStorageExt for T where T: graph::DatabaseExt + Sync + Send {} diff --git a/crates/spfs/src/storage/prelude.rs b/crates/spfs/src/storage/prelude.rs index 278f9a259..bfa572334 100644 --- a/crates/spfs/src/storage/prelude.rs +++ b/crates/spfs/src/storage/prelude.rs @@ -6,12 +6,16 @@ pub use super::config::{FromConfig, FromUrl}; pub use super::{ Address, BlobStorage, + BlobStorageExt, LayerStorage, + LayerStorageExt, ManifestStorage, PayloadStorage, PlatformStorage, + PlatformStorageExt, Repository, + RepositoryExt, RepositoryHandle, TagStorage, }; -pub use crate::graph::{Database, DatabaseView}; +pub use crate::graph::{Database, DatabaseExt, DatabaseView}; diff --git a/crates/spfs/src/storage/proxy/repository.rs b/crates/spfs/src/storage/proxy/repository.rs index e22d28a93..829fadb5b 100644 --- a/crates/spfs/src/storage/proxy/repository.rs +++ b/crates/spfs/src/storage/proxy/repository.rs @@ -165,11 +165,6 @@ impl graph::DatabaseView for ProxyRepository { #[async_trait::async_trait] impl graph::Database for ProxyRepository { - async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { - self.primary.write_object(obj).await?; - Ok(()) - } - async fn remove_object(&self, digest: encoding::Digest) -> Result<()> { self.primary.remove_object(digest).await?; Ok(()) @@ -187,6 +182,14 @@ impl graph::Database for ProxyRepository { } } +#[async_trait::async_trait] +impl graph::DatabaseExt for ProxyRepository { + async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { + self.primary.write_object(obj).await?; + Ok(()) + } +} + #[async_trait::async_trait] impl PayloadStorage for ProxyRepository { async fn has_payload(&self, digest: encoding::Digest) -> bool { diff --git a/crates/spfs/src/storage/repository.rs b/crates/spfs/src/storage/repository.rs index ea52d749c..34cffa626 100644 --- a/crates/spfs/src/storage/repository.rs +++ b/crates/spfs/src/storage/repository.rs @@ -99,16 +99,6 @@ pub trait Repository: } Ok(aliases) } - - /// Commit the data from 'reader' as a blob in this repository - async fn commit_blob(&self, reader: Pin>) -> Result { - // Safety: it is unsafe to write data without also creating a blob - // to track that payload, which is exactly what this function is doing - let (digest, size) = unsafe { self.write_data(reader).await? }; - let blob = graph::Blob::new(digest, size); - self.write_object(&blob).await?; - Ok(digest) - } } /// Blanket implementation. @@ -128,6 +118,22 @@ impl Repository for T where { } +#[async_trait] +pub trait RepositoryExt: super::PayloadStorage + graph::DatabaseExt { + /// Commit the data from 'reader' as a blob in this repository + async fn commit_blob(&self, reader: Pin>) -> Result { + // Safety: it is unsafe to write data without also creating a blob + // to track that payload, which is exactly what this function is doing + let (digest, size) = unsafe { self.write_data(reader).await? }; + let blob = graph::Blob::new(digest, size); + self.write_object(&blob).await?; + Ok(digest) + } +} + +/// Blanket implementation. +impl RepositoryExt for T where T: super::PayloadStorage + graph::DatabaseExt {} + /// Accessor methods for types only applicable to repositories that have /// payloads and renders, e.g., local repositories. pub trait LocalRepository { diff --git a/crates/spfs/src/storage/rpc/database.rs b/crates/spfs/src/storage/rpc/database.rs index c0afa685f..093ab865b 100644 --- a/crates/spfs/src/storage/rpc/database.rs +++ b/crates/spfs/src/storage/rpc/database.rs @@ -68,19 +68,6 @@ impl graph::DatabaseView for super::RpcRepository { #[async_trait::async_trait] impl graph::Database for super::RpcRepository { - async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { - let request = proto::WriteObjectRequest { - object: Some(obj.into()), - }; - self.db_client - .clone() - .write_object(request) - .await? - .into_inner() - .to_result()?; - Ok(()) - } - async fn remove_object(&self, digest: encoding::Digest) -> Result<()> { let request = proto::RemoveObjectRequest { digest: Some(digest.into()), @@ -112,3 +99,19 @@ impl graph::Database for super::RpcRepository { .to_result()?) } } + +#[async_trait::async_trait] +impl graph::DatabaseExt for super::RpcRepository { + async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { + let request = proto::WriteObjectRequest { + object: Some(obj.into()), + }; + self.db_client + .clone() + .write_object(request) + .await? + .into_inner() + .to_result()?; + Ok(()) + } +} diff --git a/crates/spfs/src/storage/tar/repository.rs b/crates/spfs/src/storage/tar/repository.rs index 14ec2396b..7a9233b1a 100644 --- a/crates/spfs/src/storage/tar/repository.rs +++ b/crates/spfs/src/storage/tar/repository.rs @@ -250,12 +250,6 @@ impl graph::DatabaseView for TarRepository { #[async_trait::async_trait] impl graph::Database for TarRepository { - async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { - self.repo.write_object(obj).await?; - self.up_to_date.store(false, Ordering::Release); - Ok(()) - } - async fn remove_object(&self, digest: encoding::Digest) -> Result<()> { self.repo.remove_object(digest).await?; self.up_to_date.store(false, Ordering::Release); @@ -278,6 +272,15 @@ impl graph::Database for TarRepository { } } +#[async_trait::async_trait] +impl graph::DatabaseExt for TarRepository { + async fn write_object(&self, obj: &graph::FlatObject) -> Result<()> { + self.repo.write_object(obj).await?; + self.up_to_date.store(false, Ordering::Release); + Ok(()) + } +} + #[async_trait::async_trait] impl PayloadStorage for TarRepository { async fn has_payload(&self, digest: encoding::Digest) -> bool { diff --git a/crates/spk-storage/src/storage/spfs.rs b/crates/spk-storage/src/storage/spfs.rs index fccdd5142..a9f144249 100644 --- a/crates/spk-storage/src/storage/spfs.rs +++ b/crates/spk-storage/src/storage/spfs.rs @@ -15,8 +15,8 @@ use itertools::Itertools; use once_cell::sync::Lazy; use relative_path::RelativePathBuf; use serde::{Deserialize, Serialize}; -use spfs::prelude::*; -use spfs::storage::{EntryType, Repository}; +use spfs::prelude::{RepositoryExt as SpfsRepositoryExt, *}; +use spfs::storage::EntryType; use spfs::tracking::{self, Tag, TagSpec}; use spk_schema::foundation::ident_build::{parse_build, Build}; use spk_schema::foundation::ident_component::Component;