diff --git a/package-lock.json b/package-lock.json index e4e7cb9963..eccc5c9a3a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -37374,7 +37374,6 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/tough-cookie/-/tough-cookie-4.0.0.tgz", "integrity": "sha512-tHdtEpQCMrc1YLrMaqXXcj6AxhYi/xgit6mZu1+EDWUn+qhUf8wMQoFIy9NXuq23zAwtcB0t/MjACGR18pcRbg==", - "dev": true, "dependencies": { "psl": "^1.1.33", "punycode": "^2.1.1", @@ -37388,7 +37387,6 @@ "version": "0.1.2", "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", "integrity": "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==", - "dev": true, "engines": { "node": ">= 4.0.0" } @@ -41595,6 +41593,7 @@ "rimraf": "2.2.6", "sanitize-html": "^2.8.1", "scroll-into-view-if-needed": "^2.2.25", + "tough-cookie": "^4.0.0", "tsscmp": "^1.0.6", "underscore": "^1.13.1", "utf-8-validate": "^5.0.2", @@ -41693,7 +41692,6 @@ "terser-webpack-plugin": "^5.3.9", "timekeeper": "^2.2.0", "to-string-loader": "^1.2.0", - "tough-cookie": "^4.0.0", "typescript": "^5.0.4", "val-loader": "^5.0.1", "webpack": "^5.83.1", @@ -74131,7 +74129,6 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/tough-cookie/-/tough-cookie-4.0.0.tgz", "integrity": "sha512-tHdtEpQCMrc1YLrMaqXXcj6AxhYi/xgit6mZu1+EDWUn+qhUf8wMQoFIy9NXuq23zAwtcB0t/MjACGR18pcRbg==", - "dev": true, "requires": { "psl": "^1.1.33", "punycode": "^2.1.1", @@ -74141,8 +74138,7 @@ "universalify": { "version": "0.1.2", "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", - "integrity": "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==", - "dev": true + "integrity": "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==" } } }, diff --git a/services/web/app/src/Features/Compile/ClsiCookieManager.js b/services/web/app/src/Features/Compile/ClsiCookieManager.js index 701fa15acf..8fdb45b40b 100644 --- a/services/web/app/src/Features/Compile/ClsiCookieManager.js +++ b/services/web/app/src/Features/Compile/ClsiCookieManager.js @@ -6,6 +6,7 @@ const RedisWrapper = require('../../infrastructure/RedisWrapper') const Cookie = require('cookie') const logger = require('@overleaf/logger') const Metrics = require('@overleaf/metrics') +const { promisifyAll } = require('../../util/promises') const clsiCookiesEnabled = (Settings.clsiCookie?.key ?? '') !== '' @@ -16,7 +17,7 @@ if (Settings.redis.clsi_cookie_secondary != null) { } module.exports = function (backendGroup) { - return { + const cookieManager = { buildKey(projectId, userId) { if (backendGroup != null) { return `clsiserver:${backendGroup}:${projectId}:${userId}` @@ -25,7 +26,7 @@ module.exports = function (backendGroup) { } }, - _getServerId( + getServerId( projectId, userId, compileGroup, @@ -69,14 +70,18 @@ module.exports = function (backendGroup) { }) return callback(err) } + if (!clsiCookiesEnabled) { + return callback() + } + const serverId = this._parseServerIdFromResponse(res) this.setServerId( projectId, userId, compileGroup, compileBackendClass, - res, + serverId, null, - function (err, serverId) { + function (err) { if (err) { logger.warn( { err, projectId }, @@ -128,20 +133,19 @@ module.exports = function (backendGroup) { userId, compileGroup, compileBackendClass, - response, + serverId, previous, callback ) { if (!clsiCookiesEnabled) { return callback() } - const serverId = this._parseServerIdFromResponse(response) if (serverId == null) { // We don't get a cookie back if it hasn't changed return rclient.expire( this.buildKey(projectId, userId), this._getTTLInSeconds(previous), - err => callback(err, undefined) + err => callback(err) ) } if (!previous) { @@ -164,7 +168,7 @@ module.exports = function (backendGroup) { ) } this._setServerIdInRedis(rclient, projectId, userId, serverId, err => - callback(err, serverId) + callback(err) ) }, @@ -181,7 +185,20 @@ module.exports = function (backendGroup) { if (!clsiCookiesEnabled) { return callback() } - rclient.del(this.buildKey(projectId, userId), callback) + rclient.del(this.buildKey(projectId, userId), err => { + if (err) { + // redis errors need wrapping as the instance may be shared + return callback( + new OError( + 'Failed to clear clsi persistence', + { projectId, userId }, + err + ) + ) + } else { + return callback() + } + }) }, getCookieJar( @@ -194,7 +211,7 @@ module.exports = function (backendGroup) { if (!clsiCookiesEnabled) { return callback(null, request.jar(), undefined) } - this._getServerId( + this.getServerId( projectId, userId, compileGroup, @@ -216,4 +233,15 @@ module.exports = function (backendGroup) { ) }, } + cookieManager.promises = promisifyAll(cookieManager, { + without: [ + '_parseServerIdFromResponse', + 'checkIsLoadSheddingEvent', + '_getTTLInSeconds', + ], + multiResult: { + getCookieJar: ['jar', 'clsiServerId'], + }, + }) + return cookieManager } diff --git a/services/web/app/src/Features/Compile/ClsiFormatChecker.js b/services/web/app/src/Features/Compile/ClsiFormatChecker.js index cc91a2a32b..235436e4f6 100644 --- a/services/web/app/src/Features/Compile/ClsiFormatChecker.js +++ b/services/web/app/src/Features/Compile/ClsiFormatChecker.js @@ -13,6 +13,7 @@ let ClsiFormatChecker const _ = require('lodash') const async = require('async') const settings = require('@overleaf/settings') +const { promisifyAll } = require('../../util/promises') module.exports = ClsiFormatChecker = { checkRecoursesForProblems(resources, callback) { @@ -84,3 +85,5 @@ module.exports = ClsiFormatChecker = { } }, } + +module.exports.promises = promisifyAll(module.exports) diff --git a/services/web/app/src/Features/Compile/ClsiManager.js b/services/web/app/src/Features/Compile/ClsiManager.js index fbd579be78..252dd7f566 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.js +++ b/services/web/app/src/Features/Compile/ClsiManager.js @@ -1,12 +1,12 @@ -const async = require('async') +const { callbackify } = require('util') +const { callbackifyMultiResult } = require('../../util/promises') +const fetch = require('node-fetch') const Settings = require('@overleaf/settings') -const request = require('request') const ProjectGetter = require('../Project/ProjectGetter') const ProjectEntityHandler = require('../Project/ProjectEntityHandler') const logger = require('@overleaf/logger') -const { URL, URLSearchParams } = require('url') const OError = require('@overleaf/o-error') - +const { Cookie } = require('tough-cookie') const ClsiCookieManager = require('./ClsiCookieManager')( Settings.apis.clsi?.backendGroupName ) @@ -21,6 +21,8 @@ const Metrics = require('@overleaf/metrics') const Errors = require('../Errors/Errors') const VALID_COMPILERS = ['pdflatex', 'latex', 'xelatex', 'lualatex'] +const OUTPUT_FILE_TIMEOUT_MS = 60000 +const CLSI_COOKIES_ENABLED = (Settings.clsiCookie?.key ?? '') !== '' function collectMetricsOnBlgFiles(outputFiles) { let topLevel = 0 @@ -38,481 +40,375 @@ function collectMetricsOnBlgFiles(outputFiles) { Metrics.count('blg_output_file', nested, 1, { path: 'nested' }) } -function sendRequest(projectId, userId, options, callback) { +async function sendRequest(projectId, userId, options) { if (options == null) { options = {} } - sendRequestOnce(projectId, userId, options, (err, status, ...result) => { - if (err != null) { - return callback(err) - } - if (status === 'conflict') { - // Try again, with a full compile - return sendRequestOnce( - projectId, - userId, - { ...options, syncType: 'full' }, - callback - ) - } else if (status === 'unavailable') { - return sendRequestOnce( - projectId, - userId, - { ...options, syncType: 'full', forceNewClsiServer: true }, - callback - ) - } - callback(null, status, ...result) - }) + let result = await sendRequestOnce(projectId, userId, options) + if (result.status === 'conflict') { + // Try again, with a full compile + result = await sendRequestOnce(projectId, userId, { + ...options, + syncType: 'full', + }) + } else if (result.status === 'unavailable') { + result = await sendRequestOnce(projectId, userId, { + ...options, + syncType: 'full', + forceNewClsiServer: true, + }) + } + return result } -function sendRequestOnce(projectId, userId, options, callback) { - if (options == null) { - options = {} - } - _buildRequest(projectId, options, (err, req) => { - if (err != null) { - if (err.message === 'no main file specified') { - return callback(null, 'validation-problems', null, null, { - mainFile: err.message, - }) - } else { - return callback( - OError.tag(err, 'Could not build request to CLSI', { - projectId, - options, - }) - ) +async function sendRequestOnce(projectId, userId, options) { + let req + try { + req = await _buildRequest(projectId, options) + } catch (err) { + if (err.message === 'no main file specified') { + return { + status: 'validation-problems', + validationProblems: { mainFile: err.message }, } + } else { + throw OError.tag(err, 'Could not build request to CLSI', { + projectId, + options, + }) } - _sendBuiltRequest( - projectId, - userId, - req, - options, - (err, status, ...result) => { - if (err != null) { - return callback( - OError.tag(err, 'CLSI compile failed', { projectId, userId }) - ) - } - callback(null, status, ...result) - } - ) - }) + } + return await _sendBuiltRequest(projectId, userId, req, options) } // for public API requests where there is no project id -function sendExternalRequest(submissionId, clsiRequest, options, callback) { +async function sendExternalRequest(submissionId, clsiRequest, options) { if (options == null) { options = {} } - _sendBuiltRequest( - submissionId, - null, - clsiRequest, - options, - (err, status, ...result) => { - if (err != null) { - return callback( - OError.tag(err, 'CLSI compile failed', { - submissionId, - options, - }) - ) - } - callback(null, status, ...result) - } - ) + return await _sendBuiltRequest(submissionId, null, clsiRequest, options) } -function stopCompile(projectId, userId, options, callback) { +async function stopCompile(projectId, userId, options) { if (options == null) { options = {} } const { compileBackendClass, compileGroup } = options - const compilerUrl = _getCompilerUrl( + const url = _getCompilerUrl( compileBackendClass, compileGroup, projectId, userId, 'compile/stop' ) - const opts = { - url: compilerUrl, - method: 'POST', - } - _makeRequest( + const opts = { method: 'POST' } + await _makeRequest( projectId, userId, compileGroup, compileBackendClass, - opts, - callback + url, + opts ) } -function deleteAuxFiles(projectId, userId, options, clsiserverid, callback) { +async function deleteAuxFiles(projectId, userId, options, clsiserverid) { if (options == null) { options = {} } const { compileBackendClass, compileGroup } = options - const compilerUrl = _getCompilerUrl( + const url = _getCompilerUrl( compileBackendClass, compileGroup, projectId, userId ) const opts = { - url: compilerUrl, method: 'DELETE', } - _makeRequestWithClsiServerId( - projectId, - userId, - compileGroup, - compileBackendClass, - opts, - clsiserverid, - clsiErr => { - // always clear the project state from the docupdater, even if there - // was a problem with the request to the clsi - DocumentUpdaterHandler.clearProjectState(projectId, docUpdaterErr => { - ClsiCookieManager.clearServerId(projectId, userId, redisError => { - if (clsiErr) { - return callback( - OError.tag(clsiErr, 'Failed to delete aux files', { projectId }) - ) - } - if (docUpdaterErr) { - return callback( - OError.tag( - docUpdaterErr, - 'Failed to clear project state in doc updater', - { projectId } - ) - ) - } - if (redisError) { - // redis errors need wrapping as the instance may be shared - return callback( - OError( - 'Failed to clear clsi persistence', - { projectId }, - redisError - ) - ) - } - callback() - }) - }) - } - ) -} -function _sendBuiltRequest(projectId, userId, req, options, callback) { - if (options == null) { - options = {} - } - if (options.forceNewClsiServer) { - // Clear clsi cookie, then try again - return ClsiCookieManager.clearServerId(projectId, userId, err => { - if (err) { - return callback(err) - } - options.forceNewClsiServer = false // backend has now been reset - _sendBuiltRequest(projectId, userId, req, options, callback) - }) - } - ClsiFormatChecker.checkRecoursesForProblems( - req.compile != null ? req.compile.resources : undefined, - (err, validationProblems) => { - if (err != null) { - return callback( - OError.tag( - err, - 'could not check resources for potential problems before sending to clsi' - ) - ) - } - if (validationProblems != null) { - logger.debug( - { projectId, validationProblems }, - 'problems with users latex before compile was attempted' - ) - return callback( - null, - 'validation-problems', - null, - null, - validationProblems - ) - } - _postToClsi( - projectId, - userId, - req, - options.compileBackendClass, - options.compileGroup, - (err, response, clsiServerId) => { - if (err != null) { - return callback( - OError.tag(err, 'error sending request to clsi', { - projectId, - userId, - }) - ) - } - const outputFiles = _parseOutputFiles( - projectId, - response && response.compile && response.compile.outputFiles - ) - collectMetricsOnBlgFiles(outputFiles) - const compile = (response && response.compile) || {} - const status = compile.status - const stats = compile.stats - const timings = compile.timings - const outputUrlPrefix = compile.outputUrlPrefix - const validationProblems = undefined - callback( - null, - status, - outputFiles, - clsiServerId, - validationProblems, - stats, - timings, - outputUrlPrefix - ) - } - ) - } - ) -} - -function _makeRequestWithClsiServerId( - projectId, - userId, - compileGroup, - compileBackendClass, - opts, - clsiserverid, - callback -) { - if (clsiserverid) { - // ignore cookies and newBackend, go straight to the clsi node - opts.qs = Object.assign( - { compileGroup, compileBackendClass, clsiserverid }, - opts.qs - ) - request(opts, (err, response, body) => { - if (err) { - return callback( - OError.tag(err, 'error making request to CLSI', { projectId }) - ) - } - callback(null, response, body) - }) - } else { - _makeRequest( + try { + await _makeRequestWithClsiServerId( projectId, userId, compileGroup, compileBackendClass, + url, opts, - callback + clsiserverid + ) + } finally { + // always clear the project state from the docupdater, even if there + // was a problem with the request to the clsi + try { + await DocumentUpdaterHandler.promises.clearProjectState(projectId) + } finally { + await ClsiCookieManager.promises.clearServerId(projectId, userId) + } + } +} + +async function _sendBuiltRequest(projectId, userId, req, options, callback) { + if (options.forceNewClsiServer) { + await ClsiCookieManager.promises.clearServerId(projectId, userId) + } + const validationProblems = + await ClsiFormatChecker.promises.checkRecoursesForProblems( + req.compile?.resources + ) + if (validationProblems != null) { + logger.debug( + { projectId, validationProblems }, + 'problems with users latex before compile was attempted' + ) + return { + status: 'validation-problems', + validationProblems, + } + } + + const { response, clsiServerId } = await _postToClsi( + projectId, + userId, + req, + options.compileBackendClass, + options.compileGroup + ) + + const outputFiles = _parseOutputFiles( + projectId, + response && response.compile && response.compile.outputFiles + ) + collectMetricsOnBlgFiles(outputFiles) + const compile = response?.compile || {} + return { + status: compile.status, + outputFiles, + clsiServerId, + stats: compile.stats, + timings: compile.timings, + outputUrlPrefix: compile.outputUrlPrefix, + } +} + +async function _makeRequestWithClsiServerId( + projectId, + userId, + compileGroup, + compileBackendClass, + url, + opts, + clsiserverid +) { + if (clsiserverid) { + // ignore cookies and newBackend, go straight to the clsi node + url.searchParams.set('compileGroup', compileGroup) + url.searchParams.set('compileBackendClass', compileBackendClass) + url.searchParams.set('clsiserverid', clsiserverid) + + let response + try { + response = await fetch(url, opts) + } catch (err) { + throw OError.tag(err, 'error making request to CLSI', { + userId, + projectId, + }) + } + + let body + try { + body = await response.json() + } catch (err) { + // some responses are empty. Ignore JSON parsing errors. + } + return { response, body } + } else { + return await _makeRequest( + projectId, + userId, + compileGroup, + compileBackendClass, + url, + opts ) } } -function _makeRequest( +async function _makeRequest( projectId, userId, compileGroup, compileBackendClass, - opts, - callback + url, + opts ) { - async.series( - { - currentBackend(cb) { - const startTime = new Date() - ClsiCookieManager.getCookieJar( - projectId, - userId, - compileGroup, - compileBackendClass, - (err, jar, clsiServerId) => { - if (err != null) { - return callback( - OError.tag(err, 'error getting cookie jar for CLSI request', { - projectId, - }) - ) - } - opts.jar = jar - const timer = new Metrics.Timer('compile.currentBackend') - request(opts, (err, response, body) => { - if (err != null) { - return callback( - OError.tag(err, 'error making request to CLSI', { - projectId, - }) - ) - } - timer.done() - Metrics.inc( - `compile.currentBackend.response.${response.statusCode}` - ) - ClsiCookieManager.setServerId( - projectId, - userId, - compileGroup, - compileBackendClass, - response, - clsiServerId, - (err, newClsiServerId) => { - if (err != null) { - callback( - OError.tag(err, 'error setting server id', { - projectId, - }) - ) - } else { - // return as soon as the standard compile has returned - callback( - null, - response, - body, - newClsiServerId || clsiServerId - ) - } - cb(err, { - response, - body, - finishTime: new Date() - startTime, - }) - } - ) - }) - } - ) - }, - newBackend(cb) { - const startTime = new Date() - _makeNewBackendRequest( - projectId, - userId, - compileGroup, - compileBackendClass, - opts, - (err, response, body) => { - if (err != null) { - logger.warn({ err }, 'Error making request to new CLSI backend') - } - if (response != null) { - Metrics.inc(`compile.newBackend.response.${response.statusCode}`) - } - cb(err, { - response, - body, - finishTime: new Date() - startTime, - }) - } - ) - }, - }, - (err, results) => { - if (err != null) { - // This was handled higher up - return - } - if (results.newBackend != null && results.newBackend.response != null) { - const currentStatusCode = results.currentBackend.response.statusCode - const newStatusCode = results.newBackend.response.statusCode - const statusCodeSame = newStatusCode === currentStatusCode - const currentCompileTime = results.currentBackend.finishTime - const newBackendCompileTime = results.newBackend.finishTime || 0 - const timeDifference = newBackendCompileTime - currentCompileTime - logger.debug( - { - statusCodeSame, - timeDifference, - currentCompileTime, - newBackendCompileTime, - projectId, - }, - 'both clsi requests returned' - ) - } - } + const currentBackendStartTime = new Date() + const clsiServerId = await ClsiCookieManager.promises.getServerId( + projectId, + userId, + compileGroup, + compileBackendClass ) -} + opts.headers = { + Accept: 'application/json', + 'Content-Type': 'application/json', + } -function _makeNewBackendRequest( - projectId, - userId, - compileGroup, - compileBackendClass, - baseOpts, - callback -) { - if (Settings.apis.clsi_new == null || Settings.apis.clsi_new.url == null) { - return callback() + if (CLSI_COOKIES_ENABLED) { + const cookie = new Cookie({ + key: Settings.clsiCookie.key, + value: clsiServerId, + }) + opts.headers.Cookie = cookie.cookieString() } - const opts = { - ...baseOpts, - url: baseOpts.url.replace( - Settings.apis.clsi.url, - Settings.apis.clsi_new.url - ), + + const timer = new Metrics.Timer('compile.currentBackend') + + let currentBackendResponse + try { + currentBackendResponse = await fetch(url, opts) + } catch (err) { + throw OError.tag(err, 'error making request to CLSI', { + projectId, + userId, + }) } - NewBackendCloudClsiCookieManager.getCookieJar( + + Metrics.inc( + `compile.currentBackend.response.${currentBackendResponse.status}` + ) + + let currentBackendBody + try { + currentBackendBody = await currentBackendResponse.json() + } catch (err) { + // some responses are empty. Ignore JSON parsing errors + } + timer.done() + let newClsiServerId + if (CLSI_COOKIES_ENABLED) { + newClsiServerId = _getClsiServerIdFromResponse(currentBackendResponse) + await ClsiCookieManager.promises.setServerId( + projectId, + userId, + compileGroup, + compileBackendClass, + newClsiServerId, + clsiServerId + ) + } + const currentCompileTime = new Date() - currentBackendStartTime + + // Start new backend request in the background + const newBackendStartTime = new Date() + _makeNewBackendRequest( projectId, userId, compileGroup, compileBackendClass, - (err, jar, clsiServerId) => { - if (err != null) { - return callback( - OError.tag(err, 'error getting cookie jar for CLSI request', { - projectId, - }) - ) - } - opts.jar = jar - const timer = new Metrics.Timer('compile.newBackend') - request(opts, (err, response, body) => { - timer.done() - if (err != null) { - return callback( - OError.tag(err, 'error making request to new CLSI', { - projectId, - opts, - }) - ) - } - NewBackendCloudClsiCookieManager.setServerId( - projectId, - userId, - compileGroup, - compileBackendClass, - response, - clsiServerId, - err => { - if (err != null) { - return callback( - OError.tag(err, 'error setting server id on new backend', { - projectId, - }) - ) - } - callback(null, response, body) - } - ) - }) - } + url, + opts ) + .then(result => { + if (result == null) { + return + } + const { response: newBackendResponse } = result + Metrics.inc(`compile.newBackend.response.${newBackendResponse.status}`) + const newBackendCompileTime = new Date() - newBackendStartTime + const currentStatusCode = currentBackendResponse.status + const newStatusCode = newBackendResponse.status + const statusCodeSame = newStatusCode === currentStatusCode + const timeDifference = newBackendCompileTime - currentCompileTime + logger.debug( + { + statusCodeSame, + timeDifference, + currentCompileTime, + newBackendCompileTime, + projectId, + }, + 'both clsi requests returned' + ) + }) + .catch(err => { + logger.warn({ err }, 'Error making request to new CLSI backend') + }) + + return { + response: currentBackendResponse, + body: currentBackendBody, + clsiServerId: newClsiServerId || clsiServerId, + } +} + +async function _makeNewBackendRequest( + projectId, + userId, + compileGroup, + compileBackendClass, + url, + opts +) { + if (Settings.apis.clsi_new?.url == null) { + return null + } + url = url + .toString() + .replace(Settings.apis.clsi.url, Settings.apis.clsi_new.url) + + const clsiServerId = + await NewBackendCloudClsiCookieManager.promises.getServerId( + projectId, + userId, + compileGroup, + compileBackendClass + ) + opts.headers = { + Accept: 'application/json', + 'Content-Type': 'application/json', + } + + if (CLSI_COOKIES_ENABLED) { + const cookie = new Cookie({ + key: Settings.clsiCookie.key, + value: clsiServerId, + }) + opts.headers.Cookie = cookie.cookieString() + } + + const timer = new Metrics.Timer('compile.newBackend') + + let response + try { + response = await fetch(url, opts) + } catch (err) { + throw OError.tag(err, 'error making request to new CLSI', { + userId, + projectId, + }) + } + + let body + try { + body = await response.json() + } catch (err) { + // Some responses are empty. Ignore JSON parsing errors + } + timer.done() + if (CLSI_COOKIES_ENABLED) { + const newClsiServerId = _getClsiServerIdFromResponse(response) + await NewBackendCloudClsiCookieManager.promises.setServerId( + projectId, + userId, + compileGroup, + compileBackendClass, + newClsiServerId, + clsiServerId + ) + } + return { response, body } } function _getCompilerUrl( @@ -529,22 +425,19 @@ function _getCompilerUrl( if (action != null) { u.pathname += `/${action}` } - u.search = new URLSearchParams({ - compileBackendClass, - compileGroup, - }).toString() - return u.href + u.searchParams.set('compileBackendClass', compileBackendClass) + u.searchParams.set('compileGroup', compileGroup) + return u } -function _postToClsi( +async function _postToClsi( projectId, userId, req, compileBackendClass, - compileGroup, - callback + compileGroup ) { - const compileUrl = _getCompilerUrl( + const url = _getCompilerUrl( compileBackendClass, compileGroup, projectId, @@ -552,55 +445,51 @@ function _postToClsi( 'compile' ) const opts = { - url: compileUrl, - json: req, + body: JSON.stringify(req), method: 'POST', } - _makeRequest( - projectId, - userId, - compileGroup, - compileBackendClass, - opts, - (err, response, body, clsiServerId) => { - if (err != null) { - return callback( - new OError( - 'failed to make request to CLSI', - { - projectId, - userId, - compileOptions: req.compile.options, - rootResourcePath: req.compile.rootResourcePath, - }, - err - ) - ) - } - if (response.statusCode >= 200 && response.statusCode < 300) { - callback(null, body, clsiServerId) - } else if (response.statusCode === 413) { - callback(null, { compile: { status: 'project-too-large' } }) - } else if (response.statusCode === 409) { - callback(null, { compile: { status: 'conflict' } }) - } else if (response.statusCode === 423) { - callback(null, { compile: { status: 'compile-in-progress' } }) - } else if (response.statusCode === 503) { - callback(null, { compile: { status: 'unavailable' } }) - } else { - callback( - new OError(`CLSI returned non-success code: ${response.statusCode}`, { - projectId, - userId, - compileOptions: req.compile.options, - rootResourcePath: req.compile.rootResourcePath, - clsiResponse: body, - statusCode: response.statusCode, - }) - ) - } - } - ) + let response, body, clsiServerId + try { + ;({ response, body, clsiServerId } = await _makeRequest( + projectId, + userId, + compileGroup, + compileBackendClass, + url, + opts + )) + } catch (err) { + throw new OError( + 'failed to make request to CLSI', + { + projectId, + userId, + compileOptions: req.compile.options, + rootResourcePath: req.compile.rootResourcePath, + }, + err + ) + } + if (response.ok) { + return { response: body, clsiServerId } + } else if (response.status === 413) { + return { response: { compile: { status: 'project-too-large' } } } + } else if (response.status === 409) { + return { response: { compile: { status: 'conflict' } } } + } else if (response.status === 423) { + return { response: { compile: { status: 'compile-in-progress' } } } + } else if (response.status === 503) { + return { response: { compile: { status: 'unavailable' } } } + } else { + throw new OError(`CLSI returned non-success code: ${response.status}`, { + projectId, + userId, + compileOptions: req.compile.options, + rootResourcePath: req.compile.rootResourcePath, + clsiResponse: body, + statusCode: response.status, + }) + } } function _parseOutputFiles(projectId, rawOutputFiles = []) { @@ -624,134 +513,104 @@ function _parseOutputFiles(projectId, rawOutputFiles = []) { return outputFiles } -function _buildRequest(projectId, options, callback) { - if (options == null) { - options = {} +async function _buildRequest(projectId, options) { + const project = await ProjectGetter.promises.getProject(projectId, { + compiler: 1, + rootDoc_id: 1, + imageName: 1, + rootFolder: 1, + }) + if (project == null) { + throw new Errors.NotFoundError(`project does not exist: ${projectId}`) + } + if (!VALID_COMPILERS.includes(project.compiler)) { + project.compiler = 'pdflatex' } - ProjectGetter.getProject( - projectId, - { compiler: 1, rootDoc_id: 1, imageName: 1, rootFolder: 1 }, - (err, project) => { - if (err != null) { - return callback(OError.tag(err, 'failed to get project', { projectId })) - } - if (project == null) { - return callback( - new Errors.NotFoundError(`project does not exist: ${projectId}`) - ) - } - if (!VALID_COMPILERS.includes(project.compiler)) { - project.compiler = 'pdflatex' - } - if (options.incrementalCompilesEnabled || options.syncType != null) { - // new way, either incremental or full - const timer = new Metrics.Timer('editor.compile-getdocs-redis') - getContentFromDocUpdaterIfMatch( - projectId, - project, - options, - (err, projectStateHash, docUpdaterDocs) => { - timer.done() - if (err != null) { - logger.error({ err, projectId }, 'error checking project state') - // note: we don't bail out when there's an error getting - // incremental files from the docupdater, we just fall back - // to a normal compile below - } - // see if we can send an incremental update to the CLSI - if ( - docUpdaterDocs != null && - options.syncType !== 'full' && - err == null - ) { - Metrics.inc('compile-from-redis') - _buildRequestFromDocupdater( - projectId, - options, - project, - projectStateHash, - docUpdaterDocs, - callback - ) - } else { - Metrics.inc('compile-from-mongo') - _buildRequestFromMongo( - projectId, - options, - project, - projectStateHash, - callback - ) - } - } - ) - } else { - // old way, always from mongo - const timer = new Metrics.Timer('editor.compile-getdocs-mongo') - _getContentFromMongo(projectId, (err, docs, files) => { - timer.done() - if (err != null) { - return callback( - OError.tag(err, 'failed to get contents from Mongo', { - projectId, - }) - ) - } - _finaliseRequest(projectId, options, project, docs, files, callback) - }) - } + if (options.incrementalCompilesEnabled || options.syncType != null) { + // new way, either incremental or full + const timer = new Metrics.Timer('editor.compile-getdocs-redis') + let projectStateHash, docUpdaterDocs + try { + ;({ projectStateHash, docs: docUpdaterDocs } = + await getContentFromDocUpdaterIfMatch(projectId, project, options)) + } catch (err) { + logger.error({ err, projectId }, 'error checking project state') + // note: we don't bail out when there's an error getting + // incremental files from the docupdater, we just fall back + // to a normal compile below } - ) + timer.done() + // see if we can send an incremental update to the CLSI + if (docUpdaterDocs != null && options.syncType !== 'full') { + Metrics.inc('compile-from-redis') + return _buildRequestFromDocupdater( + projectId, + options, + project, + projectStateHash, + docUpdaterDocs + ) + } else { + Metrics.inc('compile-from-mongo') + return await _buildRequestFromMongo( + projectId, + options, + project, + projectStateHash + ) + } + } else { + // old way, always from mongo + const timer = new Metrics.Timer('editor.compile-getdocs-mongo') + const { docs, files } = await _getContentFromMongo(projectId) + timer.done() + return _finaliseRequest(projectId, options, project, docs, files) + } } -function getContentFromDocUpdaterIfMatch( - projectId, - project, - options, - callback -) { - let projectStateHash - try { - projectStateHash = ClsiStateManager.computeHash(project, options) - } catch (err) { - return callback(err) - } - DocumentUpdaterHandler.getProjectDocsIfMatch( +async function getContentFromDocUpdaterIfMatch(projectId, project, options) { + const projectStateHash = ClsiStateManager.computeHash(project, options) + const docs = await DocumentUpdaterHandler.promises.getProjectDocsIfMatch( projectId, - projectStateHash, - (err, docs) => { - if (err != null) { - return callback( - OError.tag(err, 'Failed to get project documents', { - projectId, - projectStateHash, - }) - ) - } - callback(null, projectStateHash, docs) - } + projectStateHash ) + return { projectStateHash, docs } } -function getOutputFileStream( +async function getOutputFileStream( projectId, userId, options, clsiServerId, buildId, - outputFilePath, - callback + outputFilePath ) { - const url = `${Settings.apis.clsi.url}/project/${projectId}/user/${userId}/build/${buildId}/output/${outputFilePath}` const { compileBackendClass, compileGroup } = options - const readStream = request({ - url, + const url = new URL( + `${Settings.apis.clsi.url}/project/${projectId}/user/${userId}/build/${buildId}/output/${outputFilePath}` + ) + url.searchParams.set('compileBackendClass', compileBackendClass) + url.searchParams.set('compileGroup', compileGroup) + url.searchParams.set('clsiserverid', clsiServerId) + const response = await fetch(url, { method: 'GET', - timeout: 60 * 1000, - qs: { compileBackendClass, compileGroup, clsiserverid: clsiServerId }, + signal: AbortSignal.timeout(OUTPUT_FILE_TIMEOUT_MS), }) - callback(null, readStream) + if (!response.ok) { + // Drain the response body + await response.arrayBuffer() + throw new Errors.OutputFileFetchFailedError( + 'failed to fetch output file from CLSI', + { + projectId, + userId, + url, + status: response.status, + } + ) + } + return response.body } function _buildRequestFromDocupdater( @@ -759,19 +618,9 @@ function _buildRequestFromDocupdater( options, project, projectStateHash, - docUpdaterDocs, - callback + docUpdaterDocs ) { - let docPath - try { - docPath = ProjectEntityHandler.getAllDocPathsFromProject(project) - } catch (err) { - return callback( - OError.tag(err, 'Failed to get all doc paths from project', { - projectId, - }) - ) - } + const docPath = ProjectEntityHandler.getAllDocPathsFromProject(project) const docs = {} for (const doc of docUpdaterDocs || []) { const path = docPath[doc._id] @@ -793,62 +642,32 @@ function _buildRequestFromDocupdater( } } } - _finaliseRequest(projectId, options, project, docs, [], callback) + return _finaliseRequest(projectId, options, project, docs, []) } -function _buildRequestFromMongo( +async function _buildRequestFromMongo( projectId, options, project, - projectStateHash, - callback + projectStateHash ) { - _getContentFromMongo(projectId, (err, docs, files) => { - if (err != null) { - return callback( - OError.tag(err, 'failed to get project contents from Mongo', { - projectId, - }) - ) - } - options = { - ...options, - syncType: 'full', - syncState: projectStateHash, - } - _finaliseRequest(projectId, options, project, docs, files, callback) - }) + const { docs, files } = await _getContentFromMongo(projectId) + options = { + ...options, + syncType: 'full', + syncState: projectStateHash, + } + return _finaliseRequest(projectId, options, project, docs, files) } -function _getContentFromMongo(projectId, callback) { - DocumentUpdaterHandler.flushProjectToMongo(projectId, err => { - if (err != null) { - return callback( - OError.tag(err, 'failed to flush project to Mongo', { projectId }) - ) - } - ProjectEntityHandler.getAllDocs(projectId, (err, docs) => { - if (err != null) { - return callback( - OError.tag(err, 'failed to get project docs', { projectId }) - ) - } - ProjectEntityHandler.getAllFiles(projectId, (err, files) => { - if (err != null) { - return callback( - OError.tag(err, 'failed to get project files', { projectId }) - ) - } - if (files == null) { - files = {} - } - callback(null, docs || {}, files || {}) - }) - }) - }) +async function _getContentFromMongo(projectId) { + await DocumentUpdaterHandler.promises.flushProjectToMongo(projectId) + const docs = await ProjectEntityHandler.promises.getAllDocs(projectId) + const files = await ProjectEntityHandler.promises.getAllFiles(projectId) + return { docs, files } } -function _finaliseRequest(projectId, options, project, docs, files, callback) { +function _finaliseRequest(projectId, options, project, docs, files) { const resources = [] let flags let rootResourcePath = null @@ -897,7 +716,7 @@ function _finaliseRequest(projectId, options, project, docs, files, callback) { rootResourcePath = path.replace(/^\//, '') } } else { - return callback(new OError('no main file specified', { projectId })) + throw new OError('no main file specified', { projectId }) } } @@ -915,7 +734,7 @@ function _finaliseRequest(projectId, options, project, docs, files, callback) { flags = ['-file-line-error'] } - callback(null, { + return { compile: { options: { compiler: project.compiler, @@ -936,72 +755,78 @@ function _finaliseRequest(projectId, options, project, docs, files, callback) { rootResourcePath, resources, }, - }) + } } -function wordCount(projectId, userId, file, options, clsiserverid, callback) { +async function wordCount(projectId, userId, file, options, clsiserverid) { const { compileBackendClass, compileGroup } = options - _buildRequest(projectId, options, (err, req) => { - if (err != null) { - return callback( - OError.tag(err, 'Failed to build CLSI request', { - projectId, - options, - }) - ) + const req = await _buildRequest(projectId, options) + const filename = file || req.compile.rootResourcePath + const url = _getCompilerUrl( + compileBackendClass, + compileGroup, + projectId, + userId, + 'wordcount' + ) + url.searchParams.set('file', filename) + url.searchParams.set('image', req.compile.options.imageName) + + const opts = { + method: 'GET', + } + const { body } = await _makeRequestWithClsiServerId( + projectId, + userId, + compileGroup, + compileBackendClass, + url, + opts, + clsiserverid + ) + return body +} + +function _getClsiServerIdFromResponse(response) { + const setCookieHeaders = response.headers.raw()['set-cookie'] ?? [] + for (const header of setCookieHeaders) { + const cookie = Cookie.parse(header) + if (cookie.key === Settings.clsiCookie.key) { + return cookie.value } - const filename = file || req.compile.rootResourcePath - const wordCountUrl = _getCompilerUrl( - compileBackendClass, - compileGroup, - projectId, - userId, - 'wordcount' - ) - const opts = { - url: wordCountUrl, - qs: { - file: filename, - image: req.compile.options.imageName, - }, - json: true, - method: 'GET', - } - _makeRequestWithClsiServerId( - projectId, - userId, - compileGroup, - compileBackendClass, - opts, - clsiserverid, - (err, response, body) => { - if (err != null) { - return callback(OError.tag(err, 'CLSI request failed', { projectId })) - } - if (response.statusCode >= 200 && response.statusCode < 300) { - callback(null, body) - } else { - callback( - new OError( - `CLSI returned non-success code: ${response.statusCode}`, - { - projectId, - clsiResponse: body, - statusCode: response.statusCode, - } - ) - ) - } - } - ) - }) + } + return null } module.exports = { - sendRequest, - sendExternalRequest, - stopCompile, - deleteAuxFiles, - getOutputFileStream, - wordCount, + sendRequest: callbackifyMultiResult(sendRequest, [ + 'status', + 'outputFiles', + 'clsiServerId', + 'validationProblems', + 'stats', + 'timings', + 'outputUrlPrefix', + ]), + sendExternalRequest: callbackifyMultiResult(sendExternalRequest, [ + 'status', + 'outputFiles', + 'clsiServerId', + 'validationProblems', + 'stats', + 'timings', + 'outputUrlPrefix', + ]), + stopCompile: callbackify(stopCompile), + deleteAuxFiles: callbackify(deleteAuxFiles), + getOutputFileStream: callbackify(getOutputFileStream), + wordCount: callbackify(wordCount), + promises: { + sendRequest, + sendExternalRequest, + stopCompile, + deleteAuxFiles, + getOutputFileStream, + wordCount, + }, } diff --git a/services/web/app/src/Features/Errors/Errors.js b/services/web/app/src/Features/Errors/Errors.js index 2394c4000c..6cddb97f21 100644 --- a/services/web/app/src/Features/Errors/Errors.js +++ b/services/web/app/src/Features/Errors/Errors.js @@ -142,6 +142,8 @@ class ThirdPartyUserNotFoundError extends BackwardCompatibleError { } } +class OutputFileFetchFailedError extends OError {} + class SubscriptionAdminDeletionError extends OErrorV2CompatibleError { constructor(options) { super('subscription admins cannot be deleted', options) @@ -210,6 +212,7 @@ module.exports = { EmailExistsError, InvalidError, NotInV2Error, + OutputFileFetchFailedError, SAMLIdentityExistsError, SAMLAlreadyLinkedError, SAMLEmailNotAffiliatedError, diff --git a/services/web/app/src/Features/LinkedFiles/LinkedFilesController.js b/services/web/app/src/Features/LinkedFiles/LinkedFilesController.js index 9f28dbf2bc..9746c7c126 100644 --- a/services/web/app/src/Features/LinkedFiles/LinkedFilesController.js +++ b/services/web/app/src/Features/LinkedFiles/LinkedFilesController.js @@ -23,7 +23,6 @@ const { CompileFailedError, UrlFetchFailedError, InvalidUrlError, - OutputFileFetchFailedError, AccessDeniedError, BadEntityTypeError, BadDataError, @@ -35,6 +34,7 @@ const { RemoteServiceError, FileCannotRefreshError, } = require('./LinkedFilesErrors') +const { OutputFileFetchFailedError } = require('../Errors/Errors') const Modules = require('../../infrastructure/Modules') const { plainTextResponse } = require('../../infrastructure/Response') diff --git a/services/web/app/src/Features/LinkedFiles/LinkedFilesErrors.js b/services/web/app/src/Features/LinkedFiles/LinkedFilesErrors.js index 5a1457d51b..7737184d03 100644 --- a/services/web/app/src/Features/LinkedFiles/LinkedFilesErrors.js +++ b/services/web/app/src/Features/LinkedFiles/LinkedFilesErrors.js @@ -5,7 +5,6 @@ class UrlFetchFailedError extends BackwardCompatibleError {} class InvalidUrlError extends BackwardCompatibleError {} class CompileFailedError extends BackwardCompatibleError {} -class OutputFileFetchFailedError extends BackwardCompatibleError {} class AccessDeniedError extends BackwardCompatibleError {} @@ -31,7 +30,6 @@ module.exports = { CompileFailedError, UrlFetchFailedError, InvalidUrlError, - OutputFileFetchFailedError, AccessDeniedError, BadEntityTypeError, BadDataError, diff --git a/services/web/app/src/Features/LinkedFiles/ProjectOutputFileAgent.js b/services/web/app/src/Features/LinkedFiles/ProjectOutputFileAgent.js index cc24a1133e..387b10e319 100644 --- a/services/web/app/src/Features/LinkedFiles/ProjectOutputFileAgent.js +++ b/services/web/app/src/Features/LinkedFiles/ProjectOutputFileAgent.js @@ -7,8 +7,8 @@ const { CompileFailedError, BadDataError, AccessDeniedError, - OutputFileFetchFailedError, } = require('./LinkedFilesErrors') +const { OutputFileFetchFailedError } = require('../Errors/Errors') const LinkedFilesHandler = require('./LinkedFilesHandler') function _prepare(projectId, linkedFileData, userId, callback) { @@ -46,31 +46,20 @@ function createLinkedFile( if (err) { return callback(err) } - readStream.on('error', callback) - readStream.on('response', response => { - if (response.statusCode >= 200 && response.statusCode < 300) { - LinkedFilesHandler.importFromStream( - projectId, - readStream, - linkedFileData, - name, - parentFolderId, - userId, - (err, file) => { - if (err) { - return callback(err) - } - callback(null, file._id) - } - ) // Created - } else { - err = new OutputFileFetchFailedError( - `Output file fetch failed: ${linkedFileData.build_id}, ${linkedFileData.source_output_file_path}` - ) - err.statusCode = response.statusCode - callback(err) + LinkedFilesHandler.importFromStream( + projectId, + readStream, + linkedFileData, + name, + parentFolderId, + userId, + (err, file) => { + if (err) { + return callback(err) + } + callback(null, file._id) } - }) + ) }) }) } @@ -94,32 +83,21 @@ function refreshLinkedFile( if (err) { return callback(err) } - readStream.on('error', callback) - readStream.on('response', response => { - if (response.statusCode >= 200 && response.statusCode < 300) { - linkedFileData.build_id = newBuildId - LinkedFilesHandler.importFromStream( - projectId, - readStream, - linkedFileData, - name, - parentFolderId, - userId, - (err, file) => { - if (err) { - return callback(err) - } - callback(null, file._id) - } - ) // Created - } else { - err = new OutputFileFetchFailedError( - `Output file fetch failed: ${linkedFileData.build_id}, ${linkedFileData.source_output_file_path}` - ) - err.statusCode = response.statusCode - callback(err) + linkedFileData.build_id = newBuildId + LinkedFilesHandler.importFromStream( + projectId, + readStream, + linkedFileData, + name, + parentFolderId, + userId, + (err, file) => { + if (err) { + return callback(err) + } + callback(null, file._id) } - }) + ) } ) }) @@ -193,7 +171,6 @@ function _getFileStream(linkedFileData, userId, callback) { if (err) { return callback(err) } - readStream.pause() callback(null, readStream) } ) @@ -239,7 +216,6 @@ function _compileAndGetFileStream(linkedFileData, userId, callback) { if (err) { return callback(err) } - readStream.pause() callback(null, readStream, buildId) } ) diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index fc50d138b5..7e13dc0bef 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -1253,18 +1253,18 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { AuthorizationMiddleware.ensureUserCanReadProject, function (req, res) { const projectId = req.params.Project_id + // use a valid user id for testing + const testUserId = '123456789012345678901234' const sendRes = _.once(function (statusCode, message) { res.status(statusCode) plainTextResponse(res, message) - ClsiCookieManager.clearServerId(projectId, () => {}) + ClsiCookieManager.clearServerId(projectId, testUserId, () => {}) }) // force every compile to a new server // set a timeout let handler = setTimeout(function () { sendRes(500, 'Compiler timed out') handler = null }, 10000) - // use a valid user id for testing - const testUserId = '123456789012345678901234' // run the compile CompileManager.compile( projectId, diff --git a/services/web/package.json b/services/web/package.json index 1fae1bcbb9..2485b46830 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -242,6 +242,7 @@ "rimraf": "2.2.6", "sanitize-html": "^2.8.1", "scroll-into-view-if-needed": "^2.2.25", + "tough-cookie": "^4.0.0", "tsscmp": "^1.0.6", "underscore": "^1.13.1", "utf-8-validate": "^5.0.2", @@ -340,7 +341,6 @@ "terser-webpack-plugin": "^5.3.9", "timekeeper": "^2.2.0", "to-string-loader": "^1.2.0", - "tough-cookie": "^4.0.0", "typescript": "^5.0.4", "val-loader": "^5.0.1", "webpack": "^5.83.1", diff --git a/services/web/test/unit/src/Compile/ClsiCookieManagerTests.js b/services/web/test/unit/src/Compile/ClsiCookieManagerTests.js index 27c7bd35e4..b61991a100 100644 --- a/services/web/test/unit/src/Compile/ClsiCookieManagerTests.js +++ b/services/web/test/unit/src/Compile/ClsiCookieManagerTests.js @@ -51,7 +51,7 @@ describe('ClsiCookieManager', function () { describe('getServerId', function () { it('should call get for the key', function (done) { this.redis.get.callsArgWith(1, null, 'clsi-7') - this.ClsiCookieManager._getServerId( + this.ClsiCookieManager.getServerId( this.project_id, this.user_id, '', @@ -74,7 +74,7 @@ describe('ClsiCookieManager', function () { .stub() .yields(null) this.redis.get.callsArgWith(1, null) - this.ClsiCookieManager._getServerId( + this.ClsiCookieManager.getServerId( this.project_id, this.user_id, '', @@ -95,7 +95,7 @@ describe('ClsiCookieManager', function () { .stub() .yields(null) this.redis.get.callsArgWith(1, null, '') - this.ClsiCookieManager._getServerId( + this.ClsiCookieManager.getServerId( this.project_id, this.user_id, '', @@ -115,52 +115,91 @@ describe('ClsiCookieManager', function () { describe('_populateServerIdViaRequest', function () { beforeEach(function () { - this.response = 'some data' - this.request.post.callsArgWith(1, null, this.response) - this.ClsiCookieManager.setServerId = sinon.stub().yields(null, 'clsi-9') + this.clsiServerId = 'server-id' + this.ClsiCookieManager.setServerId = sinon.stub().yields() }) - it('should make a request to the clsi', function (done) { - this.ClsiCookieManager._populateServerIdViaRequest( - this.project_id, - this.user_id, - 'standard', - 'e2', - (err, serverId) => { - if (err) { - return done(err) - } - const args = this.ClsiCookieManager.setServerId.args[0] - args[0].should.equal(this.project_id) - args[1].should.equal(this.user_id) - args[2].should.equal('standard') - args[3].should.equal('e2') - args[4].should.deep.equal(this.response) - done() + describe('with a server id in the response', function () { + beforeEach(function () { + this.response = { + headers: { + 'set-cookie': [ + `${this.settings.clsiCookie.key}=${this.clsiServerId}`, + ], + }, } - ) + this.request.post.callsArgWith(1, null, this.response) + }) + + it('should make a request to the clsi', function (done) { + this.ClsiCookieManager._populateServerIdViaRequest( + this.project_id, + this.user_id, + 'standard', + 'e2', + (err, serverId) => { + if (err) { + return done(err) + } + const args = this.ClsiCookieManager.setServerId.args[0] + args[0].should.equal(this.project_id) + args[1].should.equal(this.user_id) + args[2].should.equal('standard') + args[3].should.equal('e2') + args[4].should.deep.equal(this.clsiServerId) + done() + } + ) + }) + + it('should return the server id', function (done) { + this.ClsiCookieManager._populateServerIdViaRequest( + this.project_id, + this.user_id, + '', + 'e2', + (err, serverId) => { + if (err) { + return done(err) + } + serverId.should.equal(this.clsiServerId) + done() + } + ) + }) }) - it('should return the server id', function (done) { - this.ClsiCookieManager._populateServerIdViaRequest( - this.project_id, - this.user_id, - '', - 'e2', - (err, serverId) => { - if (err) { - return done(err) + describe('without a server id in the response', function () { + beforeEach(function () { + this.response = { headers: {} } + this.request.post.yields(null, this.response) + }) + it('should not set the server id there is no server id in the response', function (done) { + this.ClsiCookieManager._parseServerIdFromResponse = sinon + .stub() + .returns(null) + this.ClsiCookieManager.setServerId( + this.project_id, + this.user_id, + 'standard', + 'e2', + this.clsiServerId, + null, + err => { + if (err) { + return done(err) + } + this.redis.setex.called.should.equal(false) + done() } - serverId.should.equal('clsi-9') - done() - } - ) + ) + }) }) }) describe('setServerId', function () { beforeEach(function () { - this.response = 'dsadsakj' + this.clsiServerId = 'server-id' this.ClsiCookieManager._parseServerIdFromResponse = sinon .stub() .returns('clsi-8') @@ -172,34 +211,30 @@ describe('ClsiCookieManager', function () { this.user_id, 'standard', 'e2', - this.response, + this.clsiServerId, null, err => { if (err) { return done(err) } - this.redis.setex - .calledWith( - `clsiserver:${this.project_id}:${this.user_id}`, - this.settings.clsiCookie.ttlInSeconds, - 'clsi-8' - ) - .should.equal(true) + this.redis.setex.should.have.been.calledWith( + `clsiserver:${this.project_id}:${this.user_id}`, + this.settings.clsiCookie.ttlInSeconds, + this.clsiServerId + ) done() } ) }) it('should set the server id with the regular ttl for reg instance', function (done) { - this.ClsiCookieManager._parseServerIdFromResponse = sinon - .stub() - .returns('clsi-reg-8') + this.clsiServerId = 'clsi-reg-8' this.ClsiCookieManager.setServerId( this.project_id, this.user_id, 'standard', 'e2', - this.response, + this.clsiServerId, null, err => { if (err) { @@ -208,31 +243,13 @@ describe('ClsiCookieManager', function () { expect(this.redis.setex).to.have.been.calledWith( `clsiserver:${this.project_id}:${this.user_id}`, this.settings.clsiCookie.ttlInSecondsRegular, - 'clsi-reg-8' + this.clsiServerId ) done() } ) }) - it('should return the server id', function (done) { - this.ClsiCookieManager.setServerId( - this.project_id, - this.user_id, - 'standard', - 'e2', - this.response, - null, - (err, serverId) => { - if (err) { - return done(err) - } - serverId.should.equal('clsi-8') - done() - } - ) - }) - it('should not set the server id if clsiCookies are not enabled', function (done) { delete this.settings.clsiCookie.key this.ClsiCookieManager = SandboxedModule.require(modulePath, { @@ -246,30 +263,9 @@ describe('ClsiCookieManager', function () { this.user_id, 'standard', 'e2', - this.response, + this.clsiServerId, null, - (err, serverId) => { - if (err) { - return done(err) - } - this.redis.setex.called.should.equal(false) - done() - } - ) - }) - - it('should not set the server id there is no server id in the response', function (done) { - this.ClsiCookieManager._parseServerIdFromResponse = sinon - .stub() - .returns(null) - this.ClsiCookieManager.setServerId( - this.project_id, - this.user_id, - 'standard', - 'e2', - this.response, - null, - (err, serverId) => { + err => { if (err) { return done(err) } @@ -301,19 +297,17 @@ describe('ClsiCookieManager', function () { this.user_id, 'standard', 'e2', - this.response, + this.clsiServerId, null, - (err, serverId) => { + err => { if (err) { return done(err) } - this.redis_secondary.setex - .calledWith( - `clsiserver:${this.project_id}:${this.user_id}`, - this.settings.clsiCookie.ttlInSeconds, - 'clsi-8' - ) - .should.equal(true) + this.redis_secondary.setex.should.have.been.calledWith( + `clsiserver:${this.project_id}:${this.user_id}`, + this.settings.clsiCookie.ttlInSeconds, + this.clsiServerId + ) done() } ) @@ -322,7 +316,7 @@ describe('ClsiCookieManager', function () { describe('getCookieJar', function () { beforeEach(function () { - this.ClsiCookieManager._getServerId = sinon.stub().yields(null, 'clsi-11') + this.ClsiCookieManager.getServerId = sinon.stub().yields(null, 'clsi-11') }) it('should return a jar with the cookie set populated from redis', function (done) { diff --git a/services/web/test/unit/src/Compile/ClsiManagerTests.js b/services/web/test/unit/src/Compile/ClsiManagerTests.js index 8ccee32b36..15a625dc14 100644 --- a/services/web/test/unit/src/Compile/ClsiManagerTests.js +++ b/services/web/test/unit/src/Compile/ClsiManagerTests.js @@ -1,15 +1,16 @@ +const { setTimeout } = require('timers/promises') +const _ = require('lodash') const sinon = require('sinon') const { expect } = require('chai') -const modulePath = '../../../../app/src/Features/Compile/ClsiManager.js' const SandboxedModule = require('sandboxed-module') const tk = require('timekeeper') const FILESTORE_URL = 'http://filestore.example.com' -const CLSI_URL = 'http://clsi.example.com' +const CLSI_HOST = 'clsi.example.com' +const MODULE_PATH = '../../../../app/src/Features/Compile/ClsiManager.js' describe('ClsiManager', function () { beforeEach(function () { - this.jar = { cookie: 'stuff' } this.user_id = 'user-id' this.project = { _id: 'project-id', @@ -36,38 +37,60 @@ describe('ClsiManager', function () { created: new Date(), }, } + this.clsiCookieKey = 'clsiserver' + this.clsiServerId = 'clsi-server-id' + this.newClsiServerId = 'newserver' this.rawOutputFiles = {} - this.response = { statusCode: 200 } this.responseBody = { compile: { status: 'success' }, } + this.response = { + ok: true, + status: 200, + json: sinon.stub().resolves(this.responseBody), + headers: { + raw: sinon.stub().returns({ + 'set-cookie': [`${this.clsiCookieKey}=${this.newClsiServerId}`], + }), + }, + } - this.request = sinon.stub().yields(null, this.response, this.responseBody) + this.fetch = sinon.stub().resolves(this.response) this.ClsiCookieManager = { - clearServerId: sinon.stub().yields(), - getCookieJar: sinon.stub().yields(null, this.jar), - setServerId: sinon.stub().yields(null), + promises: { + clearServerId: sinon.stub().resolves(), + getServerId: sinon.stub().resolves('clsi-server-id'), + setServerId: sinon.stub().resolves(), + }, } this.ClsiStateManager = { computeHash: sinon.stub().returns('01234567890abcdef'), } this.ClsiFormatChecker = { - checkRecoursesForProblems: sinon.stub().yields(), + promises: { + checkRecoursesForProblems: sinon.stub().resolves(), + }, } this.Project = {} this.ProjectEntityHandler = { - getAllDocs: sinon.stub().yields(null, this.docs), - getAllFiles: sinon.stub().yields(null, this.files), getAllDocPathsFromProject: sinon.stub(), + promises: { + getAllDocs: sinon.stub().resolves(this.docs), + getAllFiles: sinon.stub().resolves(this.files), + }, } this.ProjectGetter = { - findById: sinon.stub().yields(null, this.project), - getProject: sinon.stub().yields(null, this.project), + promises: { + findById: sinon.stub().resolves(this.project), + getProject: sinon.stub().resolves(this.project), + }, } this.DocumentUpdaterHandler = { - clearProjectState: sinon.stub().yields(), - flushProjectToMongo: sinon.stub().yields(), - getProjectDocsIfMatch: sinon.stub().yields(), + promises: { + clearProjectState: sinon.stub().resolves(), + flushProjectToMongo: sinon.stub().resolves(), + getProjectDocsIfMatch: sinon.stub().resolves(), + }, } this.Metrics = { Timer: class Metrics { @@ -78,28 +101,27 @@ describe('ClsiManager', function () { inc: sinon.stub(), count: sinon.stub(), } - this.SplitTestHandler = { - getAssignment: sinon.stub().yields(null, { variant: 'default' }), + this.Settings = { + apis: { + filestore: { + url: FILESTORE_URL, + secret: 'secret', + }, + clsi: { + url: `http://${CLSI_HOST}`, + defaultBackendClass: 'e2', + }, + clsi_priority: { + url: 'https://clsipremium.example.com', + }, + }, + enablePdfCaching: true, + clsiCookie: { key: 'clsiserver' }, } - this.ClsiManager = SandboxedModule.require(modulePath, { + this.ClsiManager = SandboxedModule.require(MODULE_PATH, { requires: { - '@overleaf/settings': (this.settings = { - apis: { - filestore: { - url: FILESTORE_URL, - secret: 'secret', - }, - clsi: { - url: CLSI_URL, - defaultBackendClass: 'e2', - }, - clsi_priority: { - url: 'https://clsipremium.example.com', - }, - }, - enablePdfCaching: true, - }), + '@overleaf/settings': this.Settings, '../../models/Project': { Project: this.Project, }, @@ -109,13 +131,11 @@ describe('ClsiManager', function () { this.DocumentUpdaterHandler, './ClsiCookieManager': () => this.ClsiCookieManager, './ClsiStateManager': this.ClsiStateManager, - request: this.request, + 'node-fetch': this.fetch, './ClsiFormatChecker': this.ClsiFormatChecker, '@overleaf/metrics': this.Metrics, - '../SplitTests/SplitTestHandler': this.SplitTestHandler, }, }) - this.callback = sinon.stub() tk.freeze(Date.now()) }) @@ -124,12 +144,8 @@ describe('ClsiManager', function () { }) describe('sendRequest', function () { - beforeEach(function () { - this.ClsiCookieManager.getCookieJar.yields(null, this.jar, 'clsi3') - }) - describe('with a successful compile', function () { - beforeEach(function (done) { + beforeEach(async function () { this.outputFiles = [ { url: `/project/${this.project_id}/user/${this.user_id}/build/1234/output/output.pdf`, @@ -145,58 +161,65 @@ describe('ClsiManager', function () { }, ] this.responseBody.compile.outputFiles = this.outputFiles.map( - outputFile => ({ ...outputFile, url: CLSI_URL + outputFile.url }) + outputFile => ({ + ...outputFile, + url: `http://${CLSI_HOST}${outputFile.url}`, + }) ) this.timeout = 100 - this.ClsiManager.sendRequest( + this.result = await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, { compileBackendClass: 'e2', compileGroup: 'standard', timeout: this.timeout, - }, - (err, status, outputFiles) => { - if (err) { - return done(err) - } - this.result = { status, outputFiles } - done() } ) }) it('should send the request to the CLSI', function () { - this.request.should.have.been.calledWith({ - url: `${CLSI_URL}/project/${this.project._id}/user/${this.user_id}/compile?compileBackendClass=e2&compileGroup=standard`, - method: 'POST', - json: { - compile: { - options: sinon.match({ - compiler: this.project.compiler, - imageName: this.project.imageName, - timeout: this.timeout, - draft: false, - check: undefined, - syncType: undefined, // "full" - syncState: undefined, - compileGroup: 'standard', - enablePdfCaching: false, - pdfCachingMinChunkSize: undefined, - flags: undefined, - metricsMethod: 'standard', - stopOnFirstError: false, - }), - rootResourcePath: 'main.tex', - resources: _makeResources(this.project, this.docs, this.files), + this.fetch.should.have.been.calledWith( + sinon.match( + url => + url.host === CLSI_HOST && + url.pathname === + `/project/${this.project._id}/user/${this.user_id}/compile` && + url.searchParams.get('compileBackendClass') === 'e2' && + url.searchParams.get('compileGroup') === 'standard' + ), + { + method: 'POST', + body: sinon.match(body => { + body = JSON.parse(body) + const options = body.compile.options + return ( + options.compiler === this.project.compiler && + options.imageName === this.project.imageName && + options.timeout === this.timeout && + options.draft === false && + options.compileGroup === 'standard' && + options.metricsMethod === 'standard' && + options.stopOnFirstError === false && + options.syncType === undefined && + body.compile.rootResourcePath === 'main.tex' && + _.isEqual( + body.compile.resources, + _makeResources(this.project, this.docs, this.files) + ) + ) + }), + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json', + Cookie: `${this.clsiCookieKey}=${this.clsiServerId}`, }, - }, - jar: this.jar, - }) + } + ) }) it('should get the project with the required fields', function () { - this.ProjectGetter.getProject.should.have.been.calledWith( + this.ProjectGetter.promises.getProject.should.have.been.calledWith( this.project._id, { compiler: 1, @@ -208,24 +231,24 @@ describe('ClsiManager', function () { }) it('should flush the project to the database', function () { - this.DocumentUpdaterHandler.flushProjectToMongo.should.have.been.calledWith( + this.DocumentUpdaterHandler.promises.flushProjectToMongo.should.have.been.calledWith( this.project._id ) }) it('should get all the docs', function () { - this.ProjectEntityHandler.getAllDocs.should.have.been.calledWith( + this.ProjectEntityHandler.promises.getAllDocs.should.have.been.calledWith( this.project._id ) }) it('should get all the files', function () { - this.ProjectEntityHandler.getAllFiles.should.have.been.calledWith( + this.ProjectEntityHandler.promises.getAllFiles.should.have.been.calledWith( this.project._id ) }) - it('should call the callback with the status and output files', function () { + it('should return the status and output files', function () { expect(this.result.status).to.equal('success') expect(this.result.outputFiles.map(f => f.path)).to.have.members( this.outputFiles.map(f => f.path) @@ -233,24 +256,27 @@ describe('ClsiManager', function () { }) it('should process a request with a cookie jar', function () { - expect(this.request).to.have.been.calledWith( + expect(this.fetch).to.have.been.calledWith( + sinon.match.any, sinon.match(opts => opts.jar === this.jar && opts.qs == null) ) }) it('should persist the cookie from the response', function () { - expect(this.ClsiCookieManager.setServerId).to.have.been.calledWith( + expect( + this.ClsiCookieManager.promises.setServerId + ).to.have.been.calledWith( this.project._id, this.user_id, 'standard', 'e2', - this.response + this.newClsiServerId ) }) }) describe('with ranges on the pdf and stats/timings details', function () { - beforeEach(function (done) { + beforeEach(async function () { this.ranges = [{ start: 1, end: 42, hash: 'foo' }] this.startXRefTable = 123 this.size = 456 @@ -275,49 +301,24 @@ describe('ClsiManager', function () { ] this.stats = { fooStat: 1 } this.timings = { barTiming: 2 } - this.serverId = 'clsi-server-id-42' this.responseBody.compile.outputFiles = this.outputFiles.map( - outputFile => ({ ...outputFile, url: CLSI_URL + outputFile.url }) + outputFile => ({ + ...outputFile, + url: `http://${CLSI_HOST}${outputFile.url}`, + }) ) this.responseBody.compile.stats = this.stats this.responseBody.compile.timings = this.timings - this.ClsiCookieManager.getCookieJar.yields( - null, - this.jar, - this.serverId - ) - this.ClsiManager.sendRequest( + this.result = await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - { compileBackendClass: 'e2', compileGroup: 'standard' }, - ( - err, - status, - outputFiles, - serverId, - validationError, - stats, - timings - ) => { - if (err) { - return done(err) - } - this.result = { - status, - outputFiles, - serverId, - validationError, - stats, - timings, - } - done() - } + { compileBackendClass: 'e2', compileGroup: 'standard' } ) }) it('should emit the caching details and stats/timings', function () { expect(this.result.status).to.equal('success') - expect(this.result.serverId).to.equal(this.serverId) + expect(this.result.clsiServerId).to.equal(this.newClsiServerId) expect(this.result.validationError).to.be.undefined expect(this.result.stats).to.equal(this.stats) expect(this.result.timings).to.equal(this.timings) @@ -332,15 +333,15 @@ describe('ClsiManager', function () { }) describe('with the incremental compile option', function () { - beforeEach(function (done) { + beforeEach(async function () { const doc = this.docs['/main.tex'] - this.DocumentUpdaterHandler.getProjectDocsIfMatch.yields(null, [ + this.DocumentUpdaterHandler.promises.getProjectDocsIfMatch.resolves([ { _id: doc._id, lines: doc.lines, v: 123 }, ]) this.ProjectEntityHandler.getAllDocPathsFromProject.returns({ 'mock-doc-id-1': 'main.tex', }) - this.ClsiManager.sendRequest( + this.result = await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, { @@ -350,19 +351,12 @@ describe('ClsiManager', function () { compileGroup: 'priority', enablePdfCaching: true, pdfCachingMinChunkSize: 1337, - }, - (err, status, outputFiles) => { - if (err) { - return done(err) - } - this.result = { status, outputFiles } - done() } ) }) it('should get the project with the required fields', function () { - this.ProjectGetter.getProject.should.have.been.calledWith( + this.ProjectGetter.promises.getProject.should.have.been.calledWith( this.project._id, { compiler: 1, @@ -374,153 +368,193 @@ describe('ClsiManager', function () { }) it('should not explicitly flush the project to the database', function () { - this.DocumentUpdaterHandler.flushProjectToMongo.should.not.have.been.calledWith( + this.DocumentUpdaterHandler.promises.flushProjectToMongo.should.not.have.been.calledWith( this.project._id ) }) it('should get only the live docs from the docupdater with a background flush in docupdater', function () { - this.DocumentUpdaterHandler.getProjectDocsIfMatch.should.have.been.calledWith( + this.DocumentUpdaterHandler.promises.getProjectDocsIfMatch.should.have.been.calledWith( this.project._id ) }) it('should not get any of the files', function () { - this.ProjectEntityHandler.getAllFiles.should.not.have.been.called + this.ProjectEntityHandler.promises.getAllFiles.should.not.have.been + .called }) it('should build up the CLSI request', function () { - this.request.should.have.been.calledWith({ - url: `${CLSI_URL}/project/${this.project._id}/user/${this.user_id}/compile?compileBackendClass=e2&compileGroup=priority`, - method: 'POST', - json: { - compile: { - options: { - compiler: this.project.compiler, - timeout: 100, - imageName: this.project.imageName, - draft: false, - check: undefined, - syncType: 'incremental', - syncState: '01234567890abcdef', - compileGroup: 'priority', - enablePdfCaching: true, - pdfCachingMinChunkSize: 1337, - flags: undefined, - metricsMethod: 'priority', - stopOnFirstError: false, - }, - rootResourcePath: 'main.tex', - resources: [ - { - path: 'main.tex', - content: this.docs['/main.tex'].lines.join('\n'), - }, - ], + this.fetch.should.have.been.calledWith( + sinon.match( + url => + url.hostname === CLSI_HOST && + url.pathname === + `/project/${this.project._id}/user/${this.user_id}/compile` && + url.searchParams.get('compileBackendClass') === 'e2' && + url.searchParams.get('compileGroup') === 'priority' + ), + { + method: 'POST', + body: sinon.match(body => { + const json = JSON.parse(body) + const options = json.compile.options + return ( + options.compiler === this.project.compiler && + options.timeout === 100 && + options.imageName === this.project.imageName && + options.draft === false && + options.syncType === 'incremental' && + options.syncState === '01234567890abcdef' && + options.compileGroup === 'priority' && + options.enablePdfCaching === true && + options.pdfCachingMinChunkSize === 1337 && + options.metricsMethod === 'priority' && + options.stopOnFirstError === false && + json.compile.rootResourcePath === 'main.tex' && + _.isEqual(json.compile.resources, [ + { + path: 'main.tex', + content: this.docs['/main.tex'].lines.join('\n'), + }, + ]) + ) + }), + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json', + Cookie: `${this.clsiCookieKey}=${this.clsiServerId}`, }, - }, - jar: this.jar, - }) + } + ) }) }) describe('when the root doc is set and not in the docupdater', function () { - beforeEach(function (done) { + beforeEach(async function () { const doc = this.docs['/main.tex'] - this.DocumentUpdaterHandler.getProjectDocsIfMatch.yields(null, [ + this.DocumentUpdaterHandler.promises.getProjectDocsIfMatch.resolves([ { _id: doc._id, lines: doc.lines, v: 123 }, ]) this.ProjectEntityHandler.getAllDocPathsFromProject.returns({ 'mock-doc-id-1': 'main.tex', 'mock-doc-id-2': '/chapters/chapter1.tex', }) - this.ClsiManager.sendRequest( + await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, { timeout: 100, incrementalCompilesEnabled: true, rootDoc_id: 'mock-doc-id-2', - }, - done + } ) }) it('should still change the root path', function () { - this.request.should.have.been.calledWith( - sinon.match( - opts => - opts.json.compile.rootResourcePath === 'chapters/chapter1.tex' - ) + this.fetch.should.have.been.calledWith( + sinon.match.any, + sinon.match({ + body: sinon.match( + body => + JSON.parse(body).compile.rootResourcePath === + 'chapters/chapter1.tex' + ), + }) ) }) }) describe('when root doc override is valid', function () { - beforeEach(function (done) { - this.ClsiManager.sendRequest( + beforeEach(async function () { + await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - { rootDoc_id: 'mock-doc-id-2' }, - done + { rootDoc_id: 'mock-doc-id-2' } ) }) it('should change root path', function () { - this.request.should.have.been.calledWith( - sinon.match( - opts => - opts.json.compile.rootResourcePath === 'chapters/chapter1.tex' - ) + this.fetch.should.have.been.calledWith( + sinon.match.any, + sinon.match({ + body: sinon.match( + body => + JSON.parse(body).compile.rootResourcePath === + 'chapters/chapter1.tex' + ), + }) ) }) }) describe('when root doc override is invalid', function () { - beforeEach(function (done) { - this.ClsiManager.sendRequest( + beforeEach(async function () { + await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - { rootDoc_id: 'invalid-id' }, - done + { rootDoc_id: 'invalid-id' } ) }) it('should fallback to default root doc', function () { - this.request.should.have.been.calledWith( - sinon.match(opts => opts.json.compile.rootResourcePath === 'main.tex') + this.fetch.should.have.been.calledWith( + sinon.match.any, + sinon.match({ + body: sinon.match( + body => JSON.parse(body).compile.rootResourcePath === 'main.tex' + ), + }) ) }) }) describe('when the project has an invalid compiler', function () { - beforeEach(function (done) { + beforeEach(async function () { this.project.compiler = 'context' - this.ClsiManager.sendRequest(this.project._id, this.user_id, {}, done) + await this.ClsiManager.promises.sendRequest( + this.project._id, + this.user_id, + {} + ) }) it('should set the compiler to pdflatex', function () { - expect(this.request).to.have.been.calledWith( - sinon.match(opts => opts.json.compile.options.compiler === 'pdflatex') + expect(this.fetch).to.have.been.calledWith( + sinon.match.any, + sinon.match({ + body: sinon.match( + body => JSON.parse(body).compile.options.compiler === 'pdflatex' + ), + }) ) }) }) describe('when there is no valid root document', function () { - beforeEach(function (done) { + beforeEach(async function () { this.project.rootDoc_id = 'not-valid' - this.ClsiManager.sendRequest(this.project._id, this.user_id, {}, done) + await this.ClsiManager.promises.sendRequest( + this.project._id, + this.user_id, + {} + ) }) it('should set to main.tex', function () { - expect(this.request).to.have.been.calledWith( - sinon.match(opts => opts.json.compile.rootResourcePath === 'main.tex') + expect(this.fetch).to.have.been.calledWith( + sinon.match.any, + sinon.match({ + body: sinon.match( + body => JSON.parse(body).compile.rootResourcePath === 'main.tex' + ), + }) ) }) }) describe('when there is no valid root document and no main.tex document', function () { - beforeEach(function (done) { + beforeEach(async function () { this.project.rootDoc_id = 'not-valid' this.docs = { '/other.tex': { @@ -534,18 +568,11 @@ describe('ClsiManager', function () { lines: ['Chapter 1'], }, } - this.ProjectEntityHandler.getAllDocs.yields(null, this.docs) - this.ClsiManager.sendRequest( + this.ProjectEntityHandler.promises.getAllDocs.resolves(this.docs) + this.result = await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - {}, - (err, status) => { - if (err) { - return done(err) - } - this.result = { status } - done() - } + {} ) }) @@ -555,7 +582,7 @@ describe('ClsiManager', function () { }) describe('when there is no valid root document and a single document which is not main.tex', function () { - beforeEach(function (done) { + beforeEach(async function () { this.project.rootDoc_id = 'not-valid' this.docs = { '/other.tex': { @@ -564,199 +591,200 @@ describe('ClsiManager', function () { lines: ['Hello', 'world'], }, } - this.ProjectEntityHandler.getAllDocs.yields(null, this.docs) - this.ClsiManager.sendRequest(this.project._id, this.user_id, {}, done) + this.ProjectEntityHandler.promises.getAllDocs.resolves(this.docs) + await this.ClsiManager.promises.sendRequest( + this.project._id, + this.user_id, + {} + ) }) it('should set io to the only file', function () { - expect(this.request).to.have.been.calledWith( - sinon.match( - opts => opts.json.compile.rootResourcePath === 'other.tex' - ) + expect(this.fetch).to.have.been.calledWith( + sinon.match.any, + sinon.match({ + body: sinon.match( + body => JSON.parse(body).compile.rootResourcePath === 'other.tex' + ), + }) ) }) }) describe('with the draft option', function () { - beforeEach(function (done) { - this.ClsiManager.sendRequest( + beforeEach(async function () { + await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - { timeout: 100, draft: true }, - done + { + timeout: 100, + draft: true, + } ) }) it('should add the draft option into the request', function () { - expect(this.request).to.have.been.calledWith( - sinon.match(opts => opts.json.compile.options.draft === true) + expect(this.fetch).to.have.been.calledWith( + sinon.match.any, + sinon.match({ + body: sinon.match( + body => JSON.parse(body).compile.options.draft === true + ), + }) ) }) }) describe('with a failed compile', function () { - beforeEach(function (done) { + beforeEach(async function () { this.responseBody.compile.status = 'failure' - this.ClsiManager.sendRequest( + this.result = await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - {}, - (err, status) => { - if (err) { - return done(err) - } - this.result = { status } - done() - } + {} ) }) - it('should call the callback with a failure status', function () { + it('should return a failure status', function () { expect(this.result.status).to.equal('failure') }) }) describe('with a sync conflict', function () { - beforeEach(function (done) { - this.request + beforeEach(async function () { + const conflictResponseBody = { compile: { status: 'conflict' } } + const conflictResponse = { + ...this.response, + json: sinon.stub().resolves(conflictResponseBody), + } + this.fetch .withArgs( - sinon.match(opts => opts.json.compile.options.syncType !== 'full') + sinon.match.any, + sinon.match({ + body: sinon.match( + body => JSON.parse(body).compile.options.syncType !== 'full' + ), + }) ) - .yields(null, this.response, { - compile: { status: 'conflict' }, - }) - this.ClsiManager.sendRequest( + .resolves(conflictResponse) + this.result = await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - {}, - (err, status) => { - if (err) { - return done(err) - } - this.result = { status } - done() - } + {} ) }) it('should send two requests to CLSI', function () { - this.request.should.have.been.calledTwice + this.fetch.should.have.been.calledTwice }) it('should call the CLSI first without syncType:full', function () { - const compileOptions = - this.request.getCall(0).args[0].json.compile.options + const compileOptions = JSON.parse(this.fetch.getCall(0).args[1].body) + .compile.options expect(compileOptions.syncType).to.be.undefined }) it('should call the CLSI a second time with syncType:full', function () { - const compileOptions = - this.request.getCall(1).args[0].json.compile.options + const compileOptions = JSON.parse(this.fetch.getCall(1).args[1].body) + .compile.options expect(compileOptions.syncType).to.equal('full') }) - it('should call the callback with a success status', function () { + it('should return a success status', function () { this.result.status.should.equal('success') }) }) describe('with an unavailable response', function () { - beforeEach(function (done) { - this.request.onCall(0).yields(null, this.response, { + beforeEach(async function () { + this.response.json.onCall(0).resolves({ compile: { status: 'unavailable' }, }) - this.ClsiManager.sendRequest( + this.result = await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - {}, - (err, status) => { - if (err) { - return done(err) - } - this.result = { status } - done() - } + {} ) }) it('should send two requests to CLSI', function () { - this.request.should.have.been.calledTwice + this.fetch.should.have.been.calledTwice }) it('should call the CLSI first without syncType:full', function () { - const compileOptions = - this.request.getCall(0).args[0].json.compile.options + const compileOptions = JSON.parse(this.fetch.getCall(0).args[1].body) + .compile.options expect(compileOptions.syncType).to.be.undefined }) it('should call the CLSI a second time with syncType:full', function () { - const compileOptions = - this.request.getCall(1).args[0].json.compile.options + const compileOptions = JSON.parse(this.fetch.getCall(1).args[1].body) + .compile.options expect(compileOptions.syncType).to.equal('full') }) it('should clear the CLSI server id cookie', function () { - expect(this.ClsiCookieManager.clearServerId).to.have.been.calledWith( - this.project._id, - this.user_id - ) + expect( + this.ClsiCookieManager.promises.clearServerId + ).to.have.been.calledWith(this.project._id, this.user_id) }) - it('should call the callback with a success status', function () { + it('should return a success status', function () { expect(this.result.status).to.equal('success') }) }) describe('when the resources fail the precompile check', function () { beforeEach(function () { - this.ClsiFormatChecker.checkRecoursesForProblems.yields( + this.ClsiFormatChecker.promises.checkRecoursesForProblems.rejects( new Error('failed') ) - this.ClsiManager.sendRequest( - this.project._id, - this.user_id, - {}, - this.callback - ) }) - it('should call the callback only once', function () { - this.callback.calledOnce.should.equal(true) - }) - - it('should call the callback with an error', function () { - this.callback.should.have.been.calledWith(sinon.match.instanceOf(Error)) + it('should throw an error', async function () { + await expect( + this.ClsiManager.promises.sendRequest( + this.project._id, + this.user_id, + {} + ) + ).to.be.rejected }) }) describe('when a new backend is configured', function () { - beforeEach(function (done) { - this.settings.apis.clsi_new = { url: 'https://compiles.somewhere.test' } - this.ClsiManager.sendRequest( + beforeEach(async function () { + this.Settings.apis.clsi_new = { url: 'https://compiles.somewhere.test' } + await this.ClsiManager.promises.sendRequest( this.project._id, this.user_id, - { compileBackendClass: 'e2', compileGroup: 'standard' }, - err => { - if (err) { - return done(err) - } - // wait for the background task to finish - setTimeout(done, 0) + { + compileBackendClass: 'e2', + compileGroup: 'standard', } ) + // wait for the background task to finish + await setTimeout(0) }) it('makes a request to the new backend', function () { - expect(this.request).to.have.been.calledTwice - expect(this.request).to.have.been.calledWith( - sinon.match({ - url: `${CLSI_URL}/project/${this.project._id}/user/${this.user_id}/compile?compileBackendClass=e2&compileGroup=standard`, - }) + expect(this.fetch).to.have.been.calledTwice + expect(this.fetch).to.have.been.calledWith( + sinon.match( + url => + url.host === CLSI_HOST && + url.pathname === + `/project/${this.project._id}/user/${this.user_id}/compile` && + url.searchParams.get('compileBackendClass') === 'e2' && + url.searchParams.get('compileGroup') === 'standard' + ) ) - expect(this.request).to.have.been.calledWith( - sinon.match({ - url: `${this.settings.apis.clsi_new.url}/project/${this.project._id}/user/${this.user_id}/compile?compileBackendClass=e2&compileGroup=standard`, - }) + expect(this.fetch).to.have.been.calledWith( + sinon.match( + url => + url.toString() === + `${this.Settings.apis.clsi_new.url}/project/${this.project._id}/user/${this.user_id}/compile?compileBackendClass=e2&compileGroup=standard` + ) ) }) }) @@ -766,11 +794,10 @@ describe('ClsiManager', function () { beforeEach(function () { this.submissionId = 'submission-id' this.clsiRequest = 'mock-request' - this.ClsiCookieManager.getCookieJar.yields(null, this.jar, 'clsi3') }) describe('with a successful compile', function () { - beforeEach(function (done) { + beforeEach(async function () { this.outputFiles = [ { url: `/project/${this.submissionId}/build/1234/output/output.pdf`, @@ -786,32 +813,40 @@ describe('ClsiManager', function () { }, ] this.responseBody.compile.outputFiles = this.outputFiles.map( - outputFile => ({ ...outputFile, url: CLSI_URL + outputFile.url }) + outputFile => ({ + ...outputFile, + url: `http://${CLSI_HOST}${outputFile.url}`, + }) ) - this.ClsiManager.sendExternalRequest( + this.result = await this.ClsiManager.promises.sendExternalRequest( this.submissionId, this.clsiRequest, - { compileBackendClass: 'e2', compileGroup: 'standard' }, - (err, status, outputFiles) => { - if (err) { - return done(err) - } - this.result = { status, outputFiles } - done() - } + { compileBackendClass: 'e2', compileGroup: 'standard' } ) }) it('should send the request to the CLSI', function () { - this.request.should.have.been.calledWith({ - url: `${CLSI_URL}/project/${this.submissionId}/compile?compileBackendClass=e2&compileGroup=standard`, - method: 'POST', - json: this.clsiRequest, - jar: this.jar, - }) + this.fetch.should.have.been.calledWith( + sinon.match( + url => + url.host === CLSI_HOST && + url.pathname === `/project/${this.submissionId}/compile` && + url.searchParams.get('compileBackendClass') === 'e2' && + url.searchParams.get('compileGroup') === 'standard' + ), + { + method: 'POST', + body: JSON.stringify(this.clsiRequest), + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json', + Cookie: `${this.clsiCookieKey}=${this.clsiServerId}`, + }, + } + ) }) - it('should call the callback with the status and output files', function () { + it('should return the status and output files', function () { expect(this.result.status).to.equal('success') expect(this.result.outputFiles.map(f => f.path)).to.have.members( this.outputFiles.map(f => f.path) @@ -820,160 +855,154 @@ describe('ClsiManager', function () { }) describe('with a failed compile', function () { - beforeEach(function (done) { + beforeEach(async function () { this.responseBody.compile.status = 'failure' - this.ClsiManager.sendExternalRequest( + this.result = await this.ClsiManager.promises.sendExternalRequest( this.submissionId, this.clsiRequest, - {}, - (err, status) => { - if (err) { - return done(err) - } - this.result = { status } - done() - } + {} ) }) - it('should call the callback with a failure status', function () { + it('should return a failure status', function () { expect(this.result.status).to.equal('failure') }) }) describe('when the resources fail the precompile check', function () { - beforeEach(function (done) { - this.ClsiFormatChecker.checkRecoursesForProblems.yields( + beforeEach(async function () { + this.ClsiFormatChecker.promises.checkRecoursesForProblems.rejects( new Error('failed') ) this.responseBody.compile.status = 'failure' - this.ClsiManager.sendExternalRequest( - this.submissionId, - this.clsiRequest, - {}, - err => { - this.err = err - done() - } - ) }) - it('should call the callback with an error', function () { - expect(this.err).to.be.instanceof(Error) + it('should throw an error', async function () { + await expect( + this.ClsiManager.promises.sendExternalRequest( + this.submissionId, + this.clsiRequest, + {} + ) + ).to.be.rejected }) }) }) describe('deleteAuxFiles', function () { describe('with the standard compileGroup', function () { - beforeEach(function (done) { - this.ClsiManager.deleteAuxFiles( + beforeEach(async function () { + await this.ClsiManager.promises.deleteAuxFiles( this.project._id, this.user_id, { compileBackendClass: 'e2', compileGroup: 'standard' }, - 'node-1', - done + 'node-1' ) }) it('should call the delete method in the standard CLSI', function () { - this.request.should.have.been.calledWith({ - url: `${CLSI_URL}/project/${this.project._id}/user/${this.user_id}?compileBackendClass=e2&compileGroup=standard`, - method: 'DELETE', - qs: { - compileGroup: 'standard', - compileBackendClass: 'e2', - clsiserverid: 'node-1', - }, - }) + this.fetch.should.have.been.calledWith( + sinon.match( + url => + url.host === CLSI_HOST && + url.pathname === + `/project/${this.project._id}/user/${this.user_id}` && + url.searchParams.get('compileBackendClass') === 'e2' && + url.searchParams.get('compileGroup') === 'standard' && + url.searchParams.get('clsiserverid') === 'node-1' + ), + { method: 'DELETE' } + ) }) it('should clear the project state from the docupdater', function () { - this.DocumentUpdaterHandler.clearProjectState + this.DocumentUpdaterHandler.promises.clearProjectState .calledWith(this.project._id) .should.equal(true) }) it('should clear the clsi persistance', function () { - this.ClsiCookieManager.clearServerId + this.ClsiCookieManager.promises.clearServerId .calledWith(this.project._id, this.user_id) .should.equal(true) }) it('should not add a cookie jar', function () { - expect(this.request).to.have.been.calledWith( + expect(this.fetch).to.have.been.calledWith( + sinon.match.any, sinon.match(opts => opts.jar == null) ) }) it('should not persist a cookie on response', function () { - expect(this.ClsiCookieManager.setServerId).not.to.have.been.called + expect(this.ClsiCookieManager.promises.setServerId).not.to.have.been + .called }) }) }) describe('wordCount', function () { describe('with root file', function () { - beforeEach(function (done) { - this.ClsiManager.wordCount( + beforeEach(async function () { + await this.ClsiManager.promises.wordCount( this.project._id, this.user_id, false, { compileBackendClass: 'e2', compileGroup: 'standard' }, - 'node-1', - done + 'node-1' ) }) it('should call wordCount with root file', function () { - expect(this.request).to.have.been.calledWith({ - url: `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=e2&compileGroup=standard`, - method: 'GET', - qs: { - compileGroup: 'standard', - compileBackendClass: 'e2', - clsiserverid: 'node-1', - file: 'main.tex', - image: 'mock-image-name', - }, - json: true, - }) + expect(this.fetch).to.have.been.calledWith( + sinon.match( + url => + url.toString() === + `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=e2&compileGroup=standard&file=main.tex&image=mock-image-name&clsiserverid=node-1` + ), + { method: 'GET' } + ) }) it('should not persist a cookie on response', function () { - expect(this.ClsiCookieManager.setServerId).not.to.have.been.called + expect(this.ClsiCookieManager.promises.setServerId).not.to.have.been + .called }) }) describe('with param file', function () { - beforeEach(function (done) { - this.ClsiManager.wordCount( + beforeEach(async function () { + await this.ClsiManager.promises.wordCount( this.project._id, this.user_id, 'other.tex', { compileBackendClass: 'e2', compileGroup: 'standard' }, - 'node-2', - done + 'node-2' ) }) it('should call wordCount with param file', function () { - expect(this.request).to.have.been.calledWith({ - url: `http://clsi.example.com/project/${this.project._id}/user/${this.user_id}/wordcount?compileBackendClass=e2&compileGroup=standard`, - method: 'GET', - qs: { - compileGroup: 'standard', - compileBackendClass: 'e2', - clsiserverid: 'node-2', - file: 'other.tex', - image: 'mock-image-name', - }, - json: true, - }) + expect(this.fetch).to.have.been.calledWith( + sinon.match( + url => + url.host === CLSI_HOST && + url.pathname === + `/project/${this.project._id}/user/${this.user_id}/wordcount` && + url.searchParams.get('compileBackendClass') === 'e2' && + url.searchParams.get('compileGroup') === 'standard' && + url.searchParams.get('clsiserverid') === 'node-2' && + url.searchParams.get('file') === 'other.tex' && + url.searchParams.get('image') === 'mock-image-name' + ), + { + method: 'GET', + } + ) }) it('should not persist a cookie on response', function () { - expect(this.ClsiCookieManager.setServerId).not.to.have.been.called + expect(this.ClsiCookieManager.promises.setServerId).not.to.have.been + .called }) }) })