From e781779c24c7d8d042df8d5aa677f7aeef24ba01 Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Fri, 12 Jan 2018 10:53:36 +0000 Subject: [PATCH] use LockManager.runWithLock in EditorController --- .../Features/Editor/EditorController.coffee | 110 ++++++++---------- .../Editor/EditorControllerTests.coffee | 100 +++++----------- 2 files changed, 77 insertions(+), 133 deletions(-) diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 0582d0bd87..fa94c8d546 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -20,13 +20,13 @@ module.exports = EditorController = addDoc: (project_id, folder_id, docName, docLines, source, user_id, callback = (error, doc)->)-> - LockManager.getLock project_id, (err)-> - if err? - logger.err err:err, project_id:project_id, source:source, "could not get lock to addDoc" - return callback(err) - EditorController.addDocWithoutLock project_id, folder_id, docName, docLines, source, user_id, (error, doc)-> - LockManager.releaseLock project_id, -> - callback(error, doc) + LockManager.runWithLock project_id, + (cb) -> EditorController.addDocWithoutLock project_id, folder_id, docName, docLines, source, user_id, cb + (err, doc) -> + if err? + logger.err err:err, project_id:project_id, source:source, "could add doc" + return callback err + callback null, doc addDocWithoutLock: (project_id, folder_id, docName, docLines, source, user_id, callback = (error, doc)->)-> docName = docName.trim() @@ -40,13 +40,13 @@ module.exports = EditorController = callback(err, doc) addFile: (project_id, folder_id, fileName, path, source, user_id, callback = (error, file)->)-> - LockManager.getLock project_id, (err)-> - if err? - logger.err err:err, project_id:project_id, source:source, "could not get lock to addFile" - return callback(err) - EditorController.addFileWithoutLock project_id, folder_id, fileName, path, source, user_id, (error, file)-> - LockManager.releaseLock project_id, -> - callback(error, file) + LockManager.runWithLock project_id, + (cb) -> EditorController.addFileWithoutLock project_id, folder_id, fileName, path, source, user_id, cb + (err, file) -> + if err? + logger.err err:err, project_id:project_id, source:source, "could add file" + return callback(err) + callback null, file addFileWithoutLock: (project_id, folder_id, fileName, path, source, user_id, callback = (error, file)->)-> fileName = fileName.trim() @@ -63,13 +63,13 @@ module.exports = EditorController = ProjectEntityHandler.replaceFile project_id, file_id, fsPath, user_id, callback addFolder : (project_id, folder_id, folderName, source, callback = (error, folder)->)-> - LockManager.getLock project_id, (err)-> - if err? - logger.err err:err, project_id:project_id, source:source, "could not get lock to addFolder" - return callback(err) - EditorController.addFolderWithoutLock project_id, folder_id, folderName, source, (error, folder)-> - LockManager.releaseLock project_id, -> - callback(error, folder) + LockManager.runWithLock project_id, + (cb) -> EditorController.addFolderWithoutLock project_id, folder_id, folderName, source, cb + (err, folder)-> + if err? + logger.err err:err, project_id:project_id, source:source, "could not add folder" + return callback(err) + callback null, folder addFolderWithoutLock: (project_id, folder_id, folderName, source, callback = (error, folder)->)-> folderName = folderName.trim() @@ -82,17 +82,16 @@ module.exports = EditorController = @p.notifyProjectUsersOfNewFolder project_id, folder_id, folder, (error) -> callback error, folder + mkdirp : (project_id, path, callback = (error, newFolders, lastFolder)->)-> + LockManager.runWithLock project_id, + (cb) -> EditorController.mkdirpWithoutLock project_id, path, cb + (err, newFolders, lastFolder) -> + if err? + logger.err err:err, project_id:project_id, "could not mkdirp" + return callback(err) + callback err, newFolders, lastFolder - mkdirp : (project_id, path, callback)-> - LockManager.getLock project_id, (err)-> - if err? - logger.err err:err, project_id:project_id, "could not get lock to mkdirp" - return callback(err) - EditorController.mkdirpWithoutLock project_id, path, (err, newFolders, lastFolder)-> - LockManager.releaseLock project_id, -> - callback(err, newFolders, lastFolder) - - mkdirpWithoutLock: (project_id, path, callback)-> + mkdirpWithoutLock: (project_id, path, callback = (error, newFolders, lastFolder)->)-> logger.log project_id:project_id, path:path, "making directories if they don't exist" ProjectEntityHandler.mkdirp project_id, path, (err, newFolders, lastFolder)=> if err? @@ -105,14 +104,13 @@ module.exports = EditorController = async.series jobs, (err)-> callback err, newFolders, lastFolder - deleteEntity : (project_id, entity_id, entityType, source, userId, callback)-> - LockManager.getLock project_id, (err)-> - if err? - logger.err err:err, project_id:project_id, "could not get lock to deleteEntity" - return callback(err) - EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, userId, (err)-> - LockManager.releaseLock project_id, ()-> - callback(err) + deleteEntity : (project_id, entity_id, entityType, source, userId, callback = (error)->)-> + LockManager.runWithLock project_id, + (cb) -> EditorController.deleteEntityWithoutLock project_id, entity_id, entityType, source, userId, cb + (err)-> + if err? + logger.err err:err, project_id:project_id, "could not delete entity" + callback(err) deleteEntityWithoutLock: (project_id, entity_id, entityType, source, userId, callback)-> logger.log {project_id, entity_id, entityType, source}, "start delete process of entity" @@ -144,36 +142,30 @@ module.exports = EditorController = logger.log project_id:project_id, "recived message to delete project" ProjectDeleter.deleteProject project_id, callback - renameEntity: (project_id, entity_id, entityType, newName, userId, callback)-> + renameEntity: (project_id, entity_id, entityType, newName, userId, callback = (error) ->)-> newName = sanitize.escape(newName) Metrics.inc "editor.rename-entity" logger.log entity_id:entity_id, entity_id:entity_id, entity_id:entity_id, "reciving new name for entity for project" - LockManager.getLock project_id, (err)-> - if err? - logger.err err:err, project_id:project_id, "could not get lock for rename entity" - return callback(err) - ProjectEntityHandler.renameEntity project_id, entity_id, entityType, newName, userId, (err) -> + LockManager.runWithLock project_id, + (cb) -> ProjectEntityHandler.renameEntity project_id, entity_id, entityType, newName, userId, cb + (err) -> if err? logger.err err:err, project_id:project_id, entity_id:entity_id, entityType:entityType, newName:newName, "error renaming entity" return callback(err) - LockManager.releaseLock project_id, -> - if newName.length > 0 - EditorRealTimeController.emitToRoom project_id, 'reciveEntityRename', entity_id, newName - callback?() + if newName.length > 0 + EditorRealTimeController.emitToRoom project_id, 'reciveEntityRename', entity_id, newName + callback() - moveEntity: (project_id, entity_id, folder_id, entityType, userId, callback)-> + moveEntity: (project_id, entity_id, folder_id, entityType, userId, callback = (error) ->)-> Metrics.inc "editor.move-entity" - LockManager.getLock project_id, (err)-> - if err? - logger.err err:err, project_id:project_id, "could not get lock for move entity" - return callback(err) - ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, userId, => + LockManager.runWithLock project_id, + (cb) -> ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, userId, cb + (err) -> 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?() + EditorRealTimeController.emitToRoom project_id, 'reciveEntityMove', entity_id, folder_id + callback() renameProject: (project_id, newName, callback = (err) ->) -> ProjectDetailsHandler.renameProject project_id, newName, (err) -> @@ -223,10 +215,8 @@ module.exports = EditorController = EditorRealTimeController.emitToRoom project_id, 'rootDocUpdated', newRootDocID callback() - p: notifyProjectUsersOfNewFolder: (project_id, folder_id, folder, callback = (error)->)-> logger.log project_id:project_id, folder:folder, parentFolder_id:folder_id, "sending newly created folder out to users" EditorRealTimeController.emitToRoom(project_id, "reciveNewFolder", folder_id, folder) callback() - diff --git a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee index 9dedc07275..ace7fe4d19 100644 --- a/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/unit/coffee/Editor/EditorControllerTests.coffee @@ -50,8 +50,7 @@ describe "EditorController", -> @ProjectDeleter = deleteProject: sinon.stub() @LockManager = - getLock : sinon.stub() - releaseLock : sinon.stub() + runWithLock : sinon.spy((key, runner, callback) -> runner(callback)) @EditorController = SandboxedModule.require modulePath, requires: "../../infrastructure/Server" : io : @io '../Project/ProjectEntityHandler' : @ProjectEntityHandler @@ -155,8 +154,6 @@ describe "EditorController", -> describe "addDoc", -> beforeEach -> - @LockManager.getLock.callsArgWith(1) - @LockManager.releaseLock.callsArgWith(1) @EditorController.addDocWithoutLock = sinon.stub().callsArgWith(6) it "should call addDocWithoutLock", (done)-> @@ -166,17 +163,12 @@ describe "EditorController", -> it "should take the lock", (done)-> @EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, @user_id, => - @LockManager.getLock.calledWith(@project_id).should.equal true + @LockManager.runWithLock.calledWith(@project_id).should.equal true done() - it "should release the lock", (done)-> - @EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, @user_id, => - @LockManager.releaseLock.calledWith(@project_id).should.equal true - done() - - it "should error if it can't cat the lock", (done)-> - @LockManager.getLock = sinon.stub().callsArgWith(1, "timed out") - @EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, @user_id, (err)=> + it "should propogate up any errors", (done)-> + @LockManager.runWithLock = sinon.stub().callsArgWith(2, "timed out") + @EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, @user_id, (err) => expect(err).to.exist err.should.equal "timed out" done() @@ -217,8 +209,6 @@ describe "EditorController", -> describe "addFile", -> beforeEach -> - @LockManager.getLock.callsArgWith(1) - @LockManager.releaseLock.callsArgWith(1) @EditorController.addFileWithoutLock = sinon.stub().callsArgWith(6) it "should call addFileWithoutLock", (done)-> @@ -228,20 +218,15 @@ describe "EditorController", -> it "should take the lock", (done)-> @EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, @user_id, (error, file) => - @LockManager.getLock.calledWith(@project_id).should.equal true + @LockManager.runWithLock.calledWith(@project_id).should.equal true done() - it "should release the lock", (done)-> - @EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, @user_id, (error, file) => - @LockManager.releaseLock.calledWith(@project_id).should.equal true - done() - - it "should error if it can't cat the lock", (done)-> - @LockManager.getLock = sinon.stub().callsArgWith(1, "timed out") + it "should propogate up any errors", (done)-> + @LockManager.runWithLock = sinon.stub().callsArgWith(2, "timed out") @EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, @user_id, (error, file) => expect(error).to.exist error.should.equal "timed out" - done() + done() describe "replaceFileWithoutLock", -> beforeEach -> @@ -296,10 +281,7 @@ describe "EditorController", -> describe "addFolder", -> - beforeEach -> - @LockManager.getLock.callsArgWith(1) - @LockManager.releaseLock.callsArgWith(1) @EditorController.addFolderWithoutLock = sinon.stub().callsArgWith(4) it "should call addFolderWithoutLock", (done)-> @@ -309,20 +291,15 @@ describe "EditorController", -> it "should take the lock", (done)-> @EditorController.addFolder @project_id, @folder_id, @folderName, @source, (error, file) => - @LockManager.getLock.calledWith(@project_id).should.equal true + @LockManager.runWithLock.calledWith(@project_id).should.equal true done() - it "should release the lock", (done)-> - @EditorController.addFolder @project_id, @folder_id, @folderName, @source, (error, file) => - @LockManager.releaseLock.calledWith(@project_id).should.equal true - done() - - it "should error if it can't cat the lock", (done)-> - @LockManager.getLock = sinon.stub().callsArgWith(1, "timed out") + it "should propogate up any errors", (done)-> + @LockManager.runWithLock = sinon.stub().callsArgWith(2, "timed out") @EditorController.addFolder @project_id, @folder_id, @folderName, @source, (err, file) => expect(err).to.exist err.should.equal "timed out" - done() + done() describe 'mkdirpWithoutLock :', -> @@ -346,11 +323,8 @@ describe "EditorController", -> describe "mkdirp", -> - beforeEach -> @path = "folder1/folder2" - @LockManager.getLock.callsArgWith(1) - @LockManager.releaseLock.callsArgWith(1) @EditorController.mkdirpWithoutLock = sinon.stub().callsArgWith(2) it "should call mkdirpWithoutLock", (done)-> @@ -360,25 +334,18 @@ describe "EditorController", -> it "should take the lock", (done)-> @EditorController.mkdirp @project_id, @path, (error, file) => - @LockManager.getLock.calledWith(@project_id).should.equal true + @LockManager.runWithLock.calledWith(@project_id).should.equal true done() - it "should release the lock", (done)-> - @EditorController.mkdirp @project_id, @path, (error, file) => - @LockManager.releaseLock.calledWith(@project_id).should.equal true - done() - - it "should error if it can't cat the lock", (done)-> - @LockManager.getLock = sinon.stub().callsArgWith(1, "timed out") + it "should propogate up any errors", (done)-> + @LockManager.runWithLock = sinon.stub().callsArgWith(2, "timed out") @EditorController.mkdirp @project_id, @path, (err, file) => expect(err).to.exist err.should.equal "timed out" - done() + done() describe "deleteEntity", -> beforeEach -> - @LockManager.getLock.callsArgWith(1) - @LockManager.releaseLock.callsArgWith(1) @EditorController.deleteEntityWithoutLock = sinon.stub().callsArgWith(5) it "should call deleteEntityWithoutLock", (done)-> @@ -390,16 +357,11 @@ describe "EditorController", -> it "should take the lock", (done)-> @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, => - @LockManager.getLock.calledWith(@project_id).should.equal true + @LockManager.runWithLock.calledWith(@project_id).should.equal true done() - it "should release the lock", (done)-> - @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, (error) => - @LockManager.releaseLock.calledWith(@project_id).should.equal true - done() - - it "should error if it can't cat the lock", (done)-> - @LockManager.getLock = sinon.stub().callsArgWith(1, "timed out") + it "should propogate up any errors", (done)-> + @LockManager.runWithLock = sinon.stub().callsArgWith(2, "timed out") @EditorController.deleteEntity @project_id, @entity_id, @type, @source, @user_id, (error) => expect(error).to.exist error.should.equal "timed out" @@ -472,9 +434,6 @@ describe "EditorController", -> @ProjectEntityHandler.renameEntity = sinon.stub().callsArg(5) @EditorRealTimeController.emitToRoom = sinon.stub() - @LockManager.releaseLock.callsArgWith(1) - @LockManager.getLock.callsArgWith(1) - @EditorController.renameEntity @project_id, @entity_id, @entityType, @newName, @user_id, done it "should call the project handler", -> @@ -483,12 +442,7 @@ describe "EditorController", -> .should.equal true it "should take the lock", -> - @LockManager.getLock - .calledWith(@project_id) - .should.equal true - - it "should release the lock", -> - @LockManager.releaseLock + @LockManager.runWithLock .calledWith(@project_id) .should.equal true @@ -504,8 +458,6 @@ describe "EditorController", -> @folder_id = "313dasd21dasdsa" @ProjectEntityHandler.moveEntity = sinon.stub().callsArg(5) @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, @user_id, => @@ -514,12 +466,14 @@ describe "EditorController", -> it "should take the lock", (done)-> @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => - @LockManager.getLock.calledWith(@project_id).should.equal true + @LockManager.runWithLock.calledWith(@project_id).should.equal true done() - it "should release the lock", (done)-> - @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, => - @LockManager.releaseLock.calledWith(@project_id).should.equal true + it "should propogate up any errors", (done)-> + @LockManager.runWithLock = sinon.stub().callsArgWith(2, "timed out") + @EditorController.moveEntity @project_id, @entity_id, @folder_id, @entityType, @user_id, (error) => + expect(error).to.exist + error.should.equal "timed out" done() it "should emit the update to the room", (done)->