Merge pull request #8796 from hwillson/issue-8794

Adjust mongo upserts so MongoID.ObjectID's aren't filtered from queries.

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,53 @@ 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 },
+  );
+  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 },
+  );
+  upsert(
+    { foo: { $exists: true, $type: 2 }},
+    { $setOnInsert: { bar: 'baz' } },
+    { bar: 'baz' }
+  );
+  upsert(
+    { foo: {} },
+    { $setOnInsert: { bar: 'baz' } },
+    { foo: {}, bar: 'baz' }
+  );
+
   exception({}, {$set: {_id: 'bad'}});
 
   // $bit

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,32 @@ LocalCollection._f = {
   }
 };
 
-// Oddball function used by upsert.
-LocalCollection._removeDollarOperators = function (selector) {
-  return JSON.parse(JSON.stringify(selector, (key, value) => {
-    if (! key.startsWith("$")) {
-      return value;
-    }
-  }));
+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) !== '$' && !objectOnlyHasDollarKeys(value)) {
+      if (value !== null
+          && value.constructor
+          && Object.getPrototypeOf(value) === Object.prototype) {
+        cleansed[key] = LocalCollection._removeDollarOperators(value);
+      } else {
+        cleansed[key] = value;
+      }
+    }
+  });
+  return cleansed;
 };