[web] restore downloading from history-v1 via legacy file endpoint (#23450)

GitOrigin-RevId: f6d12cfe445c7c41a62a563c9a5e7089bf94344f

services/web/app/src/Features/FileStore/FileStoreController.mjs

@@ -20,11 +20,11 @@ async function getFile(req, res) {
 
   let file
   try {
-    file = await ProjectLocator.promises.findElement({
+    ;({ element: file } = await ProjectLocator.promises.findElement({
       project_id: projectId,
       element_id: fileId,
       type: 'file',
-    })
+    }))
   } catch (err) {
     if (err instanceof Errors.NotFoundError) {
       logger.warn(
@@ -55,11 +55,11 @@ async function getFile(req, res) {
     status: Boolean(file?.hash),
   })
 
-  let stream, contentLength
+  let source, stream, contentLength
   try {
     if (Features.hasFeature('project-history-blobs') && file?.hash) {
       // Get the file from history
-      ;({ stream, contentLength } =
+      ;({ source, stream, contentLength } =
         await HistoryManager.promises.requestBlobWithFallback(
           projectId,
           file.hash,
@@ -72,6 +72,7 @@ async function getFile(req, res) {
         fileId,
         queryString
       )
+      source = 'filestore'
     }
   } catch (err) {
     if (err instanceof Errors.NotFoundError) {
@@ -96,6 +97,7 @@ async function getFile(req, res) {
   // allow the browser to cache these immutable files
   // note: both "private" and "max-age" appear to be required for caching
   res.setHeader('Cache-Control', 'private, max-age=3600')
+  res.appendHeader('X-Served-By', source)
   try {
     await pipeline(stream, res)
   } catch (err) {
@@ -117,11 +119,11 @@ async function getFileHead(req, res) {
 
   let file
   try {
-    file = await ProjectLocator.promises.findElement({
+    ;({ element: file } = await ProjectLocator.promises.findElement({
       project_id: projectId,
       element_id: fileId,
       type: 'file',
-    })
+    }))
   } catch (err) {
     if (err instanceof Errors.NotFoundError) {
       // res.sendStatus() sends a description of the status as body.
@@ -148,19 +150,19 @@ async function getFileHead(req, res) {
     status: Boolean(file?.hash),
   })
 
-  let fileSize
+  let fileSize, source
   try {
     if (Features.hasFeature('project-history-blobs') && file?.hash) {
-      const { contentLength } =
+      ;({ source, contentLength: fileSize } =
         await HistoryManager.promises.requestBlobWithFallback(
           projectId,
           file.hash,
           fileId,
           'HEAD'
-        )
-      fileSize = contentLength
+        ))
     } else {
       fileSize = await FileStoreHandler.promises.getFileSize(projectId, fileId)
+      source = 'filestore'
     }
   } catch (err) {
     if (err instanceof Errors.NotFoundError) {
@@ -172,6 +174,7 @@ async function getFileHead(req, res) {
   }
 
   res.setHeader('Content-Length', fileSize)
+  res.appendHeader('X-Served-By', source)
   res.status(200).end()
 }
 

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

@@ -26,7 +26,7 @@ const fileContent = fs.readFileSync(
 )
 
 describe('HistoryTests', function () {
-  let user, projectId, fileId, fileHash, fileURL, fileURLWithFallback
+  let user, projectId, fileId, fileHash, fileURL, blobURL, blobURLWithFallback
   let historySource, filestoreSource
 
   async function getSourceMetric(source) {
@@ -48,8 +48,9 @@ describe('HistoryTests', function () {
         '2pixel.png',
         'image/png'
       ))
-    fileURL = `/project/${projectId}/blob/${fileHash}`
-    fileURLWithFallback = `${fileURL}?fallback=${fileId}`
+    fileURL = `/project/${projectId}/file/${fileId}`
+    blobURL = `/project/${projectId}/blob/${fileHash}`
+    blobURLWithFallback = `${blobURL}?fallback=${fileId}`
     historySource = await getSourceMetric('history-v1')
     filestoreSource = await getSourceMetric('filestore')
   })
@@ -108,7 +109,7 @@ describe('HistoryTests', function () {
     describe('HEAD', function () {
       if (Features.hasFeature('project-history-blobs')) {
         it('should fetch the file size from history-v1', async function () {
-          const { response } = await user.doRequest('HEAD', fileURL)
+          const { response } = await user.doRequest('HEAD', blobURL)
           expect(response.statusCode).to.equal(200)
           expect(response.headers['x-served-by']).to.include('history-v1')
           expect(response.headers['content-length']).to.equal('3694')
@@ -117,13 +118,13 @@ describe('HistoryTests', function () {
       }
       it('should return 404 without fallback', async function () {
         MockV1HistoryApi.reset()
-        const { response } = await user.doRequest('HEAD', fileURL)
+        const { response } = await user.doRequest('HEAD', blobURL)
         expect(response.statusCode).to.equal(404)
         await expectNoIncrement()
       })
       it('should fetch the file size from filestore when missing in history-v1', async function () {
         MockV1HistoryApi.reset()
-        const { response } = await user.doRequest('HEAD', fileURLWithFallback)
+        const { response } = await user.doRequest('HEAD', blobURLWithFallback)
         expect(response.statusCode).to.equal(200)
         expect(response.headers['x-served-by']).to.include('filestore')
         expect(response.headers['content-length']).to.equal('3694')
@@ -132,7 +133,7 @@ describe('HistoryTests', function () {
       it('should return 404 with both files missing', async function () {
         MockFilestoreApi.reset()
         MockV1HistoryApi.reset()
-        const { response } = await user.doRequest('HEAD', fileURLWithFallback)
+        const { response } = await user.doRequest('HEAD', blobURLWithFallback)
         expect(response.statusCode).to.equal(404)
         await expectNoIncrement()
       })
@@ -140,14 +141,14 @@ describe('HistoryTests', function () {
     describe('GET', function () {
       if (Features.hasFeature('project-history-blobs')) {
         it('should fetch the file from history-v1', async function () {
-          const { response, body } = await user.doRequest('GET', fileURL)
+          const { response, body } = await user.doRequest('GET', blobURL)
           expect(response.statusCode).to.equal(200)
           expect(response.headers['x-served-by']).to.include('history-v1')
           expect(body).to.equal(fileContent)
           await expectHistoryV1Hit()
         })
         it('should set cache headers', async function () {
-          const { response } = await user.doRequest('GET', fileURL)
+          const { response } = await user.doRequest('GET', blobURL)
           expect(response.headers['cache-control']).to.equal(
             'private, max-age=86400, stale-while-revalidate=31536000'
           )
@@ -155,7 +156,7 @@ describe('HistoryTests', function () {
         })
         it('should return a 304 when revalidating', async function () {
           const { response, body } = await user.doRequest('GET', {
-            url: fileURL,
+            url: blobURL,
             headers: { 'If-None-Match': fileHash },
           })
           expect(response.statusCode).to.equal(304)
@@ -165,13 +166,14 @@ describe('HistoryTests', function () {
       }
       it('should return 404 without fallback', async function () {
         MockV1HistoryApi.reset()
-        const { response } = await user.doRequest('GET', fileURL)
+        const { response } = await user.doRequest('GET', blobURL)
         expect(response.statusCode).to.equal(404)
         await expectNoIncrement()
       })
       it('should not set cache headers on 404', async function () {
         MockV1HistoryApi.reset()
-        const { response } = await user.doRequest('GET', fileURL)
+        const { response } = await user.doRequest('GET', blobURL)
+        expect(response.statusCode).to.equal(404)
         expect(response.headers).not.to.have.property('cache-control')
         expect(response.headers).not.to.have.property('etag')
       })
@@ -179,7 +181,7 @@ describe('HistoryTests', function () {
         MockV1HistoryApi.reset()
         const { response, body } = await user.doRequest(
           'GET',
-          fileURLWithFallback
+          blobURLWithFallback
         )
         expect(response.statusCode).to.equal(200)
         expect(response.headers['x-served-by']).to.include('filestore')
@@ -189,10 +191,79 @@ describe('HistoryTests', function () {
       it('should return 404 with both files missing', async function () {
         MockFilestoreApi.reset()
         MockV1HistoryApi.reset()
-        const { response } = await user.doRequest('GET', fileURLWithFallback)
+        const { response } = await user.doRequest('GET', blobURLWithFallback)
         expect(response.statusCode).to.equal(404)
         await expectNoIncrement()
       })
     })
   })
+
+  // Legacy endpoint that is powered by history-v1 in SaaS
+  describe('/project/:projectId/file/:fileId', function () {
+    describe('HEAD', function () {
+      if (Features.hasFeature('project-history-blobs')) {
+        it('should fetch the file size from history-v1', async function () {
+          const { response } = await user.doRequest('HEAD', fileURL)
+          expect(response.statusCode).to.equal(200)
+          expect(response.headers['x-served-by']).to.include('history-v1')
+          expect(response.headers['content-length']).to.equal('3694')
+          await expectHistoryV1Hit()
+        })
+      }
+      it('should fetch the file size from filestore when missing in history-v1', async function () {
+        MockV1HistoryApi.reset()
+        const { response } = await user.doRequest('HEAD', blobURLWithFallback)
+        expect(response.statusCode).to.equal(200)
+        expect(response.headers['x-served-by']).to.include('filestore')
+        expect(response.headers['content-length']).to.equal('3694')
+      })
+      it('should return 404 with both files missing', async function () {
+        MockFilestoreApi.reset()
+        MockV1HistoryApi.reset()
+        const { response } = await user.doRequest('HEAD', blobURLWithFallback)
+        expect(response.statusCode).to.equal(404)
+      })
+    })
+    describe('GET', function () {
+      if (Features.hasFeature('project-history-blobs')) {
+        it('should fetch the file from history-v1', async function () {
+          const { response, body } = await user.doRequest('GET', fileURL)
+          expect(response.statusCode).to.equal(200)
+          expect(response.headers['x-served-by']).to.include('history-v1')
+          expect(body).to.equal(fileContent)
+          await expectHistoryV1Hit()
+        })
+      }
+      it('should set cache headers', async function () {
+        const { response } = await user.doRequest('GET', fileURL)
+        expect(response.headers['cache-control']).to.equal(
+          'private, max-age=3600'
+        )
+      })
+      it('should not set cache headers on 404', async function () {
+        MockV1HistoryApi.reset()
+        MockFilestoreApi.reset()
+        // The legacy filestore downloads are not properly handling 404s, so delete the file from the file-tree to trigger the 404. All the filestore code will be removed soon.
+        await user.doRequest('DELETE', fileURL)
+
+        const { response } = await user.doRequest('GET', fileURL)
+        expect(response.statusCode).to.equal(404)
+        expect(response.headers).not.to.have.property('cache-control')
+        expect(response.headers).not.to.have.property('etag')
+      })
+      it('should fetch the file size from filestore when missing in history-v1', async function () {
+        MockV1HistoryApi.reset()
+        const { response, body } = await user.doRequest('GET', fileURL)
+        expect(response.statusCode).to.equal(200)
+        expect(response.headers['x-served-by']).to.include('filestore')
+        expect(body).to.equal(fileContent)
+      })
+      it('should return 404 with both files missing', async function () {
+        MockFilestoreApi.reset()
+        MockV1HistoryApi.reset()
+        const { response } = await user.doRequest('GET', fileURL)
+        expect(response.statusCode).to.equal(404)
+      })
+    })
+  })
 })

services/web/test/unit/src/FileStore/FileStoreControllerTests.mjs

@@ -9,6 +9,7 @@ const MODULE_PATH =
 
 const expectedFileHeaders = {
   'Cache-Control': 'private, max-age=3600',
+  'X-Served-By': 'filestore',
 }
 
 describe('FileStoreController', function () {
@@ -56,7 +57,7 @@ describe('FileStoreController', function () {
   describe('getFile', function () {
     beforeEach(function () {
       this.FileStoreHandler.promises.getFileStream.resolves(this.stream)
-      this.ProjectLocator.promises.findElement.resolves(this.file)
+      this.ProjectLocator.promises.findElement.resolves({ element: this.file })
     })
 
     it('should call the file store handler with the project_id file_id and any query string', async function () {
@@ -193,6 +194,10 @@ describe('FileStoreController', function () {
   })
 
   describe('getFileHead', function () {
+    beforeEach(function () {
+      this.ProjectLocator.promises.findElement.resolves({ element: this.file })
+    })
+
     it('reports the file size', function (done) {
       const expectedFileSize = 99393
       this.FileStoreHandler.promises.getFileSize.rejects(

services/web/test/unit/src/helpers/MockResponse.js

@@ -139,6 +139,14 @@ class MockResponse {
     this.header(header, value)
   }
 
+  appendHeader(header, value) {
+    if (this.headers[header]) {
+      this.headers[header] += `, ${value}`
+    } else {
+      this.headers[header] = value
+    }
+  }
+
   setTimeout(timout) {
     this.timout = timout
   }