mirror of
https://github.com/yu-i-i/overleaf-cep.git
synced 2026-11-14 14:13:43 +01:00
[WEB + CLSI] Download as docx file feature (#32851)
* using CLSI logic for fetching the project contents and skip the .zip export * Use unique conversion directory for project-to-docx export to avoid corrupting the shared compile directory when a compile runs concurrently * Remove X-Accel-Buffering header — not needed as CLSI does not run behind nginx * moving log before sending the data * Return CLSI stream directly instead of buffering to disk on web Previously convertProjectToDocx wrote the CLSI response to a temp file on disk, then the controller read it back to stream to the client. Now the stream is returned directly and piped to the response, avoiding unnecessary disk I/O on the web server. * Use href redirect for docx export instead of fetching blob into memory * making functions and files more generic so they can be used in future for other documents exports as well * adding export-docx split test * adding unit tests * adding cypress E2E test * format:fix * renaming the route to download from convert * adding new icon for export docx button * format:fix * remove unused showExportDocumentErrorToast export and adding guard against invalid Content-Length header from CLSI * format:fix * refactor(clsi): move promisify(parse) into RequestParser * refactor: generic conversion endpoint with type as route param * refactor: use type→extension map for validated conversion types * refactor(clsi): remove --standalone flag and fix rejection test * fixing the href in cypress test * renaming function * adding type to Metrics.inc * fix: rename exportProjectDocument, add WithLock wrapper and metrics type label * format:fix * fix: hide docx export from anonymous users and add WithLock wrapper * format fix * remove redundant Content-Length validation from DocumentConversionManager * format:fix * removing trailing icon GitOrigin-RevId: e9764fefac2c4b625d23be9e942ea4a8b283c70d
This commit is contained in:
Davinder Singh
@@ -28,7 +28,11 @@ describe('ProjectDownloadsController', function () {
|
||||
)
|
||||
|
||||
vi.doMock('../../../../app/src/Features/Project/ProjectGetter.mjs', () => ({
|
||||
default: (ctx.ProjectGetter = {}),
|
||||
default: (ctx.ProjectGetter = {
|
||||
promises: {
|
||||
getProject: sinon.stub(),
|
||||
},
|
||||
}),
|
||||
}))
|
||||
|
||||
vi.doMock(
|
||||
@@ -47,6 +51,32 @@ describe('ProjectDownloadsController', function () {
|
||||
})
|
||||
)
|
||||
|
||||
vi.doMock(
|
||||
'../../../../app/src/Features/Authentication/SessionManager.mjs',
|
||||
() => ({
|
||||
default: (ctx.SessionManager = {
|
||||
getLoggedInUserId: sinon
|
||||
.stub()
|
||||
.callsFake(session => session?.user?._id),
|
||||
}),
|
||||
})
|
||||
)
|
||||
|
||||
vi.doMock(
|
||||
'../../../../app/src/Features/Uploads/DocumentConversionManager.mjs',
|
||||
() => ({
|
||||
default: (ctx.DocumentConversionManager = {
|
||||
promises: {
|
||||
convertProjectToDocument: sinon.stub(),
|
||||
},
|
||||
}),
|
||||
})
|
||||
)
|
||||
|
||||
vi.doMock('node:stream/promises', () => ({
|
||||
pipeline: (ctx.pipeline = sinon.stub().resolves()),
|
||||
}))
|
||||
|
||||
ctx.ProjectDownloadsController = (await import(modulePath)).default
|
||||
})
|
||||
|
||||
@@ -208,4 +238,107 @@ describe('ProjectDownloadsController', function () {
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('exportProjectConversion', function () {
|
||||
describe('when an unsupported type is requested', function () {
|
||||
beforeEach(async function (ctx) {
|
||||
ctx.req.params = { Project_id: 'test-project-id', type: 'unsupported' }
|
||||
ctx.req.session = { user: { _id: 'test-user-id' } }
|
||||
|
||||
await ctx.ProjectDownloadsController.exportProjectConversion(
|
||||
ctx.req,
|
||||
ctx.res,
|
||||
ctx.next
|
||||
)
|
||||
})
|
||||
|
||||
it('should return 400', function (ctx) {
|
||||
expect(ctx.res.statusCode).to.equal(400)
|
||||
})
|
||||
|
||||
it('should not call the conversion manager', function (ctx) {
|
||||
sinon.assert.notCalled(
|
||||
ctx.DocumentConversionManager.promises.convertProjectToDocument
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('with a supported type', function () {
|
||||
beforeEach(async function (ctx) {
|
||||
ctx.projectId = 'test-project-id'
|
||||
ctx.userId = 'test-user-id'
|
||||
ctx.projectName = 'My Test Project'
|
||||
ctx.exportStream = { pipe: sinon.stub() }
|
||||
ctx.contentLength = 9876
|
||||
|
||||
ctx.req.params = { Project_id: ctx.projectId, type: 'docx' }
|
||||
ctx.req.session = { user: { _id: ctx.userId } }
|
||||
ctx.req.ip = '192.168.1.1'
|
||||
|
||||
ctx.res.attachment = sinon.stub().returns(ctx.res)
|
||||
|
||||
ctx.SessionManager.getLoggedInUserId.returns(ctx.userId)
|
||||
ctx.ProjectGetter.promises.getProject.resolves({
|
||||
name: ctx.projectName,
|
||||
})
|
||||
ctx.DocumentConversionManager.promises.convertProjectToDocument.resolves(
|
||||
{
|
||||
stream: ctx.exportStream,
|
||||
contentLength: ctx.contentLength,
|
||||
}
|
||||
)
|
||||
|
||||
await ctx.ProjectDownloadsController.exportProjectConversion(
|
||||
ctx.req,
|
||||
ctx.res,
|
||||
ctx.next
|
||||
)
|
||||
})
|
||||
|
||||
it('should call convertProjectToDocument with the docx type', function (ctx) {
|
||||
sinon.assert.calledWith(
|
||||
ctx.DocumentConversionManager.promises.convertProjectToDocument,
|
||||
ctx.projectId,
|
||||
ctx.userId,
|
||||
'docx'
|
||||
)
|
||||
})
|
||||
|
||||
it('should set the Content-Length header', function (ctx) {
|
||||
expect(ctx.res.headers['Content-Length']).to.equal(ctx.contentLength)
|
||||
})
|
||||
|
||||
it('should set the attachment filename with safe project name', function (ctx) {
|
||||
sinon.assert.calledWith(ctx.res.attachment, 'My_Test_Project.docx')
|
||||
})
|
||||
|
||||
it('should set the X-Content-Type-Options header', function (ctx) {
|
||||
expect(ctx.res.headers['X-Content-Type-Options']).to.equal('nosniff')
|
||||
})
|
||||
|
||||
it('should set the X-Accel-Buffering header', function (ctx) {
|
||||
expect(ctx.res.headers['X-Accel-Buffering']).to.equal('no')
|
||||
})
|
||||
|
||||
it('should add an audit log entry', function (ctx) {
|
||||
sinon.assert.calledWith(
|
||||
ctx.ProjectAuditLogHandler.addEntryInBackground,
|
||||
ctx.projectId,
|
||||
'project-exported-docx',
|
||||
ctx.userId,
|
||||
ctx.req.ip
|
||||
)
|
||||
})
|
||||
|
||||
it('should record the action via Metrics', function (ctx) {
|
||||
ctx.Metrics.inc
|
||||
.calledWith('document-exports', 1, { type: 'docx' })
|
||||
.should.equal(true)
|
||||
})
|
||||
|
||||
it('should stream the document to the response', function (ctx) {
|
||||
sinon.assert.calledWith(ctx.pipeline, ctx.exportStream, ctx.res)
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -73,6 +73,18 @@ describe('DocumentConversionManager', function () {
|
||||
})
|
||||
)
|
||||
|
||||
ctx.ClsiManager = {
|
||||
promises: {
|
||||
buildDocumentConversionRequest: sinon
|
||||
.stub()
|
||||
.resolves({ some: 'clsi-request' }),
|
||||
},
|
||||
}
|
||||
|
||||
vi.doMock('../../../../app/src/Features/Compile/ClsiManager.mjs', () => ({
|
||||
default: ctx.ClsiManager,
|
||||
}))
|
||||
|
||||
ctx.DocumentConversionManager = (await import(MODULE_PATH)).default
|
||||
})
|
||||
|
||||
@@ -215,43 +227,64 @@ describe('DocumentConversionManager', function () {
|
||||
)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('when the Content-Length header is missing', function () {
|
||||
describe('convertProjectToDocument', function () {
|
||||
beforeEach(function (ctx) {
|
||||
ctx.projectId = 'test-project-id'
|
||||
ctx.userId = 'test-user-id'
|
||||
ctx.type = 'docx'
|
||||
ctx.mockStream = { destroy: sinon.stub() }
|
||||
ctx.response = {
|
||||
headers: { get: sinon.stub().returns(null) },
|
||||
}
|
||||
ctx.response.headers.get.withArgs('Content-Length').returns('50')
|
||||
ctx.fetchUtils.fetchStreamWithResponse.resolves({
|
||||
stream: ctx.mockStream,
|
||||
response: ctx.response,
|
||||
})
|
||||
})
|
||||
|
||||
describe('successfully', function () {
|
||||
beforeEach(async function (ctx) {
|
||||
ctx.path = '/path/to/input.docx'
|
||||
ctx.userId = 'test-user-id'
|
||||
ctx.response = {
|
||||
headers: {
|
||||
get: sinon.stub().returns(null),
|
||||
},
|
||||
}
|
||||
|
||||
ctx.fetchUtils.fetchStreamWithResponse.resolves({
|
||||
stream: 'mocked-fetch-stream',
|
||||
response: ctx.response,
|
||||
})
|
||||
|
||||
await expect(
|
||||
ctx.DocumentConversionManager.promises.convertDocxToLaTeXZipArchive(
|
||||
ctx.path,
|
||||
ctx.userId
|
||||
ctx.result =
|
||||
await ctx.DocumentConversionManager.promises.convertProjectToDocument(
|
||||
ctx.projectId,
|
||||
ctx.userId,
|
||||
ctx.type
|
||||
)
|
||||
).to.be.rejectedWith('document conversion failed')
|
||||
})
|
||||
|
||||
it('should not write the archive to disk', function (ctx) {
|
||||
sinon.assert.notCalled(ctx.fs.createWriteStream)
|
||||
sinon.assert.notCalled(ctx.nodeStream.pipeline)
|
||||
})
|
||||
|
||||
it('should attempt to clean up the output path', function (ctx) {
|
||||
it('should build the CLSI document conversion request', function (ctx) {
|
||||
sinon.assert.calledWith(
|
||||
ctx.fsPromises.unlink,
|
||||
sinon.match(
|
||||
/\/path\/to\/dump\/folder\/[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}_document-conversion\.zip/
|
||||
)
|
||||
ctx.ClsiManager.promises.buildDocumentConversionRequest,
|
||||
ctx.projectId
|
||||
)
|
||||
})
|
||||
|
||||
it('should call CLSI with the correct URL', function (ctx) {
|
||||
const expectedUrl = new URL(ctx.Settings.apis.clsi.url)
|
||||
expectedUrl.pathname = `/project/${ctx.projectId}/user/${ctx.userId}/download/project-to-document`
|
||||
expectedUrl.searchParams.set('type', ctx.type)
|
||||
expectedUrl.searchParams.set(
|
||||
'compileBackendClass',
|
||||
'test-backend-class'
|
||||
)
|
||||
expectedUrl.searchParams.set('compileGroup', 'test-compile-group')
|
||||
|
||||
sinon.assert.calledWith(
|
||||
ctx.fetchUtils.fetchStreamWithResponse,
|
||||
sinon.match(url => url.toString() === expectedUrl.toString()),
|
||||
{ method: 'POST', json: { some: 'clsi-request' } }
|
||||
)
|
||||
})
|
||||
|
||||
it('should return the stream and content length', function (ctx) {
|
||||
expect(ctx.result).to.deep.equal({
|
||||
stream: ctx.mockStream,
|
||||
contentLength: 50,
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user