SERVER-14024: validate update insert fields at the end

3 files changed, 18 insertions, 14 deletions

diff --git a/jstests/core/update_setOnInsert.js b/jstests/core/update_setOnInsert.js
index c302a7638bb..abcbdfa9819 100644
--- a/jstests/core/update_setOnInsert.js
+++ b/jstests/core/update_setOnInsert.js
@@ -36,12 +36,11 @@ dotest( true );
 t.drop();
 
 res = t.update( {_id: 1} , { $setOnInsert: { "_id.a": new Date() } } , true );
-assert(res.hasWriteError(), "$setOnInsert _id.a - " + res.toString() + tojson(t.findOne()));
+assert.writeError(res, "$setOnInsert _id.a worked" );
 
 res = t.update( {"_id.a": 4} , { $setOnInsert: { "_id.b": 1 } } , true );
-assert(res.hasWriteError(), "$setOnInsert _id.b - " + res.toString() + tojson(t.findOne()));
+assert.writeError(res, "$setOnInsert _id.a/b worked" );
 
 res = t.update( {"_id.a": 4} , { $setOnInsert: { "_id": {a:4, b:1} } } , true );
-assert(res.hasWriteError(), "$setOnInsert _id 3 - " + res.toString() + tojson(t.findOne()));
-
-db.setProfilingLevel( 0 );
+assert.writeError(res, "$setOnInsert _id.a/a+b worked" );
+db.setProfilingLevel( 0 );
\ No newline at end of file
diff --git a/jstests/core/upsert3.js b/jstests/core/upsert3.js
index d23fbabc680..cfcfa2688d1 100644
--- a/jstests/core/upsert3.js
+++ b/jstests/core/upsert3.js
@@ -50,3 +50,9 @@ res = t.update( {_id: {a:1, b:1}, "_id.a": 1} , {y: 1} , true );
 assert.writeOK(res);
 assert.docEq({_id: {a: 1, b: 1}, y: 1}, t.findOne(), "_id-4")
 t.drop()
+
+// make sure query doesn't error when creating doc for insert, 
+// since it missing the rest of the dbref fields. SERVER-14024
+res = t.update( {_id:1, "foo.$id":1}, {$set : {foo:DBRef("a", 1)}}, {upsert:true} );
+assert.writeOK(res);
+assert.docEq({_id: 1, foo:DBRef("a", 1)}, t.findOne())
diff --git a/src/mongo/db/ops/update.cpp b/src/mongo/db/ops/update.cpp
index efe83325112..53e67a9baf9 100644
--- a/src/mongo/db/ops/update.cpp
+++ b/src/mongo/db/ops/update.cpp
@@ -300,10 +300,11 @@ namespace mongo {
                 }
             }
 
-            const bool idChanged = updatedFields.findConflicts(&idFieldRef, NULL);
+            const bool checkIdField = (updatedFields.empty() && !original.isEmpty()) ||
+                                      updatedFields.findConflicts(&idFieldRef, NULL);
 
             // Add _id to fields to check since it too is immutable
-            if (idChanged)
+            if (checkIdField)
                 changedImmutableFields.keepShortest(&idFieldRef);
             else if (changedImmutableFields.empty()) {
                 // Return early if nothing changed which is immutable
@@ -730,9 +731,6 @@ namespace mongo {
         // creates the base of the update for the inserterd doc (because upsert was true)
         if (cq) {
             uassertStatusOK(driver->populateDocumentWithQueryFields(cq, doc));
-            // Validate the base doc, as taken from the query -- no fields means validate all.
-            FieldRefSet noFields;
-            uassertStatusOK(validate(BSONObj(), noFields, doc, NULL, driver->modOptions()));
             if (!driver->isDocReplacement()) {
                 opDebug->fastmodinsert = true;
                 // We need all the fields from the query to compare against for validation below.
@@ -750,8 +748,7 @@ namespace mongo {
         }
 
         // Apply the update modifications and then log the update as an insert manually.
-        FieldRefSet updatedFields;
-        status = driver->update(StringData(), &doc, NULL, &updatedFields);
+        status = driver->update(StringData(), &doc);
         if (!status.isOK()) {
             uasserted(16836, status.reason());
         }
@@ -760,15 +757,17 @@ namespace mongo {
         uassertStatusOK(ensureIdAndFirst(doc));
 
         // Validate that the object replacement or modifiers resulted in a document
-        // that contains all the immutable keys and can be stored.
+        // that contains all the immutable keys and can be stored if it isn't coming
+        // from a migration or via replication.
         if (!(request.isFromReplication() || request.isFromMigration())){
             const std::vector<FieldRef*>* immutableFields = NULL;
             if (lifecycle)
                 immutableFields = lifecycle->getImmutableFields();
 
+            FieldRefSet noFields;
             // This will only validate the modified fields if not a replacement.
             uassertStatusOK(validate(original,
-                                     updatedFields,
+                                     noFields,
                                      doc,
                                      immutableFields,
                                      driver->modOptions()) );