Calling added twice in a publication causes confusing behavior on update

[benweissmann]

Reproduction: https://github.com/benweissmann/meteor-double-added-issue

If you have a publication that calls added twice, future calls to update don't correctly clear fields.

In the reproduction linked above, I have this (very contrived) publication:

Meteor.publish('test_publication', function() {
  // We call added twice for `defective`
  this.added('foo', 'defective', {
    a: 'this should get updated',
    b: 'this should go away',
  });

  this.added('foo', 'defective', {
    a: 'this should get updated',
    b: 'this should go away',
  });

  Meteor.setTimeout(() => {
    this.changed('foo', 'defective', {
      a: 'updated A',
      b: undefined,
    });
  }, 3000);

  // We call added once for `correct`
  this.added('foo', 'correct', {
    a: 'this should get updated',
    b: 'this should go away',
  });

  Meteor.setTimeout(() => {
    this.changed('foo', 'correct', {
      a: 'updated A',
      b: undefined,
    });
  }, 3000);

  this.ready();
})

For both documents, the field a is correctly updated 3 seconds after the publication starts. However, for the defective document, the field b is not correctly unset.

I think the issue is this break: https://github.com/meteor/meteor/blob/devel/packages/ddp-server/livedata_server.js#L56 -- when we call added multiple times, we end up with two entries in the precedenceList. This is fine for changed, because we end up just updating the higher-precedence item. However, for clearing fields, it means that we clear only one entry and end up falling back to the previous entry.

I'm happy to write a PR to fix this, but I'm not sure what the correct behavior should be. I think that in most cases, the repeated call to added is erroneous, so maybe we should error on the second call to make it easy to find the problematic call? But that might be a pretty big breaking change. Alternatively, we could translate repeated calls to added into calls to changed, or fix the clearField method of SessionDocumentView to remove all the matching instances from the precedenceList.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically closed it has not had recent activity.