diff --git a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee index 71e99cc67c..9fd838f9de 100644 --- a/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee +++ b/services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee @@ -138,9 +138,21 @@ module.exports = DocumentUpdaterHandler = docUpdates = DocumentUpdaterHandler._getUpdates('doc', changes.oldDocs, changes.newDocs) fileUpdates = DocumentUpdaterHandler._getUpdates('file', changes.oldFiles, changes.newFiles) + projectVersion = changes?.newProject?.version return callback() if (docUpdates.length + fileUpdates.length) < 1 + # FIXME: remove this check and the request to get the project structure version above + # when we are confident in the use of $inc to increment the project structure version + # in all cases. + if projectVersion? && currentProject.version == projectVersion + logger.log {project_id, projectVersion}, "got project version in changes" + else if projectVersion? && currentProject.version != projectVersion + logger.error {project_id, changes, projectVersion, currentProject: currentProject.version}, "project version from db was different from changes (broken lock?)" + else + projectVersion = currentProject.version + logger.warn {project_id, changes, projectVersion}, "did not receive project version in changes" + logger.log {project_id}, "updating project structure in doc updater" DocumentUpdaterHandler._makeRequest { path: "/project/#{project_id}" @@ -148,7 +160,7 @@ module.exports = DocumentUpdaterHandler = docUpdates, fileUpdates, userId, - version: currentProject.version + version: projectVersion projectHistoryId } method: "POST" diff --git a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee index a13eb370b1..106618d72f 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee @@ -67,9 +67,15 @@ module.exports = ProjectEntityMongoUpdateHandler = self = update = "$inc": inc "$set": set - Project.update conditions, update, {}, (err) -> + # Note: Mongoose uses new:true to return the modified document + # https://mongoosejs.com/docs/api.html#model_Model.findOneAndUpdate + # but Mongo uses returnNewDocument:true instead + # https://docs.mongodb.com/manual/reference/method/db.collection.findOneAndUpdate/ + # We are using Mongoose here, but if we ever switch to a direct mongo call + # the next line will need to be updated. + Project.findOneAndUpdate conditions, update, {new:true}, (err, newProject) -> return callback(err) if err? - callback null, fileRef, project, path + callback null, fileRef, project, path, newProject mkdirp: wrapWithLock (project_id, path, options, callback) -> # defaults to case insensitive paths, use options {exactCaseMatch:true} @@ -128,7 +134,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = return callback(err) if err? startPath = entityPath.fileSystem endPath = result.path.fileSystem - changes = {oldDocs, newDocs, oldFiles, newFiles} + changes = {oldDocs, newDocs, oldFiles, newFiles, newProject} callback null, project, startPath, endPath, entity.rev, changes, callback deleteEntity: wrapWithLock (project_id, entity_id, entityType, callback) -> @@ -136,9 +142,9 @@ module.exports = ProjectEntityMongoUpdateHandler = self = return callback(error) if error? ProjectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path) -> return callback(error) if error? - self._removeElementFromMongoArray Project, project_id, path.mongo, (error) -> + self._removeElementFromMongoArray Project, project_id, path.mongo, (error, newProject) -> return callback(error) if error? - callback null, entity, path, project + callback null, entity, path, project, newProject renameEntity: wrapWithLock (project_id, entity_id, entityType, newName, callback) -> ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true, overleaf:true}, (error, project)=> @@ -162,7 +168,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = ProjectEntityHandler.getAllEntitiesFromProject newProject, (error, newDocs, newFiles) => return callback(error) if error? startPath = entPath.fileSystem - changes = {oldDocs, newDocs, oldFiles, newFiles} + changes = {oldDocs, newDocs, oldFiles, newFiles, newProject} callback null, project, startPath, endPath, entity.rev, changes, callback addFolder: wrapWithLock (project_id, parentFolder_id, folderName, callback) -> @@ -262,6 +268,8 @@ module.exports = ProjectEntityMongoUpdateHandler = self = # we need to increment the project version number for any structure change update["$inc"]["version"] = 1 # increment project version number logger.log project_id: project._id, element_id: element._id, fileType: type, folder_id: folder_id, mongopath:mongopath, "adding element to project" + # We are using Mongoose here, but if we ever switch to a direct mongo call + # the next line will need to be updated to {returnNewDocument:true} Project.findOneAndUpdate conditions, update, {"new": true}, (err, newProject)-> if err? logger.err err: err, project_id: project._id, 'error saving in putElement project' diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index 8e269fe610..f2b5df32ac 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -162,7 +162,7 @@ module.exports = ProjectEntityUpdateHandler = self = path: docPath docLines: docLines.join('\n') ] - DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newDocs}, (error) -> + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newDocs, newProject: project}, (error) -> return callback(error) if error? callback null, doc, folder_id @@ -205,7 +205,7 @@ module.exports = ProjectEntityUpdateHandler = self = path: result?.path?.fileSystem url: fileStoreUrl ] - DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newFiles}, (error) -> + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {newFiles, newProject: project}, (error) -> return callback(error) if error? callback(null, fileRef, folder_id) @@ -221,7 +221,7 @@ module.exports = ProjectEntityUpdateHandler = self = return callback(err) if err? next project_id, file_id, fsPath, linkedFileData, userId, fileRef, fileStoreUrl, callback withLock: (project_id, file_id, fsPath, linkedFileData, userId, newFileRef, fileStoreUrl, callback)-> - ProjectEntityMongoUpdateHandler.replaceFileWithNew project_id, file_id, newFileRef, (err, oldFileRef, project, path) -> + ProjectEntityMongoUpdateHandler.replaceFileWithNew project_id, file_id, newFileRef, (err, oldFileRef, project, path, newProject) -> return callback(err) if err? oldFiles = [ file: oldFileRef @@ -241,7 +241,7 @@ module.exports = ProjectEntityUpdateHandler = self = # but it is acceptable for now. TpdsUpdateSender.addFile {project_id:project._id, file_id:newFileRef._id, path:path.fileSystem, rev:oldFileRef.rev + 1, project_name:project.name}, (err) -> return callback(err) if err? - DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {oldFiles, newFiles}, callback + DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, {oldFiles, newFiles, newProject}, callback upsertDoc: wrapWithLock (project_id, folder_id, docName, docLines, source, userId, callback = (err, doc, folder_id, isNewDoc)->)-> if not SafePath.isCleanFilename docName @@ -338,9 +338,9 @@ module.exports = ProjectEntityUpdateHandler = self = logger.err err: "No entityType set", project_id: project_id, entity_id: entity_id return callback("No entityType set") entityType = entityType.toLowerCase() - ProjectEntityMongoUpdateHandler.deleteEntity project_id, entity_id, entityType, (error, entity, path, projectBeforeDeletion) -> + ProjectEntityMongoUpdateHandler.deleteEntity project_id, entity_id, entityType, (error, entity, path, projectBeforeDeletion, newProject) -> return callback(error) if error? - self._cleanUpEntity projectBeforeDeletion, entity, entityType, path.fileSystem, userId, (error) -> + self._cleanUpEntity projectBeforeDeletion, newProject, entity, entityType, path.fileSystem, userId, (error) -> return callback(error) if error? TpdsUpdateSender.deleteEntity project_id:project_id, path:path.fileSystem, project_name:projectBeforeDeletion.name, (error) -> return callback(error) if error? @@ -421,8 +421,8 @@ module.exports = ProjectEntityUpdateHandler = self = DocumentUpdaterHandler.resyncProjectHistory project_id, projectHistoryId, docs, files, callback - _cleanUpEntity: (project, entity, entityType, path, userId, callback = (error) ->) -> - self._updateProjectStructureWithDeletedEntity project, entity, entityType, path, userId, (error) -> + _cleanUpEntity: (project, newProject, entity, entityType, path, userId, callback = (error) ->) -> + self._updateProjectStructureWithDeletedEntity project, newProject, entity, entityType, path, userId, (error) -> return callback(error) if error? if(entityType.indexOf("file") != -1) self._cleanUpFile project, entity, path, userId, callback @@ -437,7 +437,7 @@ module.exports = ProjectEntityUpdateHandler = self = # methods both need to recursively iterate over the entities in folder. # These are currently using separate implementations of the recursion. In # future, these could be simplified using a common project entity iterator. - _updateProjectStructureWithDeletedEntity: (project, entity, entityType, entityPath, userId, callback = (error) ->) -> + _updateProjectStructureWithDeletedEntity: (project, newProject, entity, entityType, entityPath, userId, callback = (error) ->) -> # compute the changes to the project structure if(entityType.indexOf("file") != -1) changes = oldFiles: [ {file: entity, path: entityPath} ] @@ -454,6 +454,7 @@ module.exports = ProjectEntityUpdateHandler = self = _recurseFolder(childFolder, path.join(folderPath, childFolder.name)) _recurseFolder entity, entityPath # now send the project structure changes to the docupdater + changes.newProject = newProject project_id = project._id.toString() projectHistoryId = project.overleaf?.history?.id DocumentUpdaterHandler.updateProjectStructure project_id, projectHistoryId, userId, changes, callback diff --git a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee index 837a0f5b78..7ed705d2ea 100644 --- a/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee +++ b/services/web/test/unit/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee @@ -33,7 +33,7 @@ describe 'DocumentUpdaterHandler', -> @handler = SandboxedModule.require modulePath, requires: 'request': defaults:=> return @request 'settings-sharelatex':@settings - 'logger-sharelatex':{log:(->), error:(->)} + 'logger-sharelatex':{log:(->), error:(->), warn:(->)} '../Project/ProjectEntityHandler':@projectEntityHandler "../../models/Project": Project: @Project={} '../../Features/Project/ProjectLocator':{} diff --git a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee index 661bef2da0..7ea2dee454 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee @@ -99,7 +99,9 @@ describe 'ProjectEntityMongoUpdateHandler', -> @path = mongo: 'file.png' @newFile = _id: 'new-file-id' @newFile.linkedFileData = @linkedFileData = {provider: 'url'} + @newProject = "new-project" @ProjectLocator.findElement = sinon.stub().yields(null, @file, @path) + @ProjectModel.findOneAndUpdate = sinon.stub().yields(null, @newProject) @ProjectModel.update = sinon.stub().yields() @subject.replaceFileWithNew project_id, file_id, @newFile, @callback @@ -131,19 +133,19 @@ describe 'ProjectEntityMongoUpdateHandler', -> .should.equal true it 'increments the project version and sets the rev and created_at', -> - @ProjectModel.update + @ProjectModel.findOneAndUpdate .calledWith( { _id: project_id }, { '$inc': { 'version': 1, 'file.png.rev': 1 } '$set': { 'file.png._id': @newFile._id, 'file.png.created': new Date(), 'file.png.linkedFileData': @linkedFileData } - } - {} + }, + {new: true} ) .should.equal true it 'calls the callback', -> - @callback.calledWith(null, @file, @project, @path).should.equal true + @callback.calledWith(null, @file, @project, @path, @newProject).should.equal true describe 'mkdirp', -> beforeEach -> @@ -243,6 +245,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @doc = {lines:["1234","312343d"], rev: "1234"} @path = { mongo:"folders[0]", fileSystem:"/old_folder/somewhere.txt" } + @newProject = "new-project" @ProjectLocator.findElement = sinon.stub() .withArgs({@project, element_id: @docId, type: 'docs'}) .yields(null, @doc, @path) @@ -250,7 +253,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @subject._checkValidMove = sinon.stub().yields() @subject._removeElementFromMongoArray = sinon.stub().yields(null, @project) - @subject._putElement = sinon.stub().yields(null, path: @pathAfterMove) + @subject._putElement = sinon.stub().yields(null, path: @pathAfterMove, @newProject) @subject.moveEntity project_id, doc_id, folder_id, "docs", @callback @@ -280,7 +283,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> .should.equal true it "calls the callback", -> - changes = { @oldDocs, @newDocs, @oldFiles, @newFiles } + changes = { @oldDocs, @newDocs, @oldFiles, @newFiles, @newProject } @callback.calledWith( null, @project, @path.fileSystem, @pathAfterMove.fileSystem, @doc.rev, changes ).should.equal true @@ -321,6 +324,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> rootFolder: [_id:ObjectId()] @doc = _id: doc_id, name: "old.tex", rev: 1 @folder = _id: folder_id + @newProject = "new-project" @ProjectGetter.getProjectWithoutLock = sinon.stub().yields(null, @project) @@ -334,7 +338,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @ProjectLocator.findElement = sinon.stub().yields(null, @doc, @path, @folder) @subject._checkValidElementName = sinon.stub().yields() - @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @project) + @ProjectModel.findOneAndUpdate = sinon.stub().callsArgWith(3, null, @newProject) @subject.renameEntity project_id, doc_id, 'doc', @newName, @callback @@ -362,7 +366,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> ).should.equal true it 'calls the callback', -> - changes = { @oldDocs, @newDocs, @oldFiles, @newFiles } + changes = { @oldDocs, @newDocs, @oldFiles, @newFiles, @newProject } @callback.calledWith( null, @project, '/old.tex', '/new.tex', @doc.rev, changes ).should.equal true diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index 46ac50ab63..29e5d3593a 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -311,7 +311,7 @@ describe 'ProjectEntityUpdateHandler', -> docLines: @docLines.join('\n') ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, {newDocs}) + .calledWith(project_id, projectHistoryId, userId, {newDocs, newProject: @project}) .should.equal true describe 'adding a doc with an invalid name', -> @@ -367,7 +367,7 @@ describe 'ProjectEntityUpdateHandler', -> url: @fileUrl ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, {newFiles}) + .calledWith(project_id, projectHistoryId, userId, {newFiles, newProject: @project}) .should.equal true describe 'adding a file with an invalid name', -> @@ -393,8 +393,9 @@ describe 'ProjectEntityUpdateHandler', -> @newFile = _id: new_file_id, name: "dummy-upload-filename", rev: 0, linkedFileData: @linkedFileData @oldFile = _id: file_id, rev: 3 @path = "/path/to/file" + @newProject = "new project" @ProjectEntityMongoUpdateHandler._insertDeletedFileReference = sinon.stub().yields() - @ProjectEntityMongoUpdateHandler.replaceFileWithNew = sinon.stub().yields(null, @oldFile, @project, fileSystem: @path) + @ProjectEntityMongoUpdateHandler.replaceFileWithNew = sinon.stub().yields(null, @oldFile, @project, fileSystem: @path, @newProject) @ProjectEntityUpdateHandler.replaceFile project_id, file_id, @fileSystemPath, @linkedFileData, userId, @callback it 'uploads a new version of the file', -> @@ -429,7 +430,7 @@ describe 'ProjectEntityUpdateHandler', -> url: @newFileUrl ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, {oldFiles, newFiles}) + .calledWith(project_id, projectHistoryId, userId, {oldFiles, newFiles, newProject: @newProject}) .should.equal true describe 'upsertDoc', -> @@ -712,7 +713,8 @@ describe 'ProjectEntityUpdateHandler', -> @path = '/path/to/doc.tex' @doc = _id: doc_id @projectBeforeDeletion = _id: project_id, name: 'project' - @ProjectEntityMongoUpdateHandler.deleteEntity = sinon.stub().yields(null, @doc, {fileSystem: @path}, @projectBeforeDeletion) + @newProject = "new-project" + @ProjectEntityMongoUpdateHandler.deleteEntity = sinon.stub().yields(null, @doc, {fileSystem: @path}, @projectBeforeDeletion, @newProject) @ProjectEntityUpdateHandler._cleanUpEntity = sinon.stub().yields() @TpdsUpdateSender.deleteEntity = sinon.stub().yields() @@ -725,7 +727,7 @@ describe 'ProjectEntityUpdateHandler', -> it 'cleans up the doc in the docstore', -> @ProjectEntityUpdateHandler._cleanUpEntity - .calledWith(@projectBeforeDeletion, @doc, 'doc', @path, userId) + .calledWith(@projectBeforeDeletion, @newProject, @doc, 'doc', @path, userId) .should.equal true it 'it notifies the tpds', -> @@ -974,7 +976,8 @@ describe 'ProjectEntityUpdateHandler', -> beforeEach (done) -> @path = "/file/system/path.png" @entity = _id: @entity_id - @ProjectEntityUpdateHandler._cleanUpEntity @project, @entity, 'file', @path, userId, done + @newProject = "new-project" + @ProjectEntityUpdateHandler._cleanUpEntity @project, @newProject, @entity, 'file', @path, userId, done it "should insert the file into the deletedFiles array", -> @ProjectEntityMongoUpdateHandler._insertDeletedFileReference @@ -990,7 +993,7 @@ describe 'ProjectEntityUpdateHandler', -> it "should should send the update to the doc updater", -> oldFiles = [ file: @entity, path: @path ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, {oldFiles}) + .calledWith(project_id, projectHistoryId, userId, {oldFiles, newProject: @newProject}) .should.equal true describe "a doc", -> @@ -998,7 +1001,8 @@ describe 'ProjectEntityUpdateHandler', -> @path = "/file/system/path.tex" @ProjectEntityUpdateHandler._cleanUpDoc = sinon.stub().yields() @entity = {_id: @entity_id} - @ProjectEntityUpdateHandler._cleanUpEntity @project, @entity, 'doc', @path, userId, done + @newProject = "new-project" + @ProjectEntityUpdateHandler._cleanUpEntity @project, @newProject, @entity, 'doc', @path, userId, done it "should clean up the doc", -> @ProjectEntityUpdateHandler._cleanUpDoc @@ -1008,7 +1012,7 @@ describe 'ProjectEntityUpdateHandler', -> it "should should send the update to the doc updater", -> oldDocs = [ doc: @entity, path: @path ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, {oldDocs}) + .calledWith(project_id, projectHistoryId, userId, {oldDocs, newProject: @newProject}) .should.equal true describe "a folder", -> @@ -1026,7 +1030,8 @@ describe 'ProjectEntityUpdateHandler', -> @ProjectEntityUpdateHandler._cleanUpDoc = sinon.stub().yields() @ProjectEntityUpdateHandler._cleanUpFile = sinon.stub().yields() path = "/folder" - @ProjectEntityUpdateHandler._cleanUpEntity @project, @folder, "folder", path, userId, done + @newProject = "new-project" + @ProjectEntityUpdateHandler._cleanUpEntity @project, @newProject, @folder, "folder", path, userId, done it "should clean up all sub files", -> @ProjectEntityUpdateHandler._cleanUpFile @@ -1048,7 +1053,7 @@ describe 'ProjectEntityUpdateHandler', -> oldFiles = [ {file: @file2, path: "/folder/file-name-2"}, {file: @file1, path: "/folder/subfolder/file-name-1"} ] oldDocs = [ {doc: @doc2, path: "/folder/doc-name-2"}, { doc: @doc1, path: "/folder/subfolder/doc-name-1"} ] @DocumentUpdaterHandler.updateProjectStructure - .calledWith(project_id, projectHistoryId, userId, {oldFiles, oldDocs}) + .calledWith(project_id, projectHistoryId, userId, {oldFiles, oldDocs, newProject: @newProject}) .should.equal true describe "_cleanUpDoc", ->