From bed59cf8f621cb108b8fb182217cb96a672ddb2e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 10 Dec 2018 09:37:06 +0000 Subject: [PATCH] Merge pull request #1223 from sharelatex/bg-fast-import parallelise file uploads for v1 project import GitOrigin-RevId: d4ae617b26e6d341bccd6202a1697a6ba3fc01ad --- .../Project/ProjectEntityUpdateHandler.coffee | 100 ++++++-------- .../ProjectEntityUpdateHandlerTests.coffee | 127 +----------------- 2 files changed, 49 insertions(+), 178 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index 079065a948..5febe71fa6 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -128,20 +128,45 @@ module.exports = ProjectEntityUpdateHandler = self = logger.log project_id: project_id, "removing root doc" Project.update {_id:project_id}, {$unset: {rootDoc_id: true}}, {}, callback - addDoc: wrapWithLock (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> - if not SafePath.isCleanFilename docName - return callback new Errors.InvalidNameError("invalid element name") - self.addDocWithoutUpdatingHistory.withoutLock project_id, folder_id, docName, docLines, userId, (error, doc, folder_id, path, project) -> - return callback(error) if error? - projectHistoryId = project.overleaf?.history?.id - newDocs = [ - doc: doc - path: path - docLines: docLines.join('\n') - ] - DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newDocs}, (error) -> - return callback(error) if error? - callback null, doc, folder_id + _addDocAndSendToTpds: (project_id, folder_id, doc, callback = (error, result, project) ->)-> + ProjectEntityMongoUpdateHandler.addDoc project_id, folder_id, doc, (err, result, project) -> + if err? + logger.err err:err, project_id: project_id, folder_id: folder_id, doc_name: doc?.name, doc_id:doc?._id, "error adding file with project" + return callback(err) + TpdsUpdateSender.addDoc { + project_id: project_id, + doc_id: doc?._id, + path: result?.path?.fileSystem, + project_name: project.name, + rev: 0 + }, (err) -> + return callback(err) if err? + callback(null, result, project) + + addDoc: wrapWithLock + beforeLock: (next) -> + (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->) -> + if not SafePath.isCleanFilename docName + return callback new Errors.InvalidNameError("invalid element name") + # Put doc in docstore first, so that if it errors, we don't have a doc_id in the project + # which hasn't been created in docstore. + doc = new Doc name: docName + DocstoreManager.updateDoc project_id.toString(), doc._id.toString(), docLines, 0, {}, (err, modified, rev) -> + return callback(err) if err? + next(project_id, folder_id, doc, docName, docLines, userId, callback) + withLock: (project_id, folder_id, doc, docName, docLines, userId, callback = (error, doc, folder_id) ->) -> + ProjectEntityUpdateHandler._addDocAndSendToTpds project_id, folder_id, doc, (err, result, project) -> + return callback(err) if err? + docPath = result?.path?.fileSystem + projectHistoryId = project.overleaf?.history?.id + newDocs = [ + doc: doc + path: docPath + docLines: docLines.join('\n') + ] + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newDocs}, (error) -> + return callback(error) if error? + callback null, doc, folder_id _uploadFile: (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback = (error, fileRef, fileStoreUrl) ->)-> if not SafePath.isCleanFilename fileName @@ -156,10 +181,10 @@ module.exports = ProjectEntityUpdateHandler = self = return callback(err) callback(null, fileRef, fileStoreUrl) - _addFileAndSendToTpds: (project_id, folder_id, fileName, fileRef, callback = (error) ->)-> + _addFileAndSendToTpds: (project_id, folder_id, fileRef, callback = (error) ->)-> ProjectEntityMongoUpdateHandler.addFile project_id, folder_id, fileRef, (err, result, project) -> if err? - logger.err err:err, project_id: project_id, folder_id: folder_id, file_name: fileName, fileRef:fileRef, "error adding file with project" + logger.err err:err, project_id: project_id, folder_id: folder_id, file_name: fileRef.name, fileRef:fileRef, "error adding file with project" return callback(err) TpdsUpdateSender.addFile {project_id:project_id, file_id:fileRef._id, path:result?.path?.fileSystem, project_name:project.name, rev:fileRef.rev}, (err) -> return callback(err) if err? @@ -174,7 +199,7 @@ module.exports = ProjectEntityUpdateHandler = self = return callback(error) if error? next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback) withLock: (project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback = (error, fileRef, folder_id) ->)-> - ProjectEntityUpdateHandler._addFileAndSendToTpds project_id, folder_id, fileName, fileRef, (err, result, project) -> + ProjectEntityUpdateHandler._addFileAndSendToTpds project_id, folder_id, fileRef, (err, result, project) -> return callback(err) if err? projectHistoryId = project.overleaf?.history?.id newFiles = [ @@ -214,47 +239,6 @@ module.exports = ProjectEntityUpdateHandler = self = return callback(err) if err? DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {oldFiles, newFiles}, callback - addDocWithoutUpdatingHistory: wrapWithLock (project_id, folder_id, docName, docLines, userId, callback = (error, doc, folder_id) ->)=> - # This method should never be called directly, except when importing a project - # from Overleaf. It skips sending updates to the project history, which will break - # the history unless you are making sure it is updated in some other way. - - if not SafePath.isCleanFilename docName - return callback new Errors.InvalidNameError("invalid element name") - - # Put doc in docstore first, so that if it errors, we don't have a doc_id in the project - # which hasn't been created in docstore. - doc = new Doc name: docName - DocstoreManager.updateDoc project_id.toString(), doc._id.toString(), docLines, 0, {}, (err, modified, rev) -> - return callback(err) if err? - ProjectEntityMongoUpdateHandler.addDoc project_id, folder_id, doc, (err, result, project) -> - return callback(err) if err? - TpdsUpdateSender.addDoc { - project_id: project_id, - doc_id: doc?._id - path: result?.path?.fileSystem, - project_name: project.name, - rev: 0 - }, (err) -> - return callback(err) if err? - callback(null, doc, folder_id, result?.path?.fileSystem, project) - - addFileWithoutUpdatingHistory: wrapWithLock - # This method should never be called directly, except when importing a project - # from Overleaf. It skips sending updates to the project history, which will break - # the history unless you are making sure it is updated in some other way. - beforeLock: (next) -> - (project_id, folder_id, fileName, fsPath, linkedFileData, userId, callback) -> - if not SafePath.isCleanFilename fileName - return callback(new Errors.InvalidNameError("invalid element name")) - ProjectEntityUpdateHandler._uploadFile project_id, folder_id, fileName, fsPath, linkedFileData, userId, (error, fileRef, fileStoreUrl) -> - return callback(error) if error? - next(project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback) - withLock: (project_id, folder_id, fileName, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback = (error, fileRef, folder_id, path, fileStoreUrl) ->)-> - ProjectEntityUpdateHandler._addFileAndSendToTpds project_id, folder_id, fileName, fileRef, (err, result, project) -> - return callback(err) if err? - callback(null, fileRef, folder_id, result?.path?.fileSystem, fileStoreUrl) - upsertDoc: wrapWithLock (project_id, folder_id, docName, docLines, source, userId, callback = (err, doc, folder_id, isNewDoc)->)-> if not SafePath.isCleanFilename docName return callback new Errors.InvalidNameError("invalid element name") diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index 5c31113552..9fe175c1ca 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -293,14 +293,15 @@ describe 'ProjectEntityUpdateHandler', -> beforeEach -> @path = "/path/to/doc" - @newDoc = _id: doc_id - @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory = - withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project) - @ProjectEntityUpdateHandler.addDoc project_id, folder_id, @docName, @docLines, userId, @callback + @newDoc = name:@docName, lines:undefined, _id: doc_id, rev: 0 + @DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5) + @TpdsUpdateSender.addDoc = sinon.stub().yields() + @ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project) + @ProjectEntityUpdateHandler.addDoc project_id, doc_id, @docName, @docLines, userId, @callback it "creates the doc without history", () -> - @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory.withoutLock - .calledWith(project_id, folder_id, @docName, @docLines, userId) + @DocstoreManager.updateDoc + .calledWith(project_id, doc_id, @docLines, 0, {}) .should.equal true it "sends the change in project structure to the doc updater", () -> @@ -318,8 +319,6 @@ describe 'ProjectEntityUpdateHandler', -> @path = "/path/to/doc" @newDoc = _id: doc_id - @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory = - withoutLock: sinon.stub().yields(null, @newDoc, folder_id, @path, @project) @ProjectEntityUpdateHandler.addDoc project_id, folder_id, "*" + @docName, @docLines, userId, @callback it 'returns an error', -> @@ -433,118 +432,6 @@ describe 'ProjectEntityUpdateHandler', -> .calledWith(project_id, projectHistoryId, userId, {oldFiles, newFiles}) .should.equal true - describe 'addDocWithoutUpdatingHistory', -> - describe 'adding a doc', -> - beforeEach -> - @path = "/path/to/doc" - - @project = _id: project_id, name: 'some project' - - @TpdsUpdateSender.addDoc = sinon.stub().yields() - @DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5) - @ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project) - @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory project_id, folder_id, @docName, @docLines, userId, @callback - - it "updates the doc in the docstore", () -> - @DocstoreManager.updateDoc - .calledWith(project_id, doc_id, @docLines, 0, {}) - .should.equal true - - it "updates the doc in mongo", () -> - docMatcher = sinon.match (doc) => - doc.name == @docName - - @ProjectEntityMongoUpdateHandler.addDoc - .calledWithMatch(project_id, folder_id, docMatcher) - .should.equal true - - it "notifies the tpds", () -> - @TpdsUpdateSender.addDoc - .calledWith({ - project_id: project_id - project_name: @project.name - doc_id: doc_id - rev: 0 - path: @path - }) - .should.equal true - - it "should not should send the change in project structure to the doc updater", () -> - @DocumentUpdaterHandler.updateProjectStructure - .called - .should.equal false - - describe 'adding a doc with an invalid name', -> - beforeEach -> - @path = "/path/to/doc" - - @project = _id: project_id, name: 'some project' - - @TpdsUpdateSender.addDoc = sinon.stub().yields() - @DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5) - @ProjectEntityMongoUpdateHandler.addDoc = sinon.stub().yields(null, {path: fileSystem: @path}, @project) - @ProjectEntityUpdateHandler.addDocWithoutUpdatingHistory project_id, folder_id, "*" + @docName, @docLines, userId, @callback - - it 'returns an error', -> - errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) - @callback.calledWithMatch(errorMatcher) - .should.equal true - - describe 'addFileWithoutUpdatingHistory', -> - describe 'adding a file', -> - beforeEach -> - @path = "/path/to/file" - - @project = _id: project_id, name: 'some project' - - @TpdsUpdateSender.addFile = sinon.stub().yields() - @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) - @ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory project_id, folder_id, @fileName, @fileSystemPath, @linkedFileData, userId, @callback - - it "updates the file in the filestore", () -> - @FileStoreHandler.uploadFileFromDisk - .calledWith(project_id, file_id, @fileSystemPath) - .should.equal true - - it "updates the file in mongo", () -> - fileMatcher = sinon.match (file) => - file.name == @fileName - - @ProjectEntityMongoUpdateHandler.addFile - .calledWithMatch(project_id, folder_id, fileMatcher) - .should.equal true - - it "notifies the tpds", () -> - @TpdsUpdateSender.addFile - .calledWith({ - project_id: project_id - project_name: @project.name - file_id: file_id - rev: 0 - path: @path - }) - .should.equal true - - it "should not should send the change in project structure to the doc updater", () -> - @DocumentUpdaterHandler.updateProjectStructure - .called - .should.equal false - - describe 'adding a file with an invalid name', -> - beforeEach -> - @path = "/path/to/file" - - @project = _id: project_id, name: 'some project' - - @TpdsUpdateSender.addFile = sinon.stub().yields() - @ProjectEntityMongoUpdateHandler.addFile = sinon.stub().yields(null, {path: fileSystem: @path}, @project) - @ProjectEntityUpdateHandler.addFileWithoutUpdatingHistory project_id, folder_id, "*" + @fileName, @fileSystemPath, @linkedFileData, userId, @callback - - it 'returns an error', -> - errorMatcher = sinon.match.instanceOf(Errors.InvalidNameError) - @callback.calledWithMatch(errorMatcher) - .should.equal true - describe 'upsertDoc', -> describe 'upserting into an invalid folder', -> beforeEach ->