[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2018-11-20 Thread ASF GitHub Bot (JIRA)


[ 
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 fields = new ArrayList();
+  public void testDefaultRecordWithDuplicateFieldName() {
+String recordName = "name";
+Schema schema = Schema.createRecord(recordName, "doc", "namespace", false);
+List fields = new ArrayList();
 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

[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2018-11-20 Thread ASF subversion and git services (JIRA)


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

ASF subversion and git services commented on AVRO-1961:
---

Commit 021c01c4c2756f793a4eee3a0f914c2c131823ac in avro's branch 
refs/heads/master from [~dkulp]
[ https://gitbox.apache.org/repos/asf?p=avro.git;h=021c01c ]

Squashed commit of the following:

commit a3f86c2874ad6d86b11fc2edc908065adcefdeda
Author: Daniel Kulp 
Date:   Tue Nov 20 14:32:48 2018 -0500

Grab some more tests from other PR

commit 7d7822b5960c157ddf6db7fa15a3d797d0b286ee
Author: Joseph Pachod 
Date:   Sun Dec 11 22:50:45 2016 +0100

new javadoc with same formatting as other javadoc

commit 82e8f7af6736034f4e85a4e6e1bcaa803019082e
Author: Joseph Pachod 
Date:   Sun Dec 11 22:38:33 2016 +0100

restore initial imports

commit 0ca9815c8c884bc30c73b5c8a3c5d4c6e5188cee
Author: Joseph Pachod 
Date:   Sun Dec 11 22:36:03 2016 +0100

javadoc

commit 551e1eb4a6bdedf00260ef90576490648f9b4658
Author: Joseph Pachod 
Date:   Wed Dec 7 23:14:03 2016 +0100

format and clarify pre existing tests

commit 857da0c70a2db321d5bf521b901fe0c035f32edc
Author: Joseph Pachod 
Date:   Wed Dec 7 22:53:13 2016 +0100

AVRO-1961: Java: add isUnion and isNullable on Schema class

Closes #169


> [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)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2018-11-20 Thread ASF subversion and git services (JIRA)


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

ASF subversion and git services commented on AVRO-1961:
---

Commit 04d4423d4ee4f86e308fa761c6918223b55a6c5c in avro's branch 
refs/heads/master from [~dkulp]
[ https://gitbox.apache.org/repos/asf?p=avro.git;h=04d4423 ]

Squashed commit of the following:

commit 6a919437bff0977926b33cd56165994b22fbdf74
Author: Niels Basjes 
Date:   Fri Nov 25 10:31:40 2016 +0100

AVRO-1961: Extra flag to replace the regular getters with getters that 
return an Optional

commit 91d58b5dba7562240a84f6885037f9c74dc973e6
Author: Niels Basjes 
Date:   Tue Nov 22 17:23:18 2016 +0100

AVRO-1961: Java: Generate getters that return a Java 8 Optional.

Closes #162


> [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
>
> 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)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2018-11-20 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on AVRO-1961:
--

dkulp closed pull request #162: AVRO-1961: Java: Generate getters that return a 
Java 8 Optional.
URL: https://github.com/apache/avro/pull/162
 
 
   

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/CHANGES.txt b/CHANGES.txt
index 3553e2a93..e145d1cc1 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -16,6 +16,8 @@ Trunk (not yet released)
 AVRO-1932: Java: Allow setting the SchemaStore on generated classes. 
 (Niels Basjes)
 
+AVRO-1961: Java: Generate getters that return a Java 8 Optional. (Niels 
Basjes)
+
   OPTIMIZATIONS
 
   IMPROVEMENTS
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..36e51c9fc 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
@@ -105,6 +105,8 @@
   private VelocityEngine velocityEngine;
   private String templateDir;
   private FieldVisibility fieldVisibility = FieldVisibility.PUBLIC_DEPRECATED;
+  private boolean createOptionalGetters = false;
+  private boolean gettersReturnOptional = false;
   private boolean createSetters = true;
   private boolean createAllArgsConstructor = true;
   private String outputCharacterEncoding;
@@ -217,6 +219,28 @@ public void setCreateSetters(boolean createSetters) {
 this.createSetters = createSetters;
   }
 
+  public boolean isCreateOptionalGetters() {
+return this.createOptionalGetters;
+  }
+
+  /**
+   * Set to false to not create the getters that return an Optional.
+   */
+  public void setCreateOptionalGetters(boolean createOptionalGetters) {
+this.createOptionalGetters = createOptionalGetters;
+  }
+
+  public boolean isGettersReturnOptional() {
+return this.gettersReturnOptional;
+  }
+
+  /**
+   * Set to false to not create the getters that return an Optional.
+   */
+  public void setGettersReturnOptional(boolean gettersReturnOptional) {
+this.gettersReturnOptional = gettersReturnOptional;
+  }
+
   /**
* Set to true to use {@link java.math.BigDecimal} instead of
* {@link java.nio.ByteBuffer} for logical type "decimal"
@@ -778,6 +802,16 @@ public static String generateGetMethod(Schema schema, 
Field field) {
 return generateMethodName(schema, field, "get", "");
   }
 
+  /**
+   * Generates the name of a field accessor method that returns a Java 8 
Optional.
+   * @param schema the schema in which the field is defined.
+   * @param field the field for which to generate the accessor name.
+   * @return the name of the accessor method for the given field.
+   */
+  public static String generateGetOptionalMethod(Schema schema, Field field) {
+return generateMethodName(schema, field, "getOptional", "");
+  }
+
   /**
* Generates the name of a field mutator method.
* @param schema the schema in which the field is defined.
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..04666e42a 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.gettersReturnOptional} || ${this.createOptionalGetters})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.gettersReturnOptional})
+  /**
+   * Gets the value of the '${this.mangle($field.name(), $schema.isError())}' 
field as an Optional<${this.javaType($field.schema())}>.
+#if ($field.doc())  * $field.doc()
+#end
+   * @return The value wrapped in an 
Optional<${this.javaType($field.schema())}>.
+   */
+  public Optional<${this.javaType($field.schema())}> 
${this.generateGetMethod($schema, $field)}() {
+return 
Optional.<${this.javaType($field.schema())}>ofNullable(${this.mangle($field.name(),
 $schema.isError())});
+  }
+#

[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2018-11-20 Thread ASF subversion and git services (JIRA)


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

ASF subversion and git services commented on AVRO-1961:
---

Commit 04d4423d4ee4f86e308fa761c6918223b55a6c5c in avro's branch 
refs/heads/master from [~dkulp]
[ https://gitbox.apache.org/repos/asf?p=avro.git;h=04d4423 ]

Squashed commit of the following:

commit 6a919437bff0977926b33cd56165994b22fbdf74
Author: Niels Basjes 
Date:   Fri Nov 25 10:31:40 2016 +0100

AVRO-1961: Extra flag to replace the regular getters with getters that 
return an Optional

commit 91d58b5dba7562240a84f6885037f9c74dc973e6
Author: Niels Basjes 
Date:   Tue Nov 22 17:23:18 2016 +0100

AVRO-1961: Java: Generate getters that return a Java 8 Optional.

Closes #162


> [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
>
> 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)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2018-11-07 Thread ASF GitHub Bot (JIRA)


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

ASF GitHub Bot commented on AVRO-1961:
--

Fokko commented on issue #162: AVRO-1961: Java: Generate getters that return a 
Java 8 Optional.
URL: https://github.com/apache/avro/pull/162#issuecomment-436614690
 
 
   @nielsbasjes Are you still interested in merging this into Avro 1.9? Can you 
rebase onto master?


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: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2017-01-03 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Hi

In the end, any idea if the changes in https://github.com/apache/avro/pull/169 
will be merged, and if so in which version?

Happy new year and all ;)

best

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-13 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Finally I've pushed it all.

The option is named pojoWithOptional, adding Optional only for nullable 
fields, be it in the pojo getter/setter or in the builder. It doesn't require 
Java 8 to generate such pojos, but for using them Java 8 is welcome or some 
backport of Optional.

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-12 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Hi

I feel like I've done the "pojoWithOptional" code on my local repo: shall I 
push this to my github account and so update the above PR?

Let me know :)

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-11 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Javadoc just pushed.

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-09 Thread Doug Cutting (JIRA)

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

Doug Cutting commented on AVRO-1961:


The new public Schema methods need Javadoc comments, no?

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-07 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


PR https://github.com/apache/avro/pull/169 created for isUnion and isNullable. 
If fine for merge, then I would go on with the above plan.

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-07 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on AVRO-1961:
--

GitHub user cluelessjoe opened a pull request:

https://github.com/apache/avro/pull/169

Branch 1.8 - AVRO-1961 : add isUnion and isNullable on Schema class

isUnion is needed since the class UnionSchema is private hence not 
accessible from outside code
isNullable is needed for doing nullable schema's specific actions in 
templates

both methods add value whatever the outcome of AVRO-1961 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/cluelessjoe/avro branch-1.8

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/avro/pull/169.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #169


commit f912a42ecc7dc2ac305212c23af08b99ab355afc
Author: Joseph Pachod 
Date:   2016-12-07T21:53:13Z

AVRO-1961: Java: add isUnion and isNullable on Schema class

commit 185291033a915e8221f6e2e6f55809256d68082b
Author: Joseph Pachod 
Date:   2016-12-07T22:14:03Z

format and clarify pre existing tests




> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-07 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Hi

For your information, I'm working on a PR based on branch 1.8 and java 6. I'll 
first submit one with isUnion and isNullable (or isOptional, since I'm not sure 
of the name).
It's taking a bit longer that I would hope for various reasons, I hope you 
don't mind it, if so let me know.

@[~Yibing] indeed the specs about default value says :
default: A default value for this field, used when reading instances that lack 
this field (optional). Permitted values depend on the field's schema type, 
according to the table below. Default values for union fields correspond to the 
first schema in the union. 

I'm thinking of testing the content of the generated file directly (generating 
the .java, accessing it through a File, then parsing it for expected content), 
so no need for Java 8.

@[~nielsbasjes] in the code I'm writing I don't include anything needing Java 8 
(so it could make it in the 1.8 branch I hope).

:)

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-02 Thread Niels Basjes (JIRA)

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

Niels Basjes commented on AVRO-1961:


Yes, Java 8 is needed to run the unit tests for this.
As you can see in my proposal I build de runtime code against Java 6 and build 
and run the tests against Java 8.

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-01 Thread Yibing Shi (JIRA)

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

Yibing Shi commented on AVRO-1961:
--

{quote}
Actually, on the setter/builder/constructor, we could even go a step further: a 
required field with a default value could be with Optional as well. The 
matching getter would be without Optional. It would make the concept of default 
value type safe just as well. Sweet isn't it?
{quote}
And Optional object indicates that the value could be null. It has a different 
meaning than having a default value. That is, you can skip passing in a value 
for this field doesn't mean you can pass in a null value for this field. 
Suggest not to apply this to fields with default values.

{quote}
1. Java 8 isn't required for Avro project for this to work
All the "magic" happen uniquely through the template: the code could still 
compile with Java 6, but anyone using the "pojoWithOptional" flag should 
consume the generated pojo with Java 8 or backport Optional.
{quote}
Is Java 8 is needed to test the pojoWithOptionalGenerationTest you are going to 
add?

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-12-01 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Hi 

In the last few days I had the time to clarify why I was willing to go for 
optional: currently there's no easy way for an Avro pojo consumer to know 
whether a field is nullable or not.

This leads to many nasty NPE when consuming messages. And in early stage of 
development, if a field becomes nullable, this is invisible to all the 
consumers, even if at the time they made sure to check nullability. The code 
goes on compiling... and then may break anytime.

How could we avoid this?

Optional getFoo() for nullable fields and Foo getFoo() would non nullable 
fields would make sure everyone must handle nullabity when present, because 
nullabity would be expressed in a type safe way. No more looking around to 
check whether the field is nullable or not. And if a field becomes nullable (or 
non nullable), the consumer's code doesn't compile anymore.

Regarding setter/builder/constructor: considering the initial goal, ie making 
obvious if a field is nullable or not, and what was achieved with getter 
through typesafety, I would like the same level of warranty. Which means having 
setter/builder/constructor with Optional for nullable fields.

Once again, nullability would be ensured in a typesafe way. If it changes in 
the schema, then the code using the generated pojo doesn't compile anymore.

And we would gain extra readability: now a setter/builder/constructor would 
tell explicitly whether a field is required or not.

Actually, on the setter/builder/constructor, we could even go a step further: a 
required field with a default value could be with Optional as well. The 
matching getter would be without Optional. It would make the concept of default 
value type safe just as well. Sweet isn't it? 

And, in setter for a non nullable value, we could add some null check to make 
sure nulls don't make it through.

In the end, the name of the settings on the specific compiler, currently 
gettersReturnOptionalOnNullableFields, isn't appropriate anymore. It should be 
something like "pojoWithOptional" (suggestions welcome!).

Do we agree ?

Technically speaking:

1. Java 8 isn't required for Avro project for this to work

All the "magic" happen uniquely through the template: the code could still 
compile with Java 6, but anyone using the "pojoWithOptional" flag should 
consume the generated pojo with Java 8 or backport Optional.

2. Only 2 news methods required in the Schema class

There is a need for isNullable and hasDefault methods on the Schema class, to 
use in the template. In case this "pojoWithOptional" proposal isn't accepted, 
could these 2 methods still be added?
 
3. Not test for generated pojo in Avro's project code base AFAIK

I haven't seen any test of the generation outcome. If I'm wrong, please tell me.

If not, then I would add something along these lines:
- defaultPojoGenerationSafetyNetTest: generating a pojo with default settings 
from a moderatly complex schema and then inspecting the generated .java class 
to make sure of its content. I would use this a "safety net" for the changes 
made.
- pojoWithOptionalGenerationTest: generating a pojo with "pojoWithOptional" 
from the same schema as above and then inspecting it to make sure the above 
rules are respected.

4. No arg constructor limit

No arg constructors are needed for Avro's generated builders. Yet they may 
break the wanted type safety: an instance created with the no arg constructor 
may have non nullable fields being nulls. To limit this issue, I'm thinking of 
adding a warning in the javadoc when pojoWithOptional is set. Something along 
the lines: used internally by Avro, do not use.

5. Current pull request change imports

My current pull request, https://github.com/apache/avro/pull/165, changes the 
imports order. This is bad, I'll fix it.

Misc:

@Gabor Szadovszky:
Actually, for the setters/builders, having both setFoo(Foo foo) and 
setFoo(Optional foo) would be cumbersome when passing null, which then 
would have to be cast explicitly.

Regarding the getter, I agree with you :)

@Tom White
Having Optional getFoo() instead of Foo getFoo() is made through a 
dedicated setting. It won't break backward compatibility unless you set the 
parameter to true.

About this quote "I think routinely using it as a return value for getters 
would definitely be over-used", it all depends of the context and the intent. 
For me the goal is to get rid of NullPointerException because the message's 
consumer didn't know/care which fields are nullable. These NPEs are really 
annoying and currently require extensive unit testing to (try to) prevent. 
Having type safety there would make nullability obvious.

@Niels Basjes 
In your comment of the 23/Nov/16 11:12, you said :
"The idea is that the downstr

[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-30 Thread Tom White (JIRA)

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

Tom White commented on AVRO-1961:
-

For compatibility reasons, I don't think getFoo() should ever return 
Optional. Having getOptionalFoo() is much clearer and safer. Also, I 
noticed that the creator of Optional said "I think routinely using it as a 
return value for getters would definitely be over-use", 
http://stackoverflow.com/a/26328555, so it's not necessarily true that everyone 
will want this feature. In other words, it may never become the default.

Avro allows you to set the directory to load velocity templates from (by 
setting the org.apache.avro.specific.templates system property), so it should 
be possible to try this out with an existing Avro release. You would create a 
copy of the existing templates and change record.vm to have the getOptional*() 
methods.

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-30 Thread Gabor Szadovszky (JIRA)

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

Gabor Szadovszky commented on AVRO-1961:


Hi All,

Thanks a lot for this issue. I really like using the new features of java8.

I don't think it is that urgent to create setters for Optional. Optionals are 
more used for getting values and not for setting. However, having setters for 
both seems reasonable.

I don't really like the two different flags for having separate getters for 
Optionals and returning Optionals by the original getters. I think, one flag 
for having Optionals returned by the original getters or keeping the actual 
behaviour is enough and less confusing. I don't think we really need both 
getters in the same time. This way, the default behaviour can also be switched 
easier in later major releases. 

What do you think?

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-28 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Hi Niels

The changes I made are visible there https://github.com/apache/avro/pull/165. 
They do the following: a getter for Foo foo returns Optionnal when foo is 
nullable and gettersReturnOptionalOnNullableFields is set to true.

Furthermore, I didn't find any way in the current tests to generate some pojos 
to make sure they fit what I expect. How should I do this ?

What do you think of it ?

Best regards
cluelessjoe

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-28 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Furthermore, I didn't find any way in the current tests to generate some pojos 
to make sure they fit what I expect. How should I do this ?

Anyway, the above patch does the following: the getter is Optional 
getFoo() only when foo is nullable and gettersReturnOptional is set to true.

What do you think of it ?

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-28 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Hi Niels

I don't find a simple way to contribute back the changes I made, so I 
copy/paste them here. More precise hint welcomed, sorry for this..

diff --git i/lang/java/avro/src/main/java/org/apache/avro/Schema.java 
w/lang/java/avro/src/main/java/org/apache/avro/Schema.java
index 2019c1f..f2b3963 100644
--- i/lang/java/avro/src/main/java/org/apache/avro/Schema.java
+++ w/lang/java/avro/src/main/java/org/apache/avro/Schema.java
@@ -22,13 +22,13 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Iterator;
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.IdentityHashMap;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -38,10 +38,10 @@ import java.util.Set;
 
 import org.apache.avro.util.internal.JacksonUtils;
 import org.codehaus.jackson.JsonFactory;
+import org.codehaus.jackson.JsonGenerator;
 import org.codehaus.jackson.JsonNode;
 import org.codehaus.jackson.JsonParseException;
 import org.codehaus.jackson.JsonParser;
-import org.codehaus.jackson.JsonGenerator;
 import org.codehaus.jackson.map.ObjectMapper;
 import org.codehaus.jackson.node.DoubleNode;
 
@@ -371,6 +371,20 @@ public abstract class Schema extends JsonProperties {
"default","doc","name","order","type","aliases");
   }
 
+  public boolean isNullable() {
+if (!(this instanceof UnionSchema)) {
+  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 i/lang/java/avro/src/test/java/org/apache/avro/TestSchema.java 
w/lang/java/avro/src/test/java/org/apache/avro/TestSchema.java
index 242ee8c..bed751d 100644
--- i/lang/java/avro/src/test/java/org/apache/avro/TestSchema.java
+++ w/lang/java/avro/src/test/java/org/apache/avro/TestSchema.java
@@ -18,6 +18,7 @@
 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;
@@ -77,6 +78,20 @@ public class TestSchema {
   }
 
   @Test
+  public void testIsNullableWorksOnNullableField() {
+Schema schema = Schema.createUnion(Schema.create(Type.NULL), 
Schema.create(Type.LONG));
+
+assertTrue(schema.isNullable());
+  }
+
+  @Test
+  public void testIsNullableWorksOnNonNullableField() {
+Schema schema = Schema.createUnion(Schema.create(Type.LONG));
+
+assertFalse(schema.isNullable());
+  }
+
+  @Test
   public void testEmptyRecordSchema() {
 Schema schema = Schema.createRecord("foobar", null, null, false);
 String schemaString = schema.toString();
diff --git 
i/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
 
w/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
index 04666e4..a918e30 100644
--- 
i/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
+++ 
w/lang/java/compiler/src/main/velocity/org/apache/avro/compiler/specific/templates/java/classic/record.vm
@@ -185,7 +185,7 @@ public class ${this.mangle($schema.getName())}#if 
($schema.isError()) extends or
   }
 
 #foreach ($field in $schema.getFields())
-#if (${this.gettersReturnOptional})
+#if (${this.gettersReturnOptional} && ${field.isNullable})
   /**
* Gets the value of the '${this.mangle($field.name(), $schema.isError())}' 
field as an Optional<${this.javaType($field.schema())}>.
 #if ($field.doc())  * $field.doc()


> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-25 Thread Niels Basjes (JIRA)

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

Niels Basjes commented on AVRO-1961:


Just now (as an experiment) I added the option to the code generator to replace 
the getters with a version that returns an Optional.
So there are two flags now that (should) allow any combination of the mentioned 
getters.


> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


Re: [jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-25 Thread Anders Sundelin
There are many arguments for/against Optionals on the web, but the 
consensus seem to be that Optionals are best used as method return 
types, not as method parameters.


Therefore, I think the setters should still use the plain objects, as 
there are anyway no practical way to enforce non-nullity of the arguments.


http://stackoverflow.com/questions/31922866/why-should-java-8s-optional-not-be-used-in-arguments

http://dolszewski.com/java/java-8-optional-use-cases/

£0.02

/a


general

On 2016-11-25 00:47, Clueless Joe (JIRA) wrote:

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

Clueless Joe commented on AVRO-1961:


Thanks a lot for your answer. And don't forget I'm clueless, so it's all deeply 
open to discussion :)

I'll do a PR on your repo tomorrow night regarding the null part, so at least 
this part will be solved.

You're right to think of the transition We're using Avro at work and, to be honest, I 
was rather thinking of some breaking changes in our dev branches. A smoother 
transition would be great, but I don't see some currently. How would you  transition 
from the two getters (getFoo and getOptionalFoo) to the one Optional 
getFoo() later on?

For the setters, at least having both, with setFoo(Foo fooOrNull) and 
setFoo(Optional foo), is doable code wise. Furthermore in the builder to 
have both seems handy. I would love to have more feedbacks on the matter actually, to 
know everyone's feeling.

Best
cluelessjoe


[JAVA] Generate getters that return an Optional
---

 Key: AVRO-1961
 URL: https://issues.apache.org/jira/browse/AVRO-1961
 Project: Avro
  Issue Type: New Feature
Reporter: Niels Basjes
Assignee: Niels Basjes
Priority: Minor

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
(v6.3.4#6332)





smime.p7s
Description: S/MIME Cryptographic Signature


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-24 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Thanks a lot for your answer. And don't forget I'm clueless, so it's all deeply 
open to discussion :)

I'll do a PR on your repo tomorrow night regarding the null part, so at least 
this part will be solved.

You're right to think of the transition We're using Avro at work and, to be 
honest, I was rather thinking of some breaking changes in our dev branches. A 
smoother transition would be great, but I don't see some currently. How would 
you  transition from the two getters (getFoo and getOptionalFoo) to the one 
Optional getFoo() later on? 

For the setters, at least having both, with setFoo(Foo fooOrNull) and 
setFoo(Optional foo), is doable code wise. Furthermore in the builder to 
have both seems handy. I would love to have more feedbacks on the matter 
actually, to know everyone's feeling.

Best
cluelessjoe

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-24 Thread Niels Basjes (JIRA)

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

Niels Basjes commented on AVRO-1961:


Hi,

First of all let me make clear my experience in using Optionals is very 
limited, so I have a lot to learn and welcome any advice in this matter.

I create the 'Optional' variant for everything because that is the easiest to 
implement. Finding out if a field is nullable in the code generator is 
something I need to look into if that is needed.

The reason I left both the regular and the Optional versions remain side by 
side is because I expect that in existing applications it would be too hard to 
switch from one to the other overnight. Perhaps I'm wrong in this, perhaps we 
should have a flag to do the switch completely as you suggested.

I hadn't thought of the setters yet. For those I do think we must have both as 
the type system of Java will pick the right one depending on the type you call 
it with.



> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-23 Thread Clueless Joe (JIRA)

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

Clueless Joe commented on AVRO-1961:


Hi Niels and all

Thanks a lot for this issue, I was about to create it.

>From what I've seen in your code, you plan to let the current getter/setter 
>and add a getOptionalFoo.

Why letting the previous getter?

Indeed, IMHO, the main goal of the optional is to make sure every developers 
knows a field can hold a null value. In the current template, there's no easy 
way to figure this out, so one can easily forget to handle nullity. IMHO this 
would the main thing to target when introducing Optional: to make sure there's 
no doubt about what can be null and what can't be null.

By letting the current getter (which return nulls), the risk is still highly 
present.

When regenerating pojos with your optional, then the code would still look 
right whereas in many cases the developer may have forgotten the value can be 
null.

When activating your createOptionalGetters, what about only changing the getter 
to return Optional instead of just Foo only when the field can have a null 
value? 

As such the IDE/type would make sure the developer handles the potential null 
values.

For the setter, I've no opinion on the way to go. Currently we don't generate 
them. I guess changing the setter for potentially null values to 
setFoo(Optional foo) would make sense again in terms of revealing the 
intent. Or maybe putting both, for convenience. Maybe the setter without an 
Optional could have its parameter name completed by "OrNull" to explicit the 
potential null value, for example setFoo(Foo fooOrNull).

Whatever the choice made for the setter, we should then apply it as well to the 
builder.

Eager to read you back

Thanks again

best
cluelessjoe

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-23 Thread Niels Basjes (JIRA)

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

Niels Basjes commented on AVRO-1961:


The idea is that the downstream application code becomes more readable when 
retrieving a field that is buried deep in a nested structure.
With Optionals you can rely on the fact that there will always be a value 
returned and this removes the need for having many {{if (foo != null)}} type 
checks.
As you can see in the unit test I wrote you can do something like this:
{code}
   assertEquals("Chrome 123", Optional
  .of(request)
  .flatMap(Request::getOptionalHttpRequest)
  .flatMap(HttpRequest::getOptionalUserAgent)
  .flatMap(UserAgent::getOptionalUseragent)
  .orElse("UNKNOWN"));
{code}

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-23 Thread ASF GitHub Bot (JIRA)

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

ASF GitHub Bot commented on AVRO-1961:
--

GitHub user nielsbasjes opened a pull request:

https://github.com/apache/avro/pull/162

AVRO-1961: Java: Generate getters that return a Java 8 Optional.

NOTE: This must wait until AFTER the release of Avro 1.8.2.

This is a first attempt at making the Java code generator have the option 
(default is off) to generate Java 8 Optional getters.

A few things about this that open and must be discussed before committing:

1. This will change the building of Avro to require JDK 8 to be installed. 
Building with JDK 7 will simply fail.
2. The main runtime is still compiled with the setting "JDK 6". So in 
theory that should remain backward compatible for systems that still rely in 
JDK 6.
3. This is my first time playing with Optionals so a thorough review is 
really needed.

Also a big question is: Do we want this? 
I think it is a good idea to use the new Java features that make it easier 
for downstream developers.
Yet the risk of breaking backward compatibility may be considered to be a 
problem.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/nielsbasjes/avro AVRO-1961

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/avro/pull/162.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #162


commit 2394bcf5f7abfd38239bdafd7f30666cc3bfc32a
Author: Niels Basjes 
Date:   2016-11-22T16:23:18Z

AVRO-1961: Java: Generate getters that return a Java 8 Optional.

commit 99b6adaa4407bdf02c893ab655ae514ccaf57f9b
Author: Niels Basjes 
Date:   2016-11-23T10:27:37Z

AVRO-1961: Upgrade docker to jdk 8




> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)


[jira] [Commented] (AVRO-1961) [JAVA] Generate getters that return an Optional

2016-11-22 Thread Yibing Shi (JIRA)

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

Yibing Shi commented on AVRO-1961:
--

[~nielsbasjes], would you please provide more details of this request? Such as 
how the Optional type should be returned and what is the benefit of it.

> [JAVA] Generate getters that return an Optional
> ---
>
> Key: AVRO-1961
> URL: https://issues.apache.org/jira/browse/AVRO-1961
> Project: Avro
>  Issue Type: New Feature
>Reporter: Niels Basjes
>Assignee: Niels Basjes
>Priority: Minor
>
> 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
(v6.3.4#6332)