[docstore] finish async/await migration (#26295)

* [docstore] DocManager.getDocLines returns flat content

* [docstore] peekDoc throws NotFoundError, skip check in HttpController

* [docstore] getFullDoc throws NotFoundError, skip check in HttpController

* [docstore] migrate HealthChecker to async/await

* [docstore] migrate HttpController to async/await

* [docstore] remove .promises/callbackify wrapper from all the modules

GitOrigin-RevId: a9938b03cdd2b5e80c2c999039e8f63b20d59dc5

package-lock.json

@@ -42871,6 +42871,7 @@
     "services/docstore": {
       "name": "@overleaf/docstore",
       "dependencies": {
+        "@overleaf/fetch-utils": "*",
         "@overleaf/logger": "*",
         "@overleaf/metrics": "*",
         "@overleaf/o-error": "*",

services/docstore/app/js/DocArchiveManager.js

@@ -1,5 +1,4 @@
-const { callbackify } = require('node:util')
-const MongoManager = require('./MongoManager').promises
+const MongoManager = require('./MongoManager')
 const Errors = require('./Errors')
 const logger = require('@overleaf/logger')
 const Settings = require('@overleaf/settings')
@@ -8,29 +7,12 @@ const { ReadableString } = require('@overleaf/stream-utils')
 const RangeManager = require('./RangeManager')
 const PersistorManager = require('./PersistorManager')
 const pMap = require('p-map')
-const { streamToBuffer } = require('./StreamToBuffer').promises
+const { streamToBuffer } = require('./StreamToBuffer')
 const { BSON } = require('mongodb-legacy')
 
 const PARALLEL_JOBS = Settings.parallelArchiveJobs
 const UN_ARCHIVE_BATCH_SIZE = Settings.unArchiveBatchSize
 
-module.exports = {
-  archiveAllDocs: callbackify(archiveAllDocs),
-  archiveDoc: callbackify(archiveDoc),
-  unArchiveAllDocs: callbackify(unArchiveAllDocs),
-  unarchiveDoc: callbackify(unarchiveDoc),
-  destroyProject: callbackify(destroyProject),
-  getDoc: callbackify(getDoc),
-  promises: {
-    archiveAllDocs,
-    archiveDoc,
-    unArchiveAllDocs,
-    unarchiveDoc,
-    destroyProject,
-    getDoc,
-  },
-}
-
 async function archiveAllDocs(projectId) {
   if (!_isArchivingEnabled()) {
     return
@@ -225,3 +207,12 @@ function _isArchivingEnabled() {
 
   return true
 }
+
+module.exports = {
+  archiveAllDocs,
+  archiveDoc,
+  unArchiveAllDocs,
+  unarchiveDoc,
+  destroyProject,
+  getDoc,
+}

services/docstore/app/js/DocManager.js

@@ -5,7 +5,6 @@ const _ = require('lodash')
 const DocArchive = require('./DocArchiveManager')
 const RangeManager = require('./RangeManager')
 const Settings = require('@overleaf/settings')
-const { callbackifyAll } = require('@overleaf/promise-utils')
 const { setTimeout } = require('node:timers/promises')
 
 /**
@@ -29,7 +28,7 @@ const DocManager = {
       throw new Error('must include inS3 when getting doc')
     }
 
-    const doc = await MongoManager.promises.findDoc(projectId, docId, filter)
+    const doc = await MongoManager.findDoc(projectId, docId, filter)
 
     if (doc == null) {
       throw new Errors.NotFoundError(
@@ -38,7 +37,7 @@ const DocManager = {
     }
 
     if (doc.inS3) {
-      await DocArchive.promises.unarchiveDoc(projectId, docId)
+      await DocArchive.unarchiveDoc(projectId, docId)
       return await DocManager._getDoc(projectId, docId, filter)
     }
 
@@ -46,7 +45,7 @@ const DocManager = {
   },
 
   async isDocDeleted(projectId, docId) {
-    const doc = await MongoManager.promises.findDoc(projectId, docId, {
+    const doc = await MongoManager.findDoc(projectId, docId, {
       deleted: true,
     })
 
@@ -74,7 +73,7 @@ const DocManager = {
 
   // returns the doc without any version information
   async _peekRawDoc(projectId, docId) {
-    const doc = await MongoManager.promises.findDoc(projectId, docId, {
+    const doc = await MongoManager.findDoc(projectId, docId, {
       lines: true,
       rev: true,
       deleted: true,
@@ -91,7 +90,7 @@ const DocManager = {
 
     if (doc.inS3) {
       // skip the unarchiving to mongo when getting a doc
-      const archivedDoc = await DocArchive.promises.getDoc(projectId, docId)
+      const archivedDoc = await DocArchive.getDoc(projectId, docId)
       Object.assign(doc, archivedDoc)
     }
 
@@ -102,7 +101,7 @@ const DocManager = {
   // without unarchiving it (avoids unnecessary writes to mongo)
   async peekDoc(projectId, docId) {
     const doc = await DocManager._peekRawDoc(projectId, docId)
-    await MongoManager.promises.checkRevUnchanged(doc)
+    await MongoManager.checkRevUnchanged(doc)
     return doc
   },
 
@@ -111,16 +110,18 @@ const DocManager = {
       lines: true,
       inS3: true,
     })
-    return doc
+    if (!doc) throw new Errors.NotFoundError()
+    if (!Array.isArray(doc.lines)) throw new Errors.DocWithoutLinesError()
+    return doc.lines.join('\n')
   },
 
   async getAllDeletedDocs(projectId, filter) {
-    return await MongoManager.promises.getProjectsDeletedDocs(projectId, filter)
+    return await MongoManager.getProjectsDeletedDocs(projectId, filter)
   },
 
   async getAllNonDeletedDocs(projectId, filter) {
-    await DocArchive.promises.unArchiveAllDocs(projectId)
-    const docs = await MongoManager.promises.getProjectsDocs(
+    await DocArchive.unArchiveAllDocs(projectId)
+    const docs = await MongoManager.getProjectsDocs(
       projectId,
       { include_deleted: false },
       filter
@@ -132,11 +133,7 @@ const DocManager = {
   },
 
   async projectHasRanges(projectId) {
-    const docs = await MongoManager.promises.getProjectsDocs(
-      projectId,
-      {},
-      { _id: 1 }
-    )
+    const docs = await MongoManager.getProjectsDocs(projectId, {}, { _id: 1 })
     const docIds = docs.map(doc => doc._id)
     for (const docId of docIds) {
       const doc = await DocManager.peekDoc(projectId, docId)
@@ -247,7 +244,7 @@ const DocManager = {
       }
 
       modified = true
-      await MongoManager.promises.upsertIntoDocCollection(
+      await MongoManager.upsertIntoDocCollection(
         projectId,
         docId,
         doc?.rev,
@@ -262,11 +259,7 @@ const DocManager = {
 
   async patchDoc(projectId, docId, meta) {
     const projection = { _id: 1, deleted: true }
-    const doc = await MongoManager.promises.findDoc(
-      projectId,
-      docId,
-      projection
-    )
+    const doc = await MongoManager.findDoc(projectId, docId, projection)
     if (!doc) {
       throw new Errors.NotFoundError(
         `No such project/doc to delete: ${projectId}/${docId}`
@@ -275,7 +268,7 @@ const DocManager = {
 
     if (meta.deleted && Settings.docstore.archiveOnSoftDelete) {
       // The user will not read this doc anytime soon. Flush it out of mongo.
-      DocArchive.promises.archiveDoc(projectId, docId).catch(err => {
+      DocArchive.archiveDoc(projectId, docId).catch(err => {
         logger.warn(
           { projectId, docId, err },
           'archiving a single doc in the background failed'
@@ -283,15 +276,8 @@ const DocManager = {
       })
     }
 
-    await MongoManager.promises.patchDoc(projectId, docId, meta)
+    await MongoManager.patchDoc(projectId, docId, meta)
   },
 }
 
-module.exports = {
-  ...callbackifyAll(DocManager, {
-    multiResult: {
-      updateDoc: ['modified', 'rev'],
-    },
-  }),
-  promises: DocManager,
-}
+module.exports = DocManager

services/docstore/app/js/Errors.js

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

services/docstore/app/js/HealthChecker.js

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

services/docstore/app/js/HttpController.js

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

services/docstore/app/js/MongoManager.js

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

services/docstore/app/js/StreamToBuffer.js

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

services/docstore/package.json

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

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

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

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

@@ -181,6 +181,13 @@ module.exports = DocstoreClient = {
     )
   },
 
+  healthCheck(callback) {
+    request.get(
+      `http://127.0.0.1:${settings.internal.docstore.port}/health_check`,
+      callback
+    )
+  },
+
   getS3Doc(projectId, docId, callback) {
     getStringFromPersistor(
       Persistor,

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

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

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

@@ -17,19 +17,15 @@ describe('DocManager', function () {
     this.version = 42
 
     this.MongoManager = {
-      promises: {
-        findDoc: sinon.stub(),
-        getProjectsDocs: sinon.stub(),
-        patchDoc: sinon.stub().resolves(),
-        upsertIntoDocCollection: sinon.stub().resolves(),
-      },
+      findDoc: sinon.stub(),
+      getProjectsDocs: sinon.stub(),
+      patchDoc: sinon.stub().resolves(),
+      upsertIntoDocCollection: sinon.stub().resolves(),
     }
     this.DocArchiveManager = {
-      promises: {
-        unarchiveDoc: sinon.stub(),
-        unArchiveAllDocs: sinon.stub(),
-        archiveDoc: sinon.stub().resolves(),
-      },
+      unarchiveDoc: sinon.stub(),
+      unArchiveAllDocs: sinon.stub(),
+      archiveDoc: sinon.stub().resolves(),
     }
     this.RangeManager = {
       jsonRangesToMongo(r) {
@@ -52,7 +48,7 @@ describe('DocManager', function () {
 
   describe('getFullDoc', function () {
     beforeEach(function () {
-      this.DocManager.promises._getDoc = sinon.stub()
+      this.DocManager._getDoc = sinon.stub()
       this.doc = {
         _id: this.doc_id,
         lines: ['2134'],
@@ -60,13 +56,10 @@ describe('DocManager', function () {
     })
 
     it('should call get doc with a quick filter', async function () {
-      this.DocManager.promises._getDoc.resolves(this.doc)
-      const doc = await this.DocManager.promises.getFullDoc(
-        this.project_id,
-        this.doc_id
-      )
+      this.DocManager._getDoc.resolves(this.doc)
+      const doc = await this.DocManager.getFullDoc(this.project_id, this.doc_id)
       doc.should.equal(this.doc)
-      this.DocManager.promises._getDoc
+      this.DocManager._getDoc
         .calledWith(this.project_id, this.doc_id, {
           lines: true,
           rev: true,
@@ -79,27 +72,27 @@ describe('DocManager', function () {
     })
 
     it('should return error when get doc errors', async function () {
-      this.DocManager.promises._getDoc.rejects(this.stubbedError)
+      this.DocManager._getDoc.rejects(this.stubbedError)
       await expect(
-        this.DocManager.promises.getFullDoc(this.project_id, this.doc_id)
+        this.DocManager.getFullDoc(this.project_id, this.doc_id)
       ).to.be.rejectedWith(this.stubbedError)
     })
   })
 
   describe('getRawDoc', function () {
     beforeEach(function () {
-      this.DocManager.promises._getDoc = sinon.stub()
+      this.DocManager._getDoc = sinon.stub()
       this.doc = { lines: ['2134'] }
     })
 
     it('should call get doc with a quick filter', async function () {
-      this.DocManager.promises._getDoc.resolves(this.doc)
-      const doc = await this.DocManager.promises.getDocLines(
+      this.DocManager._getDoc.resolves(this.doc)
+      const content = await this.DocManager.getDocLines(
         this.project_id,
         this.doc_id
       )
-      doc.should.equal(this.doc)
-      this.DocManager.promises._getDoc
+      content.should.equal(this.doc.lines.join('\n'))
+      this.DocManager._getDoc
         .calledWith(this.project_id, this.doc_id, {
           lines: true,
           inS3: true,
@@ -108,11 +101,25 @@ describe('DocManager', function () {
     })
 
     it('should return error when get doc errors', async function () {
-      this.DocManager.promises._getDoc.rejects(this.stubbedError)
+      this.DocManager._getDoc.rejects(this.stubbedError)
       await expect(
-        this.DocManager.promises.getDocLines(this.project_id, this.doc_id)
+        this.DocManager.getDocLines(this.project_id, this.doc_id)
       ).to.be.rejectedWith(this.stubbedError)
     })
+
+    it('should return error when get doc does not exist', async function () {
+      this.DocManager._getDoc.resolves(null)
+      await expect(
+        this.DocManager.getDocLines(this.project_id, this.doc_id)
+      ).to.be.rejectedWith(Errors.NotFoundError)
+    })
+
+    it('should return error when get doc has no lines', async function () {
+      this.DocManager._getDoc.resolves({})
+      await expect(
+        this.DocManager.getDocLines(this.project_id, this.doc_id)
+      ).to.be.rejectedWith(Errors.DocWithoutLinesError)
+    })
   })
 
   describe('getDoc', function () {
@@ -128,26 +135,25 @@ describe('DocManager', function () {
 
     describe('when using a filter', function () {
       beforeEach(function () {
-        this.MongoManager.promises.findDoc.resolves(this.doc)
+        this.MongoManager.findDoc.resolves(this.doc)
       })
 
       it('should error if inS3 is not set to true', async function () {
         await expect(
-          this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
+          this.DocManager._getDoc(this.project_id, this.doc_id, {
             inS3: false,
           })
         ).to.be.rejected
       })
 
       it('should always get inS3 even when no filter is passed', async function () {
-        await expect(
-          this.DocManager.promises._getDoc(this.project_id, this.doc_id)
-        ).to.be.rejected
-        this.MongoManager.promises.findDoc.called.should.equal(false)
+        await expect(this.DocManager._getDoc(this.project_id, this.doc_id)).to
+          .be.rejected
+        this.MongoManager.findDoc.called.should.equal(false)
       })
 
       it('should not error if inS3 is set to true', async function () {
-        await this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
+        await this.DocManager._getDoc(this.project_id, this.doc_id, {
           inS3: true,
         })
       })
@@ -155,8 +161,8 @@ describe('DocManager', function () {
 
     describe('when the doc is in the doc collection', function () {
       beforeEach(async function () {
-        this.MongoManager.promises.findDoc.resolves(this.doc)
-        this.result = await this.DocManager.promises._getDoc(
+        this.MongoManager.findDoc.resolves(this.doc)
+        this.result = await this.DocManager._getDoc(
           this.project_id,
           this.doc_id,
           { version: true, inS3: true }
@@ -164,7 +170,7 @@ describe('DocManager', function () {
       })
 
       it('should get the doc from the doc collection', function () {
-        this.MongoManager.promises.findDoc
+        this.MongoManager.findDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
@@ -177,9 +183,9 @@ describe('DocManager', function () {
 
     describe('when MongoManager.findDoc errors', function () {
       it('should return the error', async function () {
-        this.MongoManager.promises.findDoc.rejects(this.stubbedError)
+        this.MongoManager.findDoc.rejects(this.stubbedError)
         await expect(
-          this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
+          this.DocManager._getDoc(this.project_id, this.doc_id, {
             version: true,
             inS3: true,
           })
@@ -202,15 +208,15 @@ describe('DocManager', function () {
           version: 2,
           inS3: false,
         }
-        this.MongoManager.promises.findDoc.resolves(this.doc)
-        this.DocArchiveManager.promises.unarchiveDoc.callsFake(
+        this.MongoManager.findDoc.resolves(this.doc)
+        this.DocArchiveManager.unarchiveDoc.callsFake(
           async (projectId, docId) => {
-            this.MongoManager.promises.findDoc.resolves({
+            this.MongoManager.findDoc.resolves({
               ...this.unarchivedDoc,
             })
           }
         )
-        this.result = await this.DocManager.promises._getDoc(
+        this.result = await this.DocManager._getDoc(
           this.project_id,
           this.doc_id,
           {
@@ -221,13 +227,13 @@ describe('DocManager', function () {
       })
 
       it('should call the DocArchive to unarchive the doc', function () {
-        this.DocArchiveManager.promises.unarchiveDoc
+        this.DocArchiveManager.unarchiveDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should look up the doc twice', function () {
-        this.MongoManager.promises.findDoc.calledTwice.should.equal(true)
+        this.MongoManager.findDoc.calledTwice.should.equal(true)
       })
 
       it('should return the doc', function () {
@@ -239,9 +245,9 @@ describe('DocManager', function () {
 
     describe('when the doc does not exist in the docs collection', function () {
       it('should return a NotFoundError', async function () {
-        this.MongoManager.promises.findDoc.resolves(null)
+        this.MongoManager.findDoc.resolves(null)
         await expect(
-          this.DocManager.promises._getDoc(this.project_id, this.doc_id, {
+          this.DocManager._getDoc(this.project_id, this.doc_id, {
             version: true,
             inS3: true,
           })
@@ -262,17 +268,17 @@ describe('DocManager', function () {
             lines: ['mock-lines'],
           },
         ]
-        this.MongoManager.promises.getProjectsDocs.resolves(this.docs)
-        this.DocArchiveManager.promises.unArchiveAllDocs.resolves(this.docs)
+        this.MongoManager.getProjectsDocs.resolves(this.docs)
+        this.DocArchiveManager.unArchiveAllDocs.resolves(this.docs)
         this.filter = { lines: true }
-        this.result = await this.DocManager.promises.getAllNonDeletedDocs(
+        this.result = await this.DocManager.getAllNonDeletedDocs(
           this.project_id,
           this.filter
         )
       })
 
       it('should get the project from the database', function () {
-        this.MongoManager.promises.getProjectsDocs.should.have.been.calledWith(
+        this.MongoManager.getProjectsDocs.should.have.been.calledWith(
           this.project_id,
           { include_deleted: false },
           this.filter
@@ -286,13 +292,10 @@ describe('DocManager', function () {
 
     describe('when there are no docs for the project', function () {
       it('should return a NotFoundError', async function () {
-        this.MongoManager.promises.getProjectsDocs.resolves(null)
-        this.DocArchiveManager.promises.unArchiveAllDocs.resolves(null)
+        this.MongoManager.getProjectsDocs.resolves(null)
+        this.DocArchiveManager.unArchiveAllDocs.resolves(null)
         await expect(
-          this.DocManager.promises.getAllNonDeletedDocs(
-            this.project_id,
-            this.filter
-          )
+          this.DocManager.getAllNonDeletedDocs(this.project_id, this.filter)
         ).to.be.rejectedWith(`No docs for project ${this.project_id}`)
       })
     })
@@ -303,7 +306,7 @@ describe('DocManager', function () {
       beforeEach(function () {
         this.lines = ['mock', 'doc', 'lines']
         this.rev = 77
-        this.MongoManager.promises.findDoc.resolves({
+        this.MongoManager.findDoc.resolves({
           _id: new ObjectId(this.doc_id),
         })
         this.meta = {}
@@ -311,7 +314,7 @@ describe('DocManager', function () {
 
       describe('standard path', function () {
         beforeEach(async function () {
-          await this.DocManager.promises.patchDoc(
+          await this.DocManager.patchDoc(
             this.project_id,
             this.doc_id,
             this.meta
@@ -319,14 +322,14 @@ describe('DocManager', function () {
         })
 
         it('should get the doc', function () {
-          expect(this.MongoManager.promises.findDoc).to.have.been.calledWith(
+          expect(this.MongoManager.findDoc).to.have.been.calledWith(
             this.project_id,
             this.doc_id
           )
         })
 
         it('should persist the meta', function () {
-          expect(this.MongoManager.promises.patchDoc).to.have.been.calledWith(
+          expect(this.MongoManager.patchDoc).to.have.been.calledWith(
             this.project_id,
             this.doc_id,
             this.meta
@@ -339,7 +342,7 @@ describe('DocManager', function () {
           this.settings.docstore.archiveOnSoftDelete = false
           this.meta.deleted = true
 
-          await this.DocManager.promises.patchDoc(
+          await this.DocManager.patchDoc(
             this.project_id,
             this.doc_id,
             this.meta
@@ -347,8 +350,7 @@ describe('DocManager', function () {
         })
 
         it('should not flush the doc out of mongo', function () {
-          expect(this.DocArchiveManager.promises.archiveDoc).to.not.have.been
-            .called
+          expect(this.DocArchiveManager.archiveDoc).to.not.have.been.called
         })
       })
 
@@ -356,7 +358,7 @@ describe('DocManager', function () {
         beforeEach(async function () {
           this.settings.docstore.archiveOnSoftDelete = false
           this.meta.deleted = false
-          await this.DocManager.promises.patchDoc(
+          await this.DocManager.patchDoc(
             this.project_id,
             this.doc_id,
             this.meta
@@ -364,8 +366,7 @@ describe('DocManager', function () {
         })
 
         it('should not flush the doc out of mongo', function () {
-          expect(this.DocArchiveManager.promises.archiveDoc).to.not.have.been
-            .called
+          expect(this.DocArchiveManager.archiveDoc).to.not.have.been.called
         })
       })
 
@@ -377,7 +378,7 @@ describe('DocManager', function () {
 
         describe('when the background flush succeeds', function () {
           beforeEach(async function () {
-            await this.DocManager.promises.patchDoc(
+            await this.DocManager.patchDoc(
               this.project_id,
               this.doc_id,
               this.meta
@@ -389,17 +390,18 @@ describe('DocManager', function () {
           })
 
           it('should flush the doc out of mongo', function () {
-            expect(
-              this.DocArchiveManager.promises.archiveDoc
-            ).to.have.been.calledWith(this.project_id, this.doc_id)
+            expect(this.DocArchiveManager.archiveDoc).to.have.been.calledWith(
+              this.project_id,
+              this.doc_id
+            )
           })
         })
 
         describe('when the background flush fails', function () {
           beforeEach(async function () {
             this.err = new Error('foo')
-            this.DocArchiveManager.promises.archiveDoc.rejects(this.err)
-            await this.DocManager.promises.patchDoc(
+            this.DocArchiveManager.archiveDoc.rejects(this.err)
+            await this.DocManager.patchDoc(
               this.project_id,
               this.doc_id,
               this.meta
@@ -422,9 +424,9 @@ describe('DocManager', function () {
 
     describe('when the doc does not exist', function () {
       it('should return a NotFoundError', async function () {
-        this.MongoManager.promises.findDoc.resolves(null)
+        this.MongoManager.findDoc.resolves(null)
         await expect(
-          this.DocManager.promises.patchDoc(this.project_id, this.doc_id, {})
+          this.DocManager.patchDoc(this.project_id, this.doc_id, {})
         ).to.be.rejectedWith(
           `No such project/doc to delete: ${this.project_id}/${this.doc_id}`
         )
@@ -470,13 +472,13 @@ describe('DocManager', function () {
         ranges: this.originalRanges,
       }
 
-      this.DocManager.promises._getDoc = sinon.stub()
+      this.DocManager._getDoc = sinon.stub()
     })
 
     describe('when only the doc lines have changed', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.newDocLines,
@@ -486,7 +488,7 @@ describe('DocManager', function () {
       })
 
       it('should get the existing doc', function () {
-        this.DocManager.promises._getDoc
+        this.DocManager._getDoc
           .calledWith(this.project_id, this.doc_id, {
             version: true,
             rev: true,
@@ -498,7 +500,7 @@ describe('DocManager', function () {
       })
 
       it('should upsert the document to the doc collection', function () {
-        this.MongoManager.promises.upsertIntoDocCollection
+        this.MongoManager.upsertIntoDocCollection
           .calledWith(this.project_id, this.doc_id, this.rev, {
             lines: this.newDocLines,
           })
@@ -512,9 +514,9 @@ describe('DocManager', function () {
 
     describe('when the doc ranges have changed', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
         this.RangeManager.shouldUpdateRanges.returns(true)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.oldDocLines,
@@ -524,7 +526,7 @@ describe('DocManager', function () {
       })
 
       it('should upsert the ranges', function () {
-        this.MongoManager.promises.upsertIntoDocCollection
+        this.MongoManager.upsertIntoDocCollection
           .calledWith(this.project_id, this.doc_id, this.rev, {
             ranges: this.newRanges,
           })
@@ -538,8 +540,8 @@ describe('DocManager', function () {
 
     describe('when only the version has changed', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.oldDocLines,
@@ -549,7 +551,7 @@ describe('DocManager', function () {
       })
 
       it('should update the version', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.should.have.been.calledWith(
+        this.MongoManager.upsertIntoDocCollection.should.have.been.calledWith(
           this.project_id,
           this.doc_id,
           this.rev,
@@ -564,8 +566,8 @@ describe('DocManager', function () {
 
     describe('when the doc has not changed at all', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.oldDocLines,
@@ -575,9 +577,7 @@ describe('DocManager', function () {
       })
 
       it('should not update the ranges or lines or version', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.called.should.equal(
-          false
-        )
+        this.MongoManager.upsertIntoDocCollection.called.should.equal(false)
       })
 
       it('should return the old rev and modified == false', function () {
@@ -588,7 +588,7 @@ describe('DocManager', function () {
     describe('when the version is null', function () {
       it('should return an error', async function () {
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             this.newDocLines,
@@ -602,7 +602,7 @@ describe('DocManager', function () {
     describe('when the lines are null', function () {
       it('should return an error', async function () {
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             null,
@@ -616,7 +616,7 @@ describe('DocManager', function () {
     describe('when the ranges are null', function () {
       it('should return an error', async function () {
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             this.newDocLines,
@@ -630,9 +630,9 @@ describe('DocManager', function () {
     describe('when there is a generic error getting the doc', function () {
       beforeEach(async function () {
         this.error = new Error('doc could not be found')
-        this.DocManager.promises._getDoc = sinon.stub().rejects(this.error)
+        this.DocManager._getDoc = sinon.stub().rejects(this.error)
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             this.newDocLines,
@@ -643,16 +643,15 @@ describe('DocManager', function () {
       })
 
       it('should not upsert the document to the doc collection', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.should.not.have.been
-          .called
+        this.MongoManager.upsertIntoDocCollection.should.not.have.been.called
       })
     })
 
     describe('when the version was decremented', function () {
       it('should return an error', async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
         await expect(
-          this.DocManager.promises.updateDoc(
+          this.DocManager.updateDoc(
             this.project_id,
             this.doc_id,
             this.newDocLines,
@@ -665,8 +664,8 @@ describe('DocManager', function () {
 
     describe('when the doc lines have not changed', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.oldDocLines.slice(),
@@ -676,9 +675,7 @@ describe('DocManager', function () {
       })
 
       it('should not update the doc', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.called.should.equal(
-          false
-        )
+        this.MongoManager.upsertIntoDocCollection.called.should.equal(false)
       })
 
       it('should return the existing rev', function () {
@@ -688,8 +685,8 @@ describe('DocManager', function () {
 
     describe('when the doc does not exist', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(null)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.DocManager._getDoc = sinon.stub().resolves(null)
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.newDocLines,
@@ -699,7 +696,7 @@ describe('DocManager', function () {
       })
 
       it('should upsert the document to the doc collection', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.should.have.been.calledWith(
+        this.MongoManager.upsertIntoDocCollection.should.have.been.calledWith(
           this.project_id,
           this.doc_id,
           undefined,
@@ -718,12 +715,12 @@ describe('DocManager', function () {
 
     describe('when another update is racing', function () {
       beforeEach(async function () {
-        this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc)
-        this.MongoManager.promises.upsertIntoDocCollection
+        this.DocManager._getDoc = sinon.stub().resolves(this.doc)
+        this.MongoManager.upsertIntoDocCollection
           .onFirstCall()
           .rejects(new Errors.DocRevValueError())
         this.RangeManager.shouldUpdateRanges.returns(true)
-        this.result = await this.DocManager.promises.updateDoc(
+        this.result = await this.DocManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.newDocLines,
@@ -733,7 +730,7 @@ describe('DocManager', function () {
       })
 
       it('should upsert the doc twice', function () {
-        this.MongoManager.promises.upsertIntoDocCollection.should.have.been.calledWith(
+        this.MongoManager.upsertIntoDocCollection.should.have.been.calledWith(
           this.project_id,
           this.doc_id,
           this.rev,
@@ -743,8 +740,7 @@ describe('DocManager', function () {
             version: this.version + 1,
           }
         )
-        this.MongoManager.promises.upsertIntoDocCollection.should.have.been
-          .calledTwice
+        this.MongoManager.upsertIntoDocCollection.should.have.been.calledTwice
       })
 
       it('should return the new rev', function () {

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

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

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

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