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

(cherry picked from commit 96bc79c2fa95e40a917c71b4120257f8dca038d0)

6 files changed, 384 insertions, 87 deletions

diff --git a/jstests/core/verify_update_mods.js b/jstests/core/verify_update_mods.js
index 134668e62bd..f33178e3e40 100644
--- a/jstests/core/verify_update_mods.js
+++ b/jstests/core/verify_update_mods.js
@@ -1,82 +1,313 @@
-// 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_40,
+ * # 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 (let command of[updateCommand, findAndModifyCommand]) {
+        for (let testCase of testCases) {
+            executeUpdateTestCase(Object.assign({command: command}, testCase));
+        }
+    }
+})();
diff --git a/jstests/libs/parallelTester.js b/jstests/libs/parallelTester.js
index 870e97193a0..bbac480fe21 100644
--- a/jstests/libs/parallelTester.js
+++ b/jstests/libs/parallelTester.js
@@ -238,6 +238,7 @@ if (typeof _threadInject != "undefined") {
         if (db.getMongo().readMode() === "legacy") {
             var requires_find_command = [
                 "merge_sort_collation.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..558573495da
--- /dev/null
+++ b/jstests/multiVersion/empty_update_mods_multiversion_replication.js
@@ -0,0 +1,69 @@
+/**
+ * Verifies that update commands with empty update modifiers do not produce oplog entries, and
+ * therefore do not interfere with 3.6 - 4.0 mixed-mode replication.
+ */
+(function() {
+    "use strict";
+
+    // Setup a two-node replica set - the primary is the latest version node and the secondary -
+    // 3.6.
+    TestData.replSetFeatureCompatibilityVersion = '3.6';
+    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 (let 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 7f756807d17..b94aa94fef6 100644
--- a/src/mongo/db/update/update_driver.cpp
+++ b/src/mongo/db/update/update_driver.cpp
@@ -93,15 +93,6 @@ modifiertable::ModifierType validateMod(BSONElement mod) {
                           << 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 dd4be1d8ee6..19b2618c08e 100644
--- a/src/mongo/db/update/update_driver_test.cpp
+++ b/src/mongo/db/update/update_driver_test.cpp
@@ -88,11 +88,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).transitional_ignore(),
-        AssertionException,
-        ErrorCodes::FailedToParse,
-        "'$set' is empty. You must specify a field like so: {$set: {<field>: ...}}");
+    // Verifies that {$set: {}} is accepted.
+    ASSERT_OK(driver.parse(fromjson("{$set: {}}"), arrayFilters));
 }
 
 TEST(Parse, WrongMod) {
diff --git a/src/mongo/shell/replsettest.js b/src/mongo/shell/replsettest.js
index d39f6d69129..4f20fbcc226 100644
--- a/src/mongo/shell/replsettest.js
+++ b/src/mongo/shell/replsettest.js
@@ -1772,6 +1772,14 @@ var ReplSetTest = function(opts) {
         return {master: hashes[0], slaves: hashes.slice(1)};
     };
 
+    this.findOplog = function(conn, query, limit) {
+        return conn.getDB('local')
+            .getCollection(oplogName)
+            .find(query)
+            .sort({$natural: -1})
+            .limit(limit);
+    };
+
     this.dumpOplog = function(conn, query = {}, limit = 10) {
         var log = 'Dumping the latest ' + limit + ' documents that match ' + tojson(query) +
             ' from the oplog ' + oplogName + ' of ' + conn.host;