[web] prepare filestore migration for Server Pro/CE (#27230)

* [web] prepare filestore migration for Server Pro/CE

* [history-v1] remove unused USER_FILES_BUCKET_NAME env var from script

* [server-ce] tests: write default docker-compose.override.yml on startup

* [server-ce] tests: extend access logging of host-admin for response

* [server-ce] tests: test text and binary file upload

* [server-ce] tests: add tests for filestore migration

* [web] simplify feature gate for filestore/project-history-blobs logic

Co-authored-by: Brian Gough <brian.gough@overleaf.com>

* [server-ce] test: fix flaky test helper

---------

Co-authored-by: Brian Gough <brian.gough@overleaf.com>
GitOrigin-RevId: f89bdab2749e2b7a49d609e2eac6bf621c727966

server-ce/test/Makefile

@@ -21,9 +21,11 @@ test-e2e-native:
 
 test-e2e:
 	docker compose build host-admin
+	docker compose up -d host-admin
 	docker compose up --no-log-prefix --exit-code-from=e2e e2e
 
 test-e2e-open:
+	docker compose up -d host-admin
 	docker compose up --no-log-prefix --exit-code-from=e2e-open e2e-open
 
 clean:

server-ce/test/editor.spec.ts

@@ -2,6 +2,7 @@ import {
   createNewFile,
   createProject,
   openProjectById,
+  testNewFileUpload,
 } from './helpers/project'
 import { isExcludedBySharding, startWith } from './helpers/config'
 import { ensureUserExists, login } from './helpers/login'
@@ -119,24 +120,7 @@ describe('editor', () => {
       cy.get('button').contains('New file').click({ force: true })
     })
 
-    it('can upload file', () => {
-      const name = `${uuid()}.txt`
-      const content = `Test File Content ${name}`
-      cy.get('button').contains('Upload').click({ force: true })
-      cy.get('input[type=file]')
-        .first()
-        .selectFile(
-          {
-            contents: Cypress.Buffer.from(content),
-            fileName: name,
-            lastModified: Date.now(),
-          },
-          { force: true }
-        )
-      // force: The file-tree pane is too narrow to display the full name.
-      cy.findByTestId('file-tree').findByText(name).click({ force: true })
-      cy.findByText(content)
-    })
+    testNewFileUpload()
 
     it('should not display import from URL', () => {
       cy.findByText('From external URL').should('not.exist')

server-ce/test/filestore-migration.spec.ts

@@ -0,0 +1,104 @@
+import { ensureUserExists, login } from './helpers/login'
+import {
+  createProject,
+  openProjectById,
+  prepareFileUploadTest,
+} from './helpers/project'
+import { isExcludedBySharding, startWith } from './helpers/config'
+import { prepareWaitForNextCompileSlot } from './helpers/compile'
+import { beforeWithReRunOnTestRetry } from './helpers/beforeWithReRunOnTestRetry'
+import { v4 as uuid } from 'uuid'
+import { purgeFilestoreData, runScript } from './helpers/hostAdminClient'
+
+describe('filestore migration', function () {
+  if (isExcludedBySharding('CE_CUSTOM_3')) return
+  startWith({ withDataDir: true, resetData: true, vars: {} })
+  ensureUserExists({ email: 'user@example.com' })
+
+  let projectName: string
+  let projectId: string
+  let waitForCompileRateLimitCoolOff: (fn: () => void) => void
+  const previousBinaryFiles: (() => void)[] = []
+  beforeWithReRunOnTestRetry(function () {
+    projectName = `project-${uuid()}`
+    login('user@example.com')
+    createProject(projectName, { type: 'Example project' }).then(
+      id => (projectId = id)
+    )
+    let queueReset
+    ;({ waitForCompileRateLimitCoolOff, queueReset } =
+      prepareWaitForNextCompileSlot())
+    queueReset()
+    previousBinaryFiles.push(prepareFileUploadTest(true))
+  })
+
+  beforeEach(() => {
+    login('user@example.com')
+    waitForCompileRateLimitCoolOff(() => {
+      openProjectById(projectId)
+    })
+  })
+
+  function checkFilesAreAccessible() {
+    it('can upload new binary file and read previous uploads', function () {
+      previousBinaryFiles.push(prepareFileUploadTest(true))
+      for (const check of previousBinaryFiles) {
+        check()
+      }
+    })
+
+    it('renders frog jpg', () => {
+      cy.findByTestId('file-tree').findByText('frog.jpg').click()
+      cy.get('[alt="frog.jpg"]')
+        .should('be.visible')
+        .and('have.prop', 'naturalWidth')
+        .should('be.greaterThan', 0)
+    })
+  }
+
+  describe('OVERLEAF_FILESTORE_MIGRATION_LEVEL not set', function () {
+    startWith({ withDataDir: true, vars: {} })
+    checkFilesAreAccessible()
+  })
+
+  describe('OVERLEAF_FILESTORE_MIGRATION_LEVEL=0', function () {
+    startWith({
+      withDataDir: true,
+      vars: { OVERLEAF_FILESTORE_MIGRATION_LEVEL: '0' },
+    })
+    checkFilesAreAccessible()
+
+    describe('OVERLEAF_FILESTORE_MIGRATION_LEVEL=1', function () {
+      startWith({
+        withDataDir: true,
+        vars: { OVERLEAF_FILESTORE_MIGRATION_LEVEL: '1' },
+      })
+      checkFilesAreAccessible()
+
+      describe('OVERLEAF_FILESTORE_MIGRATION_LEVEL=2', function () {
+        startWith({
+          withDataDir: true,
+          vars: { OVERLEAF_FILESTORE_MIGRATION_LEVEL: '1' },
+        })
+        before(async function () {
+          await runScript({
+            cwd: 'services/history-v1',
+            script: 'storage/scripts/back_fill_file_hash.mjs',
+          })
+        })
+        startWith({
+          withDataDir: true,
+          vars: { OVERLEAF_FILESTORE_MIGRATION_LEVEL: '2' },
+        })
+        checkFilesAreAccessible()
+
+        describe('purge filestore data', function () {
+          before(async function () {
+            await purgeFilestoreData()
+          })
+          checkFilesAreAccessible()
+        })
+      })
+    })
+  })
+})

server-ce/test/helpers/config.ts

@@ -9,6 +9,7 @@ export function isExcludedBySharding(
     | 'CE_DEFAULT'
     | 'CE_CUSTOM_1'
     | 'CE_CUSTOM_2'
+    | 'CE_CUSTOM_3'
     | 'PRO_DEFAULT_1'
     | 'PRO_DEFAULT_2'
     | 'PRO_CUSTOM_1'

server-ce/test/helpers/hostAdminClient.ts

@@ -85,6 +85,12 @@ export async function getRedisKeys() {
   return stdout.split('\n')
 }
 
+export async function purgeFilestoreData() {
+  await fetchJSON(`${hostAdminURL}/data/user_files`, {
+    method: 'DELETE',
+  })
+}
+
 async function sleep(ms: number) {
   return new Promise(resolve => {
     setTimeout(resolve, ms)

server-ce/test/helpers/project.ts

@@ -216,3 +216,43 @@ export function createNewFile() {
 
   return fileName
 }
+
+export function prepareFileUploadTest(binary = false) {
+  const name = `${uuid()}.txt`
+  const content = `Test File Content ${name}${binary ? ' \x00' : ''}`
+  cy.get('button').contains('Upload').click({ force: true })
+  cy.get('input[type=file]')
+    .first()
+    .selectFile(
+      {
+        contents: Cypress.Buffer.from(content),
+        fileName: name,
+        lastModified: Date.now(),
+      },
+      { force: true }
+    )
+
+  // wait for the upload to finish
+  cy.findByRole('treeitem', { name })
+
+  return function check() {
+    cy.findByRole('treeitem', { name }).click()
+    if (binary) {
+      cy.findByText(content).should('not.have.class', 'cm-line')
+    } else {
+      cy.findByText(content).should('have.class', 'cm-line')
+    }
+  }
+}
+
+export function testNewFileUpload() {
+  it('can upload text file', () => {
+    const check = prepareFileUploadTest(false)
+    check()
+  })
+
+  it('can upload binary file', () => {
+    const check = prepareFileUploadTest(true)
+    check()
+  })
+}

server-ce/test/host-admin.js

@@ -29,6 +29,17 @@ const IMAGES = {
   PRO: process.env.IMAGE_TAG_PRO.replace(/:.+/, ''),
 }
 
+function defaultDockerComposeOverride() {
+  return {
+    services: {
+      sharelatex: {
+        environment: {},
+      },
+      'git-bridge': {},
+    },
+  }
+}
+
 let previousConfig = ''
 
 function readDockerComposeOverride() {
@@ -38,14 +49,7 @@ function readDockerComposeOverride() {
     if (error.code !== 'ENOENT') {
       throw error
     }
-    return {
-      services: {
-        sharelatex: {
-          environment: {},
-        },
-        'git-bridge': {},
-      },
-    }
+    return defaultDockerComposeOverride
   }
 }
 
@@ -77,12 +81,21 @@ app.use(bodyParser.json())
 app.use((req, res, next) => {
   // Basic access logs
   console.log(req.method, req.url, req.body)
+  const json = res.json
+  res.json = body => {
+    console.log(req.method, req.url, req.body, '->', body)
+    json.call(res, body)
+  }
+  next()
+})
+app.use((req, res, next) => {
   // Add CORS headers
   const accessControlAllowOrigin =
     process.env.ACCESS_CONTROL_ALLOW_ORIGIN || 'http://sharelatex'
   res.setHeader('Access-Control-Allow-Origin', accessControlAllowOrigin)
   res.setHeader('Access-Control-Allow-Headers', 'Content-Type')
   res.setHeader('Access-Control-Max-Age', '3600')
+  res.setHeader('Access-Control-Allow-Methods', 'DELETE, GET, HEAD, POST, PUT')
   next()
 })
 
@@ -133,6 +146,7 @@ const allowedVars = Joi.object(
       'V1_HISTORY_URL',
       'SANDBOXED_COMPILES',
       'ALL_TEX_LIVE_DOCKER_IMAGE_NAMES',
+      'OVERLEAF_FILESTORE_MIGRATION_LEVEL',
       'OVERLEAF_TEMPLATES_USER_ID',
       'OVERLEAF_NEW_PROJECT_TEMPLATE_LINKS',
       'OVERLEAF_ALLOW_PUBLIC_ACCESS',
@@ -319,8 +333,19 @@ app.get('/redis/keys', (req, res) => {
   )
 })
 
+app.delete('/data/user_files', (req, res) => {
+  runDockerCompose(
+    'exec',
+    ['sharelatex', 'rm', '-rf', '/var/lib/overleaf/data/user_files'],
+    (error, stdout, stderr) => {
+      res.json({ error, stdout, stderr })
+    }
+  )
+})
+
 app.use(handleValidationErrors())
 
 purgeDataDir()
+writeDockerComposeOverride(defaultDockerComposeOverride())
 
 app.listen(80)

services/history-v1/storage/scripts/back_fill_file_hash.mjs

@@ -150,10 +150,6 @@ const CONCURRENT_BATCHES = parseInt(process.env.CONCURRENT_BATCHES || '2', 10)
 const RETRIES = parseInt(process.env.RETRIES || '10', 10)
 const RETRY_DELAY_MS = parseInt(process.env.RETRY_DELAY_MS || '100', 10)
 
-const USER_FILES_BUCKET_NAME = process.env.USER_FILES_BUCKET_NAME || ''
-if (!USER_FILES_BUCKET_NAME) {
-  throw new Error('env var USER_FILES_BUCKET_NAME is missing')
-}
 const RETRY_FILESTORE_404 = process.env.RETRY_FILESTORE_404 === 'true'
 const BUFFER_DIR = fs.mkdtempSync(
   process.env.BUFFER_DIR_PREFIX || '/tmp/back_fill_file_hash-'

services/web/app.mjs

@@ -56,14 +56,8 @@ if (Settings.catchErrors) {
 // Create ./data/dumpFolder if needed
 FileWriter.ensureDumpFolderExists()
 
-if (
-  !Features.hasFeature('project-history-blobs') &&
-  !Features.hasFeature('filestore')
-) {
-  throw new Error(
-    'invalid config: must enable either project-history-blobs (Settings.enableProjectHistoryBlobs=true) or enable filestore (Settings.disableFilestore=false)'
-  )
-}
+// Validate combination of feature flags.
+Features.validateSettings()
 
 // handle SIGTERM for graceful shutdown in kubernetes
 process.on('SIGTERM', function (signal) {

services/web/app/src/Features/History/HistoryURLHelper.js

@@ -8,7 +8,7 @@ function projectHistoryURLWithFilestoreFallback(
 ) {
   const filestoreURL = `${Settings.apis.filestore.url}/project/${projectId}/file/${fileRef._id}?from=${origin}`
   // TODO: When this file is converted to ES modules we will be able to use Features.hasFeature('project-history-blobs'). Currently we can't stub the feature return value in tests.
-  if (fileRef.hash && Settings.enableProjectHistoryBlobs) {
+  if (fileRef.hash && Settings.filestoreMigrationLevel >= 1) {
     return {
       url: `${Settings.apis.project_history.url}/project/${historyId}/blob/${fileRef.hash}`,
       fallbackURL: filestoreURL,

services/web/app/src/infrastructure/Features.js

@@ -19,8 +19,7 @@ const trackChangesModuleAvailable =
  * @property {boolean | undefined} enableGithubSync
  * @property {boolean | undefined} enableGitBridge
  * @property {boolean | undefined} enableHomepage
- * @property {boolean | undefined} enableProjectHistoryBlobs
- * @property {boolean | undefined} disableFilestore
+ * @property {number} filestoreMigrationLevel
  * @property {boolean | undefined} enableSaml
  * @property {boolean | undefined} ldap
  * @property {boolean | undefined} oauth
@@ -30,6 +29,14 @@ const trackChangesModuleAvailable =
  */
 
 const Features = {
+  validateSettings() {
+    if (![0, 1, 2].includes(Settings.filestoreMigrationLevel)) {
+      throw new Error(
+        `invalid OVERLEAF_FILESTORE_MIGRATION_LEVEL=${Settings.filestoreMigrationLevel}, expected 0, 1 or 2`
+      )
+    }
+  },
+
   /**
    * @returns {boolean}
    */
@@ -89,9 +96,9 @@ const Features = {
             Settings.enabledLinkedFileTypes.includes('url')
         )
       case 'project-history-blobs':
-        return Boolean(Settings.enableProjectHistoryBlobs)
+        return Settings.filestoreMigrationLevel > 0
       case 'filestore':
-        return Boolean(Settings.disableFilestore) === false
+        return Settings.filestoreMigrationLevel < 2
       case 'support':
         return supportModuleAvailable
       case 'symbol-palette':

services/web/config/settings.defaults.js

@@ -440,6 +440,9 @@ module.exports = {
     ','
   ),
 
+  filestoreMigrationLevel:
+    parseInt(process.env.OVERLEAF_FILESTORE_MIGRATION_LEVEL, 10) || 0,
+
   // i18n
   // ------
   //

services/web/modules/history-v1/test/acceptance/src/ProjectStructureTests.mjs

@@ -8,7 +8,6 @@ import _ from 'lodash'
 import ProjectGetter from '../../../../../app/src/Features/Project/ProjectGetter.js'
 import User from '../../../../../test/acceptance/src/helpers/User.mjs'
 import MockDocUpdaterApiClass from '../../../../../test/acceptance/src/mocks/MockDocUpdaterApi.mjs'
-import Features from '../../../../../app/src/infrastructure/Features.js'
 
 const { ObjectId } = mongodb
 
@@ -188,32 +187,25 @@ describe('ProjectStructureChanges', function () {
   const cases = [
     {
       label: 'with filestore disabled and project-history-blobs enabled',
-      disableFilestore: true,
-      enableProjectHistoryBlobs: true,
+      filestoreMigrationLevel: 2,
     },
     {
       label: 'with filestore enabled and project-history-blobs enabled',
-      disableFilestore: false,
-      enableProjectHistoryBlobs: true,
+      filestoreMigrationLevel: 1,
     },
     {
       label: 'with filestore enabled and project-history-blobs disabled',
-      disableFilestore: false,
-      enableProjectHistoryBlobs: false,
+      filestoreMigrationLevel: 0,
     },
   ]
-  for (const { label, disableFilestore, enableProjectHistoryBlobs } of cases) {
+  for (const { label, filestoreMigrationLevel } of cases) {
     describe(label, function () {
-      const previousDisableFilestore = Settings.disableFilestore
-      const previousEnableProjectHistoryBlobs =
-        Settings.enableProjectHistoryBlobs
+      const previousFilestoreMigrationLevel = Settings.filestoreMigrationLevel
       beforeEach(function () {
-        Settings.disableFilestore = disableFilestore
-        Settings.enableProjectHistoryBlobs = enableProjectHistoryBlobs
+        Settings.filestoreMigrationLevel = filestoreMigrationLevel
       })
       afterEach(function () {
-        Settings.disableFilestore = previousDisableFilestore
-        Settings.enableProjectHistoryBlobs = previousEnableProjectHistoryBlobs
+        Settings.filestoreMigrationLevel = previousFilestoreMigrationLevel
       })
 
       describe('creating a project from the example template', function () {
@@ -244,7 +236,7 @@ describe('ProjectStructureChanges', function () {
           expect(updates[2].type).to.equal('add-file')
           expect(updates[2].userId).to.equal(owner._id)
           expect(updates[2].pathname).to.equal('/frog.jpg')
-          if (disableFilestore) {
+          if (filestoreMigrationLevel === 2) {
             expect(updates[2].url).to.not.exist
             expect(updates[2].createdBlob).to.be.true
           } else {
@@ -301,10 +293,10 @@ describe('ProjectStructureChanges', function () {
           expect(updates[2].type).to.equal('add-file')
           expect(updates[2].userId).to.equal(owner._id)
           expect(updates[2].pathname).to.equal('/frog.jpg')
-          if (disableFilestore) {
+          if (filestoreMigrationLevel === 2) {
             expect(updates[2].url).to.not.exist
             expect(updates[2].createdBlob).to.be.true
-          } else if (Features.hasFeature('project-history-blobs')) {
+          } else if (filestoreMigrationLevel === 1) {
             expect(updates[2].url).to.be.null
           } else {
             expect(updates[2].url).to.be.a('string')
@@ -378,7 +370,7 @@ describe('ProjectStructureChanges', function () {
           expect(updates[1].type).to.equal('add-file')
           expect(updates[1].userId).to.equal(owner._id)
           expect(updates[1].pathname).to.equal('/1pixel.png')
-          if (disableFilestore) {
+          if (filestoreMigrationLevel === 2) {
             expect(updates[1].url).to.not.exist
             expect(updates[1].createdBlob).to.be.true
           } else {
@@ -478,7 +470,7 @@ describe('ProjectStructureChanges', function () {
           expect(update.type).to.equal('add-file')
           expect(update.userId).to.equal(owner._id)
           expect(update.pathname).to.equal('/1pixel.png')
-          if (disableFilestore) {
+          if (filestoreMigrationLevel === 2) {
             expect(update.url).to.not.exist
             expect(update.createdBlob).to.be.true
           } else {
@@ -516,7 +508,7 @@ describe('ProjectStructureChanges', function () {
               expect(updates[1].type).to.equal('add-file')
               expect(updates[1].userId).to.equal(owner._id)
               expect(updates[1].pathname).to.equal('/1pixel.png')
-              if (disableFilestore) {
+              if (filestoreMigrationLevel === 2) {
                 expect(updates[1].url).to.not.exist
                 expect(updates[1].createdBlob).to.be.true
               } else {
@@ -1005,7 +997,7 @@ describe('ProjectStructureChanges', function () {
             expect(update.type).to.equal('add-file')
             expect(update.userId).to.equal(owner._id)
             expect(update.pathname).to.equal('/1pixel.png')
-            if (disableFilestore) {
+            if (filestoreMigrationLevel === 2) {
               expect(update.url).to.not.exist
               expect(update.createdBlob).to.be.true
             } else {
@@ -1068,7 +1060,7 @@ describe('ProjectStructureChanges', function () {
               expect(updates[1].type).to.equal('add-file')
               expect(updates[1].userId).to.equal(owner._id)
               expect(updates[1].pathname).to.equal('/1pixel.png')
-              if (disableFilestore) {
+              if (filestoreMigrationLevel === 2) {
                 expect(updates[1].url).to.not.exist
                 expect(updates[1].createdBlob).to.be.true
               } else {

services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js

@@ -29,6 +29,7 @@ describe('DocumentUpdaterHandler', function () {
           url: 'http://project_history.example.com',
         },
       },
+      filestoreMigrationLevel: 0,
       moduleImportSequence: [],
     }
     this.source = 'dropbox'
@@ -1491,7 +1492,7 @@ describe('DocumentUpdaterHandler', function () {
 
       describe('with filestore disabled', function () {
         beforeEach(function () {
-          this.settings.disableFilestore = true
+          this.settings.filestoreMigrationLevel = 2
         })
         it('should add files without URL and with createdBlob', async function () {
           this.fileId = new ObjectId()
@@ -1700,7 +1701,7 @@ describe('DocumentUpdaterHandler', function () {
     })
     describe('with filestore disabled', function () {
       beforeEach(function () {
-        this.settings.disableFilestore = true
+        this.settings.filestoreMigrationLevel = 2
       })
       it('should add files without URL', async function () {
         const fileId1 = new ObjectId()

services/web/test/unit/src/References/ReferencesHandler.test.mjs

@@ -50,7 +50,7 @@ describe('ReferencesHandler', function () {
           filestore: { url: 'http://some.url/filestore' },
           project_history: { url: 'http://project-history.local' },
         },
-        enableProjectHistoryBlobs: true,
+        filestoreMigrationLevel: 2,
       }),
     }))
 

services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js

@@ -39,6 +39,7 @@ describe('SplitTestHandler', function () {
     }
     this.SplitTestCache.get.resolves(this.cachedSplitTests)
     this.Settings = {
+      filestoreMigrationLevel: 0,
       moduleImportSequence: [],
       overleaf: {},
       devToolbar: {

services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js

@@ -57,7 +57,7 @@ describe('TpdsUpdateSender', function () {
           url: projectHistoryUrl,
         },
       },
-      enableProjectHistoryBlobs: true,
+      filestoreMigrationLevel: true,
     }
     const getUsers = sinon.stub()
     getUsers

services/web/test/unit/src/infrastructure/FeaturesTests.js

@@ -7,6 +7,7 @@ describe('Features', function () {
     this.Features = SandboxedModule.require(modulePath, {
       requires: {
         '@overleaf/settings': (this.settings = {
+          filestoreMigrationLevel: 0,
           moduleImportSequence: [],
           enabledLinkedFileTypes: [],
         }),