SERVER-11531, SERVER-10489, SERVER-6835, SERVER-4830: Refactor update system to support immutable fields, consolodate storage validation, and misc issues.

8 files changed, 253 insertions, 9 deletions

diff --git a/jstests/rename4.js b/jstests/rename4.js
index 989fff510fc..bca6c6928a6 100644
--- a/jstests/rename4.js
+++ b/jstests/rename4.js
@@ -27,12 +27,6 @@ function bad( f ) {
 bad( "t.update( {}, {$rename:{'a':'a'}} )" );
 bad( "t.update( {}, {$rename:{'':'a'}} )" );
 bad( "t.update( {}, {$rename:{'a':''}} )" );
-bad( "t.update( {}, {$rename:{'_id':'a'}} )" );
-bad( "t.update( {}, {$rename:{'a':'_id'}} )" );
-bad( "t.update( {}, {$rename:{'_id.a':'b'}} )" );
-bad( "t.update( {}, {$rename:{'b':'_id.a'}} )" );
-bad( "t.update( {}, {$rename:{'_id.a':'_id.b'}} )" );
-bad( "t.update( {}, {$rename:{'_id.b':'_id.a'}} )" );
 bad( "t.update( {}, {$rename:{'.a':'b'}} )" );
 bad( "t.update( {}, {$rename:{'a':'.b'}} )" );
 bad( "t.update( {}, {$rename:{'a.':'b'}} )" );
@@ -42,7 +36,8 @@ bad( "t.update( {}, {$rename:{'a.$':'b'}} )" );
 bad( "t.update( {}, {$rename:{'a':'b.$'}} )" );
 
 // Only bad if input doc has field resulting in conflict
-t.save( {_id:1, a:1} );
+t.save( {_id:1, a:2} );
+bad( "t.update( {}, {$rename:{'_id':'a'}} )" );
 bad( "t.update( {}, {$set:{b:1},$rename:{'a':'b'}} )" );
 bad( "t.update( {}, {$rename:{'a':'b'},$set:{b:1}} )" );
 bad( "t.update( {}, {$rename:{'a':'b'},$set:{a:1}} )" );
@@ -53,7 +48,7 @@ bad( "t.update( {}, {$rename:{'a':'b.c'},$set:{b:1}} )" );
 
 
 t.remove({});
-t.save( {a:[1],b:{c:[1]},d:[{e:1}],f:1} );
+t.save( {a:[1],b:{c:[2]},d:[{e:3}],f:4} );
 bad( "t.update( {}, {$rename:{'a.0':'f'}} )" );
 bad( "t.update( {}, {$rename:{'a.0':'g'}} )" );
 bad( "t.update( {}, {$rename:{'f':'a.0'}} )" );
diff --git a/jstests/repl/basic1.js b/jstests/repl/basic1.js
index 4a6091d9755..ccde8874fbd 100644
--- a/jstests/repl/basic1.js
+++ b/jstests/repl/basic1.js
@@ -24,6 +24,9 @@ function check( note ){
             return;
         sleep( 200 );
     }
+    lastOpLogEntry = m.getDB("local").oplog.$main.find({op:{$ne:"n"}}).sort({$natural:-1}).limit(-1).next();
+    note = note + tojson(am.a.find().toArray()) + " != " + tojson(as.a.find().toArray())  
+                + "last oplog:" + tojson(lastOpLogEntry);
     assert.eq( x.md5 , y.md5 , note );
 }
 
diff --git a/jstests/sharding/update_immutable_fields.js b/jstests/sharding/update_immutable_fields.js
new file mode 100644
index 00000000000..71497466f33
--- /dev/null
+++ b/jstests/sharding/update_immutable_fields.js
@@ -0,0 +1,98 @@
+// Tests that updates can't change immutable fields (used in sharded system)
+var st = new ShardingTest({shards : 2,
+                           mongos : 1,
+                           verbose : 0,
+                           other : {separateConfig : 1}})
+st.stopBalancer();
+
+var mongos = st.s;
+var config = mongos.getDB("config");
+var coll = mongos.getCollection(jsTestName() + ".coll1");
+var shard0 = st.shard0;
+
+printjson(config.adminCommand({enableSharding : coll.getDB() + ""}))
+printjson(config.adminCommand({shardCollection : "" + coll, key : {a : 1}}))
+
+var getDirectShardedConn = function( st, collName ) {
+
+    var shardConnWithVersion = new Mongo( st.shard0.host );
+
+    var mockServerId = new ObjectId();
+    var configConnStr = st._configDB;
+
+    var maxChunk = st.s0.getCollection( "config.chunks" )
+                   .find({ ns : collName }).sort({ lastmod : -1 }).next();
+    
+    var ssvInitCmd = { setShardVersion : collName, 
+                       configdb : configConnStr, 
+                       serverID : mockServerId,
+                       version : maxChunk.lastmod,
+                       versionEpoch : maxChunk.lastmodEpoch };
+    
+    printjson( ssvInitCmd );
+
+    assert.commandWorked( shardConnWithVersion.getDB( "admin" ).runCommand( ssvInitCmd ) );
+    
+    return shardConnWithVersion;
+}
+
+var shard0Coll = getDirectShardedConn(st, coll.getFullName()).getCollection(coll.getFullName());
+
+// No shard key
+shard0Coll.remove()
+shard0Coll.save({_id:3})
+assert.gleError(shard0Coll.getDB(), function(gle) {
+    return "save without shard key passed - " + tojson(gle) + " doc: " + tojson(shard0Coll.findOne()) 
+});
+
+// Full shard key in save
+shard0Coll.save({_id: 1, a: 1})
+assert.gleSuccess(shard0Coll.getDB(), "save with shard key failed");
+
+// Full shard key on replacement (basically the same as above)
+shard0Coll.remove()
+shard0Coll.update({_id: 1}, {a:1}, true)
+assert.gleSuccess(shard0Coll.getDB(), "update + upsert (replacement) with shard key failed"); 
+
+// Full shard key after $set
+shard0Coll.remove()
+shard0Coll.update({_id: 1}, {$set: {a: 1}}, true)
+assert.gleSuccess(shard0Coll.getDB(), "update + upsert ($set) with shard key failed"); 
+
+// Update existing doc (replacement), same shard key value
+shard0Coll.update({_id: 1}, {a:1})
+assert.gleSuccess(shard0Coll.getDB(), "update (replacement) with shard key failed"); 
+
+//Update existing doc ($set), same shard key value
+shard0Coll.update({_id: 1}, {$set: {a: 1}})
+assert.gleSuccess(shard0Coll.getDB(), "update ($set) with shard key failed"); 
+
+// Error due to mutating the shard key (replacement)
+shard0Coll.update({_id: 1}, {b:1})
+assert.gleError(shard0Coll.getDB(), "update (replacement) removes shard key"); 
+
+// Error due to mutating the shard key ($set)
+shard0Coll.update({_id: 1}, {$unset: {a: 1}})
+assert.gleError(shard0Coll.getDB(), "update ($unset) removes shard key"); 
+
+// Error due to removing all the embedded fields.
+shard0Coll.remove()
+
+shard0Coll.save({_id: 2, a:{c:1, b:1}})
+assert.gleSuccess(shard0Coll.getDB(), "save with shard key failed -- 1"); 
+
+shard0Coll.update({}, {$unset: {"a.c": 1}})
+assert.gleError(shard0Coll.getDB(), function(gle) {
+    return "unsetting part of shard key passed - " + tojson(gle) + 
+            " doc: " + tojson(shard0Coll.findOne()) 
+});
+
+shard0Coll.update({}, {$unset: {"a.b": 1, "a.c": 1}})
+assert.gleError(shard0Coll.getDB(), function(gle) {
+    return "unsetting nested fields of shard key passed - " + tojson(gle) + 
+            " doc: " + tojson(shard0Coll.findOne()) 
+});
+
+db.adminCommand("unsetSharding");
+jsTest.log("DONE!"); // distinguishes shutdown failures
+st.stop();
diff --git a/jstests/update_dbref.js b/jstests/update_dbref.js
new file mode 100644
index 00000000000..d435b8c3416
--- /dev/null
+++ b/jstests/update_dbref.js
@@ -0,0 +1,35 @@
+// Test that we can update DBRefs, but not dbref fields outside a DBRef
+
+t = db.jstests_update_dbref;
+t.drop();
+
+t.save({_id:1, a: new DBRef("a", "b")});
+assert.docEq({_id:1, a: new DBRef("a", "b")}, t.findOne());
+
+t.update({}, {$set: {"a.$id": 2}});
+assert.gleSuccess(db, "a.$id update");
+assert.docEq({_id:1, a: new DBRef("a", 2)}, t.findOne());
+
+t.update({}, {$set: {"a.$ref": "b"}});
+assert.gleSuccess(db, "a.$ref update");
+
+assert.docEq({_id:1, a: new DBRef("b", 2)}, t.findOne());
+
+// Bad updates
+t.update({}, {$set: {"$id": 3}});
+assert.gleErrorRegex(db, /\$id/, "expected bad update because of $id")
+assert.docEq({_id:1, a: new DBRef("b", 2)}, t.findOne());
+
+t.update({}, {$set: {"$ref": "foo"}});
+assert.gleErrorRegex(db, /\$ref/, "expected bad update because of $ref")
+assert.docEq({_id:1, a: new DBRef("b", 2)}, t.findOne());
+
+t.update({}, {$set: {"$db": "aDB"}});
+assert.gleErrorRegex(db, /\$db/, "expected bad update because of $db")
+assert.docEq({_id:1, a: new DBRef("b", 2)}, t.findOne());
+
+t.update({}, {$set: {"b.$id": 2}});
+assert.gleError(db, function() { return "b.$id update -- doc:" + tojson(t.findOne())});
+
+t.update({}, {$set: {"b.$ref": 2}});
+assert.gleError(db, function() { return "b.$ref update -- doc:" + tojson(t.findOne())});
diff --git a/jstests/update_replace.js b/jstests/update_replace.js
new file mode 100644
index 00000000000..1bee0a8597a
--- /dev/null
+++ b/jstests/update_replace.js
@@ -0,0 +1,45 @@
+// This test checks validation of the replaced doc (on the server) for dots, $prefix and _id
+
+t = db.jstests_update_replace;
+t.drop();
+
+// disable client side checks so we can test the server
+DBCollection.prototype._validateForStorage = function() {};
+
+// Should not allow "." in field names
+t.save({_id:1, "a.a":1})
+assert.gleError(db, "a.a");
+
+// Should not allow "." in field names, embedded
+t.save({_id:1, a :{"a.a":1}})
+assert.gleError(db, "a: a.a");
+
+// Should not allow "$"-prefixed field names, caught before "." check
+t.save({_id:1, $a :{"a.a":1}})
+assert.gleError(db, "$a: a.a");
+
+// Should not allow "$"-prefixed field names
+t.save({_id:1, $a: 1})
+assert.gleError(db, "$a");
+
+// _id validation checks
+
+// Should not allow regex _id
+t.save({_id: /a/})
+assert.gleError(db, "_id regex");
+
+// Should not allow regex _id, even if not first
+t.save({a:2, _id: /a/})
+assert.gleError(db, "a _id regex");
+
+// Should not allow array _id
+t.save({_id: [9]})
+assert.gleError(db, "_id array");
+
+// This is fine since _id isn't a top level field
+t.save({a :{ _id: [9]}})
+assert.gleSuccess(db, "embedded _id array");
+
+// This is fine since _id isn't a top level field
+t.save({b:1, a :{ _id: [9]}})
+assert.gleSuccess(db, "b embedded _id array");
\ No newline at end of file
diff --git a/jstests/update_setOnInsert.js b/jstests/update_setOnInsert.js
index ddc699c70ed..be215ab408d 100644
--- a/jstests/update_setOnInsert.js
+++ b/jstests/update_setOnInsert.js
@@ -1,4 +1,4 @@
-
+// This tests that $setOnInsert works and allow setting the _id
 t = db.update_setOnInsert;
 
 db.setProfilingLevel( 2 );
@@ -29,3 +29,19 @@ function dotest( useIndex ) {
 
 dotest( false );
 dotest( true );
+
+
+// Cases for SERVER-9958 -- Allow _id $setOnInsert during insert (if upsert:true, and not doc found)
+t.drop();
+
+t.update( {_id: 1} , { $setOnInsert: { "_id.a": new Date() } } , true );
+assert.gleError(db, function(gle) {
+    return "$setOnInsert _id.a - " + tojson(gle) + tojson(t.findOne()) } );
+
+t.update( {"_id.a": 4} , { $setOnInsert: { "_id.b": 1 } } , true );
+assert.gleError(db, function(gle) {
+    return "$setOnInsert _id.b - " + tojson(gle) + tojson(t.findOne()) } );
+
+t.update( {"_id.a": 4} , { $setOnInsert: { "_id": {a:4, b:1} } } , true );
+assert.gleError(db, function(gle) {
+    return "$setOnInsert _id 3 - " + tojson(gle) + tojson(t.findOne()) } );
diff --git a/jstests/upsert1.js b/jstests/upsert1.js
index 15777cbbc24..3c208520b5f 100644
--- a/jstests/upsert1.js
+++ b/jstests/upsert1.js
@@ -51,6 +51,7 @@ db.createCollection("no_id", {autoIndexId:false})
 db.no_id.update({foo:1}, {$set:{a:1}}, true)
 l = db.getLastErrorCmd();
 assert.eq( l.upserted , undefined , "H1 - " + tojson(l) );
+assert( !l.err, "H1.5 No error expected - " + tojson(l) )
 assert.eq( 0, db.no_id.getIndexes().length, "H2" );
 assert.eq( 1, db.no_id.count(), "H3" );
 assert.eq( { foo : 1, a : 1 }, db.no_id.findOne(), "H4" );
diff --git a/jstests/upsert3.js b/jstests/upsert3.js
new file mode 100644
index 00000000000..5faefd05f4b
--- /dev/null
+++ b/jstests/upsert3.js
@@ -0,0 +1,51 @@
+// tests to make sure no dup fields are created when using query to do upsert
+t = db.upsert1;
+t.drop();
+
+// make sure the new _id is not duplicated
+t.update( {"a.b": 1, a: {a: 1, b: 1}} , {$inc: {y: 1}} , true );
+assert.gleError(db, function(gle) {
+    return "a.b-1 - " + tojson(gle) + " doc:" + tojson(t.findOne()) });
+
+t.update( {"_id.a": 1, _id: {a: 1, b: 1}} , {$inc : {y: 1}} , true );
+assert.gleError(db, function(gle) {
+    return "_id-1 - " + tojson(gle) + " doc:" + tojson(t.findOne()) });
+
+t.update( {_id: {a: 1, b: 1}, "_id.a": 1} , { $inc: {y: 1}} , true );
+assert.gleError(db, function(gle) {
+    return "_id-2 - " + tojson(gle) + " doc:" + tojson(t.findOne()) });
+
+// Should be redundant, but including from SERVER-11363
+t.update( {_id: {a: 1, b: 1}, "_id.a": 1} , {$setOnInsert: {y: 1}} , true );
+assert.gleError(db, function(gle) {
+    return "_id-3 - " + tojson(gle) + " doc:" + tojson(t.findOne()) });
+
+//Should be redundant, but including from SERVER-11514
+t.update( {"a": {}, "a.c": 2} , {$set: {x: 1}}, true );
+assert.gleError(db, function(gle) {
+    return "a.c-1 - " + tojson(gle) + " doc:" + tojson(t.findOne()) });
+
+// Should be redundant, but including from SERVER-4830
+t.update( {'a': {b: 1}, 'a.c': 1}, {$inc: {z: 1}}, true );
+assert.gleError(db, function(gle) {
+    return "a-1 - " + tojson(gle) + " doc:" + tojson(t.findOne()) });
+
+// Should be redundant, but including from SERVER-4830
+t.update( {a: 1, "a.b": 1, a: [1, {b: 1}]}, {$inc: {z: 1}}, true );
+assert.gleError(db, function(gle) {
+    return "a-2 - " + tojson(gle) + " doc:" + tojson(t.findOne()) });
+
+// Replacement tests
+// Query is ignored for replacements, except _id field.
+t.update( {r: {a: 1, b: 1}, "r.a": 1} , {y: 1} , true );
+assert.gleSuccess(db, "r-1");
+assert(t.findOne().y, 1, "inserted doc missing field")
+var docMinusId = t.findOne();
+delete docMinusId._id
+assert.docEq({y: 1}, docMinusId, "r-1")
+t.drop()
+
+t.update( {_id: {a:1, b:1}, "_id.a": 1} , {y: 1} , true );
+assert.gleSuccess(db, "_id-4");
+assert.docEq({_id: {a: 1, b: 1}, y: 1}, t.findOne(), "_id-4")
+t.drop()
\ No newline at end of file