From bdca2342ab1ff8f7690771ada0469a2bebbad11d Mon Sep 17 00:00:00 2001 From: David <33458145+davidmcpowell@users.noreply.github.com> Date: Tue, 23 Jan 2024 10:34:52 +0000 Subject: [PATCH] Merge pull request #16629 from overleaf/dp-mongoose-callback--project-options Convert ProjectOptionsHandler and ProjectOptionsHandlerTests to use async/await GitOrigin-RevId: 3f2902ee7c2f093b9350748824a58f00a88d02c7 --- .../Features/Project/ProjectOptionsHandler.js | 51 +++-- .../src/Project/ProjectOptionsHandlerTests.js | 179 ++++++++---------- 2 files changed, 106 insertions(+), 124 deletions(-) diff --git a/services/web/app/src/Features/Project/ProjectOptionsHandler.js b/services/web/app/src/Features/Project/ProjectOptionsHandler.js index 1ec1fa01b6..58b1828ddd 100644 --- a/services/web/app/src/Features/Project/ProjectOptionsHandler.js +++ b/services/web/app/src/Features/Project/ProjectOptionsHandler.js @@ -1,69 +1,78 @@ const { Project } = require('../../models/Project') const settings = require('@overleaf/settings') -const { promisifyAll } = require('@overleaf/promise-utils') - +const { callbackify } = require('util') const safeCompilers = ['xelatex', 'pdflatex', 'latex', 'lualatex'] const ProjectOptionsHandler = { - setCompiler(projectId, compiler, callback) { + async setCompiler(projectId, compiler) { if (!compiler) { - return callback() + return } compiler = compiler.toLowerCase() if (!safeCompilers.includes(compiler)) { - return callback(new Error(`invalid compiler: ${compiler}`)) + throw new Error(`invalid compiler: ${compiler}`) } const conditions = { _id: projectId } const update = { compiler } - Project.updateOne(conditions, update, {}, callback) + return Project.updateOne(conditions, update, {}) }, - setImageName(projectId, imageName, callback) { + async setImageName(projectId, imageName) { if (!imageName || !Array.isArray(settings.allowedImageNames)) { - return callback() + return } imageName = imageName.toLowerCase() const isAllowed = settings.allowedImageNames.find( allowed => imageName === allowed.imageName ) if (!isAllowed) { - return callback(new Error(`invalid imageName: ${imageName}`)) + throw new Error(`invalid imageName: ${imageName}`) } const conditions = { _id: projectId } const update = { imageName: settings.imageRoot + '/' + imageName } - Project.updateOne(conditions, update, {}, callback) + return Project.updateOne(conditions, update, {}) }, - setSpellCheckLanguage(projectId, languageCode, callback) { + async setSpellCheckLanguage(projectId, languageCode) { if (!Array.isArray(settings.languages)) { - return callback() + return } const language = settings.languages.find( language => language.code === languageCode ) if (languageCode && !language) { - return callback(new Error(`invalid languageCode: ${languageCode}`)) + throw new Error(`invalid languageCode: ${languageCode}`) } const conditions = { _id: projectId } const update = { spellCheckLanguage: languageCode } - Project.updateOne(conditions, update, {}, callback) + return Project.updateOne(conditions, update, {}) }, - setBrandVariationId(projectId, brandVariationId, callback) { + async setBrandVariationId(projectId, brandVariationId) { if (!brandVariationId) { - return callback() + return } const conditions = { _id: projectId } const update = { brandVariationId } - Project.updateOne(conditions, update, {}, callback) + return Project.updateOne(conditions, update, {}) }, - unsetBrandVariationId(projectId, callback) { + async unsetBrandVariationId(projectId) { const conditions = { _id: projectId } const update = { $unset: { brandVariationId: 1 } } - Project.updateOne(conditions, update, {}, callback) + return Project.updateOne(conditions, update, {}) }, } -ProjectOptionsHandler.promises = promisifyAll(ProjectOptionsHandler) -module.exports = ProjectOptionsHandler +module.exports = { + setCompiler: callbackify(ProjectOptionsHandler.setCompiler), + setImageName: callbackify(ProjectOptionsHandler.setImageName), + setSpellCheckLanguage: callbackify( + ProjectOptionsHandler.setSpellCheckLanguage + ), + setBrandVariationId: callbackify(ProjectOptionsHandler.setBrandVariationId), + unsetBrandVariationId: callbackify( + ProjectOptionsHandler.unsetBrandVariationId + ), + promises: ProjectOptionsHandler, +} diff --git a/services/web/test/unit/src/Project/ProjectOptionsHandlerTests.js b/services/web/test/unit/src/Project/ProjectOptionsHandlerTests.js index c3f2fcfae6..40326ea4a4 100644 --- a/services/web/test/unit/src/Project/ProjectOptionsHandlerTests.js +++ b/services/web/test/unit/src/Project/ProjectOptionsHandlerTests.js @@ -26,7 +26,7 @@ describe('ProjectOptionsHandler', function () { this.projectModel = Project = class Project { constructor(options) {} } - this.projectModel.updateOne = sinon.stub().yields() + this.projectModel.updateOne = sinon.stub().resolves() this.handler = SandboxedModule.require(modulePath, { requires: { @@ -47,29 +47,26 @@ describe('ProjectOptionsHandler', function () { }) describe('Setting the compiler', function () { - it('should perform and update on mongo', function (done) { - this.handler.setCompiler(projectId, 'xeLaTeX', err => { - const args = this.projectModel.updateOne.args[0] - args[0]._id.should.equal(projectId) - args[1].compiler.should.equal('xelatex') - done() - }) + it('should perform and update on mongo', async function () { + await this.handler.promises.setCompiler(projectId, 'xeLaTeX') + const args = this.projectModel.updateOne.args[0] + args[0]._id.should.equal(projectId) + args[1].compiler.should.equal('xelatex') }) - it('should not perform and update on mongo if it is not a recognised compiler', function (done) { - this.handler.setCompiler(projectId, 'something', err => { - this.projectModel.updateOne.called.should.equal(false) - done() - }) + it('should not perform and update on mongo if it is not a recognised compiler', async function () { + const fakeComplier = 'something' + expect( + this.handler.promises.setCompiler(projectId, 'something') + ).to.be.rejectedWith(`invalid compiler: ${fakeComplier}`) + + this.projectModel.updateOne.called.should.equal(false) }) describe('when called without arg', function () { - it('should callback with null', function (done) { - this.handler.setCompiler(projectId, null, err => { - expect(err).to.be.undefined - this.projectModel.updateOne.callCount.should.equal(0) - done() - }) + it('should callback with null', async function () { + await this.handler.promises.setCompiler(projectId, null) + this.projectModel.updateOne.callCount.should.equal(0) }) }) @@ -78,39 +75,34 @@ describe('ProjectOptionsHandler', function () { this.projectModel.updateOne = sinon.stub().yields('error') }) - it('should callback with error', function (done) { - this.handler.setCompiler(projectId, 'xeLaTeX', err => { - err.should.equal('error') - done() - }) + it('should be rejected', async function () { + expect(this.handler.promises.setCompiler(projectId, 'xeLaTeX')).to.be + .rejected }) }) }) describe('Setting the imageName', function () { - it('should perform and update on mongo', function (done) { - this.handler.setImageName(projectId, 'texlive-1234.5', err => { - const args = this.projectModel.updateOne.args[0] - args[0]._id.should.equal(projectId) - args[1].imageName.should.equal('docker-repo/subdir/texlive-1234.5') - done() - }) + it('should perform and update on mongo', async function () { + await this.handler.promises.setImageName(projectId, 'texlive-1234.5') + const args = this.projectModel.updateOne.args[0] + args[0]._id.should.equal(projectId) + args[1].imageName.should.equal('docker-repo/subdir/texlive-1234.5') }) - it('should not perform and update on mongo if it is not a reconised compiler', function (done) { - this.handler.setImageName(projectId, 'something', err => { - this.projectModel.updateOne.called.should.equal(false) - done() - }) + it('should not perform and update on mongo if it is not a reconised image name', async function () { + const fakeImageName = 'something' + expect( + this.handler.promises.setImageName(projectId, fakeImageName) + ).to.be.rejectedWith(`invalid imageName: ${fakeImageName}`) + + this.projectModel.updateOne.called.should.equal(false) }) describe('when called without arg', function () { - it('should callback with null', function (done) { - this.handler.setImageName(projectId, null, err => { - expect(err).to.be.undefined - this.projectModel.updateOne.callCount.should.equal(0) - done() - }) + it('should callback with null', async function () { + await this.handler.promises.setImageName(projectId, null) + this.projectModel.updateOne.callCount.should.equal(0) }) }) @@ -119,37 +111,32 @@ describe('ProjectOptionsHandler', function () { this.projectModel.updateOne = sinon.stub().yields('error') }) - it('should callback with error', function (done) { - this.handler.setImageName(projectId, 'texlive-1234.5', err => { - err.should.equal('error') - done() - }) + it('should be rejected', async function () { + expect(this.handler.promises.setImageName(projectId, 'texlive-1234.5')) + .to.be.rejected }) }) }) describe('setting the spellCheckLanguage', function () { - it('should perform and update on mongo', function (done) { - this.handler.setSpellCheckLanguage(projectId, 'fr', err => { - const args = this.projectModel.updateOne.args[0] - args[0]._id.should.equal(projectId) - args[1].spellCheckLanguage.should.equal('fr') - done() - }) + it('should perform and update on mongo', async function () { + await this.handler.promises.setSpellCheckLanguage(projectId, 'fr') + const args = this.projectModel.updateOne.args[0] + args[0]._id.should.equal(projectId) + args[1].spellCheckLanguage.should.equal('fr') }) - it('should not perform and update on mongo if it is not a reconised compiler', function (done) { - this.handler.setSpellCheckLanguage(projectId, 'no a lang', err => { - this.projectModel.updateOne.called.should.equal(false) - done() - }) + it('should not perform and update on mongo if it is not a reconised langauge', async function () { + const fakeLanguageCode = 'not a lang' + expect( + this.handler.promises.setSpellCheckLanguage(projectId, fakeLanguageCode) + ).to.be.rejectedWith(`invalid languageCode: ${fakeLanguageCode}`) + this.projectModel.updateOne.called.should.equal(false) }) - it('should perform and update on mongo if the language is blank (means turn it off)', function (done) { - this.handler.setSpellCheckLanguage(projectId, '', err => { - this.projectModel.updateOne.called.should.equal(true) - done() - }) + it('should perform and update on mongo if the language is blank (means turn it off)', async function () { + await this.handler.promises.setSpellCheckLanguage(projectId, '') + this.projectModel.updateOne.called.should.equal(true) }) describe('when mongo update error occurs', function () { @@ -157,37 +144,29 @@ describe('ProjectOptionsHandler', function () { this.projectModel.updateOne = sinon.stub().yields('error') }) - it('should callback with error', function (done) { - this.handler.setSpellCheckLanguage(projectId, 'fr', err => { - err.should.equal('error') - done() - }) + it('should be rejected', async function () { + expect(this.handler.promises.setSpellCheckLanguage(projectId)).to.be + .rejected }) }) }) describe('setting the brandVariationId', function () { - it('should perform and update on mongo', function (done) { - this.handler.setBrandVariationId(projectId, '123', err => { - const args = this.projectModel.updateOne.args[0] - args[0]._id.should.equal(projectId) - args[1].brandVariationId.should.equal('123') - done() - }) + it('should perform and update on mongo', async function () { + await this.handler.promises.setBrandVariationId(projectId, '123') + const args = this.projectModel.updateOne.args[0] + args[0]._id.should.equal(projectId) + args[1].brandVariationId.should.equal('123') }) - it('should not perform and update on mongo if there is no brand variation', function (done) { - this.handler.setBrandVariationId(projectId, null, err => { - this.projectModel.updateOne.called.should.equal(false) - done() - }) + it('should not perform and update on mongo if there is no brand variation', async function () { + await this.handler.promises.setBrandVariationId(projectId, null) + this.projectModel.updateOne.called.should.equal(false) }) - it('should not perform and update on mongo if brand variation is an empty string', function (done) { - this.handler.setBrandVariationId(projectId, '', err => { - this.projectModel.updateOne.called.should.equal(false) - done() - }) + it('should not perform and update on mongo if brand variation is an empty string', async function () { + await this.handler.promises.setBrandVariationId(projectId, '') + this.projectModel.updateOne.called.should.equal(false) }) describe('when mongo update error occurs', function () { @@ -195,23 +174,19 @@ describe('ProjectOptionsHandler', function () { this.projectModel.updateOne = sinon.stub().yields('error') }) - it('should callback with error', function (done) { - this.handler.setBrandVariationId(projectId, '123', err => { - err.should.equal('error') - done() - }) + it('should be rejected', async function () { + expect(this.handler.promises.setBrandVariationId(projectId, '123')).to + .be.rejected }) }) }) describe('unsetting the brandVariationId', function () { - it('should perform and update on mongo', function (done) { - this.handler.unsetBrandVariationId(projectId, err => { - const args = this.projectModel.updateOne.args[0] - args[0]._id.should.equal(projectId) - expect(args[1]).to.deep.equal({ $unset: { brandVariationId: 1 } }) - done() - }) + it('should perform and update on mongo', async function () { + await this.handler.promises.unsetBrandVariationId(projectId) + const args = this.projectModel.updateOne.args[0] + args[0]._id.should.equal(projectId) + expect(args[1]).to.deep.equal({ $unset: { brandVariationId: 1 } }) }) describe('when mongo update error occurs', function () { @@ -219,11 +194,9 @@ describe('ProjectOptionsHandler', function () { this.projectModel.updateOne = sinon.stub().yields('error') }) - it('should callback with error', function (done) { - this.handler.unsetBrandVariationId(projectId, err => { - err.should.equal('error') - done() - }) + it('should be rejected', async function () { + expect(this.handler.promises.unsetBrandVariationId(projectId)).to.be + .rejected }) }) })