added locks into editor controller for new/move/deletes

filesystem import manager uses the unlocked version
This commit is contained in:
Henry Oswald 2014-11-26 14:07:04 +00:00
parent bd841b4795
commit 857d867191
4 changed files with 253 additions and 38 deletions

View file

@ -17,6 +17,7 @@ TrackChangesManager = require("../TrackChanges/TrackChangesManager")
Settings = require('settings-sharelatex')
async = require('async')
ConnectedUsersManager = require("../ConnectedUsers/ConnectedUsersManager")
LockManager = require("../../infrastructure/LockManager")
_ = require('underscore')
redis = require("redis-sharelatex")
rclientPub = redis.createClient(Settings.redis.web)
@ -189,7 +190,17 @@ module.exports = EditorController =
logger.log project_id:project_id, doc_id:doc_id, "notifying users that the document has been updated"
DocumentUpdaterHandler.flushDocToMongo project_id, doc_id, callback
addDoc: (project_id, folder_id, docName, docLines, source, 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, (error, doc)->
LockManager.releaseLock project_id, ->
callback(error, doc)
addDocWithoutLock: (project_id, folder_id, docName, docLines, source, callback = (error, doc)->)->
docName = docName.trim()
logger.log {project_id, folder_id, docName, source}, "sending new doc to project"
Metrics.inc "editor.add-doc"
@ -197,7 +208,18 @@ module.exports = EditorController =
EditorRealTimeController.emitToRoom(project_id, 'reciveNewDoc', folder_id, doc, source)
callback(err, doc)
addFile: (project_id, folder_id, fileName, path, source, 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, (error, file)->
LockManager.releaseLock project_id, ->
callback(error, file)
addFileWithoutLock: (project_id, folder_id, fileName, path, source, callback = (error, file)->)->
fileName = fileName.trim()
logger.log {project_id, folder_id, fileName, path}, "sending new file to project"
Metrics.inc "editor.add-file"
@ -208,7 +230,18 @@ module.exports = EditorController =
replaceFile: (project_id, file_id, fsPath, source, callback = (error) ->)->
ProjectEntityHandler.replaceFile project_id, file_id, fsPath, callback
addFolder: (project_id, folder_id, folderName, source, callback = (error, folder)->)->
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)
addFolderWithoutLock: (project_id, folder_id, folderName, source, callback = (error, folder)->)->
folderName = folderName.trim()
logger.log {project_id, folder_id, folderName, source}, "sending new folder to project"
Metrics.inc "editor.add-folder"
@ -216,7 +249,17 @@ module.exports = EditorController =
@p.notifyProjectUsersOfNewFolder project_id, folder_id, folder, (error) ->
callback error, folder
mkdirp: (project_id, path, callback)->
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)->
logger.log project_id:project_id, path:path, "making directories if they don't exist"
ProjectEntityHandler.mkdirp project_id, path, (err, newFolders, lastFolder)=>
self = @
@ -226,7 +269,16 @@ module.exports = EditorController =
async.series jobs, (err)->
callback err, newFolders, lastFolder
deleteEntity: (project_id, entity_id, entityType, source, callback)->
deleteEntity : (project_id, entity_id, entityType, source, 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, (err)->
LockManager.releaseLock project_id, ()->
callback(err)
deleteEntityWithoutLock: (project_id, entity_id, entityType, source, callback)->
logger.log {project_id, entity_id, entityType, source}, "start delete process of entity"
Metrics.inc "editor.delete-entity"
ProjectEntityHandler.deleteEntity project_id, entity_id, entityType, =>
@ -270,7 +322,7 @@ module.exports = EditorController =
if newName.length > 0
EditorRealTimeController.emitToRoom project_id, 'reciveEntityRename', entity_id, newName
callback?()
#
moveEntity: (project_id, entity_id, folder_id, entityType, callback)->
Metrics.inc "editor.move-entity"
ProjectEntityHandler.moveEntity project_id, entity_id, folder_id, entityType, =>

View file

@ -11,7 +11,7 @@ module.exports = FileSystemImportManager =
return callback(error) if error?
content = content.replace(/\r/g, "")
lines = content.split("\n")
EditorController.addDoc project_id, folder_id, name, lines, "upload", callback
EditorController.addDocWithoutLock project_id, folder_id, name, lines, "upload", callback
addFile: (project_id, folder_id, name, path, replace, callback = (error, file)-> )->
if replace
@ -26,12 +26,12 @@ module.exports = FileSystemImportManager =
if existingFile?
EditorController.replaceFile project_id, existingFile._id, path, "upload", callback
else
EditorController.addFile project_id, folder_id, name, path, "upload", callback
EditorController.addFileWithoutLock project_id, folder_id, name, path, "upload", callback
else
EditorController.addFile project_id, folder_id, name, path, "upload", callback
EditorController.addFileWithoutLock project_id, folder_id, name, path, "upload", callback
addFolder: (project_id, folder_id, name, path, replace, callback = (error)-> ) ->
EditorController.addFolder project_id, folder_id, name, "upload", (error, new_folder) =>
EditorController.addFolderWithoutLock project_id, folder_id, name, "upload", (error, new_folder) =>
return callback(error) if error?
@addFolderContents project_id, new_folder._id, path, replace, (error) ->
return callback(error) if error?

View file

@ -1,6 +1,8 @@
SandboxedModule = require('sandboxed-module')
sinon = require('sinon')
require('chai').should()
expect = require("chai").expect
modulePath = require('path').join __dirname, '../../../../app/js/Features/Editor/EditorController'
MockClient = require "../helpers/MockClient"
assert = require('assert')
@ -58,7 +60,9 @@ describe "EditorController", ->
@ConnectedUsersManager =
markUserAsDisconnected:sinon.stub()
updateUserPosition:sinon.stub()
@LockManager =
getLock : sinon.stub()
releaseLock : sinon.stub()
@EditorController = SandboxedModule.require modulePath, requires:
"../../infrastructure/Server" : io : @io
'../Project/ProjectEditorHandler' : @ProjectEditorHandler
@ -79,6 +83,7 @@ describe "EditorController", ->
"../../infrastructure/Metrics": @Metrics = { inc: sinon.stub() }
"../TrackChanges/TrackChangesManager": @TrackChangesManager = {}
"../ConnectedUsers/ConnectedUsersManager":@ConnectedUsersManager
"../../infrastructure/LockManager":@LockManager
'redis-sharelatex':createClient:-> auth:->
"logger-sharelatex": @logger =
log: sinon.stub()
@ -475,7 +480,7 @@ describe "EditorController", ->
done()
describe 'addDoc', ->
describe 'addDocWithoutLock', ->
beforeEach ->
@ProjectEntityHandler.addDoc = ()->
@EditorRealTimeController.emitToRoom = sinon.stub()
@ -489,14 +494,14 @@ describe "EditorController", ->
it 'should add the doc using the project entity handler', (done)->
mock = sinon.mock(@ProjectEntityHandler).expects("addDoc").withArgs(@project_id, @folder_id, @docName, @docLines).callsArg(4)
@EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, ->
@EditorController.addDocWithoutLock @project_id, @folder_id, @docName, @docLines, @source, ->
mock.verify()
done()
it 'should send the update out to the users in the project', (done)->
@ProjectEntityHandler.addDoc = sinon.stub().callsArgWith(4, null, @doc, @folder_id)
@EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, =>
@EditorController.addDocWithoutLock @project_id, @folder_id, @docName, @docLines, @source, =>
@EditorRealTimeController.emitToRoom
.calledWith(@project_id, "reciveNewDoc", @folder_id, @doc, @source)
.should.equal true
@ -504,11 +509,43 @@ describe "EditorController", ->
it 'should return the doc to the callback', (done) ->
@ProjectEntityHandler.addDoc = sinon.stub().callsArgWith(4, null, @doc, @folder_id)
@EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, (error, doc) =>
@EditorController.addDocWithoutLock @project_id, @folder_id, @docName, @docLines, @source, (error, doc) =>
doc.should.equal @doc
done()
describe 'addFile :', ->
describe "addDoc", ->
beforeEach ->
@LockManager.getLock.callsArgWith(1)
@LockManager.releaseLock.callsArgWith(1)
@EditorController.addDocWithoutLock = sinon.stub().callsArgWith(5)
it "should call addDocWithoutLock", (done)->
@EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, =>
@EditorController.addDocWithoutLock.calledWith(@project_id, @folder_id, @docName, @docLines, @source).should.equal true
done()
it "should take the lock", (done)->
@EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, =>
@LockManager.getLock.calledWith(@project_id).should.equal true
done()
it "should release the lock", (done)->
@EditorController.addDoc @project_id, @folder_id, @docName, @docLines, @source, =>
@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, (err)=>
expect(err).to.exist
err.should.equal "timed out"
done()
describe 'addFileWithoutLock:', ->
beforeEach ->
@ProjectEntityHandler.addFile = ->
@EditorRealTimeController.emitToRoom = sinon.stub()
@ -520,15 +557,15 @@ describe "EditorController", ->
@stream = new ArrayBuffer()
it 'should add the folder using the project entity handler', (done)->
mock = sinon.mock(@ProjectEntityHandler).expects("addFile").withArgs(@project_id).callsArg(4)
@EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, =>
mock.verify()
@ProjectEntityHandler.addFile = sinon.stub().callsArgWith(4)
@EditorController.addFileWithoutLock @project_id, @folder_id, @fileName, @stream, @source, =>
@ProjectEntityHandler.addFile.calledWith(@project_id, @folder_id).should.equal true
done()
it 'should send the update of a new folder out to the users in the project', (done)->
@ProjectEntityHandler.addFile = sinon.stub().callsArgWith(4, null, @file, @folder_id)
@EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, =>
@EditorController.addFileWithoutLock @project_id, @folder_id, @fileName, @stream, @source, =>
@EditorRealTimeController.emitToRoom
.calledWith(@project_id, "reciveNewFile", @folder_id, @file, @source)
.should.equal true
@ -536,11 +573,43 @@ describe "EditorController", ->
it "should return the file in the callback", (done) ->
@ProjectEntityHandler.addFile = sinon.stub().callsArgWith(4, null, @file, @folder_id)
@EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, (error, file) =>
@EditorController.addFileWithoutLock @project_id, @folder_id, @fileName, @stream, @source, (error, file) =>
file.should.equal @file
done()
describe "addFile", ->
beforeEach ->
@LockManager.getLock.callsArgWith(1)
@LockManager.releaseLock.callsArgWith(1)
@EditorController.addFileWithoutLock = sinon.stub().callsArgWith(5)
it "should call addFileWithoutLock", (done)->
@EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, (error, file) =>
@EditorController.addFileWithoutLock.calledWith(@project_id, @folder_id, @fileName, @stream, @source).should.equal true
done()
it "should take the lock", (done)->
@EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, (error, file) =>
@LockManager.getLock.calledWith(@project_id).should.equal true
done()
it "should release the lock", (done)->
@EditorController.addFile @project_id, @folder_id, @fileName, @stream, @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")
@EditorController.addFile @project_id, @folder_id, @fileName, @stream, @source, (err, file) =>
expect(err).to.exist
err.should.equal "timed out"
done()
describe "replaceFile", ->
beforeEach ->
@project_id = "12dsankj"
@ -553,7 +622,7 @@ describe "EditorController", ->
@ProjectEntityHandler.replaceFile.calledWith(@project_id, @file_id, @fsPath).should.equal true
done()
describe 'addFolder :', ->
describe 'addFolderWithoutLock :', ->
beforeEach ->
@ProjectEntityHandler.addFolder = ->
@EditorRealTimeController.emitToRoom = sinon.stub()
@ -565,7 +634,7 @@ describe "EditorController", ->
it 'should add the folder using the project entity handler', (done)->
mock = sinon.mock(@ProjectEntityHandler).expects("addFolder").withArgs(@project_id, @folder_id, @folderName).callsArg(3)
@EditorController.addFolder @project_id, @folder_id, @folderName, @source, ->
@EditorController.addFolderWithoutLock @project_id, @folder_id, @folderName, @source, ->
mock.verify()
done()
@ -573,7 +642,7 @@ describe "EditorController", ->
@ProjectEntityHandler.addFolder = (project_id, folder_id, folderName, callback)=> callback(null, @folder, @folder_id)
mock = sinon.mock(@EditorController.p).expects('notifyProjectUsersOfNewFolder').withArgs(@project_id, @folder_id, @folder).callsArg(3)
@EditorController.addFolder @project_id, @folder_id, @folderName, @source, ->
@EditorController.addFolderWithoutLock @project_id, @folder_id, @folderName, @source, ->
mock.verify()
done()
@ -586,11 +655,42 @@ describe "EditorController", ->
it 'should return the folder in the callback', (done) ->
@ProjectEntityHandler.addFolder = (project_id, folder_id, folderName, callback)=> callback(null, @folder, @folder_id)
@EditorController.addFolder @project_id, @folder_id, @folderName, @source, (error, folder) =>
@EditorController.addFolderWithoutLock @project_id, @folder_id, @folderName, @source, (error, folder) =>
folder.should.equal @folder
done()
describe 'mkdirp :', ->
describe "addFolder", ->
beforeEach ->
@LockManager.getLock.callsArgWith(1)
@LockManager.releaseLock.callsArgWith(1)
@EditorController.addFolderWithoutLock = sinon.stub().callsArgWith(4)
it "should call addFolderWithoutLock", (done)->
@EditorController.addFolder @project_id, @folder_id, @folderName, @source, (error, file) =>
@EditorController.addFolderWithoutLock.calledWith(@project_id, @folder_id, @folderName, @source).should.equal true
done()
it "should take the lock", (done)->
@EditorController.addFolder @project_id, @folder_id, @folderName, @source, (error, file) =>
@LockManager.getLock.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")
@EditorController.addFolder @project_id, @folder_id, @folderName, @source, (err, file) =>
expect(err).to.exist
err.should.equal "timed out"
done()
describe 'mkdirpWithoutLock :', ->
it 'should make the dirs and notifyProjectUsersOfNewFolder', (done)->
path = "folder1/folder2"
@ -602,14 +702,77 @@ describe "EditorController", ->
@EditorController.p.notifyProjectUsersOfNewFolder = sinon.stub().callsArg(3)
@EditorController.mkdirp @project_id, path, (err, newFolders, lastFolder)=>
@EditorController.mkdirpWithoutLock @project_id, path, (err, newFolders, lastFolder)=>
@EditorController.p.notifyProjectUsersOfNewFolder.calledWith(@project_id, @folder1._id, @folder2).should.equal true
@EditorController.p.notifyProjectUsersOfNewFolder.calledWith(@project_id, @folder2._id, @folder3).should.equal true
newFolders.should.deep.equal [@folder1, @folder2, @folder3]
lastFolder.should.equal @folder3
done()
describe 'deleteEntity', ->
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)->
@EditorController.mkdirp @project_id, @path, (error, file) =>
@EditorController.mkdirpWithoutLock.calledWith(@project_id, @path).should.equal true
done()
it "should take the lock", (done)->
@EditorController.mkdirp @project_id, @path, (error, file) =>
@LockManager.getLock.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")
@EditorController.mkdirp @project_id, @path, (err, file) =>
expect(err).to.exist
err.should.equal "timed out"
done()
describe "deleteEntity", ->
beforeEach ->
@LockManager.getLock.callsArgWith(1)
@LockManager.releaseLock.callsArgWith(1)
@EditorController.deleteEntityWithoutLock = sinon.stub().callsArgWith(4)
it "should call deleteEntityWithoutLock", (done)->
@EditorController.deleteEntity @project_id, @entity_id, @type, @source, =>
@EditorController.deleteEntityWithoutLock.calledWith(@project_id, @entity_id, @type, @source).should.equal true
done()
it "should take the lock", (done)->
@EditorController.deleteEntity @project_id, @entity_id, @type, @source, =>
@LockManager.getLock.calledWith(@project_id).should.equal true
done()
it "should release the lock", (done)->
@EditorController.deleteEntity @project_id, @entity_id, @type, @source, (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")
@EditorController.deleteEntity @project_id, @entity_id, @type, @source, (err)=>
expect(err).to.exist
err.should.equal "timed out"
done()
describe 'deleteEntityWithoutLock', ->
beforeEach ->
@ProjectEntityHandler.deleteEntity = (project_id, entity_id, type, callback)-> callback()
@entity_id = "entity_id_here"
@ -619,12 +782,12 @@ describe "EditorController", ->
it 'should delete the folder using the project entity handler', (done)->
mock = sinon.mock(@ProjectEntityHandler).expects("deleteEntity").withArgs(@project_id, @entity_id, @type).callsArg(3)
@EditorController.deleteEntity @project_id, @entity_id, @type, @source, ->
@EditorController.deleteEntityWithoutLock @project_id, @entity_id, @type, @source, ->
mock.verify()
done()
it 'notify users an entity has been deleted', (done)->
@EditorController.deleteEntity @project_id, @entity_id, @type, @source, =>
@EditorController.deleteEntityWithoutLock @project_id, @entity_id, @type, @source, =>
@EditorRealTimeController.emitToRoom
.calledWith(@project_id, "removeEntity", @entity_id, @source)
.should.equal true

View file

@ -22,14 +22,14 @@ describe "FileSystemImportManager", ->
@docContent = "one\ntwo\nthree"
@docLines = @docContent.split("\n")
@fs.readFile = sinon.stub().callsArgWith(2, null, @docContent)
@EditorController.addDoc = sinon.stub().callsArg(5)
@EditorController.addDocWithoutLock = sinon.stub().callsArg(5)
@FileSystemImportManager.addDoc @project_id, @folder_id, @name, @path_on_disk, false, @callback
it "should read the file from disk", ->
@fs.readFile.calledWith(@path_on_disk, "utf8").should.equal true
it "should insert the doc", ->
@EditorController.addDoc.calledWith(@project_id, @folder_id, @name, @docLines, "upload")
@EditorController.addDocWithoutLock.calledWith(@project_id, @folder_id, @name, @docLines, "upload")
.should.equal true
describe "addDoc with windows line ending", ->
@ -37,20 +37,20 @@ describe "FileSystemImportManager", ->
@docContent = "one\r\ntwo\r\nthree"
@docLines = ["one", "two", "three"]
@fs.readFile = sinon.stub().callsArgWith(2, null, @docContent)
@EditorController.addDoc = sinon.stub().callsArg(5)
@EditorController.addDocWithoutLock = sinon.stub().callsArg(5)
@FileSystemImportManager.addDoc @project_id, @folder_id, @name, @path_on_disk, false, @callback
it "should strip the \\r characters before adding", ->
@EditorController.addDoc.calledWith(@project_id, @folder_id, @name, @docLines, "upload")
@EditorController.addDocWithoutLock.calledWith(@project_id, @folder_id, @name, @docLines, "upload")
.should.equal true
describe "addFile with replace set to false", ->
beforeEach ->
@EditorController.addFile = sinon.stub().callsArg(5)
@EditorController.addFileWithoutLock = sinon.stub().callsArg(5)
@FileSystemImportManager.addFile @project_id, @folder_id, @name, @path_on_disk, false, @callback
it "should add the file", ->
@EditorController.addFile.calledWith(@project_id, @folder_id, @name, @path_on_disk, "upload")
@EditorController.addFileWithoutLock.calledWith(@project_id, @folder_id, @name, @path_on_disk, "upload")
.should.equal true
describe "addFile with replace set to true", ->
@ -63,7 +63,7 @@ describe "FileSystemImportManager", ->
}]
}
@ProjectLocator.findElement = sinon.stub().callsArgWith(1, null, @folder)
@EditorController.addFile = sinon.stub().callsArg(5)
@EditorController.addFileWithoutLock = sinon.stub().callsArg(5)
@FileSystemImportManager.addFile @project_id, @folder_id, @name, @path_on_disk, true, @callback
it "should look up the folder", ->
@ -72,7 +72,7 @@ describe "FileSystemImportManager", ->
.should.equal true
it "should add the file", ->
@EditorController.addFile.calledWith(@project_id, @folder_id, @name, @path_on_disk, "upload")
@EditorController.addFileWithoutLock.calledWith(@project_id, @folder_id, @name, @path_on_disk, "upload")
.should.equal true
describe "when the file does exist", ->
@ -102,12 +102,12 @@ describe "FileSystemImportManager", ->
describe "addFolder", ->
beforeEach ->
@new_folder_id = "new-folder-id"
@EditorController.addFolder = sinon.stub().callsArgWith(4, null, _id: @new_folder_id)
@EditorController.addFolderWithoutLock = sinon.stub().callsArgWith(4, null, _id: @new_folder_id)
@FileSystemImportManager.addFolderContents = sinon.stub().callsArg(4)
@FileSystemImportManager.addFolder @project_id, @folder_id, @name, @path_on_disk, @replace, @callback
it "should add a folder to the project", ->
@EditorController.addFolder.calledWith(@project_id, @folder_id, @name, "upload")
@EditorController.addFolderWithoutLock.calledWith(@project_id, @folder_id, @name, "upload")
.should.equal true
it "should add the folders contents", ->