diff --git a/packages/livedata/livedata_connection.js b/packages/livedata/livedata_connection.js index 32e7c754f6..ec662c8785 100644 --- a/packages/livedata/livedata_connection.js +++ b/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. diff --git a/packages/livedata/livedata_connection_tests.js b/packages/livedata/livedata_connection_tests.js index 488e3a8c24..6e216e2d76 100644 --- a/packages/livedata/livedata_connection_tests.js +++ b/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); }); diff --git a/packages/meteor/timers.js b/packages/meteor/timers.js index a7353b1a77..bdaa636ef8 100644 --- a/packages/meteor/timers.js +++ b/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); } -}); \ No newline at end of file +});