From bda307fbb596569a8bba13387963580a10f44e43 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Mon, 6 Jun 2022 07:41:07 -0400 Subject: [PATCH] Merge pull request #8267 from overleaf/em-decaf-clsi Decaf cleanup some CLSI files GitOrigin-RevId: afb4cd6e9eb9e95703efa955b93d4fedada10e3c --- services/clsi/app.js | 110 ++-- services/clsi/app/js/CompileController.js | 498 +++++++++--------- services/clsi/app/js/LatexRunner.js | 362 ++++++------- services/clsi/app/js/RequestParser.js | 411 +++++++-------- .../test/unit/js/CompileControllerTests.js | 108 ++-- .../clsi/test/unit/js/LatexRunnerTests.js | 43 +- .../clsi/test/unit/js/RequestParserTests.js | 219 ++++---- 7 files changed, 790 insertions(+), 961 deletions(-) diff --git a/services/clsi/app.js b/services/clsi/app.js index 623fa9b3f2..386c9efad5 100644 --- a/services/clsi/app.js +++ b/services/clsi/app.js @@ -1,10 +1,3 @@ -/* - * decaffeinate suggestions: - * 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 - */ const Metrics = require('@overleaf/metrics') Metrics.initialize('clsi') @@ -13,7 +6,7 @@ const ContentController = require('./app/js/ContentController') const Settings = require('@overleaf/settings') const logger = require('@overleaf/logger') logger.initialize('clsi') -if ((Settings.sentry != null ? Settings.sentry.dsn : undefined) != null) { +if (Settings.sentry.dsn != null) { logger.initializeErrorReporting(Settings.sentry.dsn) } @@ -48,52 +41,46 @@ app.use(function (req, res, next) { req.setTimeout(TIMEOUT) res.setTimeout(TIMEOUT) res.removeHeader('X-Powered-By') - return next() + next() }) app.param('project_id', function (req, res, next, projectId) { - if (projectId != null ? projectId.match(/^[a-zA-Z0-9_-]+$/) : undefined) { - return next() + if (projectId?.match(/^[a-zA-Z0-9_-]+$/)) { + next() } else { - return next(new Error('invalid project id')) + next(new Error('invalid project id')) } }) app.param('user_id', function (req, res, next, userId) { - if (userId != null ? userId.match(/^[0-9a-f]{24}$/) : undefined) { - return next() + if (userId?.match(/^[0-9a-f]{24}$/)) { + next() } else { - return next(new Error('invalid user id')) + next(new Error('invalid user id')) } }) app.param('build_id', function (req, res, next, buildId) { - if ( - buildId != null ? buildId.match(OutputCacheManager.BUILD_REGEX) : undefined - ) { - return next() + if (buildId?.match(OutputCacheManager.BUILD_REGEX)) { + next() } else { - return next(new Error(`invalid build id ${buildId}`)) + next(new Error(`invalid build id ${buildId}`)) } }) app.param('contentId', function (req, res, next, contentId) { - if ( - contentId != null - ? contentId.match(OutputCacheManager.CONTENT_REGEX) - : undefined - ) { - return next() + if (contentId?.match(OutputCacheManager.CONTENT_REGEX)) { + next() } else { - return next(new Error(`invalid content id ${contentId}`)) + next(new Error(`invalid content id ${contentId}`)) } }) app.param('hash', function (req, res, next, hash) { - if (hash != null ? hash.match(ContentCacheManager.HASH_REGEX) : undefined) { - return next() + if (hash?.match(ContentCacheManager.HASH_REGEX)) { + next() } else { - return next(new Error(`invalid hash ${hash}`)) + next(new Error(`invalid hash ${hash}`)) } }) @@ -156,7 +143,7 @@ const staticCompileServer = ForbidSymlinks( '"' res.set('Etag', etag(path, stat)) } - return res.set('Content-Type', ContentTypeMapper.map(path)) + res.set('Content-Type', ContentTypeMapper.map(path)) }, } ) @@ -176,7 +163,7 @@ const staticOutputServer = ForbidSymlinks( '"' res.set('Etag', etag(path, stat)) } - return res.set('Content-Type', ContentTypeMapper.map(path)) + res.set('Content-Type', ContentTypeMapper.map(path)) }, } ) @@ -188,7 +175,7 @@ app.get( req.url = `/${req.params.project_id}-${req.params.user_id}/` + OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`) - return staticOutputServer(req, res, next) + staticOutputServer(req, res, next) } ) @@ -208,7 +195,7 @@ app.get( req.url = `/${req.params.project_id}/` + OutputCacheManager.path(req.params.build_id, `/${req.params[0]}`) - return staticOutputServer(req, res, next) + staticOutputServer(req, res, next) } ) @@ -221,16 +208,13 @@ app.get( 'direct request for file in compile directory' ) req.url = `/${req.params.project_id}-${req.params.user_id}/${req.params[0]}` - return staticCompileServer(req, res, next) + staticCompileServer(req, res, next) } ) app.get('/project/:project_id/output/*', function (req, res, next) { logger.warn({ url: req.url }, 'direct request for file in compile directory') - if ( - (req.query != null ? req.query.build : undefined) != null && - req.query.build.match(OutputCacheManager.BUILD_REGEX) - ) { + if (req.query?.build?.match(OutputCacheManager.BUILD_REGEX)) { // for specific build get the path from the OutputCacheManager (e.g. .clsi/buildId) req.url = `/${req.params.project_id}/` + @@ -238,12 +222,12 @@ app.get('/project/:project_id/output/*', function (req, res, next) { } else { req.url = `/${req.params.project_id}/${req.params[0]}` } - return staticCompileServer(req, res, next) + staticCompileServer(req, res, next) }) app.get('/oops', function (req, res, next) { logger.error({ err: 'hello' }, 'test error') - return res.send('error\n') + res.send('error\n') }) app.get('/oops-internal', function (req, res, next) { @@ -274,7 +258,7 @@ function runSmokeTest() { const INTERVAL = 30 * 1000 if ( smokeTest.lastRunSuccessful() && - Date.now() - CompileController.lastSuccessfulCompile < INTERVAL / 2 + CompileController.timeSinceLastSuccessfulCompile() < INTERVAL / 2 ) { logger.debug('skipping smoke tests, got recent successful user compile') return setTimeout(runSmokeTest, INTERVAL / 2) @@ -301,13 +285,13 @@ app.get('/smoke_test_force', (req, res) => smokeTest.sendNewResult(res)) app.use(function (error, req, res, next) { if (error instanceof Errors.NotFoundError) { logger.debug({ err: error, url: req.url }, 'not found error') - return res.sendStatus(404) + res.sendStatus(404) } else if (error.code === 'EPIPE') { // inspect container returns EPIPE when shutting down - return res.sendStatus(503) // send 503 Unavailable response + res.sendStatus(503) // send 503 Unavailable response } else { logger.error({ err: error, url: req.url }, 'server error') - return res.sendStatus((error != null ? error.statusCode : undefined) || 500) + res.sendStatus(error.statusCode || 500) } }) @@ -323,7 +307,7 @@ const loadTcpServer = net.createServer(function (socket) { return } logger.err({ err }, 'error with socket on load check') - return socket.destroy() + socket.destroy() }) if (STATE === 'up' && Settings.internal.load_balancer_agent.report_load) { @@ -351,10 +335,10 @@ const loadTcpServer = net.createServer(function (socket) { // Ready will cancel the maint state. socket.write(`up, ready, ${freeLoadPercentage}%\n`, 'ASCII') } - return socket.end() + socket.end() } else { socket.write(`${STATE}\n`, 'ASCII') - return socket.end() + socket.end() } }) @@ -363,31 +347,23 @@ const loadHttpServer = express() loadHttpServer.post('/state/up', function (req, res, next) { STATE = 'up' logger.debug('getting message to set server to down') - return res.sendStatus(204) + res.sendStatus(204) }) loadHttpServer.post('/state/down', function (req, res, next) { STATE = 'down' logger.debug('getting message to set server to down') - return res.sendStatus(204) + res.sendStatus(204) }) loadHttpServer.post('/state/maint', function (req, res, next) { STATE = 'maint' logger.debug('getting message to set server to maint') - return res.sendStatus(204) + res.sendStatus(204) }) -const port = - __guard__( - Settings.internal != null ? Settings.internal.clsi : undefined, - x => x.port - ) || 3013 -const host = - __guard__( - Settings.internal != null ? Settings.internal.clsi : undefined, - x1 => x1.host - ) || 'localhost' +const port = Settings.internal?.clsi?.port || 3013 +const host = Settings.internal?.clsi?.host || 'localhost' const loadTcpPort = Settings.internal.load_balancer_agent.load_port const loadHttpPort = Settings.internal.load_balancer_agent.local_port @@ -415,23 +391,15 @@ if (!module.parent) { if (error != null) { throw error } - return logger.debug(`Load tcp agent listening on load port ${loadTcpPort}`) + logger.debug(`Load tcp agent listening on load port ${loadTcpPort}`) }) loadHttpServer.listen(loadHttpPort, host, function (error) { if (error != null) { throw error } - return logger.debug( - `Load http agent listening on load port ${loadHttpPort}` - ) + logger.debug(`Load http agent listening on load port ${loadHttpPort}`) }) } module.exports = app - -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined -} diff --git a/services/clsi/app/js/CompileController.js b/services/clsi/app/js/CompileController.js index 596158f7a7..dbcb928425 100644 --- a/services/clsi/app/js/CompileController.js +++ b/services/clsi/app/js/CompileController.js @@ -1,17 +1,3 @@ -/* eslint-disable - camelcase, - 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 - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let CompileController const RequestParser = require('./RequestParser') const CompileManager = require('./CompileManager') const Settings = require('@overleaf/settings') @@ -20,254 +6,248 @@ const ProjectPersistenceManager = require('./ProjectPersistenceManager') const logger = require('@overleaf/logger') const Errors = require('./Errors') -function isImageNameAllowed(imageName) { +let lastSuccessfulCompileTimestamp = 0 + +function timeSinceLastSuccessfulCompile() { + return Date.now() - lastSuccessfulCompileTimestamp +} + +function compile(req, res, next) { + const timer = new Metrics.Timer('compile-request') + RequestParser.parse(req.body, function (error, request) { + if (error) { + return next(error) + } + timer.opts = request.metricsOpts + request.project_id = req.params.project_id + if (req.params.user_id != null) { + request.user_id = req.params.user_id + } + ProjectPersistenceManager.markProjectAsJustAccessed( + request.project_id, + function (error) { + if (error) { + return next(error) + } + CompileManager.doCompileWithLock( + request, + function (error, outputFiles, stats, timings) { + let code, status + if (outputFiles == null) { + outputFiles = [] + } + if (error instanceof Errors.AlreadyCompilingError) { + code = 423 // Http 423 Locked + status = 'compile-in-progress' + } else if (error instanceof Errors.FilesOutOfSyncError) { + code = 409 // Http 409 Conflict + status = 'retry' + } else if (error?.code === 'EPIPE') { + // docker returns EPIPE when shutting down + code = 503 // send 503 Unavailable response + status = 'unavailable' + } else if (error?.terminated) { + status = 'terminated' + } else if (error?.validate) { + status = `validation-${error.validate}` + } else if (error?.timedout) { + status = 'timedout' + logger.debug( + { err: error, project_id: request.project_id }, + 'timeout running compile' + ) + } else if (error) { + status = 'error' + code = 500 + logger.warn( + { err: error, project_id: request.project_id }, + 'error running compile' + ) + } else { + let file + status = 'failure' + for (file of outputFiles) { + if (file.path === 'output.pdf' && file.size > 0) { + status = 'success' + lastSuccessfulCompileTimestamp = Date.now() + } + } + + if (status === 'failure') { + logger.warn( + { project_id: request.project_id, outputFiles }, + 'project failed to compile successfully, no output.pdf generated' + ) + } + + // log an error if any core files are found + for (file of outputFiles) { + if (file.path === 'core') { + logger.error( + { project_id: request.project_id, req, outputFiles }, + 'core file found in output' + ) + } + } + } + + if (error) { + outputFiles = error.outputFiles || [] + } + + timer.done() + res.status(code || 200).send({ + compile: { + status, + error: error?.message || error, + stats, + timings, + outputUrlPrefix: Settings.apis.clsi.outputUrlPrefix, + outputFiles: outputFiles.map(file => ({ + url: + `${Settings.apis.clsi.url}/project/${request.project_id}` + + (request.user_id != null + ? `/user/${request.user_id}` + : '') + + (file.build != null ? `/build/${file.build}` : '') + + `/output/${file.path}`, + ...file, + })), + }, + }) + } + ) + } + ) + }) +} + +function stopCompile(req, res, next) { + const { project_id: projectId, user_id: userId } = req.params + CompileManager.stopCompile(projectId, userId, function (error) { + if (error) { + return next(error) + } + res.sendStatus(204) + }) +} + +function clearCache(req, res, next) { + ProjectPersistenceManager.clearProject( + req.params.project_id, + req.params.user_id, + function (error) { + if (error) { + return next(error) + } + // No content + res.sendStatus(204) + } + ) +} + +function syncFromCode(req, res, next) { + const { file } = req.query + const line = parseInt(req.query.line, 10) + const column = parseInt(req.query.column, 10) + const { imageName } = req.query + const projectId = req.params.project_id + const userId = req.params.user_id + + if (imageName && !_isImageNameAllowed(imageName)) { + return res.status(400).send('invalid image') + } + + CompileManager.syncFromCode( + projectId, + userId, + file, + line, + column, + imageName, + function (error, pdfPositions) { + if (error) { + return next(error) + } + res.json({ + pdf: pdfPositions, + }) + } + ) +} + +function syncFromPdf(req, res, next) { + const page = parseInt(req.query.page, 10) + const h = parseFloat(req.query.h) + const v = parseFloat(req.query.v) + const { imageName } = req.query + const projectId = req.params.project_id + const userId = req.params.user_id + + if (imageName && !_isImageNameAllowed(imageName)) { + return res.status(400).send('invalid image') + } + CompileManager.syncFromPdf( + projectId, + userId, + page, + h, + v, + imageName, + function (error, codePositions) { + if (error) { + return next(error) + } + res.json({ + code: codePositions, + }) + } + ) +} + +function wordcount(req, res, next) { + const file = req.query.file || 'main.tex' + const projectId = req.params.project_id + const userId = req.params.user_id + const { image } = req.query + if (image && !_isImageNameAllowed(image)) { + return res.status(400).send('invalid image') + } + logger.debug({ image, file, projectId }, 'word count request') + + CompileManager.wordcount( + projectId, + userId, + file, + image, + function (error, result) { + if (error) { + return next(error) + } + res.json({ + texcount: result, + }) + } + ) +} + +function status(req, res, next) { + res.send('OK') +} + +function _isImageNameAllowed(imageName) { const ALLOWED_IMAGES = Settings.clsi && Settings.clsi.docker && Settings.clsi.docker.allowedImages return !ALLOWED_IMAGES || ALLOWED_IMAGES.includes(imageName) } -module.exports = CompileController = { - lastSuccessfulCompile: 0, - - compile(req, res, next) { - if (next == null) { - next = function () {} - } - const timer = new Metrics.Timer('compile-request') - return RequestParser.parse(req.body, function (error, request) { - if (error != null) { - return next(error) - } - timer.opts = request.metricsOpts - request.project_id = req.params.project_id - if (req.params.user_id != null) { - request.user_id = req.params.user_id - } - return ProjectPersistenceManager.markProjectAsJustAccessed( - request.project_id, - function (error) { - if (error != null) { - return next(error) - } - return CompileManager.doCompileWithLock( - request, - function (error, outputFiles, stats, timings) { - let code, status - if (outputFiles == null) { - outputFiles = [] - } - if (error instanceof Errors.AlreadyCompilingError) { - code = 423 // Http 423 Locked - status = 'compile-in-progress' - } else if (error instanceof Errors.FilesOutOfSyncError) { - code = 409 // Http 409 Conflict - status = 'retry' - } else if (error && error.code === 'EPIPE') { - // docker returns EPIPE when shutting down - code = 503 // send 503 Unavailable response - status = 'unavailable' - } else if (error != null ? error.terminated : undefined) { - status = 'terminated' - } else if (error != null ? error.validate : undefined) { - status = `validation-${error.validate}` - } else if (error != null ? error.timedout : undefined) { - status = 'timedout' - logger.debug( - { err: error, project_id: request.project_id }, - 'timeout running compile' - ) - } else if (error != null) { - status = 'error' - code = 500 - logger.warn( - { err: error, project_id: request.project_id }, - 'error running compile' - ) - } else { - let file - status = 'failure' - for (file of Array.from(outputFiles)) { - if (file.path === 'output.pdf' && file.size > 0) { - status = 'success' - CompileController.lastSuccessfulCompile = Date.now() - } - } - - if (status === 'failure') { - logger.warn( - { project_id: request.project_id, outputFiles }, - 'project failed to compile successfully, no output.pdf generated' - ) - } - - // log an error if any core files are found - for (file of Array.from(outputFiles)) { - if (file.path === 'core') { - logger.error( - { project_id: request.project_id, req, outputFiles }, - 'core file found in output' - ) - } - } - } - - if (error != null) { - outputFiles = error.outputFiles || [] - } - - timer.done() - return res.status(code || 200).send({ - compile: { - status, - error: (error != null ? error.message : undefined) || error, - stats, - timings, - outputUrlPrefix: Settings.apis.clsi.outputUrlPrefix, - outputFiles: outputFiles.map(file => { - return { - url: - `${Settings.apis.clsi.url}/project/${request.project_id}` + - (request.user_id != null - ? `/user/${request.user_id}` - : '') + - (file.build != null ? `/build/${file.build}` : '') + - `/output/${file.path}`, - ...file, - } - }), - }, - }) - } - ) - } - ) - }) - }, - - stopCompile(req, res, next) { - const { project_id, user_id } = req.params - return CompileManager.stopCompile(project_id, user_id, function (error) { - if (error != null) { - return next(error) - } - return res.sendStatus(204) - }) - }, - - clearCache(req, res, next) { - if (next == null) { - next = function () {} - } - return ProjectPersistenceManager.clearProject( - req.params.project_id, - req.params.user_id, - function (error) { - if (error != null) { - return next(error) - } - return res.sendStatus(204) - } - ) - }, // No content - - syncFromCode(req, res, next) { - if (next == null) { - next = function () {} - } - const { file } = req.query - const line = parseInt(req.query.line, 10) - const column = parseInt(req.query.column, 10) - const { imageName } = req.query - const { project_id } = req.params - const { user_id } = req.params - - if (imageName && !isImageNameAllowed(imageName)) { - return res.status(400).send('invalid image') - } - - return CompileManager.syncFromCode( - project_id, - user_id, - file, - line, - column, - imageName, - function (error, pdfPositions) { - if (error != null) { - return next(error) - } - return res.json({ - pdf: pdfPositions, - }) - } - ) - }, - - syncFromPdf(req, res, next) { - if (next == null) { - next = function () {} - } - const page = parseInt(req.query.page, 10) - const h = parseFloat(req.query.h) - const v = parseFloat(req.query.v) - const { imageName } = req.query - const { project_id } = req.params - const { user_id } = req.params - - if (imageName && !isImageNameAllowed(imageName)) { - return res.status(400).send('invalid image') - } - return CompileManager.syncFromPdf( - project_id, - user_id, - page, - h, - v, - imageName, - function (error, codePositions) { - if (error != null) { - return next(error) - } - return res.json({ - code: codePositions, - }) - } - ) - }, - - wordcount(req, res, next) { - if (next == null) { - next = function () {} - } - const file = req.query.file || 'main.tex' - const { project_id } = req.params - const { user_id } = req.params - const { image } = req.query - if (image && !isImageNameAllowed(image)) { - return res.status(400).send('invalid image') - } - logger.debug({ image, file, project_id }, 'word count request') - - return CompileManager.wordcount( - project_id, - user_id, - file, - image, - function (error, result) { - if (error != null) { - return next(error) - } - return res.json({ - texcount: result, - }) - } - ) - }, - - status(req, res, next) { - if (next == null) { - next = function () {} - } - return res.send('OK') - }, +module.exports = { + compile, + stopCompile, + clearCache, + syncFromCode, + syncFromPdf, + wordcount, + status, + timeSinceLastSuccessfulCompile, } diff --git a/services/clsi/app/js/LatexRunner.js b/services/clsi/app/js/LatexRunner.js index 68bf6a299f..662e5c9ac1 100644 --- a/services/clsi/app/js/LatexRunner.js +++ b/services/clsi/app/js/LatexRunner.js @@ -1,22 +1,6 @@ -/* eslint-disable - camelcase, - 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 - * 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 LatexRunner const Path = require('path') const Settings = require('@overleaf/settings') const logger = require('@overleaf/logger') -const Metrics = require('./Metrics') const CommandRunner = require('./CommandRunner') const fs = require('fs') @@ -28,209 +12,185 @@ const TIME_V_METRICS = Object.entries({ 'sys-time': /System time.*: (\d+.\d+)/m, }) -module.exports = LatexRunner = { - runLatex(project_id, options, callback) { - let command - if (callback == null) { - callback = function () {} - } - let { +function runLatex(projectId, options, callback) { + let command + let { + directory, + mainFile, + compiler, + timeout, + image, + environment, + flags, + compileGroup, + } = options + if (!compiler) { + compiler = 'pdflatex' + } + if (!timeout) { + timeout = 60000 + } // milliseconds + + logger.debug( + { directory, - mainFile, compiler, timeout, - image, + mainFile, environment, flags, compileGroup, - } = options - if (!compiler) { - compiler = 'pdflatex' - } - if (!timeout) { - timeout = 60000 - } // milliseconds + }, + 'starting compile' + ) - logger.debug( - { - directory, - compiler, - timeout, - mainFile, - environment, - flags, - compileGroup, - }, - 'starting compile' - ) + // We want to run latexmk on the tex file which we will automatically + // generate from the Rtex/Rmd/md file. + mainFile = mainFile.replace(/\.(Rtex|md|Rmd)$/, '.tex') - // We want to run latexmk on the tex file which we will automatically - // generate from the Rtex/Rmd/md file. - mainFile = mainFile.replace(/\.(Rtex|md|Rmd)$/, '.tex') + if (compiler === 'pdflatex') { + command = _pdflatexCommand(mainFile, flags) + } else if (compiler === 'latex') { + command = _latexCommand(mainFile, flags) + } else if (compiler === 'xelatex') { + command = _xelatexCommand(mainFile, flags) + } else if (compiler === 'lualatex') { + command = _lualatexCommand(mainFile, flags) + } else { + return callback(new Error(`unknown compiler: ${compiler}`)) + } - if (compiler === 'pdflatex') { - command = LatexRunner._pdflatexCommand(mainFile, flags) - } else if (compiler === 'latex') { - command = LatexRunner._latexCommand(mainFile, flags) - } else if (compiler === 'xelatex') { - command = LatexRunner._xelatexCommand(mainFile, flags) - } else if (compiler === 'lualatex') { - command = LatexRunner._lualatexCommand(mainFile, flags) - } else { - return callback(new Error(`unknown compiler: ${compiler}`)) - } + if (Settings.clsi?.strace) { + command = ['strace', '-o', 'strace', '-ff'].concat(command) + } - if (Settings.clsi != null ? Settings.clsi.strace : undefined) { - command = ['strace', '-o', 'strace', '-ff'].concat(command) - } + const id = `${projectId}` // record running project under this id - const id = `${project_id}` // record running project under this id - - return (ProcessTable[id] = CommandRunner.run( - project_id, - command, - directory, - image, - timeout, - environment, - compileGroup, - function (error, output) { - delete ProcessTable[id] - if (error != null) { - return callback(error) - } - const runs = - __guard__( - __guard__(output != null ? output.stderr : undefined, x1 => - x1.match(/^Run number \d+ of .*latex/gm) - ), - x => x.length - ) || 0 - const failed = - __guard__(output != null ? output.stdout : undefined, x2 => - x2.match(/^Latexmk: Errors/m) - ) != null - ? 1 - : 0 - // counters from latexmk output - const stats = {} - stats['latexmk-errors'] = failed - stats['latex-runs'] = runs - stats['latex-runs-with-errors'] = failed ? runs : 0 - stats[`latex-runs-${runs}`] = 1 - stats[`latex-runs-with-errors-${runs}`] = failed ? 1 : 0 - // timing information from /usr/bin/time - const timings = {} - const stderr = (output && output.stderr) || '' - if (stderr.includes('Command being timed:')) { - // Add metrics for runs with `$ time -v ...` - for (const [timing, matcher] of TIME_V_METRICS) { - const match = stderr.match(matcher) - if (match) { - timings[timing] = parseFloat(match[1]) - } + ProcessTable[id] = CommandRunner.run( + projectId, + command, + directory, + image, + timeout, + environment, + compileGroup, + function (error, output) { + delete ProcessTable[id] + if (error) { + return callback(error) + } + const runs = + output?.stderr?.match(/^Run number \d+ of .*latex/gm)?.length || 0 + const failed = output?.stdout?.match(/^Latexmk: Errors/m) != null ? 1 : 0 + // counters from latexmk output + const stats = {} + stats['latexmk-errors'] = failed + stats['latex-runs'] = runs + stats['latex-runs-with-errors'] = failed ? runs : 0 + stats[`latex-runs-${runs}`] = 1 + stats[`latex-runs-with-errors-${runs}`] = failed ? 1 : 0 + // timing information from /usr/bin/time + const timings = {} + const stderr = (output && output.stderr) || '' + if (stderr.includes('Command being timed:')) { + // Add metrics for runs with `$ time -v ...` + for (const [timing, matcher] of TIME_V_METRICS) { + const match = stderr.match(matcher) + if (match) { + timings[timing] = parseFloat(match[1]) } } - // record output files - LatexRunner.writeLogOutput(project_id, directory, output, () => { - return callback(error, output, stats, timings) - }) } - )) - }, - - writeLogOutput(project_id, directory, output, callback) { - if (!output) { - return callback() - } - // internal method for writing non-empty log files - function _writeFile(file, content, cb) { - if (content && content.length > 0) { - fs.writeFile(file, content, err => { - if (err) { - logger.error({ project_id, file }, 'error writing log file') // don't fail on error - } - cb() - }) - } else { - cb() - } - } - // write stdout and stderr, ignoring errors - _writeFile(Path.join(directory, 'output.stdout'), output.stdout, () => { - _writeFile(Path.join(directory, 'output.stderr'), output.stderr, () => { - callback() + // record output files + _writeLogOutput(projectId, directory, output, () => { + callback(error, output, stats, timings) }) - }) - }, - - killLatex(project_id, callback) { - if (callback == null) { - callback = function () {} } - const id = `${project_id}` - logger.debug({ id }, 'killing running compile') - if (ProcessTable[id] == null) { - logger.warn({ id }, 'no such project to kill') - return callback(null) + ) +} + +function _writeLogOutput(projectId, directory, output, callback) { + if (!output) { + return callback() + } + // internal method for writing non-empty log files + function _writeFile(file, content, cb) { + if (content && content.length > 0) { + fs.writeFile(file, content, err => { + if (err) { + logger.error({ projectId, file }, 'error writing log file') // don't fail on error + } + cb() + }) } else { - return CommandRunner.kill(ProcessTable[id], callback) + cb() } - }, - - _latexmkBaseCommand(flags) { - let args = [ - 'latexmk', - '-cd', - '-f', - '-jobname=output', - '-auxdir=$COMPILE_DIR', - '-outdir=$COMPILE_DIR', - '-synctex=1', - '-interaction=batchmode', - ] - if (flags) { - args = args.concat(flags) - } - return ( - __guard__( - Settings != null ? Settings.clsi : undefined, - x => x.latexmkCommandPrefix - ) || [] - ).concat(args) - }, - - _pdflatexCommand(mainFile, flags) { - return LatexRunner._latexmkBaseCommand(flags).concat([ - '-pdf', - Path.join('$COMPILE_DIR', mainFile), - ]) - }, - - _latexCommand(mainFile, flags) { - return LatexRunner._latexmkBaseCommand(flags).concat([ - '-pdfdvi', - Path.join('$COMPILE_DIR', mainFile), - ]) - }, - - _xelatexCommand(mainFile, flags) { - return LatexRunner._latexmkBaseCommand(flags).concat([ - '-xelatex', - Path.join('$COMPILE_DIR', mainFile), - ]) - }, - - _lualatexCommand(mainFile, flags) { - return LatexRunner._latexmkBaseCommand(flags).concat([ - '-lualatex', - Path.join('$COMPILE_DIR', mainFile), - ]) - }, + } + // write stdout and stderr, ignoring errors + _writeFile(Path.join(directory, 'output.stdout'), output.stdout, () => { + _writeFile(Path.join(directory, 'output.stderr'), output.stderr, () => { + callback() + }) + }) } -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined +function killLatex(projectId, callback) { + const id = `${projectId}` + logger.debug({ id }, 'killing running compile') + if (ProcessTable[id] == null) { + logger.warn({ id }, 'no such project to kill') + callback(null) + } else { + CommandRunner.kill(ProcessTable[id], callback) + } +} + +function _latexmkBaseCommand(flags) { + let args = [ + 'latexmk', + '-cd', + '-f', + '-jobname=output', + '-auxdir=$COMPILE_DIR', + '-outdir=$COMPILE_DIR', + '-synctex=1', + '-interaction=batchmode', + ] + if (flags) { + args = args.concat(flags) + } + return (Settings.clsi?.latexmkCommandPrefix || []).concat(args) +} + +function _pdflatexCommand(mainFile, flags) { + return _latexmkBaseCommand(flags).concat([ + '-pdf', + Path.join('$COMPILE_DIR', mainFile), + ]) +} + +function _latexCommand(mainFile, flags) { + return _latexmkBaseCommand(flags).concat([ + '-pdfdvi', + Path.join('$COMPILE_DIR', mainFile), + ]) +} + +function _xelatexCommand(mainFile, flags) { + return _latexmkBaseCommand(flags).concat([ + '-xelatex', + Path.join('$COMPILE_DIR', mainFile), + ]) +} + +function _lualatexCommand(mainFile, flags) { + return _latexmkBaseCommand(flags).concat([ + '-lualatex', + Path.join('$COMPILE_DIR', mainFile), + ]) +} + +module.exports = { + runLatex, + killLatex, } diff --git a/services/clsi/app/js/RequestParser.js b/services/clsi/app/js/RequestParser.js index 58a43f7f38..bc35d75060 100644 --- a/services/clsi/app/js/RequestParser.js +++ b/services/clsi/app/js/RequestParser.js @@ -1,254 +1,215 @@ -/* eslint-disable - no-control-regex, - no-throw-literal, - no-unused-vars, - no-useless-escape, - valid-typeof, -*/ -// 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 - * DS205: Consider reworking code to avoid use of IIFEs - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let RequestParser const settings = require('@overleaf/settings') -module.exports = RequestParser = { - VALID_COMPILERS: ['pdflatex', 'latex', 'xelatex', 'lualatex'], - MAX_TIMEOUT: 600, +const VALID_COMPILERS = ['pdflatex', 'latex', 'xelatex', 'lualatex'] +const MAX_TIMEOUT = 600 - parse(body, callback) { - let resource - if (callback == null) { - callback = function () {} +function parse(body, callback) { + let resource + const response = {} + + if (body.compile == null) { + return callback( + new Error('top level object should have a compile attribute') + ) + } + + const { compile } = body + if (!compile.options) { + compile.options = {} + } + + try { + response.metricsOpts = { + path: _parseAttribute('metricsPath', compile.options.metricsPath, { + default: '', + type: 'string', + }), + method: _parseAttribute('metricsMethod', compile.options.metricsMethod, { + default: '', + type: 'string', + }), } - const response = {} - - if (body.compile == null) { - return callback( - new Error('top level object should have a compile attribute') - ) - } - - const { compile } = body - if (!compile.options) { - compile.options = {} - } - - try { - response.metricsOpts = { - path: this._parseAttribute('metricsPath', compile.options.metricsPath, { - default: '', - type: 'string', - }), - method: this._parseAttribute( - 'metricsMethod', - compile.options.metricsMethod, - { - default: '', - type: 'string', - } - ), - } - response.compiler = this._parseAttribute( - 'compiler', - compile.options.compiler, - { - validValues: this.VALID_COMPILERS, - default: 'pdflatex', - type: 'string', - } - ) - response.enablePdfCaching = this._parseAttribute( - 'enablePdfCaching', - compile.options.enablePdfCaching, - { - default: false, - type: 'boolean', - } - ) - response.timeout = this._parseAttribute( - 'timeout', - compile.options.timeout, - { - default: RequestParser.MAX_TIMEOUT, - type: 'number', - } - ) - response.imageName = this._parseAttribute( - 'imageName', - compile.options.imageName, - { - type: 'string', - validValues: - settings.clsi && - settings.clsi.docker && - settings.clsi.docker.allowedImages, - } - ) - response.draft = this._parseAttribute('draft', compile.options.draft, { + response.compiler = _parseAttribute('compiler', compile.options.compiler, { + validValues: VALID_COMPILERS, + default: 'pdflatex', + type: 'string', + }) + response.enablePdfCaching = _parseAttribute( + 'enablePdfCaching', + compile.options.enablePdfCaching, + { default: false, type: 'boolean', - }) - response.check = this._parseAttribute('check', compile.options.check, { + } + ) + response.timeout = _parseAttribute('timeout', compile.options.timeout, { + default: MAX_TIMEOUT, + type: 'number', + }) + response.imageName = _parseAttribute( + 'imageName', + compile.options.imageName, + { type: 'string', - }) - response.flags = this._parseAttribute('flags', compile.options.flags, { - default: [], - type: 'object', - }) - if (settings.allowedCompileGroups) { - response.compileGroup = this._parseAttribute( - 'compileGroup', - compile.options.compileGroup, - { - validValues: settings.allowedCompileGroups, - default: '', - type: 'string', - } - ) + validValues: + settings.clsi && + settings.clsi.docker && + settings.clsi.docker.allowedImages, } - // The syncType specifies whether the request contains all - // resources (full) or only those resources to be updated - // in-place (incremental). - response.syncType = this._parseAttribute( - 'syncType', - compile.options.syncType, + ) + response.draft = _parseAttribute('draft', compile.options.draft, { + default: false, + type: 'boolean', + }) + response.check = _parseAttribute('check', compile.options.check, { + type: 'string', + }) + response.flags = _parseAttribute('flags', compile.options.flags, { + default: [], + type: 'object', + }) + if (settings.allowedCompileGroups) { + response.compileGroup = _parseAttribute( + 'compileGroup', + compile.options.compileGroup, { - validValues: ['full', 'incremental'], + validValues: settings.allowedCompileGroups, + default: '', type: 'string', } ) - - // The syncState is an identifier passed in with the request - // which has the property that it changes when any resource is - // added, deleted, moved or renamed. - // - // on syncType full the syncState identifier is passed in and - // stored - // - // on syncType incremental the syncState identifier must match - // the stored value - response.syncState = this._parseAttribute( - 'syncState', - compile.options.syncState, - { type: 'string' } - ) - - if (response.timeout > RequestParser.MAX_TIMEOUT) { - response.timeout = RequestParser.MAX_TIMEOUT - } - response.timeout = response.timeout * 1000 // milliseconds - - response.resources = (() => { - const result = [] - for (resource of Array.from(compile.resources || [])) { - result.push(this._parseResource(resource)) - } - return result - })() - - const rootResourcePath = this._parseAttribute( - 'rootResourcePath', - compile.rootResourcePath, - { - default: 'main.tex', - type: 'string', - } - ) - const originalRootResourcePath = rootResourcePath - const sanitizedRootResourcePath = - RequestParser._sanitizePath(rootResourcePath) - response.rootResourcePath = RequestParser._checkPath( - sanitizedRootResourcePath - ) - - for (resource of Array.from(response.resources)) { - if (resource.path === originalRootResourcePath) { - resource.path = sanitizedRootResourcePath - } - } - } catch (error1) { - const error = error1 - return callback(error) } + // The syncType specifies whether the request contains all + // resources (full) or only those resources to be updated + // in-place (incremental). + response.syncType = _parseAttribute('syncType', compile.options.syncType, { + validValues: ['full', 'incremental'], + type: 'string', + }) - return callback(null, response) - }, + // The syncState is an identifier passed in with the request + // which has the property that it changes when any resource is + // added, deleted, moved or renamed. + // + // on syncType full the syncState identifier is passed in and + // stored + // + // on syncType incremental the syncState identifier must match + // the stored value + response.syncState = _parseAttribute( + 'syncState', + compile.options.syncState, + { type: 'string' } + ) - _parseResource(resource) { - let modified - if (resource.path == null || typeof resource.path !== 'string') { - throw 'all resources should have a path attribute' + if (response.timeout > MAX_TIMEOUT) { + response.timeout = MAX_TIMEOUT } + response.timeout = response.timeout * 1000 // milliseconds - if (resource.modified != null) { - modified = new Date(resource.modified) - if (isNaN(modified.getTime())) { - throw `resource modified date could not be understood: ${resource.modified}` + response.resources = (compile.resources || []).map(resource => + _parseResource(resource) + ) + + const rootResourcePath = _parseAttribute( + 'rootResourcePath', + compile.rootResourcePath, + { + default: 'main.tex', + type: 'string', + } + ) + const originalRootResourcePath = rootResourcePath + const sanitizedRootResourcePath = _sanitizePath(rootResourcePath) + response.rootResourcePath = _checkPath(sanitizedRootResourcePath) + + for (resource of response.resources) { + if (resource.path === originalRootResourcePath) { + resource.path = sanitizedRootResourcePath } } + } catch (error1) { + const error = error1 + return callback(error) + } - if (resource.url == null && resource.content == null) { - throw 'all resources should have either a url or content attribute' - } - if (resource.content != null && typeof resource.content !== 'string') { - throw 'content attribute should be a string' - } - if (resource.url != null && typeof resource.url !== 'string') { - throw 'url attribute should be a string' - } + callback(null, response) +} - return { - path: resource.path, - modified, - url: resource.url, - content: resource.content, - } - }, +function _parseResource(resource) { + let modified + if (resource.path == null || typeof resource.path !== 'string') { + throw new Error('all resources should have a path attribute') + } - _parseAttribute(name, attribute, options) { - if (attribute != null) { - if (options.validValues != null) { - if (options.validValues.indexOf(attribute) === -1) { - throw `${name} attribute should be one of: ${options.validValues.join( + if (resource.modified != null) { + modified = new Date(resource.modified) + if (isNaN(modified.getTime())) { + throw new Error( + `resource modified date could not be understood: ${resource.modified}` + ) + } + } + + if (resource.url == null && resource.content == null) { + throw new Error( + 'all resources should have either a url or content attribute' + ) + } + if (resource.content != null && typeof resource.content !== 'string') { + throw new Error('content attribute should be a string') + } + if (resource.url != null && typeof resource.url !== 'string') { + throw new Error('url attribute should be a string') + } + + return { + path: resource.path, + modified, + url: resource.url, + content: resource.content, + } +} + +function _parseAttribute(name, attribute, options) { + if (attribute != null) { + if (options.validValues != null) { + if (options.validValues.indexOf(attribute) === -1) { + throw new Error( + `${name} attribute should be one of: ${options.validValues.join( ', ' )}` - } - } - if (options.type != null) { - if (typeof attribute !== options.type) { - throw `${name} attribute should be a ${options.type}` - } - } - } else { - if (options.default != null) { - return options.default + ) } } - return attribute - }, - - _sanitizePath(path) { - // See http://php.net/manual/en/function.escapeshellcmd.php - return path.replace( - /[\#\&\;\`\|\*\?\~\<\>\^\(\)\[\]\{\}\$\\\x0A\xFF\x00]/g, - '' - ) - }, - - _checkPath(path) { - // check that the request does not use a relative path - for (const dir of Array.from(path.split('/'))) { - if (dir === '..') { - throw 'relative path in root resource' + if (options.type != null) { + // eslint-disable-next-line valid-typeof + if (typeof attribute !== options.type) { + throw new Error(`${name} attribute should be a ${options.type}`) } } - return path - }, + } else { + if (options.default != null) { + return options.default + } + } + return attribute } + +function _sanitizePath(path) { + // See http://php.net/manual/en/function.escapeshellcmd.php + // eslint-disable-next-line no-control-regex + return path.replace(/[#&;`|*?~<>^()[\]{}$\\\x0A\xFF\x00]/g, '') +} + +function _checkPath(path) { + // check that the request does not use a relative path + for (const dir of Array.from(path.split('/'))) { + if (dir === '..') { + throw new Error('relative path in root resource') + } + } + return path +} + +module.exports = { parse, MAX_TIMEOUT } diff --git a/services/clsi/test/unit/js/CompileControllerTests.js b/services/clsi/test/unit/js/CompileControllerTests.js index 71f159e069..e8a583cf8e 100644 --- a/services/clsi/test/unit/js/CompileControllerTests.js +++ b/services/clsi/test/unit/js/CompileControllerTests.js @@ -1,14 +1,3 @@ -/* 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 - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') const { expect } = require('chai') @@ -16,7 +5,6 @@ const modulePath = require('path').join( __dirname, '../../../app/js/CompileController' ) -const tk = require('timekeeper') function tryImageNameValidation(method, imageNameField) { describe('when allowedImages is set', function () { @@ -80,7 +68,7 @@ describe('CompileController', function () { this.Settings.externalUrl = 'http://www.example.com' this.req = {} this.res = {} - return (this.next = sinon.stub()) + this.next = sinon.stub() }) describe('compile', function () { @@ -118,7 +106,7 @@ describe('CompileController', function () { this.stats = { foo: 1 } this.timings = { bar: 2 } this.res.status = sinon.stub().returnsThis() - return (this.res.send = sinon.stub()) + this.res.send = sinon.stub() }) describe('successfully', function () { @@ -126,30 +114,28 @@ describe('CompileController', function () { this.CompileManager.doCompileWithLock = sinon .stub() .yields(null, this.output_files, this.stats, this.timings) - return this.CompileController.compile(this.req, this.res) + this.CompileController.compile(this.req, this.res) }) it('should parse the request', function () { - return this.RequestParser.parse - .calledWith(this.req.body) - .should.equal(true) + this.RequestParser.parse.calledWith(this.req.body).should.equal(true) }) it('should run the compile for the specified project', function () { - return this.CompileManager.doCompileWithLock + this.CompileManager.doCompileWithLock .calledWith(this.request_with_project_id) .should.equal(true) }) it('should mark the project as accessed', function () { - return this.ProjectPersistenceManager.markProjectAsJustAccessed + this.ProjectPersistenceManager.markProjectAsJustAccessed .calledWith(this.project_id) .should.equal(true) }) - return it('should return the JSON response', function () { + it('should return the JSON response', function () { this.res.status.calledWith(200).should.equal(true) - return this.res.send + this.res.send .calledWith({ compile: { status: 'success', @@ -157,12 +143,10 @@ describe('CompileController', function () { stats: this.stats, timings: this.timings, outputUrlPrefix: '/zone/b', - outputFiles: this.output_files.map(file => { - return { - url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, - ...file, - } - }), + outputFiles: this.output_files.map(file => ({ + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, + ...file, + })), }, }) .should.equal(true) @@ -188,12 +172,10 @@ describe('CompileController', function () { stats: this.stats, timings: this.timings, outputUrlPrefix: '', - outputFiles: this.output_files.map(file => { - return { - url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, - ...file, - } - }), + outputFiles: this.output_files.map(file => ({ + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, + ...file, + })), }, }) .should.equal(true) @@ -230,12 +212,10 @@ describe('CompileController', function () { stats: this.stats, timings: this.timings, outputUrlPrefix: '/zone/b', - outputFiles: this.output_files.map(file => { - return { - url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, - ...file, - } - }), + outputFiles: this.output_files.map(file => ({ + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, + ...file, + })), }, }) .should.equal(true) @@ -273,12 +253,10 @@ describe('CompileController', function () { stats: this.stats, timings: this.timings, outputUrlPrefix: '/zone/b', - outputFiles: this.output_files.map(file => { - return { - url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, - ...file, - } - }), + outputFiles: this.output_files.map(file => ({ + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, + ...file, + })), }, }) .should.equal(true) @@ -290,12 +268,12 @@ describe('CompileController', function () { this.CompileManager.doCompileWithLock = sinon .stub() .callsArgWith(1, new Error((this.message = 'error message')), null) - return this.CompileController.compile(this.req, this.res) + this.CompileController.compile(this.req, this.res) }) - return it('should return the JSON response with the error', function () { + it('should return the JSON response with the error', function () { this.res.status.calledWith(500).should.equal(true) - return this.res.send + this.res.send .calledWith({ compile: { status: 'error', @@ -318,12 +296,12 @@ describe('CompileController', function () { this.CompileManager.doCompileWithLock = sinon .stub() .callsArgWith(1, this.error, null) - return this.CompileController.compile(this.req, this.res) + this.CompileController.compile(this.req, this.res) }) - return it('should return the JSON response with the timeout status', function () { + it('should return the JSON response with the timeout status', function () { this.res.status.calledWith(200).should.equal(true) - return this.res.send + this.res.send .calledWith({ compile: { status: 'timedout', @@ -339,17 +317,17 @@ describe('CompileController', function () { }) }) - return describe('when the request returns no output files', function () { + describe('when the request returns no output files', function () { beforeEach(function () { this.CompileManager.doCompileWithLock = sinon .stub() .callsArgWith(1, null, []) - return this.CompileController.compile(this.req, this.res) + this.CompileController.compile(this.req, this.res) }) - return it('should return the JSON response with the failure status', function () { + it('should return the JSON response with the failure status', function () { this.res.status.calledWith(200).should.equal(true) - return this.res.send + this.res.send .calledWith({ compile: { error: null, @@ -383,11 +361,11 @@ describe('CompileController', function () { this.CompileManager.syncFromCode = sinon .stub() .yields(null, (this.pdfPositions = ['mock-positions'])) - return this.CompileController.syncFromCode(this.req, this.res, this.next) + this.CompileController.syncFromCode(this.req, this.res, this.next) }) it('should find the corresponding location in the PDF', function () { - return this.CompileManager.syncFromCode + this.CompileManager.syncFromCode .calledWith( this.project_id, undefined, @@ -399,7 +377,7 @@ describe('CompileController', function () { }) it('should return the positions', function () { - return this.res.json + this.res.json .calledWith({ pdf: this.pdfPositions, }) @@ -426,17 +404,17 @@ describe('CompileController', function () { this.CompileManager.syncFromPdf = sinon .stub() .yields(null, (this.codePositions = ['mock-positions'])) - return this.CompileController.syncFromPdf(this.req, this.res, this.next) + this.CompileController.syncFromPdf(this.req, this.res, this.next) }) it('should find the corresponding location in the code', function () { - return this.CompileManager.syncFromPdf + this.CompileManager.syncFromPdf .calledWith(this.project_id, undefined, this.page, this.h, this.v) .should.equal(true) }) it('should return the positions', function () { - return this.res.json + this.res.json .calledWith({ code: this.codePositions, }) @@ -446,7 +424,7 @@ describe('CompileController', function () { tryImageNameValidation('syncFromPdf', 'imageName') }) - return describe('wordcount', function () { + describe('wordcount', function () { beforeEach(function () { this.file = 'main.tex' this.project_id = 'mock-project-id' @@ -464,14 +442,14 @@ describe('CompileController', function () { it('should return the word count of a file', function () { this.CompileController.wordcount(this.req, this.res, this.next) - return this.CompileManager.wordcount + this.CompileManager.wordcount .calledWith(this.project_id, undefined, this.file, this.image) .should.equal(true) }) it('should return the texcount info', function () { this.CompileController.wordcount(this.req, this.res, this.next) - return this.res.json + this.res.json .calledWith({ texcount: this.texcount, }) diff --git a/services/clsi/test/unit/js/LatexRunnerTests.js b/services/clsi/test/unit/js/LatexRunnerTests.js index 9a32d190dc..af494b107b 100644 --- a/services/clsi/test/unit/js/LatexRunnerTests.js +++ b/services/clsi/test/unit/js/LatexRunnerTests.js @@ -1,14 +1,3 @@ -/* 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 - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') const { expect } = require('chai') @@ -16,11 +5,9 @@ const modulePath = require('path').join( __dirname, '../../../app/js/LatexRunner' ) -const Path = require('path') describe('LatexRunner', function () { beforeEach(function () { - let Timer this.LatexRunner = SandboxedModule.require(modulePath, { requires: { '@overleaf/settings': (this.Settings = { @@ -29,9 +16,9 @@ describe('LatexRunner', function () { }, }), './Metrics': { - Timer: (Timer = class Timer { + Timer: class Timer { done() {} - }), + }, }, './CommandRunner': (this.CommandRunner = {}), fs: (this.fs = { @@ -47,20 +34,20 @@ describe('LatexRunner', function () { this.compileGroup = 'compile-group' this.callback = sinon.stub() this.project_id = 'project-id-123' - return (this.env = { foo: '123' }) + this.env = { foo: '123' } }) - return describe('runLatex', function () { + describe('runLatex', function () { beforeEach(function () { - return (this.CommandRunner.run = sinon.stub().callsArgWith(7, null, { + this.CommandRunner.run = sinon.stub().callsArgWith(7, null, { stdout: 'this is stdout', stderr: 'this is stderr', - })) + }) }) describe('normally', function () { beforeEach(function (done) { - return this.LatexRunner.runLatex( + this.LatexRunner.runLatex( this.project_id, { directory: this.directory, @@ -79,7 +66,7 @@ describe('LatexRunner', function () { }) it('should run the latex command', function () { - return this.CommandRunner.run + this.CommandRunner.run .calledWith( this.project_id, sinon.match.any, @@ -145,7 +132,7 @@ describe('LatexRunner', function () { describe('with an .Rtex main file', function () { beforeEach(function () { - return this.LatexRunner.runLatex( + this.LatexRunner.runLatex( this.project_id, { directory: this.directory, @@ -158,16 +145,16 @@ describe('LatexRunner', function () { ) }) - return it('should run the latex command on the equivalent .tex file', function () { + it('should run the latex command on the equivalent .tex file', function () { const command = this.CommandRunner.run.args[0][1] const mainFile = command.slice(-1)[0] - return mainFile.should.equal('$COMPILE_DIR/main-file.tex') + mainFile.should.equal('$COMPILE_DIR/main-file.tex') }) }) - return describe('with a flags option', function () { + describe('with a flags option', function () { beforeEach(function () { - return this.LatexRunner.runLatex( + this.LatexRunner.runLatex( this.project_id, { directory: this.directory, @@ -181,14 +168,14 @@ describe('LatexRunner', function () { ) }) - return it('should include the flags in the command', function () { + it('should include the flags in the command', function () { const command = this.CommandRunner.run.args[0][1] const flags = command.filter( arg => arg === '-shell-restricted' || arg === '-halt-on-error' ) flags.length.should.equal(2) flags[0].should.equal('-shell-restricted') - return flags[1].should.equal('-halt-on-error') + flags[1].should.equal('-halt-on-error') }) }) }) diff --git a/services/clsi/test/unit/js/RequestParserTests.js b/services/clsi/test/unit/js/RequestParserTests.js index 3e24d94d5d..1574a05c9d 100644 --- a/services/clsi/test/unit/js/RequestParserTests.js +++ b/services/clsi/test/unit/js/RequestParserTests.js @@ -1,13 +1,3 @@ -/* eslint-disable - no-return-assign, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') const { expect } = require('chai') @@ -37,23 +27,23 @@ describe('RequestParser', function () { resources: [], }, } - return (this.RequestParser = SandboxedModule.require(modulePath, { + this.RequestParser = SandboxedModule.require(modulePath, { requires: { '@overleaf/settings': (this.settings = {}), }, - })) + }) }) afterEach(function () { - return tk.reset() + tk.reset() }) describe('without a top level object', function () { beforeEach(function () { - return this.RequestParser.parse([], this.callback) + this.RequestParser.parse([], this.callback) }) - return it('should return an error', function () { + it('should return an error', function () { expect(this.callback).to.have.been.called expect(this.callback.args[0][0].message).to.equal( 'top level object should have a compile attribute' @@ -63,10 +53,10 @@ describe('RequestParser', function () { describe('without a compile attribute', function () { beforeEach(function () { - return this.RequestParser.parse({}, this.callback) + this.RequestParser.parse({}, this.callback) }) - return it('should return an error', function () { + it('should return an error', function () { expect(this.callback).to.have.been.called expect(this.callback.args[0][0].message).to.equal( 'top level object should have a compile attribute' @@ -77,14 +67,15 @@ describe('RequestParser', function () { describe('without a valid compiler', function () { beforeEach(function () { this.validRequest.compile.options.compiler = 'not-a-compiler' - return this.RequestParser.parse(this.validRequest, this.callback) + this.RequestParser.parse(this.validRequest, this.callback) }) - return it('should return an error', function () { - return this.callback - .calledWith( - 'compiler attribute should be one of: pdflatex, latex, xelatex, lualatex' - ) + it('should return an error', function () { + this.callback + .calledWithMatch({ + message: + 'compiler attribute should be one of: pdflatex, latex, xelatex, lualatex', + }) .should.equal(true) }) }) @@ -92,29 +83,29 @@ describe('RequestParser', function () { describe('without a compiler specified', function () { beforeEach(function (done) { delete this.validRequest.compile.options.compiler - return this.RequestParser.parse(this.validRequest, (error, data) => { + this.RequestParser.parse(this.validRequest, (error, data) => { if (error) return done(error) this.data = data done() }) }) - return it('should set the compiler to pdflatex by default', function () { - return this.data.compiler.should.equal('pdflatex') + it('should set the compiler to pdflatex by default', function () { + this.data.compiler.should.equal('pdflatex') }) }) describe('with imageName set', function () { beforeEach(function (done) { - return this.RequestParser.parse(this.validRequest, (error, data) => { + this.RequestParser.parse(this.validRequest, (error, data) => { if (error) return done(error) this.data = data done() }) }) - return it('should set the imageName', function () { - return this.data.imageName.should.equal('basicImageName/here:2017-1') + it('should set the imageName', function () { + this.data.imageName.should.equal('basicImageName/here:2017-1') }) }) @@ -163,46 +154,44 @@ describe('RequestParser', function () { describe('with flags set', function () { beforeEach(function (done) { this.validRequest.compile.options.flags = ['-file-line-error'] - return this.RequestParser.parse(this.validRequest, (error, data) => { + this.RequestParser.parse(this.validRequest, (error, data) => { if (error) return done(error) this.data = data done() }) }) - return it('should set the flags attribute', function () { - return expect(this.data.flags).to.deep.equal(['-file-line-error']) + it('should set the flags attribute', function () { + expect(this.data.flags).to.deep.equal(['-file-line-error']) }) }) describe('with flags not specified', function () { beforeEach(function (done) { - return this.RequestParser.parse(this.validRequest, (error, data) => { + this.RequestParser.parse(this.validRequest, (error, data) => { if (error) return done(error) this.data = data done() }) }) - return it('it should have an empty flags list', function () { - return expect(this.data.flags).to.deep.equal([]) + it('it should have an empty flags list', function () { + expect(this.data.flags).to.deep.equal([]) }) }) describe('without a timeout specified', function () { beforeEach(function (done) { delete this.validRequest.compile.options.timeout - return this.RequestParser.parse(this.validRequest, (error, data) => { + this.RequestParser.parse(this.validRequest, (error, data) => { if (error) return done(error) this.data = data done() }) }) - return it('should set the timeout to MAX_TIMEOUT', function () { - return this.data.timeout.should.equal( - this.RequestParser.MAX_TIMEOUT * 1000 - ) + it('should set the timeout to MAX_TIMEOUT', function () { + this.data.timeout.should.equal(this.RequestParser.MAX_TIMEOUT * 1000) }) }) @@ -210,31 +199,29 @@ describe('RequestParser', function () { beforeEach(function (done) { this.validRequest.compile.options.timeout = this.RequestParser.MAX_TIMEOUT + 1 - return this.RequestParser.parse(this.validRequest, (error, data) => { + this.RequestParser.parse(this.validRequest, (error, data) => { if (error) return done(error) this.data = data done() }) }) - return it('should set the timeout to MAX_TIMEOUT', function () { - return this.data.timeout.should.equal( - this.RequestParser.MAX_TIMEOUT * 1000 - ) + it('should set the timeout to MAX_TIMEOUT', function () { + this.data.timeout.should.equal(this.RequestParser.MAX_TIMEOUT * 1000) }) }) describe('with a timeout', function () { beforeEach(function (done) { - return this.RequestParser.parse(this.validRequest, (error, data) => { + this.RequestParser.parse(this.validRequest, (error, data) => { if (error) return done(error) this.data = data done() }) }) - return it('should set the timeout (in milliseconds)', function () { - return this.data.timeout.should.equal( + it('should set the timeout (in milliseconds)', function () { + this.data.timeout.should.equal( this.validRequest.compile.options.timeout * 1000 ) }) @@ -244,12 +231,14 @@ describe('RequestParser', function () { beforeEach(function () { delete this.validResource.path this.validRequest.compile.resources.push(this.validResource) - return this.RequestParser.parse(this.validRequest, this.callback) + this.RequestParser.parse(this.validRequest, this.callback) }) - return it('should return an error', function () { - return this.callback - .calledWith('all resources should have a path attribute') + it('should return an error', function () { + this.callback + .calledWithMatch({ + message: 'all resources should have a path attribute', + }) .should.equal(true) }) }) @@ -259,11 +248,11 @@ describe('RequestParser', function () { this.validResource.path = this.path = 'test.tex' this.validRequest.compile.resources.push(this.validResource) this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it('should return the path in the parsed response', function () { - return this.data.resources[0].path.should.equal(this.path) + it('should return the path in the parsed response', function () { + this.data.resources[0].path.should.equal(this.path) }) }) @@ -271,15 +260,16 @@ describe('RequestParser', function () { beforeEach(function () { this.validResource.modified = 'not-a-date' this.validRequest.compile.resources.push(this.validResource) - return this.RequestParser.parse(this.validRequest, this.callback) + this.RequestParser.parse(this.validRequest, this.callback) }) - return it('should return an error', function () { - return this.callback - .calledWith( - 'resource modified date could not be understood: ' + - this.validResource.modified - ) + it('should return an error', function () { + this.callback + .calledWithMatch({ + message: + 'resource modified date could not be understood: ' + + this.validResource.modified, + }) .should.equal(true) }) }) @@ -290,12 +280,12 @@ describe('RequestParser', function () { this.validResource.modified = this.date this.validRequest.compile.resources.push(this.validResource) this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it('should return the date as a Javascript Date object', function () { + it('should return the date as a Javascript Date object', function () { ;(this.data.resources[0].modified instanceof Date).should.equal(true) - return this.data.resources[0].modified + this.data.resources[0].modified .getTime() .should.equal(Date.parse(this.date)) }) @@ -306,14 +296,15 @@ describe('RequestParser', function () { delete this.validResource.url delete this.validResource.content this.validRequest.compile.resources.push(this.validResource) - return this.RequestParser.parse(this.validRequest, this.callback) + this.RequestParser.parse(this.validRequest, this.callback) }) - return it('should return an error', function () { - return this.callback - .calledWith( - 'all resources should have either a url or content attribute' - ) + it('should return an error', function () { + this.callback + .calledWithMatch({ + message: + 'all resources should have either a url or content attribute', + }) .should.equal(true) }) }) @@ -322,12 +313,12 @@ describe('RequestParser', function () { beforeEach(function () { this.validResource.content = [] this.validRequest.compile.resources.push(this.validResource) - return this.RequestParser.parse(this.validRequest, this.callback) + this.RequestParser.parse(this.validRequest, this.callback) }) - return it('should return an error', function () { - return this.callback - .calledWith('content attribute should be a string') + it('should return an error', function () { + this.callback + .calledWithMatch({ message: 'content attribute should be a string' }) .should.equal(true) }) }) @@ -336,12 +327,12 @@ describe('RequestParser', function () { beforeEach(function () { this.validResource.url = [] this.validRequest.compile.resources.push(this.validResource) - return this.RequestParser.parse(this.validRequest, this.callback) + this.RequestParser.parse(this.validRequest, this.callback) }) - return it('should return an error', function () { - return this.callback - .calledWith('url attribute should be a string') + it('should return an error', function () { + this.callback + .calledWithMatch({ message: 'url attribute should be a string' }) .should.equal(true) }) }) @@ -351,11 +342,11 @@ describe('RequestParser', function () { this.validResource.url = this.url = 'www.example.com' this.validRequest.compile.resources.push(this.validResource) this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it('should return the url in the parsed response', function () { - return this.data.resources[0].url.should.equal(this.url) + it('should return the url in the parsed response', function () { + this.data.resources[0].url.should.equal(this.url) }) }) @@ -364,11 +355,11 @@ describe('RequestParser', function () { this.validResource.content = this.content = 'Hello world' this.validRequest.compile.resources.push(this.validResource) this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it('should return the content in the parsed response', function () { - return this.data.resources[0].content.should.equal(this.content) + it('should return the content in the parsed response', function () { + this.data.resources[0].content.should.equal(this.content) }) }) @@ -376,11 +367,11 @@ describe('RequestParser', function () { beforeEach(function () { delete this.validRequest.compile.rootResourcePath this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it("should set the root resource path to 'main.tex' by default", function () { - return this.data.rootResourcePath.should.equal('main.tex') + it("should set the root resource path to 'main.tex' by default", function () { + this.data.rootResourcePath.should.equal('main.tex') }) }) @@ -388,23 +379,25 @@ describe('RequestParser', function () { beforeEach(function () { this.validRequest.compile.rootResourcePath = this.path = 'test.tex' this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it('should return the root resource path in the parsed response', function () { - return this.data.rootResourcePath.should.equal(this.path) + it('should return the root resource path in the parsed response', function () { + this.data.rootResourcePath.should.equal(this.path) }) }) describe('with a root resource path that is not a string', function () { beforeEach(function () { this.validRequest.compile.rootResourcePath = [] - return this.RequestParser.parse(this.validRequest, this.callback) + this.RequestParser.parse(this.validRequest, this.callback) }) - return it('should return an error', function () { - return this.callback - .calledWith('rootResourcePath attribute should be a string') + it('should return an error', function () { + this.callback + .calledWithMatch({ + message: 'rootResourcePath attribute should be a string', + }) .should.equal(true) }) }) @@ -420,15 +413,15 @@ describe('RequestParser', function () { content: 'Hello world', }) this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) it('should return the escaped resource', function () { - return this.data.rootResourcePath.should.equal(this.goodPath) + this.data.rootResourcePath.should.equal(this.goodPath) }) - return it('should also escape the resource path', function () { - return this.data.resources[0].path.should.equal(this.goodPath) + it('should also escape the resource path', function () { + this.data.resources[0].path.should.equal(this.goodPath) }) }) @@ -436,12 +429,12 @@ describe('RequestParser', function () { beforeEach(function () { this.validRequest.compile.rootResourcePath = 'foo/../../bar.tex' this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it('should return an error', function () { - return this.callback - .calledWith('relative path in root resource') + it('should return an error', function () { + this.callback + .calledWithMatch({ message: 'relative path in root resource' }) .should.equal(true) }) }) @@ -450,26 +443,28 @@ describe('RequestParser', function () { beforeEach(function () { this.validRequest.compile.rootResourcePath = 'foo/#../bar.tex' this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it('should return an error', function () { - return this.callback - .calledWith('relative path in root resource') + it('should return an error', function () { + this.callback + .calledWithMatch({ message: 'relative path in root resource' }) .should.equal(true) }) }) - return describe('with an unknown syncType', function () { + describe('with an unknown syncType', function () { beforeEach(function () { this.validRequest.compile.options.syncType = 'unexpected' this.RequestParser.parse(this.validRequest, this.callback) - return (this.data = this.callback.args[0][1]) + this.data = this.callback.args[0][1] }) - return it('should return an error', function () { - return this.callback - .calledWith('syncType attribute should be one of: full, incremental') + it('should return an error', function () { + this.callback + .calledWithMatch({ + message: 'syncType attribute should be one of: full, incremental', + }) .should.equal(true) }) })