This is an automated email from the ASF dual-hosted git repository.
opwvhk pushed a commit to branch branch-1.11
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to refs/heads/branch-1.11 by this push:
new 64569aa50 AVRO-2771: Refactor custom codable check (#1720)
64569aa50 is described below
commit 64569aa50e105e53a5bec4e2449f4442697269c5
Author: Oscar Westra van Holthe - Kind <[email protected]>
AuthorDate: Tue Aug 15 21:19:27 2023 +0200
AVRO-2771: Refactor custom codable check (#1720)
* AVRO-2771: Refactor custom codable check
Moved the isError check for the "custom codable" check to fix a
backwards compatibility issue between versions 1.8.x & 1.9.x.
---
.../org/apache/avro/generic/TestGenericData.java | 58 ++++++++++++----------
lang/java/compiler/pom.xml | 1 +
.../avro/compiler/specific/SpecificCompiler.java | 6 ++-
.../apache/avro/specific/TestGeneratedCode.java | 26 ++++++++++
.../regression_error_field_in_record.avsc | 22 ++++++++
5 files changed, 85 insertions(+), 28 deletions(-)
diff --git
a/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java
b/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java
index abae2cbd4..c764dcd7f 100644
--- a/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java
+++ b/lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericData.java
@@ -17,19 +17,23 @@
*/
package org.apache.avro.generic;
-import static org.apache.avro.TestCircularReferences.Reference;
-import static org.apache.avro.TestCircularReferences.Referenceable;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParseException;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.avro.AvroRuntimeException;
+import org.apache.avro.Schema;
+import org.apache.avro.Schema.Field;
+import org.apache.avro.Schema.Type;
+import org.apache.avro.SchemaBuilder;
+import org.apache.avro.TestCircularReferences.ReferenceManager;
+import org.apache.avro.generic.GenericData.Record;
+import org.apache.avro.io.BinaryData;
+import org.apache.avro.io.BinaryEncoder;
+import org.apache.avro.io.EncoderFactory;
+import org.apache.avro.util.Utf8;
+import org.junit.Test;
+
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
@@ -47,18 +51,15 @@ import java.util.List;
import java.util.Map;
import java.util.UUID;
-import org.apache.avro.AvroRuntimeException;
-import org.apache.avro.Schema;
-import org.apache.avro.Schema.Field;
-import org.apache.avro.Schema.Type;
-import org.apache.avro.SchemaBuilder;
-import org.apache.avro.TestCircularReferences.ReferenceManager;
-import org.apache.avro.generic.GenericData.Record;
-import org.apache.avro.io.BinaryData;
-import org.apache.avro.io.BinaryEncoder;
-import org.apache.avro.io.EncoderFactory;
-import org.apache.avro.util.Utf8;
-import org.junit.Test;
+import static org.apache.avro.TestCircularReferences.Reference;
+import static org.apache.avro.TestCircularReferences.Referenceable;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
public class TestGenericData {
@@ -103,7 +104,7 @@ public class TestGenericData {
@Test(expected = AvroRuntimeException.class)
public void testRecordPutInvalidField() throws Exception {
Schema s = Schema.createRecord("schemaName", "schemaDoc", "namespace",
false);
- List<Schema.Field> fields = new ArrayList<>();
+ List<Field> fields = new ArrayList<>();
fields.add(new Schema.Field("someFieldName", s, "docs", null));
s.setFields(fields);
Record r = new GenericData.Record(s);
@@ -381,8 +382,9 @@ public class TestGenericData {
Schema schema = Schema.createArray(Schema.create(Schema.Type.INT));
GenericArray<Integer> array = new GenericData.Array<>(6, schema);
array.clear();
- for (int i = 0; i < 5; ++i)
+ for (int i = 0; i < 5; ++i) {
array.add(i);
+ }
assertEquals(5, array.size());
array.add(0, 6);
assertEquals(Integer.valueOf(6), array.get(0));
@@ -411,8 +413,9 @@ public class TestGenericData {
Schema schema = Schema.createArray(Schema.create(Schema.Type.INT));
GenericArray<Integer> array = new GenericData.Array<>(10, schema);
array.clear();
- for (int i = 0; i < 10; ++i)
+ for (int i = 0; i < 10; ++i) {
array.add(i);
+ }
assertEquals(10, array.size());
assertEquals(Integer.valueOf(0), array.get(0));
assertEquals(Integer.valueOf(9), array.get(9));
@@ -454,8 +457,9 @@ public class TestGenericData {
Schema schema = Schema.createArray(Schema.create(Schema.Type.INT));
GenericArray<Integer> array = new GenericData.Array<>(10, schema);
array.clear();
- for (int i = 0; i < 10; ++i)
+ for (int i = 0; i < 10; ++i) {
array.add(i);
+ }
assertEquals(10, array.size());
assertEquals(Integer.valueOf(0), array.get(0));
assertEquals(Integer.valueOf(5), array.get(5));
@@ -702,7 +706,9 @@ public class TestGenericData {
private enum anEnum {
ONE, TWO, THREE
- };
+ }
+
+ ;
@Test
public void validateRequiresGenericSymbolForEnumSchema() {
diff --git a/lang/java/compiler/pom.xml b/lang/java/compiler/pom.xml
index 0e49e7814..e182649a4 100644
--- a/lang/java/compiler/pom.xml
+++ b/lang/java/compiler/pom.xml
@@ -137,6 +137,7 @@
<argument>org.apache.avro.compiler.specific.SchemaTask</argument>
<argument>${project.basedir}/src/test/resources/full_record_v1.avsc</argument>
<argument>${project.basedir}/src/test/resources/full_record_v2.avsc</argument>
+
<argument>${project.basedir}/src/test/resources/regression_error_field_in_record.avsc</argument>
<argument>${project.basedir}/target/generated-test-sources/javacc</argument>
</arguments>
</configuration>
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 baa2fd793..03e8761d8 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
@@ -944,19 +944,21 @@ public class SpecificCompiler {
* record.vm can handle the schema being presented.
*/
public boolean isCustomCodable(Schema schema) {
- if (schema.isError())
- return false;
return isCustomCodable(schema, new HashSet<>());
}
private boolean isCustomCodable(Schema schema, Set<Schema> seen) {
if (!seen.add(schema))
+ // Recursive call: assume custom codable until a caller on the call
stack proves
+ // otherwise.
return true;
if (schema.getLogicalType() != null)
return false;
boolean result = true;
switch (schema.getType()) {
case RECORD:
+ if (schema.isError())
+ return false;
for (Schema.Field f : schema.getFields())
result &= isCustomCodable(f.schema(), seen);
break;
diff --git
a/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedCode.java
b/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedCode.java
index fda8579b3..7b32e652a 100644
---
a/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedCode.java
+++
b/lang/java/compiler/src/test/java/org/apache/avro/specific/TestGeneratedCode.java
@@ -28,6 +28,8 @@ import org.apache.avro.io.Decoder;
import org.apache.avro.io.DecoderFactory;
import org.apache.avro.io.DatumReader;
import org.apache.avro.io.DatumWriter;
+import org.apache.avro.specific.test.RecordWithErrorField;
+import org.apache.avro.specific.test.TestError;
import org.apache.avro.util.Utf8;
import org.junit.Assert;
@@ -87,4 +89,28 @@ public class TestGeneratedCode {
FullRecordV1 expected = new FullRecordV1(true, 87231, 731L, 54.2832F,
38.0, null, "Hello, world!");
Assert.assertEquals(expected, dst);
}
+
+ @Test
+ public void withErrorField() throws IOException {
+ TestError srcError = TestError.newBuilder().setMessage$("Oops").build();
+ RecordWithErrorField src = new RecordWithErrorField("Hi there", srcError);
+ Assert.assertFalse("Test schema with error field cannot allow for custom
coders.",
+ ((SpecificRecordBase) src).hasCustomCoders());
+ Schema schema = RecordWithErrorField.getClassSchema();
+
+ ByteArrayOutputStream out = new ByteArrayOutputStream(1024);
+ Encoder e = EncoderFactory.get().directBinaryEncoder(out, null);
+ DatumWriter<RecordWithErrorField> w = (DatumWriter<RecordWithErrorField>)
MODEL.createDatumWriter(schema);
+ w.write(src, e);
+ e.flush();
+
+ ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray());
+ Decoder d = DecoderFactory.get().directBinaryDecoder(in, null);
+ DatumReader<RecordWithErrorField> r = (DatumReader<RecordWithErrorField>)
MODEL.createDatumReader(schema);
+ RecordWithErrorField dst = r.read(null, d);
+
+ TestError expectedError =
TestError.newBuilder().setMessage$("Oops").build();
+ RecordWithErrorField expected = new RecordWithErrorField("Hi there",
expectedError);
+ Assert.assertEquals(expected, dst);
+ }
}
diff --git
a/lang/java/compiler/src/test/resources/regression_error_field_in_record.avsc
b/lang/java/compiler/src/test/resources/regression_error_field_in_record.avsc
new file mode 100644
index 000000000..e2fdcb9ad
--- /dev/null
+++
b/lang/java/compiler/src/test/resources/regression_error_field_in_record.avsc
@@ -0,0 +1,22 @@
+{
+ "type" : "record",
+ "name" : "RecordWithErrorField",
+ "doc" : "With custom coders in Avro 1.9, previously successful records
with error fields now fail to compile.",
+ "namespace" : "org.apache.avro.specific.test",
+ "fields" : [ {
+ "name" : "s",
+ "type" : [ "null", "string" ],
+ "default" : null
+ }, {
+ "name": "e",
+ "type": [ "null", {
+ "type" : "error",
+ "name" : "TestError",
+ "fields" : [ {
+ "name" : "message",
+ "type" : "string"
+ } ]
+ } ],
+ "default": null
+ } ]
+}
