From 369fc7637cb7e68a02f115bc6dfd69be15058a09 Mon Sep 17 00:00:00 2001
From: Alexis Sellier <self@cloudhead.io>
Date: Wed, 28 Jun 2023 14:35:21 +0200
Subject: [PATCH] cob: Use new `Immutable` lattice for merges

Instead of `Redactable`, which is not needed since we have an `LWWMap`,
we use a new `Immutable` semilattice.
---
 radicle-crdt/src/immutable.rs | 52 +++++++++++++++++++++++++++++++++++
 radicle-crdt/src/lib.rs       |  2 ++
 radicle/src/cob/patch.rs      | 28 ++++++++++++-------
 3 files changed, 72 insertions(+), 10 deletions(-)
 create mode 100644 radicle-crdt/src/immutable.rs

diff --git a/radicle-crdt/src/immutable.rs b/radicle-crdt/src/immutable.rs
new file mode 100644
index 00000000..252b2a1b
--- /dev/null
+++ b/radicle-crdt/src/immutable.rs
@@ -0,0 +1,52 @@
+use crate::Semilattice;
+
+/// A [`Semilattice`] that panics when attempting to merge inequal elements.
+/// Use this for types that will never merge.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub struct Immutable<T>(pub T);
+
+impl<T> Immutable<T> {
+    /// Create a new immutable object.
+    pub fn new(inner: T) -> Self {
+        Self(inner)
+    }
+}
+
+impl<T> std::ops::Deref for Immutable<T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<T: PartialEq> Semilattice for Immutable<T> {
+    fn merge(&mut self, other: Self) {
+        if self.0 != other.0 {
+            panic!("Immutable::merge: Cannot merge inequal objects");
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    #[should_panic]
+    fn test_merge_inequal() {
+        let mut a = Immutable::new(0);
+        let b = Immutable::new(1);
+
+        a.merge(b);
+    }
+
+    #[test]
+    fn test_merge_equal() {
+        let mut a = Immutable::new(1);
+        let b = Immutable::new(1);
+
+        a.merge(b);
+        assert_eq!(a, b);
+    }
+}
diff --git a/radicle-crdt/src/lib.rs b/radicle-crdt/src/lib.rs
index 1595dbd0..b5d40df7 100644
--- a/radicle-crdt/src/lib.rs
+++ b/radicle-crdt/src/lib.rs
@@ -5,6 +5,7 @@
 pub mod clock;
 pub mod gmap;
 pub mod gset;
+pub mod immutable;
 pub mod lwwmap;
 pub mod lwwreg;
 pub mod lwwset;
@@ -19,6 +20,7 @@ pub mod test;
 pub use clock::Lamport;
 pub use gmap::GMap;
 pub use gset::GSet;
+pub use immutable::Immutable;
 pub use lwwmap::LWWMap;
 pub use lwwreg::LWWReg;
 pub use lwwset::LWWSet;
diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs
index f98c794d..3e8fe05e 100644
--- a/radicle/src/cob/patch.rs
+++ b/radicle/src/cob/patch.rs
@@ -12,7 +12,9 @@ use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
 use radicle_crdt::clock;
-use radicle_crdt::{GMap, GSet, LWWMap, LWWReg, LWWSet, Lamport, Max, Redactable, Semilattice};
+use radicle_crdt::{
+    GMap, GSet, Immutable, LWWMap, LWWReg, LWWSet, Lamport, Max, Redactable, Semilattice,
+};
 
 use crate::cob;
 use crate::cob::common::{Author, Tag, Timestamp};
@@ -181,6 +183,7 @@ impl MergeTarget {
     }
 }
 
+/// Patch CRDT.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Patch {
     /// Title of the patch.
@@ -190,11 +193,20 @@ pub struct Patch {
     /// Target this patch is meant to be merged in.
     target: LWWReg<Max<MergeTarget>>,
     /// Associated tags.
+    /// Tags can be added and removed at will.
     tags: LWWSet<Tag>,
     /// Patch merges.
-    merges: LWWMap<ActorId, Redactable<Merge>>,
+    ///
+    /// Only one merge is allowed per user.
+    ///
+    /// Merges can be removed and replaced, but not modified. Generally, once a revision is merged,
+    /// it stays that way. Being able to remove merges may be useful in case of force updates
+    /// on the target branch.
+    merges: LWWMap<ActorId, Immutable<Merge>>,
     /// List of patch revisions. The initial changeset is part of the
     /// first revision.
+    ///
+    /// Revisions can be redacted, but are otherwise immutable.
     revisions: GMap<RevisionId, Redactable<Revision>>,
     /// Users assigned to review this patch.
     reviewers: LWWSet<ActorId>,
@@ -304,9 +316,7 @@ impl Patch {
 
     /// Get the merges.
     pub fn merges(&self) -> impl Iterator<Item = (&ActorId, &Merge)> {
-        self.merges
-            .iter()
-            .filter_map(|(a, m)| m.get().map(|m| (a, m)))
+        self.merges.iter().map(|(a, m)| (a, m.deref()))
     }
 
     /// Reference to the Git object containing the code on the latest revision.
@@ -603,7 +613,7 @@ impl store::FromHistory for Patch {
                         }
                         self.merges.insert(
                             op.author,
-                            Redactable::Present(Merge {
+                            Immutable::new(Merge {
                                 revision,
                                 commit,
                                 timestamp,
@@ -614,9 +624,7 @@ impl store::FromHistory for Patch {
                         let mut merges = self.merges.iter().fold(
                             HashMap::<(RevisionId, git::Oid), usize>::new(),
                             |mut acc, (_, merge)| {
-                                if let Some(merge) = merge.get() {
-                                    *acc.entry((merge.revision, merge.commit)).or_default() += 1;
-                                }
+                                *acc.entry((merge.revision, merge.commit)).or_default() += 1;
                                 acc
                             },
                         );
@@ -1799,7 +1807,7 @@ mod test {
 
         let (merger, merge) = merges.first().unwrap();
         assert_eq!(*merger, signer.public_key());
-        assert_eq!(merge.get().unwrap().commit, pr.base);
+        assert_eq!(merge.commit, pr.base);
     }
 
     #[test]