Merge pull request #17010 from overleaf/em-doc-length

Add history_doc_length property to history updates

GitOrigin-RevId: ccad09f23ae9c038480fb7228a987d8fc6fb6274

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

@@ -19,6 +19,7 @@ const { isInsert, isDelete } = require('./Utils')
 
 /**
  * @typedef {import("./types").DeleteOp} DeleteOp
+ * @typedef {import("./types").HistoryUpdate } HistoryUpdate
  * @typedef {import("./types").InsertOp} InsertOp
  * @typedef {import("./types").Op} Op
  * @typedef {import("./types").Ranges} Ranges
@@ -156,12 +157,6 @@ const UpdateManager = {
       }
       profile.log('RangesManager.applyUpdate', { sync: true })
 
-      UpdateManager._addProjectHistoryMetadataToOps(
-        appliedOps,
-        pathname,
-        projectHistoryId,
-        lines
-      )
       await RedisManager.promises.updateDocument(
         projectId,
         docId,
@@ -173,6 +168,15 @@ const UpdateManager = {
       )
       profile.log('RedisManager.updateDocument')
 
+      UpdateManager._addMetadataToHistoryUpdates(
+        historyUpdates,
+        pathname,
+        projectHistoryId,
+        lines,
+        ranges,
+        historyRangesSupport
+      )
+
       if (historyUpdates.length > 0) {
         Metrics.inc('history-queue', 1, { status: 'project-history' })
         try {
@@ -304,16 +308,31 @@ const UpdateManager = {
   },
 
   /**
-   * Add metadata to ops that will be useful to project history
+   * Add metadata that will be useful to project history
    *
-   * @param {Update[]} updates
+   * @param {HistoryUpdate[]} updates
    * @param {string} pathname
    * @param {string} projectHistoryId
    * @param {string[]} lines
+   * @param {Ranges} ranges
+   * @param {boolean} historyRangesSupport
    */
-  _addProjectHistoryMetadataToOps(updates, pathname, projectHistoryId, lines) {
+  _addMetadataToHistoryUpdates(
+    updates,
+    pathname,
+    projectHistoryId,
+    lines,
+    ranges,
+    historyRangesSupport
+  ) {
     let docLength = _.reduce(lines, (chars, line) => chars + line.length, 0)
     docLength += lines.length - 1 // count newline characters
+    let historyDocLength = docLength
+    for (const change of ranges.changes ?? []) {
+      if ('d' in change.op) {
+        historyDocLength += change.op.d.length
+      }
+    }
 
     for (const update of updates) {
       update.projectHistoryId = projectHistoryId
@@ -322,6 +341,10 @@ const UpdateManager = {
       }
       update.meta.pathname = pathname
       update.meta.doc_length = docLength
+      if (historyRangesSupport && historyDocLength !== docLength) {
+        update.meta.history_doc_length = historyDocLength
+      }
+
       // Each update may contain multiple ops, i.e.
       // [{
       // 	ops: [{i: "foo", p: 4}, {d: "bar", p:8}]
@@ -334,9 +357,21 @@ const UpdateManager = {
       for (const op of update.op) {
         if (isInsert(op)) {
           docLength += op.i.length
+          if (!op.trackedDeleteRejection) {
+            // Tracked delete rejections end up retaining characters rather
+            // than inserting
+            historyDocLength += op.i.length
+          }
         }
         if (isDelete(op)) {
           docLength -= op.d.length
+          if (!update.meta.tc || op.u) {
+            // This is either a regular delete or a tracked insert rejection.
+            // It will be translated to a delete in history.  Tracked deletes
+            // are translated into retains and don't change the history doc
+            // length.
+            historyDocLength -= op.d.length
+          }
         }
       }
     }

services/document-updater/app/js/types.ts

@@ -2,8 +2,6 @@ export type Update = {
   op: Op[]
   v: number
   meta?: {
-    pathname?: string
-    doc_length?: number
     tc?: boolean
     user_id?: string
   }
@@ -82,6 +80,7 @@ export type HistoryUpdate = {
   meta?: {
     pathname?: string
     doc_length?: number
+    history_doc_length?: number
     tc?: boolean
     user_id?: string
   }

services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js

@@ -12,6 +12,7 @@ describe('UpdateManager', function () {
     this.projectHistoryId = 'history-id-123'
     this.doc_id = 'document-id-123'
     this.lockValue = 'mock-lock-value'
+    this.pathname = '/a/b/c.tex'
 
     this.Metrics = {
       inc: sinon.stub(),
@@ -324,7 +325,6 @@ describe('UpdateManager', function () {
         'history-update-3',
       ]
       this.project_ops_length = 123
-      this.pathname = '/a/b/c.tex'
       this.DocumentManager.promises.getDoc.resolves({
         lines: this.lines,
         version: this.version,
@@ -344,7 +344,7 @@ describe('UpdateManager', function () {
         appliedOps: this.appliedOps,
       })
       this.RedisManager.promises.updateDocument.resolves()
-      this.UpdateManager.promises._addProjectHistoryMetadataToOps = sinon.stub()
+      this.UpdateManager.promises._addMetadataToHistoryUpdates = sinon.stub()
     })
 
     describe('normally', function () {
@@ -395,7 +395,7 @@ describe('UpdateManager', function () {
       })
 
       it('should add metadata to the ops', function () {
-        this.UpdateManager.promises._addProjectHistoryMetadataToOps.should.have.been.calledWith(
+        this.UpdateManager.promises._addMetadataToHistoryUpdates.should.have.been.calledWith(
           this.appliedOps,
           this.pathname,
           this.projectHistoryId,
@@ -523,13 +523,14 @@ describe('UpdateManager', function () {
     })
   })
 
-  describe('_addProjectHistoryMetadataToOps', function () {
+  describe('_addMetadataToHistoryUpdates', function () {
     it('should add projectHistoryId, pathname and doc_length metadata to the ops', function () {
       const lines = ['some', 'test', 'data']
-      const appliedOps = [
+      const historyUpdates = [
         {
           v: 42,
           op: [
+            { i: 'bing', p: 12, trackedDeleteRejection: true },
             { i: 'foo', p: 4 },
             { i: 'bar', p: 6 },
           ],
@@ -539,27 +540,45 @@ describe('UpdateManager', function () {
           op: [
             { d: 'qux', p: 4 },
             { i: 'bazbaz', p: 14 },
+            { d: 'bong', p: 28, u: true },
           ],
+          meta: {
+            tc: 'tracking-info',
+          },
+        },
+        {
+          v: 47,
+          op: [{ d: 'so', p: 0 }],
         },
         { v: 49, op: [{ i: 'penguin', p: 18 }] },
       ]
-      this.UpdateManager._addProjectHistoryMetadataToOps(
-        appliedOps,
+      const ranges = {
+        changes: [
+          { op: { d: 'bingbong', p: 12 } },
+          { op: { i: 'test', p: 5 } },
+        ],
+      }
+      this.UpdateManager._addMetadataToHistoryUpdates(
+        historyUpdates,
         this.pathname,
         this.projectHistoryId,
-        lines
+        lines,
+        ranges,
+        true
       )
-      appliedOps.should.deep.equal([
+      historyUpdates.should.deep.equal([
         {
           projectHistoryId: this.projectHistoryId,
           v: 42,
           op: [
+            { i: 'bing', p: 12, trackedDeleteRejection: true },
             { i: 'foo', p: 4 },
             { i: 'bar', p: 6 },
           ],
           meta: {
             pathname: this.pathname,
             doc_length: 14,
+            history_doc_length: 22,
           },
         },
         {
@@ -568,11 +587,24 @@ describe('UpdateManager', function () {
           op: [
             { d: 'qux', p: 4 },
             { i: 'bazbaz', p: 14 },
+            { d: 'bong', p: 28, u: true },
           ],
           meta: {
             pathname: this.pathname,
-            doc_length: 20,
-          }, // 14 + 'foo' + 'bar'
+            doc_length: 24, // 14 + 'bing' + 'foo' + 'bar'
+            history_doc_length: 28, // 22 + 'foo' + 'bar'
+            tc: 'tracking-info',
+          },
+        },
+        {
+          projectHistoryId: this.projectHistoryId,
+          v: 47,
+          op: [{ d: 'so', p: 0 }],
+          meta: {
+            pathname: this.pathname,
+            doc_length: 23, // 24 - 'qux' + 'bazbaz' - 'bong'
+            history_doc_length: 30, // 28 - 'bong' + 'bazbaz'
+          },
         },
         {
           projectHistoryId: this.projectHistoryId,
@@ -580,8 +612,9 @@ describe('UpdateManager', function () {
           op: [{ i: 'penguin', p: 18 }],
           meta: {
             pathname: this.pathname,
-            doc_length: 23,
-          }, // 14 - 'qux' + 'bazbaz'
+            doc_length: 21, // 23 - 'so'
+            history_doc_length: 28, // 30 - 'so'
+          },
         },
       ])
     })