From cc246992324f85d25ec229c713618f281d7ed563 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 1 Dec 2022 20:29:02 +0100 Subject: [PATCH] cob: Rename `Change` to `Op` Helps with the confusion with `radicle_cob::Change` and matches the name for this object in common literature. Signed-off-by: Alexis Sellier --- radicle/src/cob.rs | 4 +- radicle/src/cob/issue.rs | 48 ++++++------- radicle/src/cob/{change.rs => op.rs} | 75 +++++++++----------- radicle/src/cob/patch.rs | 102 +++++++++++++-------------- radicle/src/cob/thread.rs | 40 +++++------ 5 files changed, 132 insertions(+), 137 deletions(-) rename radicle/src/cob/{change.rs => op.rs} (54%) diff --git a/radicle/src/cob.rs b/radicle/src/cob.rs index 18d49f7f..edee3642 100644 --- a/radicle/src/cob.rs +++ b/radicle/src/cob.rs @@ -1,16 +1,16 @@ -pub mod change; pub mod common; pub mod issue; +pub mod op; pub mod patch; pub mod store; pub mod thread; -pub use change::{Actor, ActorId, Change, ChangeId}; pub use cob::{create, get, list, remove, update}; pub use cob::{ identity, object::collaboration::error, CollaborativeObject, Contents, Create, Entry, History, ObjectId, TypeName, Update, }; pub use common::*; +pub use op::{Actor, ActorId, Op, OpId}; use radicle_cob as cob; diff --git a/radicle/src/cob/issue.rs b/radicle/src/cob/issue.rs index bfa90691..e5a25c3c 100644 --- a/radicle/src/cob/issue.rs +++ b/radicle/src/cob/issue.rs @@ -11,12 +11,12 @@ use crate::cob; use crate::cob::common::{Author, Reaction, Tag}; use crate::cob::thread; use crate::cob::thread::{CommentId, Thread}; -use crate::cob::{store, ChangeId, ObjectId, TypeName}; +use crate::cob::{store, ObjectId, OpId, TypeName}; use crate::crypto::{PublicKey, Signer}; use crate::storage::git as storage; -/// Issue change. -pub type Change = crate::cob::Change; +/// Issue operation. +pub type Op = crate::cob::Op; /// Type name of an issue. pub static TYPENAME: Lazy = @@ -111,9 +111,9 @@ impl store::FromHistory for Issue { history: &radicle_cob::History, ) -> Result<(Self, clock::Lamport), store::Error> { let obj = history.traverse(Self::default(), |mut acc, entry| { - if let Ok(change) = Change::try_from(entry) { - if let Err(err) = acc.apply(change) { - log::warn!("Error applying change to issue state: {err}"); + if let Ok(op) = Op::try_from(entry) { + if let Err(err) = acc.apply(op) { + log::warn!("Error applying op to issue state: {err}"); return ControlFlow::Break(acc); } } else { @@ -154,28 +154,28 @@ impl Issue { self.thread.comments().map(|(id, comment)| (id, comment)) } - pub fn apply(&mut self, change: Change) -> Result<(), Error> { - match change.action { + pub fn apply(&mut self, op: Op) -> Result<(), Error> { + match op.action { Action::Title { title } => { - self.title.set(title, change.clock); + self.title.set(title, op.clock); } Action::Lifecycle { status } => { - self.status.set(status, change.clock); + self.status.set(status, op.clock); } Action::Tag { add, remove } => { for tag in add { - self.tags.insert(tag, change.clock); + self.tags.insert(tag, op.clock); } for tag in remove { - self.tags.remove(tag, change.clock); + self.tags.remove(tag, op.clock); } } Action::Thread { action } => { - self.thread.apply([cob::Change { + self.thread.apply([cob::Op { action, - author: change.author, - clock: change.clock, - timestamp: change.timestamp, + author: op.author, + clock: op.clock, + timestamp: op.timestamp, }]); } } @@ -205,7 +205,7 @@ impl<'a, 'g> IssueMut<'a, 'g> { } /// Lifecycle an issue. - pub fn lifecycle(&mut self, status: Status, signer: &G) -> Result { + pub fn lifecycle(&mut self, status: Status, signer: &G) -> Result { let action = Action::Lifecycle { status }; self.apply("Lifecycle", action, signer) } @@ -230,7 +230,7 @@ impl<'a, 'g> IssueMut<'a, 'g> { add: impl IntoIterator, remove: impl IntoIterator, signer: &G, - ) -> Result { + ) -> Result { let add = add.into_iter().collect::>(); let remove = remove.into_iter().collect::>(); let action = Action::Tag { add, remove }; @@ -244,7 +244,7 @@ impl<'a, 'g> IssueMut<'a, 'g> { parent: CommentId, body: S, signer: &G, - ) -> Result { + ) -> Result { let body = body.into(); assert!(self.thread.comment(&parent).is_some()); @@ -262,7 +262,7 @@ impl<'a, 'g> IssueMut<'a, 'g> { to: CommentId, reaction: Reaction, signer: &G, - ) -> Result { + ) -> Result { let action = Action::Thread { action: thread::Action::React { to, @@ -273,26 +273,26 @@ impl<'a, 'g> IssueMut<'a, 'g> { self.apply("React", action, signer) } - /// Apply a change to the issue. + /// Apply an op to the issue. pub fn apply( &mut self, msg: &'static str, action: Action, signer: &G, - ) -> Result { + ) -> Result { let cob = self .store .update(self.id, msg, action.clone(), signer) .map_err(Error::Store)?; let clock = cob.history().clock().into(); let timestamp = cob.history().timestamp().into(); - let change = Change { + let op = Op { action, author: *signer.public_key(), clock, timestamp, }; - self.issue.apply(change)?; + self.issue.apply(op)?; Ok((clock, *signer.public_key())) } diff --git a/radicle/src/cob/change.rs b/radicle/src/cob/op.rs similarity index 54% rename from radicle/src/cob/change.rs rename to radicle/src/cob/op.rs index 58920fdf..ce247123 100644 --- a/radicle/src/cob/change.rs +++ b/radicle/src/cob/op.rs @@ -8,40 +8,41 @@ use radicle_crdt::clock; use radicle_crdt::clock::Lamport; use radicle_crypto::{PublicKey, Signer}; -/// Identifies a change. -pub type ChangeId = (Lamport, ActorId); -/// The author of a change. +/// Identifies an [`Op`]. +pub type OpId = (Lamport, ActorId); +/// The author of an [`Op`]. pub type ActorId = PublicKey; -/// Error decoding a change from an entry. +/// Error decoding an operation from an entry. #[derive(Error, Debug)] -pub enum ChangeDecodeError { +pub enum OpDecodeError { #[error("deserialization from json failed: {0}")] Deserialize(#[from] serde_json::Error), } -/// The `Change` is the unit of replication. -/// Everything that can be done in the system is represented by a `Change` object. -/// Changes are applied to an accumulator to yield a final state. +/// The `Op` is the operation that is applied onto a state to form a CRDT. +/// +/// Everything that can be done in the system is represented by an `Op`. +/// Operations are applied to an accumulator to yield a final state. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub struct Change { - /// The action carried out by this change. +pub struct Op { + /// The action carried out by this operation. pub action: A, - /// The author of the change. + /// The author of the operation. pub author: ActorId, /// Lamport clock. pub clock: Lamport, - /// Timestamp of this change. + /// Timestamp of this operation. pub timestamp: clock::Physical, } -impl<'a: 'de, 'de, A: serde::Deserialize<'de>> TryFrom<&'a EntryWithClock> for Change { - type Error = ChangeDecodeError; +impl<'a: 'de, 'de, A: serde::Deserialize<'de>> TryFrom<&'a EntryWithClock> for Op { + type Error = OpDecodeError; fn try_from(entry: &'a EntryWithClock) -> Result { let action = serde_json::from_slice(entry.contents())?; - Ok(Change { + Ok(Op { action, author: *entry.actor(), clock: entry.clock().into(), @@ -50,26 +51,20 @@ impl<'a: 'de, 'de, A: serde::Deserialize<'de>> TryFrom<&'a EntryWithClock> for C } } -impl Change { - /// Get the change id. - pub fn id(&self) -> ChangeId { +impl Op { + /// Get the op id. + /// This uniquely identifies each operation in the CRDT. + pub fn id(&self) -> OpId { (self.clock, self.author) } } -impl<'de, A: Deserialize<'de>> Change { - /// Deserialize a change from a byte string. - pub fn decode(bytes: &'de [u8]) -> Result { - serde_json::from_slice(bytes) - } -} - -/// An object that can be used to create and sign changes. +/// An object that can be used to create and sign operations. #[derive(Default)] pub struct Actor { pub signer: G, pub clock: Lamport, - pub changes: BTreeMap<(Lamport, PublicKey), Change>, + pub ops: BTreeMap<(Lamport, PublicKey), Op>, } impl Actor { @@ -77,15 +72,15 @@ impl Actor { Self { signer, clock: Lamport::default(), - changes: BTreeMap::default(), + ops: BTreeMap::default(), } } - pub fn receive(&mut self, changes: impl IntoIterator>) -> Lamport { - for change in changes { - let clock = change.clock; + pub fn receive(&mut self, ops: impl IntoIterator>) -> Lamport { + for op in ops { + let clock = op.clock; - self.changes.insert((clock, change.author), change); + self.ops.insert((clock, op.author), op); self.clock.merge(clock); } self.clock @@ -93,29 +88,29 @@ impl Actor { /// Reset actor state to initial state. pub fn reset(&mut self) { - self.changes.clear(); + self.ops.clear(); self.clock = Lamport::default(); } /// Returned an ordered list of events. - pub fn timeline(&self) -> impl Iterator> { - self.changes.values() + pub fn timeline(&self) -> impl Iterator> { + self.ops.values() } - /// Create a new change. - pub fn change(&mut self, action: A) -> Change { + /// Create a new operation. + pub fn op(&mut self, action: A) -> Op { let author = *self.signer.public_key(); let clock = self.clock; let timestamp = clock::Physical::now(); - let change = Change { + let op = Op { action, author, clock, timestamp, }; - self.changes.insert((self.clock, author), change.clone()); + self.ops.insert((self.clock, author), op.clone()); self.clock.tick(); - change + op } } diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 875f79b7..319f44a8 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -16,27 +16,27 @@ use crate::cob::common::{Author, Tag, Timestamp}; use crate::cob::thread; use crate::cob::thread::CommentId; use crate::cob::thread::Thread; -use crate::cob::{store, ActorId, ChangeId, ObjectId, TypeName}; +use crate::cob::{store, ActorId, ObjectId, OpId, TypeName}; use crate::crypto::{PublicKey, Signer}; use crate::git; use crate::prelude::*; use crate::storage::git as storage; -/// The logical clock we use to order changes to patches. +/// The logical clock we use to order operations to patches. pub use clock::Lamport as Clock; /// Type name of a patch. pub static TYPENAME: Lazy = Lazy::new(|| FromStr::from_str("xyz.radicle.patch").expect("type name is valid")); -/// Patch change. -pub type Change = crate::cob::Change; +/// Patch operation. +pub type Op = crate::cob::Op; /// Identifier for a patch. pub type PatchId = ObjectId; /// Unique identifier for a patch revision. -pub type RevisionId = ChangeId; +pub type RevisionId = OpId; /// Index of a revision in the revisions list. pub type RevisionIx = usize; @@ -49,10 +49,10 @@ pub enum ApplyError { /// This error indicates that the operations are not being applied /// in causal order, which is a requirement for this CRDT. /// - /// For example, this can occur if a change references another change + /// For example, this can occur if an operation references anothern operation /// that hasn't happened yet. #[error("causal dependency {0:?} missing")] - Missing(ChangeId), + Missing(OpId), } /// Error updating or creating patches. @@ -221,36 +221,36 @@ impl Patch { matches!(self.status.get().get(), &Status::Archived) } - /// Apply a list of changes to the state. - pub fn apply(&mut self, changes: impl IntoIterator) -> Result<(), ApplyError> { - for change in changes { - self.apply_one(change)?; + /// Apply a list of operations to the state. + pub fn apply(&mut self, ops: impl IntoIterator) -> Result<(), ApplyError> { + for op in ops { + self.apply_one(op)?; } Ok(()) } - /// Apply a single change to the state. - pub fn apply_one(&mut self, change: Change) -> Result<(), ApplyError> { - let id = change.id(); - let author = Author::new(change.author); - let timestamp = change.timestamp; + /// Apply a single op to the state. + pub fn apply_one(&mut self, op: Op) -> Result<(), ApplyError> { + let id = op.id(); + let author = Author::new(op.author); + let timestamp = op.timestamp; - match change.action { + match op.action { Action::Edit { title, description, target, } => { - self.title.set(title, change.clock); - self.description.set(description, change.clock); - self.target.set(target, change.clock); + self.title.set(title, op.clock); + self.description.set(description, op.clock); + self.target.set(target, op.clock); } Action::Tag { add, remove } => { for tag in add { - self.tags.insert(tag, change.clock); + self.tags.insert(tag, op.clock); } for tag in remove { - self.tags.remove(tag, change.clock); + self.tags.remove(tag, op.clock); } } Action::Revision { base, oid } => { @@ -274,7 +274,7 @@ impl Patch { } => { if let Some(Redactable::Present(revision)) = self.revisions.get_mut(&revision) { revision.reviews.insert( - change.author, + op.author, Review::new(verdict, comment.to_owned(), inline.to_owned(), timestamp), ); } else { @@ -285,12 +285,12 @@ impl Patch { if let Some(Redactable::Present(revision)) = self.revisions.get_mut(&revision) { revision.merges.insert( Merge { - node: change.author, + node: op.author, commit, timestamp, } .into(), - change.clock, + op.clock, ); } else { return Err(ApplyError::Missing(revision)); @@ -300,10 +300,10 @@ impl Patch { // TODO(cloudhead): Make sure we can deal with redacted revisions which are added // to out of order, like in the `Merge` case. if let Some(Redactable::Present(revision)) = self.revisions.get_mut(&revision) { - revision.discussion.apply([cob::Change { + revision.discussion.apply([cob::Op { action, - author: change.author, - clock: change.clock, + author: op.author, + clock: op.clock, timestamp, }]); } else { @@ -326,9 +326,9 @@ impl store::FromHistory for Patch { history: &radicle_cob::History, ) -> Result<(Self, clock::Lamport), store::Error> { let obj = history.traverse(Self::default(), |mut acc, entry| { - if let Ok(change) = Change::try_from(entry) { - if let Err(err) = acc.apply([change]) { - log::warn!("Error applying change to patch state: {err}"); + if let Ok(op) = Op::try_from(entry) { + if let Err(err) = acc.apply([op]) { + log::warn!("Error applying op to patch state: {err}"); return ControlFlow::Break(acc); } } else { @@ -554,7 +554,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { description: String, target: MergeTarget, signer: &G, - ) -> Result { + ) -> Result { let action = Action::Edit { title, description, @@ -589,7 +589,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { comment: Option, inline: Vec, signer: &G, - ) -> Result { + ) -> Result { let action = Action::Review { revision, comment, @@ -605,7 +605,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { revision: RevisionId, commit: git::Oid, signer: &G, - ) -> Result { + ) -> Result { let action = Action::Merge { revision, commit }; self.apply("Merge revision", action, signer) } @@ -617,7 +617,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { base: impl Into, oid: impl Into, signer: &G, - ) -> Result { + ) -> Result { let description = description.into(); let base = base.into(); let oid = oid.into(); @@ -637,7 +637,7 @@ impl<'a, 'g> PatchMut<'a, 'g> { add: impl IntoIterator, remove: impl IntoIterator, signer: &G, - ) -> Result { + ) -> Result { let add = add.into_iter().collect::>(); let remove = remove.into_iter().collect::>(); let action = Action::Tag { add, remove }; @@ -645,26 +645,26 @@ impl<'a, 'g> PatchMut<'a, 'g> { self.apply("Tag", action, signer) } - /// Apply a change to the patch. + /// Apply an operation to the patch. pub fn apply( &mut self, msg: &'static str, action: Action, signer: &G, - ) -> Result { + ) -> Result { let cob = self .store .update(self.id, msg, action.clone(), signer) .map_err(Error::Store)?; let clock = cob.history().clock().into(); let timestamp = cob.history().timestamp().into(); - let change = Change { + let op = Op { action, author: *signer.public_key(), clock, timestamp, }; - self.patch.apply_one(change)?; + self.patch.apply_one(op)?; Ok((clock, *signer.public_key())) } @@ -781,13 +781,13 @@ mod test { use quickcheck::{Arbitrary, TestResult}; use super::*; - use crate::cob::change::{Actor, ActorId}; + use crate::cob::op::{Actor, ActorId}; use crate::crypto::test::signer::MockSigner; use crate::test; #[derive(Clone)] struct Changes { - permutations: [Vec; N], + permutations: [Vec; N], } impl std::fmt::Debug for Changes { @@ -805,7 +805,7 @@ mod test { impl Arbitrary for Changes { fn arbitrary(g: &mut quickcheck::Gen) -> Self { - type State = (clock::Lamport, Vec, Vec); + type State = (clock::Lamport, Vec, Vec); let author = ActorId::from([0; 32]); let rng = fastrand::Rng::with_seed(u64::arbitrary(g)); @@ -875,11 +875,11 @@ mod test { }); let mut changes = Vec::new(); - let mut permutations: [Vec; N] = array::from_fn(|_| Vec::new()); + let mut permutations: [Vec; N] = array::from_fn(|_| Vec::new()); let timestamp = Timestamp::now() + rng.u64(..60); for (clock, action) in gen.take(g.size()) { - changes.push(Change { + changes.push(Op { action, author, clock, @@ -1054,15 +1054,15 @@ mod test { let mut alice = Actor::<_, Action>::new(MockSigner::default()); let mut patch = Patch::default(); - let a1 = alice.change(Action::Revision { base, oid }); - let a2 = alice.change(Action::Redact { revision: a1.id() }); - let a3 = alice.change(Action::Review { + let a1 = alice.op(Action::Revision { base, oid }); + let a2 = alice.op(Action::Redact { revision: a1.id() }); + let a3 = alice.op(Action::Review { revision: a1.id(), comment: None, verdict: Some(Verdict::Accept), inline: vec![], }); - let a4 = alice.change(Action::Merge { + let a4 = alice.op(Action::Merge { revision: a1.id(), commit: oid, }); @@ -1085,8 +1085,8 @@ mod test { let mut p1 = Patch::default(); let mut p2 = Patch::default(); - let a1 = alice.change(Action::Revision { base, oid }); - let a2 = alice.change(Action::Redact { revision: a1.id() }); + let a1 = alice.op(Action::Revision { base, oid }); + let a2 = alice.op(Action::Redact { revision: a1.id() }); p1.apply([a1.clone(), a2.clone(), a1.clone()]).unwrap(); p2.apply([a1.clone(), a1, a2]).unwrap(); diff --git a/radicle/src/cob/thread.rs b/radicle/src/cob/thread.rs index 78e771bd..b4602ab3 100644 --- a/radicle/src/cob/thread.rs +++ b/radicle/src/cob/thread.rs @@ -11,7 +11,7 @@ use serde::{Deserialize, Serialize}; use crate::cob; use crate::cob::common::{Reaction, Timestamp}; use crate::cob::store; -use crate::cob::{ActorId, Change, ChangeId, History, TypeName}; +use crate::cob::{ActorId, History, Op, OpId, TypeName}; use crate::crypto::Signer; use crdt::clock::Lamport; @@ -24,7 +24,7 @@ pub static TYPENAME: Lazy = Lazy::new(|| FromStr::from_str("xyz.radicle.thread").expect("type name is valid")); /// Identifies a comment. -pub type CommentId = ChangeId; +pub type CommentId = OpId; /// A comment on a discussion thread. #[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -32,14 +32,14 @@ pub struct Comment { /// The comment body. pub body: String, /// Thread or comment this is a reply to. - pub reply_to: Option, + pub reply_to: Option, /// When the comment was authored. pub timestamp: Timestamp, } impl Comment { /// Create a new comment. - pub fn new(body: String, reply_to: Option, timestamp: Timestamp) -> Self { + pub fn new(body: String, reply_to: Option, timestamp: Timestamp) -> Self { Self { body, reply_to, @@ -66,13 +66,13 @@ pub enum Action { /// Comment body. body: String, /// Another comment this is a reply to. - reply_to: Option, + reply_to: Option, }, /// Redact a change. Not all changes can be redacted. - Redact { id: ChangeId }, + Redact { id: OpId }, /// React to a change. React { - to: ChangeId, + to: OpId, reaction: Reaction, active: bool, }, @@ -96,7 +96,7 @@ impl store::FromHistory for Thread { fn from_history(history: &History) -> Result<(Self, Lamport), store::Error> { let obj = history.traverse(Thread::default(), |mut acc, entry| { - if let Ok(change) = Change::try_from(entry) { + if let Ok(change) = Op::try_from(entry) { acc.apply([change]); ControlFlow::Continue(acc) } else { @@ -168,7 +168,7 @@ impl Thread { .map(|(a, r)| (a, r)) } - pub fn apply(&mut self, changes: impl IntoIterator>) { + pub fn apply(&mut self, changes: impl IntoIterator>) { for change in changes.into_iter() { let id = change.id(); @@ -257,16 +257,16 @@ impl Actor { } /// Create a new comment. - pub fn comment(&mut self, body: &str, reply_to: Option) -> Change { - self.change(Action::Comment { + pub fn comment(&mut self, body: &str, reply_to: Option) -> Op { + self.op(Action::Comment { body: String::from(body), reply_to, }) } /// Create a new redaction. - pub fn redact(&mut self, id: ChangeId) -> Change { - self.change(Action::Redact { id }) + pub fn redact(&mut self, id: OpId) -> Op { + self.op(Action::Redact { id }) } } @@ -299,7 +299,7 @@ mod tests { #[derive(Clone)] struct Changes { - permutations: [Vec>; N], + permutations: [Vec>; N], } impl std::fmt::Debug for Changes { @@ -320,7 +320,7 @@ mod tests { let author = ActorId::from([0; 32]); let rng = fastrand::Rng::with_seed(u64::arbitrary(g)); let gen = - WeightedGenerator::<(Lamport, Action), (Lamport, Vec)>::new(rng.clone()) + WeightedGenerator::<(Lamport, Action), (Lamport, Vec)>::new(rng.clone()) .variant(3, |(clock, changes), rng| { changes.push((clock.tick(), author)); @@ -357,11 +357,11 @@ mod tests { }); let mut changes = Vec::new(); - let mut permutations: [Vec>; N] = array::from_fn(|_| Vec::new()); + let mut permutations: [Vec>; N] = array::from_fn(|_| Vec::new()); let timestamp = Timestamp::now() + rng.u64(..60); for (clock, action) in gen.take(g.size().min(8)) { - changes.push(Change { + changes.push(Op { action, author, clock, @@ -493,9 +493,9 @@ mod tests { eve.receive([a2.clone()]); bob.receive([a2.clone()]); - assert_eq!(alice.changes.len(), 7); - assert_eq!(bob.changes.len(), 7); - assert_eq!(eve.changes.len(), 7); + assert_eq!(alice.ops.len(), 7); + assert_eq!(bob.ops.len(), 7); + assert_eq!(eve.ops.len(), 7); assert_eq!( bob.timeline().collect::>(),