Merge pull request #16644 from overleaf/em-promisify-update-manager

Promisify UpdateManager

GitOrigin-RevId: 2c3e21ee6ef2454f79695ca8623c3d38720ff6bf

libraries/metrics/index.js

@@ -97,7 +97,7 @@ function histogram(key, value, buckets, labels = {}) {
 }
 
 class Timer {
-  constructor(key, sampleRate = 1, labels = {}, buckets) {
+  constructor(key, sampleRate = 1, labels = {}, buckets = undefined) {
     if (typeof sampleRate === 'object') {
       // called with (key, labels, buckets)
       if (arguments.length === 3) {

libraries/redis-wrapper/RedisLocker.js

@@ -1,3 +1,4 @@
+const { promisify } = require('util')
 const metrics = require('@overleaf/metrics')
 const logger = require('@overleaf/logger')
 const os = require('os')
@@ -64,6 +65,27 @@ module.exports = class RedisLocker {
 
     // read-only copy for unit tests
     this.unlockScript = UNLOCK_SCRIPT
+
+    this.promises = {
+      checkLock: promisify(this.checkLock.bind(this)),
+      getLock: promisify(this.getLock.bind(this)),
+      releaseLock: promisify(this.releaseLock.bind(this)),
+
+      // tryLock returns two values: gotLock and lockValue. We need to merge
+      // these two values into one for the promises version.
+      tryLock: id =>
+        new Promise((resolve, reject) => {
+          this.tryLock(id, (err, gotLock, lockValue) => {
+            if (err) {
+              reject(err)
+            } else if (!gotLock) {
+              resolve(null)
+            } else {
+              resolve(lockValue)
+            }
+          })
+        }),
+    }
   }
 
   // Use a signed lock value as described in

package-lock.json

@@ -15503,8 +15503,15 @@
     "node_modules/@types/chai": {
       "version": "4.3.0",
       "resolved": "https://registry.npmjs.org/@types/chai/-/chai-4.3.0.tgz",
-      "integrity": "sha512-/ceqdqeRraGolFTcfoXNiqjyQhZzbINDngeoAq9GoHa8PPK1yNzTaxWjA6BFWp5Ua9JpXEMSS4s5i9tS0hOJtw==",
-      "dev": true
+      "integrity": "sha512-/ceqdqeRraGolFTcfoXNiqjyQhZzbINDngeoAq9GoHa8PPK1yNzTaxWjA6BFWp5Ua9JpXEMSS4s5i9tS0hOJtw=="
+    },
+    "node_modules/@types/chai-as-promised": {
+      "version": "7.1.8",
+      "resolved": "https://registry.npmjs.org/@types/chai-as-promised/-/chai-as-promised-7.1.8.tgz",
+      "integrity": "sha512-ThlRVIJhr69FLlh6IctTXFkmhtP3NpMZ2QGq69StYLyKZFp/HOp1VdKZj7RvfNWYYcJ1xlbLGLLWj1UvP5u/Gw==",
+      "dependencies": {
+        "@types/chai": "*"
+      }
     },
     "node_modules/@types/connect": {
       "version": "3.4.35",
@@ -43700,6 +43707,7 @@
         "@overleaf/ranges-tracker": "*",
         "@overleaf/redis-wrapper": "*",
         "@overleaf/settings": "*",
+        "@types/chai-as-promised": "^7.1.8",
         "async": "^3.2.2",
         "body-parser": "^1.19.0",
         "bunyan": "^1.8.15",
@@ -54201,6 +54209,7 @@
         "@overleaf/ranges-tracker": "*",
         "@overleaf/redis-wrapper": "*",
         "@overleaf/settings": "*",
+        "@types/chai-as-promised": "^7.1.8",
         "async": "^3.2.2",
         "body-parser": "^1.19.0",
         "bunyan": "^1.8.15",
@@ -61468,8 +61477,15 @@
     "@types/chai": {
       "version": "4.3.0",
       "resolved": "https://registry.npmjs.org/@types/chai/-/chai-4.3.0.tgz",
-      "integrity": "sha512-/ceqdqeRraGolFTcfoXNiqjyQhZzbINDngeoAq9GoHa8PPK1yNzTaxWjA6BFWp5Ua9JpXEMSS4s5i9tS0hOJtw==",
-      "dev": true
+      "integrity": "sha512-/ceqdqeRraGolFTcfoXNiqjyQhZzbINDngeoAq9GoHa8PPK1yNzTaxWjA6BFWp5Ua9JpXEMSS4s5i9tS0hOJtw=="
+    },
+    "@types/chai-as-promised": {
+      "version": "7.1.8",
+      "resolved": "https://registry.npmjs.org/@types/chai-as-promised/-/chai-as-promised-7.1.8.tgz",
+      "integrity": "sha512-ThlRVIJhr69FLlh6IctTXFkmhtP3NpMZ2QGq69StYLyKZFp/HOp1VdKZj7RvfNWYYcJ1xlbLGLLWj1UvP5u/Gw==",
+      "requires": {
+        "@types/chai": "*"
+      }
     },
     "@types/connect": {
       "version": "3.4.35",

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

@@ -1,4 +1,4 @@
-let DocumentManager
+const { promisifyAll } = require('@overleaf/promise-utils')
 const RedisManager = require('./RedisManager')
 const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager')
 const PersistenceManager = require('./PersistenceManager')
@@ -11,7 +11,7 @@ const RangesManager = require('./RangesManager')
 
 const MAX_UNFLUSHED_AGE = 300 * 1000 // 5 mins, document should be flushed to mongo this time after a change
 
-module.exports = DocumentManager = {
+const DocumentManager = {
   getDoc(projectId, docId, _callback) {
     const timer = new Metrics.Timer('docManager.getDoc')
     const callback = (...args) => {
@@ -680,3 +680,45 @@ module.exports = DocumentManager = {
     )
   },
 }
+
+module.exports = DocumentManager
+module.exports.promises = promisifyAll(DocumentManager, {
+  multiResult: {
+    getDoc: [
+      'lines',
+      'version',
+      'ranges',
+      'pathname',
+      'projectHistoryId',
+      'unflushedTime',
+      'alreadyLoaded',
+    ],
+    getDocWithLock: [
+      'lines',
+      'version',
+      'ranges',
+      'pathname',
+      'projectHistoryId',
+      'unflushedTime',
+      'alreadyLoaded',
+    ],
+    getDocAndFlushIfOld: ['lines', 'version'],
+    getDocAndFlushIfOldWithLock: ['lines', 'version'],
+    getDocAndRecentOps: [
+      'lines',
+      'version',
+      'ops',
+      'ranges',
+      'pathname',
+      'projectHistoryId',
+    ],
+    getDocAndRecentOpsWithLock: [
+      'lines',
+      'version',
+      'ops',
+      'ranges',
+      'pathname',
+      'projectHistoryId',
+    ],
+  },
+})

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

@@ -1,12 +1,12 @@
-let HistoryManager
 const async = require('async')
 const logger = require('@overleaf/logger')
+const { promisifyAll } = require('@overleaf/promise-utils')
 const request = require('request')
 const Settings = require('@overleaf/settings')
 const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager')
 const metrics = require('./Metrics')
 
-module.exports = HistoryManager = {
+const HistoryManager = {
   // flush changes in the background
   flushProjectChangesAsync(projectId) {
     HistoryManager.flushProjectChanges(
@@ -122,3 +122,12 @@ module.exports = HistoryManager = {
     )
   },
 }
+
+module.exports = HistoryManager
+module.exports.promises = promisifyAll(HistoryManager, {
+  without: [
+    'flushProjectChangesAsync',
+    'recordAndFlushHistoryOps',
+    'shouldFlushHistoryOps',
+  ],
+})

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

@@ -3,7 +3,6 @@ const redis = require('@overleaf/redis-wrapper')
 const rclient = redis.createClient(Settings.redis.lock)
 const keys = Settings.redis.lock.key_schema
 const RedisLocker = require('@overleaf/redis-wrapper/RedisLocker')
-const { promisify } = require('@overleaf/promise-utils')
 
 module.exports = new RedisLocker({
   rclient,
@@ -17,10 +16,3 @@ module.exports = new RedisLocker({
   metricsPrefix: 'doc',
   lockTTLSeconds: Settings.redisLockTTLSeconds,
 })
-
-module.exports.promises = {
-  checkLock: promisify(module.exports.checkLock.bind(module.exports)),
-  getLock: promisify(module.exports.getLock.bind(module.exports)),
-  releaseLock: promisify(module.exports.releaseLock.bind(module.exports)),
-  tryLock: promisify(module.exports.tryLock.bind(module.exports)),
-}

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

@@ -1,8 +1,9 @@
+const { promisify } = require('util')
+const { promisifyMultiResult } = require('@overleaf/promise-utils')
 const Settings = require('@overleaf/settings')
 const Errors = require('./Errors')
 const Metrics = require('./Metrics')
 const logger = require('@overleaf/logger')
-const { promisifyAll } = require('@overleaf/promise-utils')
 const request = require('requestretry').defaults({
   maxAttempts: 2,
   retryDelay: 10,
@@ -175,10 +176,17 @@ function setDoc(
   )
 }
 
-module.exports = { getDoc, setDoc }
-
-module.exports.promises = promisifyAll(module.exports, {
-  multiResult: {
-    getDoc: ['lines', 'version', 'ranges', 'pathname', 'projectHistoryId'],
+module.exports = {
+  getDoc,
+  setDoc,
+  promises: {
+    getDoc: promisifyMultiResult(getDoc, [
+      'lines',
+      'version',
+      'ranges',
+      'pathname',
+      'projectHistoryId',
+    ]),
+    setDoc: promisify(setDoc),
   },
-})
+}

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

@@ -1,5 +1,5 @@
-let ProjectHistoryRedisManager
 const Settings = require('@overleaf/settings')
+const { promisifyAll } = require('@overleaf/promise-utils')
 const projectHistoryKeys = Settings.redis?.project_history?.key_schema
 const rclient = require('@overleaf/redis-wrapper').createClient(
   Settings.redis.project_history
@@ -8,7 +8,7 @@ const logger = require('@overleaf/logger')
 const metrics = require('./Metrics')
 const { docIsTooLarge } = require('./Limits')
 
-module.exports = ProjectHistoryRedisManager = {
+const ProjectHistoryRedisManager = {
   queueOps(projectId, ...rest) {
     // Record metric for ops pushed onto queue
     const callback = rest.pop()
@@ -172,3 +172,6 @@ module.exports = ProjectHistoryRedisManager = {
     ProjectHistoryRedisManager.queueOps(projectId, jsonUpdate, callback)
   },
 }
+
+module.exports = ProjectHistoryRedisManager
+module.exports.promises = promisifyAll(ProjectHistoryRedisManager)

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

@@ -7,7 +7,7 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-let RangesManager
+const { promisifyAll } = require('@overleaf/promise-utils')
 const RangesTracker = require('@overleaf/ranges-tracker')
 const logger = require('@overleaf/logger')
 const Metrics = require('./Metrics')
@@ -15,7 +15,7 @@ const _ = require('lodash')
 
 const RANGE_DELTA_BUCKETS = [0, 1, 2, 3, 4, 5, 10, 20, 50]
 
-module.exports = RangesManager = {
+const RangesManager = {
   MAX_COMMENTS: 500,
   MAX_CHANGES: 2000,
 
@@ -176,3 +176,11 @@ module.exports = RangesManager = {
     return [emptyCount, totalCount]
   },
 }
+
+module.exports = RangesManager
+module.exports.promises = promisifyAll(RangesManager, {
+  without: ['_getRanges', '_emptyRangesCount'],
+  multiResult: {
+    applyUpdate: ['newRanges', 'rangesWereCollapsed'],
+  },
+})

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

@@ -10,8 +10,8 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-let RealTimeRedisManager
 const Settings = require('@overleaf/settings')
+const { promisifyAll } = require('@overleaf/promise-utils')
 const rclient = require('@overleaf/redis-wrapper').createClient(
   Settings.redis.documentupdater
 )
@@ -30,7 +30,7 @@ let COUNT = 0
 
 const MAX_OPS_PER_ITERATION = 8 // process a limited number of ops for safety
 
-module.exports = RealTimeRedisManager = {
+const RealTimeRedisManager = {
   getPendingUpdatesForDoc(docId, callback) {
     const multi = rclient.multi()
     multi.lrange(
@@ -92,3 +92,8 @@ module.exports = RealTimeRedisManager = {
     }
   },
 }
+
+module.exports = RealTimeRedisManager
+module.exports.promises = promisifyAll(RealTimeRedisManager, {
+  without: ['sendData'],
+})

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

@@ -1,16 +1,15 @@
-let RedisManager
 const Settings = require('@overleaf/settings')
 const rclient = require('@overleaf/redis-wrapper').createClient(
   Settings.redis.documentupdater
 )
 const logger = require('@overleaf/logger')
+const { promisifyAll } = require('@overleaf/promise-utils')
 const metrics = require('./Metrics')
 const Errors = require('./Errors')
 const crypto = require('crypto')
 const async = require('async')
 const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager')
 const { docIsTooLarge } = require('./Limits')
-const { promisifyAll } = require('@overleaf/promise-utils')
 
 // Sometimes Redis calls take an unexpectedly long time.  We have to be
 // quick with Redis calls because we're holding a lock that expires
@@ -28,7 +27,7 @@ const MAX_RANGES_SIZE = 3 * MEGABYTES
 
 const keys = Settings.redis.documentupdater.key_schema
 
-module.exports = RedisManager = {
+const RedisManager = {
   rclient,
 
   putDocInMemory(
@@ -619,7 +618,9 @@ module.exports = RedisManager = {
   },
 }
 
-module.exports.promises = promisifyAll(module.exports, {
+module.exports = RedisManager
+module.exports.promises = promisifyAll(RedisManager, {
+  without: ['_deserializeRanges', '_computeHash'],
   multiResult: {
     getDoc: [
       'lines',

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

@@ -10,11 +10,11 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-let ShareJsUpdateManager
 const ShareJsModel = require('./sharejs/server/model')
 const ShareJsDB = require('./ShareJsDB')
 const logger = require('@overleaf/logger')
 const Settings = require('@overleaf/settings')
+const { promisifyAll } = require('@overleaf/promise-utils')
 const Keys = require('./UpdateKeys')
 const { EventEmitter } = require('events')
 const util = require('util')
@@ -28,7 +28,7 @@ util.inherits(ShareJsModel, EventEmitter)
 
 const MAX_AGE_OF_OP = 80
 
-module.exports = ShareJsUpdateManager = {
+const ShareJsUpdateManager = {
   getNewShareJsModel(projectId, docId, lines, version) {
     const db = new ShareJsDB(projectId, docId, lines, version)
     const model = new ShareJsModel(db, {
@@ -143,3 +143,11 @@ module.exports = ShareJsUpdateManager = {
       .digest('hex')
   },
 }
+
+module.exports = ShareJsUpdateManager
+module.exports.promises = promisifyAll(ShareJsUpdateManager, {
+  without: ['getNewShareJsModel', '_listenForOps', '_sendOp', '_computeHash'],
+  multiResult: {
+    applyUpdate: ['updatedDocLines', 'version', 'appliedOps'],
+  },
+})

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

@@ -10,10 +10,10 @@
  * DS207: Consider shorter variations of null checks
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
-let SnapshotManager
+const { promisifyAll } = require('@overleaf/promise-utils')
 const { db, ObjectId } = require('./mongodb')
 
-module.exports = SnapshotManager = {
+const SnapshotManager = {
   recordSnapshot(projectId, docId, version, pathname, lines, ranges, callback) {
     try {
       projectId = new ObjectId(projectId)
@@ -76,3 +76,8 @@ module.exports = SnapshotManager = {
     }
   },
 }
+
+module.exports = SnapshotManager
+module.exports.promises = promisifyAll(SnapshotManager, {
+  without: ['jsonRangesToMongo', '_safeObjectId'],
+})

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

@@ -1,24 +1,12 @@
-/* eslint-disable
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS201: Simplify complex destructure assignments
- * DS205: Consider reworking code to avoid use of IIFEs
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let UpdateManager
+// @ts-check
+
+const { callbackifyAll } = require('@overleaf/promise-utils')
 const LockManager = require('./LockManager')
 const RedisManager = require('./RedisManager')
 const RealTimeRedisManager = require('./RealTimeRedisManager')
 const ShareJsUpdateManager = require('./ShareJsUpdateManager')
 const HistoryManager = require('./HistoryManager')
-const Settings = require('@overleaf/settings')
 const _ = require('lodash')
-const async = require('async')
 const logger = require('@overleaf/logger')
 const Metrics = require('./Metrics')
 const Errors = require('./Errors')
@@ -27,337 +15,228 @@ const RangesManager = require('./RangesManager')
 const SnapshotManager = require('./SnapshotManager')
 const Profiler = require('./Profiler')
 
-module.exports = UpdateManager = {
-  processOutstandingUpdates(projectId, docId, callback) {
-    if (!callback) {
-      callback = function () {}
-    }
+const UpdateManager = {
+  async processOutstandingUpdates(projectId, docId) {
     const timer = new Metrics.Timer('updateManager.processOutstandingUpdates')
-    UpdateManager.fetchAndApplyUpdates(projectId, docId, function (error) {
-      timer.done()
-      callback(error)
-    })
+    try {
+      await UpdateManager.fetchAndApplyUpdates(projectId, docId)
+      timer.done({ status: 'success' })
+    } catch (err) {
+      timer.done({ status: 'error' })
+      throw err
+    }
   },
 
-  processOutstandingUpdatesWithLock(projectId, docId, callback) {
-    if (!callback) {
-      callback = function () {}
-    }
+  async processOutstandingUpdatesWithLock(projectId, docId) {
     const profile = new Profiler('processOutstandingUpdatesWithLock', {
       project_id: projectId,
       doc_id: docId,
     })
-    LockManager.tryLock(docId, (error, gotLock, lockValue) => {
-      if (error) {
-        return callback(error)
-      }
-      if (!gotLock) {
-        return callback()
-      }
-      profile.log('tryLock')
-      UpdateManager.processOutstandingUpdates(
-        projectId,
-        docId,
-        function (error) {
-          if (error) {
-            return UpdateManager._handleErrorInsideLock(
-              docId,
-              lockValue,
-              error,
-              callback
-            )
-          }
-          profile.log('processOutstandingUpdates')
-          LockManager.releaseLock(docId, lockValue, error => {
-            if (error) {
-              return callback(error)
-            }
-            profile.log('releaseLock').end()
-            UpdateManager.continueProcessingUpdatesWithLock(
-              projectId,
-              docId,
-              callback
-            )
-          })
-        }
-      )
-    })
-  },
 
-  continueProcessingUpdatesWithLock(projectId, docId, callback) {
-    if (!callback) {
-      callback = function () {}
+    const lockValue = await LockManager.promises.tryLock(docId)
+    if (lockValue == null) {
+      return
     }
-    RealTimeRedisManager.getUpdatesLength(docId, (error, length) => {
-      if (error) {
-        return callback(error)
-      }
-      if (length > 0) {
-        UpdateManager.processOutstandingUpdatesWithLock(
-          projectId,
-          docId,
-          callback
-        )
-      } else {
-        callback()
-      }
-    })
+    profile.log('tryLock')
+
+    try {
+      await UpdateManager.processOutstandingUpdates(projectId, docId)
+      profile.log('processOutstandingUpdates')
+    } finally {
+      await LockManager.promises.releaseLock(docId, lockValue)
+      profile.log('releaseLock').end()
+    }
+
+    await UpdateManager.continueProcessingUpdatesWithLock(projectId, docId)
   },
 
-  fetchAndApplyUpdates(projectId, docId, callback) {
-    if (!callback) {
-      callback = function () {}
+  async continueProcessingUpdatesWithLock(projectId, docId) {
+    const length = await RealTimeRedisManager.promises.getUpdatesLength(docId)
+    if (length > 0) {
+      await UpdateManager.processOutstandingUpdatesWithLock(projectId, docId)
     }
+  },
+
+  async fetchAndApplyUpdates(projectId, docId) {
     const profile = new Profiler('fetchAndApplyUpdates', {
       project_id: projectId,
       doc_id: docId,
     })
-    RealTimeRedisManager.getPendingUpdatesForDoc(docId, (error, updates) => {
-      if (error) {
-        return callback(error)
-      }
-      logger.debug(
-        { projectId, docId, count: updates.length },
-        'processing updates'
-      )
-      if (updates.length === 0) {
-        return callback()
-      }
-      profile.log('getPendingUpdatesForDoc')
-      const doUpdate = (update, cb) =>
-        UpdateManager.applyUpdate(projectId, docId, update, function (err) {
-          profile.log('applyUpdate')
-          cb(err)
-        })
-      const finalCallback = function (err) {
-        profile.log('async done').end()
-        callback(err)
-      }
-      async.eachSeries(updates, doUpdate, finalCallback)
-    })
-  },
 
-  applyUpdate(projectId, docId, update, _callback) {
-    if (_callback == null) {
-      _callback = function () {}
+    const updates = await RealTimeRedisManager.promises.getPendingUpdatesForDoc(
+      docId
+    )
+    logger.debug(
+      { projectId, docId, count: updates.length },
+      'processing updates'
+    )
+    if (updates.length === 0) {
+      return
     }
-    const callback = function (error) {
-      if (error) {
-        RealTimeRedisManager.sendData({
-          project_id: projectId,
-          doc_id: docId,
-          error: error.message || error,
-        })
-        profile.log('sendData')
-      }
-      profile.end()
-      _callback(error)
+    profile.log('getPendingUpdatesForDoc')
+
+    for (const update of updates) {
+      await UpdateManager.applyUpdate(projectId, docId, update)
+      profile.log('applyUpdate')
     }
+    profile.log('async done').end()
+  },
 
+  async applyUpdate(projectId, docId, update) {
     const profile = new Profiler('applyUpdate', {
       project_id: projectId,
       doc_id: docId,
     })
+
     UpdateManager._sanitizeUpdate(update)
     profile.log('sanitizeUpdate', { sync: true })
-    DocumentManager.getDoc(
-      projectId,
-      docId,
-      function (error, lines, version, ranges, pathname, projectHistoryId) {
-        profile.log('getDoc')
-        if (error) {
-          return callback(error)
-        }
-        if (lines == null || version == null) {
-          return callback(
-            new Errors.NotFoundError(`document not found: ${docId}`)
-          )
-        }
-        const previousVersion = version
-        const incomingUpdateVersion = update.v
-        ShareJsUpdateManager.applyUpdate(
+
+    try {
+      let { lines, version, ranges, pathname, projectHistoryId } =
+        await DocumentManager.promises.getDoc(projectId, docId)
+      profile.log('getDoc')
+
+      if (lines == null || version == null) {
+        throw new Errors.NotFoundError(`document not found: ${docId}`)
+      }
+
+      const previousVersion = version
+      const incomingUpdateVersion = update.v
+      let updatedDocLines, appliedOps
+      ;({ updatedDocLines, version, appliedOps } =
+        await ShareJsUpdateManager.promises.applyUpdate(
           projectId,
           docId,
           update,
           lines,
-          version,
-          function (error, updatedDocLines, version, appliedOps) {
-            profile.log('sharejs.applyUpdate', {
-              // only synchronous when the update applies directly to the
-              // doc version, otherwise getPreviousDocOps is called.
-              sync: incomingUpdateVersion === previousVersion,
-            })
-            if (error) {
-              return callback(error)
-            }
-            RangesManager.applyUpdate(
-              projectId,
-              docId,
-              ranges,
-              appliedOps,
-              updatedDocLines,
-              function (error, newRanges, rangesWereCollapsed) {
-                UpdateManager._addProjectHistoryMetadataToOps(
-                  appliedOps,
-                  pathname,
-                  projectHistoryId,
-                  lines
-                )
-                profile.log('RangesManager.applyUpdate', { sync: true })
-                if (error) {
-                  return callback(error)
-                }
-                RedisManager.updateDocument(
-                  projectId,
-                  docId,
-                  updatedDocLines,
-                  version,
-                  appliedOps,
-                  newRanges,
-                  update.meta,
-                  function (error, projectOpsLength) {
-                    profile.log('RedisManager.updateDocument')
-                    if (error) {
-                      return callback(error)
-                    }
-                    HistoryManager.recordAndFlushHistoryOps(
-                      projectId,
-                      appliedOps,
-                      projectOpsLength
-                    )
-                    profile.log('recordAndFlushHistoryOps')
-                    if (rangesWereCollapsed) {
-                      Metrics.inc('doc-snapshot')
-                      logger.debug(
-                        {
-                          projectId,
-                          docId,
-                          previousVersion,
-                          lines,
-                          ranges,
-                          update,
-                        },
-                        'update collapsed some ranges, snapshotting previous content'
-                      )
-                      // Do this last, since it's a mongo call, and so potentially longest running
-                      // If it overruns the lock, it's ok, since all of our redis work is done
-                      SnapshotManager.recordSnapshot(
-                        projectId,
-                        docId,
-                        previousVersion,
-                        pathname,
-                        lines,
-                        ranges,
-                        function (error) {
-                          if (error) {
-                            logger.error(
-                              {
-                                err: error,
-                                projectId,
-                                docId,
-                                version,
-                                lines,
-                                ranges,
-                              },
-                              'error recording snapshot'
-                            )
-                            callback(error)
-                          } else {
-                            callback()
-                          }
-                        }
-                      )
-                    } else {
-                      callback()
-                    }
-                  }
-                )
-              }
-            )
-          }
+          version
+        ))
+      profile.log('sharejs.applyUpdate', {
+        // only synchronous when the update applies directly to the
+        // doc version, otherwise getPreviousDocOps is called.
+        sync: incomingUpdateVersion === previousVersion,
+      })
+
+      const { newRanges, rangesWereCollapsed } =
+        await RangesManager.promises.applyUpdate(
+          projectId,
+          docId,
+          ranges,
+          appliedOps,
+          updatedDocLines
+        )
+      profile.log('RangesManager.applyUpdate', { sync: true })
+
+      UpdateManager._addProjectHistoryMetadataToOps(
+        appliedOps,
+        pathname,
+        projectHistoryId,
+        lines
+      )
+
+      const projectOpsLength = await RedisManager.promises.updateDocument(
+        projectId,
+        docId,
+        updatedDocLines,
+        version,
+        appliedOps,
+        newRanges,
+        update.meta
+      )
+      profile.log('RedisManager.updateDocument')
+
+      HistoryManager.recordAndFlushHistoryOps(
+        projectId,
+        appliedOps,
+        projectOpsLength
+      )
+      profile.log('recordAndFlushHistoryOps')
+
+      if (rangesWereCollapsed) {
+        Metrics.inc('doc-snapshot')
+        logger.debug(
+          {
+            projectId,
+            docId,
+            previousVersion,
+            lines,
+            ranges,
+            update,
+          },
+          'update collapsed some ranges, snapshotting previous content'
+        )
+
+        // Do this last, since it's a mongo call, and so potentially longest running
+        // If it overruns the lock, it's ok, since all of our redis work is done
+        await SnapshotManager.promises.recordSnapshot(
+          projectId,
+          docId,
+          previousVersion,
+          pathname,
+          lines,
+          ranges
         )
       }
-    )
+    } catch (error) {
+      RealTimeRedisManager.sendData({
+        project_id: projectId,
+        doc_id: docId,
+        error: error instanceof Error ? error.message : error,
+      })
+      profile.log('sendData')
+      throw error
+    } finally {
+      profile.end()
+    }
   },
 
-  lockUpdatesAndDo(method, projectId, docId, ...rest) {
-    const adjustedLength = Math.max(rest.length, 1)
-    const args = rest.slice(0, adjustedLength - 1)
-    const callback = rest[adjustedLength - 1]
+  // lockUpdatesAndDo can't be promisified yet because it expects a
+  // callback-style function
+  async lockUpdatesAndDo(method, projectId, docId, ...args) {
     const profile = new Profiler('lockUpdatesAndDo', {
       project_id: projectId,
       doc_id: docId,
     })
-    return LockManager.getLock(docId, function (error, lockValue) {
-      profile.log('getLock')
-      if (error) {
-        return callback(error)
-      }
-      UpdateManager.processOutstandingUpdates(
-        projectId,
-        docId,
-        function (error) {
+
+    const lockValue = await LockManager.promises.getLock(docId)
+    profile.log('getLock')
+
+    let responseArgs
+    try {
+      await UpdateManager.processOutstandingUpdates(projectId, docId)
+      profile.log('processOutstandingUpdates')
+
+      // TODO: method is still a callback-style function. Change this when promisifying DocumentManager
+      responseArgs = await new Promise((resolve, reject) => {
+        method(projectId, docId, ...args, (error, ...responseArgs) => {
           if (error) {
-            return UpdateManager._handleErrorInsideLock(
-              docId,
-              lockValue,
-              error,
-              callback
-            )
+            reject(error)
+          } else {
+            resolve(responseArgs)
           }
-          profile.log('processOutstandingUpdates')
-          method(
-            projectId,
-            docId,
-            ...Array.from(args),
-            function (error, ...responseArgs) {
-              if (error) {
-                return UpdateManager._handleErrorInsideLock(
-                  docId,
-                  lockValue,
-                  error,
-                  callback
-                )
-              }
-              profile.log('method')
-              LockManager.releaseLock(docId, lockValue, function (error) {
-                if (error) {
-                  return callback(error)
-                }
-                profile.log('releaseLock').end()
-                callback(null, ...Array.from(responseArgs))
-                // We held the lock for a while so updates might have queued up
-                UpdateManager.continueProcessingUpdatesWithLock(
-                  projectId,
-                  docId,
-                  err => {
-                    if (err) {
-                      // The processing may fail for invalid user updates.
-                      // This can be very noisy, put them on level DEBUG
-                      //  and record a metric.
-                      Metrics.inc('background-processing-updates-error')
-                      logger.debug(
-                        { err, projectId, docId },
-                        'error processing updates in background'
-                      )
-                    }
-                  }
-                )
-              })
-            }
-          )
-        }
-      )
-    })
-  },
-
-  _handleErrorInsideLock(docId, lockValue, originalError, callback) {
-    if (!callback) {
-      callback = function () {}
+        })
+      })
+      profile.log('method')
+    } finally {
+      await LockManager.promises.releaseLock(docId, lockValue)
+      profile.log('releaseLock').end()
     }
-    LockManager.releaseLock(docId, lockValue, lockError =>
-      callback(originalError)
+
+    // We held the lock for a while so updates might have queued up
+    UpdateManager.continueProcessingUpdatesWithLock(projectId, docId).catch(
+      err => {
+        // The processing may fail for invalid user updates.
+        // This can be very noisy, put them on level DEBUG
+        //  and record a metric.
+        Metrics.inc('background-processing-updates-error')
+        logger.debug(
+          { err, projectId, docId },
+          'error processing updates in background'
+        )
+      }
     )
+
+    return responseArgs
   },
 
   _sanitizeUpdate(update) {
@@ -372,7 +251,7 @@ module.exports = UpdateManager = {
     // 16-bit character of a blackboard bold character (http://www.fileformat.info/info/unicode/char/1d400/index.htm).
     // Something must be going on client side that is screwing up the encoding and splitting the
     // two 16-bit characters so that \uD835 is standalone.
-    for (const op of Array.from(update.op || [])) {
+    for (const op of update.op || []) {
       if (op.i != null) {
         // Replace high and low surrogate characters with 'replacement character' (\uFFFD)
         op.i = op.i.replace(/[\uD800-\uDFFF]/g, '\uFFFD')
@@ -384,7 +263,7 @@ module.exports = UpdateManager = {
   _addProjectHistoryMetadataToOps(updates, pathname, projectHistoryId, lines) {
     let docLength = _.reduce(lines, (chars, line) => chars + line.length, 0)
     docLength += lines.length - 1 // count newline characters
-    return updates.forEach(function (update) {
+    updates.forEach(function (update) {
       update.projectHistoryId = projectHistoryId
       if (!update.meta) {
         update.meta = {}
@@ -400,20 +279,41 @@ module.exports = UpdateManager = {
       // We want to include the doc_length at the start of each update,
       // before it's ops are applied. However, we need to track any
       // changes to it for the next update.
-      return (() => {
-        const result = []
-        for (const op of Array.from(update.op)) {
-          if (op.i != null) {
-            docLength += op.i.length
-          }
-          if (op.d != null) {
-            result.push((docLength -= op.d.length))
-          } else {
-            result.push(undefined)
-          }
+      for (const op of update.op) {
+        if (op.i != null) {
+          docLength += op.i.length
         }
-        return result
-      })()
+        if (op.d != null) {
+          docLength -= op.d.length
+        }
+      }
     })
   },
 }
+
+const CallbackifiedUpdateManager = callbackifyAll(UpdateManager)
+
+module.exports = CallbackifiedUpdateManager
+module.exports.promises = UpdateManager
+
+module.exports.lockUpdatesAndDo = function lockUpdatesAndDo(
+  method,
+  projectId,
+  docId,
+  ...rest
+) {
+  const adjustedLength = Math.max(rest.length, 1)
+  const args = rest.slice(0, adjustedLength - 1)
+  const callback = rest[adjustedLength - 1]
+
+  // TODO: During the transition to promises, UpdateManager.lockUpdatesAndDo
+  // returns the potentially multiple arguments that must be provided to the
+  // callback in an array.
+  UpdateManager.lockUpdatesAndDo(method, projectId, docId, ...args)
+    .then(responseArgs => {
+      callback(null, ...responseArgs)
+    })
+    .catch(err => {
+      callback(err)
+    })
+}

services/document-updater/package.json

@@ -25,6 +25,7 @@
     "@overleaf/ranges-tracker": "*",
     "@overleaf/redis-wrapper": "*",
     "@overleaf/settings": "*",
+    "@types/chai-as-promised": "^7.1.8",
     "async": "^3.2.2",
     "body-parser": "^1.19.0",
     "bunyan": "^1.8.15",

services/document-updater/test/setup.js

@@ -1,12 +1,15 @@
 const chai = require('chai')
+const chaiAsPromised = require('chai-as-promised')
+const sinonChai = require('sinon-chai')
 const SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
 
 // Chai configuration
 chai.should()
+chai.use(chaiAsPromised)
 // Load sinon-chai assertions so expect(stubFn).to.have.been.calledWith('abc')
 // has a nicer failure messages
-chai.use(require('sinon-chai'))
+chai.use(sinonChai)
 
 // Global stubs
 const sandbox = sinon.createSandbox()

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

@@ -1,82 +1,113 @@
-/* eslint-disable
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS206: Consider reworking classes to avoid initClass
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
+// @ts-check
+
 const sinon = require('sinon')
-const modulePath = '../../../../app/js/UpdateManager.js'
+const { expect } = require('chai')
 const SandboxedModule = require('sandboxed-module')
 
+const MODULE_PATH = '../../../../app/js/UpdateManager.js'
+
 describe('UpdateManager', function () {
   beforeEach(function () {
-    let Profiler, Timer
     this.project_id = 'project-id-123'
     this.projectHistoryId = 'history-id-123'
     this.doc_id = 'document-id-123'
-    this.callback = sinon.stub()
-    this.UpdateManager = SandboxedModule.require(modulePath, {
+    this.lockValue = 'mock-lock-value'
+
+    this.Metrics = {
+      inc: sinon.stub(),
+      Timer: class Timer {},
+    }
+    this.Metrics.Timer.prototype.done = sinon.stub()
+
+    this.Profiler = class Profiler {}
+    this.Profiler.prototype.log = sinon.stub().returns({ end: sinon.stub() })
+    this.Profiler.prototype.end = sinon.stub()
+
+    this.LockManager = {
+      promises: {
+        tryLock: sinon.stub().resolves(this.lockValue),
+        getLock: sinon.stub().resolves(this.lockValue),
+        releaseLock: sinon.stub().resolves(),
+      },
+    }
+
+    this.RedisManager = {
+      promises: {
+        setDocument: sinon.stub().resolves(),
+        updateDocument: sinon.stub(),
+      },
+    }
+
+    this.RealTimeRedisManager = {
+      sendData: sinon.stub(),
+      promises: {
+        getUpdatesLength: sinon.stub(),
+        getPendingUpdatesForDoc: sinon.stub(),
+      },
+    }
+
+    this.ShareJsUpdateManager = {
+      promises: {
+        applyUpdate: sinon.stub(),
+      },
+    }
+
+    this.HistoryManager = {
+      recordAndFlushHistoryOps: sinon.stub(),
+    }
+
+    this.Settings = {}
+
+    this.DocumentManager = {
+      promises: {
+        getDoc: sinon.stub(),
+      },
+    }
+
+    this.RangesManager = {
+      promises: {
+        applyUpdate: sinon.stub(),
+      },
+    }
+
+    this.SnapshotManager = {
+      promises: {
+        recordSnapshot: sinon.stub().resolves(),
+      },
+    }
+
+    this.UpdateManager = SandboxedModule.require(MODULE_PATH, {
       requires: {
-        './LockManager': (this.LockManager = {}),
-        './RedisManager': (this.RedisManager = {}),
-        './RealTimeRedisManager': (this.RealTimeRedisManager = {}),
-        './ShareJsUpdateManager': (this.ShareJsUpdateManager = {}),
-        './HistoryManager': (this.HistoryManager = {}),
-        './Metrics': (this.Metrics = {
-          inc: sinon.stub(),
-          Timer: (Timer = (function () {
-            Timer = class Timer {
-              static initClass() {
-                this.prototype.done = sinon.stub()
-              }
-            }
-            Timer.initClass()
-            return Timer
-          })()),
-        }),
-        '@overleaf/settings': (this.Settings = {}),
-        './DocumentManager': (this.DocumentManager = {}),
-        './RangesManager': (this.RangesManager = {}),
-        './SnapshotManager': (this.SnapshotManager = {}),
-        './Profiler': (Profiler = (function () {
-          Profiler = class Profiler {
-            static initClass() {
-              this.prototype.log = sinon.stub().returns({ end: sinon.stub() })
-              this.prototype.end = sinon.stub()
-            }
-          }
-          Profiler.initClass()
-          return Profiler
-        })()),
+        './LockManager': this.LockManager,
+        './RedisManager': this.RedisManager,
+        './RealTimeRedisManager': this.RealTimeRedisManager,
+        './ShareJsUpdateManager': this.ShareJsUpdateManager,
+        './HistoryManager': this.HistoryManager,
+        './Metrics': this.Metrics,
+        '@overleaf/settings': this.Settings,
+        './DocumentManager': this.DocumentManager,
+        './RangesManager': this.RangesManager,
+        './SnapshotManager': this.SnapshotManager,
+        './Profiler': this.Profiler,
       },
     })
   })
 
   describe('processOutstandingUpdates', function () {
-    beforeEach(function () {
-      this.UpdateManager.fetchAndApplyUpdates = sinon.stub().callsArg(2)
-      this.UpdateManager.processOutstandingUpdates(
+    beforeEach(async function () {
+      this.UpdateManager.promises.fetchAndApplyUpdates = sinon.stub().resolves()
+      await this.UpdateManager.promises.processOutstandingUpdates(
         this.project_id,
-        this.doc_id,
-        this.callback
+        this.doc_id
       )
     })
 
     it('should apply the updates', function () {
-      this.UpdateManager.fetchAndApplyUpdates
+      this.UpdateManager.promises.fetchAndApplyUpdates
         .calledWith(this.project_id, this.doc_id)
         .should.equal(true)
     })
 
-    it('should call the callback', function () {
-      this.callback.called.should.equal(true)
-    })
-
     it('should time the execution', function () {
       this.Metrics.Timer.prototype.done.called.should.equal(true)
     })
@@ -85,219 +116,184 @@ describe('UpdateManager', function () {
   describe('processOutstandingUpdatesWithLock', function () {
     describe('when the lock is free', function () {
       beforeEach(function () {
-        this.LockManager.tryLock = sinon
+        this.UpdateManager.promises.continueProcessingUpdatesWithLock = sinon
           .stub()
-          .callsArgWith(1, null, true, (this.lockValue = 'mock-lock-value'))
-        this.LockManager.releaseLock = sinon.stub().callsArg(2)
-        this.UpdateManager.continueProcessingUpdatesWithLock = sinon
+          .resolves()
+        this.UpdateManager.promises.processOutstandingUpdates = sinon
           .stub()
-          .callsArg(2)
-        this.UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
+          .resolves()
       })
 
       describe('successfully', function () {
-        beforeEach(function () {
-          this.UpdateManager.processOutstandingUpdatesWithLock(
+        beforeEach(async function () {
+          await this.UpdateManager.promises.processOutstandingUpdatesWithLock(
             this.project_id,
-            this.doc_id,
-            this.callback
+            this.doc_id
           )
         })
 
         it('should acquire the lock', function () {
-          this.LockManager.tryLock.calledWith(this.doc_id).should.equal(true)
+          this.LockManager.promises.tryLock
+            .calledWith(this.doc_id)
+            .should.equal(true)
         })
 
         it('should free the lock', function () {
-          this.LockManager.releaseLock
+          this.LockManager.promises.releaseLock
             .calledWith(this.doc_id, this.lockValue)
             .should.equal(true)
         })
 
         it('should process the outstanding updates', function () {
-          this.UpdateManager.processOutstandingUpdates
+          this.UpdateManager.promises.processOutstandingUpdates
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
 
         it('should do everything with the lock acquired', function () {
-          this.UpdateManager.processOutstandingUpdates
-            .calledAfter(this.LockManager.tryLock)
+          this.UpdateManager.promises.processOutstandingUpdates
+            .calledAfter(this.LockManager.promises.tryLock)
             .should.equal(true)
-          this.UpdateManager.processOutstandingUpdates
-            .calledBefore(this.LockManager.releaseLock)
+          this.UpdateManager.promises.processOutstandingUpdates
+            .calledBefore(this.LockManager.promises.releaseLock)
             .should.equal(true)
         })
 
         it('should continue processing new updates that may have come in', function () {
-          this.UpdateManager.continueProcessingUpdatesWithLock
+          this.UpdateManager.promises.continueProcessingUpdatesWithLock
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
-
-        it('should return the callback', function () {
-          this.callback.called.should.equal(true)
-        })
       })
 
       describe('when processOutstandingUpdates returns an error', function () {
-        beforeEach(function () {
-          this.UpdateManager.processOutstandingUpdates = sinon
+        beforeEach(async function () {
+          this.error = new Error('Something went wrong')
+          this.UpdateManager.promises.processOutstandingUpdates = sinon
             .stub()
-            .callsArgWith(2, (this.error = new Error('Something went wrong')))
-          this.UpdateManager.processOutstandingUpdatesWithLock(
-            this.project_id,
-            this.doc_id,
-            this.callback
-          )
+            .rejects(this.error)
+          await expect(
+            this.UpdateManager.promises.processOutstandingUpdatesWithLock(
+              this.project_id,
+              this.doc_id
+            )
+          ).to.be.rejectedWith(this.error)
         })
 
         it('should free the lock', function () {
-          this.LockManager.releaseLock
+          this.LockManager.promises.releaseLock
             .calledWith(this.doc_id, this.lockValue)
             .should.equal(true)
         })
-
-        it('should return the error in the callback', function () {
-          this.callback.calledWith(this.error).should.equal(true)
-        })
       })
     })
 
     describe('when the lock is taken', function () {
-      beforeEach(function () {
-        this.LockManager.tryLock = sinon.stub().callsArgWith(1, null, false)
-        this.UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
-        this.UpdateManager.processOutstandingUpdatesWithLock(
+      beforeEach(async function () {
+        this.LockManager.promises.tryLock.resolves(null)
+        this.UpdateManager.promises.processOutstandingUpdates = sinon
+          .stub()
+          .resolves()
+        await this.UpdateManager.promises.processOutstandingUpdatesWithLock(
           this.project_id,
-          this.doc_id,
-          this.callback
+          this.doc_id
         )
       })
 
-      it('should return the callback', function () {
-        this.callback.called.should.equal(true)
-      })
-
       it('should not process the updates', function () {
-        this.UpdateManager.processOutstandingUpdates.called.should.equal(false)
+        this.UpdateManager.promises.processOutstandingUpdates.called.should.equal(
+          false
+        )
       })
     })
   })
 
   describe('continueProcessingUpdatesWithLock', function () {
     describe('when there are outstanding updates', function () {
-      beforeEach(function () {
-        this.RealTimeRedisManager.getUpdatesLength = sinon
-          .stub()
-          .callsArgWith(1, null, 3)
-        this.UpdateManager.processOutstandingUpdatesWithLock = sinon
+      beforeEach(async function () {
+        this.RealTimeRedisManager.promises.getUpdatesLength.resolves(3)
+        this.UpdateManager.promises.processOutstandingUpdatesWithLock = sinon
           .stub()
-          .callsArg(2)
-        this.UpdateManager.continueProcessingUpdatesWithLock(
+          .resolves()
+        await this.UpdateManager.promises.continueProcessingUpdatesWithLock(
           this.project_id,
-          this.doc_id,
-          this.callback
+          this.doc_id
         )
       })
 
       it('should process the outstanding updates', function () {
-        this.UpdateManager.processOutstandingUpdatesWithLock
+        this.UpdateManager.promises.processOutstandingUpdatesWithLock
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
-
-      it('should return the callback', function () {
-        this.callback.called.should.equal(true)
-      })
     })
 
     describe('when there are no outstanding updates', function () {
-      beforeEach(function () {
-        this.RealTimeRedisManager.getUpdatesLength = sinon
+      beforeEach(async function () {
+        this.RealTimeRedisManager.promises.getUpdatesLength.resolves(0)
+        this.UpdateManager.promises.processOutstandingUpdatesWithLock = sinon
           .stub()
-          .callsArgWith(1, null, 0)
-        this.UpdateManager.processOutstandingUpdatesWithLock = sinon
-          .stub()
-          .callsArg(2)
-        this.UpdateManager.continueProcessingUpdatesWithLock(
+          .resolves()
+        await this.UpdateManager.promises.continueProcessingUpdatesWithLock(
           this.project_id,
-          this.doc_id,
-          this.callback
+          this.doc_id
         )
       })
 
       it('should not try to process the outstanding updates', function () {
-        this.UpdateManager.processOutstandingUpdatesWithLock.called.should.equal(
+        this.UpdateManager.promises.processOutstandingUpdatesWithLock.called.should.equal(
           false
         )
       })
-
-      it('should return the callback', function () {
-        this.callback.called.should.equal(true)
-      })
     })
   })
 
   describe('fetchAndApplyUpdates', function () {
     describe('with updates', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.updates = [{ p: 1, t: 'foo' }]
         this.updatedDocLines = ['updated', 'lines']
         this.version = 34
-        this.RealTimeRedisManager.getPendingUpdatesForDoc = sinon
-          .stub()
-          .callsArgWith(1, null, this.updates)
-        this.UpdateManager.applyUpdate = sinon
-          .stub()
-          .callsArgWith(3, null, this.updatedDocLines, this.version)
-        this.UpdateManager.fetchAndApplyUpdates(
+        this.RealTimeRedisManager.promises.getPendingUpdatesForDoc.resolves(
+          this.updates
+        )
+        this.UpdateManager.promises.applyUpdate = sinon.stub().resolves()
+        await this.UpdateManager.promises.fetchAndApplyUpdates(
           this.project_id,
-          this.doc_id,
-          this.callback
+          this.doc_id
         )
       })
 
       it('should get the pending updates', function () {
-        this.RealTimeRedisManager.getPendingUpdatesForDoc
+        this.RealTimeRedisManager.promises.getPendingUpdatesForDoc
           .calledWith(this.doc_id)
           .should.equal(true)
       })
 
       it('should apply the updates', function () {
-        Array.from(this.updates).map(update =>
-          this.UpdateManager.applyUpdate
+        this.updates.map(update =>
+          this.UpdateManager.promises.applyUpdate
             .calledWith(this.project_id, this.doc_id, update)
             .should.equal(true)
         )
       })
-
-      it('should call the callback', function () {
-        this.callback.called.should.equal(true)
-      })
     })
 
     describe('when there are no updates', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.updates = []
-        this.RealTimeRedisManager.getPendingUpdatesForDoc = sinon
-          .stub()
-          .callsArgWith(1, null, this.updates)
-        this.UpdateManager.applyUpdate = sinon.stub()
-        this.RedisManager.setDocument = sinon.stub()
-        this.UpdateManager.fetchAndApplyUpdates(
+        this.RealTimeRedisManager.promises.getPendingUpdatesForDoc.resolves(
+          this.updates
+        )
+        this.UpdateManager.promises.applyUpdate = sinon.stub().resolves()
+        await this.UpdateManager.promises.fetchAndApplyUpdates(
           this.project_id,
-          this.doc_id,
-          this.callback
+          this.doc_id
         )
       })
 
       it('should not call applyUpdate', function () {
-        this.UpdateManager.applyUpdate.called.should.equal(false)
-      })
-
-      it('should call the callback', function () {
-        this.callback.called.should.equal(true)
+        this.UpdateManager.promises.applyUpdate.called.should.equal(false)
       })
     })
   })
@@ -315,44 +311,41 @@ describe('UpdateManager', function () {
         { v: 42, op: 'mock-op-42' },
         { v: 45, op: 'mock-op-45' },
       ]
-      this.project_ops_length = sinon.stub()
+      this.project_ops_length = 123
       this.pathname = '/a/b/c.tex'
-      this.DocumentManager.getDoc = sinon
-        .stub()
-        .yields(
-          null,
-          this.lines,
-          this.version,
-          this.ranges,
-          this.pathname,
-          this.projectHistoryId
-        )
-      this.RangesManager.applyUpdate = sinon
-        .stub()
-        .yields(null, this.updated_ranges, false)
-      this.ShareJsUpdateManager.applyUpdate = sinon
-        .stub()
-        .yields(null, this.updatedDocLines, this.version, this.appliedOps)
-      this.RedisManager.updateDocument = sinon
-        .stub()
-        .yields(null, this.project_ops_length)
-      this.RealTimeRedisManager.sendData = sinon.stub()
-      this.UpdateManager._addProjectHistoryMetadataToOps = sinon.stub()
-      this.HistoryManager.recordAndFlushHistoryOps = sinon.stub()
+      this.DocumentManager.promises.getDoc.resolves({
+        lines: this.lines,
+        version: this.version,
+        ranges: this.ranges,
+        pathname: this.pathname,
+        projectHistoryId: this.projectHistoryId,
+      })
+      this.RangesManager.promises.applyUpdate.resolves({
+        newRanges: this.updated_ranges,
+        rangesWereCollapsed: false,
+      })
+      this.ShareJsUpdateManager.promises.applyUpdate = sinon.stub().resolves({
+        updatedDocLines: this.updatedDocLines,
+        version: this.version,
+        appliedOps: this.appliedOps,
+      })
+      this.RedisManager.promises.updateDocument.resolves(
+        this.project_ops_length
+      )
+      this.UpdateManager.promises._addProjectHistoryMetadataToOps = sinon.stub()
     })
 
     describe('normally', function () {
-      beforeEach(function () {
-        this.UpdateManager.applyUpdate(
+      beforeEach(async function () {
+        await this.UpdateManager.promises.applyUpdate(
           this.project_id,
           this.doc_id,
-          this.update,
-          this.callback
+          this.update
         )
       })
 
       it('should apply the updates via ShareJS', function () {
-        this.ShareJsUpdateManager.applyUpdate
+        this.ShareJsUpdateManager.promises.applyUpdate
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -364,7 +357,7 @@ describe('UpdateManager', function () {
       })
 
       it('should update the ranges', function () {
-        this.RangesManager.applyUpdate
+        this.RangesManager.promises.applyUpdate
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -376,7 +369,7 @@ describe('UpdateManager', function () {
       })
 
       it('should save the document', function () {
-        this.RedisManager.updateDocument
+        this.RedisManager.promises.updateDocument
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -390,14 +383,12 @@ describe('UpdateManager', function () {
       })
 
       it('should add metadata to the ops', function () {
-        this.UpdateManager._addProjectHistoryMetadataToOps
-          .calledWith(
-            this.appliedOps,
-            this.pathname,
-            this.projectHistoryId,
-            this.lines
-          )
-          .should.equal(true)
+        this.UpdateManager.promises._addProjectHistoryMetadataToOps.should.have.been.calledWith(
+          this.appliedOps,
+          this.pathname,
+          this.projectHistoryId,
+          this.lines
+        )
       })
 
       it('should push the applied ops into the history queue', function () {
@@ -405,25 +396,20 @@ describe('UpdateManager', function () {
           .calledWith(this.project_id, this.appliedOps, this.project_ops_length)
           .should.equal(true)
       })
-
-      it('should call the callback', function () {
-        this.callback.called.should.equal(true)
-      })
     })
 
     describe('with UTF-16 surrogate pairs in the update', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.update = { op: [{ p: 42, i: '\uD835\uDC00' }] }
-        this.UpdateManager.applyUpdate(
+        await this.UpdateManager.promises.applyUpdate(
           this.project_id,
           this.doc_id,
-          this.update,
-          this.callback
+          this.update
         )
       })
 
       it('should apply the update but with surrogate pairs removed', function () {
-        this.ShareJsUpdateManager.applyUpdate
+        this.ShareJsUpdateManager.promises.applyUpdate
           .calledWith(this.project_id, this.doc_id, this.update)
           .should.equal(true)
 
@@ -433,15 +419,16 @@ describe('UpdateManager', function () {
     })
 
     describe('with an error', function () {
-      beforeEach(function () {
+      beforeEach(async function () {
         this.error = new Error('something went wrong')
-        this.ShareJsUpdateManager.applyUpdate = sinon.stub().yields(this.error)
-        this.UpdateManager.applyUpdate(
-          this.project_id,
-          this.doc_id,
-          this.update,
-          this.callback
-        )
+        this.ShareJsUpdateManager.promises.applyUpdate.rejects(this.error)
+        await expect(
+          this.UpdateManager.promises.applyUpdate(
+            this.project_id,
+            this.doc_id,
+            this.update
+          )
+        ).to.be.rejectedWith(this.error)
       })
 
       it('should call RealTimeRedisManager.sendData with the error', function () {
@@ -453,23 +440,18 @@ describe('UpdateManager', function () {
           })
           .should.equal(true)
       })
-
-      it('should call the callback with the error', function () {
-        this.callback.calledWith(this.error).should.equal(true)
-      })
     })
 
     describe('when ranges get collapsed', function () {
-      beforeEach(function () {
-        this.RangesManager.applyUpdate = sinon
-          .stub()
-          .yields(null, this.updated_ranges, true)
-        this.SnapshotManager.recordSnapshot = sinon.stub().yields()
-        this.UpdateManager.applyUpdate(
+      beforeEach(async function () {
+        this.RangesManager.promises.applyUpdate.resolves({
+          newRanges: this.updated_ranges,
+          rangesWereCollapsed: true,
+        })
+        await this.UpdateManager.promises.applyUpdate(
           this.project_id,
           this.doc_id,
-          this.update,
-          this.callback
+          this.update
         )
       })
 
@@ -478,7 +460,7 @@ describe('UpdateManager', function () {
       })
 
       it('should call SnapshotManager.recordSnapshot', function () {
-        this.SnapshotManager.recordSnapshot
+        this.SnapshotManager.promises.recordSnapshot
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -558,38 +540,41 @@ describe('UpdateManager', function () {
 
   describe('lockUpdatesAndDo', function () {
     beforeEach(function () {
-      this.method = sinon.stub().callsArgWith(3, null, this.response_arg1)
-      this.callback = sinon.stub()
+      this.method = sinon
+        .stub()
+        .yields(null, this.response_arg1, this.response_arg2)
       this.arg1 = 'argument 1'
       this.response_arg1 = 'response argument 1'
-      this.lockValue = 'mock-lock-value'
-      this.LockManager.getLock = sinon
-        .stub()
-        .callsArgWith(1, null, this.lockValue)
-      this.LockManager.releaseLock = sinon.stub().callsArg(2)
+      this.response_arg2 = 'response argument 2'
     })
 
     describe('successfully', function () {
-      beforeEach(function () {
-        this.UpdateManager.continueProcessingUpdatesWithLock = sinon.stub()
-        this.UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
-        this.UpdateManager.lockUpdatesAndDo(
+      beforeEach(async function () {
+        this.UpdateManager.promises.continueProcessingUpdatesWithLock = sinon
+          .stub()
+          .resolves()
+        this.UpdateManager.promises.processOutstandingUpdates = sinon
+          .stub()
+          .resolves()
+        this.response = await this.UpdateManager.promises.lockUpdatesAndDo(
           this.method,
           this.project_id,
           this.doc_id,
-          this.arg1,
-          this.callback
+          this.arg1
         )
       })
 
       it('should lock the doc', function () {
-        this.LockManager.getLock.calledWith(this.doc_id).should.equal(true)
+        this.LockManager.promises.getLock
+          .calledWith(this.doc_id)
+          .should.equal(true)
       })
 
       it('should process any outstanding updates', function () {
-        this.UpdateManager.processOutstandingUpdates
-          .calledWith(this.project_id, this.doc_id)
-          .should.equal(true)
+        this.UpdateManager.promises.processOutstandingUpdates.should.have.been.calledWith(
+          this.project_id,
+          this.doc_id
+        )
       })
 
       it('should call the method', function () {
@@ -598,76 +583,71 @@ describe('UpdateManager', function () {
           .should.equal(true)
       })
 
-      it('should return the method response to the callback', function () {
-        this.callback.calledWith(null, this.response_arg1).should.equal(true)
+      it('should return the method response arguments', function () {
+        expect(this.response).to.deep.equal([
+          this.response_arg1,
+          this.response_arg2,
+        ])
       })
 
       it('should release the lock', function () {
-        this.LockManager.releaseLock
+        this.LockManager.promises.releaseLock
           .calledWith(this.doc_id, this.lockValue)
           .should.equal(true)
       })
 
       it('should continue processing updates', function () {
-        this.UpdateManager.continueProcessingUpdatesWithLock
+        this.UpdateManager.promises.continueProcessingUpdatesWithLock
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
     })
 
     describe('when processOutstandingUpdates returns an error', function () {
-      beforeEach(function () {
-        this.UpdateManager.processOutstandingUpdates = sinon
+      beforeEach(async function () {
+        this.error = new Error('Something went wrong')
+        this.UpdateManager.promises.processOutstandingUpdates = sinon
           .stub()
-          .callsArgWith(2, (this.error = new Error('Something went wrong')))
-        this.UpdateManager.lockUpdatesAndDo(
-          this.method,
-          this.project_id,
-          this.doc_id,
-          this.arg1,
-          this.callback
-        )
+          .rejects(this.error)
+        await expect(
+          this.UpdateManager.promises.lockUpdatesAndDo(
+            this.method,
+            this.project_id,
+            this.doc_id,
+            this.arg1
+          )
+        ).to.be.rejectedWith(this.error)
       })
 
       it('should free the lock', function () {
-        this.LockManager.releaseLock
+        this.LockManager.promises.releaseLock
           .calledWith(this.doc_id, this.lockValue)
           .should.equal(true)
       })
-
-      it('should return the error in the callback', function () {
-        this.callback.calledWith(this.error).should.equal(true)
-      })
     })
 
     describe('when the method returns an error', function () {
-      beforeEach(function () {
-        this.UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
-        this.method = sinon
+      beforeEach(async function () {
+        this.error = new Error('something went wrong')
+        this.UpdateManager.promises.processOutstandingUpdates = sinon
           .stub()
-          .callsArgWith(
-            3,
-            (this.error = new Error('something went wrong')),
-            this.response_arg1
+          .resolves()
+        this.method = sinon.stub().yields(this.error)
+        await expect(
+          this.UpdateManager.promises.lockUpdatesAndDo(
+            this.method,
+            this.project_id,
+            this.doc_id,
+            this.arg1
           )
-        this.UpdateManager.lockUpdatesAndDo(
-          this.method,
-          this.project_id,
-          this.doc_id,
-          this.arg1,
-          this.callback
-        )
+        ).to.be.rejectedWith(this.error)
       })
 
       it('should free the lock', function () {
-        this.LockManager.releaseLock
+        this.LockManager.promises.releaseLock
           .calledWith(this.doc_id, this.lockValue)
           .should.equal(true)
       })
-
-      it('should return the error in the callback', function () {
-        this.callback.calledWith(this.error).should.equal(true)
-      })
     })
   })
 })