Răsfoiți Sursa

Merge pull request #18190 from overleaf/ar-add-download-all-link

[web] Add download all link for output files

GitOrigin-RevId: a275f17b6acf3130250d91462ba01588b93b62f5
Andrew Rumble 2 ani în urmă
părinte
comite
105d67bd04

+ 35 - 9
services/clsi/app/js/CompileController.js

@@ -12,6 +12,17 @@ function timeSinceLastSuccessfulCompile() {
   return Date.now() - lastSuccessfulCompileTimestamp
 }
 
+function addUrlToOutputFile(outputFile, projectId, userId) {
+  return {
+    url:
+      `${Settings.apis.clsi.url}/project/${projectId}` +
+      (userId != null ? `/user/${userId}` : '') +
+      (outputFile.build != null ? `/build/${outputFile.build}` : '') +
+      `/output/${outputFile.path}`,
+    ...outputFile,
+  }
+}
+
 function compile(req, res, next) {
   const timer = new Metrics.Timer('compile-request')
   RequestParser.parse(req.body, function (error, request) {
@@ -30,7 +41,7 @@ function compile(req, res, next) {
           return next(error)
         }
         CompileManager.doCompileWithLock(request, (error, result) => {
-          let { outputFiles, stats, timings } = result || {}
+          let { buildId, outputFiles, stats, timings } = result || {}
           let code, status
           if (outputFiles == null) {
             outputFiles = []
@@ -98,6 +109,7 @@ function compile(req, res, next) {
 
           if (error) {
             outputFiles = error.outputFiles || []
+            buildId = error.buildId
           }
 
           timer.done()
@@ -108,14 +120,28 @@ function compile(req, res, next) {
               stats,
               timings,
               outputUrlPrefix: Settings.apis.clsi.outputUrlPrefix,
-              outputFiles: outputFiles.map(file => ({
-                url:
-                  `${Settings.apis.clsi.url}/project/${request.project_id}` +
-                  (request.user_id != null ? `/user/${request.user_id}` : '') +
-                  (file.build != null ? `/build/${file.build}` : '') +
-                  `/output/${file.path}`,
-                ...file,
-              })),
+              outputFiles:
+                outputFiles.length === 0
+                  ? []
+                  : outputFiles
+                      .map(file =>
+                        addUrlToOutputFile(
+                          file,
+                          request.project_id,
+                          request.user_id
+                        )
+                      )
+                      .concat(
+                        addUrlToOutputFile(
+                          {
+                            build: buildId,
+                            path: 'output.zip',
+                            type: 'zip',
+                          },
+                          request.project_id,
+                          request.user_id
+                        )
+                      ),
             },
           })
         })

+ 10 - 6
services/clsi/app/js/CompileManager.js

@@ -208,7 +208,7 @@ async function doCompile(request) {
       Metrics.inc('compiles-timeout', 1, request.metricsOpts)
     }
 
-    const { outputFiles, allEntries } = await _saveOutputFiles({
+    const { outputFiles, allEntries, buildId } = await _saveOutputFiles({
       request,
       compileDir,
       resourceList,
@@ -216,7 +216,7 @@ async function doCompile(request) {
       timings,
     })
     error.outputFiles = outputFiles // return output files so user can check logs
-
+    error.buildId = buildId
     // Clear project if this compile was abruptly terminated
     if (error.terminated || error.timedout) {
       await clearProjectWithListing(
@@ -280,7 +280,7 @@ async function doCompile(request) {
   // Emit compile time.
   timings.compile = ts
 
-  const { outputFiles } = await _saveOutputFiles({
+  const { outputFiles, buildId } = await _saveOutputFiles({
     request,
     compileDir,
     resourceList,
@@ -296,7 +296,7 @@ async function doCompile(request) {
     emitPdfStats(stats, timings, request)
   }
 
-  return { outputFiles, stats, timings }
+  return { outputFiles, stats, timings, buildId }
 }
 
 async function _saveOutputFiles({
@@ -316,20 +316,24 @@ async function _saveOutputFiles({
   let { outputFiles, allEntries } =
     await OutputFileFinder.promises.findOutputFiles(resourceList, compileDir)
 
+  let buildId
+
   try {
-    outputFiles = await OutputCacheManager.promises.saveOutputFiles(
+    const saveResult = await OutputCacheManager.promises.saveOutputFiles(
       { request, stats, timings },
       outputFiles,
       compileDir,
       outputDir
     )
+    buildId = saveResult.buildId
+    outputFiles = saveResult.outputFiles
   } catch (err) {
     const { project_id: projectId, user_id: userId } = request
     logger.err({ projectId, userId, err }, 'failed to save output files')
   }
 
   timings.output = timer.done()
-  return { outputFiles, allEntries }
+  return { outputFiles, allEntries, buildId }
 }
 
 async function stopCompile(projectId, userId) {

+ 4 - 4
services/clsi/app/js/OutputCacheManager.js

@@ -164,7 +164,7 @@ module.exports = OutputCacheManager = {
             outputDir,
             stats,
             (err, outputFiles) => {
-              if (err) return callback(err, outputFiles)
+              if (err) return callback(err, { outputFiles, buildId })
 
               const enablePdfCaching = request.enablePdfCaching
               const enablePdfCachingDark =
@@ -173,7 +173,7 @@ module.exports = OutputCacheManager = {
                 !Settings.enablePdfCaching ||
                 (!enablePdfCaching && !enablePdfCachingDark)
               ) {
-                return callback(null, outputFiles)
+                return callback(null, { outputFiles, buildId })
               }
 
               OutputCacheManager.saveStreamsInContentDir(
@@ -191,9 +191,9 @@ module.exports = OutputCacheManager = {
                       { err, outputDir, stats, timings },
                       'pdf caching failed'
                     )
-                    return callback(null, outputFiles)
+                    return callback(null, { outputFiles, buildId })
                   }
-                  callback(err, outputFiles)
+                  callback(err, { outputFiles, buildId })
                 }
               )
             }

+ 65 - 23
services/clsi/test/unit/js/CompileControllerTests.js

@@ -50,6 +50,7 @@ function tryImageNameValidation(method, imageNameField) {
 
 describe('CompileController', function () {
   beforeEach(function () {
+    this.buildId = 'build-id-123'
     this.CompileController = SandboxedModule.require(modulePath, {
       requires: {
         './CompileManager': (this.CompileManager = {}),
@@ -118,6 +119,7 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
+          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -140,21 +142,32 @@ describe('CompileController', function () {
 
       it('should return the JSON response', function () {
         this.res.status.calledWith(200).should.equal(true)
-        this.res.send
-          .calledWith({
-            compile: {
+        console.log(this.res.send.args[0][0].compile)
+        sinon.assert.calledWith(
+          this.res.send,
+          sinon.match.has(
+            'compile',
+            sinon.match({
               status: 'success',
               error: null,
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '/zone/b',
-              outputFiles: this.output_files.map(file => ({
-                url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
-                ...file,
-              })),
-            },
-          })
-          .should.equal(true)
+              outputFiles: [
+                ...this.output_files.map(file => ({
+                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
+                  ...file,
+                })),
+                {
+                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`,
+                  build: this.buildId,
+                  path: 'output.zip',
+                  type: 'zip',
+                },
+              ],
+            })
+          )
+        )
       })
     })
 
@@ -165,6 +178,7 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
+          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -179,10 +193,18 @@ describe('CompileController', function () {
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '',
-              outputFiles: this.output_files.map(file => ({
-                url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
-                ...file,
-              })),
+              outputFiles: [
+                ...this.output_files.map(file => ({
+                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
+                  ...file,
+                })),
+                {
+                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`,
+                  build: this.buildId,
+                  path: 'output.zip',
+                  type: 'zip',
+                },
+              ],
             },
           })
           .should.equal(true)
@@ -207,6 +229,7 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
+          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -221,10 +244,18 @@ describe('CompileController', function () {
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '/zone/b',
-              outputFiles: this.output_files.map(file => ({
-                url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
-                ...file,
-              })),
+              outputFiles: [
+                ...this.output_files.map(file => ({
+                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
+                  ...file,
+                })),
+                {
+                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`,
+                  build: this.buildId,
+                  path: 'output.zip',
+                  type: 'zip',
+                },
+              ],
             },
           })
           .should.equal(true)
@@ -250,6 +281,7 @@ describe('CompileController', function () {
           outputFiles: this.output_files,
           stats: this.stats,
           timings: this.timings,
+          buildId: this.buildId,
         })
         this.CompileController.compile(this.req, this.res)
       })
@@ -264,10 +296,18 @@ describe('CompileController', function () {
               stats: this.stats,
               timings: this.timings,
               outputUrlPrefix: '/zone/b',
-              outputFiles: this.output_files.map(file => ({
-                url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
-                ...file,
-              })),
+              outputFiles: [
+                ...this.output_files.map(file => ({
+                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${file.build}/output/${file.path}`,
+                  ...file,
+                })),
+                {
+                  url: `${this.Settings.apis.clsi.url}/project/${this.project_id}/build/${this.buildId}/output/output.zip`,
+                  build: this.buildId,
+                  path: 'output.zip',
+                  type: 'zip',
+                },
+              ],
             },
           })
           .should.equal(true)
@@ -276,9 +316,11 @@ describe('CompileController', function () {
 
     describe('with an error', function () {
       beforeEach(function () {
+        const error = new Error((this.message = 'error message'))
+        error.buildId = this.buildId
         this.CompileManager.doCompileWithLock = sinon
           .stub()
-          .callsArgWith(1, new Error((this.message = 'error message')), null)
+          .callsArgWith(1, error, null)
         this.CompileController.compile(this.req, this.res)
       })
 

+ 4 - 1
services/clsi/test/unit/js/CompileManagerTests.js

@@ -35,6 +35,7 @@ describe('CompileManager', function () {
         build: 1234,
       },
     ]
+    this.buildId = 'build-id-123'
     this.commandOutput = 'Dummy output'
     this.compileBaseDir = '/compile/dir'
     this.outputBaseDir = '/output/dir'
@@ -61,7 +62,9 @@ describe('CompileManager', function () {
     }
     this.OutputCacheManager = {
       promises: {
-        saveOutputFiles: sinon.stub().resolves(this.buildFiles),
+        saveOutputFiles: sinon
+          .stub()
+          .resolves({ outputFiles: this.buildFiles, buildId: this.buildId }),
       },
     }
     this.Settings = {

+ 26 - 6
services/web/app/src/Features/Compile/CompileController.js

@@ -398,6 +398,13 @@ module.exports = CompileController = {
       if (error) {
         return next(error)
       }
+
+      const qs = {}
+
+      if (req.params.file === 'output.zip') {
+        qs.files = req.query.files
+      }
+
       const url = CompileController._getFileUrl(
         projectId,
         userId,
@@ -408,7 +415,7 @@ module.exports = CompileController = {
         projectId,
         'output-file',
         url,
-        {},
+        qs,
         req,
         res,
         next
@@ -573,10 +580,20 @@ module.exports = CompileController = {
           return next(err)
         }
         url = new URL(`${Settings.apis.clsi.url}${url}`)
-        url.search = new URLSearchParams({
-          ...persistenceOptions.qs,
-          ...qs,
-        }).toString()
+
+        const params = new URLSearchParams(persistenceOptions.qs)
+
+        for (const [key, value] of Object.entries(qs)) {
+          if (Array.isArray(value)) {
+            for (const v of value) {
+              params.append(key, v)
+            }
+            continue
+          }
+          params.append(key, value)
+        }
+
+        url.search = params.toString()
         const timer = new Metrics.Timer(
           'proxy_to_clsi',
           1,
@@ -604,7 +621,10 @@ module.exports = CompileController = {
             })
 
             for (const key of ['Content-Length', 'Content-Type']) {
-              res.setHeader(key, response.headers.get(key))
+              const headerValue = response.headers.get(key)
+              if (headerValue) {
+                res.setHeader(key, headerValue)
+              }
             }
             res.writeHead(response.status)
             return pipeline(stream, res)

+ 1 - 0
services/web/frontend/extracted-translations.json

@@ -305,6 +305,7 @@
   "doing_this_will_verify_affiliation_and_allow_log_in_2": "",
   "done": "",
   "download": "",
+  "download_all": "",
   "download_metadata": "",
   "download_pdf": "",
   "download_zip_file": "",

+ 16 - 0
services/web/frontend/js/features/pdf-preview/components/pdf-file-list.jsx

@@ -33,6 +33,18 @@ function PdfFileList({ fileList }) {
           <b>{file.path}</b>
         </MenuItem>
       ))}
+
+      {fileList.other.length + fileList.top.length > 0 && fileList.archive && (
+        <MenuItem
+          download={basename(fileList.archive)}
+          href={fileList.archive.url}
+          key={fileList.archive.path}
+        >
+          <b>
+            {t('download_all')} ({fileList.other.length + fileList.top.length})
+          </b>
+        </MenuItem>
+      )}
     </>
   )
 }
@@ -48,6 +60,10 @@ PdfFileList.propTypes = {
   fileList: PropTypes.shape({
     top: FilesArray,
     other: FilesArray,
+    archive: PropTypes.shape({
+      path: PropTypes.string.isRequired,
+      url: PropTypes.string.isRequired,
+    }),
   }),
 }
 

+ 13 - 1
services/web/frontend/js/features/pdf-preview/util/file-list.js

@@ -1,5 +1,5 @@
 const topFileTypes = ['bbl', 'gls', 'ind']
-const ignoreFiles = ['output.fls', 'output.fdb_latexmk']
+const ignoreFiles = ['output.fls', 'output.fdb_latexmk', 'output.zip']
 
 export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
   const files = { top: [], other: [] }
@@ -18,6 +18,8 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
 
     const allFiles = []
 
+    let outputArchiveFile
+
     // filter out ignored files and set some properties
     for (const file of outputFiles.values()) {
       if (!ignoreFiles.includes(file.path)) {
@@ -28,6 +30,8 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
         }
 
         allFiles.push(file)
+      } else if (file.type === 'zip') {
+        outputArchiveFile = file
       }
     }
 
@@ -52,6 +56,14 @@ export const buildFileList = (outputFiles, clsiServerId, compileGroup) => {
         files.other.push(file)
       }
     }
+
+    if (outputArchiveFile) {
+      allFiles.forEach(
+        file => file.type !== 'pdf' && params.append('files', file.path)
+      )
+      outputArchiveFile.url += `?${params.toString()}`
+      files.archive = outputArchiveFile
+    }
   }
 
   return files

+ 1 - 0
services/web/locales/en.json

@@ -430,6 +430,7 @@
   "done": "Done",
   "dont_have_account": "Don’t have an account?",
   "download": "Download",
+  "download_all": "Download all",
   "download_metadata": "Download Overleaf metadata",
   "download_pdf": "Download PDF",
   "download_zip_file": "Download .zip file",