From ef101d9ab9560f905cd784d8303299e2da832b65 Mon Sep 17 00:00:00 2001 From: Lorenz Leutgeb Date: Tue, 16 Sep 2025 22:02:09 +0200 Subject: [PATCH] radicle/crefs/protect: Module for protected refs Separate module for the concern of protecting `refs/rad`. Note that this also fixes a bug, as previously all refs *starting with* `refs/rad`, such as `refs/radieschen` were protected. --- crates/radicle-node/src/worker/fetch.rs | 7 +- crates/radicle-remote-helper/src/push.rs | 5 +- crates/radicle/src/git/canonical.rs | 1 + crates/radicle/src/git/canonical/protect.rs | 109 +++++++ crates/radicle/src/git/canonical/rules.rs | 299 ++++++-------------- crates/radicle/src/identity/crefs.rs | 17 +- crates/radicle/src/identity/doc.rs | 43 +-- crates/radicle/src/storage/git.rs | 13 +- 8 files changed, 242 insertions(+), 252 deletions(-) create mode 100644 crates/radicle/src/git/canonical/protect.rs diff --git a/crates/radicle-node/src/worker/fetch.rs b/crates/radicle-node/src/worker/fetch.rs index 66fc4648..9bc3c6ef 100644 --- a/crates/radicle-node/src/worker/fetch.rs +++ b/crates/radicle-node/src/worker/fetch.rs @@ -352,8 +352,11 @@ fn set_canonical_refs( let identity = repo.identity()?; let rules = identity .canonical_refs_or_default(|| { - let rule = identity.doc().default_branch_rule()?; - Ok::<_, CanonicalRefsError>(CanonicalRefs::from_iter([rule])) + identity + .doc() + .default_branch_rule() + .map(CanonicalRefs::new) + .map_err(CanonicalRefsError::from) })? .rules() .clone(); diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index e881e756..c022ed78 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -354,8 +354,9 @@ pub(super) fn run( PushAction::PushRef { dst } => { let identity = stored.identity()?; let crefs = identity.canonical_refs_or_default(|| { - let rule = identity.doc().default_branch_rule()?; - Ok::<_, CanonicalRefsError>(CanonicalRefs::from_iter([rule])) + Ok::<_, CanonicalRefsError>(CanonicalRefs::new( + identity.doc().default_branch_rule()?, + )) })?; let rules = crefs.rules(); let me = Did::from(nid); diff --git a/crates/radicle/src/git/canonical.rs b/crates/radicle/src/git/canonical.rs index cb54a71a..13af6b49 100644 --- a/crates/radicle/src/git/canonical.rs +++ b/crates/radicle/src/git/canonical.rs @@ -10,6 +10,7 @@ use quorum::{CommitQuorum, CommitQuorumFailure, TagQuorum, TagQuorumFailure}; mod voting; pub mod effects; +pub mod protect; pub mod rules; pub use rules::{MatchedRule, RawRule, Rules, ValidRule}; diff --git a/crates/radicle/src/git/canonical/protect.rs b/crates/radicle/src/git/canonical/protect.rs new file mode 100644 index 00000000..4326da5e --- /dev/null +++ b/crates/radicle/src/git/canonical/protect.rs @@ -0,0 +1,109 @@ +//! Some reference names are protected and cannot be used with canonical +//! references. This module contains checks for these cases. +//! +//! Protected references are: +//! 1. `refs/rad` +//! 2. Any reference matching `refs/rad/*`, e.g. `refs/rad/id`, `refs/rad/foo/bar`. + +const REFS_RAD: &str = "refs/rad"; + +/// Reference-like types, which we encounter when working with canonical references. +pub(crate) trait RefLike: AsRef + Ord + std::fmt::Display + serde::Serialize {} + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("reference-like string '{REFS_RAD}' is protected")] + RefsRad, + #[error("reference-like string '{reflike}' is protected because it starts with '{REFS_RAD}/'")] + RefsRadChild { reflike: String }, +} + +/// A witnesses that the inner reference-like value is not protected. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize)] +#[repr(transparent)] +#[serde(transparent)] +pub(super) struct Unprotected(T); + +impl Unprotected { + pub fn new(reflike: T) -> Result { + match reflike + .as_ref() + .strip_prefix(REFS_RAD) + .map(|rest| rest.get(..1)) + { + Some(None) => Err(Error::RefsRad), + Some(Some("/")) => Err(Error::RefsRadChild { + reflike: reflike.to_string(), + }), + Some(_) | None => Ok(Self(reflike)), + } + } + + pub fn into_inner(self) -> T { + self.0 + } +} + +impl AsRef for Unprotected { + fn as_ref(&self) -> &T { + &self.0 + } +} + +impl<'de, T: RefLike + serde::Deserialize<'de>> serde::Deserialize<'de> for Unprotected { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Self::new(T::deserialize(deserializer)?).map_err(serde::de::Error::custom) + } +} + +impl std::fmt::Display for Unprotected { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +/// For types that are commonly used in conjunction with [`Unprotected`] +/// have some `impl`s and companion infallible injections. +mod impls { + use crate::git::fmt::{RefString, refspec::QualifiedPattern}; + + use super::*; + + /// [`RefString`] models reference names, thus the prototype of what it + /// means to be [`RefLike`]. + impl RefLike for RefString {} + + /// A [`QualifiedPattern`] is [`RefLike`] in the sense that it matches a + /// (possibly infinite) set of [`crate::git::Qualified`]. + impl RefLike for QualifiedPattern<'_> {} +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use crate::assert_matches; + use crate::git::fmt::refname; + + use super::{Error::*, Unprotected}; + + #[test] + fn refs_rad() { + assert_matches!(Unprotected::new(refname!("refs/rad")), Err(RefsRad)) + } + + #[test] + fn refs_rad_id() { + assert_matches!( + Unprotected::new(refname!("refs/rad/id")), + Err(RefsRadChild { .. }) + ) + } + + #[test] + fn refs_radieschen() { + assert_matches!(Unprotected::new(refname!("refs/radieschen")), Ok(_)) + } +} diff --git a/crates/radicle/src/git/canonical/rules.rs b/crates/radicle/src/git/canonical/rules.rs index e99a6efd..50fac2d4 100644 --- a/crates/radicle/src/git/canonical/rules.rs +++ b/crates/radicle/src/git/canonical/rules.rs @@ -9,10 +9,8 @@ //! the first matched rule, and this can be used to calculate the //! [`Canonical::quorum`]. -use core::fmt; use std::cmp::Ordering; use std::collections::BTreeMap; -use std::sync::LazyLock; use nonempty::NonEmpty; use serde::{Deserialize, Serialize}; @@ -24,12 +22,12 @@ use crate::git::canonical; use crate::git::canonical::Canonical; use crate::git::fmt::Qualified; use crate::git::fmt::refspec::QualifiedPattern; -use crate::git::fmt::{RefString, refname}; use crate::identity::{Did, doc}; -const ASTERISK: char = '*'; +use super::protect; +use super::protect::Unprotected; -static REFS_RAD: LazyLock = LazyLock::new(|| refname!("refs/rad")); +const ASTERISK: char = '*'; /// Private trait to ensure that not any `Rule` can be deserialized. /// Implementations are provided for `Allowed` and `usize` so that `RawRule`s @@ -39,94 +37,39 @@ trait Sealed {} impl Sealed for Allowed {} impl Sealed for usize {} -/// A `Pattern` is a `QualifiedPattern` reference, however, it disallows any -/// references under the `refs/rad` hierarchy. -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -#[serde(into = "QualifiedPattern", try_from = "QualifiedPattern")] -pub struct Pattern(QualifiedPattern<'static>); +/// A pattern for a rule is an unprotected qualified reference pattern. +/// Requiring [`Unprotected`] makes rules that would create protected references +/// unrepresentable. +pub(super) type Pattern = Unprotected>; -impl fmt::Display for Pattern { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(self.0.as_str()) - } -} +pub type RawPattern = QualifiedPattern<'static>; -impl From for QualifiedPattern<'static> { - fn from(Pattern(pattern): Pattern) -> Self { - pattern - } -} - -impl<'a> TryFrom> for Pattern { - type Error = PatternError; - - fn try_from(pattern: QualifiedPattern<'a>) -> Result { - if pattern.starts_with(REFS_RAD.as_str()) { - Err(PatternError::ProtectedRef { - prefix: (*REFS_RAD).clone(), - pattern: pattern.to_owned(), - }) - } else { - Ok(Self(pattern.to_owned())) +/// Check if the `pattern` matches the `refname`. +fn matches(pattern: &RawPattern, refname: &Qualified) -> bool { + // N.b. Git's refspecs do not quite match with glob-star semantics. A + // single `*` in a refspec is expected to match all references under + // that namespace, even if they are further down the hierarchy. + // Thus, the following rules are applied: + // + // - a trailing `*` changes to `**/*` + // - a `*` in between path components changes to `**` + let spec = match pattern.as_str().split_once(ASTERISK) { + None => pattern.to_string(), + // Expand `refs/tags/*` to `refs/tags/**/*` + Some((prefix, "")) => { + let mut spec = prefix.to_string(); + spec.push_str("**/*"); + spec } - } -} - -impl<'a> TryFrom> for Pattern { - type Error = PatternError; - - fn try_from(name: Qualified<'a>) -> Result { - Self::try_from(QualifiedPattern::from(name)) - } -} - -impl Pattern { - /// Construct a [`Pattern`] that matches a branch exactly. - /// - /// The resulting [`Pattern`] will match `refs/heads/`. - pub fn refs_heads_exact(name: &git::fmt::RefStr) -> Self { - Self(QualifiedPattern::from(git::refs::branch(name))) - } - - /// Check if the `refname` matches the rule's `refspec`. - pub fn matches(&self, refname: &Qualified) -> bool { - // N.b. Git's refspecs do not quite match with glob-star semantics. A - // single `*` in a refspec is expected to match all references under - // that namespace, even if they are further down the hierarchy. - // Thus, the following rules are applied: - // - // - a trailing `*` changes to `**/*` - // - a `*` in between path components changes to `**` - let spec = match self.0.as_str().split_once(ASTERISK) { - None => self.0.to_string(), - // Expand `refs/tags/*` to `refs/tags/**/*` - Some((prefix, "")) => { - let mut spec = prefix.to_string(); - spec.push_str("**/*"); - spec - } - // Expand `refs/tags/*/v1.0` to `refs/tags/**/v1.0` - Some((prefix, suffix)) => { - let mut spec = prefix.to_string(); - spec.push_str("**"); - spec.push_str(suffix); - spec - } - }; - fast_glob::glob_match(&spec, refname.as_str()) - } -} - -impl AsRef> for Pattern { - fn as_ref(&self) -> &QualifiedPattern<'static> { - &self.0 - } -} - -impl PartialOrd for Pattern { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } + // Expand `refs/tags/*/v1.0` to `refs/tags/**/v1.0` + Some((prefix, suffix)) => { + let mut spec = prefix.to_string(); + spec.push_str("**"); + spec.push_str(suffix); + spec + } + }; + fast_glob::glob_match(&spec, refname.as_str()) } /// Patterns are ordered by their specificity. @@ -248,8 +191,8 @@ impl Ord for Pattern { } let mut result = ComponentOrdering::default(); - let mut lhs = self.0.components(); - let mut rhs = other.0.components(); + let mut lhs = self.as_ref().components(); + let mut rhs = other.as_ref().components(); loop { match (lhs.next(), rhs.next()) { (None, Some(_)) => return Ordering::Greater, // (1.) @@ -263,6 +206,12 @@ impl Ord for Pattern { } } +impl PartialOrd for Pattern { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + /// A [`Rule`] that can be serialized and deserialized safely. /// /// Should be converted to a [`ValidRule`] via [`Rule::validate`]. @@ -309,27 +258,27 @@ pub struct RawRules { /// Note that this can be a fully-qualified pattern, e.g. `refs/heads/qa`, /// as well as a wild-card pattern, e.g. `refs/tags/*`. #[serde(flatten)] - pub rules: BTreeMap, + pub rules: BTreeMap, } impl RawRules { - /// Returns an iterator over the [`Pattern`] and [`RawRule`] in the set of - /// rules. - pub fn iter(&self) -> impl Iterator { + /// Returns an iterator over the [`RawPattern`] and [`RawRule`] + /// in the set of rules. + pub fn iter(&self) -> impl Iterator { self.rules.iter() } /// Add a new [`RawRule`] to the set of rules. /// /// Returns the replaced rule, if it existed. - pub fn insert(&mut self, pattern: Pattern, rule: RawRule) -> Option { + pub fn insert(&mut self, pattern: RawPattern, rule: RawRule) -> Option { self.rules.insert(pattern, rule) } /// Remove the rule that matches the `pattern` parameter. /// /// Returns the rule if it existed. - pub fn remove(&mut self, pattern: &Pattern) -> Option { + pub fn remove(&mut self, pattern: &RawPattern) -> Option { self.rules.remove(pattern) } @@ -338,7 +287,7 @@ impl RawRules { let refname = refname.as_str(); self.rules .iter() - .any(|(pattern, _)| pattern.0.as_str() == refname) + .any(|(pattern, _)| pattern.as_str() == refname) } /// Check if the `refname` matches any existing rules, including glob @@ -346,35 +295,35 @@ impl RawRules { pub fn matches<'a, 'b>( &self, refname: &Qualified<'b>, - ) -> impl Iterator + use<'a, '_, 'b> { + ) -> impl Iterator + use<'a, '_, 'b> { let refname = refname.clone(); self.rules .iter() - .filter(move |(pattern, _)| pattern.matches(&refname)) + .filter(move |(pattern, _)| matches(pattern, &refname)) } } -impl Extend<(Pattern, RawRule)> for RawRules { - fn extend>(&mut self, iter: T) { +impl Extend<(RawPattern, RawRule)> for RawRules { + fn extend>(&mut self, iter: T) { self.rules.extend(iter) } } -impl From> for RawRules { - fn from(rules: BTreeMap) -> Self { +impl From> for RawRules { + fn from(rules: BTreeMap) -> Self { RawRules { rules } } } -impl FromIterator<(Pattern, RawRule)> for RawRules { - fn from_iter>(iter: T) -> Self { +impl FromIterator<(RawPattern, RawRule)> for RawRules { + fn from_iter>(iter: T) -> Self { iter.into_iter().collect::>().into() } } impl IntoIterator for RawRules { - type Item = (Pattern, RawRule); - type IntoIter = std::collections::btree_map::IntoIter; + type Item = (RawPattern, RawRule); + type IntoIter = std::collections::btree_map::IntoIter; fn into_iter(self) -> Self::IntoIter { self.rules.into_iter() @@ -390,40 +339,6 @@ impl IntoIterator for RawRules { /// document and validated. pub type ValidRule = Rule; -impl ValidRule { - /// Initialize a `ValidRule` for the default branch, given by `name`. The - /// rule will contain the single `did` as the allowed DID, and use a - /// threshold of `1`. - /// - /// Note that the serialization of the rule will use the `delegates` token - /// for the rule. E.g. - /// ```json - /// { - /// "pattern": "refs/heads/main", - /// "allow": ["did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi"], - /// "threshold": 1 - /// } - /// ``` - /// - /// # Errors - /// - /// If the `name` reference begins with `refs/rad`. - pub fn default_branch( - did: Did, - name: &git::fmt::RefStr, - ) -> Result<(Pattern, Self), PatternError> { - let pattern = Pattern::try_from(git::refs::branch(name).to_owned())?; - let rule = Self { - allow: ResolvedDelegates::Delegates(doc::Delegates::from(did)), - // N.B. this needs to be the minimum since we only have one - // delegate. - threshold: doc::Threshold::MIN, - extensions: json::Map::new(), - }; - Ok((pattern, rule)) - } -} - impl From for RawRule { fn from(rule: ValidRule) -> Self { let Rule { @@ -546,7 +461,7 @@ impl MatchedRule<'_> { /// cannot be duplicated. /// /// To construct the set of rules, use [`Rules::from_raw`], which validates a -/// set of [`RawRule`]s, and their [`Pattern`] references, into a set of +/// set of [`RawRule`]s, and their [`RawPattern`] references, into a set of /// [`ValidRule`]s. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize)] pub struct Rules { @@ -562,48 +477,18 @@ impl FromIterator<(Pattern, ValidRule)> for Rules { } } -impl<'a> IntoIterator for &'a Rules { - type Item = (&'a Pattern, &'a ValidRule); - type IntoIter = std::collections::btree_map::Iter<'a, Pattern, ValidRule>; - - fn into_iter(self) -> Self::IntoIter { - self.rules.iter() - } -} - -impl IntoIterator for Rules { - type Item = (Pattern, ValidRule); - type IntoIter = std::collections::btree_map::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - self.rules.into_iter() - } -} - -impl Extend<(Pattern, ValidRule)> for Rules { - fn extend>(&mut self, iter: T) { - self.rules.extend(iter) - } -} - impl From for RawRules { fn from(Rules { rules }: Rules) -> Self { Self { rules: rules .into_iter() - .map(|(pattern, rule)| (pattern, rule.into())) + .map(|(pattern, rule)| (pattern.into_inner(), rule.into())) .collect(), } } } impl Rules { - /// Returns an iterator over the [`Pattern`] and [`ValidRule`] in the set of - /// rules. - pub fn iter(&self) -> impl Iterator { - self.rules.iter() - } - /// Returns `true` is the set of rules is empty. pub fn is_empty(&self) -> bool { self.rules.is_empty() @@ -611,7 +496,7 @@ impl Rules { /// Construct a set of `Rules` given a set of `RawRule`s. pub fn from_raw( - rules: impl IntoIterator, + rules: impl IntoIterator, resolve: &mut R, ) -> Result where @@ -619,7 +504,10 @@ impl Rules { { let valid = rules .into_iter() - .map(|(pattern, rule)| rule.validate(resolve).map(|rule| (pattern, rule))) + .map(|(pattern, rule)| { + let pattern = Unprotected::new(pattern)?; + rule.validate(resolve).map(|rule| (pattern, rule)) + }) .collect::>()?; Ok(Self { rules: valid }) } @@ -628,11 +516,12 @@ impl Rules { pub fn matches<'a>( &self, refname: &Qualified<'a>, - ) -> impl Iterator + use<'a, '_> { + ) -> impl Iterator + use<'a, '_> { let refname_cloned = refname.clone(); self.rules .iter() - .filter(move |(pattern, _)| pattern.matches(&refname_cloned)) + .filter(move |(pattern, _)| matches(pattern.as_ref(), &refname_cloned)) + .map(|(pattern, rule)| (pattern.as_ref(), rule)) } /// Match given refname, take the most specific rule, and prepare evaluation @@ -657,7 +546,7 @@ impl Rules { } /// A `Rule` defines how a reference or set of references can be made canonical, -/// i.e. have a top-level `refs/*` entry – see [`Pattern`]. +/// i.e. have a top-level `refs/*` entry – see [`RawPattern`]. /// /// The [`Rule::allowed`] type is generic to allow for [`Allowed`] to be used /// for serialization and deserialization, however, the use of @@ -720,23 +609,14 @@ impl Rule { } } -#[derive(Debug, Error)] -pub enum PatternError { - #[error("cannot create rule for '{pattern}' since references under '{prefix}' are protected")] - ProtectedRef { - prefix: RefString, - pattern: QualifiedPattern<'static>, - }, -} - #[derive(Debug, Error)] pub enum ValidationError { #[error(transparent)] Threshold(#[from] doc::ThresholdError), #[error(transparent)] Delegates(#[from] doc::DelegatesError), - #[error("cannot create rule for reserved `rad` references '{pattern}'")] - RadRef { pattern: QualifiedPattern<'static> }, + #[error(transparent)] + Protected(#[from] protect::Error), } #[derive(Debug, Error)] @@ -780,8 +660,12 @@ mod tests { s.parse().unwrap() } + fn raw_pattern(qp: QualifiedPattern<'static>) -> RawPattern { + qp + } + fn pattern(qp: QualifiedPattern<'static>) -> Pattern { - Pattern::try_from(qp).unwrap() + Pattern::new(qp).unwrap() } fn resolve_from_doc(doc: &Doc) -> doc::Delegates { @@ -857,7 +741,7 @@ mod tests { "#; let expected = [ ( - pattern(qualified_pattern!("refs/heads/main")), + raw_pattern(qualified_pattern!("refs/heads/main")), Rule::new( Allowed::Set(nonempty![ did("did:key:z6MkpQTLwr8QyADGmBGAMsGttvWzP4PojUMs4hREZW5T5E3K"), @@ -867,7 +751,7 @@ mod tests { ), ), ( - pattern(qualified_pattern!("refs/tags/releases/*")), + raw_pattern(qualified_pattern!("refs/tags/releases/*")), Rule::new( Allowed::Set(nonempty![ did("did:key:z6MknLWe8A7UJxvTfY36JcB8XrP1KTLb5HFTX38hEmdY3b56"), @@ -878,7 +762,7 @@ mod tests { ), ), ( - pattern(qualified_pattern!("refs/heads/development")), + raw_pattern(qualified_pattern!("refs/heads/development")), Rule::new( Allowed::Set(nonempty![did( "did:key:z6MkhH7ENYE62JAjTiRZPU71MGZ6xCwnbyHHWfrBu3fr6PVG" @@ -887,13 +771,13 @@ mod tests { ), ), ( - pattern(qualified_pattern!("refs/heads/release/*")), + raw_pattern(qualified_pattern!("refs/heads/release/*")), Rule::new(Allowed::Delegates, 1), ), ] .into_iter() .collect::(); - let rules = serde_json::from_str::>(examples) + let rules = serde_json::from_str::>(examples) .unwrap() .into(); assert_eq!(expected, rules) @@ -1093,9 +977,12 @@ mod tests { // Duplicate rules are overwritten let rules = vec![ - (pattern.clone(), Rule::new(Allowed::Delegates, 1)), ( - pattern.clone(), + pattern.clone().into_inner(), + Rule::new(Allowed::Delegates, 1), + ), + ( + pattern.clone().into_inner(), Rule::new(doc.delegates().as_ref().clone().into(), 1), ), ]; @@ -1188,18 +1075,18 @@ mod tests { let rules = Rules::from_raw( [ ( - pattern(qualified_pattern!("refs/tags/*")), + raw_pattern(qualified_pattern!("refs/tags/*")), Rule::new(Allowed::Delegates, 1), ), ( - pattern(qualified_pattern!("refs/tags/release/*")), + raw_pattern(qualified_pattern!("refs/tags/release/*")), Rule::new(Allowed::Delegates, 1), ), // Ensure that none of the other rules apply by ensuring we need // both delegates to get the quorum of the // `refs/tags/release/candidates/v1.0` reference ( - pattern(qualified_pattern!("refs/tags/release/candidates/*")), + raw_pattern(qualified_pattern!("refs/tags/release/candidates/*")), Rule::new(Allowed::Delegates, 2), ), ], @@ -1237,9 +1124,9 @@ mod tests { #[test] fn test_special_branches() { - assert!(Pattern::try_from((*IDENTITY_BRANCH).clone()).is_err()); - assert!(Pattern::try_from((*SIGREFS_BRANCH).clone()).is_err()); - assert!(Pattern::try_from((*SIGREFS_PARENT).clone()).is_err()); - assert!(Pattern::try_from((*IDENTITY_ROOT).clone()).is_err()); + assert!(Pattern::new((*IDENTITY_BRANCH).clone().into()).is_err()); + assert!(Pattern::new((*SIGREFS_BRANCH).clone().into()).is_err()); + assert!(Pattern::new((*SIGREFS_PARENT).clone().into()).is_err()); + assert!(Pattern::new((*IDENTITY_ROOT).clone().into()).is_err()); } } diff --git a/crates/radicle/src/identity/crefs.rs b/crates/radicle/src/identity/crefs.rs index 9a089fa2..e3a39c2c 100644 --- a/crates/radicle/src/identity/crefs.rs +++ b/crates/radicle/src/identity/crefs.rs @@ -1,9 +1,6 @@ use serde::{Deserialize, Serialize}; -use crate::git::canonical::{ - ValidRule, - rules::{self, RawRules, Rules, ValidationError}, -}; +use crate::git::canonical::rules::{RawRules, Rules, ValidationError}; use super::doc::{Delegates, Payload}; @@ -95,18 +92,6 @@ impl CanonicalRefs { } } -impl FromIterator<(rules::Pattern, ValidRule)> for CanonicalRefs { - fn from_iter>(iter: T) -> Self { - Self::new(Rules::from_iter(iter)) - } -} - -impl Extend<(rules::Pattern, ValidRule)> for CanonicalRefs { - fn extend>(&mut self, iter: T) { - self.rules.extend(iter) - } -} - #[derive(Debug, thiserror::Error)] #[non_exhaustive] pub enum CanonicalRefsPayloadError { diff --git a/crates/radicle/src/identity/doc.rs b/crates/radicle/src/identity/doc.rs index 74fca7ed..6b065328 100644 --- a/crates/radicle/src/identity/doc.rs +++ b/crates/radicle/src/identity/doc.rs @@ -741,15 +741,19 @@ impl Doc { Ok(git::refs::branch(self.project()?.default_branch())) } - pub fn default_branch_rule( - &self, - ) -> Result<(rules::Pattern, rules::ValidRule), DefaultBranchRuleError> { - let pattern = rules::Pattern::refs_heads_exact(self.project()?.default_branch()); + pub fn default_branch_rule(&self) -> Result { + let pattern = git::fmt::refspec::QualifiedPattern::from(git::refs::branch( + self.project()?.default_branch(), + )); let rule = rules::Rule::new( rules::ResolvedDelegates::Delegates(self.delegates.clone()), self.threshold, ); - Ok((pattern, rule)) + Ok(rules::Rules::from_raw( + rules::RawRules::from_iter([(pattern, rule.into())]), + &mut || self.delegates.clone(), + ) + .expect("default rules are valid")) } /// Return the associated [`Visibility`] of this document. @@ -923,21 +927,20 @@ impl crefs::GetCanonicalRefs for Doc { type Error = CanonicalRefsError; fn canonical_refs(&self) -> Result, Self::Error> { - self.raw_canonical_refs().and_then(|raw| { - raw.map(|raw| { - raw.try_into_canonical_refs(&mut || self.delegates.clone()) - .map_err(CanonicalRefsError::from) - .and_then(|mut crefs| { - self.default_branch_rule() - .map_err(CanonicalRefsError::from) - .map(|rule| { - crefs.extend([rule]); - crefs - }) - }) - }) - .transpose() - }) + 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> { diff --git a/crates/radicle/src/storage/git.rs b/crates/radicle/src/storage/git.rs index 7e87c899..ada73f9f 100644 --- a/crates/radicle/src/storage/git.rs +++ b/crates/radicle/src/storage/git.rs @@ -854,12 +854,13 @@ impl ReadRepository for Repository { fn canonical_head(&self) -> Result<(Qualified<'_>, Oid), RepositoryError> { let doc = self.identity_doc()?; let refname = doc.default_branch()?.to_owned(); - let crefs = match doc.canonical_refs()? { - Some(crefs) => crefs, - // Fallback to constructing the default branch via the project - // payload - None => CanonicalRefs::from_iter([doc.default_branch_rule()?]), - }; + + let crefs = doc.canonical_refs_or_default(|| { + doc.default_branch_rule() + .map_err(RepositoryError::from) + .map(CanonicalRefs::new) + })?; + Ok(crefs .rules() .canonical(refname, self)