Enable autocomplete of reference keys feature

Prevent browser translation of icons in Pug pages

GitOrigin-RevId: 97e4d3ba70a4c95bed2c9f52e66038911625613d

.github/ISSUE_TEMPLATE/bug_report.md

@@ -1,10 +1,19 @@
+---
+name: Bug report
+about: Report a bug
+title: ''
+labels: type:bug
+assignees: ''
+
+---
+
 <!--
 
-Note: If you are using www.overleaf.com and have a problem, 
+Note: If you are using www.overleaf.com and have a problem,
       or if you would like to request a new feature please contact
       the support team at support@overleaf.com
-      
+
-      This form should only be used to report bugs in the 
+      This form should only be used to report bugs in the
       Community Edition release of Overleaf.
 
 -->

develop/docker-compose.yml

@@ -25,10 +25,10 @@ services:
     env_file:
       - dev.env
     environment:
-      - DOCKER_RUNNER=true
       - TEXLIVE_IMAGE=texlive-full # docker build texlive -t texlive-full
-      - COMPILES_HOST_DIR=${PWD}/compiles
+      - SANDBOXED_COMPILES=true
-      - OUTPUT_HOST_DIR=${PWD}/output
+      - SANDBOXED_COMPILES_HOST_DIR_COMPILES=${PWD}/compiles
+      - SANDBOXED_COMPILES_HOST_DIR_OUTPUT=${PWD}/output
     user: root
     volumes:
       - ${PWD}/compiles:/overleaf/services/clsi/compiles

libraries/access-token-encryptor/buildscript.txt

@@ -1,6 +1,6 @@
 access-token-encryptor
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/fetch-utils/buildscript.txt

@@ -1,6 +1,6 @@
 fetch-utils
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/logger/buildscript.txt

@@ -1,6 +1,6 @@
 logger
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/metrics/buildscript.txt

@@ -1,6 +1,6 @@
 metrics
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/mongo-utils/buildscript.txt

@@ -1,6 +1,6 @@
 mongo-utils
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/o-error/buildscript.txt

@@ -1,6 +1,6 @@
 o-error
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/object-persistor/buildscript.txt

@@ -1,6 +1,6 @@
 object-persistor
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/overleaf-editor-core/buildscript.txt

@@ -1,6 +1,6 @@
 overleaf-editor-core
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/overleaf-editor-core/lib/file_data/clear_tracking_props.js

@@ -1,7 +1,7 @@
 // @ts-check
 
 /**
- * @import { ClearTrackingPropsRawData } from '../types'
+ * @import { ClearTrackingPropsRawData, TrackingDirective } from '../types'
  */
 
 class ClearTrackingProps {
@@ -11,12 +11,27 @@ class ClearTrackingProps {
 
   /**
    * @param {any} other
-   * @returns {boolean}
+   * @returns {other is ClearTrackingProps}
    */
   equals(other) {
     return other instanceof ClearTrackingProps
   }
 
+  /**
+   * @param {TrackingDirective} other
+   * @returns {other is ClearTrackingProps}
+   */
+  canMergeWith(other) {
+    return other instanceof ClearTrackingProps
+  }
+
+  /**
+   * @param {TrackingDirective} other
+   */
+  mergeWith(other) {
+    return this
+  }
+
   /**
    * @returns {ClearTrackingPropsRawData}
    */

libraries/overleaf-editor-core/lib/file_data/lazy_string_file_data.js

@@ -11,7 +11,7 @@ const EditOperation = require('../operation/edit_operation')
 const EditOperationBuilder = require('../operation/edit_operation_builder')
 
 /**
- *  @import { BlobStore, ReadonlyBlobStore, RangesBlob, RawFileData, RawLazyStringFileData } from '../types'
+ *  @import { BlobStore, ReadonlyBlobStore, RangesBlob, RawHashFileData, RawLazyStringFileData } from '../types'
  */
 
 class LazyStringFileData extends FileData {
@@ -159,11 +159,11 @@ class LazyStringFileData extends FileData {
 
   /** @inheritdoc
    * @param {BlobStore} blobStore
-   * @return {Promise<RawFileData>}
+   * @return {Promise<RawHashFileData>}
    */
   async store(blobStore) {
     if (this.operations.length === 0) {
-      /** @type RawFileData */
+      /** @type RawHashFileData */
       const raw = { hash: this.hash }
       if (this.rangesHash) {
         raw.rangesHash = this.rangesHash
@@ -171,9 +171,11 @@ class LazyStringFileData extends FileData {
       return raw
     }
     const eager = await this.toEager(blobStore)
+    const raw = await eager.store(blobStore)
+    this.hash = raw.hash
+    this.rangesHash = raw.rangesHash
     this.operations.length = 0
-    /** @type RawFileData */
+    return raw
-    return await eager.store(blobStore)
   }
 }
 

libraries/overleaf-editor-core/lib/file_data/string_file_data.js

@@ -8,7 +8,7 @@ const CommentList = require('./comment_list')
 const TrackedChangeList = require('./tracked_change_list')
 
 /**
- * @import { StringFileRawData, RawFileData, BlobStore, CommentRawData } from "../types"
+ * @import { StringFileRawData, RawHashFileData, BlobStore, CommentRawData } from "../types"
  * @import { TrackedChangeRawData, RangesBlob } from "../types"
  * @import EditOperation from "../operation/edit_operation"
  */
@@ -139,7 +139,7 @@ class StringFileData extends FileData {
   /**
    * @inheritdoc
    * @param {BlobStore} blobStore
-   * @return {Promise<RawFileData>}
+   * @return {Promise<RawHashFileData>}
    */
   async store(blobStore) {
     const blob = await blobStore.putString(this.content)

libraries/overleaf-editor-core/lib/file_data/tracked_change.js

@@ -84,6 +84,21 @@ class TrackedChange {
       )
     )
   }
+
+  /**
+   * Return an equivalent tracked change whose extent is limited to the given
+   * range
+   *
+   * @param {Range} range
+   * @returns {TrackedChange | null} - the result or null if the intersection is empty
+   */
+  intersectRange(range) {
+    const intersection = this.range.intersect(range)
+    if (intersection == null) {
+      return null
+    }
+    return new TrackedChange(intersection, this.tracking)
+  }
 }
 
 module.exports = TrackedChange

libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js

@@ -2,9 +2,11 @@
 const Range = require('../range')
 const TrackedChange = require('./tracked_change')
 const TrackingProps = require('../file_data/tracking_props')
+const { InsertOp, RemoveOp, RetainOp } = require('../operation/scan_op')
 
 /**
  * @import { TrackingDirective, TrackedChangeRawData } from "../types"
+ * @import TextOperation from "../operation/text_operation"
  */
 
 class TrackedChangeList {
@@ -58,6 +60,22 @@ class TrackedChangeList {
     return this._trackedChanges.filter(change => range.contains(change.range))
   }
 
+  /**
+   * Returns tracked changes that overlap with the given range
+   * @param {Range} range
+   * @returns {TrackedChange[]}
+   */
+  intersectRange(range) {
+    const changes = []
+    for (const change of this._trackedChanges) {
+      const intersection = change.intersectRange(range)
+      if (intersection != null) {
+        changes.push(intersection)
+      }
+    }
+    return changes
+  }
+
   /**
    * Returns the tracking props for a given range.
    * @param {Range} range
@@ -89,6 +107,8 @@ class TrackedChangeList {
 
   /**
    * Collapses consecutive (and compatible) ranges
+   *
+   * @private
    * @returns {void}
    */
   _mergeRanges() {
@@ -117,12 +137,28 @@ class TrackedChangeList {
   }
 
   /**
+   * Apply an insert operation
    *
    * @param {number} cursor
    * @param {string} insertedText
    * @param {{tracking?: TrackingProps}} opts
    */
   applyInsert(cursor, insertedText, opts = {}) {
+    this._applyInsert(cursor, insertedText, opts)
+    this._mergeRanges()
+  }
+
+  /**
+   * Apply an insert operation
+   *
+   * This method will not merge ranges at the end
+   *
+   * @private
+   * @param {number} cursor
+   * @param {string} insertedText
+   * @param {{tracking?: TrackingProps}} [opts]
+   */
+  _applyInsert(cursor, insertedText, opts = {}) {
     const newTrackedChanges = []
     for (const trackedChange of this._trackedChanges) {
       if (
@@ -171,15 +207,29 @@ class TrackedChangeList {
       newTrackedChanges.push(newTrackedChange)
     }
     this._trackedChanges = newTrackedChanges
-    this._mergeRanges()
   }
 
   /**
+   * Apply a delete operation to the list of tracked changes
    *
    * @param {number} cursor
    * @param {number} length
    */
   applyDelete(cursor, length) {
+    this._applyDelete(cursor, length)
+    this._mergeRanges()
+  }
+
+  /**
+   * Apply a delete operation to the list of tracked changes
+   *
+   * This method will not merge ranges at the end
+   *
+   * @private
+   * @param {number} cursor
+   * @param {number} length
+   */
+  _applyDelete(cursor, length) {
     const newTrackedChanges = []
     for (const trackedChange of this._trackedChanges) {
       const deletedRange = new Range(cursor, length)
@@ -205,15 +255,31 @@ class TrackedChangeList {
       }
     }
     this._trackedChanges = newTrackedChanges
+  }
+
+  /**
+   * Apply a retain operation to the list of tracked changes
+   *
+   * @param {number} cursor
+   * @param {number} length
+   * @param {{tracking?: TrackingDirective}} [opts]
+   */
+  applyRetain(cursor, length, opts = {}) {
+    this._applyRetain(cursor, length, opts)
     this._mergeRanges()
   }
 
   /**
+   * Apply a retain operation to the list of tracked changes
+   *
+   * This method will not merge ranges at the end
+   *
+   * @private
    * @param {number} cursor
    * @param {number} length
    * @param {{tracking?: TrackingDirective}} opts
    */
-  applyRetain(cursor, length, opts = {}) {
+  _applyRetain(cursor, length, opts = {}) {
     // If there's no tracking info, leave everything as-is
     if (!opts.tracking) {
       return
@@ -269,6 +335,31 @@ class TrackedChangeList {
       newTrackedChanges.push(newTrackedChange)
     }
     this._trackedChanges = newTrackedChanges
+  }
+
+  /**
+   * Apply a text operation to the list of tracked changes
+   *
+   * Ranges are merged only once at the end, for performance and to avoid
+   * problematic edge cases where intermediate ranges get incorrectly merged.
+   *
+   * @param {TextOperation} operation
+   */
+  applyTextOperation(operation) {
+    // this cursor tracks the destination document that gets modified as
+    // operations are applied to it.
+    let cursor = 0
+    for (const op of operation.ops) {
+      if (op instanceof InsertOp) {
+        this._applyInsert(cursor, op.insertion, { tracking: op.tracking })
+        cursor += op.insertion.length
+      } else if (op instanceof RemoveOp) {
+        this._applyDelete(cursor, op.length)
+      } else if (op instanceof RetainOp) {
+        this._applyRetain(cursor, op.length, { tracking: op.tracking })
+        cursor += op.length
+      }
+    }
     this._mergeRanges()
   }
 }

libraries/overleaf-editor-core/lib/file_data/tracking_props.js

@@ -62,6 +62,35 @@ class TrackingProps {
       this.ts.getTime() === other.ts.getTime()
     )
   }
+
+  /**
+   * Are these tracking props compatible with the other tracking props for merging
+   * ranges?
+   *
+   * @param {TrackingDirective} other
+   * @returns {other is TrackingProps}
+   */
+  canMergeWith(other) {
+    if (!(other instanceof TrackingProps)) {
+      return false
+    }
+    return this.type === other.type && this.userId === other.userId
+  }
+
+  /**
+   * Merge two tracking props
+   *
+   * Assumes that `canMerge(other)` returns true
+   *
+   * @param {TrackingDirective} other
+   */
+  mergeWith(other) {
+    if (!this.canMergeWith(other)) {
+      throw new Error('Cannot merge with incompatible tracking props')
+    }
+    const ts = this.ts <= other.ts ? this.ts : other.ts
+    return new TrackingProps(this.type, this.userId, ts)
+  }
 }
 
 module.exports = TrackingProps

libraries/overleaf-editor-core/lib/operation/scan_op.js

@@ -175,7 +175,7 @@ class InsertOp extends ScanOp {
       return false
     }
     if (this.tracking) {
-      if (!this.tracking.equals(other.tracking)) {
+      if (!other.tracking || !this.tracking.canMergeWith(other.tracking)) {
         return false
       }
     } else if (other.tracking) {
@@ -198,7 +198,10 @@ class InsertOp extends ScanOp {
       throw new Error('Cannot merge with incompatible operation')
     }
     this.insertion += other.insertion
-    // We already have the same tracking info and commentIds
+    if (this.tracking != null && other.tracking != null) {
+      this.tracking = this.tracking.mergeWith(other.tracking)
+    }
+    // We already have the same commentIds
   }
 
   /**
@@ -306,9 +309,13 @@ class RetainOp extends ScanOp {
       return false
     }
     if (this.tracking) {
-      return this.tracking.equals(other.tracking)
+      if (!other.tracking || !this.tracking.canMergeWith(other.tracking)) {
+        return false
+      }
+    } else if (other.tracking) {
+      return false
     }
-    return !other.tracking
+    return true
   }
 
   /**
@@ -319,6 +326,9 @@ class RetainOp extends ScanOp {
       throw new Error('Cannot merge with incompatible operation')
     }
     this.length += other.length
+    if (this.tracking != null && other.tracking != null) {
+      this.tracking = this.tracking.mergeWith(other.tracking)
+    }
   }
 
   /**

libraries/overleaf-editor-core/lib/operation/text_operation.js

@@ -314,25 +314,18 @@ class TextOperation extends EditOperation {
             str
           )
         }
-        file.trackedChanges.applyRetain(result.length, op.length, {
-          tracking: op.tracking,
-        })
         result += str.slice(inputCursor, inputCursor + op.length)
         inputCursor += op.length
       } else if (op instanceof InsertOp) {
         if (containsNonBmpChars(op.insertion)) {
           throw new InvalidInsertionError(str, op.toJSON())
         }
-        file.trackedChanges.applyInsert(result.length, op.insertion, {
-          tracking: op.tracking,
-        })
         file.comments.applyInsert(
           new Range(result.length, op.insertion.length),
           { commentIds: op.commentIds }
         )
         result += op.insertion
       } else if (op instanceof RemoveOp) {
-        file.trackedChanges.applyDelete(result.length, op.length)
         file.comments.applyDelete(new Range(result.length, op.length))
         inputCursor += op.length
       } else {
@@ -352,6 +345,8 @@ class TextOperation extends EditOperation {
       throw new TextOperation.TooLongError(operation, result.length)
     }
 
+    file.trackedChanges.applyTextOperation(this)
+
     file.content = result
   }
 
@@ -400,44 +395,36 @@ class TextOperation extends EditOperation {
     for (let i = 0, l = ops.length; i < l; i++) {
       const op = ops[i]
       if (op instanceof RetainOp) {
-        // Where we need to end up after the retains
-        const target = strIndex + op.length
-        // A previous retain could have overriden some tracking info. Now we
-        // need to restore it.
-        const previousRanges = previousState.trackedChanges.inRange(
-          new Range(strIndex, op.length)
-        )
-
-        let removeTrackingInfoIfNeeded
         if (op.tracking) {
-          removeTrackingInfoIfNeeded = new ClearTrackingProps()
+          // Where we need to end up after the retains
-        }
+          const target = strIndex + op.length
+          // A previous retain could have overriden some tracking info. Now we
+          // need to restore it.
+          const previousChanges = previousState.trackedChanges.intersectRange(
+            new Range(strIndex, op.length)
+          )
 
-        for (const trackedChange of previousRanges) {
+          for (const change of previousChanges) {
-          if (strIndex < trackedChange.range.start) {
+            if (strIndex < change.range.start) {
-            inverse.retain(trackedChange.range.start - strIndex, {
+              inverse.retain(change.range.start - strIndex, {
-              tracking: removeTrackingInfoIfNeeded,
+                tracking: new ClearTrackingProps(),
+              })
+              strIndex = change.range.start
+            }
+            inverse.retain(change.range.length, {
+              tracking: change.tracking,
             })
-            strIndex = trackedChange.range.start
+            strIndex += change.range.length
           }
-          if (trackedChange.range.end < strIndex + op.length) {
+          if (strIndex < target) {
-            inverse.retain(trackedChange.range.length, {
+            inverse.retain(target - strIndex, {
-              tracking: trackedChange.tracking,
+              tracking: new ClearTrackingProps(),
             })
-            strIndex = trackedChange.range.end
+            strIndex = target
           }
-          if (trackedChange.range.end !== strIndex) {
+        } else {
-            // No need to split the range at the end
+          inverse.retain(op.length)
-            const [left] = trackedChange.range.splitAt(strIndex)
+          strIndex += op.length
-            inverse.retain(left.length, { tracking: trackedChange.tracking })
-            strIndex = left.end
-          }
-        }
-        if (strIndex < target) {
-          inverse.retain(target - strIndex, {
-            tracking: removeTrackingInfoIfNeeded,
-          })
-          strIndex = target
         }
       } else if (op instanceof InsertOp) {
         inverse.remove(op.insertion.length)

libraries/overleaf-editor-core/lib/range.js

@@ -86,10 +86,32 @@ class Range {
   }
 
   /**
-   * @param {Range} range
+   * Does this range overlap another range?
+   *
+   * Overlapping means that the two ranges have at least one character in common
+   *
+   * @param {Range} other - the other range
    */
-  overlaps(range) {
+  overlaps(other) {
-    return this.start < range.end && this.end > range.start
+    return this.start < other.end && this.end > other.start
+  }
+
+  /**
+   * Does this range overlap the start of another range?
+   *
+   * @param {Range} other - the other range
+   */
+  overlapsStart(other) {
+    return this.start <= other.start && this.end > other.start
+  }
+
+  /**
+   * Does this range overlap the end of another range?
+   *
+   * @param {Range} other - the other range
+   */
+  overlapsEnd(other) {
+    return this.start < other.end && this.end >= other.end
   }
 
   /**
@@ -227,6 +249,26 @@ class Range {
     )
     return [rangeUpToCursor, rangeAfterCursor]
   }
+
+  /**
+   * Returns the intersection of this range with another range
+   *
+   * @param {Range} other - the other range
+   * @return {Range | null} the intersection or null if the intersection is empty
+   */
+  intersect(other) {
+    if (this.contains(other)) {
+      return other
+    } else if (other.contains(this)) {
+      return this
+    } else if (other.overlapsStart(this)) {
+      return new Range(this.pos, other.end - this.start)
+    } else if (other.overlapsEnd(this)) {
+      return new Range(other.pos, this.end - other.start)
+    } else {
+      return null
+    }
+  }
 }
 
 module.exports = Range

libraries/overleaf-editor-core/test/lazy_string_file_data.test.js

@@ -193,4 +193,13 @@ describe('LazyStringFileData', function () {
     expect(fileData.getStringLength()).to.equal(longString.length)
     expect(fileData.getOperations()).to.have.length(1)
   })
+
+  it('truncates its operations after being stored', async function () {
+    const testHash = File.EMPTY_FILE_HASH
+    const fileData = new LazyStringFileData(testHash, undefined, 0)
+    fileData.edit(new TextOperation().insert('abc'))
+    const stored = await fileData.store(this.blobStore)
+    expect(fileData.hash).to.equal(stored.hash)
+    expect(fileData.operations).to.deep.equal([])
+  })
 })

libraries/overleaf-editor-core/test/range.test.js

@@ -1,4 +1,3 @@
-// @ts-check
 'use strict'
 
 const { expect } = require('chai')
@@ -449,4 +448,44 @@ describe('Range', function () {
       expect(() => range.insertAt(16, 3)).to.throw()
     })
   })
+
+  describe('intersect', function () {
+    it('should handle partially overlapping ranges', function () {
+      const range1 = new Range(5, 10)
+      const range2 = new Range(3, 6)
+      const intersection1 = range1.intersect(range2)
+      expect(intersection1.pos).to.equal(5)
+      expect(intersection1.length).to.equal(4)
+      const intersection2 = range2.intersect(range1)
+      expect(intersection2.pos).to.equal(5)
+      expect(intersection2.length).to.equal(4)
+    })
+
+    it('should intersect with itself', function () {
+      const range = new Range(5, 10)
+      const intersection = range.intersect(range)
+      expect(intersection.pos).to.equal(5)
+      expect(intersection.length).to.equal(10)
+    })
+
+    it('should handle nested ranges', function () {
+      const range1 = new Range(5, 10)
+      const range2 = new Range(7, 2)
+      const intersection1 = range1.intersect(range2)
+      expect(intersection1.pos).to.equal(7)
+      expect(intersection1.length).to.equal(2)
+      const intersection2 = range2.intersect(range1)
+      expect(intersection2.pos).to.equal(7)
+      expect(intersection2.length).to.equal(2)
+    })
+
+    it('should handle disconnected ranges', function () {
+      const range1 = new Range(5, 10)
+      const range2 = new Range(20, 30)
+      const intersection1 = range1.intersect(range2)
+      expect(intersection1).to.be.null
+      const intersection2 = range2.intersect(range1)
+      expect(intersection2).to.be.null
+    })
+  })
 })

libraries/overleaf-editor-core/test/scan_op.test.js

@@ -107,7 +107,7 @@ describe('RetainOp', function () {
     expect(op1.equals(new RetainOp(3))).to.be.true
   })
 
-  it('cannot merge with another RetainOp if tracking info is different', function () {
+  it('cannot merge with another RetainOp if the tracking user is different', function () {
     const op1 = new RetainOp(
       4,
       new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z'))
@@ -120,14 +120,14 @@ describe('RetainOp', function () {
     expect(() => op1.mergeWith(op2)).to.throw(Error)
   })
 
-  it('can merge with another RetainOp if tracking info is the same', function () {
+  it('can merge with another RetainOp if the tracking user is the same', function () {
     const op1 = new RetainOp(
       4,
       new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z'))
     )
     const op2 = new RetainOp(
       4,
-      new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z'))
+      new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:01.000Z'))
     )
     op1.mergeWith(op2)
     expect(
@@ -310,7 +310,7 @@ describe('InsertOp', function () {
     expect(() => op1.mergeWith(op2)).to.throw(Error)
   })
 
-  it('cannot merge with another InsertOp if tracking info is different', function () {
+  it('cannot merge with another InsertOp if tracking user is different', function () {
     const op1 = new InsertOp(
       'a',
       new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z'))
@@ -323,7 +323,7 @@ describe('InsertOp', function () {
     expect(() => op1.mergeWith(op2)).to.throw(Error)
   })
 
-  it('can merge with another InsertOp if tracking and comment info is the same', function () {
+  it('can merge with another InsertOp if tracking user and comment info is the same', function () {
     const op1 = new InsertOp(
       'a',
       new TrackingProps(
@@ -338,7 +338,7 @@ describe('InsertOp', function () {
       new TrackingProps(
         'insert',
         'user1',
-        new Date('2024-01-01T00:00:00.000Z')
+        new Date('2024-01-01T00:00:01.000Z')
       ),
       ['1', '2']
     )

libraries/overleaf-editor-core/test/text_operation.test.js

@@ -322,6 +322,47 @@ describe('TextOperation', function () {
         new TextOperation().retain(4).remove(4).retain(3)
       )
     })
+
+    it('undoing a tracked delete restores the tracked changes', function () {
+      expectInverseToLeadToInitialState(
+        new StringFileData(
+          'the quick brown fox jumps over the lazy dog',
+          undefined,
+          [
+            {
+              range: { pos: 5, length: 5 },
+              tracking: {
+                ts: '2023-01-01T00:00:00.000Z',
+                type: 'insert',
+                userId: 'user1',
+              },
+            },
+            {
+              range: { pos: 12, length: 3 },
+              tracking: {
+                ts: '2023-01-01T00:00:00.000Z',
+                type: 'delete',
+                userId: 'user1',
+              },
+            },
+            {
+              range: { pos: 18, length: 5 },
+              tracking: {
+                ts: '2023-01-01T00:00:00.000Z',
+                type: 'insert',
+                userId: 'user1',
+              },
+            },
+          ]
+        ),
+        new TextOperation()
+          .retain(7)
+          .retain(13, {
+            tracking: new TrackingProps('delete', 'user1', new Date()),
+          })
+          .retain(23)
+      )
+    })
   })
 
   describe('compose', function () {

libraries/promise-utils/buildscript.txt

@@ -1,6 +1,6 @@
 promise-utils
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/ranges-tracker/buildscript.txt

@@ -1,6 +1,6 @@
 ranges-tracker
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/redis-wrapper/buildscript.txt

@@ -1,6 +1,6 @@
 redis-wrapper
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/settings/buildscript.txt

@@ -1,6 +1,6 @@
 settings
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

libraries/stream-utils/buildscript.txt

@@ -1,6 +1,6 @@
 stream-utils
 --dependencies=None
---docker-repos=gcr.io/overleaf-ops
+--docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
 --env-add=
 --env-pass-through=
 --esmock-loader=False

package.json

@@ -37,7 +37,7 @@
     },
     "swagger-tools": {
       "body-parser": "1.20.3",
-      "multer": "2.0.0",
+      "multer": "2.0.1",
       "path-to-regexp": "3.3.0",
       "qs": "6.13.0"
     }

server-ce/hotfix/5.5.1/Dockerfile

@@ -0,0 +1,28 @@
+FROM sharelatex/sharelatex:5.5.0
+
+
+# fix tls configuration in redis for history-v1
+COPY pr_25168.patch .
+RUN patch -p1 < pr_25168.patch && rm pr_25168.patch
+
+# improve logging in history system
+COPY pr_26086.patch .
+RUN patch -p1 < pr_26086.patch && rm pr_26086.patch
+
+# fix create-user.mjs script
+COPY pr_26152.patch .
+RUN patch -p1 < pr_26152.patch && rm pr_26152.patch
+
+# check mongo featureCompatibilityVersion
+COPY pr_26091.patch .
+RUN patch -p1 < pr_26091.patch && rm pr_26091.patch
+
+# update multer and tar-fs
+RUN sed -i 's/"multer": "2.0.0"/"multer": "2.0.1"/g' package.json
+RUN sed -i 's/"dockerode": "^4.0.5"/"dockerode": "^4.0.7"/g' services/clsi/package.json
+RUN sed -i 's/"tar-fs": "^3.0.4"/"tar-fs": "^3.0.9"/g' services/clsi/package.json
+RUN sed -i 's/199c5ff05bd375c508f4074498237baead7f5148/4dbceda355efc3fc8ac3cf5c66c3778c8a6fdb23/g' services/web/package.json
+COPY package-lock.json.diff .
+RUN patch package-lock.json < package-lock.json.diff
+RUN npm install --omit=dev
+RUN npm install  @paralleldrive/cuid2@2.2.2 -w services/history-v1

server-ce/hotfix/5.5.1/pr_25168.patch

@@ -0,0 +1,19 @@
+--- a/services/history-v1/config/custom-environment-variables.json
++++ b/services/history-v1/config/custom-environment-variables.json
+@@ -50,12 +50,14 @@
+     "history": {
+       "host": "OVERLEAF_REDIS_HOST",
+       "password": "OVERLEAF_REDIS_PASS",
+-      "port": "OVERLEAF_REDIS_PORT"
++      "port": "OVERLEAF_REDIS_PORT",
++      "tls": "OVERLEAF_REDIS_TLS"
+     },
+     "lock": {
+       "host": "OVERLEAF_REDIS_HOST",
+       "password": "OVERLEAF_REDIS_PASS",
+-      "port": "OVERLEAF_REDIS_PORT"
++      "port": "OVERLEAF_REDIS_PORT",
++      "tls": "OVERLEAF_REDIS_TLS"
+     }
+   }
+ }

server-ce/hotfix/5.5.1/pr_26086.patch

@@ -0,0 +1,200 @@
+--- a/services/history-v1/api/controllers/project_import.js
++++ b/services/history-v1/api/controllers/project_import.js
+@@ -35,6 +35,7 @@ async function importSnapshot(req, res) {
+   try {
+     snapshot = Snapshot.fromRaw(rawSnapshot)
+   } catch (err) {
++    logger.warn({ err, projectId }, 'failed to import snapshot')
+     return render.unprocessableEntity(res)
+   }
+
+@@ -43,6 +44,7 @@ async function importSnapshot(req, res) {
+     historyId = await chunkStore.initializeProject(projectId, snapshot)
+   } catch (err) {
+     if (err instanceof chunkStore.AlreadyInitialized) {
++      logger.warn({ err, projectId }, 'already initialized')
+       return render.conflict(res)
+     } else {
+       throw err
+--- a/services/history-v1/api/controllers/projects.js
++++ b/services/history-v1/api/controllers/projects.js
+@@ -34,6 +34,7 @@ async function initializeProject(req, res, next) {
+     res.status(HTTPStatus.OK).json({ projectId })
+   } catch (err) {
+     if (err instanceof chunkStore.AlreadyInitialized) {
++      logger.warn({ err, projectId }, 'failed to initialize')
+       render.conflict(res)
+     } else {
+       throw err
+@@ -242,11 +243,15 @@ async function createProjectBlob(req, res, next) {
+     const sizeLimit = new StreamSizeLimit(maxUploadSize)
+     await pipeline(req, sizeLimit, fs.createWriteStream(tmpPath))
+     if (sizeLimit.sizeLimitExceeded) {
++      logger.warn(
++        { projectId, expectedHash, maxUploadSize },
++        'blob exceeds size threshold'
++      )
+       return render.requestEntityTooLarge(res)
+     }
+     const hash = await blobHash.fromFile(tmpPath)
+     if (hash !== expectedHash) {
+-      logger.debug({ hash, expectedHash }, 'Hash mismatch')
++      logger.warn({ projectId, hash, expectedHash }, 'Hash mismatch')
+       return render.conflict(res, 'File hash mismatch')
+     }
+
+@@ -343,6 +348,10 @@ async function copyProjectBlob(req, res, next) {
+     targetBlobStore.getBlob(blobHash),
+   ])
+   if (!sourceBlob) {
++    logger.warn(
++      { sourceProjectId, targetProjectId, blobHash },
++      'missing source blob when copying across projects'
++    )
+     return render.notFound(res)
+   }
+   // Exit early if the blob exists in the target project.
+--- a/services/history-v1/app.js
++++ b/services/history-v1/app.js
+@@ -100,11 +100,13 @@ function setupErrorHandling() {
+       })
+     }
+     if (err.code === 'ENUM_MISMATCH') {
++      logger.warn({ err, projectId }, err.message)
+       return res.status(HTTPStatus.UNPROCESSABLE_ENTITY).json({
+         message: 'invalid enum value: ' + err.paramName,
+       })
+     }
+     if (err.code === 'REQUIRED') {
++      logger.warn({ err, projectId }, err.message)
+       return res.status(HTTPStatus.UNPROCESSABLE_ENTITY).json({
+         message: err.message,
+       })
+--- a/services/project-history/app/js/HistoryStoreManager.js
++++ b/services/project-history/app/js/HistoryStoreManager.js
+@@ -35,7 +35,10 @@ class StringStream extends stream.Readable {
+ _mocks.getMostRecentChunk = (projectId, historyId, callback) => {
+   const path = `projects/${historyId}/latest/history`
+   logger.debug({ projectId, historyId }, 'getting chunk from history service')
+-  _requestChunk({ path, json: true }, callback)
++  _requestChunk({ path, json: true }, (err, chunk) => {
++    if (err) return callback(OError.tag(err))
++    callback(null, chunk)
++  })
+ }
+
+ /**
+@@ -54,7 +57,10 @@ export function getChunkAtVersion(projectId, historyId, version, callback) {
+     { projectId, historyId, version },
+     'getting chunk from history service for version'
+   )
+-  _requestChunk({ path, json: true }, callback)
++  _requestChunk({ path, json: true }, (err, chunk) => {
++    if (err) return callback(OError.tag(err))
++    callback(null, chunk)
++  })
+ }
+
+ export function getMostRecentVersion(projectId, historyId, callback) {
+@@ -68,8 +74,10 @@ export function getMostRecentVersion(projectId, historyId, callback) {
+       _.sortBy(chunk.chunk.history.changes || [], x => x.timestamp)
+     )
+     // find the latest project and doc versions in the chunk
+-    _getLatestProjectVersion(projectId, chunk, (err1, projectVersion) =>
++    _getLatestProjectVersion(projectId, chunk, (err1, projectVersion) => {
++      if (err1) err1 = OError.tag(err1)
+       _getLatestV2DocVersions(projectId, chunk, (err2, v2DocVersions) => {
++        if (err2) err2 = OError.tag(err2)
+         // return the project and doc versions
+         const projectStructureAndDocVersions = {
+           project: projectVersion,
+@@ -83,7 +91,7 @@ export function getMostRecentVersion(projectId, historyId, callback) {
+           chunk
+         )
+       })
+-    )
++    })
+   })
+ }
+
+@@ -211,7 +219,10 @@ export function getProjectBlob(historyId, blobHash, callback) {
+   logger.debug({ historyId, blobHash }, 'getting blob from history service')
+   _requestHistoryService(
+     { path: `projects/${historyId}/blobs/${blobHash}` },
+-    callback
++    (err, blob) => {
++      if (err) return callback(OError.tag(err))
++      callback(null, blob)
++    }
+   )
+ }
+
+@@ -277,7 +288,10 @@ function createBlobFromString(historyId, data, fileId, callback) {
+     (fsPath, cb) => {
+       _createBlob(historyId, fsPath, cb)
+     },
+-    callback
++    (err, hash) => {
++      if (err) return callback(OError.tag(err))
++      callback(null, hash)
++    }
+   )
+ }
+
+@@ -330,7 +344,7 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
+     try {
+       ranges = HistoryBlobTranslator.createRangeBlobDataFromUpdate(update)
+     } catch (error) {
+-      return callback(error)
++      return callback(OError.tag(error))
+     }
+     createBlobFromString(
+       historyId,
+@@ -338,7 +352,7 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
+       `project-${projectId}-doc-${update.doc}`,
+       (err, fileHash) => {
+         if (err) {
+-          return callback(err)
++          return callback(OError.tag(err))
+         }
+         if (ranges) {
+           createBlobFromString(
+@@ -347,7 +361,7 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
+             `project-${projectId}-doc-${update.doc}-ranges`,
+             (err, rangesHash) => {
+               if (err) {
+-                return callback(err)
++                return callback(OError.tag(err))
+               }
+               logger.debug(
+                 { fileHash, rangesHash },
+@@ -415,7 +429,7 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
+             },
+             (err, fileHash) => {
+               if (err) {
+-                return callback(err)
++                return callback(OError.tag(err))
+               }
+               if (update.hash && update.hash !== fileHash) {
+                 logger.warn(
+@@ -447,7 +461,7 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
+               },
+               (err, fileHash) => {
+                 if (err) {
+-                  return callback(err)
++                  return callback(OError.tag(err))
+                 }
+                 logger.debug({ fileHash }, 'created empty blob for file')
+                 callback(null, { file: fileHash })
+@@ -520,7 +534,10 @@ export function initializeProject(historyId, callback) {
+ export function deleteProject(projectId, callback) {
+   _requestHistoryService(
+     { method: 'DELETE', path: `projects/${projectId}` },
+-    callback
++    err => {
++      if (err) return callback(OError.tag(err))
++      callback(null)
++    }
+   )
+ }
+

server-ce/hotfix/5.5.1/pr_26091.patch

@@ -0,0 +1,60 @@
+--- a/services/web/modules/server-ce-scripts/scripts/check-mongodb.mjs
++++ b/services/web/modules/server-ce-scripts/scripts/check-mongodb.mjs
+@@ -7,6 +7,7 @@ import {
+ const { ObjectId } = mongodb
+
+ const MIN_MONGO_VERSION = [6, 0]
++const MIN_MONGO_FEATURE_COMPATIBILITY_VERSION = [6, 0]
+
+ async function main() {
+   let mongoClient
+@@ -18,6 +19,7 @@ async function main() {
+   }
+
+   await checkMongoVersion(mongoClient)
++  await checkFeatureCompatibilityVersion(mongoClient)
+
+   try {
+     await testTransactions(mongoClient)
+@@ -53,6 +55,41 @@ async function checkMongoVersion(mongoClient) {
+   }
+ }
+
++async function checkFeatureCompatibilityVersion(mongoClient) {
++  const {
++    featureCompatibilityVersion: { version },
++  } = await mongoClient
++    .db()
++    .admin()
++    .command({ getParameter: 1, featureCompatibilityVersion: 1 })
++  const [major, minor] = version.split('.').map(v => parseInt(v))
++  const [minMajor, minMinor] = MIN_MONGO_FEATURE_COMPATIBILITY_VERSION
++
++  if (major < minMajor || (major === minMajor && minor < minMinor)) {
++    const minVersion = MIN_MONGO_FEATURE_COMPATIBILITY_VERSION.join('.')
++    console.error(`
++The MongoDB server has featureCompatibilityVersion=${version}, but Overleaf requires at least version ${minVersion}.
++
++Open a mongo shell:
++- Overleaf Toolkit deployments: $ bin/mongo
++- Legacy docker-compose.yml deployments: $ docker exec -it mongo mongosh localhost/sharelatex
++
++In the mongo shell:
++> db.adminCommand( { setFeatureCompatibilityVersion: "${minMajor}.${minMinor}" } )
++
++Verify the new value:
++> db.adminCommand( { getParameter: 1, featureCompatibilityVersion: 1 } )
++ ...
++ {
++    featureCompatibilityVersion: { version: ${minMajor}.${minMinor}' },
++...
++
++Aborting.
++`)
++    process.exit(1)
++  }
++}
++
+ main()
+   .then(() => {
+     console.error('Mongodb is up.')

server-ce/hotfix/5.5.1/pr_26152.patch

@@ -0,0 +1,16 @@
+--- a/services/web/modules/server-ce-scripts/scripts/create-user.mjs
++++ b/services/web/modules/server-ce-scripts/scripts/create-user.mjs
+@@ -48,3 +48,13 @@ Please visit the following URL to set a password for ${email} and log in:
+     )
+   })
+ }
++
++if (filename === process.argv[1]) {
++  try {
++    await main()
++    process.exit(0)
++  } catch (error) {
++    console.error({ error })
++    process.exit(1)
++  }
++}

server-ce/test/Makefile

@@ -6,8 +6,8 @@ all: test-e2e
 # Notable the container labels com.docker.compose.project.working_dir and com.docker.compose.project.config_files need to match when creating containers from the docker host (how you started things) and from host-admin (how tests reconfigure the instance).
 export PWD = $(shell pwd)
 
-export TEX_LIVE_DOCKER_IMAGE ?= gcr.io/overleaf-ops/texlive-full:2023.1
+export TEX_LIVE_DOCKER_IMAGE ?= us-east1-docker.pkg.dev/overleaf-ops/ol-docker/texlive-full:2023.1
-export ALL_TEX_LIVE_DOCKER_IMAGES ?= gcr.io/overleaf-ops/texlive-full:2023.1,gcr.io/overleaf-ops/texlive-full:2022.1
+export ALL_TEX_LIVE_DOCKER_IMAGES ?= us-east1-docker.pkg.dev/overleaf-ops/ol-docker/texlive-full:2023.1,us-east1-docker.pkg.dev/overleaf-ops/ol-docker/texlive-full:2022.1
 export IMAGE_TAG_PRO ?= us-east1-docker.pkg.dev/overleaf-ops/ol-docker/pro:latest
 export CYPRESS_SHARD ?=
 export COMPOSE_PROJECT_NAME ?= test
@@ -20,6 +20,7 @@ test-e2e-native:
 		npm run cypress:open
 
 test-e2e:
+	docker compose build host-admin
 	docker compose up --no-log-prefix --exit-code-from=e2e e2e
 
 test-e2e-open:
@@ -45,7 +46,7 @@ prefetch_custom_compose_pull:
 prefetch_custom: prefetch_custom_texlive
 prefetch_custom_texlive:
 	echo "$$ALL_TEX_LIVE_DOCKER_IMAGES" | tr ',' '\n' | xargs -I% \
-	sh -exc 'tag=%; re_tag=quay.io/sharelatex/$${tag#*/*/}; docker pull $$tag; docker tag $$tag $$re_tag'
+	sh -exc 'tag=%; re_tag=quay.io/sharelatex/$${tag#*/*/*/}; docker pull $$tag; docker tag $$tag $$re_tag'
 
 prefetch_custom: prefetch_old
 prefetch_old:

server-ce/test/admin.spec.ts

@@ -179,6 +179,21 @@ describe('admin panel', function () {
         cy.get('nav').findByText('Manage Users').click()
       })
 
+      it('displays expected tabs', () => {
+        const tabs = ['Users', 'License Usage']
+        cy.get('[role="tab"]').each((el, index) => {
+          cy.wrap(el).findByText(tabs[index]).click()
+        })
+        cy.get('[role="tab"]').should('have.length', tabs.length)
+      })
+
+      it('license usage tab', () => {
+        cy.get('a').contains('License Usage').click()
+        cy.findByText(
+          'An active user is one who has opened a project in this Server Pro instance in the last 12 months.'
+        )
+      })
+
       describe('create users', () => {
         beforeEach(() => {
           cy.get('a').contains('New User').click()

server-ce/test/docker-compose.yml

@@ -131,7 +131,7 @@ services:
 
   saml:
     restart: always
-    image: gcr.io/overleaf-ops/saml-test
+    image: us-east1-docker.pkg.dev/overleaf-ops/ol-docker/saml-test
     environment:
       SAML_TEST_SP_ENTITY_ID: 'sharelatex-test-saml'
       SAML_BASE_URL_PATH: 'http://saml/simplesaml/'

services/chat/docker-compose.ci.yml

@@ -24,10 +24,13 @@ services:
       MOCHA_GREP: ${MOCHA_GREP}
       NODE_ENV: test
       NODE_OPTIONS: "--unhandled-rejections=strict"
+    volumes:
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     depends_on:
       mongo:
         condition: service_started
     user: node
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run test:acceptance
 
 

services/chat/docker-compose.yml

@@ -26,6 +26,7 @@ services:
       - .:/overleaf/services/chat
       - ../../node_modules:/overleaf/node_modules
       - ../../libraries:/overleaf/libraries
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     working_dir: /overleaf/services/chat
     environment:
       ELASTIC_SEARCH_DSN: es:9200
@@ -39,6 +40,7 @@ services:
     depends_on:
       mongo:
         condition: service_started
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run --silent test:acceptance
 
   mongo:

services/clsi/README.md

@@ -19,18 +19,18 @@ The CLSI can be configured through the following environment variables:
 * `ALLOWED_IMAGES` - Space separated list of allowed Docker TeX Live images
 * `CATCH_ERRORS` - Set to `true` to log uncaught exceptions
 * `COMPILE_GROUP_DOCKER_CONFIGS` - JSON string of Docker configs for compile groups
-* `COMPILES_HOST_DIR` - Working directory for LaTeX compiles
+* `SANDBOXED_COMPILES` - Set to true to use sibling containers
-* `OUTPUT_HOST_DIR` - Output directory for LaTeX compiles
+* `SANDBOXED_COMPILES_HOST_DIR_COMPILES` - Working directory for LaTeX compiles
+* `SANDBOXED_COMPILES_HOST_DIR_OUTPUT` - Output directory for LaTeX compiles
 * `COMPILE_SIZE_LIMIT` - Sets the body-parser [limit](https://github.com/expressjs/body-parser#limit)
-* `DOCKER_RUNNER` - Set to true to use sibling containers
 * `DOCKER_RUNTIME` -
 * `FILESTORE_DOMAIN_OVERRIDE` - The url for the filestore service e.g.`http://$FILESTORE_HOST:3009`
 * `FILESTORE_PARALLEL_FILE_DOWNLOADS` - Number of parallel file downloads
 * `LISTEN_ADDRESS` - The address for the RESTful service to listen on. Set to `0.0.0.0` to listen on all network interfaces
 * `PROCESS_LIFE_SPAN_LIMIT_MS` - Process life span limit in milliseconds
 * `SMOKE_TEST` - Whether to run smoke tests
-* `TEXLIVE_IMAGE` - The TeX Live Docker image to use for sibling containers, e.g. `gcr.io/overleaf-ops/texlive-full:2017.1`
+* `TEXLIVE_IMAGE` - The TeX Live Docker image to use for sibling containers, e.g. `us-east1-docker.pkg.dev/overleaf-ops/ol-docker/texlive-full:2017.1`
-* `TEX_LIVE_IMAGE_NAME_OVERRIDE` - The name of the registry for the Docker image e.g. `gcr.io/overleaf-ops`
+* `TEX_LIVE_IMAGE_NAME_OVERRIDE` - The name of the registry for the Docker image e.g. `us-east1-docker.pkg.dev/overleaf-ops/ol-docker`
 * `TEXLIVE_IMAGE_USER` - When using sibling containers, the user to run as in the TeX Live image. Defaults to `tex`
 * `TEXLIVE_OPENOUT_ANY` - Sets the `openout_any` environment variable for TeX Live (see the `\openout` primitive [documentation](http://tug.org/texinfohtml/web2c.html#tex-invocation))
 
@@ -63,10 +63,10 @@ Then start the Docker container:
 docker run --rm \
   -p 127.0.0.1:3013:3013 \
   -e LISTEN_ADDRESS=0.0.0.0 \
-  -e DOCKER_RUNNER=true \
+  -e SANDBOXED_COMPILES=true \
   -e TEXLIVE_IMAGE=texlive/texlive \
   -e TEXLIVE_IMAGE_USER=root \
-  -e COMPILES_HOST_DIR="$PWD/compiles" \
+  -e SANDBOXED_COMPILES_HOST_DIR_COMPILES="$PWD/compiles" \
   -v "$PWD/compiles:/overleaf/services/clsi/compiles" \
   -v "$PWD/cache:/overleaf/services/clsi/cache" \
   -v /var/run/docker.sock:/var/run/docker.sock \

services/clsi/buildscript.txt

@@ -2,7 +2,7 @@ clsi
 --data-dirs=cache,compiles,output
 --dependencies=
 --docker-repos=us-east1-docker.pkg.dev/overleaf-ops/ol-docker
---env-add=ENABLE_PDF_CACHING="true",PDF_CACHING_ENABLE_WORKER_POOL="true",ALLOWED_IMAGES=quay.io/sharelatex/texlive-full:2017.1,TEXLIVE_IMAGE=quay.io/sharelatex/texlive-full:2017.1,TEX_LIVE_IMAGE_NAME_OVERRIDE=us-east1-docker.pkg.dev/overleaf-ops/ol-docker,TEXLIVE_IMAGE_USER="tex",DOCKER_RUNNER="true",COMPILES_HOST_DIR=$PWD/compiles,OUTPUT_HOST_DIR=$PWD/output
+--env-add=ENABLE_PDF_CACHING="true",PDF_CACHING_ENABLE_WORKER_POOL="true",ALLOWED_IMAGES=quay.io/sharelatex/texlive-full:2017.1,TEXLIVE_IMAGE=quay.io/sharelatex/texlive-full:2017.1,TEX_LIVE_IMAGE_NAME_OVERRIDE=us-east1-docker.pkg.dev/overleaf-ops/ol-docker,TEXLIVE_IMAGE_USER="tex",SANDBOXED_COMPILES="true",SANDBOXED_COMPILES_HOST_DIR_COMPILES=$PWD/compiles,SANDBOXED_COMPILES_HOST_DIR_OUTPUT=$PWD/output
 --env-pass-through=
 --esmock-loader=False
 --node-version=22.15.1

services/clsi/docker-compose.ci.yml

@@ -29,9 +29,9 @@ services:
       TEXLIVE_IMAGE: quay.io/sharelatex/texlive-full:2017.1
       TEX_LIVE_IMAGE_NAME_OVERRIDE: us-east1-docker.pkg.dev/overleaf-ops/ol-docker
       TEXLIVE_IMAGE_USER: "tex"
-      DOCKER_RUNNER: "true"
+      SANDBOXED_COMPILES: "true"
-      COMPILES_HOST_DIR: $PWD/compiles
+      SANDBOXED_COMPILES_HOST_DIR_COMPILES: $PWD/compiles
-      OUTPUT_HOST_DIR: $PWD/output
+      SANDBOXED_COMPILES_HOST_DIR_OUTPUT: $PWD/output
     volumes:
       - ./compiles:/overleaf/services/clsi/compiles
       - /var/run/docker.sock:/var/run/docker.sock

services/clsi/docker-compose.yml

@@ -47,8 +47,8 @@ services:
       TEXLIVE_IMAGE: quay.io/sharelatex/texlive-full:2017.1
       TEX_LIVE_IMAGE_NAME_OVERRIDE: us-east1-docker.pkg.dev/overleaf-ops/ol-docker
       TEXLIVE_IMAGE_USER: "tex"
-      DOCKER_RUNNER: "true"
+      SANDBOXED_COMPILES: "true"
-      COMPILES_HOST_DIR: $PWD/compiles
+      SANDBOXED_COMPILES_HOST_DIR_COMPILES: $PWD/compiles
-      OUTPUT_HOST_DIR: $PWD/output
+      SANDBOXED_COMPILES_HOST_DIR_OUTPUT: $PWD/output
     command: npm run --silent test:acceptance
 

services/clsi/package.json

@@ -27,13 +27,13 @@
     "async": "^3.2.5",
     "body-parser": "^1.20.3",
     "bunyan": "^1.8.15",
-    "dockerode": "^4.0.5",
+    "dockerode": "^4.0.7",
     "express": "^4.21.2",
     "lodash": "^4.17.21",
     "p-limit": "^3.1.0",
     "request": "^2.88.2",
     "send": "^0.19.0",
-    "tar-fs": "^3.0.4",
+    "tar-fs": "^3.0.9",
     "workerpool": "^6.1.5"
   },
   "devDependencies": {

services/contacts/docker-compose.ci.yml

@@ -24,10 +24,13 @@ services:
       MOCHA_GREP: ${MOCHA_GREP}
       NODE_ENV: test
       NODE_OPTIONS: "--unhandled-rejections=strict"
+    volumes:
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     depends_on:
       mongo:
         condition: service_started
     user: node
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run test:acceptance
 
 

services/contacts/docker-compose.yml

@@ -26,6 +26,7 @@ services:
       - .:/overleaf/services/contacts
       - ../../node_modules:/overleaf/node_modules
       - ../../libraries:/overleaf/libraries
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     working_dir: /overleaf/services/contacts
     environment:
       ELASTIC_SEARCH_DSN: es:9200
@@ -39,6 +40,7 @@ services:
     depends_on:
       mongo:
         condition: service_started
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run --silent test:acceptance
 
   mongo:

services/contacts/package.json

@@ -6,9 +6,9 @@
   "main": "app.js",
   "scripts": {
     "start": "node app.js",
-    "test:acceptance:_run": "LOG_LEVEL=fatal mocha --loader=esmock --recursive --reporter spec --timeout 15000 --exit $@ test/acceptance/js",
+    "test:acceptance:_run": "mocha --loader=esmock --recursive --reporter spec --timeout 15000 --exit $@ test/acceptance/js",
     "test:acceptance": "npm run test:acceptance:_run -- --grep=$MOCHA_GREP",
-    "test:unit:_run": "LOG_LEVEL=fatal mocha --loader=esmock --recursive --reporter spec $@ test/unit/js",
+    "test:unit:_run": "mocha --loader=esmock --recursive --reporter spec $@ test/unit/js",
     "test:unit": "npm run test:unit:_run -- --grep=$MOCHA_GREP",
     "nodemon": "node --watch app.js",
     "lint": "eslint --max-warnings 0 --format unix .",

services/docstore/app.js

@@ -50,6 +50,14 @@ app.param('doc_id', function (req, res, next, docId) {
 app.get('/project/:project_id/doc-deleted', HttpController.getAllDeletedDocs)
 app.get('/project/:project_id/doc', HttpController.getAllDocs)
 app.get('/project/:project_id/ranges', HttpController.getAllRanges)
+app.get(
+  '/project/:project_id/comment-thread-ids',
+  HttpController.getCommentThreadIds
+)
+app.get(
+  '/project/:project_id/tracked-changes-user-ids',
+  HttpController.getTrackedChangesUserIds
+)
 app.get('/project/:project_id/has-ranges', HttpController.projectHasRanges)
 app.get('/project/:project_id/doc/:doc_id', HttpController.getDoc)
 app.get('/project/:project_id/doc/:doc_id/deleted', HttpController.isDocDeleted)

services/docstore/app/js/DocArchiveManager.js

@@ -1,5 +1,4 @@
-const { callbackify } = require('node:util')
+const MongoManager = require('./MongoManager')
-const MongoManager = require('./MongoManager').promises
 const Errors = require('./Errors')
 const logger = require('@overleaf/logger')
 const Settings = require('@overleaf/settings')
@@ -8,29 +7,12 @@ const { ReadableString } = require('@overleaf/stream-utils')
 const RangeManager = require('./RangeManager')
 const PersistorManager = require('./PersistorManager')
 const pMap = require('p-map')
-const { streamToBuffer } = require('./StreamToBuffer').promises
+const { streamToBuffer } = require('./StreamToBuffer')
 const { BSON } = require('mongodb-legacy')
 
 const PARALLEL_JOBS = Settings.parallelArchiveJobs
 const UN_ARCHIVE_BATCH_SIZE = Settings.unArchiveBatchSize
 
-module.exports = {
-  archiveAllDocs: callbackify(archiveAllDocs),
-  archiveDoc: callbackify(archiveDoc),
-  unArchiveAllDocs: callbackify(unArchiveAllDocs),
-  unarchiveDoc: callbackify(unarchiveDoc),
-  destroyProject: callbackify(destroyProject),
-  getDoc: callbackify(getDoc),
-  promises: {
-    archiveAllDocs,
-    archiveDoc,
-    unArchiveAllDocs,
-    unarchiveDoc,
-    destroyProject,
-    getDoc,
-  },
-}
-
 async function archiveAllDocs(projectId) {
   if (!_isArchivingEnabled()) {
     return
@@ -62,6 +44,8 @@ async function archiveDoc(projectId, docId) {
     throw new Error('doc has no lines')
   }
 
+  RangeManager.fixCommentIds(doc)
+
   // warn about any oversized docs already in mongo
   const linesSize = BSON.calculateObjectSize(doc.lines || {})
   const rangesSize = BSON.calculateObjectSize(doc.ranges || {})
@@ -225,3 +209,12 @@ function _isArchivingEnabled() {
 
   return true
 }
+
+module.exports = {
+  archiveAllDocs,
+  archiveDoc,
+  unArchiveAllDocs,
+  unarchiveDoc,
+  destroyProject,
+  getDoc,
+}

services/docstore/app/js/DocManager.js

@@ -5,7 +5,6 @@ const _ = require('lodash')
 const DocArchive = require('./DocArchiveManager')
 const RangeManager = require('./RangeManager')
 const Settings = require('@overleaf/settings')
-const { callbackifyAll } = require('@overleaf/promise-utils')
 const { setTimeout } = require('node:timers/promises')
 
 /**
@@ -29,7 +28,7 @@ const DocManager = {
       throw new Error('must include inS3 when getting doc')
     }
 
-    const doc = await MongoManager.promises.findDoc(projectId, docId, filter)
+    const doc = await MongoManager.findDoc(projectId, docId, filter)
 
     if (doc == null) {
       throw new Errors.NotFoundError(
@@ -38,15 +37,19 @@ const DocManager = {
     }
 
     if (doc.inS3) {
-      await DocArchive.promises.unarchiveDoc(projectId, docId)
+      await DocArchive.unarchiveDoc(projectId, docId)
       return await DocManager._getDoc(projectId, docId, filter)
     }
 
+    if (filter.ranges) {
+      RangeManager.fixCommentIds(doc)
+    }
+
     return doc
   },
 
   async isDocDeleted(projectId, docId) {
-    const doc = await MongoManager.promises.findDoc(projectId, docId, {
+    const doc = await MongoManager.findDoc(projectId, docId, {
       deleted: true,
     })
 
@@ -74,7 +77,7 @@ const DocManager = {
 
   // returns the doc without any version information
   async _peekRawDoc(projectId, docId) {
-    const doc = await MongoManager.promises.findDoc(projectId, docId, {
+    const doc = await MongoManager.findDoc(projectId, docId, {
       lines: true,
       rev: true,
       deleted: true,
@@ -91,7 +94,7 @@ const DocManager = {
 
     if (doc.inS3) {
       // skip the unarchiving to mongo when getting a doc
-      const archivedDoc = await DocArchive.promises.getDoc(projectId, docId)
+      const archivedDoc = await DocArchive.getDoc(projectId, docId)
       Object.assign(doc, archivedDoc)
     }
 
@@ -102,7 +105,7 @@ const DocManager = {
   // without unarchiving it (avoids unnecessary writes to mongo)
   async peekDoc(projectId, docId) {
     const doc = await DocManager._peekRawDoc(projectId, docId)
-    await MongoManager.promises.checkRevUnchanged(doc)
+    await MongoManager.checkRevUnchanged(doc)
     return doc
   },
 
@@ -111,16 +114,18 @@ const DocManager = {
       lines: true,
       inS3: true,
     })
-    return doc
+    if (!doc) throw new Errors.NotFoundError()
+    if (!Array.isArray(doc.lines)) throw new Errors.DocWithoutLinesError()
+    return doc.lines.join('\n')
   },
 
   async getAllDeletedDocs(projectId, filter) {
-    return await MongoManager.promises.getProjectsDeletedDocs(projectId, filter)
+    return await MongoManager.getProjectsDeletedDocs(projectId, filter)
   },
 
   async getAllNonDeletedDocs(projectId, filter) {
-    await DocArchive.promises.unArchiveAllDocs(projectId)
+    await DocArchive.unArchiveAllDocs(projectId)
-    const docs = await MongoManager.promises.getProjectsDocs(
+    const docs = await MongoManager.getProjectsDocs(
       projectId,
       { include_deleted: false },
       filter
@@ -128,15 +133,46 @@ const DocManager = {
     if (docs == null) {
       throw new Errors.NotFoundError(`No docs for project ${projectId}`)
     }
+    if (filter.ranges) {
+      for (const doc of docs) {
+        RangeManager.fixCommentIds(doc)
+      }
+    }
     return docs
   },
 
+  async getCommentThreadIds(projectId) {
+    const docs = await DocManager.getAllNonDeletedDocs(projectId, {
+      _id: true,
+      ranges: true,
+    })
+    const byDoc = new Map()
+    for (const doc of docs) {
+      const ids = new Set()
+      for (const comment of doc.ranges?.comments || []) {
+        ids.add(comment.op.t)
+      }
+      if (ids.size > 0) byDoc.set(doc._id.toString(), Array.from(ids))
+    }
+    return Object.fromEntries(byDoc.entries())
+  },
+
+  async getTrackedChangesUserIds(projectId) {
+    const docs = await DocManager.getAllNonDeletedDocs(projectId, {
+      ranges: true,
+    })
+    const userIds = new Set()
+    for (const doc of docs) {
+      for (const change of doc.ranges?.changes || []) {
+        if (change.metadata.user_id === 'anonymous-user') continue
+        userIds.add(change.metadata.user_id)
+      }
+    }
+    return Array.from(userIds)
+  },
+
   async projectHasRanges(projectId) {
-    const docs = await MongoManager.promises.getProjectsDocs(
+    const docs = await MongoManager.getProjectsDocs(projectId, {}, { _id: 1 })
-      projectId,
-      {},
-      { _id: 1 }
-    )
     const docIds = docs.map(doc => doc._id)
     for (const docId of docIds) {
       const doc = await DocManager.peekDoc(projectId, docId)
@@ -247,7 +283,7 @@ const DocManager = {
       }
 
       modified = true
-      await MongoManager.promises.upsertIntoDocCollection(
+      await MongoManager.upsertIntoDocCollection(
         projectId,
         docId,
         doc?.rev,
@@ -262,11 +298,7 @@ const DocManager = {
 
   async patchDoc(projectId, docId, meta) {
     const projection = { _id: 1, deleted: true }
-    const doc = await MongoManager.promises.findDoc(
+    const doc = await MongoManager.findDoc(projectId, docId, projection)
-      projectId,
-      docId,
-      projection
-    )
     if (!doc) {
       throw new Errors.NotFoundError(
         `No such project/doc to delete: ${projectId}/${docId}`
@@ -275,7 +307,7 @@ const DocManager = {
 
     if (meta.deleted && Settings.docstore.archiveOnSoftDelete) {
       // The user will not read this doc anytime soon. Flush it out of mongo.
-      DocArchive.promises.archiveDoc(projectId, docId).catch(err => {
+      DocArchive.archiveDoc(projectId, docId).catch(err => {
         logger.warn(
           { projectId, docId, err },
           'archiving a single doc in the background failed'
@@ -283,15 +315,8 @@ const DocManager = {
       })
     }
 
-    await MongoManager.promises.patchDoc(projectId, docId, meta)
+    await MongoManager.patchDoc(projectId, docId, meta)
   },
 }
 
-module.exports = {
+module.exports = DocManager
-  ...callbackifyAll(DocManager, {
-    multiResult: {
-      updateDoc: ['modified', 'rev'],
-    },
-  }),
-  promises: DocManager,
-}

services/docstore/app/js/Errors.js

@@ -10,10 +10,13 @@ class DocRevValueError extends OError {}
 
 class DocVersionDecrementedError extends OError {}
 
+class DocWithoutLinesError extends OError {}
+
 module.exports = {
   Md5MismatchError,
   DocModifiedError,
   DocRevValueError,
   DocVersionDecrementedError,
+  DocWithoutLinesError,
   ...Errors,
 }

services/docstore/app/js/HealthChecker.js

@@ -1,67 +1,35 @@
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const { db, ObjectId } = require('./mongodb')
-const request = require('request')
-const async = require('async')
 const _ = require('lodash')
 const crypto = require('node:crypto')
 const settings = require('@overleaf/settings')
 const { port } = settings.internal.docstore
 const logger = require('@overleaf/logger')
+const { fetchNothing, fetchJson } = require('@overleaf/fetch-utils')
 
-module.exports = {
+async function check() {
-  check(callback) {
+  const docId = new ObjectId()
-    const docId = new ObjectId()
+  const projectId = new ObjectId(settings.docstore.healthCheck.project_id)
-    const projectId = new ObjectId(settings.docstore.healthCheck.project_id)
+  const url = `http://127.0.0.1:${port}/project/${projectId}/doc/${docId}`
-    const url = `http://127.0.0.1:${port}/project/${projectId}/doc/${docId}`
+  const lines = [
-    const lines = [
+    'smoke test - delete me',
-      'smoke test - delete me',
+    `${crypto.randomBytes(32).toString('hex')}`,
-      `${crypto.randomBytes(32).toString('hex')}`,
+  ]
-    ]
+  logger.debug({ lines, url, docId, projectId }, 'running health check')
-    const getOpts = () => ({
+  let body
-      url,
+  try {
-      timeout: 3000,
+    await fetchNothing(url, {
+      method: 'POST',
+      json: { lines, version: 42, ranges: {} },
+      signal: AbortSignal.timeout(3_000),
     })
-    logger.debug({ lines, url, docId, projectId }, 'running health check')
+    body = await fetchJson(url, { signal: AbortSignal.timeout(3_000) })
-    const jobs = [
+  } finally {
-      function (cb) {
+    await db.docs.deleteOne({ _id: docId, project_id: projectId })
-        const opts = getOpts()
+  }
-        opts.json = { lines, version: 42, ranges: {} }
+  if (!_.isEqual(body?.lines, lines)) {
-        return request.post(opts, cb)
+    throw new Error(`health check lines not equal ${body.lines} != ${lines}`)
-      },
+  }
-      function (cb) {
+}
-        const opts = getOpts()
+module.exports = {
-        opts.json = true
+  check,
-        return request.get(opts, function (err, res, body) {
-          if (err != null) {
-            logger.err({ err }, 'docstore returned a error in health check get')
-            return cb(err)
-          } else if (res == null) {
-            return cb(new Error('no response from docstore with get check'))
-          } else if ((res != null ? res.statusCode : undefined) !== 200) {
-            return cb(new Error(`status code not 200, its ${res.statusCode}`))
-          } else if (
-            _.isEqual(body != null ? body.lines : undefined, lines) &&
-            (body != null ? body._id : undefined) === docId.toString()
-          ) {
-            return cb()
-          } else {
-            return cb(
-              new Error(
-                `health check lines not equal ${body.lines} != ${lines}`
-              )
-            )
-          }
-        })
-      },
-      cb => db.docs.deleteOne({ _id: docId, project_id: projectId }, cb),
-    ]
-    return async.series(jobs, callback)
-  },
 }

services/docstore/app/js/HttpController.js

@@ -4,143 +4,104 @@ const DocArchive = require('./DocArchiveManager')
 const HealthChecker = require('./HealthChecker')
 const Errors = require('./Errors')
 const Settings = require('@overleaf/settings')
+const { expressify } = require('@overleaf/promise-utils')
 
-function getDoc(req, res, next) {
+async function getDoc(req, res) {
   const { doc_id: docId, project_id: projectId } = req.params
   const includeDeleted = req.query.include_deleted === 'true'
   logger.debug({ projectId, docId }, 'getting doc')
-  DocManager.getFullDoc(projectId, docId, function (error, doc) {
+  const doc = await DocManager.getFullDoc(projectId, docId)
-    if (error) {
+  logger.debug({ docId, projectId }, 'got doc')
-      return next(error)
+  if (doc.deleted && !includeDeleted) {
-    }
+    res.sendStatus(404)
-    logger.debug({ docId, projectId }, 'got doc')
+  } else {
-    if (doc == null) {
+    res.json(_buildDocView(doc))
-      res.sendStatus(404)
+  }
-    } else if (doc.deleted && !includeDeleted) {
-      res.sendStatus(404)
-    } else {
-      res.json(_buildDocView(doc))
-    }
-  })
 }
 
-function peekDoc(req, res, next) {
+async function peekDoc(req, res) {
   const { doc_id: docId, project_id: projectId } = req.params
   logger.debug({ projectId, docId }, 'peeking doc')
-  DocManager.peekDoc(projectId, docId, function (error, doc) {
+  const doc = await DocManager.peekDoc(projectId, docId)
-    if (error) {
+  res.setHeader('x-doc-status', doc.inS3 ? 'archived' : 'active')
-      return next(error)
+  res.json(_buildDocView(doc))
-    }
-    if (doc == null) {
-      res.sendStatus(404)
-    } else {
-      res.setHeader('x-doc-status', doc.inS3 ? 'archived' : 'active')
-      res.json(_buildDocView(doc))
-    }
-  })
 }
 
-function isDocDeleted(req, res, next) {
+async function isDocDeleted(req, res) {
   const { doc_id: docId, project_id: projectId } = req.params
-  DocManager.isDocDeleted(projectId, docId, function (error, deleted) {
+  const deleted = await DocManager.isDocDeleted(projectId, docId)
-    if (error) {
+  res.json({ deleted })
-      return next(error)
-    }
-    res.json({ deleted })
-  })
 }
 
-function getRawDoc(req, res, next) {
+async function getRawDoc(req, res) {
   const { doc_id: docId, project_id: projectId } = req.params
   logger.debug({ projectId, docId }, 'getting raw doc')
-  DocManager.getDocLines(projectId, docId, function (error, doc) {
+  const content = await DocManager.getDocLines(projectId, docId)
-    if (error) {
+  res.setHeader('content-type', 'text/plain')
-      return next(error)
+  res.send(content)
-    }
-    if (doc == null) {
-      res.sendStatus(404)
-    } else {
-      res.setHeader('content-type', 'text/plain')
-      res.send(_buildRawDocView(doc))
-    }
-  })
 }
 
-function getAllDocs(req, res, next) {
+async function getAllDocs(req, res) {
   const { project_id: projectId } = req.params
   logger.debug({ projectId }, 'getting all docs')
-  DocManager.getAllNonDeletedDocs(
+  const docs = await DocManager.getAllNonDeletedDocs(projectId, {
-    projectId,
+    lines: true,
-    { lines: true, rev: true },
+    rev: true,
-    function (error, docs) {
+  })
-      if (docs == null) {
+  const docViews = _buildDocsArrayView(projectId, docs)
-        docs = []
+  for (const docView of docViews) {
-      }
+    if (!docView.lines) {
-      if (error) {
+      logger.warn({ projectId, docId: docView._id }, 'missing doc lines')
-        return next(error)
+      docView.lines = []
-      }
-      const docViews = _buildDocsArrayView(projectId, docs)
-      for (const docView of docViews) {
-        if (!docView.lines) {
-          logger.warn({ projectId, docId: docView._id }, 'missing doc lines')
-          docView.lines = []
-        }
-      }
-      res.json(docViews)
     }
-  )
+  }
+  res.json(docViews)
 }
 
-function getAllDeletedDocs(req, res, next) {
+async function getAllDeletedDocs(req, res) {
   const { project_id: projectId } = req.params
   logger.debug({ projectId }, 'getting all deleted docs')
-  DocManager.getAllDeletedDocs(
+  const docs = await DocManager.getAllDeletedDocs(projectId, {
-    projectId,
+    name: true,
-    { name: true, deletedAt: true },
+    deletedAt: true,
-    function (error, docs) {
+  })
-      if (error) {
+  res.json(
-        return next(error)
+    docs.map(doc => ({
-      }
+      _id: doc._id.toString(),
-      res.json(
+      name: doc.name,
-        docs.map(doc => ({
+      deletedAt: doc.deletedAt,
-          _id: doc._id.toString(),
+    }))
-          name: doc.name,
-          deletedAt: doc.deletedAt,
-        }))
-      )
-    }
   )
 }
 
-function getAllRanges(req, res, next) {
+async function getAllRanges(req, res) {
   const { project_id: projectId } = req.params
   logger.debug({ projectId }, 'getting all ranges')
-  DocManager.getAllNonDeletedDocs(
+  const docs = await DocManager.getAllNonDeletedDocs(projectId, {
-    projectId,
+    ranges: true,
-    { ranges: true },
-    function (error, docs) {
-      if (docs == null) {
-        docs = []
-      }
-      if (error) {
-        return next(error)
-      }
-      res.json(_buildDocsArrayView(projectId, docs))
-    }
-  )
-}
-
-function projectHasRanges(req, res, next) {
-  const { project_id: projectId } = req.params
-  DocManager.projectHasRanges(projectId, (err, projectHasRanges) => {
-    if (err) {
-      return next(err)
-    }
-    res.json({ projectHasRanges })
   })
+  res.json(_buildDocsArrayView(projectId, docs))
 }
 
-function updateDoc(req, res, next) {
+async function getCommentThreadIds(req, res) {
+  const { project_id: projectId } = req.params
+  const threadIds = await DocManager.getCommentThreadIds(projectId)
+  res.json(threadIds)
+}
+
+async function getTrackedChangesUserIds(req, res) {
+  const { project_id: projectId } = req.params
+  const userIds = await DocManager.getTrackedChangesUserIds(projectId)
+  res.json(userIds)
+}
+
+async function projectHasRanges(req, res) {
+  const { project_id: projectId } = req.params
+  const projectHasRanges = await DocManager.projectHasRanges(projectId)
+  res.json({ projectHasRanges })
+}
+
+async function updateDoc(req, res) {
   const { doc_id: docId, project_id: projectId } = req.params
   const lines = req.body?.lines
   const version = req.body?.version
@@ -172,25 +133,20 @@ function updateDoc(req, res, next) {
   }
 
   logger.debug({ projectId, docId }, 'got http request to update doc')
-  DocManager.updateDoc(
+  const { modified, rev } = await DocManager.updateDoc(
     projectId,
     docId,
     lines,
     version,
-    ranges,
+    ranges
-    function (error, modified, rev) {
-      if (error) {
-        return next(error)
-      }
-      res.json({
-        modified,
-        rev,
-      })
-    }
   )
+  res.json({
+    modified,
+    rev,
+  })
 }
 
-function patchDoc(req, res, next) {
+async function patchDoc(req, res) {
   const { doc_id: docId, project_id: projectId } = req.params
   logger.debug({ projectId, docId }, 'patching doc')
 
@@ -203,12 +159,8 @@ function patchDoc(req, res, next) {
       logger.fatal({ field }, 'joi validation for pathDoc is broken')
     }
   })
-  DocManager.patchDoc(projectId, docId, meta, function (error) {
+  await DocManager.patchDoc(projectId, docId, meta)
-    if (error) {
+  res.sendStatus(204)
-      return next(error)
-    }
-    res.sendStatus(204)
-  })
 }
 
 function _buildDocView(doc) {
@@ -221,10 +173,6 @@ function _buildDocView(doc) {
   return docView
 }
 
-function _buildRawDocView(doc) {
-  return (doc?.lines ?? []).join('\n')
-}
-
 function _buildDocsArrayView(projectId, docs) {
   const docViews = []
   for (const doc of docs) {
@@ -241,79 +189,69 @@ function _buildDocsArrayView(projectId, docs) {
   return docViews
 }
 
-function archiveAllDocs(req, res, next) {
+async function archiveAllDocs(req, res) {
   const { project_id: projectId } = req.params
   logger.debug({ projectId }, 'archiving all docs')
-  DocArchive.archiveAllDocs(projectId, function (error) {
+  await DocArchive.archiveAllDocs(projectId)
-    if (error) {
+  res.sendStatus(204)
-      return next(error)
-    }
-    res.sendStatus(204)
-  })
 }
 
-function archiveDoc(req, res, next) {
+async function archiveDoc(req, res) {
   const { doc_id: docId, project_id: projectId } = req.params
   logger.debug({ projectId, docId }, 'archiving a doc')
-  DocArchive.archiveDoc(projectId, docId, function (error) {
+  await DocArchive.archiveDoc(projectId, docId)
-    if (error) {
+  res.sendStatus(204)
-      return next(error)
-    }
-    res.sendStatus(204)
-  })
 }
 
-function unArchiveAllDocs(req, res, next) {
+async function unArchiveAllDocs(req, res) {
   const { project_id: projectId } = req.params
   logger.debug({ projectId }, 'unarchiving all docs')
-  DocArchive.unArchiveAllDocs(projectId, function (err) {
+  try {
-    if (err) {
+    await DocArchive.unArchiveAllDocs(projectId)
-      if (err instanceof Errors.DocRevValueError) {
+  } catch (err) {
-        logger.warn({ err }, 'Failed to unarchive doc')
+    if (err instanceof Errors.DocRevValueError) {
-        return res.sendStatus(409)
+      logger.warn({ err }, 'Failed to unarchive doc')
-      }
+      return res.sendStatus(409)
-      return next(err)
     }
-    res.sendStatus(200)
+    throw err
-  })
+  }
+  res.sendStatus(200)
 }
 
-function destroyProject(req, res, next) {
+async function destroyProject(req, res) {
   const { project_id: projectId } = req.params
   logger.debug({ projectId }, 'destroying all docs')
-  DocArchive.destroyProject(projectId, function (error) {
+  await DocArchive.destroyProject(projectId)
-    if (error) {
+  res.sendStatus(204)
-      return next(error)
-    }
-    res.sendStatus(204)
-  })
 }
 
-function healthCheck(req, res) {
+async function healthCheck(req, res) {
-  HealthChecker.check(function (err) {
+  try {
-    if (err) {
+    await HealthChecker.check()
-      logger.err({ err }, 'error performing health check')
+  } catch (err) {
-      res.sendStatus(500)
+    logger.err({ err }, 'error performing health check')
-    } else {
+    res.sendStatus(500)
-      res.sendStatus(200)
+    return
-    }
+  }
-  })
+  res.sendStatus(200)
 }
 
 module.exports = {
-  getDoc,
+  getDoc: expressify(getDoc),
-  peekDoc,
+  peekDoc: expressify(peekDoc),
-  isDocDeleted,
+  isDocDeleted: expressify(isDocDeleted),
-  getRawDoc,
+  getRawDoc: expressify(getRawDoc),
-  getAllDocs,
+  getAllDocs: expressify(getAllDocs),
-  getAllDeletedDocs,
+  getAllDeletedDocs: expressify(getAllDeletedDocs),
-  getAllRanges,
+  getAllRanges: expressify(getAllRanges),
-  projectHasRanges,
+  getTrackedChangesUserIds: expressify(getTrackedChangesUserIds),
-  updateDoc,
+  getCommentThreadIds: expressify(getCommentThreadIds),
-  patchDoc,
+  projectHasRanges: expressify(projectHasRanges),
-  archiveAllDocs,
+  updateDoc: expressify(updateDoc),
-  archiveDoc,
+  patchDoc: expressify(patchDoc),
-  unArchiveAllDocs,
+  archiveAllDocs: expressify(archiveAllDocs),
-  destroyProject,
+  archiveDoc: expressify(archiveDoc),
-  healthCheck,
+  unArchiveAllDocs: expressify(unArchiveAllDocs),
+  destroyProject: expressify(destroyProject),
+  healthCheck: expressify(healthCheck),
 }

services/docstore/app/js/MongoManager.js

@@ -1,7 +1,6 @@
 const { db, ObjectId } = require('./mongodb')
 const Settings = require('@overleaf/settings')
 const Errors = require('./Errors')
-const { callbackify } = require('node:util')
 
 const ARCHIVING_LOCK_DURATION_MS = Settings.archivingLockDurationMs
 
@@ -241,34 +240,17 @@ async function destroyProject(projectId) {
 }
 
 module.exports = {
-  findDoc: callbackify(findDoc),
+  findDoc,
-  getProjectsDeletedDocs: callbackify(getProjectsDeletedDocs),
+  getProjectsDeletedDocs,
-  getProjectsDocs: callbackify(getProjectsDocs),
+  getProjectsDocs,
-  getArchivedProjectDocs: callbackify(getArchivedProjectDocs),
+  getArchivedProjectDocs,
-  getNonArchivedProjectDocIds: callbackify(getNonArchivedProjectDocIds),
+  getNonArchivedProjectDocIds,
-  getNonDeletedArchivedProjectDocs: callbackify(
+  getNonDeletedArchivedProjectDocs,
-    getNonDeletedArchivedProjectDocs
+  upsertIntoDocCollection,
-  ),
+  restoreArchivedDoc,
-  upsertIntoDocCollection: callbackify(upsertIntoDocCollection),
+  patchDoc,
-  restoreArchivedDoc: callbackify(restoreArchivedDoc),
+  getDocForArchiving,
-  patchDoc: callbackify(patchDoc),
+  markDocAsArchived,
-  getDocForArchiving: callbackify(getDocForArchiving),
+  checkRevUnchanged,
-  markDocAsArchived: callbackify(markDocAsArchived),
+  destroyProject,
-  checkRevUnchanged: callbackify(checkRevUnchanged),
-  destroyProject: callbackify(destroyProject),
-  promises: {
-    findDoc,
-    getProjectsDeletedDocs,
-    getProjectsDocs,
-    getArchivedProjectDocs,
-    getNonArchivedProjectDocIds,
-    getNonDeletedArchivedProjectDocs,
-    upsertIntoDocCollection,
-    restoreArchivedDoc,
-    patchDoc,
-    getDocForArchiving,
-    markDocAsArchived,
-    checkRevUnchanged,
-    destroyProject,
-  },
 }

services/docstore/app/js/RangeManager.js

@@ -49,15 +49,25 @@ module.exports = RangeManager = {
       updateMetadata(change.metadata)
     }
     for (const comment of Array.from(ranges.comments || [])) {
-      comment.id = RangeManager._safeObjectId(comment.id)
+      // Two bugs resulted in mismatched ids, prefer the thread id from the op: https://github.com/overleaf/internal/issues/23272
-      if ((comment.op != null ? comment.op.t : undefined) != null) {
+      comment.id = RangeManager._safeObjectId(comment.op?.t || comment.id)
-        comment.op.t = RangeManager._safeObjectId(comment.op.t)
+      if (comment.op) comment.op.t = comment.id
-      }
+
+      // resolved property is added to comments when they are obtained from history, but this state doesn't belong in mongo docs collection
+      // more info: https://github.com/overleaf/internal/issues/24371#issuecomment-2913095174
+      delete comment.op?.resolved
       updateMetadata(comment.metadata)
     }
     return ranges
   },
 
+  fixCommentIds(doc) {
+    for (const comment of doc?.ranges?.comments || []) {
+      // Two bugs resulted in mismatched ids, prefer the thread id from the op: https://github.com/overleaf/internal/issues/23272
+      if (comment.op?.t) comment.id = comment.op.t
+    }
+  },
+
   _safeObjectId(data) {
     try {
       return new ObjectId(data)

services/docstore/app/js/StreamToBuffer.js

@@ -2,13 +2,9 @@ const { LoggerStream, WritableBuffer } = require('@overleaf/stream-utils')
 const Settings = require('@overleaf/settings')
 const logger = require('@overleaf/logger/logging-manager')
 const { pipeline } = require('node:stream/promises')
-const { callbackify } = require('node:util')
 
 module.exports = {
-  streamToBuffer: callbackify(streamToBuffer),
+  streamToBuffer,
-  promises: {
-    streamToBuffer,
-  },
 }
 
 async function streamToBuffer(projectId, docId, stream) {

services/docstore/docker-compose.ci.yml

@@ -27,12 +27,15 @@ services:
       MOCHA_GREP: ${MOCHA_GREP}
       NODE_ENV: test
       NODE_OPTIONS: "--unhandled-rejections=strict"
+    volumes:
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     depends_on:
       mongo:
         condition: service_started
       gcs:
         condition: service_healthy
     user: node
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run test:acceptance
 
 

services/docstore/docker-compose.yml

@@ -26,6 +26,7 @@ services:
       - .:/overleaf/services/docstore
       - ../../node_modules:/overleaf/node_modules
       - ../../libraries:/overleaf/libraries
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     working_dir: /overleaf/services/docstore
     environment:
       ELASTIC_SEARCH_DSN: es:9200
@@ -44,6 +45,7 @@ services:
         condition: service_started
       gcs:
         condition: service_healthy
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run --silent test:acceptance
 
   mongo:

services/docstore/package.json

@@ -17,6 +17,7 @@
     "types:check": "tsc --noEmit"
   },
   "dependencies": {
+    "@overleaf/fetch-utils": "*",
     "@overleaf/logger": "*",
     "@overleaf/metrics": "*",
     "@overleaf/o-error": "*",

services/docstore/test/acceptance/js/ArchiveDocsTests.js

@@ -1001,6 +1001,15 @@ describe('Archiving', function () {
         },
         version: 2,
       }
+      this.fixedRanges = {
+        ...this.doc.ranges,
+        comments: [
+          {
+            ...this.doc.ranges.comments[0],
+            id: this.doc.ranges.comments[0].op.t,
+          },
+        ],
+      }
       return DocstoreClient.createDoc(
         this.project_id,
         this.doc._id,
@@ -1048,7 +1057,7 @@ describe('Archiving', function () {
             throw error
           }
           s3Doc.lines.should.deep.equal(this.doc.lines)
-          const ranges = JSON.parse(JSON.stringify(this.doc.ranges)) // ObjectId -> String
+          const ranges = JSON.parse(JSON.stringify(this.fixedRanges)) // ObjectId -> String
           s3Doc.ranges.should.deep.equal(ranges)
           return done()
         }
@@ -1075,7 +1084,7 @@ describe('Archiving', function () {
             throw error
           }
           doc.lines.should.deep.equal(this.doc.lines)
-          doc.ranges.should.deep.equal(this.doc.ranges)
+          doc.ranges.should.deep.equal(this.fixedRanges)
           expect(doc.inS3).not.to.exist
           return done()
         })

services/docstore/test/acceptance/js/GettingAllDocsTests.js

@@ -20,30 +20,73 @@ const DocstoreClient = require('./helpers/DocstoreClient')
 describe('Getting all docs', function () {
   beforeEach(function (done) {
     this.project_id = new ObjectId()
+    this.threadId1 = new ObjectId().toString()
+    this.threadId2 = new ObjectId().toString()
     this.docs = [
       {
         _id: new ObjectId(),
         lines: ['one', 'two', 'three'],
-        ranges: { mock: 'one' },
+        ranges: {
+          comments: [
+            { id: new ObjectId().toString(), op: { t: this.threadId1 } },
+          ],
+          changes: [
+            {
+              id: new ObjectId().toString(),
+              metadata: { user_id: 'user-id-1' },
+            },
+          ],
+        },
         rev: 2,
       },
       {
         _id: new ObjectId(),
         lines: ['aaa', 'bbb', 'ccc'],
-        ranges: { mock: 'two' },
+        ranges: {
+          changes: [
+            {
+              id: new ObjectId().toString(),
+              metadata: { user_id: 'user-id-2' },
+            },
+          ],
+        },
         rev: 4,
       },
       {
         _id: new ObjectId(),
         lines: ['111', '222', '333'],
-        ranges: { mock: 'three' },
+        ranges: {
+          comments: [
+            { id: new ObjectId().toString(), op: { t: this.threadId2 } },
+          ],
+          changes: [
+            {
+              id: new ObjectId().toString(),
+              metadata: { user_id: 'anonymous-user' },
+            },
+          ],
+        },
         rev: 6,
       },
     ]
+    this.fixedRanges = this.docs.map(doc => {
+      if (!doc.ranges?.comments?.length) return doc.ranges
+      return {
+        ...doc.ranges,
+        comments: [
+          { ...doc.ranges.comments[0], id: doc.ranges.comments[0].op.t },
+        ],
+      }
+    })
     this.deleted_doc = {
       _id: new ObjectId(),
       lines: ['deleted'],
-      ranges: { mock: 'four' },
+      ranges: {
+        comments: [{ id: new ObjectId().toString(), op: { t: 'thread-id-3' } }],
+        changes: [
+          { id: new ObjectId().toString(), metadata: { user_id: 'user-id-3' } },
+        ],
+      },
       rev: 8,
     }
     const version = 42
@@ -96,7 +139,7 @@ describe('Getting all docs', function () {
     })
   })
 
-  return it('getAllRanges should return all the (non-deleted) doc ranges', function (done) {
+  it('getAllRanges should return all the (non-deleted) doc ranges', function (done) {
     return DocstoreClient.getAllRanges(this.project_id, (error, res, docs) => {
       if (error != null) {
         throw error
@@ -104,9 +147,38 @@ describe('Getting all docs', function () {
       docs.length.should.equal(this.docs.length)
       for (let i = 0; i < docs.length; i++) {
         const doc = docs[i]
-        doc.ranges.should.deep.equal(this.docs[i].ranges)
+        doc.ranges.should.deep.equal(this.fixedRanges[i])
       }
       return done()
     })
   })
+
+  it('getTrackedChangesUserIds should return all the user ids from (non-deleted) ranges', function (done) {
+    DocstoreClient.getTrackedChangesUserIds(
+      this.project_id,
+      (error, res, userIds) => {
+        if (error != null) {
+          throw error
+        }
+        userIds.should.deep.equal(['user-id-1', 'user-id-2'])
+        done()
+      }
+    )
+  })
+
+  it('getCommentThreadIds should return all the thread ids from (non-deleted) ranges', function (done) {
+    DocstoreClient.getCommentThreadIds(
+      this.project_id,
+      (error, res, threadIds) => {
+        if (error != null) {
+          throw error
+        }
+        threadIds.should.deep.equal({
+          [this.docs[0]._id.toString()]: [this.threadId1],
+          [this.docs[2]._id.toString()]: [this.threadId2],
+        })
+        done()
+      }
+    )
+  })
 })

services/docstore/test/acceptance/js/GettingDocsTests.js

@@ -28,10 +28,26 @@ describe('Getting a doc', function () {
           op: { i: 'foo', p: 3 },
           meta: {
             user_id: new ObjectId().toString(),
-            ts: new Date().toString(),
+            ts: new Date().toJSON(),
           },
         },
       ],
+      comments: [
+        {
+          id: new ObjectId().toString(),
+          op: { c: 'comment', p: 1, t: new ObjectId().toString() },
+          metadata: {
+            user_id: new ObjectId().toString(),
+            ts: new Date().toJSON(),
+          },
+        },
+      ],
+    }
+    this.fixedRanges = {
+      ...this.ranges,
+      comments: [
+        { ...this.ranges.comments[0], id: this.ranges.comments[0].op.t },
+      ],
     }
     return DocstoreApp.ensureRunning(() => {
       return DocstoreClient.createDoc(
@@ -60,7 +76,7 @@ describe('Getting a doc', function () {
           if (error) return done(error)
           doc.lines.should.deep.equal(this.lines)
           doc.version.should.equal(this.version)
-          doc.ranges.should.deep.equal(this.ranges)
+          doc.ranges.should.deep.equal(this.fixedRanges)
           return done()
         }
       )
@@ -114,7 +130,7 @@ describe('Getting a doc', function () {
           if (error) return done(error)
           doc.lines.should.deep.equal(this.lines)
           doc.version.should.equal(this.version)
-          doc.ranges.should.deep.equal(this.ranges)
+          doc.ranges.should.deep.equal(this.fixedRanges)
           doc.deleted.should.equal(true)
           return done()
         }

services/docstore/test/acceptance/js/HealthCheckerTest.js

@@ -0,0 +1,28 @@
+const { db } = require('../../../app/js/mongodb')
+const DocstoreApp = require('./helpers/DocstoreApp')
+const DocstoreClient = require('./helpers/DocstoreClient')
+const { expect } = require('chai')
+
+describe('HealthChecker', function () {
+  beforeEach('start', function (done) {
+    DocstoreApp.ensureRunning(done)
+  })
+  beforeEach('clear docs collection', async function () {
+    await db.docs.deleteMany({})
+  })
+  let res
+  beforeEach('run health check', function (done) {
+    DocstoreClient.healthCheck((err, _res) => {
+      res = _res
+      done(err)
+    })
+  })
+
+  it('should return 200', function () {
+    res.statusCode.should.equal(200)
+  })
+
+  it('should not leave any cruft behind', async function () {
+    expect(await db.docs.find({}).toArray()).to.deep.equal([])
+  })
+})

services/docstore/test/acceptance/js/helpers/DocstoreClient.js

@@ -100,6 +100,26 @@ module.exports = DocstoreClient = {
     )
   },
 
+  getCommentThreadIds(projectId, callback) {
+    request.get(
+      {
+        url: `http://127.0.0.1:${settings.internal.docstore.port}/project/${projectId}/comment-thread-ids`,
+        json: true,
+      },
+      callback
+    )
+  },
+
+  getTrackedChangesUserIds(projectId, callback) {
+    request.get(
+      {
+        url: `http://127.0.0.1:${settings.internal.docstore.port}/project/${projectId}/tracked-changes-user-ids`,
+        json: true,
+      },
+      callback
+    )
+  },
+
   updateDoc(projectId, docId, lines, version, ranges, callback) {
     return request.post(
       {
@@ -181,6 +201,13 @@ module.exports = DocstoreClient = {
     )
   },
 
+  healthCheck(callback) {
+    request.get(
+      `http://127.0.0.1:${settings.internal.docstore.port}/health_check`,
+      callback
+    )
+  },
+
   getS3Doc(projectId, docId, callback) {
     getStringFromPersistor(
       Persistor,

services/docstore/test/unit/js/DocArchiveManagerTests.js

@@ -4,7 +4,7 @@ const modulePath = '../../../app/js/DocArchiveManager.js'
 const SandboxedModule = require('sandboxed-module')
 const { ObjectId } = require('mongodb-legacy')
 const Errors = require('../../../app/js/Errors')
-const StreamToBuffer = require('../../../app/js/StreamToBuffer').promises
+const StreamToBuffer = require('../../../app/js/StreamToBuffer')
 
 describe('DocArchiveManager', function () {
   let DocArchiveManager,
@@ -31,6 +31,7 @@ describe('DocArchiveManager', function () {
 
     RangeManager = {
       jsonRangesToMongo: sinon.stub().returns({ mongo: 'ranges' }),
+      fixCommentIds: sinon.stub(),
     }
     Settings = {
       docstore: {
@@ -142,37 +143,33 @@ describe('DocArchiveManager', function () {
     }
 
     MongoManager = {
-      promises: {
+      markDocAsArchived: sinon.stub().resolves(),
-        markDocAsArchived: sinon.stub().resolves(),
+      restoreArchivedDoc: sinon.stub().resolves(),
-        restoreArchivedDoc: sinon.stub().resolves(),
+      upsertIntoDocCollection: sinon.stub().resolves(),
-        upsertIntoDocCollection: sinon.stub().resolves(),
+      getProjectsDocs: sinon.stub().resolves(mongoDocs),
-        getProjectsDocs: sinon.stub().resolves(mongoDocs),
+      getNonDeletedArchivedProjectDocs: getArchivedProjectDocs,
-        getNonDeletedArchivedProjectDocs: getArchivedProjectDocs,
+      getNonArchivedProjectDocIds,
-        getNonArchivedProjectDocIds,
+      getArchivedProjectDocs,
-        getArchivedProjectDocs,
+      findDoc: sinon.stub().callsFake(fakeGetDoc),
-        findDoc: sinon.stub().callsFake(fakeGetDoc),
+      getDocForArchiving: sinon.stub().callsFake(fakeGetDoc),
-        getDocForArchiving: sinon.stub().callsFake(fakeGetDoc),
+      destroyProject: sinon.stub().resolves(),
-        destroyProject: sinon.stub().resolves(),
-      },
     }
 
     // Wrap streamToBuffer so that we can pass in something that it expects (in
     // this case, a Promise) rather than a stubbed stream object
     streamToBuffer = {
-      promises: {
+      streamToBuffer: async () => {
-        streamToBuffer: async () => {
+        const inputStream = new Promise(resolve => {
-          const inputStream = new Promise(resolve => {
+          stream.on('data', data => resolve(data))
-            stream.on('data', data => resolve(data))
+        })
-          })
 
-          const value = await StreamToBuffer.streamToBuffer(
+        const value = await StreamToBuffer.streamToBuffer(
-            'testProjectId',
+          'testProjectId',
-            'testDocId',
+          'testDocId',
-            inputStream
+          inputStream
-          )
+        )
 
-          return value
+        return value
-        },
       },
     }
 
@@ -192,9 +189,13 @@ describe('DocArchiveManager', function () {
 
   describe('archiveDoc', function () {
     it('should resolve when passed a valid document', async function () {
-      await expect(
+      await expect(DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)).to
-        DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+        .eventually.be.fulfilled
-      ).to.eventually.be.fulfilled
+    })
+
+    it('should fix comment ids', async function () {
+      await DocArchiveManager.archiveDoc(projectId, mongoDocs[1]._id)
+      expect(RangeManager.fixCommentIds).to.have.been.called
     })
 
     it('should throw an error if the doc has no lines', async function () {
@@ -202,26 +203,26 @@ describe('DocArchiveManager', function () {
       doc.lines = null
 
       await expect(
-        DocArchiveManager.promises.archiveDoc(projectId, doc._id)
+        DocArchiveManager.archiveDoc(projectId, doc._id)
       ).to.eventually.be.rejectedWith('doc has no lines')
     })
 
     it('should add the schema version', async function () {
-      await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[1]._id)
+      await DocArchiveManager.archiveDoc(projectId, mongoDocs[1]._id)
       expect(StreamUtils.ReadableString).to.have.been.calledWith(
         sinon.match(/"schema_v":1/)
       )
     })
 
     it('should calculate the hex md5 sum of the content', async function () {
-      await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+      await DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
       expect(Crypto.createHash).to.have.been.calledWith('md5')
       expect(HashUpdate).to.have.been.calledWith(archivedDocJson)
       expect(HashDigest).to.have.been.calledWith('hex')
     })
 
     it('should pass the md5 hash to the object persistor for verification', async function () {
-      await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+      await DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
 
       expect(PersistorManager.sendStream).to.have.been.calledWith(
         sinon.match.any,
@@ -232,7 +233,7 @@ describe('DocArchiveManager', function () {
     })
 
     it('should pass the correct bucket and key to the persistor', async function () {
-      await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+      await DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
 
       expect(PersistorManager.sendStream).to.have.been.calledWith(
         Settings.docstore.bucket,
@@ -241,7 +242,7 @@ describe('DocArchiveManager', function () {
     })
 
     it('should create a stream from the encoded json and send it', async function () {
-      await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+      await DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
       expect(StreamUtils.ReadableString).to.have.been.calledWith(
         archivedDocJson
       )
@@ -253,8 +254,8 @@ describe('DocArchiveManager', function () {
     })
 
     it('should mark the doc as archived', async function () {
-      await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+      await DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
-      expect(MongoManager.promises.markDocAsArchived).to.have.been.calledWith(
+      expect(MongoManager.markDocAsArchived).to.have.been.calledWith(
         projectId,
         mongoDocs[0]._id,
         mongoDocs[0].rev
@@ -267,8 +268,8 @@ describe('DocArchiveManager', function () {
       })
 
       it('should bail out early', async function () {
-        await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+        await DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
-        expect(MongoManager.promises.getDocForArchiving).to.not.have.been.called
+        expect(MongoManager.getDocForArchiving).to.not.have.been.called
       })
     })
 
@@ -285,7 +286,7 @@ describe('DocArchiveManager', function () {
 
       it('should return an error', async function () {
         await expect(
-          DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+          DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
         ).to.eventually.be.rejectedWith('null bytes detected')
       })
     })
@@ -296,21 +297,19 @@ describe('DocArchiveManager', function () {
 
     describe('when the doc is in S3', function () {
       beforeEach(function () {
-        MongoManager.promises.findDoc = sinon
+        MongoManager.findDoc = sinon.stub().resolves({ inS3: true, rev })
-          .stub()
-          .resolves({ inS3: true, rev })
         docId = mongoDocs[0]._id
         lines = ['doc', 'lines']
         rev = 123
       })
 
       it('should resolve when passed a valid document', async function () {
-        await expect(DocArchiveManager.promises.unarchiveDoc(projectId, docId))
+        await expect(DocArchiveManager.unarchiveDoc(projectId, docId)).to
-          .to.eventually.be.fulfilled
+          .eventually.be.fulfilled
       })
 
       it('should test md5 validity with the raw buffer', async function () {
-        await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+        await DocArchiveManager.unarchiveDoc(projectId, docId)
         expect(HashUpdate).to.have.been.calledWith(
           sinon.match.instanceOf(Buffer)
         )
@@ -319,15 +318,17 @@ describe('DocArchiveManager', function () {
       it('should throw an error if the md5 does not match', async function () {
         PersistorManager.getObjectMd5Hash.resolves('badf00d')
         await expect(
-          DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+          DocArchiveManager.unarchiveDoc(projectId, docId)
         ).to.eventually.be.rejected.and.be.instanceof(Errors.Md5MismatchError)
       })
 
       it('should restore the doc in Mongo', async function () {
-        await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+        await DocArchiveManager.unarchiveDoc(projectId, docId)
-        expect(
+        expect(MongoManager.restoreArchivedDoc).to.have.been.calledWith(
-          MongoManager.promises.restoreArchivedDoc
+          projectId,
-        ).to.have.been.calledWith(projectId, docId, archivedDoc)
+          docId,
+          archivedDoc
+        )
       })
 
       describe('when archiving is not configured', function () {
@@ -337,15 +338,15 @@ describe('DocArchiveManager', function () {
 
         it('should error out on archived doc', async function () {
           await expect(
-            DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+            DocArchiveManager.unarchiveDoc(projectId, docId)
           ).to.eventually.be.rejected.and.match(
             /found archived doc, but archiving backend is not configured/
           )
         })
 
         it('should return early on non-archived doc', async function () {
-          MongoManager.promises.findDoc = sinon.stub().resolves({ rev })
+          MongoManager.findDoc = sinon.stub().resolves({ rev })
-          await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+          await DocArchiveManager.unarchiveDoc(projectId, docId)
           expect(PersistorManager.getObjectMd5Hash).to.not.have.been.called
         })
       })
@@ -363,10 +364,12 @@ describe('DocArchiveManager', function () {
           })
 
           it('should return the docs lines', async function () {
-            await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+            await DocArchiveManager.unarchiveDoc(projectId, docId)
-            expect(
+            expect(MongoManager.restoreArchivedDoc).to.have.been.calledWith(
-              MongoManager.promises.restoreArchivedDoc
+              projectId,
-            ).to.have.been.calledWith(projectId, docId, { lines, rev })
+              docId,
+              { lines, rev }
+            )
           })
         })
 
@@ -385,14 +388,16 @@ describe('DocArchiveManager', function () {
           })
 
           it('should return the doc lines and ranges', async function () {
-            await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+            await DocArchiveManager.unarchiveDoc(projectId, docId)
-            expect(
+            expect(MongoManager.restoreArchivedDoc).to.have.been.calledWith(
-              MongoManager.promises.restoreArchivedDoc
+              projectId,
-            ).to.have.been.calledWith(projectId, docId, {
+              docId,
-              lines,
+              {
-              ranges: { mongo: 'ranges' },
+                lines,
-              rev: 456,
+                ranges: { mongo: 'ranges' },
-            })
+                rev: 456,
+              }
+            )
           })
         })
 
@@ -406,10 +411,12 @@ describe('DocArchiveManager', function () {
           })
 
           it('should return only the doc lines', async function () {
-            await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+            await DocArchiveManager.unarchiveDoc(projectId, docId)
-            expect(
+            expect(MongoManager.restoreArchivedDoc).to.have.been.calledWith(
-              MongoManager.promises.restoreArchivedDoc
+              projectId,
-            ).to.have.been.calledWith(projectId, docId, { lines, rev: 456 })
+              docId,
+              { lines, rev: 456 }
+            )
           })
         })
 
@@ -423,10 +430,12 @@ describe('DocArchiveManager', function () {
           })
 
           it('should use the rev obtained from Mongo', async function () {
-            await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+            await DocArchiveManager.unarchiveDoc(projectId, docId)
-            expect(
+            expect(MongoManager.restoreArchivedDoc).to.have.been.calledWith(
-              MongoManager.promises.restoreArchivedDoc
+              projectId,
-            ).to.have.been.calledWith(projectId, docId, { lines, rev })
+              docId,
+              { lines, rev }
+            )
           })
         })
 
@@ -441,7 +450,7 @@ describe('DocArchiveManager', function () {
 
           it('should throw an error', async function () {
             await expect(
-              DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+              DocArchiveManager.unarchiveDoc(projectId, docId)
             ).to.eventually.be.rejectedWith(
               "I don't understand the doc format in s3"
             )
@@ -451,8 +460,8 @@ describe('DocArchiveManager', function () {
     })
 
     it('should not do anything if the file is already unarchived', async function () {
-      MongoManager.promises.findDoc.resolves({ inS3: false })
+      MongoManager.findDoc.resolves({ inS3: false })
-      await DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+      await DocArchiveManager.unarchiveDoc(projectId, docId)
       expect(PersistorManager.getObjectStream).not.to.have.been.called
     })
 
@@ -461,7 +470,7 @@ describe('DocArchiveManager', function () {
         .stub()
         .rejects(new Errors.NotFoundError())
       await expect(
-        DocArchiveManager.promises.unarchiveDoc(projectId, docId)
+        DocArchiveManager.unarchiveDoc(projectId, docId)
       ).to.eventually.be.rejected.and.be.instanceof(Errors.NotFoundError)
     })
   })
@@ -469,13 +478,11 @@ describe('DocArchiveManager', function () {
   describe('destroyProject', function () {
     describe('when archiving is enabled', function () {
       beforeEach(async function () {
-        await DocArchiveManager.promises.destroyProject(projectId)
+        await DocArchiveManager.destroyProject(projectId)
       })
 
       it('should delete the project in Mongo', function () {
-        expect(MongoManager.promises.destroyProject).to.have.been.calledWith(
+        expect(MongoManager.destroyProject).to.have.been.calledWith(projectId)
-          projectId
-        )
       })
 
       it('should delete the project in the persistor', function () {
@@ -489,13 +496,11 @@ describe('DocArchiveManager', function () {
     describe('when archiving is disabled', function () {
       beforeEach(async function () {
         Settings.docstore.backend = ''
-        await DocArchiveManager.promises.destroyProject(projectId)
+        await DocArchiveManager.destroyProject(projectId)
       })
 
       it('should delete the project in Mongo', function () {
-        expect(MongoManager.promises.destroyProject).to.have.been.calledWith(
+        expect(MongoManager.destroyProject).to.have.been.calledWith(projectId)
-          projectId
-        )
       })
 
       it('should not delete the project in the persistor', function () {
@@ -506,33 +511,35 @@ describe('DocArchiveManager', function () {
 
   describe('archiveAllDocs', function () {
     it('should resolve with valid arguments', async function () {
-      await expect(DocArchiveManager.promises.archiveAllDocs(projectId)).to
+      await expect(DocArchiveManager.archiveAllDocs(projectId)).to.eventually.be
-        .eventually.be.fulfilled
+        .fulfilled
     })
 
     it('should archive all project docs which are not in s3', async function () {
-      await DocArchiveManager.promises.archiveAllDocs(projectId)
+      await DocArchiveManager.archiveAllDocs(projectId)
       // not inS3
-      expect(MongoManager.promises.markDocAsArchived).to.have.been.calledWith(
+      expect(MongoManager.markDocAsArchived).to.have.been.calledWith(
         projectId,
         mongoDocs[0]._id
       )
-      expect(MongoManager.promises.markDocAsArchived).to.have.been.calledWith(
+      expect(MongoManager.markDocAsArchived).to.have.been.calledWith(
         projectId,
         mongoDocs[1]._id
       )
-      expect(MongoManager.promises.markDocAsArchived).to.have.been.calledWith(
+      expect(MongoManager.markDocAsArchived).to.have.been.calledWith(
         projectId,
         mongoDocs[4]._id
       )
 
       // inS3
-      expect(
+      expect(MongoManager.markDocAsArchived).not.to.have.been.calledWith(
-        MongoManager.promises.markDocAsArchived
+        projectId,
-      ).not.to.have.been.calledWith(projectId, mongoDocs[2]._id)
+        mongoDocs[2]._id
-      expect(
+      )
-        MongoManager.promises.markDocAsArchived
+      expect(MongoManager.markDocAsArchived).not.to.have.been.calledWith(
-      ).not.to.have.been.calledWith(projectId, mongoDocs[3]._id)
+        projectId,
+        mongoDocs[3]._id
+      )
     })
 
     describe('when archiving is not configured', function () {
@@ -541,21 +548,20 @@ describe('DocArchiveManager', function () {
       })
 
       it('should bail out early', async function () {
-        await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+        await DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
-        expect(MongoManager.promises.getNonArchivedProjectDocIds).to.not.have
+        expect(MongoManager.getNonArchivedProjectDocIds).to.not.have.been.called
-          .been.called
       })
     })
   })
 
   describe('unArchiveAllDocs', function () {
     it('should resolve with valid arguments', async function () {
-      await expect(DocArchiveManager.promises.unArchiveAllDocs(projectId)).to
+      await expect(DocArchiveManager.unArchiveAllDocs(projectId)).to.eventually
-        .eventually.be.fulfilled
+        .be.fulfilled
     })
 
     it('should unarchive all inS3 docs', async function () {
-      await DocArchiveManager.promises.unArchiveAllDocs(projectId)
+      await DocArchiveManager.unArchiveAllDocs(projectId)
 
       for (const doc of archivedDocs) {
         expect(PersistorManager.getObjectStream).to.have.been.calledWith(
@@ -571,9 +577,9 @@ describe('DocArchiveManager', function () {
       })
 
       it('should bail out early', async function () {
-        await DocArchiveManager.promises.archiveDoc(projectId, mongoDocs[0]._id)
+        await DocArchiveManager.archiveDoc(projectId, mongoDocs[0]._id)
-        expect(MongoManager.promises.getNonDeletedArchivedProjectDocs).to.not
+        expect(MongoManager.getNonDeletedArchivedProjectDocs).to.not.have.been
-          .have.been.called
+          .called
       })
     })
   })

services/docstore/test/unit/js/DocManagerTests.js

@@ -17,25 +17,22 @@ describe('DocManager', function () {
     this.version = 42
 
     this.MongoManager = {
-      promises: {
+      findDoc: sinon.stub(),
-        findDoc: sinon.stub(),
+      getProjectsDocs: sinon.stub(),
-        getProjectsDocs: sinon.stub(),
+      patchDoc: sinon.stub().resolves(),
-        patchDoc: sinon.stub().resolves(),
+      upsertIntoDocCollection: sinon.stub().resolves(),
-        upsertIntoDocCollection: sinon.stub().resolves(),
-      },
     }
     this.DocArchiveManager = {
-      promises: {
+      unarchiveDoc: sinon.stub(),
-        unarchiveDoc: sinon.stub(),
+      unArchiveAllDocs: sinon.stub(),
-        unArchiveAllDocs: sinon.stub(),
+      archiveDoc: sinon.stub().resolves(),
-        archiveDoc: sinon.stub().resolves(),
-      },
     }
     this.RangeManager = {
       jsonRangesToMongo(r) {
         return r
       },
       shouldUpdateRanges: sinon.stub().returns(false),
+      fixCommentIds: sinon.stub(),
     }
     this.settings = { docstore: {} }
 
@@ -52,7 +49,7 @@ describe('DocManager', function () {
 
   describe('getFullDoc', function () {
     beforeEach(function () {
-      this.DocManager.promises._getDoc = sinon.stub()
+      this.DocManager._getDoc = sinon.stub()
       this.doc = {
         _id: this.doc_id,
         lines: ['2134'],
@@ -60,13 +57,10 @@ describe('DocManager', function () {
     })
 
     it('should call get doc with a quick filter', async function () {
-      this.DocManager.promises._getDoc.resolves(this.doc)
+      this.DocManager._getDoc.resolves(this.doc)
-      const doc = await this.DocManager.promises.getFullDoc(
+      const doc = await this.DocManager.getFullDoc(this.project_id, this.doc_id)
-        this.project_id,
-        this.doc_id
-      )
       doc.should.equal(this.doc)
-      this.DocManager.promises._getDoc
+      this.DocManager._getDoc
         .calledWith(this.project_id, this.doc_id, {
           lines: true,
           rev: true,
@@ -79,27 +73,27 @@ describe('DocManager', function () {
     })
 
     it('should return error when get doc errors', async function () {
-      this.DocManager.promises._getDoc.rejects(this.stubbedError)
+      this.DocManager._getDoc.rejects(this.stubbedError)
       await expect(
-        this.DocManager.promises.getFullDoc(this.project_id, this.doc_id)
+        this.DocManager.getFullDoc(this.project_id, this.doc_id)
       ).to.be.rejectedWith(this.stubbedError)
     })
   })
 
   describe('getRawDoc', function () {
     beforeEach(function () {
-      this.DocManager.promises._getDoc = sinon.stub()
+      this.DocManager._getDoc = sinon.stub()
       this.doc = { lines: ['2134'] }
     })
 
     it('should call get doc with a quick filter', async function () {
-      this.DocManager.promises._getDoc.resolves(this.doc)
+      this.DocManager._getDoc.resolves(this.doc)
-      const doc = await this.DocManager.promises.getDocLines(
+      const content = await this.DocManager.getDocLines(
         this.project_id,
         this.doc_id
       )
-      doc.should.equal(this.doc)
+      content.should.equal(this.doc.lines.join('\n'))
-      this.DocManager.promises._getDoc
+      this.DocManager._getDoc
         .calledWith(this.project_id, this.doc_id, {
           lines: true,
           inS3: true,
@@ -108,11 +102,46 @@ describe('DocManager', function () {
     })
 
     it('should return error when get doc errors', async function () {
-      this.DocManager.promises._getDoc.rejects(this.stubbedError)
+      this.DocManager._getDoc.rejects(this.stubbedError)
       await expect(
-        this.DocManager.promises.getDocLines(this.project_id, this.doc_id)
+        this.DocManager.getDocLines(this.project_id, this.doc_id)
       ).to.be.rejectedWith(this.stubbedError)
     })
+
+    it('should return error when get doc does not exist', async function () {
+      this.DocManager._getDoc.resolves(null)
+      await expect(
+        this.DocManager.getDocLines(this.project_id, this.doc_id)
+      ).to.be.rejectedWith(Errors.NotFoundError)
+    })
+
+    it('should return error when get doc has no lines', async function () {
+      this.DocManager._getDoc.resolves({})
+      await expect(
+        this.DocManager.getDocLines(this.project_id, this.doc_id)
+      ).to.be.rejectedWith(Errors.DocWithoutLinesError)
+    })
+  })
+
+  describe('_getDoc', function () {
+    it('should return error when get doc does not exist', async function () {
+      this.MongoManager.findDoc.resolves(null)
+      await expect(
+        this.DocManager._getDoc(this.project_id, this.doc_id, { inS3: true })
+      ).to.be.rejectedWith(Errors.NotFoundError)
+    })
+
+    it('should fix comment ids', async function () {
+      this.MongoManager.findDoc.resolves({
+        _id: this.doc_id,
+        ranges: {},
+      })
+      await this.DocManager._getDoc(this.project_id, this.doc_id, {
+        inS3: true,
+        ranges: true,
+      })
+      expect(this.RangeManager.fixCommentIds).to.have.been.called
+    })
   })
 
   describe('getDoc', function () {
@@ -128,26 +157,25 @@ describe('DocManager', function () {
 
     describe('when using a filter', function () {
       beforeEach(function () {
-        this.MongoManager.promises.findDoc.resolves(this.doc)
+        this.MongoManager.findDoc.resolves(this.doc)
       })
 
       it('should error if inS3 is not set to true', async function () {
         await expect(
-          this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
+          this.DocManager._getDoc(this.project_id, this.doc_id, {
             inS3: false,
           })
         ).to.be.rejected
       })
 
       it('should always get inS3 even when no filter is passed', async function () {
-        await expect(
+        await expect(this.DocManager._getDoc(this.project_id, this.doc_id)).to
-          this.DocManager.promises._getDoc(this.project_id, this.doc_id)
+          .be.rejected
-        ).to.be.rejected
+        this.MongoManager.findDoc.called.should.equal(false)
-        this.MongoManager.promises.findDoc.called.should.equal(false)
       })
 
       it('should not error if inS3 is set to true', async function () {
-        await this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
+        await this.DocManager._getDoc(this.project_id, this.doc_id, {
           inS3: true,
         })
       })
@@ -155,8 +183,8 @@ describe('DocManager', function () {
 
     describe('when the doc is in the doc collection', function () {
       beforeEach(async function () {
-        this.MongoManager.promises.findDoc.resolves(this.doc)
+        this.MongoManager.findDoc.resolves(this.doc)
-        this.result = await this.DocManager.promises._getDoc(
+        this.result = await this.DocManager._getDoc(
           this.project_id,
           this.doc_id,
           { version: true, inS3: true }
@@ -164,7 +192,7 @@ describe('DocManager', function () {
       })
 
       it('should get the doc from the doc collection', function () {
-        this.MongoManager.promises.findDoc
+        this.MongoManager.findDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
@@ -177,9 +205,9 @@ describe('DocManager', function () {
 
     describe('when MongoManager.findDoc errors', function () {
       it('should return the error', async function () {
-        this.MongoManager.promises.findDoc.rejects(this.stubbedError)
+        this.MongoManager.findDoc.rejects(this.stubbedError)
         await expect(
-          this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
+          this.DocManager._getDoc(this.project_id, this.doc_id, {
             version: true,
             inS3: true,
           })
@@ -202,15 +230,15 @@ describe('DocManager', function () {
           version: 2,
           inS3: false,
         }
-        this.MongoManager.promises.findDoc.resolves(this.doc)
+        this.MongoManager.findDoc.resolves(this.doc)
-        this.DocArchiveManager.promises.unarchiveDoc.callsFake(
+        this.DocArchiveManager.unarchiveDoc.callsFake(
           async (projectId, docId) => {
-            this.MongoManager.promises.findDoc.resolves({
+            this.MongoManager.findDoc.resolves({
               ...this.unarchivedDoc,
             })
           }
         )
-        this.result = await this.DocManager.promises._getDoc(
+        this.result = await this.DocManager._getDoc(
           this.project_id,
           this.doc_id,
           {
@@ -221,13 +249,13 @@ describe('DocManager', function () {
       })
 
       it('should call the DocArchive to unarchive the doc', function () {
-        this.DocArchiveManager.promises.unarchiveDoc
+        this.DocArchiveManager.unarchiveDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should look up the doc twice', function () {
-        this.MongoManager.promises.findDoc.calledTwice.should.equal(true)
+        this.MongoManager.findDoc.calledTwice.should.equal(true)
       })
 
       it('should return the doc', function () {
@@ -239,9 +267,9 @@ describe('DocManager', function () {
 
     describe('when the doc does not exist in the docs collection', function () {
       it('should return a NotFoundError', async function () {
-        this.MongoManager.promises.findDoc.resolves(null)
+        this.MongoManager.findDoc.resolves(null)
         await expect(
-          this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
+          this.DocManager._getDoc(this.project_id, this.doc_id, {
             version: true,
             inS3: true,
           })
@@ -262,23 +290,27 @@ describe('DocManager', function () {
             lines: ['mock-lines'],
           },
         ]
-        this.MongoManager.promises.getProjectsDocs.resolves(this.docs)
+        this.MongoManager.getProjectsDocs.resolves(this.docs)
-        this.DocArchiveManager.promises.unArchiveAllDocs.resolves(this.docs)
+        this.DocArchiveManager.unArchiveAllDocs.resolves(this.docs)
-        this.filter = { lines: true }
+        this.filter = { lines: true, ranges: true }
-        this.result = await this.DocManager.promises.getAllNonDeletedDocs(
+        this.result = await this.DocManager.getAllNonDeletedDocs(
           this.project_id,
           this.filter
         )
       })
 
       it('should get the project from the database', function () {
-        this.MongoManager.promises.getProjectsDocs.should.have.been.calledWith(
+        this.MongoManager.getProjectsDocs.should.have.been.calledWith(
           this.project_id,
           { include_deleted: false },
           this.filter
         )
       })
 
+      it('should fix comment ids', async function () {
+        expect(this.RangeManager.fixCommentIds).to.have.been.called
+      })
+
       it('should return the docs', function () {
         expect(this.result).to.deep.equal(this.docs)
       })
@@ -286,13 +318,10 @@ describe('DocManager', function () {
 
     describe('when there are no docs for the project', function () {
       it('should return a NotFoundError', async function () {
-        this.MongoManager.promises.getProjectsDocs.resolves(null)
+        this.MongoManager.getProjectsDocs.resolves(null)
-        this.DocArchiveManager.promises.unArchiveAllDocs.resolves(null)
+        this.DocArchiveManager.unArchiveAllDocs.resolves(null)
         await expect(
-          this.DocManager.promises.getAllNonDeletedDocs(
+          this.DocManager.getAllNonDeletedDocs(this.project_id, this.filter)
-            this.project_id,
-            this.filter
-          )
         ).to.be.rejectedWith(`No docs for project ${this.project_id}`)
       })
     })
@@ -303,7 +332,7 @@ describe('DocManager', function () {
       beforeEach(function () {
         this.lines = ['mock', 'doc', 'lines']
         this.rev = 77
-        this.MongoManager.promises.findDoc.resolves({
+        this.MongoManager.findDoc.resolves({
           _id: new ObjectId(this.doc_id),
         })
         this.meta = {}
@@ -311,7 +340,7 @@ describe('DocManager', function () {
 
       describe('standard path', function () {
         beforeEach(async function () {
-          await this.DocManager.promises.patchDoc(
+          await this.DocManager.patchDoc(
             this.project_id,
             this.doc_id,
             this.meta
@@ -319,14 +348,14 @@ describe('DocManager', function () {
         })
 
         it('should get the doc', function () {
-          expect(this.MongoManager.promises.findDoc).to.have.been.calledWith(
+          expect(this.MongoManager.findDoc).to.have.been.calledWith(
             this.project_id,
             this.doc_id
           )
         })
 
         it('should persist the meta', function () {
-          expect(this.MongoManager.promises.patchDoc).to.have.been.calledWith(
+          expect(this.MongoManager.patchDoc).to.have.been.calledWith(
             this.project_id,
             this.doc_id,
             this.meta
@@ -339,7 +368,7 @@ describe('DocManager', function () {
           this.settings.docstore.archiveOnSoftDelete = false
           this.meta.deleted = true
 
-          await this.DocManager.promises.patchDoc(
+          await this.DocManager.patchDoc(
             this.project_id,
             this.doc_id,
             this.meta
@@ -347,8 +376,7 @@ describe('DocManager', function () {
         })
 
         it('should not flush the doc out of mongo', function () {
-          expect(this.DocArchiveManager.promises.archiveDoc).to.not.have.been
+          expect(this.DocArchiveManager.archiveDoc).to.not.have.been.called
-            .called
         })
       })
 
@@ -356,7 +384,7 @@ describe('DocManager', function () {
         beforeEach(async function () {
           this.settings.docstore.archiveOnSoftDelete = false
           this.meta.deleted = false
-          await this.DocManager.promises.patchDoc(
+          await this.DocManager.patchDoc(
             this.project_id,
             this.doc_id,
             this.meta
@@ -364,8 +392,7 @@ describe('DocManager', function () {
         })
 
         it('should not flush the doc out of mongo', function () {
-          expect(this.DocArchiveManager.promises.archiveDoc).to.not.have.been
+          expect(this.DocArchiveManager.archiveDoc).to.not.have.been.called
-            .called
         })
       })
 
@@ -377,7 +404,7 @@ describe('DocManager', function () {
 
         describe('when the background flush succeeds', function () {
           beforeEach(async function () {
-            await this.DocManager.promises.patchDoc(
+            await this.DocManager.patchDoc(
               this.project_id,
               this.doc_id,
               this.meta
@@ -389,17 +416,18 @@ describe('DocManager', function () {
           })
 
           it('should flush the doc out of mongo', function () {
-            expect(
+            expect(this.DocArchiveManager.archiveDoc).to.have.been.calledWith(
-              this.DocArchiveManager.promises.archiveDoc
+              this.project_id,
-            ).to.have.been.calledWith(this.project_id, this.doc_id)
+              this.doc_id
+            )
           })
         })
 
         describe('when the background flush fails', function () {
           beforeEach(async function () {
             this.err = new Error('foo')
-            this.DocArchiveManager.promises.archiveDoc.rejects(this.err)
+            this.DocArchiveManager.archiveDoc.rejects(this.err)
-            await this.DocManager.promises.patchDoc(
+            await this.DocManager.patchDoc(
               this.project_id,
               this.doc_id,
               this.meta
@@ -422,9 +450,9 @@ describe('DocManager', function () {
 
     describe('when the doc does not exist', function () {
       it('should return a NotFoundError', async function () {
-        this.MongoManager.promises.findDoc.resolves(null)
+        this.MongoManager.findDoc.resolves(null)
         await expect(
-          this.DocManager.promises.patchDoc(this.project_id, this.doc_id, {})
+          this.DocManager.patchDoc(this.project_id, this.doc_id, {})
         ).to.be.rejectedWith(
           `No such project/doc to delete: ${this.project_id}/${this.doc_id}`
         )
@@ -470,13 +498,13 @@ describe('DocManager', function () {
         ranges: this.originalRanges,
       }
 
-      this.DocManager.promises._getDoc = sinon.stub()
+      this.DocManager._getDoc = sinon.stub()
     })
 
     describe('when only the doc lines have changed', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.newDocLines,
@@ -486,7 +514,7 @@ describe('DocManager', function () {
       })
 
       it('should get the existing doc', function () {
-        this.DocManager.promises._getDoc
+        this.DocManager._getDoc
           .calledWith(this.project_id, this.doc_id, {
             version: true,
             rev: true,
@@ -498,7 +526,7 @@ describe('DocManager', function () {
       })
 
       it('should upsert the document to the doc collection', function () {
-        this.MongoManager.promises.upsertIntoDocCollection
+        this.MongoManager.upsertIntoDocCollection
           .calledWith(this.project_id, this.doc_id, this.rev, {
             lines: this.newDocLines,
           })
@@ -512,9 +540,9 @@ describe('DocManager', function () {
 
     describe('when the doc ranges have changed', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
         this.RangeManager.shouldUpdateRanges.returns(true)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.oldDocLines,
@@ -524,7 +552,7 @@ describe('DocManager', function () {
       })
 
       it('should upsert the ranges', function () {
-        this.MongoManager.promises.upsertIntoDocCollection
+        this.MongoManager.upsertIntoDocCollection
           .calledWith(this.project_id, this.doc_id, this.rev, {
             ranges: this.newRanges,
           })
@@ -538,8 +566,8 @@ describe('DocManager', function () {
 
     describe('when only the version has changed', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.oldDocLines,
@@ -549,7 +577,7 @@ describe('DocManager', function () {
       })
 
       it('should update the version', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.should.have.been.calledWith(
+        this.MongoManager.upsertIntoDocCollection.should.have.been.calledWith(
           this.project_id,
           this.doc_id,
           this.rev,
@@ -564,8 +592,8 @@ describe('DocManager', function () {
 
     describe('when the doc has not changed at all', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.oldDocLines,
@@ -575,9 +603,7 @@ describe('DocManager', function () {
       })
 
       it('should not update the ranges or lines or version', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.called.should.equal(
+        this.MongoManager.upsertIntoDocCollection.called.should.equal(false)
-          false
-        )
       })
 
       it('should return the old rev and modified == false', function () {
@@ -588,7 +614,7 @@ describe('DocManager', function () {
     describe('when the version is null', function () {
       it('should return an error', async function () {
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             this.newDocLines,
@@ -602,7 +628,7 @@ describe('DocManager', function () {
     describe('when the lines are null', function () {
       it('should return an error', async function () {
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             null,
@@ -616,7 +642,7 @@ describe('DocManager', function () {
     describe('when the ranges are null', function () {
       it('should return an error', async function () {
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             this.newDocLines,
@@ -630,9 +656,9 @@ describe('DocManager', function () {
     describe('when there is a generic error getting the doc', function () {
       beforeEach(async function () {
         this.error = new Error('doc could not be found')
-        this.DocManager.promises._getDoc = sinon.stub().rejects(this.error)
+        this.DocManager._getDoc = sinon.stub().rejects(this.error)
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             this.newDocLines,
@@ -643,16 +669,15 @@ describe('DocManager', function () {
       })
 
       it('should not upsert the document to the doc collection', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.should.not.have.been
+        this.MongoManager.upsertIntoDocCollection.should.not.have.been.called
-          .called
       })
     })
 
     describe('when the version was decremented', function () {
       it('should return an error', async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             this.newDocLines,
@@ -665,8 +690,8 @@ describe('DocManager', function () {
 
     describe('when the doc lines have not changed', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.oldDocLines.slice(),
@@ -676,9 +701,7 @@ describe('DocManager', function () {
       })
 
       it('should not update the doc', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.called.should.equal(
+        this.MongoManager.upsertIntoDocCollection.called.should.equal(false)
-          false
-        )
       })
 
       it('should return the existing rev', function () {
@@ -688,8 +711,8 @@ describe('DocManager', function () {
 
     describe('when the doc does not exist', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(null)
+        this.DocManager._getDoc = sinon.stub().resolves(null)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.newDocLines,
@@ -699,7 +722,7 @@ describe('DocManager', function () {
       })
 
       it('should upsert the document to the doc collection', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.should.have.been.calledWith(
+        this.MongoManager.upsertIntoDocCollection.should.have.been.calledWith(
           this.project_id,
           this.doc_id,
           undefined,
@@ -718,12 +741,12 @@ describe('DocManager', function () {
 
     describe('when another update is racing', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
-        this.MongoManager.promises.upsertIntoDocCollection
+        this.MongoManager.upsertIntoDocCollection
           .onFirstCall()
           .rejects(new Errors.DocRevValueError())
         this.RangeManager.shouldUpdateRanges.returns(true)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.newDocLines,
@@ -733,7 +756,7 @@ describe('DocManager', function () {
       })
 
       it('should upsert the doc twice', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.should.have.been.calledWith(
+        this.MongoManager.upsertIntoDocCollection.should.have.been.calledWith(
           this.project_id,
           this.doc_id,
           this.rev,
@@ -743,8 +766,7 @@ describe('DocManager', function () {
             version: this.version + 1,
           }
         )
-        this.MongoManager.promises.upsertIntoDocCollection.should.have.been
+        this.MongoManager.upsertIntoDocCollection.should.have.been.calledTwice
-          .calledTwice
       })
 
       it('should return the new rev', function () {

services/docstore/test/unit/js/HttpControllerTests.js

@@ -14,7 +14,7 @@ describe('HttpController', function () {
       max_doc_length: 2 * 1024 * 1024,
     }
     this.DocArchiveManager = {
-      unArchiveAllDocs: sinon.stub().yields(),
+      unArchiveAllDocs: sinon.stub().returns(),
     }
     this.DocManager = {}
     this.HttpController = SandboxedModule.require(modulePath, {
@@ -54,15 +54,13 @@ describe('HttpController', function () {
 
   describe('getDoc', function () {
     describe('without deleted docs', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.params = {
           project_id: this.projectId,
           doc_id: this.docId,
         }
-        this.DocManager.getFullDoc = sinon
+        this.DocManager.getFullDoc = sinon.stub().resolves(this.doc)
-          .stub()
+        await this.HttpController.getDoc(this.req, this.res, this.next)
-          .callsArgWith(2, null, this.doc)
-        this.HttpController.getDoc(this.req, this.res, this.next)
       })
 
       it('should get the document with the version (including deleted)', function () {
@@ -89,26 +87,24 @@ describe('HttpController', function () {
           project_id: this.projectId,
           doc_id: this.docId,
         }
-        this.DocManager.getFullDoc = sinon
+        this.DocManager.getFullDoc = sinon.stub().resolves(this.deletedDoc)
-          .stub()
-          .callsArgWith(2, null, this.deletedDoc)
       })
 
-      it('should get the doc from the doc manager', function () {
+      it('should get the doc from the doc manager', async function () {
-        this.HttpController.getDoc(this.req, this.res, this.next)
+        await this.HttpController.getDoc(this.req, this.res, this.next)
         this.DocManager.getFullDoc
           .calledWith(this.projectId, this.docId)
           .should.equal(true)
       })
 
-      it('should return 404 if the query string delete is not set ', function () {
+      it('should return 404 if the query string delete is not set ', async function () {
-        this.HttpController.getDoc(this.req, this.res, this.next)
+        await this.HttpController.getDoc(this.req, this.res, this.next)
         this.res.sendStatus.calledWith(404).should.equal(true)
       })
 
-      it('should return the doc as JSON if include_deleted is set to true', function () {
+      it('should return the doc as JSON if include_deleted is set to true', async function () {
         this.req.query.include_deleted = 'true'
-        this.HttpController.getDoc(this.req, this.res, this.next)
+        await this.HttpController.getDoc(this.req, this.res, this.next)
         this.res.json
           .calledWith({
             _id: this.docId,
@@ -123,13 +119,15 @@ describe('HttpController', function () {
   })
 
   describe('getRawDoc', function () {
-    beforeEach(function () {
+    beforeEach(async function () {
       this.req.params = {
         project_id: this.projectId,
         doc_id: this.docId,
       }
-      this.DocManager.getDocLines = sinon.stub().callsArgWith(2, null, this.doc)
+      this.DocManager.getDocLines = sinon
-      this.HttpController.getRawDoc(this.req, this.res, this.next)
+        .stub()
+        .resolves(this.doc.lines.join('\n'))
+      await this.HttpController.getRawDoc(this.req, this.res, this.next)
     })
 
     it('should get the document without the version', function () {
@@ -154,7 +152,7 @@ describe('HttpController', function () {
 
   describe('getAllDocs', function () {
     describe('normally', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.params = { project_id: this.projectId }
         this.docs = [
           {
@@ -168,10 +166,8 @@ describe('HttpController', function () {
             rev: 4,
           },
         ]
-        this.DocManager.getAllNonDeletedDocs = sinon
+        this.DocManager.getAllNonDeletedDocs = sinon.stub().resolves(this.docs)
-          .stub()
+        await this.HttpController.getAllDocs(this.req, this.res, this.next)
-          .callsArgWith(2, null, this.docs)
-        this.HttpController.getAllDocs(this.req, this.res, this.next)
       })
 
       it('should get all the (non-deleted) docs', function () {
@@ -199,7 +195,7 @@ describe('HttpController', function () {
     })
 
     describe('with null lines', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.params = { project_id: this.projectId }
         this.docs = [
           {
@@ -213,10 +209,8 @@ describe('HttpController', function () {
             rev: 4,
           },
         ]
-        this.DocManager.getAllNonDeletedDocs = sinon
+        this.DocManager.getAllNonDeletedDocs = sinon.stub().resolves(this.docs)
-          .stub()
+        await this.HttpController.getAllDocs(this.req, this.res, this.next)
-          .callsArgWith(2, null, this.docs)
-        this.HttpController.getAllDocs(this.req, this.res, this.next)
       })
 
       it('should return the doc with fallback lines', function () {
@@ -238,7 +232,7 @@ describe('HttpController', function () {
     })
 
     describe('with a null doc', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.params = { project_id: this.projectId }
         this.docs = [
           {
@@ -253,10 +247,8 @@ describe('HttpController', function () {
             rev: 4,
           },
         ]
-        this.DocManager.getAllNonDeletedDocs = sinon
+        this.DocManager.getAllNonDeletedDocs = sinon.stub().resolves(this.docs)
-          .stub()
+        await this.HttpController.getAllDocs(this.req, this.res, this.next)
-          .callsArgWith(2, null, this.docs)
-        this.HttpController.getAllDocs(this.req, this.res, this.next)
       })
 
       it('should return the non null docs as JSON', function () {
@@ -292,7 +284,7 @@ describe('HttpController', function () {
 
   describe('getAllRanges', function () {
     describe('normally', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.params = { project_id: this.projectId }
         this.docs = [
           {
@@ -304,10 +296,8 @@ describe('HttpController', function () {
             ranges: { mock_ranges: 'two' },
           },
         ]
-        this.DocManager.getAllNonDeletedDocs = sinon
+        this.DocManager.getAllNonDeletedDocs = sinon.stub().resolves(this.docs)
-          .stub()
+        await this.HttpController.getAllRanges(this.req, this.res, this.next)
-          .callsArgWith(2, null, this.docs)
-        this.HttpController.getAllRanges(this.req, this.res, this.next)
       })
 
       it('should get all the (non-deleted) doc ranges', function () {
@@ -342,16 +332,17 @@ describe('HttpController', function () {
     })
 
     describe('when the doc lines exist and were updated', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.body = {
           lines: (this.lines = ['hello', 'world']),
           version: (this.version = 42),
           ranges: (this.ranges = { changes: 'mock' }),
         }
+        this.rev = 5
         this.DocManager.updateDoc = sinon
           .stub()
-          .yields(null, true, (this.rev = 5))
+          .resolves({ modified: true, rev: this.rev })
-        this.HttpController.updateDoc(this.req, this.res, this.next)
+        await this.HttpController.updateDoc(this.req, this.res, this.next)
       })
 
       it('should update the document', function () {
@@ -374,16 +365,17 @@ describe('HttpController', function () {
     })
 
     describe('when the doc lines exist and were not updated', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.body = {
           lines: (this.lines = ['hello', 'world']),
           version: (this.version = 42),
           ranges: {},
         }
+        this.rev = 5
         this.DocManager.updateDoc = sinon
           .stub()
-          .yields(null, false, (this.rev = 5))
+          .resolves({ modified: false, rev: this.rev })
-        this.HttpController.updateDoc(this.req, this.res, this.next)
+        await this.HttpController.updateDoc(this.req, this.res, this.next)
       })
 
       it('should return a modified status', function () {
@@ -394,10 +386,12 @@ describe('HttpController', function () {
     })
 
     describe('when the doc lines are not provided', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.body = { version: 42, ranges: {} }
-        this.DocManager.updateDoc = sinon.stub().yields(null, false)
+        this.DocManager.updateDoc = sinon
-        this.HttpController.updateDoc(this.req, this.res, this.next)
+          .stub()
+          .resolves({ modified: false, rev: 0 })
+        await this.HttpController.updateDoc(this.req, this.res, this.next)
       })
 
       it('should not update the document', function () {
@@ -410,10 +404,12 @@ describe('HttpController', function () {
     })
 
     describe('when the doc version are not provided', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.body = { version: 42, lines: ['hello world'] }
-        this.DocManager.updateDoc = sinon.stub().yields(null, false)
+        this.DocManager.updateDoc = sinon
-        this.HttpController.updateDoc(this.req, this.res, this.next)
+          .stub()
+          .resolves({ modified: false, rev: 0 })
+        await this.HttpController.updateDoc(this.req, this.res, this.next)
       })
 
       it('should not update the document', function () {
@@ -426,10 +422,12 @@ describe('HttpController', function () {
     })
 
     describe('when the doc ranges is not provided', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.body = { lines: ['foo'], version: 42 }
-        this.DocManager.updateDoc = sinon.stub().yields(null, false)
+        this.DocManager.updateDoc = sinon
-        this.HttpController.updateDoc(this.req, this.res, this.next)
+          .stub()
+          .resolves({ modified: false, rev: 0 })
+        await this.HttpController.updateDoc(this.req, this.res, this.next)
       })
 
       it('should not update the document', function () {
@@ -442,13 +440,20 @@ describe('HttpController', function () {
     })
 
     describe('when the doc body is too large', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.body = {
           lines: (this.lines = Array(2049).fill('a'.repeat(1024))),
           version: (this.version = 42),
           ranges: (this.ranges = { changes: 'mock' }),
         }
-        this.HttpController.updateDoc(this.req, this.res, this.next)
+        this.DocManager.updateDoc = sinon
+          .stub()
+          .resolves({ modified: false, rev: 0 })
+        await this.HttpController.updateDoc(this.req, this.res, this.next)
+      })
+
+      it('should not update the document', function () {
+        this.DocManager.updateDoc.called.should.equal(false)
       })
 
       it('should return a 413 (too large) response', function () {
@@ -462,14 +467,14 @@ describe('HttpController', function () {
   })
 
   describe('patchDoc', function () {
-    beforeEach(function () {
+    beforeEach(async function () {
       this.req.params = {
         project_id: this.projectId,
         doc_id: this.docId,
       }
       this.req.body = { name: 'foo.tex' }
-      this.DocManager.patchDoc = sinon.stub().yields(null)
+      this.DocManager.patchDoc = sinon.stub().resolves()
-      this.HttpController.patchDoc(this.req, this.res, this.next)
+      await this.HttpController.patchDoc(this.req, this.res, this.next)
     })
 
     it('should delete the document', function () {
@@ -484,11 +489,11 @@ describe('HttpController', function () {
     })
 
     describe('with an invalid payload', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.req.body = { cannot: 'happen' }
 
-        this.DocManager.patchDoc = sinon.stub().yields(null)
+        this.DocManager.patchDoc = sinon.stub().resolves()
-        this.HttpController.patchDoc(this.req, this.res, this.next)
+        await this.HttpController.patchDoc(this.req, this.res, this.next)
       })
 
       it('should log a message', function () {
@@ -509,10 +514,10 @@ describe('HttpController', function () {
   })
 
   describe('archiveAllDocs', function () {
-    beforeEach(function () {
+    beforeEach(async function () {
       this.req.params = { project_id: this.projectId }
-      this.DocArchiveManager.archiveAllDocs = sinon.stub().callsArg(1)
+      this.DocArchiveManager.archiveAllDocs = sinon.stub().resolves()
-      this.HttpController.archiveAllDocs(this.req, this.res, this.next)
+      await this.HttpController.archiveAllDocs(this.req, this.res, this.next)
     })
 
     it('should archive the project', function () {
@@ -532,9 +537,12 @@ describe('HttpController', function () {
     })
 
     describe('on success', function () {
-      beforeEach(function (done) {
+      beforeEach(async function () {
-        this.res.sendStatus.callsFake(() => done())
+        await this.HttpController.unArchiveAllDocs(
-        this.HttpController.unArchiveAllDocs(this.req, this.res, this.next)
+          this.req,
+          this.res,
+          this.next
+        )
       })
 
       it('returns a 200', function () {
@@ -543,12 +551,15 @@ describe('HttpController', function () {
     })
 
     describe("when the archived rev doesn't match", function () {
-      beforeEach(function (done) {
+      beforeEach(async function () {
-        this.res.sendStatus.callsFake(() => done())
+        this.DocArchiveManager.unArchiveAllDocs.rejects(
-        this.DocArchiveManager.unArchiveAllDocs.yields(
           new Errors.DocRevValueError('bad rev')
         )
-        this.HttpController.unArchiveAllDocs(this.req, this.res, this.next)
+        await this.HttpController.unArchiveAllDocs(
+          this.req,
+          this.res,
+          this.next
+        )
       })
 
       it('returns a 409', function () {
@@ -558,10 +569,10 @@ describe('HttpController', function () {
   })
 
   describe('destroyProject', function () {
-    beforeEach(function () {
+    beforeEach(async function () {
       this.req.params = { project_id: this.projectId }
-      this.DocArchiveManager.destroyProject = sinon.stub().callsArg(1)
+      this.DocArchiveManager.destroyProject = sinon.stub().resolves()
-      this.HttpController.destroyProject(this.req, this.res, this.next)
+      await this.HttpController.destroyProject(this.req, this.res, this.next)
     })
 
     it('should destroy the docs', function () {

services/docstore/test/unit/js/MongoManagerTests.js

@@ -41,7 +41,7 @@ describe('MongoManager', function () {
       this.doc = { name: 'mock-doc' }
       this.db.docs.findOne = sinon.stub().resolves(this.doc)
       this.filter = { lines: true }
-      this.result = await this.MongoManager.promises.findDoc(
+      this.result = await this.MongoManager.findDoc(
         this.projectId,
         this.docId,
         this.filter
@@ -70,11 +70,7 @@ describe('MongoManager', function () {
   describe('patchDoc', function () {
     beforeEach(async function () {
       this.meta = { name: 'foo.tex' }
-      await this.MongoManager.promises.patchDoc(
+      await this.MongoManager.patchDoc(this.projectId, this.docId, this.meta)
-        this.projectId,
-        this.docId,
-        this.meta
-      )
     })
 
     it('should pass the parameter along', function () {
@@ -104,7 +100,7 @@ describe('MongoManager', function () {
 
     describe('with included_deleted = false', function () {
       beforeEach(async function () {
-        this.result = await this.MongoManager.promises.getProjectsDocs(
+        this.result = await this.MongoManager.getProjectsDocs(
           this.projectId,
           { include_deleted: false },
           this.filter
@@ -132,7 +128,7 @@ describe('MongoManager', function () {
 
     describe('with included_deleted = true', function () {
       beforeEach(async function () {
-        this.result = await this.MongoManager.promises.getProjectsDocs(
+        this.result = await this.MongoManager.getProjectsDocs(
           this.projectId,
           { include_deleted: true },
           this.filter
@@ -167,7 +163,7 @@ describe('MongoManager', function () {
       this.db.docs.find = sinon.stub().returns({
         toArray: sinon.stub().resolves([this.doc1, this.doc2, this.doc3]),
       })
-      this.result = await this.MongoManager.promises.getProjectsDeletedDocs(
+      this.result = await this.MongoManager.getProjectsDeletedDocs(
         this.projectId,
         this.filter
       )
@@ -203,7 +199,7 @@ describe('MongoManager', function () {
     })
 
     it('should upsert the document', async function () {
-      await this.MongoManager.promises.upsertIntoDocCollection(
+      await this.MongoManager.upsertIntoDocCollection(
         this.projectId,
         this.docId,
         this.oldRev,
@@ -223,7 +219,7 @@ describe('MongoManager', function () {
     it('should handle update error', async function () {
       this.db.docs.updateOne.rejects(this.stubbedErr)
       await expect(
-        this.MongoManager.promises.upsertIntoDocCollection(
+        this.MongoManager.upsertIntoDocCollection(
           this.projectId,
           this.docId,
           this.rev,
@@ -235,7 +231,7 @@ describe('MongoManager', function () {
     })
 
     it('should insert without a previous rev', async function () {
-      await this.MongoManager.promises.upsertIntoDocCollection(
+      await this.MongoManager.upsertIntoDocCollection(
         this.projectId,
         this.docId,
         null,
@@ -254,7 +250,7 @@ describe('MongoManager', function () {
     it('should handle generic insert error', async function () {
       this.db.docs.insertOne.rejects(this.stubbedErr)
       await expect(
-        this.MongoManager.promises.upsertIntoDocCollection(
+        this.MongoManager.upsertIntoDocCollection(
           this.projectId,
           this.docId,
           null,
@@ -266,7 +262,7 @@ describe('MongoManager', function () {
     it('should handle duplicate insert error', async function () {
       this.db.docs.insertOne.rejects({ code: 11000 })
       await expect(
-        this.MongoManager.promises.upsertIntoDocCollection(
+        this.MongoManager.upsertIntoDocCollection(
           this.projectId,
           this.docId,
           null,
@@ -280,7 +276,7 @@ describe('MongoManager', function () {
     beforeEach(async function () {
       this.projectId = new ObjectId()
       this.db.docs.deleteMany = sinon.stub().resolves()
-      await this.MongoManager.promises.destroyProject(this.projectId)
+      await this.MongoManager.destroyProject(this.projectId)
     })
 
     it('should destroy all docs', function () {
@@ -297,13 +293,13 @@ describe('MongoManager', function () {
 
     it('should not error when the rev has not changed', async function () {
       this.db.docs.findOne = sinon.stub().resolves({ rev: 1 })
-      await this.MongoManager.promises.checkRevUnchanged(this.doc)
+      await this.MongoManager.checkRevUnchanged(this.doc)
     })
 
     it('should return an error when the rev has changed', async function () {
       this.db.docs.findOne = sinon.stub().resolves({ rev: 2 })
       await expect(
-        this.MongoManager.promises.checkRevUnchanged(this.doc)
+        this.MongoManager.checkRevUnchanged(this.doc)
       ).to.be.rejectedWith(Errors.DocModifiedError)
     })
 
@@ -311,14 +307,14 @@ describe('MongoManager', function () {
       this.db.docs.findOne = sinon.stub().resolves({ rev: 2 })
       this.doc = { _id: new ObjectId(), name: 'mock-doc', rev: NaN }
       await expect(
-        this.MongoManager.promises.checkRevUnchanged(this.doc)
+        this.MongoManager.checkRevUnchanged(this.doc)
       ).to.be.rejectedWith(Errors.DocRevValueError)
     })
 
     it('should return a value error if checked doc rev is NaN', async function () {
       this.db.docs.findOne = sinon.stub().resolves({ rev: NaN })
       await expect(
-        this.MongoManager.promises.checkRevUnchanged(this.doc)
+        this.MongoManager.checkRevUnchanged(this.doc)
       ).to.be.rejectedWith(Errors.DocRevValueError)
     })
   })
@@ -334,7 +330,7 @@ describe('MongoManager', function () {
 
     describe('complete doc', function () {
       beforeEach(async function () {
-        await this.MongoManager.promises.restoreArchivedDoc(
+        await this.MongoManager.restoreArchivedDoc(
           this.projectId,
           this.docId,
           this.archivedDoc
@@ -364,7 +360,7 @@ describe('MongoManager', function () {
     describe('without ranges', function () {
       beforeEach(async function () {
         delete this.archivedDoc.ranges
-        await this.MongoManager.promises.restoreArchivedDoc(
+        await this.MongoManager.restoreArchivedDoc(
           this.projectId,
           this.docId,
           this.archivedDoc
@@ -395,7 +391,7 @@ describe('MongoManager', function () {
       it('throws a DocRevValueError', async function () {
         this.db.docs.updateOne.resolves({ matchedCount: 0 })
         await expect(
-          this.MongoManager.promises.restoreArchivedDoc(
+          this.MongoManager.restoreArchivedDoc(
             this.projectId,
             this.docId,
             this.archivedDoc

services/docstore/test/unit/js/RangeManagerTests.js

@@ -30,7 +30,7 @@ describe('RangeManager', function () {
   })
 
   describe('jsonRangesToMongo', function () {
-    it('should convert ObjectIds and dates to proper objects', function () {
+    it('should convert ObjectIds and dates to proper objects and fix comment id', function () {
       const changeId = new ObjectId().toString()
       const commentId = new ObjectId().toString()
       const userId = new ObjectId().toString()
@@ -66,7 +66,7 @@ describe('RangeManager', function () {
         ],
         comments: [
           {
-            id: new ObjectId(commentId),
+            id: new ObjectId(threadId),
             op: { c: 'foo', p: 3, t: new ObjectId(threadId) },
           },
         ],
@@ -110,7 +110,6 @@ describe('RangeManager', function () {
 
     return it('should be consistent when transformed through json -> mongo -> json', function () {
       const changeId = new ObjectId().toString()
-      const commentId = new ObjectId().toString()
       const userId = new ObjectId().toString()
       const threadId = new ObjectId().toString()
       const ts = new Date().toJSON()
@@ -127,7 +126,7 @@ describe('RangeManager', function () {
         ],
         comments: [
           {
-            id: commentId,
+            id: threadId,
             op: { c: 'foo', p: 3, t: threadId },
           },
         ],
@@ -142,6 +141,7 @@ describe('RangeManager', function () {
 
   return describe('shouldUpdateRanges', function () {
     beforeEach(function () {
+      const threadId = new ObjectId()
       this.ranges = {
         changes: [
           {
@@ -155,8 +155,8 @@ describe('RangeManager', function () {
         ],
         comments: [
           {
-            id: new ObjectId(),
+            id: threadId,
-            op: { c: 'foo', p: 3, t: new ObjectId() },
+            op: { c: 'foo', p: 3, t: threadId },
           },
         ],
       }

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

@@ -1,3 +1,4 @@
+const OError = require('@overleaf/o-error')
 const DMP = require('diff-match-patch')
 const { TextOperation } = require('overleaf-editor-core')
 const dmp = new DMP()
@@ -38,23 +39,62 @@ module.exports = {
     return ops
   },
 
-  diffAsHistoryV1EditOperation(before, after) {
+  /**
-    const diffs = dmp.diff_main(before, after)
+   * @param {import("overleaf-editor-core").StringFileData} file
+   * @param {string} after
+   * @return {TextOperation}
+   */
+  diffAsHistoryOTEditOperation(file, after) {
+    const beforeWithoutTrackedDeletes = file.getContent({
+      filterTrackedDeletes: true,
+    })
+    const diffs = dmp.diff_main(beforeWithoutTrackedDeletes, after)
     dmp.diff_cleanupSemantic(diffs)
 
+    const trackedChanges = file.trackedChanges.asSorted()
+    let nextTc = trackedChanges.shift()
+
     const op = new TextOperation()
     for (const diff of diffs) {
-      const [type, content] = diff
+      let [type, content] = diff
       if (type === this.ADDED) {
         op.insert(content)
-      } else if (type === this.REMOVED) {
+      } else if (type === this.REMOVED || type === this.UNCHANGED) {
-        op.remove(content.length)
+        while (op.baseLength + content.length > nextTc?.range.start) {
-      } else if (type === this.UNCHANGED) {
+          if (nextTc.tracking.type === 'delete') {
-        op.retain(content.length)
+            const untilRange = nextTc.range.start - op.baseLength
+            if (type === this.REMOVED) {
+              op.remove(untilRange)
+            } else if (type === this.UNCHANGED) {
+              op.retain(untilRange)
+            }
+            op.retain(nextTc.range.end - nextTc.range.start)
+            content = content.slice(untilRange)
+          }
+          nextTc = trackedChanges.shift()
+        }
+        if (type === this.REMOVED) {
+          op.remove(content.length)
+        } else if (type === this.UNCHANGED) {
+          op.retain(content.length)
+        }
       } else {
         throw new Error('Unknown type')
       }
     }
+    while (nextTc) {
+      if (
+        nextTc.tracking.type !== 'delete' ||
+        nextTc.range.start !== op.baseLength
+      ) {
+        throw new OError(
+          'StringFileData.trackedChanges out of sync: unexpected range after end of diff',
+          { nextTc, baseLength: op.baseLength }
+        )
+      }
+      op.retain(nextTc.range.end - nextTc.range.start)
+      nextTc = trackedChanges.shift()
+    }
     return op
   },
 }

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

@@ -194,9 +194,8 @@ const DocumentManager = {
     let op
     if (type === 'history-ot') {
       const file = StringFileData.fromRaw(oldLines)
-      const operation = DiffCodec.diffAsHistoryV1EditOperation(
+      const operation = DiffCodec.diffAsHistoryOTEditOperation(
-        // TODO(24596): tc support for history-ot
+        file,
-        file.getContent({ filterTrackedDeletes: true }),
         newLines.join('\n')
       )
       if (operation.isNoop()) {
@@ -536,11 +535,6 @@ const DocumentManager = {
     if (opts.historyRangesMigration) {
       historyRangesSupport = opts.historyRangesMigration === 'forwards'
     }
-    if (!Array.isArray(lines)) {
-      const file = StringFileData.fromRaw(lines)
-      // TODO(24596): tc support for history-ot
-      lines = file.getLines()
-    }
 
     await ProjectHistoryRedisManager.promises.queueResyncDocContent(
       projectId,

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

@@ -28,4 +28,19 @@ module.exports = {
     // since we didn't hit the limit in the loop, the document is within the allowed length
     return false
   },
+
+  /**
+   * @param {StringFileRawData} raw
+   * @param {number} maxDocLength
+   */
+  stringFileDataContentIsTooLarge(raw, maxDocLength) {
+    let n = raw.content.length
+    if (n <= maxDocLength) return false // definitely under the limit, no need to calculate the total size
+    for (const tc of raw.trackedChanges ?? []) {
+      if (tc.tracking.type !== 'delete') continue
+      n -= tc.range.length
+      if (n <= maxDocLength) return false // under the limit now, no need to calculate the exact size
+    }
+    return true
+  },
 }

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

@@ -8,13 +8,14 @@ const rclient = require('@overleaf/redis-wrapper').createClient(
 )
 const logger = require('@overleaf/logger')
 const metrics = require('./Metrics')
-const { docIsTooLarge } = require('./Limits')
+const { docIsTooLarge, stringFileDataContentIsTooLarge } = require('./Limits')
 const { addTrackedDeletesToContent, extractOriginOrSource } = require('./Utils')
 const HistoryConversions = require('./HistoryConversions')
 const OError = require('@overleaf/o-error')
 
 /**
  * @import { Ranges } from './types'
+ * @import { StringFileRawData } from 'overleaf-editor-core/lib/types'
  */
 
 const ProjectHistoryRedisManager = {
@@ -180,7 +181,7 @@ const ProjectHistoryRedisManager = {
    * @param {string} projectId
    * @param {string} projectHistoryId
    * @param {string} docId
-   * @param {string[]} lines
+   * @param {string[] | StringFileRawData} lines
    * @param {Ranges} ranges
    * @param {string[]} resolvedCommentIds
    * @param {number} version
@@ -204,13 +205,8 @@ const ProjectHistoryRedisManager = {
       'queue doc content resync'
     )
 
-    let content = lines.join('\n')
-    if (historyRangesSupport) {
-      content = addTrackedDeletesToContent(content, ranges.changes ?? [])
-    }
-
     const projectUpdate = {
-      resyncDocContent: { content, version },
+      resyncDocContent: { version },
       projectHistoryId,
       path: pathname,
       doc: docId,
@@ -219,17 +215,38 @@ const ProjectHistoryRedisManager = {
       },
     }
 
-    if (historyRangesSupport) {
+    let content = ''
-      projectUpdate.resyncDocContent.ranges =
+    if (Array.isArray(lines)) {
-        HistoryConversions.toHistoryRanges(ranges)
+      content = lines.join('\n')
-      projectUpdate.resyncDocContent.resolvedCommentIds = resolvedCommentIds
+      if (historyRangesSupport) {
+        content = addTrackedDeletesToContent(content, ranges.changes ?? [])
+        projectUpdate.resyncDocContent.ranges =
+          HistoryConversions.toHistoryRanges(ranges)
+        projectUpdate.resyncDocContent.resolvedCommentIds = resolvedCommentIds
+      }
+    } else {
+      content = lines.content
+      projectUpdate.resyncDocContent.historyOTRanges = {
+        comments: lines.comments,
+        trackedChanges: lines.trackedChanges,
+      }
     }
+    projectUpdate.resyncDocContent.content = content
 
     const jsonUpdate = JSON.stringify(projectUpdate)
     // Do an optimised size check on the docLines using the serialised
     // project update length as an upper bound
     const sizeBound = jsonUpdate.length
-    if (docIsTooLarge(sizeBound, lines, Settings.max_doc_length)) {
+    if (Array.isArray(lines)) {
+      if (docIsTooLarge(sizeBound, lines, Settings.max_doc_length)) {
+        throw new OError(
+          'blocking resync doc content insert into project history queue: doc is too large',
+          { projectId, docId, docSize: sizeBound }
+        )
+      }
+    } else if (
+      stringFileDataContentIsTooLarge(lines, Settings.max_doc_length)
+    ) {
       throw new OError(
         'blocking resync doc content insert into project history queue: doc is too large',
         { projectId, docId, docSize: sizeBound }

services/document-updater/docker-compose.ci.yml

@@ -28,12 +28,15 @@ services:
       MOCHA_GREP: ${MOCHA_GREP}
       NODE_ENV: test
       NODE_OPTIONS: "--unhandled-rejections=strict"
+    volumes:
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     depends_on:
       mongo:
         condition: service_started
       redis:
         condition: service_healthy
     user: node
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run test:acceptance
 
 
@@ -45,7 +48,7 @@ services:
     command: tar -czf /tmp/build/build.tar.gz --exclude=build.tar.gz --exclude-vcs .
     user: root
   redis:
-    image: redis
+    image: redis:7.4.3
     healthcheck:
       test: ping="$$(redis-cli ping)" && [ "$$ping" = 'PONG' ]
       interval: 1s

services/document-updater/docker-compose.yml

@@ -26,6 +26,7 @@ services:
       - .:/overleaf/services/document-updater
       - ../../node_modules:/overleaf/node_modules
       - ../../libraries:/overleaf/libraries
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     working_dir: /overleaf/services/document-updater
     environment:
       ELASTIC_SEARCH_DSN: es:9200
@@ -45,10 +46,11 @@ services:
         condition: service_started
       redis:
         condition: service_healthy
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run --silent test:acceptance
 
   redis:
-    image: redis
+    image: redis:7.4.3
     healthcheck:
       test: ping=$$(redis-cli ping) && [ "$$ping" = 'PONG' ]
       interval: 1s

services/document-updater/scripts/check_redis_mongo_sync_state.js

@@ -15,6 +15,7 @@ const request = require('requestretry').defaults({
   retryDelay: 10,
 })
 
+const ONLY_PROJECT_ID = process.env.ONLY_PROJECT_ID
 const AUTO_FIX_VERSION_MISMATCH =
   process.env.AUTO_FIX_VERSION_MISMATCH === 'true'
 const AUTO_FIX_PARTIALLY_DELETED_DOC_METADATA =
@@ -319,10 +320,12 @@ async function processProject(projectId) {
  * @return {Promise<{perIterationOutOfSync: number, done: boolean}>}
  */
 async function scanOnce(processed, outOfSync) {
-  const projectIds = await ProjectFlusher.promises.flushAllProjects({
+  const projectIds = ONLY_PROJECT_ID
-    limit: LIMIT,
+    ? [ONLY_PROJECT_ID]
-    dryRun: true,
+    : await ProjectFlusher.promises.flushAllProjects({
-  })
+        limit: LIMIT,
+        dryRun: true,
+      })
 
   let perIterationOutOfSync = 0
   for (const projectId of projectIds) {

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

@@ -686,4 +686,285 @@ describe('Setting a document', function () {
       })
     })
   })
+
+  describe('with track changes (history-ot)', function () {
+    const lines = ['one', 'one and a half', 'two', 'three']
+    const userId = DocUpdaterClient.randomId()
+    const ts = new Date().toISOString()
+    beforeEach(function (done) {
+      numberOfReceivedUpdates = 0
+      this.newLines = ['one', 'two', 'three']
+      this.project_id = DocUpdaterClient.randomId()
+      this.doc_id = DocUpdaterClient.randomId()
+      this.historyOTUpdate = {
+        doc: this.doc_id,
+        op: [
+          {
+            textOperation: [
+              4,
+              {
+                r: 'one and a half\n'.length,
+                tracking: {
+                  type: 'delete',
+                  userId,
+                  ts,
+                },
+              },
+              9,
+            ],
+          },
+        ],
+        v: this.version,
+        meta: { source: 'random-publicId' },
+      }
+      MockWebApi.insertDoc(this.project_id, this.doc_id, {
+        lines,
+        version: this.version,
+        otMigrationStage: 1,
+      })
+      DocUpdaterClient.preloadDoc(this.project_id, this.doc_id, error => {
+        if (error) {
+          throw error
+        }
+        DocUpdaterClient.sendUpdate(
+          this.project_id,
+          this.doc_id,
+          this.historyOTUpdate,
+          error => {
+            if (error) {
+              throw error
+            }
+            DocUpdaterClient.waitForPendingUpdates(
+              this.project_id,
+              this.doc_id,
+              done
+            )
+          }
+        )
+      })
+    })
+
+    afterEach(function () {
+      MockProjectHistoryApi.flushProject.resetHistory()
+      MockWebApi.setDocument.resetHistory()
+    })
+    it('should record tracked changes', function (done) {
+      docUpdaterRedis.get(
+        Keys.docLines({ doc_id: this.doc_id }),
+        (error, data) => {
+          if (error) {
+            throw error
+          }
+          expect(JSON.parse(data)).to.deep.equal({
+            content: lines.join('\n'),
+            trackedChanges: [
+              {
+                range: {
+                  pos: 4,
+                  length: 15,
+                },
+                tracking: {
+                  ts,
+                  type: 'delete',
+                  userId,
+                },
+              },
+            ],
+          })
+          done()
+        }
+      )
+    })
+
+    it('should apply the change', function (done) {
+      DocUpdaterClient.getDoc(
+        this.project_id,
+        this.doc_id,
+        (error, res, data) => {
+          if (error) {
+            throw error
+          }
+          expect(data.lines).to.deep.equal(this.newLines)
+          done()
+        }
+      )
+    })
+    const cases = [
+      {
+        name: 'when resetting the content',
+        lines,
+        want: {
+          content: 'one\none and a half\none and a half\ntwo\nthree',
+          trackedChanges: [
+            {
+              range: {
+                pos: 'one and a half\n'.length + 4,
+                length: 15,
+              },
+              tracking: {
+                ts,
+                type: 'delete',
+                userId,
+              },
+            },
+          ],
+        },
+      },
+      {
+        name: 'when adding content before a tracked delete',
+        lines: ['one', 'INSERT', 'two', 'three'],
+        want: {
+          content: 'one\nINSERT\none and a half\ntwo\nthree',
+          trackedChanges: [
+            {
+              range: {
+                pos: 'INSERT\n'.length + 4,
+                length: 15,
+              },
+              tracking: {
+                ts,
+                type: 'delete',
+                userId,
+              },
+            },
+          ],
+        },
+      },
+      {
+        name: 'when adding content after a tracked delete',
+        lines: ['one', 'two', 'INSERT', 'three'],
+        want: {
+          content: 'one\none and a half\ntwo\nINSERT\nthree',
+          trackedChanges: [
+            {
+              range: {
+                pos: 4,
+                length: 15,
+              },
+              tracking: {
+                ts,
+                type: 'delete',
+                userId,
+              },
+            },
+          ],
+        },
+      },
+      {
+        name: 'when deleting content before a tracked delete',
+        lines: ['two', 'three'],
+        want: {
+          content: 'one and a half\ntwo\nthree',
+          trackedChanges: [
+            {
+              range: {
+                pos: 0,
+                length: 15,
+              },
+              tracking: {
+                ts,
+                type: 'delete',
+                userId,
+              },
+            },
+          ],
+        },
+      },
+      {
+        name: 'when deleting content after a tracked delete',
+        lines: ['one', 'two'],
+        want: {
+          content: 'one\none and a half\ntwo',
+          trackedChanges: [
+            {
+              range: {
+                pos: 4,
+                length: 15,
+              },
+              tracking: {
+                ts,
+                type: 'delete',
+                userId,
+              },
+            },
+          ],
+        },
+      },
+      {
+        name: 'when deleting content immediately after a tracked delete',
+        lines: ['one', 'three'],
+        want: {
+          content: 'one\none and a half\nthree',
+          trackedChanges: [
+            {
+              range: {
+                pos: 4,
+                length: 15,
+              },
+              tracking: {
+                ts,
+                type: 'delete',
+                userId,
+              },
+            },
+          ],
+        },
+      },
+      {
+        name: 'when deleting content across a tracked delete',
+        lines: ['onethree'],
+        want: {
+          content: 'oneone and a half\nthree',
+          trackedChanges: [
+            {
+              range: {
+                pos: 3,
+                length: 15,
+              },
+              tracking: {
+                ts,
+                type: 'delete',
+                userId,
+              },
+            },
+          ],
+        },
+      },
+    ]
+
+    for (const { name, lines, want } of cases) {
+      describe(name, function () {
+        beforeEach(function (done) {
+          DocUpdaterClient.setDocLines(
+            this.project_id,
+            this.doc_id,
+            lines,
+            this.source,
+            userId,
+            false,
+            (error, res, body) => {
+              if (error) {
+                return done(error)
+              }
+              this.statusCode = res.statusCode
+              this.body = body
+              done()
+            }
+          )
+        })
+        it('should update accordingly', function (done) {
+          docUpdaterRedis.get(
+            Keys.docLines({ doc_id: this.doc_id }),
+            (error, data) => {
+              if (error) {
+                throw error
+              }
+              expect(JSON.parse(data)).to.deep.equal(want)
+              done()
+            }
+          )
+        })
+      })
+    }
+  })
 })

services/document-updater/test/unit/js/Limits/LimitsTests.js

@@ -81,4 +81,88 @@ describe('Limits', function () {
       })
     })
   })
+
+  describe('stringFileDataContentIsTooLarge', function () {
+    it('should handle small docs', function () {
+      expect(
+        this.Limits.stringFileDataContentIsTooLarge({ content: '' }, 123)
+      ).to.equal(false)
+    })
+    it('should handle docs at the limit', function () {
+      expect(
+        this.Limits.stringFileDataContentIsTooLarge(
+          { content: 'x'.repeat(123) },
+          123
+        )
+      ).to.equal(false)
+    })
+    it('should handle docs above the limit', function () {
+      expect(
+        this.Limits.stringFileDataContentIsTooLarge(
+          { content: 'x'.repeat(123 + 1) },
+          123
+        )
+      ).to.equal(true)
+    })
+    it('should handle docs above the limit and below with tracked-deletes removed', function () {
+      expect(
+        this.Limits.stringFileDataContentIsTooLarge(
+          {
+            content: 'x'.repeat(123 + 1),
+            trackedChanges: [
+              {
+                range: { pos: 1, length: 1 },
+                tracking: {
+                  type: 'delete',
+                  ts: '2025-06-16T14:31:44.910Z',
+                  userId: 'user-id',
+                },
+              },
+            ],
+          },
+          123
+        )
+      ).to.equal(false)
+    })
+    it('should handle docs above the limit and above with tracked-deletes removed', function () {
+      expect(
+        this.Limits.stringFileDataContentIsTooLarge(
+          {
+            content: 'x'.repeat(123 + 2),
+            trackedChanges: [
+              {
+                range: { pos: 1, length: 1 },
+                tracking: {
+                  type: 'delete',
+                  ts: '2025-06-16T14:31:44.910Z',
+                  userId: 'user-id',
+                },
+              },
+            ],
+          },
+          123
+        )
+      ).to.equal(true)
+    })
+    it('should handle docs above the limit and with tracked-inserts', function () {
+      expect(
+        this.Limits.stringFileDataContentIsTooLarge(
+          {
+            content: 'x'.repeat(123 + 1),
+            trackedChanges: [
+              {
+                range: { pos: 1, length: 1 },
+                tracking: {
+                  type: 'insert',
+                  ts: '2025-06-16T14:31:44.910Z',
+                  userId: 'user-id',
+                },
+              },
+            ],
+          },
+          123
+        )
+      ).to.equal(true)
+    })
+  })
 })

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

@@ -15,6 +15,7 @@ describe('ProjectHistoryRedisManager', function () {
 
     this.Limits = {
       docIsTooLarge: sinon.stub().returns(false),
+      stringFileDataContentIsTooLarge: sinon.stub().returns(false),
     }
 
     this.ProjectHistoryRedisManager = SandboxedModule.require(modulePath, {
@@ -61,22 +62,18 @@ describe('ProjectHistoryRedisManager', function () {
     })
 
     it('should queue an update', function () {
-      this.multi.rpush
+      this.multi.rpush.should.have.been.calledWithExactly(
-        .calledWithExactly(
+        `ProjectHistory:Ops:${this.project_id}`,
-          `ProjectHistory:Ops:${this.project_id}`,
+        this.ops[0],
-          this.ops[0],
+        this.ops[1]
-          this.ops[1]
+      )
-        )
-        .should.equal(true)
     })
 
     it('should set the queue timestamp if not present', function () {
-      this.multi.setnx
+      this.multi.setnx.should.have.been.calledWithExactly(
-        .calledWithExactly(
+        `ProjectHistory:FirstOpTimestamp:${this.project_id}`,
-          `ProjectHistory:FirstOpTimestamp:${this.project_id}`,
+        Date.now()
-          Date.now()
+      )
-        )
-        .should.equal(true)
     })
   })
 
@@ -118,9 +115,10 @@ describe('ProjectHistoryRedisManager', function () {
         file: this.file_id,
       }
 
-      this.ProjectHistoryRedisManager.promises.queueOps
+      this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
-        .calledWithExactly(this.project_id, JSON.stringify(update))
+        this.project_id,
-        .should.equal(true)
+        JSON.stringify(update)
+      )
     })
   })
 
@@ -166,9 +164,10 @@ describe('ProjectHistoryRedisManager', function () {
         doc: this.doc_id,
       }
 
-      this.ProjectHistoryRedisManager.promises.queueOps
+      this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
-        .calledWithExactly(this.project_id, JSON.stringify(update))
+        this.project_id,
-        .should.equal(true)
+        JSON.stringify(update)
+      )
     })
 
     it('should queue an update with file metadata', async function () {
@@ -350,9 +349,10 @@ describe('ProjectHistoryRedisManager', function () {
         doc: this.doc_id,
       }
 
-      this.ProjectHistoryRedisManager.promises.queueOps
+      this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
-        .calledWithExactly(this.project_id, JSON.stringify(update))
+        this.project_id,
-        .should.equal(true)
+        JSON.stringify(update)
+      )
     })
 
     it('should not forward ranges if history ranges support is undefined', async function () {
@@ -402,9 +402,10 @@ describe('ProjectHistoryRedisManager', function () {
         doc: this.doc_id,
       }
 
-      this.ProjectHistoryRedisManager.promises.queueOps
+      this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
-        .calledWithExactly(this.project_id, JSON.stringify(update))
+        this.project_id,
-        .should.equal(true)
+        JSON.stringify(update)
+      )
     })
 
     it('should pass "false" as the createdBlob field if not provided', async function () {
@@ -432,9 +433,10 @@ describe('ProjectHistoryRedisManager', function () {
         doc: this.doc_id,
       }
 
-      this.ProjectHistoryRedisManager.promises.queueOps
+      this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
-        .calledWithExactly(this.project_id, JSON.stringify(update))
+        this.project_id,
-        .should.equal(true)
+        JSON.stringify(update)
+      )
     })
 
     it('should pass through the value of the createdBlob field', async function () {
@@ -463,9 +465,10 @@ describe('ProjectHistoryRedisManager', function () {
         doc: this.doc_id,
       }
 
-      this.ProjectHistoryRedisManager.promises.queueOps
+      this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
-        .calledWithExactly(this.project_id, JSON.stringify(update))
+        this.project_id,
-        .should.equal(true)
+        JSON.stringify(update)
+      )
     })
   })
 
@@ -493,8 +496,8 @@ describe('ProjectHistoryRedisManager', function () {
       beforeEach(async function () {
         this.update = {
           resyncDocContent: {
-            content: 'one\ntwo',
             version: this.version,
+            content: 'one\ntwo',
           },
           projectHistoryId: this.projectHistoryId,
           path: this.pathname,
@@ -516,19 +519,18 @@ describe('ProjectHistoryRedisManager', function () {
       })
 
       it('should check if the doc is too large', function () {
-        this.Limits.docIsTooLarge
+        this.Limits.docIsTooLarge.should.have.been.calledWith(
-          .calledWith(
+          JSON.stringify(this.update).length,
-            JSON.stringify(this.update).length,
+          this.lines,
-            this.lines,
+          this.settings.max_doc_length
-            this.settings.max_doc_length
+        )
-          )
-          .should.equal(true)
       })
 
       it('should queue an update', function () {
-        this.ProjectHistoryRedisManager.promises.queueOps
+        this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
-          .calledWithExactly(this.project_id, JSON.stringify(this.update))
+          this.project_id,
-          .should.equal(true)
+          JSON.stringify(this.update)
+        )
       })
     })
 
@@ -551,9 +553,8 @@ describe('ProjectHistoryRedisManager', function () {
       })
 
       it('should not queue an update if the doc is too large', function () {
-        this.ProjectHistoryRedisManager.promises.queueOps.called.should.equal(
+        this.ProjectHistoryRedisManager.promises.queueOps.should.not.have.been
-          false
+          .called
-        )
       })
     })
 
@@ -561,10 +562,10 @@ describe('ProjectHistoryRedisManager', function () {
       beforeEach(async function () {
         this.update = {
           resyncDocContent: {
-            content: 'onedeleted\ntwo',
             version: this.version,
             ranges: this.ranges,
             resolvedCommentIds: this.resolvedCommentIds,
+            content: 'onedeleted\ntwo',
           },
           projectHistoryId: this.projectHistoryId,
           path: this.pathname,
@@ -601,9 +602,76 @@ describe('ProjectHistoryRedisManager', function () {
       })
 
       it('should queue an update', function () {
-        this.ProjectHistoryRedisManager.promises.queueOps
+        this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
-          .calledWithExactly(this.project_id, JSON.stringify(this.update))
+          this.project_id,
-          .should.equal(true)
+          JSON.stringify(this.update)
+        )
+      })
+    })
+
+    describe('history-ot', function () {
+      beforeEach(async function () {
+        this.lines = {
+          content: 'onedeleted\ntwo',
+          comments: [{ id: 'id1', ranges: [{ pos: 0, length: 3 }] }],
+          trackedChanges: [
+            {
+              range: { pos: 3, length: 7 },
+              tracking: {
+                type: 'delete',
+                userId: 'user-id',
+                ts: '2025-06-16T14:31:44.910Z',
+              },
+            },
+          ],
+        }
+        this.update = {
+          resyncDocContent: {
+            version: this.version,
+            historyOTRanges: {
+              comments: this.lines.comments,
+              trackedChanges: this.lines.trackedChanges,
+            },
+            content: this.lines.content,
+          },
+          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.resolvedCommentIds,
+          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.stringFileDataContentIsTooLarge.should.have.been.calledWith(
+          this.lines,
+          this.settings.max_doc_length
+        )
+      })
+
+      it('should queue an update', function () {
+        this.ProjectHistoryRedisManager.promises.queueOps.should.have.been.calledWithExactly(
+          this.project_id,
+          JSON.stringify(this.update)
+        )
       })
     })
   })

services/history-v1/api/app/rollout.js

@@ -0,0 +1,76 @@
+const crypto = require('node:crypto')
+
+class Rollout {
+  constructor(config) {
+    // The history buffer level is used to determine whether to queue changes
+    // in Redis or persist them directly to the chunk store.
+    // If defaults to 0 (no queuing) if not set.
+    this.historyBufferLevel = config.has('historyBufferLevel')
+      ? parseInt(config.get('historyBufferLevel'), 10)
+      : 0
+    // The forcePersistBuffer flag will ensure the buffer is fully persisted before
+    // any persist operation. Set this to true if you want to make the persisted-version
+    // in Redis match the endVersion of the latest chunk. This should be set to true
+    // when downgrading from a history buffer level that queues changes in Redis
+    // without persisting them immediately.
+    this.forcePersistBuffer = config.has('forcePersistBuffer')
+      ? config.get('forcePersistBuffer') === 'true'
+      : false
+
+    // Support gradual rollout of the next history buffer level
+    // with a percentage of projects using it.
+    this.nextHistoryBufferLevel = config.has('nextHistoryBufferLevel')
+      ? parseInt(config.get('nextHistoryBufferLevel'), 10)
+      : null
+    this.nextHistoryBufferLevelRolloutPercentage = config.has(
+      'nextHistoryBufferLevelRolloutPercentage'
+    )
+      ? parseInt(config.get('nextHistoryBufferLevelRolloutPercentage'), 10)
+      : 0
+  }
+
+  report(logger) {
+    logger.info(
+      {
+        historyBufferLevel: this.historyBufferLevel,
+        forcePersistBuffer: this.forcePersistBuffer,
+        nextHistoryBufferLevel: this.nextHistoryBufferLevel,
+        nextHistoryBufferLevelRolloutPercentage:
+          this.nextHistoryBufferLevelRolloutPercentage,
+      },
+      this.historyBufferLevel > 0 || this.forcePersistBuffer
+        ? 'using history buffer'
+        : 'history buffer disabled'
+    )
+  }
+
+  /**
+   * Get the history buffer level for a project.
+   * @param {string} projectId
+   * @returns {Object} - An object containing the history buffer level and force persist buffer flag.
+   * @property {number} historyBufferLevel - The history buffer level to use for processing changes.
+   * @property {boolean} forcePersistBuffer - If true, forces the buffer to be persisted before any operation.
+   */
+  getHistoryBufferLevelOptions(projectId) {
+    if (
+      this.nextHistoryBufferLevel > this.historyBufferLevel &&
+      this.nextHistoryBufferLevelRolloutPercentage > 0
+    ) {
+      const hash = crypto.createHash('sha1').update(projectId).digest('hex')
+      const percentage = parseInt(hash.slice(0, 8), 16) % 100
+      // If the project is in the rollout percentage, we use the next history buffer level.
+      if (percentage < this.nextHistoryBufferLevelRolloutPercentage) {
+        return {
+          historyBufferLevel: this.nextHistoryBufferLevel,
+          forcePersistBuffer: this.forcePersistBuffer,
+        }
+      }
+    }
+    return {
+      historyBufferLevel: this.historyBufferLevel,
+      forcePersistBuffer: this.forcePersistBuffer,
+    }
+  }
+}
+
+module.exports = Rollout

services/history-v1/api/controllers/project_import.js

@@ -2,6 +2,7 @@
 
 'use strict'
 
+const config = require('config')
 const { expressify } = require('@overleaf/promise-utils')
 
 const HTTPStatus = require('http-status')
@@ -21,10 +22,15 @@ const BatchBlobStore = storage.BatchBlobStore
 const BlobStore = storage.BlobStore
 const chunkStore = storage.chunkStore
 const HashCheckBlobStore = storage.HashCheckBlobStore
-const persistChanges = storage.persistChanges
+const commitChanges = storage.commitChanges
+const persistBuffer = storage.persistBuffer
 const InvalidChangeError = storage.InvalidChangeError
 
 const render = require('./render')
+const Rollout = require('../app/rollout')
+
+const rollout = new Rollout(config)
+rollout.report(logger) // display the rollout configuration in the logs
 
 async function importSnapshot(req, res) {
   const projectId = req.swagger.params.project_id.value
@@ -35,6 +41,7 @@ async function importSnapshot(req, res) {
   try {
     snapshot = Snapshot.fromRaw(rawSnapshot)
   } catch (err) {
+    logger.warn({ err, projectId }, 'failed to import snapshot')
     return render.unprocessableEntity(res)
   }
 
@@ -43,6 +50,7 @@ async function importSnapshot(req, res) {
     historyId = await chunkStore.initializeProject(projectId, snapshot)
   } catch (err) {
     if (err instanceof chunkStore.AlreadyInitialized) {
+      logger.warn({ err, projectId }, 'already initialized')
       return render.conflict(res)
     } else {
       throw err
@@ -108,7 +116,12 @@ async function importChanges(req, res, next) {
 
   let result
   try {
-    result = await persistChanges(projectId, changes, limits, endVersion)
+    const { historyBufferLevel, forcePersistBuffer } =
+      rollout.getHistoryBufferLevelOptions(projectId)
+    result = await commitChanges(projectId, changes, limits, endVersion, {
+      historyBufferLevel,
+      forcePersistBuffer,
+    })
   } catch (err) {
     if (
       err instanceof Chunk.ConflictingEndVersion ||
@@ -141,5 +154,29 @@ async function importChanges(req, res, next) {
   }
 }
 
+async function flushChanges(req, res, next) {
+  const projectId = req.swagger.params.project_id.value
+  // Use the same limits importChanges, since these are passed to persistChanges
+  const farFuture = new Date()
+  farFuture.setTime(farFuture.getTime() + 7 * 24 * 3600 * 1000)
+  const limits = {
+    maxChanges: 0,
+    minChangeTimestamp: farFuture,
+    maxChangeTimestamp: farFuture,
+    autoResync: true,
+  }
+  try {
+    await persistBuffer(projectId, limits)
+    res.status(HTTPStatus.OK).end()
+  } catch (err) {
+    if (err instanceof Chunk.NotFoundError) {
+      render.notFound(res)
+    } else {
+      throw err
+    }
+  }
+}
+
 exports.importSnapshot = expressify(importSnapshot)
 exports.importChanges = expressify(importChanges)
+exports.flushChanges = expressify(flushChanges)

services/history-v1/api/controllers/projects.js

@@ -34,6 +34,7 @@ async function initializeProject(req, res, next) {
     res.status(HTTPStatus.OK).json({ projectId })
   } catch (err) {
     if (err instanceof chunkStore.AlreadyInitialized) {
+      logger.warn({ err, projectId }, 'failed to initialize')
       render.conflict(res)
     } else {
       throw err
@@ -242,11 +243,15 @@ async function createProjectBlob(req, res, next) {
     const sizeLimit = new StreamSizeLimit(maxUploadSize)
     await pipeline(req, sizeLimit, fs.createWriteStream(tmpPath))
     if (sizeLimit.sizeLimitExceeded) {
+      logger.warn(
+        { projectId, expectedHash, maxUploadSize },
+        'blob exceeds size threshold'
+      )
       return render.requestEntityTooLarge(res)
     }
     const hash = await blobHash.fromFile(tmpPath)
     if (hash !== expectedHash) {
-      logger.debug({ hash, expectedHash }, 'Hash mismatch')
+      logger.warn({ projectId, hash, expectedHash }, 'Hash mismatch')
       return render.conflict(res, 'File hash mismatch')
     }
 
@@ -343,6 +348,10 @@ async function copyProjectBlob(req, res, next) {
     targetBlobStore.getBlob(blobHash),
   ])
   if (!sourceBlob) {
+    logger.warn(
+      { sourceProjectId, targetProjectId, blobHash },
+      'missing source blob when copying across projects'
+    )
     return render.notFound(res)
   }
   // Exit early if the blob exists in the target project.

services/history-v1/api/swagger/project_import.js

@@ -139,9 +139,45 @@ const getChanges = {
   ],
 }
 
+const flushChanges = {
+  'x-swagger-router-controller': 'project_import',
+  operationId: 'flushChanges',
+  tags: ['ProjectImport'],
+  description: 'Flush project changes from buffer to the chunk store.',
+  parameters: [
+    {
+      name: 'project_id',
+      in: 'path',
+      description: 'project id',
+      required: true,
+      type: 'string',
+    },
+  ],
+  responses: {
+    200: {
+      description: 'Success',
+      schema: {
+        $ref: '#/definitions/Project',
+      },
+    },
+    404: {
+      description: 'Not Found',
+      schema: {
+        $ref: '#/definitions/Error',
+      },
+    },
+  },
+  security: [
+    {
+      basic: [],
+    },
+  ],
+}
+
 exports.paths = {
   '/projects/{project_id}/import': { post: importSnapshot },
   '/projects/{project_id}/legacy_import': { post: importSnapshot },
   '/projects/{project_id}/changes': { get: getChanges, post: importChanges },
   '/projects/{project_id}/legacy_changes': { post: importChanges },
+  '/projects/{project_id}/flush': { post: flushChanges },
 }

services/history-v1/app.js

@@ -100,11 +100,13 @@ function setupErrorHandling() {
       })
     }
     if (err.code === 'ENUM_MISMATCH') {
+      logger.warn({ err, projectId }, err.message)
       return res.status(HTTPStatus.UNPROCESSABLE_ENTITY).json({
         message: 'invalid enum value: ' + err.paramName,
       })
     }
     if (err.code === 'REQUIRED') {
+      logger.warn({ err, projectId }, err.message)
       return res.status(HTTPStatus.UNPROCESSABLE_ENTITY).json({
         message: err.message,
       })

services/history-v1/config/custom-environment-variables.json

@@ -84,6 +84,10 @@
   "maxFileUploadSize": "MAX_FILE_UPLOAD_SIZE",
   "httpsOnly": "HTTPS_ONLY",
   "httpRequestTimeout": "HTTP_REQUEST_TIMEOUT",
+  "historyBufferLevel": "HISTORY_BUFFER_LEVEL",
+  "forcePersistBuffer": "FORCE_PERSIST_BUFFER",
+  "nextHistoryBufferLevel": "NEXT_HISTORY_BUFFER_LEVEL",
+  "nextHistoryBufferLevelRolloutPercentage": "NEXT_HISTORY_BUFFER_LEVEL_ROLLOUT_PERCENTAGE",
   "redis": {
     "queue": {
       "host": "QUEUES_REDIS_HOST",
@@ -100,5 +104,9 @@
       "password": "REDIS_PASSWORD",
       "port": "REDIS_PORT"
     }
+  },
+  "projectHistory": {
+    "host": "PROJECT_HISTORY_HOST",
+    "port": "PROJECT_HISTORY_PORT"
   }
 }

services/history-v1/config/default.json

@@ -39,5 +39,8 @@
   "databasePoolMin": "2",
   "databasePoolMax": "10",
   "httpsOnly": "false",
-  "httpRequestTimeout": "300000"
+  "httpRequestTimeout": "300000",
+  "projectHistory": {
+    "port": "3054"
+  }
 }

services/history-v1/docker-compose.ci.yml

@@ -39,6 +39,7 @@ services:
       NODE_OPTIONS: "--unhandled-rejections=strict"
     volumes:
       - ./test/acceptance/certs:/certs
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     depends_on:
       mongo:
         condition: service_started
@@ -55,6 +56,7 @@ services:
       gcs:
         condition: service_healthy
     user: node
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run test:acceptance
 
 
@@ -66,7 +68,7 @@ services:
     command: tar -czf /tmp/build/build.tar.gz --exclude=build.tar.gz --exclude-vcs .
     user: root
   redis:
-    image: redis
+    image: redis:7.4.3
     healthcheck:
       test: ping="$$(redis-cli ping)" && [ "$$ping" = 'PONG' ]
       interval: 1s

services/history-v1/docker-compose.yml

@@ -33,6 +33,7 @@ services:
       - ../../node_modules:/overleaf/node_modules
       - ../../libraries:/overleaf/libraries
       - ./test/acceptance/certs:/certs
+      - ../../bin/shared/wait_for_it:/overleaf/bin/shared/wait_for_it
     working_dir: /overleaf/services/history-v1
     environment:
       ELASTIC_SEARCH_DSN: es:9200
@@ -71,10 +72,11 @@ services:
         condition: service_completed_successfully
       gcs:
         condition: service_healthy
+    entrypoint: /overleaf/bin/shared/wait_for_it mongo:27017 --timeout=0 --
     command: npm run --silent test:acceptance
 
   redis:
-    image: redis
+    image: redis:7.4.3
     healthcheck:
       test: ping=$$(redis-cli ping) && [ "$$ping" = 'PONG' ]
       interval: 1s

services/history-v1/package.json

@@ -7,6 +7,7 @@
   "private": true,
   "dependencies": {
     "@google-cloud/secret-manager": "^5.6.0",
+    "@overleaf/fetch-utils": "*",
     "@overleaf/logger": "*",
     "@overleaf/metrics": "*",
     "@overleaf/mongo-utils": "*",
@@ -36,6 +37,7 @@
     "mongodb": "6.12.0",
     "overleaf-editor-core": "*",
     "p-limit": "^6.2.0",
+    "p-queue": "^8.1.0",
     "pg": "^8.7.1",
     "pg-query-stream": "^4.2.4",
     "swagger-tools": "^0.10.4",

services/history-v1/storage/index.js

@@ -8,6 +8,9 @@ exports.mongodb = require('./lib/mongodb')
 exports.redis = require('./lib/redis')
 exports.persistChanges = require('./lib/persist_changes')
 exports.persistor = require('./lib/persistor')
+exports.persistBuffer = require('./lib/persist_buffer')
+exports.commitChanges = require('./lib/commit_changes')
+exports.queueChanges = require('./lib/queue_changes')
 exports.ProjectArchive = require('./lib/project_archive')
 exports.streams = require('./lib/streams')
 exports.temp = require('./lib/temp')

services/history-v1/storage/lib/chunk_store/index.js

@@ -151,23 +151,48 @@ async function loadAtVersion(projectId, version, opts = {}) {
   const backend = getBackend(projectId)
   const blobStore = new BlobStore(projectId)
   const batchBlobStore = new BatchBlobStore(blobStore)
+  const latestChunkMetadata = await getLatestChunkMetadata(projectId)
 
-  const chunkRecord = await backend.getChunkForVersion(projectId, version, {
+  // When loading a chunk for a version there are three cases to consider:
-    preferNewer: opts.preferNewer,
+  // 1. If `persistedOnly` is true, we always use the requested version
-  })
+  //  to fetch the chunk.
+  // 2. If `persistedOnly` is false and the requested version is in the
+  //  persisted chunk version range, we use the requested version.
+  // 3. If `persistedOnly` is false and the requested version is ahead of
+  //  the persisted chunk versions, we fetch the latest chunk and see if
+  //  the non-persisted changes include the requested version.
+  const targetChunkVersion = opts.persistedOnly
+    ? version
+    : Math.min(latestChunkMetadata.endVersion, version)
+
+  const chunkRecord = await backend.getChunkForVersion(
+    projectId,
+    targetChunkVersion,
+    {
+      preferNewer: opts.preferNewer,
+    }
+  )
   const rawHistory = await historyStore.loadRaw(projectId, chunkRecord.id)
   const history = History.fromRaw(rawHistory)
+  const startVersion = chunkRecord.endVersion - history.countChanges()
 
   if (!opts.persistedOnly) {
+    // Try to extend the chunk with any non-persisted changes that
+    // follow the chunk's end version.
     const nonPersistedChanges = await getChunkExtension(
       projectId,
       chunkRecord.endVersion
     )
     history.pushChanges(nonPersistedChanges)
+
+    // Check that the changes do actually contain the requested version
+    if (version > chunkRecord.endVersion + nonPersistedChanges.length) {
+      throw new Chunk.VersionNotFoundError(projectId, version)
+    }
   }
 
   await lazyLoadHistoryFiles(history, batchBlobStore)
-  return new Chunk(history, chunkRecord.endVersion - history.countChanges())
+  return new Chunk(history, startVersion)
 }
 
 /**
@@ -190,6 +215,7 @@ async function loadAtTimestamp(projectId, timestamp, opts = {}) {
   const chunkRecord = await backend.getChunkForTimestamp(projectId, timestamp)
   const rawHistory = await historyStore.loadRaw(projectId, chunkRecord.id)
   const history = History.fromRaw(rawHistory)
+  const startVersion = chunkRecord.endVersion - history.countChanges()
 
   if (!opts.persistedOnly) {
     const nonPersistedChanges = await getChunkExtension(
@@ -200,7 +226,7 @@ async function loadAtTimestamp(projectId, timestamp, opts = {}) {
   }
 
   await lazyLoadHistoryFiles(history, batchBlobStore)
-  return new Chunk(history, chunkRecord.endVersion - history.countChanges())
+  return new Chunk(history, startVersion)
 }
 
 /**

services/history-v1/storage/lib/chunk_store/mongo.js

@@ -286,6 +286,27 @@ async function updateProjectRecord(
   )
 }
 
+/**
+ * @param {number} historyId
+ * @return {Promise<string>}
+ */
+async function lookupMongoProjectIdFromHistoryId(historyId) {
+  const project = await mongodb.projects.findOne(
+    // string for Object ids, number for postgres ids
+    { 'overleaf.history.id': historyId },
+    { projection: { _id: 1 } }
+  )
+  if (!project) {
+    // should not happen: We flush before allowing a project to be soft-deleted.
+    throw new OError('mongo project not found by history id', { historyId })
+  }
+  return project._id.toString()
+}
+
+async function resolveHistoryIdToMongoProjectId(projectId) {
+  return projectId
+}
+
 /**
  * Record that a chunk was replaced by a new one.
  *
@@ -533,4 +554,6 @@ module.exports = {
   deleteProjectChunks,
   getOldChunksBatch,
   deleteOldChunks,
+  lookupMongoProjectIdFromHistoryId,
+  resolveHistoryIdToMongoProjectId,
 }

services/history-v1/storage/lib/chunk_store/postgres.js

@@ -5,7 +5,10 @@ const assert = require('../assert')
 const knex = require('../knex')
 const knexReadOnly = require('../knex_read_only')
 const { ChunkVersionConflictError } = require('./errors')
-const { updateProjectRecord } = require('./mongo')
+const {
+  updateProjectRecord,
+  lookupMongoProjectIdFromHistoryId,
+} = require('./mongo')
 
 const DUPLICATE_KEY_ERROR_CODE = '23505'
 
@@ -472,6 +475,10 @@ async function generateProjectId() {
   return record.doc_id.toString()
 }
 
+async function resolveHistoryIdToMongoProjectId(projectId) {
+  return await lookupMongoProjectIdFromHistoryId(parseInt(projectId, 10))
+}
+
 module.exports = {
   getLatestChunk,
   getFirstChunkBeforeTimestamp,
@@ -488,4 +495,5 @@ module.exports = {
   getOldChunksBatch,
   deleteOldChunks,
   generateProjectId,
+  resolveHistoryIdToMongoProjectId,
 }

services/history-v1/storage/lib/chunk_store/redis.js

@@ -480,11 +480,12 @@ async function getNonPersistedChanges(projectId, baseVersion) {
 }
 
 rclient.defineCommand('set_persisted_version', {
-  numberOfKeys: 3,
+  numberOfKeys: 4,
   lua: `
     local headVersionKey = KEYS[1]
     local persistedVersionKey = KEYS[2]
-    local changesKey = KEYS[3]
+    local persistTimeKey = KEYS[3]
+    local changesKey = KEYS[4]
 
     local newPersistedVersion = tonumber(ARGV[1])
     local maxPersistedChanges = tonumber(ARGV[2])
@@ -501,9 +502,19 @@ rclient.defineCommand('set_persisted_version', {
       return 'too_low'
     end
 
+    -- Refuse to set a persisted version that is higher than the head version
+    if newPersistedVersion > headVersion then
+      return 'too_high'
+    end
+
     -- Set the persisted version
     redis.call('SET', persistedVersionKey, newPersistedVersion)
 
+    -- Clear the persist time if the persisted version now matches the head version
+    if newPersistedVersion == headVersion then
+      redis.call('DEL', persistTimeKey)
+    end
+
     -- Calculate the starting index, to keep only maxPersistedChanges beyond the persisted version
     -- Using negative indexing to count backwards from the end of the list
     local startIndex = newPersistedVersion - headVersion - maxPersistedChanges
@@ -530,6 +541,7 @@ async function setPersistedVersion(projectId, persistedVersion) {
     const keys = [
       keySchema.headVersion({ projectId }),
       keySchema.persistedVersion({ projectId }),
+      keySchema.persistTime({ projectId }),
       keySchema.changes({ projectId }),
     ]
 
@@ -541,6 +553,13 @@ async function setPersistedVersion(projectId, persistedVersion) {
       status,
     })
 
+    if (status === 'too_high') {
+      throw new VersionOutOfBoundsError(
+        'Persisted version cannot be higher than head version',
+        { projectId, persistedVersion }
+      )
+    }
+
     return status
   } catch (err) {
     metrics.inc('chunk_store.redis.set_persisted_version', 1, {
@@ -631,6 +650,7 @@ async function expireProject(projectId) {
     metrics.inc('chunk_store.redis.set_persisted_version', 1, {
       status,
     })
+    return status
   } catch (err) {
     metrics.inc('chunk_store.redis.set_persisted_version', 1, {
       status: 'error',

services/history-v1/storage/lib/commit_changes.js

@@ -0,0 +1,159 @@
+// @ts-check
+
+'use strict'
+
+const metrics = require('@overleaf/metrics')
+const redisBackend = require('./chunk_store/redis')
+const logger = require('@overleaf/logger')
+const queueChanges = require('./queue_changes')
+const persistChanges = require('./persist_changes')
+const persistBuffer = require('./persist_buffer')
+
+/**
+ * @typedef {import('overleaf-editor-core').Change} Change
+ */
+
+/**
+ * Handle incoming changes by processing them according to the specified options.
+ * @param {string} projectId
+ * @param {Change[]} changes
+ * @param {Object} limits
+ * @param {number} endVersion
+ * @param {Object} options
+ * @param {number} [options.historyBufferLevel] - The history buffer level to use for processing changes.
+ * @param {Boolean} [options.forcePersistBuffer] - If true, forces the buffer to be persisted before any operation.
+ * @return {Promise.<Object?>}
+ */
+
+async function commitChanges(
+  projectId,
+  changes,
+  limits,
+  endVersion,
+  options = {}
+) {
+  const { historyBufferLevel, forcePersistBuffer } = options
+
+  // Force the buffer to be persisted if specified.
+  if (forcePersistBuffer) {
+    try {
+      const status = await redisBackend.expireProject(projectId) // clear the project from Redis if it is persisted, returns 'not-persisted' if it was not persisted
+      if (status === 'not-persisted') {
+        await persistBuffer(projectId, limits)
+        await redisBackend.expireProject(projectId) // clear the project from Redis after persisting
+        metrics.inc('persist_buffer_force', 1, { status: 'persisted' })
+      }
+    } catch (err) {
+      metrics.inc('persist_buffer_force', 1, { status: 'error' })
+      logger.error(
+        { err, projectId },
+        'failed to persist buffer before committing changes'
+      )
+    }
+  }
+
+  metrics.inc('commit_changes', 1, {
+    history_buffer_level: historyBufferLevel || 0,
+  })
+
+  // Now handle the changes based on the configured history buffer level.
+  switch (historyBufferLevel) {
+    case 4: // Queue changes and only persist them in the background
+      await queueChanges(projectId, changes, endVersion)
+      return {}
+    case 3: // Queue changes and immediately persist with persistBuffer
+      await queueChanges(projectId, changes, endVersion)
+      return await persistBuffer(projectId, limits)
+    case 2: // Equivalent to queueChangesInRedis:true
+      await queueChangesFake(projectId, changes, endVersion)
+      return await persistChanges(projectId, changes, limits, endVersion)
+    case 1: // Queue changes with fake persist only for projects in redis already
+      await queueChangesFakeOnlyIfExists(projectId, changes, endVersion)
+      return await persistChanges(projectId, changes, limits, endVersion)
+    case 0: // Persist changes directly to the chunk store
+      return await persistChanges(projectId, changes, limits, endVersion)
+    default:
+      throw new Error(`Invalid history buffer level: ${historyBufferLevel}`)
+  }
+}
+
+/**
+ * Queues a set of changes in redis as if they had been persisted, ignoring any errors.
+ * @param {string} projectId
+ * @param {Change[]} changes
+ * @param {number} endVersion
+ * @param {Object} [options]
+ * @param {boolean} [options.onlyIfExists] - If true, only queue changes if the project
+ * already exists in Redis.
+ */
+
+async function queueChangesFake(projectId, changes, endVersion, options = {}) {
+  try {
+    await queueChanges(projectId, changes, endVersion)
+    await fakePersistRedisChanges(projectId, changes, endVersion)
+  } catch (err) {
+    logger.error({ err }, 'Chunk buffer verification failed')
+  }
+}
+
+/**
+ * Queues changes in Redis, simulating persistence, but only if the project already exists.
+ * @param {string} projectId - The ID of the project.
+ * @param {Change[]} changes - An array of changes to be queued.
+ * @param {number} endVersion - The expected version of the project before these changes are applied.
+ */
+
+async function queueChangesFakeOnlyIfExists(projectId, changes, endVersion) {
+  await queueChangesFake(projectId, changes, endVersion, {
+    onlyIfExists: true,
+  })
+}
+
+/**
+ * Simulates the persistence of changes by verifying a given set of changes against
+ * what is currently stored as non-persisted in Redis, and then updates the
+ * persisted version number in Redis.
+ *
+ * @async
+ * @param {string} projectId - The ID of the project.
+ * @param {Change[]} changesToPersist - An array of changes that are expected to be
+ *                                        persisted. These are used for verification
+ *                                        against the changes currently in Redis.
+ * @param {number} baseVersion - The base version number from which to calculate
+ *                               the new persisted version.
+ * @returns {Promise<void>} A promise that resolves when the persisted version
+ *                          in Redis has been updated.
+ */
+async function fakePersistRedisChanges(
+  projectId,
+  changesToPersist,
+  baseVersion
+) {
+  const nonPersistedChanges = await redisBackend.getNonPersistedChanges(
+    projectId,
+    baseVersion
+  )
+
+  if (
+    serializeChanges(nonPersistedChanges) === serializeChanges(changesToPersist)
+  ) {
+    metrics.inc('persist_redis_changes_verification', 1, { status: 'match' })
+  } else {
+    logger.warn({ projectId }, 'mismatch of non-persisted changes from Redis')
+    metrics.inc('persist_redis_changes_verification', 1, {
+      status: 'mismatch',
+    })
+  }
+
+  const persistedVersion = baseVersion + nonPersistedChanges.length
+  await redisBackend.setPersistedVersion(projectId, persistedVersion)
+}
+
+/**
+ * @param {Change[]} changes
+ */
+function serializeChanges(changes) {
+  return JSON.stringify(changes.map(change => change.toRaw()))
+}
+
+module.exports = commitChanges

services/history-v1/storage/lib/persist_buffer.js

@@ -0,0 +1,206 @@
+// @ts-check
+'use strict'
+
+const logger = require('@overleaf/logger')
+const metrics = require('@overleaf/metrics')
+const OError = require('@overleaf/o-error')
+const assert = require('./assert')
+const chunkStore = require('./chunk_store')
+const { BlobStore } = require('./blob_store')
+const BatchBlobStore = require('./batch_blob_store')
+const persistChanges = require('./persist_changes')
+const resyncProject = require('./resync_project')
+const redisBackend = require('./chunk_store/redis')
+
+/**
+ * Persist the changes from Redis buffer to the main storage
+ *
+ * Algorithm Outline:
+ * 1. Get the latest chunk's endVersion from the database
+ * 2. Get non-persisted changes from Redis that are after this endVersion.
+ * 3. If no such changes, exit.
+ * 4. Load file blobs for these Redis changes.
+ * 5. Run the persistChanges() algorithm to store these changes into a new chunk(s) in GCS.
+ *    - This must not decrease the endVersion. If changes were processed, it must advance.
+ * 6. Set the new persisted version (endVersion of the latest persisted chunk) in Redis.
+ *
+ * @param {string} projectId
+ * @param {Object} limits
+ * @throws {Error | OError} If a critical error occurs during persistence.
+ */
+async function persistBuffer(projectId, limits) {
+  assert.projectId(projectId)
+  logger.debug({ projectId }, 'starting persistBuffer operation')
+
+  // 1. Get the latest chunk's endVersion from GCS/main store
+  let endVersion
+  const latestChunkMetadata = await chunkStore.getLatestChunkMetadata(projectId)
+
+  if (latestChunkMetadata) {
+    endVersion = latestChunkMetadata.endVersion
+  } else {
+    endVersion = 0 // No chunks found, start from version 0
+    logger.debug({ projectId }, 'no existing chunks found in main storage')
+  }
+
+  logger.debug({ projectId, endVersion }, 'got latest persisted chunk')
+
+  // 2. Get non-persisted changes from Redis
+  const changesToPersist = await redisBackend.getNonPersistedChanges(
+    projectId,
+    endVersion
+  )
+
+  if (changesToPersist.length === 0) {
+    logger.debug(
+      { projectId, endVersion },
+      'no new changes in Redis buffer to persist'
+    )
+    metrics.inc('persist_buffer', 1, { status: 'no_changes' })
+    // No changes to persist, update the persisted version in Redis
+    // to match the current endVersion.  This shouldn't be needed
+    // unless a worker failed to update the persisted version.
+    await redisBackend.setPersistedVersion(projectId, endVersion)
+    const { chunk } = await chunkStore.loadByChunkRecord(
+      projectId,
+      latestChunkMetadata
+    )
+    // Return the result in the same format as persistChanges
+    // so that the caller can handle it uniformly.
+    return {
+      numberOfChangesPersisted: changesToPersist.length,
+      originalEndVersion: endVersion,
+      currentChunk: chunk,
+    }
+  }
+
+  logger.debug(
+    {
+      projectId,
+      endVersion,
+      count: changesToPersist.length,
+    },
+    'found changes in Redis to persist'
+  )
+
+  // 4. Load file blobs for these Redis changes. Errors will propagate.
+  const blobStore = new BlobStore(projectId)
+  const batchBlobStore = new BatchBlobStore(blobStore)
+
+  const blobHashes = new Set()
+  for (const change of changesToPersist) {
+    change.findBlobHashes(blobHashes)
+  }
+  if (blobHashes.size > 0) {
+    await batchBlobStore.preload(Array.from(blobHashes))
+  }
+  for (const change of changesToPersist) {
+    await change.loadFiles('lazy', blobStore)
+  }
+
+  // 5. Run the persistChanges() algorithm. Errors will propagate.
+  logger.debug(
+    {
+      projectId,
+      endVersion,
+      changeCount: changesToPersist.length,
+    },
+    'calling persistChanges'
+  )
+
+  const persistResult = await persistChanges(
+    projectId,
+    changesToPersist,
+    limits,
+    endVersion
+  )
+
+  if (!persistResult || !persistResult.currentChunk) {
+    metrics.inc('persist_buffer', 1, { status: 'no-chunk-error' })
+    throw new OError(
+      'persistChanges did not produce a new chunk for non-empty changes',
+      {
+        projectId,
+        endVersion,
+        changeCount: changesToPersist.length,
+      }
+    )
+  }
+
+  const newPersistedChunk = persistResult.currentChunk
+  const newEndVersion = newPersistedChunk.getEndVersion()
+
+  if (newEndVersion <= endVersion) {
+    metrics.inc('persist_buffer', 1, { status: 'chunk-version-mismatch' })
+    throw new OError(
+      'persisted chunk endVersion must be greater than current persisted chunk end version for non-empty changes',
+      {
+        projectId,
+        newEndVersion,
+        endVersion,
+        changeCount: changesToPersist.length,
+      }
+    )
+  }
+
+  logger.debug(
+    {
+      projectId,
+      oldVersion: endVersion,
+      newVersion: newEndVersion,
+    },
+    'successfully persisted changes from Redis to main storage'
+  )
+
+  // 6. Set the persisted version in Redis. Errors will propagate.
+  const status = await redisBackend.setPersistedVersion(
+    projectId,
+    newEndVersion
+  )
+
+  if (status !== 'ok') {
+    metrics.inc('persist_buffer', 1, { status: 'error-on-persisted-version' })
+    throw new OError('failed to update persisted version in Redis', {
+      projectId,
+      newEndVersion,
+      status,
+    })
+  }
+
+  logger.debug(
+    { projectId, newEndVersion },
+    'updated persisted version in Redis'
+  )
+
+  // 7. Resync the project if content hash validation failed
+  if (limits.autoResync && persistResult.resyncNeeded) {
+    if (
+      changesToPersist.some(
+        change => change.getOrigin()?.getKind() === 'history-resync'
+      )
+    ) {
+      // To avoid an infinite loop, do not resync if the current batch of
+      // changes contains a history resync.
+      logger.warn(
+        { projectId },
+        'content hash validation failed while persisting a history resync, skipping additional resync'
+      )
+    } else {
+      const backend = chunkStore.getBackend(projectId)
+      const mongoProjectId =
+        await backend.resolveHistoryIdToMongoProjectId(projectId)
+      await resyncProject(mongoProjectId)
+    }
+  }
+
+  logger.debug(
+    { projectId, finalPersistedVersion: newEndVersion },
+    'persistBuffer operation completed successfully'
+  )
+
+  metrics.inc('persist_buffer', 1, { status: 'persisted' })
+
+  return persistResult
+}
+
+module.exports = persistBuffer

services/history-v1/storage/lib/persist_changes.js

@@ -4,7 +4,6 @@
 
 const _ = require('lodash')
 const logger = require('@overleaf/logger')
-const metrics = require('@overleaf/metrics')
 
 const core = require('overleaf-editor-core')
 const Chunk = core.Chunk
@@ -15,7 +14,6 @@ const chunkStore = require('./chunk_store')
 const { BlobStore } = require('./blob_store')
 const { InvalidChangeError } = require('./errors')
 const { getContentHash } = require('./content_hash')
-const redisBackend = require('./chunk_store/redis')
 
 function countChangeBytes(change) {
   // Note: This is not quite accurate, because the raw change may contain raw
@@ -202,45 +200,6 @@ async function persistChanges(projectId, allChanges, limits, clientEndVersion) {
     currentSnapshot.applyAll(currentChunk.getChanges())
   }
 
-  async function queueChangesInRedis() {
-    const hollowSnapshot = currentSnapshot.clone()
-    // We're transforming a lazy snapshot to a hollow snapshot, so loadFiles()
-    // doesn't really need a blobStore, but its signature still requires it.
-    const blobStore = new BlobStore(projectId)
-    await hollowSnapshot.loadFiles('hollow', blobStore)
-    hollowSnapshot.applyAll(changesToPersist, { strict: true })
-    const baseVersion = currentChunk.getEndVersion()
-    await redisBackend.queueChanges(
-      projectId,
-      hollowSnapshot,
-      baseVersion,
-      changesToPersist
-    )
-  }
-
-  async function fakePersistRedisChanges() {
-    const baseVersion = currentChunk.getEndVersion()
-    const nonPersistedChanges = await redisBackend.getNonPersistedChanges(
-      projectId,
-      baseVersion
-    )
-
-    if (
-      serializeChanges(nonPersistedChanges) ===
-      serializeChanges(changesToPersist)
-    ) {
-      metrics.inc('persist_redis_changes_verification', 1, { status: 'match' })
-    } else {
-      logger.warn({ projectId }, 'mismatch of non-persisted changes from Redis')
-      metrics.inc('persist_redis_changes_verification', 1, {
-        status: 'mismatch',
-      })
-    }
-
-    const persistedVersion = baseVersion + nonPersistedChanges.length
-    await redisBackend.setPersistedVersion(projectId, persistedVersion)
-  }
-
   async function extendLastChunkIfPossible() {
     const timer = new Timer()
     const changesPushed = await fillChunk(currentChunk, changesToPersist)
@@ -289,12 +248,6 @@ async function persistChanges(projectId, allChanges, limits, clientEndVersion) {
     const numberOfChangesToPersist = oldChanges.length
 
     await loadLatestChunk()
-    try {
-      await queueChangesInRedis()
-      await fakePersistRedisChanges()
-    } catch (err) {
-      logger.error({ err }, 'Chunk buffer verification failed')
-    }
     await extendLastChunkIfPossible()
     await createNewChunksAsNeeded()
 
@@ -309,11 +262,4 @@ async function persistChanges(projectId, allChanges, limits, clientEndVersion) {
   }
 }
 
-/**
- * @param {core.Change[]} changes
- */
-function serializeChanges(changes) {
-  return JSON.stringify(changes.map(change => change.toRaw()))
-}
-
 module.exports = persistChanges

services/history-v1/storage/lib/queue_changes.js

@@ -0,0 +1,75 @@
+// @ts-check
+
+'use strict'
+
+const redisBackend = require('./chunk_store/redis')
+const { BlobStore } = require('./blob_store')
+const chunkStore = require('./chunk_store')
+const core = require('overleaf-editor-core')
+const Chunk = core.Chunk
+
+/**
+ * Queues an incoming set of changes after validating them against the current snapshot.
+ *
+ * @async
+ * @function queueChanges
+ * @param {string} projectId - The project to queue changes for.
+ * @param {Array<Object>} changesToQueue - An array of change objects to be applied and queued.
+ * @param {number} endVersion - The expected version of the project before these changes are applied.
+ *                              This is used for optimistic concurrency control.
+ * @param {Object} [opts] - Additional options for queuing changes.
+ * @throws {Chunk.ConflictingEndVersion} If the provided `endVersion` does not match the
+ *                                       current version of the project.
+ * @returns {Promise<any>} A promise that resolves with the status returned by the
+ *                         `redisBackend.queueChanges` operation.
+ */
+async function queueChanges(projectId, changesToQueue, endVersion, opts) {
+  const result = await redisBackend.getHeadSnapshot(projectId)
+  let currentSnapshot = null
+  let currentVersion = null
+  if (result) {
+    // If we have a snapshot in redis, we can use it to check the current state
+    // of the project and apply changes to it.
+    currentSnapshot = result.snapshot
+    currentVersion = result.version
+  } else {
+    // Otherwise, load the latest chunk from the chunk store.
+    const latestChunk = await chunkStore.loadLatest(projectId, {
+      persistedOnly: true,
+    })
+    // Throw an error if no latest chunk is found, indicating the project has not been initialised.
+    if (!latestChunk) {
+      throw new Chunk.NotFoundError(projectId)
+    }
+    currentSnapshot = latestChunk.getSnapshot()
+    currentSnapshot.applyAll(latestChunk.getChanges())
+    currentVersion = latestChunk.getEndVersion()
+  }
+
+  // Ensure the endVersion matches the current version of the project.
+  if (endVersion !== currentVersion) {
+    throw new Chunk.ConflictingEndVersion(endVersion, currentVersion)
+  }
+
+  // Compute the new hollow snapshot to be saved to redis.
+  const hollowSnapshot = currentSnapshot
+  const blobStore = new BlobStore(projectId)
+  await hollowSnapshot.loadFiles('hollow', blobStore)
+  // Clone the changes to avoid modifying the original ones when computing the hollow snapshot.
+  const hollowChanges = changesToQueue.map(change => change.clone())
+  for (const change of hollowChanges) {
+    await change.loadFiles('hollow', blobStore)
+  }
+  hollowSnapshot.applyAll(hollowChanges, { strict: true })
+  const baseVersion = currentVersion
+  const status = await redisBackend.queueChanges(
+    projectId,
+    hollowSnapshot,
+    baseVersion,
+    changesToQueue,
+    opts
+  )
+  return status
+}
+
+module.exports = queueChanges