fix: enable self-referencing schemas

Aiven-Open · Sep 19, 2023 · d8212b8 · d8212b8
1 parent a33b4e1
commit d8212b8
Show file tree

Hide file tree

Showing 3 changed files with 118 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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)