瀏覽代碼

Replace multi-ops with Async.series, tests passing

Shane Kilkelly 10 年之前
父節點
當前提交
5f3098df38

+ 13 - 6
services/web/app/coffee/Features/User/UserSessionsManager.coffee

@@ -68,7 +68,8 @@ module.exports = UserSessionsManager =
 			if sessionKeys.length == 0
 			if sessionKeys.length == 0
 				logger.log {user_id: user._id}, "no other sessions found, returning"
 				logger.log {user_id: user._id}, "no other sessions found, returning"
 				return callback(null, [])
 				return callback(null, [])
-			rclient.mget sessionKeys, (err, sessions) ->
+
+			Async.map sessionKeys, ((k, cb) -> rclient.get(k, cb)), (err, sessions) ->
 				if err?
 				if err?
 					logger.err {user_id: user._id}, "error getting all sessions for user from redis"
 					logger.err {user_id: user._id}, "error getting all sessions for user from redis"
 					return callback(err)
 					return callback(err)
@@ -104,12 +105,18 @@ module.exports = UserSessionsManager =
 				logger.log {user_id: user._id}, "no sessions in UserSessions set to delete, returning"
 				logger.log {user_id: user._id}, "no sessions in UserSessions set to delete, returning"
 				return callback(null)
 				return callback(null)
 			logger.log {user_id: user._id, count: keysToDelete.length}, "deleting sessions for user"
 			logger.log {user_id: user._id, count: keysToDelete.length}, "deleting sessions for user"
-			rclient.multi()
-				.del(keysToDelete)
-				.srem(sessionSetKey, keysToDelete)
-				.exec (err, result) ->
+
+			deletions = keysToDelete.map (k) ->
+				(cb) ->
+					rclient.del k, cb
+
+			Async.series deletions, (err, _result) ->
+				if err?
+					logger.err {err, user_id: user._id, sessionSetKey}, "errror revoking all sessions for user"
+					return callback(err)
+				rclient.srem sessionSetKey, keysToDelete, (err) ->
 					if err?
 					if err?
-						logger.err {err, user_id: user._id, sessionSetKey}, "error revoking all sessions for user"
+						logger.err {err, user_id: user._id, sessionSetKey}, "error removing session set for user"
 						return callback(err)
 						return callback(err)
 					callback(null)
 					callback(null)
 
 

+ 26 - 21
services/web/test/UnitTests/coffee/User/UserSessionsManagerTests.coffee

@@ -4,6 +4,7 @@ should = chai.should()
 expect = chai.expect
 expect = chai.expect
 modulePath = "../../../../app/js/Features/User/UserSessionsManager.js"
 modulePath = "../../../../app/js/Features/User/UserSessionsManager.js"
 SandboxedModule = require('sandboxed-module')
 SandboxedModule = require('sandboxed-module')
+Async = require('async')
 
 
 describe 'UserSessionsManager', ->
 describe 'UserSessionsManager', ->
 
 
@@ -45,6 +46,7 @@ describe 'UserSessionsManager', ->
 			"logger-sharelatex":   @logger
 			"logger-sharelatex":   @logger
 			"settings-sharelatex": @settings
 			"settings-sharelatex": @settings
 			'./UserSessionsRedis': @redis
 			'./UserSessionsRedis': @redis
+			'async': Async
 
 
 	describe '_sessionSetKey', ->
 	describe '_sessionSetKey', ->
 
 
@@ -247,14 +249,14 @@ describe 'UserSessionsManager', ->
 					@_checkSessions.callCount.should.equal 0
 					@_checkSessions.callCount.should.equal 0
 					done()
 					done()
 
 
-	##
 	describe 'revokeAllUserSessions', ->
 	describe 'revokeAllUserSessions', ->
 
 
 		beforeEach ->
 		beforeEach ->
 			@sessionKeys = ['sess:one', 'sess:two']
 			@sessionKeys = ['sess:one', 'sess:two']
 			@retain = []
 			@retain = []
 			@rclient.smembers.callsArgWith(1, null, @sessionKeys)
 			@rclient.smembers.callsArgWith(1, null, @sessionKeys)
-			@rclient.exec.callsArgWith(0, null)
+			@rclient.del = sinon.stub().callsArgWith(1, null)
+			@rclient.srem = sinon.stub().callsArgWith(2, null)
 			@call = (callback) =>
 			@call = (callback) =>
 				@UserSessionsManager.revokeAllUserSessions @user, @retain, callback
 				@UserSessionsManager.revokeAllUserSessions @user, @retain, callback
 
 
@@ -267,15 +269,14 @@ describe 'UserSessionsManager', ->
 		it 'should call the appropriate redis methods', (done) ->
 		it 'should call the appropriate redis methods', (done) ->
 			@call (err) =>
 			@call (err) =>
 				@rclient.smembers.callCount.should.equal 1
 				@rclient.smembers.callCount.should.equal 1
-				@rclient.multi.callCount.should.equal 1
 
 
-				@rclient.del.callCount.should.equal 1
-				expect(@rclient.del.firstCall.args[0]).to.deep.equal @sessionKeys
+				@rclient.del.callCount.should.equal 2
+				expect(@rclient.del.firstCall.args[0]).to.deep.equal @sessionKeys[0]
+				expect(@rclient.del.secondCall.args[0]).to.deep.equal @sessionKeys[1]
 
 
 				@rclient.srem.callCount.should.equal 1
 				@rclient.srem.callCount.should.equal 1
 				expect(@rclient.srem.firstCall.args[1]).to.deep.equal @sessionKeys
 				expect(@rclient.srem.firstCall.args[1]).to.deep.equal @sessionKeys
 
 
-				@rclient.exec.callCount.should.equal 1
 				done()
 				done()
 
 
 		describe 'when a session is retained', ->
 		describe 'when a session is retained', ->
@@ -284,7 +285,7 @@ describe 'UserSessionsManager', ->
 				@sessionKeys = ['sess:one', 'sess:two', 'sess:three', 'sess:four']
 				@sessionKeys = ['sess:one', 'sess:two', 'sess:three', 'sess:four']
 				@retain = ['two']
 				@retain = ['two']
 				@rclient.smembers.callsArgWith(1, null, @sessionKeys)
 				@rclient.smembers.callsArgWith(1, null, @sessionKeys)
-				@rclient.exec.callsArgWith(0, null)
+				@rclient.del = sinon.stub().callsArgWith(1, null)
 				@call = (callback) =>
 				@call = (callback) =>
 					@UserSessionsManager.revokeAllUserSessions @user, @retain, callback
 					@UserSessionsManager.revokeAllUserSessions @user, @retain, callback
 
 
@@ -297,28 +298,33 @@ describe 'UserSessionsManager', ->
 			it 'should call the appropriate redis methods', (done) ->
 			it 'should call the appropriate redis methods', (done) ->
 				@call (err) =>
 				@call (err) =>
 					@rclient.smembers.callCount.should.equal 1
 					@rclient.smembers.callCount.should.equal 1
-					@rclient.multi.callCount.should.equal 1
-					@rclient.del.callCount.should.equal 1
+					@rclient.del.callCount.should.equal @sessionKeys.length - 1
 					@rclient.srem.callCount.should.equal 1
 					@rclient.srem.callCount.should.equal 1
-					@rclient.exec.callCount.should.equal 1
 					done()
 					done()
 
 
 			it 'should remove all sessions except for the retained one', (done) ->
 			it 'should remove all sessions except for the retained one', (done) ->
 				@call (err) =>
 				@call (err) =>
-					expect(@rclient.del.firstCall.args[0]).to.deep.equal(['sess:one', 'sess:three', 'sess:four'])
+					expect(@rclient.del.firstCall.args[0]).to.deep.equal('sess:one')
+					expect(@rclient.del.secondCall.args[0]).to.deep.equal('sess:three')
+					expect(@rclient.del.thirdCall.args[0]).to.deep.equal('sess:four')
 					expect(@rclient.srem.firstCall.args[1]).to.deep.equal(['sess:one', 'sess:three', 'sess:four'])
 					expect(@rclient.srem.firstCall.args[1]).to.deep.equal(['sess:one', 'sess:three', 'sess:four'])
 					done()
 					done()
 
 
 		describe 'when rclient produces an error', ->
 		describe 'when rclient produces an error', ->
 
 
 			beforeEach ->
 			beforeEach ->
-				@rclient.exec.callsArgWith(0, new Error('woops'))
+				@rclient.del = sinon.stub().callsArgWith(1, new Error('woops'))
 
 
 			it 'should produce an error', (done) ->
 			it 'should produce an error', (done) ->
 				@call (err) =>
 				@call (err) =>
 					expect(err).to.be.instanceof Error
 					expect(err).to.be.instanceof Error
 					done()
 					done()
 
 
+			it 'should not call rclient.srem', (done) ->
+				@call (err) =>
+					@rclient.srem.callCount.should.equal 0
+					done()
+
 		describe 'when no user is supplied', ->
 		describe 'when no user is supplied', ->
 
 
 			beforeEach ->
 			beforeEach ->
@@ -334,10 +340,8 @@ describe 'UserSessionsManager', ->
 			it 'should not call the appropriate redis methods', (done) ->
 			it 'should not call the appropriate redis methods', (done) ->
 				@call (err) =>
 				@call (err) =>
 					@rclient.smembers.callCount.should.equal 0
 					@rclient.smembers.callCount.should.equal 0
-					@rclient.multi.callCount.should.equal 0
 					@rclient.del.callCount.should.equal 0
 					@rclient.del.callCount.should.equal 0
 					@rclient.srem.callCount.should.equal 0
 					@rclient.srem.callCount.should.equal 0
-					@rclient.exec.callCount.should.equal 0
 					done()
 					done()
 
 
 		describe 'when there are no keys to delete', ->
 		describe 'when there are no keys to delete', ->
@@ -354,10 +358,8 @@ describe 'UserSessionsManager', ->
 			it 'should not do the delete operation', (done) ->
 			it 'should not do the delete operation', (done) ->
 				@call (err) =>
 				@call (err) =>
 					@rclient.smembers.callCount.should.equal 1
 					@rclient.smembers.callCount.should.equal 1
-					@rclient.multi.callCount.should.equal 0
 					@rclient.del.callCount.should.equal 0
 					@rclient.del.callCount.should.equal 0
 					@rclient.srem.callCount.should.equal 0
 					@rclient.srem.callCount.should.equal 0
-					@rclient.exec.callCount.should.equal 0
 					done()
 					done()
 
 
 	describe 'touch', ->
 	describe 'touch', ->
@@ -415,7 +417,10 @@ describe 'UserSessionsManager', ->
 			]
 			]
 			@exclude = ['two']
 			@exclude = ['two']
 			@rclient.smembers.callsArgWith(1, null, @sessionKeys)
 			@rclient.smembers.callsArgWith(1, null, @sessionKeys)
-			@rclient.mget.callsArgWith(1, null, @sessions)
+			@rclient.get = sinon.stub()
+			@rclient.get.onCall(0).callsArgWith(1, null, @sessions[0])
+			@rclient.get.onCall(1).callsArgWith(1, null, @sessions[1])
+
 			@call = (callback) =>
 			@call = (callback) =>
 				@UserSessionsManager.getAllUserSessions @user, @exclude, callback
 				@UserSessionsManager.getAllUserSessions @user, @exclude, callback
 
 
@@ -437,9 +442,9 @@ describe 'UserSessionsManager', ->
 				@rclient.smembers.callCount.should.equal 1
 				@rclient.smembers.callCount.should.equal 1
 				done()
 				done()
 
 
-		it 'should have called rclient.mget', (done) ->
+		it 'should have called rclient.get', (done) ->
 			@call (err, sessions) =>
 			@call (err, sessions) =>
-				@rclient.mget.callCount.should.equal 1
+				@rclient.get.callCount.should.equal @sessionKeys.length - 1
 				done()
 				done()
 
 
 		describe 'when there are no other sessions', ->
 		describe 'when there are no other sessions', ->
@@ -484,10 +489,10 @@ describe 'UserSessionsManager', ->
 					@rclient.mget.callCount.should.equal 0
 					@rclient.mget.callCount.should.equal 0
 					done()
 					done()
 
 
-		describe 'when mget produces an error', ->
+		describe 'when get produces an error', ->
 
 
 			beforeEach ->
 			beforeEach ->
-				@rclient.mget.callsArgWith(1, new Error('woops'))
+				@rclient.get = sinon.stub().callsArgWith(1, new Error('woops'))
 
 
 			it 'should produce an error', (done) ->
 			it 'should produce an error', (done) ->
 				@call (err, sessions) =>
 				@call (err, sessions) =>

+ 17 - 10
services/web/test/acceptance/coffee/helpers/redis.coffee

@@ -3,25 +3,32 @@ redis = require('redis-sharelatex')
 logger = require("logger-sharelatex")
 logger = require("logger-sharelatex")
 Async = require('async')
 Async = require('async')
 
 
-rclient = redis.createClient(Settings.redis.web)
+UserSessionsRedis = require('../../../../app/js/Features/User/UserSessionsRedis')
+
+# rclient = redis.createClient(Settings.redis.web)
+rclient = UserSessionsRedis.client()
 
 
 module.exports =
 module.exports =
 
 
 	getUserSessions: (user, callback=(err, sessionsSet)->) ->
 	getUserSessions: (user, callback=(err, sessionsSet)->) ->
-		rclient.smembers "UserSessions:#{user._id}", (err, result) ->
+		rclient.smembers "UserSessions:{#{user._id}}", (err, result) ->
 			return callback(err, result)
 			return callback(err, result)
 
 
 	clearUserSessions: (user, callback=(err)->) ->
 	clearUserSessions: (user, callback=(err)->) ->
-		sessionSetKey = "UserSessions:#{user._id}"
+		sessionSetKey = "UserSessions:{#{user._id}}"
 		rclient.smembers sessionSetKey, (err, sessionKeys) ->
 		rclient.smembers sessionSetKey, (err, sessionKeys) ->
 			if err
 			if err
 				return callback(err)
 				return callback(err)
 			if sessionKeys.length == 0
 			if sessionKeys.length == 0
 				return callback(null)
 				return callback(null)
-			rclient.multi()
-				.del(sessionKeys)
-				.srem(sessionSetKey, sessionKeys)
-				.exec (err, result) ->
-					if err
-						return callback(err)
-					callback(null)
+			actions = sessionKeys.map (k) ->
+				(cb) ->
+					rclient.del k, (err) ->
+						cb(err)
+			Async.series(
+				actions, (err, results) ->
+					rclient.srem sessionSetKey, sessionKeys, (err) ->
+						if err
+							return callback(err)
+						callback(null)
+			)