SERVER-67649 Teach IDL reply types to ignore generic fields

5 files changed, 86 insertions, 40 deletions

diff --git a/src/mongo/idl/command_generic_argument.cpp b/src/mongo/idl/command_generic_argument.cpp
index 4d5c2861a6f..83532622893 100644
--- a/src/mongo/idl/command_generic_argument.cpp
+++ b/src/mongo/idl/command_generic_argument.cpp
@@ -37,6 +37,11 @@ bool isGenericArgument(StringData arg) {
     return Generic_args_api_v1::hasField(arg) || Generic_args_unstable_v1::hasField(arg);
 }
 
+bool isGenericReply(StringData arg) {
+    return Generic_reply_fields_api_v1::hasField(arg) ||
+        Generic_reply_fields_unstable_v1::hasField(arg);
+}
+
 bool shouldForwardToShards(StringData arg) {
     return Generic_args_api_v1::shouldForwardToShards(arg) &&
         Generic_args_unstable_v1::shouldForwardToShards(arg);
diff --git a/src/mongo/idl/command_generic_argument.h b/src/mongo/idl/command_generic_argument.h
index de8d264b14e..2b2ef2e9cd6 100644
--- a/src/mongo/idl/command_generic_argument.h
+++ b/src/mongo/idl/command_generic_argument.h
@@ -41,6 +41,13 @@ namespace mongo {
 bool isGenericArgument(StringData arg);
 
 /**
+ * Returns true if the provided reply field is one that is handled by the command processing layer
+ * and should generally be ignored by individual command implementations. In particular,
+ * replies that fail on unrecognized fields must not fail for any of these.
+ */
+bool isGenericReply(StringData arg);
+
+/**
  * Returns true if arg should be forwarded to shards.
  *
  * See 'CommandHelpers::filterCommandRequestForPassthrough'.
diff --git a/src/mongo/idl/command_generic_argument_test.cpp b/src/mongo/idl/command_generic_argument_test.cpp
index 7403b859821..46b0e23f26d 100644
--- a/src/mongo/idl/command_generic_argument_test.cpp
+++ b/src/mongo/idl/command_generic_argument_test.cpp
@@ -41,57 +41,64 @@ using namespace fmt::literals;
 // SERVER-51848. We will test that the IDL definitions match these old C++ definitions.
 struct SpecialArgRecord {
     StringData name;
-    bool isGeneric;
+    bool isGenericArgument;
+    bool isGenericReply;
     bool stripFromRequest;
     bool stripFromReply;
 };
 
 // clang-format off
 static constexpr std::array<SpecialArgRecord, 34> specials{{
-    //                                       /-isGeneric
-    //                                       |  /-stripFromRequest
-    //                                       |  |  /-stripFromReply
-    {"apiVersion"_sd,                        1, 1, 0},
-    {"apiStrict"_sd,                         1, 1, 0},
-    {"apiDeprecationErrors"_sd,              1, 1, 0},
-    {"$audit"_sd,                            1, 1, 0},
-    {"$client"_sd,                           1, 1, 0},
-    {"$configServerState"_sd,                1, 1, 1},
-    {"$db"_sd,                               1, 1, 0},
-    {"allowImplicitCollectionCreation"_sd,   1, 1, 0},
-    {"$oplogQueryData"_sd,                   1, 1, 1},
-    {"$queryOptions"_sd,                     1, 0, 0},
-    {"$readPreference"_sd,                   1, 1, 0},
-    {"$replData"_sd,                         1, 1, 1},
-    {"$clusterTime"_sd,                      1, 1, 1},
-    {"maxTimeMS"_sd,                         1, 0, 0},
-    {"readConcern"_sd,                       1, 0, 0},
-    {"databaseVersion"_sd,                   1, 1, 0},
-    {"shardVersion"_sd,                      1, 1, 0},
-    {"tracking_info"_sd,                     1, 1, 0},
-    {"writeConcern"_sd,                      1, 0, 0},
-    {"lsid"_sd,                              1, 0, 0},
-    {"clientOperationKey"_sd,                1, 0, 0},
-    {"txnNumber"_sd,                         1, 0, 0},
-    {"autocommit"_sd,                        1, 0, 0},
-    {"coordinator"_sd,                       1, 0, 0},
-    {"startTransaction"_sd,                  1, 0, 0},
-    {"stmtId"_sd,                            1, 0, 0},
-    {"$gleStats"_sd,                         0, 0, 1},
-    {"operationTime"_sd,                     0, 0, 1},
-    {"lastCommittedOpTime"_sd,               0, 0, 1},
-    {"readOnly"_sd,                          0, 0, 1},
-    {"comment"_sd,                           1, 0, 0},
-    {"maxTimeMSOpOnly"_sd,                   1, 1, 0},
-    {"$configTime"_sd,                       1, 1, 1},
-    {"$topologyTime"_sd,                     1, 1, 1}}};
+    //                                       /-isGenericArgument
+    //                                       |  /-isGenericReply
+    //                                       |  |  /-stripFromRequest
+    //                                       |  |  |  /-stripFromReply
+    {"apiVersion"_sd,                        1, 0, 1, 0},
+    {"apiStrict"_sd,                         1, 0, 1, 0},
+    {"apiDeprecationErrors"_sd,              1, 0, 1, 0},
+    {"$audit"_sd,                            1, 0, 1, 0},
+    {"$client"_sd,                           1, 0, 1, 0},
+    {"$configServerState"_sd,                1, 1, 1, 1},
+    {"$db"_sd,                               1, 0, 1, 0},
+    {"allowImplicitCollectionCreation"_sd,   1, 0, 1, 0},
+    {"$oplogQueryData"_sd,                   1, 1, 1, 1},
+    {"$queryOptions"_sd,                     1, 0, 0, 0},
+    {"$readPreference"_sd,                   1, 0, 1, 0},
+    {"$replData"_sd,                         1, 1, 1, 1},
+    {"$clusterTime"_sd,                      1, 1, 1, 1},
+    {"maxTimeMS"_sd,                         1, 0, 0, 0},
+    {"readConcern"_sd,                       1, 0, 0, 0},
+    {"databaseVersion"_sd,                   1, 0, 1, 0},
+    {"shardVersion"_sd,                      1, 0, 1, 0},
+    {"tracking_info"_sd,                     1, 0, 1, 0},
+    {"writeConcern"_sd,                      1, 0, 0, 0},
+    {"lsid"_sd,                              1, 0, 0, 0},
+    {"clientOperationKey"_sd,                1, 0, 0, 0},
+    {"txnNumber"_sd,                         1, 0, 0, 0},
+    {"autocommit"_sd,                        1, 0, 0, 0},
+    {"coordinator"_sd,                       1, 0, 0, 0},
+    {"startTransaction"_sd,                  1, 0, 0, 0},
+    {"stmtId"_sd,                            1, 0, 0, 0},
+    {"$gleStats"_sd,                         0, 1, 0, 1},
+    {"operationTime"_sd,                     0, 1, 0, 1},
+    {"lastCommittedOpTime"_sd,               0, 1, 0, 1},
+    {"readOnly"_sd,                          0, 1, 0, 1},
+    {"comment"_sd,                           1, 0, 0, 0},
+    {"maxTimeMSOpOnly"_sd,                   1, 0, 1, 0},
+    {"$configTime"_sd,                       1, 1, 1, 1},
+    {"$topologyTime"_sd,                     1, 1, 1, 1}}};
 // clang-format on
 
 TEST(CommandGenericArgument, AllGenericArgumentsAndReplyFields) {
     for (const auto& record : specials) {
-        if (isGenericArgument(record.name) != record.isGeneric) {
+        if (isGenericArgument(record.name) != record.isGenericArgument) {
             FAIL("isGenericArgument('{}') should be {}, but it's {}"_format(
-                record.name, record.isGeneric, isGenericArgument(record.name)));
+                record.name, record.isGenericArgument, isGenericArgument(record.name)));
+        }
+
+        if (isGenericReply(record.name) != record.isGenericReply) {
+            FAIL("isGenericReply('{}') should be {}, but it's {}"_format(
+                record.name, record.isGenericReply, isGenericReply(record.name)));
         }
 
         if (shouldForwardToShards(record.name) == record.stripFromRequest) {
diff --git a/src/mongo/idl/idl_test.cpp b/src/mongo/idl/idl_test.cpp
index 823ea92e70a..53750d790d1 100644
--- a/src/mongo/idl/idl_test.cpp
+++ b/src/mongo/idl/idl_test.cpp
@@ -3896,6 +3896,32 @@ TEST(IDLTypeCommand, TestCommandWithIDLAnyTypeOwnedField) {
                                                         << "b"))["anyTypeField"]);
 }
 
+TEST(IDLTypeCommand, ReplyTypeKnowsItIsReplyAtCompileTime) {
+    Reply_type_struct reply;
+    static_assert(reply.getIsCommandReply());
+    StructWithEnum nonReply;
+    static_assert(!nonReply.getIsCommandReply());
+}
+
+TEST(IDLTypeCommand, ReplyTypeCanParseWithGenericFields) {
+    // $clusterTime is not a field of Rely_type_struct, but is
+    // a field that could be part of any reply.
+    StringData genericField = "$clusterTime"_sd;
+    // This field is not part of Reply_type_struct and is also
+    // not a generic field.
+    StringData nonGenericField = "xyz123"_sd;
+    IDLParserContext ctxt("root");
+    // This contains only fields part of Reply_type_struct and generic fields
+    auto bsonValidReply = BSON("reply_field" << 42 << genericField << 1);
+    auto parsed = CommandWithReplyType::Reply::parse(ctxt, bsonValidReply);
+    ASSERT(parsed.getIsCommandReply());
+    ASSERT_EQ(parsed.getReply_field(), 42);
+
+    // This contains a field not part of Reply_type struct, so shouldn't parse
+    auto bsonInvalidReply = BSON("reply_field" << 42 << nonGenericField << 1);
+    ASSERT_THROWS(CommandWithReplyType::Reply::parse(ctxt, bsonInvalidReply), DBException);
+}
+
 TEST(IDLCommand, TestCommandTypeNamespaceCommand_WithTenant) {
     IDLParserContext ctxt("root");
 
diff --git a/src/mongo/idl/unittest.idl b/src/mongo/idl/unittest.idl
index 824621910e3..c165f7b9602 100644
--- a/src/mongo/idl/unittest.idl
+++ b/src/mongo/idl/unittest.idl
@@ -179,6 +179,7 @@ structs:
 
     reply_type_struct:
         description: Used for testing the IDL command reply_type field
+        is_command_reply: true
         fields:
             reply_field: int