svn commit: r1641896 - in /avro/trunk: CHANGES.txt lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectAllowNulls.java

[cu...@apache.org]

Author: cutting
Date: Wed Nov 26 19:33:41 2014
New Revision: 1641896

URL: http://svn.apache.org/r1641896
Log:
AVRO-1604. Java: Fix ReflectData.AllowNull to work with @Nullable annotations. Contributed by Ryan Blue.

Modified:
    avro/trunk/CHANGES.txt
    avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
    avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectAllowNulls.java

Modified: avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/trunk/CHANGES.txt?rev=1641896&r1=1641895&r2=1641896&view=diff
==============================================================================
--- avro/trunk/CHANGES.txt (original)
+++ avro/trunk/CHANGES.txt Wed Nov 26 19:33:41 2014
@@ -84,6 +84,9 @@ Trunk (not yet released)
     AVRO-1407: Java: Fix infinite loop on slow connect in NettyTransceiver.
     (Gareth Davis via cutting)
 
+    AVRO-1604. Java: Fix ReflectData.AllowNull to work with @Nullable
+    annotations. (Ryan Blue via cutting)
+
 Avro 1.7.7 (23 July 2014)
 
   NEW FEATURES

Modified: avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java?rev=1641896&r1=1641895&r2=1641896&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java (original)
+++ avro/trunk/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectData.java Wed Nov 26 19:33:41 2014
@@ -546,8 +546,23 @@ public class ReflectData extends Specifi
 
   /** Create and return a union of the null schema and the provided schema. */
   public static Schema makeNullable(Schema schema) {
-    return Schema.createUnion(Arrays.asList(Schema.create(Schema.Type.NULL),
-                                            schema));
+    if (schema.getType() == Schema.Type.UNION) {
+      // check to see if the union already contains NULL
+      for (Schema subType : schema.getTypes()) {
+        if (subType.getType() == Schema.Type.NULL) {
+          return schema;
+        }
+      }
+      // add null as the first type in a new union
+      List<Schema> withNull = new ArrayList<Schema>();
+      withNull.add(Schema.create(Schema.Type.NULL));
+      withNull.addAll(schema.getTypes());
+      return Schema.createUnion(withNull);
+    } else {
+      // create a union with null
+      return Schema.createUnion(Arrays.asList(Schema.create(Schema.Type.NULL),
+          schema));
+    }
   }
 
   private static final Map<Class<?>,Field[]> FIELDS_CACHE =

Modified: avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectAllowNulls.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectAllowNulls.java?rev=1641896&r1=1641895&r2=1641896&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectAllowNulls.java (original)
+++ avro/trunk/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectAllowNulls.java Wed Nov 26 19:33:41 2014
@@ -45,6 +45,25 @@ public class TestReflectAllowNulls {
     Primitives anObject;
   }
 
+  private static class AllowNullWithNullable {
+    @Nullable
+    Double aDouble;
+
+    @AvroSchema("[\"double\", \"long\"]")
+    Object doubleOrLong;
+
+    @Nullable
+    @AvroSchema("[\"double\", \"long\"]")
+    Object doubleOrLongOrNull1;
+
+    @AvroSchema("[\"double\", \"long\", \"null\"]")
+    Object doubleOrLongOrNull2;
+
+    @Nullable
+    @AvroSchema("[\"double\", \"long\", \"null\"]")
+    Object doubleOrLongOrNull3;
+  }
+
   @Test
   public void testPrimitives() {
     // AllowNull only makes fields nullable, so testing must use a base record
@@ -87,6 +106,42 @@ public class TestReflectAllowNulls {
         wrappers.getField("anObject").schema());
   }
 
+  @Test
+  public void testAllowNullWithNullableAnnotation() {
+    Schema withNullable = ReflectData.AllowNull.get()
+        .getSchema(AllowNullWithNullable.class);
+
+    Assert.assertEquals("Should produce a nullable double",
+        nullableSchema(double.class),
+        withNullable.getField("aDouble").schema());
+
+    Schema nullableDoubleOrLong = Schema.createUnion(Arrays.asList(
+            Schema.create(Schema.Type.NULL),
+            Schema.create(Schema.Type.DOUBLE),
+            Schema.create(Schema.Type.LONG)));
+
+    Assert.assertEquals("Should add null to a non-null union",
+        nullableDoubleOrLong,
+        withNullable.getField("doubleOrLong").schema());
+
+    Assert.assertEquals("Should add null to a non-null union",
+        nullableDoubleOrLong,
+        withNullable.getField("doubleOrLongOrNull1").schema());
+
+    Schema doubleOrLongOrNull = Schema.createUnion(Arrays.asList(
+        Schema.create(Schema.Type.DOUBLE),
+        Schema.create(Schema.Type.LONG),
+        Schema.create(Schema.Type.NULL)));
+
+    Assert.assertEquals("Should add null to a non-null union",
+        doubleOrLongOrNull,
+        withNullable.getField("doubleOrLongOrNull2").schema());
+
+    Assert.assertEquals("Should add null to a non-null union",
+        doubleOrLongOrNull,
+        withNullable.getField("doubleOrLongOrNull3").schema());
+  }
+
   private Schema requiredSchema(Class<?> type) {
     return ReflectData.get().getSchema(type);
   }