From 2d55ffba81942e9c239497dd52cc246f57bb1d1b Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Tue, 16 May 2023 17:32:01 +0200 Subject: [PATCH] cob: Redesign merge state Move `merges` from `Revision` to `Patch`, and key them by actor. This ensures that only one merge is possible per actor. We also make sure that only one revision can be merged at a time, and signal a conflict if more than one revision is considered merged. Finally, we make sure that merges have the same target-branch commit to be considered equal. --- radicle-cli/src/commands/patch.rs | 39 ++++- radicle-cli/src/commands/patch/common.rs | 4 +- radicle-cli/src/commands/patch/list.rs | 14 +- radicle-cli/src/commands/patch/show.rs | 4 +- radicle-httpd/src/api.rs | 4 +- radicle-httpd/src/api/json.rs | 9 +- radicle-httpd/src/api/v1/projects.rs | 36 ++-- radicle/src/cob/patch.rs | 203 +++++++++++++++++------ radicle/src/cob/test.rs | 57 ++++++- radicle/src/test.rs | 1 + radicle/src/test/storage.rs | 5 +- 11 files changed, 279 insertions(+), 97 deletions(-) diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs index b6fee088..287d81ab 100644 --- a/radicle-cli/src/commands/patch.rs +++ b/radicle-cli/src/commands/patch.rs @@ -101,6 +101,27 @@ pub enum OperationName { Edit, } +pub struct Filter(fn(&patch::State) -> bool); + +impl Filter { + /// Match everything. + fn all() -> Self { + Self(|_| true) + } +} + +impl Default for Filter { + fn default() -> Self { + Self(|state| matches!(state, patch::State::Open { .. })) + } +} + +impl std::fmt::Debug for Filter { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "Filter(..)") + } +} + #[derive(Debug)] pub enum Operation { Open { @@ -131,7 +152,7 @@ pub enum Operation { patch_id: Rev, }, List { - filter: Option, + filter: Filter, }, Edit { patch_id: Rev, @@ -160,7 +181,7 @@ impl Args for Options { let mut patch_id = None; let mut message = Message::default(); let mut push = true; - let mut filter = Some(patch::State::Open); + let mut filter = Filter::default(); let mut diff = false; let mut draft = false; let mut undo = false; @@ -220,19 +241,19 @@ impl Args for Options { // List options. Long("all") => { - filter = None; + filter = Filter::all(); } Long("draft") => { - filter = Some(patch::State::Draft); + filter = Filter(|s| s == &patch::State::Draft); } Long("archived") => { - filter = Some(patch::State::Archived); + filter = Filter(|s| s == &patch::State::Archived); } Long("merged") => { - filter = Some(patch::State::Merged); + filter = Filter(|s| matches!(s, patch::State::Merged { .. })); } Long("open") => { - filter = Some(patch::State::Open); + filter = Filter(|s| matches!(s, patch::State::Open { .. })); } // Common. @@ -352,8 +373,8 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { options, )?; } - Operation::List { filter } => { - list::run(&repository, &profile, filter)?; + Operation::List { filter: Filter(f) } => { + list::run(&repository, &profile, f)?; } Operation::Show { patch_id, diff } => { let patch_id = patch_id.resolve(&repository.backend)?; diff --git a/radicle-cli/src/commands/patch/common.rs b/radicle-cli/src/commands/patch/common.rs index 93572a9d..86a5c066 100644 --- a/radicle-cli/src/commands/patch/common.rs +++ b/radicle-cli/src/commands/patch/common.rs @@ -178,9 +178,7 @@ pub fn find_unmerged_with_base( let mut matches = Vec::new(); for (id, patch, clock) in proposed { - let (_, rev) = patch.latest().unwrap(); - - if !rev.merges().count() == 0 { + if patch.merges().count() != 0 { continue; } if **patch.head() == patch_head { diff --git a/radicle-cli/src/commands/patch/list.rs b/radicle-cli/src/commands/patch/list.rs index 4a3baa3e..bcc5cb6a 100644 --- a/radicle-cli/src/commands/patch/list.rs +++ b/radicle-cli/src/commands/patch/list.rs @@ -16,7 +16,7 @@ use super::common; pub fn run( repository: &Repository, profile: &Profile, - filter: Option, + filter: fn(&patch::State) -> bool, ) -> anyhow::Result<()> { let patches = Patches::open(repository)?; @@ -24,10 +24,8 @@ pub fn run( for patch in patches.all()? { let (id, patch, _) = patch?; - if let Some(filter) = filter { - if patch.state() != filter { - continue; - } + if !filter(patch.state()) { + continue; } all.push((id, patch)); } @@ -105,7 +103,7 @@ pub fn row( Ok([ match state { - patch::State::Open => term::format::positive("●").into(), + patch::State::Open { .. } => term::format::positive("●").into(), patch::State::Archived { .. } => term::format::yellow("●").into(), patch::State::Draft => term::format::dim("●").into(), patch::State::Merged { .. } => term::format::primary("✔").into(), @@ -172,8 +170,8 @@ pub fn timeline( )); } - for merge in revision.merges() { - let peer = repository.remote(&merge.node)?; + for (nid, merge) in patch.merges().filter(|(_, m)| m.revision == *revision_id) { + let peer = repository.remote(nid)?; let mut badges = Vec::new(); if peer.id == *whoami { diff --git a/radicle-cli/src/commands/patch/show.rs b/radicle-cli/src/commands/patch/show.rs index 8e00b5e8..7831377c 100644 --- a/radicle-cli/src/commands/patch/show.rs +++ b/radicle-cli/src/commands/patch/show.rs @@ -105,10 +105,10 @@ pub fn run( attrs.push([ term::format::tertiary("Status".to_owned()).into(), match state { - patch::State::Open => term::format::positive(state.to_string()), + patch::State::Open { .. } => term::format::positive(state.to_string()), patch::State::Draft => term::format::dim(state.to_string()), patch::State::Archived => term::format::yellow(state.to_string()), - patch::State::Merged => term::format::primary(state.to_string()), + patch::State::Merged { .. } => term::format::primary(state.to_string()), } .into(), ]); diff --git a/radicle-httpd/src/api.rs b/radicle-httpd/src/api.rs index a370a150..750a2984 100644 --- a/radicle-httpd/src/api.rs +++ b/radicle-httpd/src/api.rs @@ -153,10 +153,10 @@ pub enum PatchState { impl PatchState { pub fn matches(&self, patch: &patch::State) -> bool { match self { - Self::Open => matches!(patch, patch::State::Open), + Self::Open => matches!(patch, patch::State::Open { .. }), Self::Draft => matches!(patch, patch::State::Draft), Self::Archived => matches!(patch, patch::State::Archived), - Self::Merged => matches!(patch, patch::State::Merged), + Self::Merged => matches!(patch, patch::State::Merged { .. }), } } } diff --git a/radicle-httpd/src/api/json.rs b/radicle-httpd/src/api/json.rs index 2f20e2e0..88fc3caa 100644 --- a/radicle-httpd/src/api/json.rs +++ b/radicle-httpd/src/api/json.rs @@ -123,6 +123,14 @@ pub(crate) fn patch( "state": patch.state(), "target": patch.target(), "tags": patch.tags().collect::>(), + "merges": patch.merges().map(|(a, m)| { + json!({ + "author": a, + "revision": m.revision, + "commit": m.commit, + "timestamp": m.timestamp + }) + }).collect::>(), "revisions": patch.revisions().map(|(id, rev)| { json!({ "id": id, @@ -130,7 +138,6 @@ pub(crate) fn patch( "base": rev.base(), "oid": rev.head(), "refs": get_refs(repo, patch.author().id(), &rev.head()).unwrap_or(vec![]), - "merges": rev.merges().collect::>(), "discussions": rev.discussion().comments() .map(|(id, comment)| Comment::new(id, comment, rev.discussion(), aliases)) .collect::>(), diff --git a/radicle-httpd/src/api/v1/projects.rs b/radicle-httpd/src/api/v1/projects.rs index ae1c3dae..c6baea27 100644 --- a/radicle-httpd/src/api/v1/projects.rs +++ b/radicle-httpd/src/api/v1/projects.rs @@ -1745,6 +1745,7 @@ mod routes { "state": { "status": "open" }, "target": "delegates", "tags": [], + "merges": [], "revisions": [ { "id": CONTRIBUTOR_PATCH_ID, @@ -1754,7 +1755,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [], "timestamp": TIMESTAMP, "reviews": [], @@ -1784,6 +1784,7 @@ mod routes { "state": { "status": "open" }, "target": "delegates", "tags": [], + "merges": [], "revisions": [ { "id": CONTRIBUTOR_PATCH_ID, @@ -1793,7 +1794,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [], "timestamp": TIMESTAMP, "reviews": [], @@ -1862,6 +1862,7 @@ mod routes { "state": { "status": "open" }, "target": "delegates", "tags": [], + "merges": [], "revisions": [ { "id": CREATED_PATCH_ID, @@ -1871,7 +1872,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [], "timestamp": TIMESTAMP, "reviews": [], @@ -1925,6 +1925,7 @@ mod routes { "bug", "design" ], + "merges": [], "revisions": [ { "id": CONTRIBUTOR_PATCH_ID, @@ -1934,7 +1935,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [], "timestamp": TIMESTAMP, "reviews": [], @@ -1985,6 +1985,7 @@ mod routes { "state": { "status": "open" }, "target": "delegates", "tags": [], + "merges": [], "revisions": [ { "id": CONTRIBUTOR_PATCH_ID, @@ -1994,7 +1995,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [], "timestamp": TIMESTAMP, "reviews": [], @@ -2007,7 +2007,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [], "timestamp": TIMESTAMP, "reviews": [], @@ -2058,6 +2057,7 @@ mod routes { "state": { "status": "open" }, "target": "delegates", "tags": [], + "merges": [], "revisions": [ { "id": CONTRIBUTOR_PATCH_ID, @@ -2067,7 +2067,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [], "timestamp": TIMESTAMP, "reviews": [], @@ -2140,6 +2139,7 @@ mod routes { "state": { "status": "open" }, "target": "delegates", "tags": [], + "merges": [], "revisions": [ { "id": CONTRIBUTOR_PATCH_ID, @@ -2149,7 +2149,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [ { "id": CONTRIBUTOR_COMMENT_1, @@ -2236,6 +2235,7 @@ mod routes { "state": { "status": "open" }, "target": "delegates", "tags": [], + "merges": [], "revisions": [ { "id": CONTRIBUTOR_PATCH_ID, @@ -2245,7 +2245,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [], "discussions": [], "timestamp": TIMESTAMP, "reviews": [ @@ -2316,9 +2315,19 @@ mod routes { }, "title": "A new `hello world`", "description": "change `hello world` in README to something else", - "state": { "status": "merged" }, + "state": { + "status": "merged", + "revision": CONTRIBUTOR_PATCH_ID, + "commit": PARENT, + }, "target": "delegates", "tags": [], + "merges": [{ + "author": CONTRIBUTOR_NID, + "revision": CONTRIBUTOR_PATCH_ID, + "commit": PARENT, + "timestamp": TIMESTAMP, + }], "revisions": [ { "id": CONTRIBUTOR_PATCH_ID, @@ -2328,13 +2337,6 @@ mod routes { "refs": [ "refs/heads/master", ], - "merges": [ - { - "node": CONTRIBUTOR_NID, - "commit": PARENT, - "timestamp": TIMESTAMP, - }, - ], "discussions": [], "timestamp": TIMESTAMP, "reviews": [], diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 6a7753a2..26c585f3 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -1,4 +1,5 @@ #![allow(clippy::too_many_arguments)] +use std::collections::HashMap; use std::fmt; use std::ops::Deref; use std::ops::Range; @@ -11,7 +12,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use radicle_crdt::clock; -use radicle_crdt::{GMap, GSet, LWWReg, LWWSet, Lamport, Max, Redactable, Semilattice}; +use radicle_crdt::{GMap, GSet, LWWMap, LWWReg, LWWSet, Lamport, Max, Redactable, Semilattice}; use crate::cob; use crate::cob::common::{Author, Tag, Timestamp}; @@ -64,6 +65,9 @@ pub enum ApplyError { /// Error loading the identity document committed to by an operation. #[error("identity doc failed to load: {0}")] Doc(#[from] DocError), + /// The merge operation is invalid. + #[error("invalid merge operation in {0}")] + InvalidMerge(EntryId), } /// Error updating or creating patches. @@ -157,6 +161,8 @@ pub struct Patch { target: LWWReg>, /// Associated tags. tags: LWWSet, + /// Patch merges. + merges: LWWMap>, /// List of patch revisions. The initial changeset is part of the /// first revision. revisions: GMap>, @@ -170,6 +176,7 @@ impl Semilattice for Patch { self.description.merge(other.description); self.state.merge(other.state); self.target.merge(other.target); + self.merges.merge(other.merges); self.tags.merge(other.tags); self.revisions.merge(other.revisions); self.timeline.merge(other.timeline); @@ -184,6 +191,7 @@ impl Default for Patch { state: LWWReg::initial(Max::from(State::default())), target: LWWReg::initial(Max::from(MergeTarget::default())), tags: LWWSet::default(), + merges: LWWMap::default(), revisions: GMap::default(), timeline: GSet::default(), } @@ -197,8 +205,8 @@ impl Patch { } /// Current state of the patch. - pub fn state(&self) -> State { - *self.state.get().get() + pub fn state(&self) -> &State { + self.state.get().get() } /// Target this patch is meant to be merged in. @@ -253,6 +261,13 @@ impl Patch { }) } + /// Get the merges. + pub fn merges(&self) -> impl Iterator { + self.merges + .iter() + .filter_map(|(a, m)| m.get().map(|m| (a, m))) + } + /// Reference to the Git object containing the code on the latest revision. pub fn head(&self) -> &git::Oid { &self @@ -287,7 +302,7 @@ impl Patch { /// Check if the patch is open. pub fn is_open(&self) -> bool { - matches!(self.state(), State::Open) + matches!(self.state(), State::Open { .. }) } /// Check if the patch is archived. @@ -404,25 +419,56 @@ impl store::FromHistory for Patch { } } Action::Merge { revision, commit } => { - if let Some(Redactable::Present(revision)) = self.revisions.get_mut(&revision) { - revision.merges.insert( - Merge { - node: op.author, + if let Some(Redactable::Present(_)) = self.revisions.get_mut(&revision) { + let doc = repo.identity_doc_at(op.identity)?; + if !doc.is_delegate(&op.author) { + return Err(ApplyError::InvalidMerge(op.id)); + } + self.merges.insert( + op.author, + Redactable::Present(Merge { + revision, commit, timestamp, - } - .into(), + }), op.clock, ); - let doc = repo.identity_doc_at(op.identity)?; - if revision - .merges() - .filter(|m| doc.is_delegate(&m.node)) - .count() - >= doc.threshold - { - self.state.set(State::Merged, op.clock); + let mut merges = self.merges.iter().fold( + HashMap::<(RevisionId, git::Oid), usize>::new(), + |mut acc, (_, merge)| { + if let Some(merge) = merge.get() { + *acc.entry((merge.revision, merge.commit)).or_default() += 1; + } + acc + }, + ); + // Discard revisions that weren't merged by a threshold of delegates. + merges.retain(|_, count| *count >= doc.threshold); + + match merges.into_keys().collect::>().as_slice() { + [] => { + // None of the revisions met the quorum. + } + [(revision, commit)] => { + // Patch is merged. + self.state.set( + State::Merged { + revision: *revision, + commit: *commit, + }, + op.clock, + ); + } + revisions => { + // More than one revision met the quorum. + self.state.set( + State::Open { + conflicts: revisions.to_vec(), + }, + op.clock, + ); + } } } else { return Err(ApplyError::Missing(revision)); @@ -466,8 +512,6 @@ pub struct Revision { oid: git::Oid, /// Discussion around this revision. discussion: Thread, - /// Merges of this revision into other repositories. - merges: LWWSet>, /// Reviews of this revision's changes (one per actor). reviews: GMap, /// When this revision was created. @@ -489,7 +533,6 @@ impl Revision { base, oid, discussion: Thread::default(), - merges: LWWSet::default(), reviews: GMap::default(), timestamp, } @@ -524,11 +567,6 @@ impl Revision { &self.discussion } - /// Merges of this revision into other repositories. - pub fn merges(&self) -> impl Iterator { - self.merges.iter().map(|m| m.get()) - } - /// Reviews of this revision's changes (one per actor). pub fn reviews(&self) -> impl DoubleEndedIterator { self.reviews.iter() @@ -536,14 +574,29 @@ impl Revision { } /// Patch state. -#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] #[serde(rename_all = "camelCase", tag = "status")] pub enum State { - #[default] - Open, Draft, + Open { + /// Revisions that were merged and are conflicting. + #[serde(skip_serializing_if = "Vec::is_empty")] + #[serde(default)] + conflicts: Vec<(RevisionId, git::Oid)>, + }, Archived, - Merged, + Merged { + /// The revision that was merged. + revision: RevisionId, + /// The commit in the target branch that contains the changes. + commit: git::Oid, + }, +} + +impl Default for State { + fn default() -> Self { + Self::Open { conflicts: vec![] } + } } impl fmt::Display for State { @@ -551,8 +604,8 @@ impl fmt::Display for State { match self { Self::Archived => write!(f, "archived"), Self::Draft => write!(f, "draft"), - Self::Open => write!(f, "open"), - Self::Merged => write!(f, "merged"), + Self::Open { .. } => write!(f, "open"), + Self::Merged { .. } => write!(f, "merged"), } } } @@ -561,11 +614,11 @@ impl fmt::Display for State { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] #[serde(rename_all = "camelCase")] pub struct Merge { - /// Owner of repository that this patch was merged into. - pub node: NodeId, + /// Revision that was merged. + pub revision: RevisionId, /// Base branch commit that contains the revision. pub commit: git::Oid, - /// When this merged was performed. + /// When this merge was performed. pub timestamp: Timestamp, } @@ -1002,14 +1055,14 @@ impl<'a, 'g> PatchMut<'a, 'g> { if !self.is_draft() { return Ok(false); } - self.lifecycle(State::Open, signer)?; + self.lifecycle(State::Open { conflicts: vec![] }, signer)?; Ok(true) } - /// Mark an open patch as a draft. Returns `false` if the patch was not open. + /// Mark an open patch as a draft. Returns `false` if the patch was not open and free of merges. pub fn unready(&mut self, signer: &G) -> Result { - if !self.is_open() { + if !matches!(self.state(), State::Open { conflicts } if conflicts.is_empty()) { return Ok(false); } self.lifecycle(State::Draft, signer)?; @@ -1120,9 +1173,9 @@ impl<'a> Patches<'a> { .fold(PatchCounts::default(), |mut state, (_, p, _)| { match p.state() { State::Draft => state.draft += 1, - State::Open => state.open += 1, + State::Open { .. } => state.open += 1, State::Archived => state.archived += 1, - State::Merged => state.merged += 1, + State::Merged { .. } => state.merged += 1, } state }); @@ -1229,10 +1282,12 @@ mod test { use radicle_crdt::test::{assert_laws, WeightedGenerator}; + use nonempty::nonempty; use pretty_assertions::assert_eq; use qcheck::{Arbitrary, TestResult}; use super::*; + use crate::assert_matches; use crate::cob::test::Actor; use crate::crypto::test::signer::MockSigner; use crate::test; @@ -1429,7 +1484,7 @@ mod test { assert_eq!(patch.title(), "My first patch"); assert_eq!(patch.description(), "Blah blah blah."); assert_eq!(patch.author().id(), &author); - assert_eq!(patch.state(), State::Open); + assert_eq!(patch.state(), &State::Open { conflicts: vec![] }); assert_eq!(patch.target(), target); assert_eq!(patch.version(), 0); @@ -1504,13 +1559,63 @@ mod test { let patch = patches.get(&id).unwrap().unwrap(); - let (_, r) = patch.revisions().next().unwrap(); - let merges = r.merges.iter().collect::>(); + let merges = patch.merges.iter().collect::>(); assert_eq!(merges.len(), 1); - let merge = merges.first().unwrap(); - assert_eq!(merge.node, *signer.public_key()); - assert_eq!(merge.commit, pr.base); + let (merger, merge) = merges.first().unwrap(); + assert_eq!(*merger, signer.public_key()); + assert_eq!(merge.get().unwrap().commit, pr.base); + } + + #[test] + fn test_patch_merge_and_archive() { + let rid = gen::(1); + let base = git::Oid::from_str("d8711a8d43dc919fe39ae4b7c2f7b24667f5d470").unwrap(); + let commit = git::Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap(); + + let mut alice = Actor::::default(); + let mut bob = Actor::::default(); + + let proj = gen::(1); + let doc = Doc::new(proj, nonempty![alice.did(), bob.did()], 1) + .verified() + .unwrap(); + let repo = MockRepository::new(rid, doc); + let patch = alice + .patch("Some changes", "", base, commit, &repo) + .unwrap(); + let (revision, _) = patch.revisions().next().unwrap(); + + // Create two concurrent operations. + let clock = Lamport::from(2); + let identity = repo.identity_head().unwrap(); + let ops = [ + alice.op_with( + Action::Merge { + revision: *revision, + commit, + }, + clock, + identity, + ), + bob.op_with( + Action::Lifecycle { + state: State::Archived, + }, + clock, + identity, + ), + ]; + + let mut patch1 = patch.clone(); + let mut patch2 = patch.clone(); + + // Apply the ops in different orders and expect the patch state to remain the same. + patch1.apply(ops.iter().cloned(), &repo).unwrap(); + patch2.apply(ops.iter().cloned().rev(), &repo).unwrap(); + + assert_matches!(patch1.state(), &State::Merged { .. }); + assert_matches!(patch2.state(), &State::Merged { .. }); } #[test] @@ -1615,8 +1720,12 @@ mod test { fn test_revision_merge_reinsert() { let base = git::Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap(); let oid = git::Oid::from_str("518d5069f94c03427f694bb494ac1cd7d1339380").unwrap(); - let repo = gen::(1); + let id = gen::(1); let mut alice = Actor::<_, Action>::new(MockSigner::default()); + let mut doc = gen::>(1); + doc.delegates.push(alice.signer.public_key().into()); + let repo = MockRepository::new(id, doc); + let mut p1 = Patch::default(); let mut p2 = Patch::default(); diff --git a/radicle/src/cob/test.rs b/radicle/src/cob/test.rs index 5de120a8..18cd51b9 100644 --- a/radicle/src/cob/test.rs +++ b/radicle/src/cob/test.rs @@ -7,10 +7,15 @@ use serde::Serialize; use crate::cob::common::clock; use crate::cob::op::{Op, Ops}; +use crate::cob::patch; +use crate::cob::patch::Patch; use crate::cob::store::encoding; use crate::cob::History; use crate::crypto::{PublicKey, Signer}; +use crate::git; use crate::git::Oid; +use crate::prelude::Did; +use crate::storage::ReadRepository; use crate::test::arbitrary; use super::store::FromHistory; @@ -146,22 +151,62 @@ impl Actor { } /// Create a new operation. - pub fn op(&mut self, action: A) -> Op { + pub fn op_with(&mut self, action: A, clock: clock::Lamport, identity: Oid) -> Op { let id = arbitrary::oid().into(); let author = *self.signer.public_key(); - let clock = self.clock.tick(); let timestamp = clock::Physical::now(); - let identity = arbitrary::oid(); - let op = Op { + + Op { id, action, author, clock, timestamp, identity, - }; - self.ops.insert((self.clock, author), op.clone()); + } + } + + /// Create a new operation. + pub fn op(&mut self, action: A) -> Op { + let clock = self.clock.tick(); + let identity = arbitrary::oid(); + let op = self.op_with(action, clock, identity); + + self.ops.insert((self.clock, op.author), op.clone()); op } + + /// Get the actor's DID. + pub fn did(&self) -> Did { + self.signer.public_key().into() + } +} + +impl Actor { + /// Create a patch. + pub fn patch( + &mut self, + title: impl ToString, + description: impl ToString, + base: git::Oid, + oid: git::Oid, + repo: &R, + ) -> Result { + Patch::from_ops( + [ + self.op(patch::Action::Revision { + description: description.to_string(), + base, + oid, + }), + self.op(patch::Action::Edit { + title: title.to_string(), + description: description.to_string(), + target: patch::MergeTarget::default(), + }), + ], + repo, + ) + } } diff --git a/radicle/src/test.rs b/radicle/src/test.rs index 1911dc65..8144117b 100644 --- a/radicle/src/test.rs +++ b/radicle/src/test.rs @@ -59,6 +59,7 @@ pub mod setup { let oid = commit(&self.working, &refname, blobs, &[&parent]); git::push(&self.working, &REMOTE_NAME, [(&refname, &refname)]).unwrap(); + BranchWith { base: base.into(), oid, diff --git a/radicle/src/test/storage.rs b/radicle/src/test/storage.rs index 83625c27..d3602b09 100644 --- a/radicle/src/test/storage.rs +++ b/radicle/src/test/storage.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use std::io; use std::path::{Path, PathBuf}; +use std::str::FromStr; use git_ext::ref_format as fmt; @@ -222,11 +223,11 @@ impl ReadRepository for MockRepository { } fn identity_head(&self) -> Result { - todo!() + self.canonical_identity_head() } fn canonical_identity_head(&self) -> Result { - todo!() + Ok(Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap()) } }