diff --git a/services/real-time/app/coffee/RoomManager.coffee b/services/real-time/app/coffee/RoomManager.coffee index adf472e26c..0d8a8aaf06 100644 --- a/services/real-time/app/coffee/RoomManager.coffee +++ b/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) diff --git a/services/real-time/test/acceptance/coffee/LeaveDocTests.coffee b/services/real-time/test/acceptance/coffee/LeaveDocTests.coffee index 753bc79c62..e4875fcbe5 100644 --- a/services/real-time/test/acceptance/coffee/LeaveDocTests.coffee +++ b/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()