You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by rs...@apache.org on 2022/09/08 16:16:27 UTC

[avro] branch master updated: AVRO-3622: Fix compatibility check for schemas having or missing namespace (#1843)

This is an automated email from the ASF dual-hosted git repository.

rskraba pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git


The following commit(s) were added to refs/heads/master by this push:
     new 34845fd49 AVRO-3622: Fix compatibility check for schemas having or missing namespace (#1843)
34845fd49 is described below

commit 34845fd49af9cd00e98f5930c6ac6190da5ea6a3
Author: Jarkko Jaakola <91...@users.noreply.github.com>
AuthorDate: Thu Sep 8 19:16:21 2022 +0300

    AVRO-3622: Fix compatibility check for schemas having or missing namespace (#1843)
    
    * AVRO-3622 (python) Fix compatibility check for schemas having or missing namespace
    
    Python implementation does not treat the name and namespace element as Java library.
    This causes the compatibility check to fail as the name which can be fully qualified
    is not splitted to name and namespace elements.
    
    * AVRO-3622 (python) Unit test Name when inlined namespace is given
    
    * AVRO-3622 (python) Test for trailing comma in name
---
 .../org/apache/avro/TestSchemaCompatibility.java   |  4 ++-
 .../src/test/java/org/apache/avro/TestSchemas.java |  6 +++++
 lang/py/avro/name.py                               |  6 +++++
 lang/py/avro/schema.py                             |  4 +--
 lang/py/avro/test/test_compatibility.py            | 30 ++++++++++++++++++++++
 lang/py/avro/test/test_schema.py                   | 16 ++++++++++++
 6 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/lang/java/avro/src/test/java/org/apache/avro/TestSchemaCompatibility.java b/lang/java/avro/src/test/java/org/apache/avro/TestSchemaCompatibility.java
index 5f7b62842..b3515551a 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/TestSchemaCompatibility.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/TestSchemaCompatibility.java
@@ -68,6 +68,8 @@ import static org.apache.avro.TestSchemas.LONG_SCHEMA;
 import static org.apache.avro.TestSchemas.LONG_UNION_SCHEMA;
 import static org.apache.avro.TestSchemas.NS_RECORD1;
 import static org.apache.avro.TestSchemas.NS_RECORD2;
+import static org.apache.avro.TestSchemas.WITH_NS;
+import static org.apache.avro.TestSchemas.WITHOUT_NS;
 import static org.apache.avro.TestSchemas.NULL_SCHEMA;
 import static org.apache.avro.TestSchemas.ReaderWriter;
 import static org.apache.avro.TestSchemas.STRING_ARRAY_SCHEMA;
@@ -322,7 +324,7 @@ public class TestSchemaCompatibility {
 
       // This is comparing two records that have an inner array of records with
       // different namespaces.
-      new ReaderWriter(NS_RECORD1, NS_RECORD2));
+      new ReaderWriter(NS_RECORD1, NS_RECORD2), new ReaderWriter(WITHOUT_NS, WITH_NS));
 
   // -----------------------------------------------------------------------------------------------
 
diff --git a/lang/java/avro/src/test/java/org/apache/avro/TestSchemas.java b/lang/java/avro/src/test/java/org/apache/avro/TestSchemas.java
index 5005bcac4..900939b23 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/TestSchemas.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/TestSchemas.java
@@ -116,6 +116,9 @@ public class TestSchemas {
   static final Schema NS_INNER_RECORD1 = Schema.createRecord("InnerRecord1", null, "ns1", false);
   static final Schema NS_INNER_RECORD2 = Schema.createRecord("InnerRecord1", null, "ns2", false);
 
+  static final Schema WITHOUT_NS = Schema.createRecord("Record", null, null, false);
+  static final Schema WITH_NS = Schema.createRecord("ns.Record", null, null, false);
+
   static {
     EMPTY_RECORD1.setFields(Collections.emptyList());
     EMPTY_RECORD2.setFields(Collections.emptyList());
@@ -145,6 +148,9 @@ public class TestSchemas {
         .setFields(list(new Schema.Field("f1", Schema.createUnion(NULL_SCHEMA, Schema.createArray(NS_INNER_RECORD1)))));
     NS_RECORD2
         .setFields(list(new Schema.Field("f1", Schema.createUnion(NULL_SCHEMA, Schema.createArray(NS_INNER_RECORD2)))));
+
+    WITH_NS.setFields(list(new Field("f1", INT_SCHEMA, null, null)));
+    WITHOUT_NS.setFields(list(new Field("f1", INT_SCHEMA, null, null)));
   }
 
   // Recursive records
diff --git a/lang/py/avro/name.py b/lang/py/avro/name.py
index 67f1e0f3f..e7d8710fc 100644
--- a/lang/py/avro/name.py
+++ b/lang/py/avro/name.py
@@ -51,6 +51,7 @@ class Name:
     """Class to describe Avro name."""
 
     _full: Optional[str] = None
+    _name: Optional[str] = None
 
     def __init__(self, name_attr: Optional[str] = None, space_attr: Optional[str] = None, default_space: Optional[str] = None) -> None:
         """The fullname is determined in one of the following ways:
@@ -80,6 +81,7 @@ class Name:
         if name_attr == "":
             raise avro.errors.SchemaParseException("Name must not be the empty string.")
         # The empty string may be used as a namespace to indicate the null namespace.
+        self._name = name_attr.split(".")[-1]
         self._full = (
             name_attr
             if "." in name_attr or space_attr == "" or not (space_attr or default_space)
@@ -91,6 +93,10 @@ class Name:
         """Equality of names is defined on the fullname and is case-sensitive."""
         return hasattr(other, "fullname") and self.fullname == getattr(other, "fullname")
 
+    @property
+    def name(self) -> Optional[str]:
+        return self._name
+
     @property
     def fullname(self) -> Optional[str]:
         return self._full
diff --git a/lang/py/avro/schema.py b/lang/py/avro/schema.py
index 9c2b16d7d..eba1a33f8 100644
--- a/lang/py/avro/schema.py
+++ b/lang/py/avro/schema.py
@@ -263,8 +263,8 @@ class NamedSchema(Schema):
         new_name = names.add_name(name, namespace, self)
 
         # Store name and namespace as they were read in origin schema
-        self.set_prop("name", name)
-        if namespace is not None:
+        self.set_prop("name", new_name.name)
+        if new_name.space:
             self.set_prop("namespace", new_name.space)
 
         # Store full name as calculated from name, namespace
diff --git a/lang/py/avro/test/test_compatibility.py b/lang/py/avro/test/test_compatibility.py
index d67a29bbc..3c36b6f84 100644
--- a/lang/py/avro/test/test_compatibility.py
+++ b/lang/py/avro/test/test_compatibility.py
@@ -370,6 +370,34 @@ NS_RECORD2 = parse(
         }
     )
 )
+WITHOUT_NAMESPACE_RECORD = parse(
+    json.dumps(
+        {
+            "type": SchemaType.RECORD,
+            "name": "Record",
+            "fields": [
+                {
+                    "name": "f1",
+                    "type": "int",
+                }
+            ],
+        },
+    )
+)
+WITH_NAMESPACE_RECORD = parse(
+    json.dumps(
+        {
+            "type": SchemaType.RECORD,
+            "name": "ns.Record",
+            "fields": [
+                {
+                    "name": "f1",
+                    "type": "int",
+                }
+            ],
+        },
+    )
+)
 
 UNION_INT_RECORD1 = UnionSchema(
     [
@@ -583,6 +611,7 @@ class TestCompatibility(unittest.TestCase):
         self.assertIsInstance(reader, UnionSchema)
         self.assertIsInstance(writer, UnionSchema)
         self.assertFalse(self.are_compatible(reader, writer))
+
         # testReaderWriterCompatibility
         compatible_reader_writer_test_cases = [
             (BOOLEAN_SCHEMA, BOOLEAN_SCHEMA),
@@ -659,6 +688,7 @@ class TestCompatibility(unittest.TestCase):
                 ENUM_ABC_FIELD_DEFAULT_B_ENUM_DEFAULT_A_RECORD,
             ),
             (NS_RECORD1, NS_RECORD2),
+            (WITHOUT_NAMESPACE_RECORD, WITH_NAMESPACE_RECORD),
         ]
 
         for (reader, writer) in compatible_reader_writer_test_cases:
diff --git a/lang/py/avro/test/test_schema.py b/lang/py/avro/test/test_schema.py
index 5a4949301..ab61918d6 100644
--- a/lang/py/avro/test/test_schema.py
+++ b/lang/py/avro/test/test_schema.py
@@ -579,6 +579,18 @@ class TestMisc(unittest.TestCase):
         fullname = avro.schema.Name("a", "o.a.h", None).fullname
         self.assertEqual(fullname, "o.a.h.a")
 
+    def test_name_inlined_space(self):
+        """Space inlined with name is correctly splitted out."""
+        name = avro.schema.Name("o.a", None)
+        self.assertEqual(name.fullname, "o.a")
+        self.assertEqual(name.name, "a")
+        self.assertEqual(name.space, "o")
+
+        name = avro.schema.Name("o.a.h.a", None)
+        self.assertEqual(name.fullname, "o.a.h.a")
+        self.assertEqual(name.name, "a")
+        self.assertEqual(name.space, "o.a.h")
+
     def test_fullname_space_specified(self):
         """When name contains dots, namespace should be ignored."""
         fullname = avro.schema.Name("a.b.c.d", "o.a.h", None).fullname
@@ -629,6 +641,10 @@ class TestMisc(unittest.TestCase):
             None,
             None,
         )
+        # A name cannot start with dot.
+        self.assertRaises(avro.errors.InvalidName, avro.schema.Name, ".a", None, None)
+        self.assertRaises(avro.errors.InvalidName, avro.schema.Name, "o..a", None, None)
+        self.assertRaises(avro.errors.InvalidName, avro.schema.Name, "a.", None, None)
 
     def test_null_namespace(self):
         """The empty string may be used as a namespace to indicate the null namespace."""