From b482845e712d54fdee00b0548c828e6694a3f359 Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Tue, 28 Apr 2026 09:49:54 +0100 Subject: [PATCH] sigrefs/git/properties: Restore property tests These tests fell under the radar when refactoring. Restore them to the latest code so that these properties are tested. --- crates/radicle/src/git/raw.rs | 5 +- .../radicle/src/storage/refs/sigrefs/git.rs | 3 + .../storage/refs/sigrefs/git/properties.rs | 182 ++++++++++++------ .../radicle/src/storage/refs/sigrefs/write.rs | 5 + 4 files changed, 140 insertions(+), 55 deletions(-) diff --git a/crates/radicle/src/git/raw.rs b/crates/radicle/src/git/raw.rs index a27f79e3..68f589ed 100644 --- a/crates/radicle/src/git/raw.rs +++ b/crates/radicle/src/git/raw.rs @@ -28,9 +28,12 @@ pub use git2::{ AnnotatedCommit, Diff, DiffFindOptions, DiffOptions, DiffStats, MergeAnalysis, MergeOptions, }; -// Re-exports for `radicle-cli`. pub mod build { + // Re-exports for `radicle-cli`. pub use git2::build::CheckoutBuilder; + + #[cfg(test)] + pub(crate) use git2::build::TreeUpdateBuilder; } pub(crate) mod transport { diff --git a/crates/radicle/src/storage/refs/sigrefs/git.rs b/crates/radicle/src/storage/refs/sigrefs/git.rs index 86693302..6a415291 100644 --- a/crates/radicle/src/storage/refs/sigrefs/git.rs +++ b/crates/radicle/src/storage/refs/sigrefs/git.rs @@ -5,6 +5,9 @@ pub mod object; pub mod reference; +#[cfg(test)] +mod properties; + use crypto::PublicKey; use radicle_git_metadata::author::Author; use radicle_git_metadata::author::Time; diff --git a/crates/radicle/src/storage/refs/sigrefs/git/properties.rs b/crates/radicle/src/storage/refs/sigrefs/git/properties.rs index be9eb2a7..70db0700 100644 --- a/crates/radicle/src/storage/refs/sigrefs/git/properties.rs +++ b/crates/radicle/src/storage/refs/sigrefs/git/properties.rs @@ -1,4 +1,7 @@ -use crypto::{signature, test::signer::MockSigner, PublicKey, Signer as _}; +use std::path::Path; + +use crypto::test::signer::MockSigner; +use crypto::{PublicKey, Signer as _, signature}; use qcheck::TestResult; use qcheck_macros::quickcheck; use radicle_core::{NodeId, RepoId}; @@ -6,12 +9,16 @@ use radicle_git_metadata::author::{Author, Time}; use radicle_oid::Oid; use tempfile::TempDir; -use crate::storage::refs::sigrefs::{ - read::{CheckpointReason, Latest, SignedRefsReader, Tip}, - write::{Committer, SignedRefsWriter, Update}, -}; -use crate::storage::refs::Refs; +use crate::git; +use crate::storage::refs::sigrefs::VerifiedCommit; +use crate::storage::refs::sigrefs::read::{SignedRefsReader, Tip}; +use crate::storage::refs::sigrefs::write::{SignedRefsWriter, Update}; +use crate::storage::refs::{IDENTITY_ROOT, Refs}; +use super::Committer; + +/// Newtype wrapper around [`Vec`] to keep the [`Arbitrary`] implementation +/// bounded to a smaller size. #[derive(Clone, Debug)] struct BoundedVec(Vec); @@ -58,24 +65,69 @@ fn mock_committer() -> Committer { Committer::new(mock_author()) } -fn setup() -> (TempDir, git2::Repository) { - let dir = TempDir::new().unwrap(); - let repo = git2::Repository::init_bare(dir.path()).unwrap(); - (dir, repo) +/// A helper structure that sets up a minimal Radicle repository. +struct Repository { + raw: git::raw::Repository, + rid: RepoId, + root: Oid, + _tmp: TempDir, } -fn mock_root() -> RepoId { - RepoId::from(Oid::from_sha1([1; 20])) +impl Repository { + fn new() -> Repository { + let dir = TempDir::new().unwrap(); + let repo = git::raw::Repository::init_bare(dir.path()).unwrap(); + let (root, rid) = Self::write_identity(&repo); + Repository { + raw: repo, + rid, + root, + _tmp: dir, + } + } + + /// Writes a mock blob that represents the identity document, and creates a + /// commit in the repository that contains this blob. + fn write_identity(repo: &git::raw::Repository) -> (Oid, RepoId) { + let blob = repo.blob(b"identity root").unwrap(); + let empty = { + let empty = repo.treebuilder(None).unwrap(); + let tree = empty.write().unwrap(); + repo.find_tree(tree).unwrap() + }; + let tree = { + let mut tb = git::raw::build::TreeUpdateBuilder::new(); + tb.upsert( + Path::new("embeds").join(*crate::identity::doc::PATH), + blob, + git::raw::FileMode::Blob, + ); + let tree = tb.create_updated(repo, &empty).unwrap(); + repo.find_tree(tree).unwrap() + }; + let author = git::raw::Signature::now("testy", "testy@example.com").unwrap(); + let root = repo + .commit( + Some(IDENTITY_ROOT.as_str()), + &author, + &author, + "identity root", + &tree, + &[], + ) + .unwrap(); + (root.into(), RepoId::from(blob)) + } } fn write_log( refs: Refs, + rid: RepoId, namespace: NodeId, signer: &MockSigner, - repo: &git2::Repository, + repo: &git::raw::Repository, ) -> Update { - SignedRefsWriter::new(namespace, signer, repo) - .with_refs(refs) + SignedRefsWriter::new(refs, rid, namespace, repo, signer) .write( mock_committer(), "test commit".to_string(), @@ -84,36 +136,42 @@ fn write_log( .unwrap() } -fn read_log(namespace: NodeId, verifier: &Verifier, repo: &git2::Repository) -> Latest { - SignedRefsReader::new(mock_root(), Tip::Reference(namespace), repo, verifier) +fn read_log( + rid: RepoId, + namespace: NodeId, + verifier: &Verifier, + repo: &git::raw::Repository, +) -> VerifiedCommit { + SignedRefsReader::new(rid, Tip::Reference(namespace), repo, verifier) .read() .unwrap() - .unwrap() } #[quickcheck] -fn initial_commit_roundtrip(refs: Refs) -> bool { - let (_dir, repo) = setup(); +fn initial_commit_roundtrip(mut refs: Refs) -> bool { + let Repository { + raw: repo, + rid, + root, + _tmp, + } = Repository::new(); + refs.insert(IDENTITY_ROOT.to_ref_string(), root); let signer = MockSigner::default(); let namespace = *signer.public_key(); let verifier = Verifier::new(&signer); - let update = write_log(refs.clone(), namespace, &signer, &repo); + let update = write_log(refs.clone(), rid, namespace, &signer, &repo); let head_oid = match update { - Update::Changed { ref entry } => *entry.oid(), + Update::Changed { ref entry, .. } => entry.oid(), Update::Unchanged { .. } => return false, }; - let Latest { - refs: expected, - checkpoint, - .. - } = read_log(namespace, &verifier, &repo); + let verified_commit = read_log(rid, namespace, &verifier, &repo); + let head = *verified_commit.commit().oid(); + let parent = verified_commit.commit().parent().copied(); + let new_refs = verified_commit.into_sigrefs_at(namespace); - checkpoint.head() == head_oid - && checkpoint.ancestor() == head_oid - && checkpoint.reason() == CheckpointReason::Root - && expected == refs + head == head_oid && parent.is_none() && *new_refs == refs } #[quickcheck] @@ -123,56 +181,72 @@ fn chain_roundtrip(chain: BoundedVec) -> TestResult { return TestResult::discard(); } - let (_dir, repo) = setup(); + let Repository { + raw: repo, + rid, + root, + _tmp, + } = Repository::new(); let signer = MockSigner::default(); let namespace = *signer.public_key(); let verifier = Verifier::new(&signer); let mut last_changed_head = None; + let mut expected_parent = None; - for refs in chain { - let update = write_log(refs.clone(), namespace, &signer, &repo); + for mut refs in chain { + refs.insert(IDENTITY_ROOT.to_ref_string(), root); + let update = write_log(refs.clone(), rid, namespace, &signer, &repo); - if let Update::Changed { ref entry } = update { - last_changed_head = Some(*entry.oid()); + if let Update::Changed { ref entry, .. } = update { + last_changed_head = Some(entry.oid()); } - let Latest { - refs: expected, - checkpoint, - .. - } = read_log(namespace, &verifier, &repo); + let verified_commit = read_log(rid, namespace, &verifier, &repo); + let head = *verified_commit.commit().oid(); + let parent = verified_commit.commit().parent().copied(); + let new_refs = verified_commit.into_sigrefs_at(namespace); - if refs != expected { - return TestResult::failed(); - } - - if checkpoint.reason() != CheckpointReason::Root { + if refs != *new_refs { return TestResult::failed(); } if let Some(expected_head) = last_changed_head { - if checkpoint.head() != expected_head { - return TestResult::failed(); + if head != expected_head { + return TestResult::error(format!( + "expected commit to be {expected_head}, but found {head}" + )); + } + if parent != expected_parent { + return TestResult::error(format!( + "expected parent commit to be {expected_parent:?}, but found {parent:?}" + )); } } + expected_parent = Some(head); } TestResult::passed() } #[quickcheck] -fn idempotent_write(refs: Refs) -> bool { - let (_dir, repo) = setup(); +fn idempotent_write(mut refs: Refs) -> bool { + let Repository { + raw: repo, + rid, + root, + _tmp, + } = Repository::new(); + refs.insert(IDENTITY_ROOT.to_ref_string(), root); let signer = MockSigner::default(); let namespace = *signer.public_key(); - let first = write_log(refs.clone(), namespace, &signer, &repo); + let first = write_log(refs.clone(), rid, namespace, &signer, &repo); let head_oid = match first { - Update::Changed { ref entry } => *entry.oid(), + Update::Changed { ref entry, .. } => entry.oid(), Update::Unchanged { .. } => return false, }; - let second = write_log(refs, namespace, &signer, &repo); - matches!(second, Update::Unchanged { commit, .. } if commit == head_oid) + let second = write_log(refs, rid, namespace, &signer, &repo); + matches!(second, Update::Unchanged { verified } if *verified.commit().oid() == head_oid) } diff --git a/crates/radicle/src/storage/refs/sigrefs/write.rs b/crates/radicle/src/storage/refs/sigrefs/write.rs index caf25427..a427fb7f 100644 --- a/crates/radicle/src/storage/refs/sigrefs/write.rs +++ b/crates/radicle/src/storage/refs/sigrefs/write.rs @@ -199,6 +199,11 @@ pub struct Commit { } impl Commit { + #[cfg(test)] + pub(super) fn oid(&self) -> Oid { + self.oid + } + #[cfg(test)] pub(super) fn into_refs(self) -> Refs { self.refs