diff --git a/services/clsi/app/js/CompileController.js b/services/clsi/app/js/CompileController.js index 2c893226d8..cc99cf1e54 100644 --- a/services/clsi/app/js/CompileController.js +++ b/services/clsi/app/js/CompileController.js @@ -12,6 +12,17 @@ function timeSinceLastSuccessfulCompile() { return Date.now() - lastSuccessfulCompileTimestamp } +function addUrlToOutputFile(outputFile, projectId, userId) { + return { + url: + `${Settings.apis.clsi.url}/project/${projectId}` + + (userId != null ? `/user/${userId}` : '') + + (outputFile.build != null ? `/build/${outputFile.build}` : '') + + `/output/${outputFile.path}`, + ...outputFile, + } +} + function compile(req, res, next) { const timer = new Metrics.Timer('compile-request') RequestParser.parse(req.body, function (error, request) { @@ -30,7 +41,7 @@ function compile(req, res, next) { return next(error) } CompileManager.doCompileWithLock(request, (error, result) => { - let { outputFiles, stats, timings } = result || {} + let { buildId, outputFiles, stats, timings } = result || {} let code, status if (outputFiles == null) { outputFiles = [] @@ -98,6 +109,7 @@ function compile(req, res, next) { if (error) { outputFiles = error.outputFiles || [] + buildId = error.buildId } timer.done() @@ -108,14 +120,28 @@ function compile(req, res, next) { 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, - })), + outputFiles: + outputFiles.length === 0 + ? [] + : outputFiles + .map(file => + addUrlToOutputFile( + file, + request.project_id, + request.user_id + ) + ) + .concat( + addUrlToOutputFile( + { + build: buildId, + path: 'output.zip', + type: 'zip', + }, + request.project_id, + request.user_id + ) + ), }, }) }) diff --git a/services/clsi/app/js/CompileManager.js b/services/clsi/app/js/CompileManager.js index 2a685d3e86..9d1981263d 100644 --- a/services/clsi/app/js/CompileManager.js +++ b/services/clsi/app/js/CompileManager.js @@ -208,7 +208,7 @@ async function doCompile(request) { Metrics.inc('compiles-timeout', 1, request.metricsOpts) } - const { outputFiles, allEntries } = await _saveOutputFiles({ + const { outputFiles, allEntries, buildId } = await _saveOutputFiles({ request, compileDir, resourceList, @@ -216,7 +216,7 @@ async function doCompile(request) { timings, }) error.outputFiles = outputFiles // return output files so user can check logs - + error.buildId = buildId // Clear project if this compile was abruptly terminated if (error.terminated || error.timedout) { await clearProjectWithListing( @@ -280,7 +280,7 @@ async function doCompile(request) { // Emit compile time. timings.compile = ts - const { outputFiles } = await _saveOutputFiles({ + const { outputFiles, buildId } = await _saveOutputFiles({ request, compileDir, resourceList, @@ -296,7 +296,7 @@ async function doCompile(request) { emitPdfStats(stats, timings, request) } - return { outputFiles, stats, timings } + return { outputFiles, stats, timings, buildId } } async function _saveOutputFiles({ @@ -316,20 +316,24 @@ async function _saveOutputFiles({ let { outputFiles, allEntries } = await OutputFileFinder.promises.findOutputFiles(resourceList, compileDir) + let buildId + try { - outputFiles = await OutputCacheManager.promises.saveOutputFiles( + const saveResult = await OutputCacheManager.promises.saveOutputFiles( { request, stats, timings }, outputFiles, compileDir, outputDir ) + buildId = saveResult.buildId + outputFiles = saveResult.outputFiles } catch (err) { const { project_id: projectId, user_id: userId } = request logger.err({ projectId, userId, err }, 'failed to save output files') } timings.output = timer.done() - return { outputFiles, allEntries } + return { outputFiles, allEntries, buildId } } async function stopCompile(projectId, userId) { diff --git a/services/clsi/app/js/OutputCacheManager.js b/services/clsi/app/js/OutputCacheManager.js index ca80dcc2ae..cad5302000 100644 --- a/services/clsi/app/js/OutputCacheManager.js +++ b/services/clsi/app/js/OutputCacheManager.js @@ -164,7 +164,7 @@ module.exports = OutputCacheManager = { outputDir, stats, (err, outputFiles) => { - if (err) return callback(err, outputFiles) + if (err) return callback(err, { outputFiles, buildId }) const enablePdfCaching = request.enablePdfCaching const enablePdfCachingDark = @@ -173,7 +173,7 @@ module.exports = OutputCacheManager = { !Settings.enablePdfCaching || (!enablePdfCaching && !enablePdfCachingDark) ) { - return callback(null, outputFiles) + return callback(null, { outputFiles, buildId }) } OutputCacheManager.saveStreamsInContentDir( @@ -191,9 +191,9 @@ module.exports = OutputCacheManager = { { err, outputDir, stats, timings }, 'pdf caching failed' ) - return callback(null, outputFiles) + return callback(null, { outputFiles, buildId }) } - callback(err, outputFiles) + callback(err, { outputFiles, buildId }) } ) } diff --git a/services/clsi/test/unit/js/CompileControllerTests.js b/services/clsi/test/unit/js/CompileControllerTests.js index 041e03bbaf..eb5a1134b6 100644 --- a/services/clsi/test/unit/js/CompileControllerTests.js +++ b/services/clsi/test/unit/js/CompileControllerTests.js @@ -50,6 +50,7 @@ function tryImageNameValidation(method, imageNameField) { describe('CompileController', function () { beforeEach(function () { + this.buildId = 'build-id-123' this.CompileController = SandboxedModule.require(modulePath, { requires: { './CompileManager': (this.CompileManager = {}), @@ -118,6 +119,7 @@ describe('CompileController', function () { outputFiles: this.output_files, stats: this.stats, timings: this.timings, + buildId: this.buildId, }) this.CompileController.compile(this.req, this.res) }) @@ -140,21 +142,32 @@ describe('CompileController', function () { it('should return the JSON response', function () { this.res.status.calledWith(200).should.equal(true) - this.res.send - .calledWith({ - compile: { + console.log(this.res.send.args[0][0].compile) + sinon.assert.calledWith( + this.res.send, + sinon.match.has( + 'compile', + sinon.match({ status: 'success', error: null, stats: this.stats, timings: this.timings, outputUrlPrefix: '/zone/b', - 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) + outputFiles: [ + ...this.output_files.map(file => ({ + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`, + ...file, + })), + { + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`, + build: this.buildId, + path: 'output.zip', + type: 'zip', + }, + ], + }) + ) + ) }) }) @@ -165,6 +178,7 @@ describe('CompileController', function () { outputFiles: this.output_files, stats: this.stats, timings: this.timings, + buildId: this.buildId, }) this.CompileController.compile(this.req, this.res) }) @@ -179,10 +193,18 @@ describe('CompileController', function () { stats: this.stats, timings: this.timings, outputUrlPrefix: '', - outputFiles: this.output_files.map(file => ({ - 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, + })), + { + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`, + build: this.buildId, + path: 'output.zip', + type: 'zip', + }, + ], }, }) .should.equal(true) @@ -207,6 +229,7 @@ describe('CompileController', function () { outputFiles: this.output_files, stats: this.stats, timings: this.timings, + buildId: this.buildId, }) this.CompileController.compile(this.req, this.res) }) @@ -221,10 +244,18 @@ describe('CompileController', function () { stats: this.stats, timings: this.timings, outputUrlPrefix: '/zone/b', - outputFiles: this.output_files.map(file => ({ - 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, + })), + { + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`, + build: this.buildId, + path: 'output.zip', + type: 'zip', + }, + ], }, }) .should.equal(true) @@ -250,6 +281,7 @@ describe('CompileController', function () { outputFiles: this.output_files, stats: this.stats, timings: this.timings, + buildId: this.buildId, }) this.CompileController.compile(this.req, this.res) }) @@ -264,10 +296,18 @@ describe('CompileController', function () { stats: this.stats, timings: this.timings, outputUrlPrefix: '/zone/b', - outputFiles: this.output_files.map(file => ({ - 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, + })), + { + url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`, + build: this.buildId, + path: 'output.zip', + type: 'zip', + }, + ], }, }) .should.equal(true) @@ -276,9 +316,11 @@ describe('CompileController', function () { describe('with an error', function () { beforeEach(function () { + const error = new Error((this.message = 'error message')) + error.buildId = this.buildId this.CompileManager.doCompileWithLock = sinon .stub() - .callsArgWith(1, new Error((this.message = 'error message')), null) + .callsArgWith(1, error, null) this.CompileController.compile(this.req, this.res) }) diff --git a/services/clsi/test/unit/js/CompileManagerTests.js b/services/clsi/test/unit/js/CompileManagerTests.js index 6aa416afae..6f5b5baf64 100644 --- a/services/clsi/test/unit/js/CompileManagerTests.js +++ b/services/clsi/test/unit/js/CompileManagerTests.js @@ -35,6 +35,7 @@ describe('CompileManager', function () { build: 1234, }, ] + this.buildId = 'build-id-123' this.commandOutput = 'Dummy output' this.compileBaseDir = '/compile/dir' this.outputBaseDir = '/output/dir' @@ -61,7 +62,9 @@ describe('CompileManager', function () { } this.OutputCacheManager = { promises: { - saveOutputFiles: sinon.stub().resolves(this.buildFiles), + saveOutputFiles: sinon + .stub() + .resolves({ outputFiles: this.buildFiles, buildId: this.buildId }), }, } this.Settings = { diff --git a/services/web/app/src/Features/Compile/CompileController.js b/services/web/app/src/Features/Compile/CompileController.js index dbab01a8f4..61d41a3128 100644 --- a/services/web/app/src/Features/Compile/CompileController.js +++ b/services/web/app/src/Features/Compile/CompileController.js @@ -398,6 +398,13 @@ module.exports = CompileController = { if (error) { return next(error) } + + const qs = {} + + if (req.params.file === 'output.zip') { + qs.files = req.query.files + } + const url = CompileController._getFileUrl( projectId, userId, @@ -408,7 +415,7 @@ module.exports = CompileController = { projectId, 'output-file', url, - {}, + qs, req, res, next @@ -573,10 +580,20 @@ module.exports = CompileController = { return next(err) } url = new URL(`${Settings.apis.clsi.url}${url}`) - url.search = new URLSearchParams({ - ...persistenceOptions.qs, - ...qs, - }).toString() + + const params = new URLSearchParams(persistenceOptions.qs) + + for (const [key, value] of Object.entries(qs)) { + if (Array.isArray(value)) { + for (const v of value) { + params.append(key, v) + } + continue + } + params.append(key, value) + } + + url.search = params.toString() const timer = new Metrics.Timer( 'proxy_to_clsi', 1, @@ -604,7 +621,10 @@ module.exports = CompileController = { }) for (const key of ['Content-Length', 'Content-Type']) { - res.setHeader(key, response.headers.get(key)) + const headerValue = response.headers.get(key) + if (headerValue) { + res.setHeader(key, headerValue) + } } res.writeHead(response.status) return pipeline(stream, res) diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index c6d8de1928..c6d6e90de2 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -305,6 +305,7 @@ "doing_this_will_verify_affiliation_and_allow_log_in_2": "", "done": "", "download": "", + "download_all": "", "download_metadata": "", "download_pdf": "", "download_zip_file": "", diff --git a/services/web/frontend/js/features/pdf-preview/components/pdf-file-list.jsx b/services/web/frontend/js/features/pdf-preview/components/pdf-file-list.jsx index 7e2764e769..0bc0d41b5d 100644 --- a/services/web/frontend/js/features/pdf-preview/components/pdf-file-list.jsx +++ b/services/web/frontend/js/features/pdf-preview/components/pdf-file-list.jsx @@ -33,6 +33,18 @@ function PdfFileList({ fileList }) { {file.path} ))} + + {fileList.other.length + fileList.top.length > 0 && fileList.archive && ( + + + {t('download_all')} ({fileList.other.length + fileList.top.length}) + + + )} ) } @@ -48,6 +60,10 @@ PdfFileList.propTypes = { fileList: PropTypes.shape({ top: FilesArray, other: FilesArray, + archive: PropTypes.shape({ + path: PropTypes.string.isRequired, + url: PropTypes.string.isRequired, + }), }), } diff --git a/services/web/frontend/js/features/pdf-preview/util/file-list.js b/services/web/frontend/js/features/pdf-preview/util/file-list.js index c9edfa828d..c82078b4d9 100644 --- a/services/web/frontend/js/features/pdf-preview/util/file-list.js +++ b/services/web/frontend/js/features/pdf-preview/util/file-list.js @@ -1,5 +1,5 @@ const topFileTypes = ['bbl', 'gls', 'ind'] -const ignoreFiles = ['output.fls', 'output.fdb_latexmk'] +const ignoreFiles = ['output.fls', 'output.fdb_latexmk', 'output.zip'] export const buildFileList = (outputFiles, clsiServerId, compileGroup) => { const files = { top: [], other: [] } @@ -18,6 +18,8 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => { const allFiles = [] + let outputArchiveFile + // filter out ignored files and set some properties for (const file of outputFiles.values()) { if (!ignoreFiles.includes(file.path)) { @@ -28,6 +30,8 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => { } allFiles.push(file) + } else if (file.type === 'zip') { + outputArchiveFile = file } } @@ -52,6 +56,14 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => { files.other.push(file) } } + + if (outputArchiveFile) { + allFiles.forEach( + file => file.type !== 'pdf' && params.append('files', file.path) + ) + outputArchiveFile.url += `?${params.toString()}` + files.archive = outputArchiveFile + } } return files diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 897592f869..3dd65a46f6 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -430,6 +430,7 @@ "done": "Done", "dont_have_account": "Don’t have an account?", "download": "Download", + "download_all": "Download all", "download_metadata": "Download Overleaf metadata", "download_pdf": "Download PDF", "download_zip_file": "Download .zip file",