cob: Implement action authorization

Add `authorization` functions to `Patch` and `Issue` COBs.

Co-authored-by: Arastoo Bozorgi <ab1502@mun.ca>

radicle-cli/examples/workflow/3-issues.md

@@ -26,51 +26,11 @@ $ rad issue list
 ╰────────────────────────────────────────────────────────────────────────────────────────────────╯
 ```
 
-Great! Now we've documented the issue for ourselves and others.
+Great! Now we've documented the issue for ourselves and others. But wait, we've
-
+found an important detail about the car's power requirements. It will help
-Just like with other project management systems, the issue can be assigned to
+whoever works on a fix.
-others to work on.  This is to ensure work is not duplicated.
-
-Let's assign this issue to ourself.
-
-```
-$ rad assign 2f6eb49efac492327f71437b6bfc01b49afa0981 --to did:key:z6Mkt67GdsW7715MEfRuP4pSZxJRJh6kj6Y48WRqVv4N1tRk
-```
-
-It will now show in the list of issues assigned to us.
-
-```
-$ rad issue list --assigned
-╭────────────────────────────────────────────────────────────────────────────────────────────────╮
-│ ●   ID        Title                         Author           Labels   Assignees   Opened       │
-├────────────────────────────────────────────────────────────────────────────────────────────────┤
-│ ●   2f6eb49   flux capacitor underpowered   bob      (you)            bob         [    ..    ] │
-╰────────────────────────────────────────────────────────────────────────────────────────────────╯
-```
-
-In addition, you can see that when you run `rad issue show` you are listed under the `Assignees`.
-
-```
-$ rad issue show 2f6eb49
-╭─────────────────────────────────────────────────────────╮
-│ Title      flux capacitor underpowered                  │
-│ Issue      2f6eb49efac492327f71437b6bfc01b49afa0981     │
-│ Author     bob (you)                                    │
-│ Assignees  z6Mkt67…v4N1tRk                              │
-│ Status     open                                         │
-│                                                         │
-│ Flux capacitor power requirements exceed current supply │
-╰─────────────────────────────────────────────────────────╯
-```
-
-Note: this can always be undone with the `unassign` subcommand.
-
-Great, now we have communicated to the world about our car's defect.
-
-But wait! We've found an important detail about the car's power requirements.
-It will help whoever works on a fix.
 
 ```
 $ rad comment 2f6eb49efac492327f71437b6bfc01b49afa0981 --message 'The flux capacitor needs 1.21 Gigawatts'
-24ab347afda760e77d565f9cb013c6db560f44fd
+d2b50873009b93680698aef4f57f43f7e850b651
 ```

radicle-cob/src/history.rs

@@ -60,12 +60,11 @@ impl History {
     /// accumulator value of type `A`. However, unlike `fold` the function `f`
     /// may prune branches from the dependency graph by returning
     /// `ControlFlow::Break`.
-    pub fn traverse<F, A>(&self, init: A, mut f: F) -> A
+    pub fn traverse<F, A>(&self, init: A, roots: &[EntryId], mut f: F) -> A
     where
         F: for<'r> FnMut(A, &'r EntryId, &'r Entry) -> ControlFlow<A, A>,
     {
-        self.graph
+        self.graph.fold(roots, init, |acc, k, v| f(acc, k, v))
-            .fold(&[self.root], init, |acc, k, v| f(acc, k, v))
     }
 
     /// Return a topologically-sorted list of history entries.
@@ -114,4 +113,12 @@ impl History {
             .get(&self.root)
             .expect("History::root: the root entry must be present in the graph")
     }
+
+    /// Get the children of the given entry.
+    pub fn children_of(&self, id: &EntryId) -> Vec<EntryId> {
+        self.graph
+            .get(id)
+            .map(|n| n.dependents.iter().cloned().collect())
+            .unwrap_or_default()
+    }
 }

radicle-cob/src/tests.rs

@@ -211,21 +211,26 @@ fn traverse_cobs() {
     )
     .unwrap();
 
+    let root = object.history.root().id;
     // traverse over the history and filter by changes that were only authorized by terry
-    let contents = object.history().traverse(Vec::new(), |mut acc, _, entry| {
+    let contents = object
-        if entry.author() == terry_signer.public_key() {
+        .history()
-            acc.push(entry.contents().head.clone());
+        .traverse(Vec::new(), &[root], |mut acc, _, entry| {
-        }
+            if entry.author() == terry_signer.public_key() {
-        ControlFlow::Continue(acc)
+                acc.push(entry.contents().head.clone());
-    });
+            }
+            ControlFlow::Continue(acc)
+        });
 
     assert_eq!(contents, vec![b"issue 1".to_vec()]);
 
     // traverse over the history and filter by changes that were only authorized by neil
-    let contents = object.history().traverse(Vec::new(), |mut acc, _, entry| {
+    let contents = object
-        acc.push(entry.contents().head.clone());
+        .history()
-        ControlFlow::Continue(acc)
+        .traverse(Vec::new(), &[root], |mut acc, _, entry| {
-    });
+            acc.push(entry.contents().head.clone());
+            ControlFlow::Continue(acc)
+        });
 
     assert_eq!(contents, vec![b"issue 1".to_vec(), b"issue 2".to_vec()]);
 }

radicle/src/cob/common.rs

@@ -24,6 +24,10 @@ impl Author {
     pub fn id(&self) -> &Did {
         &self.id
     }
+
+    pub fn public_key(&self) -> &PublicKey {
+        self.id.as_key()
+    }
 }
 
 impl From<PublicKey> for Author {
@@ -278,6 +282,27 @@ impl std::str::FromStr for Uri {
     }
 }
 
+/// The result of an authorization check on an COB action.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum Authorization {
+    /// Action is allowed.
+    Allow,
+    /// Action is denied.
+    Deny,
+    /// Authorization cannot be determined due to missing object, eg. due to redaction.
+    Unknown,
+}
+
+impl From<bool> for Authorization {
+    fn from(value: bool) -> Self {
+        if value {
+            Self::Allow
+        } else {
+            Self::Deny
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

radicle/src/cob/identity.rs

@@ -323,11 +323,10 @@ impl store::FromHistory for Proposal {
         &TYPENAME
     }
 
-    fn validate(&self) -> Result<(), Self::Error> {
+    fn init<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
-        if self.revisions.is_empty() {
+        let mut identity = Self::default();
-            return Err(ApplyError::Validate("no revisions found"));
+        identity.apply(op, repo)?;
-        }
+        Ok(identity)
-        Ok(())
     }
 
     fn apply<R: ReadRepository>(&mut self, op: Op, _repo: &R) -> Result<(), Self::Error> {

radicle/src/cob/issue.rs

@@ -7,15 +7,16 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
 use crate::cob;
-use crate::cob::common::{Author, Label, Reaction, Timestamp, Uri};
+use crate::cob::common::{Author, Authorization, Label, Reaction, Timestamp, Uri};
 use crate::cob::store::Transaction;
 use crate::cob::store::{FromHistory as _, HistoryAction};
 use crate::cob::thread;
-use crate::cob::thread::{CommentId, Thread};
+use crate::cob::thread::{Comment, CommentId, Thread};
 use crate::cob::{store, ActorId, Embed, EntryId, ObjectId, TypeName};
 use crate::crypto::Signer;
 use crate::git;
-use crate::prelude::{Did, ReadRepository};
+use crate::identity::doc::{Doc, DocError};
+use crate::prelude::{Did, ReadRepository, Verified};
 use crate::storage::WriteRepository;
 
 /// Issue operation.
@@ -31,16 +32,23 @@ pub type IssueId = ObjectId;
 /// Error updating or creating issues.
 #[derive(Error, Debug)]
 pub enum Error {
-    #[error("apply failed")]
+    /// Error loading the identity document.
-    Apply,
+    #[error("identity doc failed to load: {0}")]
-    #[error("validation failed: {0}")]
+    Doc(#[from] DocError),
-    Validate(&'static str),
     #[error("description missing")]
     DescriptionMissing,
     #[error("thread apply failed: {0}")]
     Thread(#[from] thread::Error),
     #[error("store: {0}")]
     Store(#[from] store::Error),
+    #[error("history: {0}")]
+    History(Box<dyn std::error::Error + Sync + Send + 'static>),
+    /// Action not authorized.
+    #[error("{0} not authorized to apply {1:?}")]
+    NotAuthorized(ActorId, Action),
+    /// General error initializing an issue.
+    #[error("initialization failed: {0}")]
+    Init(&'static str),
 }
 
 /// Reason why an issue was closed.
@@ -91,7 +99,7 @@ impl State {
 }
 
 /// Issue state. Accumulates [`Action`].
-#[derive(Debug, Clone, PartialEq, Eq, Default)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Issue {
     /// Actors assigned to this issue.
     pub(super) assignees: BTreeSet<Did>,
@@ -113,25 +121,52 @@ impl store::FromHistory for Issue {
         &TYPENAME
     }
 
-    fn validate(&self) -> Result<(), Self::Error> {
+    fn init<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
-        if self.title.is_empty() {
+        let mut actions = op.actions.into_iter();
-            return Err(Error::Validate("title is empty"));
+        let Some(Action::Comment { body, reply_to: None, embeds }) = actions.next() else {
+            return Err(Error::Init("the first action must be of type `comment`"));
+        };
+        let comment = Comment::new(op.author, body, None, None, embeds, op.timestamp);
+        let thread = Thread::new(op.id, comment);
+        let mut issue = Issue::new(thread);
+
+        for action in actions {
+            issue.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
         }
-        if self.thread.validate().is_err() {
+        Ok(issue)
-            return Err(Error::Validate("invalid thread"));
-        }
-        Ok(())
     }
 
     fn apply<R: ReadRepository>(&mut self, op: Op, repo: &R) -> Result<(), Error> {
+        let doc = repo.identity_doc_at(op.identity)?.verified()?;
         for action in op.actions {
-            self.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
+            match self.authorization(&action, &op.author, &doc) {
+                Authorization::Allow => {
+                    self.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
+                }
+                Authorization::Deny => {
+                    return Err(Error::NotAuthorized(op.author, action));
+                }
+                Authorization::Unknown => {
+                    continue;
+                }
+            }
         }
         Ok(())
     }
 }
 
 impl Issue {
+    /// Construct a new issue.
+    pub fn new(thread: Thread) -> Self {
+        Self {
+            assignees: BTreeSet::default(),
+            title: String::default(),
+            state: State::default(),
+            labels: BTreeSet::default(),
+            thread,
+        }
+    }
+
     pub fn assigned(&self) -> impl Iterator<Item = &Did> + '_ {
         self.assignees.iter()
     }
@@ -180,6 +215,46 @@ impl Issue {
     pub fn comments(&self) -> impl Iterator<Item = (&CommentId, &thread::Comment)> {
         self.thread.comments()
     }
+
+    /// Apply authorization rules on issue actions.
+    pub fn authorization(
+        &self,
+        action: &Action,
+        actor: &ActorId,
+        doc: &Doc<Verified>,
+    ) -> Authorization {
+        if doc.is_delegate(actor) {
+            // A delegate is authorized to do all actions.
+            return Authorization::Allow;
+        }
+        let author: ActorId = *self.author().id().as_key();
+
+        match action {
+            // Only delegate can assign someone to an issue.
+            Action::Assign { .. } => Authorization::Deny,
+            // Issue authors can edit their own issues.
+            Action::Edit { .. } => Authorization::from(*actor == author),
+            // Issue authors can close or re-open their own issue.
+            Action::Lifecycle { state } => Authorization::from(match state {
+                State::Closed { .. } => *actor == author,
+                State::Open => *actor == author,
+            }),
+            // Only delegate can label an issue.
+            Action::Label { .. } => Authorization::Deny,
+            // All roles can comment on an issues
+            Action::Comment { .. } => Authorization::Allow,
+            // All roles can edit or redact their own comments.
+            Action::CommentEdit { id, .. } | Action::CommentRedact { id, .. } => {
+                if let Some(comment) = self.comment(id) {
+                    Authorization::from(*actor == comment.author())
+                } else {
+                    Authorization::Unknown
+                }
+            }
+            // All roles can react to a comment on an issue.
+            Action::CommentReact { .. } => Authorization::Allow,
+        }
+    }
 }
 
 impl Issue {

radicle/src/cob/patch.rs

@@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
 use crate::cob;
-use crate::cob::common::{Author, Label, Reaction, Timestamp};
+use crate::cob::common::{Author, Authorization, Label, Reaction, Timestamp};
 use crate::cob::store::Transaction;
 use crate::cob::store::{FromHistory as _, HistoryAction};
 use crate::cob::thread;
@@ -105,18 +105,20 @@ pub enum Error {
     /// Error loading the document payload.
     #[error("payload failed to load: {0}")]
     Payload(#[from] PayloadError),
-    /// The merge operation is invalid.
-    #[error("invalid merge operation in {0}")]
-    InvalidMerge(EntryId),
     /// Git error.
     #[error("git: {0}")]
     Git(#[from] git::ext::Error),
-    /// Validation error.
-    #[error("validation failed: {0}")]
-    Validate(&'static str),
     /// Store error.
     #[error("store: {0}")]
     Store(#[from] store::Error),
+    #[error("history error: {0}")]
+    History(Box<dyn std::error::Error + Sync + Send + 'static>),
+    /// Action not authorized by the author
+    #[error("{0} not authorized to apply {1:?}")]
+    NotAuthorized(ActorId, Action),
+    /// Initialization failed.
+    #[error("initialization failed: {0}")]
+    Init(&'static str),
 }
 
 /// Patch operation.
@@ -341,7 +343,7 @@ impl MergeTarget {
 }
 
 /// Patch state.
-#[derive(Debug, Clone, PartialEq, Eq, Default)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Patch {
     /// Title of the patch.
     pub(super) title: String,
@@ -374,6 +376,24 @@ pub struct Patch {
 }
 
 impl Patch {
+    /// Create a valid patch
+    pub fn new(
+        title: String,
+        target: MergeTarget,
+        (revision_id, revision): (RevisionId, Revision),
+    ) -> Self {
+        Self {
+            title,
+            state: State::default(),
+            target,
+            labels: BTreeSet::default(),
+            merges: BTreeMap::default(),
+            revisions: BTreeMap::from([(revision_id, Some(revision))]),
+            assignees: BTreeSet::default(),
+            timeline: vec![revision_id.into_inner()],
+            reviews: BTreeMap::default(),
+        }
+    }
     /// Title of the patch.
     pub fn title(&self) -> &str {
         self.title.as_str()
@@ -523,6 +543,118 @@ impl Patch {
     pub fn is_draft(&self) -> bool {
         matches!(self.state(), State::Draft)
     }
+
+    /// Apply authorization rules on patch actions.
+    pub fn authorization(
+        &self,
+        action: &Action,
+        actor: &ActorId,
+        doc: &Doc<Verified>,
+    ) -> Result<Authorization, Error> {
+        if doc.is_delegate(actor) {
+            // A delegate is authorized to do all actions.
+            return Ok(Authorization::Allow);
+        }
+        let author = self.author().id().as_key();
+        let outcome = match action {
+            // The patch author can edit the patch and change its state.
+            Action::Edit { .. } => Authorization::from(actor == author),
+            Action::Lifecycle { state } => Authorization::from(match state {
+                Lifecycle::Open { .. } => actor == author,
+                Lifecycle::Draft { .. } => actor == author,
+                Lifecycle::Archived { .. } => actor == author,
+            }),
+            // Only delegates can carry out these actions.
+            Action::Label { .. } => Authorization::Deny,
+            Action::Assign { .. } => Authorization::Deny,
+            Action::Merge { .. } => match self.target() {
+                MergeTarget::Delegates => Authorization::Deny,
+            },
+            // Anyone can submit a review.
+            Action::Review { .. } => Authorization::Allow,
+            // The review author can edit a review.
+            Action::ReviewEdit { review, .. } => {
+                if let Some((_, review)) = lookup::review(self, review)? {
+                    Authorization::from(actor == review.author.public_key())
+                } else {
+                    // Redacted.
+                    Authorization::Unknown
+                }
+            }
+            Action::ReviewRedact { review, .. } => {
+                if let Some((_, review)) = lookup::review(self, review)? {
+                    Authorization::from(actor == review.author.public_key())
+                } else {
+                    // Redacted.
+                    Authorization::Unknown
+                }
+            }
+            // Anyone can comment on a review.
+            Action::ReviewComment { .. } => Authorization::Allow,
+            // The comment author can edit and redact their own comment.
+            Action::ReviewCommentEdit {
+                review, comment, ..
+            }
+            | Action::ReviewCommentRedact { review, comment } => {
+                if let Some((_, review)) = lookup::review(self, review)? {
+                    if let Some(comment) = review.comments.comment(comment) {
+                        return Ok(Authorization::from(*actor == comment.author()));
+                    }
+                }
+                // Redacted.
+                Authorization::Unknown
+            }
+            // Anyone can react to a review comment.
+            Action::ReviewCommentReact { .. } => Authorization::Allow,
+            // The reviewer, commenter or revision author can resolve and unresolve review comments.
+            Action::ReviewCommentResolve { review, comment }
+            | Action::ReviewCommentUnresolve { review, comment } => {
+                if let Some((revision, review)) = lookup::review(self, review)? {
+                    if let Some(comment) = review.comments.comment(comment) {
+                        return Ok(Authorization::from(
+                            actor == &comment.author()
+                                || actor == review.author.public_key()
+                                || actor == revision.author.public_key(),
+                        ));
+                    }
+                }
+                // Redacted.
+                Authorization::Unknown
+            }
+            // Only patch authors can propose revisions.
+            Action::Revision { .. } => Authorization::from(actor == author),
+            // Only the revision author can edit or redact their revision.
+            Action::RevisionEdit { revision, .. } | Action::RevisionRedact { revision, .. } => {
+                if let Some(revision) = lookup::revision(self, revision)? {
+                    Authorization::from(actor == revision.author.public_key())
+                } else {
+                    // Redacted.
+                    Authorization::Unknown
+                }
+            }
+            // Anyone can react to or comment on a revision.
+            Action::RevisionReact { .. } => Authorization::Allow,
+            Action::RevisionComment { .. } => Authorization::Allow,
+            // Only the comment author can edit or redact their comment.
+            Action::RevisionCommentEdit {
+                revision, comment, ..
+            }
+            | Action::RevisionCommentRedact {
+                revision, comment, ..
+            } => {
+                if let Some(revision) = lookup::revision(self, revision)? {
+                    if let Some(comment) = revision.discussion.comment(comment) {
+                        return Ok(Authorization::from(actor == &comment.author()));
+                    }
+                }
+                // Redacted.
+                Authorization::Unknown
+            }
+            // Anyone can react to a revision.
+            Action::RevisionCommentReact { .. } => Authorization::Allow,
+        };
+        Ok(outcome)
+    }
 }
 
 impl Patch {
@@ -533,7 +665,7 @@ impl Patch {
         entry: EntryId,
         author: ActorId,
         timestamp: Timestamp,
-        identity: git::Oid,
+        identity: &Doc<Verified>,
         repo: &R,
     ) -> Result<(), Error> {
         match action {
@@ -576,19 +708,18 @@ impl Patch {
                 summary,
                 verdict,
             } => {
-                let Some(Some((revision, author))) =
+                let Some(Some((revision, author))) = self.reviews.get(&review) else {
-                        self.reviews.get(&review) else {
+                    return Err(Error::Missing(review.into_inner()));
-                            return Err(Error::Missing(review.into_inner()));
+                };
-                    };
                 let Some(rev) = self.revisions.get_mut(revision) else {
-                        return Err(Error::Missing(revision.into_inner()));
+                    return Err(Error::Missing(revision.into_inner()));
-                    };
+                };
                 // If the revision was redacted concurrently, there's nothing to do.
                 // Likewise, if the review was redacted concurrently, there's nothing to do.
                 if let Some(rev) = rev {
                     let Some(review) = rev.reviews.get_mut(author) else {
-                            return Err(Error::Missing(review.into_inner()));
+                        return Err(Error::Missing(review.into_inner()));
-                        };
+                    };
                     if let Some(review) = review {
                         review.summary = summary;
                         review.verdict = verdict;
@@ -621,7 +752,7 @@ impl Patch {
                 active,
                 location,
             } => {
-                if let Some(revision) = lookup::revision(self, &revision)? {
+                if let Some(revision) = lookup::revision_mut(self, &revision)? {
                     let key = (author, reaction);
                     let reactions = revision.reactions.entry(location).or_default();
 
@@ -652,14 +783,20 @@ impl Patch {
                 labels,
             } => {
                 let Some(rev) = self.revisions.get_mut(&revision) else {
-                        return Err(Error::Missing(revision.into_inner()));
+                    return Err(Error::Missing(revision.into_inner()));
-                    };
+                };
                 if let Some(rev) = rev {
                     // Nb. Applying two reviews by the same author is not allowed and
                     // results in the review being redacted.
                     rev.reviews.insert(
                         author,
-                        Some(Review::new(verdict, summary.to_owned(), labels, timestamp)),
+                        Some(Review::new(
+                            Author::new(author),
+                            verdict,
+                            summary.to_owned(),
+                            labels,
+                            timestamp,
+                        )),
                     );
                     // Update reviews index.
                     self.reviews
@@ -672,7 +809,7 @@ impl Patch {
                 reaction,
                 active,
             } => {
-                if let Some(review) = lookup::review(self, &review)? {
+                if let Some(review) = lookup::review_mut(self, &review)? {
                     thread::react(
                         &mut review.comments,
                         entry,
@@ -684,7 +821,7 @@ impl Patch {
                 }
             }
             Action::ReviewCommentRedact { review, comment } => {
-                if let Some(review) = lookup::review(self, &review)? {
+                if let Some(review) = lookup::review_mut(self, &review)? {
                     thread::redact(&mut review.comments, entry, comment)?;
                 }
             }
@@ -693,7 +830,7 @@ impl Patch {
                 comment,
                 body,
             } => {
-                if let Some(review) = lookup::review(self, &review)? {
+                if let Some(review) = lookup::review_mut(self, &review)? {
                     thread::edit(
                         &mut review.comments,
                         entry,
@@ -705,12 +842,12 @@ impl Patch {
                 }
             }
             Action::ReviewCommentResolve { review, comment } => {
-                if let Some(review) = lookup::review(self, &review)? {
+                if let Some(review) = lookup::review_mut(self, &review)? {
                     thread::resolve(&mut review.comments, entry, comment)?;
                 }
             }
             Action::ReviewCommentUnresolve { review, comment } => {
-                if let Some(review) = lookup::review(self, &review)? {
+                if let Some(review) = lookup::review_mut(self, &review)? {
                     thread::unresolve(&mut review.comments, entry, comment)?;
                 }
             }
@@ -720,7 +857,7 @@ impl Patch {
                 location,
                 reply_to,
             } => {
-                if let Some(review) = lookup::review(self, &review)? {
+                if let Some(review) = lookup::review_mut(self, &review)? {
                     thread::comment(
                         &mut review.comments,
                         entry,
@@ -736,41 +873,36 @@ impl Patch {
             Action::ReviewRedact { review } => {
                 // Redactions must have observed a review to be valid.
                 let Some(locator) = self.reviews.get_mut(&review) else {
-                        return Err(Error::Missing(review.into_inner()));
+                    return Err(Error::Missing(review.into_inner()));
-                    };
+                };
                 // If the review is already redacted, do nothing.
                 let Some((revision, reviewer)) = locator else {
-                        return Ok(());
+                    return Ok(());
-                    };
+                };
                 // The revision must have existed at some point.
                 let Some(redactable) = self.revisions.get_mut(revision) else {
-                        return Err(Error::Missing(revision.into_inner()));
+                    return Err(Error::Missing(revision.into_inner()));
-                    };
+                };
                 // But it could be redacted.
                 let Some(revision) = redactable else {
-                        return Ok(());
+                    return Ok(());
-                    };
+                };
                 // The review must have existed as well.
                 let Some(review) = revision.reviews.get_mut(reviewer) else {
-                        return Err(Error::Missing(review.into_inner()));
+                    return Err(Error::Missing(review.into_inner()));
-                    };
+                };
                 // Set both the review and the locator in the review index to redacted.
                 *review = None;
                 *locator = None;
             }
             Action::Merge { revision, commit } => {
                 // If the revision was redacted before the merge, ignore the merge.
-                if lookup::revision(self, &revision)?.is_none() {
+                if lookup::revision_mut(self, &revision)?.is_none() {
                     return Ok(());
                 };
-                let doc = repo.identity_doc_at(identity)?.verified()?;
-
                 match self.target() {
                     MergeTarget::Delegates => {
-                        if !doc.is_delegate(&author) {
+                        let proj = identity.project()?;
-                            return Err(Error::InvalidMerge(entry));
-                        }
-                        let proj = doc.project()?;
                         let branch = git::refs::branch(proj.default_branch());
 
                         // Nb. We don't return an error in case the merge commit is not an
@@ -804,7 +936,7 @@ impl Patch {
                     },
                 );
                 // Discard revisions that weren't merged by a threshold of delegates.
-                merges.retain(|_, count| *count >= doc.threshold);
+                merges.retain(|_, count| *count >= identity.threshold);
 
                 match merges.into_keys().collect::<Vec<_>>().as_slice() {
                     [] => {
@@ -832,7 +964,7 @@ impl Patch {
                 reply_to,
                 ..
             } => {
-                if let Some(revision) = lookup::revision(self, &revision)? {
+                if let Some(revision) = lookup::revision_mut(self, &revision)? {
                     thread::comment(
                         &mut revision.discussion,
                         entry,
@@ -850,7 +982,7 @@ impl Patch {
                 comment,
                 body,
             } => {
-                if let Some(revision) = lookup::revision(self, &revision)? {
+                if let Some(revision) = lookup::revision_mut(self, &revision)? {
                     thread::edit(
                         &mut revision.discussion,
                         entry,
@@ -862,7 +994,7 @@ impl Patch {
                 }
             }
             Action::RevisionCommentRedact { revision, comment } => {
-                if let Some(revision) = lookup::revision(self, &revision)? {
+                if let Some(revision) = lookup::revision_mut(self, &revision)? {
                     thread::redact(&mut revision.discussion, entry, comment)?;
                 }
             }
@@ -872,7 +1004,7 @@ impl Patch {
                 reaction,
                 active,
             } => {
-                if let Some(revision) = lookup::revision(self, &revision)? {
+                if let Some(revision) = lookup::revision_mut(self, &revision)? {
                     thread::react(
                         &mut revision.discussion,
                         entry,
@@ -896,22 +1028,53 @@ impl store::FromHistory for Patch {
         &TYPENAME
     }
 
-    fn validate(&self) -> Result<(), Self::Error> {
+    fn init<R: ReadRepository>(op: Op, repo: &R) -> Result<Self, Self::Error> {
-        if self.revisions.is_empty() {
+        let mut actions = op.actions.into_iter();
-            return Err(Error::Validate("no revisions found"));
+        let Some(Action::Revision { description, base, oid, resolves }) = actions.next() else {
+            return Err(Error::Init("the first action must be of type `revision`"));
+        };
+        let Some(Action::Edit { title, target }) = actions.next() else {
+            return Err(Error::Init("the second action must be of type `edit`"));
+        };
+        let revision = Revision::new(
+            op.author.into(),
+            description,
+            base,
+            oid,
+            op.timestamp,
+            resolves,
+        );
+        let mut patch = Patch::new(title, target, (RevisionId(op.id), revision));
+        let doc = repo.identity_doc_at(op.identity)?.verified()?;
+
+        for action in actions {
+            patch.action(action, op.id, op.author, op.timestamp, &doc, repo)?;
         }
-        if self.title().is_empty() {
+        Ok(patch)
-            return Err(Error::Validate("empty title"));
-        }
-        Ok(())
     }
 
     fn apply<R: ReadRepository>(&mut self, op: Op, repo: &R) -> Result<(), Error> {
         debug_assert!(!self.timeline.contains(&op.id));
         self.timeline.push(op.id);
 
+        let doc = repo.identity_doc_at(op.identity)?.verified()?;
+
         for action in op.actions {
-            self.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
+            match self.authorization(&action, &op.author, &doc)? {
+                Authorization::Allow => {
+                    self.action(action, op.id, op.author, op.timestamp, &doc, repo)?;
+                }
+                Authorization::Deny => {
+                    return Err(Error::NotAuthorized(op.author, action));
+                }
+                Authorization::Unknown => {
+                    // In this case, since there is not enough information to determine
+                    // whether the action is authorized or not, we simply ignore it.
+                    // It's likely that the target object was redacted, and we can't
+                    // verify whether the action would have been allowed or not.
+                    continue;
+                }
+            }
         }
         Ok(())
     }
@@ -921,6 +1084,19 @@ mod lookup {
     use super::*;
 
     pub fn revision<'a>(
+        patch: &'a Patch,
+        revision: &RevisionId,
+    ) -> Result<Option<&'a Revision>, Error> {
+        match patch.revisions.get(revision) {
+            Some(Some(revision)) => Ok(Some(revision)),
+            // Redacted.
+            Some(None) => Ok(None),
+            // Missing. Causal error.
+            None => Err(Error::Missing(revision.into_inner())),
+        }
+    }
+
+    pub fn revision_mut<'a>(
         patch: &'a mut Patch,
         revision: &RevisionId,
     ) -> Result<Option<&'a mut Revision>, Error> {
@@ -934,6 +1110,35 @@ mod lookup {
     }
 
     pub fn review<'a>(
+        patch: &'a Patch,
+        review: &ReviewId,
+    ) -> Result<Option<(&'a Revision, &'a Review)>, Error> {
+        match patch.reviews.get(review) {
+            Some(Some((revision, author))) => {
+                match patch.revisions.get(revision) {
+                    Some(Some(r)) => {
+                        let Some(review) = r.reviews.get(author) else {
+                            return Err(Error::Missing(review.into_inner()));
+                        };
+                        Ok(review.as_ref().map(|review| (r, review)))
+                    }
+                    Some(None) => {
+                        // If the revision was redacted concurrently, there's nothing to do.
+                        // Likewise, if the review was redacted concurrently, there's nothing to do.
+                        Ok(None)
+                    }
+                    None => Err(Error::Missing(revision.into_inner())),
+                }
+            }
+            Some(None) => {
+                // Redacted.
+                Ok(None)
+            }
+            None => Err(Error::Missing(review.into_inner())),
+        }
+    }
+
+    pub fn review_mut<'a>(
         patch: &'a mut Patch,
         review: &ReviewId,
     ) -> Result<Option<&'a mut Review>, Error> {
@@ -942,8 +1147,8 @@ mod lookup {
                 match patch.revisions.get_mut(revision) {
                     Some(Some(r)) => {
                         let Some(review) = r.reviews.get_mut(author) else {
-                        return Err(Error::Missing(review.into_inner()));
+                            return Err(Error::Missing(review.into_inner()));
-                    };
+                        };
                         Ok(review.as_mut())
                     }
                     Some(None) => {
@@ -1180,8 +1385,10 @@ pub struct CodeLocation {
 }
 
 /// A patch review on a revision.
-#[derive(Debug, Default, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Review {
+    /// Review author.
+    pub(super) author: Author,
     /// Review verdict.
     ///
     /// The verdict cannot be changed, since revisions are immutable.
@@ -1218,12 +1425,14 @@ impl Serialize for Review {
 
 impl Review {
     pub fn new(
+        author: Author,
         verdict: Option<Verdict>,
         summary: Option<String>,
         labels: Vec<Label>,
         timestamp: Timestamp,
     ) -> Self {
         Self {
+            author,
             verdict,
             summary,
             comments: Thread::default(),
@@ -2192,37 +2401,57 @@ mod test {
         let base = git::Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap();
         let oid = git::Oid::from_str("518d5069f94c03427f694bb494ac1cd7d1339380").unwrap();
         let mut alice = Actor::new(MockSigner::default());
-        let mut patch = Patch::default();
+        let rid = gen::<Id>(1);
-        let repo = gen::<MockRepository>(1);
+        let doc = Doc::new(
+            gen::<Project>(1),
+            nonempty::NonEmpty::new(alice.did()),
+            1,
+            identity::Visibility::Public,
+        )
+        .verified()
+        .unwrap();
+        let repo = MockRepository::new(rid, doc);
 
-        let a1 = alice.op::<Patch>(Action::Revision {
+        let a1 = alice.op::<Patch>([
-            description: String::new(),
+            Action::Revision {
+                description: String::new(),
+                base,
+                oid,
+                resolves: Default::default(),
+            },
+            Action::Edit {
+                title: String::from("My patch"),
+                target: MergeTarget::Delegates,
+            },
+        ]);
+        let a2 = alice.op::<Patch>([Action::Revision {
+            description: String::from("Second revision"),
             base,
             oid,
             resolves: Default::default(),
-        });
+        }]);
-        let a2 = alice.op::<Patch>(Action::RevisionRedact {
+        let a3 = alice.op::<Patch>([Action::RevisionRedact {
-            revision: RevisionId(a1.id()),
+            revision: RevisionId(a2.id()),
-        });
+        }]);
-        let a3 = alice.op::<Patch>(Action::Review {
+        let a4 = alice.op::<Patch>([Action::Review {
-            revision: RevisionId(a1.id()),
+            revision: RevisionId(a2.id()),
             summary: None,
             verdict: Some(Verdict::Accept),
             labels: vec![],
-        });
+        }]);
-        let a4 = alice.op::<Patch>(Action::Merge {
+        let a5 = alice.op::<Patch>([Action::Merge {
-            revision: RevisionId(a1.id()),
+            revision: RevisionId(a2.id()),
             commit: oid,
-        });
+        }]);
 
-        patch.apply(a1, &repo).unwrap();
+        let mut patch = Patch::from_ops([a1, a2], &repo).unwrap();
-        assert!(patch.revisions().next().is_some());
+        assert_eq!(patch.revisions().count(), 2);
-
-        patch.apply(a2, &repo).unwrap();
-        assert!(patch.revisions().next().is_none());
 
         patch.apply(a3, &repo).unwrap();
+        assert_eq!(patch.revisions().count(), 1);
+
         patch.apply(a4, &repo).unwrap();
+        patch.apply(a5, &repo).unwrap();
     }
 
     #[test]
@@ -2234,22 +2463,32 @@ mod test {
         let alice = MockSigner::default();
         let bob = MockSigner::default();
         let mut h0: cob::test::HistoryBuilder<Patch> = cob::test::history(
-            &Action::Revision {
+            &[
-                description: String::from("Original"),
+                Action::Revision {
-                base,
+                    description: String::from("Original"),
-                oid,
+                    base,
-                resolves: Default::default(),
+                    oid,
-            },
+                    resolves: Default::default(),
+                },
+                Action::Edit {
+                    title: String::from("Some patch"),
+                    target: MergeTarget::Delegates,
+                },
+            ],
             time,
             &alice,
         );
         h0.commit(
-            &Action::Edit {
+            &Action::Revision {
-                title: String::from("Some patch"),
+                description: String::from("New"),
-                target: MergeTarget::Delegates,
+                base,
+                oid,
+                resolves: Default::default(),
             },
             &alice,
         );
+        let patch = Patch::from_history(&h0, &repo).unwrap();
+        assert_eq!(patch.revisions().count(), 2);
 
         let mut h1 = h0.clone();
         h1.commit(
@@ -2272,7 +2511,7 @@ mod test {
         h0.merge(h2);
 
         let patch = Patch::from_history(&h0, &repo).unwrap();
-        assert_eq!(patch.revisions().count(), 0);
+        assert_eq!(patch.revisions().count(), 1);
     }
 
     #[test]
@@ -2283,18 +2522,24 @@ mod test {
         let repo = gen::<MockRepository>(1);
         let reaction = Reaction::new('👍').expect("failed to create a reaction");
 
-        let a1 = alice.op::<Patch>(Action::Revision {
+        let a1 = alice.op::<Patch>([
-            description: String::new(),
+            Action::Revision {
-            base,
+                description: String::new(),
-            oid,
+                base,
-            resolves: Default::default(),
+                oid,
-        });
+                resolves: Default::default(),
-        let a2 = alice.op::<Patch>(Action::RevisionReact {
+            },
+            Action::Edit {
+                title: String::from("My patch"),
+                target: MergeTarget::Delegates,
+            },
+        ]);
+        let a2 = alice.op::<Patch>([Action::RevisionReact {
             revision: RevisionId(a1.id()),
             location: None,
             reaction,
             active: true,
-        });
+        }]);
         let patch = Patch::from_ops([a1, a2], &repo).unwrap();
 
         let (_, r1) = patch.revisions().next().unwrap();

radicle/src/cob/store.rs

@@ -1,9 +1,9 @@
 //! Generic COB storage.
 #![allow(clippy::large_enum_variant)]
 #![allow(clippy::type_complexity)]
+use std::fmt::Debug;
 use std::marker::PhantomData;
 use std::ops::ControlFlow;
-use std::sync::Arc;
 
 use nonempty::NonEmpty;
 use serde::{Deserialize, Serialize};
@@ -19,7 +19,15 @@ use crate::storage::git as storage;
 use crate::storage::SignRepository;
 use crate::{cob, identity};
 
-pub trait HistoryAction: std::fmt::Debug {
+#[derive(Debug, thiserror::Error)]
+pub enum HistoryError<T: FromHistory> {
+    #[error("apply: {0}")]
+    Apply(T::Error),
+    #[error("operation decoding failed: {0}")]
+    Op(#[from] cob::op::OpEncodingError),
+}
+
+pub trait HistoryAction: Debug {
     /// Parent objects this action depends on. For example, patch revisions
     /// have the commit objects as their parent.
     fn parents(&self) -> Vec<git::Oid> {
@@ -29,7 +37,7 @@ pub trait HistoryAction: std::fmt::Debug {
 
 /// A type that can be materialized from an event history.
 /// All collaborative objects implement this trait.
-pub trait FromHistory: Sized + Default + PartialEq {
+pub trait FromHistory: Sized + PartialEq + Debug {
     /// The underlying action composing each operation.
     type Action: HistoryAction + for<'de> Deserialize<'de> + Serialize;
     /// Error returned by `apply` function.
@@ -38,6 +46,9 @@ pub trait FromHistory: Sized + Default + PartialEq {
     /// The object type name.
     fn type_name() -> &'static TypeName;
 
+    /// Initialize a collarorative object
+    fn init<R: ReadRepository>(op: Op<Self::Action>, repo: &R) -> Result<Self, Self::Error>;
+
     /// Apply a list of operations to the state.
     fn apply<R: ReadRepository>(
         &mut self,
@@ -45,21 +56,26 @@ pub trait FromHistory: Sized + Default + PartialEq {
         repo: &R,
     ) -> Result<(), Self::Error>;
 
-    /// Validate the object. Returns an error if the object is invalid.
-    fn validate(&self) -> Result<(), Self::Error>;
-
     /// Create an object from a history.
-    fn from_history<R: ReadRepository>(history: &History, repo: &R) -> Result<Self, Self::Error> {
+    fn from_history<R: ReadRepository>(
+        history: &History,
+        repo: &R,
+    ) -> Result<Self, HistoryError<Self>> {
         self::from_history::<R, Self>(history, repo)
     }
 
+    #[cfg(test)]
     /// Create an object from individual operations.
     /// Returns an error if any of the operations fails to apply.
     fn from_ops<R: ReadRepository>(
         ops: impl IntoIterator<Item = Op<Self::Action>>,
         repo: &R,
     ) -> Result<Self, Self::Error> {
-        let mut state = Self::default();
+        let mut ops = ops.into_iter();
+        let Some(init) = ops.next() else {
+            panic!("FromHistory::from_ops: operations list is empty");
+        };
+        let mut state = Self::init(init, repo)?;
         for op in ops {
             state.apply(op, repo)?;
         }
@@ -72,8 +88,12 @@ pub trait FromHistory: Sized + Default + PartialEq {
 pub fn from_history<R: ReadRepository, T: FromHistory>(
     history: &History,
     repo: &R,
-) -> Result<T, T::Error> {
+) -> Result<T, HistoryError<T>> {
-    let obj = history.traverse(T::default(), |mut acc, _, entry| {
+    let root = history.root();
+    let children = history.children_of(root.id());
+    let op = Op::try_from(root)?;
+    let initial = T::init(op, repo).map_err(HistoryError::Apply)?;
+    let obj = history.traverse(initial, &children, |mut acc, _, entry| {
         match Op::try_from(entry) {
             Ok(op) => {
                 if let Err(err) = acc.apply(op, repo) {
@@ -89,8 +109,6 @@ pub fn from_history<R: ReadRepository, T: FromHistory>(
         ControlFlow::Continue(acc)
     });
 
-    obj.validate()?;
-
     Ok(obj)
 }
 
@@ -111,8 +129,6 @@ pub enum Error {
     Serialize(#[from] serde_json::Error),
     #[error("object `{1}` of type `{0}` was not found")]
     NotFound(TypeName, ObjectId),
-    #[error("apply: {0}")]
-    Apply(Arc<dyn std::error::Error + Sync + Send + 'static>),
     #[error("signed refs: {0}")]
     SignRefs(#[from] storage::Error),
     #[error("failed to find reference '{name}': {err}")]
@@ -121,11 +137,13 @@ pub enum Error {
         #[source]
         err: git::Error,
     },
+    #[error("history: {0}")]
+    History(Box<dyn std::error::Error + Sync + Send + 'static>),
 }
 
 impl Error {
-    fn apply(e: impl std::error::Error + Sync + Send + 'static) -> Self {
+    fn history(e: impl std::error::Error + Send + Sync + 'static) -> Self {
-        Self::Apply(Arc::new(e))
+        Self::History(Box::new(e))
     }
 }
 
@@ -158,7 +176,7 @@ impl<'a, T, R: ReadRepository> Store<'a, T, R> {
 impl<'a, T, R> Store<'a, T, R>
 where
     R: ReadRepository + SignRepository + cob::Store,
-    T: FromHistory,
+    T: FromHistory + 'static,
     T::Action: Serialize,
 {
     /// Update an object.
@@ -218,7 +236,7 @@ where
                 contents,
             },
         )?;
-        let object = T::from_history(cob.history(), self.repo).map_err(Error::apply)?;
+        let object = T::from_history(cob.history(), self.repo).map_err(Error::history)?;
 
         self.repo.sign_refs(signer).map_err(Error::SignRefs)?;
 
@@ -250,7 +268,7 @@ where
 impl<'a, T, R> Store<'a, T, R>
 where
     R: ReadRepository + cob::Store,
-    T: FromHistory,
+    T: FromHistory + 'static,
     T::Action: Serialize,
 {
     /// Get an object.
@@ -258,7 +276,7 @@ where
         let cob = cob::get(self.repo, T::type_name(), id)?;
 
         if let Some(cob) = cob {
-            let obj = T::from_history(cob.history(), self.repo).map_err(Error::apply)?;
+            let obj = T::from_history(cob.history(), self.repo).map_err(Error::history)?;
 
             Ok(Some(obj))
         } else {
@@ -271,7 +289,7 @@ where
         let raw = cob::list(self.repo, T::type_name())?;
 
         Ok(raw.into_iter().map(|o| {
-            let obj = T::from_history(o.history(), self.repo).map_err(Error::apply)?;
+            let obj = T::from_history(o.history(), self.repo).map_err(Error::history)?;
             Ok((*o.id(), obj))
         }))
     }
@@ -297,7 +315,7 @@ pub struct Transaction<T: FromHistory> {
     embeds: Vec<Embed>,
 }
 
-impl<T: FromHistory> Transaction<T> {
+impl<T: FromHistory + 'static> Transaction<T> {
     /// Create a new transaction.
     pub fn new(actor: ActorId) -> Self {
         Self {
@@ -395,10 +413,13 @@ pub fn ops<R: cob::Store>(
     let cob = cob::get(repo, type_name, id)?;
 
     if let Some(cob) = cob {
-        let ops = cob.history().traverse(Vec::new(), |mut ops, _, entry| {
+        let root = cob.history().root();
-            ops.push(Op::from(entry.clone()));
+        let ops = cob
-            ControlFlow::Continue(ops)
+            .history()
-        });
+            .traverse(Vec::new(), &[root.id], |mut ops, _, entry| {
+                ops.push(Op::from(entry.clone()));
+                ControlFlow::Continue(ops)
+            });
         Ok(ops)
     } else {
         Err(Error::NotFound(type_name.clone(), *id))

radicle/src/cob/test.rs

@@ -57,18 +57,23 @@ impl<T: FromHistory> HistoryBuilder<T>
 where
     T::Action: for<'de> Deserialize<'de> + Serialize + Eq + 'static,
 {
-    pub fn new<G: Signer>(action: &T::Action, time: Timestamp, signer: &G) -> HistoryBuilder<T> {
+    pub fn new<G: Signer>(actions: &[T::Action], time: Timestamp, signer: &G) -> HistoryBuilder<T> {
         let resource = arbitrary::oid();
         let revision = arbitrary::oid();
-        let (data, root) = encoded::<T, _>(action, time, [], signer);
+        let (contents, oids): (Vec<Vec<u8>>, Vec<Oid>) = actions
+            .iter()
+            .map(|a| encoded::<T, _>(a, time, [], signer))
+            .unzip();
+        let contents = NonEmpty::from_vec(contents).unwrap();
+        let root = oids.first().unwrap();
         let manifest = Manifest::new(T::type_name().clone(), Version::default());
-        let signature = signer.sign(data.as_slice());
+        let signature = signer.sign(&[0]);
         let signature = ExtendedSignature::new(*signer.public_key(), signature);
         let change = Entry {
-            id: root,
+            id: *root,
             signature,
             resource,
-            contents: NonEmpty::new(data),
+            contents,
             timestamp: time.as_secs(),
             revision,
             parents: vec![],
@@ -125,14 +130,14 @@ impl<A> Deref for HistoryBuilder<A> {
 
 /// Create a new test history.
 pub fn history<T: FromHistory, G: Signer>(
-    action: &T::Action,
+    actions: &[T::Action],
     time: Timestamp,
     signer: &G,
 ) -> HistoryBuilder<T>
 where
     T::Action: Serialize + Eq + 'static,
 {
-    HistoryBuilder::new(action, time, signer)
+    HistoryBuilder::new(actions, time, signer)
 }
 
 /// An object that can be used to create and sign operations.
@@ -156,22 +161,23 @@ impl<G: Signer> Actor<G> {
     /// Create a new operation.
     pub fn op_with<T: FromHistory>(
         &mut self,
-        action: T::Action,
+        actions: impl IntoIterator<Item = T::Action>,
         identity: Oid,
         timestamp: Timestamp,
     ) -> Op<T::Action>
     where
         T::Action: Clone + Serialize,
     {
+        let actions = actions.into_iter().collect::<Vec<_>>();
         let data = encoding::encode(serde_json::json!({
-            "action": action,
+            "action": actions,
             "nonce": fastrand::u64(..),
         }))
         .unwrap();
         let oid = git::raw::Oid::hash_object(git::raw::ObjectType::Blob, &data).unwrap();
         let id = oid.into();
         let author = *self.signer.public_key();
-        let actions = NonEmpty::new(action);
+        let actions = NonEmpty::from_vec(actions).unwrap();
         let manifest = Manifest::new(T::type_name().clone(), Version::default());
         let parents = vec![];
 
@@ -187,14 +193,17 @@ impl<G: Signer> Actor<G> {
     }
 
     /// Create a new operation.
-    pub fn op<T: FromHistory>(&mut self, action: T::Action) -> Op<T::Action>
+    pub fn op<T: FromHistory>(
+        &mut self,
+        actions: impl IntoIterator<Item = T::Action>,
+    ) -> Op<T::Action>
     where
         T::Action: Clone + Serialize,
     {
         let identity = arbitrary::oid();
         let timestamp = Timestamp::now();
 
-        self.op_with::<T>(action, identity, timestamp)
+        self.op_with::<T>(actions, identity, timestamp)
     }
 
     /// Get the actor's DID.
@@ -213,19 +222,19 @@ impl<G: Signer> Actor<G> {
         oid: git::Oid,
         repo: &R,
     ) -> Result<Patch, patch::Error> {
-        Patch::from_ops(
+        Patch::init(
-            [
+            self.op::<Patch>([
-                self.op::<Patch>(patch::Action::Revision {
+                patch::Action::Revision {
                     description: description.to_string(),
                     base,
                     oid,
                     resolves: Default::default(),
-                }),
+                },
-                self.op::<Patch>(patch::Action::Edit {
+                patch::Action::Edit {
                     title: title.to_string(),
                     target: patch::MergeTarget::default(),
-                }),
+                },
-            ],
+            ]),
             repo,
         )
     }

radicle/src/cob/thread.rs

@@ -9,6 +9,7 @@ use thiserror::Error;
 use crate::cob;
 use crate::cob::common::{Reaction, Timestamp, Uri};
 use crate::cob::{ActorId, Embed, EntryId, Op};
+use crate::git;
 use crate::prelude::ReadRepository;
 
 /// Type name of a thread, as well as the domain for all thread operations.
@@ -28,15 +29,15 @@ pub enum Error {
     /// that hasn't happened yet.
     #[error("causal dependency {0:?} missing")]
     Missing(EntryId),
-    /// Validation error.
-    #[error("validation failed: {0}")]
-    Validate(&'static str),
     /// Error with comment operation.
     #[error("comment {0} is invalid")]
     Comment(EntryId),
     /// Error with edit operation.
     #[error("edit {0} is invalid")]
     Edit(EntryId),
+    /// Object initialization failed.
+    #[error("initialization failed: {0}")]
+    Init(&'static str),
 }
 
 /// Identifies a comment.
@@ -258,7 +259,7 @@ impl<T> Thread<T> {
     pub fn new(id: CommentId, comment: T) -> Self {
         Self {
             comments: BTreeMap::from_iter([(id, Some(comment))]),
-            timeline: Vec::default(),
+            timeline: vec![id],
         }
     }
 
@@ -300,6 +301,39 @@ impl<T> Thread<T> {
     }
 }
 
+impl Thread {
+    /// Apply a single action to the thread.
+    fn action<R: ReadRepository>(
+        &mut self,
+        action: Action,
+        entry: EntryId,
+        author: ActorId,
+        timestamp: Timestamp,
+        _identity: git::Oid,
+        _repo: &R,
+    ) -> Result<(), Error> {
+        match action {
+            Action::Comment { body, reply_to } => {
+                comment(self, entry, author, timestamp, body, reply_to, None, vec![])?;
+            }
+            Action::Edit { id, body } => {
+                edit(self, entry, id, timestamp, body, vec![])?;
+            }
+            Action::Redact { id } => {
+                redact(self, entry, id)?;
+            }
+            Action::React {
+                to,
+                reaction,
+                active,
+            } => {
+                react(self, entry, author, to, reaction, active)?;
+            }
+        }
+        Ok(())
+    }
+}
+
 impl<L> Thread<Comment<L>> {
     pub fn replies<'a>(
         &'a self,
@@ -324,37 +358,36 @@ impl cob::store::FromHistory for Thread {
         &TYPENAME
     }
 
-    fn validate(&self) -> Result<(), Self::Error> {
+    fn init<R: ReadRepository>(op: Op<Action>, repo: &R) -> Result<Self, Self::Error> {
-        if self.comments.is_empty() {
+        let author = op.author;
-            return Err(Error::Validate("no comments found"));
+        let entry = op.id;
+        let timestamp = op.timestamp;
+        let mut actions = op.actions.into_iter();
+        let Some(Action::Comment { body, reply_to: None }) = actions.next() else {
+            return Err(Error::Init("missing initial comment"));
+        };
+
+        let mut thread = Thread::default();
+        comment(
+            &mut thread,
+            entry,
+            author,
+            timestamp,
+            body,
+            None,
+            None,
+            vec![],
+        )?;
+
+        for action in actions {
+            thread.action(action, entry, author, timestamp, op.identity, repo)?;
         }
-        Ok(())
+        Ok(thread)
     }
 
-    fn apply<R: ReadRepository>(&mut self, op: Op<Action>, _repo: &R) -> Result<(), Error> {
+    fn apply<R: ReadRepository>(&mut self, op: Op<Action>, repo: &R) -> Result<(), Error> {
-        let id = op.id;
-        let author = op.author;
-        let timestamp = op.timestamp;
-
         for action in op.actions {
-            match action {
+            self.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
-                Action::Comment { body, reply_to } => {
-                    comment(self, id, author, timestamp, body, reply_to, None, vec![])?;
-                }
-                Action::Edit { id, body } => {
-                    edit(self, op.id, id, timestamp, body, vec![])?;
-                }
-                Action::Redact { id } => {
-                    redact(self, op.id, id)?;
-                }
-                Action::React {
-                    to,
-                    reaction,
-                    active,
-                } => {
-                    react(self, op.id, author, to, reaction, active)?;
-                }
-            }
         }
         Ok(())
     }
@@ -527,23 +560,23 @@ mod tests {
 
         /// Create a new comment.
         pub fn comment(&mut self, body: &str, reply_to: Option<CommentId>) -> Op<Action> {
-            self.op::<Thread>(Action::Comment {
+            self.op::<Thread>([Action::Comment {
                 body: String::from(body),
                 reply_to,
-            })
+            }])
         }
 
         /// Create a new redaction.
         pub fn redact(&mut self, id: CommentId) -> Op<Action> {
-            self.op::<Thread>(Action::Redact { id })
+            self.op::<Thread>([Action::Redact { id }])
         }
 
         /// Edit a comment.
         pub fn edit(&mut self, id: CommentId, body: &str) -> Op<Action> {
-            self.op::<Thread>(Action::Edit {
+            self.op::<Thread>([Action::Edit {
                 id,
                 body: body.to_owned(),
-            })
+            }])
         }
     }
 
@@ -615,10 +648,10 @@ mod tests {
         let time = Timestamp::now();
 
         let mut a = test::history::<Thread, _>(
-            &Action::Comment {
+            &[Action::Comment {
                 body: "Thread root".to_owned(),
                 reply_to: None,
-            },
+            }],
             time,
             &alice,
         );
@@ -682,10 +715,10 @@ mod tests {
         let time = Timestamp::now();
 
         let mut a = test::history::<Thread, _>(
-            &Action::Comment {
+            &[Action::Comment {
                 body: "Thread root".to_owned(),
                 reply_to: None,
-            },
+            }],
             time,
             &alice,
         );
@@ -715,10 +748,10 @@ mod tests {
         let timestamp = Timestamp::from_secs(timestamp);
 
         let h0 = test::history::<Thread, _>(
-            &Action::Comment {
+            &[Action::Comment {
                 body: "Thread root".to_owned(),
                 reply_to: None,
-            },
+            }],
             timestamp,
             &alice,
         );