avro-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nielsbas...@apache.org
Subject avro git commit: AVRO-1967: Java: Fix NPE when calling getXyzBuilder on instance where the xyz is null
Date Sat, 17 Dec 2016 11:03:23 GMT
Repository: avro
Updated Branches:
  refs/heads/master a53a4fd10 -> 152fa0954


AVRO-1967: Java: Fix NPE when calling getXyzBuilder on instance where the xyz is null


Project: http://git-wip-us.apache.org/repos/asf/avro/repo
Commit: http://git-wip-us.apache.org/repos/asf/avro/commit/152fa095
Tree: http://git-wip-us.apache.org/repos/asf/avro/tree/152fa095
Diff: http://git-wip-us.apache.org/repos/asf/avro/diff/152fa095

Branch: refs/heads/master
Commit: 152fa0954bb74e8e1e627751ccab55ada4f5b375
Parents: a53a4fd
Author: Niels Basjes <nbasjes@bol.com>
Authored: Thu Dec 1 13:44:57 2016 +0100
Committer: Niels Basjes <nielsbasjes@apache.org>
Committed: Sat Dec 17 11:57:56 2016 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |  3 +
 .../specific/templates/java/classic/record.vm   | 14 +++-
 .../avro/specific/TestSpecificBuilderTree.java  | 77 ++++++++++++++++++++
 .../avro/examples/baseball/Player.java          | 14 +++-
 .../tools/src/test/compiler/output/Player.java  | 14 +++-
 5 files changed, 113 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/avro/blob/152fa095/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index fa11bb8..d5341e8 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -106,6 +106,9 @@ Trunk (not yet released)
 
     AVRO-1966: Java: Fix NPE When copying builder with nullable record. (Niels Basjes)
 
+    AVRO-1967: Java: Fix NPE when calling getXyzBuilder on instance where the xyz is null
+    (Niels Basjes)
+
 Avro 1.8.1 (14 May 2016)
 
   INCOMPATIBLE CHANGES

http://git-wip-us.apache.org/repos/asf/avro/blob/152fa095/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
----------------------------------------------------------------------
diff --git a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
index c333dd0..ccec4b6 100644
--- a/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
+++ b/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
@@ -221,7 +221,11 @@ public class ${this.mangle($schema.getName())}#if ($schema.isError())
extends or
    * @return A new ${this.mangle($schema.getName())} RecordBuilder
    */
   public static #if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder
newBuilder(#if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder
other) {
-    return new #if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder(other);
+    if (other == null) {
+      return new #if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder();
+    } else {
+      return new #if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder(other);
+    }
   }
 
   /**
@@ -230,7 +234,11 @@ public class ${this.mangle($schema.getName())}#if ($schema.isError())
extends or
    * @return A new ${this.mangle($schema.getName())} RecordBuilder
    */
   public static #if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder
newBuilder(#if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}
other) {
-    return new #if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder(other);
+    if (other == null) {
+      return new #if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder();
+    } else {
+      return new #if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder(other);
+    }
   }
 
   /**
@@ -279,7 +287,7 @@ public class ${this.mangle($schema.getName())}#if ($schema.isError())
extends or
      * @param other The existing instance to copy.
      */
     private Builder(#if ($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}
other) {
-      #if ($schema.isError())super(other)#else
+#if ($schema.isError())      super(other)#else
       super(SCHEMA$)#end;
 #foreach ($field in $schema.getFields())
       if (isValidValue(fields()[$field.pos()], other.${this.mangle($field.name(), $schema.isError())}))
{

http://git-wip-us.apache.org/repos/asf/avro/blob/152fa095/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificBuilderTree.java
----------------------------------------------------------------------
diff --git a/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificBuilderTree.java
b/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificBuilderTree.java
index 87b9e6f..6a69833 100644
--- a/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificBuilderTree.java
+++ b/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificBuilderTree.java
@@ -27,6 +27,7 @@ import java.util.ArrayList;
 import static org.apache.avro.test.nullable.Nullable.*;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 public class TestSpecificBuilderTree {
@@ -283,4 +284,80 @@ public class TestSpecificBuilderTree {
     builderCopy.getNullableRecordBuilder();
   }
 
+  @Test
+  public void copyBuilderWithNullablesAndSetToNull() {
+    // Create builder with all values default to null, yet unset.
+    RecordWithNullables.Builder builder = RecordWithNullables.newBuilder();
+
+    // Ensure all values have not been set
+    assertFalse(builder.hasNullableRecordBuilder());
+    assertFalse(builder.hasNullableRecord());
+    assertFalse(builder.hasNullableString());
+    assertFalse(builder.hasNullableLong  ());
+    assertFalse(builder.hasNullableInt   ());
+    assertFalse(builder.hasNullableMap   ());
+    assertFalse(builder.hasNullableArray ());
+
+    // Set all values to null
+    builder.setNullableRecordBuilder(null);
+    builder.setNullableRecord(null);
+    builder.setNullableString(null);
+    builder.setNullableLong  (null);
+    builder.setNullableInt   (null);
+    builder.setNullableMap   (null);
+    builder.setNullableArray (null);
+
+    // A Builder remains False because it is null
+    assertFalse(builder.hasNullableRecordBuilder());
+
+    // Ensure all values have been set
+    assertTrue(builder.hasNullableRecord());
+    assertTrue(builder.hasNullableString());
+    assertTrue(builder.hasNullableLong  ());
+    assertTrue(builder.hasNullableInt   ());
+    assertTrue(builder.hasNullableMap   ());
+    assertTrue(builder.hasNullableArray ());
+
+    // Implicitly create a builder instance and clear the actual value.
+    builder.getNullableRecordBuilder();
+    assertTrue(builder.hasNullableRecordBuilder());
+    assertFalse(builder.hasNullableRecord());
+
+    // Create a copy of this builder.
+    RecordWithNullables.Builder builderCopy = RecordWithNullables.newBuilder(builder);
+
+    // Ensure all values are still the same
+    assertTrue(builder.hasNullableRecordBuilder());
+    assertFalse(builder.hasNullableRecord());
+    assertTrue(builder.hasNullableString());
+    assertTrue(builder.hasNullableLong  ());
+    assertTrue(builder.hasNullableInt   ());
+    assertTrue(builder.hasNullableMap   ());
+    assertTrue(builder.hasNullableArray ());
+  }
+
+  @Test
+  public void getBuilderForRecordWithNullRecord() {
+    // Create a record with all nullable fields set to the default value : null
+    RecordWithNullables recordWithNullables = RecordWithNullables.newBuilder().build();
+
+    // Now create a Builder using this record as the base
+    RecordWithNullables.Builder builder = RecordWithNullables.newBuilder(recordWithNullables);
+
+    // In the past this caused an NPE
+    builder.getNullableRecordBuilder();
+  }
+
+  @Test
+  public void getBuilderForNullRecord() {
+    // In the past this caused an NPE
+    RecordWithNullables.newBuilder((RecordWithNullables)null);
+  }
+
+  @Test
+  public void getBuilderForNullBuilder() {
+    // In the past this caused an NPE
+    RecordWithNullables.newBuilder((RecordWithNullables.Builder)null);
+  }
+
 }

http://git-wip-us.apache.org/repos/asf/avro/blob/152fa095/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
----------------------------------------------------------------------
diff --git a/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
b/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
index da58e20..4dff5ef 100644
--- a/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
+++ b/lang/java/tools/src/test/compiler/output-string/avro/examples/baseball/Player.java
@@ -182,7 +182,11 @@ public class Player extends org.apache.avro.specific.SpecificRecordBase
implemen
    * @return A new Player RecordBuilder
    */
   public static avro.examples.baseball.Player.Builder newBuilder(avro.examples.baseball.Player.Builder
other) {
-    return new avro.examples.baseball.Player.Builder(other);
+    if (other == null) {
+      return new avro.examples.baseball.Player.Builder();
+    } else {
+      return new avro.examples.baseball.Player.Builder(other);
+    }
   }
 
   /**
@@ -191,7 +195,11 @@ public class Player extends org.apache.avro.specific.SpecificRecordBase
implemen
    * @return A new Player RecordBuilder
    */
   public static avro.examples.baseball.Player.Builder newBuilder(avro.examples.baseball.Player
other) {
-    return new avro.examples.baseball.Player.Builder(other);
+    if (other == null) {
+      return new avro.examples.baseball.Player.Builder();
+    } else {
+      return new avro.examples.baseball.Player.Builder(other);
+    }
   }
 
   /**
@@ -240,7 +248,7 @@ public class Player extends org.apache.avro.specific.SpecificRecordBase
implemen
      * @param other The existing instance to copy.
      */
     private Builder(avro.examples.baseball.Player other) {
-            super(SCHEMA$);
+      super(SCHEMA$);
       if (isValidValue(fields()[0], other.number)) {
         this.number = data().deepCopy(fields()[0].schema(), other.number);
         fieldSetFlags()[0] = true;

http://git-wip-us.apache.org/repos/asf/avro/blob/152fa095/lang/java/tools/src/test/compiler/output/Player.java
----------------------------------------------------------------------
diff --git a/lang/java/tools/src/test/compiler/output/Player.java b/lang/java/tools/src/test/compiler/output/Player.java
index 0a0b882..26fcbc0 100644
--- a/lang/java/tools/src/test/compiler/output/Player.java
+++ b/lang/java/tools/src/test/compiler/output/Player.java
@@ -182,7 +182,11 @@ public class Player extends org.apache.avro.specific.SpecificRecordBase
implemen
    * @return A new Player RecordBuilder
    */
   public static avro.examples.baseball.Player.Builder newBuilder(avro.examples.baseball.Player.Builder
other) {
-    return new avro.examples.baseball.Player.Builder(other);
+    if (other == null) {
+      return new avro.examples.baseball.Player.Builder();
+    } else {
+      return new avro.examples.baseball.Player.Builder(other);
+    }
   }
 
   /**
@@ -191,7 +195,11 @@ public class Player extends org.apache.avro.specific.SpecificRecordBase
implemen
    * @return A new Player RecordBuilder
    */
   public static avro.examples.baseball.Player.Builder newBuilder(avro.examples.baseball.Player
other) {
-    return new avro.examples.baseball.Player.Builder(other);
+    if (other == null) {
+      return new avro.examples.baseball.Player.Builder();
+    } else {
+      return new avro.examples.baseball.Player.Builder(other);
+    }
   }
 
   /**
@@ -240,7 +248,7 @@ public class Player extends org.apache.avro.specific.SpecificRecordBase
implemen
      * @param other The existing instance to copy.
      */
     private Builder(avro.examples.baseball.Player other) {
-            super(SCHEMA$);
+      super(SCHEMA$);
       if (isValidValue(fields()[0], other.number)) {
         this.number = data().deepCopy(fields()[0].schema(), other.number);
         fieldSetFlags()[0] = true;


Mime
View raw message