Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: enable self-referencing schemas #716

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 21 additions & 7 deletions karapace/protobuf/dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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')
Expand Down
15 changes: 14 additions & 1 deletion karapace/protobuf/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)

Expand Down
83 changes: 83 additions & 0 deletions tests/unit/protobuf/test_protobuf_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)