SERVER-61980 ensure oplog contains "unique" if collMod "hidden" parameter is a no-op

2 files changed, 40 insertions, 6 deletions

diff --git a/jstests/noPassthrough/hidden_index_noop.js b/jstests/noPassthrough/hidden_index_noop.js
index 9412fe69695..9c8a9830093 100644
--- a/jstests/noPassthrough/hidden_index_noop.js
+++ b/jstests/noPassthrough/hidden_index_noop.js
@@ -1,10 +1,16 @@
 /**
  * Validate that the 'collMod' command with 'hidden' field will return expected result document for
  * the command and generate expected oplog entries in which hiding a hidden index or un-hiding a
- * visible index will be a no-op if TTL index option is not involved.
+ * visible index will be a no-op if no other index option (TTL or unique) is involved.
  *
  * @tags: [
- *   requires_replication,
+ *  # TODO(SERVER-61181): Fix validation errors under ephemeralForTest.
+ *  incompatible_with_eft,
+ *  # TODO(SERVER-61182): Fix WiredTigerKVEngine::alterIdentMetadata() under inMemory.
+ *  requires_persistence,
+ *  # Replication requires journaling support so this tag also implies exclusion from
+ *  # --nojournal test configurations.
+ *  requires_replication,
  * ]
  */
 
@@ -23,6 +29,10 @@ const primaryDB = primary.getDB(dbName);
 const primaryColl = primaryDB[collName];
 const oplogColl = primary.getDB("local")['oplog.rs'];
 
+const collModIndexUniqueEnabled =
+    assert.commandWorked(primary.adminCommand({getParameter: 1, featureFlagCollModIndexUnique: 1}))
+        .featureFlagCollModIndexUnique.value;
+
 // Validate that the generated oplog entries filtered by given filter are what we expect.
 function validateCollModOplogEntryCount(hiddenFilter, expectedCount) {
     let filter = {
@@ -40,11 +50,13 @@ function validateResultForCollMod(result, expectedResult) {
     assert.eq(result.hidden_new, expectedResult.hidden_new, result);
     assert.eq(result.expireAfterSeconds_old, expectedResult.expireAfterSeconds_old, result);
     assert.eq(result.expireAfterSeconds_new, expectedResult.expireAfterSeconds_new, result);
+    assert.eq(result.unique_new, expectedResult.unique_new, result);
 }
 
 primaryColl.drop();
 assert.commandWorked(primaryColl.createIndex({a: 1}));
 assert.commandWorked(primaryColl.createIndex({b: 1}, {expireAfterSeconds: 5}));
+assert.commandWorked(primaryColl.createIndex({c: 1}));
 
 // Hiding a non-hidden index will generate the oplog entry with a 'hidden_old: false'.
 let result = assert.commandWorked(primaryColl.hideIndex('a_1'));
@@ -83,9 +95,30 @@ validateCollModOplogEntryCount({
                                1);
 
 // Test that the index was successfully modified.
-const idxSpec = GetIndexHelpers.findByName(primaryColl.getIndexes(), "b_1");
+let idxSpec = GetIndexHelpers.findByName(primaryColl.getIndexes(), "b_1");
 assert.eq(idxSpec.hidden, undefined);
 assert.eq(idxSpec.expireAfterSeconds, 10);
 
+// Validate that if both 'unique' and 'hidden' options are specified but the 'hidden'
+// option is a no-op, the operation as a whole will NOT be a no-op - instead, it will generate an
+// oplog entry with only 'unique'. Ditto for the command result returned to the user.
+if (collModIndexUniqueEnabled) {
+    result = assert.commandWorked(primaryDB.runCommand({
+        "collMod": primaryColl.getName(),
+        "index": {"name": "c_1", "unique": true, "hidden": false},
+    }));
+    validateResultForCollMod(result, {unique_new: true});
+    validateCollModOplogEntryCount({
+        "o.index.unique": true,
+    },
+                                   1);
+
+    // Test that the index was successfully modified.
+    idxSpec = GetIndexHelpers.findByName(primaryColl.getIndexes(), "c_1");
+    assert.eq(idxSpec.hidden, undefined);
+    assert.eq(idxSpec.expireAfterSeconds, undefined);
+    assert(idxSpec.unique, tojson(idxSpec));
+}
+
 rst.stopSet();
 })();
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp
index 7994de0fb7a..dae99458ba8 100644
--- a/src/mongo/db/catalog/coll_mod.cpp
+++ b/src/mongo/db/catalog/coll_mod.cpp
@@ -287,9 +287,10 @@ StatusWith<ParsedCollModRequest> parseCollModRequest(OperationContext* opCtx,
                 cmr.numModifications++;
                 // Hiding a hidden index or unhiding a visible index should be treated as a no-op.
                 if (cmrIndex->idx->hidden() == cmrIndex->indexHidden.booleanSafe()) {
-                    // If the collMod includes "expireAfterSeconds", remove the no-op "hidden"
-                    // parameter and write the remaining "index" object to the oplog entry builder.
-                    if (!cmrIndex->indexExpireAfterSeconds.eoo()) {
+                    // If the collMod includes "expireAfterSeconds" or "unique", remove the no-op
+                    // "hidden" parameter and write the remaining "index" object to the oplog entry
+                    // builder.
+                    if (!cmrIndex->indexExpireAfterSeconds.eoo() || !cmrIndex->indexUnique.eoo()) {
                         oplogEntryBuilder->append(fieldName, indexObj.removeField("hidden"));
                     }
                     // Un-set "indexHidden" in ParsedCollModRequest, and skip the automatic write to