diff --git a/services/web/app/coffee/Features/Editor/EditorController.coffee b/services/web/app/coffee/Features/Editor/EditorController.coffee index 4b19458e03..39a0bb02f0 100644 --- a/services/web/app/coffee/Features/Editor/EditorController.coffee +++ b/services/web/app/coffee/Features/Editor/EditorController.coffee @@ -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, => diff --git a/services/web/app/coffee/Features/Uploads/FileSystemImportManager.coffee b/services/web/app/coffee/Features/Uploads/FileSystemImportManager.coffee index f24770ff8f..4f0c4dcc0b 100644 --- a/services/web/app/coffee/Features/Uploads/FileSystemImportManager.coffee +++ b/services/web/app/coffee/Features/Uploads/FileSystemImportManager.coffee @@ -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? diff --git a/services/web/app/coffee/infrastructure/LockManager.coffee b/services/web/app/coffee/infrastructure/LockManager.coffee new file mode 100644 index 0000000000..32ae2af765 --- /dev/null +++ b/services/web/app/coffee/infrastructure/LockManager.coffee @@ -0,0 +1,54 @@ +metrics = require('./Metrics') +Settings = require('settings-sharelatex') +redis = require("redis-sharelatex") +rclient = redis.createClient(Settings.redis.web) +logger = require "logger-sharelatex" + +module.exports = LockManager = + LOCK_TEST_INTERVAL: 50 # 50ms between each test of the lock + MAX_LOCK_WAIT_TIME: 10000 # 10s maximum time to spend trying to get the lock + REDIS_LOCK_EXPIRY: 30 # seconds. Time until lock auto expires in redis. + + _blockingKey : (key)-> "Blocking:"+key + + tryLock : (key, callback = (err, isFree)->)-> + rclient.set LockManager._blockingKey(key), "locked", "EX", LockManager.REDIS_LOCK_EXPIRY, "NX", (err, gotLock)-> + return callback(err) if err? + if gotLock == "OK" + metrics.inc "doc-not-blocking" + callback err, true + else + metrics.inc "doc-blocking" + logger.log key: key, redis_response: gotLock, "doc is locked" + callback err, false + + getLock: (key, callback = (error) ->) -> + startTime = Date.now() + do attempt = () -> + if Date.now() - startTime > LockManager.MAX_LOCK_WAIT_TIME + return callback(new Error("Timeout")) + + LockManager.tryLock key, (error, gotLock) -> + return callback(error) if error? + if gotLock + callback(null) + else + setTimeout attempt, LockManager.LOCK_TEST_INTERVAL + + checkLock: (key, callback = (err, isFree)->)-> + multi = rclient.multi() + multi.exists LockManager._blockingKey(key) + multi.exec (err, replys)-> + return callback(err) if err? + exists = parseInt replys[0] + if exists == 1 + metrics.inc "doc-blocking" + callback err, false + else + metrics.inc "doc-not-blocking" + callback err, true + + releaseLock: (key, callback)-> + rclient.del LockManager._blockingKey(key), callback + + diff --git a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee index 3f0211d4c2..d4e05c8bb1 100644 --- a/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee @@ -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 diff --git a/services/web/test/UnitTests/coffee/Uploads/FileSystemImportManagerTests.coffee b/services/web/test/UnitTests/coffee/Uploads/FileSystemImportManagerTests.coffee index 6771151f59..fa385f7261 100644 --- a/services/web/test/UnitTests/coffee/Uploads/FileSystemImportManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Uploads/FileSystemImportManagerTests.coffee @@ -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", -> diff --git a/services/web/test/UnitTests/coffee/infrastructure/LockManager/CheckingTheLock.coffee b/services/web/test/UnitTests/coffee/infrastructure/LockManager/CheckingTheLock.coffee new file mode 100644 index 0000000000..3f56c346ac --- /dev/null +++ b/services/web/test/UnitTests/coffee/infrastructure/LockManager/CheckingTheLock.coffee @@ -0,0 +1,48 @@ +sinon = require('sinon') +assert = require('assert') +path = require('path') +modulePath = path.join __dirname, '../../../../../app/js/infrastructure/LockManager.js' +project_id = 1234 +doc_id = 5678 +blockingKey = "Blocking:#{doc_id}" +SandboxedModule = require('sandboxed-module') + +describe 'Lock Manager - checking the lock', ()-> + + existsStub = sinon.stub() + setStub = sinon.stub() + exireStub = sinon.stub() + execStub = sinon.stub() + + mocks = + "logger-sharelatex": log:-> + + "redis-sharelatex": + createClient : ()-> + auth:-> + multi: -> + exists: existsStub + expire: exireStub + set: setStub + exec: execStub + LockManager = SandboxedModule.require(modulePath, requires: mocks) + + it 'should check if lock exists but not set or expire', (done)-> + execStub.callsArgWith(0, null, ["1"]) + LockManager.checkLock doc_id, (err, docIsLocked)-> + existsStub.calledWith(blockingKey).should.equal true + setStub.called.should.equal false + exireStub.called.should.equal false + done() + + it 'should return true if the key does not exists', (done)-> + execStub.callsArgWith(0, null, "0") + LockManager.checkLock doc_id, (err, free)-> + free.should.equal true + done() + + it 'should return false if the key does exists', (done)-> + execStub.callsArgWith(0, null, "1") + LockManager.checkLock doc_id, (err, free)-> + free.should.equal false + done() diff --git a/services/web/test/UnitTests/coffee/infrastructure/LockManager/ReleasingTheLock.coffee b/services/web/test/UnitTests/coffee/infrastructure/LockManager/ReleasingTheLock.coffee new file mode 100644 index 0000000000..8eb0f4d75c --- /dev/null +++ b/services/web/test/UnitTests/coffee/infrastructure/LockManager/ReleasingTheLock.coffee @@ -0,0 +1,26 @@ +sinon = require('sinon') +assert = require('assert') +path = require('path') +modulePath = path.join __dirname, '../../../../../app/js/infrastructure/LockManager.js' +project_id = 1234 +doc_id = 5678 +SandboxedModule = require('sandboxed-module') + +describe 'LockManager - releasing the lock', ()-> + + deleteStub = sinon.stub().callsArgWith(1) + mocks = + "logger-sharelatex": log:-> + + "redis-sharelatex": + createClient : ()-> + auth:-> + del:deleteStub + + LockManager = SandboxedModule.require(modulePath, requires: mocks) + + it 'should put a all data into memory', (done)-> + LockManager.releaseLock doc_id, -> + deleteStub.calledWith("Blocking:#{doc_id}").should.equal true + done() + diff --git a/services/web/test/UnitTests/coffee/infrastructure/LockManager/getLockTests.coffee b/services/web/test/UnitTests/coffee/infrastructure/LockManager/getLockTests.coffee new file mode 100644 index 0000000000..5441577da6 --- /dev/null +++ b/services/web/test/UnitTests/coffee/infrastructure/LockManager/getLockTests.coffee @@ -0,0 +1,70 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +path = require('path') +modulePath = path.join __dirname, '../../../../../app/js/infrastructure/LockManager.js' +SandboxedModule = require('sandboxed-module') + +describe 'LockManager - getting the lock', -> + beforeEach -> + @LockManager = SandboxedModule.require modulePath, requires: + "logger-sharelatex": log:-> + "redis-sharelatex": + createClient : () => + auth:-> + @callback = sinon.stub() + @doc_id = "doc-id-123" + + describe "when the lock is not set", -> + beforeEach (done) -> + @LockManager.tryLock = sinon.stub().callsArgWith(1, null, true) + @LockManager.getLock @doc_id, (args...) => + @callback(args...) + done() + + it "should try to get the lock", -> + @LockManager.tryLock + .calledWith(@doc_id) + .should.equal true + + it "should only need to try once", -> + @LockManager.tryLock.callCount.should.equal 1 + + it "should return the callback", -> + @callback.calledWith(null).should.equal true + + describe "when the lock is initially set", -> + beforeEach (done) -> + startTime = Date.now() + @LockManager.LOCK_TEST_INTERVAL = 5 + @LockManager.tryLock = (doc_id, callback = (error, isFree) ->) -> + if Date.now() - startTime < 20 + callback null, false + else + callback null, true + sinon.spy @LockManager, "tryLock" + + @LockManager.getLock @doc_id, (args...) => + @callback(args...) + done() + + it "should call tryLock multiple times until free", -> + (@LockManager.tryLock.callCount > 1).should.equal true + + it "should return the callback", -> + @callback.calledWith(null).should.equal true + + describe "when the lock times out", -> + beforeEach (done) -> + time = Date.now() + @LockManager.MAX_LOCK_WAIT_TIME = 5 + @LockManager.tryLock = sinon.stub().callsArgWith(1, null, false) + @LockManager.getLock @doc_id, (args...) => + @callback(args...) + done() + + it "should return the callback with an error", -> + @callback.calledWith(new Error("timeout")).should.equal true + + + diff --git a/services/web/test/UnitTests/coffee/infrastructure/LockManager/tryLockTests.coffee b/services/web/test/UnitTests/coffee/infrastructure/LockManager/tryLockTests.coffee new file mode 100644 index 0000000000..3d5a19f265 --- /dev/null +++ b/services/web/test/UnitTests/coffee/infrastructure/LockManager/tryLockTests.coffee @@ -0,0 +1,38 @@ +sinon = require('sinon') +chai = require('chai') +should = chai.should() +path = require('path') +modulePath = path.join __dirname, '../../../../../app/js/infrastructure/LockManager.js' +SandboxedModule = require('sandboxed-module') + +describe 'LockManager - trying the lock', -> + beforeEach -> + @LockManager = SandboxedModule.require modulePath, requires: + "logger-sharelatex": log:-> + "redis-sharelatex": + createClient : () => + auth:-> + set: @set = sinon.stub() + @callback = sinon.stub() + @doc_id = "doc-id-123" + + describe "when the lock is not set", -> + beforeEach -> + @set.callsArgWith(5, null, "OK") + @LockManager.tryLock @doc_id, @callback + + it "should set the lock key with an expiry if it is not set", -> + @set.calledWith("Blocking:#{@doc_id}", "locked", "EX", 30, "NX") + .should.equal true + + it "should return the callback with true", -> + @callback.calledWith(null, true).should.equal true + + describe "when the lock is already set", -> + beforeEach -> + @set.callsArgWith(5, null, null) + @LockManager.tryLock @doc_id, @callback + + it "should return the callback with false", -> + @callback.calledWith(null, false).should.equal true +