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

GitOrigin-RevId: f6d12cfe445c7c41a62a563c9a5e7089bf94344f
2026-05-31 12:51:35 +02:00 · 2025-02-07 08:38:49 +00:00
parent 4a715fdfd1
commit 71709bf2b0
4 changed files with 132 additions and 45 deletions
--- a/services/web/app/src/Features/FileStore/FileStoreController.mjs
+++ b/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()
 }

--- a/services/web/test/acceptance/src/HistoryTests.mjs
+++ b/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,11 +133,97 @@ 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()
      })
    })
+    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', 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', blobURL)
+          expect(response.headers['cache-control']).to.equal(
+            'private, max-age=86400, stale-while-revalidate=31536000'
+          )
+          expect(response.headers.etag).to.equal(fileHash)
+        })
+        it('should return a 304 when revalidating', async function () {
+          const { response, body } = await user.doRequest('GET', {
+            url: blobURL,
+            headers: { 'If-None-Match': fileHash },
+          })
+          expect(response.statusCode).to.equal(304)
+          expect(response.headers.etag).to.equal(fileHash)
+          expect(body).to.equal('')
+        })
+      }
+      it('should return 404 without fallback', async function () {
+        MockV1HistoryApi.reset()
+        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', blobURL)
+        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',
+          blobURLWithFallback
+        )
+        expect(response.statusCode).to.equal(200)
+        expect(response.headers['x-served-by']).to.include('filestore')
+        expect(body).to.equal(fileContent)
+        await expectFilestoreHit()
+      })
+      it('should return 404 with both files missing', async function () {
+        MockFilestoreApi.reset()
+        MockV1HistoryApi.reset()
+        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 () {
@@ -146,52 +233,36 @@ describe('HistoryTests', function () {
          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=86400, stale-while-revalidate=31536000'
-          )
-          expect(response.headers.etag).to.equal(fileHash)
-        })
-        it('should return a 304 when revalidating', async function () {
-          const { response, body } = await user.doRequest('GET', {
-            url: fileURL,
-            headers: { 'If-None-Match': fileHash },
-          })
-          expect(response.statusCode).to.equal(304)
-          expect(response.headers.etag).to.equal(fileHash)
-          expect(body).to.equal('')
-        })
      }
-      it('should return 404 without fallback', async function () {
-        MockV1HistoryApi.reset()
+      it('should set cache headers', async function () {
        const { response } = await user.doRequest('GET', fileURL)
-        expect(response.statusCode).to.equal(404)
-        await expectNoIncrement()
+        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',
-          fileURLWithFallback
-        )
+        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)
-        await expectFilestoreHit()
      })
      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', fileURL)
        expect(response.statusCode).to.equal(404)
-        await expectNoIncrement()
      })
    })
  })
--- a/services/web/test/unit/src/FileStore/FileStoreControllerTests.mjs
+++ b/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(
--- a/services/web/test/unit/src/helpers/MockResponse.js
+++ b/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
  }