diff --git a/karapace/protobuf/dependency.py b/karapace/protobuf/dependency.py index af5121488..c2694ba03 100644 --- a/karapace/protobuf/dependency.py +++ b/karapace/protobuf/dependency.py @@ -8,7 +8,7 @@ from karapace.dependency import DependencyVerifierResult from karapace.protobuf.known_dependency import DependenciesHardcoded, KnownDependency from karapace.protobuf.one_of_element import OneOfElement -from typing import List +from typing import List, Optional, Set class ProtobufDependencyVerifier: @@ -35,13 +35,31 @@ def add_used_type(self, parent: str, element_type: str) -> None: def add_import(self, import_name: str) -> None: self.import_path.append(import_name) + def is_type_declared( + self, + used_type: str, + declared_index: Set[str], + father_child_type: Optional[str], + used_type_with_scope: Optional[str], + ) -> bool: + return ( + used_type in declared_index + or (used_type_with_scope is not None and used_type_with_scope in declared_index) + or (father_child_type is not None and father_child_type in declared_index) + or "." + used_type in declared_index + ) + def verify(self) -> DependencyVerifierResult: declared_index = set(self.declared_types) for used_type in self.used_types: delimiter = used_type.rfind(";") - used_type_with_scope = "" + father_child_type = None + used_type_with_scope = None if delimiter != -1: used_type_with_scope = used_type[:delimiter] + "." + used_type[delimiter + 1 :] + father_delimiter = used_type[:delimiter].find(".") + if father_delimiter != -1: + father_child_type = used_type[:father_delimiter] + "." + used_type[delimiter + 1 :] used_type = used_type[delimiter + 1 :] if used_type in DependenciesHardcoded.index: @@ -51,11 +69,7 @@ def verify(self) -> DependencyVerifierResult: if known_pkg is not None and known_pkg in self.import_path: continue - if ( - used_type in declared_index - or (delimiter != -1 and used_type_with_scope in declared_index) - or "." + used_type in declared_index - ): + if self.is_type_declared(used_type, declared_index, father_child_type, used_type_with_scope): continue return DependencyVerifierResult(False, f'type "{used_type}" is not defined') diff --git a/karapace/protobuf/schema.py b/karapace/protobuf/schema.py index a0d91b1cb..3829f9056 100644 --- a/karapace/protobuf/schema.py +++ b/karapace/protobuf/schema.py @@ -161,6 +161,9 @@ def _process_nested_type( ): verifier.add_declared_type(package_name + "." + parent_name + "." + element_type.name) verifier.add_declared_type(parent_name + "." + element_type.name) + ancestor_only = parent_name.find(".") + if ancestor_only != -1: + verifier.add_declared_type(parent_name[:ancestor_only] + "." + element_type.name) if isinstance(element_type, MessageElement): for one_of in element_type.one_ofs: @@ -169,7 +172,17 @@ def _process_nested_type( one_of_parent_name = parent_name + "." + element_type.name process_one_of(verifier, package_name, one_of_parent_name, one_of) for field in element_type.fields: - verifier.add_used_type(parent_name, field.element_type) + # since we declare the subtype in the same level of the scope, it's legit + # use the same scoping when declare the dependent type. + if field.element_type in [defined_in_same_scope.name for defined_in_same_scope in element_type.nested_types]: + verifier.add_used_type(parent_name + "." + element_type.name, field.element_type) + else: + ancestor_only = parent_name.find(".") + if ancestor_only != -1: + verifier.add_used_type(parent_name[:ancestor_only], field.element_type) + else: + verifier.add_used_type(parent_name, field.element_type) + for nested_type in element_type.nested_types: self._process_nested_type(verifier, package_name, parent_name + "." + element_type.name, nested_type) diff --git a/tests/unit/protobuf/test_protobuf_schema.py b/tests/unit/protobuf/test_protobuf_schema.py index 774ea3279..6021b45c3 100644 --- a/tests/unit/protobuf/test_protobuf_schema.py +++ b/tests/unit/protobuf/test_protobuf_schema.py @@ -293,3 +293,86 @@ def test_protobuf_field_compatible_alter_to_oneof(): protobuf_schema1.compare(protobuf_schema2, result) assert result.is_compatible() + + +def test_protobuf_self_referencing_schema(): + proto1 = """\ + syntax = "proto3"; + + package fancy.company.in.party.v1; + message MyFirstMessage { + string my_fancy_string = 1; + } + message AnotherMessage { + message WowANestedMessage { + enum BamFancyEnum { + // Hei! This is a comment! + MY_AWESOME_FIELD = 0; + } + BamFancyEnum im_tricky_im_referring_to_the_previous_enum = 1; + } + } + """ + + assert isinstance(ValidatedTypedSchema.parse(SchemaType.PROTOBUF, proto1).schema, ProtobufSchema) + + proto2 = """\ + syntax = "proto3"; + + package fancy.company.in.party.v1; + message AnotherMessage { + enum BamFancyEnum { + // Hei! This is a comment! + MY_AWESOME_FIELD = 0; + } + message WowANestedMessage { + message DeeplyNestedMsg { + BamFancyEnum im_tricky_im_referring_to_the_previous_enum = 1; + } + } + } + """ + + assert isinstance(ValidatedTypedSchema.parse(SchemaType.PROTOBUF, proto2).schema, ProtobufSchema) + + proto3 = """\ + syntax = "proto3"; + + package fancy.company.in.party.v1; + message AnotherMessage { + enum BamFancyEnum { + // Hei! This is a comment! + MY_AWESOME_FIELD = 0; + } + message WowANestedMessage { + message DeeplyNestedMsg { + message AnotherLevelOfNesting { + BamFancyEnum im_tricky_im_referring_to_the_previous_enum = 1; + } + } + } + } + """ + + assert isinstance(ValidatedTypedSchema.parse(SchemaType.PROTOBUF, proto3).schema, ProtobufSchema) + + proto4 = """\ + syntax = "proto3"; + + package fancy.company.in.party.v1; + message AnotherMessage { + message WowANestedMessage { + enum BamFancyEnum { + // Hei! This is a comment! + MY_AWESOME_FIELD = 0; + } + message DeeplyNestedMsg { + message AnotherLevelOfNesting { + BamFancyEnum im_tricky_im_referring_to_the_previous_enum = 1; + } + } + } + } + """ + + assert isinstance(ValidatedTypedSchema.parse(SchemaType.PROTOBUF, proto4).schema, ProtobufSchema)