Fix bug when text is deleted simultaneously in different replicas. Gi…

…ve descriptive names to previous fuzztests.
jorendorff · Apr 9, 2016 · cf48620 · cf48620
1 parent 6806355
commit cf48620
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 5 deletions.
diff --git a/lib/rga.js b/lib/rga.js
@@ -475,6 +475,11 @@ RGA.AceEditorRGA = function AceEditorRga(id, editor, history, queue) {
   function applyOpToEditor(op) {
     switch (op.type) {
     case "addRight":
+      if (self._index.has(op.w.timestamp)) {
+        // This character was already added.
+        throw new Error("bug - message delivered twice to " + self._id + ": ", JSON.stringify(op));
+      }
+
       var loc = self.getRowColumnAfter(op.t, op.w.timestamp);
       //console.log("inserting character", op.w.atom, "at", loc);
       withEditorCallbacksDisabled(function () {
@@ -484,6 +489,11 @@ RGA.AceEditorRGA = function AceEditorRga(id, editor, history, queue) {
 
     case "remove":
       //console.log("remove:", op.t, " from:", self);
+      if (self._index.get(op.t).removed) {
+        // This character has already been removed. Nothing to do.
+        break;
+      }
+
       var loc = self.getRowColumnBefore(op.t);
       var removingNewline = editorSession.getDocument().getLine(loc.row).length === loc.column;
       var whatToRemove = {
@@ -509,9 +519,10 @@ RGA.AceEditorRGA = function AceEditorRga(id, editor, history, queue) {
 
     //console.log("editorReplica.downstream: received op (from socket):", op);
 
-    // Since applyOpToEditor uses editorReplica to look up the location of the
-    // inserted/deleted character in the document, we have to call that first,
-    // before modifying editorReplica.
+    // Since applyOpToEditor uses the RGA to look up the location of the
+    // inserted/deleted character in the document, and determine whether it has in fact
+    // already been inserted/deleted, we have to call that first,
+    // before modifying the RGA.
     applyOpToEditor(op);  // first update the editor
     _base_downstream.call(this, source, op);  // then update the RGA
     lastText = self.editor.getValue();

diff --git a/test/test_editor.js b/test/test_editor.js
@@ -224,7 +224,7 @@ describe("RGA.AceEditorRGA", () => {
     assert.strictEqual(y.text(), "HOME*RUN");
   });
 
-  it("passes fuzztest #1", () => {
+  it("copes when both editors have new input at the same time", () => {
     let q = new MockEventQueue;
     let e0q = new MockEventQueue;
     let e0 = new MockAceEditor(e0q);
@@ -241,7 +241,7 @@ describe("RGA.AceEditorRGA", () => {
     assert.strictEqual(e1.getValue(), "\n\n");
   });
 
-  it("passes fuzztest #2", () => {
+  it("orders characters the same when text is inserted simultaneously in different replicas", () => {
     let q = new MockEventQueue;
     let e0 = new MockAceEditor(q);
     let a0 = new RGA.AceEditorRGA(0, e0, undefined, q);
@@ -254,4 +254,16 @@ describe("RGA.AceEditorRGA", () => {
     assert.strictEqual(e0.getValue(), "YX");
     assert.strictEqual(e1.getValue(), "YX");
   });
+
+  it("copes when the same text is deleted simultaneously in different replicas", () => {
+    let q = new MockEventQueue;
+    let a1 = new RGA.AceEditorRGA(1, new MockAceEditor(q), undefined, q);
+    let a2 = new RGA.AceEditorRGA(2, new MockAceEditor(q), undefined, q);
+    RGA.tie(a1, a2);
+    a1.editor.setValue("atta");
+    q.drain();
+    a2.editor.setValue("ta");
+    a1.editor.setValue("ta");
+    q.drain();
+  });
 });