Merge pull request #22183 from overleaf/jpa-zip-from-hash

[web] build project zip using history-v1 blobs with filestore fallback

GitOrigin-RevId: 2e197ae83e7ac8bcfe44091c327b721825b13a05

services/web/app/src/Features/Downloads/ProjectZipStreamManager.mjs

@@ -3,7 +3,7 @@ import async from 'async'
 import logger from '@overleaf/logger'
 import ProjectEntityHandler from '../Project/ProjectEntityHandler.js'
 import ProjectGetter from '../Project/ProjectGetter.js'
-import FileStoreHandler from '../FileStore/FileStoreHandler.js'
+import HistoryManager from '../History/HistoryManager.js'
 let ProjectZipStreamManager
 
 export default ProjectZipStreamManager = {
@@ -114,11 +114,11 @@ export default ProjectZipStreamManager = {
         return callback(error)
       }
       const jobs = Object.entries(files).map(([path, file]) => cb => {
-        FileStoreHandler.getFileStream(
+        HistoryManager.requestBlobWithFallback(
           projectId,
+          file.hash,
           file._id,
-          {},
-          (error, stream) => {
+          (error, result) => {
             if (error) {
               logger.warn(
                 { err: error, projectId, fileId: file._id },
@@ -129,6 +129,7 @@ export default ProjectZipStreamManager = {
             if (path[0] === '/') {
               path = path.slice(1)
             }
+            const { stream } = result
             archive.append(stream, { name: path })
             stream.on('end', () => cb())
           }

services/web/test/acceptance/src/HistoryTests.mjs

@@ -6,6 +6,8 @@ import MockV1HistoryApiClass from './mocks/MockV1HistoryApi.js'
 import ProjectGetter from '../../../app/src/Features/Project/ProjectGetter.js'
 import MockFilestoreApiClass from './mocks/MockFilestoreApi.js'
 import { fileURLToPath } from 'node:url'
+import sinon from 'sinon'
+import logger from '@overleaf/logger'
 import Metrics from './helpers/metrics.js'
 const User = UserHelper.promises
 
@@ -64,6 +66,41 @@ describe('HistoryTests', function () {
     expect(await getSourceMetric('filestore')).to.equal(filestoreSource)
   }
 
+  describe('/project/:projectId/download/zip', function () {
+    let spy, downloadZIPURL
+    beforeEach(async function () {
+      spy = sinon.spy(logger, 'error')
+      downloadZIPURL = `/project/${projectId}/download/zip`
+    })
+    afterEach(function () {
+      spy.restore()
+    })
+    it('should work from history-v1', async function () {
+      const { response, body } = await user.doRequest('GET', downloadZIPURL)
+      expect(response.statusCode).to.equal(200)
+      expect(body).to.include('2pixel.png')
+      await expectHistoryV1Hit()
+    })
+    it('should work from filestore', async function () {
+      MockV1HistoryApi.reset()
+      const { response, body } = await user.doRequest('GET', downloadZIPURL)
+      expect(response.statusCode).to.equal(200)
+      expect(body).to.include('2pixel.png')
+      await expectFilestoreHit()
+    })
+    it('should not include when missing in both places', async function () {
+      MockFilestoreApi.reset()
+      MockV1HistoryApi.reset()
+      const { response, body } = await user.doRequest('GET', downloadZIPURL)
+      expect(response.statusCode).to.equal(200)
+      expect(
+        spy.args.find(([, msg]) => msg === 'error adding files to zip stream')
+      ).to.exist
+      expect(body).to.not.include('2pixel.png')
+      await expectNoIncrement()
+    })
+  })
+
   describe('/project/:projectId/blob/:hash', function () {
     describe('HEAD', function () {
       it('should fetch the file size from history-v1', async function () {

services/web/test/unit/src/Downloads/ProjectZipStreamManagerTests.mjs

@@ -32,8 +32,8 @@ describe('ProjectZipStreamManager', function () {
       '@overleaf/logger': this.logger,
       '../../../../app/src/Features/Project/ProjectEntityHandler':
         (this.ProjectEntityHandler = {}),
-      '../../../../app/src/Features/FileStore/FileStoreHandler':
-        (this.FileStoreHandler = {}),
+      '../../../../app/src/Features/History/HistoryManager.js':
+        (this.HistoryManager = {}),
       '../../../../app/src/Features/Project/ProjectGetter':
         (this.ProjectGetter = {}),
     }))
@@ -366,9 +366,11 @@ describe('ProjectZipStreamManager', function () {
       this.files = {
         '/image.png': {
           _id: 'file-id-1',
+          hash: 'abc',
         },
         '/folder/picture.png': {
           _id: 'file-id-2',
+          hash: 'def',
         },
       }
       this.streams = {
@@ -378,11 +380,15 @@ describe('ProjectZipStreamManager', function () {
       this.ProjectEntityHandler.getAllFiles = sinon
         .stub()
         .callsArgWith(1, null, this.files)
-      this.FileStoreHandler.getFileStream = (projectId, fileId, ...rest) => {
-        const [, callback] = rest
-        return callback(null, this.streams[fileId])
+      this.HistoryManager.requestBlobWithFallback = (
+        projectId,
+        hash,
+        fileId,
+        callback
+      ) => {
+        return callback(null, { stream: this.streams[fileId] })
       }
-      sinon.spy(this.FileStoreHandler, 'getFileStream')
+      sinon.spy(this.HistoryManager, 'requestBlobWithFallback')
       this.ProjectZipStreamManager.addAllFilesToArchive(
         this.project_id,
         this.archive,
@@ -410,8 +416,8 @@ describe('ProjectZipStreamManager', function () {
         for (const path in this.files) {
           const file = this.files[path]
           result.push(
-            this.FileStoreHandler.getFileStream
-              .calledWith(this.project_id, file._id)
+            this.HistoryManager.requestBlobWithFallback
+              .calledWith(this.project_id, file.hash, file._id)
               .should.equal(true)
           )
         }