Merge pull request #31 from sharelatex/bg-error-check-on-unlock

add error check on unlock

services/document-updater/app/coffee/LockManager.coffee

@@ -41,7 +41,7 @@ module.exports = LockManager =
 				logger.log {doc_id}, "doc is locked"
 				callback err, false
 
-	getLock: (doc_id, callback = (error) ->) ->
+	getLock: (doc_id, callback = (error, lockValue) ->) ->
 		startTime = Date.now()
 		do attempt = () ->
 			if Date.now() - startTime > LockManager.MAX_LOCK_WAIT_TIME
@@ -70,6 +70,10 @@ module.exports = LockManager =
 
 	releaseLock: (doc_id, lockValue, callback)->
 		key = keys.blockingKey(doc_id:doc_id)
-		rclient.eval LockManager.unlockScript, 1, key, lockValue, callback
-
-	
+		rclient.eval LockManager.unlockScript, 1, key, lockValue, (err, result) ->
+			if err?
+				return callback(err)
+			if result? and result isnt 1 # successful unlock should release exactly one key
+				logger.error {doc_id:doc_id, lockValue:lockValue, redis_err:err, redis_result:result}, "unlocking error"
+				return callback(new Error("tried to release timed out lock"))
+			callback(err,result)

services/document-updater/test/unit/coffee/LockManager/ReleasingTheLock.coffee

@@ -8,21 +8,36 @@ doc_id     = 5678
 SandboxedModule = require('sandboxed-module')
 
 describe 'LockManager - releasing the lock', ()->
+	beforeEach ->
+		@client = {
+			auth: ->
+			eval: sinon.stub()
+		}
+		mocks =
+			"logger-sharelatex":
+				log:->
+				error:->
+			"redis-sharelatex":
+				createClient : () => @client
+			"./Metrics": {inc: () ->}
+		@LockManager = SandboxedModule.require(modulePath, requires: mocks)
+		@lockValue = "lock-value-stub"
 
-	evalStub = sinon.stub().yields(1)
-	mocks =
-		"logger-sharelatex": log:->
-		"redis-sharelatex":
-			createClient : ()->
-				auth:->
-				eval: evalStub
-		"./Metrics": {inc: () ->}
-	
-	LockManager = SandboxedModule.require(modulePath, requires: mocks)
+	describe "when the lock is current", ->
+		beforeEach ->
+			@client.eval = sinon.stub().yields(null, 1)
+			@LockManager.releaseLock doc_id, @lockValue, @callback
 
-	it 'should put a all data into memory', (done)->
-		lockValue = "lock-value-stub"
-		LockManager.releaseLock doc_id, lockValue, ->
-			evalStub.calledWith(LockManager.unlockScript, 1, "Blocking:#{doc_id}", lockValue).should.equal true
-			done()
+		it 'should clear the data from redis', ->
+			@client.eval.calledWith(@LockManager.unlockScript, 1, "Blocking:#{doc_id}", @lockValue).should.equal true
 
+		it 'should call the callback', ->
+			@callback.called.should.equal true
+
+	describe "when the lock has expired", ->
+		beforeEach ->
+			@client.eval = sinon.stub().yields(null, 0)
+			@LockManager.releaseLock doc_id, @lockValue, @callback
+
+		it 'should return an error if the lock has expired', ->
+			@callback.calledWith(new Error("tried to release timed out lock")).should.equal true