Merge pull request #67 from overleaf/bg-fix-join-leave

fix async behaviour of join/leave

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

@@ -46,13 +46,15 @@ module.exports = RoomManager =
 
     joinEntity: (client, entity, id, callback) ->
         beforeCount = @_clientsInRoom(client, id)
+        # client joins room immediately but joinDoc request does not complete
+        # until room is subscribed
+        client.join id
         # is this a new room? if so, subscribe
         if beforeCount == 0
             logger.log {entity, id}, "room is now active"
             RoomEvents.once "#{entity}-subscribed-#{id}", (err) ->
                 # only allow the client to join when all the relevant channels have subscribed
-                logger.log {client: client.id, entity, id, beforeCount}, "client joined room after subscribing channel"
-                client.join id
+                logger.log {client: client.id, entity, id, beforeCount}, "client joined new room and subscribed to channel"
                 callback(err)
             RoomEvents.emit "#{entity}-active", id
             IdMap.set(id, entity)

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

@@ -1,10 +1,12 @@
 chai = require("chai")
 expect = chai.expect
 chai.should()
+sinon = require("sinon")
 
 RealTimeClient = require "./helpers/RealTimeClient"
 MockDocUpdaterServer = require "./helpers/MockDocUpdaterServer"
 FixturesManager = require "./helpers/FixturesManager"
+logger = require("logger-sharelatex")
 
 async = require "async"
 
@@ -13,9 +15,13 @@ describe "leaveDoc", ->
 		@lines = ["test", "doc", "lines"]
 		@version = 42
 		@ops = ["mock", "doc", "ops"]
+		sinon.spy(logger, "error")
+	
+	after ->
+		logger.error.restore() # remove the spy
 			
 	describe "when joined to a doc", ->
-		before (done) ->
+		beforeEach (done) ->
 			async.series [
 				(cb) =>
 					FixturesManager.setUpProject {
@@ -39,7 +45,7 @@ describe "leaveDoc", ->
 			], done
 							
 		describe "then leaving the doc", ->
-			before (done) ->
+			beforeEach (done) ->
 				@client.emit "leaveDoc", @doc_id, (error) ->
 					throw error if error?
 					done()
@@ -48,3 +54,19 @@ describe "leaveDoc", ->
 				RealTimeClient.getConnectedClient @client.socket.sessionid, (error, client) =>
 					expect(@doc_id in client.rooms).to.equal false
 					done()
+
+		describe "when sending a leaveDoc request before the previous joinDoc request has completed", ->
+			beforeEach (done) ->
+				@client.emit "leaveDoc", @doc_id, () ->
+				@client.emit "joinDoc", @doc_id, () ->
+				@client.emit "leaveDoc", @doc_id, (error) ->
+					throw error if error?
+					done()
+
+			it "should not trigger an error", ->
+				sinon.assert.neverCalledWith(logger.error, sinon.match.any, "not subscribed - shouldn't happen")
+
+			it "should have left the doc room", (done) ->
+				RealTimeClient.getConnectedClient @client.socket.sessionid, (error, client) =>
+					expect(@doc_id in client.rooms).to.equal false
+					done()