[ 
https://issues.apache.org/jira/browse/AVRO-1961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16693717#comment-16693717
 ] 

ASF GitHub Bot commented on AVRO-1961:
--------------------------------------

dkulp closed pull request #169: Branch 1.8 - AVRO-1961 : [JAVA] Generate 
getters that return an Optional
URL: https://github.com/apache/avro/pull/169
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/lang/java/avro/src/main/java/org/apache/avro/Schema.java 
b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
index 2019c1f7d..7a962a09d 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/Schema.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/Schema.java
@@ -371,6 +371,26 @@ final boolean equalCachedHash(Schema other) {
                        "default","doc","name","order","type","aliases");
   }
 
+  /** Returns true if this record is an union type. */
+  public boolean isUnion(){
+    return this instanceof UnionSchema;
+  }
+
+  /** Returns true if this record is an union type containing null. */
+  public boolean isNullable() {
+    if (!isUnion()) {
+      return getType().equals(Schema.Type.NULL);
+    }
+
+    for (Schema schema : getTypes()) {
+      if (schema.isNullable()) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
   /** A field within a record. */
   public static class Field extends JsonProperties {
 
diff --git a/lang/java/avro/src/test/java/org/apache/avro/TestSchema.java 
b/lang/java/avro/src/test/java/org/apache/avro/TestSchema.java
index 242ee8ca0..f75b0ed5f 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/TestSchema.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/TestSchema.java
@@ -18,11 +18,13 @@
 package org.apache.avro;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import org.apache.avro.Schema.Field;
@@ -33,35 +35,36 @@
   @Test
   public void testSplitSchemaBuild() {
     Schema s = SchemaBuilder
-       .record("HandshakeRequest")
-       .namespace("org.apache.avro.ipc").fields()
-         .name("clientProtocol").type().optional().stringType()
-         .name("meta").type().optional().map().values().bytesType()
-         .endRecord();
+        .record("HandshakeRequest")
+        .namespace("org.apache.avro.ipc").fields()
+        .name("clientProtocol").type().optional().stringType()
+        .name("meta").type().optional().map().values().bytesType()
+        .endRecord();
 
     String schemaString = s.toString();
-    final int mid = schemaString.length() / 2;
+    int mid = schemaString.length() / 2;
 
     Schema parsedStringSchema = new 
org.apache.avro.Schema.Parser().parse(s.toString());
     Schema parsedArrayOfStringSchema =
-      new org.apache.avro.Schema.Parser().parse
-      (schemaString.substring(0, mid), schemaString.substring(mid));
+        new org.apache.avro.Schema.Parser().parse
+            (schemaString.substring(0, mid), schemaString.substring(mid));
     assertNotNull(parsedStringSchema);
     assertNotNull(parsedArrayOfStringSchema);
     assertEquals(parsedStringSchema.toString(), 
parsedArrayOfStringSchema.toString());
   }
 
   @Test
-  public void testDuplicateRecordFieldName() {
-    final Schema schema = Schema.createRecord("RecordName", null, null, false);
-    final List<Field> fields = new ArrayList<Field>();
+  public void testDefaultRecordWithDuplicateFieldName() {
+    String recordName = "name";
+    Schema schema = Schema.createRecord(recordName, "doc", "namespace", false);
+    List<Field> fields = new ArrayList<Field>();
     fields.add(new Field("field_name", Schema.create(Type.NULL), null, null));
     fields.add(new Field("field_name", Schema.create(Type.INT), null, null));
     try {
       schema.setFields(fields);
       fail("Should not be able to create a record with duplicate field name.");
     } catch (AvroRuntimeException are) {
-      assertTrue(are.getMessage().contains("Duplicate field field_name in 
record RecordName"));
+      assertTrue(are.getMessage().contains("Duplicate field field_name in 
record " + recordName));
     }
   }
 
@@ -76,9 +79,23 @@ public void testCreateUnionVarargs() {
     assertEquals(expected, schema);
   }
 
+  @Test
+  public void testRecordWithNullDoc() {
+    Schema schema = Schema.createRecord("name", null, "namespace", false);
+    String schemaString = schema.toString();
+    assertNotNull(schemaString);
+  }
+
+  @Test
+  public void testRecordWithNullNamespace() {
+    Schema schema = Schema.createRecord("name", "doc", null, false);
+    String schemaString = schema.toString();
+    assertNotNull(schemaString);
+  }
+
   @Test
   public void testEmptyRecordSchema() {
-    Schema schema = Schema.createRecord("foobar", null, null, false);
+    Schema schema = createDefaultRecord();
     String schemaString = schema.toString();
     assertNotNull(schemaString);
   }
@@ -88,7 +105,8 @@ public void testSchemaWithFields() {
     List<Field> fields = new ArrayList<Field>();
     fields.add(new Field("field_name1", Schema.create(Type.NULL), null, null));
     fields.add(new Field("field_name2", Schema.create(Type.INT), null, null));
-    Schema schema = Schema.createRecord("foobar", null, null, false, fields);
+    Schema schema = createDefaultRecord();
+    schema.setFields(fields);
     String schemaString = schema.toString();
     assertNotNull(schemaString);
     assertEquals(2, schema.getFields().size());
@@ -96,8 +114,71 @@ public void testSchemaWithFields() {
 
   @Test(expected = NullPointerException.class)
   public void testSchemaWithNullFields() {
-    Schema.createRecord("foobar", null, null, false, null);
+    Schema.createRecord("name", "doc", "namespace", false, null);
+  }
+
+  @Test
+  public void testIsUnionOnUnionWithMultipleElements() {
+    Schema schema = Schema.createUnion(Schema.create(Type.NULL), 
Schema.create(Type.LONG));
+    assertTrue(schema.isUnion());
+  }
+
+  @Test
+  public void testIsUnionOnUnionWithOneElement() {
+    Schema schema = Schema.createUnion(Schema.create(Type.LONG));
+    assertTrue(schema.isUnion());
+  }
+
+  @Test
+  public void testIsUnionOnRecord() {
+    Schema schema = createDefaultRecord();
+    assertFalse(schema.isUnion());
+  }
+
+  @Test
+  public void testIsUnionOnArray() {
+    Schema schema = Schema.createArray(Schema.create(Type.LONG));
+    assertFalse(schema.isUnion());
+  }
+
+  @Test
+  public void testIsUnionOnEnum() {
+    Schema schema = Schema.createEnum("name", "doc", "namespace", 
Collections.singletonList("value"));
+    assertFalse(schema.isUnion());
+  }
+
+  @Test
+  public void testIsUnionOnFixed() {
+    Schema schema = Schema.createFixed("name", "doc", "space", 10);
+    assertFalse(schema.isUnion());
+  }
+
+  @Test
+  public void testIsUnionOnMap() {
+    Schema schema = Schema.createMap(Schema.create(Type.LONG));
+    assertFalse(schema.isUnion());
   }
 
+  @Test
+  public void testIsNullableOnUnionWithNull() {
+    Schema schema = Schema.createUnion(Schema.create(Type.NULL), 
Schema.create(Type.LONG));
+    assertTrue(schema.isNullable());
+  }
+
+  @Test
+  public void testIsNullableOnUnionWithoutNull() {
+    Schema schema = Schema.createUnion(Schema.create(Type.LONG));
+    assertFalse(schema.isNullable());
+  }
+
+  @Test
+  public void testIsNullableOnRecord() {
+    Schema schema = createDefaultRecord();
+    assertFalse(schema.isNullable());
+  }
+
+  private Schema createDefaultRecord() {
+    return Schema.createRecord("name", "doc", "namespace", false);
+  }
 
 }
diff --git 
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java
 
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java
index 295949313..8158972f9 100644
--- 
a/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java
+++ 
b/lang/java/compiler/src/main/java/org/apache/avro/compiler/specific/SpecificCompiler.java
@@ -106,6 +106,7 @@
   private String templateDir;
   private FieldVisibility fieldVisibility = FieldVisibility.PUBLIC_DEPRECATED;
   private boolean createSetters = true;
+  private boolean pojoWithOptional = false;
   private boolean createAllArgsConstructor = true;
   private String outputCharacterEncoding;
   private boolean enableDecimalLogicalType = false;
@@ -206,6 +207,9 @@ public void setFieldVisibility(FieldVisibility 
fieldVisibility) {
     this.fieldVisibility = fieldVisibility;
   }
 
+  /**
+   * @return true if the record fields should have setter methods.
+   */
   public boolean isCreateSetters() {
       return this.createSetters;
   }
@@ -217,6 +221,20 @@ public void setCreateSetters(boolean createSetters) {
     this.createSetters = createSetters;
   }
 
+  /**
+   * @return true if the record nullable fields should have 
getter/setter/builder methods with Optional.
+   */
+  public boolean isPojoWithOptional() {
+    return this.pojoWithOptional;
+  }
+
+  /**
+   * Set to false to not create getter/setter methods with Optional for 
nullable fields of the record.
+   */
+  public void setPojoWithOptional(boolean pojoWithOptional) {
+    this.pojoWithOptional = pojoWithOptional;
+  }
+
   /**
    * Set to true to use {@link java.math.BigDecimal} instead of
    * {@link java.nio.ByteBuffer} for logical type "decimal"
@@ -585,6 +603,16 @@ public String javaType(Schema schema) {
     return javaType(schema, true);
   }
 
+  /** Utility for template use.  Returns true if any field of the current 
schema is nullable. */
+  public boolean containsNullableField(Schema schema) {
+    for (Field field : schema.getFields()) {
+      if (field.schema().isNullable()) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   private String javaType(Schema schema, boolean checkConvertedLogicalType) {
     if (checkConvertedLogicalType) {
       String convertedLogicalType = getConvertedLogicalType(schema);
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 85c5e9d2f..ecd09ecde 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
@@ -25,6 +25,7 @@ import org.apache.avro.message.BinaryMessageEncoder;
 import org.apache.avro.message.BinaryMessageDecoder;
 import org.apache.avro.message.SchemaStore;
 #end
+#if (${this.pojoWithOptional} && ${this.containsNullableField($schema)})import 
java.util.Optional;#end
 
 @SuppressWarnings("all")
 #if ($schema.getDoc())
@@ -184,6 +185,17 @@ public class ${this.mangle($schema.getName())}#if 
($schema.isError()) extends or
   }
 
 #foreach ($field in $schema.getFields())
+#if (${this.pojoWithOptional} && ${field.schema().isNullable()})
+  /**
+   * Gets the value of the nullable '${this.mangle($field.name(), 
$schema.isError())}' field.
+#if ($field.doc())      * $field.doc()
+#end
+   * @return The value of the '${this.mangle($field.name(), 
$schema.isError())}' field.
+   */
+  public Optional<${this.javaType($field.schema())}> 
${this.generateGetMethod($schema, $field)}() {
+    return Optional.ofNullable(${this.mangle($field.name(), 
$schema.isError())});
+  }
+#else
   /**
    * Gets the value of the '${this.mangle($field.name(), $schema.isError())}' 
field.
 #if ($field.doc())   * @return $field.doc()
@@ -193,8 +205,20 @@ public class ${this.mangle($schema.getName())}#if 
($schema.isError()) extends or
   public ${this.javaType($field.schema())} ${this.generateGetMethod($schema, 
$field)}() {
     return ${this.mangle($field.name(), $schema.isError())};
   }
+#end
 
 #if ($this.createSetters)
+#if (${this.pojoWithOptional} && ${field.schema().isNullable()})
+/**
+* Sets the value of the nullable '${this.mangle($field.name(), 
$schema.isError())}' field.
+    #if ($field.doc())   * $field.doc()
+    #end
+* @param value the value to set.
+*/
+public void ${this.generateSetMethod($schema, 
$field)}(Optional<${this.javaType($field.schema())}> value) {
+this.${this.mangle($field.name(), $schema.isError())} = value.orElse(null);
+}
+#else
   /**
    * Sets the value of the '${this.mangle($field.name(), $schema.isError())}' 
field.
 #if ($field.doc())   * $field.doc()
@@ -205,6 +229,7 @@ public class ${this.mangle($schema.getName())}#if 
($schema.isError()) extends or
     this.${this.mangle($field.name(), $schema.isError())} = value;
   }
 #end
+#end
 
 #end
   /**
@@ -319,10 +344,41 @@ public class ${this.mangle($schema.getName())}#if 
($schema.isError()) extends or
 #end
 
 #foreach ($field in $schema.getFields())
+#if (${this.pojoWithOptional} && ${field.schema().isNullable()})
+
+    /**
+      * Gets the value of the nullable '${this.mangle($field.name(), 
$schema.isError())}' field.
+        #if ($field.doc())      * $field.doc()
+        #end
+      * @return The value.
+      */
+    public Optional<${this.javaType($field.schema())}> 
${this.generateGetMethod($schema, $field)}() {
+      return Optional.ofNullable(${this.mangle($field.name(), 
$schema.isError())});
+    }
+
+    /**
+      * Sets the value of the nullable '${this.mangle($field.name(), 
$schema.isError())}' field.
+        #if ($field.doc())      * $field.doc()
+        #end
+      * @param value The value of '${this.mangle($field.name(), 
$schema.isError())}'.
+      * @return This builder.
+      */
+    public #if 
($schema.getNamespace())$schema.getNamespace().#end${this.mangle($schema.getName())}.Builder
 ${this.generateSetMethod($schema, 
$field)}(Optional<${this.javaType($field.schema())}> value) {
+    ${this.javaType($field.schema())} effectiveValue = value.orElse(null);
+    validate(fields()[$field.pos()], effectiveValue);
+        #if (${this.hasBuilder($field.schema())})
+        this.${this.mangle($field.name(), $schema.isError())}Builder = null;
+        #end
+    this.${this.mangle($field.name(), $schema.isError())} = effectiveValue;
+    fieldSetFlags()[$field.pos()] = true;
+    return this;
+    }
+#else
+
     /**
       * Gets the value of the '${this.mangle($field.name(), 
$schema.isError())}' field.
-#if ($field.doc())      * $field.doc()
-#end
+        #if ($field.doc())      * $field.doc()
+        #end
       * @return The value.
       */
     public ${this.javaType($field.schema())} ${this.generateGetMethod($schema, 
$field)}() {
@@ -345,6 +401,7 @@ public class ${this.mangle($schema.getName())}#if 
($schema.isError()) extends or
       fieldSetFlags()[$field.pos()] = true;
       return this;
     }
+#end
 
     /**
       * Checks whether the '${this.mangle($field.name(), $schema.isError())}' 
field has been set.
diff --git 
a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
 
b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
index 07328d300..94aac9f86 100644
--- 
a/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
+++ 
b/lang/java/compiler/src/test/java/org/apache/avro/compiler/specific/TestSpecificCompiler.java
@@ -532,7 +532,65 @@ public void 
testConversionInstanceWithDecimalLogicalTypeEnabled() throws Excepti
         "null", compiler.conversionInstance(uuidSchema));
   }
 
-  public void testToFromByteBuffer() {
+  @Test
+  public void testPojoWithOptionalTurnedOffByDefault() throws IOException {
+    SpecificCompiler compiler = createCompiler();
+    assertFalse(compiler.isPojoWithOptional());
+    compiler.compileToDestination(this.src, this.outputDir);
+    assertTrue(this.outputFile.exists());
+    BufferedReader reader = null;
+    try {
+      reader = new BufferedReader(new FileReader(this.outputFile));
+      String line;
+      while ((line = reader.readLine()) != null) {
+        line = line.trim();
+        assertFalse(line.contains("Optional"));
+      }
+    } finally {
+      if (reader != null)
+        reader.close();
+    }
+  }
 
+  @Test
+  public void testPojoWithOptionalCreatedWhenOptionTurnedOn() throws 
IOException {
+    SpecificCompiler compiler = createCompiler();
+    compiler.setPojoWithOptional(true);
+    assertTrue(compiler.isPojoWithOptional());
+    compiler.compileToDestination(this.src, this.outputDir);
+    assertTrue(this.outputFile.exists());
+    int optionalFound = 0;
+    BufferedReader reader = null;
+    try {
+      reader = new BufferedReader(new FileReader(this.outputFile));
+
+      String line;
+      while ((line = reader.readLine()) != null) {
+        line = line.trim();
+        if (line.contains("Optional")) {
+          optionalFound++;
+        }
+      }
+    } finally {
+      if (reader != null)
+        reader.close();
+    }
+    assertEquals(7, optionalFound);
+  }
+
+  @Test
+  public void testContainsNullableFieldWorksOnRecordWithoutField(){
+    Schema recordWithoutFieldSchema = 
SchemaBuilder.builder().record("recordWithoutField").fields().endRecord();
+    assertFalse( new 
SpecificCompiler().containsNullableField(recordWithoutFieldSchema));
+  }
+
+  @Test
+  public void testContainsNullableFieldWorksOnSchemaWithoutNullableField(){
+    Schema recordWithoutNullableFieldSchema = SchemaBuilder.builder()
+        .record("recordWithoutNullableField")
+        .fields()
+        .name("value").type().intType().noDefault()
+        .endRecord();
+    assertFalse( new 
SpecificCompiler().containsNullableField(recordWithoutNullableFieldSchema));
   }
 }
diff --git a/lang/java/compiler/src/test/resources/simple_record.avsc 
b/lang/java/compiler/src/test/resources/simple_record.avsc
index 85781c5c9..d78fd17e6 100644
--- a/lang/java/compiler/src/test/resources/simple_record.avsc
+++ b/lang/java/compiler/src/test/resources/simple_record.avsc
@@ -2,6 +2,7 @@
   "type": "record", 
   "name": "SimpleRecord",
   "fields" : [
-    {"name": "value", "type": "int"}
+    {"name": "value", "type": "int"},
+    {"name": "nullableValue", "type": ["null","int"], "doc" : "doc"}
   ]
 }
\ No newline at end of file
diff --git 
a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/AbstractAvroMojo.java
 
b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/AbstractAvroMojo.java
index 9d78ede91..191598d37 100644
--- 
a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/AbstractAvroMojo.java
+++ 
b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/AbstractAvroMojo.java
@@ -122,6 +122,14 @@
    */
   protected boolean createSetters;
 
+  /**
+   * Determines whether or not to have getter/setter/builder methods with 
Optional for nullable fields of the record.
+   * The default is not to have getter/setter/builder methods with Optional.
+   *
+   * @parameter default-value="false"
+   */
+  protected boolean pojoWithOptional;
+
   /**
    * Determines whether or not to use Java classes for decimal types
    *
diff --git 
a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/IDLProtocolMojo.java
 
b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/IDLProtocolMojo.java
index 3f61ac2a2..63221ea6e 100644
--- 
a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/IDLProtocolMojo.java
+++ 
b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/IDLProtocolMojo.java
@@ -93,6 +93,7 @@ protected void doCompile(String filename, File 
sourceDirectory, File outputDirec
       compiler.setTemplateDir(templateDirectory);
       compiler.setFieldVisibility(getFieldVisibility());
       compiler.setCreateSetters(createSetters);
+      compiler.setPojoWithOptional(pojoWithOptional);
       compiler.setEnableDecimalLogicalType(enableDecimalLogicalType);
       compiler.compileToDestination(null, outputDirectory);
     } catch (ParseException e) {
diff --git 
a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/ProtocolMojo.java 
b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/ProtocolMojo.java
index a30a08245..cf55597a5 100644
--- 
a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/ProtocolMojo.java
+++ 
b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/ProtocolMojo.java
@@ -61,6 +61,7 @@ protected void doCompile(String filename, File 
sourceDirectory, File outputDirec
     compiler.setStringType(StringType.valueOf(stringType));
     compiler.setFieldVisibility(getFieldVisibility());
     compiler.setCreateSetters(createSetters);
+    compiler.setPojoWithOptional(pojoWithOptional);
     compiler.setEnableDecimalLogicalType(enableDecimalLogicalType);
     compiler.compileToDestination(src, outputDirectory);
   }
diff --git 
a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/SchemaMojo.java 
b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/SchemaMojo.java
index 9a849207f..6557b7229 100644
--- a/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/SchemaMojo.java
+++ b/lang/java/maven-plugin/src/main/java/org/apache/avro/mojo/SchemaMojo.java
@@ -78,6 +78,7 @@ protected void doCompile(String filename, File 
sourceDirectory, File outputDirec
     compiler.setStringType(StringType.valueOf(stringType));
     compiler.setFieldVisibility(getFieldVisibility());
     compiler.setCreateSetters(createSetters);
+    compiler.setPojoWithOptional(pojoWithOptional);
     compiler.setEnableDecimalLogicalType(enableDecimalLogicalType);
     
compiler.setOutputCharacterEncoding(project.getProperties().getProperty("project.build.sourceEncoding"));
     compiler.compileToDestination(src, outputDirectory);


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> [JAVA] Generate getters that return an Optional
> -----------------------------------------------
>
>                 Key: AVRO-1961
>                 URL: https://issues.apache.org/jira/browse/AVRO-1961
>             Project: Apache Avro
>          Issue Type: New Feature
>            Reporter: Niels Basjes
>            Assignee: Niels Basjes
>            Priority: Minor
>             Fix For: 1.9.0
>
>
> A colleague of mine indicated that having getters that return an Optional 
> (java 8 thingy) would be very useful for him.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to