From f27fe2c9a4ac2b4c259c1e4e7643da01f54d377c Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Tue, 13 Jun 2017 10:14:19 -0400 Subject: [PATCH 1/3] Adjust mongo upserts so MongoID.ObjectID's aren't filtered from queries. --- packages/minimongo/minimongo_tests.js | 24 +++++++++++++++++++++--- packages/minimongo/selector.js | 22 +++++++++++++++------- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index b541915d8a..6a800c2f7d 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2495,13 +2495,13 @@ Tinytest.add("minimongo - modify", function (test) { {b: [9,9]}]}, {'a.b': {$near: [5, 5]}}, {$set: {'a.$.b': 'k'}}, - {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); + {"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]}]}); + {"a":[{"b":"k"},{"b":[[3,3],[4,4]]},{"b":[9,9]}]}); modifyWithQuery({a: [{b: [1,1]}, {b: [ [3,3], [4,4] ]}, {b: [9,9]}]}, @@ -2524,7 +2524,7 @@ Tinytest.add("minimongo - modify", function (test) { {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]}]}); + {"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]}]}, @@ -2864,6 +2864,24 @@ Tinytest.add("minimongo - modify", function (test) { { a: 123 } ); + // Tests for https://github.com/meteor/meteor/issues/8794. + const testObjectId = new MongoID.ObjectID(); + upsert( + { _id: testObjectId }, + { $setOnInsert: { a: 123 } }, + { _id: testObjectId, a: 123 }, + ); + upsert( + { someOtherId: testObjectId }, + { $setOnInsert: { a: 123 } }, + { someOtherId: testObjectId, a: 123 }, + ); + upsert( + { a: { $eq: testObjectId } }, + { $setOnInsert: { a: 123 } }, + { a: 123 }, + ); + exception({}, {$set: {_id: 'bad'}}); // $bit diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index 777db112d4..15fb94d61b 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -441,7 +441,7 @@ var VALUE_OPERATORS = { // There are two kinds of geodata in MongoDB: legacy coordinate pairs and // GeoJSON. They use different distance metrics, too. GeoJSON queries are - // marked with a $geometry property, though legacy coordinates can be + // marked with a $geometry property, though legacy coordinates can be // matched using $geometry. var maxDistance, point, distance; @@ -1251,11 +1251,19 @@ LocalCollection._f = { } }; -// Oddball function used by upsert. +// Oddball function used by upsert. Make sure that when we're removing +// $ operators we don't accidently remove the "oid" (MongoID.ObjectID) +// EJSON type (which includes $type and $value properties). LocalCollection._removeDollarOperators = function (selector) { - return JSON.parse(JSON.stringify(selector, (key, value) => { - if (! key.startsWith("$")) { - return value; - } - })); + return EJSON.parse( + JSON.stringify( + EJSON.toJSONValue(selector), + function (key, value) { + if (EJSON.fromJSONValue(this) instanceof MongoID.ObjectID + || !key.startsWith("$")) { + return value; + } + } + ) + ); }; From a3d26855f92e8094875d6974cbbe3438b8fb7ddf Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Tue, 13 Jun 2017 15:32:45 -0400 Subject: [PATCH 2/3] Adjusted _removeDollarOperators to better handle EJSON types. --- packages/minimongo/minimongo_tests.js | 19 ++++++++++++++++ packages/minimongo/selector.js | 31 +++++++++++++++------------ 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 6a800c2f7d..75c26b8731 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2881,6 +2881,25 @@ Tinytest.add("minimongo - modify", function (test) { { $setOnInsert: { a: 123 } }, { a: 123 }, ); + const testDate = new Date('2017-01-01'); + upsert( + { someDate: testDate }, + { $setOnInsert: { a: 123 } }, + { someDate: testDate, a: 123 }, + ); + upsert( + { + a: Object.create(null, { + $exists: { + writable: true, + configurable: true, + value: true + } + }), + }, + { $setOnInsert: { a: 123 } }, + { a: 123 }, + ); exception({}, {$set: {_id: 'bad'}}); diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index 15fb94d61b..02a702fb10 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -1251,19 +1251,22 @@ LocalCollection._f = { } }; -// Oddball function used by upsert. Make sure that when we're removing -// $ operators we don't accidently remove the "oid" (MongoID.ObjectID) -// EJSON type (which includes $type and $value properties). -LocalCollection._removeDollarOperators = function (selector) { - return EJSON.parse( - JSON.stringify( - EJSON.toJSONValue(selector), - function (key, value) { - if (EJSON.fromJSONValue(this) instanceof MongoID.ObjectID - || !key.startsWith("$")) { - return value; - } +// Oddball function used by upsert. +LocalCollection._removeDollarOperators = (selector) => { + const cleansed = {}; + + Object.keys(selector).forEach((key) => { + const value = selector[key]; + if (key.charAt(0) !== '$') { + if (value !== null + && value.constructor + && value.constructor.prototype === Object.prototype) { + cleansed[key] = LocalCollection._removeDollarOperators(value); + } else { + cleansed[key] = value; } - ) - ); + } + }); + + return cleansed; }; From 28114a86ff3021873aff3854b5b23e4224886c3e Mon Sep 17 00:00:00 2001 From: Hugh Willson Date: Thu, 15 Jun 2017 11:43:07 -0400 Subject: [PATCH 3/3] Adjustments to make sure empty objects aren't saved when stripping $ keys. --- packages/minimongo/minimongo_tests.js | 10 ++++++++++ packages/minimongo/selector.js | 22 ++++++++++++++++------ 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js index 75c26b8731..8e85819d29 100644 --- a/packages/minimongo/minimongo_tests.js +++ b/packages/minimongo/minimongo_tests.js @@ -2900,6 +2900,16 @@ Tinytest.add("minimongo - modify", function (test) { { $setOnInsert: { a: 123 } }, { a: 123 }, ); + upsert( + { foo: { $exists: true, $type: 2 }}, + { $setOnInsert: { bar: 'baz' } }, + { bar: 'baz' } + ); + upsert( + { foo: {} }, + { $setOnInsert: { bar: 'baz' } }, + { foo: {}, bar: 'baz' } + ); exception({}, {$set: {_id: 'bad'}}); diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js index 02a702fb10..affe510ebf 100644 --- a/packages/minimongo/selector.js +++ b/packages/minimongo/selector.js @@ -1251,22 +1251,32 @@ LocalCollection._f = { } }; -// Oddball function used by upsert. -LocalCollection._removeDollarOperators = (selector) => { - const cleansed = {}; +const objectOnlyHasDollarKeys = (object) => { + const keys = Object.keys(object); + return keys.length > 0 && keys.every(key => key.charAt(0) === '$'); +}; +// When performing an upsert, the incoming selector object can be re-used as +// the upsert modifier object, as long as Mongo query and projection +// operators (prefixed with a $ character) are removed from the newly +// created modifier object. This function attempts to strip all $ based Mongo +// operators when creating the upsert modifier object. +// NOTE: There is a known issue here in that some Mongo $ based opeartors +// should not actually be stripped. +// See https://github.com/meteor/meteor/issues/8806. +LocalCollection._removeDollarOperators = (selector) => { + let cleansed = {}; Object.keys(selector).forEach((key) => { const value = selector[key]; - if (key.charAt(0) !== '$') { + if (key.charAt(0) !== '$' && !objectOnlyHasDollarKeys(value)) { if (value !== null && value.constructor - && value.constructor.prototype === Object.prototype) { + && Object.getPrototypeOf(value) === Object.prototype) { cleansed[key] = LocalCollection._removeDollarOperators(value); } else { cleansed[key] = value; } } }); - return cleansed; };