diff --git a/services/document-updater/app/js/ProjectManager.js b/services/document-updater/app/js/ProjectManager.js index cdd4c11482..dc755d56e0 100644 --- a/services/document-updater/app/js/ProjectManager.js +++ b/services/document-updater/app/js/ProjectManager.js @@ -2,334 +2,233 @@ const RedisManager = require('./RedisManager') const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager') const DocumentManager = require('./DocumentManager') const HistoryManager = require('./HistoryManager') -const async = require('async') const logger = require('@overleaf/logger') const Metrics = require('./Metrics') const Errors = require('./Errors') -const { promisifyAll } = require('@overleaf/promise-utils') +const { callbackifyAll } = require('@overleaf/promise-utils') -function flushProjectWithLocks(projectId, _callback) { +async function flushProjectWithLocks(projectId) { const timer = new Metrics.Timer('projectManager.flushProjectWithLocks') - const callback = function (...args) { - timer.done() - _callback(...args) + const docIds = await RedisManager.promises.getDocIdsInProject(projectId) + + logger.debug({ projectId, docIds }, 'flushing docs') + const errors = [] + for (const docId of docIds) { + try { + await DocumentManager.promises.flushDocIfLoadedWithLock(projectId, docId) + } catch (error) { + if (error instanceof Errors.NotFoundError) { + logger.warn( + { err: error, projectId, docId }, + 'found deleted doc when flushing' + ) + } else { + logger.error({ err: error, projectId, docId }, 'error flushing doc') + errors.push(error) + } + } } - RedisManager.getDocIdsInProject(projectId, (error, docIds) => { - if (error) { - return callback(error) - } - const errors = [] - const jobs = docIds.map(docId => callback => { - DocumentManager.flushDocIfLoadedWithLock(projectId, docId, error => { - if (error instanceof Errors.NotFoundError) { - logger.warn( - { err: error, projectId, docId }, - 'found deleted doc when flushing' - ) - callback() - } else if (error) { - logger.error({ err: error, projectId, docId }, 'error flushing doc') - errors.push(error) - callback() - } else { - callback() - } - }) - }) - - logger.debug({ projectId, docIds }, 'flushing docs') - async.series(jobs, () => { - if (errors.length > 0) { - callback(new Error('Errors flushing docs. See log for details')) - } else { - callback(null) - } - }) - }) + timer.done() + if (errors.length > 0) { + throw new Error('Errors flushing docs. See log for details') + } } -function flushAndDeleteProjectWithLocks(projectId, options, _callback) { +async function flushAndDeleteProjectWithLocks(projectId, options) { const timer = new Metrics.Timer( 'projectManager.flushAndDeleteProjectWithLocks' ) - const callback = function (...args) { - timer.done() - _callback(...args) + + const docIds = await RedisManager.promises.getDocIdsInProject(projectId) + + logger.debug({ projectId, docIds }, 'deleting docs') + const errors = [] + for (const docId of docIds) { + try { + await DocumentManager.promises.flushAndDeleteDocWithLock( + projectId, + docId, + {} + ) + } catch (error) { + logger.error({ err: error, projectId, docId }, 'error deleting doc') + errors.push(error) + } } - RedisManager.getDocIdsInProject(projectId, (error, docIds) => { - if (error) { - return callback(error) - } - const errors = [] - const jobs = docIds.map(docId => callback => { - DocumentManager.flushAndDeleteDocWithLock(projectId, docId, {}, error => { - if (error) { - logger.error({ err: error, projectId, docId }, 'error deleting doc') - errors.push(error) - } - callback() - }) - }) - - logger.debug({ projectId, docIds }, 'deleting docs') - async.series(jobs, () => - // When deleting the project here we want to ensure that project - // history is completely flushed because the project may be - // deleted in web after this call completes, and so further - // attempts to flush would fail after that. - HistoryManager.flushProjectChanges(projectId, options, error => { - if (errors.length > 0) { - callback(new Error('Errors deleting docs. See log for details')) - } else if (error) { - callback(error) - } else { - callback(null) - } - }) - ) - }) + // When deleting the project here we want to ensure that project + // history is completely flushed because the project may be + // deleted in web after this call completes, and so further + // attempts to flush would fail after that. + await HistoryManager.promises.flushProjectChanges(projectId, options) + timer.done() + if (errors.length > 0) { + throw new Error('Errors deleting docs. See log for details') + } } -function queueFlushAndDeleteProject(projectId, callback) { - RedisManager.queueFlushAndDeleteProject(projectId, error => { - if (error) { - logger.error( - { projectId, error }, - 'error adding project to flush and delete queue' - ) - return callback(error) - } - Metrics.inc('queued-delete') - callback() - }) +async function queueFlushAndDeleteProject(projectId) { + await RedisManager.promises.queueFlushAndDeleteProject(projectId) + Metrics.inc('queued-delete') } -function getProjectDocsTimestamps(projectId, callback) { - RedisManager.getDocIdsInProject(projectId, (error, docIds) => { - if (error) { - return callback(error) - } - if (docIds.length === 0) { - return callback(null, []) - } - RedisManager.getDocTimestamps(docIds, (error, timestamps) => { - if (error) { - return callback(error) - } - callback(null, timestamps) - }) - }) +async function getProjectDocsTimestamps(projectId, callback) { + const docIds = await RedisManager.promises.getDocIdsInProject(projectId) + if (docIds.length === 0) { + return [] + } + + const timestamps = await RedisManager.promises.getDocTimestamps(docIds) + return timestamps } -function getProjectDocsAndFlushIfOld( +async function getProjectDocsAndFlushIfOld( projectId, projectStateHash, - excludeVersions, - _callback + excludeVersions ) { const timer = new Metrics.Timer('projectManager.getProjectDocsAndFlushIfOld') - const callback = function (...args) { + + const projectStateChanged = + await RedisManager.promises.checkOrSetProjectState( + projectId, + projectStateHash + ) + + // we can't return docs if project structure has changed + if (projectStateChanged) { timer.done() - _callback(...args) + throw new Errors.ProjectStateChangedError('project state changed') } - RedisManager.checkOrSetProjectState( - projectId, - projectStateHash, - (error, projectStateChanged) => { - if (error) { - logger.error( - { err: error, projectId }, - 'error getting/setting project state in getProjectDocsAndFlushIfOld' - ) - return callback(error) - } - // we can't return docs if project structure has changed - if (projectStateChanged) { - return callback( - new Errors.ProjectStateChangedError('project state changed') - ) - } - // project structure hasn't changed, return doc content from redis - RedisManager.getDocIdsInProject(projectId, (error, docIds) => { - if (error) { - logger.error( - { err: error, projectId }, - 'error getting doc ids in getProjectDocs' - ) - return callback(error) - } - // get the doc lines from redis - const jobs = docIds.map(docId => cb => { - DocumentManager.getDocAndFlushIfOldWithLock( - projectId, - docId, - (err, lines, version) => { - if (err) { - logger.error( - { err, projectId, docId }, - 'error getting project doc lines in getProjectDocsAndFlushIfOld' - ) - return cb(err) - } - const doc = { _id: docId, lines, v: version } // create a doc object to return - cb(null, doc) - } - ) - }) - async.series(jobs, (error, docs) => { - if (error) { - return callback(error) - } - callback(null, docs) - }) - }) - } - ) + // project structure hasn't changed, return doc content from redis + const docs = [] + const docIds = await RedisManager.promises.getDocIdsInProject(projectId) + for (const docId of docIds) { + const { lines, version } = + await DocumentManager.promises.getDocAndFlushIfOldWithLock( + projectId, + docId + ) + docs.push({ _id: docId, lines, v: version }) + } + + timer.done() + return docs } -function clearProjectState(projectId, callback) { - RedisManager.clearProjectState(projectId, callback) +async function clearProjectState(projectId) { + await RedisManager.promises.clearProjectState(projectId) } -function updateProjectWithLocks( +async function updateProjectWithLocks( projectId, projectHistoryId, userId, updates, projectVersion, - source, - _callback + source ) { const timer = new Metrics.Timer('projectManager.updateProject') - const callback = function (...args) { - timer.done() - _callback(...args) - } let projectSubversion = 0 // project versions can have multiple operations let projectOpsLength = 0 - function handleUpdate(update, cb) { + for (const update of updates) { update.version = `${projectVersion}.${projectSubversion++}` switch (update.type) { case 'add-doc': - ProjectHistoryRedisManager.queueAddEntity( - projectId, - projectHistoryId, - 'doc', - update.id, - userId, - update, - source, - (error, count) => { - projectOpsLength = count - cb(error) - } - ) - break - case 'rename-doc': - if (!update.newPathname) { - // an empty newPathname signifies a delete, so there is no need to - // update the pathname in redis - ProjectHistoryRedisManager.queueRenameEntity( + projectOpsLength = + await ProjectHistoryRedisManager.promises.queueAddEntity( projectId, projectHistoryId, 'doc', update.id, userId, update, - source, - (error, count) => { - projectOpsLength = count - cb(error) - } + source ) + break + case 'rename-doc': + if (!update.newPathname) { + // an empty newPathname signifies a delete, so there is no need to + // update the pathname in redis + projectOpsLength = + await ProjectHistoryRedisManager.promises.queueRenameEntity( + projectId, + projectHistoryId, + 'doc', + update.id, + userId, + update, + source + ) } else { // rename the doc in redis before queuing the update - DocumentManager.renameDocWithLock( + await DocumentManager.promises.renameDocWithLock( projectId, update.id, userId, update, - projectHistoryId, - error => { - if (error) { - return cb(error) - } - ProjectHistoryRedisManager.queueRenameEntity( - projectId, - projectHistoryId, - 'doc', - update.id, - userId, - update, - source, - (error, count) => { - projectOpsLength = count - cb(error) - } - ) - } + projectHistoryId ) + projectOpsLength = + await ProjectHistoryRedisManager.promises.queueRenameEntity( + projectId, + projectHistoryId, + 'doc', + update.id, + userId, + update, + source + ) } break case 'add-file': - ProjectHistoryRedisManager.queueAddEntity( - projectId, - projectHistoryId, - 'file', - update.id, - userId, - update, - source, - (error, count) => { - projectOpsLength = count - cb(error) - } - ) + projectOpsLength = + await ProjectHistoryRedisManager.promises.queueAddEntity( + projectId, + projectHistoryId, + 'file', + update.id, + userId, + update, + source + ) break case 'rename-file': - ProjectHistoryRedisManager.queueRenameEntity( - projectId, - projectHistoryId, - 'file', - update.id, - userId, - update, - source, - (error, count) => { - projectOpsLength = count - cb(error) - } - ) + projectOpsLength = + await ProjectHistoryRedisManager.promises.queueRenameEntity( + projectId, + projectHistoryId, + 'file', + update.id, + userId, + update, + source + ) break default: - cb(new Error(`Unknown update type: ${update.type}`)) + throw new Error(`Unknown update type: ${update.type}`) } } - async.eachSeries(updates, handleUpdate, error => { - if (error) { - return callback(error) - } - if ( - HistoryManager.shouldFlushHistoryOps( - projectId, - projectOpsLength, - updates.length, - HistoryManager.FLUSH_PROJECT_EVERY_N_OPS - ) - ) { - HistoryManager.flushProjectChangesAsync(projectId) - } - callback() - }) + if ( + HistoryManager.shouldFlushHistoryOps( + projectId, + projectOpsLength, + updates.length, + HistoryManager.FLUSH_PROJECT_EVERY_N_OPS + ) + ) { + HistoryManager.flushProjectChangesAsync(projectId) + } + + timer.done() } -module.exports = { +const ProjectManager = { flushProjectWithLocks, flushAndDeleteProjectWithLocks, queueFlushAndDeleteProject, @@ -339,4 +238,7 @@ module.exports = { updateProjectWithLocks, } -module.exports.promises = promisifyAll(module.exports) +module.exports = { + ...callbackifyAll(ProjectManager), + promises: ProjectManager, +} diff --git a/services/document-updater/test/unit/js/ProjectManager/flushAndDeleteProjectTests.js b/services/document-updater/test/unit/js/ProjectManager/flushAndDeleteProjectTests.js index b92fc8bb8d..e354741d52 100644 --- a/services/document-updater/test/unit/js/ProjectManager/flushAndDeleteProjectTests.js +++ b/services/document-updater/test/unit/js/ProjectManager/flushAndDeleteProjectTests.js @@ -1,157 +1,125 @@ -/* eslint-disable - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS206: Consider reworking classes to avoid initClass - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ +const { expect } = require('chai') const sinon = require('sinon') -const modulePath = '../../../../app/js/ProjectManager.js' const SandboxedModule = require('sandboxed-module') +const MODULE_PATH = '../../../../app/js/ProjectManager.js' + describe('ProjectManager - flushAndDeleteProject', function () { beforeEach(function () { - let Timer - this.LockManager = { - getLock: sinon.stub().yields(), - releaseLock: sinon.stub().yields(), + this.project_id = 'project-id-123' + + this.RedisManager = { + promises: { + getDocIdsInProject: sinon.stub(), + }, } - this.ProjectManager = SandboxedModule.require(modulePath, { + + this.ProjectHistoryRedisManager = {} + + this.DocumentManager = { + promises: { + flushAndDeleteDocWithLock: sinon.stub().resolves(), + }, + } + + this.HistoryManager = { + promises: { + flushProjectChanges: sinon.stub().resolves(), + }, + } + + this.Metrics = { + Timer: class Timer {}, + } + this.Metrics.Timer.prototype.done = sinon.stub() + + this.ProjectManager = SandboxedModule.require(MODULE_PATH, { requires: { - './RedisManager': (this.RedisManager = {}), - './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}), - './DocumentManager': (this.DocumentManager = {}), - './HistoryManager': (this.HistoryManager = { - flushProjectChanges: sinon.stub().callsArg(2), - }), - './LockManager': this.LockManager, - './Metrics': (this.Metrics = { - Timer: (Timer = (function () { - Timer = class Timer { - static initClass() { - this.prototype.done = sinon.stub() - } - } - Timer.initClass() - return Timer - })()), - }), + './RedisManager': this.RedisManager, + './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager, + './DocumentManager': this.DocumentManager, + './HistoryManager': this.HistoryManager, + './Metrics': this.Metrics, }, }) - this.project_id = 'project-id-123' - return (this.callback = sinon.stub()) }) describe('successfully', function () { - beforeEach(function (done) { + beforeEach(async function () { this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3'] - this.RedisManager.getDocIdsInProject = sinon - .stub() - .callsArgWith(1, null, this.doc_ids) - this.DocumentManager.flushAndDeleteDocWithLock = sinon.stub().callsArg(3) - return this.ProjectManager.flushAndDeleteProjectWithLocks( + this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids) + await this.ProjectManager.promises.flushAndDeleteProjectWithLocks( this.project_id, - {}, - error => { - this.callback(error) - return done() - } + {} ) }) it('should get the doc ids in the project', function () { - return this.RedisManager.getDocIdsInProject - .calledWith(this.project_id) - .should.equal(true) + this.RedisManager.promises.getDocIdsInProject.should.have.been.calledWith( + this.project_id + ) }) it('should delete each doc in the project', function () { - return Array.from(this.doc_ids).map(docId => - this.DocumentManager.flushAndDeleteDocWithLock - .calledWith(this.project_id, docId, {}) - .should.equal(true) - ) + for (const docId of this.doc_ids) { + this.DocumentManager.promises.flushAndDeleteDocWithLock.should.have.been.calledWith( + this.project_id, + docId, + {} + ) + } }) it('should flush project history', function () { - return this.HistoryManager.flushProjectChanges - .calledWith(this.project_id, {}) - .should.equal(true) + this.HistoryManager.promises.flushProjectChanges.should.have.been.calledWith( + this.project_id, + {} + ) }) - it('should call the callback without error', function () { - return this.callback.calledWith(null).should.equal(true) - }) - - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) - return describe('when a doc errors', function () { - beforeEach(function (done) { + describe('when a doc errors', function () { + beforeEach(async function () { this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3'] - this.RedisManager.getDocIdsInProject = sinon - .stub() - .callsArgWith(1, null, this.doc_ids) - this.DocumentManager.flushAndDeleteDocWithLock = sinon.spy( - (projectId, docId, options, callback) => { + this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids) + this.DocumentManager.promises.flushAndDeleteDocWithLock.callsFake( + async (projectId, docId, options) => { if (docId === 'doc-id-1') { - return callback( - (this.error = new Error('oops, something went wrong')) - ) - } else { - return callback() + throw new Error('oops, something went wrong') } } ) - return this.ProjectManager.flushAndDeleteProjectWithLocks( - this.project_id, - {}, - error => { - this.callback(error) - return done() - } - ) + await expect( + this.ProjectManager.promises.flushAndDeleteProjectWithLocks( + this.project_id, + {} + ) + ).to.be.rejected }) it('should still flush each doc in the project', function () { - return Array.from(this.doc_ids).map(docId => - this.DocumentManager.flushAndDeleteDocWithLock - .calledWith(this.project_id, docId, {}) - .should.equal(true) - ) + for (const docId of this.doc_ids) { + this.DocumentManager.promises.flushAndDeleteDocWithLock.should.have.been.calledWith( + this.project_id, + docId, + {} + ) + } }) it('should still flush project history', function () { - return this.HistoryManager.flushProjectChanges - .calledWith(this.project_id, {}) - .should.equal(true) + this.HistoryManager.promises.flushProjectChanges.should.have.been.calledWith( + this.project_id, + {} + ) }) - it('should record the error', function () { - return this.logger.error - .calledWith( - { err: this.error, projectId: this.project_id, docId: 'doc-id-1' }, - 'error deleting doc' - ) - .should.equal(true) - }) - - it('should call the callback with an error', function () { - return this.callback - .calledWith(sinon.match.instanceOf(Error)) - .should.equal(true) - }) - - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) }) diff --git a/services/document-updater/test/unit/js/ProjectManager/flushProjectTests.js b/services/document-updater/test/unit/js/ProjectManager/flushProjectTests.js index 7eea0d2df7..70a75d5fa9 100644 --- a/services/document-updater/test/unit/js/ProjectManager/flushProjectTests.js +++ b/services/document-updater/test/unit/js/ProjectManager/flushProjectTests.js @@ -1,145 +1,100 @@ -/* eslint-disable - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS206: Consider reworking classes to avoid initClass - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ +const { expect } = require('chai') const sinon = require('sinon') -const modulePath = '../../../../app/js/ProjectManager.js' const SandboxedModule = require('sandboxed-module') +const MODULE_PATH = '../../../../app/js/ProjectManager.js' + describe('ProjectManager - flushProject', function () { beforeEach(function () { - let Timer - this.LockManager = { - getLock: sinon.stub().yields(), - releaseLock: sinon.stub().yields(), + this.project_id = 'project-id-123' + + this.RedisManager = { + promises: { + getDocIdsInProject: sinon.stub(), + }, } - this.ProjectManager = SandboxedModule.require(modulePath, { + + this.ProjectHistoryRedisManager = {} + + this.Metrics = { + Timer: class Timer {}, + } + + this.DocumentManager = { + promises: { + flushDocIfLoadedWithLock: sinon.stub().resolves(), + }, + } + + this.HistoryManager = {} + + this.Metrics.Timer.prototype.done = sinon.stub() + + this.ProjectManager = SandboxedModule.require(MODULE_PATH, { requires: { - './RedisManager': (this.RedisManager = {}), - './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}), - './DocumentManager': (this.DocumentManager = {}), - './HistoryManager': (this.HistoryManager = {}), - './LockManager': this.LockManager, - './Metrics': (this.Metrics = { - Timer: (Timer = (function () { - Timer = class Timer { - static initClass() { - this.prototype.done = sinon.stub() - } - } - Timer.initClass() - return Timer - })()), - }), + './RedisManager': this.RedisManager, + './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager, + './DocumentManager': this.DocumentManager, + './HistoryManager': this.HistoryManager, + './Metrics': this.Metrics, }, }) - this.project_id = 'project-id-123' - return (this.callback = sinon.stub()) }) describe('successfully', function () { - beforeEach(function (done) { + beforeEach(async function () { this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3'] - this.RedisManager.getDocIdsInProject = sinon - .stub() - .callsArgWith(1, null, this.doc_ids) - this.DocumentManager.flushDocIfLoadedWithLock = sinon.stub().callsArg(2) - return this.ProjectManager.flushProjectWithLocks( - this.project_id, - error => { - this.callback(error) - return done() - } - ) + this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids) + await this.ProjectManager.promises.flushProjectWithLocks(this.project_id) }) it('should get the doc ids in the project', function () { - return this.RedisManager.getDocIdsInProject - .calledWith(this.project_id) - .should.equal(true) + this.RedisManager.promises.getDocIdsInProject.should.have.been.calledWith( + this.project_id + ) }) it('should flush each doc in the project', function () { - return Array.from(this.doc_ids).map(docId => - this.DocumentManager.flushDocIfLoadedWithLock - .calledWith(this.project_id, docId) - .should.equal(true) - ) + for (const docId of this.doc_ids) { + this.DocumentManager.promises.flushDocIfLoadedWithLock.should.have.been.calledWith( + this.project_id, + docId + ) + } }) - it('should call the callback without error', function () { - return this.callback.calledWith(null).should.equal(true) - }) - - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) - return describe('when a doc errors', function () { - beforeEach(function (done) { + describe('when a doc errors', function () { + beforeEach(async function () { this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3'] - this.RedisManager.getDocIdsInProject = sinon - .stub() - .callsArgWith(1, null, this.doc_ids) - this.DocumentManager.flushDocIfLoadedWithLock = sinon.spy( - (projectId, docId, callback) => { - if (callback == null) { - callback = function () {} - } + this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids) + this.DocumentManager.promises.flushDocIfLoadedWithLock.callsFake( + async (projectId, docId) => { if (docId === 'doc-id-1') { - return callback( - (this.error = new Error('oops, something went wrong')) - ) - } else { - return callback() + throw new Error('oops, something went wrong') } } ) - return this.ProjectManager.flushProjectWithLocks( - this.project_id, - error => { - this.callback(error) - return done() - } - ) + await expect( + this.ProjectManager.promises.flushProjectWithLocks(this.project_id) + ).to.be.rejected }) it('should still flush each doc in the project', function () { - return Array.from(this.doc_ids).map(docId => - this.DocumentManager.flushDocIfLoadedWithLock - .calledWith(this.project_id, docId) - .should.equal(true) - ) - }) - - it('should record the error', function () { - return this.logger.error - .calledWith( - { err: this.error, projectId: this.project_id, docId: 'doc-id-1' }, - 'error flushing doc' + for (const docId of this.doc_ids) { + this.DocumentManager.promises.flushDocIfLoadedWithLock.should.have.been.calledWith( + this.project_id, + docId ) - .should.equal(true) + } }) - it('should call the callback with an error', function () { - return this.callback - .calledWith(sinon.match.instanceOf(Error)) - .should.equal(true) - }) - - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) }) diff --git a/services/document-updater/test/unit/js/ProjectManager/getProjectDocsTests.js b/services/document-updater/test/unit/js/ProjectManager/getProjectDocsTests.js index 1a04d71777..db95ec3ce2 100644 --- a/services/document-updater/test/unit/js/ProjectManager/getProjectDocsTests.js +++ b/services/document-updater/test/unit/js/ProjectManager/getProjectDocsTests.js @@ -1,55 +1,47 @@ -/* eslint-disable - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS206: Consider reworking classes to avoid initClass - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ +const { expect } = require('chai') const sinon = require('sinon') -const modulePath = '../../../../app/js/ProjectManager.js' const SandboxedModule = require('sandboxed-module') const Errors = require('../../../../app/js/Errors.js') +const MODULE_PATH = '../../../../app/js/ProjectManager.js' + describe('ProjectManager - getProjectDocsAndFlushIfOld', function () { beforeEach(function () { - let Timer - this.LockManager = { - getLock: sinon.stub().yields(), - releaseLock: sinon.stub().yields(), + this.RedisManager = { + promises: { + checkOrSetProjectState: sinon.stub().resolves(), + getDocIdsInProject: sinon.stub(), + clearProjectState: sinon.stub().resolves(), + }, } - this.ProjectManager = SandboxedModule.require(modulePath, { + this.ProjectHistoryRedisManager = {} + this.DocumentManager = { + promises: { + getDocAndFlushIfOldWithLock: sinon.stub(), + }, + } + this.HistoryManager = {} + this.Metrics = { + Timer: class Timer {}, + } + this.Metrics.Timer.prototype.done = sinon.stub() + + this.ProjectManager = SandboxedModule.require(MODULE_PATH, { requires: { - './RedisManager': (this.RedisManager = {}), - './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}), - './DocumentManager': (this.DocumentManager = {}), - './HistoryManager': (this.HistoryManager = {}), - './LockManager': this.LockManager, - './Metrics': (this.Metrics = { - Timer: (Timer = (function () { - Timer = class Timer { - static initClass() { - this.prototype.done = sinon.stub() - } - } - Timer.initClass() - return Timer - })()), - }), + './RedisManager': this.RedisManager, + './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager, + './DocumentManager': this.DocumentManager, + './HistoryManager': this.HistoryManager, + './Metrics': this.Metrics, './Errors': Errors, }, }) this.project_id = 'project-id-123' - this.callback = sinon.stub() - return (this.doc_versions = [111, 222, 333]) + this.doc_versions = [111, 222, 333] }) describe('successfully', function () { - beforeEach(function (done) { + beforeEach(async function () { this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3'] this.doc_lines = [ ['aaa', 'aaa'], @@ -73,152 +65,104 @@ describe('ProjectManager - getProjectDocsAndFlushIfOld', function () { v: this.doc_versions[2], }, ] - this.RedisManager.checkOrSetProjectState = sinon - .stub() - .callsArgWith(2, null) - this.RedisManager.getDocIdsInProject = sinon - .stub() - .callsArgWith(1, null, this.doc_ids) - this.DocumentManager.getDocAndFlushIfOldWithLock = sinon.stub() - this.DocumentManager.getDocAndFlushIfOldWithLock + this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids) + this.DocumentManager.promises.getDocAndFlushIfOldWithLock .withArgs(this.project_id, this.doc_ids[0]) - .callsArgWith(2, null, this.doc_lines[0], this.doc_versions[0]) - this.DocumentManager.getDocAndFlushIfOldWithLock + .resolves({ lines: this.doc_lines[0], version: this.doc_versions[0] }) + this.DocumentManager.promises.getDocAndFlushIfOldWithLock .withArgs(this.project_id, this.doc_ids[1]) - .callsArgWith(2, null, this.doc_lines[1], this.doc_versions[1]) - this.DocumentManager.getDocAndFlushIfOldWithLock + .resolves({ lines: this.doc_lines[1], version: this.doc_versions[1] }) + this.DocumentManager.promises.getDocAndFlushIfOldWithLock .withArgs(this.project_id, this.doc_ids[2]) - .callsArgWith(2, null, this.doc_lines[2], this.doc_versions[2]) - return this.ProjectManager.getProjectDocsAndFlushIfOld( - this.project_id, - this.projectStateHash, - this.excludeVersions, - (error, docs) => { - this.callback(error, docs) - return done() - } - ) + .resolves({ lines: this.doc_lines[2], version: this.doc_versions[2] }) + this.result = + await this.ProjectManager.promises.getProjectDocsAndFlushIfOld( + this.project_id, + this.projectStateHash, + this.excludeVersions + ) }) it('should check the project state', function () { - return this.RedisManager.checkOrSetProjectState - .calledWith(this.project_id, this.projectStateHash) - .should.equal(true) + this.RedisManager.promises.checkOrSetProjectState.should.have.been.calledWith( + this.project_id, + this.projectStateHash + ) }) it('should get the doc ids in the project', function () { - return this.RedisManager.getDocIdsInProject - .calledWith(this.project_id) - .should.equal(true) + this.RedisManager.promises.getDocIdsInProject.should.have.been.calledWith( + this.project_id + ) }) - it('should call the callback without error', function () { - return this.callback.calledWith(null, this.docs).should.equal(true) + it('should return docs', function () { + expect(this.result).to.deep.equal(this.docs) }) - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) describe('when the state does not match', function () { - beforeEach(function (done) { + beforeEach(async function () { this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3'] - this.RedisManager.checkOrSetProjectState = sinon - .stub() - .callsArgWith(2, null, true) - return this.ProjectManager.getProjectDocsAndFlushIfOld( - this.project_id, - this.projectStateHash, - this.excludeVersions, - (error, docs) => { - this.callback(error, docs) - return done() - } - ) + this.RedisManager.promises.checkOrSetProjectState.resolves(true) + await expect( + this.ProjectManager.promises.getProjectDocsAndFlushIfOld( + this.project_id, + this.projectStateHash, + this.excludeVersions + ) + ).to.be.rejectedWith(Errors.ProjectStateChangedError) }) it('should check the project state', function () { - return this.RedisManager.checkOrSetProjectState - .calledWith(this.project_id, this.projectStateHash) - .should.equal(true) + this.RedisManager.promises.checkOrSetProjectState.should.have.been.calledWith( + this.project_id, + this.projectStateHash + ) }) - it('should call the callback with an error', function () { - return this.callback - .calledWith(sinon.match.instanceOf(Errors.ProjectStateChangedError)) - .should.equal(true) - }) - - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) describe('when a doc errors', function () { - beforeEach(function (done) { + it('should call the callback with an error', async function () { this.doc_ids = ['doc-id-1', 'doc-id-2', 'doc-id-3'] - this.RedisManager.checkOrSetProjectState = sinon - .stub() - .callsArgWith(2, null) - this.RedisManager.getDocIdsInProject = sinon - .stub() - .callsArgWith(1, null, this.doc_ids) - this.DocumentManager.getDocAndFlushIfOldWithLock = sinon.stub() - this.DocumentManager.getDocAndFlushIfOldWithLock + this.error = new Error('oops') + this.RedisManager.promises.getDocIdsInProject.resolves(this.doc_ids) + this.DocumentManager.promises.getDocAndFlushIfOldWithLock .withArgs(this.project_id, 'doc-id-1') - .callsArgWith(2, null, ['test doc content'], this.doc_versions[1]) - this.DocumentManager.getDocAndFlushIfOldWithLock + .resolves({ + lines: ['test doc content'], + version: this.doc_versions[1], + }) + this.DocumentManager.promises.getDocAndFlushIfOldWithLock .withArgs(this.project_id, 'doc-id-2') - .callsArgWith(2, (this.error = new Error('oops'))) // trigger an error - return this.ProjectManager.getProjectDocsAndFlushIfOld( - this.project_id, - this.projectStateHash, - this.excludeVersions, - (error, docs) => { - this.callback(error) - return done() - } - ) - }) - - it('should record the error', function () { - return this.logger.error - .calledWith( - { err: this.error, projectId: this.project_id, docId: 'doc-id-2' }, - 'error getting project doc lines in getProjectDocsAndFlushIfOld' + .rejects(this.error) + await expect( + this.ProjectManager.promises.getProjectDocsAndFlushIfOld( + this.project_id, + this.projectStateHash, + this.excludeVersions ) - .should.equal(true) - }) - - it('should call the callback with an error', function () { - return this.callback - .calledWith(sinon.match.instanceOf(Error)) - .should.equal(true) - }) - - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + ).to.be.rejected }) }) - return describe('clearing the project state with clearProjectState', function () { - beforeEach(function (done) { - this.RedisManager.clearProjectState = sinon.stub().callsArg(1) - return this.ProjectManager.clearProjectState(this.project_id, error => { - this.callback(error) - return done() - }) + describe('clearing the project state with clearProjectState', function () { + beforeEach(async function () { + await this.ProjectManager.promises.clearProjectState(this.project_id) }) it('should clear the project state', function () { - return this.RedisManager.clearProjectState - .calledWith(this.project_id) - .should.equal(true) - }) - - return it('should call the callback', function () { - return this.callback.called.should.equal(true) + this.RedisManager.promises.clearProjectState.should.have.been.calledWith( + this.project_id + ) }) }) }) diff --git a/services/document-updater/test/unit/js/ProjectManager/updateProjectTests.js b/services/document-updater/test/unit/js/ProjectManager/updateProjectTests.js index 6db8317e6d..1a459af8cf 100644 --- a/services/document-updater/test/unit/js/ProjectManager/updateProjectTests.js +++ b/services/document-updater/test/unit/js/ProjectManager/updateProjectTests.js @@ -1,3 +1,4 @@ +const { expect } = require('chai') const sinon = require('sinon') const modulePath = '../../../../app/js/ProjectManager.js' const SandboxedModule = require('sandboxed-module') @@ -7,20 +8,20 @@ describe('ProjectManager', function () { beforeEach(function () { this.RedisManager = {} this.ProjectHistoryRedisManager = { - queueRenameEntity: sinon.stub().yields(), - queueAddEntity: sinon.stub().yields(), + promises: { + queueRenameEntity: sinon.stub().resolves(), + queueAddEntity: sinon.stub().resolves(), + }, } this.DocumentManager = { - renameDocWithLock: sinon.stub().yields(), + promises: { + renameDocWithLock: sinon.stub().resolves(), + }, } this.HistoryManager = { flushProjectChangesAsync: sinon.stub(), shouldFlushHistoryOps: sinon.stub().returns(false), } - this.LockManager = { - getLock: sinon.stub().yields(), - releaseLock: sinon.stub().yields(), - } this.Metrics = { Timer: class Timer {}, } @@ -32,7 +33,6 @@ describe('ProjectManager', function () { './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager, './DocumentManager': this.DocumentManager, './HistoryManager': this.HistoryManager, - './LockManager': this.LockManager, './Metrics': this.Metrics, }, }) @@ -42,7 +42,6 @@ describe('ProjectManager', function () { this.user_id = 'user-id-123' this.version = 1234567 this.source = 'editor' - this.callback = sinon.stub() }) describe('updateProjectWithLocks', function () { @@ -74,15 +73,14 @@ describe('ProjectManager', function () { }) describe('successfully', function () { - beforeEach(function () { - this.ProjectManager.updateProjectWithLocks( + beforeEach(async function () { + await this.ProjectManager.promises.updateProjectWithLocks( this.project_id, this.projectHistoryId, this.user_id, this.updates, this.version, - this.source, - this.callback + this.source ) }) @@ -95,24 +93,20 @@ describe('ProjectManager', function () { this.secondDocUpdate, { version: `${this.version}.1` } ) - this.DocumentManager.renameDocWithLock - .calledWith( - this.project_id, - this.firstDocUpdate.id, - this.user_id, - firstDocUpdateWithVersion, - this.projectHistoryId - ) - .should.equal(true) - this.DocumentManager.renameDocWithLock - .calledWith( - this.project_id, - this.secondDocUpdate.id, - this.user_id, - secondDocUpdateWithVersion, - this.projectHistoryId - ) - .should.equal(true) + this.DocumentManager.promises.renameDocWithLock.should.have.been.calledWith( + this.project_id, + this.firstDocUpdate.id, + this.user_id, + firstDocUpdateWithVersion, + this.projectHistoryId + ) + this.DocumentManager.promises.renameDocWithLock.should.have.been.calledWith( + this.project_id, + this.secondDocUpdate.id, + this.user_id, + secondDocUpdateWithVersion, + this.projectHistoryId + ) }) it('should rename the files in the updates', function () { @@ -121,17 +115,15 @@ describe('ProjectManager', function () { this.firstFileUpdate, { version: `${this.version}.2` } ) - this.ProjectHistoryRedisManager.queueRenameEntity - .calledWith( - this.project_id, - this.projectHistoryId, - 'file', - this.firstFileUpdate.id, - this.user_id, - firstFileUpdateWithVersion, - this.source - ) - .should.equal(true) + this.ProjectHistoryRedisManager.promises.queueRenameEntity.should.have.been.calledWith( + this.project_id, + this.projectHistoryId, + 'file', + this.firstFileUpdate.id, + this.user_id, + firstFileUpdateWithVersion, + this.source + ) }) it('should not flush the history', function () { @@ -139,63 +131,54 @@ describe('ProjectManager', function () { .calledWith(this.project_id) .should.equal(false) }) - - it('should call the callback', function () { - this.callback.called.should.equal(true) - }) }) describe('when renaming a doc fails', function () { - beforeEach(function () { - this.error = new Error('error') - this.DocumentManager.renameDocWithLock.yields(this.error) - this.ProjectManager.updateProjectWithLocks( - this.project_id, - this.projectHistoryId, - this.user_id, - this.updates, - this.version, - this.source, - this.callback + it('throws an error', async function () { + this.DocumentManager.promises.renameDocWithLock.rejects( + new Error('error') ) - }) - - it('should call the callback with the error', function () { - this.callback.calledWith(this.error).should.equal(true) + await expect( + this.ProjectManager.promises.updateProjectWithLocks( + this.project_id, + this.projectHistoryId, + this.user_id, + this.updates, + this.version, + this.source + ) + ).to.be.rejected }) }) describe('when renaming a file fails', function () { - beforeEach(function () { - this.error = new Error('error') - this.ProjectHistoryRedisManager.queueRenameEntity.yields(this.error) - this.ProjectManager.updateProjectWithLocks( - this.project_id, - this.projectHistoryId, - this.user_id, - this.updates, - this.version, - this.source, - this.callback + it('throws an error', async function () { + this.ProjectHistoryRedisManager.promises.queueRenameEntity.rejects( + new Error('error') ) - }) - - it('should call the callback with the error', function () { - this.callback.calledWith(this.error).should.equal(true) + await expect( + this.ProjectManager.promises.updateProjectWithLocks( + this.project_id, + this.projectHistoryId, + this.user_id, + this.updates, + this.version, + this.source + ) + ).to.be.rejected }) }) describe('with enough ops to flush', function () { - beforeEach(function () { + beforeEach(async function () { this.HistoryManager.shouldFlushHistoryOps.returns(true) - this.ProjectManager.updateProjectWithLocks( + await this.ProjectManager.promises.updateProjectWithLocks( this.project_id, this.projectHistoryId, this.user_id, this.updates, this.version, - this.source, - this.callback + this.source ) }) @@ -238,15 +221,14 @@ describe('ProjectManager', function () { }) describe('successfully', function () { - beforeEach(function () { - this.ProjectManager.updateProjectWithLocks( + beforeEach(async function () { + await this.ProjectManager.promises.updateProjectWithLocks( this.project_id, this.projectHistoryId, this.user_id, this.updates, this.version, - this.source, - this.callback + this.source ) }) @@ -259,7 +241,7 @@ describe('ProjectManager', function () { this.secondDocUpdate, { version: `${this.version}.1` } ) - this.ProjectHistoryRedisManager.queueAddEntity + this.ProjectHistoryRedisManager.promises.queueAddEntity .getCall(0) .calledWith( this.project_id, @@ -271,7 +253,7 @@ describe('ProjectManager', function () { this.source ) .should.equal(true) - this.ProjectHistoryRedisManager.queueAddEntity + this.ProjectHistoryRedisManager.promises.queueAddEntity .getCall(1) .calledWith( this.project_id, @@ -296,7 +278,7 @@ describe('ProjectManager', function () { this.secondFileUpdate, { version: `${this.version}.3` } ) - this.ProjectHistoryRedisManager.queueAddEntity + this.ProjectHistoryRedisManager.promises.queueAddEntity .getCall(2) .calledWith( this.project_id, @@ -308,7 +290,7 @@ describe('ProjectManager', function () { this.source ) .should.equal(true) - this.ProjectHistoryRedisManager.queueAddEntity + this.ProjectHistoryRedisManager.promises.queueAddEntity .getCall(3) .calledWith( this.project_id, @@ -327,63 +309,55 @@ describe('ProjectManager', function () { .calledWith(this.project_id) .should.equal(false) }) - - it('should call the callback', function () { - this.callback.called.should.equal(true) - }) }) describe('when adding a doc fails', function () { - beforeEach(function () { + it('it should throw an error', async function () { this.error = new Error('error') - this.ProjectHistoryRedisManager.queueAddEntity.yields(this.error) - this.ProjectManager.updateProjectWithLocks( - this.project_id, - this.projectHistoryId, - this.user_id, - this.updates, - this.version, - this.source, - this.callback + this.ProjectHistoryRedisManager.promises.queueAddEntity.rejects( + this.error ) - }) - - it('should call the callback with the error', function () { - this.callback.calledWith(this.error).should.equal(true) + await expect( + this.ProjectManager.promises.updateProjectWithLocks( + this.project_id, + this.projectHistoryId, + this.user_id, + this.updates, + this.version, + this.source + ) + ).to.be.rejected }) }) describe('when adding a file fails', function () { - beforeEach(function () { - this.error = new Error('error') - this.ProjectHistoryRedisManager.queueAddEntity.yields(this.error) - this.ProjectManager.updateProjectWithLocks( - this.project_id, - this.projectHistoryId, - this.user_id, - this.updates, - this.version, - this.source, - this.callback + beforeEach(async function () { + this.ProjectHistoryRedisManager.promises.queueAddEntity.rejects( + new Error('error') ) - }) - - it('should call the callback with the error', function () { - this.callback.calledWith(this.error).should.equal(true) + await expect( + this.ProjectManager.promises.updateProjectWithLocks( + this.project_id, + this.projectHistoryId, + this.user_id, + this.updates, + this.version, + this.source + ) + ).to.be.rejected }) }) describe('with enough ops to flush', function () { - beforeEach(function () { + beforeEach(async function () { this.HistoryManager.shouldFlushHistoryOps.returns(true) - this.ProjectManager.updateProjectWithLocks( + await this.ProjectManager.promises.updateProjectWithLocks( this.project_id, this.projectHistoryId, this.user_id, this.updates, this.version, - this.source, - this.callback + this.source ) }) @@ -396,21 +370,18 @@ describe('ProjectManager', function () { }) describe('when given an unknown operation type', function () { - beforeEach(function () { + it('throws an error', async function () { this.updates = [{ type: 'brew-coffee' }] - this.ProjectManager.updateProjectWithLocks( - this.project_id, - this.projectHistoryId, - this.user_id, - this.updates, - this.version, - this.source, - this.callback - ) - }) - - it('should call back with an error', function () { - this.callback.calledWith(sinon.match.instanceOf(Error)).should.be.true + await expect( + this.ProjectManager.promises.updateProjectWithLocks( + this.project_id, + this.projectHistoryId, + this.user_id, + this.updates, + this.version, + this.source + ) + ).to.be.rejected }) }) })