From f397d79439bbbfb98a03ba665eff1a4cd3d26e6e Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Wed, 8 Nov 2023 09:17:19 -0500 Subject: [PATCH] Merge pull request #15648 from overleaf/em-promisify-doc-manager Promisify DocManager GitOrigin-RevId: c9ab368086492900e1617d5d96943d405f25883d --- package-lock.json | 2 + services/docstore/app/js/DocManager.js | 571 +++++-------- services/docstore/app/js/MongoManager.js | 56 +- services/docstore/package.json | 1 + services/docstore/test/setup.js | 2 + .../docstore/test/unit/js/DocManagerTests.js | 778 ++++++++---------- .../test/unit/js/MongoManagerTests.js | 52 +- 7 files changed, 598 insertions(+), 864 deletions(-) diff --git a/package-lock.json b/package-lock.json index d55fe7bf2e..f9d675e1f9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38001,6 +38001,7 @@ "@overleaf/metrics": "*", "@overleaf/o-error": "*", "@overleaf/object-persistor": "*", + "@overleaf/promise-utils": "*", "@overleaf/settings": "*", "@overleaf/stream-utils": "^0.1.0", "async": "^3.2.2", @@ -46489,6 +46490,7 @@ "@overleaf/metrics": "*", "@overleaf/o-error": "*", "@overleaf/object-persistor": "*", + "@overleaf/promise-utils": "*", "@overleaf/settings": "*", "@overleaf/stream-utils": "^0.1.0", "async": "^3.2.2", diff --git a/services/docstore/app/js/DocManager.js b/services/docstore/app/js/DocManager.js index b68f928d77..288d2725e5 100644 --- a/services/docstore/app/js/DocManager.js +++ b/services/docstore/app/js/DocManager.js @@ -1,16 +1,3 @@ -/* eslint-disable - no-dupe-keys, - no-undef, -*/ -// 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 - */ -let DocManager const MongoManager = require('./MongoManager') const Errors = require('./Errors') const logger = require('@overleaf/logger') @@ -18,394 +5,286 @@ 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('timers/promises') -module.exports = DocManager = { +const DocManager = { // TODO: For historical reasons, the doc version is currently stored in the docOps // collection (which is all that this collection contains). In future, we should // migrate this version property to be part of the docs collection, to guarantee // consitency between lines and version when writing/reading, and for a simpler schema. - _getDoc(projectId, docId, filter, callback) { + async _getDoc(projectId, docId, filter) { if (filter == null) { filter = {} } - if (callback == null) { - callback = function () {} - } if (filter.inS3 !== true) { - return callback(new Error('must include inS3 when getting doc')) + throw new Error('must include inS3 when getting doc') } - return MongoManager.findDoc(projectId, docId, filter, function (err, doc) { - if (err != null) { - return callback(err) - } else if (doc == null) { - return callback( - new Errors.NotFoundError( - `No such doc: ${docId} in project ${projectId}` - ) - ) - } else if (doc != null ? doc.inS3 : undefined) { - return DocArchive.unarchiveDoc(projectId, docId, function (err) { - if (err != null) { - logger.err({ err, projectId, docId }, 'error unarchiving doc') - return callback(err) - } - return DocManager._getDoc(projectId, docId, filter, callback) - }) - } else { - if (filter.version) { - return MongoManager.getDocVersion(docId, function (error, version) { - if (error != null) { - return callback(error) - } - doc.version = version - return callback(err, doc) - }) - } else { - return callback(err, doc) - } - } + const doc = await MongoManager.promises.findDoc(projectId, docId, filter) + + if (doc == null) { + throw new Errors.NotFoundError( + `No such doc: ${docId} in project ${projectId}` + ) + } + + if (doc.inS3) { + await DocArchive.promises.unarchiveDoc(projectId, docId) + return await DocManager._getDoc(projectId, docId, filter) + } + + if (filter.version) { + const version = await MongoManager.promises.getDocVersion(docId) + doc.version = version + } + + return doc + }, + + async isDocDeleted(projectId, docId) { + const doc = await MongoManager.promises.findDoc(projectId, docId, { + deleted: true, }) - }, - isDocDeleted(projectId, docId, callback) { - MongoManager.findDoc( - projectId, - docId, - { deleted: true }, - function (err, doc) { - if (err) { - return callback(err) - } - if (!doc) { - return callback( - new Errors.NotFoundError( - `No such project/doc: ${projectId}/${docId}` - ) - ) - } - // `doc.deleted` is `undefined` for non deleted docs - callback(null, Boolean(doc.deleted)) - } - ) - }, - - getFullDoc(projectId, docId, callback) { - if (callback == null) { - callback = function () {} + if (!doc) { + throw new Errors.NotFoundError( + `No such project/doc: ${projectId}/${docId}` + ) } - return DocManager._getDoc( - projectId, - docId, - { - lines: true, - rev: true, - deleted: true, - version: true, - ranges: true, - inS3: true, - }, - function (err, doc) { - if (err != null) { - return callback(err) - } - return callback(err, doc) - } - ) + + // `doc.deleted` is `undefined` for non deleted docs + return Boolean(doc.deleted) + }, + + async getFullDoc(projectId, docId) { + const doc = await DocManager._getDoc(projectId, docId, { + lines: true, + rev: true, + deleted: true, + version: true, + ranges: true, + inS3: true, + }) + return doc }, // returns the doc without any version information - _peekRawDoc(projectId, docId, callback) { - MongoManager.findDoc( - projectId, - docId, - { - lines: true, - rev: true, - deleted: true, - version: true, - ranges: true, - inS3: true, - }, - (err, doc) => { - if (err) return callback(err) - if (doc == null) { - return callback( - new Errors.NotFoundError( - `No such doc: ${docId} in project ${projectId}` - ) - ) - } - if (doc && !doc.inS3) { - return callback(null, doc) - } - // skip the unarchiving to mongo when getting a doc - DocArchive.getDoc(projectId, docId, function (err, archivedDoc) { - if (err != null) { - logger.err( - { err, projectId, docId }, - 'error getting doc from archive' - ) - return callback(err) - } - Object.assign(doc, archivedDoc) - callback(null, doc) - }) - } - ) + async _peekRawDoc(projectId, docId) { + const doc = await MongoManager.promises.findDoc(projectId, docId, { + lines: true, + rev: true, + deleted: true, + version: true, + ranges: true, + inS3: true, + }) + + if (doc == null) { + throw new Errors.NotFoundError( + `No such doc: ${docId} in project ${projectId}` + ) + } + + if (doc.inS3) { + // skip the unarchiving to mongo when getting a doc + const archivedDoc = await DocArchive.promises.getDoc(projectId, docId) + Object.assign(doc, archivedDoc) + } + + return doc }, // get the doc from mongo if possible, or from the persistent store otherwise, // without unarchiving it (avoids unnecessary writes to mongo) - peekDoc(projectId, docId, callback) { - DocManager._peekRawDoc(projectId, docId, (err, doc) => { - if (err) { - return callback(err) - } - MongoManager.withRevCheck( - doc, - MongoManager.getDocVersion, - function (error, version) { - // If the doc has been modified while we were retrieving it, we - // will get a DocModified error - if (error != null) { - return callback(error) - } - doc.version = version - return callback(err, doc) - } - ) - }) + async peekDoc(projectId, docId) { + const doc = await DocManager._peekRawDoc(projectId, docId) + const version = await MongoManager.promises.getDocVersion(docId) + await MongoManager.promises.checkRevUnchanged(doc) + doc.version = version + return doc }, - getDocLines(projectId, docId, callback) { - if (callback == null) { - callback = function () {} - } - return DocManager._getDoc( + async getDocLines(projectId, docId) { + const doc = await DocManager._getDoc(projectId, docId, { + lines: true, + inS3: true, + }) + return doc + }, + + async getAllDeletedDocs(projectId, filter) { + return await MongoManager.promises.getProjectsDeletedDocs(projectId, filter) + }, + + async getAllNonDeletedDocs(projectId, filter) { + await DocArchive.promises.unArchiveAllDocs(projectId) + const docs = await MongoManager.promises.getProjectsDocs( projectId, - docId, - { lines: true, inS3: true }, - function (err, doc) { - if (err != null) { - return callback(err) - } - return callback(err, doc) - } + { include_deleted: false }, + filter ) - }, - - getAllDeletedDocs(projectId, filter, callback) { - MongoManager.getProjectsDeletedDocs(projectId, filter, callback) - }, - - getAllNonDeletedDocs(projectId, filter, callback) { - if (callback == null) { - callback = function () {} + if (docs == null) { + throw new Errors.NotFoundError(`No docs for project ${projectId}`) } - return DocArchive.unArchiveAllDocs(projectId, function (error) { - if (error != null) { - return callback(error) - } - return MongoManager.getProjectsDocs( - projectId, - { include_deleted: false }, - filter, - function (error, docs) { - if (typeof err !== 'undefined' && err !== null) { - return callback(error) - } else if (docs == null) { - return callback( - new Errors.NotFoundError(`No docs for project ${projectId}`) - ) - } else { - return callback(null, docs) - } - } - ) - }) + return docs }, - updateDoc(projectId, docId, lines, version, ranges, callback) { - DocManager._tryUpdateDoc( - projectId, - docId, - lines, - version, - ranges, - (err, modified, rev) => { - if (err && err instanceof Errors.DocRevValueError) { + async updateDoc(projectId, docId, lines, version, ranges) { + const MAX_ATTEMPTS = 2 + for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { + try { + const { modified, rev } = await DocManager._tryUpdateDoc( + projectId, + docId, + lines, + version, + ranges + ) + return { modified, rev } + } catch (err) { + if (err instanceof Errors.DocRevValueError && attempt < MAX_ATTEMPTS) { // Another updateDoc call was racing with ours. // Retry once in a bit. logger.warn( { projectId, docId, err }, 'detected concurrent updateDoc call' ) - setTimeout(() => { - DocManager._tryUpdateDoc( - projectId, - docId, - lines, - version, - ranges, - callback - ) - }, 100 + Math.random() * 100) + await setTimeout(100 + Math.random() * 100) + continue } else { - callback(err, modified, rev) + throw err } } - ) + } }, - _tryUpdateDoc(projectId, docId, lines, version, ranges, callback) { - if (callback == null) { - callback = function () {} - } + async _tryUpdateDoc(projectId, docId, lines, version, ranges) { if (lines == null || version == null || ranges == null) { - return callback(new Error('no lines, version or ranges provided')) + throw new Error('no lines, version or ranges provided') } - return DocManager._getDoc( - projectId, - docId, - { + let doc + try { + doc = await DocManager._getDoc(projectId, docId, { version: true, rev: true, lines: true, - version: true, ranges: true, inS3: true, - }, - function (err, doc) { - let updateLines, updateRanges, updateVersion - if (err != null && !(err instanceof Errors.NotFoundError)) { - logger.err( - { projectId, docId, err }, - 'error getting document for update' - ) - return callback(err) - } + }) + } catch (err) { + if (err instanceof Errors.NotFoundError) { + doc = null + } else { + throw err + } + } - ranges = RangeManager.jsonRangesToMongo(ranges) + ranges = RangeManager.jsonRangesToMongo(ranges) - if (doc == null) { - // If the document doesn't exist, we'll make sure to create/update all parts of it. - updateLines = true - updateVersion = true - updateRanges = true - } else { - if (doc.version > version) { - // Reject update when the version was decremented. - // Potential reasons: racing flush, broken history. - return callback( - new Errors.DocVersionDecrementedError('rejecting stale update', { - updateVersion: version, - flushedVersion: doc.version, - }) - ) - } - updateLines = !_.isEqual(doc.lines, lines) - updateVersion = doc.version !== version - updateRanges = RangeManager.shouldUpdateRanges(doc.ranges, ranges) - } - - let modified = false - let rev = (doc != null ? doc.rev : undefined) || 0 - - const updateLinesAndRangesIfNeeded = function (cb) { - if (updateLines || updateRanges) { - const update = {} - if (updateLines) { - update.lines = lines - } - if (updateRanges) { - update.ranges = ranges - } - logger.debug({ projectId, docId }, 'updating doc lines and ranges') - - modified = true - rev += 1 // rev will be incremented in mongo by MongoManager.upsertIntoDocCollection - return MongoManager.upsertIntoDocCollection( - projectId, - docId, - doc?.rev, - update, - cb - ) - } else { - logger.debug( - { projectId, docId }, - 'doc lines have not changed - not updating' - ) - return cb() - } - } - - const updateVersionIfNeeded = function (cb) { - if (updateVersion) { - logger.debug( - { - projectId, - docId, - oldVersion: doc != null ? doc.version : undefined, - newVersion: version, - }, - 'updating doc version' - ) - modified = true - return MongoManager.setDocVersion(docId, version, cb) - } else { - logger.debug( - { projectId, docId, version }, - 'doc version has not changed - not updating' - ) - return cb() - } - } - - return updateLinesAndRangesIfNeeded(function (error) { - if (error != null) { - return callback(error) - } - return updateVersionIfNeeded(function (error) { - if (error != null) { - return callback(error) - } - return callback(null, modified, rev) - }) + let updateLines, updateRanges, updateVersion + if (doc == null) { + // If the document doesn't exist, we'll make sure to create/update all parts of it. + updateLines = true + updateVersion = true + updateRanges = true + } else { + if (doc.version > version) { + // Reject update when the version was decremented. + // Potential reasons: racing flush, broken history. + throw new Errors.DocVersionDecrementedError('rejecting stale update', { + updateVersion: version, + flushedVersion: doc.version, }) } - ) + updateLines = !_.isEqual(doc.lines, lines) + updateVersion = doc.version !== version + updateRanges = RangeManager.shouldUpdateRanges(doc.ranges, ranges) + } + + let modified = false + let rev = doc?.rev || 0 + + if (updateLines || updateRanges) { + const update = {} + if (updateLines) { + update.lines = lines + } + if (updateRanges) { + update.ranges = ranges + } + logger.debug({ projectId, docId }, 'updating doc lines and ranges') + + modified = true + rev += 1 // rev will be incremented in mongo by MongoManager.upsertIntoDocCollection + await MongoManager.promises.upsertIntoDocCollection( + projectId, + docId, + doc?.rev, + update + ) + } else { + logger.debug( + { projectId, docId }, + 'doc lines have not changed - not updating' + ) + } + + if (updateVersion) { + logger.debug( + { + projectId, + docId, + oldVersion: doc?.version, + newVersion: version, + }, + 'updating doc version' + ) + modified = true + await MongoManager.promises.setDocVersion(docId, version) + } else { + logger.debug( + { projectId, docId, version }, + 'doc version has not changed - not updating' + ) + } + + return { modified, rev } }, - patchDoc(projectId, docId, meta, callback) { + async patchDoc(projectId, docId, meta) { const projection = { _id: 1, deleted: true } - MongoManager.findDoc(projectId, docId, projection, (error, doc) => { - if (error != null) { - return callback(error) - } - if (!doc) { - return callback( - new Errors.NotFoundError( - `No such project/doc to delete: ${projectId}/${docId}` - ) + const doc = await MongoManager.promises.findDoc( + projectId, + docId, + projection + ) + if (!doc) { + throw new Errors.NotFoundError( + `No such project/doc to delete: ${projectId}/${docId}` + ) + } + + 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 => { + logger.warn( + { projectId, docId, err }, + 'archiving a single doc in the background failed' ) - } + }) + } - if (meta.deleted && Settings.docstore.archiveOnSoftDelete) { - // The user will not read this doc anytime soon. Flush it out of mongo. - DocArchive.archiveDoc(projectId, docId, err => { - if (err) { - logger.warn( - { projectId, docId, err }, - 'archiving a single doc in the background failed' - ) - } - }) - } - - MongoManager.patchDoc(projectId, docId, meta, callback) - }) + await MongoManager.promises.patchDoc(projectId, docId, meta) }, } + +module.exports = { + ...callbackifyAll(DocManager, { + multiResult: { + updateDoc: ['modified', 'rev'], + }, + }), + promises: DocManager, +} diff --git a/services/docstore/app/js/MongoManager.js b/services/docstore/app/js/MongoManager.js index 2abb135838..8297b790c8 100644 --- a/services/docstore/app/js/MongoManager.js +++ b/services/docstore/app/js/MongoManager.js @@ -280,34 +280,34 @@ function getDocRev(docId, callback) { ) } -// Helper method to support optimistic locking. Call the provided method for -// an existing doc and return the result if the rev in mongo is unchanged when -// checked afterwards. If the rev has changed, return a DocModifiedError. -function withRevCheck(doc, method, callback) { - method(doc._id, function (err, result) { +/** + * Helper method to support optimistic locking. + * + * Check that the rev of an existing doc is unchanged. If the rev has + * changed, return a DocModifiedError. + */ +function checkRevUnchanged(doc, callback) { + getDocRev(doc._id, function (err, currentRev) { if (err) return callback(err) - getDocRev(doc._id, function (err, currentRev) { - if (err) return callback(err) - if (isNaN(currentRev) || isNaN(doc.rev)) { - return callback( - new Errors.DocRevValueError('doc rev is NaN', { - doc_id: doc._id, - rev: doc.rev, - currentRev, - }) - ) - } - if (doc.rev !== currentRev) { - return callback( - new Errors.DocModifiedError('doc rev has changed', { - doc_id: doc._id, - rev: doc.rev, - currentRev, - }) - ) - } - callback(null, result) - }) + if (isNaN(currentRev) || isNaN(doc.rev)) { + return callback( + new Errors.DocRevValueError('doc rev is NaN', { + doc_id: doc._id, + rev: doc.rev, + currentRev, + }) + ) + } + if (doc.rev !== currentRev) { + return callback( + new Errors.DocModifiedError('doc rev has changed', { + doc_id: doc._id, + rev: doc.rev, + currentRev, + }) + ) + } + callback() }) } @@ -342,7 +342,7 @@ module.exports = { markDocAsArchived, getDocVersion, setDocVersion, - withRevCheck, + checkRevUnchanged, destroyProject, } diff --git a/services/docstore/package.json b/services/docstore/package.json index 5df199c972..c52f7f13eb 100644 --- a/services/docstore/package.json +++ b/services/docstore/package.json @@ -20,6 +20,7 @@ "@overleaf/metrics": "*", "@overleaf/o-error": "*", "@overleaf/object-persistor": "*", + "@overleaf/promise-utils": "*", "@overleaf/settings": "*", "@overleaf/stream-utils": "^0.1.0", "async": "^3.2.2", diff --git a/services/docstore/test/setup.js b/services/docstore/test/setup.js index 9358e01c7e..92cafdea58 100644 --- a/services/docstore/test/setup.js +++ b/services/docstore/test/setup.js @@ -3,6 +3,7 @@ const sinon = require('sinon') const sinonChai = require('sinon-chai') const chaiAsPromised = require('chai-as-promised') const SandboxedModule = require('sandboxed-module') +const timersPromises = require('timers/promises') process.env.BACKEND = 'gcs' @@ -29,6 +30,7 @@ const stubs = { SandboxedModule.configure({ requires: { '@overleaf/logger': stubs.logger, + 'timers/promises': timersPromises, }, globals: { Buffer, JSON, Math, console, process }, }) diff --git a/services/docstore/test/unit/js/DocManagerTests.js b/services/docstore/test/unit/js/DocManagerTests.js index bbafbe1c69..c47bf360f7 100644 --- a/services/docstore/test/unit/js/DocManagerTests.js +++ b/services/docstore/test/unit/js/DocManagerTests.js @@ -1,125 +1,115 @@ -/* eslint-disable - no-dupe-keys, - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') -const { assert, expect } = require('chai') +const { expect } = require('chai') const modulePath = require('path').join(__dirname, '../../../app/js/DocManager') const { ObjectId } = require('mongodb') const Errors = require('../../../app/js/Errors') describe('DocManager', function () { beforeEach(function () { + this.MongoManager = { + promises: { + findDoc: sinon.stub(), + getDocVersion: sinon.stub(), + getProjectsDocs: sinon.stub(), + patchDoc: sinon.stub().resolves(), + upsertIntoDocCollection: sinon.stub().resolves(), + setDocVersion: sinon.stub().resolves(), + }, + } + this.DocArchiveManager = { + promises: { + unarchiveDoc: sinon.stub(), + unArchiveAllDocs: sinon.stub(), + archiveDoc: sinon.stub().resolves(), + }, + } + this.RangeManager = { + jsonRangesToMongo(r) { + return r + }, + shouldUpdateRanges: sinon.stub().returns(false), + } + this.settings = { docstore: {} } + this.DocManager = SandboxedModule.require(modulePath, { requires: { - './MongoManager': (this.MongoManager = {}), - './DocArchiveManager': (this.DocArchiveManager = {}), - './RangeManager': (this.RangeManager = { - jsonRangesToMongo(r) { - return r - }, - shouldUpdateRanges: sinon.stub().returns(false), - }), - '@overleaf/settings': (this.settings = { docstore: {} }), + './MongoManager': this.MongoManager, + './DocArchiveManager': this.DocArchiveManager, + './RangeManager': this.RangeManager, + '@overleaf/settings': this.settings, './Errors': Errors, }, }) + this.doc_id = ObjectId().toString() this.project_id = ObjectId().toString() this.another_project_id = ObjectId().toString() - this.callback = sinon.stub() - return (this.stubbedError = new Error('blew up')) + this.stubbedError = new Error('blew up') }) describe('getFullDoc', function () { beforeEach(function () { - this.DocManager._getDoc = sinon.stub() - return (this.doc = { + this.DocManager.promises._getDoc = sinon.stub() + this.doc = { _id: this.doc_id, lines: ['2134'], - }) + } }) - it('should call get doc with a quick filter', function (done) { - this.DocManager._getDoc.callsArgWith(3, null, this.doc) - return this.DocManager.getFullDoc( + 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, - (err, doc) => { - if (err) return done(err) - doc.should.equal(this.doc) - this.DocManager._getDoc - .calledWith(this.project_id, this.doc_id, { - lines: true, - rev: true, - deleted: true, - version: true, - ranges: true, - inS3: true, - }) - .should.equal(true) - return done() - } + this.doc_id ) + doc.should.equal(this.doc) + this.DocManager.promises._getDoc + .calledWith(this.project_id, this.doc_id, { + lines: true, + rev: true, + deleted: true, + version: true, + ranges: true, + inS3: true, + }) + .should.equal(true) }) - return it('should return error when get doc errors', function (done) { - this.DocManager._getDoc.callsArgWith(3, 'error') - return this.DocManager.getFullDoc( - this.project_id, - this.doc_id, - (err, exist) => { - err.should.equal('error') - return done() - } - ) + it('should return error when get doc errors', async function () { + this.DocManager.promises._getDoc.rejects(this.stubbedError) + await expect( + this.DocManager.promises.getFullDoc(this.project_id, this.doc_id) + ).to.be.rejectedWith(this.stubbedError) }) }) describe('getRawDoc', function () { beforeEach(function () { - this.DocManager._getDoc = sinon.stub() - return (this.doc = { lines: ['2134'] }) + this.DocManager.promises._getDoc = sinon.stub() + this.doc = { lines: ['2134'] } }) - it('should call get doc with a quick filter', function (done) { - this.DocManager._getDoc.callsArgWith(3, null, this.doc) - return this.DocManager.getDocLines( + 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.project_id, - this.doc_id, - (err, doc) => { - if (err) return done(err) - doc.should.equal(this.doc) - this.DocManager._getDoc - .calledWith(this.project_id, this.doc_id, { - lines: true, - inS3: true, - }) - .should.equal(true) - return done() - } + this.doc_id ) + doc.should.equal(this.doc) + this.DocManager.promises._getDoc + .calledWith(this.project_id, this.doc_id, { + lines: true, + inS3: true, + }) + .should.equal(true) }) - return it('should return error when get doc errors', function (done) { - this.DocManager._getDoc.callsArgWith(3, 'error') - return this.DocManager.getDocLines( - this.project_id, - this.doc_id, - (err, exist) => { - err.should.equal('error') - return done() - } - ) + it('should return error when get doc errors', async function () { + this.DocManager.promises._getDoc.rejects(this.stubbedError) + await expect( + this.DocManager.promises.getDocLines(this.project_id, this.doc_id) + ).to.be.rejectedWith(this.stubbedError) }) }) @@ -132,182 +122,152 @@ describe('DocManager', function () { lines: ['mock-lines'], } this.version = 42 - this.MongoManager.findDoc = sinon.stub() - return (this.MongoManager.getDocVersion = sinon - .stub() - .yields(null, this.version)) + this.MongoManager.promises.getDocVersion.resolves(this.version) }) describe('when using a filter', function () { beforeEach(function () { - return this.MongoManager.findDoc.yields(null, this.doc) + this.MongoManager.promises.findDoc.resolves(this.doc) }) - it('should error if inS3 is not set to true', function (done) { - return this.DocManager._getDoc( - this.project_id, - this.doc_id, - { inS3: false }, - err => { - expect(err).to.exist - return done() - } - ) + it('should error if inS3 is not set to true', async function () { + await expect( + this.DocManager.promises._getDoc(this.project_id, this.doc_id, { + inS3: false, + }) + ).to.be.rejected }) - it('should always get inS3 even when no filter is passed', function (done) { - return this.DocManager._getDoc( - this.project_id, - this.doc_id, - undefined, - err => { - this.MongoManager.findDoc.called.should.equal(false) - expect(err).to.exist - return done() - } - ) + 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) }) - return it('should not error if inS3 is set to true', function (done) { - return this.DocManager._getDoc( - this.project_id, - this.doc_id, - { inS3: true }, - err => { - expect(err).to.not.exist - return done() - } - ) + it('should not error if inS3 is set to true', async function () { + await this.DocManager.promises._getDoc(this.project_id, this.doc_id, { + inS3: true, + }) }) }) describe('when the doc is in the doc collection', function () { - beforeEach(function () { - this.MongoManager.findDoc.yields(null, this.doc) - return this.DocManager._getDoc( + beforeEach(async function () { + this.MongoManager.promises.findDoc.resolves(this.doc) + this.result = await this.DocManager.promises._getDoc( this.project_id, this.doc_id, - { version: true, inS3: true }, - this.callback + { version: true, inS3: true } ) }) it('should get the doc from the doc collection', function () { - return this.MongoManager.findDoc + this.MongoManager.promises.findDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should get the doc version from the docOps collection', function () { - return this.MongoManager.getDocVersion + this.MongoManager.promises.getDocVersion .calledWith(this.doc_id) .should.equal(true) }) - return it('should return the callback with the doc with the version', function () { - this.callback.called.should.equal(true) - const doc = this.callback.args[0][1] - doc.lines.should.equal(this.doc.lines) - return doc.version.should.equal(this.version) + it('should return the callback with the doc with the version', function () { + this.result.lines.should.equal(this.doc.lines) + this.result.version.should.equal(this.version) }) }) describe('without the version filter', function () { - beforeEach(function () { - this.MongoManager.findDoc.yields(null, this.doc) - return this.DocManager._getDoc( - this.project_id, - this.doc_id, - { version: false, inS3: true }, - this.callback - ) + beforeEach(async function () { + this.MongoManager.promises.findDoc.resolves(this.doc) + await this.DocManager.promises._getDoc(this.project_id, this.doc_id, { + version: false, + inS3: true, + }) }) - return it('should not get the doc version from the docOps collection', function () { - return this.MongoManager.getDocVersion.called.should.equal(false) + it('should not get the doc version from the docOps collection', function () { + this.MongoManager.promises.getDocVersion.called.should.equal(false) }) }) describe('when MongoManager.findDoc errors', function () { - beforeEach(function () { - this.MongoManager.findDoc.yields(this.stubbedError) - return this.DocManager._getDoc( - this.project_id, - this.doc_id, - { version: true, inS3: true }, - this.callback - ) - }) - - return it('should return the error', function () { - return this.callback.calledWith(this.stubbedError).should.equal(true) + it('should return the error', async function () { + this.MongoManager.promises.findDoc.rejects(this.stubbedError) + await expect( + this.DocManager.promises._getDoc(this.project_id, this.doc_id, { + version: true, + inS3: true, + }) + ).to.be.rejectedWith(this.stubbedError) }) }) describe('when the doc is archived', function () { - beforeEach(function () { + beforeEach(async function () { this.doc = { _id: this.doc_id, project_id: this.project_id, - lines: ['mock-lines'], inS3: true, } - this.MongoManager.findDoc.yields(null, this.doc) - this.DocArchiveManager.unarchiveDoc = (projectId, docId, callback) => { - this.doc.inS3 = false - return callback() + this.unarchivedDoc = { + _id: this.doc_id, + project_id: this.project_id, + lines: ['mock-lines'], + inS3: false, } - sinon.spy(this.DocArchiveManager, 'unarchiveDoc') - return this.DocManager._getDoc( + this.MongoManager.promises.findDoc.resolves(this.doc) + this.DocArchiveManager.promises.unarchiveDoc.callsFake( + async (projectId, docId) => { + this.MongoManager.promises.findDoc.resolves(this.unarchivedDoc) + } + ) + this.result = await this.DocManager.promises._getDoc( this.project_id, this.doc_id, - { version: true, inS3: true }, - this.callback + { + version: true, + inS3: true, + } ) }) it('should call the DocArchive to unarchive the doc', function () { - return this.DocArchiveManager.unarchiveDoc + this.DocArchiveManager.promises.unarchiveDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should look up the doc twice', function () { - return this.MongoManager.findDoc.calledTwice.should.equal(true) + this.MongoManager.promises.findDoc.calledTwice.should.equal(true) }) - return it('should return the doc', function () { - return this.callback.calledWith(null, this.doc).should.equal(true) + it('should return the doc', function () { + expect(this.result).to.deep.equal(this.unarchivedDoc) }) }) - return describe('when the doc does not exist in the docs collection', function () { - beforeEach(function () { - this.MongoManager.findDoc = sinon.stub().yields(null, null) - return this.DocManager._getDoc( - this.project_id, - this.doc_id, - { version: true, inS3: true }, - this.callback + 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) + await expect( + this.DocManager.promises._getDoc(this.project_id, this.doc_id, { + version: true, + inS3: true, + }) + ).to.be.rejectedWith( + `No such doc: ${this.doc_id} in project ${this.project_id}` ) }) - - return it('should return a NotFoundError', function () { - return this.callback - .calledWith( - sinon.match.has( - 'message', - `No such doc: ${this.doc_id} in project ${this.project_id}` - ) - ) - .should.equal(true) - }) }) }) describe('getAllNonDeletedDocs', function () { describe('when the project exists', function () { - beforeEach(function () { + beforeEach(async function () { this.docs = [ { _id: this.doc_id, @@ -315,14 +275,10 @@ describe('DocManager', function () { lines: ['mock-lines'], }, ] - this.MongoManager.getProjectsDocs = sinon - .stub() - .callsArgWith(3, null, this.docs) - this.DocArchiveManager.unArchiveAllDocs = sinon - .stub() - .callsArgWith(1, null, this.docs) + this.MongoManager.promises.getProjectsDocs.resolves(this.docs) + this.DocArchiveManager.promises.unArchiveAllDocs.resolves(this.docs) this.filter = { lines: true } - return this.DocManager.getAllNonDeletedDocs( + this.result = await this.DocManager.promises.getAllNonDeletedDocs( this.project_id, this.filter, this.callback @@ -330,37 +286,28 @@ describe('DocManager', function () { }) it('should get the project from the database', function () { - return this.MongoManager.getProjectsDocs - .calledWith(this.project_id, { include_deleted: false }, this.filter) - .should.equal(true) - }) - - return it('should return the docs', function () { - return this.callback.calledWith(null, this.docs).should.equal(true) - }) - }) - - return describe('when there are no docs for the project', function () { - beforeEach(function () { - this.MongoManager.getProjectsDocs = sinon - .stub() - .callsArgWith(3, null, null) - this.DocArchiveManager.unArchiveAllDocs = sinon - .stub() - .callsArgWith(1, null) - return this.DocManager.getAllNonDeletedDocs( + this.MongoManager.promises.getProjectsDocs.should.have.been.calledWith( this.project_id, - this.filter, - this.callback + { include_deleted: false }, + this.filter ) }) - return it('should return a NotFoundError', function () { - return this.callback - .calledWith( - sinon.match.has('message', `No docs for project ${this.project_id}`) + it('should return the docs', function () { + expect(this.result).to.deep.equal(this.docs) + }) + }) + + 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) + await expect( + this.DocManager.promises.getAllNonDeletedDocs( + this.project_id, + this.filter ) - .should.equal(true) + ).to.be.rejectedWith(`No docs for project ${this.project_id}`) }) }) }) @@ -370,79 +317,69 @@ describe('DocManager', function () { beforeEach(function () { this.lines = ['mock', 'doc', 'lines'] this.rev = 77 - this.MongoManager.findDoc = sinon - .stub() - .yields(null, { _id: ObjectId(this.doc_id) }) - this.MongoManager.patchDoc = sinon.stub().yields(null) - this.DocArchiveManager.archiveDoc = sinon.stub().yields(null) + this.MongoManager.promises.findDoc.resolves({ + _id: ObjectId(this.doc_id), + }) this.meta = {} }) describe('standard path', function () { - beforeEach(function (done) { - this.callback = sinon.stub().callsFake(done) - this.DocManager.patchDoc( - this.project_id, - this.doc_id, - this.meta, - this.callback - ) - }) - - it('should get the doc', function () { - expect(this.MongoManager.findDoc).to.have.been.calledWith( - this.project_id, - this.doc_id - ) - }) - - it('should persist the meta', function () { - expect(this.MongoManager.patchDoc).to.have.been.calledWith( + beforeEach(async function () { + await this.DocManager.promises.patchDoc( this.project_id, this.doc_id, this.meta ) }) - it('should return the callback', function () { - expect(this.callback).to.have.been.calledWith(null) + it('should get the doc', function () { + expect(this.MongoManager.promises.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( + this.project_id, + this.doc_id, + this.meta + ) }) }) describe('background flush disabled and deleting a doc', function () { - beforeEach(function (done) { + beforeEach(async function () { this.settings.docstore.archiveOnSoftDelete = false this.meta.deleted = true - this.callback = sinon.stub().callsFake(done) - this.DocManager.patchDoc( + await this.DocManager.promises.patchDoc( this.project_id, this.doc_id, - this.meta, - this.callback + this.meta ) }) it('should not flush the doc out of mongo', function () { - expect(this.DocArchiveManager.archiveDoc).to.not.have.been.called + expect(this.DocArchiveManager.promises.archiveDoc).to.not.have.been + .called }) }) describe('background flush enabled and not deleting a doc', function () { - beforeEach(function (done) { + beforeEach(async function () { this.settings.docstore.archiveOnSoftDelete = false this.meta.deleted = false - this.callback = sinon.stub().callsFake(done) - this.DocManager.patchDoc( + await this.DocManager.promises.patchDoc( this.project_id, this.doc_id, - this.meta, - this.callback + this.meta ) }) it('should not flush the doc out of mongo', function () { - expect(this.DocArchiveManager.archiveDoc).to.not.have.been.called + expect(this.DocArchiveManager.promises.archiveDoc).to.not.have.been + .called }) }) @@ -453,14 +390,11 @@ describe('DocManager', function () { }) describe('when the background flush succeeds', function () { - beforeEach(function (done) { - this.DocArchiveManager.archiveDoc = sinon.stub().yields(null) - this.callback = sinon.stub().callsFake(done) - this.DocManager.patchDoc( + beforeEach(async function () { + await this.DocManager.promises.patchDoc( this.project_id, this.doc_id, - this.meta, - this.callback + this.meta ) }) @@ -469,23 +403,20 @@ describe('DocManager', function () { }) it('should flush the doc out of mongo', function () { - expect(this.DocArchiveManager.archiveDoc).to.have.been.calledWith( - this.project_id, - this.doc_id - ) + expect( + this.DocArchiveManager.promises.archiveDoc + ).to.have.been.calledWith(this.project_id, this.doc_id) }) }) describe('when the background flush fails', function () { - beforeEach(function (done) { + beforeEach(async function () { this.err = new Error('foo') - this.DocArchiveManager.archiveDoc = sinon.stub().yields(this.err) - this.callback = sinon.stub().callsFake(done) - this.DocManager.patchDoc( + this.DocArchiveManager.promises.archiveDoc.rejects(this.err) + await this.DocManager.promises.patchDoc( this.project_id, this.doc_id, - this.meta, - this.callback + this.meta ) }) @@ -499,37 +430,23 @@ describe('DocManager', function () { 'archiving a single doc in the background failed' ) }) - - it('should not fail the delete process', function () { - expect(this.callback).to.have.been.calledWith(null) - }) }) }) }) describe('when the doc does not exist', function () { - beforeEach(function () { - this.MongoManager.findDoc = sinon.stub().yields(null) - this.DocManager.patchDoc( - this.project_id, - this.doc_id, - {}, - this.callback - ) - }) - - it('should return a NotFoundError', function () { - expect(this.callback).to.have.been.calledWith( - sinon.match.has( - 'message', - `No such project/doc to delete: ${this.project_id}/${this.doc_id}` - ) + it('should return a NotFoundError', async function () { + this.MongoManager.promises.findDoc.resolves(null) + await expect( + this.DocManager.promises.patchDoc(this.project_id, this.doc_id, {}) + ).to.be.rejectedWith( + `No such project/doc to delete: ${this.project_id}/${this.doc_id}` ) }) }) }) - return describe('updateDoc', function () { + describe('updateDoc', function () { beforeEach(function () { this.oldDocLines = ['old', 'doc', 'lines'] this.newDocLines = ['new', 'doc', 'lines'] @@ -567,31 +484,27 @@ describe('DocManager', function () { ranges: this.originalRanges, } - this.MongoManager.upsertIntoDocCollection = sinon.stub().yields() - this.MongoManager.setDocVersion = sinon.stub().yields() - return (this.DocManager._getDoc = sinon.stub()) + this.DocManager.promises._getDoc = sinon.stub() }) describe('when only the doc lines have changed', function () { - beforeEach(function () { - this.DocManager._getDoc = sinon.stub().callsArgWith(3, null, this.doc) - return this.DocManager.updateDoc( + beforeEach(async function () { + this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc) + this.result = await this.DocManager.promises.updateDoc( this.project_id, this.doc_id, this.newDocLines, this.version, - this.originalRanges, - this.callback + this.originalRanges ) }) it('should get the existing doc', function () { - return this.DocManager._getDoc + this.DocManager.promises._getDoc .calledWith(this.project_id, this.doc_id, { version: true, rev: true, lines: true, - version: true, ranges: true, inS3: true, }) @@ -599,7 +512,7 @@ describe('DocManager', function () { }) it('should upsert the document to the doc collection', function () { - return this.MongoManager.upsertIntoDocCollection + this.MongoManager.promises.upsertIntoDocCollection .calledWith(this.project_id, this.doc_id, this.rev, { lines: this.newDocLines, }) @@ -607,32 +520,29 @@ describe('DocManager', function () { }) it('should not update the version', function () { - return this.MongoManager.setDocVersion.called.should.equal(false) + this.MongoManager.promises.setDocVersion.called.should.equal(false) }) - return it('should return the callback with the new rev', function () { - return this.callback - .calledWith(null, true, this.rev + 1) - .should.equal(true) + it('should return the new rev', function () { + expect(this.result).to.deep.equal({ modified: true, rev: this.rev + 1 }) }) }) describe('when the doc ranges have changed', function () { - beforeEach(function () { - this.DocManager._getDoc = sinon.stub().callsArgWith(3, null, this.doc) + beforeEach(async function () { + this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc) this.RangeManager.shouldUpdateRanges.returns(true) - return this.DocManager.updateDoc( + this.result = await this.DocManager.promises.updateDoc( this.project_id, this.doc_id, this.oldDocLines, this.version, - this.newRanges, - this.callback + this.newRanges ) }) it('should upsert the ranges', function () { - return this.MongoManager.upsertIntoDocCollection + this.MongoManager.promises.upsertIntoDocCollection .calledWith(this.project_id, this.doc_id, this.rev, { ranges: this.newRanges, }) @@ -640,190 +550,153 @@ describe('DocManager', function () { }) it('should not update the version', function () { - return this.MongoManager.setDocVersion.called.should.equal(false) + this.MongoManager.promises.setDocVersion.called.should.equal(false) }) - return it('should return the callback with the new rev', function () { - return this.callback - .calledWith(null, true, this.rev + 1) - .should.equal(true) + it('should return the new rev', function () { + expect(this.result).to.deep.equal({ modified: true, rev: this.rev + 1 }) }) }) describe('when only the version has changed', function () { - beforeEach(function () { - this.DocManager._getDoc = sinon.stub().callsArgWith(3, null, this.doc) - return this.DocManager.updateDoc( + beforeEach(async function () { + this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc) + this.result = await this.DocManager.promises.updateDoc( this.project_id, this.doc_id, this.oldDocLines, this.version + 1, - this.originalRanges, - this.callback + this.originalRanges ) }) it('should not change the lines or ranges', function () { - return this.MongoManager.upsertIntoDocCollection.called.should.equal( + this.MongoManager.promises.upsertIntoDocCollection.called.should.equal( false ) }) it('should update the version', function () { - return this.MongoManager.setDocVersion + this.MongoManager.promises.setDocVersion .calledWith(this.doc_id, this.version + 1) .should.equal(true) }) - return it('should return the callback with the old rev', function () { - return this.callback.calledWith(null, true, this.rev).should.equal(true) + it('should return the callback with the old rev', function () { + expect(this.result).to.deep.equal({ modified: true, rev: this.rev }) }) }) describe('when the doc has not changed at all', function () { - beforeEach(function () { - this.DocManager._getDoc = sinon.stub().callsArgWith(3, null, this.doc) - return this.DocManager.updateDoc( + beforeEach(async function () { + this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc) + this.result = await this.DocManager.promises.updateDoc( this.project_id, this.doc_id, this.oldDocLines, this.version, - this.originalRanges, - this.callback + this.originalRanges ) }) it('should not update the ranges or lines', function () { - return this.MongoManager.upsertIntoDocCollection.called.should.equal( + this.MongoManager.promises.upsertIntoDocCollection.called.should.equal( false ) }) it('should not update the version', function () { - return this.MongoManager.setDocVersion.called.should.equal(false) + this.MongoManager.promises.setDocVersion.called.should.equal(false) }) - return it('should return the callback with the old rev and modified == false', function () { - return this.callback - .calledWith(null, false, this.rev) - .should.equal(true) + it('should return the callback with the old rev and modified == false', function () { + expect(this.result).to.deep.equal({ modified: false, rev: this.rev }) }) }) describe('when the version is null', function () { - beforeEach(function () { - return this.DocManager.updateDoc( - this.project_id, - this.doc_id, - this.newDocLines, - null, - this.originalRanges, - this.callback - ) - }) - - return it('should return an error', function () { - return this.callback - .calledWith( - sinon.match.has('message', 'no lines, version or ranges provided') + it('should return an error', async function () { + await expect( + this.DocManager.promises.updateDoc( + this.project_id, + this.doc_id, + this.newDocLines, + null, + this.originalRanges ) - .should.equal(true) + ).to.be.rejectedWith('no lines, version or ranges provided') }) }) describe('when the lines are null', function () { - beforeEach(function () { - return this.DocManager.updateDoc( - this.project_id, - this.doc_id, - null, - this.version, - this.originalRanges, - this.callback - ) - }) - - return it('should return an error', function () { - return this.callback - .calledWith( - sinon.match.has('message', 'no lines, version or ranges provided') + it('should return an error', async function () { + await expect( + this.DocManager.promises.updateDoc( + this.project_id, + this.doc_id, + null, + this.version, + this.originalRanges, + this.callback ) - .should.equal(true) + ).to.be.rejectedWith('no lines, version or ranges provided') }) }) describe('when the ranges are null', function () { - beforeEach(function () { - return this.DocManager.updateDoc( - this.project_id, - this.doc_id, - this.newDocLines, - this.version, - null, - this.callback - ) - }) - - return it('should return an error', function () { - return this.callback - .calledWith( - sinon.match.has('message', 'no lines, version or ranges provided') + it('should return an error', async function () { + await expect( + this.DocManager.promises.updateDoc( + this.project_id, + this.doc_id, + this.newDocLines, + this.version, + null ) - .should.equal(true) + ).to.be.rejectedWith('no lines, version or ranges provided') }) }) describe('when there is a generic error getting the doc', function () { - beforeEach(function () { + beforeEach(async function () { this.error = new Error('doc could not be found') - this.DocManager._getDoc = sinon - .stub() - .callsArgWith(3, this.error, null, null) - return this.DocManager.updateDoc( - this.project_id, - this.doc_id, - this.newDocLines, - this.version, - this.originalRanges, - this.callback - ) + this.DocManager.promises._getDoc = sinon.stub().rejects(this.error) + await expect( + this.DocManager.promises.updateDoc( + this.project_id, + this.doc_id, + this.newDocLines, + this.version, + this.originalRanges + ) + ).to.be.rejectedWith(this.error) }) it('should not upsert the document to the doc collection', function () { - return this.MongoManager.upsertIntoDocCollection.called.should.equal( - false - ) - }) - - return it('should return the callback with the error', function () { - return this.callback.calledWith(this.error).should.equal(true) + this.MongoManager.promises.upsertIntoDocCollection.should.not.have.been + .called }) }) describe('when the version was decremented', function () { - beforeEach(function () { - this.DocManager._getDoc = sinon.stub().yields(null, this.doc) - this.DocManager.updateDoc( - this.project_id, - this.doc_id, - this.newDocLines, - this.version - 1, - this.originalRanges, - this.callback - ) - }) - - it('should return an error', function () { - this.callback.should.have.been.calledWith( - sinon.match.instanceOf(Errors.DocVersionDecrementedError) - ) + it('should return an error', async function () { + this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc) + await expect( + this.DocManager.promises.updateDoc( + this.project_id, + this.doc_id, + this.newDocLines, + this.version - 1, + this.originalRanges + ) + ).to.be.rejectedWith(Errors.DocVersionDecrementedError) }) }) describe('when the doc lines have not changed', function () { - beforeEach(function () { - this.DocManager._getDoc = sinon.stub().callsArgWith(3, null, this.doc) - return this.DocManager.updateDoc( + beforeEach(async function () { + this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc) + this.result = await this.DocManager.promises.updateDoc( this.project_id, this.doc_id, this.oldDocLines.slice(), @@ -834,33 +707,30 @@ describe('DocManager', function () { }) it('should not update the doc', function () { - return this.MongoManager.upsertIntoDocCollection.called.should.equal( + this.MongoManager.promises.upsertIntoDocCollection.called.should.equal( false ) }) - return it('should return the callback with the existing rev', function () { - return this.callback - .calledWith(null, false, this.rev) - .should.equal(true) + it('should return the callback with the existing rev', function () { + expect(this.result).to.deep.equal({ modified: false, rev: this.rev }) }) }) describe('when the doc does not exist', function () { - beforeEach(function () { - this.DocManager._getDoc = sinon.stub().callsArgWith(3, null, null, null) - return this.DocManager.updateDoc( + beforeEach(async function () { + this.DocManager.promises._getDoc = sinon.stub().resolves(null) + this.result = await this.DocManager.promises.updateDoc( this.project_id, this.doc_id, this.newDocLines, this.version, - this.originalRanges, - this.callback + this.originalRanges ) }) it('should upsert the document to the doc collection', function () { - return this.MongoManager.upsertIntoDocCollection + this.MongoManager.promises.upsertIntoDocCollection .calledWith(this.project_id, this.doc_id, undefined, { lines: this.newDocLines, ranges: this.originalRanges, @@ -869,37 +739,34 @@ describe('DocManager', function () { }) it('should set the version', function () { - return this.MongoManager.setDocVersion + this.MongoManager.promises.setDocVersion .calledWith(this.doc_id, this.version) .should.equal(true) }) - return it('should return the callback with the new rev', function () { - return this.callback.calledWith(null, true, 1).should.equal(true) + it('should return the callback with the new rev', function () { + expect(this.result).to.deep.equal({ modified: true, rev: 1 }) }) }) describe('when another update is racing', function () { - beforeEach(function (done) { - this.DocManager._getDoc = sinon.stub().yields(null, this.doc) - this.MongoManager.upsertIntoDocCollection + beforeEach(async function () { + this.DocManager.promises._getDoc = sinon.stub().resolves(this.doc) + this.MongoManager.promises.upsertIntoDocCollection .onFirstCall() - .yields(new Errors.DocRevValueError()) - this.MongoManager.upsertIntoDocCollection.onSecondCall().yields(null) + .rejects(new Errors.DocRevValueError()) this.RangeManager.shouldUpdateRanges.returns(true) - this.callback.callsFake(done) - this.DocManager.updateDoc( + this.result = await this.DocManager.promises.updateDoc( this.project_id, this.doc_id, this.newDocLines, this.version + 1, - this.newRanges, - this.callback + this.newRanges ) }) it('should upsert the doc twice', function () { - this.MongoManager.upsertIntoDocCollection.should.have.been.calledWith( + this.MongoManager.promises.upsertIntoDocCollection.should.have.been.calledWith( this.project_id, this.doc_id, this.rev, @@ -908,15 +775,16 @@ describe('DocManager', function () { lines: this.newDocLines, } ) - this.MongoManager.upsertIntoDocCollection.should.have.been.calledTwice + this.MongoManager.promises.upsertIntoDocCollection.should.have.been + .calledTwice }) it('should update the version once', function () { - this.MongoManager.setDocVersion.should.have.been.calledOnce + this.MongoManager.promises.setDocVersion.should.have.been.calledOnce }) it('should return the callback with the new rev', function () { - this.callback.should.have.been.calledWith(null, true, this.rev + 1) + expect(this.result).to.deep.equal({ modified: true, rev: this.rev + 1 }) }) }) }) diff --git a/services/docstore/test/unit/js/MongoManagerTests.js b/services/docstore/test/unit/js/MongoManagerTests.js index 97fe193400..6e90253847 100644 --- a/services/docstore/test/unit/js/MongoManagerTests.js +++ b/services/docstore/test/unit/js/MongoManagerTests.js @@ -403,60 +403,42 @@ describe('MongoManager', function () { }) }) - describe('withRevCheck', function () { + describe('checkRevUnchanged', function () { this.beforeEach(function () { this.doc = { _id: ObjectId(), name: 'mock-doc', rev: 1 } - this.testFunction = sinon.stub().yields(null, 'foo') }) it('should call the callback when the rev has not changed', function (done) { this.db.docs.findOne = sinon.stub().callsArgWith(2, null, { rev: 1 }) - this.MongoManager.withRevCheck( - this.doc, - this.testFunction, - (err, result) => { - result.should.equal('foo') - assert.isNull(err) - done() - } - ) + this.MongoManager.checkRevUnchanged(this.doc, err => { + assert.isUndefined(err) + done() + }) }) it('should return an error when the rev has changed', function (done) { this.db.docs.findOne = sinon.stub().callsArgWith(2, null, { rev: 2 }) - this.MongoManager.withRevCheck( - this.doc, - this.testFunction, - (err, result) => { - err.should.be.instanceof(Errors.DocModifiedError) - done() - } - ) + this.MongoManager.checkRevUnchanged(this.doc, err => { + err.should.be.instanceof(Errors.DocModifiedError) + done() + }) }) it('should return a value error if incoming rev is NaN', function (done) { this.db.docs.findOne = sinon.stub().callsArgWith(2, null, { rev: 2 }) this.doc = { _id: ObjectId(), name: 'mock-doc', rev: NaN } - this.MongoManager.withRevCheck( - this.doc, - this.testFunction, - (err, result) => { - err.should.be.instanceof(Errors.DocRevValueError) - done() - } - ) + this.MongoManager.checkRevUnchanged(this.doc, err => { + err.should.be.instanceof(Errors.DocRevValueError) + done() + }) }) it('should return a value error if checked doc rev is NaN', function (done) { this.db.docs.findOne = sinon.stub().callsArgWith(2, null, { rev: NaN }) - this.MongoManager.withRevCheck( - this.doc, - this.testFunction, - (err, result) => { - err.should.be.instanceof(Errors.DocRevValueError) - done() - } - ) + this.MongoManager.checkRevUnchanged(this.doc, err => { + err.should.be.instanceof(Errors.DocRevValueError) + done() + }) }) })