Removed fields aren't removed from mini-mongo

[CyberCyclone]

I've noticed that fields that have been removed from a document from the resulting pipeline aren't removed from the mini-mongo.

I've looked through the package and found that the sub.changed(localOptions.clientCollection, doc_id, doc); line is passing the doc from the pipeline. Looking at the Meteor documentation, here's what the fields variable does.

this.changed(collection, id, fields) -> https://docs.meteor.com/api/pubsub.html#Subscription-changed

The fields in the document that have changed, together with their new values. If a field is not present in fields it was left unchanged; if it is present in fields and has a value of undefined it was removed from the document. If _id is present it is ignored.

Based on this, since the fields are not marked as undefined, they are not removed from the subscription.

My question is, is this intentional from the plugins point of view? I feel like they should be removed because the pipeline doesn't return "changes", it returns documents as they should be.

If this is a bug, is there a suggestion on the best way to fix it? The most I can think of is to lookup the document, find the fields that are missing, add them to the doc that was returned from the pipeline and set them to undefined.

[robfallows]

Hmm. I guess this is more of an enhancement request than a bug, since it's currently working as designed. However, I see your point.

In order to address this, I think the observeChanges at https://github.com/robfallows/tunguska-reactive-aggregate/blob/d599d9d5ae3d395008cf8f48a099cab29fc00f6b/aggregate.js#L173 will need to become observe and the added, changed and removed at https://github.com/robfallows/tunguska-reactive-aggregate/blob/d599d9d5ae3d395008cf8f48a099cab29fc00f6b/aggregate.js#L174-L180 will need refactoring to use the document properties (https://docs.meteor.com/api/collections.html#Mongo-Cursor-observe).

Note that observe is likely to be more inefficient than observeChanges, so I also think this behaviour should be opt-in (through the options parameter).

I'd be happy to look at a PR, if you have the time to submit one. Otherwise, I can't guarantee a quick turnaround).

[CyberCyclone]

Thanks for that @robfallows. I think the situation you were thinking of is actually different to what mine is. The issue isn't related to the observed changes, but the differences in the pipeline results. As in, the results from the pipeline are 100% different to what the collection data is and therefore the observer changes are only used to trigger the pipeline.

From a wider use, if a document has a field $unset, and the pipeline gets the document, it won't have that field removed from the subscription.

In my case, I'm using a pipeline to count the documents that match a criteria. So when the observer notices that the documents have changed, I need to get a new count. The observer is looking for the field "status" which changes, but the pipeline returns a doc that looks like this:

{ "_id": "TotalTransactions", 'TotalPendingTransactions': 1, 'TotalProcessingTransactions': 1 'TotalAwaitingCollectionTransactions': 1 'TotalWaitingForOrderTransactions': 1 }

Since the pipeline only ever returns 1 document, and the combination of using $facet, $match and $count, a field value of '0' is impossible (without using hacky / wasteful means). Instead it doesn't return that field. So in my situation, it's very obvious that the difference between the pipeline doc before and after doesn't remove the field in the subscription.

The solution that I've come up with is this:

// If we got here, doc_id must be a string
if (!sub._ids[doc_id]) {
    sub.added(localOptions.clientCollection, doc_id, doc);
} else {
    // Since the pipeline fields might have been removed, we need to find the differences and define them as 'undefined' so the sub removes them.
    let previousFields = [...sub._session.collectionViews.get(localOptions.clientCollection).documents.get(doc_id).dataByKey.keys()];
    previousFields.forEach(field => {
        // At this point they are undefined because they no longer exist in the new doc, they're not literally set as undefined
        if(doc[field] === undefined){
            // We need to explicitly define this as undefined so the sub will remove them.
            doc[field] = undefined;
        }
    })
    sub.changed(localOptions.clientCollection, doc_id, doc);
}
sub._ids[doc_id] = sub._iteration;

I don't know if this solution will work in all scenarios as I don't really know much about the subscription obj that I'm accessing in order to get the current subscription data before it's updated. I've also only tested it in my use case which seams to solve it. It seems pretty simple so I don't really see it having much of a performance impact.

[robfallows]

Ah. Yes, I misunderstood. Your code looks like a good way to make use of the mergebox to ensure undefined fields are set appropriately.

Please submit a PR to get the credit! 🙂

[robfallows]

Closed by #24