From c8ab3ec1ee7fff730008df5252890ee1da270d71 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 2 Jun 2022 10:21:08 -0400 Subject: [PATCH] Merge pull request #8253 from overleaf/em-decaf-cleanup-compile-controller Decaf cleanup CompileController and CompileManager GitOrigin-RevId: 97384c9f76a2487c04f3d8d6a384cefdfcd083d4 --- .../src/Features/Compile/CompileController.js | 373 +++++++----------- .../src/Features/Compile/CompileManager.js | 195 ++++----- .../src/Compile/CompileControllerTests.js | 251 ++++++------ .../unit/src/Compile/CompileManagerTests.js | 141 ++++--- 4 files changed, 402 insertions(+), 558 deletions(-) diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index 39263d4496..7f237d0b1c 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/services/web/app/src/Features/Compile/CompileController.js @@ -1,17 +1,3 @@ -/* eslint-disable - camelcase, - n/handle-callback-err, - max-len, - 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 - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ let CompileController const OError = require('@overleaf/o-error') const Metrics = require('@overleaf/metrics') @@ -24,7 +10,7 @@ const Settings = require('@overleaf/settings') const SessionManager = require('../Authentication/SessionManager') const RateLimiter = require('../../infrastructure/RateLimiter') const ClsiCookieManager = require('./ClsiCookieManager')( - Settings.apis.clsi != null ? Settings.apis.clsi.backendGroupName : undefined + Settings.apis.clsi?.backendGroupName ) const Path = require('path') const SplitTestHandler = require('../SplitTests/SplitTestHandler') @@ -42,11 +28,11 @@ function getImageNameForProject(projectId, callback) { module.exports = CompileController = { compile(req, res, next) { res.setTimeout(COMPILE_TIMEOUT_MS) - const project_id = req.params.Project_id + const projectId = req.params.Project_id const isAutoCompile = !!req.query.auto_compile const enablePdfCaching = !!req.query.enable_pdf_caching const fileLineErrors = !!req.query.file_line_errors - const user_id = SessionManager.getLoggedInUserId(req.session) + const userId = SessionManager.getLoggedInUserId(req.session) const options = { isAutoCompile, enablePdfCaching, @@ -76,8 +62,8 @@ module.exports = CompileController = { } CompileManager.compile( - project_id, - user_id, + projectId, + userId, options, ( error, @@ -118,7 +104,7 @@ module.exports = CompileController = { res.json({ status, outputFiles, - compileGroup: limits != null ? limits.compileGroup : undefined, + compileGroup: limits?.compileGroup, clsiServerId, validationProblems, stats, @@ -132,58 +118,46 @@ module.exports = CompileController = { }, stopCompile(req, res, next) { - if (next == null) { - next = function () {} - } - const project_id = req.params.Project_id - const user_id = SessionManager.getLoggedInUserId(req.session) - return CompileManager.stopCompile(project_id, user_id, function (error) { - if (error != null) { + const projectId = req.params.Project_id + const userId = SessionManager.getLoggedInUserId(req.session) + CompileManager.stopCompile(projectId, userId, function (error) { + if (error) { return next(error) } - return res.sendStatus(200) + res.sendStatus(200) }) }, // Used for submissions through the public API compileSubmission(req, res, next) { - if (next == null) { - next = function () {} - } res.setTimeout(COMPILE_TIMEOUT_MS) - const { submission_id } = req.params + const submissionId = req.params.submission_id const options = {} - if ((req.body != null ? req.body.rootResourcePath : undefined) != null) { + if (req.body?.rootResourcePath != null) { options.rootResourcePath = req.body.rootResourcePath } - if (req.body != null ? req.body.compiler : undefined) { + if (req.body?.compiler) { options.compiler = req.body.compiler } - if (req.body != null ? req.body.draft : undefined) { + if (req.body?.draft) { options.draft = req.body.draft } - if ( - ['validate', 'error', 'silent'].includes( - req.body != null ? req.body.check : undefined - ) - ) { + if (['validate', 'error', 'silent'].includes(req.body?.check)) { options.check = req.body.check } options.compileGroup = - (req.body != null ? req.body.compileGroup : undefined) || - Settings.defaultFeatures.compileGroup + req.body?.compileGroup || Settings.defaultFeatures.compileGroup options.timeout = - (req.body != null ? req.body.timeout : undefined) || - Settings.defaultFeatures.compileTimeout - return ClsiManager.sendExternalRequest( - submission_id, + req.body?.timeout || Settings.defaultFeatures.compileTimeout + ClsiManager.sendExternalRequest( + submissionId, req.body, options, function (error, status, outputFiles, clsiServerId, validationProblems) { - if (error != null) { + if (error) { return next(error) } - return res.json({ + res.json({ status, outputFiles, clsiServerId, @@ -194,36 +168,32 @@ module.exports = CompileController = { }, _compileAsUser(req, callback) { - // callback with user_id if per-user, undefined otherwise + // callback with userId if per-user, undefined otherwise if (!Settings.disablePerUserCompiles) { - const user_id = SessionManager.getLoggedInUserId(req.session) - return callback(null, user_id) + const userId = SessionManager.getLoggedInUserId(req.session) + callback(null, userId) } else { - return callback() + callback() } }, // do a per-project compile, not per-user _downloadAsUser(req, callback) { - // callback with user_id if per-user, undefined otherwise + // callback with userId if per-user, undefined otherwise if (!Settings.disablePerUserCompiles) { - const user_id = SessionManager.getLoggedInUserId(req.session) - return callback(null, user_id) + const userId = SessionManager.getLoggedInUserId(req.session) + callback(null, userId) } else { - return callback() + callback() } }, // do a per-project compile, not per-user downloadPdf(req, res, next) { - if (next == null) { - next = function () {} - } Metrics.inc('pdf-downloads') - const project_id = req.params.Project_id - const isPdfjsPartialDownload = - req.query != null ? req.query.pdfng : undefined + const projectId = req.params.Project_id + const isPdfjsPartialDownload = req.query?.pdfng const rateLimit = function (callback) { if (isPdfjsPartialDownload) { - return callback(null, true) + callback(null, true) } else { const rateLimitOpts = { endpointName: 'full-pdf-download', @@ -231,56 +201,49 @@ module.exports = CompileController = { subjectName: req.ip, timeInterval: 60 * 60, } - return RateLimiter.addCount(rateLimitOpts, callback) + RateLimiter.addCount(rateLimitOpts, callback) } } - return ProjectGetter.getProject( - project_id, - { name: 1 }, - function (err, project) { - res.contentType('application/pdf') - const filename = `${CompileController._getSafeProjectName(project)}.pdf` - - if (req.query.popupDownload) { - res.setContentDisposition('attachment', { filename }) - } else { - res.setContentDisposition('', { filename }) - } - - return rateLimit(function (err, canContinue) { - if (err != null) { - logger.err({ err }, 'error checking rate limit for pdf download') - return res.sendStatus(500) - } else if (!canContinue) { - logger.debug( - { project_id, ip: req.ip }, - 'rate limit hit downloading pdf' - ) - return res.sendStatus(500) - } else { - return CompileController._downloadAsUser( - req, - function (error, user_id) { - const url = CompileController._getFileUrl( - project_id, - user_id, - req.params.build_id, - 'output.pdf' - ) - return CompileController.proxyToClsi( - project_id, - url, - req, - res, - next - ) - } - ) - } - }) + ProjectGetter.getProject(projectId, { name: 1 }, function (err, project) { + if (err) { + return next(err) } - ) + res.contentType('application/pdf') + const filename = `${CompileController._getSafeProjectName(project)}.pdf` + + if (req.query.popupDownload) { + res.setContentDisposition('attachment', { filename }) + } else { + res.setContentDisposition('', { filename }) + } + + rateLimit(function (err, canContinue) { + if (err) { + logger.err({ err }, 'error checking rate limit for pdf download') + res.sendStatus(500) + } else if (!canContinue) { + logger.debug( + { projectId, ip: req.ip }, + 'rate limit hit downloading pdf' + ) + res.sendStatus(500) + } else { + CompileController._downloadAsUser(req, function (error, userId) { + if (error) { + return next(error) + } + const url = CompileController._getFileUrl( + projectId, + userId, + req.params.build_id, + 'output.pdf' + ) + CompileController.proxyToClsi(projectId, url, req, res, next) + }) + } + }) + }) }, _getSafeProjectName(project) { @@ -290,81 +253,75 @@ module.exports = CompileController = { }, deleteAuxFiles(req, res, next) { - const project_id = req.params.Project_id + const projectId = req.params.Project_id const { clsiserverid } = req.query - return CompileController._compileAsUser(req, function (error, user_id) { - if (error != null) { + CompileController._compileAsUser(req, function (error, userId) { + if (error) { return next(error) } CompileManager.deleteAuxFiles( - project_id, - user_id, + projectId, + userId, clsiserverid, function (error) { - if (error != null) { + if (error) { return next(error) } - return res.sendStatus(200) + res.sendStatus(200) } ) }) }, - // this is only used by templates, so is not called with a user_id + // this is only used by templates, so is not called with a userId compileAndDownloadPdf(req, res, next) { - const { project_id } = req.params - // pass user_id as null, since templates are an "anonymous" compile - return CompileManager.compile(project_id, null, {}, function (err) { - if (err != null) { + const projectId = req.params.project_id + // pass userId as null, since templates are an "anonymous" compile + CompileManager.compile(projectId, null, {}, function (err) { + if (err) { logger.err( - { err, project_id }, + { err, projectId }, 'something went wrong compile and downloading pdf' ) res.sendStatus(500) } - const url = `/project/${project_id}/output/output.pdf` - return CompileController.proxyToClsi(project_id, url, req, res, next) + const url = `/project/${projectId}/output/output.pdf` + CompileController.proxyToClsi(projectId, url, req, res, next) }) }, getFileFromClsi(req, res, next) { - if (next == null) { - next = function () {} - } - const project_id = req.params.Project_id - return CompileController._downloadAsUser(req, function (error, user_id) { - if (error != null) { + const projectId = req.params.Project_id + CompileController._downloadAsUser(req, function (error, userId) { + if (error) { return next(error) } const url = CompileController._getFileUrl( - project_id, - user_id, + projectId, + userId, req.params.build_id, req.params.file ) - return CompileController.proxyToClsi(project_id, url, req, res, next) + CompileController.proxyToClsi(projectId, url, req, res, next) }) }, getFileFromClsiWithoutUser(req, res, next) { - if (next == null) { - next = function () {} - } - const { submission_id } = req.params + const submissionId = req.params.submission_id const url = CompileController._getFileUrl( - submission_id, + submissionId, null, req.params.build_id, req.params.file ) const limits = { compileGroup: - (req.body != null ? req.body.compileGroup : undefined) || + req.body?.compileGroup || req.query?.compileGroup || Settings.defaultFeatures.compileGroup, } - return CompileController.proxyToClsiWithLimits( - submission_id, + CompileController.proxyToClsiWithLimits( + submissionId, url, limits, req, @@ -374,70 +331,58 @@ module.exports = CompileController = { }, // compute a GET file url for a given project, user (optional), build (optional) and file - _getFileUrl(project_id, user_id, build_id, file) { + _getFileUrl(projectId, userId, buildId, file) { let url - if (user_id != null && build_id != null) { - url = `/project/${project_id}/user/${user_id}/build/${build_id}/output/${file}` - } else if (user_id != null) { - url = `/project/${project_id}/user/${user_id}/output/${file}` - } else if (build_id != null) { - url = `/project/${project_id}/build/${build_id}/output/${file}` + if (userId != null && buildId != null) { + url = `/project/${projectId}/user/${userId}/build/${buildId}/output/${file}` + } else if (userId != null) { + url = `/project/${projectId}/user/${userId}/output/${file}` + } else if (buildId != null) { + url = `/project/${projectId}/build/${buildId}/output/${file}` } else { - url = `/project/${project_id}/output/${file}` + url = `/project/${projectId}/output/${file}` } return url }, // compute a POST url for a project, user (optional) and action - _getUrl(project_id, user_id, action) { - let path = `/project/${project_id}` - if (user_id != null) { - path += `/user/${user_id}` + _getUrl(projectId, userId, action) { + let path = `/project/${projectId}` + if (userId != null) { + path += `/user/${userId}` } return `${path}/${action}` }, proxySyncPdf(req, res, next) { - if (next == null) { - next = function () {} - } - const project_id = req.params.Project_id + const projectId = req.params.Project_id const { page, h, v } = req.query - if (!(page != null ? page.match(/^\d+$/) : undefined)) { + if (!page?.match(/^\d+$/)) { return next(new Error('invalid page parameter')) } - if (!(h != null ? h.match(/^-?\d+\.\d+$/) : undefined)) { + if (!h?.match(/^-?\d+\.\d+$/)) { return next(new Error('invalid h parameter')) } - if (!(v != null ? v.match(/^-?\d+\.\d+$/) : undefined)) { + if (!v?.match(/^-?\d+\.\d+$/)) { return next(new Error('invalid v parameter')) } // whether this request is going to a per-user container - return CompileController._compileAsUser(req, function (error, user_id) { - if (error != null) { + CompileController._compileAsUser(req, function (error, userId) { + if (error) { return next(error) } - getImageNameForProject(project_id, (error, imageName) => { + getImageNameForProject(projectId, (error, imageName) => { if (error) return next(error) - const url = CompileController._getUrl(project_id, user_id, 'sync/pdf') + const url = CompileController._getUrl(projectId, userId, 'sync/pdf') const destination = { url, qs: { page, h, v, imageName } } - return CompileController.proxyToClsi( - project_id, - destination, - req, - res, - next - ) + CompileController.proxyToClsi(projectId, destination, req, res, next) }) }) }, proxySyncCode(req, res, next) { - if (next == null) { - next = function () {} - } - const project_id = req.params.Project_id + const projectId = req.params.Project_id const { file, line, column } = req.query if (file == null) { return next(new Error('missing file parameter')) @@ -451,39 +396,30 @@ module.exports = CompileController = { if (Path.resolve('/', testPath) !== `/${testPath}`) { return next(new Error('invalid file parameter')) } - if (!(line != null ? line.match(/^\d+$/) : undefined)) { + if (!line?.match(/^\d+$/)) { return next(new Error('invalid line parameter')) } - if (!(column != null ? column.match(/^\d+$/) : undefined)) { + if (!column?.match(/^\d+$/)) { return next(new Error('invalid column parameter')) } - return CompileController._compileAsUser(req, function (error, user_id) { - if (error != null) { + CompileController._compileAsUser(req, function (error, userId) { + if (error) { return next(error) } - getImageNameForProject(project_id, (error, imageName) => { + getImageNameForProject(projectId, (error, imageName) => { if (error) return next(error) - const url = CompileController._getUrl(project_id, user_id, 'sync/code') + const url = CompileController._getUrl(projectId, userId, 'sync/code') const destination = { url, qs: { file, line, column, imageName } } - return CompileController.proxyToClsi( - project_id, - destination, - req, - res, - next - ) + CompileController.proxyToClsi(projectId, destination, req, res, next) }) }) }, - proxyToClsi(project_id, url, req, res, next) { - if (next == null) { - next = function () {} - } - if (req.query != null ? req.query.compileGroup : undefined) { - return CompileController.proxyToClsiWithLimits( - project_id, + proxyToClsi(projectId, url, req, res, next) { + if (req.query?.compileGroup) { + CompileController.proxyToClsiWithLimits( + projectId, url, { compileGroup: req.query.compileGroup }, req, @@ -491,14 +427,14 @@ module.exports = CompileController = { next ) } else { - return CompileManager.getProjectCompileLimits( - project_id, + CompileManager.getProjectCompileLimits( + projectId, function (error, limits) { - if (error != null) { + if (error) { return next(error) } - return CompileController.proxyToClsiWithLimits( - project_id, + CompileController.proxyToClsiWithLimits( + projectId, url, limits, req, @@ -510,17 +446,14 @@ module.exports = CompileController = { } }, - proxyToClsiWithLimits(project_id, url, limits, req, res, next) { - if (next == null) { - next = function () {} - } + proxyToClsiWithLimits(projectId, url, limits, req, res, next) { _getPersistenceOptions( req, - project_id, + projectId, limits.compileGroup, (err, persistenceOptions) => { let qs - if (err != null) { + if (err) { OError.tag(err, 'error getting cookie jar for clsi request') return next(err) } @@ -543,10 +476,7 @@ module.exports = CompileController = { options.qs = Object.assign(options.qs || {}, qs) } // if we have a build parameter, pass it through to the clsi - if ( - (req.query != null ? req.query.pdfng : undefined) && - (req.query != null ? req.query.build : undefined) != null - ) { + if (req.query?.pdfng && req.query?.build != null) { // only for new pdf viewer if (options.qs == null) { options.qs = {} @@ -555,10 +485,9 @@ module.exports = CompileController = { } // if we are byte serving pdfs, pass through If-* and Range headers // do not send any others, there's a proxying loop if Host: is passed! - if (req.query != null ? req.query.pdfng : undefined) { + if (req.query?.pdfng) { const newHeaders = {} for (const h in req.headers) { - const v = req.headers[h] if (/^(If-|Range)/i.test(h)) { newHeaders[h] = req.headers[h] } @@ -567,7 +496,7 @@ module.exports = CompileController = { } const proxy = request(options) proxy.pipe(res) - return proxy.on('error', error => + proxy.on('error', error => logger.warn({ err: error, url }, 'CLSI proxy error') ) } @@ -575,23 +504,23 @@ module.exports = CompileController = { }, wordCount(req, res, next) { - const project_id = req.params.Project_id + const projectId = req.params.Project_id const file = req.query.file || false const { clsiserverid } = req.query - return CompileController._compileAsUser(req, function (error, user_id) { - if (error != null) { + CompileController._compileAsUser(req, function (error, userId) { + if (error) { return next(error) } CompileManager.wordCount( - project_id, - user_id, + projectId, + userId, file, clsiserverid, function (error, body) { - if (error != null) { + if (error) { return next(error) } - return res.json(body) + res.json(body) } ) }) @@ -600,13 +529,13 @@ module.exports = CompileController = { function _getPersistenceOptions(req, projectId, compileGroup, callback) { const { clsiserverid } = req.query - const user_id = SessionManager.getLoggedInUserId(req) + const userId = SessionManager.getLoggedInUserId(req) if (clsiserverid && typeof clsiserverid === 'string') { callback(null, { qs: { clsiserverid, compileGroup } }) } else { ClsiCookieManager.getCookieJar( projectId, - user_id, + userId, compileGroup, (err, jar) => { callback(err, { jar }) diff --git a/services/web/app/src/Features/Compile/CompileManager.js b/services/web/app/src/Features/Compile/CompileManager.js index 2c38207fd1..a4bbcc7d78 100644 --- a/services/web/app/src/Features/Compile/CompileManager.js +++ b/services/web/app/src/Features/Compile/CompileManager.js @@ -1,18 +1,3 @@ -/* eslint-disable - camelcase, - n/handle-callback-err, - max-len, -*/ -// 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 - * DS103: Rewrite code to no longer use __guard__ - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ let CompileManager const Settings = require('@overleaf/settings') const RedisWrapper = require('../../infrastructure/RedisWrapper') @@ -25,48 +10,42 @@ const Metrics = require('@overleaf/metrics') const rateLimiter = require('../../infrastructure/RateLimiter') module.exports = CompileManager = { - compile(project_id, user_id, options, _callback) { - if (options == null) { - options = {} - } - if (_callback == null) { - _callback = function () {} - } + compile(projectId, userId, options = {}, _callback) { const timer = new Metrics.Timer('editor.compile') const callback = function (...args) { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } - return CompileManager._checkIfRecentlyCompiled( - project_id, - user_id, + CompileManager._checkIfRecentlyCompiled( + projectId, + userId, function (error, recentlyCompiled) { - if (error != null) { + if (error) { return callback(error) } if (recentlyCompiled) { return callback(null, 'too-recently-compiled', []) } - return CompileManager._checkIfAutoCompileLimitHasBeenHit( + CompileManager._checkIfAutoCompileLimitHasBeenHit( options.isAutoCompile, 'everyone', function (err, canCompile) { - if (!canCompile) { + if (err || !canCompile) { return callback(null, 'autocompile-backoff', []) } - return ProjectRootDocManager.ensureRootDocumentIsSet( - project_id, + ProjectRootDocManager.ensureRootDocumentIsSet( + projectId, function (error) { - if (error != null) { + if (error) { return callback(error) } - return CompileManager.getProjectCompileLimits( - project_id, + CompileManager.getProjectCompileLimits( + projectId, function (error, limits) { - if (error != null) { + if (error) { return callback(error) } for (const key in limits) { @@ -74,19 +53,19 @@ module.exports = CompileManager = { options[key] = value } // Put a lower limit on autocompiles for free users, based on compileGroup - return CompileManager._checkCompileGroupAutoCompileLimit( + CompileManager._checkCompileGroupAutoCompileLimit( options.isAutoCompile, limits.compileGroup, function (err, canCompile) { - if (!canCompile) { + if (err || !canCompile) { return callback(null, 'autocompile-backoff', []) } - // only pass user_id down to clsi if this is a per-user compile + // only pass userId down to clsi if this is a per-user compile const compileAsUser = Settings.disablePerUserCompiles ? undefined - : user_id - return ClsiManager.sendRequest( - project_id, + : userId + ClsiManager.sendRequest( + projectId, compileAsUser, options, function ( @@ -99,10 +78,10 @@ module.exports = CompileManager = { timings, outputUrlPrefix ) { - if (error != null) { + if (error) { return callback(error) } - return callback( + callback( null, status, outputFiles, @@ -127,66 +106,51 @@ module.exports = CompileManager = { ) }, - stopCompile(project_id, user_id, callback) { - if (callback == null) { - callback = function () {} - } - return CompileManager.getProjectCompileLimits( - project_id, - function (error, limits) { - if (error != null) { - return callback(error) - } - return ClsiManager.stopCompile(project_id, user_id, limits, callback) + stopCompile(projectId, userId, callback) { + CompileManager.getProjectCompileLimits(projectId, function (error, limits) { + if (error) { + return callback(error) } - ) + ClsiManager.stopCompile(projectId, userId, limits, callback) + }) }, - deleteAuxFiles(project_id, user_id, clsiserverid, callback) { - if (callback == null) { - callback = function () {} - } - return CompileManager.getProjectCompileLimits( - project_id, - function (error, limits) { - if (error != null) { - return callback(error) - } - ClsiManager.deleteAuxFiles( - project_id, - user_id, - limits, - clsiserverid, - callback - ) + deleteAuxFiles(projectId, userId, clsiserverid, callback) { + CompileManager.getProjectCompileLimits(projectId, function (error, limits) { + if (error) { + return callback(error) } - ) + ClsiManager.deleteAuxFiles( + projectId, + userId, + limits, + clsiserverid, + callback + ) + }) }, - getProjectCompileLimits(project_id, callback) { - if (callback == null) { - callback = function () {} - } - return ProjectGetter.getProject( - project_id, + getProjectCompileLimits(projectId, callback) { + ProjectGetter.getProject( + projectId, { owner_ref: 1 }, function (error, project) { - if (error != null) { + if (error) { return callback(error) } - return UserGetter.getUser( + UserGetter.getUser( project.owner_ref, { alphaProgram: 1, betaProgram: 1, features: 1 }, function (err, owner) { - if (error != null) { - return callback(error) + if (err) { + return callback(err) } const ownerFeatures = (owner && owner.features) || {} // put alpha users into their own compile group if (owner && owner.alphaProgram) { ownerFeatures.compileGroup = 'alpha' } - return callback(null, { + callback(null, { timeout: ownerFeatures.compileTimeout || Settings.defaultFeatures.compileTimeout, @@ -201,54 +165,45 @@ module.exports = CompileManager = { }, COMPILE_DELAY: 1, // seconds - _checkIfRecentlyCompiled(project_id, user_id, callback) { - if (callback == null) { - callback = function () {} - } - const key = `compile:${project_id}:${user_id}` - return rclient.set( + _checkIfRecentlyCompiled(projectId, userId, callback) { + const key = `compile:${projectId}:${userId}` + rclient.set( key, true, 'EX', this.COMPILE_DELAY, 'NX', function (error, ok) { - if (error != null) { + if (error) { return callback(error) } if (ok === 'OK') { - return callback(null, false) + callback(null, false) } else { - return callback(null, true) + callback(null, true) } } ) }, _checkCompileGroupAutoCompileLimit(isAutoCompile, compileGroup, callback) { - if (callback == null) { - callback = function () {} - } if (!isAutoCompile) { return callback(null, true) } if (compileGroup === 'standard') { // apply extra limits to the standard compile group - return CompileManager._checkIfAutoCompileLimitHasBeenHit( + CompileManager._checkIfAutoCompileLimitHasBeenHit( isAutoCompile, compileGroup, callback ) } else { Metrics.inc(`auto-compile-${compileGroup}`) - return callback(null, true) + callback(null, true) } }, // always allow priority group users to compile _checkIfAutoCompileLimitHasBeenHit(isAutoCompile, compileGroup, callback) { - if (callback == null) { - callback = function () {} - } if (!isAutoCompile) { return callback(null, true) } @@ -259,36 +214,30 @@ module.exports = CompileManager = { subjectName: compileGroup, throttle: Settings.rateLimit.autoCompile[compileGroup] || 25, } - return rateLimiter.addCount(opts, function (err, canCompile) { - if (err != null) { + rateLimiter.addCount(opts, function (err, canCompile) { + if (err) { canCompile = false } if (!canCompile) { Metrics.inc(`auto-compile-${compileGroup}-limited`) } - return callback(err, canCompile) + callback(null, canCompile) }) }, - wordCount(project_id, user_id, file, clsiserverid, callback) { - if (callback == null) { - callback = function () {} - } - return CompileManager.getProjectCompileLimits( - project_id, - function (error, limits) { - if (error != null) { - return callback(error) - } - ClsiManager.wordCount( - project_id, - user_id, - file, - limits, - clsiserverid, - callback - ) + wordCount(projectId, userId, file, clsiserverid, callback) { + CompileManager.getProjectCompileLimits(projectId, function (error, limits) { + if (error) { + return callback(error) } - ) + ClsiManager.wordCount( + projectId, + userId, + file, + limits, + clsiserverid, + callback + ) + }) }, } diff --git a/services/web/test/unit/src/Compile/CompileControllerTests.js b/services/web/test/unit/src/Compile/CompileControllerTests.js index 6d2db992a3..6020acba28 100644 --- a/services/web/test/unit/src/Compile/CompileControllerTests.js +++ b/services/web/test/unit/src/Compile/CompileControllerTests.js @@ -1,18 +1,5 @@ -/* eslint-disable - camelcase, - max-len, - 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 sinon = require('sinon') -const { assert, expect } = require('chai') +const { expect } = require('chai') const modulePath = '../../../../app/src/Features/Compile/CompileController.js' const SandboxedModule = require('sandboxed-module') const MockRequest = require('../helpers/MockRequest') @@ -76,28 +63,28 @@ describe('CompileController', function () { }, }, }) - this.project_id = 'project-id' + this.projectId = 'project-id' this.next = sinon.stub() this.req = new MockRequest() - return (this.res = new MockResponse()) + this.res = new MockResponse() }) describe('compile', function () { beforeEach(function () { - this.req.params = { Project_id: this.project_id } + this.req.params = { Project_id: this.projectId } this.req.session = {} - return (this.CompileManager.compile = sinon.stub().callsArgWith( + this.CompileManager.compile = sinon.stub().callsArgWith( 3, null, (this.status = 'success'), (this.outputFiles = [ { path: 'output.pdf', - url: `/project/${this.project_id}/user/${this.user_id}/build/id/output.pdf`, + url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`, type: 'pdf', }, ]) - )) + ) }) describe('zonal downloads', function () { @@ -110,7 +97,7 @@ describe('CompileController', function () { (this.outputFiles = [ { path: 'output.pdf', - url: `/project/${this.project_id}/user/${this.user_id}/build/id/output.pdf`, + url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`, type: 'pdf', }, ]), @@ -134,7 +121,7 @@ describe('CompileController', function () { { path: 'output.pdf', // The previous clsi version sent the zone prefix in the url - url: `/zone/b/project/${this.project_id}/user/${this.user_id}/build/id/output.pdf`, + url: `/zone/b/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`, type: 'pdf', }, ]) @@ -150,7 +137,7 @@ describe('CompileController', function () { outputFiles: [ { path: 'output.pdf', - url: `/project/${this.project_id}/user/${this.user_id}/build/id/output.pdf`, + url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`, type: 'pdf', }, ], @@ -200,7 +187,7 @@ describe('CompileController', function () { outputFiles: [ { path: 'output.pdf', - url: `/project/${this.project_id}/user/${this.user_id}/build/id/output.pdf`, + url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`, type: 'pdf', }, ], @@ -224,7 +211,7 @@ describe('CompileController', function () { outputFiles: [ { path: 'output.pdf', - url: `/project/${this.project_id}/user/${this.user_id}/build/id/output.pdf`, + url: `/project/${this.projectId}/user/${this.user_id}/build/id/output.pdf`, type: 'pdf', }, ], @@ -237,18 +224,18 @@ describe('CompileController', function () { describe('when not an auto compile', function () { beforeEach(function () { - return this.CompileController.compile(this.req, this.res, this.next) + this.CompileController.compile(this.req, this.res, this.next) }) it('should look up the user id', function () { - return this.SessionManager.getLoggedInUserId + this.SessionManager.getLoggedInUserId .calledWith(this.req.session) .should.equal(true) }) it('should do the compile without the auto compile flag', function () { - return this.CompileManager.compile - .calledWith(this.project_id, this.user_id, { + this.CompileManager.compile + .calledWith(this.projectId, this.user_id, { isAutoCompile: false, enablePdfCaching: false, fileLineErrors: false, @@ -274,12 +261,12 @@ describe('CompileController', function () { describe('when an auto compile', function () { beforeEach(function () { this.req.query = { auto_compile: 'true' } - return this.CompileController.compile(this.req, this.res, this.next) + this.CompileController.compile(this.req, this.res, this.next) }) it('should do the compile with the auto compile flag', function () { - return this.CompileManager.compile - .calledWith(this.project_id, this.user_id, { + this.CompileManager.compile + .calledWith(this.projectId, this.user_id, { isAutoCompile: true, enablePdfCaching: false, fileLineErrors: false, @@ -291,12 +278,12 @@ describe('CompileController', function () { describe('with the draft attribute', function () { beforeEach(function () { this.req.body = { draft: true } - return this.CompileController.compile(this.req, this.res, this.next) + this.CompileController.compile(this.req, this.res, this.next) }) it('should do the compile without the draft compile flag', function () { - return this.CompileManager.compile - .calledWith(this.project_id, this.user_id, { + this.CompileManager.compile + .calledWith(this.projectId, this.user_id, { isAutoCompile: false, enablePdfCaching: false, draft: true, @@ -312,7 +299,7 @@ describe('CompileController', function () { this.submission_id = 'sub-1234' this.req.params = { submission_id: this.submission_id } this.req.body = {} - return (this.ClsiManager.sendExternalRequest = sinon + this.ClsiManager.sendExternalRequest = sinon .stub() .callsArgWith( 3, @@ -321,20 +308,18 @@ describe('CompileController', function () { (this.outputFiles = ['mock-output-files']), (this.clsiServerId = 'mock-server-id'), (this.validationProblems = null) - )) + ) }) it('should set the content-type of the response to application/json', function () { this.CompileController.compileSubmission(this.req, this.res, this.next) - return this.res.contentType - .calledWith('application/json') - .should.equal(true) + this.res.contentType.calledWith('application/json').should.equal(true) }) it('should send a successful response reporting the status and files', function () { this.CompileController.compileSubmission(this.req, this.res, this.next) this.res.statusCode.should.equal(200) - return this.res.body.should.equal( + this.res.body.should.equal( JSON.stringify({ status: this.status, outputFiles: this.outputFiles, @@ -350,15 +335,11 @@ describe('CompileController', function () { compileGroup: 'special', timeout: 600, } - return this.CompileController.compileSubmission( - this.req, - this.res, - this.next - ) + this.CompileController.compileSubmission(this.req, this.res, this.next) }) it('should use the supplied values', function () { - return this.ClsiManager.sendExternalRequest + this.ClsiManager.sendExternalRequest .calledWith( this.submission_id, { compileGroup: 'special', timeout: 600 }, @@ -376,15 +357,11 @@ describe('CompileController', function () { draft: true, check: 'validate', } - return this.CompileController.compileSubmission( - this.req, - this.res, - this.next - ) + this.CompileController.compileSubmission(this.req, this.res, this.next) }) it('should use the other options but default values for compileGroup and timeout', function () { - return this.ClsiManager.sendExternalRequest + this.ClsiManager.sendExternalRequest .calledWith( this.submission_id, { @@ -409,49 +386,47 @@ describe('CompileController', function () { describe('downloadPdf', function () { beforeEach(function () { - this.req.params = { Project_id: this.project_id } + this.req.params = { Project_id: this.projectId } this.req.query = { pdfng: true } this.project = { name: 'test namè' } - return (this.ProjectGetter.getProject = sinon + this.ProjectGetter.getProject = sinon .stub() - .callsArgWith(2, null, this.project)) + .callsArgWith(2, null, this.project) }) describe('when downloading for embedding', function () { beforeEach(function () { this.CompileController.proxyToClsi = sinon.stub() this.RateLimiter.addCount.callsArgWith(1, null, true) - return this.CompileController.downloadPdf(this.req, this.res, this.next) + this.CompileController.downloadPdf(this.req, this.res, this.next) }) it('should look up the project', function () { - return this.ProjectGetter.getProject - .calledWith(this.project_id, { name: 1 }) + this.ProjectGetter.getProject + .calledWith(this.projectId, { name: 1 }) .should.equal(true) }) it('should set the content-type of the response to application/pdf', function () { - return this.res.contentType - .calledWith('application/pdf') - .should.equal(true) + this.res.contentType.calledWith('application/pdf').should.equal(true) }) it('should set the content-disposition header with a safe version of the project name', function () { - return this.res.setContentDisposition + this.res.setContentDisposition .calledWith('', { filename: 'test_nam_.pdf' }) .should.equal(true) }) it('should increment the pdf-downloads metric', function () { - return this.Metrics.inc.calledWith('pdf-downloads').should.equal(true) + this.Metrics.inc.calledWith('pdf-downloads').should.equal(true) }) it('should proxy the PDF from the CLSI', function () { - return this.CompileController.proxyToClsi + this.CompileController.proxyToClsi .calledWith( - this.project_id, - `/project/${this.project_id}/user/${this.user_id}/output/output.pdf`, + this.projectId, + `/project/${this.projectId}/user/${this.user_id}/output/output.pdf`, this.req, this.res, this.next @@ -465,14 +440,14 @@ describe('CompileController', function () { this.req.params.build_id = this.buildId = '1234-5678' this.CompileController.proxyToClsi = sinon.stub() this.RateLimiter.addCount.callsArgWith(1, null, true) - return this.CompileController.downloadPdf(this.req, this.res, this.next) + this.CompileController.downloadPdf(this.req, this.res, this.next) }) it('should proxy the PDF from the CLSI, with a build-id', function () { - return this.CompileController.proxyToClsi + this.CompileController.proxyToClsi .calledWith( - this.project_id, - `/project/${this.project_id}/user/${this.user_id}/build/${this.buildId}/output/output.pdf`, + this.projectId, + `/project/${this.projectId}/user/${this.user_id}/build/${this.buildId}/output/output.pdf`, this.req, this.res, this.next @@ -485,21 +460,21 @@ describe('CompileController', function () { it('should check the rate limiter when pdfng is not set', function (done) { this.req.query = {} this.RateLimiter.addCount.callsArgWith(1, null, true) - this.CompileController.proxyToClsi = (project_id, url) => { + this.CompileController.proxyToClsi = (projectId, url) => { this.RateLimiter.addCount.args[0][0].throttle.should.equal(1000) - return done() + done() } - return this.CompileController.downloadPdf(this.req, this.res) + this.CompileController.downloadPdf(this.req, this.res) }) it('should check the rate limiter when pdfng is false', function (done) { this.req.query = { pdfng: false } this.RateLimiter.addCount.callsArgWith(1, null, true) - this.CompileController.proxyToClsi = (project_id, url) => { + this.CompileController.proxyToClsi = (projectId, url) => { this.RateLimiter.addCount.args[0][0].throttle.should.equal(1000) - return done() + done() } - return this.CompileController.downloadPdf(this.req, this.res) + this.CompileController.downloadPdf(this.req, this.res) }) }) }) @@ -516,12 +491,12 @@ describe('CompileController', function () { } this.req.body = {} this.expected_url = `/project/${this.submission_id}/build/${this.build_id}/output/${this.file}` - return (this.CompileController.proxyToClsiWithLimits = sinon.stub()) + this.CompileController.proxyToClsiWithLimits = sinon.stub() }) describe('without limits specified', function () { beforeEach(function () { - return this.CompileController.getFileFromClsiWithoutUser( + this.CompileController.getFileFromClsiWithoutUser( this.req, this.res, this.next @@ -529,7 +504,7 @@ describe('CompileController', function () { }) it('should proxy to CLSI with correct URL and default limits', function () { - return this.CompileController.proxyToClsiWithLimits + this.CompileController.proxyToClsiWithLimits .calledWith(this.submission_id, this.expected_url, { compileGroup: 'standard', }) @@ -540,7 +515,7 @@ describe('CompileController', function () { describe('with limits specified', function () { beforeEach(function () { this.req.body = { compileTimeout: 600, compileGroup: 'special' } - return this.CompileController.getFileFromClsiWithoutUser( + this.CompileController.getFileFromClsiWithoutUser( this.req, this.res, this.next @@ -548,7 +523,7 @@ describe('CompileController', function () { }) it('should proxy to CLSI with correct URL and specified limits', function () { - return this.CompileController.proxyToClsiWithLimits + this.CompileController.proxyToClsiWithLimits .calledWith(this.submission_id, this.expected_url, { compileGroup: 'special', }) @@ -560,7 +535,7 @@ describe('CompileController', function () { let file, line, column, imageName beforeEach(function (done) { - this.req.params = { Project_id: this.project_id } + this.req.params = { Project_id: this.projectId } file = 'main.tex' line = String(Date.now()) column = String(Date.now() + 1) @@ -578,9 +553,9 @@ describe('CompileController', function () { it('should proxy the request with an imageName', function () { expect(this.CompileController.proxyToClsi).to.have.been.calledWith( - this.project_id, + this.projectId, { - url: `/project/${this.project_id}/user/${this.user_id}/sync/code`, + url: `/project/${this.projectId}/user/${this.user_id}/sync/code`, qs: { file, line, column, imageName }, }, this.req, @@ -594,7 +569,7 @@ describe('CompileController', function () { let page, h, v, imageName beforeEach(function (done) { - this.req.params = { Project_id: this.project_id } + this.req.params = { Project_id: this.projectId } page = String(Date.now()) h = String(Math.random()) v = String(Math.random()) @@ -612,9 +587,9 @@ describe('CompileController', function () { it('should proxy the request with an imageName', function () { expect(this.CompileController.proxyToClsi).to.have.been.calledWith( - this.project_id, + this.projectId, { - url: `/project/${this.project_id}/user/${this.user_id}/sync/pdf`, + url: `/project/${this.projectId}/user/${this.user_id}/sync/pdf`, qs: { page, h, v, imageName }, }, this.req, @@ -637,12 +612,12 @@ describe('CompileController', function () { headers: { mock: 'header' }, } this.req.method = 'mock-method' - return (this.req.headers = { + this.req.headers = { Mock: 'Headers', Range: '123-456', 'If-Range': 'abcdef', 'If-Modified-Since': 'Mon, 15 Dec 2014 15:23:56 GMT', - }) + } }) describe('old pdf viewer', function () { @@ -651,8 +626,8 @@ describe('CompileController', function () { this.CompileManager.getProjectCompileLimits = sinon .stub() .callsArgWith(1, null, { compileGroup: 'standard' }) - return this.CompileController.proxyToClsi( - this.project_id, + this.CompileController.proxyToClsi( + this.projectId, (this.url = '/test'), this.req, this.res, @@ -661,7 +636,7 @@ describe('CompileController', function () { }) it('should open a request to the CLSI', function () { - return this.request + this.request .calledWith({ jar: this.jar, method: this.req.method, @@ -672,11 +647,11 @@ describe('CompileController', function () { }) it('should pass the request on to the client', function () { - return this.proxy.pipe.calledWith(this.res).should.equal(true) + this.proxy.pipe.calledWith(this.res).should.equal(true) }) it('should bind an error handle to the request proxy', function () { - return this.proxy.on.calledWith('error').should.equal(true) + this.proxy.on.calledWith('error').should.equal(true) }) }) @@ -685,8 +660,8 @@ describe('CompileController', function () { this.CompileManager.getProjectCompileLimits = sinon .stub() .callsArgWith(1, null, { compileGroup: 'priority' }) - return this.CompileController.proxyToClsi( - this.project_id, + this.CompileController.proxyToClsi( + this.projectId, (this.url = '/test'), this.req, this.res, @@ -698,8 +673,8 @@ describe('CompileController', function () { describe('user with standard priority via query string', function () { beforeEach(function () { this.req.query = { compileGroup: 'standard' } - return this.CompileController.proxyToClsi( - this.project_id, + this.CompileController.proxyToClsi( + this.projectId, (this.url = '/test'), this.req, this.res, @@ -708,7 +683,7 @@ describe('CompileController', function () { }) it('should open a request to the CLSI', function () { - return this.request + this.request .calledWith({ jar: this.jar, method: this.req.method, @@ -719,19 +694,19 @@ describe('CompileController', function () { }) it('should pass the request on to the client', function () { - return this.proxy.pipe.calledWith(this.res).should.equal(true) + this.proxy.pipe.calledWith(this.res).should.equal(true) }) it('should bind an error handle to the request proxy', function () { - return this.proxy.on.calledWith('error').should.equal(true) + this.proxy.on.calledWith('error').should.equal(true) }) }) describe('user with non-existent priority via query string', function () { beforeEach(function () { this.req.query = { compileGroup: 'foobar' } - return this.CompileController.proxyToClsi( - this.project_id, + this.CompileController.proxyToClsi( + this.projectId, (this.url = '/test'), this.req, this.res, @@ -740,7 +715,7 @@ describe('CompileController', function () { }) it('should proxy to the standard url', function () { - return this.request + this.request .calledWith({ jar: this.jar, method: this.req.method, @@ -757,8 +732,8 @@ describe('CompileController', function () { .stub() .callsArgWith(1, null, { compileGroup: 'standard' }) this.req.query = { build: 1234 } - return this.CompileController.proxyToClsi( - this.project_id, + this.CompileController.proxyToClsi( + this.projectId, (this.url = '/test'), this.req, this.res, @@ -767,7 +742,7 @@ describe('CompileController', function () { }) it('should proxy to the standard url without the build parameter', function () { - return this.request + this.request .calledWith({ jar: this.jar, method: this.req.method, @@ -781,15 +756,15 @@ describe('CompileController', function () { describe('new pdf viewer', function () { beforeEach(function () { - return (this.req.query = { pdfng: true }) + this.req.query = { pdfng: true } }) describe('user with standard priority', function () { beforeEach(function () { this.CompileManager.getProjectCompileLimits = sinon .stub() .callsArgWith(1, null, { compileGroup: 'standard' }) - return this.CompileController.proxyToClsi( - this.project_id, + this.CompileController.proxyToClsi( + this.projectId, (this.url = '/test'), this.req, this.res, @@ -798,7 +773,7 @@ describe('CompileController', function () { }) it('should open a request to the CLSI', function () { - return this.request + this.request .calledWith({ jar: this.jar, method: this.req.method, @@ -814,11 +789,11 @@ describe('CompileController', function () { }) it('should pass the request on to the client', function () { - return this.proxy.pipe.calledWith(this.res).should.equal(true) + this.proxy.pipe.calledWith(this.res).should.equal(true) }) it('should bind an error handle to the request proxy', function () { - return this.proxy.on.calledWith('error').should.equal(true) + this.proxy.on.calledWith('error').should.equal(true) }) }) @@ -828,8 +803,8 @@ describe('CompileController', function () { .stub() .callsArgWith(1, null, { compileGroup: 'standard' }) this.req.query = { build: 1234, pdfng: true } - return this.CompileController.proxyToClsi( - this.project_id, + this.CompileController.proxyToClsi( + this.projectId, (this.url = '/test'), this.req, this.res, @@ -838,7 +813,7 @@ describe('CompileController', function () { }) it('should proxy to the standard url with the build parameter', function () { - return this.request + this.request .calledWith({ jar: this.jar, method: this.req.method, @@ -860,24 +835,20 @@ describe('CompileController', function () { describe('deleteAuxFiles', function () { beforeEach(function () { this.CompileManager.deleteAuxFiles = sinon.stub().yields() - this.req.params = { Project_id: this.project_id } + this.req.params = { Project_id: this.projectId } this.req.query = { clsiserverid: 'node-1' } this.res.sendStatus = sinon.stub() - return this.CompileController.deleteAuxFiles( - this.req, - this.res, - this.next - ) + this.CompileController.deleteAuxFiles(this.req, this.res, this.next) }) it('should proxy to the CLSI', function () { - return this.CompileManager.deleteAuxFiles - .calledWith(this.project_id, this.user_id, 'node-1') + this.CompileManager.deleteAuxFiles + .calledWith(this.projectId, this.user_id, 'node-1') .should.equal(true) }) it('should return a 200', function () { - return this.res.sendStatus.calledWith(200).should.equal(true) + this.res.sendStatus.calledWith(200).should.equal(true) }) }) @@ -885,39 +856,39 @@ describe('CompileController', function () { beforeEach(function () { this.req = { params: { - project_id: this.project_id, + project_id: this.projectId, }, } this.CompileManager.compile.callsArgWith(3) this.CompileController.proxyToClsi = sinon.stub() - return (this.res = { send: () => {} }) + this.res = { send: () => {} } }) it('should call compile in the compile manager', function (done) { this.CompileController.compileAndDownloadPdf(this.req, this.res) - this.CompileManager.compile.calledWith(this.project_id).should.equal(true) - return done() + this.CompileManager.compile.calledWith(this.projectId).should.equal(true) + done() }) it('should proxy the res to the clsi with correct url', function (done) { this.CompileController.compileAndDownloadPdf(this.req, this.res) sinon.assert.calledWith( this.CompileController.proxyToClsi, - this.project_id, - `/project/${this.project_id}/output/output.pdf`, + this.projectId, + `/project/${this.projectId}/output/output.pdf`, this.req, this.res ) this.CompileController.proxyToClsi .calledWith( - this.project_id, - `/project/${this.project_id}/output/output.pdf`, + this.projectId, + `/project/${this.projectId}/output/output.pdf`, this.req, this.res ) .should.equal(true) - return done() + done() }) }) @@ -926,21 +897,21 @@ describe('CompileController', function () { this.CompileManager.wordCount = sinon .stub() .yields(null, { content: 'body' }) - this.req.params = { Project_id: this.project_id } + this.req.params = { Project_id: this.projectId } this.req.query = { clsiserverid: 'node-42' } this.res.json = sinon.stub() this.res.contentType = sinon.stub() - return this.CompileController.wordCount(this.req, this.res, this.next) + this.CompileController.wordCount(this.req, this.res, this.next) }) it('should proxy to the CLSI', function () { - return this.CompileManager.wordCount - .calledWith(this.project_id, this.user_id, false, 'node-42') + this.CompileManager.wordCount + .calledWith(this.projectId, this.user_id, false, 'node-42') .should.equal(true) }) it('should return a 200 and body', function () { - return this.res.json.calledWith({ content: 'body' }).should.equal(true) + this.res.json.calledWith({ content: 'body' }).should.equal(true) }) }) }) diff --git a/services/web/test/unit/src/Compile/CompileManagerTests.js b/services/web/test/unit/src/Compile/CompileManagerTests.js index a4fc9b23b4..44b46a8c69 100644 --- a/services/web/test/unit/src/Compile/CompileManagerTests.js +++ b/services/web/test/unit/src/Compile/CompileManagerTests.js @@ -1,28 +1,18 @@ -/* eslint-disable - n/handle-callback-err, - max-len, - 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 sinon = require('sinon') -const { assert, expect } = require('chai') const modulePath = '../../../../app/src/Features/Compile/CompileManager.js' const SandboxedModule = require('sandboxed-module') describe('CompileManager', function () { beforeEach(function () { - let Timer this.rateLimitGetStub = sinon.stub() - const { rateLimitGetStub } = this this.ratelimiter = { addCount: sinon.stub() } + this.timer = { + done: sinon.stub(), + } + this.Metrics = { + Timer: sinon.stub().returns(this.timer), + inc: sinon.stub(), + } this.CompileManager = SandboxedModule.require(modulePath, { requires: { '@overleaf/settings': (this.settings = { @@ -30,36 +20,23 @@ describe('CompileManager', function () { rateLimit: { autoCompile: {} }, }), '../../infrastructure/RedisWrapper': { - client: () => { - return (this.rclient = { auth() {} }) - }, + client: () => (this.rclient = { auth() {} }), }, '../Project/ProjectRootDocManager': (this.ProjectRootDocManager = {}), '../Project/ProjectGetter': (this.ProjectGetter = {}), '../User/UserGetter': (this.UserGetter = {}), './ClsiManager': (this.ClsiManager = {}), '../../infrastructure/RateLimiter': this.ratelimiter, - '@overleaf/metrics': (this.Metrics = { - Timer: (Timer = (function () { - Timer = class Timer { - static initClass() { - this.prototype.done = sinon.stub() - } - } - Timer.initClass() - return Timer - })()), - inc: sinon.stub(), - }), + '@overleaf/metrics': this.Metrics, }, }) this.project_id = 'mock-project-id-123' this.user_id = 'mock-user-id-123' this.callback = sinon.stub() - return (this.limits = { + this.limits = { timeout: 42, compileGroup: 'standard', - }) + } }) describe('compile', function () { @@ -73,7 +50,7 @@ describe('CompileManager', function () { this.CompileManager.getProjectCompileLimits = sinon .stub() .callsArgWith(1, null, this.limits) - return (this.ClsiManager.sendRequest = sinon + this.ClsiManager.sendRequest = sinon .stub() .callsArgWith( 3, @@ -81,7 +58,7 @@ describe('CompileManager', function () { (this.status = 'mock-status'), (this.outputFiles = 'mock output files'), (this.output = 'mock output') - )) + ) }) describe('succesfully', function () { @@ -91,7 +68,7 @@ describe('CompileManager', function () { compileGroup, cb ) => cb(null, true) - return this.CompileManager.compile( + this.CompileManager.compile( this.project_id, this.user_id, {}, @@ -100,25 +77,25 @@ describe('CompileManager', function () { }) it('should check the project has not been recently compiled', function () { - return this.CompileManager._checkIfRecentlyCompiled + this.CompileManager._checkIfRecentlyCompiled .calledWith(this.project_id, this.user_id) .should.equal(true) }) it('should ensure that the root document is set', function () { - return this.ProjectRootDocManager.ensureRootDocumentIsSet + this.ProjectRootDocManager.ensureRootDocumentIsSet .calledWith(this.project_id) .should.equal(true) }) it('should get the project compile limits', function () { - return this.CompileManager.getProjectCompileLimits + this.CompileManager.getProjectCompileLimits .calledWith(this.project_id) .should.equal(true) }) it('should run the compile with the compile limits', function () { - return this.ClsiManager.sendRequest + this.ClsiManager.sendRequest .calledWith(this.project_id, this.user_id, { timeout: this.limits.timeout, compileGroup: 'standard', @@ -127,13 +104,13 @@ describe('CompileManager', function () { }) it('should call the callback with the output', function () { - return this.callback + this.callback .calledWith(null, this.status, this.outputFiles, this.output) .should.equal(true) }) it('should time the compile', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.timer.done.called.should.equal(true) }) }) @@ -147,13 +124,16 @@ describe('CompileManager', function () { this.CompileManager._checkIfRecentlyCompiled = sinon .stub() .callsArgWith(2, null, true) - return this.CompileManager.compile( + this.CompileManager.compile( this.project_id, this.user_id, {}, (err, status) => { + if (err) { + return done(err) + } status.should.equal('too-recently-compiled') - return done() + done() } ) }) @@ -164,13 +144,16 @@ describe('CompileManager', function () { this.CompileManager._checkIfAutoCompileLimitHasBeenHit = sinon .stub() .callsArgWith(2, null, false) - return this.CompileManager.compile( + this.CompileManager.compile( this.project_id, this.user_id, {}, (err, status) => { + if (err) { + return done(err) + } status.should.equal('autocompile-backoff') - return done() + done() } ) }) @@ -193,20 +176,20 @@ describe('CompileManager', function () { this.UserGetter.getUser = sinon .stub() .callsArgWith(2, null, (this.user = { features: this.features })) - return this.CompileManager.getProjectCompileLimits( + this.CompileManager.getProjectCompileLimits( this.project_id, this.callback ) }) it('should look up the owner of the project', function () { - return this.ProjectGetter.getProject + this.ProjectGetter.getProject .calledWith(this.project_id, { owner_ref: 1 }) .should.equal(true) }) it("should look up the owner's features", function () { - return this.UserGetter.getUser + this.UserGetter.getUser .calledWith(this.project.owner_ref, { alphaProgram: 1, betaProgram: 1, @@ -216,7 +199,7 @@ describe('CompileManager', function () { }) it('should return the limits', function () { - return this.callback + this.callback .calledWith(null, { timeout: this.timeout, compileGroup: this.group, @@ -235,7 +218,7 @@ describe('CompileManager', function () { (this.limits = { compileGroup: 'mock-compile-group' }) ) this.ClsiManager.deleteAuxFiles = sinon.stub().callsArg(3) - return this.CompileManager.deleteAuxFiles( + this.CompileManager.deleteAuxFiles( this.project_id, this.user_id, this.callback @@ -243,19 +226,19 @@ describe('CompileManager', function () { }) it('should look up the compile group to use', function () { - return this.CompileManager.getProjectCompileLimits + this.CompileManager.getProjectCompileLimits .calledWith(this.project_id) .should.equal(true) }) it('should delete the aux files', function () { - return this.ClsiManager.deleteAuxFiles + this.ClsiManager.deleteAuxFiles .calledWith(this.project_id, this.user_id, this.limits) .should.equal(true) }) it('should call the callback', function () { - return this.callback.called.should.equal(true) + this.callback.called.should.equal(true) }) }) @@ -263,7 +246,7 @@ describe('CompileManager', function () { describe('when the key exists in redis', function () { beforeEach(function () { this.rclient.set = sinon.stub().callsArgWith(5, null, null) - return this.CompileManager._checkIfRecentlyCompiled( + this.CompileManager._checkIfRecentlyCompiled( this.project_id, this.user_id, this.callback @@ -271,7 +254,7 @@ describe('CompileManager', function () { }) it('should try to set the key', function () { - return this.rclient.set + this.rclient.set .calledWith( `compile:${this.project_id}:${this.user_id}`, true, @@ -283,14 +266,14 @@ describe('CompileManager', function () { }) it('should call the callback with true', function () { - return this.callback.calledWith(null, true).should.equal(true) + this.callback.calledWith(null, true).should.equal(true) }) }) describe('when the key does not exist in redis', function () { beforeEach(function () { this.rclient.set = sinon.stub().callsArgWith(5, null, 'OK') - return this.CompileManager._checkIfRecentlyCompiled( + this.CompileManager._checkIfRecentlyCompiled( this.project_id, this.user_id, this.callback @@ -298,7 +281,7 @@ describe('CompileManager', function () { }) it('should try to set the key', function () { - return this.rclient.set + this.rclient.set .calledWith( `compile:${this.project_id}:${this.user_id}`, true, @@ -310,7 +293,7 @@ describe('CompileManager', function () { }) it('should call the callback with false', function () { - return this.callback.calledWith(null, false).should.equal(true) + this.callback.calledWith(null, false).should.equal(true) }) }) }) @@ -318,53 +301,65 @@ describe('CompileManager', function () { describe('_checkIfAutoCompileLimitHasBeenHit', function () { it('should be able to compile if it is not an autocompile', function (done) { this.ratelimiter.addCount.callsArgWith(2, null, true) - return this.CompileManager._checkIfAutoCompileLimitHasBeenHit( + this.CompileManager._checkIfAutoCompileLimitHasBeenHit( false, 'everyone', (err, canCompile) => { + if (err) { + return done(err) + } canCompile.should.equal(true) - return done() + done() } ) }) it('should be able to compile if rate limit has remianing', function (done) { this.ratelimiter.addCount.callsArgWith(1, null, true) - return this.CompileManager._checkIfAutoCompileLimitHasBeenHit( + this.CompileManager._checkIfAutoCompileLimitHasBeenHit( true, 'everyone', (err, canCompile) => { + if (err) { + return done(err) + } const args = this.ratelimiter.addCount.args[0][0] args.throttle.should.equal(25) args.subjectName.should.equal('everyone') args.timeInterval.should.equal(20) args.endpointName.should.equal('auto_compile') canCompile.should.equal(true) - return done() + done() } ) }) it('should be not able to compile if rate limit has no remianing', function (done) { this.ratelimiter.addCount.callsArgWith(1, null, false) - return this.CompileManager._checkIfAutoCompileLimitHasBeenHit( + this.CompileManager._checkIfAutoCompileLimitHasBeenHit( true, 'everyone', (err, canCompile) => { + if (err) { + return done(err) + } canCompile.should.equal(false) - return done() + done() } ) }) it('should return false if there is an error in the rate limit', function (done) { this.ratelimiter.addCount.callsArgWith(1, 'error') - return this.CompileManager._checkIfAutoCompileLimitHasBeenHit( + this.CompileManager._checkIfAutoCompileLimitHasBeenHit( true, 'everyone', (err, canCompile) => { + if (err) { + return done(err) + } canCompile.should.equal(false) - return done() + done() } ) }) @@ -380,7 +375,7 @@ describe('CompileManager', function () { (this.limits = { compileGroup: 'mock-compile-group' }) ) this.ClsiManager.wordCount = sinon.stub().callsArg(4) - return this.CompileManager.wordCount( + this.CompileManager.wordCount( this.project_id, this.user_id, false, @@ -389,19 +384,19 @@ describe('CompileManager', function () { }) it('should look up the compile group to use', function () { - return this.CompileManager.getProjectCompileLimits + this.CompileManager.getProjectCompileLimits .calledWith(this.project_id) .should.equal(true) }) it('should call wordCount for project', function () { - return this.ClsiManager.wordCount + this.ClsiManager.wordCount .calledWith(this.project_id, this.user_id, false, this.limits) .should.equal(true) }) it('should call the callback', function () { - return this.callback.called.should.equal(true) + this.callback.called.should.equal(true) }) }) })