diff --git a/karapace/protobuf/dependency.py b/karapace/protobuf/dependency.py index c2694ba03..c0b04d2bf 100644 --- a/karapace/protobuf/dependency.py +++ b/karapace/protobuf/dependency.py @@ -11,13 +11,22 @@ from typing import List, Optional, Set +class FieldNotUniquelyIdentifiableException(Exception): + pass + + class ProtobufDependencyVerifier: def __init__(self) -> None: self.declared_types: List[str] = [] self.used_types: List[str] = [] self.import_path: List[str] = [] - def add_declared_type(self, full_name: str) -> None: + def add_declared_type(self, full_name: str, uniquely: bool = False) -> None: + if uniquely and full_name in self.declared_types: + raise FieldNotUniquelyIdentifiableException( + f"{full_name} is not currently identifiable from the parser, " + f"validating this message lead to break the schema evolution!" + ) self.declared_types.append(full_name) def add_used_type(self, parent: str, element_type: str) -> None: diff --git a/karapace/protobuf/schema.py b/karapace/protobuf/schema.py index 2dc3bc591..097129af1 100644 --- a/karapace/protobuf/schema.py +++ b/karapace/protobuf/schema.py @@ -161,9 +161,10 @@ def _process_nested_type( ): verifier.add_declared_type(package_name + "." + parent_name + "." + element_type.name) verifier.add_declared_type(parent_name + "." + element_type.name) - anchestor_only = parent_name.find(".") - if anchestor_only != -1: - verifier.add_declared_type(parent_name[:anchestor_only] + "." + element_type.name) + first_ancestral = parent_name.find(".") + if first_ancestral != -1: + # adding the first father and the type name, this should be unique to identify which is which. + verifier.add_declared_type(parent_name[:first_ancestral] + "." + element_type.name, uniquely=True) if isinstance(element_type, MessageElement): for one_of in element_type.one_ofs: diff --git a/tests/unit/protobuf/test_protobuf_schema.py b/tests/unit/protobuf/test_protobuf_schema.py index 6021b45c3..1956411d4 100644 --- a/tests/unit/protobuf/test_protobuf_schema.py +++ b/tests/unit/protobuf/test_protobuf_schema.py @@ -2,7 +2,9 @@ Copyright (c) 2023 Aiven Ltd See LICENSE for details """ +from _pytest.python_api import raises from karapace.protobuf.compare_result import CompareResult +from karapace.protobuf.dependency import FieldNotUniquelyIdentifiableException from karapace.protobuf.kotlin_wrapper import trim_margin from karapace.protobuf.location import Location from karapace.protobuf.schema import ProtobufSchema @@ -376,3 +378,35 @@ def test_protobuf_self_referencing_schema(): """ assert isinstance(ValidatedTypedSchema.parse(SchemaType.PROTOBUF, proto4).schema, ProtobufSchema) + + +def test_illegal_redefine_objects_in_same_scope(): + proto1 = """\ + 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 { + enum BamFancyEnum { + // Hei! This is a comment! + MY_AWESOME_FIELD = 0; + } + message AnotherLevelOfNesting { + BamFancyEnum im_tricky_im_referring_to_the_previous_enum = 1; + } + } + } + } + """ + with raises(FieldNotUniquelyIdentifiableException) as e: + assert isinstance(ValidatedTypedSchema.parse(SchemaType.PROTOBUF, proto1).schema, ProtobufSchema) + + assert ( + e.value.args[0] == "AnotherMessage.BamFancyEnum is not currently identifiable from the parser, " + "validating this message lead to break the schema evolution!" + )