diff options
-rw-r--r-- | buildscripts/idl/idl/binder.py | 18 | ||||
-rw-r--r-- | buildscripts/idl/idl/errors.py | 8 | ||||
-rw-r--r-- | buildscripts/idl/idl/generator.py | 172 | ||||
-rw-r--r-- | buildscripts/idl/idl/parser.py | 17 | ||||
-rw-r--r-- | buildscripts/idl/idl/syntax.py | 34 | ||||
-rw-r--r-- | buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl | 42 | ||||
-rw-r--r-- | buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl | 42 | ||||
-rw-r--r-- | buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl | 40 | ||||
-rw-r--r-- | buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl | 38 | ||||
-rw-r--r-- | buildscripts/idl/tests/test_compatibility.py | 18 | ||||
-rw-r--r-- | src/mongo/idl/idl_test.cpp | 59 | ||||
-rw-r--r-- | src/mongo/idl/unittest.idl | 17 |
12 files changed, 437 insertions, 68 deletions
diff --git a/buildscripts/idl/idl/binder.py b/buildscripts/idl/idl/binder.py index 713f339fb89..8213dbb262b 100644 --- a/buildscripts/idl/idl/binder.py +++ b/buildscripts/idl/idl/binder.py @@ -424,10 +424,14 @@ def _bind_struct_field(ctxt, ast_field, idl_type): def _bind_variant_field(ctxt, ast_field, idl_type): - # type: (errors.ParserContext, ast.Field, syntax.VariantType) -> None + # type: (errors.ParserContext, ast.Field, Union[syntax.VariantType, syntax.ArrayType]) -> None ast_field.type = _bind_type(idl_type) ast_field.type.is_variant = True + if isinstance(idl_type, syntax.ArrayType): + assert isinstance(idl_type.element_type, syntax.VariantType) + idl_type = idl_type.element_type + _validate_bson_types_list(ctxt, idl_type, "field") for alternative in idl_type.variant_types: @@ -493,7 +497,6 @@ def _bind_command_type(ctxt, parsed_spec, command): if isinstance(base_type, syntax.Struct): _bind_struct_field(ctxt, ast_field, syntax_symbol) elif isinstance(base_type, syntax.VariantType): - # Arrays of variants aren't supported for now. assert isinstance(syntax_symbol, syntax.VariantType) _bind_variant_field(ctxt, ast_field, cast(syntax.VariantType, syntax_symbol)) else: @@ -816,8 +819,9 @@ def _validate_doc_sequence_field(ctxt, ast_field): assert ast_field.type.is_array # The only allowed BSON type for a doc_sequence field is "object" - if ast_field.type.bson_serialization_type != ['object']: - ctxt.add_bad_non_object_as_doc_sequence_error(ast_field, ast_field.name) + for serialization_type in ast_field.type.bson_serialization_type: + if serialization_type != 'object': + ctxt.add_bad_non_object_as_doc_sequence_error(ast_field, ast_field.name) def _normalize_method_name(cpp_type_name, cpp_method_name): @@ -1038,9 +1042,9 @@ def _bind_field(ctxt, parsed_spec, field): ast_field.type.serializer = enum_type_info.get_enum_serializer_name() ast_field.type.deserializer = enum_type_info.get_enum_deserializer_name() elif isinstance(base_type, syntax.VariantType): - # Arrays of variants aren't supported for now. - assert isinstance(syntax_symbol, syntax.VariantType) - _bind_variant_field(ctxt, ast_field, cast(syntax.VariantType, syntax_symbol)) + # syntax_symbol is an Array for arrays of variant. + assert isinstance(syntax_symbol, (syntax.ArrayType, syntax.VariantType)) + _bind_variant_field(ctxt, ast_field, syntax_symbol) else: assert isinstance(base_type, syntax.Type) diff --git a/buildscripts/idl/idl/errors.py b/buildscripts/idl/idl/errors.py index c15b795b012..3098fdee09b 100644 --- a/buildscripts/idl/idl/errors.py +++ b/buildscripts/idl/idl/errors.py @@ -129,6 +129,7 @@ ERROR_ID_EMPTY_ACCESS_CHECK = "ID0089" ERROR_ID_MISSING_ACCESS_CHECK = "ID0090" ERROR_ID_STABILITY_UNKNOWN_VALUE = "ID0091" ERROR_ID_DUPLICATE_UNSTABLE_STABILITY = "ID0092" +ERROR_ID_INVALID_ARRAY_VARIANT = "ID0093" class IDLError(Exception): @@ -702,6 +703,13 @@ class ParserContext(object): "Field '%s' cannot be a variant with an enum alternative type '%s'" % (field_name, type_name)) + def add_bad_array_variant_types_error(self, location, type_name): + # type: (common.SourceLocation, str) -> None + """Add an error about a field type having a malformed type name.""" + self._add_error(location, ERROR_ID_INVALID_ARRAY_VARIANT, + ("'%s' is not a valid array variant type. A valid array variant type" + + " is in the form 'array<variant<type_name1, ...>>'.") % (type_name)) + def is_scalar_non_negative_int_node(self, node, node_name): # type: (Union[yaml.nodes.MappingNode, yaml.nodes.ScalarNode, yaml.nodes.SequenceNode], str) -> bool """Return True if this YAML node is a Scalar and a valid non-negative int.""" diff --git a/buildscripts/idl/idl/generator.py b/buildscripts/idl/idl/generator.py index de555cf12f9..782ee60f827 100644 --- a/buildscripts/idl/idl/generator.py +++ b/buildscripts/idl/idl/generator.py @@ -1311,8 +1311,19 @@ class _CppSourceFileWriter(_CppFileWriterBase): self._writer.write_empty_line() with self._predicate(_get_bson_type_check('arrayElement', 'arrayCtxt', ast_type)): - array_value = self._gen_field_deserializer_expression( - 'arrayElement', field, ast_type, tenant) + if ast_type.is_variant: + # _gen_variant_deserializer generates code to parse the variant into the variable "_" + field.cpp_name, + # so we create a local variable '_tmp' + # and change cpp_name (for the duration of the _gen_variant_deserializer call) to 'tmp' so we can pass '_tmp' + # to values.emplace_back below. + self._writer.write_line('%s _tmp;' % ast_type.cpp_type) + cpp_name = field.cpp_name + field.cpp_name = 'tmp' + array_value = self._gen_variant_deserializer(field, 'arrayElement', tenant) + field.cpp_name = cpp_name + else: + array_value = self._gen_field_deserializer_expression( + 'arrayElement', field, ast_type, tenant) self._writer.write_line('values.emplace_back(%s);' % (array_value)) with self._block('else {', '}'): @@ -1335,7 +1346,7 @@ class _CppSourceFileWriter(_CppFileWriterBase): self._writer.write_line('%s = std::move(values);' % (_get_field_member_name(field))) def _gen_variant_deserializer(self, field, bson_element, tenant): - # type: (ast.Field, str, str) -> None + # type: (ast.Field, str, str) -> str """Generate the C++ deserializer piece for a variant field.""" self._writer.write_empty_line() self._writer.write_line('const BSONType variantType = %s.type();' % (bson_element, )) @@ -1397,34 +1408,10 @@ class _CppSourceFileWriter(_CppFileWriterBase): if is_multiple_structs: self._writer.write_line('{') self._writer.indent() - self._writer.write_line('auto firstElement = element.Obj().firstElement();') - beginning_str = '' - - for variant_type in field.type.variant_struct_types: - if is_multiple_structs: - struct_type = variant_type.first_element_field_name - self._writer.write_line('%sif (firstElement.fieldNameStringData() == "%s") {' % - (beginning_str, struct_type)) - beginning_str = '} else ' - self._writer.indent() - object_value = '%s::parse(ctxt, %s.Obj())' % (variant_type.cpp_type, bson_element) - - if field.chained_struct_field: - self._writer.write_line( - '%s.%s(%s);' % (_get_field_member_name(field.chained_struct_field), - _get_field_member_setter_name(field), object_value)) - else: - self._writer.write_line( - '%s = %s;' % (_get_field_member_name(field), object_value)) - if is_multiple_structs: - self._writer.unindent() - if is_multiple_structs: - self._writer.write_line('} else {') - self._writer.indent() self._writer.write_line( - 'ctxt.throwUnknownField(firstElement.fieldNameStringData());') - self._writer.unindent() - self._writer.write_line('}') + 'auto firstElement = %s.Obj().firstElement();' % bson_element) + self._gen_variant_deserializer_helper(field, field_name=_get_field_member_name(field), + bson_element='%s.Obj()' % bson_element) self._writer.write_line('break;') if is_multiple_structs: self._writer.unindent() @@ -1443,6 +1430,37 @@ class _CppSourceFileWriter(_CppFileWriterBase): # End of outer switch statement. self._writer.write_line('}') + # Used by _gen_array_deserializer for arrays of variant. + return _get_field_member_name(field) + + def _gen_variant_deserializer_helper(self, field, field_name, bson_element): + beginning_str = '' + is_multiple_structs = len(field.type.variant_struct_types) > 1 + + for variant_type in field.type.variant_struct_types: + if is_multiple_structs: + struct_type = variant_type.first_element_field_name + self._writer.write_line('%sif (firstElement.fieldNameStringData() == "%s") {' % + (beginning_str, struct_type)) + beginning_str = '} else ' + self._writer.indent() + object_value = '%s::parse(ctxt, %s)' % (variant_type.cpp_type, bson_element) + + if field.chained_struct_field: + self._writer.write_line( + '%s.%s(%s);' % (_get_field_member_name(field.chained_struct_field), + _get_field_member_setter_name(field), object_value)) + else: + self._writer.write_line('%s = %s;' % (field_name, object_value)) + if is_multiple_structs: + self._writer.unindent() + if is_multiple_structs: + self._writer.write_line('} else {') + self._writer.indent() + self._writer.write_line('ctxt.throwUnknownField(firstElement.fieldNameStringData());') + self._writer.unindent() + self._writer.write_line('}') + def _gen_usage_check(self, field, bson_element, field_usage_check): # type: (ast.Field, str, _FieldUsageCheckerBase) -> None """Generate the field usage check and insert the required field check.""" @@ -1552,8 +1570,15 @@ class _CppSourceFileWriter(_CppFileWriterBase): self._writer.write_line('IDLParserContext tempContext(%s, &ctxt, %s);' % (_get_field_constant_name(field), tenant)) array_value = '%s::parse(tempContext, sequenceObject)' % (field.type.cpp_type, ) + elif field.type.is_variant: + self._writer.write_line('%s _tmp;' % field.type.cpp_type) + self._writer.write_line('auto firstElement = sequenceObject.firstElement();') + self._gen_variant_deserializer_helper(field, field_name='_tmp', + bson_element='sequenceObject') + array_value = '_tmp' else: - assert field.type.bson_serialization_type == ['object'] + for serialization_type in field.type.bson_serialization_type: + assert serialization_type == 'object' if field.type.deserializer: array_value = '%s(sequenceObject)' % (field.type.deserializer) else: @@ -2072,6 +2097,20 @@ class _CppSourceFileWriter(_CppFileWriterBase): 'BSONObjBuilder subObjBuilder(builder->subobjStart(${field_name}));') self._writer.write_template('${access_member}.serialize(&subObjBuilder);') + def _gen_serializer_method_array_variant(self, field): + template_params = { + 'field_name': _get_field_constant_name(field), + 'access_member': 'item', + } + + with self._with_template(template_params): + self._writer.write_template( + 'BSONArrayBuilder arrayBuilder(builder->subarrayStart(${field_name}));') + with self._block('for (const auto& item : %s) {' % _access_member(field), '}'): + self._writer.write_line('BSONObjBuilder subObjBuilder(arrayBuilder.subobjStart());') + self._gen_serializer_method_variant_helper(field, template_params, + builder='&subObjBuilder') + def _gen_serializer_method_variant(self, field): # type: (ast.Field) -> None """Generate the serialize method definition for a variant type.""" @@ -2081,26 +2120,32 @@ class _CppSourceFileWriter(_CppFileWriterBase): } with self._with_template(template_params): - with self._block('stdx::visit(OverloadedVisitor{', '}, ${access_member});'): - for variant_type in itertools.chain( - field.type.variant_types, - field.type.variant_struct_types if field.type.variant_struct_types else []): - - template_params[ - 'cpp_type'] = 'std::vector<' + variant_type.cpp_type + '>' if variant_type.is_array else variant_type.cpp_type - - with self._block('[builder](const ${cpp_type}& value) {', '},'): - bson_cpp_type = cpp_types.get_bson_cpp_type(variant_type) - if bson_cpp_type and bson_cpp_type.has_serializer(): - assert not field.type.is_array - expression = bson_cpp_type.gen_serializer_expression( - self._writer, 'value') - template_params['expression'] = expression - self._writer.write_template( - 'builder->append(${field_name}, ${expression});') - else: - self._writer.write_template( - 'idl::idlSerialize(builder, ${field_name}, value);') + self._gen_serializer_method_variant_helper(field, template_params) + + def _gen_serializer_method_variant_helper(self, field, template_params, builder='builder'): + # type: (ast.Field, Dict[str, str], str) -> None + + with self._block('stdx::visit(OverloadedVisitor{', '}, ${access_member});'): + for variant_type in itertools.chain( + field.type.variant_types, + field.type.variant_struct_types if field.type.variant_struct_types else []): + + template_params[ + 'cpp_type'] = 'std::vector<' + variant_type.cpp_type + '>' if variant_type.is_array else variant_type.cpp_type + + with self._block('[%s](const ${cpp_type}& value) {' % builder, '},'): + bson_cpp_type = cpp_types.get_bson_cpp_type(variant_type) + if field.type.is_variant and field.type.is_array: + self._writer.write_template('value.serialize(%s);' % builder) + elif bson_cpp_type and bson_cpp_type.has_serializer(): + assert not field.type.is_array + expression = bson_cpp_type.gen_serializer_expression(self._writer, 'value') + template_params['expression'] = expression + self._writer.write_template( + 'builder->append(${field_name}, ${expression});') + else: + self._writer.write_template( + 'idl::idlSerialize(builder, ${field_name}, value);') def _gen_serializer_method_common(self, field): # type: (ast.Field) -> None @@ -2121,12 +2166,15 @@ class _CppSourceFileWriter(_CppFileWriterBase): optional_block_start = '{' with self._block(optional_block_start, '}'): - if not field.type.is_struct: if needs_custom_serializer: self._gen_serializer_method_custom(field) elif field.type.is_variant: - self._gen_serializer_method_variant(field) + if field.type.is_array: + # For array of variants, is_variant == is_array == True. + self._gen_serializer_method_array_variant(field) + else: + self._gen_serializer_method_variant(field) else: # Generate default serialization using BSONObjBuilder::append # Note: BSONObjBuilder::append has overrides for std::vector also @@ -2240,9 +2288,23 @@ class _CppSourceFileWriter(_CppFileWriterBase): 'documentSequence.name = %s.toString();' % (_get_field_constant_name(field))) with self._block('for (const auto& item : %s) {' % (_access_member(field)), '}'): - if not field.type.is_struct: - if field.type.serializer: + if field.type.is_variant: + # _gen_serializer_method_variant expects builder to be a pointer. + self._writer.write_line('BSONObjBuilder objBuilder;') + self._writer.write_line('BSONObjBuilder* builder = &objBuilder;') + + template_params = { + 'field_name': _get_field_constant_name(field), + 'access_member': 'item', + } + + with self._with_template(template_params): + self._gen_serializer_method_variant_helper(field, template_params) + + self._writer.write_line( + 'documentSequence.objs.push_back(builder->obj());') + elif field.type.serializer: self._writer.write_line('documentSequence.objs.push_back(item.%s());' % (writer.get_method_name(field.type.serializer))) else: diff --git a/buildscripts/idl/idl/parser.py b/buildscripts/idl/idl/parser.py index c394369563d..712b2154a1b 100644 --- a/buildscripts/idl/idl/parser.py +++ b/buildscripts/idl/idl/parser.py @@ -331,6 +331,23 @@ def _parse_field_type(ctxt, node): return variant else: assert node.id == "scalar" + if node.value.startswith('array<variant<'): + variant_types = syntax.parse_array_variant_types(node.value) + variant = syntax.FieldTypeVariant(ctxt.file_name, node.start_mark.line, + node.start_mark.column) + if variant_types is None: + location = common.SourceLocation(ctxt.file_name, node.start_mark.line, + node.start_mark.column) + ctxt.add_bad_array_variant_types_error(location, node.value) + else: + for variant_type in variant_types: + single = syntax.FieldTypeSingle(ctxt.file_name, node.start_mark.line, + node.start_mark.column) + single.type_name = variant_type + variant.variant.append(single) + + return syntax.FieldTypeArray(variant) + single = syntax.FieldTypeSingle(ctxt.file_name, node.start_mark.line, node.start_mark.column) diff --git a/buildscripts/idl/idl/syntax.py b/buildscripts/idl/idl/syntax.py index 2735a38ba42..a46188d6be8 100644 --- a/buildscripts/idl/idl/syntax.py +++ b/buildscripts/idl/idl/syntax.py @@ -70,6 +70,26 @@ class IDLSpec(object): self.feature_flags = [] # type: List[FeatureFlag] +def parse_array_variant_types(name): + # type: (str) -> List[str] + """Parse a type name of the form 'array<variant<type1, type2, ...>>' and extract types.""" + if not name.startswith("array<variant<") and not name.endswith(">>"): + return None + + name = name[len("array<variant<"):] + name = name[:-2] + + variant_types = [] + for variant_type in name.split(','): + variant_type = variant_type.strip() + # Ban array<variant<..., array<...>, ...>> types. + if variant_type.startswith("array<") and variant_type.endswith(">"): + return None + variant_types.append(variant_type) + + return variant_types + + def parse_array_type(name): # type: (str) -> str """Parse a type name of the form 'array<type>' and extract type.""" @@ -276,7 +296,8 @@ class SymbolTable(object): element_type = self.resolve_field_type(ctxt, location, field_name, field_type.element_type) if not element_type: - ctxt.add_unknown_type_error(location, field_name, field_type.element_type.type_name) + ctxt.add_unknown_type_error(location, field_name, + field_type.element_type.debug_string()) return None if isinstance(element_type, Enum): @@ -385,8 +406,11 @@ class ArrayType(Type): self.name = f'array<{element_type.name}>' self.element_type = element_type if isinstance(element_type, Type): - assert element_type.cpp_type - self.cpp_type = f'std::vector<{element_type.cpp_type}>' + if element_type.cpp_type: + self.cpp_type = f'std::vector<{element_type.cpp_type}>' + else: + assert isinstance(element_type, VariantType) + # cpp_type can't be set here for array of variants as element_type.cpp_type is not set yet. class VariantType(Type): @@ -748,9 +772,9 @@ class FieldTypeArray(FieldType): """An array field's type, before it is resolved to a Type instance.""" def __init__(self, element_type): - # type: (FieldTypeSingle) -> None + # type: (Union[FieldTypeSingle, FieldTypeVariant]) -> None """Construct a FieldTypeArray.""" - self.element_type = element_type # type: FieldTypeSingle + self.element_type = element_type # type: Union[FieldTypeSingle, FieldTypeVariant] super(FieldTypeArray, self).__init__(element_type.file_name, element_type.line, element_type.column) diff --git a/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl b/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl index 92f2241afc6..7a91fc3ab2d 100644 --- a/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl +++ b/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl @@ -244,6 +244,14 @@ structs: type: int stability: stable + DeleteParameterStruct: + description: "The new command parameter's type and the + old command parameter's type are both structs" + fields: + delete: + type: int + stability: stable + EnumOrStructFieldReply: description: "This reply contains a field that is a non-enum or struct type in the old command but is an enum or struct in the new command" @@ -622,6 +630,16 @@ structs: type: variant: [int, string, InsertParameterStruct, UpdateParameterStruct] + NewArrayVariantStructNotSubsetReply: + description: "This reply contains a field whose new array<variant> types are not a subset + of the old array<variant> types" + is_command_reply: true + fields: + variantStructNotSubsetReplyField: + stability: stable + type: + array<variant<InsertParameterStruct, UpdateParameterStruct, DeleteParameterStruct>> + NewVariantStructNotSubsetReplyWithArray: description: "This reply contains a field whose new variant types are not a subset of the old variant types" @@ -2190,6 +2208,16 @@ commands: api_version: "1" reply_type: NewVariantStructNotSubsetReplyTwo + newReplyFieldArrayVariantStructNotSubset: + description: "new command fails because its reply field type is a array<variant> type that is not + a subset of the old reply field array<variant> types" + command_name: newReplyFieldArrayVariantStructNotSubset + namespace: ignored + cpp_name: newReplyFieldArrayVariantStructNotSubset + strict: true + api_version: "1" + reply_type: NewArrayVariantStructNotSubsetReply + newReplyFieldVariantStructNotSubsetWithArray: description: "new command fails because its reply field type is a variant type that is not a subset of the old reply field variant types" @@ -2266,6 +2294,20 @@ commands: type: variant: [int, bool, InsertParameterStruct] + newParamArrayVariantNotSuperset: + description: "new command fails because its parameter type is a array<variant> type that is not + a superset of the old parameter array<variant> types" + command_name: newParamArrayVariantNotSuperset + namespace: ignored + cpp_name: newParamArrayVariantNotSuperset + strict: true + api_version: "1" + reply_type: OkReply + fields: + arrayVariantNotSupersetParam: + stability: stable + type: array<variant<InsertParameterStruct, UpdateParameterStruct>> + newParamVariantNotSupersetWithArray: description: "new command fails because its parameter type is a variant type that is not a superset of the old parameter variant types" diff --git a/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl b/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl index 9632b19fbcc..5afe3366985 100644 --- a/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl +++ b/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl @@ -246,6 +246,14 @@ structs: type: int stability: stable + DeleteParameterStruct: + description: "The new command parameter's type and the + old command parameter's type are both structs" + fields: + delete: + type: int + stability: stable + EnumOrStructFieldReply: description: "This reply contains a field that is a non-enum or struct type in the old command but is an enum or struct in the new command" @@ -620,6 +628,16 @@ structs: type: variant: [int, string, InsertParameterStruct] + NewArrayVariantStructNotSubsetReply: + description: "This reply contains a field whose new array<variant> types are not a subset + of the old array<variant> types" + is_command_reply: true + fields: + variantStructNotSubsetReplyField: + stability: stable + type: + array<variant<InsertParameterStruct, UpdateParameterStruct>> + NewVariantStructNotSubsetReplyWithArray: description: "This reply contains a field whose new variant types are not a subset of the old variant types" @@ -2217,6 +2235,16 @@ commands: api_version: "1" reply_type: NewVariantStructNotSubsetReplyTwo + newReplyFieldArrayVariantStructNotSubset: + description: "new command fails because its reply field type is a array<variant> type that is not + a subset of the old reply field array<variant> types" + command_name: newReplyFieldArrayVariantStructNotSubset + namespace: ignored + cpp_name: newReplyFieldArrayVariantStructNotSubset + strict: true + api_version: "1" + reply_type: NewArrayVariantStructNotSubsetReply + newReplyFieldVariantStructNotSubsetWithArray: description: "new command fails because its reply field type is a variant type that is not a subset of the old reply field variant types" @@ -2313,6 +2341,20 @@ commands: type: variant: [int, bool, InsertParameterStruct, UpdateParameterStruct] + newParamArrayVariantNotSuperset: + description: "new command fails because its parameter type is a array<variant> type that is not + a superset of the old parameter array<variant> types" + command_name: newParamArrayVariantNotSuperset + namespace: ignored + cpp_name: newParamArrayVariantNotSuperset + strict: true + api_version: "1" + reply_type: OkReply + fields: + arrayVariantNotSupersetParam: + stability: stable + type: array<variant<InsertParameterStruct, UpdateParameterStruct, DeleteParameterStruct>> + newParamVariantNotSupersetWithArray: description: "new command fails because its parameter type is a variant type that is not a superset of the old parameter variant types" diff --git a/buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl b/buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl index 536d407ab71..7b55a99b5b5 100644 --- a/buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl +++ b/buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl @@ -276,6 +276,16 @@ structs: type: variant: [int, string, intStringBoolToIntString] + NewArrayVariantSubsetReply: + description: "This reply contains a field whose new array<variant> types are a subset + of the old array<variant> types" + is_command_reply: true + fields: + variantSubsetReplyFieldThree: + stability: stable + type: + array<variant<int, string, intStringBoolToIntString>> + VariantRecursiveReply: description: "This reply contains a field that has a new variant type that is compatible with the old variant type" @@ -525,6 +535,15 @@ structs: type: variant: [int, string, InsertParameterStruct, UpdateParameterStruct] + ArrayVariantStructSupersetTwoStruct: + description: "This struct contains a field where the new array<variant> types contains two structs + while the old one contains one" + fields: + variantStructSupersetField: + stability: stable + type: + array<variant<int, string, InsertParameterStruct, UpdateParameterStruct>> + VariantStructRecursiveStruct: description: "This struct contains a field where the old variant struct and the new variant struct are compatible" @@ -1491,6 +1510,16 @@ commands: api_version: "1" reply_type: NewVariantSubsetReplyThree + newReplyFieldArrayVariantSubset: + description: "new command when its reply field type is a array<variant> type that is + a subset of the old reply field array<variant> types" + command_name: newReplyFieldArrayVariantSubset + namespace: ignored + cpp_name: newReplyFieldArrayVariantSubset + strict: true + api_version: "1" + reply_type: NewArrayVariantSubsetReply + replyFieldVariantRecursive: description: "new command passes when its reply field type is a variant type that is compatible with the old reply field variant type" @@ -1774,6 +1803,17 @@ commands: api_version: "1" reply_type: OkReply + newCommandTypeArrayVariantStructSuperset: + description: "new command passes because its type is a array<variant> type that is + a superset of the old command type array<variant> types" + command_name: newCommandTypeArrayVariantStructSuperset + namespace: type + type: ArrayVariantStructSupersetTwoStruct + cpp_name: newCommandTypeArrayVariantStructSuperset + strict: true + api_version: "1" + reply_type: OkReply + newCommandTypeVariantStructRecursive: description: "new command passes because its type has a variant struct type that is compatible with the old command type variant diff --git a/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl b/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl index 6a699b9a38c..a87a2b79fc8 100644 --- a/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl +++ b/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl @@ -281,6 +281,16 @@ structs: variant: [int, string, intStringBoolToIntString, StructFieldTypeRecursiveReplyTwo] + NewArrayVariantSubsetReply: + description: "This reply contains a field whose new array<variant> types are a subset + of the old array<variant> types" + is_command_reply: true + fields: + variantSubsetReplyFieldThree: + stability: stable + type: + array<variant<int, string, intStringBoolToIntString, StructFieldTypeRecursiveReplyTwo>> + VariantRecursiveReply: description: "This reply contains a field that has a new variant type that is compatible with the old variant type" @@ -353,6 +363,14 @@ structs: type: int stability: stable + UpdateParameterStruct: + description: "The new command parameter's type and the + old command parameter's type are both structs" + fields: + update: + type: int + stability: stable + CommandParamStructRecursiveOne: description: "This command parameter struct type contains a unstable field while the new struct field is stable and optional" @@ -514,6 +532,15 @@ structs: type: variant: [int, string, InsertParameterStruct] + ArrayVariantStructSupersetTwoStruct: + description: "This struct contains a field where the new array<variant> types contains two structs + while the old one contains one" + fields: + variantStructSupersetField: + stability: stable + type: + array<variant<int, string, InsertParameterStruct>> + VariantStructRecursiveStruct: description: "This struct contains a field where the old variant struct and the new variant struct are compatible" @@ -1723,6 +1750,17 @@ commands: api_version: "1" reply_type: OkReply + newCommandTypeArrayVariantStructSuperset: + description: "new command passes because its type is a array<variant> type that is + a superset of the old command type array<variant> types" + command_name: newCommandTypeArrayVariantStructSuperset + namespace: type + type: ArrayVariantStructSupersetTwoStruct + cpp_name: newCommandTypeArrayVariantStructSuperset + strict: true + api_version: "1" + reply_type: OkReply + newCommandTypeVariantStructRecursive: description: "new command passes because its type has a variant struct type that is compatible with the old command type variant diff --git a/buildscripts/idl/tests/test_compatibility.py b/buildscripts/idl/tests/test_compatibility.py index 40f112675c6..97f08f5075d 100644 --- a/buildscripts/idl/tests/test_compatibility.py +++ b/buildscripts/idl/tests/test_compatibility.py @@ -945,6 +945,14 @@ class TestIDLCompatibilityChecker(unittest.TestCase): str(new_reply_field_variant_struct_not_subset_two_error), "newReplyFieldVariantStructNotSubsetTwo") + new_reply_field_array_variant_struct_not_subset_error = error_collection.get_error_by_command_name( + "newReplyFieldArrayVariantStructNotSubset") + self.assertTrue(new_reply_field_array_variant_struct_not_subset_error.error_id == + idl_compatibility_errors.ERROR_ID_NEW_REPLY_FIELD_VARIANT_TYPE_NOT_SUBSET) + self.assertRegex( + str(new_reply_field_array_variant_struct_not_subset_error), + "newReplyFieldArrayVariantStructNotSubset") + new_reply_field_variant_struct_recursive_error = error_collection.get_error_by_command_name( "replyFieldVariantStructRecursive") self.assertTrue(new_reply_field_variant_struct_recursive_error.error_id == @@ -1053,6 +1061,14 @@ class TestIDLCompatibilityChecker(unittest.TestCase): self.assertRegex( str(new_param_variant_not_superset_three_error), "newParamVariantNotSupersetThree") + new_param_array_variant_not_superset_error = error_collection.get_error_by_command_name( + "newParamArrayVariantNotSuperset") + self.assertTrue( + new_param_array_variant_not_superset_error.error_id == + idl_compatibility_errors.ERROR_ID_NEW_COMMAND_PARAMETER_VARIANT_TYPE_NOT_SUPERSET) + self.assertRegex( + str(new_param_array_variant_not_superset_error), "newParamArrayVariantNotSuperset") + new_param_type_not_variant_error = error_collection.get_error_by_command_name( "newParamTypeNotVariant") self.assertTrue(new_param_type_not_variant_error.error_id == @@ -1493,7 +1509,7 @@ class TestIDLCompatibilityChecker(unittest.TestCase): self.assertRegex( str(new_command_type_field_added_as_stable_error), "newStableTypeFieldAdded") - self.assertEqual(error_collection.count(), 211) + self.assertEqual(error_collection.count(), 213) def test_generic_argument_compatibility_pass(self): """Tests that compatible old and new generic_argument.idl files should pass.""" diff --git a/src/mongo/idl/idl_test.cpp b/src/mongo/idl/idl_test.cpp index 888e30d5dca..456912d33e1 100644 --- a/src/mongo/idl/idl_test.cpp +++ b/src/mongo/idl/idl_test.cpp @@ -1656,6 +1656,26 @@ TEST(IDLArrayTests, TestSimpleArrays) { } } +// Positive: Array of variant +TEST(IDLArrayTests, TestArrayOfVariant) { + IDLParserContext ctxt("root"); + auto testDoc = BSON("value" << BSON_ARRAY(BSON("insert" << 13) << BSON("update" + << "some word"))); + auto parsed = One_array_variant::parse(ctxt, testDoc); + ASSERT_BSONOBJ_EQ(testDoc, parsed.toBSON()); + + ASSERT_EQ(stdx::get<Insert_variant_struct>(parsed.getValue()[0]).getInsert(), 13); + ASSERT_EQ(stdx::get<Update_variant_struct>(parsed.getValue()[1]).getUpdate(), "some word"); + + // Positive: Test we can roundtrip from the just parsed document + { + BSONObjBuilder builder; + parsed.serialize(&builder); + auto loopbackDoc = builder.obj(); + ASSERT_BSONOBJ_EQ(testDoc, loopbackDoc); + } +} + // Positive: Optional Arrays TEST(IDLArrayTests, TestSimpleOptionalArrays) { IDLParserContext ctxt("root"); @@ -3371,6 +3391,45 @@ TEST(IDLDocSequence, TestNonStrict) { } } +TEST(IDLDocSequence, TestArrayVariant) { + IDLParserContext ctxt("root"); + + auto testTempDoc = BSON("DocSequenceCommandArrayVariant" + << "coll1" + << "$db" + << "db" + << "structs" + << BSON_ARRAY(BSON("update" + << "foo") + << BSON("insert" << 12))); + + OpMsgRequest request; + request.body = testTempDoc; + + auto testStruct = DocSequenceCommandArrayVariant::parse(ctxt, request); + ASSERT_EQUALS(2UL, testStruct.getStructs().size()); + ASSERT_EQ(stdx::get<Update_variant_struct>(testStruct.getStructs()[0]).getUpdate(), "foo"); + ASSERT_EQ(stdx::get<Insert_variant_struct>(testStruct.getStructs()[1]).getInsert(), 12); + + assert_same_types<decltype(testStruct.getNamespace()), const NamespaceString&>(); + + // Positive: Test we can round trip to a document sequence from the just parsed document + { + OpMsgRequest loopbackRequest = testStruct.serialize(BSONObj()); + + assertOpMsgEquals(request, loopbackRequest); + ASSERT_EQUALS(loopbackRequest.sequences.size(), 1UL); // just "structs" + ASSERT_EQUALS(loopbackRequest.sequences[0].name, "structs"); + ASSERT_EQUALS(loopbackRequest.sequences[0].objs.size(), 2); + + // Verify doc sequence part of DocSequenceCommandArrayVariant::parseProtected too. + testStruct = DocSequenceCommandArrayVariant::parse(ctxt, loopbackRequest); + ASSERT_EQUALS(2UL, testStruct.getStructs().size()); + ASSERT_EQ(stdx::get<Update_variant_struct>(testStruct.getStructs()[0]).getUpdate(), "foo"); + ASSERT_EQ(stdx::get<Insert_variant_struct>(testStruct.getStructs()[1]).getInsert(), 12); + } +} + // Postive: Test a Command known field does not propagate from passthrough to the final BSON if it // is included as a field in the command. TEST(IDLCommand, TestKnownFieldDuplicate) { diff --git a/src/mongo/idl/unittest.idl b/src/mongo/idl/unittest.idl index 8dfbad45e38..94b630328c0 100644 --- a/src/mongo/idl/unittest.idl +++ b/src/mongo/idl/unittest.idl @@ -770,6 +770,13 @@ structs: variant: [int, string] optional: true + one_array_variant: + description: UnitTest for an array of variant + strict: false + fields: + value: + type: array<variant<update_variant_struct, insert_variant_struct>> + two_variants: description: UnitTest for a struct with two variant fields strict: false @@ -1029,6 +1036,16 @@ commands: supports_doc_sequence: true optional: true + DocSequenceCommandArrayVariant: + description: UnitTest for a basic command with a array<variant> field marked with supports_doc_sequence + command_name: DocSequenceCommandArrayVariant + namespace: concatenate_with_db + api_version: "" + fields: + structs: + type: array<variant<update_variant_struct, insert_variant_struct>> + supports_doc_sequence: true + DocSequenceCommandNonStrict: description: UnitTest for a basic command with fields marked with supports_doc_sequence and non-strict parsing command_name: DocSequenceCommandNonStrict |