From 4b9b4fdf31ddd2103e244a89d9726a7c451e8041 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Wed, 21 Feb 2024 11:36:00 +0000 Subject: [PATCH] Allow individual docs to be downloaded from the file tree (#17137) GitOrigin-RevId: d0b2ce9f3a252e34f452155ed83c3c04e7916ef0 --- .../DocumentUpdaterController.js | 50 ++++++++++ .../DocumentUpdater/DocumentUpdaterHandler.js | 8 +- services/web/app/src/router.js | 6 ++ .../contexts/file-tree-actionable.tsx | 7 +- .../web/test/acceptance/src/helpers/User.js | 4 +- .../DocumentUpdaterControllerTests.js | 92 +++++++++++++++++++ .../web/test/unit/src/helpers/MockResponse.js | 2 +- 7 files changed, 164 insertions(+), 5 deletions(-) create mode 100644 services/web/app/src/Features/DocumentUpdater/DocumentUpdaterController.js create mode 100644 services/web/test/unit/src/DocumentUpdater/DocumentUpdaterControllerTests.js diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterController.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterController.js new file mode 100644 index 0000000000..ad7c0bc6ad --- /dev/null +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterController.js @@ -0,0 +1,50 @@ +const logger = require('@overleaf/logger') +const DocumentUpdaterHandler = require('./DocumentUpdaterHandler') +const ProjectLocator = require('../Project/ProjectLocator') +const { plainTextResponse } = require('../../infrastructure/Response') +const { expressify } = require('@overleaf/promise-utils') + +async function getDoc(req, res) { + const projectId = req.params.Project_id + const docId = req.params.Doc_id + + try { + const { element: doc } = await ProjectLocator.promises.findElement({ + project_id: projectId, + element_id: docId, + type: 'doc', + }) + + const { lines } = await DocumentUpdaterHandler.promises.getDocument( + projectId, + docId, + -1 // latest version only + ) + + res.setContentDisposition('attachment', { filename: doc.name }) + plainTextResponse(res, lines.join('\n')) + } catch (err) { + if (err.name === 'NotFoundError') { + logger.warn( + { err, projectId, docId }, + 'entity not found when downloading doc' + ) + + return res.sendStatus(404) + } + + logger.err( + { err, projectId, docId }, + 'error getting document for downloading' + ) + + return res.sendStatus(500) + } +} + +module.exports = { + getDoc: expressify(getDoc), + promises: { + getDoc, + }, +} diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index 0a45c50997..f077c28313 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js @@ -6,6 +6,7 @@ const async = require('async') const logger = require('@overleaf/logger') const metrics = require('@overleaf/metrics') const { promisify } = require('util') +const { promisifyMultiResult } = require('@overleaf/promise-utils') module.exports = { flushProjectToMongo, @@ -27,7 +28,12 @@ module.exports = { flushProjectToMongoAndDelete: promisify(flushProjectToMongoAndDelete), flushDocToMongo: promisify(flushDocToMongo), deleteDoc: promisify(deleteDoc), - getDocument: promisify(getDocument), + getDocument: promisifyMultiResult(getDocument, [ + 'lines', + 'version', + 'ranges', + 'ops', + ]), setDocument: promisify(setDocument), getProjectDocsIfMatch: promisify(getProjectDocsIfMatch), clearProjectState: promisify(clearProjectState), diff --git a/services/web/app/src/router.js b/services/web/app/src/router.js index 315281e41a..65d72e9bdf 100644 --- a/services/web/app/src/router.js +++ b/services/web/app/src/router.js @@ -31,6 +31,7 @@ const ClsiCookieManager = require('./Features/Compile/ClsiCookieManager')( const HealthCheckController = require('./Features/HealthCheck/HealthCheckController') const ProjectDownloadsController = require('./Features/Downloads/ProjectDownloadsController') const FileStoreController = require('./Features/FileStore/FileStoreController') +const DocumentUpdaterController = require('./Features/DocumentUpdater/DocumentUpdaterController') const HistoryController = require('./Features/History/HistoryController') const ExportsController = require('./Features/Exports/ExportsController') const PasswordResetRouter = require('./Features/PasswordReset/PasswordResetRouter') @@ -490,6 +491,11 @@ function initialize(webRouter, privateApiRouter, publicApiRouter) { AuthorizationMiddleware.ensureUserCanReadProject, FileStoreController.getFile ) + webRouter.get( + '/Project/:Project_id/doc/:Doc_id/download', // "download" suffix to avoid conflict with private API route at doc/:doc_id + AuthorizationMiddleware.ensureUserCanReadProject, + DocumentUpdaterController.getDoc + ) webRouter.post( '/project/:Project_id/settings', validate({ diff --git a/services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.tsx b/services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.tsx index 17712705c9..be2ee58c16 100644 --- a/services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.tsx +++ b/services/web/frontend/js/features/file-tree/contexts/file-tree-actionable.tsx @@ -475,14 +475,19 @@ export const FileTreeActionableProvider: FC<{ } }, []) - // build the path for downloading a single file + // build the path for downloading a single file or doc const downloadPath = useMemo(() => { if (selectedEntityIds.size === 1) { const [selectedEntityId] = selectedEntityIds const selectedEntity = findInTree(fileTreeData, selectedEntityId) + if (selectedEntity?.type === 'fileRef') { return `/project/${projectId}/file/${selectedEntityId}` } + + if (selectedEntity?.type === 'doc') { + return `/project/${projectId}/doc/${selectedEntityId}/download` + } } }, [fileTreeData, projectId, selectedEntityIds]) diff --git a/services/web/test/acceptance/src/helpers/User.js b/services/web/test/acceptance/src/helpers/User.js index 3bacdb97f9..5c3b1613f9 100644 --- a/services/web/test/acceptance/src/helpers/User.js +++ b/services/web/test/acceptance/src/helpers/User.js @@ -550,7 +550,7 @@ class User { } uploadFileInProject(projectId, folderId, file, name, contentType, callback) { - const imageFile = fs.createReadStream( + const fileStream = fs.createReadStream( Path.resolve(Path.join(__dirname, '..', '..', 'files', file)) ) @@ -563,7 +563,7 @@ class User { formData: { name, qqfile: { - value: imageFile, + value: fileStream, options: { filename: name, contentType, diff --git a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterControllerTests.js b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterControllerTests.js new file mode 100644 index 0000000000..d33677f440 --- /dev/null +++ b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterControllerTests.js @@ -0,0 +1,92 @@ +const sinon = require('sinon') +const { expect } = require('chai') +const SandboxedModule = require('sandboxed-module') +const MockResponse = require('../helpers/MockResponse') + +const MODULE_PATH = + '../../../../app/src/Features/DocumentUpdater/DocumentUpdaterController.js' + +describe('DocumentUpdaterController', function () { + beforeEach(function () { + this.DocumentUpdaterHandler = { + promises: { + getDocument: sinon.stub(), + }, + } + this.ProjectLocator = { + promises: { + findElement: sinon.stub(), + }, + } + this.controller = SandboxedModule.require(MODULE_PATH, { + requires: { + '@overleaf/settings': this.settings, + '../Project/ProjectLocator': this.ProjectLocator, + './DocumentUpdaterHandler': this.DocumentUpdaterHandler, + }, + }) + this.projectId = '2k3j1lk3j21lk3j' + this.fileId = '12321kklj1lk3jk12' + this.req = { + params: { + Project_id: this.projectId, + Doc_id: this.docId, + }, + get(key) { + return undefined + }, + } + this.lines = ['test', '', 'testing'] + this.res = new MockResponse() + this.doc = { name: 'myfile.tex' } + }) + + describe('getDoc', function () { + beforeEach(function () { + this.DocumentUpdaterHandler.promises.getDocument.resolves({ + lines: this.lines, + }) + this.ProjectLocator.promises.findElement.resolves({ + element: this.doc, + }) + this.res = new MockResponse() + }) + + it('should call the document updater handler with the project_id and doc_id', async function () { + await this.controller.promises.getDoc(this.req, this.res) + + expect( + this.DocumentUpdaterHandler.promises.getDocument + ).to.have.been.calledOnceWith( + this.req.params.Project_id, + this.req.params.Doc_id, + -1 + ) + }) + + it('should return the content', async function () { + await this.controller.promises.getDoc(this.req, this.res) + expect(this.res.statusCode).to.equal(200) + expect(this.res.body).to.equal('test\n\ntesting') + }) + + it('should find the doc in the project', async function () { + await this.controller.promises.getDoc(this.req, this.res) + expect( + this.ProjectLocator.promises.findElement + ).to.have.been.calledOnceWith({ + project_id: this.projectId, + element_id: this.docId, + type: 'doc', + }) + }) + + it('should set the Content-Disposition header', async function () { + await this.controller.promises.getDoc(this.req, this.res) + expect(this.res.setContentDisposition).to.have.been.calledWith( + 'attachment', + { filename: this.doc.name } + ) + }) + }) +}) diff --git a/services/web/test/unit/src/helpers/MockResponse.js b/services/web/test/unit/src/helpers/MockResponse.js index 6ade53e06b..1fbac41a0c 100644 --- a/services/web/test/unit/src/helpers/MockResponse.js +++ b/services/web/test/unit/src/helpers/MockResponse.js @@ -18,7 +18,7 @@ const contentDisposition = require('content-disposition') class MockResponse { static initClass() { // Added via ExpressLocals. - this.prototype.setContentDisposition = sinon.stub() + this.prototype.setContentDisposition = sinon.stub() // FIXME: should be reset between each test } constructor() {