From 29045b908590000b566dc1ada97200a4eaa5bae4 Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Wed, 30 Jul 2025 16:43:11 +0200 Subject: [PATCH] [web] download binary files in clsi from filestore via new endpoints (#27505) * [history-v1] use String.padStart instead of lodash.padStart * [web] download binary files in clsi from filestore via new endpoints * [server-ce] tests: Cypress.env() is parsing boolean values * [server-ce] tests: run history migration as root GitOrigin-RevId: bdf6c0e542531ccc4b3f13d2ed68ca0d31e580e9 --- libraries/object-persistor/src/FSPersistor.js | 6 +- server-ce/config/settings.js | 11 + server-ce/test/docker-compose.yml | 1 + server-ce/test/filestore-migration.spec.ts | 408 ++++++++++-------- server-ce/test/helpers/hostAdminClient.ts | 3 + server-ce/test/host-admin.js | 5 +- services/filestore/app.js | 11 + services/filestore/app/js/FileController.js | 1 + services/filestore/app/js/KeyBuilder.js | 19 + .../app/js}/project_key.js | 3 +- .../history-v1/storage/lib/project_key.js | 5 +- .../acceptance/js/storage/project_key.test.js | 2 + .../app/src/Features/Compile/ClsiManager.js | 5 +- .../src/Features/History/HistoryManager.js | 37 +- .../test/unit/src/Compile/ClsiManagerTests.js | 13 +- .../unit/src/History/HistoryManagerTests.js | 44 +- 16 files changed, 334 insertions(+), 240 deletions(-) rename services/{web/app/src/Features/History => filestore/app/js}/project_key.js (90%) diff --git a/libraries/object-persistor/src/FSPersistor.js b/libraries/object-persistor/src/FSPersistor.js index 38a81407df..0b5891d2b2 100644 --- a/libraries/object-persistor/src/FSPersistor.js +++ b/libraries/object-persistor/src/FSPersistor.js @@ -86,7 +86,7 @@ module.exports = class FSPersistor extends AbstractPersistor { metric: 'fs.ingress', // ingress to us from disk bucket: location, }) - const fsPath = this._getFsPath(location, name) + const fsPath = this._getFsPath(location, name, opts.useSubdirectories) try { opts.fd = await fsPromises.open(fsPath, 'r') @@ -295,9 +295,9 @@ module.exports = class FSPersistor extends AbstractPersistor { await fsPromises.rm(dirPath, { force: true, recursive: true }) } - _getFsPath(location, key) { + _getFsPath(location, key, useSubdirectories = false) { key = key.replace(/\/$/, '') - if (!this.useSubdirectories) { + if (!this.useSubdirectories && !useSubdirectories) { key = key.replace(/\//g, '_') } return Path.join(location, key) diff --git a/server-ce/config/settings.js b/server-ce/config/settings.js index 47d34fd870..c686a019f6 100644 --- a/server-ce/config/settings.js +++ b/server-ce/config/settings.js @@ -441,6 +441,8 @@ switch (process.env.OVERLEAF_FILESTORE_BACKEND) { user_files: process.env.OVERLEAF_FILESTORE_USER_FILES_BUCKET_NAME, template_files: process.env.OVERLEAF_FILESTORE_TEMPLATE_FILES_BUCKET_NAME, + project_blobs: process.env.OVERLEAF_HISTORY_PROJECT_BLOBS_BUCKET, + global_blobs: process.env.OVERLEAF_HISTORY_BLOBS_BUCKET, }, s3: { key: @@ -463,6 +465,15 @@ switch (process.env.OVERLEAF_FILESTORE_BACKEND) { stores: { user_files: Path.join(DATA_DIR, 'user_files'), template_files: Path.join(DATA_DIR, 'template_files'), + + // NOTE: The below paths are hard-coded in server-ce/config/production.json, so hard code them here as well. + // We can use DATA_DIR after switching history-v1 from 'config' to '@overleaf/settings'. + project_blobs: + process.env.OVERLEAF_HISTORY_PROJECT_BLOBS_BUCKET || + '/var/lib/overleaf/data/history/overleaf-project-blobs', + global_blobs: + process.env.OVERLEAF_HISTORY_BLOBS_BUCKET || + '/var/lib/overleaf/data/history/overleaf-global-blobs', }, } } diff --git a/server-ce/test/docker-compose.yml b/server-ce/test/docker-compose.yml index 56fd201157..33ef2029b2 100644 --- a/server-ce/test/docker-compose.yml +++ b/server-ce/test/docker-compose.yml @@ -75,6 +75,7 @@ services: environment: CYPRESS_SHARD: CYPRESS_BASE_URL: http://sharelatex + CYPRESS_FULL_FILESTORE_MIGRATION: SPEC_PATTERN: '**/*.spec.{js,jsx,ts,tsx}' depends_on: sharelatex: diff --git a/server-ce/test/filestore-migration.spec.ts b/server-ce/test/filestore-migration.spec.ts index 27e93903a0..65170230b6 100644 --- a/server-ce/test/filestore-migration.spec.ts +++ b/server-ce/test/filestore-migration.spec.ts @@ -1,5 +1,6 @@ -import { DEFAULT_PASSWORD, login } from './helpers/login' +import { DEFAULT_PASSWORD, ensureUserExists, login } from './helpers/login' import { + createProject, expectFileExists, openProjectById, prepareFileUploadTest, @@ -35,7 +36,8 @@ describe('filestore migration', function () { SHARELATEX_MONGO_URL: 'mongodb://mongo/sharelatex', SHARELATEX_REDIS_HOST: 'redis', } - let projectName: string + const projectName = `project-${uuid()}` + let defaultImage: string let projectId: string let waitForCompileRateLimitCoolOff: (fn: () => void) => void const previousBinaryFiles: (() => void)[] = [] @@ -49,7 +51,7 @@ describe('filestore migration', function () { } function addNewBinaryFileAndCheckPrevious( - universeSelector = 'img[alt="universe.jpg"]' + universeSelector = `img[alt="${defaultImage}"]` ) { before(function () { login(email) @@ -61,7 +63,7 @@ describe('filestore migration', function () { for (const check of previousBinaryFiles) { check() } - cy.findByRole('treeitem', { name: 'universe.jpg' }).click() + cy.findByRole('treeitem', { name: defaultImage }).click() cy.get(universeSelector) .should('be.visible') .and('have.prop', 'naturalWidth') @@ -71,184 +73,223 @@ describe('filestore migration', function () { }) } - // -------------- - // Server Pro 1.x - startWith({ - pro: true, - resetData: true, - withDataDir: true, - vars: sharelatexBrandedVars, - version: '1.2.4', - mongoVersion: '5.0', - }) - - let activateURL: string - before(async function () { - const { stdout } = await runGruntTask({ - task: 'user:create-admin', - args: ['--email', email], + if (Cypress.env('FULL_FILESTORE_MIGRATION')) { + // -------------- + // Server Pro 1.x + startWith({ + pro: true, + resetData: true, + withDataDir: true, + vars: sharelatexBrandedVars, + version: '1.2.4', + mongoVersion: '5.0', }) - ;[activateURL] = stdout.match( - /http:\/\/.+\/user\/password\/set\?passwordResetToken=\S+/ - )! - }) - before(function () { - activateUserVersion1x(activateURL) - login(email) - projectName = `project-${uuid()}` - cy.visit('/project') + defaultImage = 'universe.jpg' - // Legacy angular based UI uses links instead of buttons - cy.findByRole('link', { - name: /Create First Project|New Project/, - }).click() - cy.findByRole('link', { name: 'Example Project' }).click() - cy.findByPlaceholderText('Project Name').type(projectName) - cy.findByRole('button', { name: 'Create' }).click() - cy.url() - .should('match', /\/project\/[a-fA-F0-9]{24}/) - .then(url => (projectId = url.split('/').pop()!)) - let queueReset - ;({ waitForCompileRateLimitCoolOff, queueReset } = - prepareWaitForNextCompileSlot()) - queueReset() + let activateURL: string + before(async function () { + const { stdout } = await runGruntTask({ + task: 'user:create-admin', + args: ['--email', email], + }) + ;[activateURL] = stdout.match( + /http:\/\/.+\/user\/password\/set\?passwordResetToken=\S+/ + )! + }) + before(function () { + activateUserVersion1x(activateURL) + login(email) + cy.visit('/project') - // Create a new binary file - cy.get(`a[tooltip="Upload"]`).click() - const name = `${uuid()}.txt` - // Binary file detection is not sophisticated in version 1.x - const binName = name.replace('.txt', '.bin') - const content = `Test File Content ${name} \x00` - cy.get('input[type=file]') - .first() - .selectFile( + // Legacy angular based UI uses links instead of buttons + cy.findByRole('link', { + name: /Create First Project|New Project/, + }).click() + cy.findByRole('link', { name: 'Example Project' }).click() + cy.findByPlaceholderText('Project Name').type(projectName) + cy.findByRole('button', { name: 'Create' }).click() + cy.url() + .should('match', /\/project\/[a-fA-F0-9]{24}/) + .then(url => (projectId = url.split('/').pop()!)) + let queueReset + ;({ waitForCompileRateLimitCoolOff, queueReset } = + prepareWaitForNextCompileSlot()) + queueReset() + + // Create a new binary file + cy.get(`a[tooltip="Upload"]`).click() + const name = `${uuid()}.txt` + // Binary file detection is not sophisticated in version 1.x + const binName = name.replace('.txt', '.bin') + const content = `Test File Content ${name} \x00` + cy.get('input[type=file]') + .first() + .selectFile( + { + contents: Cypress.Buffer.from(content), + fileName: binName, + lastModified: Date.now(), + }, + { force: true } + ) + // Rename back to .txt to enable preview + cy.findByText(binName).click() + cy.findByText(binName).dblclick() + cy.focused().type(name + '{del}'.repeat('.bin'.length) + '{enter}') + // Switch back and forth + cy.findByText('universe.jpg').click() + cy.findByText(name).click() + cy.findByText(content) + .parent() + .parent() + .should('have.class', 'text-preview') + + previousBinaryFiles.push(() => expectFileExists(name, true, content)) + avoid502() + }) + + // -------------- + // Server Pro 2.x + startWith({ + pro: true, + withDataDir: true, + vars: sharelatexBrandedVars, + version: '2.7.1', + mongoVersion: '5.0', + }) + before(function () { + // Cypress strips the Content-Length header: https://github.com/cypress-io/cypress/issues/16469 + // Server Pro 2.x does not gracefully handle a missing value. + cy.intercept( { - contents: Cypress.Buffer.from(content), - fileName: binName, - lastModified: Date.now(), + method: 'HEAD', + url: `http://sharelatex/project/${projectId}/file/*`, + times: previousBinaryFiles.length + 1, }, - { force: true } + req => { + req.continue(res => { + res.headers['Content-Length'] = '60' + }) + } ) - // Rename back to .txt to enable preview - cy.findByText(binName).click() - cy.findByText(binName).dblclick() - cy.focused().type(name + '{del}'.repeat('.bin'.length) + '{enter}') - // Switch back and forth - cy.findByText('universe.jpg').click() - cy.findByText(name).click() - cy.findByText(content) - .parent() - .parent() - .should('have.class', 'text-preview') + }) + // Server Pro 2.x does not have alt tags on images. + addNewBinaryFileAndCheckPrevious('img') - previousBinaryFiles.push(() => expectFileExists(name, true, content)) - avoid502() - }) + // ---------------------------------- + // Server Pro 3.x + history migration + startWith({ + pro: true, + withDataDir: true, + vars: sharelatexBrandedVars, + version: '3.5.13', + mongoVersion: '5.0', + }) + addNewBinaryFileAndCheckPrevious() // before history migration + before(async function () { + await runScript({ + cwd: 'services/web', + script: 'scripts/history/migrate_history.js', + args: [ + '--force-clean', + '--fix-invalid-characters', + '--convert-large-docs-to-file', + ], + hasOverleafEnv: false, + user: 'root', + }) + }) + before(async function () { + await runScript({ + cwd: 'services/web', + script: 'scripts/history/clean_sl_history_data.js', + hasOverleafEnv: false, + }) + }) + addNewBinaryFileAndCheckPrevious() // after history migration - // -------------- - // Server Pro 2.x - startWith({ - pro: true, - withDataDir: true, - vars: sharelatexBrandedVars, - version: '2.7.1', - mongoVersion: '5.0', - }) - before(function () { - // Cypress strips the Content-Length header: https://github.com/cypress-io/cypress/issues/16469 - // Server Pro 2.x does not gracefully handle a missing value. - cy.intercept( - { - method: 'HEAD', - url: `http://sharelatex/project/${projectId}/file/*`, - times: previousBinaryFiles.length + 1, - }, - req => { - req.continue(res => { - res.headers['Content-Length'] = '60' - }) + // ------------------------------ + // Server Pro 4.x + mongo upgrade + startWith({ + pro: true, + withDataDir: true, + vars: sharelatexBrandedVars, + version: '4.2.9', + mongoVersion: '5.0', + }) + startWith({ + pro: true, + withDataDir: true, + vars: sharelatexBrandedVars, + version: '4.2.9', + mongoVersion: '6.0', + }) + before(async function () { + await setMongoFeatureCompatibilityVersion('6.0') + }) + addNewBinaryFileAndCheckPrevious() + + // ------------------------------------------ + // Server Pro 5.x + mongo upgrade 6 -> 7 -> 8 + startWith({ + pro: true, + withDataDir: true, + mongoVersion: '6.0', + }) + startWith({ + pro: true, + withDataDir: true, + mongoVersion: '7.0', + }) + before(async function () { + await setMongoFeatureCompatibilityVersion('7.0') + }) + startWith({ + pro: true, + withDataDir: true, + // implicit mongo upgrade to 8.0 + }) + before(async function () { + await setMongoFeatureCompatibilityVersion('8.0') + }) + } else { + // 5.x + startWith({ pro: true, withDataDir: true }) + defaultImage = 'frog.jpg' + ensureUserExists({ email }) + before(function () { + login(email) + createProject(projectName, { type: 'Example project', open: false }).then( + id => (projectId = id) + ) + ;({ waitForCompileRateLimitCoolOff } = prepareWaitForNextCompileSlot()) + }) + } + addNewBinaryFileAndCheckPrevious() + + function ensureStopOnFirstErrorIsActive() { + cy.findByRole('button', { name: 'Toggle compile options menu' }).click() + cy.findByRole('menuitem', { + name: 'Stop on first error', + }).then(el => { + // NOTE: THIS IS BAD, but the selected option is otherwise not accessible :/ + if ( + el.get()[0]?.querySelector('.material-symbol')?.textContent !== 'check' + ) { + cy.findByRole('menuitem', { + name: 'Stop on first error', + }).click() + // Clicking on "Stop on first error" closes the mode. Open it again. + cy.findByRole('button', { name: 'Toggle compile options menu' }).click() } - ) - }) - // Server Pro 2.x does not have alt tags on images. - addNewBinaryFileAndCheckPrevious('img') - - // ---------------------------------- - // Server Pro 3.x + history migration - startWith({ - pro: true, - withDataDir: true, - vars: sharelatexBrandedVars, - version: '3.5.13', - mongoVersion: '5.0', - }) - addNewBinaryFileAndCheckPrevious() // before history migration - before(async function () { - await runScript({ - cwd: 'services/web', - script: 'scripts/history/migrate_history.js', - args: [ - '--force-clean', - '--fix-invalid-characters', - '--convert-large-docs-to-file', - ], - hasOverleafEnv: false, }) - }) - before(async function () { - await runScript({ - cwd: 'services/web', - script: 'scripts/history/clean_sl_history_data.js', - hasOverleafEnv: false, + cy.findByRole('menuitem', { + name: 'Stop on first error', + }).within(() => { + cy.findByText('check').should('be.visible') }) - }) - addNewBinaryFileAndCheckPrevious() // after history migration - - // ------------------------------ - // Server Pro 4.x + mongo upgrade - startWith({ - pro: true, - withDataDir: true, - vars: sharelatexBrandedVars, - version: '4.2.9', - mongoVersion: '5.0', - }) - startWith({ - pro: true, - withDataDir: true, - vars: sharelatexBrandedVars, - version: '4.2.9', - mongoVersion: '6.0', - }) - before(async function () { - await setMongoFeatureCompatibilityVersion('6.0') - }) - addNewBinaryFileAndCheckPrevious() - - // ------------------------------------------ - // Server Pro 5.x + mongo upgrade 6 -> 7 -> 8 - startWith({ - pro: true, - withDataDir: true, - mongoVersion: '6.0', - }) - startWith({ - pro: true, - withDataDir: true, - mongoVersion: '7.0', - }) - before(async function () { - await setMongoFeatureCompatibilityVersion('7.0') - }) - startWith({ - pro: true, - withDataDir: true, - // implicit mongo upgrade to 8.0 - }) - before(async function () { - await setMongoFeatureCompatibilityVersion('8.0') - }) - addNewBinaryFileAndCheckPrevious() + cy.findByRole('button', { name: 'Toggle compile options menu' }).click() + } // ------------------- // filestore-migration @@ -257,6 +298,7 @@ describe('filestore migration', function () { waitForCompileRateLimitCoolOff(() => { openProjectById(projectId) }) + ensureStopOnFirstErrorIsActive() }) function checkFilesAreAccessible() { @@ -267,13 +309,31 @@ describe('filestore migration', function () { } }) - it('renders universe jpg', () => { - cy.findByTestId('file-tree').findByText('universe.jpg').click() - cy.get('[alt="universe.jpg"]') + it('renders image of example project', () => { + cy.findByTestId('file-tree').findByText(defaultImage).click() + cy.get(`[alt="${defaultImage}"]`) .should('be.visible') .and('have.prop', 'naturalWidth') .should('be.greaterThan', 0) }) + + it('can recompile from scratch', function () { + const id = uuid() + cy.findByText('\\maketitle').parent().click() + cy.findByText('\\maketitle') + .parent() + .type(`\n\\section{{}Test Section ${id}}`) + + waitForCompileRateLimitCoolOff(() => { + cy.findByRole('button', { name: 'Toggle compile options menu' }).click() + + cy.findByRole('menuitem', { + name: 'Recompile from scratch', + }).trigger('click') + }) + + cy.get('.pdf-viewer').should('contain.text', `Test Section ${id}`) + }) } describe('OVERLEAF_FILESTORE_MIGRATION_LEVEL not set', function () { diff --git a/server-ce/test/helpers/hostAdminClient.ts b/server-ce/test/helpers/hostAdminClient.ts index 56bd7f2b46..180976c628 100644 --- a/server-ce/test/helpers/hostAdminClient.ts +++ b/server-ce/test/helpers/hostAdminClient.ts @@ -65,11 +65,13 @@ export async function runScript({ cwd, script, args = [], + user = 'www-data', hasOverleafEnv = true, }: { cwd: string script: string args?: string[] + user?: string hasOverleafEnv?: boolean }) { return await fetchJSON(`${hostAdminURL}/run/script`, { @@ -78,6 +80,7 @@ export async function runScript({ cwd, script, args, + user, hasOverleafEnv, }), }) diff --git a/server-ce/test/host-admin.js b/server-ce/test/host-admin.js index 4bdc01bce6..ee695058bf 100644 --- a/server-ce/test/host-admin.js +++ b/server-ce/test/host-admin.js @@ -107,13 +107,14 @@ app.post( cwd: Joi.string().required(), script: Joi.string().required(), args: Joi.array().items(Joi.string()), + user: Joi.string().required(), hasOverleafEnv: Joi.boolean().required(), }, }, { allowUnknown: false } ), (req, res) => { - const { cwd, script, args, hasOverleafEnv } = req.body + const { cwd, script, args, user, hasOverleafEnv } = req.body const env = hasOverleafEnv ? 'source /etc/overleaf/env.sh || source /etc/sharelatex/env.sh' @@ -127,7 +128,7 @@ app.post( 'sharelatex', 'bash', '-c', - `source /etc/container_environment.sh && ${env} && /sbin/setuser www-data node ${JSON.stringify(script)} ${args.map(a => JSON.stringify(a)).join(' ')}`, + `source /etc/container_environment.sh && ${env} && /sbin/setuser ${user} node ${script} ${args.map(a => JSON.stringify(a)).join(' ')}`, ], (error, stdout, stderr) => { res.json({ diff --git a/services/filestore/app.js b/services/filestore/app.js index 24741e079c..e69515ed7d 100644 --- a/services/filestore/app.js +++ b/services/filestore/app.js @@ -119,6 +119,17 @@ app.get( fileController.getFile ) +app.get( + '/history/global/hash/:hash', + keyBuilder.globalBlobFileKeyMiddleware, + fileController.getFile +) +app.get( + '/history/project/:historyId/hash/:hash', + keyBuilder.projectBlobFileKeyMiddleware, + fileController.getFile +) + app.get('/status', function (req, res) { if (settings.shuttingDown) { res.sendStatus(503) // Service unavailable diff --git a/services/filestore/app/js/FileController.js b/services/filestore/app/js/FileController.js index 127bbcc20f..2f77bd015d 100644 --- a/services/filestore/app/js/FileController.js +++ b/services/filestore/app/js/FileController.js @@ -25,6 +25,7 @@ function getFile(req, res, next) { format, style, } + if (req.useSubdirectories) options.useSubdirectories = true metrics.inc('getFile') req.requestLogger.setMessage('getting file') diff --git a/services/filestore/app/js/KeyBuilder.js b/services/filestore/app/js/KeyBuilder.js index f67a0e81d7..66c7381710 100644 --- a/services/filestore/app/js/KeyBuilder.js +++ b/services/filestore/app/js/KeyBuilder.js @@ -1,4 +1,5 @@ const settings = require('@overleaf/settings') +const projectKey = require('./project_key') module.exports = { getConvertedFolderKey, @@ -6,6 +7,8 @@ module.exports = { userFileKeyMiddleware, userProjectKeyMiddleware, bucketFileKeyMiddleware, + globalBlobFileKeyMiddleware, + projectBlobFileKeyMiddleware, templateFileKeyMiddleware, } @@ -50,6 +53,22 @@ function bucketFileKeyMiddleware(req, res, next) { next() } +function globalBlobFileKeyMiddleware(req, res, next) { + req.bucket = settings.filestore.stores.global_blobs + const { hash } = req.params + req.key = `${hash.slice(0, 2)}/${hash.slice(2, 4)}/${hash.slice(4)}` + req.useSubdirectories = true + next() +} + +function projectBlobFileKeyMiddleware(req, res, next) { + req.bucket = settings.filestore.stores.project_blobs + const { historyId, hash } = req.params + req.key = `${projectKey.format(historyId)}/${hash.slice(0, 2)}/${hash.slice(2)}` + req.useSubdirectories = true + next() +} + function templateFileKeyMiddleware(req, res, next) { const { template_id: templateId, diff --git a/services/web/app/src/Features/History/project_key.js b/services/filestore/app/js/project_key.js similarity index 90% rename from services/web/app/src/Features/History/project_key.js rename to services/filestore/app/js/project_key.js index a4722db09a..55da401c38 100644 --- a/services/web/app/src/Features/History/project_key.js +++ b/services/filestore/app/js/project_key.js @@ -1,5 +1,4 @@ // Keep in sync with services/history-v1/storage/lib/project_key.js -const _ = require('lodash') const path = require('node:path') // @@ -13,7 +12,7 @@ function format(projectId) { } function pad(number) { - return _.padStart(number, 9, '0') + return (number || 0).toString().padStart(9, '0') } function naiveReverse(string) { diff --git a/services/history-v1/storage/lib/project_key.js b/services/history-v1/storage/lib/project_key.js index 03fb2a5141..cd559860d7 100644 --- a/services/history-v1/storage/lib/project_key.js +++ b/services/history-v1/storage/lib/project_key.js @@ -1,5 +1,4 @@ -// Keep in sync with services/web/app/src/Features/History/project_key.js -const _ = require('lodash') +// Keep in sync with services/filestore/app/js/project_key.js const path = require('node:path') // @@ -13,7 +12,7 @@ function format(projectId) { } function pad(number) { - return _.padStart(number, 9, '0') + return (number || 0).toString().padStart(9, '0') } function naiveReverse(string) { diff --git a/services/history-v1/test/acceptance/js/storage/project_key.test.js b/services/history-v1/test/acceptance/js/storage/project_key.test.js index 4aa6c1f504..63cc5e0c9b 100644 --- a/services/history-v1/test/acceptance/js/storage/project_key.test.js +++ b/services/history-v1/test/acceptance/js/storage/project_key.test.js @@ -13,6 +13,8 @@ describe('projectKey', function () { }) it('pads numbers with zeros to length 9', function () { + expect(pad(undefined)).to.equal('000000000') + expect(pad(null)).to.equal('000000000') expect(pad(1)).to.equal('000000001') expect(pad(10)).to.equal('000000010') expect(pad(100000000)).to.equal('100000000') diff --git a/services/web/app/src/Features/Compile/ClsiManager.js b/services/web/app/src/Features/Compile/ClsiManager.js index 6f11297248..19370684dd 100644 --- a/services/web/app/src/Features/Compile/ClsiManager.js +++ b/services/web/app/src/Features/Compile/ClsiManager.js @@ -26,7 +26,7 @@ const DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandle const Metrics = require('@overleaf/metrics') const Errors = require('../Errors/Errors') const ClsiCacheHandler = require('./ClsiCacheHandler') -const { getBlobLocation } = require('../History/HistoryManager') +const { getFilestoreBlobURL } = require('../History/HistoryManager') const VALID_COMPILERS = ['pdflatex', 'latex', 'xelatex', 'lualatex'] const OUTPUT_FILE_TIMEOUT_MS = 60000 @@ -755,8 +755,7 @@ function _finaliseRequest(projectId, options, project, docs, files) { let url = filestoreURL let fallbackURL if (file.hash && Features.hasFeature('project-history-blobs')) { - const { bucket, key } = getBlobLocation(historyId, file.hash) - url = `${Settings.apis.filestore.url}/bucket/${bucket}/key/${key}` + url = getFilestoreBlobURL(historyId, file.hash) fallbackURL = filestoreURL } resources.push({ diff --git a/services/web/app/src/Features/History/HistoryManager.js b/services/web/app/src/Features/History/HistoryManager.js index 42d7e229bf..a2fb201399 100644 --- a/services/web/app/src/Features/History/HistoryManager.js +++ b/services/web/app/src/Features/History/HistoryManager.js @@ -15,11 +15,6 @@ const { db, ObjectId, waitForDb } = require('../../infrastructure/mongodb') const Metrics = require('@overleaf/metrics') const logger = require('@overleaf/logger') const { NotFoundError } = require('../Errors/Errors') -const projectKey = require('./project_key') - -// BEGIN copy from services/history-v1/storage/lib/blob_store/index.js - -const GLOBAL_BLOBS = new Set() // CHANGE FROM SOURCE: only store hashes. const HISTORY_V1_URL = settings.apis.v1_history.url const HISTORY_V1_BASIC_AUTH = { @@ -27,27 +22,9 @@ const HISTORY_V1_BASIC_AUTH = { password: settings.apis.v1_history.pass, } -function makeGlobalKey(hash) { - return `${hash.slice(0, 2)}/${hash.slice(2, 4)}/${hash.slice(4)}` -} +// BEGIN copy from services/history-v1/storage/lib/blob_store/index.js -function makeProjectKey(projectId, hash) { - return `${projectKey.format(projectId)}/${hash.slice(0, 2)}/${hash.slice(2)}` -} - -function getBlobLocation(projectId, hash) { - if (GLOBAL_BLOBS.has(hash)) { - return { - bucket: settings.apis.v1_history.buckets.globalBlobs, - key: makeGlobalKey(hash), - } - } else { - return { - bucket: settings.apis.v1_history.buckets.projectBlobs, - key: makeProjectKey(projectId, hash), - } - } -} +const GLOBAL_BLOBS = new Set() // CHANGE FROM SOURCE: only store hashes. async function loadGlobalBlobs() { await waitForDb() // CHANGE FROM SOURCE: wait for db before running query. @@ -59,6 +36,14 @@ async function loadGlobalBlobs() { // END copy from services/history-v1/storage/lib/blob_store/index.js +function getFilestoreBlobURL(historyId, hash) { + if (GLOBAL_BLOBS.has(hash)) { + return `${settings.apis.filestore.url}/history/global/hash/${hash}` + } else { + return `${settings.apis.filestore.url}/history/project/${historyId}/hash/${hash}` + } +} + async function initializeProject(projectId) { const body = await fetchJson(`${settings.apis.project_history.url}/project`, { method: 'POST', @@ -421,7 +406,7 @@ function _userView(user) { const loadGlobalBlobsPromise = loadGlobalBlobs() module.exports = { - getBlobLocation, + getFilestoreBlobURL, loadGlobalBlobsPromise, initializeProject: callbackify(initializeProject), flushProject: callbackify(flushProject), diff --git a/services/web/test/unit/src/Compile/ClsiManagerTests.js b/services/web/test/unit/src/Compile/ClsiManagerTests.js index b9b7d9af37..e35c8735cb 100644 --- a/services/web/test/unit/src/Compile/ClsiManagerTests.js +++ b/services/web/test/unit/src/Compile/ClsiManagerTests.js @@ -153,14 +153,11 @@ describe('ClsiManager', function () { hasFeature: sinon.stub().withArgs('project-history-blobs').returns(true), } this.HistoryManager = { - getBlobLocation: sinon.stub().callsFake((historyId, hash) => { + getFilestoreBlobURL: sinon.stub().callsFake((historyId, hash) => { if (hash === GLOBAL_BLOB_HASH) { - return { - bucket: 'global-blobs', - key: 'aa/aa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', - } + return `${FILESTORE_URL}/history/global/hash/${hash}` } - return { bucket: 'project-blobs', key: `${historyId}/${hash}` } + return `${FILESTORE_URL}/history/project/${historyId}/hash/${hash}` }), } @@ -1053,10 +1050,10 @@ function _makeResources(project, docs, files) { for (const [path, file] of Object.entries(files)) { let url, fallbackURL if (file.hash === GLOBAL_BLOB_HASH) { - url = `${FILESTORE_URL}/bucket/global-blobs/key/aa/aa/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa` + url = `${FILESTORE_URL}/history/global/hash/${file.hash}` fallbackURL = `${FILESTORE_URL}/project/${project._id}/file/${file._id}` } else if (file.hash) { - url = `${FILESTORE_URL}/bucket/project-blobs/key/${project.overleaf.history.id}/${file.hash}` + url = `${FILESTORE_URL}/history/project/${project.overleaf.history.id}/hash/${file.hash}` fallbackURL = `${FILESTORE_URL}/project/${project._id}/file/${file._id}` } else { url = `${FILESTORE_URL}/project/${project._id}/file/${file._id}` diff --git a/services/web/test/unit/src/History/HistoryManagerTests.js b/services/web/test/unit/src/History/HistoryManagerTests.js index aa59cda4e6..b38984c75e 100644 --- a/services/web/test/unit/src/History/HistoryManagerTests.js +++ b/services/web/test/unit/src/History/HistoryManagerTests.js @@ -10,12 +10,10 @@ const { const MODULE_PATH = '../../../../app/src/Features/History/HistoryManager' -const GLOBAL_BLOBS = { - e69de29bb2d1d6434b8b29ae775ad8c2e48c5391: - 'e6/9d/e29bb2d1d6434b8b29ae775ad8c2e48c5391', - '02426c2b3a484003ca42ed52b374b7907b757d12': - '02/42/6c2b3a484003ca42ed52b374b7907b757d12', -} +const GLOBAL_BLOBS = [ + 'e69de29bb2d1d6434b8b29ae775ad8c2e48c5391', + '02426c2b3a484003ca42ed52b374b7907b757d12', +] describe('HistoryManager', function () { before(async function () { @@ -24,7 +22,7 @@ describe('HistoryManager', function () { before(cleanupTestDatabase) before(async function () { await db.projectHistoryGlobalBlobs.insertMany( - Object.keys(GLOBAL_BLOBS).map(sha => ({ + GLOBAL_BLOBS.map(sha => ({ _id: sha, byteLength: 0, stringLength: 0, @@ -48,6 +46,9 @@ describe('HistoryManager', function () { this.v1HistoryPassword = 'verysecret' this.settings = { apis: { + filestore: { + url: 'http://filestore.example.com', + }, project_history: { url: this.projectHistoryUrl, }, @@ -100,25 +101,30 @@ describe('HistoryManager', function () { }) }) - describe('getBlobLocation', function () { + describe('getFilestoreBlobURL', function () { beforeEach(async function () { await this.HistoryManager.loadGlobalBlobsPromise }) it('should return a global blob location', function () { - for (const [sha, key] of Object.entries(GLOBAL_BLOBS)) { - expect(this.HistoryManager.getBlobLocation('42', sha)).to.deep.equal({ - bucket: this.settings.apis.v1_history.buckets.globalBlobs, - key, - }) + for (const sha of GLOBAL_BLOBS) { + expect(this.HistoryManager.getFilestoreBlobURL('42', sha)).to.equal( + `${this.settings.apis.filestore.url}/history/global/hash/${sha}` + ) } }) - it('should return a project blob location', function () { + it('should return a project blob location for a v1 project', function () { + const historyId = 42 const sha = '6ddfa0578a67fe5ad6623a8665ec9aafce1eb5ca' - const key = '240/000/000/6d/dfa0578a67fe5ad6623a8665ec9aafce1eb5ca' - expect(this.HistoryManager.getBlobLocation('42', sha)).to.deep.equal({ - bucket: this.settings.apis.v1_history.buckets.projectBlobs, - key, - }) + expect(this.HistoryManager.getFilestoreBlobURL(historyId, sha)).to.equal( + `${this.settings.apis.filestore.url}/history/project/${historyId}/hash/${sha}` + ) + }) + it('should return a project blob location for a mongo project', function () { + const historyId = '424242424242424242424242' + const sha = '6ddfa0578a67fe5ad6623a8665ec9aafce1eb5ca' + expect(this.HistoryManager.getFilestoreBlobURL(historyId, sha)).to.equal( + `${this.settings.apis.filestore.url}/history/project/${historyId}/hash/${sha}` + ) }) })