From c764566148088301acb75ab1f02afba31724dff8 Mon Sep 17 00:00:00 2001 From: andrew rumble Date: Thu, 6 Jun 2024 09:51:16 +0100 Subject: [PATCH] Allow all files to be in zip (in same directory) GitOrigin-RevId: 14645a0c3db88faf00e2718b9574b5892ac3efcb --- services/clsi/app/js/OutputController.js | 9 +----- .../clsi/app/js/OutputFileArchiveManager.js | 28 +++++++++---------- .../test/unit/js/OutputControllerTests.js | 12 +------- .../unit/js/OutputFileArchiveManagerTests.js | 9 ++---- 4 files changed, 18 insertions(+), 40 deletions(-) diff --git a/services/clsi/app/js/OutputController.js b/services/clsi/app/js/OutputController.js index 1155ee31cb..13f3dce84c 100644 --- a/services/clsi/app/js/OutputController.js +++ b/services/clsi/app/js/OutputController.js @@ -2,20 +2,13 @@ const logger = require('@overleaf/logger') const OutputFileArchiveManager = require('./OutputFileArchiveManager') const { expressify } = require('@overleaf/promise-utils') -function cleanFiles(files) { - if (!Array.isArray(files)) { - return [] - } - return files.filter(file => /^output\./g.test(file)) -} - async function createOutputZip(req, res) { const { project_id: projectId, user_id: userId, build_id: buildId, } = req.params - const files = cleanFiles(req.query.files) + const files = Array.isArray(req.query.files) ? req.query.files : [] logger.debug({ projectId, userId, buildId, files }, 'Will create zip file') const archive = await OutputFileArchiveManager.archiveFilesForBuild( diff --git a/services/clsi/app/js/OutputFileArchiveManager.js b/services/clsi/app/js/OutputFileArchiveManager.js index 23ab7c2133..b165d850df 100644 --- a/services/clsi/app/js/OutputFileArchiveManager.js +++ b/services/clsi/app/js/OutputFileArchiveManager.js @@ -4,7 +4,7 @@ const OutputCacheManager = require('./OutputCacheManager') const OutputFileFinder = require('./OutputFileFinder') const Settings = require('@overleaf/settings') const { open } = require('node:fs/promises') -const path = require('node:path') +const path = require('path') const { NotFoundError } = require('./Errors') function getContentDir(projectId, userId) { @@ -19,8 +19,6 @@ function getContentDir(projectId, userId) { module.exports = OutputFileArchiveManager = { async archiveFilesForBuild(projectId, userId, build, files = []) { - const contentDir = getContentDir(projectId, userId) - const validFiles = await (files.length > 0 ? this._getRequestedOutputFiles(projectId, userId, build, files) : this._getAllOutputFiles(projectId, userId, build)) @@ -33,11 +31,9 @@ module.exports = OutputFileArchiveManager = { for (const file of validFiles) { try { - const fileHandle = await open( - `${contentDir}${OutputCacheManager.path(build, file)}` - ) + const fileHandle = await open(file, 'r') const fileStream = fileHandle.createReadStream() - archive.append(fileStream, { name: file }) + archive.append(fileStream, { name: path.basename(file) }) } catch (error) { missingFiles.push(file) } @@ -63,7 +59,9 @@ module.exports = OutputFileArchiveManager = { `${contentDir}${OutputCacheManager.path(build, '.')}` ) - return outputFiles.map(({ path }) => path) + return outputFiles.map( + ({ path }) => `${contentDir}${OutputCacheManager.path(build, path)}` + ) } catch (error) { if ( error.code === 'ENOENT' || @@ -77,14 +75,14 @@ module.exports = OutputFileArchiveManager = { }, async _getRequestedOutputFiles(projectId, userId, build, files) { - const outputFiles = new Set( - await OutputFileArchiveManager._getAllOutputFiles( - projectId, - userId, - build - ) + const outputFiles = await OutputFileArchiveManager._getAllOutputFiles( + projectId, + userId, + build ) - return files.filter(file => outputFiles.has(file)) + return files.filter(file => + outputFiles.some(outputFile => file === path.basename(outputFile)) + ) }, } diff --git a/services/clsi/test/unit/js/OutputControllerTests.js b/services/clsi/test/unit/js/OutputControllerTests.js index 3b9ce38773..49fa1fe5f3 100644 --- a/services/clsi/test/unit/js/OutputControllerTests.js +++ b/services/clsi/test/unit/js/OutputControllerTests.js @@ -37,23 +37,13 @@ describe('OutputController', function () { build_id: 'build-id-123', }, query: { - files: ['output.tex', 'not-output.tex'], + files: ['output.tex'], }, } this.archive.pipe.callsFake(() => done()) this.OutputController.createOutputZip(this.req, this.res) }) - it('does not pass files that do not start with "output" to OutputFileArchiveManager', function () { - sinon.assert.calledWith( - this.archiveFilesForBuild, - 'project-id-123', - 'user-id-123', - 'build-id-123', - ['output.tex'] - ) - }) - it('pipes the archive to the response', function () { sinon.assert.calledWith(this.archive.pipe, this.res) }) diff --git a/services/clsi/test/unit/js/OutputFileArchiveManagerTests.js b/services/clsi/test/unit/js/OutputFileArchiveManagerTests.js index c93231e704..6a6c3190ad 100644 --- a/services/clsi/test/unit/js/OutputFileArchiveManagerTests.js +++ b/services/clsi/test/unit/js/OutputFileArchiveManagerTests.js @@ -50,9 +50,6 @@ describe('OutputFileArchiveManager', function () { './OutputCacheManager': this.OutputCacheManger, archiver: this.archiver, 'node:fs/promises': this.fs, - 'node:path': { - basename: sinon.stub().callsFake(path => path.split('/').pop()), - }, '@overleaf/settings': { path: { outputDir: this.outputDir, @@ -140,14 +137,14 @@ describe('OutputFileArchiveManager', function () { expect(this.archive.append.callCount).to.equal(2) sinon.assert.calledWith( this.archive.append, - `handle: ${this.outputDir}/${projectId}-${userId}/${buildId}/file_1`, + `handle: file_1`, sinon.match({ name: 'file_1', }) ) sinon.assert.calledWith( this.archive.append, - `handle: ${this.outputDir}/${projectId}-${userId}/${buildId}/file_4`, + `handle: file_4`, sinon.match({ name: 'file_4', }) @@ -185,7 +182,7 @@ describe('OutputFileArchiveManager', function () { it('adds the files that were found to the archive', function () { sinon.assert.calledWith( this.archive.append, - `handle: ${this.outputDir}/${projectId}-${userId}/${buildId}/file_1`, + `handle: file_1`, sinon.match({ name: 'file_1' }) ) })