From dde1f844fc97f773aa8a792c1367a39055151285 Mon Sep 17 00:00:00 2001 From: Elia Migliore Date: Wed, 10 Apr 2024 11:57:16 +0200 Subject: [PATCH] Iteration after the reviews --- karapace/protobuf/compare_type_lists.py | 6 +- karapace/protobuf/enum_element.py | 5 +- karapace/protobuf/extend_element.py | 3 +- karapace/protobuf/group_element.py | 3 +- karapace/protobuf/message_element.py | 20 ++- karapace/protobuf/one_of_element.py | 7 +- karapace/protobuf/proto_file_element.py | 18 +- karapace/protobuf/proto_normalizations.py | 164 ++++++++++++------ karapace/protobuf/serialization.py | 4 +- karapace/protobuf/service_element.py | 5 +- karapace/protobuf/type_element.py | 6 +- karapace/schema_models.py | 16 +- karapace/schema_reader.py | 1 + tests/integration/test_schema_protobuf.py | 81 +++------ .../protobuf/test_protobuf_normalization.py | 12 +- 15 files changed, 200 insertions(+), 151 deletions(-) diff --git a/karapace/protobuf/compare_type_lists.py b/karapace/protobuf/compare_type_lists.py index 0dfe26547..d4d181a95 100644 --- a/karapace/protobuf/compare_type_lists.py +++ b/karapace/protobuf/compare_type_lists.py @@ -11,12 +11,12 @@ from karapace.protobuf.exception import IllegalStateException from karapace.protobuf.message_element import MessageElement from karapace.protobuf.type_element import TypeElement -from typing import List +from typing import Sequence def compare_type_lists( - self_types_list: List[TypeElement], - other_types_list: List[TypeElement], + self_types_list: Sequence[TypeElement], + other_types_list: Sequence[TypeElement], result: CompareResult, compare_types: CompareTypes, ) -> CompareResult: diff --git a/karapace/protobuf/enum_element.py b/karapace/protobuf/enum_element.py index 37a49f566..dcee9522c 100644 --- a/karapace/protobuf/enum_element.py +++ b/karapace/protobuf/enum_element.py @@ -14,6 +14,7 @@ from karapace.protobuf.option_element import OptionElement from karapace.protobuf.type_element import TypeElement from karapace.protobuf.utils import append_documentation, append_indented +from typing import Sequence class EnumElement(TypeElement): @@ -22,8 +23,8 @@ def __init__( location: Location, name: str, documentation: str = "", - options: list[OptionElement] | None = None, - constants: list[EnumConstantElement] | None = None, + options: Sequence[OptionElement] | None = None, + constants: Sequence[EnumConstantElement] | None = None, ) -> None: # Enums do not allow nested type declarations. super().__init__(location, name, documentation, options or [], []) diff --git a/karapace/protobuf/extend_element.py b/karapace/protobuf/extend_element.py index 906b4f42e..da8229650 100644 --- a/karapace/protobuf/extend_element.py +++ b/karapace/protobuf/extend_element.py @@ -10,6 +10,7 @@ from karapace.protobuf.field_element import FieldElement from karapace.protobuf.location import Location from karapace.protobuf.utils import append_documentation, append_indented +from typing import Sequence @dataclass @@ -17,7 +18,7 @@ class ExtendElement: location: Location name: str documentation: str = "" - fields: list[FieldElement] | None = None + fields: Sequence[FieldElement] | None = None def to_schema(self) -> str: result: list[str] = [] diff --git a/karapace/protobuf/group_element.py b/karapace/protobuf/group_element.py index 8f0307817..1eeecf31c 100644 --- a/karapace/protobuf/group_element.py +++ b/karapace/protobuf/group_element.py @@ -11,6 +11,7 @@ from karapace.protobuf.field_element import FieldElement from karapace.protobuf.location import Location from karapace.protobuf.utils import append_documentation, append_indented +from typing import Sequence @dataclass @@ -20,7 +21,7 @@ class GroupElement: name: str tag: int documentation: str = "" - fields: list[FieldElement] | None = None + fields: Sequence[FieldElement] | None = None def to_schema(self) -> str: result: list[str] = [] diff --git a/karapace/protobuf/message_element.py b/karapace/protobuf/message_element.py index 0c9dd8d44..57eb03fa6 100644 --- a/karapace/protobuf/message_element.py +++ b/karapace/protobuf/message_element.py @@ -19,21 +19,27 @@ from karapace.protobuf.reserved_element import ReservedElement from karapace.protobuf.type_element import TypeElement from karapace.protobuf.utils import append_documentation, append_indented +from typing import Sequence class MessageElement(TypeElement): + nested_types: Sequence[TypeElement] + fields: Sequence[FieldElement] + one_ofs: Sequence[OneOfElement] + groups: Sequence[GroupElement] + def __init__( self, location: Location, name: str, documentation: str = "", - nested_types: list[TypeElement] | None = None, - options: list[OptionElement] | None = None, - reserveds: list[ReservedElement] | None = None, - fields: list[FieldElement] | None = None, - one_ofs: list[OneOfElement] | None = None, - extensions: list[ExtensionsElement] | None = None, - groups: list[GroupElement] | None = None, + nested_types: Sequence[TypeElement] | None = None, + options: Sequence[OptionElement] | None = None, + reserveds: Sequence[ReservedElement] | None = None, + fields: Sequence[FieldElement] | None = None, + one_ofs: Sequence[OneOfElement] | None = None, + extensions: Sequence[ExtensionsElement] | None = None, + groups: Sequence[GroupElement] | None = None, ) -> None: super().__init__(location, name, documentation, options or [], nested_types or []) self.reserveds = reserveds or [] diff --git a/karapace/protobuf/one_of_element.py b/karapace/protobuf/one_of_element.py index 3d7d1993f..278886e23 100644 --- a/karapace/protobuf/one_of_element.py +++ b/karapace/protobuf/one_of_element.py @@ -14,6 +14,7 @@ from karapace.protobuf.group_element import GroupElement from karapace.protobuf.option_element import OptionElement from karapace.protobuf.utils import append_documentation, append_indented +from typing import Sequence class OneOfElement: @@ -21,9 +22,9 @@ def __init__( self, name: str, documentation: str = "", - fields: list[FieldElement] | None = None, - groups: list[GroupElement] | None = None, - options: list[OptionElement] | None = None, + fields: Sequence[FieldElement] | None = None, + groups: Sequence[GroupElement] | None = None, + options: Sequence[OptionElement] | None = None, ) -> None: self.name = name self.documentation = documentation diff --git a/karapace/protobuf/proto_file_element.py b/karapace/protobuf/proto_file_element.py index b60ff33ab..c9f4be031 100644 --- a/karapace/protobuf/proto_file_element.py +++ b/karapace/protobuf/proto_file_element.py @@ -13,7 +13,7 @@ from karapace.protobuf.service_element import ServiceElement from karapace.protobuf.syntax import Syntax from karapace.protobuf.type_element import TypeElement -from typing import Dict, List, NewType, Optional +from typing import Dict, List, NewType, Optional, Sequence def _collect_dependencies_types(compare_types: CompareTypes, dependencies: Optional[Dict[str, Dependency]], is_self: bool): @@ -37,17 +37,21 @@ def _collect_dependencies_types(compare_types: CompareTypes, dependencies: Optio class ProtoFileElement: + types: Sequence[TypeElement] + services: Sequence[ServiceElement] + extend_declarations: Sequence[ExtendElement] + def __init__( self, location: Location, package_name: Optional[PackageName] = None, syntax: Optional[Syntax] = None, - imports: Optional[List[TypeName]] = None, - public_imports: Optional[List[TypeName]] = None, - types: Optional[List[TypeElement]] = None, - services: Optional[List[ServiceElement]] = None, - extend_declarations: Optional[List[ExtendElement]] = None, - options: Optional[List[OptionElement]] = None, + imports: Optional[Sequence[TypeName]] = None, + public_imports: Optional[Sequence[TypeName]] = None, + types: Optional[Sequence[TypeElement]] = None, + services: Optional[Sequence[ServiceElement]] = None, + extend_declarations: Optional[Sequence[ExtendElement]] = None, + options: Optional[Sequence[OptionElement]] = None, ) -> None: if types is None: types = list() diff --git a/karapace/protobuf/proto_normalizations.py b/karapace/protobuf/proto_normalizations.py index c7f2c824c..9f65a2623 100644 --- a/karapace/protobuf/proto_normalizations.py +++ b/karapace/protobuf/proto_normalizations.py @@ -2,6 +2,10 @@ Copyright (c) 2024 Aiven Ltd See LICENSE for details """ + +from __future__ import annotations + +from karapace.dependency import Dependency from karapace.protobuf.enum_constant_element import EnumConstantElement from karapace.protobuf.enum_element import EnumElement from karapace.protobuf.extend_element import ExtendElement @@ -12,23 +16,86 @@ from karapace.protobuf.option_element import OptionElement from karapace.protobuf.proto_file_element import ProtoFileElement from karapace.protobuf.rpc_element import RpcElement +from karapace.protobuf.schema import ProtobufSchema from karapace.protobuf.service_element import ServiceElement from karapace.protobuf.type_element import TypeElement -from karapace.typing import StrEnum -from typing import List - +from karapace.schema_references import Reference +from typing import Mapping, Sequence -class ProtobufNormalisationOptions(StrEnum): - sort_options = "sort_options" +import abc def sort_by_name(element: OptionElement) -> str: return element.name -def type_field_element_with_sorted_options(type_field: FieldElement) -> FieldElement: +class NormalizedRpcElement(RpcElement): + pass + + +class NormalizedServiceElement(ServiceElement): + rpcs: Sequence[NormalizedRpcElement] | None = None + + +class NormalizedFieldElement(FieldElement): + pass + + +class NormalizedExtendElement(ExtendElement): + fields: Sequence[NormalizedFieldElement] | None = None + + +class NormalizedTypeElement(TypeElement, abc.ABC): + nested_types: Sequence[NormalizedTypeElement] + + +class NormalizedProtoFileElement(ProtoFileElement): + types: Sequence[NormalizedTypeElement] + services: Sequence[NormalizedServiceElement] + extend_declarations: Sequence[NormalizedExtendElement] + + +class NormalizedMessageElement(MessageElement, NormalizedTypeElement): + nested_types: Sequence[NormalizedTypeElement] + fields: Sequence[NormalizedFieldElement] + one_ofs: Sequence[OneOfElement] + groups: Sequence[GroupElement] + + +class NormalizedEnumConstantElement(EnumConstantElement): + pass + + +class NormalizedEnumElement(EnumElement, NormalizedTypeElement): + constants: Sequence[NormalizedEnumConstantElement] + + +class NormalizedGroupElement(GroupElement): + fields: Sequence[NormalizedFieldElement] | None = None + + +class NormalizedProtobufSchema(ProtobufSchema): + proto_file_element: NormalizedProtoFileElement + + def __init__( + self, + schema: str, + references: Sequence[Reference] | None = None, + dependencies: Mapping[str, Dependency] | None = None, + proto_file_element: ProtoFileElement | None = None, + ) -> None: + super().__init__(schema, references, dependencies, proto_file_element) + self.proto_file_element = normalize(self.proto_file_element) + + +class NormalizedOneOfElement(OneOfElement): + fields: Sequence[NormalizedFieldElement] + groups: Sequence[NormalizedGroupElement] + + +def type_field_element_with_sorted_options(type_field: FieldElement) -> NormalizedFieldElement: sorted_options = None if type_field.options is None else list(sorted(type_field.options, key=sort_by_name)) - return FieldElement( + return NormalizedFieldElement( location=type_field.location, label=type_field.label, element_type=type_field.element_type, @@ -41,9 +108,9 @@ def type_field_element_with_sorted_options(type_field: FieldElement) -> FieldEle ) -def enum_constant_element_with_sorted_options(enum_constant: EnumConstantElement) -> EnumConstantElement: +def enum_constant_element_with_sorted_options(enum_constant: EnumConstantElement) -> NormalizedEnumConstantElement: sorted_options = None if enum_constant.options is None else list(sorted(enum_constant.options, key=sort_by_name)) - return EnumConstantElement( + return NormalizedEnumConstantElement( location=enum_constant.location, name=enum_constant.name, tag=enum_constant.tag, @@ -52,14 +119,14 @@ def enum_constant_element_with_sorted_options(enum_constant: EnumConstantElement ) -def enum_element_with_sorted_options(enum_element: EnumElement) -> EnumElement: +def enum_element_with_sorted_options(enum_element: EnumElement) -> NormalizedEnumElement: sorted_options = None if enum_element.options is None else list(sorted(enum_element.options, key=sort_by_name)) constants_with_sorted_options = ( None if enum_element.constants is None else [enum_constant_element_with_sorted_options(constant) for constant in enum_element.constants] ) - return EnumElement( + return NormalizedEnumElement( location=enum_element.location, name=enum_element.name, documentation=enum_element.documentation, @@ -68,11 +135,11 @@ def enum_element_with_sorted_options(enum_element: EnumElement) -> EnumElement: ) -def groups_with_sorted_options(group: GroupElement) -> GroupElement: +def groups_with_sorted_options(group: GroupElement) -> NormalizedGroupElement: sorted_fields = ( None if group.fields is None else [type_field_element_with_sorted_options(field) for field in group.fields] ) - return GroupElement( + return NormalizedGroupElement( label=group.label, location=group.location, name=group.name, @@ -82,12 +149,12 @@ def groups_with_sorted_options(group: GroupElement) -> GroupElement: ) -def one_ofs_with_sorted_options(one_ofs: OneOfElement) -> OneOfElement: +def one_ofs_with_sorted_options(one_ofs: OneOfElement) -> NormalizedOneOfElement: sorted_options = None if one_ofs.options is None else list(sorted(one_ofs.options, key=sort_by_name)) sorted_fields = [type_field_element_with_sorted_options(field) for field in one_ofs.fields] sorted_groups = [groups_with_sorted_options(group) for group in one_ofs.groups] - return OneOfElement( + return NormalizedOneOfElement( name=one_ofs.name, documentation=one_ofs.documentation, fields=sorted_fields, @@ -96,17 +163,17 @@ def one_ofs_with_sorted_options(one_ofs: OneOfElement) -> OneOfElement: ) -def message_element_with_sorted_options(message_element: MessageElement) -> MessageElement: +def message_element_with_sorted_options(message_element: MessageElement) -> NormalizedMessageElement: sorted_options = None if message_element.options is None else list(sorted(message_element.options, key=sort_by_name)) - sorted_neasted_types = [type_element_with_sorted_options(nested_type) for nested_type in message_element.nested_types] + sorted_nested_types = [type_element_with_sorted_options(nested_type) for nested_type in message_element.nested_types] sorted_fields = [type_field_element_with_sorted_options(field) for field in message_element.fields] sorted_one_ofs = [one_ofs_with_sorted_options(one_of) for one_of in message_element.one_ofs] - return MessageElement( + return NormalizedMessageElement( location=message_element.location, name=message_element.name, documentation=message_element.documentation, - nested_types=sorted_neasted_types, + nested_types=sorted_nested_types, options=sorted_options, reserveds=message_element.reserveds, fields=sorted_fields, @@ -116,19 +183,19 @@ def message_element_with_sorted_options(message_element: MessageElement) -> Mess ) -def type_element_with_sorted_options(type_element: TypeElement) -> TypeElement: - sorted_neasted_types: List[TypeElement] = [] +def type_element_with_sorted_options(type_element: TypeElement) -> NormalizedTypeElement: + sorted_nested_types: list[TypeElement] = [] for nested_type in type_element.nested_types: if isinstance(nested_type, EnumElement): - sorted_neasted_types.append(enum_element_with_sorted_options(nested_type)) + sorted_nested_types.append(enum_element_with_sorted_options(nested_type)) elif isinstance(nested_type, MessageElement): - sorted_neasted_types.append(message_element_with_sorted_options(nested_type)) + sorted_nested_types.append(message_element_with_sorted_options(nested_type)) else: raise ValueError("Unknown type element") # tried with assert_never but it did not work # doing it here since the subtypes do not declare the nested_types property - type_element.nested_types = sorted_neasted_types + type_element.nested_types = sorted_nested_types if isinstance(type_element, EnumElement): return enum_element_with_sorted_options(type_element) @@ -139,13 +206,13 @@ def type_element_with_sorted_options(type_element: TypeElement) -> TypeElement: raise ValueError("Unknown type element") # tried with assert_never but it did not work -def extends_element_with_sorted_options(extend_element: ExtendElement) -> ExtendElement: +def extends_element_with_sorted_options(extend_element: ExtendElement) -> NormalizedExtendElement: sorted_fields = ( None if extend_element.fields is None else [type_field_element_with_sorted_options(field) for field in extend_element.fields] ) - return ExtendElement( + return NormalizedExtendElement( location=extend_element.location, name=extend_element.name, documentation=extend_element.documentation, @@ -153,9 +220,9 @@ def extends_element_with_sorted_options(extend_element: ExtendElement) -> Extend ) -def rpc_element_with_sorted_options(rpc: RpcElement) -> RpcElement: +def rpc_element_with_sorted_options(rpc: RpcElement) -> NormalizedRpcElement: sorted_options = None if rpc.options is None else list(sorted(rpc.options, key=sort_by_name)) - return RpcElement( + return NormalizedRpcElement( location=rpc.location, name=rpc.name, documentation=rpc.documentation, @@ -167,13 +234,13 @@ def rpc_element_with_sorted_options(rpc: RpcElement) -> RpcElement: ) -def service_element_with_sorted_options(service_element: ServiceElement) -> ServiceElement: +def service_element_with_sorted_options(service_element: ServiceElement) -> NormalizedServiceElement: sorted_options = None if service_element.options is None else list(sorted(service_element.options, key=sort_by_name)) sorted_rpc = ( None if service_element.rpcs is None else [rpc_element_with_sorted_options(rpc) for rpc in service_element.rpcs] ) - return ServiceElement( + return NormalizedServiceElement( location=service_element.location, name=service_element.name, documentation=service_element.documentation, @@ -182,23 +249,19 @@ def service_element_with_sorted_options(service_element: ServiceElement) -> Serv ) -def normalize_options_ordered(proto_file_element: ProtoFileElement) -> ProtoFileElement: - sorted_types = [type_element_with_sorted_options(type_element) for type_element in proto_file_element.types] - sorted_options = ( - None if proto_file_element.options is None else list(sorted(proto_file_element.options, key=sort_by_name)) - ) - sorted_services = ( - None - if proto_file_element.services is None - else [service_element_with_sorted_options(service) for service in proto_file_element.services] - ) - sorted_extend_declarations = ( - None - if proto_file_element.extend_declarations is None - else [extends_element_with_sorted_options(extend) for extend in proto_file_element.extend_declarations] - ) +def normalize(proto_file_element: ProtoFileElement) -> NormalizedProtoFileElement: + sorted_types: Sequence[NormalizedTypeElement] = [ + type_element_with_sorted_options(type_element) for type_element in proto_file_element.types + ] + sorted_options = list(sorted(proto_file_element.options, key=sort_by_name)) + sorted_services: Sequence[NormalizedServiceElement] = [ + service_element_with_sorted_options(service) for service in proto_file_element.services + ] + sorted_extend_declarations: Sequence[NormalizedExtendElement] = [ + extends_element_with_sorted_options(extend) for extend in proto_file_element.extend_declarations + ] - return ProtoFileElement( + return NormalizedProtoFileElement( location=proto_file_element.location, package_name=proto_file_element.package_name, syntax=proto_file_element.syntax, @@ -209,12 +272,3 @@ def normalize_options_ordered(proto_file_element: ProtoFileElement) -> ProtoFile extend_declarations=sorted_extend_declarations, options=sorted_options, ) - - -# if other normalizations are added we will switch to a more generic approach: -# def normalize_parsed_file(proto_file_element: ProtoFileElement, -# normalization: ProtobufNormalisationOptions) -> ProtoFileElement: -# if normalization == ProtobufNormalisationOptions.sort_options: -# return normalize_options_ordered(proto_file_element) -# else: -# assert_never(normalization) diff --git a/karapace/protobuf/serialization.py b/karapace/protobuf/serialization.py index 59943d21c..abc01247d 100644 --- a/karapace/protobuf/serialization.py +++ b/karapace/protobuf/serialization.py @@ -19,7 +19,7 @@ from karapace.protobuf.syntax import Syntax from karapace.protobuf.type_element import TypeElement from types import MappingProxyType -from typing import Any +from typing import Any, Sequence import base64 import google.protobuf.descriptor @@ -269,7 +269,7 @@ def _serialize_msgtype(t: MessageElement) -> google.protobuf.descriptor_pb2.Desc return d -def _serialize_options(options: list[OptionElement], result: google.protobuf.descriptor_pb2.FileOptions) -> None: +def _serialize_options(options: Sequence[OptionElement], result: google.protobuf.descriptor_pb2.FileOptions) -> None: for opt in options: if opt.name == ("java_package"): result.java_package = opt.value diff --git a/karapace/protobuf/service_element.py b/karapace/protobuf/service_element.py index 5ccc3bb6d..ed714c58c 100644 --- a/karapace/protobuf/service_element.py +++ b/karapace/protobuf/service_element.py @@ -11,6 +11,7 @@ from karapace.protobuf.option_element import OptionElement from karapace.protobuf.rpc_element import RpcElement from karapace.protobuf.utils import append_documentation, append_indented +from typing import Sequence @dataclass @@ -18,8 +19,8 @@ class ServiceElement: location: Location name: str documentation: str = "" - rpcs: list[RpcElement] | None = None - options: list[OptionElement] | None = None + rpcs: Sequence[RpcElement] | None = None + options: Sequence[OptionElement] | None = None def to_schema(self) -> str: result: list[str] = [] diff --git a/karapace/protobuf/type_element.py b/karapace/protobuf/type_element.py index c33753ed6..ec840a801 100644 --- a/karapace/protobuf/type_element.py +++ b/karapace/protobuf/type_element.py @@ -8,7 +8,7 @@ from dataclasses import dataclass from karapace.protobuf.location import Location -from typing import TYPE_CHECKING +from typing import Sequence, TYPE_CHECKING if TYPE_CHECKING: from karapace.protobuf.compare_result import CompareResult @@ -21,8 +21,8 @@ class TypeElement: location: Location name: str documentation: str - options: list[OptionElement] - nested_types: list[TypeElement] + options: Sequence[OptionElement] + nested_types: Sequence[TypeElement] def to_schema(self) -> str: """Convert the object to valid protobuf syntax. diff --git a/karapace/schema_models.py b/karapace/schema_models.py index 0215ebc69..46e3832d5 100644 --- a/karapace/schema_models.py +++ b/karapace/schema_models.py @@ -19,7 +19,7 @@ ProtobufUnresolvedDependencyException, SchemaParseException as ProtobufSchemaParseException, ) -from karapace.protobuf.proto_normalizations import normalize_options_ordered +from karapace.protobuf.proto_normalizations import NormalizedProtobufSchema from karapace.protobuf.schema import ProtobufSchema from karapace.schema_references import Reference from karapace.schema_type import SchemaType @@ -71,15 +71,16 @@ def parse_protobuf_schema_definition( ProtobufUnresolvedDependencyException if Protobuf dependency cannot be resolved. """ - protobuf_schema = ProtobufSchema(schema_definition, references, dependencies) + protobuf_schema = ( + ProtobufSchema(schema_definition, references, dependencies) + if not normalize + else NormalizedProtobufSchema(schema_definition, references, dependencies) + ) if validate_references: result = protobuf_schema.verify_schema_dependencies() if not result.result: raise ProtobufUnresolvedDependencyException(f"{result.message}") - if protobuf_schema.proto_file_element is not None and normalize: - protobuf_schema.proto_file_element = normalize_options_ordered(protobuf_schema.proto_file_element) - return protobuf_schema @@ -174,6 +175,7 @@ def schema(self) -> Draft7Validator | AvroSchema | ProtobufSchema: validate_avro_enum_symbols=True, references=self.references, dependencies=self.dependencies, + normalize=False, ) return parsed_typed_schema.schema @@ -210,7 +212,9 @@ def parse( elif schema_type is SchemaType.PROTOBUF: try: - parsed_schema = parse_protobuf_schema_definition(schema_str, references, dependencies, normalize=normalize) + parsed_schema = parse_protobuf_schema_definition( + schema_str, references, dependencies, validate_references=True, normalize=normalize + ) except ( TypeError, SchemaError, diff --git a/karapace/schema_reader.py b/karapace/schema_reader.py index 750992686..a2be0d46e 100644 --- a/karapace/schema_reader.py +++ b/karapace/schema_reader.py @@ -510,6 +510,7 @@ def _handle_msg_schema(self, key: dict, value: dict | None) -> None: resolved_references, resolved_dependencies, validate_references=False, + normalize=False, ) schema_str = str(parsed_schema) except InvalidSchema: diff --git a/tests/integration/test_schema_protobuf.py b/tests/integration/test_schema_protobuf.py index 778be5776..ede01737a 100644 --- a/tests/integration/test_schema_protobuf.py +++ b/tests/integration/test_schema_protobuf.py @@ -1272,49 +1272,51 @@ async def test_protobuf_update_ordering(registry_async_client: Client) -> None: assert schema_id != res.json()["id"] -async def test_protobuf_normalization_of_options(registry_async_client: Client) -> None: - subject = create_subject_name_factory("test_protobuf_normalization")() - - schema_with_option_unordered_1 = """\ +SCHEMA_WITH_OPTION_UNORDERDERED = """\ syntax = "proto3"; package tc4; option java_package = "com.example"; -option java_outer_classname = "FredProto"; -option java_multiple_files = true; -option java_generic_services = true; option java_generate_equals_and_hash = true; option java_string_check_utf8 = true; +option java_multiple_files = true; +option java_outer_classname = "FredProto"; +option java_generic_services = true; message Foo { string code = 1; } """ - body = {"schemaType": "PROTOBUF", "schema": schema_with_option_unordered_1} - res = await registry_async_client.post(f"subjects/{subject}/versions?normalize=true", json=body) - - assert res.status_code == 200 - assert "id" in res.json() - original_schema_id = res.json()["id"] - schema_with_option_unordered_2 = """\ +SCHEMA_WITH_OPTION_ORDERED = """\ syntax = "proto3"; package tc4; -option java_package = "com.example"; option java_generate_equals_and_hash = true; -option java_string_check_utf8 = true; +option java_generic_services = true; option java_multiple_files = true; option java_outer_classname = "FredProto"; -option java_generic_services = true; +option java_package = "com.example"; +option java_string_check_utf8 = true; message Foo { string code = 1; } """ - body = {"schemaType": "PROTOBUF", "schema": schema_with_option_unordered_2} + +async def test_registering_normalized_schema(registry_async_client: Client) -> None: + subject = create_subject_name_factory("test_protobuf_normalization")() + + body = {"schemaType": "PROTOBUF", "schema": SCHEMA_WITH_OPTION_ORDERED} + res = await registry_async_client.post(f"subjects/{subject}/versions?normalize=true", json=body) + + assert res.status_code == 200 + assert "id" in res.json() + original_schema_id = res.json()["id"] + + body = {"schemaType": "PROTOBUF", "schema": SCHEMA_WITH_OPTION_UNORDERDERED} res = await registry_async_client.post(f"subjects/{subject}", json=body) assert res.status_code == 404 @@ -1325,51 +1327,24 @@ async def test_protobuf_normalization_of_options(registry_async_client: Client) assert original_schema_id == res.json()["id"] -async def test_protobuf_normalization_of_options_specify_version(registry_async_client: Client) -> None: +async def test_normalized_schema_idempotence_produce_and_fetch(registry_async_client: Client) -> None: subject = create_subject_name_factory("test_protobuf_normalization")() - schema_with_option_unordered_1 = """\ -syntax = "proto3"; -package tc4; - -option java_package = "com.example"; -option java_outer_classname = "FredProto"; -option java_multiple_files = true; -option java_generic_services = true; -option java_generate_equals_and_hash = true; -option java_string_check_utf8 = true; - -message Foo { - string code = 1; -} -""" - - body = {"schemaType": "PROTOBUF", "schema": schema_with_option_unordered_1} + body = {"schemaType": "PROTOBUF", "schema": SCHEMA_WITH_OPTION_UNORDERDERED} res = await registry_async_client.post(f"subjects/{subject}/versions?normalize=true", json=body) assert res.status_code == 200 assert "id" in res.json() original_schema_id = res.json()["id"] - schema_with_option_unordered_2 = """\ -syntax = "proto3"; -package tc4; - -option java_package = "com.example"; -option java_generate_equals_and_hash = true; -option java_string_check_utf8 = true; -option java_multiple_files = true; -option java_outer_classname = "FredProto"; -option java_generic_services = true; - -message Foo { - string code = 1; -} -""" - - body = {"schemaType": "PROTOBUF", "schema": schema_with_option_unordered_2} + body = {"schemaType": "PROTOBUF", "schema": SCHEMA_WITH_OPTION_ORDERED} res = await registry_async_client.post(f"subjects/{subject}/versions?normalize=true", json=body) assert res.status_code == 200 assert "id" in res.json() assert original_schema_id == res.json()["id"] + + res = await registry_async_client.get(f"/schemas/ids/{original_schema_id}") + assert res.status_code == 200 + assert "schema" in res.json() + assert res.json()["schema"] == SCHEMA_WITH_OPTION_ORDERED diff --git a/tests/unit/protobuf/test_protobuf_normalization.py b/tests/unit/protobuf/test_protobuf_normalization.py index 4d68e5e70..b772b293c 100644 --- a/tests/unit/protobuf/test_protobuf_normalization.py +++ b/tests/unit/protobuf/test_protobuf_normalization.py @@ -4,7 +4,7 @@ """ from karapace.protobuf.compare_result import CompareResult from karapace.protobuf.location import Location -from karapace.protobuf.proto_normalizations import normalize_options_ordered +from karapace.protobuf.proto_normalizations import normalize from karapace.protobuf.proto_parser import ProtoParser import pytest @@ -366,7 +366,7 @@ string fieldX = 4; - message NeastedFoo { + message NestedFoo { string fieldA = 1; option (my_option) = "my_value"; option (my_option2) = "my_value2"; @@ -438,7 +438,7 @@ string fieldX = 4; - message NeastedFoo { + message NestedFoo { string fieldA = 1; option (my_option2) = "my_value2"; option (my_option) = "my_value"; @@ -510,11 +510,11 @@ (PROTO_WITH_COMPLEX_SCHEMA_ORDERED, PROTO_WITH_COMPLEX_SCHEMA_UNORDERED), ), ) -def test_different_options_order_its_correctly_normalized(ordered_schema: str, unordered_schema: str) -> None: +def test_differently_ordered_options_normalizes_equally(ordered_schema: str, unordered_schema: str) -> None: ordered_proto = ProtoParser.parse(location, ordered_schema) unordered_proto = ProtoParser.parse(location, unordered_schema) result = CompareResult() - normalize_options_ordered(ordered_proto).compare(normalize_options_ordered(unordered_proto), result) + normalize(ordered_proto).compare(normalize(unordered_proto), result) assert result.is_compatible() - assert normalize_options_ordered(ordered_proto).to_schema() == normalize_options_ordered(unordered_proto).to_schema() + assert normalize(ordered_proto).to_schema() == normalize(unordered_proto).to_schema()