Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
cob: Create proper `OpId` type
Alexis Sellier committed 3 years ago
commit 911c194fb3ceb8ead034baaff0fa5c4f9ce4fecc
parent 49cd26c0b3d7cd71f6dda79d0626bfbf00626dc6
6 files changed +115 -79
modified radicle-httpd/src/api/v1/projects.rs
@@ -423,9 +423,11 @@ impl<'a> FromIterator<(&'a CommentId, &'a thread::Comment)> for Comments {
    fn from_iter<I: IntoIterator<Item = (&'a CommentId, &'a thread::Comment)>>(iter: I) -> Self {
        let mut comments = Vec::new();

-
        for (comment_id, comment) in iter {
+
        for (_, comment) in iter {
            comments.push(Comment {
-
                author: Author { id: comment_id.1 },
+
                author: Author {
+
                    id: comment.author(),
+
                },
                body: comment.body().to_owned(),
                reactions: [],
                timestamp: comment.timestamp(),
modified radicle/src/cob/issue.rs
@@ -20,7 +20,7 @@ use crate::storage::git as storage;
use super::op::Ops;

/// Issue operation.
-
pub type Op = crate::cob::Op<Action>;
+
pub type Op = cob::Op<Action>;

/// Type name of an issue.
pub static TYPENAME: Lazy<TypeName> =
@@ -155,7 +155,7 @@ impl Issue {
        self.thread
            .comments()
            .next()
-
            .map(|((_, pk), _)| Author::new(*pk))
+
            .map(|(_, c)| Author::new(c.author()))
    }

    pub fn description(&self) -> Option<&str> {
@@ -192,12 +192,8 @@ impl Issue {
                    }
                }
                Action::Thread { action } => {
-
                    self.thread.apply([cob::Op {
-
                        action,
-
                        author: op.author,
-
                        clock: op.clock,
-
                        timestamp: op.timestamp,
-
                    }])?;
+
                    self.thread
+
                        .apply([cob::Op::new(action, op.author, op.timestamp, op.clock)])?;
                }
            }
        }
@@ -626,7 +622,7 @@ mod test {
            .create("My first issue", "Blah blah blah.", &[], &signer)
            .unwrap();

-
        let comment = (clock::Lamport::default(), *signer.public_key());
+
        let comment = OpId::initial(*signer.public_key());
        let reaction = Reaction::new('🥳').unwrap();
        issue.react(comment, reaction, &signer).unwrap();

@@ -648,7 +644,7 @@ mod test {
        let mut issue = issues
            .create("My first issue", "Blah blah blah.", &[], &signer)
            .unwrap();
-
        let root = (clock::Lamport::default(), author);
+
        let root = OpId::initial(author);

        let c1 = issue.comment("Hi hi hi.", root, &signer).unwrap();
        let c2 = issue.comment("Ha ha ha.", root, &signer).unwrap();
@@ -710,23 +706,23 @@ mod test {
            .unwrap();

        // The initial thread op id is always the same.
-
        let c0 = (clock::Lamport::initial(), author);
+
        let c0 = OpId::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();
-
        let ((_, a0), c0) = &issue.comments().nth(0).unwrap();
-
        let ((_, a1), c1) = &issue.comments().nth(1).unwrap();
-
        let ((_, a2), c2) = &issue.comments().nth(2).unwrap();
+
        let (_, c0) = &issue.comments().nth(0).unwrap();
+
        let (_, c1) = &issue.comments().nth(1).unwrap();
+
        let (_, c2) = &issue.comments().nth(2).unwrap();

        assert_eq!(c0.body(), "Blah blah blah.");
-
        assert_eq!(a0, &author);
+
        assert_eq!(c0.author(), author);
        assert_eq!(c1.body(), "Ho ho ho.");
-
        assert_eq!(a1, &author);
+
        assert_eq!(c1.author(), author);
        assert_eq!(c2.body(), "Ha ha ha.");
-
        assert_eq!(a2, &author);
+
        assert_eq!(c2.author(), author);
    }

    #[test]
modified radicle/src/cob/op.rs
@@ -1,6 +1,7 @@
use std::collections::BTreeMap;

use nonempty::NonEmpty;
+
use serde::{Deserialize, Serialize};
use thiserror::Error;

use radicle_cob::history::EntryWithClock;
@@ -8,16 +9,37 @@ use radicle_crdt::clock;
use radicle_crdt::clock::Lamport;
use radicle_crypto::{PublicKey, Signer};

-
/// Identifies an [`Op`].
-
pub type OpId = (Lamport, ActorId);
+
/// Identifies an [`Op`] internally and within the change graph.
+
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
+
pub struct OpId(Lamport, ActorId);
+

+
impl OpId {
+
    /// Create a new operation id.
+
    pub fn new(clock: Lamport, actor: ActorId) -> Self {
+
        Self(clock, actor)
+
    }
+

+
    /// Get the initial operation id for the given actor.
+
    pub fn initial(actor: ActorId) -> Self {
+
        Self(Lamport::initial(), actor)
+
    }
+

+
    /// Get operation id clock.
+
    pub fn clock(&self) -> Lamport {
+
        self.0
+
    }
+
}
+

/// The author of an [`Op`].
pub type ActorId = PublicKey;

/// Error decoding an operation from an entry.
#[derive(Error, Debug)]
-
pub enum OpDecodeError {
-
    #[error("deserialization from json failed: {0}")]
-
    Deserialize(#[from] serde_json::Error),
+
pub enum OpEncodingError {
+
    #[error("encoding failed: {0}")]
+
    Encoding(#[from] serde_json::Error),
+
    #[error("git: {0}")]
+
    Git(#[from] git2::Error),
}

/// The `Op` is the operation that is applied onto a state to form a CRDT.
@@ -36,16 +58,33 @@ pub struct Op<A> {
    pub timestamp: clock::Physical,
}

+
impl<A: Serialize> Op<A> {
+
    pub fn new(
+
        action: A,
+
        author: ActorId,
+
        timestamp: impl Into<clock::Physical>,
+
        clock: Lamport,
+
    ) -> Self {
+
        Self {
+
            action,
+
            author,
+
            clock,
+
            timestamp: timestamp.into(),
+
        }
+
    }
+
}
+

pub struct Ops<A>(pub NonEmpty<Op<A>>);

impl<'a, A> TryFrom<&'a EntryWithClock> for Ops<A>
where
    for<'de> A: serde::Deserialize<'de>,
{
-
    type Error = OpDecodeError;
+
    type Error = OpEncodingError;

    fn try_from(entry: &'a EntryWithClock) -> Result<Self, Self::Error> {
        let mut clock = entry.clock().into();
+

        entry
            .contents()
            .clone()
@@ -69,7 +108,7 @@ impl<A> Op<A> {
    /// Get the op id.
    /// This uniquely identifies each operation in the CRDT.
    pub fn id(&self) -> OpId {
-
        (self.clock, self.author)
+
        OpId(self.clock, self.author)
    }
}

modified radicle/src/cob/patch.rs
@@ -33,7 +33,7 @@ pub static TYPENAME: Lazy<TypeName> =
    Lazy::new(|| FromStr::from_str("xyz.radicle.patch").expect("type name is valid"));

/// Patch operation.
-
pub type Op = crate::cob::Op<Action>;
+
pub type Op = cob::Op<Action>;

/// Identifier for a patch.
pub type PatchId = ObjectId;
@@ -300,12 +300,9 @@ 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::Op {
-
                            action,
-
                            author: op.author,
-
                            clock: op.clock,
-
                            timestamp,
-
                        }])?;
+
                        revision
+
                            .discussion
+
                            .apply([cob::Op::new(action, op.author, timestamp, op.clock)])?;
                    } else {
                        return Err(ApplyError::Missing(revision));
                    }
@@ -937,7 +934,7 @@ mod test {
                    let base = oids[rng.usize(..oids.len())];

                    if rng.bool() {
-
                        revisions.push((clock.tick(), author));
+
                        revisions.push(OpId::new(clock.tick(), author));
                    }
                    Some((*clock, Action::Revision { base, oid }))
                });
@@ -947,12 +944,7 @@ mod test {
            let timestamp = Timestamp::now() + rng.u64(..60);

            for (clock, action) in gen.take(g.size()) {
-
                changes.push(Op {
-
                    action,
-
                    author,
-
                    clock,
-
                    timestamp,
-
                });
+
                changes.push(Op::new(action, author, timestamp, clock));
            }

            for p in &mut permutations {
@@ -1250,11 +1242,11 @@ mod test {
        assert_eq!(patch.description(), Some("Blah blah blah."));
        assert_eq!(patch.version(), 0);

-
        let ((c1, _), (c2, _)) = patch
+
        let (r1, t1) = patch
            .update("I've made changes.", base, rev1_oid, &signer)
            .unwrap();
-
        assert_eq!(c1.get(), 3);
-
        assert_eq!(c2.get(), 4);
+
        assert_eq!(r1.clock().get(), 3);
+
        assert_eq!(t1.clock().get(), 4);

        let id = patch.id;
        let patch = patches.get(&id).unwrap().unwrap();
modified radicle/src/cob/store.rs
@@ -9,6 +9,7 @@ use serde::Serialize;

use crate::cob;
use crate::cob::common::Author;
+
use crate::cob::op::OpId;
use crate::cob::CollaborativeObject;
use crate::cob::{ActorId, Create, History, ObjectId, TypeName, Update};
use crate::crypto::PublicKey;
@@ -250,11 +251,11 @@ impl<T: FromHistory> Transaction<T> {
        // Otherwise, it means it was the first operation of our COB. In that case
        // we set our clock to the initial clock value (0), and return that.
        if let Some(ref mut clock) = self.clock {
-
            (clock.tick(), self.actor)
+
            OpId::new(clock.tick(), self.actor)
        } else {
            self.clock = Some(Lamport::initial());

-
            (Lamport::initial(), self.actor)
+
            OpId::initial(self.actor)
        }
    }

modified radicle/src/cob/thread.rs
@@ -42,6 +42,8 @@ pub struct Edit {
/// A comment on a discussion thread.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Comment {
+
    /// Comment author.
+
    author: ActorId,
    /// The comment body.
    edits: GMap<Lamport, Max<Edit>>,
    /// Comment this is a reply to.
@@ -51,10 +53,16 @@ pub struct Comment {

impl Comment {
    /// Create a new comment.
-
    pub fn new(body: String, reply_to: Option<CommentId>, timestamp: Timestamp) -> Self {
+
    pub fn new(
+
        author: ActorId,
+
        body: String,
+
        reply_to: Option<CommentId>,
+
        timestamp: Timestamp,
+
    ) -> Self {
        let edit = Edit { body, timestamp };

        Self {
+
            author,
            edits: GMap::singleton(Lamport::initial(), Max::from(edit)),
            reply_to,
        }
@@ -82,6 +90,11 @@ impl Comment {
            .timestamp
    }

+
    /// Return the comment author.
+
    pub fn author(&self) -> ActorId {
+
        self.author
+
    }
+

    /// Return the comment this is a reply to. Returns nothing if this is the root comment.
    pub fn reply_to(&self) -> Option<CommentId> {
        self.reply_to
@@ -119,7 +132,7 @@ pub enum Action {
        /// 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>,
+
        reply_to: Option<CommentId>,
    },
    /// Edit a comment.
    Edit { id: CommentId, body: String },
@@ -156,6 +169,13 @@ impl Semilattice for Thread {
}

impl Thread {
+
    pub fn new(id: CommentId, comment: Comment) -> Self {
+
        Self {
+
            comments: GMap::singleton(id, Redactable::Present(comment)),
+
            reactions: GMap::default(),
+
        }
+
    }
+

    pub fn is_initialized(&self) -> bool {
        !self.comments.is_empty()
    }
@@ -211,17 +231,19 @@ impl Thread {
    pub fn apply(&mut self, ops: impl IntoIterator<Item = Op<Action>>) -> Result<(), OpError> {
        for op in ops.into_iter() {
            let id = op.id();
+
            let author = op.author;
+
            let timestamp = op.timestamp;

            match op.action {
                Action::Comment { body, reply_to } => {
                    self.comments.insert(
                        id,
-
                        Redactable::Present(Comment::new(body, reply_to, op.timestamp)),
+
                        Redactable::Present(Comment::new(author, body, reply_to, timestamp)),
                    );
                }
                Action::Edit { id, body } => {
                    if let Some(Redactable::Present(comment)) = self.comments.get_mut(&id) {
-
                        comment.edit(op.clock, body, op.timestamp);
+
                        comment.edit(op.clock, body, timestamp);
                    } else {
                        return Err(OpError::Missing(id));
                    }
@@ -388,11 +410,11 @@ 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 root = OpId::initial(author);
            let gen =
                WeightedGenerator::<(Lamport, Action), (Lamport, BTreeSet<OpId>)>::new(rng.clone())
                    .variant(3, |(clock, comments), rng| {
-
                        comments.insert((clock.tick(), author));
+
                        comments.insert(OpId::new(clock.tick(), author));

                        Some((
                            *clock,
@@ -441,25 +463,20 @@ mod tests {
                        Some((clock.tick(), Action::Redact { id }))
                    });

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

            for (clock, action) in gen.take(g.size()) {
                let timestamp = Timestamp::now() + rng.u64(..60);
-
                ops.push(Op {
-
                    action,
-
                    author,
-
                    clock,
-
                    timestamp,
-
                });
+
                ops.push(Op::new(action, author, timestamp, clock));
            }

            for p in &mut permutations {
@@ -487,31 +504,21 @@ mod tests {
    #[test]
    fn test_redact_comment() {
        let tmp = tempfile::tempdir().unwrap();
-
        let (_, signer, repository) = radicle::test::setup::context(&tmp);
-
        let store = setup::store(&signer, &repository);
+
        let (_, signer, _) = radicle::test::setup::context(&tmp);
        let mut alice = Actor::new(signer);
+
        let mut thread = Thread::default();

        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", a0.action, &alice.signer)
-
            .unwrap();
-
        let comment = store
-
            .update(id, "Thread updated", a1.action, &alice.signer)
-
            .unwrap();
-
        store
-
            .update(id, "Thread updated", a2.action, &alice.signer)
-
            .unwrap();
+
        thread.apply([a0, a1.clone(), a2]).unwrap();
+
        assert_eq!(thread.comments().count(), 3);

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

-
        let (thread, _) = store.get(&id).unwrap().unwrap();
        let (_, comment0) = thread.comments().nth(0).unwrap();
        let (_, comment1) = thread.comments().nth(1).unwrap();

@@ -550,7 +557,6 @@ mod tests {
        let tmp = tempfile::tempdir().unwrap();
        let (_, signer, repository) = radicle::test::setup::context(&tmp);
        let store = setup::store(&signer, &repository);
-

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

        let a0 = alice.comment("Thread root", None);