From ce11d69f233d5bf6ec0bce2f02489946e0f2fb28 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 1 Dec 2022 13:21:22 +0100 Subject: [PATCH] cob: Move tags out of `Thread` Signed-off-by: Alexis Sellier --- radicle/src/cob/issue.rs | 32 ++++++--- radicle/src/cob/patch.rs | 23 +++++-- radicle/src/cob/thread.rs | 132 ++++++++++---------------------------- 3 files changed, 74 insertions(+), 113 deletions(-) diff --git a/radicle/src/cob/issue.rs b/radicle/src/cob/issue.rs index 40a3fc8b..9e130ad0 100644 --- a/radicle/src/cob/issue.rs +++ b/radicle/src/cob/issue.rs @@ -4,7 +4,7 @@ use std::str::FromStr; use once_cell::sync::Lazy; use radicle_crdt as crdt; use radicle_crdt::clock; -use radicle_crdt::{ChangeId, LWWReg, Max, Semilattice}; +use radicle_crdt::{ChangeId, LWWReg, LWWSet, Max, Semilattice}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -76,6 +76,7 @@ pub struct Issue { // TODO(cloudhead): Title should bias towards shorter strings. title: LWWReg, clock::Lamport>, status: LWWReg, clock::Lamport>, + tags: LWWSet, thread: Thread, } @@ -92,6 +93,7 @@ impl Default for Issue { Self { title: Max::from(String::default()).into(), status: Max::from(Status::default()).into(), + tags: LWWSet::default(), thread: Thread::default(), } } @@ -138,7 +140,7 @@ impl Issue { } pub fn tags(&self) -> impl Iterator { - self.thread.tags() + self.tags.iter() } pub fn author(&self) -> Option { @@ -164,6 +166,14 @@ impl Issue { Action::Lifecycle { status } => { self.status.set(status, change.clock); } + Action::Tag { add, remove } => { + for tag in add { + self.tags.insert(tag, change.clock); + } + for tag in remove { + self.tags.remove(tag, change.clock); + } + } Action::Thread { action } => { self.thread.apply([crdt::Change { action, @@ -221,13 +231,14 @@ impl<'a, 'g> IssueMut<'a, 'g> { /// Tag an issue. pub fn tag( &mut self, - tags: impl IntoIterator, + add: impl IntoIterator, + remove: impl IntoIterator, signer: &G, ) -> Result { - let tags = tags.into_iter().collect::>(); - let action = Action::Thread { - action: thread::Action::Tag { tags }, - }; + let add = add.into_iter().collect::>(); + let remove = remove.into_iter().collect::>(); + let action = Action::Tag { add, remove }; + self.apply("Tag", action, signer) } @@ -362,7 +373,7 @@ impl<'a> Issues<'a> { }; issue.comment(description, signer)?; - issue.tag(tags.to_owned(), signer)?; + issue.tag(tags.to_owned(), [], signer)?; Ok(issue) } @@ -378,6 +389,7 @@ impl<'a> Issues<'a> { pub enum Action { Title { title: String }, Lifecycle { status: Status }, + Tag { add: Vec, remove: Vec }, Thread { action: thread::Action }, } @@ -519,8 +531,8 @@ mod test { let bug_tag = Tag::new("bug").unwrap(); let wontfix_tag = Tag::new("wontfix").unwrap(); - issue.tag([bug_tag.clone()], &signer).unwrap(); - issue.tag([wontfix_tag.clone()], &signer).unwrap(); + issue.tag([bug_tag.clone()], [], &signer).unwrap(); + issue.tag([wontfix_tag.clone()], [], &signer).unwrap(); let id = issue.id; let issue = issues.get(&id).unwrap().unwrap(); diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index cbbdb6d8..dc8e180e 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -823,7 +823,7 @@ mod test { impl Arbitrary for Changes { fn arbitrary(g: &mut quickcheck::Gen) -> Self { - type State = (clock::Lamport, Vec); + type State = (clock::Lamport, Vec, Vec); let author = ActorId::from([0; 32]); let rng = fastrand::Rng::with_seed(u64::arbitrary(g)); @@ -840,7 +840,7 @@ mod test { .collect::>(); let gen = WeightedGenerator::<(clock::Lamport, Action), State>::new(rng.clone()) - .variant(1, |(clock, _), rng| { + .variant(1, |(clock, _, _), rng| { Some(( clock.tick(), Action::Edit { @@ -850,7 +850,7 @@ mod test { }, )) }) - .variant(1, |(clock, revisions), rng| { + .variant(1, |(clock, revisions, _), rng| { if revisions.is_empty() { return None; } @@ -859,7 +859,22 @@ mod test { Some((clock.tick(), Action::Merge { revision, commit })) }) - .variant(1, |(clock, revisions), rng| { + .variant(1, |(clock, _, tags), rng| { + let add = iter::repeat_with(|| rng.alphabetic()) + .take(rng.usize(0..=3)) + .map(|c| Tag::new(c).unwrap()) + .collect::>(); + let remove = tags + .iter() + .take(rng.usize(0..=tags.len())) + .cloned() + .collect(); + for tag in &add { + tags.push(tag.clone()); + } + Some((clock.tick(), Action::Tag { add, remove })) + }) + .variant(1, |(clock, revisions, _), rng| { let oid = oids[rng.usize(..oids.len())]; let base = oids[rng.usize(..oids.len())]; diff --git a/radicle/src/cob/thread.rs b/radicle/src/cob/thread.rs index ff876c77..0b3d915c 100644 --- a/radicle/src/cob/thread.rs +++ b/radicle/src/cob/thread.rs @@ -8,13 +8,12 @@ use once_cell::sync::Lazy; use radicle_crdt as crdt; use serde::{Deserialize, Serialize}; -use crate::cob::common::{Reaction, Tag, Timestamp}; +use crate::cob::common::{Reaction, Timestamp}; use crate::cob::store; use crate::cob::{History, TypeName}; use crate::crypto::Signer; use crdt::clock::Lamport; -use crdt::lwwreg::LWWReg; use crdt::lwwset::LWWSet; use crdt::redactable::Redactable; use crdt::{ActorId, Change, ChangeId, Semilattice}; @@ -70,10 +69,6 @@ pub enum Action { }, /// Redact a change. Not all changes can be redacted. Redact { id: ChangeId }, - /// Add tags to the thread. - Tag { tags: Vec }, - /// Remove tags from the thread. - Untag { tags: Vec }, /// React to a change. React { to: ChangeId, @@ -94,8 +89,6 @@ impl Action { pub struct Thread { /// The comments under the thread. comments: BTreeMap>, - /// Associated tags. - tags: BTreeMap>, /// Reactions to changes. reactions: BTreeMap>, } @@ -128,7 +121,6 @@ impl store::FromHistory for Thread { impl Semilattice for Thread { fn merge(&mut self, other: Self) { self.comments.merge(other.comments); - self.tags.merge(other.tags); self.reactions.merge(other.reactions); } } @@ -211,22 +203,6 @@ impl Thread { .and_modify(|e| e.merge(Redactable::Redacted)) .or_insert(Redactable::Redacted); } - Action::Tag { tags } => { - for tag in tags { - self.tags - .entry(tag) - .and_modify(|r| r.set(true, change.clock)) - .or_insert_with(|| LWWReg::new(true, change.clock)); - } - } - Action::Untag { tags } => { - for tag in tags { - self.tags - .entry(tag) - .and_modify(|r| r.set(false, change.clock)) - .or_insert_with(|| LWWReg::new(false, change.clock)); - } - } Action::React { to, reaction, @@ -264,12 +240,6 @@ impl Thread { } }) } - - pub fn tags(&self) -> impl Iterator + '_ { - self.tags - .iter() - .filter_map(|(tag, r)| if *r.get() { Some(tag) } else { None }) - } } /// An object that can be used to create and sign changes. @@ -305,16 +275,6 @@ impl Actor { }) } - /// Add a tag. - pub fn tag(&mut self, tag: Tag) -> Change { - self.change(Action::Tag { tags: vec![tag] }) - } - - /// Remove a tag. - pub fn untag(&mut self, tag: Tag) -> Change { - self.change(Action::Untag { tags: vec![tag] }) - } - /// Create a new redaction. pub fn redact(&mut self, id: ChangeId) -> Change { self.change(Action::Redact { id }) @@ -371,67 +331,41 @@ mod tests { let author = ActorId::from([0; 32]); let rng = fastrand::Rng::with_seed(u64::arbitrary(g)); let gen = - WeightedGenerator::<(Lamport, Action), (Lamport, Vec, Vec)>::new( - rng.clone(), - ) - .variant(3, |(clock, _, changes), rng| { - changes.push((clock.tick(), author)); + WeightedGenerator::<(Lamport, Action), (Lamport, Vec)>::new(rng.clone()) + .variant(3, |(clock, changes), rng| { + changes.push((clock.tick(), author)); - Some(( - *clock, - Action::Comment { - body: iter::repeat_with(|| rng.alphabetic()).take(16).collect(), - reply_to: None, - }, - )) - }) - .variant(2, |(clock, _, changes), rng| { - if changes.is_empty() { - return None; - } - let to = changes[rng.usize(..changes.len())]; + Some(( + *clock, + Action::Comment { + body: iter::repeat_with(|| rng.alphabetic()).take(16).collect(), + reply_to: None, + }, + )) + }) + .variant(2, |(clock, changes), rng| { + if changes.is_empty() { + return None; + } + let to = changes[rng.usize(..changes.len())]; - Some(( - clock.tick(), - Action::React { - to, - reaction: Reaction::new('✨').unwrap(), - active: rng.bool(), - }, - )) - }) - .variant(2, |(clock, _, changes), rng| { - if changes.is_empty() { - return None; - } - let id = changes[rng.usize(..changes.len())]; + Some(( + clock.tick(), + Action::React { + to, + reaction: Reaction::new('✨').unwrap(), + active: rng.bool(), + }, + )) + }) + .variant(2, |(clock, changes), rng| { + if changes.is_empty() { + return None; + } + let id = changes[rng.usize(..changes.len())]; - Some((clock.tick(), Action::Redact { id })) - }) - .variant(2, |(clock, tags, _), rng| { - let tag = if tags.is_empty() || rng.bool() { - let tag = iter::repeat_with(|| rng.alphabetic()) - .take(8) - .collect::(); - let tag = Tag::new(tag).unwrap(); - - tags.push(tag.clone()); - tag - } else { - tags[rng.usize(..tags.len())].clone() - }; - - Some((clock.tick(), Action::Tag { tags: vec![tag] })) - }) - .variant(2, |(clock, tags, _), rng| { - if tags.is_empty() { - return None; - } - let tag = tags[rng.usize(..tags.len())].clone(); - clock.tick(); - - Some((clock.tick(), Action::Untag { tags: vec![tag] })) - }); + Some((clock.tick(), Action::Redact { id })) + }); let mut changes = Vec::new(); let mut permutations: [Vec>; N] = array::from_fn(|_| Vec::new());