diff options
| -rw-r--r-- | jstests/core/find_and_modify.js | 66 | ||||
| -rw-r--r-- | src/mongo/db/commands/find_and_modify.cpp | 25 |
2 files changed, 82 insertions, 9 deletions
diff --git a/jstests/core/find_and_modify.js b/jstests/core/find_and_modify.js index d384f02b4bc..afaeda3d9a9 100644 --- a/jstests/core/find_and_modify.js +++ b/jstests/core/find_and_modify.js @@ -45,19 +45,81 @@ assert.throws(function() { t.findAndModify({query:{x:1}, update:{y:2}, remove:tr assert.throws(function() { t.findAndModify({query:{x:1}, update:{y:2}, new:true, remove:true}); }); assert.throws(function() { t.findAndModify({query:{x:1}, upsert:true, remove:true}); }); -// SERVER-17372 +// +// SERVER-17387: Find and modify should throw in the case of invalid projection. +// + t.drop(); + +// Insert case. var cmdRes = db.runCommand({ findAndModify: t.getName(), query: {_id: "miss"}, update: {$inc: {y: 1}}, + fields: {foo: {$pop: ["bar"]}}, + upsert: true, + new: true +}); +assert.commandFailed(cmdRes); + +t.insert({_id: "found"}); + +// Update with upsert + new. +cmdRes = db.runCommand({ + findAndModify: t.getName(), + query: {_id: "found"}, + update: {$inc: {y: 1}}, + fields: {foo: {$pop: ["bar"]}}, + upsert: true, + new: true +}); +assert.commandFailed(cmdRes); + +// Update with just new: true. +cmdRes = db.runCommand({ + findAndModify: t.getName(), + query: {_id: "found"}, + update: {$inc: {y: 1}}, + fields: {foo: {$pop: ["bar"]}}, + new: true +}); +assert.commandFailed(cmdRes); + +// Update with just upsert: true. +cmdRes = db.runCommand({ + findAndModify: t.getName(), + query: {_id: "found"}, + update: {$inc: {y: 1}}, + fields: {foo: {$pop: ["bar"]}}, + upsert: true +}); +assert.commandFailed(cmdRes); + +// Update with neither upsert nor new flags. +cmdRes = db.runCommand({ + findAndModify: t.getName(), + query: {_id: "found"}, + update: {$inc: {y: 1}}, + fields: {foo: {$pop: ["bar"]}}, +}); +assert.commandFailed(cmdRes); + +// +// SERVER-17372 +// + +t.drop(); +cmdRes = db.runCommand({ + findAndModify: t.getName(), + query: {_id: "miss"}, + update: {$inc: {y: 1}}, upsert: true }); assert.commandWorked(cmdRes); assert("value" in cmdRes); assert.eq(null, cmdRes.value); -var cmdRes = db.runCommand({ +cmdRes = db.runCommand({ findAndModify: t.getName(), query: {_id: "missagain"}, update: {$inc: {y: 1}}, diff --git a/src/mongo/db/commands/find_and_modify.cpp b/src/mongo/db/commands/find_and_modify.cpp index 38d75deb5b9..acd9dbed9b1 100644 --- a/src/mongo/db/commands/find_and_modify.cpp +++ b/src/mongo/db/commands/find_and_modify.cpp @@ -372,6 +372,14 @@ namespace mongo { if ( found ) { deleteObjects(txn, ctx.db(), ns, queryModified, PlanExecutor::YIELD_MANUAL, true, true); + + // Committing the WUOW can close the current snapshot. Until this happens, the + // snapshot id should not have changed. + invariant(txn->recoveryUnit()->getSnapshotId() == snapshotDoc.snapshotId()); + + // Must commit the write before doing anything that could throw. + wuow.commit(); + BSONObjBuilder le( result.subobjStart( "lastErrorObject" ) ); le.appendNumber( "n" , 1 ); le.done(); @@ -424,6 +432,9 @@ namespace mongo { // for some BSON manipulation). repl::logOp(txn, "i", collection->ns().ns().c_str(), newDoc); + // Must commit the write and logOp() before doing anything that could throw. + wuow.commit(); + // The third argument indicates whether or not we have something for the // 'value' field returned by a findAndModify command. // @@ -470,6 +481,13 @@ namespace mongo { invariant(res.existing); LOG(3) << "update result: " << res; + // Committing the WUOW can close the current snapshot. Until this happens, the + // snapshot id should not have changed. + invariant(txn->recoveryUnit()->getSnapshotId() == snapshotDoc.snapshotId()); + + // Must commit the write before doing anything that could throw. + wuow.commit(); + if (returnNew) { dassert(!res.newObj.isEmpty()); _appendHelper(result, res.newObj, true, fields, whereCallback); @@ -485,13 +503,6 @@ namespace mongo { } } - // Committing the WUOW can close the current snapshot. Until this happens, the - // snapshot id should not have changed. - if (found) { - invariant(txn->recoveryUnit()->getSnapshotId() == snapshotDoc.snapshotId()); - } - wuow.commit(); - return true; } |