Reactivity bug in fairly specific situations

[znewsham]

I've found a situation where redis-oplog does not correctly observe updates to a document, that I believe it could/should

consider the following:

//server.js
Meteor.publish('something', function() {
   return Meteor.users.find({ someFieldToQuery: "some value" }, { fields: { someOtherField: 1 }, sort: { doesntMatter: 1 });
});

//client.js
Meteor.subscribe("something");

/// meteor shell
Meteor.users.update({ someFieldToQuery: "some value" }, { $set: { someOtherField: "new value" } })

In this situation, redis-oplog does not notify the subscribing client of changes to the document. This can be "resolved" by including all fields necessary to fullfill the query. In the example above, adding someFieldToQuery: 1 to the fields object resolves.

I believe the issue arises from the use of observableCollection.isEligible(doc) in the limit-sort.js handleUpdate function and the resulting call requery(observableCollection); when it has been determined that the document is not eligible for re-querying.

Once it's been determined that the document in question is in the observable collection, an update should always take place.

I'm guessing this is a pretty rare situation, I've been working with redis-oplog for a few years and not come across it before. But would be nice to fix (I've patched it locally for now)

[ramezrafla]

@znewsham

1. Your publish returns findOne which is a single-time doc, so you have no reactivity
2. Why would it use limit-sort, shouldn't be default? You don't have a limit in your query

[znewsham]

Sorry - my example was bad, let me correct, I tried to simplify the example and went too far. I'll edit

[znewsham]

@ramezrafla should be "correct" now. It does lead to an interesting question of whether the same bug exists in default though. For reference, here is my fix:

const handleUpdate = function(observableCollection, doc, modifiedFields) {
    if (observableCollection.contains(doc._id)) {
        if (observableCollection.isEligible(doc)) {
            if (
                hasSortFields(observableCollection.options.sort, modifiedFields)
            ) {
                requery(
                    observableCollection,
                    doc,
                    Events.UPDATE,
                    modifiedFields
                );
            } else {
                observableCollection.change(doc, modifiedFields);
            }
        } else {
            if (!requery(observableCollection)) {
              observableCollection.change(doc, modifiedFields);
            }
        }
    } else {
        if (observableCollection.isEligible(doc)) {
            requery(
                observableCollection,
                doc,
                Events.UPDATE,
                modifiedFields
            );
        }
    }
};

I also modified requery to return true if it adds or changes a document internally.

[znewsham]

Any movement on this @theodorDiaconu The bug is even worse than this - the same thing exists with inserts on the default case - so I'm going to assume it exists everywhere :(

[znewsham]

Ok - the insert variant of the problem is slightly more subtle - the issue is with _getFieldsOfInterest and selectors that just contain an and:

{
$and: [selector1, selector2]
}

[theodorDiaconu]

@znewsham I see, can you tell me the full query you are searching?

[znewsham]

@theodorDiaconu I'm thinking this might be a versioning problem with underscore - I can reproduce it trivially with { $and: [ { a: 1 }, { b: 2 } ] }. The problem appears to be in extractFieldsFromFilter.js

function extractFieldsFromFilters(filters) {
    let filterFields = [];

    _.each(filters, (value, field) => {
        if (field[0] !== '$') {
            filterFields.push(field);
        }
    });

    deepFilterFieldsArray.forEach(field => {
        if (filters[field]) {
            filters[field].forEach(element => {
                _.union(filterFields, extractFieldsFromFilters(element));
            });
        }
    });

    deepFilterFieldsObject.forEach(field => {
        if (filters[field]) {
            _.union(filterFields, extractFieldsFromFilters(filters[field]));
        }
    });

    return filterFields;
}

Swapping _.union(filterFields, extractFieldsFromFilters(element)); -> filterFields = _.union(filterFields, extractFieldsFromFilters(element)); resolves the issue