Merge pull request #22533 from overleaf/ar-only-use-history-for-blobs-when-enabled

[web] only use history for blobs when enabled

GitOrigin-RevId: 010983e9b29657d4c594e03945dca5700577bf0a
This commit is contained in:
Brian Gough
2025-01-10 09:05:32 +00:00
committed by Copybot
parent eb1b939f5a
commit 2ce82fabab
22 changed files with 303 additions and 150 deletions
@@ -9,6 +9,7 @@ import { fileURLToPath } from 'node:url'
import sinon from 'sinon'
import logger from '@overleaf/logger'
import Metrics from './helpers/metrics.js'
import Features from '../../../app/src/infrastructure/Features.js'
const User = UserHelper.promises
let MockV1HistoryApi, MockFilestoreApi
@@ -75,41 +76,45 @@ describe('HistoryTests', function () {
afterEach(function () {
spy.restore()
})
it('should work from history-v1', async function () {
const { response, body } = await user.doRequest('GET', downloadZIPURL)
expect(response.statusCode).to.equal(200)
expect(body).to.include('2pixel.png')
await expectHistoryV1Hit()
})
it('should work from filestore', async function () {
MockV1HistoryApi.reset()
const { response, body } = await user.doRequest('GET', downloadZIPURL)
expect(response.statusCode).to.equal(200)
expect(body).to.include('2pixel.png')
await expectFilestoreHit()
})
it('should not include when missing in both places', async function () {
MockFilestoreApi.reset()
MockV1HistoryApi.reset()
const { response, body } = await user.doRequest('GET', downloadZIPURL)
expect(response.statusCode).to.equal(200)
expect(
spy.args.find(([, msg]) => msg === 'error adding files to zip stream')
).to.exist
expect(body).to.not.include('2pixel.png')
await expectNoIncrement()
})
if (Features.hasFeature('project-history-blobs')) {
it('should work from history-v1', async function () {
const { response, body } = await user.doRequest('GET', downloadZIPURL)
expect(response.statusCode).to.equal(200)
expect(body).to.include('2pixel.png')
await expectHistoryV1Hit()
})
it('should work from filestore', async function () {
MockV1HistoryApi.reset()
const { response, body } = await user.doRequest('GET', downloadZIPURL)
expect(response.statusCode).to.equal(200)
expect(body).to.include('2pixel.png')
await expectFilestoreHit()
})
it('should not include when missing in both places', async function () {
MockFilestoreApi.reset()
MockV1HistoryApi.reset()
const { response, body } = await user.doRequest('GET', downloadZIPURL)
expect(response.statusCode).to.equal(200)
expect(
spy.args.find(([, msg]) => msg === 'error adding files to zip stream')
).to.exist
expect(body).to.not.include('2pixel.png')
await expectNoIncrement()
})
}
})
describe('/project/:projectId/blob/:hash', function () {
describe('HEAD', function () {
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()
})
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 return 404 without fallback', async function () {
MockV1HistoryApi.reset()
const { response } = await user.doRequest('HEAD', fileURL)
@@ -133,13 +138,15 @@ describe('HistoryTests', function () {
})
})
describe('GET', function () {
it('should fetch the file from history-v1', async function () {
const { response, body } = await user.doRequest('GET', fileURL)
expect(response.statusCode).to.equal(200)
expect(response.headers['x-served-by']).to.include('history-v1')
expect(body).to.equal(fileContent)
await expectHistoryV1Hit()
})
if (Features.hasFeature('project-history-blobs')) {
it('should fetch the file from history-v1', async function () {
const { response, body } = await user.doRequest('GET', fileURL)
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 return 404 without fallback', async function () {
MockV1HistoryApi.reset()
const { response } = await user.doRequest('GET', fileURL)
@@ -1,5 +1,6 @@
export function resetMeta() {
window.metaAttributesCache = new Map()
window.metaAttributesCache.set('ol-projectHistoryBlobsEnabled', true)
window.metaAttributesCache.set('ol-i18n', { currentLangCode: 'en' })
window.metaAttributesCache.set('ol-ExposedSettings', {
appName: 'Overleaf',
@@ -144,6 +144,9 @@ describe('ClsiManager', function () {
enablePdfCaching: true,
clsiCookie: { key: 'clsiserver' },
}
this.Features = {
hasFeature: sinon.stub().withArgs('project-history-blobs').returns(true),
}
this.HistoryManager = {
getBlobLocation: sinon.stub().callsFake((historyId, hash) => {
if (hash === GLOBAL_BLOB_HASH) {
@@ -162,6 +165,7 @@ describe('ClsiManager', function () {
'../../models/Project': {
Project: this.Project,
},
'../../infrastructure/Features': this.Features,
'../Project/ProjectEntityHandler': this.ProjectEntityHandler,
'../Project/ProjectGetter': this.ProjectGetter,
'../DocumentUpdater/DocumentUpdaterHandler':
@@ -27,6 +27,7 @@ describe('ProjectZipStreamManager', function () {
info: sinon.stub(),
debug: sinon.stub(),
}
return (this.ProjectZipStreamManager = await esmock.strict(modulePath, {
archiver: (this.archiver = sinon.stub().returns(this.archive)),
'@overleaf/logger': this.logger,
@@ -36,6 +37,14 @@ describe('ProjectZipStreamManager', function () {
(this.HistoryManager = {}),
'../../../../app/src/Features/Project/ProjectGetter':
(this.ProjectGetter = {}),
'../../../../app/src/Features/FileStore/FileStoreHandler':
(this.FileStoreHandler = {}),
'../../../../app/src/infrastructure/Features': (this.Features = {
hasFeature: sinon
.stub()
.withArgs('project-history-blobs')
.returns(true),
}),
}))
})
@@ -380,65 +389,89 @@ describe('ProjectZipStreamManager', function () {
this.ProjectEntityHandler.getAllFiles = sinon
.stub()
.callsArgWith(1, null, this.files)
this.HistoryManager.requestBlobWithFallback = (
projectId,
hash,
fileId,
callback
) => {
return callback(null, { stream: this.streams[fileId] })
}
sinon.spy(this.HistoryManager, 'requestBlobWithFallback')
this.ProjectZipStreamManager.addAllFilesToArchive(
this.project_id,
this.archive,
this.callback
)
return (() => {
const result = []
})
describe('with project-history-blobs feature enabled', function () {
beforeEach(function () {
this.HistoryManager.requestBlobWithFallback = (
projectId,
hash,
fileId,
callback
) => {
return callback(null, { stream: this.streams[fileId] })
}
sinon.spy(this.HistoryManager, 'requestBlobWithFallback')
this.ProjectZipStreamManager.addAllFilesToArchive(
this.project_id,
this.archive,
this.callback
)
for (const path in this.streams) {
const stream = this.streams[path]
result.push(stream.emit('end'))
stream.emit('end')
}
return result
})()
})
})
it('should get the files for the project', function () {
return this.ProjectEntityHandler.getAllFiles
.calledWith(this.project_id)
.should.equal(true)
})
it('should get the files for the project', function () {
return this.ProjectEntityHandler.getAllFiles
.calledWith(this.project_id)
.should.equal(true)
})
it('should get a stream for each file', function () {
return (() => {
const result = []
it('should get a stream for each file', function () {
for (const path in this.files) {
const file = this.files[path]
result.push(
this.HistoryManager.requestBlobWithFallback
.calledWith(this.project_id, file.hash, file._id)
.should.equal(true)
)
}
return result
})()
})
it('should add each file to the archive', function () {
return (() => {
const result = []
this.HistoryManager.requestBlobWithFallback
.calledWith(this.project_id, file.hash, file._id)
.should.equal(true)
}
})
it('should add each file to the archive', function () {
for (let path in this.files) {
const file = this.files[path]
path = path.slice(1) // remove "/"
result.push(
this.archive.append
.calledWith(this.streams[file._id], { name: path })
.should.equal(true)
)
this.archive.append
.calledWith(this.streams[file._id], { name: path })
.should.equal(true)
}
return result
})()
})
})
describe('with project-history-blobs feature disabled', function () {
beforeEach(function () {
this.FileStoreHandler.getFileStream = (
projectId,
fileId,
query,
callback
) => callback(null, this.streams[fileId])
sinon.spy(this.FileStoreHandler, 'getFileStream')
this.Features.hasFeature
.withArgs('project-history-blobs')
.returns(false)
this.ProjectZipStreamManager.addAllFilesToArchive(
this.project_id,
this.archive,
this.callback
)
for (const path in this.streams) {
const stream = this.streams[path]
stream.emit('end')
}
})
it('should get a stream for each file', function () {
for (const path in this.files) {
const file = this.files[path]
this.FileStoreHandler.getFileStream
.calledWith(this.project_id, file._id)
.should.equal(true)
}
})
})
})
})
@@ -8,6 +8,7 @@ const MODULE_PATH = '../../../../app/src/Features/FileStore/FileStoreHandler.js'
describe('FileStoreHandler', function () {
beforeEach(function () {
this.fileSize = 999
this.fs = {
createReadStream: sinon.stub(),
lstat: sinon.stub().callsArgWith(1, null, {
@@ -40,7 +41,6 @@ describe('FileStoreHandler', function () {
this.fileId = 'file_id_here'
this.projectId = '1312312312'
this.historyId = 123
this.fileSize = 999
this.hashValue = '2aae6c35c94fcfb415dbe95f408b9ce91ee846ed'
this.fsPath = 'uploads/myfile.eps'
this.getFileUrl = (projectId, fileId) =>
@@ -69,6 +69,10 @@ describe('FileStoreHandler', function () {
}),
}
this.Features = {
hasFeature: sinon.stub(),
}
this.handler = SandboxedModule.require(MODULE_PATH, {
requires: {
'@overleaf/settings': this.settings,
@@ -76,6 +80,7 @@ describe('FileStoreHandler', function () {
'../History/HistoryManager': this.HistoryManager,
'../Project/ProjectDetailsHandler': this.ProjectDetailsHandler,
'./FileHashManager': this.FileHashManager,
'../../infrastructure/Features': this.Features,
// FIXME: need to stub File object here
'../../models/File': {
File: this.FileModel,
@@ -134,31 +139,55 @@ describe('FileStoreHandler', function () {
)
})
it('should upload the file to the history store as a blob', function (done) {
this.fs.createReadStream.returns({
pipe() {},
on(type, cb) {
if (type === 'open') {
cb()
describe('when project-history-blobs feature is enabled', function () {
it('should upload the file to the history store as a blob', function (done) {
this.fs.createReadStream.returns({
pipe() {},
on(type, cb) {
if (type === 'open') {
cb()
}
},
})
this.Features.hasFeature.withArgs('project-history-blobs').returns(true)
this.handler.uploadFileFromDisk(
this.projectId,
this.fileArgs,
this.fsPath,
() => {
this.HistoryManager.uploadBlobFromDisk
.calledWith(
this.historyId,
this.hashValue,
this.fileSize,
this.fsPath
)
.should.equal(true)
done()
}
},
)
})
})
describe('when project-history-blobs feature is disabled', function () {
it('should not upload the file to the history store as a blob', function (done) {
this.fs.createReadStream.returns({
pipe() {},
on(type, cb) {
if (type === 'open') {
cb()
}
},
})
this.handler.uploadFileFromDisk(
this.projectId,
this.fileArgs,
this.fsPath,
() => {
this.HistoryManager.uploadBlobFromDisk.called.should.equal(false)
done()
}
)
})
this.handler.uploadFileFromDisk(
this.projectId,
this.fileArgs,
this.fsPath,
() => {
this.HistoryManager.uploadBlobFromDisk
.calledWith(
this.historyId,
this.hashValue,
this.fileSize,
this.fsPath
)
.should.equal(true)
done()
}
)
})
it('should create read stream', function (done) {
@@ -221,6 +221,9 @@ describe('ProjectDuplicator', function () {
flushProjectToTpds: sinon.stub().resolves(),
},
}
this.Features = {
hasFeature: sinon.stub().withArgs('project-history-blobs').returns(true),
}
this.ProjectDuplicator = SandboxedModule.require(MODULE_PATH, {
requires: {
@@ -241,6 +244,7 @@ describe('ProjectDuplicator', function () {
'../ThirdPartyDataStore/TpdsProjectFlusher': this.TpdsProjectFlusher,
'../Tags/TagsHandler': this.TagsHandler,
'../History/HistoryManager': this.HistoryManager,
'../../infrastructure/Features': this.Features,
},
})
})
@@ -54,6 +54,7 @@ describe('ReferencesHandler', function () {
filestore: { url: 'http://some.url/filestore' },
project_history: { url: 'http://project-history.local' },
},
enableProjectHistoryBlobs: true,
}),
request: (this.request = {
get: sinon.stub(),
@@ -57,6 +57,7 @@ describe('TpdsUpdateSender', function () {
url: projectHistoryUrl,
},
},
enableProjectHistoryBlobs: true,
}
const getUsers = sinon.stub()
getUsers