[project-history] block filestore reads from old queue entries (#23096)

GitOrigin-RevId: 9952cb66e542b17a6a3b5e3b2609d53dc8c371fd

services/project-history/app/js/HistoryStoreManager.js

@@ -363,6 +363,9 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
           new OError('no filestore URL provided and blob was not created')
         )
       }
+      if (!Settings.apis.filestore.enabled) {
+        return callback(new OError('blocking filestore read', { update }))
+      }
 
       fetchStream(filestoreURL, {
         signal: AbortSignal.timeout(HTTP_REQUEST_TIMEOUT),

services/project-history/config/settings.defaults.cjs

@@ -27,6 +27,7 @@ module.exports = {
       url: `http://${process.env.DOCSTORE_HOST || '127.0.0.1'}:3016`,
     },
     filestore: {
+      enabled: process.env.FILESTORE_ENABLED !== 'false',
       url: `http://${process.env.FILESTORE_HOST || '127.0.0.1'}:3009`,
     },
     web: {

services/project-history/test/acceptance/js/SyncTests.js

@@ -5,6 +5,7 @@ import request from 'request'
 import assert from 'node:assert'
 import mongodb from 'mongodb-legacy'
 import logger from '@overleaf/logger'
+import Settings from '@overleaf/settings'
 import * as ProjectHistoryClient from './helpers/ProjectHistoryClient.js'
 import * as ProjectHistoryApp from './helpers/ProjectHistoryApp.js'
 import sinon from 'sinon'
@@ -621,6 +622,142 @@ describe('Syncing with web and doc-updater', function () {
             }
           )
         })
+        describe('with filestore disabled', function () {
+          before(function () {
+            Settings.apis.filestore.enabled = false
+          })
+          after(function () {
+            Settings.apis.filestore.enabled = true
+          })
+          it('should record error when blob is missing', function (done) {
+            MockHistoryStore()
+              .get(`/api/projects/${historyId}/latest/history`)
+              .reply(200, {
+                chunk: {
+                  history: {
+                    snapshot: {
+                      files: {
+                        persistedFile: { hash: EMPTY_FILE_HASH, byteLength: 0 },
+                      },
+                    },
+                    changes: [],
+                  },
+                  startVersion: 0,
+                },
+              })
+
+            const fileContents = Buffer.from([1, 2, 3])
+            const fileHash = 'aed2973e4b8a7ff1b30ff5c4751e5a2b38989e74'
+
+            MockFileStore()
+              .get(`/project/${this.project_id}/file/${this.file_id}`)
+              .reply(200, fileContents)
+            const headBlob = MockHistoryStore()
+              .head(`/api/projects/${historyId}/blobs/${fileHash}`)
+              .times(3) // three retries
+              .reply(404)
+            const createBlob = MockHistoryStore()
+              .put(`/api/projects/${historyId}/blobs/${fileHash}`, fileContents)
+              .reply(201)
+
+            const addFile = MockHistoryStore()
+              .post(`/api/projects/${historyId}/legacy_changes`, body => {
+                expect(body).to.deep.equal([
+                  {
+                    v2Authors: [],
+                    authors: [],
+                    timestamp: this.timestamp.toJSON(),
+                    operations: [
+                      {
+                        pathname: 'test.png',
+                        file: {
+                          hash: fileHash,
+                        },
+                      },
+                    ],
+                    origin: { kind: 'test-origin' },
+                  },
+                ])
+                return true
+              })
+              .query({ end_version: 0 })
+              .reply(204)
+
+            async.series(
+              [
+                cb => {
+                  ProjectHistoryClient.resyncHistory(this.project_id, cb)
+                },
+                cb => {
+                  const update = {
+                    projectHistoryId: historyId,
+                    resyncProjectStructure: {
+                      docs: [],
+                      files: [
+                        {
+                          file: this.file_id,
+                          path: '/test.png',
+                          _hash: fileHash,
+                          url: `http://127.0.0.1:3009/project/${this.project_id}/file/${this.file_id}`,
+                        },
+                        { path: '/persistedFile' },
+                      ],
+                    },
+                    meta: {
+                      ts: this.timestamp,
+                    },
+                  }
+                  ProjectHistoryClient.pushRawUpdate(
+                    this.project_id,
+                    update,
+                    cb
+                  )
+                },
+                cb => {
+                  ProjectHistoryClient.flushProject(
+                    this.project_id,
+                    {
+                      allowErrors: true,
+                    },
+                    (err, res) => {
+                      if (err) return cb(err)
+                      assert(
+                        res.statusCode === 500,
+                        'resync should have failed'
+                      )
+                      cb()
+                    }
+                  )
+                },
+              ],
+              error => {
+                if (error) {
+                  throw error
+                }
+                assert(
+                  loggerError.calledWithMatch(
+                    sinon.match.any,
+                    'blocking filestore read'
+                  ),
+                  'error logged on 500'
+                )
+                assert(
+                  headBlob.isDone(),
+                  'HEAD /api/projects/:historyId/blobs/:hash should have been called'
+                )
+                assert(
+                  !createBlob.isDone(),
+                  '/api/projects/:historyId/blobs/:hash should have been skipped'
+                )
+                assert(
+                  !addFile.isDone(),
+                  `/api/projects/${historyId}/changes should have been skipped`
+                )
+                done()
+              }
+            )
+          })
+        })
       })
 
       describe("when a file exists which shouldn't", function () {

services/project-history/test/unit/js/HistoryStoreManager/HistoryStoreManagerTests.js

@@ -22,6 +22,7 @@ describe('HistoryStoreManager', function () {
       },
       apis: {
         filestore: {
+          enabled: true,
           url: 'http://filestore.overleaf.production',
         },
       },
@@ -424,6 +425,30 @@ describe('HistoryStoreManager', function () {
       })
     })
 
+    describe('with filestore disabled', function () {
+      beforeEach(function (done) {
+        this.settings.apis.filestore.enabled = false
+        this.file_id = '012345678901234567890123'
+        this.update = {
+          file: true,
+          url: `http://filestore.other.cloud.provider/project/${this.projectId}/file/${this.file_id}`,
+          hash: this.hash,
+        }
+        this.HistoryStoreManager.createBlobForUpdate(
+          this.projectId,
+          this.historyId,
+          this.update,
+          err => {
+            expect(err).to.match(/blocking filestore read/)
+            done()
+          }
+        )
+      })
+      it('should not request the file', function () {
+        expect(this.FetchUtils.fetchStream).to.not.have.been.called
+      })
+    })
+
     describe('for a file update with an invalid filestore location', function () {
       beforeEach(function (done) {
         this.invalid_id = '000000000000000000000000'