Merge pull request #20826 from overleaf/revert-20799-em-ranges-tracker-sanity-checks

Revert "Sanity check for tracked changes in document-updater"

GitOrigin-RevId: 7876d57298d0f5dbd54929fdf69bce2976f16a9f

services/document-updater/app/js/DocumentManager.js

@@ -264,12 +264,7 @@ const DocumentManager = {
       throw new Errors.NotFoundError(`document not found: ${docId}`)
     }
 
-    const newRanges = RangesManager.acceptChanges(
+    const newRanges = RangesManager.acceptChanges(changeIds, ranges)
-      projectId,
-      docId,
-      changeIds,
-      ranges
-    )
 
     await RedisManager.promises.updateDocument(
       projectId,

services/document-updater/app/js/RangesManager.js

@@ -80,8 +80,6 @@ const RangesManager = {
       }
     }
 
-    sanityCheckTrackedChanges(projectId, docId, rangesTracker.changes)
-
     if (
       rangesTracker.changes?.length > RangesManager.MAX_CHANGES ||
       rangesTracker.comments?.length > RangesManager.MAX_COMMENTS
@@ -129,12 +127,11 @@ const RangesManager = {
     return { newRanges, rangesWereCollapsed, historyUpdates }
   },
 
-  acceptChanges(projectId, docId, changeIds, ranges) {
+  acceptChanges(changeIds, ranges) {
     const { changes, comments } = ranges
     logger.debug(`accepting ${changeIds.length} changes in ranges`)
     const rangesTracker = new RangesTracker(changes, comments)
     rangesTracker.removeChangeIds(changeIds)
-    sanityCheckTrackedChanges(projectId, docId, rangesTracker.changes)
     const newRanges = RangesManager._getRanges(rangesTracker)
     return newRanges
   },
@@ -560,66 +557,4 @@ function getCroppedCommentOps(op, comments) {
   return historyCommentOps
 }
 
-/**
- * Check some tracked changes assumptions:
- *
- * - Tracked changes can't be empty
- * - Tracked inserts can't overlap with another tracked change
- * - There can't be two tracked deletes at the same position
- * - Ranges should be ordered by position, deletes before inserts
- * - Every tracked change id should be unique
- *
- * If any assumption isn't upheld, log a warning.
- *
- * @param {string} projectId
- * @param {string} docId
- * @param {TrackedChange[]} changes
- */
-function sanityCheckTrackedChanges(projectId, docId, changes) {
-  const idsSeen = new Set()
-  let lastDeletePos = -1 // allow a tracked delete at position 0
-  let lastInsertEnd = 0
-  let ok = true
-  for (const change of changes) {
-    if (idsSeen.has(change.id)) {
-      ok = false
-      break
-    }
-    idsSeen.add(change.id)
-
-    const op = change.op
-    if ('i' in op) {
-      if (op.i.length === 0 || op.p < lastDeletePos || op.p < lastInsertEnd) {
-        ok = false
-        break
-      }
-      lastInsertEnd = op.p + op.i.length
-    } else if ('d' in op) {
-      if (op.d.length === 0 || op.p <= lastDeletePos || op.p < lastInsertEnd) {
-        ok = false
-        break
-      }
-      lastDeletePos = op.p
-    }
-  }
-
-  if (ok) {
-    return
-  }
-
-  const changeRanges = []
-  for (const change of changes) {
-    if ('i' in change.op) {
-      changeRanges.push({ p: change.op.p, i: change.op.i.length })
-    } else if ('d' in change.op) {
-      changeRanges.push({ p: change.op.p, d: change.op.d.length })
-    }
-  }
-
-  logger.warn(
-    { projectId, docId, changes: changeRanges },
-    'Malformed tracked changes detected'
-  )
-}
-
 module.exports = RangesManager

services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js

@@ -691,8 +691,6 @@ describe('DocumentManager', function () {
 
       it('should apply the accept change to the ranges', function () {
         this.RangesManager.acceptChanges.should.have.been.calledWith(
-          this.project_id,
-          this.doc_id,
           [this.change_id],
           this.ranges
         )
@@ -724,12 +722,7 @@ describe('DocumentManager', function () {
 
       it('should apply the accept change to the ranges', function () {
         this.RangesManager.acceptChanges
-          .calledWith(
+          .calledWith(this.change_ids, this.ranges)
-            this.project_id,
-            this.doc_id,
-            this.change_ids,
-            this.ranges
-          )
           .should.equal(true)
       })
     })

services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js

@@ -668,8 +668,6 @@ describe('RangesManager', function () {
       beforeEach(function () {
         this.change_ids = [this.ranges.changes[1].id]
         this.result = this.RangesManager.acceptChanges(
-          this.project_id,
-          this.doc_id,
           this.change_ids,
           this.ranges
         )
@@ -716,8 +714,6 @@ describe('RangesManager', function () {
           this.ranges.changes[4].id,
         ]
         this.result = this.RangesManager.acceptChanges(
-          this.project_id,
-          this.doc_id,
           this.change_ids,
           this.ranges
         )