From 1f8a710f0d4c44bcb0df7f2b08207b9239213a83 Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Thu, 28 Nov 2024 15:00:49 +0000 Subject: [PATCH] Merge pull request #22161 from overleaf/bg-issue22125 Copy blobs when duplicating project GitOrigin-RevId: a2965082ddaa621e10cfe831b2701a019e45a9ac --- .../src/Features/History/HistoryManager.js | 16 +++++++ .../src/Features/Project/ProjectDuplicator.js | 32 ++++++++++++++ .../acceptance/src/mocks/MockV1HistoryApi.js | 12 +++++ .../src/Project/ProjectDuplicatorTests.js | 44 ++++++++++++++++++- 4 files changed, 103 insertions(+), 1 deletion(-) diff --git a/services/web/app/src/Features/History/HistoryManager.js b/services/web/app/src/Features/History/HistoryManager.js index dbb399ac29..707771ca0b 100644 --- a/services/web/app/src/Features/History/HistoryManager.js +++ b/services/web/app/src/Features/History/HistoryManager.js @@ -139,6 +139,20 @@ async function uploadBlobFromDisk(historyId, hash, byteLength, fsPath) { }) } +async function copyBlob(sourceHistoryId, targetHistoryId, hash) { + const url = `${settings.apis.v1_history.url}/projects/${targetHistoryId}/blobs/${hash}` + await fetchNothing( + `${url}?${new URLSearchParams({ copyFrom: sourceHistoryId })}`, + { + method: 'POST', + basicAuth: { + user: settings.apis.v1_history.user, + password: settings.apis.v1_history.pass, + }, + } + ) +} + async function requestBlobWithFallback( projectId, hash, @@ -351,6 +365,7 @@ module.exports = { injectUserDetails: callbackify(injectUserDetails), getCurrentContent: callbackify(getCurrentContent), uploadBlobFromDisk: callbackify(uploadBlobFromDisk), + copyBlob: callbackify(copyBlob), requestBlobWithFallback: callbackify(requestBlobWithFallback), promises: { initializeProject, @@ -362,6 +377,7 @@ module.exports = { getCurrentContent, getContentAtVersion, uploadBlobFromDisk, + copyBlob, requestBlobWithFallback, }, } diff --git a/services/web/app/src/Features/Project/ProjectDuplicator.js b/services/web/app/src/Features/Project/ProjectDuplicator.js index 87c71d2448..18594d3ff8 100644 --- a/services/web/app/src/Features/Project/ProjectDuplicator.js +++ b/services/web/app/src/Features/Project/ProjectDuplicator.js @@ -1,5 +1,6 @@ const { callbackify } = require('util') const Path = require('path') +const logger = require('@overleaf/logger') const OError = require('@overleaf/o-error') const { promiseMapWithLimit } = require('@overleaf/promise-utils') const { Doc } = require('../../models/Doc') @@ -7,6 +8,7 @@ const { File } = require('../../models/File') const DocstoreManager = require('../Docstore/DocstoreManager') const DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler') const FileStoreHandler = require('../FileStore/FileStoreHandler') +const HistoryManager = require('../History/HistoryManager') const ProjectCreationHandler = require('./ProjectCreationHandler') const ProjectDeleter = require('./ProjectDeleter') const ProjectEntityMongoUpdateHandler = require('./ProjectEntityMongoUpdateHandler') @@ -36,6 +38,7 @@ async function duplicate(owner, originalProjectId, newProjectName, tags = []) { rootDoc_id: true, fromV1TemplateId: true, fromV1TemplateVersionId: true, + overleaf: true, } ) const { path: rootDocPath } = await ProjectLocator.promises.findRootDoc({ @@ -185,6 +188,15 @@ async function _getDocLinesForProject(projectId) { } async function _copyFiles(sourceEntries, sourceProject, targetProject) { + const sourceHistoryId = sourceProject.overleaf?.history?.id + const targetHistoryId = targetProject.overleaf?.history?.id + if (!sourceHistoryId) { + throw new OError('missing history id', { sourceProject }) + } + if (!targetHistoryId) { + throw new OError('missing history id', { targetProject }) + } + const targetEntries = await promiseMapWithLimit( 5, sourceEntries, @@ -199,6 +211,26 @@ async function _copyFiles(sourceEntries, sourceProject, targetProject) { if (sourceFile.hash != null) { file.hash = sourceFile.hash } + if (file.hash != null) { + try { + await HistoryManager.promises.copyBlob( + sourceHistoryId, + targetHistoryId, + file.hash + ) + } catch (err) { + logger.error( + { + err, + sourceProjectId: sourceProject._id, + targetProjectId: targetProject._id, + sourceFile, + sourceHistoryId, + }, + 'unexpected error copying blob' + ) + } + } const url = await FileStoreHandler.promises.copyFile( sourceProject._id, sourceFile._id, diff --git a/services/web/test/acceptance/src/mocks/MockV1HistoryApi.js b/services/web/test/acceptance/src/mocks/MockV1HistoryApi.js index d1e0ca755e..1a109c43ac 100644 --- a/services/web/test/acceptance/src/mocks/MockV1HistoryApi.js +++ b/services/web/test/acceptance/src/mocks/MockV1HistoryApi.js @@ -4,6 +4,7 @@ const { zipAttachment, prepareZipAttachment, } = require('../../../../app/src/infrastructure/Response') +const Joi = require('joi') class MockV1HistoryApi extends AbstractMockApi { reset() { @@ -105,6 +106,17 @@ class MockV1HistoryApi extends AbstractMockApi { if (!buf) return res.status(404).end() res.status(200).end(buf) }) + + this.app.post('/api/projects/:project_id/blobs/:hash', (req, res, next) => { + const schema = Joi.object({ + copyFrom: Joi.number().required(), + }) + const { error } = schema.validate(req.query) + if (error) { + return res.sendStatus(400) + } + res.sendStatus(204) + }) } } diff --git a/services/web/test/unit/src/Project/ProjectDuplicatorTests.js b/services/web/test/unit/src/Project/ProjectDuplicatorTests.js index 44c3914062..df53bbb044 100644 --- a/services/web/test/unit/src/Project/ProjectDuplicatorTests.js +++ b/services/web/test/unit/src/Project/ProjectDuplicatorTests.js @@ -13,7 +13,7 @@ describe('ProjectDuplicator', function () { this.doc0Lines = ['zero'] this.doc1Lines = ['one'] this.doc2Lines = ['two'] - this.file0 = { name: 'file0', _id: 'file0' } + this.file0 = { name: 'file0', _id: 'file0', hash: 'abcde' } this.file1 = { name: 'file1', _id: 'file1' } this.file2 = { name: 'file2', @@ -48,6 +48,7 @@ describe('ProjectDuplicator', function () { rootDoc_id: this.doc0._id, rootFolder: [this.rootFolder], compiler: 'this_is_a_Compiler', + overleaf: { history: { id: 123456 } }, } this.doc0Path = '/rootDocHere' this.doc1Path = '/level1folder/level1folderDocName' @@ -144,6 +145,16 @@ describe('ProjectDuplicator', function () { copyFile: sinon.stub().resolves(this.filestoreUrl), }, } + this.HistoryManager = { + promises: { + copyBlob: sinon.stub().callsFake((historyId, newHistoryId, hash) => { + if (hash === 'abcde') { + return Promise.reject(new Error('copy blob error')) + } + return Promise.resolve() + }), + }, + } this.TagsHandler = { promises: { addProjectToTags: sinon.stub().resolves({ @@ -226,6 +237,7 @@ describe('ProjectDuplicator', function () { './ProjectOptionsHandler': this.ProjectOptionsHandler, '../ThirdPartyDataStore/TpdsProjectFlusher': this.TpdsProjectFlusher, '../Tags/TagsHandler': this.TagsHandler, + '../History/HistoryManager': this.HistoryManager, }, }) }) @@ -258,6 +270,36 @@ describe('ProjectDuplicator', function () { } }) + it('should duplicate the files with hashes by copying the blobs in history v1', function () { + for (const file of [this.file0, this.file2]) { + this.HistoryManager.promises.copyBlob.should.have.been.calledWith( + this.project.overleaf.history.id, + this.newProject.overleaf.history.id, + file.hash + ) + } + }) + + it('should ignore any errors when copying the blobs in history v1', async function () { + await expect( + this.HistoryManager.promises.copyBlob( + this.project.overleaf.history.id, + this.newProject.overleaf.history.id, + this.file0.hash + ) + ).to.be.rejectedWith('copy blob error') + }) + + it('should not try to copy the blobs for any files without hashes', function () { + for (const file of [this.file1]) { + this.HistoryManager.promises.copyBlob.should.not.have.been.calledWith( + this.project.overleaf.history.id, + this.newProject.overleaf.history.id, + file.hash + ) + } + }) + it('should copy files to the filestore', function () { for (const file of [this.file0, this.file1, this.file2]) { this.FileStoreHandler.promises.copyFile.should.have.been.calledWith(