⚡️ Avoid repeat subscription requests

At the moment, if a `Presence` instance is already subscribed, calling
`presence.subscribe()` will send another subscription request, which
can cause unnecessary server load, including:

 - [creating a new Pub/Sub subscription][1], and
 - [re-requesting all other presences][2]

This change updates our presence subscription logic to early return if
our desired subscription state already matches both `wantSubscribe`
**and** `subscribed`. We also reset `subscribed` to `false` when our
connection is disconnected to correctly update our state.

[1]: https://github.com/share/sharedb/blob/62e4ec5d46c0dcb097931e9dae60240aaba6b2b3/lib/agent.js#L811
[2]: https://github.com/share/sharedb/blob/62e4ec5d46c0dcb097931e9dae60240aaba6b2b3/lib/agent.js#L820

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,18 @@ 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 && wantSubscribe === this.subscribed) {
+    if (callback) util.nextTick(callback);
+    return;
+  }
+  this.wantSubscribe = wantSubscribe;
   var action = this.wantSubscribe ? ACTIONS.presenceSubscribe : ACTIONS.presenceUnsubscribe;
   var seq = this.connection._presenceSeq++;
   this._subscriptionCallbacksBySeq[seq] = callback;
@@ -139,7 +149,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();

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,18 @@ 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('throws an error when trying to create a presence with a non-string ID', function() {
     expect(function() {
       presence1.create(123);