Mark user as connected for cursor updates when joining project

services/real-time/app/coffee/ConnectedUsersManager.coffee

@@ -21,7 +21,7 @@ module.exports =
 	# update. This way we don't care if the connected_user key has expired when
 	# we receive a cursor update. 
 	updateUserPosition: (project_id, client_id, user, cursorData, callback = (err)->)->
-		logger.log project_id:project_id, client_id:client_id, "marking user as connected"
+		logger.log project_id:project_id, client_id:client_id, "marking user as joined or connected"
 
 		multi = rclient.multi()
 		
@@ -30,9 +30,9 @@ module.exports =
 		
 		multi.hset buildUserKey(project_id, client_id), "last_updated_at", Date.now()
 		multi.hset buildUserKey(project_id, client_id), "user_id", user._id
-		multi.hset buildUserKey(project_id, client_id), "first_name", user.first_name
-		multi.hset buildUserKey(project_id, client_id), "last_name", user.last_name
-		multi.hset buildUserKey(project_id, client_id), "email", user.email
+		multi.hset buildUserKey(project_id, client_id), "first_name", user.first_name or ""
+		multi.hset buildUserKey(project_id, client_id), "last_name", user.last_name or ""
+		multi.hset buildUserKey(project_id, client_id), "email", user.email or ""
 		
 		if cursorData?
 			multi.hset buildUserKey(project_id, client_id), "cursorData", JSON.stringify(cursorData)
@@ -61,18 +61,25 @@ module.exports =
 			else
 				result.connected = true
 				result.client_id = client_id
+				console.log "RESULT", result
 				if result.cursorData?
-					result.cursorData = JSON.parse(result.cursorData)
+					try
+						result.cursorData = JSON.parse(result.cursorData)
+					catch e
+						logger.error {err: e, project_id, client_id, cursorData: result.cursorData}, "error parsing cursorData JSON" 
+						return callback e
 			callback err, result
 
 	getConnectedUsers: (project_id, callback)->
 		self = @
 		rclient.smembers buildProjectSetKey(project_id), (err, results)->
+			return callback(err) if err?
 			jobs = results.map (client_id)->
 				(cb)->
 					self._getConnectedUser(project_id, client_id, cb)
-			async.series jobs, (err, users)->
+			async.series jobs, (err, users = [])->
+				return callback(err) if err?
 				users = users.filter (user) ->
-					user.connected
-				callback err, users
+					user?.connected
+				callback null, users
 

services/real-time/app/coffee/Router.coffee

@@ -60,4 +60,14 @@ module.exports = Router =
 						return callback {message: "Something went wrong"}
 					else
 						callback(null, args...)
+						
+			client.on "getConnectedUsers", (callback = (error, users) ->) ->
+				WebsocketController.getConnectedUsers client, (err, users) ->
+					if err?
+						Utils.getClientAttributes client, ["project_id", "user_id", "doc_id"], (_, {project_id, user_id, doc_id}) ->
+							logger.error {err, client_id: client.id, user_id, project_id, doc_id}, "server side error in getConnectedUsers"
+						# Don't return raw error to prevent leaking server side info
+						return callback {message: "Something went wrong"}
+					else
+						callback(null, users)
 		

services/real-time/app/coffee/WebsocketController.coffee

@@ -2,6 +2,7 @@ logger = require "logger-sharelatex"
 WebApiManager = require "./WebApiManager"
 AuthorizationManager = require "./AuthorizationManager"
 DocumentUpdaterManager = require "./DocumentUpdaterManager"
+ConnectedUsersManager = require "./ConnectedUsersManager"
 Utils = require "./Utils"
 
 module.exports = WebsocketController =
@@ -36,6 +37,9 @@ module.exports = WebsocketController =
 			
 			callback null, project, privilegeLevel, WebsocketController.PROTOCOL_VERSION
 			
+			# No need to block for setting the user as connected in the cursor tracking
+			ConnectedUsersManager.updateUserPosition project_id, client.id, user, null, () ->
+			
 	joinDoc: (client, doc_id, fromVersion = -1, callback = (error, doclines, version, ops) ->) ->
 		Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) ->
 			logger.log {user_id, project_id, doc_id, fromVersion, client_id: client.id}, "client joining doc"
@@ -65,3 +69,13 @@ module.exports = WebsocketController =
 			logger.log {user_id, project_id, doc_id, client_id: client.id}, "client leaving doc"
 		client.leave doc_id
 		callback()
+		
+	getConnectedUsers: (client, callback = (error, users) ->) ->
+		AuthorizationManager.assertClientCanViewProject client, (error) ->
+			return callback(error) if error?
+			client.get "project_id", (error, project_id) ->
+				return callback(error) if error?
+				return callback(new Error("no project_id found on client")) if !project_id?
+				ConnectedUsersManager.getConnectedUsers project_id, (error, users) ->
+					return callback(error) if error?
+					callback null, users

services/real-time/test/acceptance/coffee/JoinProjectTests.coffee

@@ -45,6 +45,16 @@ describe "joinProject", ->
 			RealTimeClient.getConnectedClient @client.socket.sessionid, (error, client) =>
 				expect(@project_id in client.rooms).to.equal true
 				done()
+				
+		it "should have marked the user as connected", (done) ->
+			@client.emit "getConnectedUsers", (error, users) =>
+				connected = false
+				for user in users
+					if user.client_id == @client.socket.sessionid and user.user_id == @user_id
+						connected = true
+						break
+				expect(connected).to.equal true
+				done()
 			
 	describe "when not authorized", ->
 		before (done) ->

services/real-time/test/unit/coffee/WebsocketControllerTests.coffee

@@ -28,6 +28,7 @@ describe 'WebsocketController', ->
 			"./WebApiManager": @WebApiManager = {}
 			"./AuthorizationManager": @AuthorizationManager = {}
 			"./DocumentUpdaterManager": @DocumentUpdaterManager = {}
+			"./ConnectedUsersManager": @ConnectedUsersManager = {}
 			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
 	
 	afterEach ->
@@ -36,6 +37,7 @@ describe 'WebsocketController', ->
 	describe "joinProject", ->
 		describe "when authorised", ->
 			beforeEach ->
+				@client.id = "mock-client-id"
 				@project = {
 					name: "Test Project"
 					owner: {
@@ -43,6 +45,7 @@ describe 'WebsocketController', ->
 					}
 				}
 				@privilegeLevel = "owner"
+				@ConnectedUsersManager.updateUserPosition = sinon.stub().callsArg(4)
 				@WebApiManager.joinProject = sinon.stub().callsArgWith(2, null, @project, @privilegeLevel)
 				@WebsocketController.joinProject @client, @user, @project_id, @callback
 				
@@ -88,6 +91,11 @@ describe 'WebsocketController', ->
 				@callback
 					.calledWith(null, @project, @privilegeLevel, @WebsocketController.PROTOCOL_VERSION)
 					.should.equal true
+					
+			it "should mark the user as connected in ConnectedUsersManager", ->
+				@ConnectedUsersManager.updateUserPosition
+					.calledWith(@project_id, @client.id, @user, null)
+					.should.equal true
 				
 		describe "when not authorized", ->
 			beforeEach ->
@@ -170,4 +178,43 @@ describe 'WebsocketController', ->
 				.calledWith(@doc_id).should.equal true
 				
 		it "should call the callback", ->
-			@callback.called.should.equal true
+			@callback.called.should.equal true
+			
+	describe "getConnectedUsers", ->
+		beforeEach ->
+			@client.params.project_id = @project_id
+			@users = ["mock", "users"]
+			@ConnectedUsersManager.getConnectedUsers = sinon.stub().callsArgWith(1, null, @users)
+			
+		describe "when authorized", ->
+			beforeEach ->
+				@AuthorizationManager.assertClientCanViewProject = sinon.stub().callsArgWith(1, null)
+				@WebsocketController.getConnectedUsers @client, @callback
+				
+			it "should check that the client is authorized to view the project", ->
+				@AuthorizationManager.assertClientCanViewProject
+					.calledWith(@client)
+					.should.equal true
+					
+			it "should get the connected users for the project", ->
+				@ConnectedUsersManager.getConnectedUsers
+					.calledWith(@project_id)
+					.should.equal true
+					
+			it "should return the users", ->
+				@callback.calledWith(null, @users).should.equal true
+			
+		describe "when not authorized", ->
+			beforeEach ->
+				@AuthorizationManager.assertClientCanViewProject = sinon.stub().callsArgWith(1, @err = new Error("not authorized"))
+				@WebsocketController.getConnectedUsers @client, @callback
+					
+			it "should not get the connected users for the project", ->
+				@ConnectedUsersManager.getConnectedUsers
+					.called
+					.should.equal false
+					
+			it "should return an error", ->
+				@callback.calledWith(@err).should.equal true
+				
+