Просмотр исходного кода

Merge pull request #28205 from overleaf/em-promisify-project-manager

Promisify ProjectManager

GitOrigin-RevId: d10807a9a7f11428dd6bfe41210bfd0ce32c37ba
Eric Mc Sween 1 год назад
Родитель
Сommit
2995b29829

+ 159 - 257
services/document-updater/app/js/ProjectManager.js

@@ -2,334 +2,233 @@ const RedisManager = require('./RedisManager')
 const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager')
 const DocumentManager = require('./DocumentManager')
 const HistoryManager = require('./HistoryManager')
-const async = require('async')
 const logger = require('@overleaf/logger')
 const Metrics = require('./Metrics')
 const Errors = require('./Errors')
-const { promisifyAll } = require('@overleaf/promise-utils')
+const { callbackifyAll } = require('@overleaf/promise-utils')
 
-function flushProjectWithLocks(projectId, _callback) {
+async function flushProjectWithLocks(projectId) {
   const timer = new Metrics.Timer('projectManager.flushProjectWithLocks')
-  const callback = function (...args) {
-    timer.done()
-    _callback(...args)
-  }
-
-  RedisManager.getDocIdsInProject(projectId, (error, docIds) => {
-    if (error) {
-      return callback(error)
-    }
-    const errors = []
-    const jobs = docIds.map(docId => callback => {
-      DocumentManager.flushDocIfLoadedWithLock(projectId, docId, error => {
-        if (error instanceof Errors.NotFoundError) {
-          logger.warn(
-            { err: error, projectId, docId },
-            'found deleted doc when flushing'
-          )
-          callback()
-        } else if (error) {
-          logger.error({ err: error, projectId, docId }, 'error flushing doc')
-          errors.push(error)
-          callback()
-        } else {
-          callback()
-        }
-      })
-    })
-
-    logger.debug({ projectId, docIds }, 'flushing docs')
-    async.series(jobs, () => {
-      if (errors.length > 0) {
-        callback(new Error('Errors flushing docs. See log for details'))
+  const docIds = await RedisManager.promises.getDocIdsInProject(projectId)
+
+  logger.debug({ projectId, docIds }, 'flushing docs')
+  const errors = []
+  for (const docId of docIds) {
+    try {
+      await DocumentManager.promises.flushDocIfLoadedWithLock(projectId, docId)
+    } catch (error) {
+      if (error instanceof Errors.NotFoundError) {
+        logger.warn(
+          { err: error, projectId, docId },
+          'found deleted doc when flushing'
+        )
       } else {
-        callback(null)
+        logger.error({ err: error, projectId, docId }, 'error flushing doc')
+        errors.push(error)
       }
-    })
-  })
+    }
+  }
+
+  timer.done()
+  if (errors.length > 0) {
+    throw new Error('Errors flushing docs. See log for details')
+  }
 }
 
-function flushAndDeleteProjectWithLocks(projectId, options, _callback) {
+async function flushAndDeleteProjectWithLocks(projectId, options) {
   const timer = new Metrics.Timer(
     'projectManager.flushAndDeleteProjectWithLocks'
   )
-  const callback = function (...args) {
-    timer.done()
-    _callback(...args)
-  }
 
-  RedisManager.getDocIdsInProject(projectId, (error, docIds) => {
-    if (error) {
-      return callback(error)
+  const docIds = await RedisManager.promises.getDocIdsInProject(projectId)
+
+  logger.debug({ projectId, docIds }, 'deleting docs')
+  const errors = []
+  for (const docId of docIds) {
+    try {
+      await DocumentManager.promises.flushAndDeleteDocWithLock(
+        projectId,
+        docId,
+        {}
+      )
+    } catch (error) {
+      logger.error({ err: error, projectId, docId }, 'error deleting doc')
+      errors.push(error)
     }
-    const errors = []
-    const jobs = docIds.map(docId => callback => {
-      DocumentManager.flushAndDeleteDocWithLock(projectId, docId, {}, error => {
-        if (error) {
-          logger.error({ err: error, projectId, docId }, 'error deleting doc')
-          errors.push(error)
-        }
-        callback()
-      })
-    })
+  }
 
-    logger.debug({ projectId, docIds }, 'deleting docs')
-    async.series(jobs, () =>
-      // When deleting the project here we want to ensure that project
-      // history is completely flushed because the project may be
-      // deleted in web after this call completes, and so further
-      // attempts to flush would fail after that.
-      HistoryManager.flushProjectChanges(projectId, options, error => {
-        if (errors.length > 0) {
-          callback(new Error('Errors deleting docs. See log for details'))
-        } else if (error) {
-          callback(error)
-        } else {
-          callback(null)
-        }
-      })
-    )
-  })
+  // When deleting the project here we want to ensure that project
+  // history is completely flushed because the project may be
+  // deleted in web after this call completes, and so further
+  // attempts to flush would fail after that.
+  await HistoryManager.promises.flushProjectChanges(projectId, options)
+  timer.done()
+  if (errors.length > 0) {
+    throw new Error('Errors deleting docs. See log for details')
+  }
 }
 
-function queueFlushAndDeleteProject(projectId, callback) {
-  RedisManager.queueFlushAndDeleteProject(projectId, error => {
-    if (error) {
-      logger.error(
-        { projectId, error },
-        'error adding project to flush and delete queue'
-      )
-      return callback(error)
-    }
-    Metrics.inc('queued-delete')
-    callback()
-  })
+async function queueFlushAndDeleteProject(projectId) {
+  await RedisManager.promises.queueFlushAndDeleteProject(projectId)
+  Metrics.inc('queued-delete')
 }
 
-function getProjectDocsTimestamps(projectId, callback) {
-  RedisManager.getDocIdsInProject(projectId, (error, docIds) => {
-    if (error) {
-      return callback(error)
-    }
-    if (docIds.length === 0) {
-      return callback(null, [])
-    }
-    RedisManager.getDocTimestamps(docIds, (error, timestamps) => {
-      if (error) {
-        return callback(error)
-      }
-      callback(null, timestamps)
-    })
-  })
+async function getProjectDocsTimestamps(projectId, callback) {
+  const docIds = await RedisManager.promises.getDocIdsInProject(projectId)
+  if (docIds.length === 0) {
+    return []
+  }
+
+  const timestamps = await RedisManager.promises.getDocTimestamps(docIds)
+  return timestamps
 }
 
-function getProjectDocsAndFlushIfOld(
+async function getProjectDocsAndFlushIfOld(
   projectId,
   projectStateHash,
-  excludeVersions,
-  _callback
+  excludeVersions
 ) {
   const timer = new Metrics.Timer('projectManager.getProjectDocsAndFlushIfOld')
-  const callback = function (...args) {
+
+  const projectStateChanged =
+    await RedisManager.promises.checkOrSetProjectState(
+      projectId,
+      projectStateHash
+    )
+
+  // we can't return docs if project structure has changed
+  if (projectStateChanged) {
     timer.done()
-    _callback(...args)
+    throw new Errors.ProjectStateChangedError('project state changed')
   }
 
-  RedisManager.checkOrSetProjectState(
-    projectId,
-    projectStateHash,
-    (error, projectStateChanged) => {
-      if (error) {
-        logger.error(
-          { err: error, projectId },
-          'error getting/setting project state in getProjectDocsAndFlushIfOld'
-        )
-        return callback(error)
-      }
-      // we can't return docs if project structure has changed
-      if (projectStateChanged) {
-        return callback(
-          new Errors.ProjectStateChangedError('project state changed')
-        )
-      }
-      // project structure hasn't changed, return doc content from redis
-      RedisManager.getDocIdsInProject(projectId, (error, docIds) => {
-        if (error) {
-          logger.error(
-            { err: error, projectId },
-            'error getting doc ids in getProjectDocs'
-          )
-          return callback(error)
-        }
-        // get the doc lines from redis
-        const jobs = docIds.map(docId => cb => {
-          DocumentManager.getDocAndFlushIfOldWithLock(
-            projectId,
-            docId,
-            (err, lines, version) => {
-              if (err) {
-                logger.error(
-                  { err, projectId, docId },
-                  'error getting project doc lines in getProjectDocsAndFlushIfOld'
-                )
-                return cb(err)
-              }
-              const doc = { _id: docId, lines, v: version } // create a doc object to return
-              cb(null, doc)
-            }
-          )
-        })
-        async.series(jobs, (error, docs) => {
-          if (error) {
-            return callback(error)
-          }
-          callback(null, docs)
-        })
-      })
-    }
-  )
+  // project structure hasn't changed, return doc content from redis
+  const docs = []
+  const docIds = await RedisManager.promises.getDocIdsInProject(projectId)
+  for (const docId of docIds) {
+    const { lines, version } =
+      await DocumentManager.promises.getDocAndFlushIfOldWithLock(
+        projectId,
+        docId
+      )
+    docs.push({ _id: docId, lines, v: version })
+  }
+
+  timer.done()
+  return docs
 }
 
-function clearProjectState(projectId, callback) {
-  RedisManager.clearProjectState(projectId, callback)
+async function clearProjectState(projectId) {
+  await RedisManager.promises.clearProjectState(projectId)
 }
 
-function updateProjectWithLocks(
+async function updateProjectWithLocks(
   projectId,
   projectHistoryId,
   userId,
   updates,
   projectVersion,
-  source,
-  _callback
+  source
 ) {
   const timer = new Metrics.Timer('projectManager.updateProject')
-  const callback = function (...args) {
-    timer.done()
-    _callback(...args)
-  }
 
   let projectSubversion = 0 // project versions can have multiple operations
   let projectOpsLength = 0
 
-  function handleUpdate(update, cb) {
+  for (const update of updates) {
     update.version = `${projectVersion}.${projectSubversion++}`
     switch (update.type) {
       case 'add-doc':
-        ProjectHistoryRedisManager.queueAddEntity(
-          projectId,
-          projectHistoryId,
-          'doc',
-          update.id,
-          userId,
-          update,
-          source,
-          (error, count) => {
-            projectOpsLength = count
-            cb(error)
-          }
-        )
-        break
-      case 'rename-doc':
-        if (!update.newPathname) {
-          // an empty newPathname signifies a delete, so there is no need to
-          // update the pathname in redis
-          ProjectHistoryRedisManager.queueRenameEntity(
+        projectOpsLength =
+          await ProjectHistoryRedisManager.promises.queueAddEntity(
             projectId,
             projectHistoryId,
             'doc',
             update.id,
             userId,
             update,
-            source,
-            (error, count) => {
-              projectOpsLength = count
-              cb(error)
-            }
+            source
           )
+        break
+      case 'rename-doc':
+        if (!update.newPathname) {
+          // an empty newPathname signifies a delete, so there is no need to
+          // update the pathname in redis
+          projectOpsLength =
+            await ProjectHistoryRedisManager.promises.queueRenameEntity(
+              projectId,
+              projectHistoryId,
+              'doc',
+              update.id,
+              userId,
+              update,
+              source
+            )
         } else {
           // rename the doc in redis before queuing the update
-          DocumentManager.renameDocWithLock(
+          await DocumentManager.promises.renameDocWithLock(
             projectId,
             update.id,
             userId,
             update,
-            projectHistoryId,
-            error => {
-              if (error) {
-                return cb(error)
-              }
-              ProjectHistoryRedisManager.queueRenameEntity(
-                projectId,
-                projectHistoryId,
-                'doc',
-                update.id,
-                userId,
-                update,
-                source,
-                (error, count) => {
-                  projectOpsLength = count
-                  cb(error)
-                }
-              )
-            }
+            projectHistoryId
           )
+          projectOpsLength =
+            await ProjectHistoryRedisManager.promises.queueRenameEntity(
+              projectId,
+              projectHistoryId,
+              'doc',
+              update.id,
+              userId,
+              update,
+              source
+            )
         }
         break
       case 'add-file':
-        ProjectHistoryRedisManager.queueAddEntity(
-          projectId,
-          projectHistoryId,
-          'file',
-          update.id,
-          userId,
-          update,
-          source,
-          (error, count) => {
-            projectOpsLength = count
-            cb(error)
-          }
-        )
+        projectOpsLength =
+          await ProjectHistoryRedisManager.promises.queueAddEntity(
+            projectId,
+            projectHistoryId,
+            'file',
+            update.id,
+            userId,
+            update,
+            source
+          )
         break
       case 'rename-file':
-        ProjectHistoryRedisManager.queueRenameEntity(
-          projectId,
-          projectHistoryId,
-          'file',
-          update.id,
-          userId,
-          update,
-          source,
-          (error, count) => {
-            projectOpsLength = count
-            cb(error)
-          }
-        )
+        projectOpsLength =
+          await ProjectHistoryRedisManager.promises.queueRenameEntity(
+            projectId,
+            projectHistoryId,
+            'file',
+            update.id,
+            userId,
+            update,
+            source
+          )
         break
       default:
-        cb(new Error(`Unknown update type: ${update.type}`))
+        throw new Error(`Unknown update type: ${update.type}`)
     }
   }
 
-  async.eachSeries(updates, handleUpdate, error => {
-    if (error) {
-      return callback(error)
-    }
-    if (
-      HistoryManager.shouldFlushHistoryOps(
-        projectId,
-        projectOpsLength,
-        updates.length,
-        HistoryManager.FLUSH_PROJECT_EVERY_N_OPS
-      )
-    ) {
-      HistoryManager.flushProjectChangesAsync(projectId)
-    }
-    callback()
-  })
+  if (
+    HistoryManager.shouldFlushHistoryOps(
+      projectId,
+      projectOpsLength,
+      updates.length,
+      HistoryManager.FLUSH_PROJECT_EVERY_N_OPS
+    )
+  ) {
+    HistoryManager.flushProjectChangesAsync(projectId)
+  }
+
+  timer.done()
 }
 
-module.exports = {
+const ProjectManager = {
   flushProjectWithLocks,
   flushAndDeleteProjectWithLocks,
   queueFlushAndDeleteProject,
@@ -339,4 +238,7 @@ module.exports = {
   updateProjectWithLocks,
 }
 
-module.exports.promises = promisifyAll(module.exports)
+module.exports = {
+  ...callbackifyAll(ProjectManager),
+  promises: ProjectManager,
+}

+ 80 - 112
services/document-updater/test/unit/js/ProjectManager/flushAndDeleteProjectTests.js

@@ -1,157 +1,125 @@
-/* eslint-disable
-    no-return-assign,
-    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
- * DS102: Remove unnecessary code created because of implicit returns
- * DS206: Consider reworking classes to avoid initClass
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
+const { expect } = require('chai')
 const sinon = require('sinon')
-const modulePath = '../../../../app/js/ProjectManager.js'
 const SandboxedModule = require('sandboxed-module')
 
+const MODULE_PATH = '../../../../app/js/ProjectManager.js'
+
 describe('ProjectManager - flushAndDeleteProject', function () {
   beforeEach(function () {
-    let Timer
-    this.LockManager = {
-      getLock: sinon.stub().yields(),
-      releaseLock: sinon.stub().yields(),
+    this.project_id = 'project-id-123'
+
+    this.RedisManager = {
+      promises: {
+        getDocIdsInProject: sinon.stub(),
+      },
+    }
+
+    this.ProjectHistoryRedisManager = {}
+
+    this.DocumentManager = {
+      promises: {
+        flushAndDeleteDocWithLock: sinon.stub().resolves(),
+      },
+    }
+
+    this.HistoryManager = {
+      promises: {
+        flushProjectChanges: sinon.stub().resolves(),
+      },
     }
-    this.ProjectManager = SandboxedModule.require(modulePath, {
+
+    this.Metrics = {
+      Timer: class Timer {},
+    }
+    this.Metrics.Timer.prototype.done = sinon.stub()
+
+    this.ProjectManager = SandboxedModule.require(MODULE_PATH, {
       requires: {
-        './RedisManager': (this.RedisManager = {}),
-        './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}),
-        './DocumentManager': (this.DocumentManager = {}),
-        './HistoryManager': (this.HistoryManager = {
-          flushProjectChanges: sinon.stub().callsArg(2),
-        }),
-        './LockManager': this.LockManager,
-        './Metrics': (this.Metrics = {
-          Timer: (Timer = (function () {
-            Timer = class Timer {
-              static initClass() {
-                this.prototype.done = sinon.stub()
-              }
-            }
-            Timer.initClass()
-            return Timer
-          })()),
-        }),
+        './RedisManager': this.RedisManager,
+        './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager,
+        './DocumentManager': this.DocumentManager,
+        './HistoryManager': this.HistoryManager,
+        './Metrics': this.Metrics,
       },
     })
-    this.project_id = 'project-id-123'
-    return (this.callback = sinon.stub())
   })
 
   describe('successfully', function () {
-    beforeEach(function (done) {
+    beforeEach(async function () {
       this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3']
-      this.RedisManager.getDocIdsInProject = sinon
-        .stub()
-        .callsArgWith(1, null, this.doc_ids)
-      this.DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArg(3)
-      return this.ProjectManager.flushAndDeleteProjectWithLocks(
+      this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids)
+      await this.ProjectManager.promises.flushAndDeleteProjectWithLocks(
         this.project_id,
-        {},
-        error => {
-          this.callback(error)
-          return done()
-        }
+        {}
       )
     })
 
     it('should get the doc ids in the project', function () {
-      return this.RedisManager.getDocIdsInProject
-        .calledWith(this.project_id)
-        .should.equal(true)
+      this.RedisManager.promises.getDocIdsInProject.should.have.been.calledWith(
+        this.project_id
+      )
     })
 
     it('should delete each doc in the project', function () {
-      return Array.from(this.doc_ids).map(docId =>
-        this.DocumentManager.flushAndDeleteDocWithLock
-          .calledWith(this.project_id, docId, {})
-          .should.equal(true)
-      )
+      for (const docId of this.doc_ids) {
+        this.DocumentManager.promises.flushAndDeleteDocWithLock.should.have.been.calledWith(
+          this.project_id,
+          docId,
+          {}
+        )
+      }
     })
 
     it('should flush project history', function () {
-      return this.HistoryManager.flushProjectChanges
-        .calledWith(this.project_id, {})
-        .should.equal(true)
-    })
-
-    it('should call the callback without error', function () {
-      return this.callback.calledWith(null).should.equal(true)
+      this.HistoryManager.promises.flushProjectChanges.should.have.been.calledWith(
+        this.project_id,
+        {}
+      )
     })
 
-    return it('should time the execution', function () {
-      return this.Metrics.Timer.prototype.done.called.should.equal(true)
+    it('should time the execution', function () {
+      this.Metrics.Timer.prototype.done.called.should.equal(true)
     })
   })
 
-  return describe('when a doc errors', function () {
-    beforeEach(function (done) {
+  describe('when a doc errors', function () {
+    beforeEach(async function () {
       this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3']
-      this.RedisManager.getDocIdsInProject = sinon
-        .stub()
-        .callsArgWith(1, null, this.doc_ids)
-      this.DocumentManager.flushAndDeleteDocWithLock = sinon.spy(
-        (projectId, docId, options, callback) => {
+      this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids)
+      this.DocumentManager.promises.flushAndDeleteDocWithLock.callsFake(
+        async (projectId, docId, options) => {
           if (docId === 'doc-id-1') {
-            return callback(
-              (this.error = new Error('oops, something went wrong'))
-            )
-          } else {
-            return callback()
+            throw new Error('oops, something went wrong')
           }
         }
       )
-      return this.ProjectManager.flushAndDeleteProjectWithLocks(
-        this.project_id,
-        {},
-        error => {
-          this.callback(error)
-          return done()
-        }
-      )
+      await expect(
+        this.ProjectManager.promises.flushAndDeleteProjectWithLocks(
+          this.project_id,
+          {}
+        )
+      ).to.be.rejected
     })
 
     it('should still flush each doc in the project', function () {
-      return Array.from(this.doc_ids).map(docId =>
-        this.DocumentManager.flushAndDeleteDocWithLock
-          .calledWith(this.project_id, docId, {})
-          .should.equal(true)
-      )
-    })
-
-    it('should still flush project history', function () {
-      return this.HistoryManager.flushProjectChanges
-        .calledWith(this.project_id, {})
-        .should.equal(true)
-    })
-
-    it('should record the error', function () {
-      return this.logger.error
-        .calledWith(
-          { err: this.error, projectId: this.project_id, docId: 'doc-id-1' },
-          'error deleting doc'
+      for (const docId of this.doc_ids) {
+        this.DocumentManager.promises.flushAndDeleteDocWithLock.should.have.been.calledWith(
+          this.project_id,
+          docId,
+          {}
         )
-        .should.equal(true)
+      }
     })
 
-    it('should call the callback with an error', function () {
-      return this.callback
-        .calledWith(sinon.match.instanceOf(Error))
-        .should.equal(true)
+    it('should still flush project history', function () {
+      this.HistoryManager.promises.flushProjectChanges.should.have.been.calledWith(
+        this.project_id,
+        {}
+      )
     })
 
-    return it('should time the execution', function () {
-      return this.Metrics.Timer.prototype.done.called.should.equal(true)
+    it('should time the execution', function () {
+      this.Metrics.Timer.prototype.done.called.should.equal(true)
     })
   })
 })

+ 62 - 107
services/document-updater/test/unit/js/ProjectManager/flushProjectTests.js

@@ -1,145 +1,100 @@
-/* eslint-disable
-    no-return-assign,
-    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
- * DS102: Remove unnecessary code created because of implicit returns
- * DS206: Consider reworking classes to avoid initClass
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
+const { expect } = require('chai')
 const sinon = require('sinon')
-const modulePath = '../../../../app/js/ProjectManager.js'
 const SandboxedModule = require('sandboxed-module')
 
+const MODULE_PATH = '../../../../app/js/ProjectManager.js'
+
 describe('ProjectManager - flushProject', function () {
   beforeEach(function () {
-    let Timer
-    this.LockManager = {
-      getLock: sinon.stub().yields(),
-      releaseLock: sinon.stub().yields(),
+    this.project_id = 'project-id-123'
+
+    this.RedisManager = {
+      promises: {
+        getDocIdsInProject: sinon.stub(),
+      },
     }
-    this.ProjectManager = SandboxedModule.require(modulePath, {
+
+    this.ProjectHistoryRedisManager = {}
+
+    this.Metrics = {
+      Timer: class Timer {},
+    }
+
+    this.DocumentManager = {
+      promises: {
+        flushDocIfLoadedWithLock: sinon.stub().resolves(),
+      },
+    }
+
+    this.HistoryManager = {}
+
+    this.Metrics.Timer.prototype.done = sinon.stub()
+
+    this.ProjectManager = SandboxedModule.require(MODULE_PATH, {
       requires: {
-        './RedisManager': (this.RedisManager = {}),
-        './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}),
-        './DocumentManager': (this.DocumentManager = {}),
-        './HistoryManager': (this.HistoryManager = {}),
-        './LockManager': this.LockManager,
-        './Metrics': (this.Metrics = {
-          Timer: (Timer = (function () {
-            Timer = class Timer {
-              static initClass() {
-                this.prototype.done = sinon.stub()
-              }
-            }
-            Timer.initClass()
-            return Timer
-          })()),
-        }),
+        './RedisManager': this.RedisManager,
+        './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager,
+        './DocumentManager': this.DocumentManager,
+        './HistoryManager': this.HistoryManager,
+        './Metrics': this.Metrics,
       },
     })
-    this.project_id = 'project-id-123'
-    return (this.callback = sinon.stub())
   })
 
   describe('successfully', function () {
-    beforeEach(function (done) {
+    beforeEach(async function () {
       this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3']
-      this.RedisManager.getDocIdsInProject = sinon
-        .stub()
-        .callsArgWith(1, null, this.doc_ids)
-      this.DocumentManager.flushDocIfLoadedWithLock = sinon.stub().callsArg(2)
-      return this.ProjectManager.flushProjectWithLocks(
-        this.project_id,
-        error => {
-          this.callback(error)
-          return done()
-        }
-      )
+      this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids)
+      await this.ProjectManager.promises.flushProjectWithLocks(this.project_id)
     })
 
     it('should get the doc ids in the project', function () {
-      return this.RedisManager.getDocIdsInProject
-        .calledWith(this.project_id)
-        .should.equal(true)
-    })
-
-    it('should flush each doc in the project', function () {
-      return Array.from(this.doc_ids).map(docId =>
-        this.DocumentManager.flushDocIfLoadedWithLock
-          .calledWith(this.project_id, docId)
-          .should.equal(true)
+      this.RedisManager.promises.getDocIdsInProject.should.have.been.calledWith(
+        this.project_id
       )
     })
 
-    it('should call the callback without error', function () {
-      return this.callback.calledWith(null).should.equal(true)
+    it('should flush each doc in the project', function () {
+      for (const docId of this.doc_ids) {
+        this.DocumentManager.promises.flushDocIfLoadedWithLock.should.have.been.calledWith(
+          this.project_id,
+          docId
+        )
+      }
     })
 
-    return it('should time the execution', function () {
-      return this.Metrics.Timer.prototype.done.called.should.equal(true)
+    it('should time the execution', function () {
+      this.Metrics.Timer.prototype.done.called.should.equal(true)
     })
   })
 
-  return describe('when a doc errors', function () {
-    beforeEach(function (done) {
+  describe('when a doc errors', function () {
+    beforeEach(async function () {
       this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3']
-      this.RedisManager.getDocIdsInProject = sinon
-        .stub()
-        .callsArgWith(1, null, this.doc_ids)
-      this.DocumentManager.flushDocIfLoadedWithLock = sinon.spy(
-        (projectId, docId, callback) => {
-          if (callback == null) {
-            callback = function () {}
-          }
+      this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids)
+      this.DocumentManager.promises.flushDocIfLoadedWithLock.callsFake(
+        async (projectId, docId) => {
           if (docId === 'doc-id-1') {
-            return callback(
-              (this.error = new Error('oops, something went wrong'))
-            )
-          } else {
-            return callback()
+            throw new Error('oops, something went wrong')
           }
         }
       )
-      return this.ProjectManager.flushProjectWithLocks(
-        this.project_id,
-        error => {
-          this.callback(error)
-          return done()
-        }
-      )
+      await expect(
+        this.ProjectManager.promises.flushProjectWithLocks(this.project_id)
+      ).to.be.rejected
     })
 
     it('should still flush each doc in the project', function () {
-      return Array.from(this.doc_ids).map(docId =>
-        this.DocumentManager.flushDocIfLoadedWithLock
-          .calledWith(this.project_id, docId)
-          .should.equal(true)
-      )
-    })
-
-    it('should record the error', function () {
-      return this.logger.error
-        .calledWith(
-          { err: this.error, projectId: this.project_id, docId: 'doc-id-1' },
-          'error flushing doc'
+      for (const docId of this.doc_ids) {
+        this.DocumentManager.promises.flushDocIfLoadedWithLock.should.have.been.calledWith(
+          this.project_id,
+          docId
         )
-        .should.equal(true)
-    })
-
-    it('should call the callback with an error', function () {
-      return this.callback
-        .calledWith(sinon.match.instanceOf(Error))
-        .should.equal(true)
+      }
     })
 
-    return it('should time the execution', function () {
-      return this.Metrics.Timer.prototype.done.called.should.equal(true)
+    it('should time the execution', function () {
+      this.Metrics.Timer.prototype.done.called.should.equal(true)
     })
   })
 })

+ 90 - 146
services/document-updater/test/unit/js/ProjectManager/getProjectDocsTests.js

@@ -1,55 +1,47 @@
-/* eslint-disable
-    no-return-assign,
-    no-unused-vars,
-*/
-// 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
- * DS206: Consider reworking classes to avoid initClass
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
+const { expect } = require('chai')
 const sinon = require('sinon')
-const modulePath = '../../../../app/js/ProjectManager.js'
 const SandboxedModule = require('sandboxed-module')
 const Errors = require('../../../../app/js/Errors.js')
 
+const MODULE_PATH = '../../../../app/js/ProjectManager.js'
+
 describe('ProjectManager - getProjectDocsAndFlushIfOld', function () {
   beforeEach(function () {
-    let Timer
-    this.LockManager = {
-      getLock: sinon.stub().yields(),
-      releaseLock: sinon.stub().yields(),
+    this.RedisManager = {
+      promises: {
+        checkOrSetProjectState: sinon.stub().resolves(),
+        getDocIdsInProject: sinon.stub(),
+        clearProjectState: sinon.stub().resolves(),
+      },
+    }
+    this.ProjectHistoryRedisManager = {}
+    this.DocumentManager = {
+      promises: {
+        getDocAndFlushIfOldWithLock: sinon.stub(),
+      },
     }
-    this.ProjectManager = SandboxedModule.require(modulePath, {
+    this.HistoryManager = {}
+    this.Metrics = {
+      Timer: class Timer {},
+    }
+    this.Metrics.Timer.prototype.done = sinon.stub()
+
+    this.ProjectManager = SandboxedModule.require(MODULE_PATH, {
       requires: {
-        './RedisManager': (this.RedisManager = {}),
-        './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}),
-        './DocumentManager': (this.DocumentManager = {}),
-        './HistoryManager': (this.HistoryManager = {}),
-        './LockManager': this.LockManager,
-        './Metrics': (this.Metrics = {
-          Timer: (Timer = (function () {
-            Timer = class Timer {
-              static initClass() {
-                this.prototype.done = sinon.stub()
-              }
-            }
-            Timer.initClass()
-            return Timer
-          })()),
-        }),
+        './RedisManager': this.RedisManager,
+        './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager,
+        './DocumentManager': this.DocumentManager,
+        './HistoryManager': this.HistoryManager,
+        './Metrics': this.Metrics,
         './Errors': Errors,
       },
     })
     this.project_id = 'project-id-123'
-    this.callback = sinon.stub()
-    return (this.doc_versions = [111, 222, 333])
+    this.doc_versions = [111, 222, 333]
   })
 
   describe('successfully', function () {
-    beforeEach(function (done) {
+    beforeEach(async function () {
       this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3']
       this.doc_lines = [
         ['aaa', 'aaa'],
@@ -73,152 +65,104 @@ describe('ProjectManager - getProjectDocsAndFlushIfOld', function () {
           v: this.doc_versions[2],
         },
       ]
-      this.RedisManager.checkOrSetProjectState = sinon
-        .stub()
-        .callsArgWith(2, null)
-      this.RedisManager.getDocIdsInProject = sinon
-        .stub()
-        .callsArgWith(1, null, this.doc_ids)
-      this.DocumentManager.getDocAndFlushIfOldWithLock = sinon.stub()
-      this.DocumentManager.getDocAndFlushIfOldWithLock
+      this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids)
+      this.DocumentManager.promises.getDocAndFlushIfOldWithLock
         .withArgs(this.project_id, this.doc_ids[0])
-        .callsArgWith(2, null, this.doc_lines[0], this.doc_versions[0])
-      this.DocumentManager.getDocAndFlushIfOldWithLock
+        .resolves({ lines: this.doc_lines[0], version: this.doc_versions[0] })
+      this.DocumentManager.promises.getDocAndFlushIfOldWithLock
         .withArgs(this.project_id, this.doc_ids[1])
-        .callsArgWith(2, null, this.doc_lines[1], this.doc_versions[1])
-      this.DocumentManager.getDocAndFlushIfOldWithLock
+        .resolves({ lines: this.doc_lines[1], version: this.doc_versions[1] })
+      this.DocumentManager.promises.getDocAndFlushIfOldWithLock
         .withArgs(this.project_id, this.doc_ids[2])
-        .callsArgWith(2, null, this.doc_lines[2], this.doc_versions[2])
-      return this.ProjectManager.getProjectDocsAndFlushIfOld(
-        this.project_id,
-        this.projectStateHash,
-        this.excludeVersions,
-        (error, docs) => {
-          this.callback(error, docs)
-          return done()
-        }
-      )
+        .resolves({ lines: this.doc_lines[2], version: this.doc_versions[2] })
+      this.result =
+        await this.ProjectManager.promises.getProjectDocsAndFlushIfOld(
+          this.project_id,
+          this.projectStateHash,
+          this.excludeVersions
+        )
     })
 
     it('should check the project state', function () {
-      return this.RedisManager.checkOrSetProjectState
-        .calledWith(this.project_id, this.projectStateHash)
-        .should.equal(true)
+      this.RedisManager.promises.checkOrSetProjectState.should.have.been.calledWith(
+        this.project_id,
+        this.projectStateHash
+      )
     })
 
     it('should get the doc ids in the project', function () {
-      return this.RedisManager.getDocIdsInProject
-        .calledWith(this.project_id)
-        .should.equal(true)
+      this.RedisManager.promises.getDocIdsInProject.should.have.been.calledWith(
+        this.project_id
+      )
     })
 
-    it('should call the callback without error', function () {
-      return this.callback.calledWith(null, this.docs).should.equal(true)
+    it('should return docs', function () {
+      expect(this.result).to.deep.equal(this.docs)
     })
 
-    return it('should time the execution', function () {
-      return this.Metrics.Timer.prototype.done.called.should.equal(true)
+    it('should time the execution', function () {
+      this.Metrics.Timer.prototype.done.called.should.equal(true)
     })
   })
 
   describe('when the state does not match', function () {
-    beforeEach(function (done) {
+    beforeEach(async function () {
       this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3']
-      this.RedisManager.checkOrSetProjectState = sinon
-        .stub()
-        .callsArgWith(2, null, true)
-      return this.ProjectManager.getProjectDocsAndFlushIfOld(
-        this.project_id,
-        this.projectStateHash,
-        this.excludeVersions,
-        (error, docs) => {
-          this.callback(error, docs)
-          return done()
-        }
-      )
+      this.RedisManager.promises.checkOrSetProjectState.resolves(true)
+      await expect(
+        this.ProjectManager.promises.getProjectDocsAndFlushIfOld(
+          this.project_id,
+          this.projectStateHash,
+          this.excludeVersions
+        )
+      ).to.be.rejectedWith(Errors.ProjectStateChangedError)
     })
 
     it('should check the project state', function () {
-      return this.RedisManager.checkOrSetProjectState
-        .calledWith(this.project_id, this.projectStateHash)
-        .should.equal(true)
-    })
-
-    it('should call the callback with an error', function () {
-      return this.callback
-        .calledWith(sinon.match.instanceOf(Errors.ProjectStateChangedError))
-        .should.equal(true)
+      this.RedisManager.promises.checkOrSetProjectState.should.have.been.calledWith(
+        this.project_id,
+        this.projectStateHash
+      )
     })
 
-    return it('should time the execution', function () {
-      return this.Metrics.Timer.prototype.done.called.should.equal(true)
+    it('should time the execution', function () {
+      this.Metrics.Timer.prototype.done.called.should.equal(true)
     })
   })
 
   describe('when a doc errors', function () {
-    beforeEach(function (done) {
+    it('should call the callback with an error', async function () {
       this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3']
-      this.RedisManager.checkOrSetProjectState = sinon
-        .stub()
-        .callsArgWith(2, null)
-      this.RedisManager.getDocIdsInProject = sinon
-        .stub()
-        .callsArgWith(1, null, this.doc_ids)
-      this.DocumentManager.getDocAndFlushIfOldWithLock = sinon.stub()
-      this.DocumentManager.getDocAndFlushIfOldWithLock
+      this.error = new Error('oops')
+      this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids)
+      this.DocumentManager.promises.getDocAndFlushIfOldWithLock
         .withArgs(this.project_id, 'doc-id-1')
-        .callsArgWith(2, null, ['test doc content'], this.doc_versions[1])
-      this.DocumentManager.getDocAndFlushIfOldWithLock
+        .resolves({
+          lines: ['test doc content'],
+          version: this.doc_versions[1],
+        })
+      this.DocumentManager.promises.getDocAndFlushIfOldWithLock
         .withArgs(this.project_id, 'doc-id-2')
-        .callsArgWith(2, (this.error = new Error('oops'))) // trigger an error
-      return this.ProjectManager.getProjectDocsAndFlushIfOld(
-        this.project_id,
-        this.projectStateHash,
-        this.excludeVersions,
-        (error, docs) => {
-          this.callback(error)
-          return done()
-        }
-      )
-    })
-
-    it('should record the error', function () {
-      return this.logger.error
-        .calledWith(
-          { err: this.error, projectId: this.project_id, docId: 'doc-id-2' },
-          'error getting project doc lines in getProjectDocsAndFlushIfOld'
+        .rejects(this.error)
+      await expect(
+        this.ProjectManager.promises.getProjectDocsAndFlushIfOld(
+          this.project_id,
+          this.projectStateHash,
+          this.excludeVersions
         )
-        .should.equal(true)
-    })
-
-    it('should call the callback with an error', function () {
-      return this.callback
-        .calledWith(sinon.match.instanceOf(Error))
-        .should.equal(true)
-    })
-
-    return it('should time the execution', function () {
-      return this.Metrics.Timer.prototype.done.called.should.equal(true)
+      ).to.be.rejected
     })
   })
 
-  return describe('clearing the project state with clearProjectState', function () {
-    beforeEach(function (done) {
-      this.RedisManager.clearProjectState = sinon.stub().callsArg(1)
-      return this.ProjectManager.clearProjectState(this.project_id, error => {
-        this.callback(error)
-        return done()
-      })
+  describe('clearing the project state with clearProjectState', function () {
+    beforeEach(async function () {
+      await this.ProjectManager.promises.clearProjectState(this.project_id)
     })
 
     it('should clear the project state', function () {
-      return this.RedisManager.clearProjectState
-        .calledWith(this.project_id)
-        .should.equal(true)
-    })
-
-    return it('should call the callback', function () {
-      return this.callback.called.should.equal(true)
+      this.RedisManager.promises.clearProjectState.should.have.been.calledWith(
+        this.project_id
+      )
     })
   })
 })

+ 110 - 139
services/document-updater/test/unit/js/ProjectManager/updateProjectTests.js

@@ -1,3 +1,4 @@
+const { expect } = require('chai')
 const sinon = require('sinon')
 const modulePath = '../../../../app/js/ProjectManager.js'
 const SandboxedModule = require('sandboxed-module')
@@ -7,20 +8,20 @@ describe('ProjectManager', function () {
   beforeEach(function () {
     this.RedisManager = {}
     this.ProjectHistoryRedisManager = {
-      queueRenameEntity: sinon.stub().yields(),
-      queueAddEntity: sinon.stub().yields(),
+      promises: {
+        queueRenameEntity: sinon.stub().resolves(),
+        queueAddEntity: sinon.stub().resolves(),
+      },
     }
     this.DocumentManager = {
-      renameDocWithLock: sinon.stub().yields(),
+      promises: {
+        renameDocWithLock: sinon.stub().resolves(),
+      },
     }
     this.HistoryManager = {
       flushProjectChangesAsync: sinon.stub(),
       shouldFlushHistoryOps: sinon.stub().returns(false),
     }
-    this.LockManager = {
-      getLock: sinon.stub().yields(),
-      releaseLock: sinon.stub().yields(),
-    }
     this.Metrics = {
       Timer: class Timer {},
     }
@@ -32,7 +33,6 @@ describe('ProjectManager', function () {
         './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager,
         './DocumentManager': this.DocumentManager,
         './HistoryManager': this.HistoryManager,
-        './LockManager': this.LockManager,
         './Metrics': this.Metrics,
       },
     })
@@ -42,7 +42,6 @@ describe('ProjectManager', function () {
     this.user_id = 'user-id-123'
     this.version = 1234567
     this.source = 'editor'
-    this.callback = sinon.stub()
   })
 
   describe('updateProjectWithLocks', function () {
@@ -74,15 +73,14 @@ describe('ProjectManager', function () {
       })
 
       describe('successfully', function () {
-        beforeEach(function () {
-          this.ProjectManager.updateProjectWithLocks(
+        beforeEach(async function () {
+          await this.ProjectManager.promises.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
             this.updates,
             this.version,
-            this.source,
-            this.callback
+            this.source
           )
         })
 
@@ -95,24 +93,20 @@ describe('ProjectManager', function () {
             this.secondDocUpdate,
             { version: `${this.version}.1` }
           )
-          this.DocumentManager.renameDocWithLock
-            .calledWith(
-              this.project_id,
-              this.firstDocUpdate.id,
-              this.user_id,
-              firstDocUpdateWithVersion,
-              this.projectHistoryId
-            )
-            .should.equal(true)
-          this.DocumentManager.renameDocWithLock
-            .calledWith(
-              this.project_id,
-              this.secondDocUpdate.id,
-              this.user_id,
-              secondDocUpdateWithVersion,
-              this.projectHistoryId
-            )
-            .should.equal(true)
+          this.DocumentManager.promises.renameDocWithLock.should.have.been.calledWith(
+            this.project_id,
+            this.firstDocUpdate.id,
+            this.user_id,
+            firstDocUpdateWithVersion,
+            this.projectHistoryId
+          )
+          this.DocumentManager.promises.renameDocWithLock.should.have.been.calledWith(
+            this.project_id,
+            this.secondDocUpdate.id,
+            this.user_id,
+            secondDocUpdateWithVersion,
+            this.projectHistoryId
+          )
         })
 
         it('should rename the files in the updates', function () {
@@ -121,17 +115,15 @@ describe('ProjectManager', function () {
             this.firstFileUpdate,
             { version: `${this.version}.2` }
           )
-          this.ProjectHistoryRedisManager.queueRenameEntity
-            .calledWith(
-              this.project_id,
-              this.projectHistoryId,
-              'file',
-              this.firstFileUpdate.id,
-              this.user_id,
-              firstFileUpdateWithVersion,
-              this.source
-            )
-            .should.equal(true)
+          this.ProjectHistoryRedisManager.promises.queueRenameEntity.should.have.been.calledWith(
+            this.project_id,
+            this.projectHistoryId,
+            'file',
+            this.firstFileUpdate.id,
+            this.user_id,
+            firstFileUpdateWithVersion,
+            this.source
+          )
         })
 
         it('should not flush the history', function () {
@@ -139,63 +131,54 @@ describe('ProjectManager', function () {
             .calledWith(this.project_id)
             .should.equal(false)
         })
-
-        it('should call the callback', function () {
-          this.callback.called.should.equal(true)
-        })
       })
 
       describe('when renaming a doc fails', function () {
-        beforeEach(function () {
-          this.error = new Error('error')
-          this.DocumentManager.renameDocWithLock.yields(this.error)
-          this.ProjectManager.updateProjectWithLocks(
-            this.project_id,
-            this.projectHistoryId,
-            this.user_id,
-            this.updates,
-            this.version,
-            this.source,
-            this.callback
+        it('throws an error', async function () {
+          this.DocumentManager.promises.renameDocWithLock.rejects(
+            new Error('error')
           )
-        })
-
-        it('should call the callback with the error', function () {
-          this.callback.calledWith(this.error).should.equal(true)
+          await expect(
+            this.ProjectManager.promises.updateProjectWithLocks(
+              this.project_id,
+              this.projectHistoryId,
+              this.user_id,
+              this.updates,
+              this.version,
+              this.source
+            )
+          ).to.be.rejected
         })
       })
 
       describe('when renaming a file fails', function () {
-        beforeEach(function () {
-          this.error = new Error('error')
-          this.ProjectHistoryRedisManager.queueRenameEntity.yields(this.error)
-          this.ProjectManager.updateProjectWithLocks(
-            this.project_id,
-            this.projectHistoryId,
-            this.user_id,
-            this.updates,
-            this.version,
-            this.source,
-            this.callback
+        it('throws an error', async function () {
+          this.ProjectHistoryRedisManager.promises.queueRenameEntity.rejects(
+            new Error('error')
           )
-        })
-
-        it('should call the callback with the error', function () {
-          this.callback.calledWith(this.error).should.equal(true)
+          await expect(
+            this.ProjectManager.promises.updateProjectWithLocks(
+              this.project_id,
+              this.projectHistoryId,
+              this.user_id,
+              this.updates,
+              this.version,
+              this.source
+            )
+          ).to.be.rejected
         })
       })
 
       describe('with enough ops to flush', function () {
-        beforeEach(function () {
+        beforeEach(async function () {
           this.HistoryManager.shouldFlushHistoryOps.returns(true)
-          this.ProjectManager.updateProjectWithLocks(
+          await this.ProjectManager.promises.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
             this.updates,
             this.version,
-            this.source,
-            this.callback
+            this.source
           )
         })
 
@@ -238,15 +221,14 @@ describe('ProjectManager', function () {
       })
 
       describe('successfully', function () {
-        beforeEach(function () {
-          this.ProjectManager.updateProjectWithLocks(
+        beforeEach(async function () {
+          await this.ProjectManager.promises.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
             this.updates,
             this.version,
-            this.source,
-            this.callback
+            this.source
           )
         })
 
@@ -259,7 +241,7 @@ describe('ProjectManager', function () {
             this.secondDocUpdate,
             { version: `${this.version}.1` }
           )
-          this.ProjectHistoryRedisManager.queueAddEntity
+          this.ProjectHistoryRedisManager.promises.queueAddEntity
             .getCall(0)
             .calledWith(
               this.project_id,
@@ -271,7 +253,7 @@ describe('ProjectManager', function () {
               this.source
             )
             .should.equal(true)
-          this.ProjectHistoryRedisManager.queueAddEntity
+          this.ProjectHistoryRedisManager.promises.queueAddEntity
             .getCall(1)
             .calledWith(
               this.project_id,
@@ -296,7 +278,7 @@ describe('ProjectManager', function () {
             this.secondFileUpdate,
             { version: `${this.version}.3` }
           )
-          this.ProjectHistoryRedisManager.queueAddEntity
+          this.ProjectHistoryRedisManager.promises.queueAddEntity
             .getCall(2)
             .calledWith(
               this.project_id,
@@ -308,7 +290,7 @@ describe('ProjectManager', function () {
               this.source
             )
             .should.equal(true)
-          this.ProjectHistoryRedisManager.queueAddEntity
+          this.ProjectHistoryRedisManager.promises.queueAddEntity
             .getCall(3)
             .calledWith(
               this.project_id,
@@ -327,63 +309,55 @@ describe('ProjectManager', function () {
             .calledWith(this.project_id)
             .should.equal(false)
         })
-
-        it('should call the callback', function () {
-          this.callback.called.should.equal(true)
-        })
       })
 
       describe('when adding a doc fails', function () {
-        beforeEach(function () {
+        it('it should throw an error', async function () {
           this.error = new Error('error')
-          this.ProjectHistoryRedisManager.queueAddEntity.yields(this.error)
-          this.ProjectManager.updateProjectWithLocks(
-            this.project_id,
-            this.projectHistoryId,
-            this.user_id,
-            this.updates,
-            this.version,
-            this.source,
-            this.callback
+          this.ProjectHistoryRedisManager.promises.queueAddEntity.rejects(
+            this.error
           )
-        })
-
-        it('should call the callback with the error', function () {
-          this.callback.calledWith(this.error).should.equal(true)
+          await expect(
+            this.ProjectManager.promises.updateProjectWithLocks(
+              this.project_id,
+              this.projectHistoryId,
+              this.user_id,
+              this.updates,
+              this.version,
+              this.source
+            )
+          ).to.be.rejected
         })
       })
 
       describe('when adding a file fails', function () {
-        beforeEach(function () {
-          this.error = new Error('error')
-          this.ProjectHistoryRedisManager.queueAddEntity.yields(this.error)
-          this.ProjectManager.updateProjectWithLocks(
-            this.project_id,
-            this.projectHistoryId,
-            this.user_id,
-            this.updates,
-            this.version,
-            this.source,
-            this.callback
+        beforeEach(async function () {
+          this.ProjectHistoryRedisManager.promises.queueAddEntity.rejects(
+            new Error('error')
           )
-        })
-
-        it('should call the callback with the error', function () {
-          this.callback.calledWith(this.error).should.equal(true)
+          await expect(
+            this.ProjectManager.promises.updateProjectWithLocks(
+              this.project_id,
+              this.projectHistoryId,
+              this.user_id,
+              this.updates,
+              this.version,
+              this.source
+            )
+          ).to.be.rejected
         })
       })
 
       describe('with enough ops to flush', function () {
-        beforeEach(function () {
+        beforeEach(async function () {
           this.HistoryManager.shouldFlushHistoryOps.returns(true)
-          this.ProjectManager.updateProjectWithLocks(
+          await this.ProjectManager.promises.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
             this.updates,
             this.version,
-            this.source,
-            this.callback
+            this.source
           )
         })
 
@@ -396,21 +370,18 @@ describe('ProjectManager', function () {
     })
 
     describe('when given an unknown operation type', function () {
-      beforeEach(function () {
+      it('throws an error', async function () {
         this.updates = [{ type: 'brew-coffee' }]
-        this.ProjectManager.updateProjectWithLocks(
-          this.project_id,
-          this.projectHistoryId,
-          this.user_id,
-          this.updates,
-          this.version,
-          this.source,
-          this.callback
-        )
-      })
-
-      it('should call back with an error', function () {
-        this.callback.calledWith(sinon.match.instanceOf(Error)).should.be.true
+        await expect(
+          this.ProjectManager.promises.updateProjectWithLocks(
+            this.project_id,
+            this.projectHistoryId,
+            this.user_id,
+            this.updates,
+            this.version,
+            this.source
+          )
+        ).to.be.rejected
       })
     })
   })