diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js index 27e5f643ca..b97663a8a7 100644 --- a/packages/minimongo/minimongo.js +++ b/packages/minimongo/minimongo.js @@ -699,7 +699,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) { } if (!options) options = {}; - var matcher = new Minimongo.Matcher(selector); + var matcher = new Minimongo.Matcher(selector, true); // Save the original results of any query that we might need to // _recomputeResults on, because _modifyAndNotify will mutate the objects in diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index ad38b97131..57b006e03e 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2301,13 +2301,6 @@ Tinytest.add("minimongo - modify", function (test) { {'a.x': 1, 'a.y': 3}, {$set: {'a.$.z': 5}}, {a: [{x: 1}, {y: 3, z: 5}]}); - // with $near, make sure it finds the closest one - modifyWithQuery({a: [{b: [1,1]}, - {b: [ [3,3], [4,4] ]}, - {b: [9,9]}]}, - {'a.b': {$near: [5, 5]}}, - {$set: {'a.$.b': 'k'}}, - {a: [{b: [1,1]}, {b: 'k'}, {b: [9,9]}]}); modifyWithQuery({a: [{x: 1}, {y: 1}, {x: 1, y: 1}]}, {a: {$elemMatch: {x: 1, y: 1}}}, {$set: {'a.$.x': 2}}, @@ -2316,6 +2309,56 @@ Tinytest.add("minimongo - modify", function (test) { {'a.b': {$elemMatch: {x: 1, y: 1}}}, {$set: {'a.$.b': 3}}, {a: [{b: 3}]}); + // with $near, make sure it does not find the closest one (#3599) + modifyWithQuery({a: []}, + {'a.b': {$near: [5, 5]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[]}); + modifyWithQuery({a: [{b: [ [3,3], [4,4] ]}]}, + {'a.b': {$near: [5, 5]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"}]}); + modifyWithQuery({a: [{b: [1,1]}, + {b: [ [3,3], [4,4] ]}, + {b: [9,9]}]}, + {'a.b': {$near: [5, 5]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + modifyWithQuery({a: [{b: [1,1]}, + {b: [ [3,3], [4,4] ]}, + {b: [9,9]}]}, + {'a.b': {$near: [9, 9], $maxDistance: 1}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + modifyWithQuery({a: [{b: [1,1]}, + {b: [ [3,3], [4,4] ]}, + {b: [9,9]}]}, + {'a.b': {$near: [9, 9]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + modifyWithQuery({a: [{b: [9,9]}, + {b: [ [3,3], [4,4] ]}, + {b: [9,9]}]}, + {'a.b': {$near: [9, 9]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + modifyWithQuery({a: [{b:[4,3]}, + {c: [1,1]}]}, + {'a.c': {$near: [1, 1]}}, + {$set: {'a.$.c': 'k'}}, + {"a":[{"c": "k", "b":[4,3]},{"c":[1,1]}]}); + modifyWithQuery({a: [{c: [9,9]}, + {b: [ [3,3], [4,4] ]}, + {b: [1,1]}]}, + {'a.b': {$near: [1, 1]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"c": [9,9], "b":"k"},{"b": [ [3,3], [4,4]]},{"b":[1,1]}]}); + modifyWithQuery({a: [{c: [9,9], b:[4,3]}, + {b: [ [3,3], [4,4] ]}, + {b: [1,1]}]}, + {'a.b': {$near: [1, 1]}}, + {$set: {'a.$.b': 'k'}}, + {"a":[{"c": [9,9], "b":"k"},{"b": [ [3,3], [4,4]]},{"b":[1,1]}]}); // $inc modify({a: 1, b: 2}, {$inc: {a: 10}}, {a: 11, b: 2}); @@ -3220,13 +3263,14 @@ Tinytest.add("minimongo - $near operator tests", function (test) { // 'y'. testNear([2, 2], 1000, ['x', 'y']); - // Ensure that distance is used as a tie-breaker for sort. + // issue #3599 + // Ensure that distance is not used as a tie-breaker for sort. test.equal( _.pluck(coll.find({'a.b': {$near: [1, 1]}}, {sort: {k: 1}}).fetch(), '_id'), ['x', 'y']); test.equal( _.pluck(coll.find({'a.b': {$near: [5, 5]}}, {sort: {k: 1}}).fetch(), '_id'), - ['y', 'x']); + ['x', 'y']); var operations = []; var cbs = log_callbacks(operations); diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index f3034094d1..7088fa751a 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -18,7 +18,7 @@ // Main entry point. // var matcher = new Minimongo.Matcher({a: {$gt: 5}}); // if (matcher.documentMatches({a: 7})) ... -Minimongo.Matcher = function (selector) { +Minimongo.Matcher = function (selector, isUpdate = false) { var self = this; // A set (object mapping string -> *) of all of the document paths looked // at by the selector. Also includes the empty string if it may look at any @@ -41,6 +41,10 @@ Minimongo.Matcher = function (selector) { // Sorter._useWithMatcher. self._selector = null; self._docMatcher = self._compileSelector(selector); + // Set to true if selection is done for an update operation + // Default is false + // Used for $near array update (issue #3599) + self._isUpdate = isUpdate; }; _.extend(Minimongo.Matcher.prototype, { @@ -484,23 +488,29 @@ var VALUE_OPERATORS = { // each within-$maxDistance branching point. branchedValues = expandArraysInBranches(branchedValues); var result = {result: false}; - _.each(branchedValues, function (branch) { - if (!(typeof branch.value === "object")){ - return; + _.every(branchedValues, function (branch) { + // if operation is an update, don't skip branches, just return the first one (#3599) + if (!matcher._isUpdate){ + if (!(typeof branch.value === "object")){ + return true; + } + var curDistance = distance(branch.value); + // Skip branches that aren't real points or are too far away. + if (curDistance === null || curDistance > maxDistance) + return true; + // Skip anything that's a tie. + if (result.distance !== undefined && result.distance <= curDistance) + return true; } - var curDistance = distance(branch.value); - // Skip branches that aren't real points or are too far away. - if (curDistance === null || curDistance > maxDistance) - return; - // Skip anything that's a tie. - if (result.distance !== undefined && result.distance <= curDistance) - return; result.result = true; result.distance = curDistance; if (!branch.arrayIndices) delete result.arrayIndices; else result.arrayIndices = branch.arrayIndices; + if (matcher._isUpdate) + return false; + return true; }); return result; }; diff --git a/packages/minimongo/sort.js b/packages/minimongo/sort.js index 7c90b8f650..aebbcc2d4f 100644 --- a/packages/minimongo/sort.js +++ b/packages/minimongo/sort.js @@ -81,23 +81,25 @@ _.extend(Minimongo.Sorter.prototype, { getComparator: function (options) { var self = this; - // If we have no distances, just use the comparator from the source - // specification (which defaults to "everything is equal". - if (!options || !options.distances) { + // If sort is specified or have no distances, just use the comparator from + // the source specification (which defaults to "everything is equal". + // issue #3599 + // https://docs.mongodb.com/manual/reference/operator/query/near/#sort-operation + // sort effectively overrides $near + if (self._sortSpecParts.length || !options || !options.distances) { return self._getBaseComparator(); } var distances = options.distances; - // Return a comparator which first tries the sort specification, and if that - // says "it's equal", breaks ties using $near distances. - return composeComparators([self._getBaseComparator(), function (a, b) { + // Return a comparator which compares using $near distances. + return function (a, b) { if (!distances.has(a._id)) throw Error("Missing distance for " + a._id); if (!distances.has(b._id)) throw Error("Missing distance for " + b._id); return distances.get(a._id) - distances.get(b._id); - }]); + }; }, _getPaths: function () {