diff --git a/lib/client/presence/presence.js b/lib/client/presence/presence.js index 9aba6b60c..3a5bf4330 100644 --- a/lib/client/presence/presence.js +++ b/lib/client/presence/presence.js @@ -28,12 +28,12 @@ function Presence(connection, channel) { } emitter.mixin(Presence); -Presence.prototype.subscribe = function(callback) { - this._sendSubscriptionAction(true, callback); +Presence.prototype.subscribe = function(options, callback) { + this._sendSubscriptionAction(true, options, callback); }; -Presence.prototype.unsubscribe = function(callback) { - this._sendSubscriptionAction(false, callback); +Presence.prototype.unsubscribe = function(options, callback) { + this._sendSubscriptionAction(false, options, callback); }; Presence.prototype.create = function(id) { @@ -83,8 +83,21 @@ Presence.prototype.destroy = function(callback) { }); }; -Presence.prototype._sendSubscriptionAction = function(wantSubscribe, callback) { - this.wantSubscribe = !!wantSubscribe; +Presence.prototype._sendSubscriptionAction = function(wantSubscribe, options, callback) { + if (typeof options === 'function') { + callback = options; + options = {}; + } + options = options || {}; + wantSubscribe = !!wantSubscribe; + if (!options.force && wantSubscribe === this.wantSubscribe) { + if (!callback) return; + if (wantSubscribe === this.subscribed) return util.nextTick(callback); + if (Object.keys(this._subscriptionCallbacksBySeq).length) { + return this._combineSubscribeCallbackWithLastAdded(callback); + } + } + this.wantSubscribe = wantSubscribe; var action = this.wantSubscribe ? ACTIONS.presenceSubscribe : ACTIONS.presenceUnsubscribe; var seq = this.connection._presenceSeq++; this._subscriptionCallbacksBySeq[seq] = callback; @@ -139,7 +152,10 @@ Presence.prototype._removeRemotePresence = function(id) { }; Presence.prototype._onConnectionStateChanged = function() { - if (!this.connection.canSend) return; + if (!this.connection.canSend) { + this.subscribed = false; + return; + } this._resubscribe(); for (var id in this.localPresences) { this.localPresences[id]._sendPending(); @@ -184,3 +200,14 @@ Presence.prototype._callEachOrEmit = function(callbacks, error) { var called = util.callEach(callbacks, error); if (!called && error) this.emit('error', error); }; + +Presence.prototype._combineSubscribeCallbackWithLastAdded = function(callback) { + var seqs = Object.keys(this._subscriptionCallbacksBySeq); + var lastSeq = seqs[seqs.length - 1]; + var originalCallback = this._subscriptionCallbacksBySeq[lastSeq]; + if (!originalCallback) return this._subscriptionCallbacksBySeq[lastSeq] = callback; + this._subscriptionCallbacksBySeq[lastSeq] = function(error) { + originalCallback(error); + callback(error); + }; +}; diff --git a/lib/client/presence/remote-doc-presence.js b/lib/client/presence/remote-doc-presence.js index deb9a34a4..e52e442c8 100644 --- a/lib/client/presence/remote-doc-presence.js +++ b/lib/client/presence/remote-doc-presence.js @@ -107,7 +107,7 @@ RemoteDocPresence.prototype._catchUpStalePresence = function() { this._doc.fetch(); // We're already subscribed, but we send another subscribe message // to force presence updates from other clients - this.presence.subscribe(); + this.presence.subscribe({force: true}); return false; } diff --git a/test/client/presence/presence.js b/test/client/presence/presence.js index f7a52762b..490942765 100644 --- a/test/client/presence/presence.js +++ b/test/client/presence/presence.js @@ -401,6 +401,46 @@ describe('Presence', function() { ], done); }); + it('does not send another subscribe request if already subscribed', function(done) { + var sendPresenceAction = sinon.spy(connection1, '_sendPresenceAction'); + async.series([ + presence1.subscribe.bind(presence1), + presence1.subscribe.bind(presence1), + function(next) { + expect(sendPresenceAction).to.have.been.calledOnce; + next(); + } + ], done); + }); + + it('only subscribes once when calling multiple in parallel', function(done) { + var sendPresenceAction = sinon.spy(connection1, '_sendPresenceAction'); + async.series([ + function(next) { + async.parallel([ + presence1.subscribe.bind(presence1), + presence1.subscribe.bind(presence1) + ], next); + }, + function(next) { + expect(sendPresenceAction).to.have.been.calledOnce; + next(); + } + ], done); + }); + + it('subscribes once when calling again after no callback', function(done) { + var sendPresenceAction = sinon.spy(connection1, '_sendPresenceAction'); + presence1.subscribe(); // no callback + async.series([ + presence1.subscribe.bind(presence1), + function(next) { + expect(sendPresenceAction).to.have.been.calledOnce; + next(); + } + ], done); + }); + it('throws an error when trying to create a presence with a non-string ID', function() { expect(function() { presence1.create(123);