From dda994c26247f5b8eeb7fc789735f05e94d82b0e Mon Sep 17 00:00:00 2001
From: Seba Kerckhof <seba.kerckhof@gmail.com>
Date: Sat, 17 Jun 2017 11:33:35 +0200
Subject: [PATCH 01/22] Make minimongo upsert compliant with mongo behavior
 (https://github.com/meteor/meteor/issues/8806)

---
 packages/minimongo/minimongo.js       |  36 ++++----
 packages/minimongo/minimongo_tests.js |  58 ++++++++++++
 packages/minimongo/package.js         |   1 +
 packages/minimongo/selector.js        |  32 +------
 packages/minimongo/upsert_document.js | 123 ++++++++++++++++++++++++++
 packages/mongo/mongo_driver.js        |  65 ++++----------
 6 files changed, 220 insertions(+), 95 deletions(-)
 create mode 100644 packages/minimongo/upsert_document.js

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index b97663a8a7..59229269bc 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -783,22 +783,7 @@ LocalCollection.prototype.update = function (selector, mod, options, callback) {
   var insertedId;
   if (updateCount === 0 && options.upsert) {
 
-    let selectorModifier = LocalCollection._selectorIsId(selector) 
-      ? { _id: selector } 
-      : selector;
-
-    selectorModifier = LocalCollection._removeDollarOperators(selectorModifier);
-
-    const newDoc = {};
-    if (selectorModifier._id) {
-      newDoc._id = selectorModifier._id;
-      delete selectorModifier._id;
-    }
-
-    // This double _modify call is made to help work around an issue where collection 
-    // upserts won't work properly, with nested properties (see issue #8631).
-    LocalCollection._modify(newDoc, {$set: selectorModifier});
-    LocalCollection._modify(newDoc, mod, {isInsert: true});
+    const newDoc = LocalCollection._createUpsertDocument(selector, mod);
 
     if (! newDoc._id && options.insertedId)
       newDoc._id = options.insertedId;
@@ -1132,3 +1117,22 @@ LocalCollection.prototype.resumeObservers = function () {
   }
   self._observeQueue.drain();
 };
+
+
+// Calculates the document to insert in case we're doing an upsert and the selector
+// does not match any elements
+LocalCollection._createUpsertDocument = function (selector, modifier) {
+    let selectorDocument = populateDocumentWithQueryFields(selector);
+
+    const newDoc = {};
+    if (selectorDocument._id) {
+      newDoc._id = selectorDocument._id;
+      delete selectorDocument._id;
+    }
+
+    // This double _modify call is made to help with nested properties (see issue #8631).
+    LocalCollection._modify(newDoc, {$set: selectorDocument});
+    LocalCollection._modify(newDoc, modifier, {isInsert: true});
+
+    return newDoc;
+};
diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js
index 8e85819d29..96525d8b0c 100644
--- a/packages/minimongo/minimongo_tests.js
+++ b/packages/minimongo/minimongo_tests.js
@@ -2911,6 +2911,64 @@ Tinytest.add("minimongo - modify", function (test) {
     { foo: {}, bar: 'baz' }
   );
 
+
+  // Tests for https://github.com/meteor/meteor/issues/8806
+
+  upsert({"a": {"b": undefined, "c": null}}, {"$set": {"c": "foo"}}, {"a": {"b": undefined, "c": null}, "c": "foo"})
+  upsert({"a": {"$eq": "bar" }}, {"$set": {"c": "foo"}}, {"a": "bar", "c": "foo"})
+  // $all with 1 statement is similar to $eq
+  upsert({"a": {"$all": ["bar"] }}, {"$set": {"c": "foo"}}, {"a": "bar", "c": "foo"})
+  upsert({"a": {"$eq": "bar" }, "b": "baz"}, {"$set": {"c": "foo"}}, {"a": "bar", "b": "baz", "c": "foo"})
+
+  upsert({"a": {"$exists": true}}, {"$set": {"c": "foo"}}, {"c": "foo"})
+  upsert({"a": {"$exists": true, "$eq": "foo"}}, {"$set": {"c": "foo"}}, {"a": "foo", "c": "foo"})
+  upsert({"a": {"$gt": 3, "$eq": 2}}, {"$set": {"c": "foo"}}, {"a": 2, "c": "foo"})
+
+  // $and
+  upsert({"$and": [{"a": {"$eq": "bar"}}]}, {"$set": {"c": "foo"}}, {"a": "bar", "c": "foo"})
+  upsert({"$and": [{"a": {"$all": ["bar"]}}]}, {"$set": {"c": "foo"}}, {"a": "bar", "c": "foo"})
+  upsert({"$and": [{"a": {"$all": ["bar"]}}]}, {"$set": {"c": "foo"}}, {"a": "bar", "c": "foo"})
+
+  // $or with one statement is handled similar to $and
+  upsert({"$or": [{"a": "bar"}]}, {"$set": {"c": "foo"}}, {"a": "bar", "c": "foo"})
+
+  // $or with multiple statements is ignored
+  upsert({"$or": [{"a": "bar"}, {"b": "baz"}]}, {"$set": {"c": "foo"}}, {"c": "foo"})
+
+  // Negative logical operators are ignored
+  upsert({"$nor": [{"a": "bar"}]}, {"$set": {"c": "foo"}}, {"c": "foo"})
+
+  // Filter out empty objects after filtering out operators
+  upsert({"a": {"$exists": true}}, {"$set": {"c": "foo"}}, {"c": "foo"})
+
+  // But leave actual empty objects
+  upsert({"a": {}}, {"$set": {"c": "foo"}}, {"a": {}, "c": "foo"})
+
+  // Test nested fields
+  upsert({"$and": [{"a.a": "foo"}, {"$or": [{"a.b": "baz"}]}]}, {"$set": {"c": "foo"}}, {"a": {"a": "foo", "b": "baz"}, "c": "foo"})
+
+  // Nested fields don't work with literal objects
+  upsertException({"a": {}, "a.b": "foo"}, {});
+
+  // You can't set the same field twice
+  upsertException({"$and": [{"a": "foo"}, {"a": "foo"}]}, {}); //not even with same value
+  upsertException({"a": {"$all": ["foo", "bar"]}}, {});
+  upsertException({"$and": [{"a": {"$eq": "foo"}}, {"$or": [{"a": {"$all": ["bar"]}}]}]}, {});
+
+  // You can't have nested dotted fields
+  upsertException({"a": {"foo.bar": "baz"}}, {});
+
+  // You can't have dollar-prefixed fields above the first level (logical operators not counted)
+  upsertException({"a": {"a": {"$eq": "foo"}}}, {});
+  upsertException({"a": {"a": {"$exists": true}}}, {});
+
+  // You can't mix operators with other fields
+  upsertException({"a": {"$eq": "bar", "b": "foo"}}, {})
+  upsertException({"a": {"b": "foo", "$eq": "bar"}}, {})
+
+  var mongoIdForUpsert = new MongoID.ObjectID();
+  upsert({_id: mongoIdForUpsert},{$setOnInsert:{a:123}},{a:123})
+
   exception({}, {$set: {_id: 'bad'}});
 
   // $bit
diff --git a/packages/minimongo/package.js b/packages/minimongo/package.js
index c389236fd5..e576c8ecf0 100644
--- a/packages/minimongo/package.js
+++ b/packages/minimongo/package.js
@@ -28,6 +28,7 @@ Package.onUse(function (api) {
     'minimongo.js',
     'wrap_transform.js',
     'helpers.js',
+    'upsert_document.js',
     'selector.js',
     'sort.js',
     'projection.js',
diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js
index affe510ebf..67bf94a167 100644
--- a/packages/minimongo/selector.js
+++ b/packages/minimongo/selector.js
@@ -1249,34 +1249,4 @@ LocalCollection._f = {
       throw Error("Sorting not supported on Javascript code"); // XXX
     throw Error("Unknown type to sort");
   }
-};
-
-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;
-};
+};
\ No newline at end of file
diff --git a/packages/minimongo/upsert_document.js b/packages/minimongo/upsert_document.js
new file mode 100644
index 0000000000..f0a55aa476
--- /dev/null
+++ b/packages/minimongo/upsert_document.js
@@ -0,0 +1,123 @@
+// Creating a document from an upsert is quite tricky.
+// E.g. this selector: {"$or": [{"b.foo": {"$all": ["bar"]}}]}, should result in: {"b.foo": "bar"}
+// But this selector: {"$or": [{"b": {"foo": {"$all": ["bar"]}}}]} should throw an error
+
+// Some rules (found mainly with trial & error, so there might be more):
+// - handle all childs of $and (or implicit $and)
+// - handle $or nodes with exactly 1 child
+// - ignore $or nodes with more than 1 child
+// - ignore $nor and $not nodes
+// - throw when a value can not be set unambiguously
+// - every value for $all should be dealt with as separate $eq-s
+// - threat all children of $all as $eq setters (=> set if $all.length === 1, otherwise throw error)
+// - you can not mix '$'-prefixed keys and non-'$'-prefixed keys
+// - you can only have dotted keys on a root-level
+// - you can not have '$'-prefixed keys more than one-level deep in an object
+
+// Fills a document with certain fields from an upsert selector
+populateDocumentWithQueryFields = function (query, document = {}) {
+
+    if (Object.getPrototypeOf(query) === Object.prototype) {
+        // handle implicit $and
+        for (let [key, value] of Object.entries(query)) {
+            // handle explicit $and
+            if (key === "$and") {
+                value.forEach(sq => populateDocumentWithQueryFields(sq, document))
+            }
+            // handle $or nodes with exactly 1 child
+            else if (key === "$or") {
+                if (value.length === 1) {
+                    populateDocumentWithQueryFields(value[0], document)
+                }
+            }
+            //Ignore other '$'-prefixed logical selectors
+            else if (key[0] !== "$") {
+                populateDocumentWithKeyValue(document, key, value);
+            }
+        }
+    } else {
+        // Handle meteor-specific shortcut for selecting _id
+        if (LocalCollection._selectorIsId(query)) {
+            insertIntoDocument(document, "_id", query);
+        }
+    }
+
+    return document;
+
+}
+
+// Handles one key/value pair to put in the selector document
+function populateDocumentWithKeyValue(document, key, value) {
+    if (value && Object.getPrototypeOf(value) === Object.prototype) {
+        populateDocumentWithObject(document, key, value);
+    } else {
+        insertIntoDocument(document, key, value);
+    }
+}
+
+
+// Handles a key, value to put in the selector document if the value is an object
+function populateDocumentWithObject(document, key, value) {
+    const keys = Object.keys(value);
+    const unprefixedKeys = keys.filter(k => k[0] !== "$");
+
+    if (unprefixedKeys.length > 0 || !keys.length) {
+        //Literal (possibly empty) object ( or empty object ) 
+        //Don't allow mixing '$'-prefixed with non-'$'-prefixed fields
+        if (keys.length !== unprefixedKeys.length) {
+            throw new Error(`unknown operator: ${unprefixedKeys[0]}`)
+        } else {
+            validateObject(value, key);
+            insertIntoDocument(document, key, value);
+        }
+    } else {
+        for (let [k, v] of Object.entries(value)) {
+            if (k === "$eq") {
+                populateDocumentWithKeyValue(document, key, v);
+            }
+            //every value for $all should be dealt with as separate $eq-s
+            else if (k === "$all") {
+                v.forEach(vx => populateDocumentWithKeyValue(document, key, vx))
+            }
+        }
+    }
+}
+
+// Actually inserts a key value into the selector document
+// However, this checks there is no ambiguity in setting the value for the given key, throws otherwise
+function insertIntoDocument(document, key, value) {
+
+    Object.keys(document).forEach(existingKey => {
+        if (
+            (existingKey.length > key.length && existingKey.indexOf(key) === 0)
+            || (key.length > existingKey.length && key.indexOf(existingKey) === 0)
+        ) {
+            throw new Error(`cannot infer query fields to set, both paths '${existingKey}' and '${key}' are matched`)
+        }
+        else if (existingKey === key) {
+            throw new Error(`cannot infer query fields to set, path ${key} is matched twice`)
+        }
+    })
+
+    document[key] = value;
+}
+
+// Recursively validates an object that is nested more than one level deep
+function validateObject(obj, path) {
+    if (obj && Object.getPrototypeOf(obj) === Object.prototype) {
+        for (let [key, value] of Object.entries(obj)) {
+            validateKeyInPath(key, path);
+            validateObject(value, path + "." + key);
+        }
+    }
+}
+
+// Validates the key in a path. Objects that are nested more then 1 level cannot have dotted fields or fields starting with '$'
+function validateKeyInPath(key, path) {
+    if (key.includes(".")) {
+        throw new Error(`The dotted field '${key}' in '${path}.${key}' is not valid for storage.`)
+    }
+    if (key[0] === "$") {
+        throw new Error(`The dollar ($) prefixed field  '${path}.${key}' is not valid for storage.`)
+    }
+}
\ No newline at end of file
diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index 9922b0394d..582cf8e8b9 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -512,7 +512,21 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
     var mongoMod = replaceTypes(mod, replaceMeteorAtomWithMongo);
 
     var isModify = isModificationMod(mongoMod);
-    var knownId = selector._id || mod._id;
+
+    var newDoc;
+    // Run this code up front so that it fails fast if someone uses
+    // a Mongo update operator we don't support.
+    if (isModify) {
+      // We've already run replaceTypes/replaceMeteorAtomWithMongo on
+      // selector and mod.  We assume it doesn't matter, as far as
+      // the behavior of modifiers is concerned, whether `_modify`
+      // is run on EJSON or on mongo-converted EJSON.
+      newDoc = LocalCollection._createUpsertDocument(selector, mod);
+    } else {
+      newDoc = mod;
+    }
+
+    var knownId = newDoc._id;
 
     if (options._forbidReplace && ! isModify) {
       var err = new Error("Invalid modifier. Replacements are forbidden.");
@@ -538,7 +552,7 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
       //     https://github.com/meteor/meteor/issues/2278#issuecomment-64252706
       simulateUpsertWithInsertedId(
         collection, mongoSelector, mongoMod,
-        isModify, options,
+        newDoc, options,
         // This callback does not need to be bindEnvironment'ed because
         // simulateUpsertWithInsertedId() wraps it and then passes it through
         // bindEnvironmentForWrite.
@@ -653,7 +667,7 @@ MongoConnection._isCannotChangeIdError = function (err) {
 };
 
 var simulateUpsertWithInsertedId = function (collection, selector, mod,
-                                             isModify, options, callback) {
+                                             newDoc, options, callback) {
   // STRATEGY:  First try doing a plain update.  If it affected 0 documents,
   // then without affecting the database, we know we should probably do an
   // insert.  We then do a *conditional* insert that will fail in the case
@@ -668,51 +682,6 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod,
   // disagreeing on the existence of an object will cause writes to fail
   // in theory).
 
-  var newDoc;
-  // Run this code up front so that it fails fast if someone uses
-  // a Mongo update operator we don't support.
-  if (isModify) {
-    // We've already run replaceTypes/replaceMeteorAtomWithMongo on
-    // selector and mod.  We assume it doesn't matter, as far as
-    // the behavior of modifiers is concerned, whether `_modify`
-    // is run on EJSON or on mongo-converted EJSON.
-    var selectorDoc = LocalCollection._removeDollarOperators(selector);
-
-    newDoc = selectorDoc;
-
-    // Convert dotted keys into objects. (Resolves issue #4522).
-    _.each(newDoc, function (value, key) {
-      var trail = key.split(".");
-
-      if (trail.length > 1) {
-        //Key is dotted. Convert it into an object.
-        delete newDoc[key];
-
-        var obj = newDoc,
-            leaf = trail.pop();
-
-        // XXX It is not quite certain what should be done if there are clashing
-        // keys on the trail of the dotted key. For now we will just override it
-        // It wouldn't be a very sane query in the first place, but should look
-        // up what mongo does in this case.
-
-        while ((key = trail.shift())) {
-          if (typeof obj[key] !== "object") {
-            obj[key] = {};
-          }
-
-          obj = obj[key];
-        }
-
-        obj[leaf] = value;
-      }
-    });
-
-    LocalCollection._modify(newDoc, mod, {isInsert: true});
-  } else {
-    newDoc = mod;
-  }
-
   var insertedId = options.insertedId; // must exist
   var mongoOptsForUpdate = {
     safe: true,

From 29b6913f3113b5529c2026a06e86144bbb98c580 Mon Sep 17 00:00:00 2001
From: Seba Kerckhof <seba.kerckhof@gmail.com>
Date: Sat, 17 Jun 2017 13:18:02 +0200
Subject: [PATCH 02/22] Add test for
 https://github.com/meteor/meteor/issues/5294

---
 packages/minimongo/minimongo_tests.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js
index 96525d8b0c..2f7c25ce2a 100644
--- a/packages/minimongo/minimongo_tests.js
+++ b/packages/minimongo/minimongo_tests.js
@@ -2947,6 +2947,9 @@ Tinytest.add("minimongo - modify", function (test) {
   // Test nested fields
   upsert({"$and": [{"a.a": "foo"}, {"$or": [{"a.b": "baz"}]}]}, {"$set": {"c": "foo"}}, {"a": {"a": "foo", "b": "baz"}, "c": "foo"})
 
+  // Test for https://github.com/meteor/meteor/issues/5294
+  upsert({"a": {"$ne": 444}},{"$push": {"a": 123}}, {"a": [123]})
+
   // Nested fields don't work with literal objects
   upsertException({"a": {}, "a.b": "foo"}, {});
 
@@ -2967,7 +2970,7 @@ Tinytest.add("minimongo - modify", function (test) {
   upsertException({"a": {"b": "foo", "$eq": "bar"}}, {})
 
   var mongoIdForUpsert = new MongoID.ObjectID();
-  upsert({_id: mongoIdForUpsert},{$setOnInsert:{a:123}},{a:123})
+  upsert({_id: mongoIdForUpsert},{$setOnInsert: {a: 123}},{a: 123})
 
   exception({}, {$set: {_id: 'bad'}});
 

From 3580edfeaee9e7248c71462869e8cd027b285958 Mon Sep 17 00:00:00 2001
From: Seba Kerckhof <seba.kerckhof@gmail.com>
Date: Sat, 17 Jun 2017 13:35:17 +0200
Subject: [PATCH 03/22] Deal with regexp shorthand notation

---
 packages/minimongo/minimongo_tests.js | 11 ++++++++++-
 packages/minimongo/upsert_document.js |  2 +-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js
index 2f7c25ce2a..d13315f86f 100644
--- a/packages/minimongo/minimongo_tests.js
+++ b/packages/minimongo/minimongo_tests.js
@@ -2944,6 +2944,9 @@ Tinytest.add("minimongo - modify", function (test) {
   // But leave actual empty objects
   upsert({"a": {}}, {"$set": {"c": "foo"}}, {"a": {}, "c": "foo"})
 
+   // Also filter out shorthand regexp notation
+  upsert({"a": /a/}, {"$set": {"c": "foo"}}, {"c": "foo"})
+
   // Test nested fields
   upsert({"$and": [{"a.a": "foo"}, {"$or": [{"a.b": "baz"}]}]}, {"$set": {"c": "foo"}}, {"a": {"a": "foo", "b": "baz"}, "c": "foo"})
 
@@ -2969,9 +2972,15 @@ Tinytest.add("minimongo - modify", function (test) {
   upsertException({"a": {"$eq": "bar", "b": "foo"}}, {})
   upsertException({"a": {"b": "foo", "$eq": "bar"}}, {})
 
-  var mongoIdForUpsert = new MongoID.ObjectID();
+  var mongoIdForUpsert = new MongoID.ObjectID('44915733af80844fa1cef07a');
   upsert({_id: mongoIdForUpsert},{$setOnInsert: {a: 123}},{a: 123})
 
+  // Test for https://github.com/meteor/meteor/issues/7758
+  upsert({n_id: mongoIdForUpsert, c_n: "bar"},
+    {$set: { t_t_o: "foo"}},
+    {n_id: mongoIdForUpsert, t_t_o: "foo", c_n: "bar"});
+
+
   exception({}, {$set: {_id: 'bad'}});
 
   // $bit
diff --git a/packages/minimongo/upsert_document.js b/packages/minimongo/upsert_document.js
index f0a55aa476..f0dea52cf3 100644
--- a/packages/minimongo/upsert_document.js
+++ b/packages/minimongo/upsert_document.js
@@ -50,7 +50,7 @@ populateDocumentWithQueryFields = function (query, document = {}) {
 function populateDocumentWithKeyValue(document, key, value) {
     if (value && Object.getPrototypeOf(value) === Object.prototype) {
         populateDocumentWithObject(document, key, value);
-    } else {
+    } else if (!(value instanceof RegExp)) {
         insertIntoDocument(document, key, value);
     }
 }

From c02e96d82e9dea41d345e17328dafffb7ca2b32f Mon Sep 17 00:00:00 2001
From: Seba Kerckhof <seba.kerckhof@gmail.com>
Date: Mon, 19 Jun 2017 22:33:08 +0200
Subject: [PATCH 04/22] Create mongo-typed upsert document

---
 packages/minimongo/minimongo.js | 25 ++++++++++++++++++++++--
 packages/mongo/mongo_driver.js  | 34 +++++++--------------------------
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index 59229269bc..1af6547190 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -1118,10 +1118,30 @@ LocalCollection.prototype.resumeObservers = function () {
   self._observeQueue.drain();
 };
 
+LocalCollection._isModificationMod = function (mod) {
+  var isReplace = false;
+  var isModify = false;
+  for (var k in mod) {
+    if (k.substr(0, 1) === '$') {
+      isModify = true;
+    } else {
+      isReplace = true;
+    }
+  }
+  if (isModify && isReplace) {
+    throw new Error(
+      "Update parameter cannot have both modifier and non-modifier fields.");
+  }
+  return isModify;
+};
 
 // Calculates the document to insert in case we're doing an upsert and the selector
 // does not match any elements
 LocalCollection._createUpsertDocument = function (selector, modifier) {
+  if (!LocalCollection._isModificationMod(modifier)) {
+    return modifier;
+  } else {
+
     let selectorDocument = populateDocumentWithQueryFields(selector);
 
     const newDoc = {};
@@ -1131,8 +1151,9 @@ LocalCollection._createUpsertDocument = function (selector, modifier) {
     }
 
     // This double _modify call is made to help with nested properties (see issue #8631).
-    LocalCollection._modify(newDoc, {$set: selectorDocument});
-    LocalCollection._modify(newDoc, modifier, {isInsert: true});
+    LocalCollection._modify(newDoc, { $set: selectorDocument });
+    LocalCollection._modify(newDoc, modifier, { isInsert: true });
 
     return newDoc;
+  }
 };
diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index 582cf8e8b9..8c2bf72f88 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -513,18 +513,15 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
 
     var isModify = isModificationMod(mongoMod);
 
-    var newDoc;
+    var isModify = LocalCollection._isModificationMod(mongoMod);
+
+    // We've already run replaceTypes/replaceMeteorAtomWithMongo on
+    // selector and mod.  We assume it doesn't matter, as far as
+    // the behavior of modifiers is concerned, whether `_modify`
+    // is run on EJSON or on mongo-converted EJSON.
+    var newDoc = LocalCollection._createUpsertDocument(mongoSelector, mongoMod);
     // Run this code up front so that it fails fast if someone uses
     // a Mongo update operator we don't support.
-    if (isModify) {
-      // We've already run replaceTypes/replaceMeteorAtomWithMongo on
-      // selector and mod.  We assume it doesn't matter, as far as
-      // the behavior of modifiers is concerned, whether `_modify`
-      // is run on EJSON or on mongo-converted EJSON.
-      newDoc = LocalCollection._createUpsertDocument(selector, mod);
-    } else {
-      newDoc = mod;
-    }
 
     var knownId = newDoc._id;
 
@@ -596,23 +593,6 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
   }
 };
 
-var isModificationMod = function (mod) {
-  var isReplace = false;
-  var isModify = false;
-  for (var k in mod) {
-    if (k.substr(0, 1) === '$') {
-      isModify = true;
-    } else {
-      isReplace = true;
-    }
-  }
-  if (isModify && isReplace) {
-    throw new Error(
-      "Update parameter cannot have both modifier and non-modifier fields.");
-  }
-  return isModify;
-};
-
 var transformResult = function (driverResult) {
   var meteorResult = { numberAffected: 0 };
   if (driverResult) {

From 5153d0778e8eb591ca06aba0229b43e54d25f19e Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Thu, 22 Jun 2017 14:03:01 +0200
Subject: [PATCH 05/22] Add rules for ambiguous _id values

---
 packages/minimongo/minimongo.js       | 44 ++++++++++++++++-----------
 packages/minimongo/minimongo_tests.js |  9 ++++++
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index 1af6547190..b3bc6da0a5 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -1138,22 +1138,30 @@ LocalCollection._isModificationMod = function (mod) {
 // Calculates the document to insert in case we're doing an upsert and the selector
 // does not match any elements
 LocalCollection._createUpsertDocument = function (selector, modifier) {
-  if (!LocalCollection._isModificationMod(modifier)) {
-    return modifier;
-  } else {
-
-    let selectorDocument = populateDocumentWithQueryFields(selector);
-
-    const newDoc = {};
-    if (selectorDocument._id) {
-      newDoc._id = selectorDocument._id;
-      delete selectorDocument._id;
-    }
-
-    // This double _modify call is made to help with nested properties (see issue #8631).
-    LocalCollection._modify(newDoc, { $set: selectorDocument });
-    LocalCollection._modify(newDoc, modifier, { isInsert: true });
-
-    return newDoc;
+  let selectorDocument = populateDocumentWithQueryFields(selector)
+  const isModify = LocalCollection._isModificationMod(modifier)
+  
+  const newDoc = {}
+  if (selectorDocument._id) {
+    newDoc._id = selectorDocument._id
+    delete selectorDocument._id
   }
-};
+
+  // This double _modify call is made to help with nested properties (see issue #8631).
+  LocalCollection._modify(newDoc, { $set: selectorDocument })
+  LocalCollection._modify(newDoc, modifier, { isInsert: true })
+
+  if (selectorDocument._id && newDoc._id !== selectorDocument._id) {
+    if (isModify) {
+      throw new Error(`After applying the update to the document {_id: "${selectorDocument._id}" , ...}, the (immutable) field '_id' was found to have been altered to _id: "${newDoc._id}"`)
+    }else {
+      throw new Error(`The _id field cannot be changed from {_id: "${selectorDocument._id}"} to {_id: "${newDoc._id}"}`)
+    }
+  }
+
+  if (isModify) {
+    return newDoc
+  } else {
+    return modifier
+  }
+}
diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js
index d13315f86f..996745ca3c 100644
--- a/packages/minimongo/minimongo_tests.js
+++ b/packages/minimongo/minimongo_tests.js
@@ -2951,11 +2951,20 @@ Tinytest.add("minimongo - modify", function (test) {
   upsert({"$and": [{"a.a": "foo"}, {"$or": [{"a.b": "baz"}]}]}, {"$set": {"c": "foo"}}, {"a": {"a": "foo", "b": "baz"}, "c": "foo"})
 
   // Test for https://github.com/meteor/meteor/issues/5294
+  //
   upsert({"a": {"$ne": 444}},{"$push": {"a": 123}}, {"a": [123]})
 
+  // Mod takes precedence over query
+  upsert({"a": "foo"},{"a": "bar"}, {"a": "bar"})
+  upsert({"a": "foo"},{"$set":{"a": "bar"}}, {"a": "bar"})
+
   // Nested fields don't work with literal objects
   upsertException({"a": {}, "a.b": "foo"}, {});
 
+  // You can't have an ambiguious ID
+  upsertException({"_id":"foo"}, {"_id":"bar"});
+  upsertException({"_id":"foo"}, {"$set":{"_id":"bar"}});
+
   // You can't set the same field twice
   upsertException({"$and": [{"a": "foo"}, {"a": "foo"}]}, {}); //not even with same value
   upsertException({"a": {"$all": ["foo", "bar"]}}, {});

From bb3c02900f9a8f453e8270b775a05c7c6cbb0a0d Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Thu, 22 Jun 2017 14:05:18 +0200
Subject: [PATCH 06/22] Remove mongo 2.4-specific check

---
 packages/mongo/mongo_driver.js | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index 8c2bf72f88..332b791d7d 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -620,24 +620,15 @@ var NUM_OPTIMISTIC_TRIES = 3;
 
 // exposed for testing
 MongoConnection._isCannotChangeIdError = function (err) {
-  // First check for what this error looked like in Mongo 2.4.  Either of these
-  // checks should work, but just to be safe...
-  if (err.code === 13596) {
-    return true;
-  }
 
   // Mongo 3.2.* returns error as next Object:
-  // {name: String, code: Number, err: String}
-  // Older Mongo returns:
   // {name: String, code: Number, errmsg: String}
+  // Older Mongo returns:
+  // {name: String, code: Number, err: String}
   var error = err.errmsg || err.err;
 
-  if (error.indexOf('cannot change _id of a document') === 0) {
-    return true;
-  }
-
-  // Now look for what it looks like in Mongo 2.6.  We don't use the error code
-  // here, because the error code we observed it producing (16837) appears to be
+  // We don't use the error code here
+  // because the error code we observed it producing (16837) appears to be
   // a far more generic error code based on examining the source.
   if (error.indexOf('The _id field cannot be changed') === 0) {
     return true;

From 2c9d46de19c925a9a304a1dd21ad9282a6a0c5d5 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Fri, 23 Jun 2017 00:23:34 +0200
Subject: [PATCH 07/22] Move _id change validation to LocalCollection._modify

---
 packages/minimongo/minimongo.js | 13 +++++--------
 packages/minimongo/modify.js    | 15 ++++++++-------
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index b3bc6da0a5..5997e80dd9 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -1148,20 +1148,17 @@ LocalCollection._createUpsertDocument = function (selector, modifier) {
   }
 
   // This double _modify call is made to help with nested properties (see issue #8631).
+  // We do this even if it's a replacement for validation purposes (e.g. ambiguous id's)
   LocalCollection._modify(newDoc, { $set: selectorDocument })
   LocalCollection._modify(newDoc, modifier, { isInsert: true })
 
-  if (selectorDocument._id && newDoc._id !== selectorDocument._id) {
-    if (isModify) {
-      throw new Error(`After applying the update to the document {_id: "${selectorDocument._id}" , ...}, the (immutable) field '_id' was found to have been altered to _id: "${newDoc._id}"`)
-    }else {
-      throw new Error(`The _id field cannot be changed from {_id: "${selectorDocument._id}"} to {_id: "${newDoc._id}"}`)
-    }
-  }
-
   if (isModify) {
     return newDoc
   } else {
+    // Replacement can take _id from query document
+    if (newDoc._id) {
+      modifier._id = newDoc._id;
+    }
     return modifier
   }
 }
diff --git a/packages/minimongo/modify.js b/packages/minimongo/modify.js
index e0df49a274..dafb84fe66 100644
--- a/packages/minimongo/modify.js
+++ b/packages/minimongo/modify.js
@@ -25,9 +25,10 @@ LocalCollection._modify = function (doc, mod, options) {
   var newDoc;
 
   if (!isModifier) {
-    if (mod._id && !EJSON.equals(doc._id, mod._id))
-      throw MinimongoError("Cannot change the _id of a document");
-
+    if (mod._id && doc._id && !EJSON.equals(doc._id, mod._id)) {
+      throw MinimongoError(`The _id field cannot be changed from {_id: "${doc._id}"} to {_id: "${mod._id}"}`);
+    }
+    
     // replace the whole document
     assertHasValidFieldNames(mod);
     newDoc = mod;
@@ -47,10 +48,6 @@ LocalCollection._modify = function (doc, mod, options) {
           throw MinimongoError("An empty update path is not valid.");
         }
 
-        if (keypath === '_id' && op !== '$setOnInsert') {
-          throw MinimongoError("Mod on _id not allowed");
-        }
-
         var keyparts = keypath.split('.');
 
         if (! _.all(keyparts, _.identity)) {
@@ -70,6 +67,10 @@ LocalCollection._modify = function (doc, mod, options) {
         modFunc(target, field, arg, keypath, newDoc);
       });
     });
+
+    if (doc._id && !EJSON.equals(doc._id, newDoc._id)) {
+      throw MinimongoError(`After applying the update to the document {_id: "${doc._id}" , ...}, the (immutable) field '_id' was found to have been altered to _id: "${newDoc._id}"`);
+    }
   }
 
   // move new document into place.

From dac2f64d59bad4561812f17f96bea5d88ec27995 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Fri, 23 Jun 2017 00:24:15 +0200
Subject: [PATCH 08/22] Track wether the _id is auto generated when doing an
 upsert

---
 packages/mongo/collection.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/mongo/collection.js b/packages/mongo/collection.js
index 4bb0993f1b..b22eb0248a 100644
--- a/packages/mongo/collection.js
+++ b/packages/mongo/collection.js
@@ -562,6 +562,7 @@ Mongo.Collection.prototype.update = function update(selector, modifier, ...optio
       insertedId = options.insertedId;
     } else if (!selector || !selector._id) {
       insertedId = this._makeNewID();
+      options.generatedId = true;
       options.insertedId = insertedId;
     }
   }

From 1205655f21a11d7e3bb3261b14613028067a8b4d Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Fri, 23 Jun 2017 00:25:00 +0200
Subject: [PATCH 09/22] Expand minimongo tests

---
 packages/minimongo/minimongo_tests.js | 36 ++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js
index 996745ca3c..6623d4619b 100644
--- a/packages/minimongo/minimongo_tests.js
+++ b/packages/minimongo/minimongo_tests.js
@@ -2340,7 +2340,10 @@ Tinytest.add("minimongo - modify", function (test) {
     // The query is relevant for 'a.$.b'.
     coll.update(query, mod);
     var actual = coll.findOne();
-    delete actual._id;  // added by insert
+
+    if(!expected._id){
+      delete actual._id;  // added by insert
+    }
 
     if (typeof expected === "function") {
       expected(actual, EJSON.stringify({input: doc, mod: mod}));
@@ -2379,6 +2382,21 @@ Tinytest.add("minimongo - modify", function (test) {
     test.equal(actual, expected);
   };
 
+  var upsertUpdate = function (initialDoc, query, mod, expected) {
+    var coll = new LocalCollection;
+
+    coll.insert(initialDoc);
+    var result = coll.upsert(query, mod);
+
+    var actual = coll.findOne();
+
+    if (!expected._id) {
+      delete actual._id;
+    }
+
+    test.equal(actual, expected);
+  }
+
   var upsertException = function (query, mod) {
     var coll = new LocalCollection;
     test.throws(function(){
@@ -2608,8 +2626,12 @@ Tinytest.add("minimongo - modify", function (test) {
   modify({a: [1], b: 2}, {$set: {'a.2': 9}}, {a: [1, null, 9], b: 2});
   modify({a: {b: 1}}, {$set: {'a.c': 9}}, {a: {b: 1, c: 9}});
   modify({}, {$set: {'x._id': 4}}, {x: {_id: 4}});
+  
+  // Changing _id is disallowed
   exception({}, {$set: {_id: 4}});
-  exception({_id: 4}, {$set: {_id: 4}});  // even not-changing _id is bad
+  exception({_id: 1}, {$set: {_id: 4}})
+  modify({_id: 4}, {$set: {_id: 4}}, {_id: 4});  // not-changing _id is not bad
+  
   //restricted field names
   exception({a:{}}, {$set:{a:{$a:1}}});
   exception({ a: {} }, { $set: { a: { c:
@@ -2951,12 +2973,14 @@ Tinytest.add("minimongo - modify", function (test) {
   upsert({"$and": [{"a.a": "foo"}, {"$or": [{"a.b": "baz"}]}]}, {"$set": {"c": "foo"}}, {"a": {"a": "foo", "b": "baz"}, "c": "foo"})
 
   // Test for https://github.com/meteor/meteor/issues/5294
-  //
-  upsert({"a": {"$ne": 444}},{"$push": {"a": 123}}, {"a": [123]})
+  upsert({"a": {"$ne": 444}}, {"$push": {"a": 123}}, {"a": [123]})
 
   // Mod takes precedence over query
-  upsert({"a": "foo"},{"a": "bar"}, {"a": "bar"})
-  upsert({"a": "foo"},{"$set":{"a": "bar"}}, {"a": "bar"})
+  upsert({"a": "foo"}, {"a": "bar"}, {"a": "bar"})
+  upsert({"a": "foo"}, {"$set":{"a": "bar"}}, {"a": "bar"})
+
+  // Replacement can take _id from query
+  upsert({"_id": "foo", "foo": "bar"}, {"bar": "foo"}, {"_id": "foo", "bar": "foo"})
 
   // Nested fields don't work with literal objects
   upsertException({"a": {}, "a.b": "foo"}, {});

From e06f88bdd42e869f9100a7d77136570c9ff0d4de Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Fri, 23 Jun 2017 00:40:55 +0200
Subject: [PATCH 10/22] Update mongo driver test

---
 packages/mongo/mongo_livedata_tests.js | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packages/mongo/mongo_livedata_tests.js b/packages/mongo/mongo_livedata_tests.js
index 1e1ea0174e..5b725e44d2 100644
--- a/packages/mongo/mongo_livedata_tests.js
+++ b/packages/mongo/mongo_livedata_tests.js
@@ -1646,14 +1646,15 @@ if (Meteor.isServer) {
     var run = test.runId();
     var coll = new Mongo.Collection("livedata_upsert_errorparse_collection_"+run, collectionOptions);
 
-    coll.insert({_id: 'foobar'});
+    coll.insert({_id:'foobar', foo: 'bar'});
     var err;
     try {
-      coll.update({_id: 'foobar'}, {_id: 'cowbar'});
+      coll.update({foo: 'bar'}, {_id: 'cowbar'});
     } catch (e) {
       err = e;
     }
     test.isTrue(err);
+    console.log("WUUUK");
     test.isTrue(MongoInternals.Connection._isCannotChangeIdError(err));
 
     try {

From 1fae40b204b31bac38608c01d824c018dc58ffb4 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Fri, 23 Jun 2017 00:41:53 +0200
Subject: [PATCH 11/22] Change logic of simulated upsert to always result in
 same result as normal upsert

---
 packages/mongo/mongo_driver.js | 119 +++++++++++++++++++--------------
 1 file changed, 68 insertions(+), 51 deletions(-)

diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index 332b791d7d..10fd206590 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -511,21 +511,29 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
     var mongoSelector = replaceTypes(selector, replaceMeteorAtomWithMongo);
     var mongoMod = replaceTypes(mod, replaceMeteorAtomWithMongo);
 
-    var isModify = isModificationMod(mongoMod);
-
     var isModify = LocalCollection._isModificationMod(mongoMod);
 
     // We've already run replaceTypes/replaceMeteorAtomWithMongo on
     // selector and mod.  We assume it doesn't matter, as far as
     // the behavior of modifiers is concerned, whether `_modify`
     // is run on EJSON or on mongo-converted EJSON.
-    var newDoc = LocalCollection._createUpsertDocument(mongoSelector, mongoMod);
+
     // Run this code up front so that it fails fast if someone uses
     // a Mongo update operator we don't support.
-
+    let newDoc;
+    try {
+       newDoc = LocalCollection._createUpsertDocument(mongoSelector, mongoMod);
+    } catch (err) {
+      if (callback) {
+        return callback(err);
+      } else {
+        throw err;
+      }
+    }
+  
     var knownId = newDoc._id;
 
-    if (options._forbidReplace && ! isModify) {
+    if (options._forbidReplace && !isModify) {
       var err = new Error("Invalid modifier. Replacements are forbidden.");
       if (callback) {
         return callback(err);
@@ -534,22 +542,21 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
       }
     }
 
-    if (options.upsert && (! knownId) && options.insertedId) {
-      // XXX If we know we're using Mongo 2.6 (and this isn't a replacement)
-      //     we should be able to just use $setOnInsert instead of this
-      //     simulated upsert thing. (We can't use $setOnInsert with
-      //     replacements because there's nowhere to write it, and $setOnInsert
-      //     can't set _id on Mongo 2.4.)
-      //
-      //     Also, in the future we could do a real upsert for the mongo id
-      //     generation case, if the the node mongo driver gives us back the id
-      //     of the upserted doc (which our current version does not).
-      //
-      //     For more context, see
-      //     https://github.com/meteor/meteor/issues/2278#issuecomment-64252706
+    if (options.upsert 
+      && !isModify && !knownId && options.insertedId
+      && (!(options.insertedId instanceof Mongo.ObjectID) || !options.generatedId)) {
+      // In case of an upsert with a replacement, where there is no _id defined
+      // in either the query or the replacement doc, mongo will generate an id itself. 
+      // Therefore we need this special strategy if we want to control the id ourselves.
+
+      // We don't need to do this when:
+      // - This is not a replacement, so we can add an _id to $setOnInsert
+      // - The id is defined by query or mod we can just add it to the replacement doc
+      // - The user did not specify any id preference and the id is a Mongo ObjectId, 
+      //     then we can just let Mongo generate the id
+
       simulateUpsertWithInsertedId(
-        collection, mongoSelector, mongoMod,
-        newDoc, options,
+        collection, mongoSelector, mongoMod, options,
         // This callback does not need to be bindEnvironment'ed because
         // simulateUpsertWithInsertedId() wraps it and then passes it through
         // bindEnvironmentForWrite.
@@ -565,6 +572,15 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
         }
       );
     } else {
+      
+      if (!knownId && isModify) {
+        if (!mongoMod.hasOwnProperty('$setOnInsert')) {
+          mongoMod.$setOnInsert = {};
+        }
+        knownId = options.insertedId;
+        mongoMod.$setOnInsert._id = replaceMeteorAtomWithMongo(options.insertedId);
+      }
+
       collection.update(
         mongoSelector, mongoMod, mongoOpts,
         bindEnvironmentForWrite(function (err, result) {
@@ -630,7 +646,8 @@ MongoConnection._isCannotChangeIdError = function (err) {
   // We don't use the error code here
   // because the error code we observed it producing (16837) appears to be
   // a far more generic error code based on examining the source.
-  if (error.indexOf('The _id field cannot be changed') === 0) {
+  if (error.indexOf('The _id field cannot be changed') === 0
+    || error.indexOf("the (immutable) field '_id' was found to have been altered to _id") !== -1) {
     return true;
   }
 
@@ -638,15 +655,14 @@ MongoConnection._isCannotChangeIdError = function (err) {
 };
 
 var simulateUpsertWithInsertedId = function (collection, selector, mod,
-                                             newDoc, options, callback) {
-  // STRATEGY:  First try doing a plain update.  If it affected 0 documents,
-  // then without affecting the database, we know we should probably do an
-  // insert.  We then do a *conditional* insert that will fail in the case
-  // of a race condition.  This conditional insert is actually an
-  // upsert-replace with an _id, which will never successfully update an
-  // existing document.  If this upsert fails with an error saying it
-  // couldn't change an existing _id, then we know an intervening write has
-  // caused the query to match something.  We go back to step one and repeat.
+                                             options, callback) {
+  // STRATEGY: First try doing an upsert with a generated ID.
+  // If this throws an error about changing the ID on an existing document
+  // then without affecting the database, we know we should probably try
+  // an update without the generated ID. If it affected 0 documents, 
+  // then without affecting the database, we the document that first
+  // gave the error is probably removed and we need to try an insert again
+  // We go back to step one and repeat.
   // Like all "optimistic write" schemes, we rely on the fact that it's
   // unlikely our writes will continue to be interfered with under normal
   // circumstances (though sufficiently heavy contention with writers
@@ -663,33 +679,33 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod,
     upsert: true
   };
 
+  var replacementWithId = _.extend(
+    replaceTypes({_id: insertedId}, replaceMeteorAtomWithMongo),
+    mod);
+
   var tries = NUM_OPTIMISTIC_TRIES;
 
   var doUpdate = function () {
+    collection.update(selector, mod, mongoOptsForUpdate,
+                      bindEnvironmentForWrite(function (err, result) {
+                        if (err) {
+                          callback(err);
+                        } else if (result && result.result.n != 0) {
+                          callback(null, {
+                            numberAffected: result.result.n
+                          });
+                        } else {
+                          doConditionalInsert();
+                        }
+                      }));
+  };
+
+  var doConditionalInsert = function () {
     tries--;
     if (! tries) {
       callback(new Error("Upsert failed after " + NUM_OPTIMISTIC_TRIES + " tries."));
     } else {
-      collection.update(selector, mod, mongoOptsForUpdate,
-                        bindEnvironmentForWrite(function (err, result) {
-                          if (err) {
-                            callback(err);
-                          } else if (result && result.result.n != 0) {
-                            callback(null, {
-                              numberAffected: result.result.n
-                            });
-                          } else {
-                            doConditionalInsert();
-                          }
-                        }));
-    }
-  };
-
-  var doConditionalInsert = function () {
-    var replacementWithId = _.extend(
-      replaceTypes({_id: insertedId}, replaceMeteorAtomWithMongo),
-      newDoc);
-    collection.update(selector, replacementWithId, mongoOptsForInsert,
+      collection.update(selector, replacementWithId, mongoOptsForInsert,
                       bindEnvironmentForWrite(function (err, result) {
                         if (err) {
                           // figure out if this is a
@@ -707,9 +723,10 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod,
                           });
                         }
                       }));
+    }
   };
 
-  doUpdate();
+  doConditionalInsert();
 };
 
 _.each(["insert", "update", "remove", "dropCollection", "dropDatabase"], function (method) {

From 8a7db6bdc55a07d53816dd2a4c8c0572c50dec25 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Fri, 23 Jun 2017 12:18:22 +0200
Subject: [PATCH 12/22] Fix mongo driver when using object ids

---
 packages/mongo/mongo_driver.js | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index 10fd206590..c981709cda 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -520,18 +520,19 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
 
     // Run this code up front so that it fails fast if someone uses
     // a Mongo update operator we don't support.
-    let newDoc;
-    try {
-       newDoc = LocalCollection._createUpsertDocument(mongoSelector, mongoMod);
-    } catch (err) {
-      if (callback) {
-        return callback(err);
-      } else {
-        throw err;
+    let knownId;
+    if (options.upsert) {
+      try {
+        let newDoc = LocalCollection._createUpsertDocument(selector, mod);
+        knownId = newDoc._id;
+      } catch (err) {
+        if (callback) {
+          return callback(err);
+        } else {
+          throw err;
+        }
       }
     }
-  
-    var knownId = newDoc._id;
 
     if (options._forbidReplace && !isModify) {
       var err = new Error("Invalid modifier. Replacements are forbidden.");
@@ -573,12 +574,12 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
       );
     } else {
       
-      if (!knownId && isModify) {
+      if (options.upsert && !knownId && isModify) {
         if (!mongoMod.hasOwnProperty('$setOnInsert')) {
           mongoMod.$setOnInsert = {};
         }
         knownId = options.insertedId;
-        mongoMod.$setOnInsert._id = replaceMeteorAtomWithMongo(options.insertedId);
+        Object.assign(mongoMod.$setOnInsert, replaceTypes({_id: options.insertedId}, replaceMeteorAtomWithMongo));
       }
 
       collection.update(
@@ -679,7 +680,7 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod,
     upsert: true
   };
 
-  var replacementWithId = _.extend(
+  var replacementWithId = Object.assign(
     replaceTypes({_id: insertedId}, replaceMeteorAtomWithMongo),
     mod);
 

From 12d03f26076abe72e30a13634ef76e591ced4784 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Fri, 23 Jun 2017 13:51:30 +0200
Subject: [PATCH 13/22] Don't modify the input variables when creating upsert
 document

---
 packages/minimongo/minimongo.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index 5997e80dd9..8a17490a2e 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -1156,6 +1156,7 @@ LocalCollection._createUpsertDocument = function (selector, modifier) {
     return newDoc
   } else {
     // Replacement can take _id from query document
+    modifier = EJSON.clone(modifier);
     if (newDoc._id) {
       modifier._id = newDoc._id;
     }

From 970044402e38518948418c118a2cff2d01ca06f1 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Fri, 23 Jun 2017 20:37:16 +0200
Subject: [PATCH 14/22] Fail faster in case of forbidden replace

---
 packages/mongo/mongo_driver.js | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index c981709cda..b2dd4d6a27 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -513,6 +513,15 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
 
     var isModify = LocalCollection._isModificationMod(mongoMod);
 
+    if (options._forbidReplace && !isModify) {
+      var err = new Error("Invalid modifier. Replacements are forbidden.");
+      if (callback) {
+        return callback(err);
+      } else {
+        throw err;
+      }
+    }
+
     // We've already run replaceTypes/replaceMeteorAtomWithMongo on
     // selector and mod.  We assume it doesn't matter, as far as
     // the behavior of modifiers is concerned, whether `_modify`
@@ -534,15 +543,6 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
       }
     }
 
-    if (options._forbidReplace && !isModify) {
-      var err = new Error("Invalid modifier. Replacements are forbidden.");
-      if (callback) {
-        return callback(err);
-      } else {
-        throw err;
-      }
-    }
-
     if (options.upsert 
       && !isModify && !knownId && options.insertedId
       && (!(options.insertedId instanceof Mongo.ObjectID) || !options.generatedId)) {
@@ -581,7 +581,7 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
         knownId = options.insertedId;
         Object.assign(mongoMod.$setOnInsert, replaceTypes({_id: options.insertedId}, replaceMeteorAtomWithMongo));
       }
-
+      
       collection.update(
         mongoSelector, mongoMod, mongoOpts,
         bindEnvironmentForWrite(function (err, result) {

From b252e31f499bae4d02763df2c4593a12cae21c1a Mon Sep 17 00:00:00 2001
From: Seba Kerckhof <seba.kerckhof@gmail.com>
Date: Fri, 23 Jun 2017 23:38:48 +0200
Subject: [PATCH 15/22] Fix most tests

---
 packages/mongo/mongo_driver.js | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index b2dd4d6a27..05c1b14c24 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -574,7 +574,7 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
       );
     } else {
       
-      if (options.upsert && !knownId && isModify) {
+      if (options.upsert && !knownId && options.insertedId && isModify) {
         if (!mongoMod.hasOwnProperty('$setOnInsert')) {
           mongoMod.$setOnInsert = {};
         }
@@ -591,10 +591,14 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
               // If this was an upsert() call, and we ended up
               // inserting a new doc and we know its id, then
               // return that id as well.
-
-              if (options.upsert && meteorResult.insertedId && knownId) {
-                meteorResult.insertedId = knownId;
+              if (options.upsert && meteorResult.insertedId) {
+                if (knownId) {
+                  meteorResult.insertedId = knownId;
+                } else if (meteorResult.insertedId instanceof MongoDB.ObjectID) {
+                  meteorResult.insertedId = new Mongo.ObjectID(meteorResult.insertedId.toHexString());
+                }
               }
+
               callback(err, meteorResult);
             } else {
               callback(err, meteorResult.numberAffected);

From dbb24f5788b017408430b3f6b5b1fe4643fe002e Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Sat, 24 Jun 2017 00:12:37 +0200
Subject: [PATCH 16/22] Revert order of simulated upsert to original order

---
 packages/mongo/mongo_driver.js | 54 +++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index 05c1b14c24..e5cbce46fa 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -691,7 +691,11 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod,
   var tries = NUM_OPTIMISTIC_TRIES;
 
   var doUpdate = function () {
-    collection.update(selector, mod, mongoOptsForUpdate,
+    tries--;
+    if (! tries) {
+      callback(new Error("Upsert failed after " + NUM_OPTIMISTIC_TRIES + " tries."));
+    } else {
+      collection.update(selector, mod, mongoOptsForUpdate,
                       bindEnvironmentForWrite(function (err, result) {
                         if (err) {
                           callback(err);
@@ -703,35 +707,31 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod,
                           doConditionalInsert();
                         }
                       }));
-  };
-
-  var doConditionalInsert = function () {
-    tries--;
-    if (! tries) {
-      callback(new Error("Upsert failed after " + NUM_OPTIMISTIC_TRIES + " tries."));
-    } else {
-      collection.update(selector, replacementWithId, mongoOptsForInsert,
-                      bindEnvironmentForWrite(function (err, result) {
-                        if (err) {
-                          // figure out if this is a
-                          // "cannot change _id of document" error, and
-                          // if so, try doUpdate() again, up to 3 times.
-                          if (MongoConnection._isCannotChangeIdError(err)) {
-                            doUpdate();
-                          } else {
-                            callback(err);
-                          }
-                        } else {
-                          callback(null, {
-                            numberAffected: result.result.upserted.length,
-                            insertedId: insertedId,
-                          });
-                        }
-                      }));
     }
   };
 
-  doConditionalInsert();
+  var doConditionalInsert = function () {
+    collection.update(selector, replacementWithId, mongoOptsForInsert,
+                    bindEnvironmentForWrite(function (err, result) {
+                      if (err) {
+                        // figure out if this is a
+                        // "cannot change _id of document" error, and
+                        // if so, try doUpdate() again, up to 3 times.
+                        if (MongoConnection._isCannotChangeIdError(err)) {
+                          doUpdate();
+                        } else {
+                          callback(err);
+                        }
+                      } else {
+                        callback(null, {
+                          numberAffected: result.result.upserted.length,
+                          insertedId: insertedId,
+                        });
+                      }
+                    }));
+  };
+
+  doUpdate();
 };
 
 _.each(["insert", "update", "remove", "dropCollection", "dropDatabase"], function (method) {

From 6105f62069a537cc2cde2cc5a93741d7da04aad1 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Sat, 24 Jun 2017 00:12:52 +0200
Subject: [PATCH 17/22] finish tests (hopefully)

---
 packages/minimongo/minimongo_tests.js  | 5 ++++-
 packages/mongo/mongo_livedata_tests.js | 1 -
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/packages/minimongo/minimongo_tests.js b/packages/minimongo/minimongo_tests.js
index 6623d4619b..6d1e5b9e3c 100644
--- a/packages/minimongo/minimongo_tests.js
+++ b/packages/minimongo/minimongo_tests.js
@@ -2341,7 +2341,7 @@ Tinytest.add("minimongo - modify", function (test) {
     coll.update(query, mod);
     var actual = coll.findOne();
 
-    if(!expected._id){
+    if (!expected._id) {
       delete actual._id;  // added by insert
     }
 
@@ -2982,6 +2982,9 @@ Tinytest.add("minimongo - modify", function (test) {
   // Replacement can take _id from query
   upsert({"_id": "foo", "foo": "bar"}, {"bar": "foo"}, {"_id": "foo", "bar": "foo"})
 
+  // Replacement update keeps _id
+  upsertUpdate({"_id": "foo", "bar": "baz"}, {"_id":"foo"}, {"bar": "crow"}, {"_id": "foo", "bar": "crow"});
+  
   // Nested fields don't work with literal objects
   upsertException({"a": {}, "a.b": "foo"}, {});
 
diff --git a/packages/mongo/mongo_livedata_tests.js b/packages/mongo/mongo_livedata_tests.js
index 5b725e44d2..1df0a927e2 100644
--- a/packages/mongo/mongo_livedata_tests.js
+++ b/packages/mongo/mongo_livedata_tests.js
@@ -1654,7 +1654,6 @@ if (Meteor.isServer) {
       err = e;
     }
     test.isTrue(err);
-    console.log("WUUUK");
     test.isTrue(MongoInternals.Connection._isCannotChangeIdError(err));
 
     try {

From d89d6987fb57217d4727c59d8f86de26b5ecca57 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Sat, 24 Jun 2017 00:20:32 +0200
Subject: [PATCH 18/22] Restore original indentation

---
 packages/mongo/mongo_driver.js | 52 +++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index e5cbce46fa..16f507df07 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -696,39 +696,39 @@ var simulateUpsertWithInsertedId = function (collection, selector, mod,
       callback(new Error("Upsert failed after " + NUM_OPTIMISTIC_TRIES + " tries."));
     } else {
       collection.update(selector, mod, mongoOptsForUpdate,
-                      bindEnvironmentForWrite(function (err, result) {
-                        if (err) {
-                          callback(err);
-                        } else if (result && result.result.n != 0) {
-                          callback(null, {
-                            numberAffected: result.result.n
-                          });
-                        } else {
-                          doConditionalInsert();
-                        }
-                      }));
+                        bindEnvironmentForWrite(function (err, result) {
+                          if (err) {
+                            callback(err);
+                          } else if (result && result.result.n != 0) {
+                            callback(null, {
+                              numberAffected: result.result.n
+                            });
+                          } else {
+                            doConditionalInsert();
+                          }
+                        }));
     }
   };
 
   var doConditionalInsert = function () {
     collection.update(selector, replacementWithId, mongoOptsForInsert,
-                    bindEnvironmentForWrite(function (err, result) {
-                      if (err) {
-                        // figure out if this is a
-                        // "cannot change _id of document" error, and
-                        // if so, try doUpdate() again, up to 3 times.
-                        if (MongoConnection._isCannotChangeIdError(err)) {
-                          doUpdate();
+                      bindEnvironmentForWrite(function (err, result) {
+                        if (err) {
+                          // figure out if this is a
+                          // "cannot change _id of document" error, and
+                          // if so, try doUpdate() again, up to 3 times.
+                          if (MongoConnection._isCannotChangeIdError(err)) {
+                            doUpdate();
+                          } else {
+                            callback(err);
+                          }
                         } else {
-                          callback(err);
+                          callback(null, {
+                            numberAffected: result.result.upserted.length,
+                            insertedId: insertedId,
+                          });
                         }
-                      } else {
-                        callback(null, {
-                          numberAffected: result.result.upserted.length,
-                          insertedId: insertedId,
-                        });
-                      }
-                    }));
+                      }));
   };
 
   doUpdate();

From 4531b3893c16148ebdf8ec06d6a74b0207a6b28a Mon Sep 17 00:00:00 2001
From: Seba Kerckhof <seba.kerckhof@gmail.com>
Date: Sun, 25 Jun 2017 21:26:24 +0200
Subject: [PATCH 19/22] Optimize by only shallow cloning when modifier is
 replacement

---
 packages/minimongo/minimongo.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index 8a17490a2e..445d3b33b5 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -1156,10 +1156,10 @@ LocalCollection._createUpsertDocument = function (selector, modifier) {
     return newDoc
   } else {
     // Replacement can take _id from query document
-    modifier = EJSON.clone(modifier);
+    const replacement = Object.assign({}, modifier);
     if (newDoc._id) {
-      modifier._id = newDoc._id;
+      replacement._id = newDoc._id;
     }
-    return modifier
+    return replacement
   }
 }

From b5a9fde887daea4eed1204c1a6f79c9a983c1af7 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Mon, 10 Jul 2017 18:28:46 +0200
Subject: [PATCH 20/22] Adjust styling as per PR review

---
 packages/minimongo/minimongo.js       |   1 +
 packages/minimongo/modify.js          |   4 +-
 packages/minimongo/upsert_document.js | 174 +++++++++++++-------------
 3 files changed, 92 insertions(+), 87 deletions(-)

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index 445d3b33b5..895bb55ad5 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -1,4 +1,5 @@
 import { assertHasValidFieldNames } from './validation.js';
+import populateDocumentWithQueryFields from './upsert_document.js';
 
 // XXX type checking on selectors (graceful error if malformed)
 
diff --git a/packages/minimongo/modify.js b/packages/minimongo/modify.js
index dafb84fe66..8ebd7a6a05 100644
--- a/packages/minimongo/modify.js
+++ b/packages/minimongo/modify.js
@@ -69,7 +69,9 @@ LocalCollection._modify = function (doc, mod, options) {
     });
 
     if (doc._id && !EJSON.equals(doc._id, newDoc._id)) {
-      throw MinimongoError(`After applying the update to the document {_id: "${doc._id}" , ...}, the (immutable) field '_id' was found to have been altered to _id: "${newDoc._id}"`);
+      throw MinimongoError('After applying the update to the document {_id: ' +
+      `"${doc._id}" , ...}, the (immutable) field '_id' was found to have` +
+      ` been altered to _id: "${newDoc._id}"`);
     }
   }
 
diff --git a/packages/minimongo/upsert_document.js b/packages/minimongo/upsert_document.js
index f0dea52cf3..18b97d75c9 100644
--- a/packages/minimongo/upsert_document.js
+++ b/packages/minimongo/upsert_document.js
@@ -15,109 +15,111 @@
 // - you can not have '$'-prefixed keys more than one-level deep in an object
 
 // Fills a document with certain fields from an upsert selector
-populateDocumentWithQueryFields = function (query, document = {}) {
-
-    if (Object.getPrototypeOf(query) === Object.prototype) {
-        // handle implicit $and
-        for (let [key, value] of Object.entries(query)) {
-            // handle explicit $and
-            if (key === "$and") {
-                value.forEach(sq => populateDocumentWithQueryFields(sq, document))
-            }
-            // handle $or nodes with exactly 1 child
-            else if (key === "$or") {
-                if (value.length === 1) {
-                    populateDocumentWithQueryFields(value[0], document)
-                }
-            }
-            //Ignore other '$'-prefixed logical selectors
-            else if (key[0] !== "$") {
-                populateDocumentWithKeyValue(document, key, value);
-            }
-        }
-    } else {
-        // Handle meteor-specific shortcut for selecting _id
-        if (LocalCollection._selectorIsId(query)) {
-            insertIntoDocument(document, "_id", query);
+export default function populateDocumentWithQueryFields (query, document = {}) {
+  if (Object.getPrototypeOf(query) === Object.prototype) {
+    // handle implicit $and
+    Object.keys(query).forEach(function (key) {
+      const value = query[key];
+      if (key === '$and') {
+        // handle explicit $and
+        value.forEach(sq => populateDocumentWithQueryFields(sq, document));
+      } else if (key === '$or') {
+        // handle $or nodes with exactly 1 child
+        if (value.length === 1) {
+          populateDocumentWithQueryFields(value[0], document);
         }
+      } else if (key[0] !== '$') {
+        // Ignore other '$'-prefixed logical selectors
+        populateDocumentWithKeyValue(document, key, value);
+      }
+    })
+  } else {
+    // Handle meteor-specific shortcut for selecting _id
+    if (LocalCollection._selectorIsId(query)) {
+      insertIntoDocument(document, '_id', query);
     }
+  }
 
-    return document;
-
+  return document;
 }
 
 // Handles one key/value pair to put in the selector document
-function populateDocumentWithKeyValue(document, key, value) {
-    if (value && Object.getPrototypeOf(value) === Object.prototype) {
-        populateDocumentWithObject(document, key, value);
-    } else if (!(value instanceof RegExp)) {
-        insertIntoDocument(document, key, value);
-    }
+function populateDocumentWithKeyValue (document, key, value) {
+  if (value && Object.getPrototypeOf(value) === Object.prototype) {
+    populateDocumentWithObject(document, key, value);
+  } else if (!(value instanceof RegExp)) {
+    insertIntoDocument(document, key, value);
+  }
 }
 
+// Handles a key, value pair to put in the selector document 
+// if the value is an object
+function populateDocumentWithObject (document, key, value) {
+  const keys = Object.keys(value);
+  const unprefixedKeys = keys.filter(k => k[0] !== '$');
 
-// Handles a key, value to put in the selector document if the value is an object
-function populateDocumentWithObject(document, key, value) {
-    const keys = Object.keys(value);
-    const unprefixedKeys = keys.filter(k => k[0] !== "$");
-
-    if (unprefixedKeys.length > 0 || !keys.length) {
-        //Literal (possibly empty) object ( or empty object ) 
-        //Don't allow mixing '$'-prefixed with non-'$'-prefixed fields
-        if (keys.length !== unprefixedKeys.length) {
-            throw new Error(`unknown operator: ${unprefixedKeys[0]}`)
-        } else {
-            validateObject(value, key);
-            insertIntoDocument(document, key, value);
-        }
+  if (unprefixedKeys.length > 0 || !keys.length) {
+    // Literal (possibly empty) object ( or empty object ) 
+    // Don't allow mixing '$'-prefixed with non-'$'-prefixed fields
+    if (keys.length !== unprefixedKeys.length) {
+      throw new Error(`unknown operator: ${unprefixedKeys[0]}`);
     } else {
-        for (let [k, v] of Object.entries(value)) {
-            if (k === "$eq") {
-                populateDocumentWithKeyValue(document, key, v);
-            }
-            //every value for $all should be dealt with as separate $eq-s
-            else if (k === "$all") {
-                v.forEach(vx => populateDocumentWithKeyValue(document, key, vx))
-            }
-        }
+      validateObject(value, key);
+      insertIntoDocument(document, key, value);
     }
+  } else {
+    Object.keys(value).forEach(function (k) {
+      const v = value[k];
+      if (k === '$eq') {
+        populateDocumentWithKeyValue(document, key, v);
+      } else if (k === '$all') {
+        // every value for $all should be dealt with as separate $eq-s
+        v.forEach(vx => populateDocumentWithKeyValue(document, key, vx));
+      }
+    })
+  }
 }
 
 // Actually inserts a key value into the selector document
-// However, this checks there is no ambiguity in setting the value for the given key, throws otherwise
-function insertIntoDocument(document, key, value) {
+// However, this checks there is no ambiguity in setting 
+// the value for the given key, throws otherwise
+function insertIntoDocument (document, key, value) {
+  Object.keys(document).forEach(existingKey => {
+    if (
+      (existingKey.length > key.length && existingKey.indexOf(key) === 0)
+      || (key.length > existingKey.length && key.indexOf(existingKey) === 0)
+    ) {
+      throw new Error('cannot infer query fields to set, both paths ' +
+        `${existingKey}' and '${key}' are matched`);
+    } else if (existingKey === key) {
+      throw new Error(`cannot infer query fields to set, path ${key} ` +
+        'is matched twice');
+    }
+  })
 
-    Object.keys(document).forEach(existingKey => {
-        if (
-            (existingKey.length > key.length && existingKey.indexOf(key) === 0)
-            || (key.length > existingKey.length && key.indexOf(existingKey) === 0)
-        ) {
-            throw new Error(`cannot infer query fields to set, both paths '${existingKey}' and '${key}' are matched`)
-        }
-        else if (existingKey === key) {
-            throw new Error(`cannot infer query fields to set, path ${key} is matched twice`)
-        }
-    })
-
-    document[key] = value;
+  document[key] = value;
 }
 
 // Recursively validates an object that is nested more than one level deep
-function validateObject(obj, path) {
-    if (obj && Object.getPrototypeOf(obj) === Object.prototype) {
-        for (let [key, value] of Object.entries(obj)) {
-            validateKeyInPath(key, path);
-            validateObject(value, path + "." + key);
-        }
-    }
+function validateObject (obj, path) {
+  if (obj && Object.getPrototypeOf(obj) === Object.prototype) {
+    Object.keys(obj).forEach(function (key) {
+      validateKeyInPath(key, path);
+      validateObject(obj[key], path + '.' + key);
+    })
+  }
 }
 
-// Validates the key in a path. Objects that are nested more then 1 level cannot have dotted fields or fields starting with '$'
-function validateKeyInPath(key, path) {
-    if (key.includes(".")) {
-        throw new Error(`The dotted field '${key}' in '${path}.${key}' is not valid for storage.`)
-    }
-    if (key[0] === "$") {
-        throw new Error(`The dollar ($) prefixed field  '${path}.${key}' is not valid for storage.`)
-    }
-}
\ No newline at end of file
+// Validates the key in a path. 
+// Objects that are nested more then 1 level cannot have dotted fields 
+// or fields starting with '$'
+function validateKeyInPath (key, path) {
+  if (key.includes('.')) {
+    throw new Error(`The dotted field '${key}' in '${path}.${key}' ` +
+      'is not valid for storage.');
+  }
+  if (key[0] === '$') {
+    throw new Error(`The dollar ($) prefixed field  '${path}.${key}' ` +
+      'is not valid for storage.');
+  }
+}

From 11c7eabf7f8c91f5e6768d0961ffcec50f756835 Mon Sep 17 00:00:00 2001
From: seke <sebastian.kerckhof@barco.com>
Date: Mon, 10 Jul 2017 18:29:08 +0200
Subject: [PATCH 21/22] Add entry to History.md

---
 History.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/History.md b/History.md
index 68ff8fa9f1..99af701950 100644
--- a/History.md
+++ b/History.md
@@ -42,6 +42,11 @@
   [PR #8754](https://github.com/meteor/meteor/pull/8754)
   [Issue #1173](https://github.com/meteor/meteor/issues/1173)
 
+* The `minimongo` and `mongo` packages are now compliant with the upsert behavior
+  of MongoDB 2.6 and higher. **As a result support for MongoDB 2.4 has been dropped.**
+  This mainly changes the effect of the selector on newly inserted documents.
+  [PR #8815](https://github.com/meteor/meteor/pull/8815)
+
 * `reactive-dict` now supports setting initial data when defining a named
   `ReactiveDict`. No longer run migration logic when used on the server,
   this is to prevent duplicate name error on reloads. Initial data is now

From e3e75236750e90303765c5f89889455302426a93 Mon Sep 17 00:00:00 2001
From: Ben Newman <ben@meteor.com>
Date: Thu, 13 Jul 2017 18:37:29 -0400
Subject: [PATCH 22/22] Minor style tweaks (mostly adding missing semicolons).

---
 packages/minimongo/minimongo.js       | 33 ++++++++++++++-------------
 packages/minimongo/selector.js        |  2 +-
 packages/minimongo/upsert_document.js | 15 ++++++------
 packages/mongo/mongo_driver.js        |  9 +++++---
 4 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/packages/minimongo/minimongo.js b/packages/minimongo/minimongo.js
index 895bb55ad5..28df4180ec 100644
--- a/packages/minimongo/minimongo.js
+++ b/packages/minimongo/minimongo.js
@@ -1139,28 +1139,29 @@ LocalCollection._isModificationMod = function (mod) {
 // Calculates the document to insert in case we're doing an upsert and the selector
 // does not match any elements
 LocalCollection._createUpsertDocument = function (selector, modifier) {
-  let selectorDocument = populateDocumentWithQueryFields(selector)
-  const isModify = LocalCollection._isModificationMod(modifier)
+  let selectorDocument = populateDocumentWithQueryFields(selector);
+  const isModify = LocalCollection._isModificationMod(modifier);
   
-  const newDoc = {}
+  const newDoc = {};
   if (selectorDocument._id) {
-    newDoc._id = selectorDocument._id
-    delete selectorDocument._id
+    newDoc._id = selectorDocument._id;
+    delete selectorDocument._id;
   }
 
   // This double _modify call is made to help with nested properties (see issue #8631).
   // We do this even if it's a replacement for validation purposes (e.g. ambiguous id's)
-  LocalCollection._modify(newDoc, { $set: selectorDocument })
-  LocalCollection._modify(newDoc, modifier, { isInsert: true })
+  LocalCollection._modify(newDoc, { $set: selectorDocument });
+  LocalCollection._modify(newDoc, modifier, { isInsert: true });
 
   if (isModify) {
-    return newDoc
-  } else {
-    // Replacement can take _id from query document
-    const replacement = Object.assign({}, modifier);
-    if (newDoc._id) {
-      replacement._id = newDoc._id;
-    }
-    return replacement
+    return newDoc;
   }
-}
+
+  // Replacement can take _id from query document
+  const replacement = Object.assign({}, modifier);
+  if (newDoc._id) {
+    replacement._id = newDoc._id;
+  }
+
+  return replacement
+};
diff --git a/packages/minimongo/selector.js b/packages/minimongo/selector.js
index 67bf94a167..840d06c9ed 100644
--- a/packages/minimongo/selector.js
+++ b/packages/minimongo/selector.js
@@ -1249,4 +1249,4 @@ LocalCollection._f = {
       throw Error("Sorting not supported on Javascript code"); // XXX
     throw Error("Unknown type to sort");
   }
-};
\ No newline at end of file
+};
diff --git a/packages/minimongo/upsert_document.js b/packages/minimongo/upsert_document.js
index 18b97d75c9..e5d9d864b7 100644
--- a/packages/minimongo/upsert_document.js
+++ b/packages/minimongo/upsert_document.js
@@ -63,10 +63,9 @@ function populateDocumentWithObject (document, key, value) {
     // Don't allow mixing '$'-prefixed with non-'$'-prefixed fields
     if (keys.length !== unprefixedKeys.length) {
       throw new Error(`unknown operator: ${unprefixedKeys[0]}`);
-    } else {
-      validateObject(value, key);
-      insertIntoDocument(document, key, value);
     }
+    validateObject(value, key);
+    insertIntoDocument(document, key, value);
   } else {
     Object.keys(value).forEach(function (k) {
       const v = value[k];
@@ -76,7 +75,7 @@ function populateDocumentWithObject (document, key, value) {
         // every value for $all should be dealt with as separate $eq-s
         v.forEach(vx => populateDocumentWithKeyValue(document, key, vx));
       }
-    })
+    });
   }
 }
 
@@ -90,12 +89,12 @@ function insertIntoDocument (document, key, value) {
       || (key.length > existingKey.length && key.indexOf(existingKey) === 0)
     ) {
       throw new Error('cannot infer query fields to set, both paths ' +
-        `${existingKey}' and '${key}' are matched`);
+        `'${existingKey}' and '${key}' are matched`);
     } else if (existingKey === key) {
-      throw new Error(`cannot infer query fields to set, path ${key} ` +
+      throw new Error(`cannot infer query fields to set, path '${key}' ` +
         'is matched twice');
     }
-  })
+  });
 
   document[key] = value;
 }
@@ -106,7 +105,7 @@ function validateObject (obj, path) {
     Object.keys(obj).forEach(function (key) {
       validateKeyInPath(key, path);
       validateObject(obj[key], path + '.' + key);
-    })
+    });
   }
 }
 
diff --git a/packages/mongo/mongo_driver.js b/packages/mongo/mongo_driver.js
index 16f507df07..476fe7199a 100644
--- a/packages/mongo/mongo_driver.js
+++ b/packages/mongo/mongo_driver.js
@@ -543,9 +543,12 @@ MongoConnection.prototype._update = function (collection_name, selector, mod,
       }
     }
 
-    if (options.upsert 
-      && !isModify && !knownId && options.insertedId
-      && (!(options.insertedId instanceof Mongo.ObjectID) || !options.generatedId)) {
+    if (options.upsert &&
+        ! isModify &&
+        ! knownId &&
+        options.insertedId &&
+        ! (options.insertedId instanceof Mongo.ObjectID &&
+           options.generatedId)) {
       // In case of an upsert with a replacement, where there is no _id defined
       // in either the query or the replacement doc, mongo will generate an id itself. 
       // Therefore we need this special strategy if we want to control the id ourselves.