Copilot commented on code in PR #3917:
URL: https://github.com/apache/avro/pull/3917#discussion_r3707522827
##########
lang/java/avro/src/test/java/org/apache/avro/io/FastReaderBuilderJavaClassTest.java:
##########
@@ -18,99 +18,190 @@
package org.apache.avro.io;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.math.BigDecimal;
+import java.util.Map;
import org.apache.avro.Schema;
+import org.apache.avro.SchemaBuilder;
import org.apache.avro.generic.GenericData;
import org.apache.avro.generic.GenericDatumReader;
import org.apache.avro.generic.GenericDatumWriter;
import org.apache.avro.generic.GenericRecord;
+import org.apache.avro.generic.GenericRecordBuilder;
+import org.apache.avro.specific.SpecificData;
+import org.apache.avro.util.Utf8;
import org.junit.jupiter.api.Test;
/**
- * Tests for FastReaderBuilder behavior with schemas containing "java-class"
- * attributes.
+ * Tests for FastReaderBuilder behavior with schemas containing
+ * {@link SpecificData#CLASS_PROP} and {@link SpecificData#KEY_CLASS_PROP}
+ * attributes. Note that {@link SpecificData#ELEMENT_PROP} isn't tested because
+ * it is only used by ReflectData.
*/
public class FastReaderBuilderJavaClassTest {
+ private static final Schema SCHEMA_RECORD_WITH_NULLABLE_CLASS_PROP =
SchemaBuilder.record("NullableStringRecord")
+
.fields().requiredString("id").name("price").type().unionOf().nullType().and()
+
.type(SchemaBuilder.builder().stringBuilder().prop(SpecificData.CLASS_PROP,
"java.math.BigDecimal").endString())
+ .endUnion().noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_NULLABLE_CLASS_PROP = new
GenericRecordBuilder(
+ SCHEMA_RECORD_WITH_NULLABLE_CLASS_PROP).set("id", "123").set("price",
"-0.0002").build();
+
+ private static final Schema SCHEMA_RECORD_WITH_CLASS_PROP =
SchemaBuilder.record("StringRecord").fields()
+ .requiredString("id").name("price")
+
.type(SchemaBuilder.builder().stringBuilder().prop(SpecificData.CLASS_PROP,
"java.math.BigDecimal").endString())
+ .noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_CLASS_PROP = new
GenericRecordBuilder(SCHEMA_RECORD_WITH_CLASS_PROP)
+ .set("id", "123").set("price", "-0.0002").build();
+
+ private static final Schema SCHEMA_RECORD_WITH_MAP_KEY_CLASS_PROP =
SchemaBuilder.record("MapRecord").fields()
+
.requiredString("id").name("prices").type().map().prop(SpecificData.KEY_CLASS_PROP,
"java.math.BigDecimal")
+ .values().stringType().noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_MAP_KEY_CLASS_PROP = new
GenericRecordBuilder(
+ SCHEMA_RECORD_WITH_MAP_KEY_CLASS_PROP).set("id", "123")
+ .set("prices", Map.of("-0.0002", "cheap", "12345.678",
"expensive")).build();
+
+ /**
+ * Reusable round-trip logic for a record, using the given model.
+ */
+ public static GenericRecord roundTrip(GenericRecord record, GenericData
model) throws IOException {
+ byte[] serialized;
+
+ try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+ GenericDatumWriter<GenericRecord> writer = new
GenericDatumWriter<>(record.getSchema());
+ BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(baos, null);
+ writer.write(record, encoder);
+ encoder.flush();
+ serialized = baos.toByteArray();
+ }
+
+ GenericDatumReader<GenericRecord> reader = new
GenericDatumReader<>(record.getSchema(), record.getSchema(), model);
+ BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(serialized,
null);
+ return reader.read(null, decoder);
+ }
+
/**
- * Tests that GenericDatumReader can deserialize records with string fields
that
- * have a "java-class" attribute (e.g., BigDecimal).
- *
- * This test reproduces a bug where
+ * Tests that a plain GenericDatumReader (GenericData model) ignores the
+ * {@link SpecificData#CLASS_PROP} attribute on a string field inside a
union,
+ * matching the classic (non fast-reader) behavior of GenericData.
+ * <p>
+ * This test also reproduces a bug (AVRO-4225) where
* FastReaderBuilder.getTransformingStringReader() casts the result of
- * stringReader.read() directly to String, but in GenericData mode the reader
- * returns Utf8, causing a ClassCastException.
+ * stringReader.read() directly to String, but GenericData returns Utf8,
causing
+ * a ClassCastException
*/
@Test
- void genericDatumReaderWithJavaClassAttribute() throws IOException {
- // Schema with a string field that has "java-class": "java.math.BigDecimal"
- // This is a common pattern for representing decimal values as strings
- String schemaJson = "{\n" + " \"type\": \"record\",\n" + " \"name\":
\"TestRecord\",\n" + " \"fields\": [\n"
- + " {\"name\": \"id\", \"type\": \"string\"},\n" + " {\"name\":
\"price\", \"type\": [\"null\", {\n"
- + " \"type\": \"string\",\n" + " \"java-class\":
\"java.math.BigDecimal\"\n" + " }]}\n" + " ]\n"
- + "}";
-
- Schema schema = new Schema.Parser().parse(schemaJson);
-
- GenericRecord record = new GenericData.Record(schema);
- record.put("id", "123");
- record.put("price", "-0.0002");
-
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- GenericDatumWriter<GenericRecord> writer = new
GenericDatumWriter<>(schema);
- BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(out, null);
- writer.write(record, encoder);
- encoder.flush();
-
- byte[] serialized = out.toByteArray();
-
- // Deserialize using GenericDatumReader (which uses FastReaderBuilder by
- // default)
- GenericDatumReader<GenericRecord> reader = new
GenericDatumReader<>(schema);
- BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(serialized,
null);
+ void genericDataModelIgnoresJavaClassPropWithStringUnion() throws
IOException {
+ // This round trip shouldn't cause a ClassCastException (AVRO-4225)
+ GenericRecord result = roundTrip(RECORD_WITH_NULLABLE_CLASS_PROP,
GenericData.get());
Review Comment:
This test relies on the global default for fast-reader enablement (system
property + GenericData singleton). If fast-reader is disabled in the test JVM,
this won’t exercise FastReaderBuilder and may not catch regressions like
AVRO-4225. Prefer using a fresh model instance with fast-reader explicitly
enabled.
This issue also appears in the following locations of the same file:
- line 120
- line 169
##########
lang/java/avro/src/test/java/org/apache/avro/io/FastReaderBuilderJavaClassTest.java:
##########
@@ -18,99 +18,190 @@
package org.apache.avro.io;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.math.BigDecimal;
+import java.util.Map;
import org.apache.avro.Schema;
+import org.apache.avro.SchemaBuilder;
import org.apache.avro.generic.GenericData;
import org.apache.avro.generic.GenericDatumReader;
import org.apache.avro.generic.GenericDatumWriter;
import org.apache.avro.generic.GenericRecord;
+import org.apache.avro.generic.GenericRecordBuilder;
+import org.apache.avro.specific.SpecificData;
+import org.apache.avro.util.Utf8;
import org.junit.jupiter.api.Test;
/**
- * Tests for FastReaderBuilder behavior with schemas containing "java-class"
- * attributes.
+ * Tests for FastReaderBuilder behavior with schemas containing
+ * {@link SpecificData#CLASS_PROP} and {@link SpecificData#KEY_CLASS_PROP}
+ * attributes. Note that {@link SpecificData#ELEMENT_PROP} isn't tested because
+ * it is only used by ReflectData.
*/
public class FastReaderBuilderJavaClassTest {
+ private static final Schema SCHEMA_RECORD_WITH_NULLABLE_CLASS_PROP =
SchemaBuilder.record("NullableStringRecord")
+
.fields().requiredString("id").name("price").type().unionOf().nullType().and()
+
.type(SchemaBuilder.builder().stringBuilder().prop(SpecificData.CLASS_PROP,
"java.math.BigDecimal").endString())
+ .endUnion().noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_NULLABLE_CLASS_PROP = new
GenericRecordBuilder(
+ SCHEMA_RECORD_WITH_NULLABLE_CLASS_PROP).set("id", "123").set("price",
"-0.0002").build();
+
+ private static final Schema SCHEMA_RECORD_WITH_CLASS_PROP =
SchemaBuilder.record("StringRecord").fields()
+ .requiredString("id").name("price")
+
.type(SchemaBuilder.builder().stringBuilder().prop(SpecificData.CLASS_PROP,
"java.math.BigDecimal").endString())
+ .noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_CLASS_PROP = new
GenericRecordBuilder(SCHEMA_RECORD_WITH_CLASS_PROP)
+ .set("id", "123").set("price", "-0.0002").build();
+
+ private static final Schema SCHEMA_RECORD_WITH_MAP_KEY_CLASS_PROP =
SchemaBuilder.record("MapRecord").fields()
+
.requiredString("id").name("prices").type().map().prop(SpecificData.KEY_CLASS_PROP,
"java.math.BigDecimal")
+ .values().stringType().noDefault().endRecord();
+
+ private static final GenericRecord RECORD_WITH_MAP_KEY_CLASS_PROP = new
GenericRecordBuilder(
+ SCHEMA_RECORD_WITH_MAP_KEY_CLASS_PROP).set("id", "123")
+ .set("prices", Map.of("-0.0002", "cheap", "12345.678",
"expensive")).build();
+
+ /**
+ * Reusable round-trip logic for a record, using the given model.
+ */
+ public static GenericRecord roundTrip(GenericRecord record, GenericData
model) throws IOException {
+ byte[] serialized;
+
+ try (ByteArrayOutputStream baos = new ByteArrayOutputStream()) {
+ GenericDatumWriter<GenericRecord> writer = new
GenericDatumWriter<>(record.getSchema());
+ BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(baos, null);
+ writer.write(record, encoder);
+ encoder.flush();
+ serialized = baos.toByteArray();
+ }
+
+ GenericDatumReader<GenericRecord> reader = new
GenericDatumReader<>(record.getSchema(), record.getSchema(), model);
+ BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(serialized,
null);
+ return reader.read(null, decoder);
+ }
+
/**
- * Tests that GenericDatumReader can deserialize records with string fields
that
- * have a "java-class" attribute (e.g., BigDecimal).
- *
- * This test reproduces a bug where
+ * Tests that a plain GenericDatumReader (GenericData model) ignores the
+ * {@link SpecificData#CLASS_PROP} attribute on a string field inside a
union,
+ * matching the classic (non fast-reader) behavior of GenericData.
+ * <p>
+ * This test also reproduces a bug (AVRO-4225) where
* FastReaderBuilder.getTransformingStringReader() casts the result of
- * stringReader.read() directly to String, but in GenericData mode the reader
- * returns Utf8, causing a ClassCastException.
+ * stringReader.read() directly to String, but GenericData returns Utf8,
causing
+ * a ClassCastException
*/
@Test
- void genericDatumReaderWithJavaClassAttribute() throws IOException {
- // Schema with a string field that has "java-class": "java.math.BigDecimal"
- // This is a common pattern for representing decimal values as strings
- String schemaJson = "{\n" + " \"type\": \"record\",\n" + " \"name\":
\"TestRecord\",\n" + " \"fields\": [\n"
- + " {\"name\": \"id\", \"type\": \"string\"},\n" + " {\"name\":
\"price\", \"type\": [\"null\", {\n"
- + " \"type\": \"string\",\n" + " \"java-class\":
\"java.math.BigDecimal\"\n" + " }]}\n" + " ]\n"
- + "}";
-
- Schema schema = new Schema.Parser().parse(schemaJson);
-
- GenericRecord record = new GenericData.Record(schema);
- record.put("id", "123");
- record.put("price", "-0.0002");
-
- ByteArrayOutputStream out = new ByteArrayOutputStream();
- GenericDatumWriter<GenericRecord> writer = new
GenericDatumWriter<>(schema);
- BinaryEncoder encoder = EncoderFactory.get().binaryEncoder(out, null);
- writer.write(record, encoder);
- encoder.flush();
-
- byte[] serialized = out.toByteArray();
-
- // Deserialize using GenericDatumReader (which uses FastReaderBuilder by
- // default)
- GenericDatumReader<GenericRecord> reader = new
GenericDatumReader<>(schema);
- BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(serialized,
null);
+ void genericDataModelIgnoresJavaClassPropWithStringUnion() throws
IOException {
+ // This round trip shouldn't cause a ClassCastException (AVRO-4225)
+ GenericRecord result = roundTrip(RECORD_WITH_NULLABLE_CLASS_PROP,
GenericData.get());
+
+ assertNotNull(result);
+ assertInstanceOf(Utf8.class, result.get("id"));
+ assertEquals("123", result.get("id").toString());
+ assertInstanceOf(Utf8.class, result.get("price"), "GenericData should
ignore 'java-class'");
+ assertEquals("-0.0002", result.get("price").toString());
+ }
- // AVRO-4225 this should not throw ClassCastException: Utf8 cannot be cast
- // to String
- GenericRecord result = reader.read(null, decoder);
+ /**
+ * Tests that a plain GenericDatumReader (GenericData model) ignores the
+ * {@link SpecificData#CLASS_PROP} attribute on a direct (non-union) string
+ * field.
+ */
+ @Test
+ void genericDataModelIgnoresJavaClassPropWithString() throws IOException {
+ GenericRecord result = roundTrip(RECORD_WITH_CLASS_PROP,
GenericData.get());
assertNotNull(result);
+ assertInstanceOf(Utf8.class, result.get("id"));
assertEquals("123", result.get("id").toString());
+ assertInstanceOf(Utf8.class, result.get("price"), "GenericData should
ignore 'java-class'");
assertEquals("-0.0002", result.get("price").toString());
}
/**
- * Tests that GenericDatumReader can deserialize records with a direct string
- * field (not in a union) that has a "java-class" attribute.
+ * Tests that a GenericDatumReader built on the SpecificData model uses
+ * {@link SpecificData#CLASS_PROP} on a string field inside a union,
+ * transforming it into the named class without throwing ClassCastException.
*/
@Test
- void genericDatumReaderWithDirectJavaClassString() throws IOException {
- String schemaJson = "{\n" + " \"type\": \"record\",\n" + " \"name\":
\"TestRecord\",\n" + " \"fields\": [\n"
- + " {\"name\": \"amount\", \"type\": {\n" + " \"type\":
\"string\",\n"
- + " \"java-class\": \"java.math.BigDecimal\"\n" + " }}\n" + "
]\n" + "}";
+ void specificDataModelUsesJavaClassProp() throws IOException {
+ GenericRecord result = roundTrip(RECORD_WITH_NULLABLE_CLASS_PROP,
SpecificData.get());
Review Comment:
This test should explicitly enable fast-reader on a fresh SpecificData
instance to ensure it validates the FastReaderBuilder path regardless of
JVM/system-property configuration.
This issue also appears in the following locations of the same file:
- line 152
- line 192
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
