Merge branch 'the-shell-inc-master'

package.json

@@ -49,7 +49,7 @@
     "sinon-chai": "^2.8.0"
   },
   "dependencies": {
-    "ddp.js": "^2.1.0",
+    "ddp.js": "^2.2.0",
     "lodash.assign": "^4.0.9",
     "wolfy87-eventemitter": "^5.0.0"
   }

src/base-mixins/subscriptions.js

@@ -21,13 +21,12 @@ import fingerprintSub from "../common/fingerprint-sub";
 *   they are not exported so they don't clutter the Asteroid class prototype.
 */
 
-function restartSubscription ({id, name, params, stillInQueue}) {
+function restartSubscription (sub) {
     // Only restart the subscription if it isn't still in ddp's queue.
-    if (!stillInQueue) {
-        // The subscription must be deleted *before* re-subscribing, otherwise
-        // `subscribe` hits the cache and does nothing
-        this.subscriptions.cache.del(id);
-        this.subscribe(name, ...params);
+    if (!sub.stillInQueue) {
+        this.resubscribe(sub);
+    } else {
+        sub.stillInQueue = false;
     }
 }
 
@@ -64,6 +63,11 @@ export function unsubscribe (id) {
     this.ddp.unsub(id);
 }
 
+export function resubscribe (sub) {
+    this.ddp.sub(sub.name, sub.params, sub.id);
+    sub.stillInQueue = (this.ddp.status !== "connected");
+}
+
 /*
 *   Init method
 */

test/unit/base-mixins/subscriptions.js

@@ -3,6 +3,7 @@ import chaiAsPromised from "chai-as-promised";
 import sinon from "sinon";
 import sinonChai from "sinon-chai";
 import EventEmitter from "wolfy87-eventemitter";
+import assign from "lodash.assign";
 
 import * as subscriptionsMixin from "base-mixins/subscriptions";
 
@@ -95,8 +96,12 @@ describe("`subscriptions` mixin", () => {
 
         it("triggers a re-subscription to cached subscriptions which are not still in ddp's queue", () => {
             const instance = {
-                ddp: new EventEmitter(),
-                subscribe: sinon.spy()
+                ddp: assign(new EventEmitter(), {
+                    sub: sinon.spy(),
+                    status: false // something that is not "connected"
+                }),
+                subscribe: sinon.spy(),
+                resubscribe: sinon.spy()
             };
             subscriptionsMixin.init.call(instance);
             instance.subscriptions.cache.add({
@@ -113,15 +118,19 @@ describe("`subscriptions` mixin", () => {
                 params: ["2", "22", "222"],
                 stillInQueue: false
             });
+            instance.ddp.status = "connected";
             instance.ddp.emit("connected");
-            expect(instance.subscribe.firstCall).to.have.been.calledWith("n1", "1", "11", "111");
-            expect(instance.subscribe.secondCall).to.have.been.calledWith("n2", "2", "22", "222");
+            expect(instance.resubscribe).to.have.callCount(2);
         });
 
         it("doesn't trigger a re-subscription to cached subscriptions which are still in ddp's queue", () => {
             const instance = {
-                ddp: new EventEmitter(),
-                subscribe: sinon.spy()
+                ddp: assign(new EventEmitter(), {
+                    sub: sinon.spy(),
+                    status: false // something that is not "connected"
+                }),
+                subscribe: sinon.spy(),
+                resubscribe: sinon.spy()
             };
             subscriptionsMixin.init.call(instance);
             instance.subscriptions.cache.add({
@@ -138,8 +147,23 @@ describe("`subscriptions` mixin", () => {
                 params: ["2", "22", "222"],
                 stillInQueue: false
             });
+
+            instance.ddp.status = "connected";
             instance.ddp.emit("connected");
-            expect(instance.subscribe).to.have.callCount(1);
+
+            // re-subscribe should only be called for subscriptions
+            // those were not in queue already
+            expect(instance.resubscribe).to.have.callCount(1);
+
+            // all subscriptions should be removed from the queue
+            instance.subscriptions.cache.forEach(sub => {
+                expect(sub).to.have.property("stillInQueue", false);
+            });
+
+            // re a re-connection happens, resubscribe should be 
+            // called twice more, reconnecting both subscribtions
+            instance.ddp.emit("connected");
+            expect(instance.resubscribe).to.have.callCount(3);
         });
 
     });
@@ -185,6 +209,51 @@ describe("`subscriptions` mixin", () => {
 
     });
 
+    describe("`resubscribe` method", () => {
+        it("sets stillInQueue if ddp status is not 'connected'", () => {
+            var idCounter = 0;
+            const instance = {
+                ddp: assign(new EventEmitter(), {
+                    sub: sinon.spy(function (name, params, id) {
+                        return id || ++idCounter;
+                    }),
+                    status: false // something that is not "connected"
+                }),
+                subscribe: subscriptionsMixin.subscribe,
+                resubscribe: subscriptionsMixin.resubscribe
+            };
+            subscriptionsMixin.init.call(instance);
+
+            instance.subscribe("1", "1", "11", "111");
+
+            expect(instance.ddp.sub).to.have.callCount(1);
+
+            instance.ddp.status = "connected";
+            instance.ddp.emit("connected");
+
+            expect(instance.ddp.sub).to.have.callCount(1);
+
+            instance.subscribe("2", "2", "22", "222");
+
+            expect(instance.ddp.sub).to.have.callCount(2);
+
+            instance.subscriptions.cache.forEach(sub => {
+                expect(sub.stillInQueue).to.equal(false);
+            });
+
+            // reconnect happens!
+            instance.ddp.emit("connected");
+
+            // both subscriptions should be re-subscribed
+            expect(instance.ddp.sub).to.have.callCount(4);
+
+            // all subscriptions should be removed from the queue
+            instance.subscriptions.cache.forEach(sub => {
+                expect(sub.stillInQueue).to.equal(false);
+            });
+        });
+    });
+
     describe("`unsubscribe` method", () => {
 
         it("calls `ddp.unsub`", () => {