From 62d92b70dd9e509375203cdcf6e6af98dd3e4e72 Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Mon, 11 May 2026 12:29:44 +0100 Subject: [PATCH] Merge pull request #33341 from overleaf/mj-two-step-export-web [web] Add two-step pandoc conversion download GitOrigin-RevId: 093f435a497a7583d2b4d23558415cc442f84553 --- .../app/src/Features/Compile/ClsiManager.mjs | 8 +- .../Downloads/ProjectDownloadsController.mjs | 105 +++++++-- .../Uploads/DocumentConversionManager.mjs | 32 ++- services/web/app/src/router.mjs | 13 ++ .../web/frontend/extracted-translations.json | 4 +- .../ide-react/components/global-toasts.tsx | 21 +- .../components/toolbar/download-project.tsx | 25 +- .../toolbar/export-document-toasts.tsx | 57 ++++- .../ide-react/hooks/use-convert-project.ts | 44 ++++ services/web/locales/en.json | 4 +- .../ProjectDownloadsController.test.mjs | 214 +++++++++++++++--- .../DocumentConversionManager.test.mjs | 109 ++++++++- 12 files changed, 554 insertions(+), 82 deletions(-) create mode 100644 services/web/frontend/js/features/ide-react/hooks/use-convert-project.ts diff --git a/services/web/app/src/Features/Compile/ClsiManager.mjs b/services/web/app/src/Features/Compile/ClsiManager.mjs index e2802af54e..d56e634b77 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.mjs +++ b/services/web/app/src/Features/Compile/ClsiManager.mjs @@ -444,7 +444,7 @@ async function _makeRequest( timer.done() let newClsiServerId if (CLSI_COOKIES_ENABLED) { - newClsiServerId = _getClsiServerIdFromResponse(response) + newClsiServerId = getClsiServerIdFromResponse(response) await ClsiCookieManager.promises.setServerId( projectId, userId, @@ -603,7 +603,7 @@ async function _makeNewBackendRequest( timer.done() let newClsiServerId if (CLSI_COOKIES_ENABLED) { - newClsiServerId = _getClsiServerIdFromResponse(response) + newClsiServerId = getClsiServerIdFromResponse(response) await NewBackendCloudClsiCookieManager.promises.setServerId( projectId, userId, @@ -1268,7 +1268,7 @@ async function syncTeX( } } -function _getClsiServerIdFromResponse(response) { +function getClsiServerIdFromResponse(response) { const setCookieHeaders = response.headers.raw()['set-cookie'] ?? [] for (const header of setCookieHeaders) { const cookie = Cookie.parse(header) @@ -1307,6 +1307,8 @@ export default { getOutputFileStream: callbackify(getOutputFileStream), wordCount: callbackify(wordCount), syncTeX: callbackify(syncTeX), + getClsiServerIdFromResponse, + CLSI_COOKIES_ENABLED, promises: { sendRequest, sendExternalRequest, diff --git a/services/web/app/src/Features/Downloads/ProjectDownloadsController.mjs b/services/web/app/src/Features/Downloads/ProjectDownloadsController.mjs index b39bb558ee..f8a964e45e 100644 --- a/services/web/app/src/Features/Downloads/ProjectDownloadsController.mjs +++ b/services/web/app/src/Features/Downloads/ProjectDownloadsController.mjs @@ -1,4 +1,5 @@ import Metrics from '@overleaf/metrics' +import Settings from '@overleaf/settings' import ProjectGetter from '../Project/ProjectGetter.mjs' import ProjectZipStreamManager from './ProjectZipStreamManager.mjs' import DocumentUpdaterHandler from '../DocumentUpdater/DocumentUpdaterHandler.mjs' @@ -6,56 +7,130 @@ import { prepareZipAttachment } from '../../infrastructure/Response.mjs' import SessionManager from '../Authentication/SessionManager.mjs' import ProjectAuditLogHandler from '../Project/ProjectAuditLogHandler.mjs' import DocumentConversionManager from '../Uploads/DocumentConversionManager.mjs' +import Validation from '../../infrastructure/Validation.mjs' import { expressify } from '@overleaf/promise-utils' import { pipeline } from 'node:stream/promises' +const { z, zz, parseReq } = Validation + const SUPPORTED_CONVERSION_TYPES = new Map([ ['docx', 'docx'], ['markdown', 'zip'], ]) +const exportProjectConversionSchema = z.object({ + params: z.object({ + Project_id: zz.objectId(), + type: z.enum([...SUPPORTED_CONVERSION_TYPES.keys()]), + }), + query: z.object({ + responseFormat: z.enum(['json', 'stream']).optional().default('stream'), + }), +}) + +const downloadPreparedProjectExportSchema = z.object({ + params: z.object({ + Project_id: zz.objectId(), + buildId: zz.buildId(), + conversionId: z.uuid(), + file: zz.filepath(), + type: z.enum([...SUPPORTED_CONVERSION_TYPES.keys()]), + }), + query: z.object({ + clsiserverid: zz.clsiServerId().optional(), + }), +}) + // Keep in sync with the logic for PDF files in CompileController function getSafeProjectName(project) { return project.name.replace(/[^\p{L}\p{Nd}]/gu, '_') } -async function exportProjectConversion(req, res) { - const type = req.params.type +async function _streamConvertedDocumentToResponse( + res, + { projectId, type, conversionId, buildId, clsiServerId, file } +) { const extension = SUPPORTED_CONVERSION_TYPES.get(type) - if (!extension) { - return res.sendStatus(400) - } - const userId = SessionManager.getLoggedInUserId(req.session) - const projectId = req.params.Project_id - Metrics.inc('document-exports', 1, { type }) - const project = await ProjectGetter.promises.getProject(projectId, { name: true, }) - const safeFileName = getSafeProjectName(project) const { stream, contentLength } = + await DocumentConversionManager.promises.streamConvertedProjectDocument({ + conversionId, + buildId, + clsiServerId, + file, + }) + res.setHeader('Content-Length', contentLength) + res.attachment(`${safeFileName}.${extension}`) + res.setHeader('X-Content-Type-Options', 'nosniff') + res.setHeader('X-Accel-Buffering', 'no') + await pipeline(stream, res) +} + +async function exportProjectConversion(req, res) { + const { params, query } = parseReq(req, exportProjectConversionSchema) + const { Project_id: projectId, type } = params + const { responseFormat } = query + const userId = SessionManager.getLoggedInUserId(req.session) + Metrics.inc('document-exports', 1, { type }) + + const { conversionId, buildId, clsiServerId, file } = await DocumentConversionManager.promises.convertProjectToDocument( projectId, userId, type ) - res.setHeader('Content-Length', contentLength) - res.attachment(`${safeFileName}.${extension}`) - res.setHeader('X-Content-Type-Options', 'nosniff') - res.setHeader('X-Accel-Buffering', 'no') ProjectAuditLogHandler.addEntryInBackground( projectId, `project-exported-${type}`, userId, req.ip ) - await pipeline(stream, res) + + if (responseFormat === 'json') { + const downloadUrl = new URL( + `/project/${projectId}/download/conversion/${conversionId}/${type}/build/${buildId}/output/${file}`, + Settings.siteUrl + ) + if (clsiServerId) { + downloadUrl.searchParams.set('clsiserverid', clsiServerId) + } + return res.json({ + downloadUrl: downloadUrl.pathname + downloadUrl.search, + }) + } + + await _streamConvertedDocumentToResponse(res, { + projectId, + type, + conversionId, + buildId, + clsiServerId, + file, + }) +} + +async function downloadPreparedProjectExport(req, res) { + const { params, query } = parseReq(req, downloadPreparedProjectExportSchema) + const { Project_id: projectId, conversionId, buildId, file, type } = params + const { clsiserverid: clsiServerId } = query + + await _streamConvertedDocumentToResponse(res, { + projectId, + type, + conversionId, + buildId, + clsiServerId, + file, + }) } export default { exportProjectConversion: expressify(exportProjectConversion), + downloadPreparedProjectExport: expressify(downloadPreparedProjectExport), downloadProject(req, res, next) { const userId = SessionManager.getLoggedInUserId(req.session) diff --git a/services/web/app/src/Features/Uploads/DocumentConversionManager.mjs b/services/web/app/src/Features/Uploads/DocumentConversionManager.mjs index 5654f12830..8324da982b 100644 --- a/services/web/app/src/Features/Uploads/DocumentConversionManager.mjs +++ b/services/web/app/src/Features/Uploads/DocumentConversionManager.mjs @@ -1,11 +1,15 @@ import Settings from '@overleaf/settings' import CompileManager from '../Compile/CompileManager.mjs' import ClsiManager from '../Compile/ClsiManager.mjs' +import { getOutputFileURL } from '../Compile/ClsiURLHelpers.mjs' import fs from 'node:fs' import fsPromises from 'node:fs/promises' import logger from '@overleaf/logger' import Path from 'node:path' -import { fetchStreamWithResponse } from '@overleaf/fetch-utils' +import { + fetchJsonWithResponse, + fetchStreamWithResponse, +} from '@overleaf/fetch-utils' import { pipeline } from 'node:stream/promises' import OError from '@overleaf/o-error' import FormData from 'form-data' @@ -85,6 +89,7 @@ async function convertProjectToDocument(projectId, userId, type) { const clsiUrl = new URL(Settings.apis.clsi.url) clsiUrl.pathname = `/project/${projectId}/user/${userId}/download/project-to-document` clsiUrl.searchParams.set('type', type) + clsiUrl.searchParams.set('responseFormat', 'json') clsiUrl.searchParams.set('compileBackendClass', limits.compileBackendClass) clsiUrl.searchParams.set('compileGroup', limits.compileGroup) @@ -93,11 +98,33 @@ async function convertProjectToDocument(projectId, userId, type) { 'sending project to CLSI for document conversion' ) - const { stream, response } = await fetchStreamWithResponse(clsiUrl, { + const { json, response } = await fetchJsonWithResponse(clsiUrl, { method: 'POST', json: clsiRequest, }) + const { conversionId, buildId, file } = json + const clsiServerId = ClsiManager.CLSI_COOKIES_ENABLED + ? ClsiManager.getClsiServerIdFromResponse(response) + : undefined + return { conversionId, buildId, clsiServerId, file } +} + +async function streamConvertedProjectDocument({ + conversionId, + buildId, + clsiServerId, + file, +}) { + const downloadUrl = getOutputFileURL( + conversionId, + null, + buildId, + file, + clsiServerId ?? undefined + ) + + const { stream, response } = await fetchStreamWithResponse(downloadUrl) const contentLength = parseInt(response.headers.get('Content-Length'), 10) return { stream, contentLength } @@ -107,5 +134,6 @@ export default { promises: { convertDocumentToLaTeXZipArchive, convertProjectToDocument, + streamConvertedProjectDocument, }, } diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs index 8e27a91563..251066a07d 100644 --- a/services/web/app/src/router.mjs +++ b/services/web/app/src/router.mjs @@ -197,6 +197,10 @@ const rateLimiters = { points: 5, duration: 60, }), + documentExportDownload: new RateLimiter('document-export-download', { + points: 30, + duration: 60, + }), } async function initialize(webRouter, privateApiRouter, publicApiRouter) { @@ -759,6 +763,15 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) { AuthorizationMiddleware.ensureUserCanReadProject, ProjectDownloadsController.exportProjectConversion ) + webRouter.get( + '/project/:Project_id/download/conversion/:conversionId/:type/build/:buildId/output/:file(.*)', + AuthenticationController.requireLogin(), + RateLimiterMiddleware.rateLimit(rateLimiters.documentExportDownload, { + params: ['Project_id'], + }), + AuthorizationMiddleware.ensureUserCanReadProject, + ProjectDownloadsController.downloadPreparedProjectExport + ) } webRouter.get( diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 2316222e54..3b801d4baf 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -634,7 +634,6 @@ "export_as_docx": "", "export_as_markdown": "", "export_csv": "", - "export_document_error": "", "export_project_to_github": "", "failed": "", "failed_to_consent_to_workbench_terms": "", @@ -1433,6 +1432,7 @@ "premium": "", "premium_feature": "", "premium_plan_label": "", + "preparing_for_export": "", "presentation_mode": "", "press_shift_space_for_suggestions": "", "press_space_to_open_the_ai_assistant": "", @@ -1980,6 +1980,7 @@ "the_code_editor_color_scheme": "", "the_code_editor_color_scheme_dark_mode": "", "the_code_editor_color_scheme_light_mode": "", + "the_document_contains_formatting_we_werent_able_to_convert_contact_support_if_you_need_help": "", "the_following_files_already_exist_in_this_project": "", "the_following_files_and_folders_already_exist_in_this_project": "", "the_following_folder_already_exists_in_this_project": "", @@ -2317,6 +2318,7 @@ "warnings": "", "we_are_unable_to_opt_you_into_this_experiment": "", "we_cant_find_any_sections_or_subsections_in_this_file": "", + "we_couldnt_export_this_document": "", "we_do_not_share_personal_information": "", "we_got_your_request": "", "we_logged_you_in": "", diff --git a/services/web/frontend/js/features/ide-react/components/global-toasts.tsx b/services/web/frontend/js/features/ide-react/components/global-toasts.tsx index 938dc7a662..8e799d2a18 100644 --- a/services/web/frontend/js/features/ide-react/components/global-toasts.tsx +++ b/services/web/frontend/js/features/ide-react/components/global-toasts.tsx @@ -40,7 +40,7 @@ let toastCounter = 1 export const GlobalToasts = memo(function GlobalToasts() { const [toasts, setToasts] = useState< - { component: ReactElement; id: string }[] + { component: ReactElement; id: string; handle?: string }[] >([]) const removeToast = useCallback((id: string) => { @@ -72,13 +72,13 @@ export const GlobalToasts = memo(function GlobalToasts() { ) const addToast = useCallback( - (key: string, data?: any) => { + (key: string, handle?: string, data?: any) => { const id = `toast-${toastCounter++}` const component = createToast(id, key, data) if (!component) { return } - setToasts(current => [...current, { id, component }]) + setToasts(current => [...current, { id, handle, component }]) }, [createToast] ) @@ -89,14 +89,25 @@ export const GlobalToasts = memo(function GlobalToasts() { debugConsole.error('No key provided for toast') return } - const { key, ...rest } = event.detail - addToast(key, rest) + const { key, handle, ...rest } = event.detail + addToast(key, handle, rest) }, [addToast] ) useEventListener('ide:show-toast', showToastListener) + const dismissToastListener = useCallback((event: CustomEvent) => { + const { handle } = event.detail || {} + if (!handle) { + debugConsole.error('No handle provided for dismissing toast') + return + } + setToasts(current => current.filter(toast => toast.handle !== handle)) + }, []) + + useEventListener('ide:dismiss-toast', dismissToastListener) + return ( {toasts.map(({ component, id }) => ( diff --git a/services/web/frontend/js/features/ide-react/components/toolbar/download-project.tsx b/services/web/frontend/js/features/ide-react/components/toolbar/download-project.tsx index db5ffd0713..139eb78ff3 100644 --- a/services/web/frontend/js/features/ide-react/components/toolbar/download-project.tsx +++ b/services/web/frontend/js/features/ide-react/components/toolbar/download-project.tsx @@ -9,6 +9,7 @@ import { useTranslation } from 'react-i18next' import { useEditorAnalytics } from '@/shared/hooks/use-editor-analytics' import getMeta from '@/utils/meta' import { useFeatureFlag } from '@/shared/context/split-test-context' +import useConvertProject from '../../hooks/use-convert-project' export const DownloadProjectZip = () => { const { t } = useTranslation() @@ -105,11 +106,11 @@ export const DownloadProjectPDF = () => { export const ExportProjectDocx = () => { const { t } = useTranslation() - const { projectId } = useProjectContext() const exportDocxEnabled = useFeatureFlag('export-docx') const enablePandocConversions = getMeta('ol-ExposedSettings')?.enablePandocConversions const anonymous = getMeta('ol-anonymous') + const downloadConversion = useConvertProject('docx') const showExportDocx = exportDocxEnabled && enablePandocConversions && !anonymous @@ -120,12 +121,12 @@ export const ExportProjectDocx = () => { ? [ { id: 'export-as-docx', - href: `/project/${projectId}/download/conversion/docx`, + handler: downloadConversion, label: t('export_as_docx'), }, ] : [], - [t, showExportDocx, projectId] + [t, showExportDocx, downloadConversion] ) if (!showExportDocx) { @@ -133,11 +134,7 @@ export const ExportProjectDocx = () => { } return ( - + {t('export_as_docx')} ) @@ -145,11 +142,11 @@ export const ExportProjectDocx = () => { export const ExportProjectMarkdown = () => { const { t } = useTranslation() - const { projectId } = useProjectContext() const exportMarkdownEnabled = useFeatureFlag('export-markdown') const enablePandocConversions = getMeta('ol-ExposedSettings')?.enablePandocConversions const anonymous = getMeta('ol-anonymous') + const downloadConversion = useConvertProject('markdown') const showExportMarkdown = exportMarkdownEnabled && enablePandocConversions && !anonymous @@ -160,12 +157,12 @@ export const ExportProjectMarkdown = () => { ? [ { id: 'export-as-markdown', - href: `/project/${projectId}/download/conversion/markdown`, + handler: downloadConversion, label: t('export_as_markdown'), }, ] : [], - [t, showExportMarkdown, projectId] + [t, showExportMarkdown, downloadConversion] ) if (!showExportMarkdown) { @@ -173,11 +170,7 @@ export const ExportProjectMarkdown = () => { } return ( - + {t('export_as_markdown')} ) diff --git a/services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx b/services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx index eb037fd662..34ab19187d 100644 --- a/services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx +++ b/services/web/frontend/js/features/ide-react/components/toolbar/export-document-toasts.tsx @@ -1,9 +1,27 @@ import { GlobalToastGeneratorEntry } from '@/features/ide-react/components/global-toasts' -import { useTranslation } from 'react-i18next' +import { Trans, useTranslation } from 'react-i18next' + +const PreparingExportToast = () => { + const { t } = useTranslation() + return {t('preparing_for_export')} +} const ExportDocumentErrorToast = () => { const { t } = useTranslation() - return {t('export_document_error')} + return ( + <> +

+ {t('we_couldnt_export_this_document')} +

+ , + ]} + /> + + ) } const generators: GlobalToastGeneratorEntry[] = [ @@ -17,6 +35,41 @@ const generators: GlobalToastGeneratorEntry[] = [ isDismissible: true, }), }, + { + key: 'export-document:preparing', + generator: () => ({ + content: , + type: 'info', + autoHide: false, + isDismissible: true, + }), + }, ] export default generators + +export const showExportDocumentError = () => { + window.dispatchEvent( + new CustomEvent('ide:show-toast', { + detail: { key: 'export-document:error' }, + }) + ) +} + +export const showPreparingExportToast = () => { + const handle = `export-document-preparing-${Date.now()}` + window.dispatchEvent( + new CustomEvent('ide:show-toast', { + detail: { key: 'export-document:preparing', handle }, + }) + ) + return handle +} + +export const hidePreparingExportToast = (handle: string) => { + window.dispatchEvent( + new CustomEvent('ide:dismiss-toast', { + detail: { key: 'export-document:preparing', handle }, + }) + ) +} diff --git a/services/web/frontend/js/features/ide-react/hooks/use-convert-project.ts b/services/web/frontend/js/features/ide-react/hooks/use-convert-project.ts new file mode 100644 index 0000000000..bc1924cf89 --- /dev/null +++ b/services/web/frontend/js/features/ide-react/hooks/use-convert-project.ts @@ -0,0 +1,44 @@ +import { getJSON } from '@/infrastructure/fetch-json' +import { useLocation } from '@/shared/hooks/use-location' +import { debugConsole } from '@/utils/debugging' +import { useProjectContext } from '@/shared/context/project-context' +import { useCallback } from 'react' +import { + hidePreparingExportToast, + showExportDocumentError, + showPreparingExportToast, +} from '../components/toolbar/export-document-toasts' + +const SLOW_CONVERSION_THRESHOLD = 2000 + +export default function useConvertProject(type: 'docx' | 'markdown') { + const { projectId } = useProjectContext() + const location = useLocation() + const triggerConversion = useCallback(async () => { + let handle: string | undefined + const toastTimer = setTimeout(() => { + handle = showPreparingExportToast() + }, SLOW_CONVERSION_THRESHOLD) + const hidePreparingToast = () => { + clearTimeout(toastTimer) + if (handle) hidePreparingExportToast(handle) + } + try { + const response = await getJSON( + `/project/${projectId}/download/conversion/${type}?responseFormat=json` + ) + hidePreparingToast() + const { downloadUrl } = response + if (downloadUrl) { + const url = new URL(downloadUrl, window.location.origin) + location.assign(url.toString()) + } + } catch (error) { + hidePreparingToast() + showExportDocumentError() + debugConsole.error(error) + } + }, [projectId, type, location]) + + return triggerConversion +} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index f2e278783b..49610a3194 100644 --- a/services/web/locales/en.json +++ b/services/web/locales/en.json @@ -839,7 +839,6 @@ "export_as_docx": "Export as Word document (.docx)", "export_as_markdown": "Export as Markdown (.md)", "export_csv": "Export CSV", - "export_document_error": "Export failed. Please try again.", "export_project_to_github": "Export Project to GitHub", "failed": "Failed", "failed_to_consent_to_workbench_terms": "Failed to consent to workbench terms. Please try again later.", @@ -1909,6 +1908,7 @@ "premium": "Premium", "premium_feature": "Premium feature", "premium_plan_label": "You’re using Overleaf Premium", + "preparing_for_export": "Preparing for export…", "presentation": "Presentation", "presentation_mode": "Presentation mode", "press_shift_space_for_suggestions": "Press Shift+Space for suggestions", @@ -2586,6 +2586,7 @@ "the_code_editor_color_scheme": "The code editor color scheme", "the_code_editor_color_scheme_dark_mode": "The code editor color scheme for dark mode", "the_code_editor_color_scheme_light_mode": "The code editor color scheme for light mode", + "the_document_contains_formatting_we_werent_able_to_convert_contact_support_if_you_need_help": "The document contains formatting we weren’t able to convert. <0>Contact support if you need help.", "the_file_supplied_is_of_an_unsupported_type ": "The link to open this content on Overleaf pointed to the wrong kind of file. Valid file types are .tex documents and .zip files. If this keeps happening for links on a particular site, please report this to them.", "the_following_files_already_exist_in_this_project": "The following files already exist in this project:", "the_following_files_and_folders_already_exist_in_this_project": "The following files and folders already exist in this project:", @@ -2967,6 +2968,7 @@ "we_are_unable_to_opt_you_into_this_experiment": "We are unable to opt you into this experiment at this time, please ensure your organization has allowed this feature, or try again later.", "we_cant_confirm_this_email": "We can’t confirm this email", "we_cant_find_any_sections_or_subsections_in_this_file": "We can’t find any sections or subsections in this file", + "we_couldnt_export_this_document": "We couldn’t export this document", "we_do_not_share_personal_information": "See our <0>Privacy Notice for details of how we treat your personal data", "we_got_your_request": "We’ve got your request", "we_logged_you_in": "We have logged you in.", diff --git a/services/web/test/unit/src/Downloads/ProjectDownloadsController.test.mjs b/services/web/test/unit/src/Downloads/ProjectDownloadsController.test.mjs index 1c7164e5f3..fc663acf80 100644 --- a/services/web/test/unit/src/Downloads/ProjectDownloadsController.test.mjs +++ b/services/web/test/unit/src/Downloads/ProjectDownloadsController.test.mjs @@ -68,11 +68,18 @@ describe('ProjectDownloadsController', function () { default: (ctx.DocumentConversionManager = { promises: { convertProjectToDocument: sinon.stub(), + streamConvertedProjectDocument: sinon.stub(), }, }), }) ) + vi.doMock('@overleaf/settings', () => ({ + default: (ctx.Settings = { + siteUrl: 'https://overleaf.example.com', + }), + })) + vi.doMock('node:stream/promises', () => ({ pipeline: (ctx.pipeline = sinon.stub().resolves()), })) @@ -240,39 +247,23 @@ describe('ProjectDownloadsController', function () { }) describe('exportProjectConversion', function () { - describe('when an unsupported type is requested', function () { + describe('with a supported type (default streaming)', 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.projectId = '5e9b1c2a3b4c5d6e7f8a9b0c' ctx.userId = 'test-user-id' ctx.projectName = 'My Test Project' ctx.exportStream = { pipe: sinon.stub() } ctx.contentLength = 9876 + ctx.conversionIds = { + conversionId: '12345678-1234-4234-8234-123456789012', + buildId: '0123456789a-0123456789abcdef', + clsiServerId: 'clsi-server-1', + file: 'output.docx', + } ctx.req.params = { Project_id: ctx.projectId, type: 'docx' } ctx.req.session = { user: { _id: ctx.userId } } + ctx.req.query = {} ctx.req.ip = '192.168.1.1' ctx.res.attachment = sinon.stub().returns(ctx.res) @@ -282,6 +273,9 @@ describe('ProjectDownloadsController', function () { name: ctx.projectName, }) ctx.DocumentConversionManager.promises.convertProjectToDocument.resolves( + ctx.conversionIds + ) + ctx.DocumentConversionManager.promises.streamConvertedProjectDocument.resolves( { stream: ctx.exportStream, contentLength: ctx.contentLength, @@ -304,6 +298,13 @@ describe('ProjectDownloadsController', function () { ) }) + it('should fetch the prepared document via streamConvertedProjectDocument', function (ctx) { + sinon.assert.calledWith( + ctx.DocumentConversionManager.promises.streamConvertedProjectDocument, + ctx.conversionIds + ) + }) + it('should set the Content-Length header', function (ctx) { expect(ctx.res.headers['Content-Length']).to.equal(ctx.contentLength) }) @@ -341,9 +342,93 @@ describe('ProjectDownloadsController', function () { }) }) + describe('with responseFormat=json', function () { + beforeEach(async function (ctx) { + ctx.projectId = '5e9b1c2a3b4c5d6e7f8a9b0c' + ctx.userId = 'test-user-id' + ctx.req.params = { Project_id: ctx.projectId, type: 'docx' } + ctx.req.query = { responseFormat: 'json' } + ctx.req.session = { user: { _id: ctx.userId } } + ctx.req.ip = '192.168.1.1' + + ctx.res.json = sinon.stub().returns(ctx.res) + + ctx.SessionManager.getLoggedInUserId.returns(ctx.userId) + ctx.DocumentConversionManager.promises.convertProjectToDocument.resolves( + { + conversionId: '12345678-1234-4234-8234-123456789012', + buildId: '0123456789a-0123456789abcdef', + clsiServerId: 'clsi-server-1', + file: 'output.docx', + } + ) + + await ctx.ProjectDownloadsController.exportProjectConversion( + ctx.req, + ctx.res, + ctx.next + ) + }) + + it('should call convertProjectToDocument', function (ctx) { + sinon.assert.calledWith( + ctx.DocumentConversionManager.promises.convertProjectToDocument, + ctx.projectId, + ctx.userId, + 'docx' + ) + }) + + it('should not stream a document', function (ctx) { + sinon.assert.notCalled( + ctx.DocumentConversionManager.promises.streamConvertedProjectDocument + ) + sinon.assert.notCalled(ctx.pipeline) + }) + + 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 respond with a download URL pointing at the prepared output route', function (ctx) { + sinon.assert.calledOnce(ctx.res.json) + const arg = ctx.res.json.firstCall.args[0] + expect(arg.downloadUrl).to.equal( + '/project/5e9b1c2a3b4c5d6e7f8a9b0c/download/conversion/12345678-1234-4234-8234-123456789012/docx/build/0123456789a-0123456789abcdef/output/output.docx?clsiserverid=clsi-server-1' + ) + }) + + it('should omit the clsiserverid param when no clsiServerId is returned', async function (ctx) { + ctx.res.json.resetHistory() + ctx.DocumentConversionManager.promises.convertProjectToDocument.resolves( + { + conversionId: '12345678-1234-4234-8234-123456789012', + buildId: '0123456789a-0123456789abcdef', + clsiServerId: null, + file: 'output.docx', + } + ) + + await ctx.ProjectDownloadsController.exportProjectConversion( + ctx.req, + ctx.res, + ctx.next + ) + const arg = ctx.res.json.firstCall.args[0] + const url = new URL(arg.downloadUrl, 'http://localhost') + expect(url.searchParams.has('clsiserverid')).to.equal(false) + }) + }) + describe('with type=markdown', function () { beforeEach(async function (ctx) { - ctx.projectId = 'test-project-id' + ctx.projectId = '5e9b1c2a3b4c5d6e7f8a9b0c' ctx.userId = 'test-user-id' ctx.projectName = 'My Test Project' ctx.exportStream = { pipe: sinon.stub() } @@ -351,6 +436,7 @@ describe('ProjectDownloadsController', function () { ctx.req.params = { Project_id: ctx.projectId, type: 'markdown' } ctx.req.session = { user: { _id: ctx.userId } } + ctx.req.query = {} ctx.req.ip = '192.168.1.1' ctx.res.attachment = sinon.stub().returns(ctx.res) @@ -360,6 +446,14 @@ describe('ProjectDownloadsController', function () { name: ctx.projectName, }) ctx.DocumentConversionManager.promises.convertProjectToDocument.resolves( + { + conversionId: '12345678-1234-4234-8234-123456789012', + buildId: '0123456789a-0123456789abcdef', + clsiServerId: 'clsi-server-1', + file: 'output.zip', + } + ) + ctx.DocumentConversionManager.promises.streamConvertedProjectDocument.resolves( { stream: ctx.exportStream, contentLength: ctx.contentLength, @@ -407,4 +501,72 @@ describe('ProjectDownloadsController', function () { }) }) }) + + describe('downloadPreparedProjectExport', function () { + describe('with a supported type', function () { + beforeEach(async function (ctx) { + ctx.projectId = '5e9b1c2a3b4c5d6e7f8a9b0c' + ctx.projectName = 'My Test Project' + ctx.exportStream = { pipe: sinon.stub() } + ctx.contentLength = 9876 + + ctx.req.params = { + Project_id: ctx.projectId, + type: 'docx', + conversionId: '12345678-1234-4234-8234-123456789012', + buildId: '0123456789a-0123456789abcdef', + file: 'output.docx', + } + ctx.req.query = { + clsiserverid: 'clsi-server-1', + } + + ctx.res.attachment = sinon.stub().returns(ctx.res) + + ctx.ProjectGetter.promises.getProject.resolves({ + name: ctx.projectName, + }) + ctx.DocumentConversionManager.promises.streamConvertedProjectDocument.resolves( + { + stream: ctx.exportStream, + contentLength: ctx.contentLength, + } + ) + + await ctx.ProjectDownloadsController.downloadPreparedProjectExport( + ctx.req, + ctx.res, + ctx.next + ) + }) + + it('should call streamConvertedProjectDocument with the query params', function (ctx) { + sinon.assert.calledWith( + ctx.DocumentConversionManager.promises.streamConvertedProjectDocument, + { + conversionId: '12345678-1234-4234-8234-123456789012', + buildId: '0123456789a-0123456789abcdef', + clsiServerId: 'clsi-server-1', + file: 'output.docx', + } + ) + }) + + 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 Content-Length header', function (ctx) { + expect(ctx.res.headers['Content-Length']).to.equal(ctx.contentLength) + }) + + it('should stream the document to the response', function (ctx) { + sinon.assert.calledWith(ctx.pipeline, ctx.exportStream, ctx.res) + }) + + it('should not log an audit entry', function (ctx) { + sinon.assert.notCalled(ctx.ProjectAuditLogHandler.addEntryInBackground) + }) + }) + }) }) diff --git a/services/web/test/unit/src/Uploads/DocumentConversionManager.test.mjs b/services/web/test/unit/src/Uploads/DocumentConversionManager.test.mjs index 9a4175705d..42cf4677fc 100644 --- a/services/web/test/unit/src/Uploads/DocumentConversionManager.test.mjs +++ b/services/web/test/unit/src/Uploads/DocumentConversionManager.test.mjs @@ -18,6 +18,7 @@ describe('DocumentConversionManager', function () { } ctx.fetchUtils = { + fetchJsonWithResponse: sinon.stub().resolves(), fetchStreamWithResponse: sinon.stub().resolves(), } @@ -42,6 +43,7 @@ describe('DocumentConversionManager', function () { apis: { clsi: { url: 'http://mock-clsi-url', + downloadHost: 'http://mock-clsi-download-host', }, }, } @@ -55,6 +57,7 @@ describe('DocumentConversionManager', function () { })) vi.doMock('@overleaf/fetch-utils', () => ({ + fetchJsonWithResponse: ctx.fetchUtils.fetchJsonWithResponse, fetchStreamWithResponse: ctx.fetchUtils.fetchStreamWithResponse, })) @@ -72,8 +75,11 @@ describe('DocumentConversionManager', function () { default: ctx.CompileManager, }) ) + ctx.getClsiServerIdFromResponse = sinon.stub().returns('mock-clsi-server') ctx.ClsiManager = { + getClsiServerIdFromResponse: ctx.getClsiServerIdFromResponse, + CLSI_COOKIES_ENABLED: true, promises: { buildDocumentConversionRequest: sinon .stub() @@ -300,18 +306,21 @@ describe('DocumentConversionManager', function () { 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, + ctx.conversionId = '12345678-1234-4234-8234-123456789012' + ctx.buildId = '0123456789a-0123456789abcdef' + ctx.file = 'output.docx' + ctx.postResponse = { headers: {} } + ctx.fetchUtils.fetchJsonWithResponse.resolves({ + json: { + conversionId: ctx.conversionId, + buildId: ctx.buildId, + file: ctx.file, + }, + response: ctx.postResponse, }) }) - describe('successfully converts the document', function () { + describe('successfully', function () { beforeEach(async function (ctx) { ctx.result = await ctx.DocumentConversionManager.promises.convertProjectToDocument( @@ -328,10 +337,11 @@ describe('DocumentConversionManager', function () { ) }) - it('should call CLSI with the correct URL', function (ctx) { + it('should POST to CLSI with responseFormat=json and compile metadata', 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('responseFormat', 'json') expectedUrl.searchParams.set( 'compileBackendClass', 'test-backend-class' @@ -339,12 +349,71 @@ describe('DocumentConversionManager', function () { expectedUrl.searchParams.set('compileGroup', 'test-compile-group') sinon.assert.calledWith( - ctx.fetchUtils.fetchStreamWithResponse, + ctx.fetchUtils.fetchJsonWithResponse, sinon.match(url => url.toString() === expectedUrl.toString()), { method: 'POST', json: { some: 'clsi-request' } } ) }) + it('should extract clsiServerId from the POST response cookie', function (ctx) { + sinon.assert.calledWith( + ctx.getClsiServerIdFromResponse, + ctx.postResponse + ) + }) + + it('should return the conversion identifiers', function (ctx) { + expect(ctx.result).to.deep.equal({ + conversionId: ctx.conversionId, + buildId: ctx.buildId, + clsiServerId: 'mock-clsi-server', + file: ctx.file, + }) + }) + }) + }) + + describe('streamConvertedProjectDocument', function () { + beforeEach(function (ctx) { + ctx.conversionId = '12345678-1234-4234-8234-123456789012' + ctx.buildId = '0123456789a-0123456789abcdef' + ctx.file = 'output.docx' + ctx.clsiServerId = 'clsi-server-1' + 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('with a clsiServerId', function () { + beforeEach(async function (ctx) { + ctx.result = + await ctx.DocumentConversionManager.promises.streamConvertedProjectDocument( + { + conversionId: ctx.conversionId, + buildId: ctx.buildId, + clsiServerId: ctx.clsiServerId, + file: ctx.file, + } + ) + }) + + it('should GET the file from clsi-nginx with the clsiserverid query param', function (ctx) { + const expectedUrl = new URL(ctx.Settings.apis.clsi.downloadHost) + expectedUrl.pathname = `/project/${ctx.conversionId}/build/${ctx.buildId}/output/${ctx.file}` + expectedUrl.searchParams.set('clsiserverid', ctx.clsiServerId) + + sinon.assert.calledWith( + ctx.fetchUtils.fetchStreamWithResponse, + sinon.match(url => url.toString() === expectedUrl.toString()) + ) + }) + it('should return the stream and content length', function (ctx) { expect(ctx.result).to.deep.equal({ stream: ctx.mockStream, @@ -352,5 +421,23 @@ describe('DocumentConversionManager', function () { }) }) }) + + describe('without a clsiServerId', function () { + beforeEach(async function (ctx) { + await ctx.DocumentConversionManager.promises.streamConvertedProjectDocument( + { + conversionId: ctx.conversionId, + buildId: ctx.buildId, + clsiServerId: undefined, + file: ctx.file, + } + ) + }) + + it('should not include the clsiserverid query param', function (ctx) { + const url = ctx.fetchUtils.fetchStreamWithResponse.firstCall.args[0] + expect(url.searchParams.has('clsiserverid')).to.equal(false) + }) + }) }) })