From b98712c551e8ab27c33e1a5e7c694fa36c3334ce Mon Sep 17 00:00:00 2001 From: Scott Hernandez Date: Thu, 7 Nov 2013 12:42:20 -0500 Subject: SERVER-11531, SERVER-10489, SERVER-6835, SERVER-4830: Refactor update system to support immutable fields, consolodate storage validation, and misc issues. --- jstests/rename4.js | 11 +--- jstests/repl/basic1.js | 3 + jstests/sharding/update_immutable_fields.js | 98 +++++++++++++++++++++++++++++ jstests/update_dbref.js | 35 +++++++++++ jstests/update_replace.js | 45 +++++++++++++ jstests/update_setOnInsert.js | 18 +++++- jstests/upsert1.js | 1 + jstests/upsert3.js | 51 +++++++++++++++ 8 files changed, 253 insertions(+), 9 deletions(-) create mode 100644 jstests/sharding/update_immutable_fields.js create mode 100644 jstests/update_dbref.js create mode 100644 jstests/update_replace.js create mode 100644 jstests/upsert3.js (limited to 'jstests') 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 -- cgit v1.2.1