From 25c3699862ede6ca975ae90f9b747ad25e2df46e Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Tue, 10 Jun 2025 15:21:00 +0200 Subject: [PATCH] [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 | 1 + services/docstore/app/js/DocArchiveManager.js | 31 +- services/docstore/app/js/DocManager.js | 50 ++- services/docstore/app/js/Errors.js | 3 + services/docstore/app/js/HealthChecker.js | 84 ++--- services/docstore/app/js/HttpController.js | 288 +++++++----------- services/docstore/app/js/MongoManager.js | 44 +-- services/docstore/app/js/StreamToBuffer.js | 6 +- services/docstore/package.json | 1 + .../test/acceptance/js/HealthCheckerTest.js | 28 ++ .../acceptance/js/helpers/DocstoreClient.js | 7 + .../test/unit/js/DocArchiveManagerTests.js | 212 ++++++------- .../docstore/test/unit/js/DocManagerTests.js | 224 +++++++------- .../test/unit/js/HttpControllerTests.js | 159 +++++----- .../test/unit/js/MongoManagerTests.js | 40 ++- 15 files changed, 534 insertions(+), 644 deletions(-) create mode 100644 services/docstore/test/acceptance/js/HealthCheckerTest.js diff --git a/package-lock.json b/package-lock.json index c0967e0977..ce75c110c4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42871,6 +42871,7 @@ "services/docstore": { "name": "@overleaf/docstore", "dependencies": { + "@overleaf/fetch-utils": "*", "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/o-error": "*", diff --git a/services/docstore/app/js/DocArchiveManager.js b/services/docstore/app/js/DocArchiveManager.js index 4390afe18f..d332a27651 100644 --- a/services/docstore/app/js/DocArchiveManager.js +++ b/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, +} diff --git a/services/docstore/app/js/DocManager.js b/services/docstore/app/js/DocManager.js index a9ed99425c..9b80f83eb9 100644 --- a/services/docstore/app/js/DocManager.js +++ b/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 diff --git a/services/docstore/app/js/Errors.js b/services/docstore/app/js/Errors.js index bbdbe75c08..7b150cc0db 100644 --- a/services/docstore/app/js/Errors.js +++ b/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, } diff --git a/services/docstore/app/js/HealthChecker.js b/services/docstore/app/js/HealthChecker.js index 34cd5c973c..a5b7ad7e9a 100644 --- a/services/docstore/app/js/HealthChecker.js +++ b/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, } diff --git a/services/docstore/app/js/HttpController.js b/services/docstore/app/js/HttpController.js index 1c4e137033..895e8e8e7b 100644 --- a/services/docstore/app/js/HttpController.js +++ b/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)) - } - ) -} - -function projectHasRanges(req, res, next) { - const { project_id: projectId } = req.params - DocManager.projectHasRanges(projectId, (err, projectHasRanges) => { - if (err) { - return next(err) - } - res.json({ projectHasRanges }) + const docs = await DocManager.getAllNonDeletedDocs(projectId, { + ranges: true, }) + res.json(_buildDocsArrayView(projectId, docs)) } -function updateDoc(req, res, next) { +async function projectHasRanges(req, res) { + const { project_id: projectId } = req.params + const projectHasRanges = await DocManager.projectHasRanges(projectId) + res.json({ projectHasRanges }) +} + +async function updateDoc(req, res) { const { doc_id: docId, project_id: projectId } = req.params const lines = req.body?.lines const version = req.body?.version @@ -172,25 +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) + try { + await DocArchive.unArchiveAllDocs(projectId) + } catch (err) { + if (err instanceof Errors.DocRevValueError) { + logger.warn({ err }, 'Failed to unarchive doc') + return res.sendStatus(409) } - res.sendStatus(200) - }) + throw err + } + res.sendStatus(200) } -function destroyProject(req, res, next) { +async function destroyProject(req, res) { const { project_id: projectId } = req.params logger.debug({ projectId }, 'destroying all docs') - DocArchive.destroyProject(projectId, function (error) { - 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), } diff --git a/services/docstore/app/js/MongoManager.js b/services/docstore/app/js/MongoManager.js index ad1a2d2b40..ef101f91c0 100644 --- a/services/docstore/app/js/MongoManager.js +++ b/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, } diff --git a/services/docstore/app/js/StreamToBuffer.js b/services/docstore/app/js/StreamToBuffer.js index 7de146cd11..09215a7367 100644 --- a/services/docstore/app/js/StreamToBuffer.js +++ b/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) { diff --git a/services/docstore/package.json b/services/docstore/package.json index e505f731d3..bf5857fd49 100644 --- a/services/docstore/package.json +++ b/services/docstore/package.json @@ -17,6 +17,7 @@ "types:check": "tsc --noEmit" }, "dependencies": { + "@overleaf/fetch-utils": "*", "@overleaf/logger": "*", "@overleaf/metrics": "*", "@overleaf/o-error": "*", diff --git a/services/docstore/test/acceptance/js/HealthCheckerTest.js b/services/docstore/test/acceptance/js/HealthCheckerTest.js new file mode 100644 index 0000000000..b25a45312b --- /dev/null +++ b/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([]) + }) +}) diff --git a/services/docstore/test/acceptance/js/helpers/DocstoreClient.js b/services/docstore/test/acceptance/js/helpers/DocstoreClient.js index 790ec8f237..d8fe94829b 100644 --- a/services/docstore/test/acceptance/js/helpers/DocstoreClient.js +++ b/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, diff --git a/services/docstore/test/unit/js/DocArchiveManagerTests.js b/services/docstore/test/unit/js/DocArchiveManagerTests.js index a57f9806c8..fbc1667314 100644 --- a/services/docstore/test/unit/js/DocArchiveManagerTests.js +++ b/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 }) }) }) diff --git a/services/docstore/test/unit/js/DocManagerTests.js b/services/docstore/test/unit/js/DocManagerTests.js index 8405520e6e..f207f8e993 100644 --- a/services/docstore/test/unit/js/DocManagerTests.js +++ b/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 () { diff --git a/services/docstore/test/unit/js/HttpControllerTests.js b/services/docstore/test/unit/js/HttpControllerTests.js index bf78696890..ab491ec150 100644 --- a/services/docstore/test/unit/js/HttpControllerTests.js +++ b/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 () { diff --git a/services/docstore/test/unit/js/MongoManagerTests.js b/services/docstore/test/unit/js/MongoManagerTests.js index 4f8467db76..b96b661df4 100644 --- a/services/docstore/test/unit/js/MongoManagerTests.js +++ b/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