Change removed messages to just take a single ID.

This commit is contained in:
David Glasser
2012-12-18 12:19:25 -08:00
parent 91b1f3b15c
commit a93567eed1
6 changed files with 102 additions and 115 deletions

View File

@@ -1002,26 +1002,22 @@ _.extend(Meteor._LivedataConnection.prototype, {
_process_removed: function (msg, updates) {
var self = this;
var idsRemovedNow = [];
_.each(msg.ids, function (removedId) {
var serverDoc = Meteor._get(self._serverDocuments, msg.collection, removedId);
if (serverDoc) {
// Some outstanding stub wrote here.
if (serverDoc.document === undefined) {
throw new Error("It doesn't make sense to be deleting something we don't know exists: "
+ removedId);
}
serverDoc.document = undefined;
} else {
idsRemovedNow.push(removedId);
var serverDoc = Meteor._get(
self._serverDocuments, msg.collection, msg.id);
if (serverDoc) {
// Some outstanding stub wrote here.
if (serverDoc.document === undefined) {
throw new Error("It doesn't make sense to be deleting something we don't know exists: "
+ msg.id);
}
});
if (!_.isEmpty(idsRemovedNow))
serverDoc.document = undefined;
} else {
self._pushUpdate(updates, msg.collection, {
msg: 'removed',
collection: msg.collection,
ids: idsRemovedNow
id: msg.id
});
}
},
_process_updated: function (msg, updates) {

View File

@@ -124,7 +124,6 @@ _.extend(Meteor._SessionCollectionView.prototype, {
diff: function (previous) {
var self = this;
var removedIds = [];
diffObjects(previous.documents, self.documents, {
both: _.bind(self.diffDocument, self),
@@ -133,11 +132,9 @@ _.extend(Meteor._SessionCollectionView.prototype, {
},
leftOnly: function (id, prevDV) {
removedIds.push(id);
self.callbacks.removed(self.collectionName, id);
}
});
if (!_.isEmpty(removedIds))
self.callbacks.removed(self.collectionName, removedIds);
},
diffDocument: function (id, prevDV, nowDV) {
@@ -195,33 +192,28 @@ _.extend(Meteor._SessionCollectionView.prototype, {
self.callbacks.changed(self.collectionName, id, changedResult, clearedResult);
},
removed: function (subscriptionId, ids) {
removed: function (subscriptionId, id) {
var self = this;
var removedIds = [];
_.each(ids, function (id) {
var docView = self.documents[id];
if (!docView) {
throw new Error("Removed nonexistent document " + id);
}
delete docView.existsIn[subscriptionId];
if (_.isEmpty(docView.existsIn)) {
// it is gone from everyone
removedIds.push(id);
delete self.documents[id];
} else {
var changed = {};
var cleared = [];
// remove this subscription from every precedence list
// and record the changes
_.each(docView.dataByKey, function (precedenceList, key) {
docView.clearField(subscriptionId, key, changed, cleared);
});
var docView = self.documents[id];
if (!docView) {
throw new Error("Removed nonexistent document " + id);
}
delete docView.existsIn[subscriptionId];
if (_.isEmpty(docView.existsIn)) {
// it is gone from everyone
self.callbacks.removed(self.collectionName, id);
delete self.documents[id];
} else {
var changed = {};
var cleared = [];
// remove this subscription from every precedence list
// and record the changes
_.each(docView.dataByKey, function (precedenceList, key) {
docView.clearField(subscriptionId, key, changed, cleared);
});
self.callbacks.changed(self.collectionName, id, changed, cleared);
}
});
if (!_.isEmpty(removedIds))
self.callbacks.removed(self.collectionName, removedIds);
self.callbacks.changed(self.collectionName, id, changed, cleared);
}
}
});
/******************************************************************************/
@@ -301,10 +293,10 @@ _.extend(Meteor._LivedataSession.prototype, {
}
},
sendRemoved: function (collectionName, ids) {
sendRemoved: function (collectionName, id) {
var self = this;
if (self._isSending)
self.send({msg: "removed", collection: collectionName, ids: ids});
self.send({msg: "removed", collection: collectionName, id: id});
},
getSendCallbacks: function () {
@@ -333,10 +325,10 @@ _.extend(Meteor._LivedataSession.prototype, {
view.added(subscriptionId, id, fields);
},
removed: function (subscriptionId, collectionName, ids) {
removed: function (subscriptionId, collectionName, id) {
var self = this;
var view = self.getCollectionView(collectionName);
view.removed(subscriptionId, ids);
view.removed(subscriptionId, id);
if (view.isEmpty()) {
delete self.collectionViews[collectionName];
}
@@ -653,7 +645,9 @@ _.extend(Meteor._LivedataSession.prototype, {
});
},
leftOnly: function (collectionName, leftValue) {
self.sendRemoved(collectionName, _.keys(leftValue.documents));
_.each(leftValue.documents, function (doc, id) {
self.sendRemoved(collectionName, id);
});
}
});
},
@@ -829,15 +823,12 @@ _.extend(Meteor._LivedataSubscription.prototype, {
self._session.changed(self._subscriptionId, collectionName, id, fields);
},
removed: function (collectionName, ids) {
removed: function (collectionName, id) {
var self = this;
_.each(ids, function(id) {
// we don't bother to delete sets of things in a collection if the
// collection is empty. It could break below, where we iterate over
// it removing items.
delete self._documents[collectionName][id];
});
self._session.removed(self._subscriptionId, collectionName, ids);
// We don't bother to delete sets of things in a collection if the
// collection is empty. It could break _removeAllDocuments.
delete self._documents[collectionName][id];
self._session.removed(self._subscriptionId, collectionName, id);
},
complete: function () {
@@ -861,7 +852,11 @@ _.extend(Meteor._LivedataSubscription.prototype, {
_removeAllDocuments: function () {
var self = this;
_.each(self._documents, function(collectionDocs, collectionName) {
self.removed(collectionName, _.keys(collectionDocs));
// Iterate over _.keys instead of the dictionary itself, since we'll be
// mutating it.
_.each(_.keys(collectionDocs), function (id) {
self.removed(collectionName, id);
});
});
}

View File

@@ -317,7 +317,7 @@ if (Meteor.isClient) {
if (msg.msg === 'added')
++actualAddedMessageCount;
else if (msg.msg === 'removed')
actualRemovedMessageCount += msg.ids.length;
++actualRemovedMessageCount;
else
test.fail({unexpected: JSON.stringify(msg)});
});

View File

@@ -9,8 +9,8 @@ var newView = function(test) {
return;
results.push({fun: 'changed', id: id, changed: changed, cleared: cleared});
},
removed: function (collection, ids) {
results.push({fun: 'removed', ids: ids});
removed: function (collection, id) {
results.push({fun: 'removed', id: id});
}
});
var v = {
@@ -44,11 +44,11 @@ Tinytest.add('livedata - sessionview - exists reveal', function (test) {
v.added("B", "A1", {});
v.expectNoResult();
v.removed("A", ["A1"]);
v.removed("A", "A1");
v.expectNoResult();
v.removed("B", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("B", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -60,14 +60,14 @@ Tinytest.add('livedata - sessionview - field reveal', function (test) {
v.expectNoResult();
v.added("B", "A1", {foo: "baz"});
v.removed("A", ["A1"]);
v.removed("A", "A1");
v.expectResult({fun: 'changed', id: "A1", changed: {foo: "baz"}, cleared: []});
v.expectNoResult();
// Somewhere in here we must have changed foo to baz. Legal either on the
// added or on the removed, but only once.
v.removed("B", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("B", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -82,8 +82,8 @@ Tinytest.add('livedata - sessionview - field change', function (test) {
v.expectResult({fun: 'changed', id: "A1", changed: {foo: "baz"}, cleared: []});
v.expectNoResult();
v.removed("A", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("A", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -98,8 +98,8 @@ Tinytest.add('livedata - sessionview - field clear', function (test) {
v.expectResult({fun: 'changed', id: "A1", changed: {}, cleared: ["foo"]});
v.expectNoResult();
v.removed("A", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("A", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -122,8 +122,8 @@ Tinytest.add('livedata - sessionview - add, remove, add', function (test) {
v.expectResult({fun: 'added', id: "A1", fields: {foo: "bar"}});
v.expectNoResult();
v.removed("A", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("A", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
v.added("A", "A1", {foo: "bar"});
@@ -145,10 +145,10 @@ Tinytest.add('livedata - sessionview - field clear reveal', function (test) {
v.expectResult({fun: 'changed', id: "A1", changed: {foo: "baz"}, cleared: []});
v.expectNoResult();
v.removed("A", ["A1"]);
v.removed("A", "A1");
v.expectNoResult();
v.removed("B", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("B", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -174,10 +174,10 @@ Tinytest.add('livedata - sessionview - change to canonical value produces no cha
v.changed("B", "A1", {foo: canon}, []);
v.expectNoResult();
v.removed("A", ["A1"]);
v.removed("A", "A1");
v.expectNoResult();
v.removed("B", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("B", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -194,10 +194,10 @@ Tinytest.add('livedata - sessionview - new field of canonical value produces no
v.changed("B", "A1", {foo: "bar"}, []);
v.expectNoResult();
v.removed("A", ["A1"]);
v.removed("A", "A1");
v.expectNoResult();
v.removed("B", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("B", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -216,9 +216,9 @@ Tinytest.add('livedata - sessionview - clear all clears only once', function (te
v.expectResult({fun: 'changed', id: "A1", changed: {}, cleared: ["foo"]});
v.expectNoResult();
v.removed("A", ["A1"]);
v.removed("A", "A1");
v.expectNoResult();
v.removed("B", ["A1"]);
v.removed("B", "A1");
v.expectNoResult();
});
@@ -237,9 +237,9 @@ Tinytest.add('livedata - sessionview - change all changes only once', function (
v.expectResult({fun: 'changed', id: "A1", changed: {foo: "baz"}, cleared: []});
v.expectNoResult();
v.removed("A", ["A1"]);
v.removed("A", "A1");
v.expectNoResult();
v.removed("B", ["A1"]);
v.removed("B", "A1");
v.expectNoResult();
});
@@ -256,11 +256,11 @@ Tinytest.add('livedata - sessionview - multiple operations at once in a change',
v.expectResult({fun: 'changed', id: "A1", changed: {foo: "baz", thing: "stuff"}, cleared: ["baz"]});
v.expectNoResult();
v.removed("A", ["A1"]);
v.removed("A", "A1");
v.expectResult({fun: 'changed', id: "A1", changed: {}, cleared: ["thing"]});
v.expectNoResult();
v.removed("B", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("B", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -278,16 +278,16 @@ Tinytest.add('livedata - sessionview - more than one document', function (test)
v.expectResult({fun: 'changed', id: "A1", changed: {thing: "stuff"}, cleared: ["foo", "baz"]});
v.expectNoResult();
v.removed("A", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("A", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
v.removed("A", ["A2"]);
v.expectResult({fun: 'removed', ids: ["A2"]});
v.removed("A", "A2");
v.expectResult({fun: 'removed', id: "A2"});
v.expectNoResult();
});
Tinytest.add('livedata - sessionview - multiple docs removed at once', function (test) {
Tinytest.add('livedata - sessionview - multiple docs removed', function (test) {
var v = newView(test);
v.added("A", "A1", {foo: "bar", baz: "quux"});
@@ -299,8 +299,10 @@ Tinytest.add('livedata - sessionview - multiple docs removed at once', function
v.expectResult({fun: 'added', id: "A2", fields: {foo: "baz"}});
v.expectNoResult();
v.removed("A", ["A1", "A2"]);
v.expectResult({fun: 'removed', ids: ["A1", "A2"]});
v.removed("A", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.removed("A", "A2");
v.expectResult({fun: 'removed', id: "A2"});
v.expectNoResult();
});
@@ -320,12 +322,13 @@ Tinytest.add('livedata - sessionview - complicated sequence', function (test) {
v.expectResult({fun: 'changed', id: "A1", changed: {foo: "baz", thing: "stuff"}, cleared: ["baz"]});
v.expectNoResult();
v.removed("A", ["A1", "A2"]);
v.removed("A", "A1");
v.removed("A", "A2");
v.expectResult({fun: 'changed', id: "A1", changed: {}, cleared: ["thing"]});
v.expectResult({fun: 'removed', ids: ["A2"]});
v.expectResult({fun: 'removed', id: "A2"});
v.expectNoResult();
v.removed("B", ["A1"]);
v.expectResult({fun: 'removed', ids: ["A1"]});
v.removed("B", "A1");
v.expectResult({fun: 'removed', id: "A1"});
v.expectNoResult();
});
@@ -339,11 +342,11 @@ Tinytest.add('livedata - sessionview - added becomes changed', function (test) {
v.expectResult({fun: 'changed', id: 'A1', changed: {hi: 'there'},
cleared: []});
v.removed('A', ['A1']);
v.removed('A', 'A1');
v.expectResult({fun: 'changed', id: 'A1', changed: {}, cleared: ['foo']});
v.removed('B', ['A1']);
v.expectResult({fun: 'removed', ids: ['A1']});
v.removed('B', 'A1');
v.expectResult({fun: 'removed', id: 'A1'});
});
Tinytest.add('livedata - sessionview - weird key names', function (test) {

View File

@@ -68,17 +68,6 @@ Meteor.Collection = function (name, options) {
// Apply an update.
// XXX better specify this interface (not in terms of a wire message)?
update: function (msg) {
// Handle removed separately, since it's the only one without an 'id'
// field.
if (msg.msg === 'removed') {
_.each(msg.ids, function (removedId) {
if (!self._collection.findOne(removedId))
throw new Error("Expected to find a document to remove");
self._collection.remove(removedId);
});
return;
}
var doc = self._collection.findOne(msg.id);
// Is this a "replace the whole doc" message coming from the quiescence
@@ -100,6 +89,10 @@ Meteor.Collection = function (name, options) {
if (doc)
throw new Error("Expected not to find a document already present for an add");
self._collection.insert(_.extend({_id: msg.id}, msg.fields));
} else if (msg.msg === 'removed') {
if (!doc)
throw new Error("Expected to find a document already present for removed");
self._collection.remove(msg.id);
} else if (msg.msg === 'changed') {
if (!doc)
throw new Error("Expected to find a document to change");

View File

@@ -311,7 +311,7 @@ Cursor.prototype._publishCursor = function (sub) {
sub.changed(collection, obj._id, fields);
},
removed: function (oldObj) {
sub.removed(collection, [oldObj._id]);
sub.removed(collection, oldObj._id);
}
});