Merge pull request #237 from sharelatex/hof-editor-controller-locking

Project Locking: EditorController
This commit is contained in:
James Allen 2018-01-15 14:21:03 +00:00 committed by GitHub
commit 67061154fe
6 changed files with 110 additions and 147 deletions

View file

@ -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()
@ -59,17 +59,17 @@ module.exports = EditorController =
EditorRealTimeController.emitToRoom(project_id, 'reciveNewFile', folder_id, fileRef, source)
callback(err, fileRef)
replaceFile: (project_id, file_id, fsPath, source, user_id, callback = (error) ->)->
replaceFileWithoutLock: (project_id, file_id, fsPath, source, user_id, callback = (error) ->)->
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,31 +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"
ProjectEntityHandler.renameEntity project_id, entity_id, entityType, newName, userId, (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)
if newName.length > 0
EditorRealTimeController.emitToRoom project_id, 'reciveEntityRename', entity_id, newName
callback?()
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)
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) ->
@ -218,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()

View file

@ -69,7 +69,7 @@ module.exports =
logger.err err:err, project_id:project_id, file_id:file_id, path:path, "error processing file"
return callback(err)
else if file_id?
editorController.replaceFile project_id, file_id, fsPath, source, user_id, finish
editorController.replaceFileWithoutLock project_id, file_id, fsPath, source, user_id, finish
else
editorController.addFile project_id, folder?._id, fileName, fsPath, source, user_id, finish

View file

@ -50,7 +50,7 @@ module.exports = FileSystemImportManager =
existingFile = fileRef
break
if existingFile?
EditorController.replaceFile project_id, existingFile._id, path, "upload", user_id, callback
EditorController.replaceFileWithoutLock project_id, existingFile._id, path, "upload", user_id, callback
else
EditorController.addFileWithoutLock project_id, folder_id, name, path, "upload", user_id, callback

View file

@ -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,25 +218,17 @@ 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 "replaceFile", ->
describe "replaceFileWithoutLock", ->
beforeEach ->
@project_id = "12dsankj"
@file_id = "file_id_here"
@ -254,8 +236,10 @@ describe "EditorController", ->
it 'should send the replace file message to the editor controller', (done)->
@ProjectEntityHandler.replaceFile = sinon.stub().callsArgWith(4)
@EditorController.replaceFile @project_id, @file_id, @fsPath, @source, @user_id, =>
@ProjectEntityHandler.replaceFile.calledWith(@project_id, @file_id, @fsPath, @user_id).should.equal true
@EditorController.replaceFileWithoutLock @project_id, @file_id, @fsPath, @source, @user_id, =>
@ProjectEntityHandler.replaceFile
.calledWith(@project_id, @file_id, @fsPath, @user_id)
.should.equal true
done()
describe 'addFolderWithoutLock :', ->
@ -297,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)->
@ -310,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 :', ->
@ -347,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)->
@ -361,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)->
@ -391,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"
@ -466,23 +427,29 @@ describe "EditorController", ->
describe "renameEntity", ->
beforeEach ->
beforeEach (done) ->
@entity_id = "entity_id_here"
@entityType = "doc"
@newName = "bobsfile.tex"
@ProjectEntityHandler.renameEntity = sinon.stub().callsArg(5)
@EditorRealTimeController.emitToRoom = sinon.stub()
it "should call the project handler", (done)->
@EditorController.renameEntity @project_id, @entity_id, @entityType, @newName, @user_id, =>
@ProjectEntityHandler.renameEntity.calledWith(@project_id, @entity_id, @entityType, @newName, @user_id).should.equal true
done()
@EditorController.renameEntity @project_id, @entity_id, @entityType, @newName, @user_id, done
it "should call the project handler", ->
@ProjectEntityHandler.renameEntity
.calledWith(@project_id, @entity_id, @entityType, @newName, @user_id)
.should.equal true
it "should emit the update to the room", (done)->
@EditorController.renameEntity @project_id, @entity_id, @entityType, @newName, @user_id, =>
@EditorRealTimeController.emitToRoom.calledWith(@project_id, 'reciveEntityRename', @entity_id, @newName).should.equal true
done()
it "should take the lock", ->
@LockManager.runWithLock
.calledWith(@project_id)
.should.equal true
it "should emit the update to the room", ->
@EditorRealTimeController.emitToRoom
.calledWith(@project_id, 'reciveEntityRename', @entity_id, @newName)
.should.equal true
describe "moveEntity", ->
beforeEach ->
@ -491,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, =>
@ -501,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)->

View file

@ -115,7 +115,7 @@ describe 'UpdateMerger :', ->
@fileName = "file.png"
@fsPath = "fs/path.tex"
@editorController.addFile = sinon.stub().callsArg(6)
@editorController.replaceFile = sinon.stub().callsArg(5)
@editorController.replaceFileWithoutLock = sinon.stub().callsArg(5)
@editorController.deleteEntity = sinon.stub()
@editorController.mkdirp = sinon.stub().withArgs(@project_id).callsArgWith(2, null, [@folder], @folder)
@updateMerger.p.writeStreamToDisk = sinon.stub().withArgs(@project_id, @file_id, @update).callsArgWith(3, null, @fsPath)
@ -123,14 +123,14 @@ describe 'UpdateMerger :', ->
it 'should replace file if the file already exists', (done)->
@updateMerger.p.processFile @project_id, @file_id, @fsPath, @path, @source, @user_id, =>
@editorController.addFile.called.should.equal false
@editorController.replaceFile.calledWith(@project_id, @file_id, @fsPath, @source, @user_id).should.equal true
@editorController.replaceFileWithoutLock.calledWith(@project_id, @file_id, @fsPath, @source, @user_id).should.equal true
done()
it 'should call add file if the file does not exist', (done)->
@updateMerger.p.processFile @project_id, undefined, @fsPath, @path, @source, @user_id, =>
@editorController.mkdirp.calledWith(@project_id, "folder/").should.equal true
@editorController.addFile.calledWith(@project_id, @folder_id, @fileName, @fsPath, @source, @user_id).should.equal true
@editorController.replaceFile.called.should.equal false
@editorController.replaceFileWithoutLock.called.should.equal false
done()
describe 'delete entity :', (done)->

View file

@ -126,12 +126,12 @@ describe "FileSystemImportManager", ->
beforeEach ->
@EditorController.addFileWithoutLock = sinon.stub()
@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, false)
@EditorController.replaceFile = sinon.stub()
@EditorController.replaceFileWithoutLock = sinon.stub()
@FileSystemImportManager.addFile @user_id, @project_id, @folder_id, @name, @path_on_disk, false, @callback
it "should node add the file", ->
@EditorController.addFileWithoutLock.called.should.equal false
@EditorController.replaceFile.called.should.equal false
@EditorController.replaceFileWithoutLock.called.should.equal false
describe "addFile with replace set to true", ->
describe "when the file doesn't exist", ->
@ -169,7 +169,7 @@ describe "FileSystemImportManager", ->
}
@FileSystemImportManager._isSafeOnFileSystem = sinon.stub().callsArgWith(1, null, true)
@ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, @folder)
@EditorController.replaceFile = sinon.stub().callsArg(5)
@EditorController.replaceFileWithoutLock = sinon.stub().callsArg(5)
@FileSystemImportManager.addFile @user_id, @project_id, @folder_id, @name, @path_on_disk, true, @callback
it "should look up the folder", ->
@ -178,7 +178,8 @@ describe "FileSystemImportManager", ->
.should.equal true
it "should replace the file", ->
@EditorController.replaceFile.calledWith(@project_id, @file_id, @path_on_disk, "upload", @user_id)
@EditorController.replaceFileWithoutLock
.calledWith(@project_id, @file_id, @path_on_disk, "upload", @user_id)
.should.equal true
describe "addFolder", ->