Merge pull request #628 from share/presence-subscribe-once

⚡️ Avoid repeat subscription requests

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);
+  };
+};

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;
   }
 

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);