Return-Path: X-Original-To: apmail-avro-commits-archive@www.apache.org Delivered-To: apmail-avro-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 5D8B69A1D for ; Thu, 9 Feb 2012 00:15:45 +0000 (UTC) Received: (qmail 59714 invoked by uid 500); 9 Feb 2012 00:15:45 -0000 Delivered-To: apmail-avro-commits-archive@avro.apache.org Received: (qmail 59661 invoked by uid 500); 9 Feb 2012 00:15:44 -0000 Mailing-List: contact commits-help@avro.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@avro.apache.org Delivered-To: mailing list commits@avro.apache.org Received: (qmail 59654 invoked by uid 99); 9 Feb 2012 00:15:44 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Feb 2012 00:15:44 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Feb 2012 00:15:42 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id E305823888E4 for ; Thu, 9 Feb 2012 00:15:22 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: 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/ Date: Thu, 09 Feb 2012 00:15:22 -0000 To: commits@avro.apache.org From: cutting@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20120209001522.E305823888E4@eris.apache.org> 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 fields = new ArrayList(); 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() {