diff --git a/server-ce/test/Makefile b/server-ce/test/Makefile index 6c56b7e8fe..fb7c980293 100644 --- a/server-ce/test/Makefile +++ b/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: diff --git a/server-ce/test/editor.spec.ts b/server-ce/test/editor.spec.ts index d0060518de..3e57b94f8f 100644 --- a/server-ce/test/editor.spec.ts +++ b/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') diff --git a/server-ce/test/filestore-migration.spec.ts b/server-ce/test/filestore-migration.spec.ts new file mode 100644 index 0000000000..25875ad374 --- /dev/null +++ b/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() + }) + }) + }) + }) +}) diff --git a/server-ce/test/helpers/config.ts b/server-ce/test/helpers/config.ts index 030e70ceb5..78e81be1f7 100644 --- a/server-ce/test/helpers/config.ts +++ b/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' diff --git a/server-ce/test/helpers/hostAdminClient.ts b/server-ce/test/helpers/hostAdminClient.ts index cafeaa2db6..dadfe2b059 100644 --- a/server-ce/test/helpers/hostAdminClient.ts +++ b/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) diff --git a/server-ce/test/helpers/project.ts b/server-ce/test/helpers/project.ts index abcce3f9b2..4b3197afed 100644 --- a/server-ce/test/helpers/project.ts +++ b/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() + }) +} diff --git a/server-ce/test/host-admin.js b/server-ce/test/host-admin.js index f73209d58f..b3dcd72b1f 100644 --- a/server-ce/test/host-admin.js +++ b/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) diff --git a/services/history-v1/storage/scripts/back_fill_file_hash.mjs b/services/history-v1/storage/scripts/back_fill_file_hash.mjs index 0ccadaf5a9..2e12328e5c 100644 --- a/services/history-v1/storage/scripts/back_fill_file_hash.mjs +++ b/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-' diff --git a/services/web/app.mjs b/services/web/app.mjs index b7c723da3d..3f54cc36a8 100644 --- a/services/web/app.mjs +++ b/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) { diff --git a/services/web/app/src/Features/History/HistoryURLHelper.js b/services/web/app/src/Features/History/HistoryURLHelper.js index 8b8d8cbdd7..acb43ced68 100644 --- a/services/web/app/src/Features/History/HistoryURLHelper.js +++ b/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, diff --git a/services/web/app/src/infrastructure/Features.js b/services/web/app/src/infrastructure/Features.js index aaf51103b9..6147e70e0f 100644 --- a/services/web/app/src/infrastructure/Features.js +++ b/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': diff --git a/services/web/config/settings.defaults.js b/services/web/config/settings.defaults.js index bd0730d5d0..4df63ebd7c 100644 --- a/services/web/config/settings.defaults.js +++ b/services/web/config/settings.defaults.js @@ -440,6 +440,9 @@ module.exports = { ',' ), + filestoreMigrationLevel: + parseInt(process.env.OVERLEAF_FILESTORE_MIGRATION_LEVEL, 10) || 0, + // i18n // ------ // diff --git a/services/web/modules/history-v1/test/acceptance/src/ProjectStructureTests.mjs b/services/web/modules/history-v1/test/acceptance/src/ProjectStructureTests.mjs index 6d7037ac15..9bd87674ce 100644 --- a/services/web/modules/history-v1/test/acceptance/src/ProjectStructureTests.mjs +++ b/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 { diff --git a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js index fba5dc87d4..fdb3945075 100644 --- a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js +++ b/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() diff --git a/services/web/test/unit/src/References/ReferencesHandler.test.mjs b/services/web/test/unit/src/References/ReferencesHandler.test.mjs index 1b5d2c1ba0..971d815e2e 100644 --- a/services/web/test/unit/src/References/ReferencesHandler.test.mjs +++ b/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, }), })) diff --git a/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js b/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js index a682f0c954..4970492eee 100644 --- a/services/web/test/unit/src/SplitTests/SplitTestHandlerTests.js +++ b/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: { diff --git a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js b/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js index f7f733388e..a0f0276f4a 100644 --- a/services/web/test/unit/src/ThirdPartyDataStore/TpdsUpdateSenderTests.js +++ b/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 diff --git a/services/web/test/unit/src/infrastructure/FeaturesTests.js b/services/web/test/unit/src/infrastructure/FeaturesTests.js index dcdf1e4e62..b6d0090b3c 100644 --- a/services/web/test/unit/src/infrastructure/FeaturesTests.js +++ b/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: [], }),