From cbc6fb232d0d3a99b5534ba8427bcb4f99ed86cc Mon Sep 17 00:00:00 2001 From: Hayden Faulds Date: Thu, 1 Feb 2018 15:31:42 +0000 Subject: [PATCH] add mongo transaction lock when getting or updating projects --- .../ProjectEntityMongoUpdateHandler.coffee | 41 ++++++++------ .../Project/ProjectEntityUpdateHandler.coffee | 2 +- .../Features/Project/ProjectGetter.coffee | 13 ++++- .../coffee/infrastructure/LockManager.coffee | 53 ++++++++++++------- ...rojectEntityMongoUpdateHandlerTests.coffee | 27 ++++++---- .../ProjectEntityUpdateHandlerTests.coffee | 5 +- .../LockManager/CheckingTheLock.coffee | 12 ++--- .../LockManager/ReleasingTheLock.coffee | 7 ++- .../LockManager/getLockTests.coffee | 20 +++---- .../LockManager/tryLockTests.coffee | 11 ++-- 10 files changed, 115 insertions(+), 76 deletions(-) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee index 251ff10577..49b8a8f430 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityMongoUpdateHandler.coffee @@ -6,15 +6,24 @@ settings = require('settings-sharelatex') CooldownManager = require '../Cooldown/CooldownManager' Errors = require '../Errors/Errors' Folder = require('../../models/Folder').Folder +LockManager = require('../../infrastructure/LockManager') Project = require('../../models/Project').Project ProjectEntityHandler = require('./ProjectEntityHandler') ProjectGetter = require('./ProjectGetter') ProjectLocator = require('./ProjectLocator') SafePath = require './SafePath' +wrapWithLock = (methodWithoutLock) -> + methodWithLock = (project_id, args..., callback) -> + LockManager.mongoTransactionLock.runWithLock project_id, + (cb) -> methodWithoutLock project_id, args..., cb + callback + methodWithLock.withoutLock = methodWithoutLock + methodWithLock + module.exports = ProjectEntityMongoUpdateHandler = self = - addDoc: (project_id, folder_id, doc, callback = (err, result) ->) -> - ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project) -> + addDoc: wrapWithLock (project_id, folder_id, doc, callback = (err, result) ->) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add doc" return callback(err) @@ -22,8 +31,8 @@ module.exports = ProjectEntityMongoUpdateHandler = self = self._confirmFolder project, folder_id, (folder_id) => self._putElement project, folder_id, doc, "doc", callback - addFile: (project_id, folder_id, fileRef, callback = (error, result, project) ->)-> - ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project) -> + addFile: wrapWithLock (project_id, folder_id, fileRef, callback = (error, result, project) ->)-> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add file" return callback(err) @@ -31,8 +40,8 @@ module.exports = ProjectEntityMongoUpdateHandler = self = self._confirmFolder project, folder_id, (folder_id)-> self._putElement project, folder_id, fileRef, "file", callback - replaceFile: (project_id, file_id, callback) -> - ProjectGetter.getProject project_id, {rootFolder: true, name:true}, (err, project) -> + replaceFile: wrapWithLock (project_id, file_id, callback) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder: true, name:true}, (err, project) -> return callback(err) if err? ProjectLocator.findElement {project:project, element_id: file_id, type: 'file'}, (err, fileRef, path)=> return callback(err) if err? @@ -48,7 +57,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = return callback(err) if err? callback null, fileRef, project, path - mkdirp: (project_id, path, callback) -> + mkdirp: wrapWithLock (project_id, path, callback) -> folders = path.split('/') folders = _.select folders, (folder)-> return folder.length != 0 @@ -69,7 +78,7 @@ module.exports = ProjectEntityMongoUpdateHandler = self = ProjectLocator.findElementByPath project, builtUpPath, (err, foundFolder)=> if !foundFolder? logger.log path:path, project_id:project._id, folderName:folderName, "making folder from mkdirp" - self.addFolder project_id, parentFolder_id, folderName, (err, newFolder, parentFolder_id)-> + self.addFolder.withoutLock project_id, parentFolder_id, folderName, (err, newFolder, parentFolder_id)-> return callback(err) if err? newFolder.parentFolder_id = parentFolder_id previousFolders.push newFolder @@ -86,8 +95,8 @@ module.exports = ProjectEntityMongoUpdateHandler = self = !folder.filterOut callback null, folders, lastFolder - moveEntity: (project_id, entity_id, destFolderId, entityType, callback = (error) ->) -> - ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project) -> + moveEntity: wrapWithLock (project_id, entity_id, destFolderId, entityType, callback = (error) ->) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (err, project) -> return callback(err) if err? ProjectLocator.findElement {project, element_id: entity_id, type: entityType}, (err, entity, entityPath)-> return callback(err) if err? @@ -106,8 +115,8 @@ module.exports = ProjectEntityMongoUpdateHandler = self = changes = {oldDocs, newDocs, oldFiles, newFiles} callback null, project.name, startPath, endPath, entity.rev, changes, callback - deleteEntity: (project_id, entity_id, entityType, callback) -> - ProjectGetter.getProject project_id, {name:true, rootFolder:true}, (error, project) -> + deleteEntity: wrapWithLock (project_id, entity_id, entityType, callback) -> + ProjectGetter.getProjectWithoutLock project_id, {name:true, rootFolder:true}, (error, project) -> return callback(error) if error? ProjectLocator.findElement {project: project, element_id: entity_id, type: entityType}, (error, entity, path) -> return callback(error) if error? @@ -115,8 +124,8 @@ module.exports = ProjectEntityMongoUpdateHandler = self = return callback(error) if error? callback null, entity, path, project - renameEntity: (project_id, entity_id, entityType, newName, callback) -> - ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (error, project)=> + renameEntity: wrapWithLock (project_id, entity_id, entityType, newName, callback) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (error, project)=> return callback(error) if error? ProjectEntityHandler.getAllEntitiesFromProject project, (error, oldDocs, oldFiles) => return callback(error) if error? @@ -138,8 +147,8 @@ module.exports = ProjectEntityMongoUpdateHandler = self = changes = {oldDocs, newDocs, oldFiles, newFiles} callback null, project.name, startPath, endPath, entity.rev, changes, callback - addFolder: (project_id, parentFolder_id, folderName, callback) -> - ProjectGetter.getProject project_id, {rootFolder:true, name:true}, (err, project) -> + addFolder: wrapWithLock (project_id, parentFolder_id, folderName, callback) -> + ProjectGetter.getProjectWithoutLock project_id, {rootFolder:true, name:true}, (err, project) -> if err? logger.err project_id:project_id, err:err, "error getting project for add folder" return callback(err) diff --git a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee index 0d2864081c..54bfc385ec 100644 --- a/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee +++ b/services/web/app/coffee/Features/Project/ProjectEntityUpdateHandler.coffee @@ -20,7 +20,7 @@ TpdsUpdateSender = require('../ThirdPartyDataStore/TpdsUpdateSender') wrapWithLock = (methodWithoutLock) -> methodWithLock = (project_id, args..., callback) -> - LockManager.runWithLock project_id, + LockManager.sequentialProjectStructureUpdateLock.runWithLock project_id, (cb) -> methodWithoutLock project_id, args..., cb callback methodWithLock.withoutLock = methodWithoutLock diff --git a/services/web/app/coffee/Features/Project/ProjectGetter.coffee b/services/web/app/coffee/Features/Project/ProjectGetter.coffee index 4506292fbb..302dccea77 100644 --- a/services/web/app/coffee/Features/Project/ProjectGetter.coffee +++ b/services/web/app/coffee/Features/Project/ProjectGetter.coffee @@ -5,6 +5,7 @@ ObjectId = mongojs.ObjectId async = require "async" Project = require("../../models/Project").Project logger = require("logger-sharelatex") +LockManager = require("../../infrastructure/LockManager") module.exports = ProjectGetter = EXCLUDE_DEPTH: 8 @@ -26,7 +27,15 @@ module.exports = ProjectGetter = callback error, projects[0] - getProject: (query, projection, callback = (error, project) ->) -> + getProject: (project_id, projection, callback = (error, project) ->) -> + if projection?.rootFolder + LockManager.mongoTransactionLock.runWithLock project_id, + (cb) -> ProjectGetter.getProjectWithoutLock project_id, projection, cb + callback + else + ProjectGetter.getProjectWithoutLock project_id, projection, callback + + getProjectWithoutLock: (query, projection, callback = (error, project) ->) -> if !query? return callback("no query provided") @@ -44,7 +53,7 @@ module.exports = ProjectGetter = logger.log query:query, err:err, type:typeof(query), "malformed get request" return callback(err) - db.projects.find query, projection, (err, project)-> + db.projects.find query, projection, (err, project) -> if err? logger.err err:err, query:query, projection:projection, "error getting project" return callback(err) diff --git a/services/web/app/coffee/infrastructure/LockManager.coffee b/services/web/app/coffee/infrastructure/LockManager.coffee index 5532d5bd94..5e1b42dcb0 100644 --- a/services/web/app/coffee/infrastructure/LockManager.coffee +++ b/services/web/app/coffee/infrastructure/LockManager.coffee @@ -9,10 +9,36 @@ module.exports = LockManager = 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)-> "lock:web:{#{key}}" + # This lock is used whenever we read or write to an existing project's + # structure. Some operations to project structure cannot be done atomically + # in mongo, this lock is used to prevent reading the structure between two + # parts of a staged update. + # + # This lock should only be called by ProjectEntityHandler and ProjectGetter + mongoTransactionLock: + runWithLock: (project_id, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> + LockManager._runWithLock "lock:web:mongoTransaction:{#{project_id}}", runner, callback - tryLock : (key, callback = (err, isFree)->)-> - rclient.set LockManager._blockingKey(key), "locked", "EX", LockManager.REDIS_LOCK_EXPIRY, "NX", (err, gotLock)-> + # This lock is used to make sure that the project structure updates are made + # sequentially. In particular the updates must be made in mongo and sent to + # the doc-updater in the same order. + # + # This lock is generally called at a high level. + sequentialProjectStructureUpdateLock: + runWithLock: (project_id, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> + LockManager._runWithLock "lock:web:sequentialProjectStructureUpdateLock:{#{project_id}}", runner, callback + + _runWithLock: (key, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> + LockManager._getLock key, (error) -> + return callback(error) if error? + runner (error1, values...) -> + LockManager._releaseLock key, (error2) -> + error = error1 or error2 + return callback(error) if error? + callback null, values... + + _tryLock : (key, callback = (err, isFree)->)-> + rclient.set key, "locked", "EX", LockManager.REDIS_LOCK_EXPIRY, "NX", (err, gotLock)-> return callback(err) if err? if gotLock == "OK" metrics.inc "lock-not-blocking" @@ -22,22 +48,22 @@ module.exports = LockManager = logger.log key: key, redis_response: gotLock, "lock is locked" callback err, false - getLock: (key, callback = (error) ->) -> + _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) -> + 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)->)-> + _checkLock: (key, callback = (err, isFree)->)-> multi = rclient.multi() - multi.exists LockManager._blockingKey(key) + multi.exists key multi.exec (err, replys)-> return callback(err) if err? exists = parseInt replys[0] @@ -48,14 +74,5 @@ module.exports = LockManager = metrics.inc "lock-not-blocking" callback err, true - releaseLock: (key, callback)-> - rclient.del LockManager._blockingKey(key), callback - - runWithLock: (key, runner = ( (releaseLock = (error) ->) -> ), callback = ( (error) -> )) -> - LockManager.getLock key, (error) -> - return callback(error) if error? - runner (error1, values...) -> - LockManager.releaseLock key, (error2) -> - error = error1 or error2 - return callback(error) if error? - callback null, values... + _releaseLock: (key, callback)-> + rclient.del key, callback diff --git a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee index 6b45537465..fa1faafab5 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityMongoUpdateHandlerTests.coffee @@ -35,11 +35,15 @@ describe 'ProjectEntityMongoUpdateHandler', -> } "../Cooldown/CooldownManager": @CooldownManager = {} '../../models/Folder': Folder:@FolderModel + "../../infrastructure/LockManager":@LockManager = + mongoTransactionLock: + runWithLock: + sinon.spy((key, runner, callback) -> runner(callback)) '../../models/Project': Project:@ProjectModel = {} './ProjectEntityHandler': @ProjectEntityHandler = {} './ProjectLocator': @ProjectLocator = {} "./ProjectGetter": @ProjectGetter = - getProject: sinon.stub().yields(null, @project) + getProjectWithoutLock: sinon.stub().yields(null, @project) afterEach -> tk.reset() @@ -53,7 +57,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @subject.addDoc project_id, folder_id, @doc, @callback it 'gets the project', -> - @ProjectGetter.getProject + @ProjectGetter.getProjectWithoutLock .calledWith(project_id, {rootFolder:true, name: true}) .should.equal true @@ -76,7 +80,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @subject.addFile project_id, folder_id, @file, @callback it 'gets the project', -> - @ProjectGetter.getProject + @ProjectGetter.getProjectWithoutLock .calledWith(project_id, {rootFolder:true, name: true}) .should.equal true @@ -100,7 +104,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @subject.replaceFile project_id, file_id, @callback it 'gets the project', -> - @ProjectGetter.getProject + @ProjectGetter.getProjectWithoutLock .calledWith(project_id, {rootFolder:true, name: true}) .should.equal true @@ -141,8 +145,9 @@ describe 'ProjectEntityMongoUpdateHandler', -> cb "level1 is not the last foler " else cb null, @parentFolder - @subject.addFolder = (project_id, parentFolder_id, folderName, callback) => - callback null, {name:folderName}, @parentFolder_id + @subject.addFolder = + withoutLock: (project_id, parentFolder_id, folderName, callback) => + callback null, {name:folderName}, @parentFolder_id it 'should return the root folder if the path is just a slash', (done)-> path = "/" @@ -217,7 +222,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @subject.moveEntity project_id, doc_id, folder_id, "docs", @callback it 'should get the project', -> - @ProjectGetter.getProject + @ProjectGetter.getProjectWithoutLock .calledWith(project_id, {rootFolder:true, name:true}) .should.equal true @@ -256,7 +261,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @subject.deleteEntity project_id, doc_id, 'doc', @callback it "should get the project", -> - @ProjectGetter.getProject + @ProjectGetter.getProjectWithoutLock .calledWith(project_id, {name:true, rootFolder:true}) .should.equal true @@ -284,7 +289,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @doc = _id: doc_id, name: "old.tex", rev: 1 @folder = _id: folder_id - @ProjectGetter.getProject = sinon.stub().yields(null, @project) + @ProjectGetter.getProjectWithoutLock = sinon.stub().yields(null, @project) @ProjectEntityHandler.getAllEntitiesFromProject = sinon.stub() @ProjectEntityHandler.getAllEntitiesFromProject @@ -301,7 +306,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @subject.renameEntity project_id, doc_id, 'doc', @newName, @callback it 'should get the project', -> - @ProjectGetter.getProject + @ProjectGetter.getProjectWithoutLock .calledWith(project_id, {rootFolder:true, name:true}) .should.equal true @@ -339,7 +344,7 @@ describe 'ProjectEntityMongoUpdateHandler', -> @subject.addFolder project_id, folder_id, @folderName, @callback it 'gets the project', -> - @ProjectGetter.getProject + @ProjectGetter.getProjectWithoutLock .calledWith(project_id, {rootFolder:true, name: true}) .should.equal true diff --git a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee index d1fca71bc1..b498fcc1f1 100644 --- a/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee +++ b/services/web/test/unit/coffee/Project/ProjectEntityUpdateHandlerTests.coffee @@ -52,8 +52,9 @@ describe 'ProjectEntityUpdateHandler', -> '../../models/File': File:@FileModel '../FileStore/FileStoreHandler':@FileStoreHandler "../../infrastructure/LockManager":@LockManager = - runWithLock: - sinon.spy((key, runner, callback) -> runner(callback)) + sequentialProjectStructureUpdateLock: + runWithLock: + sinon.spy((key, runner, callback) -> runner(callback)) '../../models/Project': Project:@ProjectModel = {} "./ProjectGetter": @ProjectGetter = {} './ProjectLocator': @ProjectLocator = {} diff --git a/services/web/test/unit/coffee/infrastructure/LockManager/CheckingTheLock.coffee b/services/web/test/unit/coffee/infrastructure/LockManager/CheckingTheLock.coffee index cf56778ec8..80fcdd1dc2 100644 --- a/services/web/test/unit/coffee/infrastructure/LockManager/CheckingTheLock.coffee +++ b/services/web/test/unit/coffee/infrastructure/LockManager/CheckingTheLock.coffee @@ -2,9 +2,7 @@ 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 = "lock:web:{#{doc_id}}" +lockKey = "lock:web:{#{5678}}" SandboxedModule = require('sandboxed-module') describe 'LockManager - checking the lock', ()-> @@ -29,20 +27,20 @@ describe 'LockManager - checking the lock', ()-> 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 + LockManager._checkLock lockKey, (err, docIsLocked)-> + existsStub.calledWith(lockKey).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)-> + LockManager._checkLock lockKey, (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)-> + LockManager._checkLock lockKey, (err, free)-> free.should.equal false done() diff --git a/services/web/test/unit/coffee/infrastructure/LockManager/ReleasingTheLock.coffee b/services/web/test/unit/coffee/infrastructure/LockManager/ReleasingTheLock.coffee index 8ccab0a757..256f72f95f 100644 --- a/services/web/test/unit/coffee/infrastructure/LockManager/ReleasingTheLock.coffee +++ b/services/web/test/unit/coffee/infrastructure/LockManager/ReleasingTheLock.coffee @@ -2,8 +2,7 @@ sinon = require('sinon') assert = require('assert') path = require('path') modulePath = path.join __dirname, '../../../../../app/js/infrastructure/LockManager.js' -project_id = 1234 -doc_id = 5678 +lockKey = "lock:web:{#{5678}}" SandboxedModule = require('sandboxed-module') describe 'LockManager - releasing the lock', ()-> @@ -20,7 +19,7 @@ describe 'LockManager - releasing the lock', ()-> LockManager = SandboxedModule.require(modulePath, requires: mocks) it 'should put a all data into memory', (done)-> - LockManager.releaseLock doc_id, -> - deleteStub.calledWith("lock:web:{#{doc_id}}").should.equal true + LockManager._releaseLock lockKey, -> + deleteStub.calledWith(lockKey).should.equal true done() diff --git a/services/web/test/unit/coffee/infrastructure/LockManager/getLockTests.coffee b/services/web/test/unit/coffee/infrastructure/LockManager/getLockTests.coffee index 16ae0a8b8c..754d1f2a85 100644 --- a/services/web/test/unit/coffee/infrastructure/LockManager/getLockTests.coffee +++ b/services/web/test/unit/coffee/infrastructure/LockManager/getLockTests.coffee @@ -20,18 +20,18 @@ describe 'LockManager - getting the lock', -> describe "when the lock is not set", -> beforeEach (done) -> - @LockManager.tryLock = sinon.stub().callsArgWith(1, null, true) - @LockManager.getLock @doc_id, (args...) => + @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 + @LockManager._tryLock .calledWith(@doc_id) .should.equal true it "should only need to try once", -> - @LockManager.tryLock.callCount.should.equal 1 + @LockManager._tryLock.callCount.should.equal 1 it "should return the callback", -> @callback.calledWith(null).should.equal true @@ -41,20 +41,20 @@ describe 'LockManager - getting the lock', -> startTime = Date.now() tries = 0 @LockManager.LOCK_TEST_INTERVAL = 5 - @LockManager.tryLock = (doc_id, callback = (error, isFree) ->) -> + @LockManager._tryLock = (doc_id, callback = (error, isFree) ->) -> if (Date.now() - startTime < 20) or (tries < 2) tries = tries + 1 callback null, false else callback null, true - sinon.spy @LockManager, "tryLock" + sinon.spy @LockManager, "_tryLock" - @LockManager.getLock @doc_id, (args...) => + @LockManager._getLock @doc_id, (args...) => @callback(args...) done() it "should call tryLock multiple times until free", -> - (@LockManager.tryLock.callCount > 1).should.equal true + (@LockManager._tryLock.callCount > 1).should.equal true it "should return the callback", -> @callback.calledWith(null).should.equal true @@ -63,8 +63,8 @@ describe 'LockManager - getting the lock', -> beforeEach (done) -> time = Date.now() @LockManager.MAX_LOCK_WAIT_TIME = 5 - @LockManager.tryLock = sinon.stub().callsArgWith(1, null, false) - @LockManager.getLock @doc_id, (args...) => + @LockManager._tryLock = sinon.stub().callsArgWith(1, null, false) + @LockManager._getLock @doc_id, (args...) => @callback(args...) done() diff --git a/services/web/test/unit/coffee/infrastructure/LockManager/tryLockTests.coffee b/services/web/test/unit/coffee/infrastructure/LockManager/tryLockTests.coffee index 5397e18cb2..3ea837df92 100644 --- a/services/web/test/unit/coffee/infrastructure/LockManager/tryLockTests.coffee +++ b/services/web/test/unit/coffee/infrastructure/LockManager/tryLockTests.coffee @@ -14,17 +14,18 @@ describe 'LockManager - trying the lock', -> auth:-> set: @set = sinon.stub() "settings-sharelatex":{redis:{}} - "metrics-sharelatex": inc:-> + "metrics-sharelatex": inc:-> @callback = sinon.stub() @doc_id = "doc-id-123" - + @key = "lock:web:{#{@doc_id}}" + describe "when the lock is not set", -> beforeEach -> @set.callsArgWith(5, null, "OK") - @LockManager.tryLock @doc_id, @callback + @LockManager._tryLock @key, @callback it "should set the lock key with an expiry if it is not set", -> - @set.calledWith("lock:web:{#{@doc_id}}", "locked", "EX", 30, "NX") + @set.calledWith(@key, "locked", "EX", 30, "NX") .should.equal true it "should return the callback with true", -> @@ -33,7 +34,7 @@ describe 'LockManager - trying the lock', -> describe "when the lock is already set", -> beforeEach -> @set.callsArgWith(5, null, null) - @LockManager.tryLock @doc_id, @callback + @LockManager._tryLock @key, @callback it "should return the callback with false", -> @callback.calledWith(null, false).should.equal true