From 6cc809b9562b32a867be122fdcb68fd06f358a8b Mon Sep 17 00:00:00 2001 From: David Glasser Date: Tue, 11 Dec 2012 17:26:56 -0800 Subject: [PATCH] Avoid unnecessary equality comparison in clearField. Minor test changes. --- packages/livedata/livedata_server.js | 12 ++++++---- packages/livedata/session_view_tests.js | 30 ++++++++++++++----------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/packages/livedata/livedata_server.js b/packages/livedata/livedata_server.js index b0497b6685..8620b4114a 100644 --- a/packages/livedata/livedata_server.js +++ b/packages/livedata/livedata_server.js @@ -20,11 +20,14 @@ _.extend(Meteor._SessionDocumentView.prototype, { if (!precedenceList) { throw new Error("Could not find field to clear " + key); } - var old = precedenceList[0]; - var precedence; + var removedValue = undefined; for (var i = 0; i < precedenceList.length; i++) { - precedence = precedenceList[i]; + var precedence = precedenceList[i]; if (precedence.subscriptionId === subscriptionId) { + // The view's value can only change if this subscription is the one that + // used to have precedence. + if (i === 0) + removedValue = precedence.value; precedenceList.splice(i, 1); break; } @@ -32,7 +35,8 @@ _.extend(Meteor._SessionDocumentView.prototype, { if (_.isEmpty(precedenceList)) { delete self.dataByKey[key]; clearCollector.push(key); - } else if (!_.isEqual(old.value, precedenceList[0].value)) { + } else if (removedValue !== undefined && + !_.isEqual(removedValue, precedenceList[0].value)) { changeCollector[key] = precedenceList[0].value; } }, diff --git a/packages/livedata/session_view_tests.js b/packages/livedata/session_view_tests.js index 2832a5deb4..063c9b51cb 100644 --- a/packages/livedata/session_view_tests.js +++ b/packages/livedata/session_view_tests.js @@ -11,25 +11,25 @@ var newView = function(test) { results.push({fun: 'removed', ids: ids}); } }); - var ret = { + var v = { view: view, results: results }; _.each(["added", "changed", "removed"], function (it) { - ret[it] = _.bind(view[it], view); + v[it] = _.bind(view[it], view); }); - ret.expectResult = function (result) { + v.expectResult = function (result) { test.equal(results.shift(), result); }; - ret.expectNoResult = function () { + v.expectNoResult = function () { test.equal(results, []); }; - ret.drain = function() { + v.drain = function() { var ret = results; results = []; return ret; }; - return ret; + return v; }; Tinytest.add('livedata - sessionview - exists reveal', function (test) { @@ -150,7 +150,7 @@ Tinytest.add('livedata - sessionview - field clear reveal', function (test) { v.expectNoResult(); }); -Tinytest.add('livedata - sessionview - change to cannonical value produces no change', function (test) { +Tinytest.add('livedata - sessionview - change to canonical value produces no change', function (test) { var v = newView(test); v.added("A", {_id: "A1", foo: "bar"}); @@ -159,13 +159,17 @@ Tinytest.add('livedata - sessionview - change to cannonical value produces no ch v.added("B", {_id: "A1", foo: "baz"}); - var cannon = "bar"; - if (!_.isEmpty(v.drain())) { + var canon = "bar"; + var maybeResults = v.drain(); + if (!_.isEmpty(maybeResults)) { // if something happened, it was a change message to baz. - // if nothing did, cannon is still bar. - cannon = "baz"; + // if nothing did, canon is still bar. + test.length(maybeResults, 1); + test.equal(maybeResults[0], {fun: 'added', id: "A1", changed: {foo: "baz"}, + cleared: []}); + canon = "baz"; } - v.changed("B", "A1", {foo: cannon}, []); + v.changed("B", "A1", {foo: canon}, []); v.expectNoResult(); v.removed("A", ["A1"]); @@ -175,7 +179,7 @@ Tinytest.add('livedata - sessionview - change to cannonical value produces no ch v.expectNoResult(); }); -Tinytest.add('livedata - sessionview - new field of cannonical value produces no change', function (test) { +Tinytest.add('livedata - sessionview - new field of canonical value produces no change', function (test) { var v = newView(test); v.added("A", {_id: "A1", foo: "bar"});