From a370cfa5c69eea0c70090398b1a2f101d9bc2afa Mon Sep 17 00:00:00 2001
From: Richard Hansen <rhansen@rhansen.org>
Date: Sat, 11 Dec 2021 20:03:35 -0500
Subject: [PATCH] Pad: Don't create no-op revisions

---
 src/node/db/Pad.js                    | 6 ++++--
 src/node/handler/PadMessageHandler.js | 6 ++++--
 src/node/utils/ImportHtml.js          | 2 +-
 src/static/js/collab_client.js        | 6 ++++--
 src/tests/backend/specs/api/pad.js    | 2 +-
 src/tests/backend/specs/messages.js   | 8 ++++----
 6 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/node/db/Pad.js b/src/node/db/Pad.js
index 664ad55ff..abca75102 100644
--- a/src/node/db/Pad.js
+++ b/src/node/db/Pad.js
@@ -82,6 +82,9 @@ Pad.prototype.appendRevision = async function (aChangeset, author) {
   }
 
   const newAText = Changeset.applyToAText(aChangeset, this.atext, this.pool);
+  if (newAText.text === this.atext.text && newAText.attribs === this.atext.attribs) {
+    return this.head;
+  }
   Changeset.copyAText(newAText, this.atext);
 
   const newRev = ++this.head;
@@ -268,8 +271,7 @@ Pad.prototype.setText = async function (newText) {
     changeset = Changeset.makeSplice(oldText, 0, oldText.length - 1, newText);
   }
 
-  // append the changeset
-  if (newText !== oldText) await this.appendRevision(changeset);
+  await this.appendRevision(changeset);
 };
 
 Pad.prototype.appendText = async function (newText) {
diff --git a/src/node/handler/PadMessageHandler.js b/src/node/handler/PadMessageHandler.js
index cdc795a3d..f50f7c331 100644
--- a/src/node/handler/PadMessageHandler.js
+++ b/src/node/handler/PadMessageHandler.js
@@ -640,7 +640,9 @@ const handleUserChanges = async (socket, message) => {
     }
 
     const newRev = await pad.appendRevision(rebasedChangeset, thisSession.author);
-    assert.equal(newRev, r + 1);
+    // The head revision will either stay the same or increase by 1 depending on whether the
+    // changeset has a net effect.
+    assert([r, r + 1].includes(newRev));
 
     const correctionChangeset = _correctMarkersInPad(pad.atext, pad.pool);
     if (correctionChangeset) {
@@ -658,7 +660,7 @@ const handleUserChanges = async (socket, message) => {
     assert.equal(thisSession.rev, r);
     socket.json.send({type: 'COLLABROOM', data: {type: 'ACCEPT_COMMIT', newRev}});
     thisSession.rev = newRev;
-    thisSession.time = await pad.getRevisionDate(newRev);
+    if (newRev !== r) thisSession.time = await pad.getRevisionDate(newRev);
     await exports.updatePadClients(pad);
   } catch (err) {
     socket.json.send({disconnect: 'badChangeset'});
diff --git a/src/node/utils/ImportHtml.js b/src/node/utils/ImportHtml.js
index 059d57af6..12a99ef79 100644
--- a/src/node/utils/ImportHtml.js
+++ b/src/node/utils/ImportHtml.js
@@ -85,5 +85,5 @@ exports.setPadHTML = async (pad, html) => {
 
   apiLogger.debug(`The changeset: ${theChangeset}`);
   await pad.setText('\n');
-  if (!Changeset.isIdentity(theChangeset)) await pad.appendRevision(theChangeset);
+  await pad.appendRevision(theChangeset);
 };
diff --git a/src/static/js/collab_client.js b/src/static/js/collab_client.js
index 849fff5fc..38dc22af4 100644
--- a/src/static/js/collab_client.js
+++ b/src/static/js/collab_client.js
@@ -207,8 +207,10 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)
       });
     } else if (msg.type === 'ACCEPT_COMMIT') {
       serverMessageTaskQueue.enqueue(() => {
-        const newRev = msg.newRev;
-        if (newRev !== (rev + 1)) {
+        const {newRev} = msg;
+        // newRev will equal rev if the changeset has no net effect (identity changeset, or removing
+        // and re-adding the same characters with the same attributes).
+        if (![rev, rev + 1].includes(newRev)) {
           window.console.warn(`bad message revision on ACCEPT_COMMIT: ${newRev} not ${rev + 1}`);
           // setChannelState("DISCONNECTED", "badmessage_acceptcommit");
           return;
diff --git a/src/tests/backend/specs/api/pad.js b/src/tests/backend/specs/api/pad.js
index 41c30b8a0..64f10f012 100644
--- a/src/tests/backend/specs/api/pad.js
+++ b/src/tests/backend/specs/api/pad.js
@@ -278,7 +278,7 @@ describe(__filename, function () {
       const res = await agent.post(endPoint('setText'))
           .send({
             padID: testPadId,
-            text: 'testTextTwo',
+            text: 'testTextThree',
           })
           .expect(200)
           .expect('Content-Type', /json/);
diff --git a/src/tests/backend/specs/messages.js b/src/tests/backend/specs/messages.js
index 7e58ca5e7..55d3c7a19 100644
--- a/src/tests/backend/specs/messages.js
+++ b/src/tests/backend/specs/messages.js
@@ -84,19 +84,19 @@ describe(__filename, function () {
       assert.equal(pad.text(), 'hello\n');
     });
 
-    it('identity changeset is accepted', async function () {
+    it('identity changeset is accepted, has no effect', async function () {
       sendUserChanges('Z:1>5+5$hello');
       await assertAccepted(rev + 1);
       sendUserChanges('Z:6>0$');
-      await assertAccepted(rev + 1);
+      await assertAccepted(rev);
       assert.equal(pad.text(), 'hello\n');
     });
 
-    it('non-identity changeset with no net change is accepted', async function () {
+    it('non-identity changeset with no net change is accepted, has no effect', async function () {
       sendUserChanges('Z:1>5+5$hello');
       await assertAccepted(rev + 1);
       sendUserChanges('Z:6>0-5+5$hello');
-      await assertAccepted(rev + 1);
+      await assertAccepted(rev);
       assert.equal(pad.text(), 'hello\n');
     });
   });