mirror
/
mongo
mirror of https://github.com/mongodb/mongo
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SERVER-112856 idl: list any missing required fields on serialization (#43797) 

GitOrigin-RevId: 2fba4f42baaa38a7856e2e38cf0ffea2d0dbda4d
This commit is contained in:
Billy Donahue 2025-11-11 12:57:07 -05:00 committed by MongoDB Bot
parent 856b9e329b
commit 142458d618
4 changed files with 132 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
buildscripts/idl/idl/generator.py
View File

@ -167,8 +167,8 @@ def _get_bson_type_check(bson_element, ctxt_name, ast_type):
)
else:
return (
f'MONGO_likely({ctxt_name}.checkAndAssertTypes({bson_element}, '
f'{_std_array_expr("BSONType", [bson.cpp_bson_type_name(b) for b in bson_types])}))'
f"MONGO_likely({ctxt_name}.checkAndAssertTypes({bson_element}, "
f"{_std_array_expr('BSONType', [bson.cpp_bson_type_name(b) for b in bson_types])}))"
)
@ -229,9 +229,10 @@ def _gen_field_element_name(field):
return "BSONElement_%s" % (common.title_case(field.cpp_name))
def _gen_mark_present(field_name):
# type: (str) -> str
return f"_hasMembers.markPresent(static_cast<size_t>(RequiredFields::{field_name}));"
def _gen_mark_present(field):
# type: (ast.Field) -> str
kname = _get_field_kname(field)
return f"_hasMembers.markPresent(fieldToRequiredFieldPositions[size_t(Field::{kname})]);"
def _is_parse(field):
@ -759,7 +760,7 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
body = f"{_get_field_member_getter_name(field.nested_chained_parent)}().{memfn}(std::move(value));"
else:
body = cpp_type_info.get_setter_body(_get_field_member_name(field), validator)
set_has = _gen_mark_present(field.cpp_name) if is_serial else ""
set_has = _gen_mark_present(field) if is_serial else ""
with self._block(f"void {memfn}({setter_type} value) {{", "}"):
self._writer.write_line(body)
@ -875,16 +876,25 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
with self._block("enum class Field {", "};"):
for f in struct.fields:
self._writer.write_line(f"{_get_field_kname(f)},")
with self._block("static constexpr std::array fieldNames{", "};"):
with self._block(
f"static constexpr std::array<::mongo::idl::FieldMetadata, {len(struct.fields)}> fieldMetadata{{{{",
"}};",
):
required_field_names = [f.cpp_name for f in _get_required_fields(struct)]
for f in struct.fields:
self._writer.write_line(f'"{f.name}"_sd,')
self._writer.write_empty_line()
def gen_required_field_enum(self, struct):
name = f.name
req = "false"
for rf in required_field_names:
if rf == f.cpp_name:
req = "true"
self._writer.write_line(f'{{"{name}"_sd, {req}}},')
self._writer.write_line(
"enum class RequiredFields : size_t { %s };"
% ", ".join([f.cpp_name for f in _get_required_fields(struct)])
"static constexpr std::array fieldNames = mongo::idl::extractNames<fieldMetadata>();"
)
self._writer.write_line(
"static constexpr std::array fieldToRequiredFieldPositions = mongo::idl::extractRequiredFieldPositions<fieldMetadata>();"
)
self._writer.write_empty_line()
def gen_authorization_contract_declaration(self, struct):
# type: (ast.Struct) -> None
@ -1420,7 +1430,6 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
self.write_unindented_line("private:")
self._writer.write_line("struct FieldInfo;")
self.gen_required_field_enum(struct)
self.write_empty_line()
if struct.generate_comparison_operators:
@ -1466,7 +1475,7 @@ class _CppHeaderFileWriter(_CppFileWriterBase):
scp.name, "Default", scp.default, scp.condition, scp.mod_visibility
)
self._writer.write_line(
f"{make_mod_tag(scp.mod_visibility)}constexpr inline auto {_get_constant(scp.name + 'Name')} = \"{scp.name}\"_sd;"
f'{make_mod_tag(scp.mod_visibility)}constexpr inline auto {_get_constant(scp.name + "Name")} = "{scp.name}"_sd;'
)
self._gen_extern_declaration(
scp.cpp_vartype, scp.cpp_varname, scp.condition, scp.mod_visibility
@ -1753,7 +1762,7 @@ class _CppSourceFileWriter(_CppFileWriterBase):
bson.cpp_bson_type_name(t.bson_serialization_type[0]) for t in array_types
]
self._writer.write_line(
f'ctxt.throwBadType({bson_element}, {_std_array_expr("BSONType", expected_types)});'
f"ctxt.throwBadType({bson_element}, {_std_array_expr('BSONType', expected_types)});"
)
self._writer.write_line("break;")
self._writer.unindent()
@ -1796,7 +1805,7 @@ class _CppSourceFileWriter(_CppFileWriterBase):
bson.cpp_bson_type_name(t.bson_serialization_type[0]) for t in scalar_types
]
self._writer.write_line(
f'ctxt.throwBadType({bson_element}, ' f'{_std_array_expr("BSONType", expected_types)});'
f"ctxt.throwBadType({bson_element}, {_std_array_expr('BSONType', expected_types)});"
)
self._writer.write_line("break;")
self._writer.unindent()
@ -1867,7 +1876,7 @@ class _CppSourceFileWriter(_CppFileWriterBase):
field_usage_check.add(field, bson_element)
if _is_required_serializer_field(field):
self._writer.write_line(_gen_mark_present(field.cpp_name))
self._writer.write_line(_gen_mark_present(field))
def gen_field_deserializer(
self,
@ -2156,11 +2165,15 @@ class _CppSourceFileWriter(_CppFileWriterBase):
initializers_str = ": " + ", ".join(initializers)
with self._block("%s %s {" % (constructor.get_definition(), initializers_str), "}"):
db_field = None
for field in _get_required_fields(struct):
if field.name == "$db":
db_field = field
if not (field.name == "$db" and initializes_db_name) and not default_init:
self._writer.write_line(_gen_mark_present(field.cpp_name))
self._writer.write_line(_gen_mark_present(field))
if initializes_db_name:
self._writer.write_line(_gen_mark_present("dbName"))
assert db_field is not None
self._writer.write_line(_gen_mark_present(db_field))
self._writer.write_empty_line()
def gen_constructors(self, struct):
@ -2569,7 +2582,7 @@ class _CppSourceFileWriter(_CppFileWriterBase):
field_usage_check.add(field, "sequence.name")
if _is_required_serializer_field(field):
self._writer.write_line(_gen_mark_present(field.cpp_name))
self._writer.write_line(_gen_mark_present(field))
self.gen_doc_sequence_deserializer(
field, "request.getValidatedTenantId()"
@ -2911,7 +2924,9 @@ class _CppSourceFileWriter(_CppFileWriterBase):
]
if required_fields:
self._writer.write_line("_hasMembers.required();")
self._writer.write_line(
"handleMissingRequiredFields(_hasMembers, fieldMetadata, fieldToRequiredFieldPositions);"
)
self._writer.write_empty_line()
# Serialize the namespace as the first field

8
buildscripts/idl/tests/test_generator.py
View File

@ -177,7 +177,7 @@ class TestGenerator(testcase.IDLTestcase):
""")
expected = dedent("""
void Pokedex::serialize(BSONObjBuilder* builder) const {
_hasMembers.required();
handleMissingRequiredFields(_hasMembers, fieldMetadata, fieldToRequiredFieldPositions);
{
BSONArrayBuilder arrayBuilder(builder->subarrayStart(kKnownPokemonsFieldName));
@ -218,7 +218,7 @@ class TestGenerator(testcase.IDLTestcase):
""")
expected = dedent("""
void Pokedex::serialize(BSONObjBuilder* builder) const {
_hasMembers.required();
handleMissingRequiredFields(_hasMembers, fieldMetadata, fieldToRequiredFieldPositions);
{
BSONArrayBuilder arrayBuilder(builder->subarrayStart(kKnownPokemonsFieldName));
@ -264,7 +264,7 @@ class TestGenerator(testcase.IDLTestcase):
expected = dedent("""
void QueryShapeSpec::serialize(BSONObjBuilder* builder, const SerializationOptions& options) const {
_hasMembers.required();
handleMissingRequiredFields(_hasMembers, fieldMetadata, fieldToRequiredFieldPositions);
{
const BSONObj localObject = _internalObject.toBSON(options);
@ -309,7 +309,7 @@ class TestGenerator(testcase.IDLTestcase):
expected = dedent("""
void QueryShapeSpec::serialize(BSONObjBuilder* builder, const SerializationOptions& options) const {
_hasMembers.required();
handleMissingRequiredFields(_hasMembers, fieldMetadata, fieldToRequiredFieldPositions);
{
BSONArrayBuilder arrayBuilder(builder->subarrayStart(kInternalObjectArrayFieldName));

93
src/mongo/idl/idl_parser.h
View File

@ -64,6 +64,33 @@
namespace mongo {
namespace idl {
struct FieldMetadata {
StringData name;
bool required;
};
template <const auto& fieldMeta>
consteval std::array<StringData, fieldMeta.size()> extractNames() {
std::array<StringData, fieldMeta.size()> out;
for (size_t i = 0; i != fieldMeta.size(); ++i)
out[i] = fieldMeta[i].name;
return out;
}
/**
* Returns an array that maps each field to its position into a sequence containing only the
* required fields. Non-required fields are mapped to -1.
*/
template <const auto& fieldMeta>
consteval auto extractRequiredFieldPositions() {
std::array<size_t, fieldMeta.size()> out;
size_t w = 0;
for (size_t i = 0; i < fieldMeta.size(); ++i)
out[i] = fieldMeta[i].required ? w++ : size_t(-1);
return out;
}
template <typename T>
using HasBSONSerializeOp = decltype(std::declval<T>().serialize(std::declval<BSONObjBuilder*>()));
@ -156,39 +183,69 @@ T preparsedValue(A&&... args) {
return idlPreparsedValue(stdx::type_identity<T>{}, std::forward<A>(args)...);
}
enum DebugEnabled : bool {};
constexpr inline DebugEnabled kIsDebug{kDebugBuild};
/**
* HasMembers tracks the presence of required fields in debug mode, and is a noop class in
* production builds.
*/
template <size_t N, DebugEnabled = kIsDebug>
class HasMembers;
template <size_t N>
class HasMembers<N, DebugEnabled{false}> {
class HasMembers {
public:
void required() const {}
void markPresent(size_t pos) {}
};
static constexpr size_t size() {
return N;
}
template <size_t N>
class HasMembers<N, DebugEnabled{true}> {
public:
void required() const {
invariant(_hasField.all());
bool hasAllRequired() const {
if constexpr (kDebugBuild) {
return _hasField.all();
} else {
return true;
}
}
void markPresent(size_t pos) {
_hasField.set(pos, true);
if constexpr (kDebugBuild) {
_hasField.set(pos, true);
}
}
/**
* In debug builds, a field is ok if it's present.
* In non-debug builds, no field is present, and this shouldn't be called at all.
*/
bool get(size_t pos) const {
if constexpr (kDebugBuild) {
return _hasField[pos];
} else {
MONGO_UNREACHABLE;
}
}
private:
std::bitset<N> _hasField;
struct Empty {};
MONGO_COMPILER_NO_UNIQUE_ADDRESS std::conditional_t<kDebugBuild, std::bitset<N>, Empty>
_hasField;
};
void handleMissingRequiredFields(const auto& hasMembers,
std::span<const FieldMetadata> meta,
std::span<const size_t> fieldToRequiredFieldPositions) {
if constexpr (kDebugBuild) {
if (hasMembers.hasAllRequired())
return;
std::string msg = "Missing required fields: ";
StringData sep;
size_t reqIdx = 0;
for (size_t i = 0; i < meta.size(); ++i) {
auto&& [name, required] = meta[i];
if (required && !hasMembers.get(reqIdx++)) {
msg += std::exchange(sep, ", "_sd);
msg += name;
}
}
invariant(false, msg);
}
}
/** Support routines for IDL-generated comparison operators */
namespace relop {

26
src/mongo/idl/idl_test.cpp
View File

@ -2094,7 +2094,9 @@ void attemptToSerializeIncompleteStruct() {
}
#ifdef MONGO_CONFIG_DEBUG_BUILD
DEATH_TEST(IDLSerializeTests, TestUninitializedRequiredFieldsDiesDebug, "invariant") {
DEATH_TEST(IDLSerializeTests,
TestUninitializedRequiredFieldsDiesDebug,
"Missing required fields: field3") {
// This should invariant because the required field3 is uninitialized.
attemptToSerializeIncompleteStruct();
}
@ -2115,15 +2117,19 @@ TEST(IDLArrayTests, TestSimpleArrays) {
uint8_t array15[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15};
uint8_t array16[] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
auto testDoc = BSON("field1" << BSON_ARRAY("Foo" << "Bar"
<< "???")
<< "field2" << BSON_ARRAY(1 << 2 << 3) << "field3"
<< BSON_ARRAY(1.2 << 3.4 << 5.6) << "field4"
<< BSON_ARRAY(BSONBinData(array1, 3, BinDataGeneral)
<< BSONBinData(array2, 3, BinDataGeneral))
<< "field5"
<< BSON_ARRAY(BSONBinData(array15, 16, newUUID)
<< BSONBinData(array16, 16, newUUID)));
auto testDoc = [&] {
BSONObjBuilder bob;
BSONArrayBuilder{bob.subarrayStart("field1")}.append("Foo").append("Bar").append("???");
BSONArrayBuilder{bob.subarrayStart("field2")}.append(1).append(2).append(3);
BSONArrayBuilder{bob.subarrayStart("field3")}.append(1.2).append(3.4).append(5.6);
BSONArrayBuilder{bob.subarrayStart("field4")}
.append(BSONBinData(array1, 3, BinDataGeneral))
.append(BSONBinData(array2, 3, BinDataGeneral));
BSONArrayBuilder{bob.subarrayStart("field5")}
.append(BSONBinData(array15, 16, newUUID))
.append(BSONBinData(array16, 16, newUUID));
return bob.obj();
}();
auto testStruct = Simple_array_fields::parse(testDoc);
assert_same_types<decltype(testStruct.getField1()), std::vector<StringData>>();