radicle: force new or initial construction of LWWReg

Using LWWReg::from will always set the clock value to the default,
which can end up being used wrong.

Instead, introduce a constructor method `initial` to indicate that it
should be used on initial construction, while `new` should be used for
newer values of the register.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett

radicle-crdt/src/lwwreg.rs

@@ -14,6 +14,16 @@ pub struct LWWReg<T, C = clock::Lamport> {
 }
 
 impl<T: Semilattice, C: PartialOrd> LWWReg<T, C> {
+    pub fn initial(value: T) -> Self
+    where
+        C: Default,
+    {
+        Self {
+            clock: Max::from(C::default()),
+            value,
+        }
+    }
+
     pub fn new(value: T, clock: C) -> Self {
         Self {
             clock: Max::from(clock),
@@ -46,15 +56,6 @@ impl<T: Semilattice, C: PartialOrd> LWWReg<T, C> {
     }
 }
 
-impl<T, C: Default> From<T> for LWWReg<T, C> {
-    fn from(value: T) -> Self {
-        Self {
-            clock: Max::from(C::default()),
-            value,
-        }
-    }
-}
-
 impl<T: Default, C: Default + Bounded> Default for LWWReg<T, C> {
     fn default() -> Self {
         Self {

radicle/src/cob/identity.rs

@@ -167,9 +167,9 @@ impl Semilattice for Proposal {
 impl Default for Proposal {
     fn default() -> Self {
         Self {
-            title: Max::from(String::default()).into(),
+            title: LWWReg::initial(Max::from(String::default())),
-            description: Max::from(String::default()).into(),
+            description: LWWReg::initial(Max::from(String::default())),
-            state: Max::from(State::default()).into(),
+            state: LWWReg::initial(Max::from(State::default())),
             revisions: GMap::default(),
             timeline: GSet::default(),
         }

radicle/src/cob/issue.rs

@@ -83,9 +83,9 @@ pub struct Issue {
     /// Actors assigned to this issue.
     assignees: LWWSet<ActorId>,
     /// Title of the issue.
-    title: LWWReg<Max<String>, clock::Lamport>,
+    title: LWWReg<Max<String>>,
     /// Current state of the issue.
-    state: LWWReg<Max<State>, clock::Lamport>,
+    state: LWWReg<Max<State>>,
     /// Associated tags.
     tags: LWWSet<Tag>,
     /// Discussion around this issue.
@@ -106,8 +106,8 @@ impl Default for Issue {
     fn default() -> Self {
         Self {
             assignees: LWWSet::default(),
-            title: Max::from(String::default()).into(),
+            title: LWWReg::initial(Max::from(String::default())),
-            state: Max::from(State::default()).into(),
+            state: LWWReg::initial(Max::from(State::default())),
             tags: LWWSet::default(),
             thread: Thread::default(),
         }

radicle/src/cob/patch.rs

@@ -160,10 +160,10 @@ impl Semilattice for Patch {
 impl Default for Patch {
     fn default() -> Self {
         Self {
-            title: Max::from(String::default()).into(),
+            title: LWWReg::initial(Max::from(String::default())),
-            description: Max::from(String::default()).into(),
+            description: LWWReg::initial(Max::from(String::default())),
-            state: Max::from(State::default()).into(),
+            state: LWWReg::initial(Max::from(State::default())),
-            target: Max::from(MergeTarget::default()).into(),
+            target: LWWReg::initial(Max::from(MergeTarget::default())),
             tags: LWWSet::default(),
             revisions: GMap::default(),
             timeline: GSet::default(),
@@ -326,6 +326,7 @@ impl store::FromHistory for Patch {
                             base,
                             oid,
                             timestamp,
+                            op.clock,
                         )),
                     );
                 }
@@ -417,10 +418,11 @@ impl Revision {
         base: git::Oid,
         oid: git::Oid,
         timestamp: Timestamp,
+        clock: Clock,
     ) -> Self {
         Self {
             author,
-            description: LWWReg::from(Max::from(description)),
+            description: LWWReg::new(Max::from(description), clock),
             base,
             oid,
             discussion: Thread::default(),
@@ -1053,6 +1055,7 @@ impl<'a> Patches<'a> {
 
 #[cfg(test)]
 mod test {
+    use std::path::Path;
     use std::str::FromStr;
     use std::{array, iter};
 
@@ -1460,6 +1463,7 @@ mod test {
         let (_, signer, project) = test::setup::context(&tmp);
         let base = git::Oid::from_str("cb18e95ada2bb38aadd8e6cef0963ce37a87add3").unwrap();
         let oid = git::Oid::from_str("518d5069f94c03427f694bb494ac1cd7d1339380").unwrap();
+        let blob = git::Oid::from_str("518d5069f94c03427f694bb494ac1cd7d133999").unwrap();
         let mut patches = Patches::open(&project).unwrap();
         let mut patch = patches
             .create(
@@ -1476,17 +1480,33 @@ mod test {
         let (rid, _) = patch.latest().unwrap();
         let rid = *rid;
 
+        let inline = vec![CodeComment {
+            location: CodeLocation {
+                blob,
+                path: Path::new("file.rs").to_path_buf(),
+                commit: oid,
+                lines: 1..3,
+            },
+            comment: "Nice!".to_owned(),
+            timestamp: Timestamp::new(0),
+        }];
         patch
             .review(
                 rid,
                 Some(Verdict::Accept),
                 Some("LGTM".to_owned()),
-                vec![],
+                inline.clone(),
                 &signer,
             )
             .unwrap();
         patch
-            .review(rid, Some(Verdict::Reject), None, vec![], &signer)
+            .review(
+                rid,
+                Some(Verdict::Reject),
+                Some("LGTM".to_owned()),
+                vec![],
+                &signer,
+            )
             .unwrap(); // Overwrite the verdict.
 
         let id = patch.id;
@@ -1496,15 +1516,23 @@ mod test {
 
         let review = revision.reviews.get(signer.public_key()).unwrap();
         assert_eq!(review.verdict(), Some(Verdict::Reject));
-        assert_eq!(review.comment(), None);
+        assert_eq!(review.comment(), Some("LGTM"));
+        assert_eq!(review.inline().cloned().collect::<Vec<_>>(), inline);
 
         patch
-            .review(rid, None, Some("Whoops!".to_owned()), vec![], &signer)
+            .review(
+                rid,
+                Some(Verdict::Reject),
+                Some("Whoops!".to_owned()),
+                vec![],
+                &signer,
+            )
             .unwrap(); // Overwrite the comment.
         let (_, revision) = patch.latest().unwrap();
         let review = revision.reviews.get(signer.public_key()).unwrap();
-        assert_eq!(review.verdict(), None);
+        assert_eq!(review.verdict(), Some(Verdict::Reject));
         assert_eq!(review.comment(), Some("Whoops!"));
+        assert_eq!(review.inline().cloned().collect::<Vec<_>>(), inline);
     }
 
     #[test]