github/overleaf
1
0
Fork 0
mirror of https://github.com/overleaf/overleaf.git synced 2025-04-22 02:18:27 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #22213 from overleaf/ar-avoid-duplicate-blob-writes

Browse source 
Avoid duplicate blob writes

GitOrigin-RevId: 5e7a472b235bae5ef84389dbbe791189e951908a
This commit is contained in:
Andrew Rumble 2024-12-03 12:07:35 +00:00 • committed by Copybot
parent f676eca2b8
commit 40b781eb0a
12 changed files with 143 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
services/document-updater/app/js/ProjectHistoryRedisManager.js
View file

@ -122,6 +122,7 @@ const ProjectHistoryRedisManager = {
hash: projectUpdate.hash,
metadata: projectUpdate.metadata,
projectHistoryId,
createdBlob: projectUpdate.createdBlob ?? false,
}
if (ranges) {
projectUpdate.ranges = ranges

66
services/document-updater/test/unit/js/ProjectHistoryRedisManager/ProjectHistoryRedisManagerTests.js
View file

@ -162,6 +162,7 @@ describe('ProjectHistoryRedisManager', function () {
},
version: this.version,
projectHistoryId: this.projectHistoryId,
createdBlob: false,
doc: this.doc_id,
}
@ -207,6 +208,7 @@ describe('ProjectHistoryRedisManager', function () {
hash: '1337',
metadata,
projectHistoryId: this.projectHistoryId,
createdBlob: false,
file: fileId,
}
@ -291,6 +293,7 @@ describe('ProjectHistoryRedisManager', function () {
},
version: this.version,
projectHistoryId: this.projectHistoryId,
createdBlob: false,
ranges: historyCompatibleRanges,
doc: this.doc_id,
}
@ -343,6 +346,7 @@ describe('ProjectHistoryRedisManager', function () {
},
version: this.version,
projectHistoryId: this.projectHistoryId,
createdBlob: false,
doc: this.doc_id,
}
@ -394,6 +398,68 @@ describe('ProjectHistoryRedisManager', function () {
},
version: this.version,
projectHistoryId: this.projectHistoryId,
createdBlob: false,
doc: this.doc_id,
}
this.ProjectHistoryRedisManager.promises.queueOps
.calledWithExactly(this.project_id, JSON.stringify(update))
.should.equal(true)
})
it('should pass "false" as the createdBlob field if not provided', async function () {
await this.ProjectHistoryRedisManager.promises.queueAddEntity(
this.project_id,
this.projectHistoryId,
'doc',
this.doc_id,
this.user_id,
this.rawUpdate,
this.source
)
const update = {
pathname: this.pathname,
docLines: this.docLines,
meta: {
user_id: this.user_id,
ts: new Date(),
source: this.source,
},
version: this.version,
projectHistoryId: this.projectHistoryId,
createdBlob: false,
doc: this.doc_id,
}
this.ProjectHistoryRedisManager.promises.queueOps
.calledWithExactly(this.project_id, JSON.stringify(update))
.should.equal(true)
})
it('should pass through the value of the createdBlob field', async function () {
this.rawUpdate.createdBlob = true
await this.ProjectHistoryRedisManager.promises.queueAddEntity(
this.project_id,
this.projectHistoryId,
'doc',
this.doc_id,
this.user_id,
this.rawUpdate,
this.source
)
const update = {
pathname: this.pathname,
docLines: this.docLines,
meta: {
user_id: this.user_id,
ts: new Date(),
source: this.source,
},
version: this.version,
projectHistoryId: this.projectHistoryId,
createdBlob: true,
doc: this.doc_id,
}

10
services/project-history/app/js/HistoryStoreManager.js
View file

@ -300,8 +300,18 @@ export function createBlobForUpdate(projectId, historyId, update, callback) {
if (urlMatch[1] !== projectId) {
return callback(new OError('invalid project for blob creation'))
}
const fileId = urlMatch[2]
const filestoreURL = `${Settings.apis.filestore.url}/project/${projectId}/file/${fileId}`
if (update.createdBlob) {
logger.debug(
{ projectId, fileId },
'Skipping blob creation as it has already been created'
)
return callback(null, { file: update.hash })
}
fetchStream(filestoreURL, {
signal: AbortSignal.timeout(HTTP_REQUEST_TIMEOUT),
})

1
services/project-history/app/js/types.ts
View file

@ -76,6 +76,7 @@ export type AddFileUpdate = ProjectUpdateBase & {
file: string
url: string
hash: string
createdBlob?: boolean
metadata?: LinkedFileData
}

38
services/project-history/test/unit/js/HistoryStoreManager/HistoryStoreManagerTests.js
View file

@ -491,6 +491,44 @@ describe('HistoryStoreManager', function () {
expect(this.actualHash).to.equal(this.hash)
})
})
describe('when the createdBlob flag is set on the update', function () {
beforeEach(function (done) {
this.file_id = '012345678901234567890123'
this.update = {
file: true,
createdBlob: 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, { file: hash }) => {
if (err) {
return done(err)
}
this.actualHash = hash
done()
}
)
})
it('should call the callback with the existing hash', function () {
expect(this.actualHash).to.equal(this.hash)
})
it('should not request the file from the filestore', function () {
expect(this.FetchUtils.fetchStream).to.not.have.been.called
})
it('should log a debug level message', function () {
expect(this.logger.debug).to.have.been.calledWith(
{ projectId: this.projectId, fileId: this.file_id },
'Skipping blob creation as it has already been created'
)
})
})
})
describe('getProjectBlob', function () {

1
services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js
View file

@ -490,6 +490,7 @@ function _getUpdates(
url: newEntity.url,
hash: newEntity.file != null ? newEntity.file.hash : undefined,
metadata: buildFileMetadataForHistory(newEntity.file),
createdBlob: newEntity.createdBlob ?? false,
})
} else if (newEntity.path !== oldEntity.path) {
// entity renamed

4
services/web/app/src/Features/FileStore/FileStoreHandler.js
View file

@ -100,7 +100,7 @@ const FileStoreHandler = {
})
return callback(err)
}
callback(err, result.url, result.fileRef)
callback(err, result.url, result.fileRef, true)
}
)
}
@ -303,6 +303,6 @@ module.exports = FileStoreHandler
module.exports.promises = promisifyAll(FileStoreHandler, {
multiResult: {
uploadFileFromDisk: ['url', 'fileRef'],
uploadFileFromDiskWithHistoryId: ['url', 'fileRef'],
uploadFileFromDiskWithHistoryId: ['url', 'fileRef', 'createdBlob'],
},
})

4
services/web/app/src/Features/Project/ProjectDuplicator.js
View file

@ -211,6 +211,7 @@ async function _copyFiles(sourceEntries, sourceProject, targetProject) {
if (sourceFile.hash != null) {
file.hash = sourceFile.hash
}
let createdBlob = false
if (file.hash != null) {
try {
await HistoryManager.promises.copyBlob(
@ -218,6 +219,7 @@ async function _copyFiles(sourceEntries, sourceProject, targetProject) {
targetHistoryId,
file.hash
)
createdBlob = true
} catch (err) {
logger.error(
{
@ -237,7 +239,7 @@ async function _copyFiles(sourceEntries, sourceProject, targetProject) {
targetProject._id,
file._id
)
return { file, path, url }
return { createdBlob, file, path, url }
}
)
return targetEntries

4
services/web/app/src/Features/Uploads/ProjectUploadManager.js
View file

@ -194,14 +194,14 @@ async function _createFile(project, projectPath, fsPath) {
throw new OError('missing history id')
}
const fileName = Path.basename(projectPath)
const { fileRef, url } =
const { createdBlob, fileRef, url } =
await FileStoreHandler.promises.uploadFileFromDiskWithHistoryId(
projectId,
historyId,
{ name: fileName },
fsPath
)
return { file: fileRef, path: projectPath, url }
return { createdBlob, file: fileRef, path: projectPath, url }
}
async function _notifyDocumentUpdater(project, userId, changes) {

5
services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js
View file

@ -1132,6 +1132,7 @@ describe('DocumentUpdaterHandler', function () {
hash: undefined,
ranges: undefined,
metadata: undefined,
createdBlob: false,
},
]
@ -1184,6 +1185,7 @@ describe('DocumentUpdaterHandler', function () {
historyRangesSupport: false,
hash: '12345',
ranges: undefined,
createdBlob: false,
metadata: undefined,
},
]
@ -1296,6 +1298,7 @@ describe('DocumentUpdaterHandler', function () {
hash: undefined,
ranges: undefined,
metadata: undefined,
createdBlob: false,
},
]
@ -1398,6 +1401,7 @@ describe('DocumentUpdaterHandler', function () {
hash: undefined,
ranges: this.ranges,
metadata: undefined,
createdBlob: false,
},
]
@ -1444,6 +1448,7 @@ describe('DocumentUpdaterHandler', function () {
hash: undefined,
ranges: this.ranges,
metadata: undefined,
createdBlob: false,
},
]

3
services/web/test/unit/src/Project/ProjectDuplicatorTests.js
View file

@ -105,16 +105,19 @@ describe('ProjectDuplicator', function () {
]
this.fileEntries = [
{
createdBlob: false,
path: this.file0Path,
file: this.newFile0,
url: this.filestoreUrl,
},
{
createdBlob: false,
path: this.file1Path,
file: this.newFile1,
url: this.filestoreUrl,
},
{
createdBlob: true,
path: this.file2Path,
file: this.newFile2,
url: this.filestoreUrl,

15
services/web/test/unit/src/Uploads/ProjectUploadManagerTests.js
View file

@ -58,7 +58,12 @@ describe('ProjectUploadManager', function () {
},
]
this.fileEntries = [
{ file: this.file, path: `/${this.file.name}`, url: this.fileStoreUrl },
{
file: this.file,
path: `/${this.file.name}`,
url: this.fileStoreUrl,
createdBlob: true,
},
]
this.fs = {
@ -94,9 +99,11 @@ describe('ProjectUploadManager', function () {
}
this.FileStoreHandler = {
promises: {
uploadFileFromDiskWithHistoryId: sinon
.stub()
.resolves({ fileRef: this.file, url: this.fileStoreUrl }),
uploadFileFromDiskWithHistoryId: sinon.stub().resolves({
fileRef: this.file,
url: this.fileStoreUrl,
createdBlob: true,
}),
},
}
this.FileSystemImportManager = {