From ddeafa1d7c5337f4ed046f68027447111978e0cc Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 16 Feb 2026 14:49:14 +0100 Subject: [PATCH] [web] fallback to clsi-cache from regular output file download (#31555) * [web] fallback to clsi-cache from regular output file download * [web] add comment on req.params assignment GitOrigin-RevId: 9d0c2e0a8e12d322aa5754c732a38f19adeac716 --- .../Features/Compile/ClsiCacheController.mjs | 30 ++++++ .../Features/Compile/CompileController.mjs | 34 ++++++ .../js/features/pdf-preview/util/file-list.ts | 11 +- .../features/pdf-preview/util/output-files.ts | 1 + ...ompile-and-download-project-pdf-button.tsx | 3 +- ...e-and-download-project-pdf-button.test.tsx | 9 +- .../frontend/helpers/editor-providers.tsx | 2 + .../src/Compile/CompileController.test.mjs | 101 ++++++++++++++++++ 8 files changed, 185 insertions(+), 6 deletions(-) diff --git a/services/web/app/src/Features/Compile/ClsiCacheController.mjs b/services/web/app/src/Features/Compile/ClsiCacheController.mjs index e2dbfdaac3..5148ac47ff 100644 --- a/services/web/app/src/Features/Compile/ClsiCacheController.mjs +++ b/services/web/app/src/Features/Compile/ClsiCacheController.mjs @@ -23,6 +23,35 @@ import Metrics from '@overleaf/metrics' */ async function downloadFromCache(req, res) { const { Project_id: projectId, buildId, filename } = req.params + return await _downloadFromCacheWithParams( + req, + res, + projectId, + buildId, + filename + ) +} + +/** + * Download a file from a specific build on the clsi-cache. + * + * @param req + * @param res + * @param projectId + * @param buildId + * @param filename + * @param projectId + * @param buildId + * @param filename + * @return {Promise<*>} + */ +async function _downloadFromCacheWithParams( + req, + res, + projectId, + buildId, + filename +) { const userId = CompileController._getUserIdForCompile(req) const signal = AbortSignal.timeout(60 * 1000) let location, projectName @@ -156,6 +185,7 @@ async function getLatestBuildFromCache(req, res) { } export default { + _downloadFromCacheWithParams, downloadFromCache: expressify(downloadFromCache), getLatestBuildFromCache: expressify(getLatestBuildFromCache), } diff --git a/services/web/app/src/Features/Compile/CompileController.mjs b/services/web/app/src/Features/Compile/CompileController.mjs index a518125257..857c1ae718 100644 --- a/services/web/app/src/Features/Compile/CompileController.mjs +++ b/services/web/app/src/Features/Compile/CompileController.mjs @@ -22,6 +22,7 @@ import { RequestFailedError, } from '@overleaf/fetch-utils' import Features from '../../infrastructure/Features.mjs' +import ClsiCacheController from './ClsiCacheController.mjs' const { z, zz, parseReq } = Validation const ClsiCookieManager = ClsiCookieManagerFactory( @@ -341,6 +342,8 @@ const _CompileController = { req.params.build_id, 'output.pdf' ) + // Align params with the generic output file download (via getFileFromClsi / getFileFromClsiWithoutUser). + req.params.file = 'output.pdf' await CompileController._proxyToClsi( projectId, 'output-file', @@ -580,6 +583,16 @@ const _CompileController = { timer.labels.status = 'success' timer.done() } catch (err) { + if (canTryClsiCacheFallback(req, res, action, err)) { + await ClsiCacheController._downloadFromCacheWithParams( + req, + res, + projectId, + `${req.query.editorId}-${req.params.build_id}`, + req.params.file + ) + return + } const reqAborted = Boolean(req.destroyed) const status = reqAborted ? 'req-aborted-late' : 'error' timer.labels.status = status @@ -676,6 +689,27 @@ async function _getPersistenceOptions( } } +function canTryClsiCacheFallback(req, res, action, err) { + const reqAborted = Boolean(req.destroyed) + const streamingStarted = Boolean(res.headersSent) + return ( + action === 'output-file' && + err instanceof RequestFailedError && + err.response.status === 404 && + !streamingStarted && + !reqAborted && + req.params.build_id && + req.query.editorId && + req.params.file && + // clsi-cache only has a small subset of files available outside the tar-ball + // The ClsiCacheHandler will validate the filename again. + (['output.log', 'output.pdf', 'output.synctex.gz'].includes( + req.params.file + ) || + req.params.file.endsWith('.blg')) + ) +} + const CompileController = { COMPILE_TIMEOUT_MS, compile: expressify(_CompileController.compile), diff --git a/services/web/frontend/js/features/pdf-preview/util/file-list.ts b/services/web/frontend/js/features/pdf-preview/util/file-list.ts index a8a37a9e2b..205ba978aa 100644 --- a/services/web/frontend/js/features/pdf-preview/util/file-list.ts +++ b/services/web/frontend/js/features/pdf-preview/util/file-list.ts @@ -1,6 +1,7 @@ import { CompileOutputFile, CompileResponseData, + PDFFile, } from '../../../../../types/compile' import { PdfFileDataList } from '@/features/pdf-preview/util/types' @@ -26,8 +27,14 @@ export function buildFileList( if (fromCache) { params.set('clsiserverid', clsiCacheShard || 'cache') - } else if (clsiServerId) { - params.set('clsiserverid', clsiServerId) + } else { + if (clsiServerId) { + params.set('clsiserverid', clsiServerId) + } + const pdf = outputFiles.get('output.pdf') as PDFFile + if (pdf) { + params.set('editorId', pdf.editorId) + } } if (compileGroup) { params.set('compileGroup', compileGroup) diff --git a/services/web/frontend/js/features/pdf-preview/util/output-files.ts b/services/web/frontend/js/features/pdf-preview/util/output-files.ts index dedbaa9a33..f869311d29 100644 --- a/services/web/frontend/js/features/pdf-preview/util/output-files.ts +++ b/services/web/frontend/js/features/pdf-preview/util/output-files.ts @@ -52,6 +52,7 @@ export function handleOutputFiles( } else { // build the URL for downloading the PDF params.set('popupDownload', 'true') // save PDF download as file + params.set('editorId', outputFile.editorId) outputFile.pdfDownloadUrl = `/download/project/${projectId}/build/${outputFile.build}/output/output.pdf?${params}` } diff --git a/services/web/frontend/js/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button.tsx b/services/web/frontend/js/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button.tsx index 6263d2789f..47ff920860 100644 --- a/services/web/frontend/js/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button.tsx +++ b/services/web/frontend/js/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button.tsx @@ -19,7 +19,7 @@ import OLIconButton from '@/shared/components/ol/ol-icon-button' import getMeta from '@/utils/meta' import { v4 as uuid } from 'uuid' -const FAKE_EDITOR_ID = uuid() +export const FAKE_EDITOR_ID = uuid() type CompileAndDownloadProjectPDFButtonProps = { project: Project @@ -102,6 +102,7 @@ function CompileAndDownloadProjectPDFButton({ const params = new URLSearchParams({ compileGroup: data.compileGroup, popupDownload: 'true', + editorId: FAKE_EDITOR_ID, }) if (data.clsiServerId) { params.set('clsiserverid', data.clsiServerId) diff --git a/services/web/test/frontend/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button.test.tsx b/services/web/test/frontend/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button.test.tsx index 3af8c889f9..0d4d08ceb6 100644 --- a/services/web/test/frontend/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button.test.tsx +++ b/services/web/test/frontend/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button.test.tsx @@ -3,7 +3,10 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react' import sinon from 'sinon' import { projectsData } from '../../../../fixtures/projects-data' import { location } from '@/shared/components/location' -import { CompileAndDownloadProjectPDFButtonTooltip } from '../../../../../../../../frontend/js/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button' +import { + CompileAndDownloadProjectPDFButtonTooltip, + FAKE_EDITOR_ID, +} from '@/features/project-list/components/table/cells/action-buttons/compile-and-download-project-pdf-button' import fetchMock from 'fetch-mock' import * as eventTracking from '@/infrastructure/event-tracking' import getMeta from '@/utils/meta' @@ -116,7 +119,7 @@ describe('', function () { expect(assignStub).to.have.been.calledOnce expect(assignStub).to.have.been.calledWith( - `/download/project/${projectsData[0].id}/build/123-321/output/output.pdf?compileGroup=standard&popupDownload=true&clsiserverid=server-1` + `/download/project/${projectsData[0].id}/build/123-321/output/output.pdf?compileGroup=standard&popupDownload=true&editorId=${FAKE_EDITOR_ID}&clsiserverid=server-1` ) expect(sendMBSpy).to.have.been.calledOnce @@ -154,7 +157,7 @@ describe('', function () { expect(assignStub).to.have.been.calledOnce expect(assignStub).to.have.been.calledWith( - `/download/project/${projectsData[0].id}/build/123-321/output/output.pdf?compileGroup=standard&popupDownload=true&clsiserverid=server-1` + `/download/project/${projectsData[0].id}/build/123-321/output/output.pdf?compileGroup=standard&popupDownload=true&editorId=${FAKE_EDITOR_ID}&clsiserverid=server-1` ) expect(sendMBSpy).to.have.been.calledOnce diff --git a/services/web/test/frontend/helpers/editor-providers.tsx b/services/web/test/frontend/helpers/editor-providers.tsx index 8b5035dd40..1b52991b07 100644 --- a/services/web/test/frontend/helpers/editor-providers.tsx +++ b/services/web/test/frontend/helpers/editor-providers.tsx @@ -56,6 +56,7 @@ import { type CompileContext } from '@/shared/context/local-compile-context' import { EditorContext } from '@/shared/context/editor-context' import { Cobranding } from '@ol-types/cobranding' import { TutorialContext } from '@/shared/context/tutorial-context' +import { EDITOR_SESSION_ID } from '@/features/pdf-preview/util/metrics' // these constants can be imported in tests instead of // using magic strings @@ -778,6 +779,7 @@ const makeDetachCompileProvider = (mockCompileOnLoad: boolean = false) => { const params = [ data.compileGroup && `compileGroup=${data.compileGroup}`, data.clsiServerId && `clsiserverid=${data.clsiServerId}`, + `editorId=${EDITOR_SESSION_ID}`, 'popupDownload=true', ] .filter(Boolean) diff --git a/services/web/test/unit/src/Compile/CompileController.test.mjs b/services/web/test/unit/src/Compile/CompileController.test.mjs index 501772f3e8..9e7109ba69 100644 --- a/services/web/test/unit/src/Compile/CompileController.test.mjs +++ b/services/web/test/unit/src/Compile/CompileController.test.mjs @@ -19,6 +19,9 @@ describe('CompileController', function () { compileTimeout: 100, }, } + ctx.ClsiCacheController = { + _downloadFromCacheWithParams: sinon.stub().resolves(), + } ctx.CompileManager = { promises: { compile: sinon.stub(), @@ -113,6 +116,13 @@ describe('CompileController', function () { }), })) + vi.doMock( + '../../../../app/src/Features/Compile/ClsiCacheController', + () => ({ + default: ctx.ClsiCacheController, + }) + ) + vi.doMock('../../../../app/src/Features/Compile/CompileManager', () => ({ default: ctx.CompileManager, })) @@ -801,6 +811,97 @@ describe('CompileController', function () { }) }) + describe('when the output.pdf does not exist', function () { + beforeEach(async function (ctx) { + ctx.req.params.file = 'output.pdf' + ctx.req.params.build_id = ctx.build_id + ctx.url = `/project/${ctx.projectId}/build/${ctx.build_id}/output/${ctx.req.params.file}` + ctx.editorId = '00000000-0000-0000-0000-000000000042' + ctx.req.query = { editorId: ctx.editorId } + ctx.CompileManager.promises.getProjectCompileLimits = sinon + .stub() + .resolves({ + compileGroup: 'priority', + compileBackendClass: 'c4d', + }) + ctx.fetchUtils.fetchStreamWithResponse.rejects( + new RequestFailedError( + `${ctx.settings.apis.clsi.url}${ctx.url}?compileGroup=priority&compileBackendClass=c4d`, + { method: 'GET' }, + { status: 404 } + ) + ) + await ctx.CompileController._proxyToClsi( + ctx.projectId, + 'output-file', + ctx.url, + {}, + ctx.req, + ctx.res, + ctx.next + ) + }) + + it('should open a request to the CLSI', function (ctx) { + ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith( + `${ctx.settings.apis.clsi.url}${ctx.url}?compileGroup=priority&compileBackendClass=c4d` + ) + }) + + it('should fallback to clsi-cache', function (ctx) { + ctx.ClsiCacheController._downloadFromCacheWithParams.should.have.been.calledWith( + ctx.req, + ctx.res, + ctx.projectId, + `${ctx.editorId}-${ctx.build_id}`, + 'output.pdf' + ) + }) + }) + describe('when the output.stderr does not exist', function () { + beforeEach(async function (ctx) { + ctx.req.params.file = 'output.stderr' + ctx.req.params.build_id = ctx.build_id + ctx.url = `/project/${ctx.projectId}/build/${ctx.build_id}/output/${ctx.req.params.file}` + ctx.editorId = '00000000-0000-0000-0000-000000000042' + ctx.req.query = { editorId: ctx.editorId } + ctx.CompileManager.promises.getProjectCompileLimits = sinon + .stub() + .resolves({ + compileGroup: 'priority', + compileBackendClass: 'c4d', + }) + ctx.fetchUtils.fetchStreamWithResponse.rejects( + new RequestFailedError( + `${ctx.settings.apis.clsi.url}${ctx.url}?compileGroup=priority&compileBackendClass=c4d`, + { method: 'GET' }, + { status: 404 } + ) + ) + await ctx.CompileController._proxyToClsi( + ctx.projectId, + 'output-file', + ctx.url, + {}, + ctx.req, + ctx.res, + ctx.next + ) + }) + + it('should open a request to the CLSI', function (ctx) { + ctx.fetchUtils.fetchStreamWithResponse.should.have.been.calledWith( + `${ctx.settings.apis.clsi.url}${ctx.url}?compileGroup=priority&compileBackendClass=c4d` + ) + }) + + it('should not fallback to clsi-cache', function (ctx) { + ctx.ClsiCacheController._downloadFromCacheWithParams.should.not.have + .been.called + ctx.res.statusCode.should.equal(404) + }) + }) + describe('user with standard priority via query string', function () { beforeEach(async function (ctx) { ctx.req.query = { compileGroup: 'standard' }