From d963f2257fa5109fe1aeb7e0e5c9e968f84a6d4f Mon Sep 17 00:00:00 2001
From: Felix Ableitner <me@nutomic.com>
Date: Tue, 28 Nov 2023 13:13:36 +0100
Subject: [PATCH] make automatic conflict resolution work

---
 src/api.rs      | 45 +++++++++++++++++++++------------------------
 tests/common.rs |  2 +-
 tests/test.rs   | 41 ++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 62 insertions(+), 26 deletions(-)

diff --git a/src/api.rs b/src/api.rs
index 88e03cd..90be32f 100644
--- a/src/api.rs
+++ b/src/api.rs
@@ -108,30 +108,27 @@ impl DbConflict {
         let ancestor = generate_article_version(&original_article.edits, &self.previous_version)?;
 
         let patch = Patch::from_str(&self.diff)?;
-        if let Ok(new_text) = apply(&original_article.text, &patch) {
-            // patch applies cleanly so we are done
-            // federate the change
-            submit_article_update(data, new_text, &original_article).await?;
-            // remove conflict from db
-            let mut lock = data.conflicts.lock().unwrap();
-            lock.retain(|c| c.id != self.id);
-            Ok(None)
-        } else {
-            // there is a merge conflict, do three-way-merge
-            // apply self.diff to ancestor to get `ours`
-            let ours = apply(&ancestor, &patch)?;
-            // if it returns ok the merge was successful, which is impossible based on apply failing
-            // above. so we unconditionally read the three-way-merge string from err field
-            let merge = merge(&ancestor, &ours, &original_article.text)
-                .err()
-                .unwrap();
-
-            Ok(Some(ApiConflict {
-                id: self.id,
-                three_way_merge: merge,
-                article_id: original_article.ap_id.clone(),
-                previous_version: original_article.latest_version,
-            }))
+        // apply self.diff to ancestor to get `ours`
+        let ours = apply(&ancestor, &patch)?;
+        match merge(&ancestor, &ours, &original_article.text) {
+            Ok(new_text) => {
+                // patch applies cleanly so we are done
+                // federate the change
+                submit_article_update(data, new_text, &original_article).await?;
+                // remove conflict from db
+                let mut lock = data.conflicts.lock().unwrap();
+                lock.retain(|c| c.id != self.id);
+                Ok(None)
+            }
+            Err(three_way_merge) => {
+                // there is a merge conflict, user needs to do three-way-merge
+                Ok(Some(ApiConflict {
+                    id: self.id,
+                    three_way_merge,
+                    article_id: original_article.ap_id.clone(),
+                    previous_version: original_article.latest_version,
+                }))
+            }
         }
     }
 }
diff --git a/tests/common.rs b/tests/common.rs
index 11cac8f..1c78469 100644
--- a/tests/common.rs
+++ b/tests/common.rs
@@ -66,7 +66,7 @@ impl TestData {
     }
 }
 
-pub const TEST_ARTICLE_DEFAULT_TEXT: &str = "empty\n";
+pub const TEST_ARTICLE_DEFAULT_TEXT: &str = "some\nexample\ntext\n";
 
 pub async fn create_article(hostname: &str, title: String) -> MyResult<DbArticle> {
     let create_form = CreateArticleData {
diff --git a/tests/test.rs b/tests/test.rs
index 46edd52..384a9a2 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -246,7 +246,7 @@ async fn test_local_edit_conflict() -> MyResult<()> {
     let edit_res = edit_article_with_conflict(data.hostname_alpha, &edit_form)
         .await?
         .unwrap();
-    assert_eq!("<<<<<<< ours\nIpsum Lorem\n||||||| original\nempty\n=======\nLorem Ipsum\n>>>>>>> theirs\n", edit_res.three_way_merge);
+    assert_eq!("<<<<<<< ours\nIpsum Lorem\n||||||| original\nsome\nexample\ntext\n=======\nLorem Ipsum\n>>>>>>> theirs\n", edit_res.three_way_merge);
 
     let conflicts: Vec<ApiConflict> =
         get_query(data.hostname_alpha, "edit_conflicts", None::<()>).await?;
@@ -340,3 +340,42 @@ async fn test_federated_edit_conflict() -> MyResult<()> {
 
     data.stop()
 }
+
+#[tokio::test]
+#[serial]
+async fn test_overlapping_edits_no_conflict() -> MyResult<()> {
+    let data = TestData::start();
+
+    // create new article
+    let title = "Manu_Chao".to_string();
+    let create_res = create_article(data.hostname_alpha, title.clone()).await?;
+    assert_eq!(title, create_res.title);
+    assert!(create_res.local);
+
+    // one user edits article
+    let edit_form = EditArticleData {
+        ap_id: create_res.ap_id.clone(),
+        new_text: "my\nexample\ntext\n".to_string(),
+        previous_version: create_res.latest_version.clone(),
+        resolve_conflict_id: None,
+    };
+    let edit_res = edit_article(data.hostname_alpha, &create_res.title, &edit_form).await?;
+    assert_eq!(edit_res.text, edit_form.new_text);
+    assert_eq!(2, edit_res.edits.len());
+
+    // another user edits article, without being aware of previous edit
+    let edit_form = EditArticleData {
+        ap_id: create_res.ap_id.clone(),
+        new_text: "some\nexample\narticle\n".to_string(),
+        previous_version: create_res.latest_version,
+        resolve_conflict_id: None,
+    };
+    let edit_res = edit_article(data.hostname_alpha, &title, &edit_form).await?;
+    let conflicts: Vec<ApiConflict> =
+        get_query(data.hostname_alpha, "edit_conflicts", None::<()>).await?;
+    assert_eq!(0, conflicts.len());
+    assert_eq!(3, edit_res.edits.len());
+    assert_eq!("my\nexample\narticle\n", edit_res.text);
+
+    data.stop()
+}