Skip to content

Commit

Permalink
Merge pull request apache#283 from cutting/master
Browse files Browse the repository at this point in the history 
AVRO-2143. Fix Java reflect to stop using dollar sign in namespaces of nested classes.
  • Loading branch information
@cutting
cutting authored Feb 22, 2018
2 parents 338f250 + 340ef22 commit 407c48a
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 20 deletions.
2 changes: 1 addition & 1 deletion lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ protected Schema createSchema(Type type, Map<String,Schema> names) {
String name = c.getSimpleName();
String space = c.getPackage() == null ? "" : c.getPackage().getName();
if (c.getEnclosingClass() != null) // nested class
space = c.getEnclosingClass().getName() + "$";
space = c.getEnclosingClass().getName();
Union union = c.getAnnotation(Union.class);
if (union != null) { // union annotated
return getAnnotatedUnion(union, names);
Expand Down
16 changes: 14 additions & 2 deletions lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ public Class getClass(Schema schema) {
try {
c = ClassUtils.forName(getClassLoader(), getClassName(schema));
} catch (ClassNotFoundException e) {
c = NO_CLASS;
try { // nested class?
c = ClassUtils.forName(getClassLoader(), getNestedClassName(schema));
} catch (ClassNotFoundException ex) {
c = NO_CLASS;
}
}
classCache.put(name, c);
}
Expand Down Expand Up @@ -205,10 +209,18 @@ public static String getClassName(Schema schema) {
String name = schema.getName();
if (namespace == null || "".equals(namespace))
return name;
String dot = namespace.endsWith("$") ? "" : ".";
String dot = namespace.endsWith("$") ? "" : "."; // back-compatibly handle $
return namespace + dot + name;
}

private String getNestedClassName(Schema schema) {
String namespace = schema.getNamespace();
String name = schema.getName();
if (namespace == null || "".equals(namespace))
return name;
return namespace + "$" + name;
}

private final LoadingCache<java.lang.reflect.Type,Schema> schemaCache =
CacheBuilder.newBuilder()
.weakKeys()
Expand Down
38 changes: 24 additions & 14 deletions lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public class TestReflect {
@Test public void testUnionWithEnum() {
Schema s = new Schema.Parser().parse
("[\"null\", {\"type\":\"enum\",\"name\":\"E\",\"namespace\":" +
"\"org.apache.avro.reflect.TestReflect$\",\"symbols\":[\"A\",\"B\"]}]");
"\"org.apache.avro.reflect.TestReflect\",\"symbols\":[\"A\",\"B\"]}]");
GenericData data = ReflectData.get();
assertEquals(1, data.resolveUnion(s, E.A));
}
Expand Down Expand Up @@ -525,50 +525,50 @@ void checkReadWrite(Object object, Schema s) throws Exception {
public static enum E { A, B };
@Test public void testEnum() throws Exception {
check(E.class, "{\"type\":\"enum\",\"name\":\"E\",\"namespace\":"
+"\"org.apache.avro.reflect.TestReflect$\",\"symbols\":[\"A\",\"B\"]}");
+"\"org.apache.avro.reflect.TestReflect\",\"symbols\":[\"A\",\"B\"]}");
}

public static class R { int a; long b; }
@Test public void testRecord() throws Exception {
check(R.class, "{\"type\":\"record\",\"name\":\"R\",\"namespace\":"
+"\"org.apache.avro.reflect.TestReflect$\",\"fields\":["
+"\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+"{\"name\":\"a\",\"type\":\"int\"},"
+"{\"name\":\"b\",\"type\":\"long\"}]}");
}

public static class RAvroIgnore { @AvroIgnore int a; }
@Test public void testAnnotationAvroIgnore() throws Exception {
check(RAvroIgnore.class, "{\"type\":\"record\",\"name\":\"RAvroIgnore\",\"namespace\":"
+"\"org.apache.avro.reflect.TestReflect$\",\"fields\":[]}");
+"\"org.apache.avro.reflect.TestReflect\",\"fields\":[]}");
}

public static class RAvroMeta { @AvroMeta(key="K", value="V") int a; }
@Test public void testAnnotationAvroMeta() throws Exception {
check(RAvroMeta.class, "{\"type\":\"record\",\"name\":\"RAvroMeta\",\"namespace\":"
+"\"org.apache.avro.reflect.TestReflect$\",\"fields\":["
+"\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+"{\"name\":\"a\",\"type\":\"int\",\"K\":\"V\"}]}");
}

public static class RAvroName { @AvroName("b") int a; }
@Test public void testAnnotationAvroName() throws Exception {
check(RAvroName.class, "{\"type\":\"record\",\"name\":\"RAvroName\",\"namespace\":"
+"\"org.apache.avro.reflect.TestReflect$\",\"fields\":["
+"\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+"{\"name\":\"b\",\"type\":\"int\"}]}");
}

public static class RAvroNameCollide { @AvroName("b") int a; int b; }
@Test(expected=Exception.class)
public void testAnnotationAvroNameCollide() throws Exception {
check(RAvroNameCollide.class, "{\"type\":\"record\",\"name\":\"RAvroNameCollide\",\"namespace\":"
+"\"org.apache.avro.reflect.TestReflect$\",\"fields\":["
+"\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+"{\"name\":\"b\",\"type\":\"int\"},"
+"{\"name\":\"b\",\"type\":\"int\"}]}");
}

public static class RAvroStringableField { @Stringable int a; }
public void testAnnotationAvroStringableFields() throws Exception {
check(RAvroStringableField.class, "{\"type\":\"record\",\"name\":\"RAvroNameCollide\",\"namespace\":"
+"\"org.apache.avro.reflect.TestReflect$\",\"fields\":["
+"\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+"{\"name\":\"a\",\"type\":\"String\"}]}");
}

Expand Down Expand Up @@ -703,7 +703,7 @@ public void testMultipleAnnotations() throws IOException {
public void testAvroEncodeInducing() throws IOException {
Schema schm = ReflectData.get().getSchema(AvroEncRecord.class);
assertEquals(schm.toString(), "{\"type\":\"record\",\"name\":\"AvroEncRecord\",\"namespace" +
"\":\"org.apache.avro.reflect.TestReflect$\",\"fields\":[{\"name\":\"date\"," +
"\":\"org.apache.avro.reflect.TestReflect\",\"fields\":[{\"name\":\"date\"," +
"\"type\":{\"type\":\"long\",\"CustomEncoding\":\"DateAsLongEncoding\"}}]}");
}

Expand Down Expand Up @@ -1059,9 +1059,19 @@ private static class AliasC { }

@Test
public void testAvroAliasOnClass() {
check(AliasA.class, "{\"type\":\"record\",\"name\":\"AliasA\",\"namespace\":\"org.apache.avro.reflect.TestReflect$\",\"fields\":[],\"aliases\":[\"b.a\"]}");
check(AliasB.class, "{\"type\":\"record\",\"name\":\"AliasB\",\"namespace\":\"org.apache.avro.reflect.TestReflect$\",\"fields\":[],\"aliases\":[\"a\"]}");
check(AliasC.class, "{\"type\":\"record\",\"name\":\"AliasC\",\"namespace\":\"org.apache.avro.reflect.TestReflect$\",\"fields\":[],\"aliases\":[\"a\"]}");
check(AliasA.class, "{\"type\":\"record\",\"name\":\"AliasA\",\"namespace\":\"org.apache.avro.reflect.TestReflect\",\"fields\":[],\"aliases\":[\"b.a\"]}");
check(AliasB.class, "{\"type\":\"record\",\"name\":\"AliasB\",\"namespace\":\"org.apache.avro.reflect.TestReflect\",\"fields\":[],\"aliases\":[\"a\"]}");
check(AliasC.class, "{\"type\":\"record\",\"name\":\"AliasC\",\"namespace\":\"org.apache.avro.reflect.TestReflect\",\"fields\":[],\"aliases\":[\"a\"]}");
}

private static class Z {}

@Test public void testDollarTerminatedNamespaceCompatibility() {
ReflectData data = ReflectData.get();
Schema s = new Schema.Parser().parse
("{\"type\":\"record\",\"name\":\"Z\",\"namespace\":\"org.apache.avro.reflect.TestReflect$\",\"fields\":[]}");
assertEquals(data.getSchema(data.getClass(s)).toString(),
"{\"type\":\"record\",\"name\":\"Z\",\"namespace\":\"org.apache.avro.reflect.TestReflect\",\"fields\":[]}");
}

private static class ClassWithAliasOnField {
Expand All @@ -1078,7 +1088,7 @@ private static class ClassWithAliasAndNamespaceOnField {
public void testAvroAliasOnField() {

Schema expectedSchema = SchemaBuilder.record(ClassWithAliasOnField.class.getSimpleName())
.namespace("org.apache.avro.reflect.TestReflect$").fields().name("primitiveField").aliases("aliasName")
.namespace("org.apache.avro.reflect.TestReflect").fields().name("primitiveField").aliases("aliasName")
.type(Schema.create(org.apache.avro.Schema.Type.INT)).noDefault().endRecord();

check(ClassWithAliasOnField.class, expectedSchema.toString());
Expand All @@ -1098,7 +1108,7 @@ private static class DefaultTest {
public void testAvroDefault() {
check(DefaultTest.class,
"{\"type\":\"record\",\"name\":\"DefaultTest\","
+"\"namespace\":\"org.apache.avro.reflect.TestReflect$\",\"fields\":["
+"\"namespace\":\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+"{\"name\":\"foo\",\"type\":\"int\",\"default\":1}]}");
}

Expand Down
6 changes: 3 additions & 3 deletions lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectLogicalTypes.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public int hashCode() {
public void testDecimalBytes() throws IOException {
Schema schema = REFLECT.getSchema(DecimalRecordBytes.class);
Assert.assertEquals("Should have the correct record name",
"org.apache.avro.reflect.TestReflectLogicalTypes$",
"org.apache.avro.reflect.TestReflectLogicalTypes",
schema.getNamespace());
Assert.assertEquals("Should have the correct record name",
"DecimalRecordBytes",
Expand Down Expand Up @@ -179,7 +179,7 @@ public int hashCode() {
public void testDecimalFixed() throws IOException {
Schema schema = REFLECT.getSchema(DecimalRecordFixed.class);
Assert.assertEquals("Should have the correct record name",
"org.apache.avro.reflect.TestReflectLogicalTypes$",
"org.apache.avro.reflect.TestReflectLogicalTypes",
schema.getNamespace());
Assert.assertEquals("Should have the correct record name",
"DecimalRecordFixed",
Expand Down Expand Up @@ -298,7 +298,7 @@ public LogicalType fromSchema(Schema schema) {

Schema schema = model.getSchema(PairRecord.class);
Assert.assertEquals("Should have the correct record name",
"org.apache.avro.reflect.TestReflectLogicalTypes$",
"org.apache.avro.reflect.TestReflectLogicalTypes",
schema.getNamespace());
Assert.assertEquals("Should have the correct record name",
"PairRecord",
Expand Down

0 comments on commit 407c48a

Please sign in to comment.