Explorar o código

[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
Jakob Ackermann hai 5 meses
pai
achega
ddeafa1d7c

+ 30 - 0
services/web/app/src/Features/Compile/ClsiCacheController.mjs

@@ -23,6 +23,35 @@ import Metrics from '@overleaf/metrics'
  */
  */
 async function downloadFromCache(req, res) {
 async function downloadFromCache(req, res) {
   const { Project_id: projectId, buildId, filename } = req.params
   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 userId = CompileController._getUserIdForCompile(req)
   const signal = AbortSignal.timeout(60 * 1000)
   const signal = AbortSignal.timeout(60 * 1000)
   let location, projectName
   let location, projectName
@@ -156,6 +185,7 @@ async function getLatestBuildFromCache(req, res) {
 }
 }
 
 
 export default {
 export default {
+  _downloadFromCacheWithParams,
   downloadFromCache: expressify(downloadFromCache),
   downloadFromCache: expressify(downloadFromCache),
   getLatestBuildFromCache: expressify(getLatestBuildFromCache),
   getLatestBuildFromCache: expressify(getLatestBuildFromCache),
 }
 }

+ 34 - 0
services/web/app/src/Features/Compile/CompileController.mjs

@@ -22,6 +22,7 @@ import {
   RequestFailedError,
   RequestFailedError,
 } from '@overleaf/fetch-utils'
 } from '@overleaf/fetch-utils'
 import Features from '../../infrastructure/Features.mjs'
 import Features from '../../infrastructure/Features.mjs'
+import ClsiCacheController from './ClsiCacheController.mjs'
 
 
 const { z, zz, parseReq } = Validation
 const { z, zz, parseReq } = Validation
 const ClsiCookieManager = ClsiCookieManagerFactory(
 const ClsiCookieManager = ClsiCookieManagerFactory(
@@ -341,6 +342,8 @@ const _CompileController = {
         req.params.build_id,
         req.params.build_id,
         'output.pdf'
         'output.pdf'
       )
       )
+      // Align params with the generic output file download (via getFileFromClsi / getFileFromClsiWithoutUser).
+      req.params.file = 'output.pdf'
       await CompileController._proxyToClsi(
       await CompileController._proxyToClsi(
         projectId,
         projectId,
         'output-file',
         'output-file',
@@ -580,6 +583,16 @@ const _CompileController = {
       timer.labels.status = 'success'
       timer.labels.status = 'success'
       timer.done()
       timer.done()
     } catch (err) {
     } 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 reqAborted = Boolean(req.destroyed)
       const status = reqAborted ? 'req-aborted-late' : 'error'
       const status = reqAborted ? 'req-aborted-late' : 'error'
       timer.labels.status = status
       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 = {
 const CompileController = {
   COMPILE_TIMEOUT_MS,
   COMPILE_TIMEOUT_MS,
   compile: expressify(_CompileController.compile),
   compile: expressify(_CompileController.compile),

+ 9 - 2
services/web/frontend/js/features/pdf-preview/util/file-list.ts

@@ -1,6 +1,7 @@
 import {
 import {
   CompileOutputFile,
   CompileOutputFile,
   CompileResponseData,
   CompileResponseData,
+  PDFFile,
 } from '../../../../../types/compile'
 } from '../../../../../types/compile'
 import { PdfFileDataList } from '@/features/pdf-preview/util/types'
 import { PdfFileDataList } from '@/features/pdf-preview/util/types'
 
 
@@ -26,8 +27,14 @@ export function buildFileList(
 
 
     if (fromCache) {
     if (fromCache) {
       params.set('clsiserverid', clsiCacheShard || 'cache')
       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) {
     if (compileGroup) {
       params.set('compileGroup', compileGroup)
       params.set('compileGroup', compileGroup)

+ 1 - 0
services/web/frontend/js/features/pdf-preview/util/output-files.ts

@@ -52,6 +52,7 @@ export function handleOutputFiles(
   } else {
   } else {
     // build the URL for downloading the PDF
     // build the URL for downloading the PDF
     params.set('popupDownload', 'true') // save PDF download as file
     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}`
     outputFile.pdfDownloadUrl = `/download/project/${projectId}/build/${outputFile.build}/output/output.pdf?${params}`
   }
   }

+ 2 - 1
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 getMeta from '@/utils/meta'
 import { v4 as uuid } from 'uuid'
 import { v4 as uuid } from 'uuid'
 
 
-const FAKE_EDITOR_ID = uuid()
+export const FAKE_EDITOR_ID = uuid()
 
 
 type CompileAndDownloadProjectPDFButtonProps = {
 type CompileAndDownloadProjectPDFButtonProps = {
   project: Project
   project: Project
@@ -102,6 +102,7 @@ function CompileAndDownloadProjectPDFButton({
           const params = new URLSearchParams({
           const params = new URLSearchParams({
             compileGroup: data.compileGroup,
             compileGroup: data.compileGroup,
             popupDownload: 'true',
             popupDownload: 'true',
+            editorId: FAKE_EDITOR_ID,
           })
           })
           if (data.clsiServerId) {
           if (data.clsiServerId) {
             params.set('clsiserverid', data.clsiServerId)
             params.set('clsiserverid', data.clsiServerId)

+ 6 - 3
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 sinon from 'sinon'
 import { projectsData } from '../../../../fixtures/projects-data'
 import { projectsData } from '../../../../fixtures/projects-data'
 import { location } from '@/shared/components/location'
 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 fetchMock from 'fetch-mock'
 import * as eventTracking from '@/infrastructure/event-tracking'
 import * as eventTracking from '@/infrastructure/event-tracking'
 import getMeta from '@/utils/meta'
 import getMeta from '@/utils/meta'
@@ -116,7 +119,7 @@ describe('<CompileAndDownloadProjectPDFButton />', function () {
 
 
     expect(assignStub).to.have.been.calledOnce
     expect(assignStub).to.have.been.calledOnce
     expect(assignStub).to.have.been.calledWith(
     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
     expect(sendMBSpy).to.have.been.calledOnce
@@ -154,7 +157,7 @@ describe('<CompileAndDownloadProjectPDFButton />', function () {
 
 
     expect(assignStub).to.have.been.calledOnce
     expect(assignStub).to.have.been.calledOnce
     expect(assignStub).to.have.been.calledWith(
     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
     expect(sendMBSpy).to.have.been.calledOnce

+ 2 - 0
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 { EditorContext } from '@/shared/context/editor-context'
 import { Cobranding } from '@ol-types/cobranding'
 import { Cobranding } from '@ol-types/cobranding'
 import { TutorialContext } from '@/shared/context/tutorial-context'
 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
 // these constants can be imported in tests instead of
 // using magic strings
 // using magic strings
@@ -778,6 +779,7 @@ const makeDetachCompileProvider = (mockCompileOnLoad: boolean = false) => {
           const params = [
           const params = [
             data.compileGroup && `compileGroup=${data.compileGroup}`,
             data.compileGroup && `compileGroup=${data.compileGroup}`,
             data.clsiServerId && `clsiserverid=${data.clsiServerId}`,
             data.clsiServerId && `clsiserverid=${data.clsiServerId}`,
+            `editorId=${EDITOR_SESSION_ID}`,
             'popupDownload=true',
             'popupDownload=true',
           ]
           ]
             .filter(Boolean)
             .filter(Boolean)

+ 101 - 0
services/web/test/unit/src/Compile/CompileController.test.mjs

@@ -19,6 +19,9 @@ describe('CompileController', function () {
         compileTimeout: 100,
         compileTimeout: 100,
       },
       },
     }
     }
+    ctx.ClsiCacheController = {
+      _downloadFromCacheWithParams: sinon.stub().resolves(),
+    }
     ctx.CompileManager = {
     ctx.CompileManager = {
       promises: {
       promises: {
         compile: sinon.stub(),
         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', () => ({
     vi.doMock('../../../../app/src/Features/Compile/CompileManager', () => ({
       default: ctx.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 () {
       describe('user with standard priority via query string', function () {
         beforeEach(async function (ctx) {
         beforeEach(async function (ctx) {
           ctx.req.query = { compileGroup: 'standard' }
           ctx.req.query = { compileGroup: 'standard' }