Skip to content

Commit

Permalink
Update avro to 1.11.3 (backport to 2.18 branch) (#512)
Browse files Browse the repository at this point in the history
* Update avro to 1.11.3

Namespace for nested classes no longer ends with '$'. This is how avro library generates schema since version 1.9. See: AVRO-2143
Please note that resolution of nested classes without '$' was implemented long ago in c570549.

Fixes #167

* Remove '$' from Avro namespaces when Java class has multiple nesting levels

Avro before 1.11 was generating schemas with '$' in namespace if class had multiple nesting levels. To fix compatibility with avro 1.11+ make sure all dollar characters are replaced by dots.
Related: AVRO-2757
  • Loading branch information
rafalh committed Aug 24, 2024
1 parent 9e27d43 commit 7e3c869
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 13 deletions.
2 changes: 1 addition & 1 deletion avro/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ abstractions.
<dependency>
<groupId>org.apache.avro</groupId>
<artifactId>avro</artifactId>
<version>1.8.2</version>
<version>1.11.3</version>
</dependency>

<!-- and for testing we need logback -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ protected static String getNamespace(Class<?> cls) {
// NOTE: was reverted in 2.8.8, but is enabled for Jackson 2.9.
Class<?> enclosing = cls.getEnclosingClass();
if (enclosing != null) {
return enclosing.getName() + "$";
// 23-Aug-2024: Changed as per [dataformats-binary#167]
// Enclosing class may also be nested
return enclosing.getName().replace('$', '.');
}
Package pkg = cls.getPackage();
return (pkg == null) ? "" : pkg.getName();
Expand Down Expand Up @@ -351,6 +353,8 @@ public static String getFullName(Schema schema) {
if (namespace == null) {
return name;
}
// 23-Aug-2024: [dataformats-binary#167] Still needed for backwards-compatibility
// with schemas that use dollar sign for nested classes (Apache Avro before 1.9)
final int len = namespace.length();
if (namespace.charAt(len-1) == '$') {
return namespace + name;
Expand Down Expand Up @@ -441,13 +445,25 @@ private static String _resolve(FullNameKey key) {
// Check if this is a nested class
// 19-Sep-2020, tatu: This is a horrible, horribly inefficient and all-around
// wrong mechanism. To be abolished if possible.
final String nestedClassName = key.nameWithSeparator('$');
try {
Class.forName(nestedClassName);
return nestedClassName;
} catch (ClassNotFoundException e) {
// Could not find a nested class, must be a regular class
return key.nameWithSeparator('.');
// 23-Aug-2024:[dataformats-binary#167] Based on SpecificData::getClass
// from Apache Avro. Initially assume that namespace is a Java package
StringBuilder sb = new StringBuilder(key.nameWithSeparator('.'));
int lastDot = sb.length();
while (true) {
try {
// Try to resolve the class
String className = sb.toString();
Class.forName(className);
return className;
} catch (ClassNotFoundException e) {
// Class does not exist - perhaps last dot is actually a nested class
lastDot = sb.lastIndexOf(".", lastDot);
if (lastDot == -1) {
// No more dots so we are unable to resolve, should we throw an exception?
return key.nameWithSeparator('.');
}
sb.setCharAt(lastDot, '$');
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@ enum EnumWithoutAvroNamespaceAnnotation {FOO, BAR;}
@AvroNamespace("EnumWithAvroNamespaceAnnotation.namespace")
enum EnumWithAvroNamespaceAnnotation {FOO, BAR;}

static class Foo {
static class Bar {
static class ClassWithMultipleNestingLevels {
}

enum EnumWithMultipleNestingLevels {FOO, BAR;}
}
}

@Test
public void class_without_AvroNamespace_test() throws Exception {
// GIVEN
Expand All @@ -35,7 +44,7 @@ public void class_without_AvroNamespace_test() throws Exception {

// THEN
assertThat(actualSchema.getNamespace())
.isEqualTo("com.fasterxml.jackson.dataformat.avro.annotation.AvroNamespaceTest$");
.isEqualTo("com.fasterxml.jackson.dataformat.avro.annotation.AvroNamespaceTest");
}

@Test
Expand All @@ -53,6 +62,21 @@ public void class_with_AvroNamespace_test() throws Exception {
.isEqualTo("ClassWithAvroNamespaceAnnotation.namespace");
}

@Test
public void class_with_multiple_nesting_levels_test() throws Exception {
// GIVEN
AvroMapper mapper = new AvroMapper();
AvroSchemaGenerator gen = new AvroSchemaGenerator();

// WHEN
mapper.acceptJsonFormatVisitor(Foo.Bar.ClassWithMultipleNestingLevels.class, gen);
Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();

// THEN
assertThat(actualSchema.getNamespace())
.isEqualTo("com.fasterxml.jackson.dataformat.avro.annotation.AvroNamespaceTest.Foo.Bar");
}

@Test
public void enum_without_AvroNamespace_test() throws Exception {
// GIVEN
Expand All @@ -65,7 +89,7 @@ public void enum_without_AvroNamespace_test() throws Exception {

// THEN
assertThat(actualSchema.getNamespace())
.isEqualTo("com.fasterxml.jackson.dataformat.avro.annotation.AvroNamespaceTest$");
.isEqualTo("com.fasterxml.jackson.dataformat.avro.annotation.AvroNamespaceTest");
}

@Test
Expand All @@ -83,4 +107,18 @@ public void enum_with_AvroNamespace_test() throws Exception {
.isEqualTo("EnumWithAvroNamespaceAnnotation.namespace");
}

@Test
public void enum_with_multiple_nesting_levels_test() throws Exception {
// GIVEN
AvroMapper mapper = new AvroMapper();
AvroSchemaGenerator gen = new AvroSchemaGenerator();

// WHEN
mapper.acceptJsonFormatVisitor(Foo.Bar.EnumWithMultipleNestingLevels.class, gen);
Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();

// THEN
assertThat(actualSchema.getNamespace())
.isEqualTo("com.fasterxml.jackson.dataformat.avro.annotation.AvroNamespaceTest.Foo.Bar");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

public class AvroAliasTest extends InteropTestBase {

@AvroAlias(alias = "Employee", space = "com.fasterxml.jackson.dataformat.avro.AvroTestBase$")
@AvroAlias(alias = "Employee", space = "com.fasterxml.jackson.dataformat.avro.AvroTestBase")
public static class NewEmployee {

public String name;
Expand All @@ -40,7 +40,7 @@ public static class AliasedNameEmployee {
public AliasedNameEmployee boss;
}

@AvroAlias(alias = "Size", space = "com.fasterxml.jackson.dataformat.avro.AvroTestBase$")
@AvroAlias(alias = "Size", space = "com.fasterxml.jackson.dataformat.avro.AvroTestBase")
public enum NewSize {
SMALL,
LARGE;
Expand Down
4 changes: 4 additions & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,10 @@ Yoann Vernageau (@yvrng)
when source is an empty `InputStream`
(2.17.1)

Rafał Harabień (@rafalh)
* Contributed fix for #167: (avro) Incompatibility with Avro >=1.9.0 (upgrade to Avro 1.11.3)
(2.18.0)

PJ Fanning (@pjfanning)
* Contributed #484: (protobuf) Rework synchronization in `ProtobufMapper`
(2.18.0)
Expand Down
3 changes: 3 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ Active maintainers:

2.18.0 (not yet released)

#167: (avro) Incompatibility with Avro >=1.9.0 (upgrade to Avro 1.11.3)
(reported by @Sage-Pierce)
(fix contributed by Rafał H)
#484: (protobuf) Rework synchronization in `ProtobufMapper`
(contributed by @pjfanning)
#494: (avro) Avro Schema generation: allow mapping Java Enum properties to
Expand Down

0 comments on commit 7e3c869

Please sign in to comment.