SERVER-38909 Permit empty update modifiers, treating as a no-op rather than an error

(cherry picked from commit ad51c2da0567585391d9e02529d4a495aa460c4d)

5 files changed, 372 insertions, 85 deletions

diff --git a/jstests/core/verify_update_mods.js b/jstests/core/verify_update_mods.js
index 134668e62bd..184b98ef14d 100644
--- a/jstests/core/verify_update_mods.js
+++ b/jstests/core/verify_update_mods.js
@@ -1,82 +1,310 @@
-// Cannot implicitly shard accessed collections because of following errmsg: A single
-// update/delete on a sharded collection must contain an exact match on _id or contain the shard
-// key.
-// @tags: [assumes_unsharded_collection, requires_non_retryable_writes]
+/**
+ * Tests update and findAndModify command behavior with update modifiers.
+ *
+ * @tags: [
+ * requires_fcv_42,
+ * # The test is designed to work with an unsharded collection.
+ * assumes_unsharded_collection,
+ * # The coll.update command does not work with $set operator in compatibility write mode.
+ * requires_find_command,
+ * # Performs modifications that if repeated would fail the test.
+ * requires_non_retryable_writes,
+ * ]
+ */
+(function() {
+"use strict";
 
-// Verify update mods exist
-var res;
-t = db.update_mods;
-t.drop();
+const testDB = db.getSiblingDB(jsTestName());
+assert.commandWorked(testDB.dropDatabase());
+const coll = testDB.update_modifiers;
 
-t.save({_id: 1});
-res = t.update({}, {$set: {a: 1}});
-assert.writeOK(res);
-t.remove({});
+// Executes a test case which inserts documents into the test collection, issues an update command,
+// and verifies that the results are as expected.
+function executeUpdateTestCase(testCase) {
+    jsTestLog(tojson(testCase));
 
-t.save({_id: 1});
-res = t.update({}, {$unset: {a: 1}});
-assert.writeOK(res);
-t.remove({});
+    // Remove all existing documents and then insert the new test case's documents.
+    assert.commandWorked(coll.remove({}));
+    assert.commandWorked(coll.insert(testCase.inputDocuments));
 
-t.save({_id: 1});
-res = t.update({}, {$inc: {a: 1}});
-assert.writeOK(res);
-t.remove({});
+    // Issue the update command specified in the test case.
+    const result = testCase.command(coll, testCase.query, testCase.update, testCase.arrayFilters);
 
-t.save({_id: 1});
-res = t.update({}, {$mul: {a: 1}});
-assert.writeOK(res);
-t.remove({});
+    if (testCase.expectedErrorCode == undefined) {
+        // Verify that the command succeeded and collection's contents match the expected results.
+        assert.commandWorked(result);
+        assert.docEq(coll.find({}).sort({_id: 1}).toArray(), testCase.expectedResults);
+    } else {
+        assert.commandFailedWithCode(result, testCase.expectedErrorCode);
+    }
+}
 
-t.save({_id: 1});
-res = t.update({}, {$push: {a: 1}});
-assert.writeOK(res);
-t.remove({});
+// Issues the update command and returns the response.
+function updateCommand(coll, query, update, arrayFilters) {
+    const commandOptions = {upsert: true};
+    if (arrayFilters !== undefined) {
+        commandOptions.arrayFilters = arrayFilters;
+    }
+    return coll.update(query, update, commandOptions);
+}
 
-res = t.update({}, {$addToSet: {a: 1}});
-assert.writeOK(res);
-t.remove({});
+// Issues the findAndModify command and returns the response.
+function findAndModifyCommand(coll, query, update, arrayFilters) {
+    const command = {query: query, update: update, upsert: true};
+    if (arrayFilters !== undefined) {
+        command.arrayFilters = arrayFilters;
+    }
+    return coll.runCommand("findAndModify", command);
+}
 
-t.save({_id: 1});
-res = t.update({}, {$pull: {a: 1}});
-assert.writeOK(res);
-t.remove({});
+// Tests all relevant update modifiers with set and empty update documents.
+const testCases = [
+    {
+        query: {_id: 1},
+        update: {$set: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 2},
+        update: {$set: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}, {_id: 2}],
+    },
+    {
+        query: {_id: 1},
+        update: {$set: {a: 2}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 2}],
+    },
+    {
+        query: {_id: 1},
+        update: {$set: {}},
+        arrayFilters: [{"element": {$gt: 6}}],
+        inputDocuments: [{_id: 1, a: [1]}],
+        expectedErrorCode: ErrorCodes.FailedToParse,
+    },
+    {
+        query: {_id: 1},
+        update: {$unset: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$unset: {a: 1}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$inc: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$inc: {a: 1}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 2}],
+    },
+    {
+        query: {_id: 1},
+        update: {$mul: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$mul: {a: 2}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 2}],
+    },
+    {
+        query: {_id: 1},
+        update: {$push: {}},
+        inputDocuments: [{_id: 1, a: [1]}],
+        expectedResults: [{_id: 1, a: [1]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$push: {a: 2}},
+        inputDocuments: [{_id: 1, a: [1]}],
+        expectedResults: [{_id: 1, a: [1, 2]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$addToSet: {}},
+        inputDocuments: [{_id: 1, a: [1]}],
+        expectedResults: [{_id: 1, a: [1]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$addToSet: {a: 2}},
+        inputDocuments: [{_id: 1, a: [1]}],
+        expectedResults: [{_id: 1, a: [1, 2]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pull: {}},
+        inputDocuments: [{_id: 1, a: [1]}],
+        expectedResults: [{_id: 1, a: [1]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pull: {a: 1}},
+        inputDocuments: [{_id: 1, a: [1]}],
+        expectedResults: [{_id: 1, a: []}],
+    },
+    {
+        query: {_id: 1},
+        update: {$rename: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$rename: {a: "b"}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, b: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$bit: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$bit: {a: {and: NumberLong(1)}}},
+        inputDocuments: [{_id: 1, a: NumberLong(2)}],
+        expectedResults: [{_id: 1, a: NumberLong(0)}],
+    },
+    {
+        query: {_id: 1},
+        update: {$bit: {a: {and: NumberLong(3)}}},
+        inputDocuments: [],
+        expectedResults: [{_id: 1, a: NumberLong(0)}],
+    },
+    {
+        query: {_id: 1},
+        update: {$bit: {b: {or: NumberLong(3)}}},
+        inputDocuments: [],
+        expectedResults: [{_id: 1, b: NumberLong(3)}],
+    },
+    {
+        query: {_id: 1},
+        update: {$bit: {"c.d": {or: NumberInt(3)}}},
+        inputDocuments: [],
+        expectedResults: [{_id: 1, c: {d: NumberInt(3)}}],
+    },
+    {
+        query: {_id: 1},
+        update: {$max: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$max: {a: 2}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 2}],
+    },
+    {
+        query: {_id: 1},
+        update: {$min: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$min: {a: 0}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 0}],
+    },
+    {
+        query: {_id: 1},
+        update: {$currentDate: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$setOnInsert: {}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 2},
+        update: {$setOnInsert: {a: 1}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}, {_id: 2, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$setOnInsert: {a: 2}},
+        inputDocuments: [{_id: 1, a: 1}],
+        expectedResults: [{_id: 1, a: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pop: {}},
+        inputDocuments: [{_id: 1, a: [1]}],
+        expectedResults: [{_id: 1, a: [1]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pop: {a: true}},
+        inputDocuments: [{_id: 1, a: [1, 2]}],
+        expectedErrorCode: ErrorCodes.FailedToParse,
+    },
+    {
+        query: {_id: 1},
+        update: {$pop: {a: 1}},
+        inputDocuments: [{_id: 1, a: [1, 2]}],
+        expectedResults: [{_id: 1, a: [1]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pop: {a: -1}},
+        inputDocuments: [{_id: 1, a: [1, 2]}],
+        expectedResults: [{_id: 1, a: [2]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pop: {a: 1}},
+        inputDocuments: [{_id: 1, a: []}],
+        expectedResults: [{_id: 1, a: []}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pop: {a: 1}},
+        inputDocuments: [],
+        expectedResults: [{_id: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$currentDate: {}},
+        inputDocuments: [{_id: 1}],
+        expectedResults: [{_id: 1}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pullAll: {}},
+        inputDocuments: [{_id: 1, a: [1, 2]}],
+        expectedResults: [{_id: 1, a: [1, 2]}],
+    },
+    {
+        query: {_id: 1},
+        update: {$pullAll: {a: [1]}},
+        inputDocuments: [{_id: 1, a: [1, 2, 1]}],
+        expectedResults: [{_id: 1, a: [2]}],
+    },
+];
 
-t.save({_id: 1});
-res = t.update({}, {$pop: {a: true}});
-assert.writeError(res);
-t.remove({});
-
-t.save({_id: 1});
-res = t.update({}, {$rename: {a: "b"}});
-assert.writeOK(res);
-t.remove({});
-
-t.save({_id: 1});
-res = t.update({}, {$bit: {a: {and: NumberLong(1)}}});
-assert.writeOK(res);
-t.remove({});
-
-// SERVER-3223 test $bit can do an upsert
-t.update({_id: 1}, {$bit: {a: {and: NumberLong(3)}}}, true);
-assert.eq(t.findOne({_id: 1}).a, NumberLong(0), "$bit upsert with and");
-t.update({_id: 2}, {$bit: {b: {or: NumberLong(3)}}}, true);
-assert.eq(t.findOne({_id: 2}).b, NumberLong(3), "$bit upsert with or (long)");
-t.update({_id: 3}, {$bit: {"c.d": {or: NumberInt(3)}}}, true);
-assert.eq(t.findOne({_id: 3}).c.d, NumberInt(3), "$bit upsert with or (int)");
-t.remove({});
-
-t.save({_id: 1});
-res = t.update({}, {$currentDate: {a: true}});
-assert.writeOK(res);
-t.remove({});
-
-t.save({_id: 1});
-res = t.update({}, {$max: {a: 1}});
-assert.writeOK(res);
-t.remove({});
-
-t.save({_id: 1});
-res = t.update({}, {$min: {a: 1}});
-assert.writeOK(res);
-t.remove({});
+for (const command of [updateCommand, findAndModifyCommand]) {
+    for (const testCase of testCases) {
+        executeUpdateTestCase(Object.assign({command: command}, testCase));
+    }
+}
+})();
diff --git a/jstests/libs/parallelTester.js b/jstests/libs/parallelTester.js
index 5628b232abb..fa75f509612 100644
--- a/jstests/libs/parallelTester.js
+++ b/jstests/libs/parallelTester.js
@@ -224,6 +224,7 @@ if (typeof _threadInject != "undefined") {
                 "merge_sort_collation.js",
                 "update_pipeline_shell_helpers.js",
                 "update_with_pipeline.js",
+                "verify_update_mods.js",
                 "views/views_aggregation.js",
                 "views/views_change.js",
                 "views/views_drop.js",
diff --git a/jstests/multiVersion/empty_update_mods_multiversion_replication.js b/jstests/multiVersion/empty_update_mods_multiversion_replication.js
new file mode 100644
index 00000000000..e16134ef4f6
--- /dev/null
+++ b/jstests/multiVersion/empty_update_mods_multiversion_replication.js
@@ -0,0 +1,68 @@
+/**
+ * Verifies that update commands with empty update modifiers do not produce oplog entries, and
+ * therefore do not interfere with 4.0 - 4.2 mixed-mode replication.
+ */
+(function() {
+"use strict";
+
+// Setup a two-node replica set - the primary is the latest version node and the secondary - 4.0.
+TestData.replSetFeatureCompatibilityVersion = '4.0';
+const rst = new ReplSetTest({
+    name: jsTestName(),
+    nodes: [
+        {binVersion: "latest"},
+        {rsConfig: {priority: 0, votes: 0}},
+    ]
+});
+rst.startSet();
+rst.initiate();
+rst.restart(1, {binVersion: "last-stable"});
+
+const testDB = rst.getPrimary().getDB("test");
+const coll = testDB[jsTestName()];
+
+// Insert a test document.
+assert.commandWorked(coll.insert({_id: 1, a: 1}));
+
+// Issue update commands with empty update modifiers on the latest version node.
+const updateModifiers = [
+    "$set",
+    "$unset",
+    "$inc",
+    "$mul",
+    "$push",
+    "$addToSet",
+    "$pull",
+    "$rename",
+    "$bit",
+    "$max",
+    "$min",
+    "$currentDate",
+    "$setOnInsert",
+    "$pop",
+    "$pullAll"
+];
+for (const modifier of updateModifiers) {
+    assert.commandWorked(coll.update({_id: 1}, {[modifier]: {}}));
+}
+
+// Verify that the oplog does not have any entries for the update commands with empty update
+// modifiers.
+const oplogMatches = rst.findOplog(rst.getPrimary(), {ns: coll.getFullName()}, 10).itcount();
+assert.eq(1,
+          oplogMatches,
+          rst.findOplog(rst.getPrimary(),
+                        {},
+                        10)
+              .toArray());  // Only insert record is found.
+
+// Issue an update command that modifies the document.
+assert.commandWorked(coll.update({_id: 1}, {$set: {a: 2}}));
+
+// Verify that the update command was propagated to the secondary node.
+rst.awaitReplication();
+assert.docEq(coll.find().toArray(),
+             rst.getSecondary().getCollection(coll.getFullName()).find().toArray());
+
+rst.stopSet();
+})();
\ No newline at end of file
diff --git a/src/mongo/db/update/update_driver.cpp b/src/mongo/db/update/update_driver.cpp
index 0349deb4e8f..d3fae0f1a6f 100644
--- a/src/mongo/db/update/update_driver.cpp
+++ b/src/mongo/db/update/update_driver.cpp
@@ -86,13 +86,6 @@ modifiertable::ModifierType validateMod(BSONElement mod) {
                           << " not {" << mod << "}",
             mod.type() == BSONType::Object);
 
-    uassert(ErrorCodes::FailedToParse,
-            str::stream() << "'" << mod.fieldName()
-                          << "' is empty. You must specify a field like so: "
-                             "{"
-                          << mod.fieldName() << ": {<field>: ...}}",
-            !mod.embeddedObject().isEmpty());
-
     return modType;
 }
 
diff --git a/src/mongo/db/update/update_driver_test.cpp b/src/mongo/db/update/update_driver_test.cpp
index dcfab0d74fb..53e43250963 100644
--- a/src/mongo/db/update/update_driver_test.cpp
+++ b/src/mongo/db/update/update_driver_test.cpp
@@ -121,11 +121,8 @@ TEST(Parse, EmptyMod) {
     boost::intrusive_ptr<ExpressionContextForTest> expCtx(new ExpressionContextForTest());
     UpdateDriver driver(expCtx);
     std::map<StringData, std::unique_ptr<ExpressionWithPlaceholder>> arrayFilters;
-    ASSERT_THROWS_CODE_AND_WHAT(
-        driver.parse(fromjson("{$set:{}}"), arrayFilters),
-        AssertionException,
-        ErrorCodes::FailedToParse,
-        "'$set' is empty. You must specify a field like so: {$set: {<field>: ...}}");
+    // Verifies that {$set: {}} is accepted.
+    ASSERT_DOES_NOT_THROW(driver.parse(fromjson("{$set: {}}"), arrayFilters));
 }
 
 TEST(Parse, WrongMod) {