Merge pull request #17711 from overleaf/em-resync-tracked-deletes

Include tracked deletes in resync doc update

GitOrigin-RevId: 410f9435f92ff44184f60a2695be27d7b819a1e2

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

@@ -320,7 +320,7 @@ const DocumentManager = {
 
   async resyncDocContents(projectId, docId, path) {
     logger.debug({ projectId, docId, path }, 'start resyncing doc contents')
-    let { lines, version, projectHistoryId } =
+    let { lines, ranges, version, projectHistoryId, historyRangesSupport } =
       await RedisManager.promises.getDoc(projectId, docId)
 
     // To avoid issues where the same docId appears with different paths,
@@ -333,7 +333,7 @@ const DocumentManager = {
         { projectId, docId },
         'resyncing doc contents - not found in redis - retrieving from web'
       )
-      ;({ lines, version, projectHistoryId } =
+      ;({ lines, ranges, version, projectHistoryId, historyRangesSupport } =
         await PersistenceManager.promises.getDoc(projectId, docId, {
           peek: true,
         }))
@@ -349,9 +349,11 @@ const DocumentManager = {
       projectHistoryId,
       docId,
       lines,
+      ranges,
       version,
       // use the path from the resyncProjectStructure update
-      path
+      path,
+      historyRangesSupport
     )
   },
 

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

@@ -1,3 +1,5 @@
+// @ts-check
+
 const Settings = require('@overleaf/settings')
 const { callbackifyAll } = require('@overleaf/promise-utils')
 const projectHistoryKeys = Settings.redis?.project_history?.key_schema
@@ -7,8 +9,13 @@ const rclient = require('@overleaf/redis-wrapper').createClient(
 const logger = require('@overleaf/logger')
 const metrics = require('./Metrics')
 const { docIsTooLarge } = require('./Limits')
+const { addTrackedDeletesToContent } = require('./Utils')
 const OError = require('@overleaf/o-error')
 
+/**
+ * @typedef {import('./types').Ranges} Ranges
+ */
+
 const ProjectHistoryRedisManager = {
   async queueOps(projectId, ...ops) {
     // Record metric for ops pushed onto queue
@@ -119,23 +126,41 @@ const ProjectHistoryRedisManager = {
     return await ProjectHistoryRedisManager.queueOps(projectId, jsonUpdate)
   },
 
+  /**
+   * Add a resync doc update to the project-history queue
+   *
+   * @param {string} projectId
+   * @param {string} projectHistoryId
+   * @param {string} docId
+   * @param {string[]} lines
+   * @param {Ranges} ranges
+   * @param {number} version
+   * @param {string} pathname
+   * @param {boolean} historyRangesSupport
+   * @return {Promise<number>} the number of ops added
+   */
   async queueResyncDocContent(
     projectId,
     projectHistoryId,
     docId,
     lines,
+    ranges,
     version,
-    pathname
+    pathname,
+    historyRangesSupport
   ) {
     logger.debug(
       { projectId, docId, lines, version, pathname },
       'queue doc content resync'
     )
+
+    let content = lines.join('\n')
+    if (historyRangesSupport) {
+      content = addTrackedDeletesToContent(content, ranges.changes ?? [])
+    }
+
     const projectUpdate = {
-      resyncDocContent: {
-        content: lines.join('\n'),
-        version,
-      },
+      resyncDocContent: { content, version },
       projectHistoryId,
       path: pathname,
       doc: docId,

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

@@ -5,6 +5,7 @@
  * @typedef {import('./types').DeleteOp} DeleteOp
  * @typedef {import('./types').InsertOp} InsertOp
  * @typedef {import('./types').Op} Op
+ * @typedef {import('./types').TrackedChange} TrackedChange
  */
 
 /**
@@ -37,4 +38,32 @@ function isComment(op) {
   return 'c' in op && op.c != null
 }
 
-module.exports = { isInsert, isDelete, isComment }
+/**
+ * Adds given tracked deletes to the given content.
+ *
+ * The history system includes tracked deletes in the document content.
+ *
+ * @param {string} content
+ * @param {TrackedChange[]} trackedChanges
+ * @return {string} content for the history service
+ */
+function addTrackedDeletesToContent(content, trackedChanges) {
+  let cursor = 0
+  let result = ''
+  for (const change of trackedChanges) {
+    if (isDelete(change.op)) {
+      // Add the content before the tracked delete
+      result += content.slice(cursor, change.op.p)
+      cursor = change.op.p
+      // Add the content of the tracked delete
+      result += change.op.d
+    }
+  }
+
+  // Add the content after all tracked deletes
+  result += content.slice(cursor)
+
+  return result
+}
+
+module.exports = { isInsert, isDelete, isComment, addTrackedDeletesToContent }

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

@@ -987,6 +987,7 @@ describe('DocumentManager', function () {
           ranges: this.ranges,
           pathname: this.pathname,
           projectHistoryId: this.projectHistoryId,
+          historyRangesSupport: this.historyRangesSupport,
         })
         await this.DocumentManager.promises.resyncDocContents(
           this.project_id,
@@ -1008,8 +1009,10 @@ describe('DocumentManager', function () {
             this.projectHistoryId,
             this.doc_id,
             this.lines,
+            this.ranges,
             this.version,
-            this.pathnameFromProjectStructureUpdate
+            this.pathnameFromProjectStructureUpdate,
+            this.historyRangesSupport
           )
           .should.equal(true)
       })
@@ -1025,6 +1028,7 @@ describe('DocumentManager', function () {
           ranges: this.ranges,
           pathname: this.pathname,
           projectHistoryId: this.projectHistoryId,
+          historyRangesSupport: this.historyRangesSupport,
         })
         await this.DocumentManager.promises.resyncDocContents(
           this.project_id,
@@ -1052,8 +1056,10 @@ describe('DocumentManager', function () {
             this.projectHistoryId,
             this.doc_id,
             this.lines,
+            this.ranges,
             this.version,
-            this.pathnameFromProjectStructureUpdate
+            this.pathnameFromProjectStructureUpdate,
+            this.historyRangesSupport
           )
           .should.equal(true)
       })

services/document-updater/test/unit/js/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.js

@@ -177,22 +177,12 @@ describe('ProjectHistoryRedisManager', function () {
       beforeEach(function () {
         this.doc_id = 1234
         this.lines = ['one', 'two']
+        this.ranges = {
+          changes: [{ op: { i: 'ne', p: 1 } }, { op: { d: 'deleted', p: 3 } }],
+        }
         this.version = 2
         this.pathname = '/path'
 
-        this.update = {
-          resyncDocContent: {
-            content: this.lines.join('\n'),
-            version: this.version,
-          },
-          projectHistoryId: this.projectHistoryId,
-          path: this.pathname,
-          doc: this.doc_id,
-          meta: {
-            ts: new Date(),
-          },
-        }
-
         this.ProjectHistoryRedisManager.promises.queueOps = sinon
           .stub()
           .resolves()
@@ -200,15 +190,29 @@ describe('ProjectHistoryRedisManager', function () {
 
       describe('with a good doc', function () {
         beforeEach(async function () {
+          this.update = {
+            resyncDocContent: {
+              content: 'one\ntwo',
+              version: this.version,
+            },
+            projectHistoryId: this.projectHistoryId,
+            path: this.pathname,
+            doc: this.doc_id,
+            meta: { ts: new Date() },
+          }
+
           await this.ProjectHistoryRedisManager.promises.queueResyncDocContent(
             this.project_id,
             this.projectHistoryId,
             this.doc_id,
             this.lines,
+            this.ranges,
             this.version,
-            this.pathname
+            this.pathname,
+            false
           )
         })
+
         it('should check if the doc is too large', function () {
           this.Limits.docIsTooLarge
             .calledWith(
@@ -235,8 +239,10 @@ describe('ProjectHistoryRedisManager', function () {
               this.projectHistoryId,
               this.doc_id,
               this.lines,
+              this.ranges,
               this.version,
-              this.pathname
+              this.pathname,
+              false
             )
           ).to.be.rejected
         })
@@ -247,6 +253,47 @@ describe('ProjectHistoryRedisManager', function () {
           )
         })
       })
+
+      describe('when history ranges support is enabled', function () {
+        beforeEach(async function () {
+          this.update = {
+            resyncDocContent: {
+              content: 'onedeleted\ntwo',
+              version: this.version,
+            },
+            projectHistoryId: this.projectHistoryId,
+            path: this.pathname,
+            doc: this.doc_id,
+            meta: { ts: new Date() },
+          }
+
+          await this.ProjectHistoryRedisManager.promises.queueResyncDocContent(
+            this.project_id,
+            this.projectHistoryId,
+            this.doc_id,
+            this.lines,
+            this.ranges,
+            this.version,
+            this.pathname,
+            true
+          )
+        })
+
+        it('should include tracked deletes in the update', function () {
+          this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
+            this.project_id,
+            JSON.stringify(this.update)
+          )
+        })
+
+        it('should check the doc length without tracked deletes', function () {
+          this.Limits.docIsTooLarge.should.have.been.calledWith(
+            JSON.stringify(this.update).length,
+            this.lines,
+            this.settings.max_doc_length
+          )
+        })
+      })
     })
   })
 })

services/document-updater/test/unit/js/UtilsTests.js

@@ -0,0 +1,27 @@
+// @ts-check
+
+const { expect } = require('chai')
+const Utils = require('../../../app/js/Utils')
+
+describe('Utils', function () {
+  describe('addTrackedDeletesToContent', function () {
+    it("doesn't modify text without tracked deletes", function () {
+      const content = 'the quick brown fox'
+      const trackedChanges = []
+      const result = Utils.addTrackedDeletesToContent(content, trackedChanges)
+      expect(result).to.equal(content)
+    })
+
+    it('adds tracked deletes to text but skips tracked inserts', function () {
+      const content = 'the brown fox jumps over the dog'
+      const metadata = { user_id: 'user1', ts: new Date().toString() }
+      const trackedChanges = [
+        { id: 'tc1', op: { d: 'quick ', p: 4 }, metadata },
+        { id: 'tc2', op: { i: 'brown ', p: 5 }, metadata },
+        { id: 'tc3', op: { d: 'lazy ', p: 29 }, metadata },
+      ]
+      const result = Utils.addTrackedDeletesToContent(content, trackedChanges)
+      expect(result).to.equal('the quick brown fox jumps over the lazy dog')
+    })
+  })
+})