From e40fe86ff8ea92dde1e53394d16954b833f10195 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Fri, 3 Oct 2025 15:52:25 +0100 Subject: [PATCH] fetch: use `AsRef` Allow the fetch interface to accept anything that implements `AsRef`. This allows flexibility in the types that the `Handle` can accept. This change is motivated by wanting to introduce a type that is a temporary repository that wraps a `Repository`. --- crates/radicle-fetch/src/handle.rs | 79 ++++++++++++++----------- crates/radicle-fetch/src/lib.rs | 13 ++-- crates/radicle-fetch/src/sigrefs.rs | 19 ++++-- crates/radicle-fetch/src/state.rs | 40 +++++++++---- crates/radicle-node/src/worker/fetch.rs | 4 +- crates/radicle/src/storage/git.rs | 6 ++ 6 files changed, 101 insertions(+), 60 deletions(-) diff --git a/crates/radicle-fetch/src/handle.rs b/crates/radicle-fetch/src/handle.rs index 572c3719..a252fc75 100644 --- a/crates/radicle-fetch/src/handle.rs +++ b/crates/radicle-fetch/src/handle.rs @@ -12,9 +12,9 @@ use crate::policy::{Allowed, BlockList}; use crate::transport::{ConnectionStream, Transport}; /// The handle used for pulling or cloning changes from a remote peer. -pub struct Handle { +pub struct Handle { pub(crate) local: PublicKey, - repo: Repository, + repo: R, pub(crate) allowed: Allowed, pub(crate) transport: Transport, /// The set of keys we will ignore when fetching from a @@ -29,39 +29,11 @@ pub struct Handle { pub(crate) interrupt: Arc, } -impl Handle { - pub fn new( - local: PublicKey, - repo: Repository, - follow: Allowed, - blocked: BlockList, - connection: S, - ) -> Result - where - S: ConnectionStream, - { - let git_dir = repo.backend.path().to_path_buf(); - let transport = Transport::new(git_dir, BString::from(repo.id.canonical()), connection); - - Ok(Self { - local, - repo, - allowed: follow, - transport, - blocked, - interrupt: Arc::new(AtomicBool::new(false)), - }) - } - +impl Handle { pub fn is_blocked(&self, key: &PublicKey) -> bool { self.blocked.is_blocked(key) } - #[inline] - pub fn repository(&self) -> &Repository { - &self.repo - } - #[inline] pub fn local(&self) -> &PublicKey { &self.local @@ -71,15 +43,52 @@ impl Handle { self.interrupt.store(true, atomic::Ordering::Relaxed); } - pub fn verified(&self, head: Oid) -> Result { - Ok(self.repo.identity_doc_at(head)?.doc) - } - pub fn allowed(&self) -> Allowed { self.allowed.clone() } } +impl Handle +where + R: AsRef, +{ + pub fn new( + local: PublicKey, + repo: R, + follow: Allowed, + blocked: BlockList, + connection: S, + ) -> Result + where + S: ConnectionStream, + { + let git_dir = repo.as_ref().backend.path().to_path_buf(); + let transport = Transport::new( + git_dir, + BString::from(repo.as_ref().id.canonical()), + connection, + ); + + Ok(Self { + local, + repo, + allowed: follow, + transport, + blocked, + interrupt: Arc::new(AtomicBool::new(false)), + }) + } + + #[inline] + pub fn repository(&self) -> &Repository { + self.repo.as_ref() + } + + pub fn verified(&self, head: Oid) -> Result { + Ok(self.repository().identity_doc_at(head)?.doc) + } +} + pub mod error { use radicle::node::policy; use radicle::prelude::RepoId; diff --git a/crates/radicle-fetch/src/lib.rs b/crates/radicle-fetch/src/lib.rs index 9ca05e0c..cd1c2007 100644 --- a/crates/radicle-fetch/src/lib.rs +++ b/crates/radicle-fetch/src/lib.rs @@ -16,6 +16,7 @@ use gix_protocol::handshake; pub use gix_protocol::{transport::bstr::ByteSlice, RemoteProgress}; pub use handle::Handle; pub use policy::{Allowed, BlockList, Scope}; +use radicle::storage::git::Repository; pub use state::{FetchLimit, FetchResult}; pub use transport::Transport; @@ -47,13 +48,14 @@ pub enum Error { /// It is expected that the local peer has a copy of the repository /// and is pulling new changes. If the repository does not exist, then /// [`clone`] should be used. -pub fn pull( - handle: &mut Handle, +pub fn pull( + handle: &mut Handle, limit: FetchLimit, remote: PublicKey, refs_at: Option>, ) -> Result where + R: AsRef, S: transport::ConnectionStream, { let start = Instant::now(); @@ -83,12 +85,13 @@ where /// /// It is expected that the local peer has an empty repository which /// they want to populate with the `remote`'s view of the project. -pub fn clone( - handle: &mut Handle, +pub fn clone( + handle: &mut Handle, limit: FetchLimit, remote: PublicKey, ) -> Result where + R: AsRef, S: transport::ConnectionStream, { let start = Instant::now(); @@ -120,7 +123,7 @@ where result } -fn perform_handshake(handle: &mut Handle) -> Result +fn perform_handshake(handle: &mut Handle) -> Result where S: transport::ConnectionStream, { diff --git a/crates/radicle-fetch/src/sigrefs.rs b/crates/radicle-fetch/src/sigrefs.rs index b89f4821..5f353435 100644 --- a/crates/radicle-fetch/src/sigrefs.rs +++ b/crates/radicle-fetch/src/sigrefs.rs @@ -1,6 +1,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::ops::{Deref, Not as _}; +use radicle::storage::git::Repository; pub use radicle::storage::refs::SignedRefsAt; pub use radicle::storage::{git::Validation, Validations}; use radicle::{crypto::PublicKey, storage::ValidateRepository}; @@ -53,10 +54,13 @@ impl DelegateStatus { /// Construct a `DelegateStatus` with [`SignedRefsAt`] signed reference /// data, if it can be found in `repo`. - pub fn load( + pub fn load( self, - cached: &Cached, - ) -> Result>, radicle::storage::refs::Error> { + cached: &Cached, + ) -> Result>, radicle::storage::refs::Error> + where + R: AsRef, + { let remote = *self.remote(); self.traverse(|_| cached.load(&remote)) } @@ -102,10 +106,13 @@ impl RemoteRefs { /// /// If the sigrefs are missing for a given remote, regardless of delegate /// status, then that remote is filtered out. - pub(crate) fn load<'a, S>( - cached: &Cached, + pub(crate) fn load<'a, R, S>( + cached: &Cached, remotes: impl Iterator, - ) -> Result { + ) -> Result + where + R: AsRef, + { remotes .filter_map(|id| match cached.load(id) { Ok(None) => None, diff --git a/crates/radicle-fetch/src/state.rs b/crates/radicle-fetch/src/state.rs index b3232939..e8c4c948 100644 --- a/crates/radicle-fetch/src/state.rs +++ b/crates/radicle-fetch/src/state.rs @@ -8,6 +8,7 @@ use radicle::identity::{Did, Doc, DocError}; use radicle::prelude::Verified; use radicle::storage; +use radicle::storage::git::Repository; use radicle::storage::refs::RefsAt; use radicle::storage::{ git::Validation, Remote, RemoteId, RemoteRepository, Remotes, ValidateRepository, Validations, @@ -196,7 +197,10 @@ impl FetchState { ap } - pub(crate) fn as_cached<'a, S>(&'a mut self, handle: &'a mut Handle) -> Cached<'a, S> { + pub(crate) fn as_cached<'a, R, S>( + &'a mut self, + handle: &'a mut Handle, + ) -> Cached<'a, R, S> { Cached { handle, state: self, @@ -207,13 +211,14 @@ impl FetchState { impl FetchState { /// Perform the ls-refs and fetch for the given `step`. The result /// of these processes is kept track of in the internal state. - pub(super) fn run_stage( + pub(super) fn run_stage( &mut self, - handle: &mut Handle, + handle: &mut Handle, handshake: &handshake::Outcome, step: &F, ) -> Result, error::Step> where + R: AsRef, S: transport::ConnectionStream, F: ProtocolStage, { @@ -280,9 +285,9 @@ impl FetchState { /// The resulting [`sigrefs::RemoteRefs`] will be the set of /// `rad/sigrefs` of the fetched remotes. #[allow(clippy::too_many_arguments)] - fn run_special_refs( + fn run_special_refs( &mut self, - handle: &mut Handle, + handle: &mut Handle, handshake: &handshake::Outcome, delegates: BTreeSet, threshold: usize, @@ -291,6 +296,7 @@ impl FetchState { refs_at: Option>, ) -> Result where + R: AsRef, S: transport::ConnectionStream, { match refs_at { @@ -348,15 +354,16 @@ impl FetchState { /// of updating tips. /// 7. Apply the valid tips, iff no delegates failed validation. /// 8. Signal to the other side that the process has completed. - pub(super) fn run( + pub(super) fn run( mut self, - handle: &mut Handle, + handle: &mut Handle, handshake: &handshake::Outcome, limit: FetchLimit, remote: PublicKey, refs_at: Option>, ) -> Result where + R: AsRef, S: transport::ConnectionStream, { let start = Instant::now(); @@ -598,12 +605,15 @@ impl FetchState { /// A cached version of [`Handle`] by using the underlying /// [`FetchState`]'s data for performing lookups. -pub(crate) struct Cached<'a, S> { - handle: &'a mut Handle, +pub(crate) struct Cached<'a, R, S> { + handle: &'a mut Handle, state: &'a mut FetchState, } -impl Cached<'_, S> { +impl Cached<'_, R, S> +where + R: AsRef, +{ /// Resolves `refname` to its [`ObjectId`] by first looking at the /// [`FetchState`] and falling back to the [`Handle::refdb`]. pub fn refname_to_id<'b, N>( @@ -651,7 +661,10 @@ impl Cached<'_, S> { } } -impl RemoteRepository for Cached<'_, S> { +impl RemoteRepository for Cached<'_, R, S> +where + R: AsRef, +{ fn remote(&self, remote: &RemoteId) -> Result { // N.b. this is unused so we just delegate to the underlying // repository for a correct implementation. @@ -671,7 +684,10 @@ impl RemoteRepository for Cached<'_, S> { } } -impl ValidateRepository for Cached<'_, S> { +impl ValidateRepository for Cached<'_, R, S> +where + R: AsRef, +{ // N.b. we don't verify the `rad/id` of each remote since they may // not have a reference to the COB if they have not interacted // with it. diff --git a/crates/radicle-node/src/worker/fetch.rs b/crates/radicle-node/src/worker/fetch.rs index b5302060..159e8afa 100644 --- a/crates/radicle-node/src/worker/fetch.rs +++ b/crates/radicle-node/src/worker/fetch.rs @@ -27,11 +27,11 @@ use super::channels::ChannelsFlush; pub enum Handle { Clone { - handle: radicle_fetch::Handle, + handle: radicle_fetch::Handle, tmp: tempfile::TempDir, }, Pull { - handle: radicle_fetch::Handle, + handle: radicle_fetch::Handle, notifications: node::notifications::StoreWriter, }, } diff --git a/crates/radicle/src/storage/git.rs b/crates/radicle/src/storage/git.rs index 4bc313d1..cfea5837 100644 --- a/crates/radicle/src/storage/git.rs +++ b/crates/radicle/src/storage/git.rs @@ -285,6 +285,12 @@ pub struct Repository { pub backend: git2::Repository, } +impl AsRef for Repository { + fn as_ref(&self) -> &Repository { + self + } +} + impl git::canonical::effects::Ancestry for Repository { fn graph_ahead_behind( &self,