cob: Move action handling into its own function

This change is in preparation for further changes where action handling
is called from multiple places.

radicle/src/cob/issue.rs

@@ -12,8 +12,9 @@ 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::{store, Embed, EntryId, ObjectId, TypeName};
+use crate::cob::{store, ActorId, Embed, EntryId, ObjectId, TypeName};
 use crate::crypto::Signer;
+use crate::git;
 use crate::prelude::{Did, ReadRepository};
 use crate::storage::WriteRepository;
 
@@ -139,51 +140,9 @@ impl store::FromHistory for Issue {
         }
     }
 
-    fn apply<R: ReadRepository>(&mut self, op: Op, _repo: &R) -> Result<(), Error> {
+    fn apply<R: ReadRepository>(&mut self, op: Op, repo: &R) -> Result<(), Error> {
         for action in op.actions {
-            match action {
+            self.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
-                Action::Assign { assignees } => {
-                    self.assignees = BTreeSet::from_iter(assignees);
-                }
-                Action::Edit { title } => {
-                    self.title = title;
-                }
-                Action::Lifecycle { state } => {
-                    self.state = state;
-                }
-                Action::Label { labels } => {
-                    self.labels = BTreeSet::from_iter(labels);
-                }
-                Action::Comment {
-                    body,
-                    reply_to,
-                    embeds,
-                } => {
-                    thread::comment(
-                        &mut self.thread,
-                        op.id,
-                        op.author,
-                        op.timestamp,
-                        body,
-                        reply_to,
-                        None,
-                        embeds,
-                    )?;
-                }
-                Action::CommentEdit { id, body, embeds } => {
-                    thread::edit(&mut self.thread, op.id, id, op.timestamp, body, embeds)?;
-                }
-                Action::CommentRedact { id } => {
-                    thread::redact(&mut self.thread, op.id, id)?;
-                }
-                Action::CommentReact {
-                    id,
-                    reaction,
-                    active,
-                } => {
-                    thread::react(&mut self.thread, op.id, op.author, id, reaction, active)?;
-                }
-            }
         }
         Ok(())
     }
@@ -240,6 +199,64 @@ impl Issue {
     }
 }
 
+impl Issue {
+    /// Apply a single action to the issue.
+    fn action<R: ReadRepository>(
+        &mut self,
+        action: Action,
+        entry: EntryId,
+        author: ActorId,
+        timestamp: Timestamp,
+        _identity: git::Oid,
+        _repo: &R,
+    ) -> Result<(), Error> {
+        match action {
+            Action::Assign { assignees } => {
+                self.assignees = BTreeSet::from_iter(assignees);
+            }
+            Action::Edit { title } => {
+                self.title = title;
+            }
+            Action::Lifecycle { state } => {
+                self.state = state;
+            }
+            Action::Label { labels } => {
+                self.labels = BTreeSet::from_iter(labels);
+            }
+            Action::Comment {
+                body,
+                reply_to,
+                embeds,
+            } => {
+                thread::comment(
+                    &mut self.thread,
+                    entry,
+                    author,
+                    timestamp,
+                    body,
+                    reply_to,
+                    None,
+                    embeds,
+                )?;
+            }
+            Action::CommentEdit { id, body, embeds } => {
+                thread::edit(&mut self.thread, entry, id, timestamp, body, embeds)?;
+            }
+            Action::CommentRedact { id } => {
+                thread::redact(&mut self.thread, entry, id)?;
+            }
+            Action::CommentReact {
+                id,
+                reaction,
+                active,
+            } => {
+                thread::react(&mut self.thread, entry, author, id, reaction, active)?;
+            }
+        }
+        Ok(())
+    }
+}
+
 impl Deref for Issue {
     type Target = Thread;
 

radicle/src/cob/patch.rs

@@ -525,33 +525,17 @@ impl Patch {
     }
 }
 
-impl store::FromHistory for Patch {
+impl Patch {
-    type Action = Action;
+    /// Apply a single action to the patch.
-    type Error = Error;
+    fn action<R: ReadRepository>(
-
+        &mut self,
-    fn type_name() -> &'static TypeName {
+        action: Action,
-        &TYPENAME
+        entry: EntryId,
-    }
+        author: ActorId,
-
+        timestamp: Timestamp,
-    fn validate(&self) -> Result<(), Self::Error> {
+        identity: git::Oid,
-        if self.revisions.is_empty() {
+        repo: &R,
-            return Err(Error::Validate("no revisions found"));
+    ) -> Result<(), Error> {
-        }
-        if self.title().is_empty() {
-            return Err(Error::Validate("empty title"));
-        }
-        Ok(())
-    }
-
-    fn apply<R: ReadRepository>(&mut self, op: Op, repo: &R) -> Result<(), Error> {
-        let author = Author::new(op.author);
-        let timestamp = op.timestamp;
-
-        debug_assert!(!self.timeline.contains(&op.id));
-
-        self.timeline.push(op.id);
-
-        for action in op.actions {
         match action {
             Action::Edit { title, target } => {
                 self.title = title;
@@ -617,12 +601,12 @@ impl store::FromHistory for Patch {
                 oid,
                 resolves,
             } => {
-                    debug_assert!(!self.revisions.contains_key(&op.id));
+                debug_assert!(!self.revisions.contains_key(&entry));
 
                 self.revisions.insert(
-                        RevisionId(op.id),
+                    RevisionId(entry),
                     Some(Revision::new(
-                            author.clone(),
+                        author.into(),
                         description,
                         base,
                         oid,
@@ -638,7 +622,7 @@ impl store::FromHistory for Patch {
                 location,
             } => {
                 if let Some(revision) = lookup::revision(self, &revision)? {
-                        let key = (op.author, reaction);
+                    let key = (author, reaction);
                     let reactions = revision.reactions.entry(location).or_default();
 
                     if active {
@@ -674,12 +658,12 @@ impl store::FromHistory for Patch {
                     // Nb. Applying two reviews by the same author is not allowed and
                     // results in the review being redacted.
                     rev.reviews.insert(
-                            op.author,
+                        author,
                         Some(Review::new(verdict, summary.to_owned(), labels, timestamp)),
                     );
                     // Update reviews index.
                     self.reviews
-                            .insert(ReviewId(op.id), Some((revision, op.author)));
+                        .insert(ReviewId(entry), Some((revision, author)));
                 }
             }
             Action::ReviewCommentReact {
@@ -691,8 +675,8 @@ impl store::FromHistory for Patch {
                 if let Some(review) = lookup::review(self, &review)? {
                     thread::react(
                         &mut review.comments,
-                            op.id,
+                        entry,
-                            author.id.into(),
+                        author,
                         comment,
                         reaction,
                         active,
@@ -701,7 +685,7 @@ impl store::FromHistory for Patch {
             }
             Action::ReviewCommentRedact { review, comment } => {
                 if let Some(review) = lookup::review(self, &review)? {
-                        thread::redact(&mut review.comments, op.id, comment)?;
+                    thread::redact(&mut review.comments, entry, comment)?;
                 }
             }
             Action::ReviewCommentEdit {
@@ -712,7 +696,7 @@ impl store::FromHistory for Patch {
                 if let Some(review) = lookup::review(self, &review)? {
                     thread::edit(
                         &mut review.comments,
-                            op.id,
+                        entry,
                         comment,
                         timestamp,
                         body,
@@ -722,12 +706,12 @@ impl store::FromHistory for Patch {
             }
             Action::ReviewCommentResolve { review, comment } => {
                 if let Some(review) = lookup::review(self, &review)? {
-                        thread::resolve(&mut review.comments, op.id, comment)?;
+                    thread::resolve(&mut review.comments, entry, comment)?;
                 }
             }
             Action::ReviewCommentUnresolve { review, comment } => {
                 if let Some(review) = lookup::review(self, &review)? {
-                        thread::unresolve(&mut review.comments, op.id, comment)?;
+                    thread::unresolve(&mut review.comments, entry, comment)?;
                 }
             }
             Action::ReviewComment {
@@ -739,8 +723,8 @@ impl store::FromHistory for Patch {
                 if let Some(review) = lookup::review(self, &review)? {
                     thread::comment(
                         &mut review.comments,
-                            op.id,
+                        entry,
-                            author.id.into(),
+                        author,
                         timestamp,
                         body,
                         reply_to,
@@ -779,12 +763,12 @@ impl store::FromHistory for Patch {
                 if lookup::revision(self, &revision)?.is_none() {
                     return Ok(());
                 };
-                    let doc = repo.identity_doc_at(op.identity)?.verified()?;
+                let doc = repo.identity_doc_at(identity)?.verified()?;
 
                 match self.target() {
                     MergeTarget::Delegates => {
-                            if !doc.is_delegate(&op.author) {
+                        if !doc.is_delegate(&author) {
-                                return Err(Error::InvalidMerge(op.id));
+                            return Err(Error::InvalidMerge(entry));
                         }
                         let proj = doc.project()?;
                         let branch = git::refs::branch(proj.default_branch());
@@ -795,16 +779,16 @@ impl store::FromHistory for Patch {
                         // of the merge author. We simply skip it, which allows archiving in
                         // case of a rebase off the master branch, or a redaction of the
                         // merge.
-                            let Ok(head) = repo.reference_oid(&op.author, &branch) else {
+                        let Ok(head) = repo.reference_oid(&author, &branch) else {
-                                continue;
+                            return Ok(());
                         };
                         if commit != head && !repo.is_ancestor_of(commit, head)? {
-                                continue;
+                            return Ok(());
                         }
                     }
                 }
                 self.merges.insert(
-                        op.author,
+                    author,
                     Merge {
                         revision,
                         commit,
@@ -851,9 +835,9 @@ impl store::FromHistory for Patch {
                 if let Some(revision) = lookup::revision(self, &revision)? {
                     thread::comment(
                         &mut revision.discussion,
-                            op.id,
+                        entry,
-                            op.author,
+                        author,
-                            op.timestamp,
+                        timestamp,
                         body,
                         reply_to,
                         None,
@@ -869,9 +853,9 @@ impl store::FromHistory for Patch {
                 if let Some(revision) = lookup::revision(self, &revision)? {
                     thread::edit(
                         &mut revision.discussion,
-                            op.id,
+                        entry,
                         comment,
-                            op.timestamp,
+                        timestamp,
                         body,
                         vec![],
                     )?;
@@ -879,7 +863,7 @@ impl store::FromHistory for Patch {
             }
             Action::RevisionCommentRedact { revision, comment } => {
                 if let Some(revision) = lookup::revision(self, &revision)? {
-                        thread::redact(&mut revision.discussion, op.id, comment)?;
+                    thread::redact(&mut revision.discussion, entry, comment)?;
                 }
             }
             Action::RevisionCommentReact {
@@ -891,8 +875,8 @@ impl store::FromHistory for Patch {
                 if let Some(revision) = lookup::revision(self, &revision)? {
                     thread::react(
                         &mut revision.discussion,
-                            op.id,
+                        entry,
-                            op.author,
+                        author,
                         comment,
                         reaction,
                         active,
@@ -900,6 +884,34 @@ impl store::FromHistory for Patch {
                 }
             }
         }
+        Ok(())
+    }
+}
+
+impl store::FromHistory for Patch {
+    type Action = Action;
+    type Error = Error;
+
+    fn type_name() -> &'static TypeName {
+        &TYPENAME
+    }
+
+    fn validate(&self) -> Result<(), Self::Error> {
+        if self.revisions.is_empty() {
+            return Err(Error::Validate("no revisions found"));
+        }
+        if self.title().is_empty() {
+            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);
+
+        for action in op.actions {
+            self.action(action, op.id, op.author, op.timestamp, op.identity, repo)?;
         }
         Ok(())
     }