You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by rs...@apache.org on 2022/10/20 17:49:19 UTC
[avro] branch master updated: AVRO-3644: handle java.util.Optional as a nullable value (#1915)
This is an automated email from the ASF dual-hosted git repository.
rskraba pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to refs/heads/master by this push:
new 627b00549 AVRO-3644: handle java.util.Optional as a nullable value (#1915)
627b00549 is described below
commit 627b005493bb8abe6a0ffc53c709ad1b7967c769
Author: Daniel Heinrich <da...@gmail.com>
AuthorDate: Thu Oct 20 19:49:13 2022 +0200
AVRO-3644: handle java.util.Optional as a nullable value (#1915)
Why:
Javas Optional type can not be used in any meaningful way with the
reflection based schema generation. It is for example not possible
to write a generic custom encoding.
How does it help with resolving the issue:
Optional can be used to make fields null safe in their usage.
Side effects:
I think that this change should not have any side effects, because
Optional fields could not have been used in the past.
---
.../apache/avro/reflect/ReflectDatumReader.java | 10 +++
.../apache/avro/reflect/ReflectDatumWriter.java | 3 +
.../org/apache/avro/specific/SpecificData.java | 3 +
.../java/org/apache/avro/reflect/TestReflect.java | 13 ++++
.../avro/reflect/TestReflectDatumReader.java | 83 ++++++++++++++++++++++
5 files changed, 112 insertions(+)
diff --git a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java
index ebdca36cc..2a8fcee9f 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java
@@ -25,6 +25,7 @@ import java.util.HashSet;
import java.util.HashMap;
import java.util.Collection;
import java.util.Map;
+import java.util.Optional;
import org.apache.avro.AvroRuntimeException;
import org.apache.avro.Conversion;
@@ -297,6 +298,15 @@ public class ReflectDatumReader<T> extends SpecificDatumReader<T> {
return;
}
}
+ if (Optional.class.isAssignableFrom(accessor.getField().getType())) {
+ try {
+ Object value = readWithoutConversion(oldDatum, field.schema(), in);
+ accessor.set(record, Optional.ofNullable(value));
+ return;
+ } catch (IllegalAccessException e) {
+ throw new AvroRuntimeException("Failed to set " + field);
+ }
+ }
try {
accessor.set(record, readWithoutConversion(oldDatum, field.schema(), in));
return;
diff --git a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java
index aa59c1d91..25555d99e 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumWriter.java
@@ -22,6 +22,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import org.apache.avro.AvroRuntimeException;
@@ -153,6 +154,8 @@ public class ReflectDatumWriter<T> extends SpecificDatumWriter<T> {
entryList.add(new MapEntry(e.getKey(), e.getValue()));
}
datum = entryList;
+ } else if (datum instanceof Optional) {
+ datum = ((Optional) datum).orElse(null);
}
try {
super.write(schema, datum, out);
diff --git a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java
index d03e7af63..072b1bf90 100644
--- a/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java
+++ b/lang/java/avro/src/main/java/org/apache/avro/specific/SpecificData.java
@@ -47,6 +47,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.concurrent.ConcurrentHashMap;
@@ -384,6 +385,8 @@ public class SpecificData extends GenericData {
if (!(key instanceof Class && CharSequence.class.isAssignableFrom((Class<?>) key)))
throw new AvroTypeException("Map key class not CharSequence: " + SchemaUtil.describe(key));
return Schema.createMap(createSchema(value, names));
+ } else if (Optional.class.isAssignableFrom(raw)) {
+ return Schema.createUnion(Schema.create(Schema.Type.NULL), createSchema(params[0], names));
} else {
return createSchema(raw, names);
}
diff --git a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java
index e3065d59b..63497a382 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflect.java
@@ -35,6 +35,7 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Random;
import org.apache.avro.AvroRuntimeException;
import org.apache.avro.AvroTypeException;
@@ -1281,6 +1282,18 @@ public class TestReflect {
check(ClassWithMultipleAliasesOnField.class, expectedSchema.toString());
}
+ private static class OptionalTest {
+ Optional<Integer> foo;
+ }
+
+ @Test
+ public void testOptional() {
+ check(OptionalTest.class,
+ "{\"type\":\"record\",\"name\":\"OptionalTest\","
+ + "\"namespace\":\"org.apache.avro.reflect.TestReflect\",\"fields\":["
+ + "{\"name\":\"foo\",\"type\":[\"null\",\"int\"],\"default\":null}]}");
+ }
+
private static class DefaultTest {
@AvroDefault("1")
int foo;
diff --git a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectDatumReader.java b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectDatumReader.java
index 3b97eab04..286e217f9 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectDatumReader.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/reflect/TestReflectDatumReader.java
@@ -28,6 +28,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Set;
import java.util.Map;
+import java.util.Optional;
import org.apache.avro.io.Decoder;
import org.apache.avro.io.DecoderFactory;
@@ -125,6 +126,40 @@ public class TestReflectDatumReader {
assertEquals(pojoWithMap, deserialized);
}
+ @Test
+ public void testRead_PojoWithOptional() throws IOException {
+ PojoWithOptional pojoWithOptional = new PojoWithOptional();
+ pojoWithOptional.setId(42);
+ pojoWithOptional.setRelatedId(Optional.of(13));
+
+ byte[] serializedBytes = serializeWithReflectDatumWriter(pojoWithOptional, PojoWithOptional.class);
+
+ Decoder decoder = DecoderFactory.get().binaryDecoder(serializedBytes, null);
+ ReflectDatumReader<PojoWithOptional> reflectDatumReader = new ReflectDatumReader<>(PojoWithOptional.class);
+
+ PojoWithOptional deserialized = new PojoWithOptional();
+ reflectDatumReader.read(deserialized, decoder);
+
+ assertEquals(pojoWithOptional, deserialized);
+ }
+
+ @Test
+ public void testRead_PojoWithEmptyOptional() throws IOException {
+ PojoWithOptional pojoWithOptional = new PojoWithOptional();
+ pojoWithOptional.setId(42);
+ pojoWithOptional.setRelatedId(Optional.empty());
+
+ byte[] serializedBytes = serializeWithReflectDatumWriter(pojoWithOptional, PojoWithOptional.class);
+
+ Decoder decoder = DecoderFactory.get().binaryDecoder(serializedBytes, null);
+ ReflectDatumReader<PojoWithOptional> reflectDatumReader = new ReflectDatumReader<>(PojoWithOptional.class);
+
+ PojoWithOptional deserialized = new PojoWithOptional();
+ reflectDatumReader.read(deserialized, decoder);
+
+ assertEquals(pojoWithOptional, deserialized);
+ }
+
public static class PojoWithList {
private int id;
private List<Integer> relatedIds;
@@ -309,4 +344,52 @@ public class TestReflectDatumReader {
return relatedIds.equals(other.relatedIds);
}
}
+
+ public static class PojoWithOptional {
+ private int id;
+
+ private Optional<Integer> relatedId;
+
+ public int getId() {
+ return id;
+ }
+
+ public void setId(int id) {
+ this.id = id;
+ }
+
+ public Optional<Integer> getRelatedId() {
+ return relatedId;
+ }
+
+ public void setRelatedId(Optional<Integer> relatedId) {
+ this.relatedId = relatedId;
+ }
+
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + id;
+ result = prime * result + ((relatedId == null) ? 0 : relatedId.hashCode());
+ return result;
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (obj == null)
+ return false;
+ if (getClass() != obj.getClass())
+ return false;
+ PojoWithOptional other = (PojoWithOptional) obj;
+ if (id != other.id)
+ return false;
+ if (relatedId == null) {
+ return other.relatedId == null;
+ } else
+ return relatedId.equals(other.relatedId);
+ }
+ }
}