Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
Start adding support for replies in COBs
Alexis Sellier committed 3 years ago
commit b32ae984ce87dba7f5732aa3c5119e0791d45190
parent ac3ee5f7090daf85080f1998f5f669c4ab796825
3 files changed +178 -115
modified radicle/src/cob/issue.rs
@@ -209,14 +209,22 @@ impl store::Transaction<Issue> {
        self.push(Action::Lifecycle { state })
    }

-
    /// Comment on an issue.
-
    pub fn comment<S: ToString>(&mut self, body: S) -> CommentId {
+
    /// Create the issue thread.
+
    pub fn thread<S: ToString>(&mut self, body: S) -> CommentId {
        self.push(Action::from(thread::Action::Comment {
            body: body.to_string(),
            reply_to: None,
        }))
    }

+
    /// Comment on an issue.
+
    pub fn comment<S: ToString>(&mut self, body: S, reply_to: CommentId) -> CommentId {
+
        self.push(Action::from(thread::Action::Comment {
+
            body: body.to_string(),
+
            reply_to: Some(reply_to),
+
        }))
+
    }
+

    /// Tag an issue.
    pub fn tag(
        &mut self,
@@ -229,16 +237,6 @@ impl store::Transaction<Issue> {
        self.push(Action::Tag { add, remove })
    }

-
    /// Reply to on an issue comment.
-
    pub fn reply<S: ToString>(&mut self, parent: CommentId, body: S) -> OpId {
-
        let body = body.to_string();
-

-
        self.push(Action::from(thread::Action::Comment {
-
            body,
-
            reply_to: Some(parent),
-
        }))
-
    }
-

    /// React to an issue comment.
    pub fn react(&mut self, to: CommentId, reaction: Reaction) -> OpId {
        self.push(Action::Thread {
@@ -269,13 +267,24 @@ impl<'a, 'g> IssueMut<'a, 'g> {
        self.transaction("Lifecycle", signer, |tx| tx.lifecycle(state))
    }

+
    /// Create the issue thread.
+
    pub fn thread<G: Signer, S: ToString>(
+
        &mut self,
+
        body: S,
+
        signer: &G,
+
    ) -> Result<CommentId, Error> {
+
        self.transaction("Create thread", signer, |tx| tx.thread(body))
+
    }
+

    /// Comment on an issue.
    pub fn comment<G: Signer, S: ToString>(
        &mut self,
        body: S,
+
        reply_to: CommentId,
        signer: &G,
    ) -> Result<CommentId, Error> {
-
        self.transaction("Comment", signer, |tx| tx.comment(body))
+
        assert!(self.thread.comment(&reply_to).is_some());
+
        self.transaction("Comment", signer, |tx| tx.comment(body, reply_to))
    }

    /// Tag an issue.
@@ -288,17 +297,6 @@ impl<'a, 'g> IssueMut<'a, 'g> {
        self.transaction("Tag", signer, |tx| tx.tag(add, remove))
    }

-
    /// Reply to on an issue comment.
-
    pub fn reply<G: Signer, S: ToString>(
-
        &mut self,
-
        parent: CommentId,
-
        body: S,
-
        signer: &G,
-
    ) -> Result<OpId, Error> {
-
        assert!(self.thread.comment(&parent).is_some());
-
        self.transaction("Reply", signer, |tx| tx.reply(parent, body))
-
    }
-

    /// React to an issue comment.
    pub fn react<G: Signer>(
        &mut self,
@@ -391,8 +389,8 @@ impl<'a> Issues<'a> {
    ) -> Result<IssueMut<'a, 'g>, Error> {
        let (id, issue, clock) =
            Transaction::initial("Create issue", &mut self.raw, signer, |tx| {
+
                tx.thread(description);
                tx.edit(title);
-
                tx.comment(description);
                tx.tag(tags.to_owned(), []);
            })?;
        // Just a sanity check that our clock is advancing as expected.
@@ -524,22 +522,37 @@ mod test {
    fn test_issue_reply() {
        let tmp = tempfile::tempdir().unwrap();
        let (_, signer, project) = test::setup::context(&tmp);
+
        let author = *signer.public_key();
        let mut issues = Issues::open(*signer.public_key(), &project).unwrap();
        let mut issue = issues
            .create("My first issue", "Blah blah blah.", &[], &signer)
            .unwrap();
-
        let comment = issue.comment("Ho ho ho.", &signer).unwrap();
+
        let root = (clock::Lamport::default(), author);

-
        issue.reply(comment, "Hi hi hi.", &signer).unwrap();
-
        issue.reply(comment, "Ha ha ha.", &signer).unwrap();
+
        let c1 = issue.comment("Hi hi hi.", root, &signer).unwrap();
+
        let c2 = issue.comment("Ha ha ha.", root, &signer).unwrap();

        let id = issue.id;
-
        let issue = issues.get(&id).unwrap().unwrap();
-
        let (_, reply1) = &issue.replies(&comment).nth(0).unwrap();
-
        let (_, reply2) = &issue.replies(&comment).nth(1).unwrap();
+
        let mut issue = issues.get_mut(&id).unwrap();
+
        let (_, reply1) = &issue.replies(&root).nth(0).unwrap();
+
        let (_, reply2) = &issue.replies(&root).nth(1).unwrap();

        assert_eq!(reply1.body, "Hi hi hi.");
        assert_eq!(reply2.body, "Ha ha ha.");
+

+
        issue.comment("Re: Hi.", c1, &signer).unwrap();
+
        issue.comment("Re: Ha.", c2, &signer).unwrap();
+
        issue.comment("Re: Ha. Ha.", c2, &signer).unwrap();
+
        issue.comment("Re: Ha. Ha. Ha.", c2, &signer).unwrap();
+

+
        let issue = issues.get(&id).unwrap().unwrap();
+
        assert_eq!(&issue.replies(&c1).nth(0).unwrap().1.body, "Re: Hi.");
+
        assert_eq!(&issue.replies(&c2).nth(0).unwrap().1.body, "Re: Ha.");
+
        assert_eq!(&issue.replies(&c2).nth(1).unwrap().1.body, "Re: Ha. Ha.");
+
        assert_eq!(
+
            &issue.replies(&c2).nth(2).unwrap().1.body,
+
            "Re: Ha. Ha. Ha."
+
        );
    }

    #[test]
@@ -575,8 +588,11 @@ mod test {
            .create("My first issue", "Blah blah blah.", &[], &signer)
            .unwrap();

-
        issue.comment("Ho ho ho.", &signer).unwrap();
-
        issue.comment("Ha ha ha.", &signer).unwrap();
+
        // The initial thread op id is always the same.
+
        let c0 = (clock::Lamport::initial(), author);
+

+
        issue.comment("Ho ho ho.", c0, &signer).unwrap();
+
        issue.comment("Ha ha ha.", c0, &signer).unwrap();

        let id = issue.id;
        let issue = issues.get(&id).unwrap().unwrap();
modified radicle/src/cob/patch.rs
@@ -372,7 +372,8 @@ impl Revision {
    }

    pub fn description(&self) -> Option<&str> {
-
        self.discussion.first()
+
        let (_, comment) = self.discussion.root()?;
+
        Some(comment.body.as_str())
    }
}

@@ -534,8 +535,8 @@ impl store::Transaction<Patch> {
        })
    }

-
    /// Comment on a patch revision.
-
    pub fn comment<S: ToString>(&mut self, revision: RevisionId, body: S) -> OpId {
+
    /// Start a patch revision discussion.
+
    pub fn thread<S: ToString>(&mut self, revision: RevisionId, body: S) -> OpId {
        self.push(Action::Thread {
            revision,
            action: thread::Action::Comment {
@@ -545,6 +546,22 @@ impl store::Transaction<Patch> {
        })
    }

+
    /// Comment on a patch revision.
+
    pub fn comment<S: ToString>(
+
        &mut self,
+
        revision: RevisionId,
+
        body: S,
+
        reply_to: CommentId,
+
    ) -> OpId {
+
        self.push(Action::Thread {
+
            revision,
+
            action: thread::Action::Comment {
+
                body: body.to_string(),
+
                reply_to: Some(reply_to),
+
            },
+
        })
+
    }
+

    /// Review a patch revision.
    pub fn review(
        &mut self,
@@ -582,7 +599,7 @@ impl store::Transaction<Patch> {
        oid: impl Into<git::Oid>,
    ) -> (OpId, OpId) {
        let revision = self.revision(base, oid);
-
        let comment = self.comment(revision, description);
+
        let comment = self.thread(revision, description);

        (revision, comment)
    }
@@ -664,9 +681,10 @@ impl<'a, 'g> PatchMut<'a, 'g> {
        &mut self,
        revision: RevisionId,
        body: S,
+
        reply_to: CommentId,
        signer: &G,
    ) -> Result<CommentId, Error> {
-
        self.transaction("Comment", signer, |tx| tx.comment(revision, body))
+
        self.transaction("Comment", signer, |tx| tx.comment(revision, body, reply_to))
    }

    /// Review a patch revision.
@@ -703,7 +721,7 @@ impl<'a, 'g> PatchMut<'a, 'g> {
    ) -> Result<(OpId, OpId), Error> {
        self.transaction("Add revision", signer, |tx| {
            let r = tx.revision(base, oid);
-
            let c = tx.comment(r, description);
+
            let c = tx.thread(r, description);

            (r, c)
        })
@@ -950,21 +968,18 @@ mod test {
            let [p1, p2, p3] = log.permutations;

            let mut t1 = t.clone();
-
            match t1.apply(p1) {
-
                Ok(()) => {}
-
                Err(ApplyError::Missing(_)) => return TestResult::discard(),
+
            if t1.apply(p1).is_err() {
+
                return TestResult::discard();
            }

            let mut t2 = t.clone();
-
            match t2.apply(p2) {
-
                Ok(()) => {}
-
                Err(ApplyError::Missing(_)) => return TestResult::discard(),
+
            if t2.apply(p2).is_err() {
+
                return TestResult::discard();
            }

            let mut t3 = t;
-
            match t3.apply(p3) {
-
                Ok(()) => {}
-
                Err(ApplyError::Missing(_)) => return TestResult::discard(),
+
            if t3.apply(p3).is_err() {
+
                return TestResult::discard();
            }

            assert_eq!(t1, t2);
@@ -976,7 +991,7 @@ mod test {

        qcheck::QuickCheck::new()
            .min_tests_passed(100)
-
            .gen(qcheck::Gen::new(8))
+
            .gen(qcheck::Gen::new(7))
            .quickcheck(property as fn(Changes<3>) -> TestResult);
    }

modified radicle/src/cob/thread.rs
@@ -29,19 +29,20 @@ pub static TYPENAME: Lazy<TypeName> =
pub type CommentId = OpId;

/// A comment on a discussion thread.
-
#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct Comment {
    /// The comment body.
    pub body: String,
-
    /// Thread or comment this is a reply to.
-
    pub reply_to: Option<OpId>,
+
    /// Comment this is a reply to.
+
    /// Should always be set, except for the root comment.
+
    pub reply_to: Option<CommentId>,
    /// When the comment was authored.
    pub timestamp: Timestamp,
}

impl Comment {
    /// Create a new comment.
-
    pub fn new(body: String, reply_to: Option<OpId>, timestamp: Timestamp) -> Self {
+
    pub fn new(body: String, reply_to: Option<CommentId>, timestamp: Timestamp) -> Self {
        Self {
            body,
            reply_to,
@@ -68,7 +69,9 @@ pub enum Action {
    Comment {
        /// Comment body.
        body: String,
-
        /// Another comment this is a reply to.
+
        /// Comment this is a reply to.
+
        /// Should be [`None`] if it's the top-level comment.
+
        /// Should be the root [`CommentId`] if it's a top-level comment.
        reply_to: Option<OpId>,
    },
    /// Redact a change. Not all changes can be redacted.
@@ -105,8 +108,8 @@ 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(Ops(changes)) = Ops::try_from(entry) {
-
                acc.apply(changes);
+
            if let Ok(Ops(ops)) = Ops::try_from(entry) {
+
                acc.apply(ops);
                ControlFlow::Continue(acc)
            } else {
                ControlFlow::Break(acc)
@@ -132,6 +135,10 @@ impl Deref for Thread {
}

impl Thread {
+
    pub fn is_initialized(&self) -> bool {
+
        !self.comments.is_empty()
+
    }
+

    pub fn comment(&self, id: &CommentId) -> Option<&Comment> {
        if let Some(Redactable::Present(comment)) = self.comments.get(id) {
            Some(comment)
@@ -140,11 +147,10 @@ impl Thread {
        }
    }

-
    pub fn first(&self) -> Option<&str> {
+
    pub fn root(&self) -> Option<(&CommentId, &Comment)> {
        self.comments
-
            .values()
-
            .filter_map(|r| r.get())
-
            .map(|c| c.body.as_str())
+
            .iter()
+
            .filter_map(|(id, r)| r.get().map(|comment| (id, comment)))
            .next()
    }

@@ -153,8 +159,8 @@ impl Thread {
        to: &'a CommentId,
    ) -> impl Iterator<Item = (&CommentId, &Comment)> {
        self.comments().filter_map(move |(id, c)| {
-
            if let Some(parent) = &c.reply_to {
-
                if parent == to {
+
            if let Some(reply_to) = c.reply_to {
+
                if &reply_to == to {
                    return Some((id, c));
                }
            }
@@ -179,8 +185,10 @@ impl Thread {

            match op.action {
                Action::Comment { body, reply_to } => {
-
                    let present = Redactable::Present(Comment::new(body, reply_to, op.timestamp));
-
                    self.comments.insert(id, present);
+
                    self.comments.insert(
+
                        id,
+
                        Redactable::Present(Comment::new(body, reply_to, op.timestamp)),
+
                    );
                }
                Action::Redact { id } => {
                    self.comments.insert(id, Redactable::Redacted);
@@ -273,9 +281,9 @@ mod tests {
    use std::{array, iter};

    use crate::crypto::test::signer::MockSigner;
+
    use nonempty::NonEmpty;
    use pretty_assertions::assert_eq;
-
    use qcheck::Arbitrary;
-
    use qcheck_macros::quickcheck;
+
    use qcheck::{Arbitrary, TestResult};

    use super::*;
    use crate as radicle;
@@ -303,6 +311,7 @@ mod tests {
        fn arbitrary(g: &mut qcheck::Gen) -> Self {
            let author = ActorId::from([0; 32]);
            let rng = fastrand::Rng::with_seed(u64::arbitrary(g));
+
            let root = (Lamport::initial(), author);
            let gen =
                WeightedGenerator::<(Lamport, Action), (Lamport, Vec<OpId>)>::new(rng.clone())
                    .variant(3, |(clock, changes), rng| {
@@ -312,7 +321,7 @@ mod tests {
                            *clock,
                            Action::Comment {
                                body: iter::repeat_with(|| rng.alphabetic()).take(16).collect(),
-
                                reply_to: None,
+
                                reply_to: Some(root),
                            },
                        ))
                    })
@@ -340,11 +349,19 @@ mod tests {
                        Some((clock.tick(), Action::Redact { id }))
                    });

-
            let mut changes = Vec::new();
+
            let mut changes = vec![Op {
+
                action: Action::Comment {
+
                    body: String::default(),
+
                    reply_to: None,
+
                },
+
                author,
+
                clock: Lamport::initial(),
+
                timestamp: Timestamp::now(),
+
            }];
            let mut permutations: [Vec<Op<Action>>; N] = array::from_fn(|_| Vec::new());
-
            let timestamp = Timestamp::now() + rng.u64(..60);

-
            for (clock, action) in gen.take(g.size().min(8)) {
+
            for (clock, action) in gen.take(g.size()) {
+
                let timestamp = Timestamp::now() + rng.u64(..60);
                changes.push(Op {
                    action,
                    author,
@@ -370,23 +387,24 @@ mod tests {
            radicle::cob::store::Store::<Thread>::open(*signer.public_key(), &repository).unwrap();
        let mut alice = Actor::new(signer);

-
        let a1 = alice.comment("First comment", None);
-
        let a2 = alice.comment("Second comment", None);
-
        let a3 = alice.comment("Third comment", None);
+
        let a0 = alice.comment("First comment", None);
+
        let a1 = alice.comment("Second comment", Some(a0.id()));
+
        let a2 = alice.comment("Third comment", Some(a0.id()));

        let (id, _, _) = store
-
            .create("Thread created", a1.action, &alice.signer)
+
            .create("Thread created", a0.action, &alice.signer)
            .unwrap();
-
        let second = store
-
            .update(id, "Thread updated", a2.action, &alice.signer)
+
        let comment = store
+
            .update(id, "Thread updated", a1.action, &alice.signer)
            .unwrap();
        store
-
            .update(id, "Thread updated", a3.action, &alice.signer)
+
            .update(id, "Thread updated", a2.action, &alice.signer)
            .unwrap();

-
        let a4 = alice.redact((second.history().clock().into(), *alice.signer.public_key()));
+
        // Redact the second comment.
+
        let a3 = alice.redact((comment.history().clock().into(), *alice.signer.public_key()));
        store
-
            .update(id, "Comment redacted", a4.action, &alice.signer)
+
            .update(id, "Comment redacted", a3.action, &alice.signer)
            .unwrap();

        let (thread, _) = store.get(&id).unwrap().unwrap();
@@ -407,17 +425,20 @@ mod tests {

        let mut alice = Actor::new(signer);

-
        let a1 = alice.comment("First comment", None);
-
        let a2 = alice.comment("Second comment", None);
+
        let a0 = alice.comment("Thread root", None);
+
        let a1 = alice.comment("First comment", Some(a0.id()));
+
        let a2 = alice.comment("Second comment", Some(a0.id()));

        let mut expected = Thread::default();
-
        expected.apply([a1.clone(), a2.clone()]);
+
        expected.apply([a0.clone(), a1.clone(), a2.clone()]);

        let (id, _, _) = store
-
            .create("Thread created", a1.action, &alice.signer)
+
            .create("Thread created", a0.action, &alice.signer)
            .unwrap();
+

+
        let actions = NonEmpty::from_vec(vec![a1.action, a2.action]).unwrap();
        store
-
            .update(id, "Thread updated", a2.action, &alice.signer)
+
            .update(id, "Thread updated", actions, &alice.signer)
            .unwrap();

        let (actual, _) = store.get(&id).unwrap().unwrap();
@@ -430,18 +451,19 @@ mod tests {
        let mut alice = Actor::<MockSigner>::default();
        let mut bob = Actor::<MockSigner>::default();

-
        let a1 = alice.comment("First comment", None);
-
        let a2 = alice.comment("Second comment", None);
+
        let a0 = alice.comment("Thread root", None);
+
        let a1 = alice.comment("First comment", Some(a0.id()));
+
        let a2 = alice.comment("Second comment", Some(a0.id()));

-
        bob.receive([a1.clone(), a2.clone()]);
+
        bob.receive([a0.clone(), a1.clone(), a2.clone()]);
        assert_eq!(
            bob.timeline().collect::<Vec<_>>(),
            alice.timeline().collect::<Vec<_>>()
        );
-
        assert_eq!(alice.timeline().collect::<Vec<_>>(), vec![&a1, &a2]);
+
        assert_eq!(alice.timeline().collect::<Vec<_>>(), vec![&a0, &a1, &a2]);

        bob.reset();
-
        bob.receive([a2, a1]);
+
        bob.receive([a0, a2, a1]);
        assert_eq!(
            bob.timeline().collect::<Vec<_>>(),
            alice.timeline().collect::<Vec<_>>()
@@ -454,32 +476,33 @@ mod tests {
        let mut bob = Actor::<MockSigner>::default();
        let mut eve = Actor::<MockSigner>::default();

-
        let a1 = alice.comment("First comment", None);
+
        let a0 = alice.comment("Thread root", None);
+
        let a1 = alice.comment("First comment", Some(a0.id()));

-
        bob.receive([a1.clone()]);
+
        bob.receive([a0.clone(), a1.clone()]);

-
        let b0 = bob.comment("Bob's first reply to Alice", None);
-
        let b1 = bob.comment("Bob's second reply to Alice", None);
+
        let b0 = bob.comment("Bob's first reply to Alice", Some(a0.id()));
+
        let b1 = bob.comment("Bob's second reply to Alice", Some(a0.id()));

-
        eve.receive([b1.clone(), b0.clone()]);
-
        let e0 = eve.comment("Eve's first reply to Alice", None);
+
        eve.receive([a0.clone(), b1.clone(), b0.clone()]);
+
        let e0 = eve.comment("Eve's first reply to Alice", Some(a0.id()));

        bob.receive([e0.clone()]);
-
        let b2 = bob.comment("Bob's third reply to Alice", None);
+
        let b2 = bob.comment("Bob's third reply to Alice", Some(a0.id()));

        eve.receive([b2.clone(), a1.clone()]);
-
        let e1 = eve.comment("Eve's second reply to Alice", None);
+
        let e1 = eve.comment("Eve's second reply to Alice", Some(a0.id()));

        alice.receive([b0.clone(), b1.clone(), b2.clone(), e0.clone(), e1.clone()]);
        bob.receive([e1.clone()]);

-
        let a2 = alice.comment("Second comment", None);
+
        let a2 = alice.comment("Second comment", Some(a0.id()));
        eve.receive([a2.clone()]);
        bob.receive([a2.clone()]);

-
        assert_eq!(alice.ops.len(), 7);
-
        assert_eq!(bob.ops.len(), 7);
-
        assert_eq!(eve.ops.len(), 7);
+
        assert_eq!(alice.ops.len(), 8);
+
        assert_eq!(bob.ops.len(), 8);
+
        assert_eq!(eve.ops.len(), 8);

        assert_eq!(
            bob.timeline().collect::<Vec<_>>(),
@@ -490,27 +513,36 @@ mod tests {
            alice.timeline().collect::<Vec<_>>()
        );
        assert_eq!(
-
            vec![&a1, &b0, &b1, &e0, &b2, &e1, &a2],
+
            vec![&a0, &a1, &b0, &b1, &e0, &b2, &e1, &a2],
            alice.timeline().collect::<Vec<_>>(),
        );
    }

-
    #[quickcheck]
-
    fn prop_invariants(log: Changes<3>) {
-
        let t = Thread::default();
-
        let [p1, p2, p3] = log.permutations;
+
    #[test]
+
    fn prop_invariants() {
+
        fn property(log: Changes<3>) -> TestResult {
+
            let t = Thread::default();
+
            let [p1, p2, p3] = log.permutations;
+

+
            let mut t1 = t.clone();
+
            t1.apply(p1);

-
        let mut t1 = t.clone();
-
        t1.apply(p1);
+
            let mut t2 = t.clone();
+
            t2.apply(p2);

-
        let mut t2 = t.clone();
-
        t2.apply(p2);
+
            let mut t3 = t;
+
            t3.apply(p3);

-
        let mut t3 = t;
-
        t3.apply(p3);
+
            assert_eq!(t1, t2);
+
            assert_eq!(t2, t3);
+
            assert_laws(&t1, &t2, &t3);

-
        assert_eq!(t1, t2);
-
        assert_eq!(t2, t3);
-
        assert_laws(&t1, &t2, &t3);
+
            TestResult::passed()
+
        }
+
        qcheck::QuickCheck::new()
+
            .min_tests_passed(100)
+
            .max_tests(10000)
+
            .gen(qcheck::Gen::new(7))
+
            .quickcheck(property as fn(Changes<3>) -> TestResult);
    }
}