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