Merge pull request #134 from overleaf/em-ordered-updates

Accept single list of updates for file tree operations

services/document-updater/.eslintrc

@@ -8,7 +8,7 @@
     "prettier/standard"
   ],
   "parserOptions": {
-    "ecmaVersion": 2017
+    "ecmaVersion": 2018
   },
   "plugins": [
     "mocha",

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

@@ -330,19 +330,23 @@ function updateProject(req, res, next) {
     userId,
     docUpdates,
     fileUpdates,
+    updates,
     version
   } = req.body
   logger.log(
-    { projectId, docUpdates, fileUpdates, version },
+    { projectId, updates, docUpdates, fileUpdates, version },
     'updating project via http'
   )
-
+  const allUpdates = _mergeUpdates(
+    docUpdates || [],
+    fileUpdates || [],
+    updates || []
+  )
   ProjectManager.updateProjectWithLocks(
     projectId,
     projectHistoryId,
     userId,
-    docUpdates,
-    fileUpdates,
+    allUpdates,
     version,
     (error) => {
       timer.done()
@@ -412,3 +416,23 @@ function flushQueuedProjects(req, res, next) {
     }
   })
 }
+
+/**
+ * Merge updates from the previous project update interface (docUpdates +
+ * fileUpdates) and the new update interface (updates).
+ */
+function _mergeUpdates(docUpdates, fileUpdates, updates) {
+  const mergedUpdates = []
+  for (const update of docUpdates) {
+    const type = update.docLines != null ? 'add-doc' : 'rename-doc'
+    mergedUpdates.push({ type, ...update })
+  }
+  for (const update of fileUpdates) {
+    const type = update.url != null ? 'add-file' : 'rename-file'
+    mergedUpdates.push({ type, ...update })
+  }
+  for (const update of updates) {
+    mergedUpdates.push(update)
+  }
+  return mergedUpdates
+}

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

@@ -1,18 +1,3 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    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
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let ProjectManager
 const RedisManager = require('./RedisManager')
 const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager')
 const DocumentManager = require('./DocumentManager')
@@ -22,372 +7,300 @@ const logger = require('logger-sharelatex')
 const Metrics = require('./Metrics')
 const Errors = require('./Errors')
 
-module.exports = ProjectManager = {
-  flushProjectWithLocks(project_id, _callback) {
-    if (_callback == null) {
-      _callback = function (error) {}
-    }
-    const timer = new Metrics.Timer('projectManager.flushProjectWithLocks')
-    const callback = function (...args) {
-      timer.done()
-      return _callback(...Array.from(args || []))
-    }
+module.exports = {
+  flushProjectWithLocks,
+  flushAndDeleteProjectWithLocks,
+  queueFlushAndDeleteProject,
+  getProjectDocsTimestamps,
+  getProjectDocsAndFlushIfOld,
+  clearProjectState,
+  updateProjectWithLocks
+}
 
-    return RedisManager.getDocIdsInProject(project_id, function (
-      error,
-      doc_ids
-    ) {
-      if (error != null) {
-        return callback(error)
-      }
-      const jobs = []
-      const errors = []
-      for (const doc_id of Array.from(doc_ids || [])) {
-        ;((doc_id) =>
-          jobs.push((callback) =>
-            DocumentManager.flushDocIfLoadedWithLock(
-              project_id,
-              doc_id,
-              function (error) {
-                if (error != null && error instanceof Errors.NotFoundError) {
-                  logger.warn(
-                    { err: error, project_id, doc_id },
-                    'found deleted doc when flushing'
-                  )
-                  return callback()
-                } else if (error != null) {
-                  logger.error(
-                    { err: error, project_id, doc_id },
-                    'error flushing doc'
-                  )
-                  errors.push(error)
-                  return callback()
-                } else {
-                  return callback()
-                }
-              }
-            )
-          ))(doc_id)
-      }
+function flushProjectWithLocks(projectId, _callback) {
+  const timer = new Metrics.Timer('projectManager.flushProjectWithLocks')
+  const callback = function (...args) {
+    timer.done()
+    _callback(...args)
+  }
 
-      logger.log({ project_id, doc_ids }, 'flushing docs')
-      return async.series(jobs, function () {
-        if (errors.length > 0) {
-          return callback(
-            new Error('Errors flushing docs. See log for details')
+  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 {
-          return callback(null)
+          callback()
         }
       })
     })
-  },
 
-  flushAndDeleteProjectWithLocks(project_id, options, _callback) {
-    if (_callback == null) {
-      _callback = function (error) {}
-    }
-    const timer = new Metrics.Timer(
-      'projectManager.flushAndDeleteProjectWithLocks'
-    )
-    const callback = function (...args) {
-      timer.done()
-      return _callback(...Array.from(args || []))
-    }
-
-    return RedisManager.getDocIdsInProject(project_id, function (
-      error,
-      doc_ids
-    ) {
-      if (error != null) {
-        return callback(error)
-      }
-      const jobs = []
-      const errors = []
-      for (const doc_id of Array.from(doc_ids || [])) {
-        ;((doc_id) =>
-          jobs.push((callback) =>
-            DocumentManager.flushAndDeleteDocWithLock(
-              project_id,
-              doc_id,
-              {},
-              function (error) {
-                if (error != null) {
-                  logger.error(
-                    { err: error, project_id, doc_id },
-                    'error deleting doc'
-                  )
-                  errors.push(error)
-                }
-                return callback()
-              }
-            )
-          ))(doc_id)
+    logger.log({ projectId, docIds }, 'flushing docs')
+    async.series(jobs, () => {
+      if (errors.length > 0) {
+        callback(new Error('Errors flushing docs. See log for details'))
+      } else {
+        callback(null)
       }
+    })
+  })
+}
 
-      logger.log({ project_id, doc_ids }, 'deleting docs')
-      return 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(project_id, options, function (
-          error
-        ) {
-          if (errors.length > 0) {
-            return callback(
-              new Error('Errors deleting docs. See log for details')
-            )
-          } else if (error != null) {
-            return callback(error)
-          } else {
-            return callback(null)
+function flushAndDeleteProjectWithLocks(projectId, options, _callback) {
+  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 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()
+        }
       )
     })
-  },
 
-  queueFlushAndDeleteProject(project_id, callback) {
-    if (callback == null) {
-      callback = function (error) {}
+    logger.log({ 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)
+        }
+      })
+    )
+  })
+}
+
+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)
     }
-    return RedisManager.queueFlushAndDeleteProject(project_id, function (
-      error
-    ) {
-      if (error != null) {
-        logger.error(
-          { project_id, error },
-          'error adding project to flush and delete queue'
-        )
-        return callback(error)
-      }
-      Metrics.inc('queued-delete')
-      return callback()
-    })
-  },
+    Metrics.inc('queued-delete')
+    callback()
+  })
+}
 
-  getProjectDocsTimestamps(project_id, callback) {
-    if (callback == null) {
-      callback = function (error) {}
+function getProjectDocsTimestamps(projectId, callback) {
+  RedisManager.getDocIdsInProject(projectId, (error, docIds) => {
+    if (error) {
+      return callback(error)
     }
-    return RedisManager.getDocIdsInProject(project_id, function (
-      error,
-      doc_ids
-    ) {
-      if (error != null) {
+    if (docIds.length === 0) {
+      return callback(null, [])
+    }
+    RedisManager.getDocTimestamps(docIds, (error, timestamps) => {
+      if (error) {
         return callback(error)
       }
-      if (!(doc_ids != null ? doc_ids.length : undefined)) {
-        return callback(null, [])
-      }
-      return RedisManager.getDocTimestamps(doc_ids, function (
-        error,
-        timestamps
-      ) {
-        if (error != null) {
-          return callback(error)
-        }
-        return callback(null, timestamps)
-      })
+      callback(null, timestamps)
     })
-  },
+  })
+}
 
-  getProjectDocsAndFlushIfOld(
-    project_id,
-    projectStateHash,
-    excludeVersions,
-    _callback
-  ) {
-    if (excludeVersions == null) {
-      excludeVersions = {}
-    }
-    if (_callback == null) {
-      _callback = function (error, docs) {}
-    }
-    const timer = new Metrics.Timer(
-      'projectManager.getProjectDocsAndFlushIfOld'
-    )
-    const callback = function (...args) {
-      timer.done()
-      return _callback(...Array.from(args || []))
-    }
+function getProjectDocsAndFlushIfOld(
+  projectId,
+  projectStateHash,
+  excludeVersions,
+  _callback
+) {
+  const timer = new Metrics.Timer('projectManager.getProjectDocsAndFlushIfOld')
+  const callback = function (...args) {
+    timer.done()
+    _callback(...args)
+  }
 
-    return RedisManager.checkOrSetProjectState(
-      project_id,
-      projectStateHash,
-      function (error, projectStateChanged) {
-        if (error != null) {
+  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(
+          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, project_id },
-            'error getting/setting project state in getProjectDocsAndFlushIfOld'
+            { err: error, projectId },
+            'error getting doc ids in getProjectDocs'
           )
           return callback(error)
         }
-        // we can't return docs if project structure has changed
-        if (projectStateChanged) {
-          return callback(
-            Errors.ProjectStateChangedError('project state changed')
+        // 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)
+            }
           )
-        }
-        // project structure hasn't changed, return doc content from redis
-        return RedisManager.getDocIdsInProject(project_id, function (
-          error,
-          doc_ids
-        ) {
-          if (error != null) {
-            logger.error(
-              { err: error, project_id },
-              'error getting doc ids in getProjectDocs'
-            )
+        })
+        async.series(jobs, (error, docs) => {
+          if (error) {
             return callback(error)
           }
-          const jobs = []
-          for (const doc_id of Array.from(doc_ids || [])) {
-            ;((doc_id) =>
-              jobs.push((
-                cb // get the doc lines from redis
-              ) =>
-                DocumentManager.getDocAndFlushIfOldWithLock(
-                  project_id,
-                  doc_id,
-                  function (err, lines, version) {
-                    if (err != null) {
-                      logger.error(
-                        { err, project_id, doc_id },
-                        'error getting project doc lines in getProjectDocsAndFlushIfOld'
-                      )
-                      return cb(err)
-                    }
-                    const doc = { _id: doc_id, lines, v: version } // create a doc object to return
-                    return cb(null, doc)
-                  }
-                )
-              ))(doc_id)
-          }
-          return async.series(jobs, function (error, docs) {
-            if (error != null) {
-              return callback(error)
-            }
-            return callback(null, docs)
-          })
+          callback(null, docs)
         })
-      }
-    )
-  },
-
-  clearProjectState(project_id, callback) {
-    if (callback == null) {
-      callback = function (error) {}
+      })
     }
-    return RedisManager.clearProjectState(project_id, callback)
-  },
+  )
+}
 
-  updateProjectWithLocks(
-    project_id,
-    projectHistoryId,
-    user_id,
-    docUpdates,
-    fileUpdates,
-    version,
-    _callback
-  ) {
-    if (_callback == null) {
-      _callback = function (error) {}
-    }
-    const timer = new Metrics.Timer('projectManager.updateProject')
-    const callback = function (...args) {
-      timer.done()
-      return _callback(...Array.from(args || []))
-    }
+function clearProjectState(projectId, callback) {
+  RedisManager.clearProjectState(projectId, callback)
+}
 
-    const project_version = version
-    let project_subversion = 0 // project versions can have multiple operations
+function updateProjectWithLocks(
+  projectId,
+  projectHistoryId,
+  userId,
+  updates,
+  projectVersion,
+  _callback
+) {
+  const timer = new Metrics.Timer('projectManager.updateProject')
+  const callback = function (...args) {
+    timer.done()
+    _callback(...args)
+  }
 
-    let project_ops_length = 0
+  let projectSubversion = 0 // project versions can have multiple operations
+  let projectOpsLength = 0
 
-    const handleDocUpdate = function (projectUpdate, cb) {
-      const doc_id = projectUpdate.id
-      projectUpdate.version = `${project_version}.${project_subversion++}`
-      if (projectUpdate.docLines != null) {
-        return ProjectHistoryRedisManager.queueAddEntity(
-          project_id,
+  function handleUpdate(update, cb) {
+    update.version = `${projectVersion}.${projectSubversion++}`
+    switch (update.type) {
+      case 'add-doc':
+        ProjectHistoryRedisManager.queueAddEntity(
+          projectId,
           projectHistoryId,
           'doc',
-          doc_id,
-          user_id,
-          projectUpdate,
-          function (error, count) {
-            project_ops_length = count
-            return cb(error)
+          update.id,
+          userId,
+          update,
+          (error, count) => {
+            projectOpsLength = count
+            cb(error)
           }
         )
-      } else {
-        return DocumentManager.renameDocWithLock(
-          project_id,
-          doc_id,
-          user_id,
-          projectUpdate,
+        break
+      case 'rename-doc':
+        DocumentManager.renameDocWithLock(
+          projectId,
+          update.id,
+          userId,
+          update,
           projectHistoryId,
-          function (error, count) {
-            project_ops_length = count
-            return cb(error)
+          (error, count) => {
+            projectOpsLength = count
+            cb(error)
           }
         )
-      }
-    }
-
-    const handleFileUpdate = function (projectUpdate, cb) {
-      const file_id = projectUpdate.id
-      projectUpdate.version = `${project_version}.${project_subversion++}`
-      if (projectUpdate.url != null) {
-        return ProjectHistoryRedisManager.queueAddEntity(
-          project_id,
+        break
+      case 'add-file':
+        ProjectHistoryRedisManager.queueAddEntity(
+          projectId,
           projectHistoryId,
           'file',
-          file_id,
-          user_id,
-          projectUpdate,
-          function (error, count) {
-            project_ops_length = count
-            return cb(error)
+          update.id,
+          userId,
+          update,
+          (error, count) => {
+            projectOpsLength = count
+            cb(error)
           }
         )
-      } else {
-        return ProjectHistoryRedisManager.queueRenameEntity(
-          project_id,
+        break
+      case 'rename-file':
+        ProjectHistoryRedisManager.queueRenameEntity(
+          projectId,
           projectHistoryId,
           'file',
-          file_id,
-          user_id,
-          projectUpdate,
-          function (error, count) {
-            project_ops_length = count
-            return cb(error)
+          update.id,
+          userId,
+          update,
+          (error, count) => {
+            projectOpsLength = count
+            cb(error)
           }
         )
-      }
+        break
+      default:
+        cb(new Error(`Unknown update type: ${update.type}`))
     }
-
-    return async.eachSeries(docUpdates, handleDocUpdate, function (error) {
-      if (error != null) {
-        return callback(error)
-      }
-      return async.eachSeries(fileUpdates, handleFileUpdate, function (error) {
-        if (error != null) {
-          return callback(error)
-        }
-        if (
-          HistoryManager.shouldFlushHistoryOps(
-            project_ops_length,
-            docUpdates.length + fileUpdates.length,
-            HistoryManager.FLUSH_PROJECT_EVERY_N_OPS
-          )
-        ) {
-          HistoryManager.flushProjectChangesAsync(project_id)
-        }
-        return callback()
-      })
-    })
   }
+
+  async.eachSeries(updates, handleUpdate, (error) => {
+    if (error) {
+      return callback(error)
+    }
+    if (
+      HistoryManager.shouldFlushHistoryOps(
+        projectOpsLength,
+        updates.length,
+        HistoryManager.FLUSH_PROJECT_EVERY_N_OPS
+      )
+    ) {
+      HistoryManager.flushProjectChangesAsync(projectId)
+    }
+    callback()
+  })
 }

services/document-updater/package.json

@@ -55,7 +55,7 @@
     "prettier": "^2.0.5",
     "prettier-eslint-cli": "^5.0.0",
     "sandboxed-module": "~0.2.0",
-    "sinon": "~1.5.2",
+    "sinon": "^9.0.2",
     "timekeeper": "^2.0.0"
   }
 }

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

@@ -121,26 +121,26 @@ describe('Flushing a doc to Mongo', function () {
         version: this.version
       })
       let t = 30000
-      sinon.stub(
-        MockWebApi,
-        'setDocument',
-        (
-          project_id,
-          doc_id,
-          lines,
-          version,
-          ranges,
-          lastUpdatedAt,
-          lastUpdatedBy,
-          callback
-        ) => {
-          if (callback == null) {
-            callback = function (error) {}
+      sinon
+        .stub(MockWebApi, 'setDocument')
+        .callsFake(
+          (
+            project_id,
+            doc_id,
+            lines,
+            version,
+            ranges,
+            lastUpdatedAt,
+            lastUpdatedBy,
+            callback
+          ) => {
+            if (callback == null) {
+              callback = function (error) {}
+            }
+            setTimeout(callback, t)
+            return (t = 0)
           }
-          setTimeout(callback, t)
-          return (t = 0)
-        }
-      )
+        )
       return DocUpdaterClient.preloadDoc(this.project_id, this.doc_id, done)
     })
 

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

@@ -216,12 +216,14 @@ describe('Getting a document', function () {
         DocUpdaterClient.randomId(),
         DocUpdaterClient.randomId()
       ])
-      sinon.stub(MockWebApi, 'getDocument', (project_id, doc_id, callback) => {
-        if (callback == null) {
-          callback = function (error, doc) {}
-        }
-        return callback(new Error('oops'))
-      })
+      sinon
+        .stub(MockWebApi, 'getDocument')
+        .callsFake((project_id, doc_id, callback) => {
+          if (callback == null) {
+            callback = function (error, doc) {}
+          }
+          return callback(new Error('oops'))
+        })
       return DocUpdaterClient.getDoc(
         this.project_id,
         this.doc_id,
@@ -248,12 +250,14 @@ describe('Getting a document', function () {
         DocUpdaterClient.randomId(),
         DocUpdaterClient.randomId()
       ])
-      sinon.stub(MockWebApi, 'getDocument', (project_id, doc_id, callback) => {
-        if (callback == null) {
-          callback = function (error, doc) {}
-        }
-        return setTimeout(callback, 30000)
-      })
+      sinon
+        .stub(MockWebApi, 'getDocument')
+        .callsFake((project_id, doc_id, callback) => {
+          if (callback == null) {
+            callback = function (error, doc) {}
+          }
+          return setTimeout(callback, 30000)
+        })
       return done()
     })
 

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

@@ -88,9 +88,9 @@ describe('Setting a document', function () {
     })
 
     after(function () {
-      MockTrackChangesApi.flushDoc.reset()
-      MockProjectHistoryApi.flushProject.reset()
-      MockWebApi.setDocument.reset()
+      MockTrackChangesApi.flushDoc.resetHistory()
+      MockProjectHistoryApi.flushProject.resetHistory()
+      MockWebApi.setDocument.resetHistory()
     })
 
     it('should return a 204 status code', function () {
@@ -171,9 +171,9 @@ describe('Setting a document', function () {
     })
 
     after(function () {
-      MockTrackChangesApi.flushDoc.reset()
-      MockProjectHistoryApi.flushProject.reset()
-      MockWebApi.setDocument.reset()
+      MockTrackChangesApi.flushDoc.resetHistory()
+      MockProjectHistoryApi.flushProject.resetHistory()
+      MockWebApi.setDocument.resetHistory()
     })
 
     it('should return a 204 status code', function () {
@@ -254,9 +254,9 @@ describe('Setting a document', function () {
       })
 
       after(function () {
-        MockTrackChangesApi.flushDoc.reset()
-        MockProjectHistoryApi.flushProject.reset()
-        MockWebApi.setDocument.reset()
+        MockTrackChangesApi.flushDoc.resetHistory()
+        MockProjectHistoryApi.flushProject.resetHistory()
+        MockWebApi.setDocument.resetHistory()
       })
 
       it(`should return a ${testCase.expectedStatusCode} status code`, function () {
@@ -310,9 +310,9 @@ describe('Setting a document', function () {
     })
 
     after(function () {
-      MockTrackChangesApi.flushDoc.reset()
-      MockProjectHistoryApi.flushProject.reset()
-      MockWebApi.setDocument.reset()
+      MockTrackChangesApi.flushDoc.resetHistory()
+      MockProjectHistoryApi.flushProject.resetHistory()
+      MockWebApi.setDocument.resetHistory()
     })
 
     it('should return a 204 status code', function () {
@@ -388,9 +388,9 @@ describe('Setting a document', function () {
       })
 
       after(function () {
-        MockTrackChangesApi.flushDoc.reset()
-        MockProjectHistoryApi.flushProject.reset()
-        MockWebApi.setDocument.reset()
+        MockTrackChangesApi.flushDoc.resetHistory()
+        MockProjectHistoryApi.flushProject.resetHistory()
+        MockWebApi.setDocument.resetHistory()
       })
 
       it('should undo the tracked changes', function (done) {
@@ -451,9 +451,9 @@ describe('Setting a document', function () {
       })
 
       after(function () {
-        MockTrackChangesApi.flushDoc.reset()
-        MockProjectHistoryApi.flushProject.reset()
-        MockWebApi.setDocument.reset()
+        MockTrackChangesApi.flushDoc.resetHistory()
+        MockProjectHistoryApi.flushProject.resetHistory()
+        MockWebApi.setDocument.resetHistory()
       })
 
       it('should not undo the tracked changes', function (done) {

services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js

@@ -52,7 +52,8 @@ describe('DocumentManager', function () {
         './RealTimeRedisManager': (this.RealTimeRedisManager = {}),
         './DiffCodec': (this.DiffCodec = {}),
         './UpdateManager': (this.UpdateManager = {}),
-        './RangesManager': (this.RangesManager = {})
+        './RangesManager': (this.RangesManager = {}),
+        './Errors': Errors
       }
     })
     this.project_id = 'project-id-123'
@@ -765,10 +766,9 @@ describe('DocumentManager', function () {
       })
 
       return it('should call the callback with a not found error', function () {
-        const error = new Errors.NotFoundError(
-          `document not found: ${this.doc_id}`
-        )
-        return this.callback.calledWith(error).should.equal(true)
+        return this.callback
+          .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
+          .should.equal(true)
       })
     })
   })
@@ -848,10 +848,9 @@ describe('DocumentManager', function () {
       })
 
       return it('should call the callback with a not found error', function () {
-        const error = new Errors.NotFoundError(
-          `document not found: ${this.doc_id}`
-        )
-        return this.callback.calledWith(error).should.equal(true)
+        return this.callback
+          .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
+          .should.equal(true)
       })
     })
   })

services/document-updater/test/unit/js/HistoryRedisManager/HistoryRedisManagerTests.js

@@ -95,7 +95,7 @@ describe('HistoryRedisManager', function () {
 
       return it('should call the callback with an error', function () {
         return this.callback
-          .calledWith(new Error('cannot push no ops'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })

services/document-updater/test/unit/js/HttpController/HttpControllerTests.js

@@ -159,7 +159,7 @@ describe('HttpController', function () {
 
       it('should call next with NotFoundError', function () {
         this.next
-          .calledWith(new Errors.NotFoundError('not found'))
+          .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
     })
@@ -173,7 +173,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
@@ -252,7 +252,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
 
@@ -327,7 +327,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
@@ -387,7 +387,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
@@ -477,7 +477,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
@@ -550,7 +550,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
@@ -640,7 +640,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
@@ -703,7 +703,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
@@ -804,17 +804,39 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
 
-  describe('updateProject', function () {
+  describe('updateProject (split doc and file updates)', function () {
     beforeEach(function () {
       this.projectHistoryId = 'history-id-123'
       this.userId = 'user-id-123'
-      this.docUpdates = sinon.stub()
-      this.fileUpdates = sinon.stub()
+      this.docUpdates = [
+        { id: 1, pathname: 'thesis.tex', newPathname: 'book.tex' },
+        { id: 2, pathname: 'article.tex', docLines: 'hello' }
+      ]
+      this.fileUpdates = [
+        { id: 3, pathname: 'apple.png', newPathname: 'banana.png' },
+        { id: 4, url: 'filestore.example.com/4' }
+      ]
+      this.expectedUpdates = [
+        {
+          type: 'rename-doc',
+          id: 1,
+          pathname: 'thesis.tex',
+          newPathname: 'book.tex'
+        },
+        { type: 'add-doc', id: 2, pathname: 'article.tex', docLines: 'hello' },
+        {
+          type: 'rename-file',
+          id: 3,
+          pathname: 'apple.png',
+          newPathname: 'banana.png'
+        },
+        { type: 'add-file', id: 4, url: 'filestore.example.com/4' }
+      ]
       this.version = 1234567
       this.req = {
         query: {},
@@ -832,10 +854,84 @@ describe('HttpController', function () {
     })
 
     describe('successfully', function () {
+      beforeEach(function () {
+        this.ProjectManager.updateProjectWithLocks = sinon.stub().yields()
+        this.HttpController.updateProject(this.req, this.res, this.next)
+      })
+
+      it('should accept the change', function () {
+        this.ProjectManager.updateProjectWithLocks
+          .calledWith(
+            this.project_id,
+            this.projectHistoryId,
+            this.userId,
+            this.expectedUpdates,
+            this.version
+          )
+          .should.equal(true)
+      })
+
+      it('should return a successful No Content response', function () {
+        this.res.sendStatus.calledWith(204).should.equal(true)
+      })
+
+      it('should time the request', function () {
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
+      })
+    })
+
+    describe('when an errors occurs', function () {
       beforeEach(function () {
         this.ProjectManager.updateProjectWithLocks = sinon
           .stub()
-          .callsArgWith(6)
+          .yields(new Error('oops'))
+        this.HttpController.updateProject(this.req, this.res, this.next)
+      })
+
+      it('should call next with the error', function () {
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
+      })
+    })
+  })
+
+  describe('updateProject (single updates parameter)', function () {
+    beforeEach(function () {
+      this.projectHistoryId = 'history-id-123'
+      this.userId = 'user-id-123'
+      this.updates = [
+        {
+          type: 'rename-doc',
+          id: 1,
+          pathname: 'thesis.tex',
+          newPathname: 'book.tex'
+        },
+        { type: 'add-doc', id: 2, pathname: 'article.tex', docLines: 'hello' },
+        {
+          type: 'rename-file',
+          id: 3,
+          pathname: 'apple.png',
+          newPathname: 'banana.png'
+        },
+        { type: 'add-file', id: 4, url: 'filestore.example.com/4' }
+      ]
+      this.version = 1234567
+      this.req = {
+        query: {},
+        body: {
+          projectHistoryId: this.projectHistoryId,
+          userId: this.userId,
+          updates: this.updates,
+          version: this.version
+        },
+        params: {
+          project_id: this.project_id
+        }
+      }
+    })
+
+    describe('successfully', function () {
+      beforeEach(function () {
+        this.ProjectManager.updateProjectWithLocks = sinon.stub().yields()
         this.HttpController.updateProject(this.req, this.res, this.next)
       })
 
@@ -845,8 +941,7 @@ describe('HttpController', function () {
             this.project_id,
             this.projectHistoryId,
             this.userId,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version
           )
           .should.equal(true)
@@ -865,12 +960,12 @@ describe('HttpController', function () {
       beforeEach(function () {
         this.ProjectManager.updateProjectWithLocks = sinon
           .stub()
-          .callsArgWith(6, new Error('oops'))
+          .yields(new Error('oops'))
         this.HttpController.updateProject(this.req, this.res, this.next)
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })
@@ -925,7 +1020,7 @@ describe('HttpController', function () {
       })
 
       it('should call next with the error', function () {
-        this.next.calledWith(new Error('oops')).should.equal(true)
+        this.next.calledWith(sinon.match.instanceOf(Error)).should.equal(true)
       })
     })
   })

services/document-updater/test/unit/js/LockManager/ReleasingTheLock.js

@@ -92,7 +92,7 @@ describe('LockManager - releasing the lock', function () {
 
     return it('should return an error if the lock has expired', function () {
       return this.callback
-        .calledWith(new Error('tried to release timed out lock'))
+        .calledWith(sinon.match.instanceOf(Error))
         .should.equal(true)
     })
   })

services/document-updater/test/unit/js/LockManager/getLockTests.js

@@ -119,9 +119,13 @@ describe('LockManager - getting the lock', function () {
     })
 
     return it('should return the callback with an error', function () {
-      const e = new Error('Timeout')
-      e.doc_id = this.doc_id
-      return this.callback.calledWith(e).should.equal(true)
+      return this.callback
+        .calledWith(
+          sinon.match
+            .instanceOf(Error)
+            .and(sinon.match.has('doc_id', this.doc_id))
+        )
+        .should.equal(true)
     })
   })
 })

services/document-updater/test/unit/js/LockManager/tryLockTests.js

@@ -142,8 +142,9 @@ describe('LockManager - trying the lock', function () {
       })
 
       return it('should return the callback with an error', function () {
-        const e = new Error('tried to release timed out lock')
-        return this.callback.calledWith(e).should.equal(true)
+        return this.callback
+          .calledWith(sinon.match.instanceOf(Error))
+          .should.equal(true)
       })
     })
   })

services/document-updater/test/unit/js/PersistenceManager/PersistenceManagerTests.js

@@ -41,7 +41,8 @@ describe('PersistenceManager', function () {
         'logger-sharelatex': (this.logger = {
           log: sinon.stub(),
           err: sinon.stub()
-        })
+        }),
+        './Errors': Errors
       }
     })
     this.project_id = 'project-id-123'
@@ -171,7 +172,7 @@ describe('PersistenceManager', function () {
 
       it('should return a NotFoundError', function () {
         return this.callback
-          .calledWith(new Errors.NotFoundError('not found'))
+          .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
 
@@ -198,7 +199,7 @@ describe('PersistenceManager', function () {
 
       it('should return an error', function () {
         return this.callback
-          .calledWith(new Error('web api error'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
 
@@ -231,7 +232,7 @@ describe('PersistenceManager', function () {
 
       return it('should return and error', function () {
         return this.callback
-          .calledWith(new Error('web API response had no doc lines'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -254,7 +255,7 @@ describe('PersistenceManager', function () {
 
       return it('should return and error', function () {
         return this.callback
-          .calledWith(new Error('web API response had no valid doc version'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -277,7 +278,7 @@ describe('PersistenceManager', function () {
 
       return it('should return and error', function () {
         return this.callback
-          .calledWith(new Error('web API response had no valid doc pathname'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -386,7 +387,7 @@ describe('PersistenceManager', function () {
 
       it('should return a NotFoundError', function () {
         return this.callback
-          .calledWith(new Errors.NotFoundError('not found'))
+          .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
 
@@ -418,7 +419,7 @@ describe('PersistenceManager', function () {
 
       it('should return an error', function () {
         return this.callback
-          .calledWith(new Error('web api error'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
 

services/document-updater/test/unit/js/ProjectManager/flushAndDeleteProjectTests.js

@@ -140,14 +140,16 @@ describe('ProjectManager - flushAndDeleteProject', function () {
     it('should record the error', function () {
       return this.logger.error
         .calledWith(
-          { err: this.error, project_id: this.project_id, doc_id: 'doc-id-1' },
+          { err: this.error, projectId: this.project_id, docId: 'doc-id-1' },
           'error deleting doc'
         )
         .should.equal(true)
     })
 
     it('should call the callback with an error', function () {
-      return this.callback.calledWith(new Error()).should.equal(true)
+      return this.callback
+        .calledWith(sinon.match.instanceOf(Error))
+        .should.equal(true)
     })
 
     return it('should time the execution', function () {

services/document-updater/test/unit/js/ProjectManager/flushProjectTests.js

@@ -129,14 +129,16 @@ describe('ProjectManager - flushProject', function () {
     it('should record the error', function () {
       return this.logger.error
         .calledWith(
-          { err: this.error, project_id: this.project_id, doc_id: 'doc-id-1' },
+          { err: this.error, projectId: this.project_id, docId: 'doc-id-1' },
           'error flushing doc'
         )
         .should.equal(true)
     })
 
     it('should call the callback with an error', function () {
-      return this.callback.calledWith(new Error()).should.equal(true)
+      return this.callback
+        .calledWith(sinon.match.instanceOf(Error))
+        .should.equal(true)
     })
 
     return it('should time the execution', function () {

services/document-updater/test/unit/js/ProjectManager/getProjectDocsTests.js

@@ -40,7 +40,8 @@ describe('ProjectManager - getProjectDocsAndFlushIfOld', function () {
             Timer.initClass()
             return Timer
           })())
-        })
+        }),
+        './Errors': Errors
       }
     })
     this.project_id = 'project-id-123'
@@ -146,9 +147,7 @@ describe('ProjectManager - getProjectDocsAndFlushIfOld', function () {
 
     it('should call the callback with an error', function () {
       return this.callback
-        .calledWith(
-          new Errors.ProjectStateChangedError('project state changed')
-        )
+        .calledWith(sinon.match.instanceOf(Errors.ProjectStateChangedError))
         .should.equal(true)
     })
 
@@ -187,14 +186,16 @@ describe('ProjectManager - getProjectDocsAndFlushIfOld', function () {
     it('should record the error', function () {
       return this.logger.error
         .calledWith(
-          { err: this.error, project_id: this.project_id, doc_id: 'doc-id-2' },
+          { err: this.error, projectId: this.project_id, docId: 'doc-id-2' },
           'error getting project doc lines in getProjectDocsAndFlushIfOld'
         )
         .should.equal(true)
     })
 
     it('should call the callback with an error', function () {
-      return this.callback.calledWith(new Error('oops')).should.equal(true)
+      return this.callback
+        .calledWith(sinon.match.instanceOf(Error))
+        .should.equal(true)
     })
 
     return it('should time the execution', function () {

services/document-updater/test/unit/js/ProjectManager/updateProjectTests.js

@@ -1,46 +1,40 @@
-/* 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 sinon = require('sinon')
-const chai = require('chai')
-const should = chai.should()
 const modulePath = '../../../../app/js/ProjectManager.js'
 const SandboxedModule = require('sandboxed-module')
 const _ = require('lodash')
 
 describe('ProjectManager', function () {
   beforeEach(function () {
-    let Timer
+    this.RedisManager = {}
+    this.ProjectHistoryRedisManager = {
+      queueRenameEntity: sinon.stub().yields(),
+      queueAddEntity: sinon.stub().yields()
+    }
+    this.DocumentManager = {
+      renameDocWithLock: sinon.stub().yields()
+    }
+    this.HistoryManager = {
+      flushProjectChangesAsync: sinon.stub(),
+      shouldFlushHistoryOps: sinon.stub().returns(false)
+    }
+    this.Metrics = {
+      Timer: class Timer {}
+    }
+    this.Metrics.Timer.prototype.done = sinon.stub()
+
+    this.logger = {
+      log: sinon.stub(),
+      error: sinon.stub()
+    }
+
     this.ProjectManager = SandboxedModule.require(modulePath, {
       requires: {
-        './RedisManager': (this.RedisManager = {}),
-        './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}),
-        './DocumentManager': (this.DocumentManager = {}),
-        'logger-sharelatex': (this.logger = {
-          log: sinon.stub(),
-          error: sinon.stub()
-        }),
-        './HistoryManager': (this.HistoryManager = {}),
-        './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,
+        'logger-sharelatex': this.logger,
+        './HistoryManager': this.HistoryManager,
+        './Metrics': this.Metrics
       }
     })
 
@@ -48,45 +42,44 @@ describe('ProjectManager', function () {
     this.projectHistoryId = 'history-id-123'
     this.user_id = 'user-id-123'
     this.version = 1234567
-    this.HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(false)
-    this.HistoryManager.flushProjectChangesAsync = sinon.stub()
-    return (this.callback = sinon.stub())
+    this.callback = sinon.stub()
   })
 
-  return describe('updateProjectWithLocks', function () {
+  describe('updateProjectWithLocks', function () {
     describe('rename operations', function () {
       beforeEach(function () {
         this.firstDocUpdate = {
+          type: 'rename-doc',
           id: 1,
           pathname: 'foo',
           newPathname: 'foo'
         }
         this.secondDocUpdate = {
+          type: 'rename-doc',
           id: 2,
           pathname: 'bar',
           newPathname: 'bar2'
         }
-        this.docUpdates = [this.firstDocUpdate, this.secondDocUpdate]
         this.firstFileUpdate = {
+          type: 'rename-file',
           id: 2,
           pathname: 'bar',
           newPathname: 'bar2'
         }
-        this.fileUpdates = [this.firstFileUpdate]
-        this.DocumentManager.renameDocWithLock = sinon.stub().yields()
-        return (this.ProjectHistoryRedisManager.queueRenameEntity = sinon
-          .stub()
-          .yields())
+        this.updates = [
+          this.firstDocUpdate,
+          this.secondDocUpdate,
+          this.firstFileUpdate
+        ]
       })
 
       describe('successfully', function () {
         beforeEach(function () {
-          return this.ProjectManager.updateProjectWithLocks(
+          this.ProjectManager.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version,
             this.callback
           )
@@ -110,7 +103,7 @@ describe('ProjectManager', function () {
               this.projectHistoryId
             )
             .should.equal(true)
-          return this.DocumentManager.renameDocWithLock
+          this.DocumentManager.renameDocWithLock
             .calledWith(
               this.project_id,
               this.secondDocUpdate.id,
@@ -127,7 +120,7 @@ describe('ProjectManager', function () {
             this.firstFileUpdate,
             { version: `${this.version}.2` }
           )
-          return this.ProjectHistoryRedisManager.queueRenameEntity
+          this.ProjectHistoryRedisManager.queueRenameEntity
             .calledWith(
               this.project_id,
               this.projectHistoryId,
@@ -140,115 +133,112 @@ describe('ProjectManager', function () {
         })
 
         it('should not flush the history', function () {
-          return this.HistoryManager.flushProjectChangesAsync
+          this.HistoryManager.flushProjectChangesAsync
             .calledWith(this.project_id)
             .should.equal(false)
         })
 
-        return it('should call the callback', function () {
-          return this.callback.called.should.equal(true)
+        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 = sinon
-            .stub()
-            .yields(this.error)
-          return this.ProjectManager.updateProjectWithLocks(
+          this.DocumentManager.renameDocWithLock.yields(this.error)
+          this.ProjectManager.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version,
             this.callback
           )
         })
 
-        return it('should call the callback with the error', function () {
-          return this.callback.calledWith(this.error).should.equal(true)
+        it('should call the callback with the error', function () {
+          this.callback.calledWith(this.error).should.equal(true)
         })
       })
 
       describe('when renaming a file fails', function () {
         beforeEach(function () {
           this.error = new Error('error')
-          this.ProjectHistoryRedisManager.queueRenameEntity = sinon
-            .stub()
-            .yields(this.error)
-          return this.ProjectManager.updateProjectWithLocks(
+          this.ProjectHistoryRedisManager.queueRenameEntity.yields(this.error)
+          this.ProjectManager.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version,
             this.callback
           )
         })
 
-        return it('should call the callback with the error', function () {
-          return this.callback.calledWith(this.error).should.equal(true)
+        it('should call the callback with the error', function () {
+          this.callback.calledWith(this.error).should.equal(true)
         })
       })
 
-      return describe('with enough ops to flush', function () {
+      describe('with enough ops to flush', function () {
         beforeEach(function () {
-          this.HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(true)
-          return this.ProjectManager.updateProjectWithLocks(
+          this.HistoryManager.shouldFlushHistoryOps.returns(true)
+          this.ProjectManager.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version,
             this.callback
           )
         })
 
-        return it('should flush the history', function () {
-          return this.HistoryManager.flushProjectChangesAsync
+        it('should flush the history', function () {
+          this.HistoryManager.flushProjectChangesAsync
             .calledWith(this.project_id)
             .should.equal(true)
         })
       })
     })
 
-    return describe('add operations', function () {
+    describe('add operations', function () {
       beforeEach(function () {
         this.firstDocUpdate = {
+          type: 'add-doc',
           id: 1,
           docLines: 'a\nb'
         }
         this.secondDocUpdate = {
+          type: 'add-doc',
           id: 2,
           docLines: 'a\nb'
         }
-        this.docUpdates = [this.firstDocUpdate, this.secondDocUpdate]
         this.firstFileUpdate = {
+          type: 'add-file',
           id: 3,
           url: 'filestore.example.com/2'
         }
         this.secondFileUpdate = {
+          type: 'add-file',
           id: 4,
           url: 'filestore.example.com/3'
         }
-        this.fileUpdates = [this.firstFileUpdate, this.secondFileUpdate]
-        return (this.ProjectHistoryRedisManager.queueAddEntity = sinon
-          .stub()
-          .yields())
+        this.updates = [
+          this.firstDocUpdate,
+          this.secondDocUpdate,
+          this.firstFileUpdate,
+          this.secondFileUpdate
+        ]
       })
 
       describe('successfully', function () {
         beforeEach(function () {
-          return this.ProjectManager.updateProjectWithLocks(
+          this.ProjectManager.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version,
             this.callback
           )
@@ -274,7 +264,7 @@ describe('ProjectManager', function () {
               firstDocUpdateWithVersion
             )
             .should.equal(true)
-          return this.ProjectHistoryRedisManager.queueAddEntity
+          this.ProjectHistoryRedisManager.queueAddEntity
             .getCall(1)
             .calledWith(
               this.project_id,
@@ -309,7 +299,7 @@ describe('ProjectManager', function () {
               firstFileUpdateWithVersion
             )
             .should.equal(true)
-          return this.ProjectHistoryRedisManager.queueAddEntity
+          this.ProjectHistoryRedisManager.queueAddEntity
             .getCall(3)
             .calledWith(
               this.project_id,
@@ -323,80 +313,91 @@ describe('ProjectManager', function () {
         })
 
         it('should not flush the history', function () {
-          return this.HistoryManager.flushProjectChangesAsync
+          this.HistoryManager.flushProjectChangesAsync
             .calledWith(this.project_id)
             .should.equal(false)
         })
 
-        return it('should call the callback', function () {
-          return this.callback.called.should.equal(true)
+        it('should call the callback', function () {
+          this.callback.called.should.equal(true)
         })
       })
 
       describe('when adding a doc fails', function () {
         beforeEach(function () {
           this.error = new Error('error')
-          this.ProjectHistoryRedisManager.queueAddEntity = sinon
-            .stub()
-            .yields(this.error)
-          return this.ProjectManager.updateProjectWithLocks(
+          this.ProjectHistoryRedisManager.queueAddEntity.yields(this.error)
+          this.ProjectManager.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version,
             this.callback
           )
         })
 
-        return it('should call the callback with the error', function () {
-          return this.callback.calledWith(this.error).should.equal(true)
+        it('should call the callback with the error', function () {
+          this.callback.calledWith(this.error).should.equal(true)
         })
       })
 
       describe('when adding a file fails', function () {
         beforeEach(function () {
           this.error = new Error('error')
-          this.ProjectHistoryRedisManager.queueAddEntity = sinon
-            .stub()
-            .yields(this.error)
-          return this.ProjectManager.updateProjectWithLocks(
+          this.ProjectHistoryRedisManager.queueAddEntity.yields(this.error)
+          this.ProjectManager.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version,
             this.callback
           )
         })
 
-        return it('should call the callback with the error', function () {
-          return this.callback.calledWith(this.error).should.equal(true)
+        it('should call the callback with the error', function () {
+          this.callback.calledWith(this.error).should.equal(true)
         })
       })
 
-      return describe('with enough ops to flush', function () {
+      describe('with enough ops to flush', function () {
         beforeEach(function () {
-          this.HistoryManager.shouldFlushHistoryOps = sinon.stub().returns(true)
-          return this.ProjectManager.updateProjectWithLocks(
+          this.HistoryManager.shouldFlushHistoryOps.returns(true)
+          this.ProjectManager.updateProjectWithLocks(
             this.project_id,
             this.projectHistoryId,
             this.user_id,
-            this.docUpdates,
-            this.fileUpdates,
+            this.updates,
             this.version,
             this.callback
           )
         })
 
-        return it('should flush the history', function () {
-          return this.HistoryManager.flushProjectChangesAsync
+        it('should flush the history', function () {
+          this.HistoryManager.flushProjectChangesAsync
             .calledWith(this.project_id)
             .should.equal(true)
         })
       })
     })
+
+    describe('when given an unknown operation type', function () {
+      beforeEach(function () {
+        this.updates = [{ type: 'brew-coffee' }]
+        this.ProjectManager.updateProjectWithLocks(
+          this.project_id,
+          this.projectHistoryId,
+          this.user_id,
+          this.updates,
+          this.version,
+          this.callback
+        )
+      })
+
+      it('should call back with an error', function () {
+        this.callback.calledWith(sinon.match.instanceOf(Error)).should.be.true
+      })
+    })
   })
 })

services/document-updater/test/unit/js/RealTimeRedisManager/RealTimeRedisManagerTests.js

@@ -118,7 +118,7 @@ describe('RealTimeRedisManager', function () {
 
       return it('should return an error to the callback', function () {
         return this.callback
-          .calledWith(new Error('JSON parse error'))
+          .calledWith(sinon.match.has('name', 'SyntaxError'))
           .should.equal(true)
       })
     })

services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js

@@ -398,7 +398,7 @@ describe('RedisManager', function () {
 
       return it('should return an error', function () {
         return this.callback
-          .calledWith(new Error('redis getDoc exceeded timeout'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -426,7 +426,7 @@ describe('RedisManager', function () {
 
       return it('should return an error', function () {
         return this.callback
-          .calledWith(new Errors.NotFoundError('not found'))
+          .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
     })
@@ -543,11 +543,7 @@ describe('RedisManager', function () {
 
       it('should return an error', function () {
         return this.callback
-          .calledWith(
-            new Errors.OpRangeNotAvailableError(
-              'doc ops range is not loaded in redis'
-            )
-          )
+          .calledWith(sinon.match.instanceOf(Errors.OpRangeNotAvailableError))
           .should.equal(true)
       })
 
@@ -588,7 +584,7 @@ describe('RedisManager', function () {
 
       return it('should return an error', function () {
         return this.callback
-          .calledWith(new Error('redis getPreviousDocOps exceeded timeout'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -854,9 +850,7 @@ describe('RedisManager', function () {
 
       return it('should call the callback with an error', function () {
         return this.callback
-          .calledWith(
-            new Error(`Version mismatch. '${this.doc_id}' is corrupted.`)
-          )
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -954,7 +948,7 @@ describe('RedisManager', function () {
 
       return it('should call the callback with an error', function () {
         return this.callback
-          .calledWith(new Error('null bytes found in doc lines'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -985,7 +979,7 @@ describe('RedisManager', function () {
 
       return it('should call the callback with the error', function () {
         return this.callback
-          .calledWith(new Error('ranges are too large'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -1157,7 +1151,7 @@ describe('RedisManager', function () {
 
       return it('should call the callback with an error', function () {
         return this.callback
-          .calledWith(new Error('null bytes found in doc lines'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
@@ -1185,7 +1179,7 @@ describe('RedisManager', function () {
 
       return it('should call the callback with the error', function () {
         return this.callback
-          .calledWith(new Error('ranges are too large'))
+          .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })

services/document-updater/test/unit/js/ShareJsDB/ShareJsDBTests.js

@@ -26,7 +26,8 @@ describe('ShareJsDB', function () {
     this.callback = sinon.stub()
     this.ShareJsDB = SandboxedModule.require(modulePath, {
       requires: {
-        './RedisManager': (this.RedisManager = {})
+        './RedisManager': (this.RedisManager = {}),
+        './Errors': Errors
       }
     })
 
@@ -68,7 +69,7 @@ describe('ShareJsDB', function () {
 
       return it('should return the callback with a NotFoundError', function () {
         return this.callback
-          .calledWith(new Errors.NotFoundError('not found'))
+          .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
     })

services/document-updater/test/unit/js/ShareJsUpdateManager/ShareJsUpdateManagerTests.js

@@ -194,7 +194,9 @@ describe('ShareJsUpdateManager', function () {
       })
 
       return it('should call the callback with the error', function () {
-        return this.callback.calledWith(this.error).should.equal(true)
+        return this.callback
+          .calledWith(sinon.match.instanceOf(Error))
+          .should.equal(true)
       })
     })
   })