From ba9bc3a2e8ca47274105c2323b1d5b196ee78a7e Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 15 Mar 2016 11:29:59 +0000 Subject: [PATCH 1/3] check that element being inserted has an _id --- .../coffee/Features/Project/ProjectEntityHandler.coffee | 2 +- .../coffee/Project/ProjectEntityHandlerTests.coffee | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 9b771625b3..7d3f7df6e2 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -501,7 +501,7 @@ module.exports = ProjectEntityHandler = elementType = "fileRefs" return elementType - if !element? + if !element? or !element._id? e = new Error("no element passed to be inserted") logger.err project_id:project._id, folder_id:folder_id, element:element, type:type, "failed trying to insert element as it was null" return callback(e) diff --git a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee index a44a16c10b..0e2441cf8d 100644 --- a/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee @@ -1048,6 +1048,13 @@ describe 'ProjectEntityHandler', -> @projectLocator.findElement.args[0][0].element_id.should.equal @project.rootFolder[0]._id done() + it "should error if the element has no _id", (done)-> + doc = + name:"something" + @ProjectEntityHandler._putElement @project, @folder._id, doc, "doc", (err)=> + @ProjectModel.update.called.should.equal false + done() + describe "_countElements", -> From 76b3a78988eb7b03403f03909e407e4e36bec4b0 Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 15 Mar 2016 12:29:41 +0000 Subject: [PATCH 2/3] added lock around move element --- .../coffee/Features/Editor/EditorController.coffee | 13 +++++++++---- .../Features/Project/ProjectEntityHandler.coffee | 2 +- .../coffee/Editor/EditorControllerTests.coffee | 11 +++++++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 38f788dd41..476ba96174 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -164,12 +164,17 @@ module.exports = EditorController = moveEntity: (project_id, entity_id, folder_id, entityType, callback)-> Metrics.inc "editor.move-entity" - ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, => + LockManager.getLock project_id, (err)-> if err? - logger.err err:err, project_id:project_id, entity_id:entity_id, folder_id:folder_id, "error moving entity" + logger.err err:err, project_id:project_id, "could not get lock for move entity" return callback(err) - EditorRealTimeController.emitToRoom project_id, 'reciveEntityMove', entity_id, folder_id - callback?() + ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, => + if err? + logger.err err:err, project_id:project_id, entity_id:entity_id, folder_id:folder_id, "error moving entity" + return callback(err) + LockManager.releaseLock project_id, -> + EditorRealTimeController.emitToRoom project_id, 'reciveEntityMove', entity_id, folder_id + callback?() renameProject: (project_id, newName, callback = (err) ->) -> ProjectDetailsHandler.renameProject project_id, newName, -> diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 7d3f7df6e2..8544ed7884 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -520,13 +520,13 @@ module.exports = ProjectEntityHandler = newPath = fileSystem: "#{path.fileSystem}/#{element.name}" mongo: path.mongo - logger.log project_id: project._id, element_id: element._id, fileType: type, folder_id: folder_id, "adding element to project" id = element._id+'' element._id = require('mongoose').Types.ObjectId(id) conditions = _id:project._id mongopath = "#{path.mongo}.#{type}" update = "$push":{} update["$push"][mongopath] = element + logger.log project_id: project._id, element_id: element._id, fileType: type, folder_id: folder_id, mongopath:mongopath, "adding element to project" Project.update conditions, update, {}, (err)-> if err? logger.err err: err, project_id: project._id, 'error saving in putElement project' diff --git a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee index a60830b226..fd56fbb021 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee @@ -518,12 +518,23 @@ describe "EditorController", -> @folder_id = "313dasd21dasdsa" @ProjectEntityHandler.moveEntity = sinon.stub().callsArgWith(4, @err) @EditorRealTimeController.emitToRoom = sinon.stub() + @LockManager.releaseLock.callsArgWith(1) + @LockManager.getLock.callsArgWith(1) it "should call the ProjectEntityHandler", (done)-> @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => @ProjectEntityHandler.moveEntity.calledWith(@project_id, @entity_id, @folder_id, @entityType).should.equal true done() + it "should take the lock", (done)-> + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => + @LockManager.getLock.calledWith(@project_id).should.equal true + done() + + it "should release the lock", (done)-> + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => + @LockManager.releaseLock.calledWith(@project_id).should.equal true + done() it "should emit the update to the room", (done)-> @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, => From 60a39f82c1782d5a3518502524e864514c3ebf52 Mon Sep 17 00:00:00 2001 From: James Allen Date: Tue, 15 Mar 2016 15:14:54 +0000 Subject: [PATCH 3/3] Fix off by one bug in moving folders from using an out of date project structure --- .../Project/ProjectEntityHandler.coffee | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee index 8544ed7884..fff91d2ce3 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee @@ -344,15 +344,18 @@ module.exports = ProjectEntityHandler = return callback(error) if error? self._removeElementFromMongoArray Project, project_id, path.mongo, (err)-> return callback(err) if err? - ProjectEntityHandler._putElement project, destinationFolder_id, entity, entityType, (err, result)-> + # We've updated the project structure by removing the element, so must refresh it. + ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project)=> return callback(err) if err? - opts = - project_id:project_id - project_name:project.name - startPath:path.fileSystem - endPath:result.path.fileSystem, - rev:entity.rev - tpdsUpdateSender.moveEntity opts, callback + ProjectEntityHandler._putElement project, destinationFolder_id, entity, entityType, (err, result)-> + return callback(err) if err? + opts = + project_id:project_id + project_name:project.name + startPath:path.fileSystem + endPath:result.path.fileSystem, + rev:entity.rev + tpdsUpdateSender.moveEntity opts, callback deleteEntity: (project_id, entity_id, entityType, callback = (error) ->)-> self = @