Send operations to project-history when accepting tracked changes (#17599)

* added getHistoryOpForAcceptedChange in RangesManager

* rename adjustHistoryUpdatesMetadata to be treated as public

* handle retain op in UpdateTranslator and updateCompressor

* send op to project-history in acceptChanges

* use promises.queueOps

* use ranges in getHistoryOpForAcceptedChange

* using rangesWithChangeRemoved

* acceptChanges acceptance test

* using change.op.hpos

* Revert "using change.op.hpos"

This reverts commit f53333b5099c840ab8fb8bb08df198ad6cfa2d84.

* use getHistoryOpForAcceptedChanges

* fix historyDocLength

* Revert "rename adjustHistoryUpdatesMetadata to be treated as public"

This reverts commit 2ba9443fd040a5c953828584285887c00dc40ea6.

* fix typescript issues

* sort changes before creating history updates

* fix tests

* sinon spy RangesManager.getHistoryUpdatesForAcceptedChanges

* added unit tests

* sort deletes before inserts

* use getDocLength function

* fix docLength calculation

* fix typo

* allow all retains

* fix lint error

* refactor RangesTests

* fix ts error

* fix history_doc_length calculation in RangesManager

* remove retain tracking check from UpdateCompressor

* use makeRanges() properly in tests

* refactor acceptance tests

GitOrigin-RevId: ab12ec53c5f52c20d44827c6037335e048f2edb0

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

@@ -237,10 +237,14 @@ const DocumentManager = {
       changeIds = []
     }
 
-    const { lines, version, ranges } = await DocumentManager.getDoc(
-      projectId,
-      docId
-    )
+    const {
+      lines,
+      version,
+      ranges,
+      pathname,
+      projectHistoryId,
+      historyRangesSupport,
+    } = await DocumentManager.getDoc(projectId, docId)
     if (lines == null || version == null) {
       throw new Errors.NotFoundError(`document not found: ${docId}`)
     }
@@ -256,6 +260,26 @@ const DocumentManager = {
       newRanges,
       {}
     )
+
+    if (historyRangesSupport) {
+      const historyUpdates = RangesManager.getHistoryUpdatesForAcceptedChanges({
+        docId,
+        acceptedChangeIds: changeIds,
+        changes: ranges.changes || [],
+        lines,
+        pathname,
+        projectHistoryId,
+      })
+
+      if (historyUpdates.length === 0) {
+        return
+      }
+
+      await ProjectHistoryRedisManager.promises.queueOps(
+        projectId,
+        ...historyUpdates.map(op => JSON.stringify(op))
+      )
+    }
   },
 
   async updateCommentState(projectId, docId, commentId, userId, resolved) {

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

@@ -5,7 +5,7 @@ const logger = require('@overleaf/logger')
 const OError = require('@overleaf/o-error')
 const Metrics = require('./Metrics')
 const _ = require('lodash')
-const { isInsert, isDelete, isComment } = require('./Utils')
+const { isInsert, isDelete, isComment, getDocLength } = require('./Utils')
 
 /**
  * @typedef {import('./types').Comment} Comment
@@ -15,6 +15,7 @@ const { isInsert, isDelete, isComment } = require('./Utils')
  * @typedef {import('./types').HistoryDeleteOp} HistoryDeleteOp
  * @typedef {import('./types').HistoryDeleteTrackedChange} HistoryDeleteTrackedChange
  * @typedef {import('./types').HistoryInsertOp} HistoryInsertOp
+ * @typedef {import('./types').HistoryRetainOp} HistoryRetainOp
  * @typedef {import('./types').HistoryOp} HistoryOp
  * @typedef {import('./types').HistoryUpdate} HistoryUpdate
  * @typedef {import('./types').InsertOp} InsertOp
@@ -138,6 +139,118 @@ const RangesManager = {
     return newRanges
   },
 
+  /**
+   *
+   * @param {object} args
+   * @param {string} args.docId
+   * @param {string[]} args.acceptedChangeIds
+   * @param {TrackedChange[]} args.changes
+   * @param {string} args.pathname
+   * @param {string} args.projectHistoryId
+   * @param {string[]} args.lines
+   */
+  getHistoryUpdatesForAcceptedChanges({
+    docId,
+    acceptedChangeIds,
+    changes,
+    pathname,
+    projectHistoryId,
+    lines,
+  }) {
+    /** @type {(change: TrackedChange) => boolean} */
+    const isAccepted = change => acceptedChangeIds.includes(change.id)
+
+    const historyOps = []
+
+    // Keep ops in order of offset, with deletes before inserts
+    const sortedChanges = changes.slice().sort(function (c1, c2) {
+      const result = c1.op.p - c2.op.p
+      if (result !== 0) {
+        return result
+      } else if (isInsert(c1.op) && isDelete(c2.op)) {
+        return 1
+      } else if (isDelete(c1.op) && isInsert(c2.op)) {
+        return -1
+      } else {
+        return 0
+      }
+    })
+
+    const docLength = getDocLength(lines)
+    let historyDocLength = docLength
+    for (const change of sortedChanges) {
+      if (isDelete(change.op)) {
+        historyDocLength += change.op.d.length
+      }
+    }
+
+    let unacceptedDeletes = 0
+    for (const change of sortedChanges) {
+      /** @type {HistoryOp | undefined} */
+      let op
+
+      if (isDelete(change.op)) {
+        if (isAccepted(change)) {
+          op = {
+            p: change.op.p,
+            d: change.op.d,
+          }
+          if (unacceptedDeletes > 0) {
+            op.hpos = op.p + unacceptedDeletes
+          }
+        } else {
+          unacceptedDeletes += change.op.d.length
+        }
+      } else if (isInsert(change.op)) {
+        if (isAccepted(change)) {
+          op = {
+            p: change.op.p,
+            r: change.op.i,
+            tracking: {
+              type: 'none',
+              userId: change.metadata.user_id,
+              ts: change.metadata.ts,
+            },
+          }
+          if (unacceptedDeletes > 0) {
+            op.hpos = op.p + unacceptedDeletes
+          }
+        }
+      }
+
+      if (!op) {
+        continue
+      }
+
+      /** @type {HistoryUpdate} */
+      const historyOp = {
+        doc: docId,
+        op: [op],
+        meta: {
+          ...change.metadata,
+          doc_length: docLength,
+          pathname,
+        },
+      }
+
+      if (projectHistoryId) {
+        historyOp.projectHistoryId = projectHistoryId
+      }
+
+      if (historyOp.meta && historyDocLength !== docLength) {
+        historyOp.meta.history_doc_length = historyDocLength
+      }
+
+      historyOps.push(historyOp)
+
+      if (isDelete(change.op) && isAccepted(change)) {
+        historyDocLength -= change.op.d.length
+      }
+    }
+
+    return historyOps
+  },
+
   _getRanges(rangesTracker) {
     // Return the minimal data structure needed, since most documents won't have any
     // changes or comments

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

@@ -7,7 +7,6 @@ const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager')
 const RealTimeRedisManager = require('./RealTimeRedisManager')
 const ShareJsUpdateManager = require('./ShareJsUpdateManager')
 const HistoryManager = require('./HistoryManager')
-const _ = require('lodash')
 const logger = require('@overleaf/logger')
 const Metrics = require('./Metrics')
 const Errors = require('./Errors')
@@ -15,7 +14,7 @@ const DocumentManager = require('./DocumentManager')
 const RangesManager = require('./RangesManager')
 const SnapshotManager = require('./SnapshotManager')
 const Profiler = require('./Profiler')
-const { isInsert, isDelete } = require('./Utils')
+const { isInsert, isDelete, getDocLength } = require('./Utils')
 
 /**
  * @typedef {import("./types").DeleteOp} DeleteOp
@@ -308,11 +307,7 @@ const UpdateManager = {
     ranges,
     historyRangesSupport
   ) {
-    let docLength = _.reduce(lines, (chars, line) => chars + line.length, 0)
-    // Add newline characters. Lines are joined by newlines, but the last line
-    // doesn't include a newline. We must make a special case for an empty list
-    // so that it doesn't report a doc length of -1.
-    docLength += Math.max(lines.length - 1, 0)
+    let docLength = getDocLength(lines)
     let historyDocLength = docLength
     for (const change of ranges.changes ?? []) {
       if ('d' in change.op) {

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

@@ -1,4 +1,5 @@
 // @ts-check
+const _ = require('lodash')
 
 /**
  * @typedef {import('./types').CommentOp} CommentOp
@@ -38,6 +39,22 @@ function isComment(op) {
   return 'c' in op && op.c != null
 }
 
+/**
+ * Get the length of a document from its lines
+ *
+ * @param {string[]} lines
+ * @returns {number}
+ */
+function getDocLength(lines) {
+  let docLength = _.reduce(lines, (chars, line) => chars + line.length, 0)
+  // Add newline characters. Lines are joined by newlines, but the last line
+  // doesn't include a newline. We must make a special case for an empty list
+  // so that it doesn't report a doc length of -1.
+  docLength += Math.max(lines.length - 1, 0)
+
+  return docLength
+}
+
 /**
  * Adds given tracked deletes to the given content.
  *
@@ -66,4 +83,10 @@ function addTrackedDeletesToContent(content, trackedChanges) {
   return result
 }
 
-module.exports = { isInsert, isDelete, isComment, addTrackedDeletesToContent }
+module.exports = {
+  isInsert,
+  isDelete,
+  isComment,
+  addTrackedDeletesToContent,
+  getDocLength,
+}

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

@@ -1,4 +1,7 @@
+import { TrackingPropsRawData } from 'overleaf-editor-core/types/lib/types'
+
 export type Update = {
+  doc: string
   op: Op[]
   v: number
   meta?: {
@@ -8,7 +11,7 @@ export type Update = {
   projectHistoryId?: string
 }
 
-export type Op = InsertOp | DeleteOp | CommentOp
+export type Op = InsertOp | DeleteOp | CommentOp | RetainOp
 
 export type InsertOp = {
   i: string
@@ -16,6 +19,11 @@ export type InsertOp = {
   u?: boolean
 }
 
+export type RetainOp = {
+  r: string
+  p: number
+}
+
 export type DeleteOp = {
   d: string
   p: number
@@ -52,7 +60,7 @@ export type TrackedChange = {
   }
 }
 
-export type HistoryOp = HistoryInsertOp | HistoryDeleteOp | HistoryCommentOp
+export type HistoryOp = HistoryInsertOp | HistoryDeleteOp | HistoryCommentOp | HistoryRetainOp
 
 export type HistoryInsertOp = InsertOp & {
   commentIds?: string[]
@@ -60,6 +68,11 @@ export type HistoryInsertOp = InsertOp & {
   trackedDeleteRejection?: boolean
 }
 
+export type HistoryRetainOp = RetainOp & {
+  hpos?: number
+  tracking?: TrackingPropsRawData
+}
+
 export type HistoryDeleteOp = DeleteOp & {
   hpos?: number
   trackedChanges?: HistoryDeleteTrackedChange[]
@@ -78,7 +91,8 @@ export type HistoryCommentOp = CommentOp & {
 
 export type HistoryUpdate = {
   op: HistoryOp[]
-  v: number
+  doc: string
+  v?: number
   meta?: {
     pathname?: string
     doc_length?: number

services/document-updater/test/acceptance/js/RangesTests.js

@@ -6,6 +6,9 @@ const { db, ObjectId } = require('../../../app/js/mongodb')
 const MockWebApi = require('./helpers/MockWebApi')
 const DocUpdaterClient = require('./helpers/DocUpdaterClient')
 const DocUpdaterApp = require('./helpers/DocUpdaterApp')
+const RangesManager = require('../../../app/js/RangesManager')
+
+const sandbox = sinon.createSandbox()
 
 describe('Ranges', function () {
   before(function (done) {
@@ -309,7 +312,7 @@ describe('Ranges', function () {
 
   describe('accepting a change', function () {
     beforeEach(function (done) {
-      sinon.spy(MockWebApi, 'setDocument')
+      sandbox.spy(MockWebApi, 'setDocument')
       this.project_id = DocUpdaterClient.randomId()
       this.user_id = DocUpdaterClient.randomId()
       this.id_seed = '587357bd35e64f6157'
@@ -361,7 +364,7 @@ describe('Ranges', function () {
       })
     })
     afterEach(function () {
-      MockWebApi.setDocument.restore()
+      sandbox.restore()
     })
 
     it('should remove the change after accepting', function (done) {
@@ -424,6 +427,219 @@ describe('Ranges', function () {
     })
   })
 
+  describe('accepting multiple changes', function () {
+    beforeEach(function (done) {
+      this.getHistoryUpdatesSpy = sandbox.spy(
+        RangesManager,
+        'getHistoryUpdatesForAcceptedChanges'
+      )
+
+      this.project_id = DocUpdaterClient.randomId()
+      this.user_id = DocUpdaterClient.randomId()
+      this.doc = {
+        id: DocUpdaterClient.randomId(),
+        lines: ['aaa', 'bbb', 'ccc', 'ddd', 'eee'],
+      }
+
+      DocUpdaterClient.enableHistoryRangesSupport(this.doc.id, (error, res) => {
+        if (error) {
+          throw error
+        }
+        MockWebApi.insertDoc(this.project_id, this.doc.id, {
+          lines: this.doc.lines,
+          version: 0,
+        })
+
+        DocUpdaterClient.preloadDoc(this.project_id, this.doc.id, error => {
+          if (error != null) {
+            throw error
+          }
+
+          this.id_seed_1 = 'tc_1'
+          this.id_seed_2 = 'tc_2'
+          this.id_seed_3 = 'tc_3'
+
+          this.updates = [
+            {
+              doc: this.doc.id,
+              op: [{ d: 'bbb', p: 4 }],
+              v: 0,
+              meta: {
+                user_id: this.user_id,
+                tc: this.id_seed_1,
+              },
+            },
+            {
+              doc: this.doc.id,
+              op: [{ d: 'ccc', p: 5 }],
+              v: 1,
+              meta: {
+                user_id: this.user_id,
+                tc: this.id_seed_2,
+              },
+            },
+            {
+              doc: this.doc.id,
+              op: [{ d: 'ddd', p: 6 }],
+              v: 2,
+              meta: {
+                user_id: this.user_id,
+                tc: this.id_seed_3,
+              },
+            },
+          ]
+
+          DocUpdaterClient.sendUpdates(
+            this.project_id,
+            this.doc.id,
+            this.updates,
+            error => {
+              if (error != null) {
+                throw error
+              }
+              setTimeout(() => {
+                DocUpdaterClient.getDoc(
+                  this.project_id,
+                  this.doc.id,
+                  (error, res, data) => {
+                    if (error != null) {
+                      throw error
+                    }
+                    const { ranges } = data
+                    const changeOps = ranges.changes
+                      .map(change => change.op)
+                      .flat()
+                    changeOps.should.deep.equal([
+                      { d: 'bbb', p: 4 },
+                      { d: 'ccc', p: 5 },
+                      { d: 'ddd', p: 6 },
+                    ])
+                    done()
+                  }
+                )
+              }, 200)
+            }
+          )
+        })
+      })
+    })
+    afterEach(function () {
+      sandbox.restore()
+    })
+
+    it('accepting changes in order', function (done) {
+      DocUpdaterClient.acceptChanges(
+        this.project_id,
+        this.doc.id,
+        [
+          this.id_seed_1 + '000001',
+          this.id_seed_2 + '000001',
+          this.id_seed_3 + '000001',
+        ],
+        error => {
+          if (error != null) {
+            throw error
+          }
+
+          const historyUpdates = this.getHistoryUpdatesSpy.returnValues[0]
+          expect(historyUpdates[0]).to.deep.equal({
+            doc: this.doc.id,
+            meta: {
+              pathname: '/a/b/c.tex',
+              doc_length: 10,
+              history_doc_length: 19,
+              ts: historyUpdates[0].meta.ts,
+              user_id: this.user_id,
+            },
+            op: [{ p: 4, d: 'bbb' }],
+          })
+
+          expect(historyUpdates[1]).to.deep.equal({
+            doc: this.doc.id,
+            meta: {
+              pathname: '/a/b/c.tex',
+              doc_length: 10,
+              history_doc_length: 16,
+              ts: historyUpdates[1].meta.ts,
+              user_id: this.user_id,
+            },
+            op: [{ p: 5, d: 'ccc' }],
+          })
+
+          expect(historyUpdates[2]).to.deep.equal({
+            doc: this.doc.id,
+            meta: {
+              pathname: '/a/b/c.tex',
+              doc_length: 10,
+              history_doc_length: 13,
+              ts: historyUpdates[2].meta.ts,
+              user_id: this.user_id,
+            },
+            op: [{ p: 6, d: 'ddd' }],
+          })
+
+          done()
+        }
+      )
+    })
+
+    it('accepting changes in reverse order', function (done) {
+      DocUpdaterClient.acceptChanges(
+        this.project_id,
+        this.doc.id,
+        [
+          this.id_seed_3 + '000001',
+          this.id_seed_2 + '000001',
+          this.id_seed_1 + '000001',
+        ],
+        error => {
+          if (error != null) {
+            throw error
+          }
+
+          const historyUpdates = this.getHistoryUpdatesSpy.returnValues[0]
+          expect(historyUpdates[0]).to.deep.equal({
+            doc: this.doc.id,
+            meta: {
+              pathname: '/a/b/c.tex',
+              doc_length: 10,
+              history_doc_length: 19,
+              ts: historyUpdates[0].meta.ts,
+              user_id: this.user_id,
+            },
+            op: [{ p: 4, d: 'bbb' }],
+          })
+
+          expect(historyUpdates[1]).to.deep.equal({
+            doc: this.doc.id,
+            meta: {
+              pathname: '/a/b/c.tex',
+              doc_length: 10,
+              history_doc_length: 16,
+              ts: historyUpdates[1].meta.ts,
+              user_id: this.user_id,
+            },
+            op: [{ p: 5, d: 'ccc' }],
+          })
+
+          expect(historyUpdates[2]).to.deep.equal({
+            doc: this.doc.id,
+            meta: {
+              pathname: '/a/b/c.tex',
+              doc_length: 10,
+              history_doc_length: 13,
+              ts: historyUpdates[2].meta.ts,
+              user_id: this.user_id,
+            },
+            op: [{ p: 6, d: 'ddd' }],
+          })
+
+          done()
+        }
+      )
+    })
+  })
+
   describe('deleting a comment range', function () {
     before(function (done) {
       this.project_id = DocUpdaterClient.randomId()

services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js

@@ -119,6 +119,10 @@ module.exports = DocUpdaterClient = {
     )
   },
 
+  enableHistoryRangesSupport(docId, cb) {
+    rclient.sadd(keys.historyRangesSupport(), docId, cb)
+  },
+
   preloadDoc(projectId, docId, callback) {
     DocUpdaterClient.getDoc(projectId, docId, callback)
   },
@@ -193,6 +197,16 @@ module.exports = DocUpdaterClient = {
     )
   },
 
+  acceptChanges(projectId, docId, changeIds, callback) {
+    request.post(
+      {
+        url: `http://localhost:3003/project/${projectId}/doc/${docId}/change/accept`,
+        json: { change_ids: changeIds },
+      },
+      callback
+    )
+  },
+
   removeComment(projectId, docId, comment, callback) {
     request.del(
       `http://localhost:3003/project/${projectId}/doc/${docId}/comment/${comment}`,

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

@@ -637,6 +637,265 @@ describe('RangesManager', function () {
       })
     })
   })
+
+  describe('getHistoryUpdatesForAcceptedChanges', function () {
+    beforeEach(function () {
+      this.RangesManager = SandboxedModule.require(MODULE_PATH, {
+        requires: {
+          '@overleaf/ranges-tracker': (this.RangesTracker =
+            SandboxedModule.require('@overleaf/ranges-tracker')),
+          '@overleaf/metrics': {},
+        },
+      })
+    })
+
+    it('should create history updates for accepted track inserts', function () {
+      // 'one two three four five' <-- text before changes
+      const ranges = {
+        comments: [],
+        changes: makeRanges([
+          { i: 'lorem', p: 0 },
+          { i: 'ipsum', p: 15 },
+        ]),
+      }
+      const lines = ['loremone two thipsumree four five']
+
+      const result = this.RangesManager.getHistoryUpdatesForAcceptedChanges({
+        docId: this.doc_id,
+        acceptedChangeIds: ranges.changes.map(change => change.id),
+        changes: ranges.changes,
+        pathname: '',
+        projectHistoryId: '',
+        lines,
+      })
+
+      expect(result).to.deep.equal([
+        {
+          doc: this.doc_id,
+          meta: {
+            user_id: TEST_USER_ID,
+            doc_length: 33,
+            pathname: '',
+            ts: ranges.changes[0].metadata.ts,
+          },
+          op: [
+            {
+              r: 'lorem',
+              p: 0,
+              tracking: {
+                type: 'none',
+                userId: this.user_id,
+                ts: ranges.changes[0].metadata.ts,
+              },
+            },
+          ],
+        },
+        {
+          doc: this.doc_id,
+          meta: {
+            user_id: TEST_USER_ID,
+            doc_length: 33,
+            pathname: '',
+            ts: ranges.changes[1].metadata.ts,
+          },
+          op: [
+            {
+              r: 'ipsum',
+              p: 15,
+              tracking: {
+                type: 'none',
+                userId: this.user_id,
+                ts: ranges.changes[1].metadata.ts,
+              },
+            },
+          ],
+        },
+      ])
+    })
+
+    it('should create history updates for accepted track deletes', function () {
+      // 'one two three four five' <-- text before changes
+      const ranges = {
+        comments: [],
+        changes: makeRanges([
+          { d: 'two', p: 4 },
+          { d: 'three', p: 5 },
+        ]),
+      }
+      const lines = ['one   four five']
+
+      const result = this.RangesManager.getHistoryUpdatesForAcceptedChanges({
+        docId: this.doc_id,
+        acceptedChangeIds: ranges.changes.map(change => change.id),
+        changes: ranges.changes,
+        pathname: '',
+        projectHistoryId: '',
+        lines,
+      })
+
+      expect(result).to.deep.equal([
+        {
+          doc: this.doc_id,
+          meta: {
+            user_id: TEST_USER_ID,
+            doc_length: 15,
+            history_doc_length: 23,
+            pathname: '',
+            ts: ranges.changes[0].metadata.ts,
+          },
+          op: [
+            {
+              d: 'two',
+              p: 4,
+            },
+          ],
+        },
+        {
+          doc: this.doc_id,
+          meta: {
+            user_id: TEST_USER_ID,
+            doc_length: 15,
+            history_doc_length: 20,
+            pathname: '',
+            ts: ranges.changes[1].metadata.ts,
+          },
+          op: [
+            {
+              d: 'three',
+              p: 5,
+            },
+          ],
+        },
+      ])
+    })
+
+    it('should create history updates with unaccepted deletes', function () {
+      // 'one two three four five' <-- text before changes
+      const ranges = {
+        comments: [],
+        changes: makeRanges([
+          { d: 'two', p: 4 },
+          { d: 'three', p: 5 },
+        ]),
+      }
+      const lines = ['one   four five']
+
+      const result = this.RangesManager.getHistoryUpdatesForAcceptedChanges({
+        docId: this.doc_id,
+        acceptedChangeIds: [ranges.changes[1].id],
+        changes: ranges.changes,
+        pathname: '',
+        projectHistoryId: '',
+        lines,
+      })
+
+      expect(result).to.deep.equal([
+        {
+          doc: this.doc_id,
+          meta: {
+            user_id: TEST_USER_ID,
+            doc_length: 15,
+            history_doc_length: 23,
+            pathname: '',
+            ts: ranges.changes[1].metadata.ts,
+          },
+          op: [
+            {
+              d: 'three',
+              p: 5,
+              hpos: 8,
+            },
+          ],
+        },
+      ])
+    })
+
+    it('should create history updates with mixed track changes', function () {
+      // 'one two three four five' <-- text before changes
+      const ranges = {
+        comments: [],
+        changes: makeRanges([
+          { d: 'two', p: 4 },
+          { d: 'three', p: 5 },
+          { i: 'xxx ', p: 6 },
+          { d: 'five', p: 15 },
+        ]),
+      }
+      const lines = ['one   xxx four ']
+
+      const result = this.RangesManager.getHistoryUpdatesForAcceptedChanges({
+        docId: this.doc_id,
+        acceptedChangeIds: [
+          ranges.changes[0].id,
+          // ranges.changes[1].id - second delete is not accepted
+          ranges.changes[2].id,
+          ranges.changes[3].id,
+        ],
+        changes: ranges.changes,
+        pathname: '',
+        projectHistoryId: '',
+        lines,
+      })
+
+      expect(result).to.deep.equal([
+        {
+          doc: this.doc_id,
+          meta: {
+            user_id: TEST_USER_ID,
+            doc_length: 15,
+            history_doc_length: 27,
+            pathname: '',
+            ts: ranges.changes[0].metadata.ts,
+          },
+          op: [
+            {
+              d: 'two',
+              p: 4,
+            },
+          ],
+        },
+        {
+          doc: this.doc_id,
+          meta: {
+            user_id: TEST_USER_ID,
+            doc_length: 15,
+            history_doc_length: 24,
+            pathname: '',
+            ts: ranges.changes[1].metadata.ts,
+          },
+          op: [
+            {
+              r: 'xxx ',
+              p: 6,
+              hpos: 11,
+              tracking: {
+                type: 'none',
+                userId: this.user_id,
+                ts: ranges.changes[1].metadata.ts,
+              },
+            },
+          ],
+        },
+        {
+          doc: this.doc_id,
+          meta: {
+            user_id: TEST_USER_ID,
+            doc_length: 15,
+            history_doc_length: 24,
+            pathname: '',
+            ts: ranges.changes[2].metadata.ts,
+          },
+          op: [
+            {
+              d: 'five',
+              p: 15,
+              hpos: 20,
+            },
+          ],
+        },
+      ])
+    })
+  })
 })
 
 function makeRanges(ops) {
@@ -646,7 +905,7 @@ function makeRanges(ops) {
     changes.push({
       id: id.toString(),
       op,
-      metadata: { user_id: TEST_USER_ID },
+      metadata: { user_id: TEST_USER_ID, ts: new Date() },
     })
     id += 1
   }

services/project-history/app/js/UpdateCompressor.js

@@ -71,6 +71,8 @@ function adjustLengthByOp(length, op, opts = {}) {
     } else {
       return length - op.d.length
     }
+  } else if ('r' in op && op.r != null) {
+    return length
   } else if ('c' in op && op.c != null) {
     return length
   } else {

services/project-history/app/js/UpdateTranslator.js

@@ -12,6 +12,7 @@ import * as OperationsCompressor from './OperationsCompressor.js'
  * @typedef {import('./types').DeleteOp} DeleteCommentUpdate
  * @typedef {import('./types').DeleteOp} DeleteOp
  * @typedef {import('./types').InsertOp} InsertOp
+ * @typedef {import('./types').RetainOp} RetainOp
  * @typedef {import('./types').Op} Op
  * @typedef {import('./types').RawScanOp} RawScanOp
  * @typedef {import('./types').RenameUpdate} RenameUpdate
@@ -292,7 +293,7 @@ class OperationsBuilder {
       return
     }
 
-    if (!isInsert(op) && !isDelete(op)) {
+    if (!isInsert(op) && !isDelete(op) && !isRetain(op)) {
       throw new Errors.UnexpectedOpTypeError('unexpected op type', { op })
     }
 
@@ -330,6 +331,14 @@ class OperationsBuilder {
       }
     }
 
+    if (isRetain(op)) {
+      if (op.tracking) {
+        this.retain(op.r.length, { tracking: op.tracking })
+      } else {
+        this.retain(op.r.length)
+      }
+    }
+
     if (isDelete(op)) {
       const changes = op.trackedChanges ?? []
 
@@ -465,6 +474,14 @@ function isInsert(op) {
   return 'i' in op && op.i != null
 }
 
+/**
+ * @param {Op} op
+ * @returns {op is RetainOp}
+ */
+function isRetain(op) {
+  return 'r' in op && op.r != null
+}
+
 /**
  * @param {Op} op
  * @returns {op is DeleteOp}

services/project-history/app/js/types.ts

@@ -62,7 +62,14 @@ export type RenameUpdate = ProjectUpdateBase & {
   new_pathname: string
 }
 
-export type Op = InsertOp | DeleteOp | CommentOp
+export type Op = RetainOp | InsertOp | DeleteOp | CommentOp
+
+export type RetainOp = {
+  r: string
+  p: number
+  hpos?: number
+  tracking?: TrackingProps
+}
 
 export type InsertOp = {
   i: string

services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js

@@ -97,6 +97,42 @@ describe('UpdateCompressor', function () {
       ])
     })
 
+    it('should not ignore retain ops with tracking data', function () {
+      expect(
+        this.UpdateCompressor.convertToSingleOpUpdates([
+          {
+            op: [
+              (this.op1 = { p: 0, i: 'Foo' }),
+              (this.op2 = {
+                p: 9,
+                r: 'baz',
+                tracking: { type: 'none', ts: this.ts1, userId: this.user_id },
+              }),
+              (this.op3 = { p: 6, i: 'bar' }),
+            ],
+            meta: { ts: this.ts1, user_id: this.user_id, doc_length: 10 },
+            v: 42,
+          },
+        ])
+      ).to.deep.equal([
+        {
+          op: this.op1,
+          meta: { ts: this.ts1, user_id: this.user_id, doc_length: 10 },
+          v: 42,
+        },
+        {
+          op: this.op2,
+          meta: { ts: this.ts1, user_id: this.user_id, doc_length: 13 },
+          v: 42,
+        },
+        {
+          op: this.op3,
+          meta: { ts: this.ts1, user_id: this.user_id, doc_length: 13 },
+          v: 42,
+        },
+      ])
+    })
+
     it('should update doc_length when splitting after an insert', function () {
       expect(
         this.UpdateCompressor.convertToSingleOpUpdates([

services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js

@@ -568,6 +568,118 @@ describe('UpdateTranslator', function () {
         ])
       })
 
+      it('should translate retains without tracking data', function () {
+        const updates = [
+          {
+            update: {
+              doc: this.doc_id,
+              op: [
+                {
+                  p: 3,
+                  r: 'lo',
+                },
+              ],
+              v: this.version,
+              meta: {
+                user_id: this.user_id,
+                ts: new Date(this.timestamp).getTime(),
+                pathname: '/main.tex',
+                doc_length: 20,
+              },
+            },
+          },
+        ]
+
+        const changes = this.UpdateTranslator.convertToChanges(
+          this.project_id,
+          updates
+        ).map(change => change.toRaw())
+
+        expect(changes).to.deep.equal([
+          {
+            authors: [],
+            operations: [
+              {
+                pathname: 'main.tex',
+                textOperation: [20],
+              },
+            ],
+            v2Authors: [this.user_id],
+            timestamp: this.timestamp,
+            v2DocVersions: {
+              [this.doc_id]: {
+                pathname: 'main.tex',
+                v: 0,
+              },
+            },
+          },
+        ])
+      })
+
+      it('can translate retains with tracking data', function () {
+        const updates = [
+          {
+            update: {
+              doc: this.doc_id,
+              op: [
+                {
+                  p: 3,
+                  r: 'lo',
+                  tracking: {
+                    type: 'none',
+                    ts: this.timestamp,
+                    userId: this.user_id,
+                  },
+                },
+              ],
+              v: this.version,
+              meta: {
+                user_id: this.user_id,
+                ts: new Date(this.timestamp).getTime(),
+                pathname: '/main.tex',
+                doc_length: 20,
+              },
+            },
+          },
+        ]
+
+        const changes = this.UpdateTranslator.convertToChanges(
+          this.project_id,
+          updates
+        ).map(change => change.toRaw())
+
+        expect(changes).to.deep.equal([
+          {
+            authors: [],
+            operations: [
+              {
+                pathname: 'main.tex',
+                textOperation: [
+                  3,
+                  {
+                    r: 2,
+                    tracking: {
+                      type: 'none',
+                      ts: this.timestamp,
+                      userId: this.user_id,
+                    },
+                  },
+                  15,
+                ],
+              },
+            ],
+            v2Authors: [this.user_id],
+            timestamp: this.timestamp,
+            v2DocVersions: {
+              '59bfd450e3028c4d40a1e9ab': {
+                pathname: 'main.tex',
+                v: 0,
+              },
+            },
+          },
+        ])
+      })
+
       it('can translate insertions at the start and end (with zero retained)', function () {
         const updates = [
           {