SERVER-67649 Teach IDL reply types to ignore generic fields

11 files changed, 121 insertions, 45 deletions

diff --git a/buildscripts/idl/idl/ast.py b/buildscripts/idl/idl/ast.py
index 63cb4502039..840ee8689da 100644
--- a/buildscripts/idl/idl/ast.py
+++ b/buildscripts/idl/idl/ast.py
@@ -145,6 +145,7 @@ class Struct(common.SourceLocation):
         self.allow_global_collection_name = False  # type: bool
         self.non_const_getter = False  # type: bool
         self.cpp_validator_func = None  # type: str
+        self.is_command_reply = False  # type: bool
         super(Struct, self).__init__(file_name, line, column)
 
 
diff --git a/buildscripts/idl/idl/binder.py b/buildscripts/idl/idl/binder.py
index 56cb2a53636..e50e8322410 100644
--- a/buildscripts/idl/idl/binder.py
+++ b/buildscripts/idl/idl/binder.py
@@ -271,6 +271,7 @@ def _bind_struct_common(ctxt, parsed_spec, struct, ast_struct):
     ast_struct.qualified_cpp_name = _get_struct_qualified_cpp_name(struct)
     ast_struct.allow_global_collection_name = struct.allow_global_collection_name
     ast_struct.non_const_getter = struct.non_const_getter
+    ast_struct.is_command_reply = struct.is_command_reply
 
     # Validate naming restrictions
     if ast_struct.name.startswith("array<"):
diff --git a/buildscripts/idl/idl/generator.py b/buildscripts/idl/idl/generator.py
index f48fd3542d2..b4a996a17a2 100644
--- a/buildscripts/idl/idl/generator.py
+++ b/buildscripts/idl/idl/generator.py
@@ -36,7 +36,7 @@ import re
 import sys
 import textwrap
 from abc import ABCMeta, abstractmethod
-from typing import Dict, Iterable, List, Mapping, Tuple, Union
+from typing import Dict, Iterable, List, Mapping, Tuple, Union, cast
 
 from . import (ast, bson, common, cpp_types, enum_types, generic_field_list_types, struct_types,
                writer)
@@ -589,6 +589,12 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
         set_has = f'{_get_has_field_member_name(field)} = true;' if is_serial else ''
         self._writer.write_line(f'void {memfn}({param_type} value) {{ {body} {set_has} }}')
 
+    def gen_constexpr_getters(self):
+        # type: () -> None
+        """Generate the getters for constexpr data."""
+        self._writer.write_line(
+            'constexpr bool getIsCommandReply() const { return _isCommandReply; }')
+
     def gen_member(self, field):
         # type: (ast.Field) -> None
         """Generate the C++ class member definition for a field."""
@@ -609,6 +615,12 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
         else:
             self._writer.write_line('%s %s;' % (member_type, member_name))
 
+    def gen_constexpr_members(self, struct):
+        # type: (ast.Struct) -> None
+        """Generate the C++ class member definition for constexpr data."""
+        cpp_string_val = "true" if struct.is_command_reply else "false"
+        self._writer.write_line(f'static constexpr bool _isCommandReply{{{cpp_string_val}}};')
+
     def gen_serializer_member(self, field):
         # type: (ast.Field) -> None
         """Generate the C++ class bool has_<field> member definition for a field."""
@@ -1038,10 +1050,9 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
                 self.gen_enum_functions(idl_enum)
                 self._writer.write_empty_line()
 
-            spec_and_structs = spec.structs
-            spec_and_structs += spec.commands
+            all_structs = spec.structs + cast(List[ast.Struct], spec.commands)
 
-            for struct in spec_and_structs:
+            for struct in all_structs:
                 self.gen_description_comment(struct.description)
                 with self.gen_class_declaration_block(struct.cpp_name):
                     self.write_unindented_line('public:')
@@ -1078,6 +1089,11 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
                             self.gen_getter(struct, field)
                             if not struct.immutable and not field.chained_struct_field:
                                 self.gen_setter(field)
+
+                    # Generate getters for any constexpr/compile-time struct data
+                    self.write_empty_line()
+                    self.gen_constexpr_getters()
+
                     self.write_unindented_line('protected:')
                     self.gen_protected_serializer_methods(struct)
 
@@ -1112,6 +1128,9 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
                         if _is_required_serializer_field(field):
                             self.gen_serializer_member(field)
 
+                    # Write constexpr struct data
+                    self.gen_constexpr_members(struct)
+
                 self.write_empty_line()
 
             field_lists_list: Iterable[Iterable[ast.FieldListBase]]
@@ -1657,6 +1676,10 @@ class _CppSourceFileWriter(_CppFileWriterBase):
                 if isinstance(struct, ast.Command):
                     command_predicate = "!mongo::isGenericArgument(fieldName)"
 
+                # Ditto for command replies
+                if struct.is_command_reply:
+                    command_predicate = "!mongo::isGenericReply(fieldName)"
+
                 with self._block('else {', '}'):
                     with self._predicate(command_predicate):
                         self._writer.write_line('ctxt.throwUnknownField(fieldName);')
@@ -2670,7 +2693,9 @@ class _CppSourceFileWriter(_CppFileWriterBase):
                 self.gen_description_comment(idl_enum.description)
                 self.gen_enum_definition(idl_enum)
 
-            for struct in spec.structs:
+            all_structs = spec.structs + cast(List[ast.Struct], spec.commands)
+
+            for struct in all_structs:
                 self.gen_string_constants_definitions(struct)
                 self.write_empty_line()
 
diff --git a/buildscripts/idl/idl/parser.py b/buildscripts/idl/idl/parser.py
index d62dd69bb6e..870f6142f68 100644
--- a/buildscripts/idl/idl/parser.py
+++ b/buildscripts/idl/idl/parser.py
@@ -528,6 +528,7 @@ def _parse_struct(ctxt, spec, name, node):
             "generate_comparison_operators": _RuleDesc("bool_scalar"),
             "non_const_getter": _RuleDesc('bool_scalar'),
             "cpp_validator_func": _RuleDesc('scalar'),
+            "is_command_reply": _RuleDesc('bool_scalar'),
         })
 
     # PyLint has difficulty with some iterables: https://github.com/PyCQA/pylint/issues/3105
diff --git a/buildscripts/idl/idl/syntax.py b/buildscripts/idl/idl/syntax.py
index 79bfdd002be..1e24a445df2 100644
--- a/buildscripts/idl/idl/syntax.py
+++ b/buildscripts/idl/idl/syntax.py
@@ -531,6 +531,7 @@ class Struct(common.SourceLocation):
         self.allow_global_collection_name = False  # type: bool
         self.non_const_getter = False  # type: bool
         self.cpp_validator_func = None  # type: str
+        self.is_command_reply = False  # type: bool
 
         # Command only property
         self.cpp_name = None  # type: str
diff --git a/src/mongo/db/coll_mod.idl b/src/mongo/db/coll_mod.idl
index 10f29489aad..0ed9e210074 100644
--- a/src/mongo/db/coll_mod.idl
+++ b/src/mongo/db/coll_mod.idl
@@ -75,6 +75,7 @@ structs:
     CollModReply:
         description: "The collMod command's reply."
         strict: true
+        is_command_reply: true
         fields:
             expireAfterSeconds_old:
                 optional: true
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