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.
This commit is contained in:
Lorenz Leutgeb 2025-09-18 15:38:33 +02:00 committed by Fintan Halpenny
parent ef101d9ab9
commit aa28567035
6 changed files with 30 additions and 80 deletions

View File

@ -1,6 +1,4 @@
use radicle::cob::store::access::ReadOnly; use radicle::cob::store::access::ReadOnly;
use radicle::identity::CanonicalRefs;
use radicle::identity::doc::CanonicalRefsError;
use radicle::storage::git::TempRepository; use radicle::storage::git::TempRepository;
pub(crate) use radicle_protocol::worker::fetch::error; pub(crate) use radicle_protocol::worker::fetch::error;
@ -11,7 +9,6 @@ use localtime::LocalTime;
use radicle::cob::TypedId; use radicle::cob::TypedId;
use radicle::crypto::PublicKey; use radicle::crypto::PublicKey;
use radicle::identity::crefs::GetCanonicalRefs as _;
use radicle::prelude::NodeId; use radicle::prelude::NodeId;
use radicle::prelude::RepoId; use radicle::prelude::RepoId;
use radicle::storage::git::Repository; use radicle::storage::git::Repository;
@ -350,16 +347,7 @@ fn set_canonical_refs(
applied: &Applied, applied: &Applied,
) -> Result<Option<UpdatedCanonicalRefs>, error::Canonical> { ) -> Result<Option<UpdatedCanonicalRefs>, error::Canonical> {
let identity = repo.identity()?; let identity = repo.identity()?;
let rules = identity let rules = identity.doc().canonical_refs()?.rules().clone();
.canonical_refs_or_default(|| {
identity
.doc()
.default_branch_rule()
.map(CanonicalRefs::new)
.map_err(CanonicalRefsError::from)
})?
.rules()
.clone();
let mut updated_refs = UpdatedCanonicalRefs::default(); let mut updated_refs = UpdatedCanonicalRefs::default();
let refnames = applied let refnames = applied

View File

@ -9,8 +9,6 @@ use std::str::FromStr;
use std::{assert_eq, io}; use std::{assert_eq, io};
use radicle::cob::store::access::WriteAs; use radicle::cob::store::access::WriteAs;
use radicle::identity::crefs::GetCanonicalRefs as _;
use radicle::identity::doc::CanonicalRefsError;
use thiserror::Error; use thiserror::Error;
use radicle::Profile; use radicle::Profile;
@ -20,7 +18,7 @@ use radicle::cob::patch;
use radicle::cob::patch::cache::Patches as _; use radicle::cob::patch::cache::Patches as _;
use radicle::crypto; use radicle::crypto;
use radicle::explorer::ExplorerResource; use radicle::explorer::ExplorerResource;
use radicle::identity::{CanonicalRefs, Did}; use radicle::identity::Did;
use radicle::node; use radicle::node;
use radicle::node::NodeId; use radicle::node::NodeId;
use radicle::storage; use radicle::storage;
@ -353,11 +351,7 @@ pub(super) fn run(
), ),
PushAction::PushRef { dst } => { PushAction::PushRef { dst } => {
let identity = stored.identity()?; let identity = stored.identity()?;
let crefs = identity.canonical_refs_or_default(|| { let crefs = identity.doc().canonical_refs()?;
Ok::<_, CanonicalRefsError>(CanonicalRefs::new(
identity.doc().default_branch_rule()?,
))
})?;
let rules = crefs.rules(); let rules = crefs.rules();
let me = Did::from(nid); let me = Did::from(nid);

View File

@ -6,35 +6,15 @@ use super::doc::{Delegates, Payload};
/// Implemented by any data type or store that can return [`CanonicalRefs`] and /// Implemented by any data type or store that can return [`CanonicalRefs`] and
/// [`RawCanonicalRefs`]. /// [`RawCanonicalRefs`].
pub trait GetCanonicalRefs { pub trait GetRawCanonicalRefs {
type Error: std::error::Error + Send + Sync + 'static; type Error: std::error::Error + Send + Sync + 'static;
/// Retrieve the [`CanonicalRefs`], returning `Some` if they are not /// Retrieve the [`RawCanonicalRefs`], returning `Some` if they are
/// present, and `None` if they are missing. /// present, and `None` if they are absent.
///
/// [`Self::Error`] is used to return any domain-specific error by the
/// implementing type.
fn canonical_refs(&self) -> Result<Option<CanonicalRefs>, Self::Error>;
/// Retrieve the [`RawCanonicalRefs`], 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 /// [`Self::Error`] is used to return any domain-specific error by the
/// implementing type. /// implementing type.
fn raw_canonical_refs(&self) -> Result<Option<RawCanonicalRefs>, Self::Error>; fn raw_canonical_refs(&self) -> Result<Option<RawCanonicalRefs>, 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<D, E>(&self, default: D) -> Result<CanonicalRefs, E>
where
D: Fn() -> Result<CanonicalRefs, E>,
E: From<Self::Error>,
{
match self.canonical_refs()? {
Some(crefs) => Ok(crefs),
None => Ok(default()?),
}
}
} }
/// Configuration for canonical references and their rules. /// Configuration for canonical references and their rules.

View File

@ -756,6 +756,23 @@ impl Doc {
.expect("default rules are valid")) .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<CanonicalRefs, CanonicalRefsError> {
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. /// Return the associated [`Visibility`] of this document.
pub fn visibility(&self) -> &Visibility { pub fn visibility(&self) -> &Visibility {
&self.visibility &self.visibility
@ -923,26 +940,9 @@ pub enum CanonicalRefsError {
DefaultBranch(#[from] DefaultBranchRuleError), DefaultBranch(#[from] DefaultBranchRuleError),
} }
impl crefs::GetCanonicalRefs for Doc { impl crefs::GetRawCanonicalRefs for Doc {
type Error = CanonicalRefsError; type Error = CanonicalRefsError;
fn canonical_refs(&self) -> Result<Option<CanonicalRefs>, 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<Option<RawCanonicalRefs>, Self::Error> { fn raw_canonical_refs(&self) -> Result<Option<RawCanonicalRefs>, Self::Error> {
let value = self.payload.get(&PayloadId::canonical_refs()); let value = self.payload.get(&PayloadId::canonical_refs());
let crefs = value 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; type Error = CanonicalRefsError;
fn canonical_refs(&self) -> Result<Option<CanonicalRefs>, Self::Error> {
Ok(None)
}
fn raw_canonical_refs(&self) -> Result<Option<RawCanonicalRefs>, Self::Error> { fn raw_canonical_refs(&self) -> Result<Option<RawCanonicalRefs>, Self::Error> {
let value = self.payload.get(&PayloadId::canonical_refs()); let value = self.payload.get(&PayloadId::canonical_refs());
let crefs = value let crefs = value

View File

@ -6,7 +6,6 @@ use serde_json as json;
use crate::{ use crate::{
git, git,
identity::crefs::GetCanonicalRefs as _,
prelude::Did, prelude::Did,
storage::{self, ReadRepository, RepositoryError, refs}, storage::{self, ReadRepository, RepositoryError, refs},
}; };
@ -212,6 +211,7 @@ pub fn verify(raw: RawDoc) -> Result<Doc, error::DocVerification> {
// Ensure that if we have canonical reference rules and a project, that no // 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 // rule exists for the default branch. This rule must be synthesized when
// constructing the canonical reference rules. // constructing the canonical reference rules.
use super::crefs::GetRawCanonicalRefs as _;
match raw match raw
.raw_canonical_refs() .raw_canonical_refs()
.map(|rcrefs| rcrefs.and_then(|c| project.map(|p| (c, p)))) .map(|rcrefs| rcrefs.and_then(|c| project.map(|p| (c, p))))
@ -277,10 +277,7 @@ mod test {
use crate::{ use crate::{
git, git,
identity::{ identity::doc::{PayloadId, update::error},
crefs::GetCanonicalRefs,
doc::{PayloadId, update::error},
},
prelude::RawDoc, prelude::RawDoc,
test::arbitrary, test::arbitrary,
}; };
@ -362,7 +359,7 @@ mod test {
) )
.unwrap(); .unwrap();
let verified = super::verify(raw).unwrap(); let verified = super::verify(raw).unwrap();
let crefs = verified.canonical_refs().unwrap().unwrap(); let crefs = verified.canonical_refs().unwrap();
assert!( assert!(
crefs.rules().matches(&branch).next().is_some(), crefs.rules().matches(&branch).next().is_some(),
"Default branch rule is missing!" "Default branch rule is missing!"

View File

@ -13,9 +13,8 @@ use std::{fs, io};
use crate::git::canonical::Quorum; use crate::git::canonical::Quorum;
use crate::git::raw::ErrorExt as _; use crate::git::raw::ErrorExt as _;
use crate::identity::crefs::GetCanonicalRefs as _;
use crate::identity::doc::DocError; 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::identity::{Identity, Project};
use crate::node::device::Device; use crate::node::device::Device;
use crate::storage::refs::{FeatureLevel, Refs, SignedRefs}; use crate::storage::refs::{FeatureLevel, Refs, SignedRefs};
@ -855,11 +854,7 @@ impl ReadRepository for Repository {
let doc = self.identity_doc()?; let doc = self.identity_doc()?;
let refname = doc.default_branch()?.to_owned(); let refname = doc.default_branch()?.to_owned();
let crefs = doc.canonical_refs_or_default(|| { let crefs = doc.canonical_refs()?;
doc.default_branch_rule()
.map_err(RepositoryError::from)
.map(CanonicalRefs::new)
})?;
Ok(crefs Ok(crefs
.rules() .rules()