You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by cu...@apache.org on 2012/02/09 01:15:22 UTC

svn commit: r1242190 - in /avro/trunk: ./ lang/java/avro/src/main/java/org/apache/avro/data/ lang/java/avro/src/main/java/org/apache/avro/generic/ lang/java/avro/src/test/java/org/apache/avro/generic/ lang/java/ipc/src/test/java/org/apache/avro/specific/

Author: cutting
Date: Thu Feb  9 00:15:22 2012
New Revision: 1242190

URL: http://svn.apache.org/viewvc?rev=1242190&view=rev
Log:
AVRO-1007. Java: Enhance builder API's validity checks.

Modified:
    avro/trunk/CHANGES.txt
    avro/trunk/lang/java/avro/src/main/java/org/apache/avro/data/RecordBuilderBase.java
    avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericRecordBuilder.java
    avro/trunk/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericRecordBuilder.java
    avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificErrorBuilder.java
    avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificRecordBuilder.java

Modified: avro/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/avro/trunk/CHANGES.txt?rev=1242190&r1=1242189&r2=1242190&view=diff
==============================================================================
--- avro/trunk/CHANGES.txt (original)
+++ avro/trunk/CHANGES.txt Thu Feb  9 00:15:22 2012
@@ -70,6 +70,9 @@ Avro 1.6.2 (unreleased)
     AVRO-971. Java: Permit IDL imports from classpath in Maven.
     (Victor Chau via cutting)
 
+    AVRO-1007. Java: Enhance builder API's validity checks.
+    (jbaldassari & cutting)
+
   BUG FIXES
 
     AVRO-962. Java: Fix Maven plugin to support string type override.

Modified: avro/trunk/lang/java/avro/src/main/java/org/apache/avro/data/RecordBuilderBase.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/main/java/org/apache/avro/data/RecordBuilderBase.java?rev=1242190&r1=1242189&r2=1242190&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/main/java/org/apache/avro/data/RecordBuilderBase.java (original)
+++ avro/trunk/lang/java/avro/src/main/java/org/apache/avro/data/RecordBuilderBase.java Thu Feb  9 00:15:22 2012
@@ -86,7 +86,7 @@ public abstract class RecordBuilderBase<
    * 1. If the value is not null, or the field type is null, or the field type 
    * is a union which accepts nulls, returns.
    * 2. Else, if the field has a default value, returns.
-   * 3. Otherwise throws NullPointerException 
+   * 3. Otherwise throws AvroRuntimeException. 
    * @param field the field to validate.
    * @param value the value to validate.
    * @throws NullPointerException if value is null and the given field does 
@@ -146,12 +146,14 @@ public abstract class RecordBuilderBase<
    */
   @SuppressWarnings({ "rawtypes", "unchecked" })
   protected Object defaultValue(Field field) throws IOException {    
-    if (field.schema().getType() == Type.NULL) {
-      return null;
-    }
-    
     JsonNode defaultJsonValue = field.defaultValue();
     if (defaultJsonValue == null) {
+      throw new AvroRuntimeException("Field " + field + " not set and has no default value");
+    }
+    if (defaultJsonValue.isNull()
+        && (field.schema().getType() == Type.NULL
+            || (field.schema().getType() == Type.UNION
+                && field.schema().getTypes().get(0).getType() == Type.NULL))) {
       return null;
     }
     

Modified: avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericRecordBuilder.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericRecordBuilder.java?rev=1242190&r1=1242189&r2=1242190&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericRecordBuilder.java (original)
+++ avro/trunk/lang/java/avro/src/main/java/org/apache/avro/generic/GenericRecordBuilder.java Thu Feb  9 00:15:22 2012
@@ -222,7 +222,6 @@ public class GenericRecordBuilder extend
    * If the field has been set, the set value is returned (even if it's null).
    * If the field hasn't been set and has a default value, the default value 
    * is returned.
-   * Otherwise, null is returned.
    * @param field the field whose value should be retrieved.
    * @return the value set for the given field, the field's default value, 
    * or null.

Modified: avro/trunk/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericRecordBuilder.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericRecordBuilder.java?rev=1242190&r1=1242189&r2=1242190&view=diff
==============================================================================
--- avro/trunk/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericRecordBuilder.java (original)
+++ avro/trunk/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericRecordBuilder.java Thu Feb  9 00:15:22 2012
@@ -21,11 +21,13 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 
+import org.apache.avro.AvroRuntimeException;
 import org.apache.avro.Schema;
 import org.apache.avro.Schema.Field;
 import org.apache.avro.Schema.Type;
 import org.apache.avro.generic.GenericData.Record;
 import org.codehaus.jackson.node.TextNode;
+import org.codehaus.jackson.node.NullNode;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -77,12 +79,34 @@ public class TestGenericRecordBuilder {
     new GenericRecordBuilder(recordSchema()).set("intField", null);
   }
   
+  @Test(expected=org.apache.avro.AvroRuntimeException.class)
+  public void buildWithoutSettingRequiredFields1() {
+    new GenericRecordBuilder(recordSchema()).build();
+  }
+  
+  @Test()
+  public void buildWithoutSettingRequiredFields2() {
+    try {
+      new GenericRecordBuilder(recordSchema()).
+      set("anArray", Arrays.asList(new String[] { "one" })).
+      build();
+      Assert.fail("Should have thrown " + 
+          AvroRuntimeException.class.getCanonicalName());
+    } catch (AvroRuntimeException e) {
+      Assert.assertTrue(e.getMessage().contains("intField"));
+    }
+  }
+  
   /** Creates a test record schema */
   private static Schema recordSchema() {
     List<Field> fields = new ArrayList<Field>();
     fields.add(new Field("id", Schema.create(Type.STRING), null, new TextNode("0")));
     fields.add(new Field("intField", Schema.create(Type.INT), null, null));
     fields.add(new Field("anArray", Schema.createArray(Schema.create(Type.STRING)), null, null));
+    fields.add(new Field("optionalInt", Schema.createUnion
+                         (Arrays.asList(Schema.create(Type.NULL),
+                                        Schema.create(Type.INT))),
+                         null, NullNode.getInstance()));
     Schema schema = Schema.createRecord("Foo", "test", "mytest", false);
     schema.setFields(fields);
     return schema;

Modified: avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificErrorBuilder.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificErrorBuilder.java?rev=1242190&r1=1242189&r2=1242190&view=diff
==============================================================================
--- avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificErrorBuilder.java (original)
+++ avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificErrorBuilder.java Thu Feb  9 00:15:22 2012
@@ -49,10 +49,11 @@ public class TestSpecificErrorBuilder {
         TestError.newBuilder(testErrorBuilder));
     Assert.assertEquals(testErrorBuilder, TestError.newBuilder(testError));
     
-    Assert.assertEquals(
-        new TestError("value", new NullPointerException()),
+    TestError error = new TestError("value", new NullPointerException());
+    error.setMessage$("message");
+    Assert.assertEquals(error,
         TestError.newBuilder().setValue("value").
-          setCause(new NullPointerException()).build());
+          setCause(new NullPointerException()).setMessage$("message").build());
     
     // Test clear
     testErrorBuilder.clearValue();

Modified: avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificRecordBuilder.java
URL: http://svn.apache.org/viewvc/avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificRecordBuilder.java?rev=1242190&r1=1242189&r2=1242190&view=diff
==============================================================================
--- avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificRecordBuilder.java (original)
+++ avro/trunk/lang/java/ipc/src/test/java/org/apache/avro/specific/TestSpecificRecordBuilder.java Thu Feb  9 00:15:22 2012
@@ -25,6 +25,7 @@ import java.util.List;
 
 import junit.framework.Assert;
 
+import org.apache.avro.AvroRuntimeException;
 import org.apache.avro.Foo;
 import org.apache.avro.Interop;
 import org.apache.avro.Kind;
@@ -119,6 +120,7 @@ public class TestSpecificRecordBuilder {
   @Test
   public void testInterop() {
     Interop interop = Interop.newBuilder()
+        .setNullField(null)
         .setArrayField(Arrays.asList(new Double[] { 3.14159265, 6.022 }))
         .setBoolField(true)
         .setBytesField(ByteBuffer.allocate(4).put(new byte[] { 3, 2, 1, 0 }))
@@ -157,7 +159,36 @@ public class TestSpecificRecordBuilder {
   public void attemptToSetNonNullableFieldToNull() {
     Person.newBuilder().setName(null);
   }
-  
+
+  @Test(expected=org.apache.avro.AvroRuntimeException.class)
+  public void buildWithoutSettingRequiredFields1() {
+    Person.newBuilder().build();
+  }
+
+  @Test
+  public void buildWithoutSettingRequiredFields2() {
+    // Omit required non-primitive field
+    try {
+      Person.newBuilder().setYearOfBirth(1900).setState("MA").build();
+      Assert.fail("Should have thrown " + AvroRuntimeException.class.getCanonicalName());
+    } catch (AvroRuntimeException e) {
+      // Exception should mention that the 'name' field has not been set
+      Assert.assertTrue(e.getMessage().contains("name"));
+    }
+  }
+
+  @Test
+  public void buildWithoutSettingRequiredFields3() {
+    // Omit required primitive field
+    try {
+      Person.newBuilder().setName("Anon").setState("CA").build();
+      Assert.fail("Should have thrown " + AvroRuntimeException.class.getCanonicalName());
+    } catch (AvroRuntimeException e) {
+      // Exception should mention that the 'year_of_birth' field has not been set
+      Assert.assertTrue(e.getMessage().contains("year_of_birth"));
+    }
+  }
+
   @Ignore
   @Test
   public void testBuilderPerformance() {