[AVRO] Make RecordVisitor _avroSchema and _fields properties final (#600)

MichalFoksa · web-flow · commit 60b2dab3febd · 2025-06-15T10:41:58.000-07:00
diff --git a/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java b/avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java
@@ -62,11 +62,9 @@ public class RecordVisitor
      */
     private final Schema _typeSchema;
 
-    // !!! 19-May-2025: TODO: make final in 2.20
-    protected Schema _avroSchema;
+    protected final Schema _avroSchema;
 
-    // !!! 19-May-2025: TODO: make final in 2.20
-    protected List<Schema.Field> _fields = new ArrayList<>();
+    protected final List<Schema.Field> _fields = new ArrayList<>();
 
     public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperImpl visitorWrapper)
     {
@@ -82,12 +80,11 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
             _typeSchema = null;
         } else {
             // If Avro schema for this _type results in UNION I want to know Avro type where to assign fields
-            _avroSchema = AvroSchemaHelper.initializeRecordSchema(bean);
-            _typeSchema = _avroSchema;
+            _typeSchema = AvroSchemaHelper.initializeRecordSchema(bean);
             _overridden = false;
             AvroMeta meta = bean.getClassInfo().getAnnotation(AvroMeta.class);
             if (meta != null) {
-                _avroSchema.addProp(meta.key(), meta.value());
+                _typeSchema.addProp(meta.key(), meta.value());
             }
 
             List<NamedType> subTypes = getProvider().getAnnotationIntrospector().findSubtypes(bean.getClassInfo());
@@ -126,10 +123,12 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
                             unionSchemas.add(subTypeSchema);
                         }
                     }
-                    _avroSchema = Schema.createUnion(new ArrayList<>(unionSchemas));
                 } catch (JsonMappingException jme) {
                     throw new RuntimeJsonMappingException("Failed to build schema", jme);
                 }
+                _avroSchema = Schema.createUnion(new ArrayList<>(unionSchemas));
+            } else {
+                _avroSchema = _typeSchema;
             }
         }
         _visitorWrapper.getSchemas().addSchema(type, _avroSchema);
diff --git a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/BigDecimal_schemaCreationTest.java b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/BigDecimal_schemaCreationTest.java
@@ -39,8 +39,6 @@ public void testSchemaCreation_withLogicalTypesDisabled_onBigDecimalWithAvroDeci
         // because logical types are disabled by default.
         final Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();
 
-        System.out.println(BigDecimalWithAvroDecimalAnnotationWrapper.class.getSimpleName() + " schema:" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getField("bigDecimalValue")).isNotNull();
         Schema bigDecimalValue = actualSchema.getField("bigDecimalValue").schema();
@@ -59,8 +57,6 @@ public void testSchemaCreation_withLogicalTypesEnabled_onBigDecimalWithAvroDecim
         MAPPER.acceptJsonFormatVisitor(BigDecimalWithAvroDecimalAnnotationWrapper.class, gen);
         final Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();
 
-        System.out.println(BigDecimalWithAvroDecimalAnnotationWrapper.class.getSimpleName() + " schema:" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getField("bigDecimalValue")).isNotNull();
         Schema bigDecimalValue = actualSchema.getField("bigDecimalValue").schema();
@@ -90,8 +86,6 @@ public void testSchemaCreation_withLogicalTypesEnabled_onBigDecimalWithAvroDecim
         MAPPER.acceptJsonFormatVisitor(BigDecimalWithAvroDecimalAnnotationToFixedWrapper.class, gen);
         final Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();
 
-        System.out.println(BigDecimalWithAvroDecimalAnnotationToFixedWrapper.class.getSimpleName() + " schema:" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getField("bigDecimalValue")).isNotNull();
 
diff --git a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/jsr310/AvroJavaTimeModule_schemaCreationTest.java b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/jsr310/AvroJavaTimeModule_schemaCreationTest.java
@@ -47,8 +47,6 @@ public void testSchemaCreation(Class<?> testClass,
         mapper.acceptJsonFormatVisitor(testClass, gen);
         Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();
 
-//        System.out.println(testClass.getName() + " schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo(expectedType);
         assertThat(actualSchema.getProp(LogicalType.LOGICAL_TYPE_PROP)).isEqualTo(expectedLogicalType);
diff --git a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/DateTimeVisitor_builtAvroSchemaTest.java b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/DateTimeVisitor_builtAvroSchemaTest.java
@@ -68,8 +68,6 @@ public void builtAvroSchemaTest(Class<?> testClass,
         // WHEN
         Schema actualSchema = dateTimeVisitor.builtAvroSchema();
 
-//        System.out.println(testClass.getName() + " schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo(expectedAvroType);
         assertThat(actualSchema.getProp(LogicalType.LOGICAL_TYPE_PROP)).isEqualTo(expectedLogicalType);
diff --git a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/Enum_schemaCreationTest.java b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/Enum_schemaCreationTest.java
@@ -27,8 +27,6 @@ public void testJavaEnumToAvroEnum_test() throws JsonMappingException {
         MAPPER.acceptJsonFormatVisitor(NumbersEnum.class , gen);
         Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();
 
-        System.out.println("schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo( Schema.Type.ENUM);
         assertThat(actualSchema.getEnumSymbols()).containsExactlyInAnyOrder("ONE", "TWO", "THREE");
@@ -44,8 +42,6 @@ public void testJavaEnumToAvroString_test() throws JsonMappingException {
         MAPPER.acceptJsonFormatVisitor(NumbersEnum.class , gen);
         Schema actualSchema = gen.getGeneratedSchema().getAvroSchema();
 
-        System.out.println("schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo( Schema.Type.STRING);
 
diff --git a/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/PolymorphicTypeAnnotationsTest.java b/avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/PolymorphicTypeAnnotationsTest.java
@@ -45,8 +45,6 @@ public void subclasses_of_interface_test() throws Exception {
         // WHEN
         Schema actualSchema = MAPPER.schemaFor(AnimalInterface.class).getAvroSchema();
 
-        // System.out.println("Animal schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
         // Because AnimalInterface is interface and AbstractMammal is abstract, they are not expected to be among types in union
@@ -82,8 +80,6 @@ public void jsonSubTypes_on_concrete_class_test() throws Exception {
         // WHEN
         Schema actualSchema = MAPPER.schemaFor(Fruit.class).getAvroSchema();
 
-        // System.out.println("Fruit schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
         assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(fruitItselfSchema, appleSchema, pearSchema);
@@ -142,8 +138,6 @@ public void jsonSubTypes_of_jsonSubTypes_test() throws Exception {
         // WHEN
         Schema actualSchema = MAPPER.schemaFor(Vehicle.class).getAvroSchema();
 
-        // System.out.println("Vehicle schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
         assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(
@@ -187,8 +181,6 @@ public void class_is_referenced_twice_in_hierarchy_test() throws Exception {
         // WHEN
         Schema actualSchema = MAPPER.schemaFor(ElementInterface.class).getAvroSchema();
 
-        // System.out.println("ElementInterface schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
         // ElementInterface and AbstractGas are not concrete classes they are not expected to be among types in union
@@ -223,8 +215,6 @@ public void base_class_explicitly_in_JsonSubTypes_annotation_test() throws Excep
         // WHEN
         Schema actualSchema = MAPPER.schemaFor(Image.class).getAvroSchema();
 
-        // System.out.println("Image schema:\n" + actualSchema.toString(true));
-
         // THEN
         assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
         assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(imageItselfSchema, jpegSchema, pngSchema);

Original file line number	Diff line number	Diff line change
`@@ -62,11 +62,9 @@ public class RecordVisitor`
`62`	`62`	`*/`
`63`	`63`	`private final Schema _typeSchema;`
`64`	`64`
`65`		`- // !!! 19-May-2025: TODO: make final in 2.20`
`66`		`- protected Schema _avroSchema;`
	`65`	`+ protected final Schema _avroSchema;`
`67`	`66`
`68`		`- // !!! 19-May-2025: TODO: make final in 2.20`
`69`		`- protected List<Schema.Field> _fields = new ArrayList<>();`
	`67`	`+ protected final List<Schema.Field> _fields = new ArrayList<>();`
`70`	`68`
`71`	`69`	`public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperImpl visitorWrapper)`
`72`	`70`	`{`
`@@ -82,12 +80,11 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm`
`82`	`80`	`_typeSchema = null;`
`83`	`81`	`} else {`
`84`	`82`	`// If Avro schema for this _type results in UNION I want to know Avro type where to assign fields`
`85`		`- _avroSchema = AvroSchemaHelper.initializeRecordSchema(bean);`
`86`		`- _typeSchema = _avroSchema;`
	`83`	`+ _typeSchema = AvroSchemaHelper.initializeRecordSchema(bean);`
`87`	`84`	`_overridden = false;`
`88`	`85`	`AvroMeta meta = bean.getClassInfo().getAnnotation(AvroMeta.class);`
`89`	`86`	`if (meta != null) {`
`90`		`- _avroSchema.addProp(meta.key(), meta.value());`
	`87`	`+ _typeSchema.addProp(meta.key(), meta.value());`
`91`	`88`	`}`
`92`	`89`
`93`	`90`	`List<NamedType> subTypes = getProvider().getAnnotationIntrospector().findSubtypes(bean.getClassInfo());`
`@@ -126,10 +123,12 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm`
`126`	`123`	`unionSchemas.add(subTypeSchema);`
`127`	`124`	`}`
`128`	`125`	`}`
`129`		`- _avroSchema = Schema.createUnion(new ArrayList<>(unionSchemas));`
`130`	`126`	`} catch (JsonMappingException jme) {`
`131`	`127`	`throw new RuntimeJsonMappingException("Failed to build schema", jme);`
`132`	`128`	`}`
	`129`	`+ _avroSchema = Schema.createUnion(new ArrayList<>(unionSchemas));`
	`130`	`+ } else {`
	`131`	`+ _avroSchema = _typeSchema;`
`133`	`132`	`}`
`134`	`133`	`}`
`135`	`134`	`_visitorWrapper.getSchemas().addSchema(type, _avroSchema);`