ignore spurious requests to leave other docs

This commit is contained in:
Brian Gough 2019-07-29 15:19:08 +01:00
parent 1926f97dbf
commit 478a727c61
3 changed files with 66 additions and 2 deletions

View file

@ -32,7 +32,9 @@ module.exports = RoomManager =
# channel subscriptions... but it will be safer if we leave them
# explicitly, and then socket.io will just regard this as a client that
# has not joined any rooms and do a final disconnection.
for id in @_roomsClientIsIn(client)
roomsToLeave = @_roomsClientIsIn(client)
logger.log {client: client.id, roomsToLeave: roomsToLeave}, "client leaving project"
for id in roomsToLeave
entity = IdMap.get(id)
@leaveEntity client, entity, id
@ -66,6 +68,12 @@ module.exports = RoomManager =
callback()
leaveEntity: (client, entity, id) ->
# Ignore any requests to leave when the client is not actually in the
# room. This can happen if the client sends spurious leaveDoc requests
# for old docs after a reconnection.
if !@_clientAlreadyInRoom(client, id)
logger.warn {client: client.id, entity, id}, "ignoring request from client to leave room it is not in"
return
client.leave id
afterCount = @_clientsInRoom(client, id)
logger.log {client: client.id, entity, id, afterCount}, "client left room"
@ -93,3 +101,8 @@ module.exports = RoomManager =
[prefix, room] = fullRoomPath.split('/', 2)
room
return roomList
_clientAlreadyInRoom: (client, room) ->
nsp = client.namespace.name
name = (nsp + '/') + room;
return client.manager.roomClients?[client.id]?[name]

View file

@ -16,9 +16,12 @@ describe "leaveDoc", ->
@version = 42
@ops = ["mock", "doc", "ops"]
sinon.spy(logger, "error")
sinon.spy(logger, "warn")
@other_doc_id = FixturesManager.getRandomId()
after ->
logger.error.restore() # remove the spy
logger.warn.restore()
describe "when joined to a doc", ->
beforeEach (done) ->
@ -70,3 +73,12 @@ describe "leaveDoc", ->
RealTimeClient.getConnectedClient @client.socket.sessionid, (error, client) =>
expect(@doc_id in client.rooms).to.equal false
done()
describe "when sending a leaveDoc for a room the client has not joined ", ->
beforeEach (done) ->
@client.emit "leaveDoc", @other_doc_id, (error) ->
throw error if error?
done()
it "should trigger a warning only", ->
sinon.assert.calledWith(logger.warn, sinon.match.any, "ignoring request from client to leave room it is not in")

View file

@ -12,9 +12,10 @@ describe 'RoomManager', ->
@client = {namespace: {name: ''}, id: "first-client"}
@RoomManager = SandboxedModule.require modulePath, requires:
"settings-sharelatex": @settings = {}
"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
"logger-sharelatex": @logger = { log: sinon.stub(), warn: sinon.stub(), error: sinon.stub() }
"metrics-sharelatex": @metrics = { gauge: sinon.stub() }
@RoomManager._clientsInRoom = sinon.stub()
@RoomManager._clientAlreadyInRoom = sinon.stub()
@RoomEvents = @RoomManager.eventSource()
sinon.spy(@RoomEvents, 'emit')
sinon.spy(@RoomEvents, 'once')
@ -112,6 +113,9 @@ describe 'RoomManager', ->
describe "when doc room will be empty after this client has left", ->
beforeEach ->
@RoomManager._clientAlreadyInRoom
.withArgs(@client, @doc_id)
.returns(true)
@RoomManager._clientsInRoom
.withArgs(@client, @doc_id)
.onCall(0).returns(0)
@ -128,6 +132,9 @@ describe 'RoomManager', ->
describe "when there are other clients in the doc room", ->
beforeEach ->
@RoomManager._clientAlreadyInRoom
.withArgs(@client, @doc_id)
.returns(true)
@RoomManager._clientsInRoom
.withArgs(@client, @doc_id)
.onCall(0).returns(123)
@ -140,6 +147,24 @@ describe 'RoomManager', ->
it "should not emit any events", ->
@RoomEvents.emit.called.should.equal false
describe "when the client is not in the doc room", ->
beforeEach ->
@RoomManager._clientAlreadyInRoom
.withArgs(@client, @doc_id)
.returns(false)
@RoomManager._clientsInRoom
.withArgs(@client, @doc_id)
.onCall(0).returns(0)
@client.leave = sinon.stub()
@RoomManager.leaveDoc @client, @doc_id
it "should not leave the room", ->
@client.leave.called.should.equal false
it "should not emit any events", ->
@RoomEvents.emit.called.should.equal false
describe "leaveProjectAndDocs", ->
@ -167,6 +192,13 @@ describe 'RoomManager', ->
.withArgs(@client, @project_id)
.onCall(0).returns(0)
.onCall(1).returns(0)
@RoomManager._clientAlreadyInRoom
.withArgs(@client, @doc_id)
.returns(true)
.withArgs(@client, @other_doc_id)
.returns(true)
.withArgs(@client, @project_id)
.returns(true)
@RoomEvents.on 'project-active', (id) =>
setTimeout () =>
@RoomEvents.emit "project-subscribed-#{id}"
@ -212,6 +244,13 @@ describe 'RoomManager', ->
.withArgs(@client, @project_id)
.onFirstCall().returns(123)
.onSecondCall().returns(122)
@RoomManager._clientAlreadyInRoom
.withArgs(@client, @doc_id)
.returns(true)
.withArgs(@client, @other_doc_id)
.returns(true)
.withArgs(@client, @project_id)
.returns(true)
@RoomManager.leaveProjectAndDocs @client
it "should leave all the docs", ->