Skip to content

Commit

Permalink
Merge pull request #227 from zendesk/jden/fix_presence_disposing
Browse files Browse the repository at this point in the history
Don't delete presence_store when disposing presence resource
  • Loading branch information
junosuarez committed Feb 23, 2016
2 parents 0a7c554 + ebbec34 commit e725bf4
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 3 deletions.
1 change: 1 addition & 0 deletions src/core/resources/presence/index.js
Expand Up @@ -234,6 +234,7 @@ Presence.prototype.fullRead = function (callback) {

Presence.prototype.destroy = function () {
this.manager.destroy()
Resource.prototype.destroy.call(this)
}

Presence.setBackend = function (backend) {
Expand Down
2 changes: 0 additions & 2 deletions src/core/resources/presence/presence_manager.js
Expand Up @@ -61,8 +61,6 @@ PresenceManager.prototype.destroy = function () {
if (this.handleRedisReply) {
this.handleRedisReply = function () {}
}

delete this.store
}
// FIXME: Method signature is getting unmanageable
PresenceManager.prototype.addClient = function (clientSessionId, userId, userType,
Expand Down
4 changes: 3 additions & 1 deletion src/core/resources/resource.js
Expand Up @@ -135,7 +135,9 @@ Resource.prototype.handleMessage = function (clientSession, message) {
}
}

Resource.prototype.destroy = function () {}
Resource.prototype.destroy = function () {
this.destroyed = true
}

Resource.setBackend = function (backend) {
// noop
Expand Down
4 changes: 4 additions & 0 deletions src/server/server.js
Expand Up @@ -197,6 +197,10 @@ Server.prototype._onSentryDown = function (sentryId) {
var started = Date.now()

nonblocking(presences).forEach(function (presence) {
if (presence.destroyed) {
return
}

var clientSessionIds = presence.manager.store.clientSessionIdsForSentryId(sentryId)
expected += clientSessionIds.length

Expand Down
9 changes: 9 additions & 0 deletions test/presence.unit.test.js
Expand Up @@ -5,6 +5,7 @@ var Persistence = require('persistence')
var Common = require('./common.js')
var Presence = require('../src/core/resources/presence')
var sinon = require('sinon')
var expect = require('chai').expect

describe('given a presence resource', function () {
var presence, client, client2
Expand Down Expand Up @@ -535,6 +536,14 @@ describe('given a presence resource', function () {
presence.get(fakeClient, { options: { version: 2 } })
})
})

describe('#destroy()', function () {
it('sets resource.destroyed flag to true', function () {
expect(!presence.destroyed).to.equal(true)
presence.destroy()
expect(presence.destroyed).to.equal(true)
})
})
})

describe('a presence resource', function () {
Expand Down
10 changes: 10 additions & 0 deletions test/server.unit.test.js
Expand Up @@ -286,6 +286,16 @@ describe('given a server', function () {
})
})

it('ignores presences that are destroyed during processing', function (done) {
radarServer._onSentryDown('sentry1')
radarServer.resources[234].destroyed = true
radarServer.on('profiling', function () {
expect(radarServer.resources[123].manager.disconnectRemoteClient).to.have.been.calledTwice
expect(radarServer.resources[234].manager.disconnectRemoteClient).not.to.have.been.called
done()
})
})

it('emits profiling event', function (done) {
radarServer._onSentryDown('sentry1')
radarServer.on('profiling', function (e) {
Expand Down

0 comments on commit e725bf4

Please sign in to comment.