From aa28567035c4b177096749bd875c917e31985e93 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Thu, 18 Sep 2025 15:38:33 +0200 Subject: [PATCH] radicle/crefs: Refactor `GetCanonicalRefs` There is only one type that can really construct canonical references, which is the the identity document. Remove the corresponding method from the trait and rename it to `GetRawCanonicalRefs` accordingly. --- crates/radicle-node/src/worker/fetch.rs | 14 +------- crates/radicle-remote-helper/src/push.rs | 10 ++---- crates/radicle/src/identity/crefs.rs | 26 ++------------ crates/radicle/src/identity/doc.rs | 42 ++++++++++------------- crates/radicle/src/identity/doc/update.rs | 9 ++--- crates/radicle/src/storage/git.rs | 9 ++--- 6 files changed, 30 insertions(+), 80 deletions(-) diff --git a/crates/radicle-node/src/worker/fetch.rs b/crates/radicle-node/src/worker/fetch.rs index 9bc3c6ef..f143d98d 100644 --- a/crates/radicle-node/src/worker/fetch.rs +++ b/crates/radicle-node/src/worker/fetch.rs @@ -1,6 +1,4 @@ use radicle::cob::store::access::ReadOnly; -use radicle::identity::CanonicalRefs; -use radicle::identity::doc::CanonicalRefsError; use radicle::storage::git::TempRepository; pub(crate) use radicle_protocol::worker::fetch::error; @@ -11,7 +9,6 @@ use localtime::LocalTime; use radicle::cob::TypedId; use radicle::crypto::PublicKey; -use radicle::identity::crefs::GetCanonicalRefs as _; use radicle::prelude::NodeId; use radicle::prelude::RepoId; use radicle::storage::git::Repository; @@ -350,16 +347,7 @@ fn set_canonical_refs( applied: &Applied, ) -> Result, error::Canonical> { let identity = repo.identity()?; - let rules = identity - .canonical_refs_or_default(|| { - identity - .doc() - .default_branch_rule() - .map(CanonicalRefs::new) - .map_err(CanonicalRefsError::from) - })? - .rules() - .clone(); + let rules = identity.doc().canonical_refs()?.rules().clone(); let mut updated_refs = UpdatedCanonicalRefs::default(); let refnames = applied diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index c022ed78..0ec93a22 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -9,8 +9,6 @@ use std::str::FromStr; use std::{assert_eq, io}; use radicle::cob::store::access::WriteAs; -use radicle::identity::crefs::GetCanonicalRefs as _; -use radicle::identity::doc::CanonicalRefsError; use thiserror::Error; use radicle::Profile; @@ -20,7 +18,7 @@ use radicle::cob::patch; use radicle::cob::patch::cache::Patches as _; use radicle::crypto; use radicle::explorer::ExplorerResource; -use radicle::identity::{CanonicalRefs, Did}; +use radicle::identity::Did; use radicle::node; use radicle::node::NodeId; use radicle::storage; @@ -353,11 +351,7 @@ pub(super) fn run( ), PushAction::PushRef { dst } => { let identity = stored.identity()?; - let crefs = identity.canonical_refs_or_default(|| { - Ok::<_, CanonicalRefsError>(CanonicalRefs::new( - identity.doc().default_branch_rule()?, - )) - })?; + let crefs = identity.doc().canonical_refs()?; let rules = crefs.rules(); let me = Did::from(nid); diff --git a/crates/radicle/src/identity/crefs.rs b/crates/radicle/src/identity/crefs.rs index e3a39c2c..88951301 100644 --- a/crates/radicle/src/identity/crefs.rs +++ b/crates/radicle/src/identity/crefs.rs @@ -6,35 +6,15 @@ use super::doc::{Delegates, Payload}; /// Implemented by any data type or store that can return [`CanonicalRefs`] and /// [`RawCanonicalRefs`]. -pub trait GetCanonicalRefs { +pub trait GetRawCanonicalRefs { type Error: std::error::Error + Send + Sync + 'static; - /// Retrieve the [`CanonicalRefs`], returning `Some` if they are not - /// present, and `None` if they are missing. - /// - /// [`Self::Error`] is used to return any domain-specific error by the - /// implementing type. - fn canonical_refs(&self) -> Result, Self::Error>; - - /// Retrieve the [`RawCanonicalRefs`], returning `Some` if they are not - /// present, and `None` if they are missing. + /// Retrieve the [`RawCanonicalRefs`], returning `Some` if they are + /// present, and `None` if they are absent. /// /// [`Self::Error`] is used to return any domain-specific error by the /// implementing type. fn raw_canonical_refs(&self) -> Result, Self::Error>; - - /// Retrieve the [`CanonicalRefs`], and in the case of `None`, then use the - /// `default` function to return a default set of [`CanonicalRefs`]. - fn canonical_refs_or_default(&self, default: D) -> Result - where - D: Fn() -> Result, - E: From, - { - match self.canonical_refs()? { - Some(crefs) => Ok(crefs), - None => Ok(default()?), - } - } } /// Configuration for canonical references and their rules. diff --git a/crates/radicle/src/identity/doc.rs b/crates/radicle/src/identity/doc.rs index 6b065328..a57d59b1 100644 --- a/crates/radicle/src/identity/doc.rs +++ b/crates/radicle/src/identity/doc.rs @@ -756,6 +756,23 @@ impl Doc { .expect("default rules are valid")) } + /// Construct the canonical references for this document. + /// The implementation of [`crefs::RawCanonicalRefs`] is used to + /// obtain the payload identified by [`PayloadId::canonical_refs`], if it + /// exists. + /// The resulting [`CanonicalRefs`] are constructed by extension with + /// [`Self::default_branch_rule`]. + pub fn canonical_refs(&self) -> Result { + use crefs::GetRawCanonicalRefs; + let raw_crefs = self.raw_canonical_refs()?.unwrap_or_default(); + + let mut raw_rules = raw_crefs.raw_rules().clone(); + raw_rules.extend(rules::RawRules::from(self.default_branch_rule()?)); + + let raw_crefs = RawCanonicalRefs::new(raw_rules); + Ok(raw_crefs.try_into_canonical_refs(&mut || self.delegates.clone())?) + } + /// Return the associated [`Visibility`] of this document. pub fn visibility(&self) -> &Visibility { &self.visibility @@ -923,26 +940,9 @@ pub enum CanonicalRefsError { DefaultBranch(#[from] DefaultBranchRuleError), } -impl crefs::GetCanonicalRefs for Doc { +impl crefs::GetRawCanonicalRefs for Doc { type Error = CanonicalRefsError; - fn canonical_refs(&self) -> Result, Self::Error> { - let Some(raw_crefs) = self.raw_canonical_refs()? else { - return Ok(None); - }; - - let mut raw_rules = raw_crefs.raw_rules().clone(); - - let default_branch_rule = self.default_branch_rule()?; - raw_rules.extend(rules::RawRules::from(default_branch_rule)); - - let raw_crefs = RawCanonicalRefs::new(raw_rules); - - Ok(Some( - raw_crefs.try_into_canonical_refs(&mut || self.delegates.clone())?, - )) - } - fn raw_canonical_refs(&self) -> Result, Self::Error> { let value = self.payload.get(&PayloadId::canonical_refs()); let crefs = value @@ -954,13 +954,9 @@ impl crefs::GetCanonicalRefs for Doc { } } -impl crefs::GetCanonicalRefs for RawDoc { +impl crefs::GetRawCanonicalRefs for RawDoc { type Error = CanonicalRefsError; - fn canonical_refs(&self) -> Result, Self::Error> { - Ok(None) - } - fn raw_canonical_refs(&self) -> Result, Self::Error> { let value = self.payload.get(&PayloadId::canonical_refs()); let crefs = value diff --git a/crates/radicle/src/identity/doc/update.rs b/crates/radicle/src/identity/doc/update.rs index affa7642..ddd1e964 100644 --- a/crates/radicle/src/identity/doc/update.rs +++ b/crates/radicle/src/identity/doc/update.rs @@ -6,7 +6,6 @@ use serde_json as json; use crate::{ git, - identity::crefs::GetCanonicalRefs as _, prelude::Did, storage::{self, ReadRepository, RepositoryError, refs}, }; @@ -212,6 +211,7 @@ pub fn verify(raw: RawDoc) -> Result { // Ensure that if we have canonical reference rules and a project, that no // rule exists for the default branch. This rule must be synthesized when // constructing the canonical reference rules. + use super::crefs::GetRawCanonicalRefs as _; match raw .raw_canonical_refs() .map(|rcrefs| rcrefs.and_then(|c| project.map(|p| (c, p)))) @@ -277,10 +277,7 @@ mod test { use crate::{ git, - identity::{ - crefs::GetCanonicalRefs, - doc::{PayloadId, update::error}, - }, + identity::doc::{PayloadId, update::error}, prelude::RawDoc, test::arbitrary, }; @@ -362,7 +359,7 @@ mod test { ) .unwrap(); let verified = super::verify(raw).unwrap(); - let crefs = verified.canonical_refs().unwrap().unwrap(); + let crefs = verified.canonical_refs().unwrap(); assert!( crefs.rules().matches(&branch).next().is_some(), "Default branch rule is missing!" diff --git a/crates/radicle/src/storage/git.rs b/crates/radicle/src/storage/git.rs index ada73f9f..d3107a98 100644 --- a/crates/radicle/src/storage/git.rs +++ b/crates/radicle/src/storage/git.rs @@ -13,9 +13,8 @@ use std::{fs, io}; use crate::git::canonical::Quorum; use crate::git::raw::ErrorExt as _; -use crate::identity::crefs::GetCanonicalRefs as _; use crate::identity::doc::DocError; -use crate::identity::{CanonicalRefs, Doc, DocAt, RepoId}; +use crate::identity::{Doc, DocAt, RepoId}; use crate::identity::{Identity, Project}; use crate::node::device::Device; use crate::storage::refs::{FeatureLevel, Refs, SignedRefs}; @@ -855,11 +854,7 @@ impl ReadRepository for Repository { let doc = self.identity_doc()?; let refname = doc.default_branch()?.to_owned(); - let crefs = doc.canonical_refs_or_default(|| { - doc.default_branch_rule() - .map_err(RepositoryError::from) - .map(CanonicalRefs::new) - })?; + let crefs = doc.canonical_refs()?; Ok(crefs .rules()