Make sure that the client doesn't get confused by an unsub followed immediately

by an identical sub.

Previously, it would fail to subscribe. This confused the meteor.currentUser
subscription in accounts_client.js. Reproduction:

  - Create user X with email (which sends a confirmation email)
  - Log out.
  - Log in as User Y in tab 1.
  - Follow the confirmation link in tab 2. This leaves you logged in
    as User Y.
  - Tab 1's localstorage poller notices that there's a new token and logs in
    with it.
  - After a successful login, Accounts._makeClientLoggedIn unsubs from
    meteor.currentUser and immediately resubs. It thinks there's already an
    existing sub, so it doesn't send the sub message and does immediately call
    the ready callback (which sets userLoaded() to true).
  - The unsub gets sent and the object in Meteor.users() gets depopulated.

Now Meteor.userLoaded() is true but Meteor.user() is empty.

packages/livedata/livedata_connection.js

@@ -223,7 +223,11 @@ _.extend(Meteor._LivedataConnection.prototype, {
     if (args.length && typeof args[args.length - 1] === "function")
       var callback = args.pop();
 
-    var existing = self.subs.find({name: name, args: args}, {reactive: false}).fetch();
+    // Look for existing subs (ignore those with count=0, since they're going to
+    // get removed on the next time through the event loop).
+    var existing = self.subs.find(
+      {name: name, args: args, count: {$gt: 0}},
+      {reactive: false}).fetch();
 
     if (existing && existing[0]) {
       // already subbed, inc count.

packages/livedata/livedata_connection_tests.js

@@ -59,7 +59,7 @@ Tinytest.add("livedata stub - receive data", function (test) {
   test.isUndefined(conn.queued[coll_name]);
 });
 
-Tinytest.add("livedata stub - subscribe", function (test) {
+Tinytest.addAsync("livedata stub - subscribe", function (test, onComplete) {
   var stream = new Meteor._StubStream();
   var conn = newConnection(stream);
 
@@ -72,6 +72,7 @@ Tinytest.add("livedata stub - subscribe", function (test) {
   });
   test.isFalse(callback_fired);
 
+  test.length(stream.sent, 1);
   var message = JSON.parse(stream.sent.shift());
   var id = message.id;
   delete message.id;
@@ -80,6 +81,34 @@ Tinytest.add("livedata stub - subscribe", function (test) {
   // get the sub satisfied. callback fires.
   stream.receive({msg: 'data', 'subs': [id]});
   test.isTrue(callback_fired);
+
+  // This defers the actual unsub message, so we need to set a timeout
+  // to observe the message. We also test that we can resubscribe even
+  // before the unsub has been sent.
+  //
+  // Note: it would be perfectly fine for livedata_connection to send the unsub
+  // synchronously, so if this test fails just because we've made that change,
+  // that's OK! This is a regression test for a failure case where it *never*
+  // sent the unsub if there was a quick resub afterwards.
+  //
+  // XXX rewrite Meteor.defer to guarantee ordered execution so we don't have to
+  // use setTimeout
+  sub.stop();
+  conn.subscribe('my_data');
+
+  test.length(stream.sent, 1);
+  message = JSON.parse(stream.sent.shift());
+  var id2 = message.id;
+  test.notEqual(id, id2);
+  delete message.id;
+  test.equal(message, {msg: 'sub', name: 'my_data', params: []});
+
+  setTimeout(function() {
+    test.length(stream.sent, 1);
+    var message = JSON.parse(stream.sent.shift());
+    test.equal(message, {msg: 'unsub', id: id});
+    onComplete();
+  }, 10);
 });
 
 

packages/meteor/timers.js

@@ -42,10 +42,15 @@ _.extend(Meteor, {
   },
 
   // won't be necessary once we clobber the global setTimeout
+  //
+  // XXX consider making this guarantee ordering of defer'd callbacks, like
+  // Meteor._atFlush or Node's nextTick (in practice). Then tests can do:
+  //    callSomethingThatDefersSomeWork();
+  //    Meteor.defer(expect(somethingThatValidatesThatTheWorkHappened));
   defer: function (f) {
     // Older Firefox will pass an argument to the setTimeout callback
     // function, indicating the "actual lateness." It's non-standard,
     // so for defer, standardize on not having it.
     Meteor.setTimeout(function () {f();}, 0);
   }
-});
+});