Merge pull request #135 from overleaf/jpa-skip-leave-project-for-invalid-clients

[WebsocketController] skip leaveProject when joinProject didn't complete

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

@@ -52,23 +52,14 @@ module.exports = WebsocketController =
 	# is determined by FLUSH_IF_EMPTY_DELAY.
 	FLUSH_IF_EMPTY_DELAY: 500 #ms
 	leaveProject: (io, client, callback = (error) ->) ->
-		metrics.inc "editor.leave-project"
 		Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) ->
 			return callback(error) if error?
+			return callback() unless project_id # client did not join project
 
+			metrics.inc "editor.leave-project"
 			logger.log {project_id, user_id, client_id: client.id}, "client leaving project"
 			WebsocketLoadBalancer.emitToRoom project_id, "clientTracking.clientDisconnected", client.id
 
-			# bail out if the client had not managed to authenticate or join
-			# the project.  Prevents downstream errors in docupdater from
-			# flushProjectToMongoAndDelete with null project_id.
-			if not user_id?
-				logger.log {client_id: client.id}, "client leaving, unknown user"
-				return callback()
-			if not project_id?
-				logger.log {user_id: user_id, client_id: client.id}, "client leaving, not in project"
-				return callback()
-
 			# We can do this in the background
 			ConnectedUsersManager.markUserAsDisconnected project_id, client.id, (err) ->
 				if err?

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

@@ -165,6 +165,19 @@ describe 'WebsocketController', ->
 			@WebsocketController.FLUSH_IF_EMPTY_DELAY = 0
 			tk.reset() # Allow setTimeout to work.
 
+		describe "when the client did not joined a project yet", ->
+			beforeEach (done) ->
+				@client.params = {}
+				@WebsocketController.leaveProject @io, @client, done
+
+			it "should bail out when calling leaveProject", () ->
+				@WebsocketLoadBalancer.emitToRoom.called.should.equal false
+				@RoomManager.leaveProjectAndDocs.called.should.equal false
+				@ConnectedUsersManager.markUserAsDisconnected.called.should.equal false
+
+			it "should not inc any metric", () ->
+				@metrics.inc.called.should.equal false
+
 		describe "when the project is empty", ->
 			beforeEach (done) ->
 				@clientsInRoom = []
@@ -223,8 +236,8 @@ describe 'WebsocketController', ->
 					.calledWith(@project_id)
 					.should.equal false
 
-			it "should increment the leave-project metric", ->
-				@metrics.inc.calledWith("editor.leave-project").should.equal true
+			it "should not increment the leave-project metric", ->
+				@metrics.inc.calledWith("editor.leave-project").should.equal false
 
 		describe "when client has not joined a project", ->
 			beforeEach (done) ->
@@ -247,8 +260,8 @@ describe 'WebsocketController', ->
 					.calledWith(@project_id)
 					.should.equal false
 
-			it "should increment the leave-project metric", ->
-				@metrics.inc.calledWith("editor.leave-project").should.equal true
+			it "should not increment the leave-project metric", ->
+				@metrics.inc.calledWith("editor.leave-project").should.equal false
 
 	describe "joinDoc", ->
 		beforeEach ->