Skip to content

Commit 63e3481

Browse files
MichalFoksaMichalFoksa
committed
#589: Record visitor includes concrete base class (base class is not abstract or interface) into union of types.
1 parent 0922af7 commit 63e3481

File tree

2 files changed

+193
-17
lines changed

2 files changed

+193
-17
lines changed

‎avro/src/main/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor.java

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ public class RecordVisitor
3131
*/
3232
protected final boolean _overridden;
3333

34+
/**
35+
* When Avro schema for this JavaType ({@code _type}) results in UNION of multiple Avro types, _typeSchema keeps track
36+
* which Avro type in the UNION represents this JavaType ({@code _type}) so that fields of this JavaType can be set to the right Avro type by {@code builtAvroSchema()}.
37+
*/
38+
// TODO: When _thisSchema is not null, then _overridden must be true, therefore (_overridden == true) can be replaced with (_thisSchema != null),
39+
// but it might be considered API change cause _overridden has protected access modifier.
40+
private Schema _typeSchema;
3441
protected Schema _avroSchema;
3542

3643
protected List<Schema.Field> _fields = new ArrayList<>();
@@ -42,32 +49,45 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
4249
_visitorWrapper = visitorWrapper;
4350
// Check if the schema for this record is overridden
4451
BeanDescription bean = getProvider().getConfig().introspectDirectClassAnnotations(_type);
45-
List<NamedType> subTypes = getProvider().getAnnotationIntrospector().findSubtypes(bean.getClassInfo());
4652
AvroSchema ann = bean.getClassInfo().getAnnotation(AvroSchema.class);
4753
if (ann != null) {
4854
_avroSchema = AvroSchemaHelper.parseJsonSchema(ann.value());
4955
_overridden = true;
50-
} else if (subTypes != null && !subTypes.isEmpty()) {
51-
List<Schema> unionSchemas = new ArrayList<>();
52-
try {
53-
for (NamedType subType : subTypes) {
54-
JsonSerializer<?> ser = getProvider().findValueSerializer(subType.getType());
55-
VisitorFormatWrapperImpl visitor = _visitorWrapper.createChildWrapper();
56-
ser.acceptJsonFormatVisitor(visitor, getProvider().getTypeFactory().constructType(subType.getType()));
57-
unionSchemas.add(visitor.getAvroSchema());
58-
}
59-
_avroSchema = Schema.createUnion(unionSchemas);
60-
_overridden = true;
61-
} catch (JsonMappingException jme) {
62-
throw new RuntimeException("Failed to build schema", jme);
63-
}
6456
} else {
65-
_avroSchema = AvroSchemaHelper.initializeRecordSchema(bean);
57+
// If Avro schema for this _type results in UNION I want to know Avro type where to assign fields
58+
_typeSchema = AvroSchemaHelper.initializeRecordSchema(bean);
59+
_avroSchema = _typeSchema;
6660
_overridden = false;
6761
AvroMeta meta = bean.getClassInfo().getAnnotation(AvroMeta.class);
6862
if (meta != null) {
6963
_avroSchema.addProp(meta.key(), meta.value());
7064
}
65+
66+
List<NamedType> subTypes = getProvider().getAnnotationIntrospector().findSubtypes(bean.getClassInfo());
67+
if (subTypes != null && !subTypes.isEmpty()) {
68+
List<Schema> unionSchemas = new ArrayList<>();
69+
// Initialize with this schema
70+
if (_type.isConcrete()) {
71+
unionSchemas.add(_typeSchema);
72+
}
73+
try {
74+
for (NamedType subType : subTypes) {
75+
JsonSerializer<?> ser = getProvider().findValueSerializer(subType.getType());
76+
VisitorFormatWrapperImpl visitor = _visitorWrapper.createChildWrapper();
77+
ser.acceptJsonFormatVisitor(visitor, getProvider().getTypeFactory().constructType(subType.getType()));
78+
Schema subTypeSchema = visitor.getAvroSchema();
79+
// If subType schema is also UNION, include all its types into this union
80+
if (subTypeSchema.getType() == Type.UNION) {
81+
unionSchemas.addAll(subTypeSchema.getTypes());
82+
} else {
83+
unionSchemas.add(subTypeSchema);
84+
}
85+
}
86+
_avroSchema = Schema.createUnion(unionSchemas);
87+
} catch (JsonMappingException jme) {
88+
throw new RuntimeException("Failed to build schema", jme);
89+
}
90+
}
7191
}
7292
_visitorWrapper.getSchemas().addSchema(type, _avroSchema);
7393
}
@@ -76,7 +96,7 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
7696
public Schema builtAvroSchema() {
7797
if (!_overridden) {
7898
// Assumption now is that we are done, so let's assign fields
79-
_avroSchema.setFields(_fields);
99+
_typeSchema.setFields(_fields);
80100
}
81101
return _avroSchema;
82102
}

‎avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/RecordVisitor_schemaCreationTest.java

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
package com.fasterxml.jackson.dataformat.avro.schema;
2+
3+
import com.fasterxml.jackson.annotation.JsonSubTypes;
4+
import com.fasterxml.jackson.databind.JsonMappingException;
5+
import com.fasterxml.jackson.dataformat.avro.AvroMapper;
6+
import com.fasterxml.jackson.dataformat.avro.annotation.AvroNamespace;
7+
import org.apache.avro.Schema;
8+
import org.junit.jupiter.api.Test;
9+
10+
import java.io.IOException;
11+
12+
import static org.assertj.core.api.Assertions.assertThat;
13+
14+
public class RecordVisitor_schemaCreationTest {
15+
16+
private static final AvroMapper MAPPER = AvroMapper.builder().build();
17+
// it is easier maintain string schema representation when namespace is constant, rather than being inferend from this class package name
18+
private static final String TEST_NAMESPACE = "test";
19+
20+
@JsonSubTypes({
21+
@JsonSubTypes.Type(value = Cat.class),
22+
@JsonSubTypes.Type(value = Dog.class),
23+
})
24+
private interface Animal {
25+
}
26+
27+
private static abstract class AbstractMammal implements Animal {
28+
public int legs;
29+
}
30+
31+
static class Cat extends AbstractMammal {
32+
public String color;
33+
}
34+
35+
static class Dog extends AbstractMammal {
36+
public int size;
37+
}
38+
39+
@Test
40+
public void subclasses_of_interface_test() throws JsonMappingException {
41+
// GIVEN
42+
final Schema catSchema = MAPPER.schemaFor(Cat.class).getAvroSchema();
43+
final Schema dogSchema = MAPPER.schemaFor(Dog.class).getAvroSchema();
44+
45+
// WHEN
46+
Schema actualSchema = MAPPER.schemaFor(Animal.class).getAvroSchema();
47+
48+
System.out.println("Animal schema:\n" + actualSchema.toString(true));
49+
50+
// THEN
51+
assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
52+
// Because Animal is interface and Mammal is abstract, they are not expected to be among types in union
53+
assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(catSchema, dogSchema);
54+
}
55+
56+
@JsonSubTypes({
57+
@JsonSubTypes.Type(value = Apple.class),
58+
@JsonSubTypes.Type(value = Pear.class),
59+
})
60+
@AvroNamespace(TEST_NAMESPACE) // @AvroNamespace makes it easier to create schema string representation
61+
static class Fruit {
62+
public boolean eatable;
63+
}
64+
65+
private static final String FRUIT_ITSELF_SCHEMA_STR = "{\"type\":\"record\",\"name\":\"Fruit\",\"namespace\":\"test\",\"fields\":[{\"name\":\"eatable\",\"type\":\"boolean\"}]}";
66+
67+
static class Apple extends Fruit {
68+
public String color;
69+
}
70+
71+
static class Pear extends Fruit {
72+
public int seeds;
73+
}
74+
75+
@Test
76+
public void subclasses_of_concrete_class_test() throws IOException {
77+
// GIVEN
78+
final Schema fruitItselfSchema = MAPPER.schemaFrom(FRUIT_ITSELF_SCHEMA_STR).getAvroSchema();
79+
final Schema appleSchema = MAPPER.schemaFor(Apple.class).getAvroSchema();
80+
final Schema pearSchema = MAPPER.schemaFor(Pear.class).getAvroSchema();
81+
82+
// WHEN
83+
Schema actualSchema = MAPPER.schemaFor(Fruit.class).getAvroSchema();
84+
85+
System.out.println("Fruit schema:\n" + actualSchema.toString(true));
86+
87+
// THEN
88+
assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
89+
assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(fruitItselfSchema, appleSchema, pearSchema);
90+
}
91+
92+
@JsonSubTypes({
93+
@JsonSubTypes.Type(value = LandVehicle.class),
94+
@JsonSubTypes.Type(value = AbstractWaterVehicle.class),
95+
})
96+
@AvroNamespace(TEST_NAMESPACE)
97+
private static class Vehicle {
98+
}
99+
100+
private static final String VEHICLE_ITSELF_SCHEMA_STR = "{\"type\":\"record\",\"name\":\"Vehicle\",\"namespace\":\"test\",\"fields\":[]}";
101+
102+
@JsonSubTypes({
103+
@JsonSubTypes.Type(value = Car.class),
104+
@JsonSubTypes.Type(value = MotorCycle.class),
105+
})
106+
@AvroNamespace(TEST_NAMESPACE)
107+
private static class LandVehicle extends Vehicle {
108+
}
109+
110+
private static final String LAND_VEHICLE_ITSELF_SCHEMA_STR = "{\"type\":\"record\",\"name\":\"LandVehicle\",\"namespace\":\"test\",\"fields\":[]}";
111+
112+
private static class Car extends LandVehicle {
113+
}
114+
115+
private static class MotorCycle extends LandVehicle {
116+
}
117+
118+
@JsonSubTypes({
119+
@JsonSubTypes.Type(value = Boat.class),
120+
@JsonSubTypes.Type(value = Submarine.class),
121+
})
122+
private static abstract class AbstractWaterVehicle extends Vehicle {
123+
public int propellers;
124+
}
125+
126+
private static class Boat extends AbstractWaterVehicle {
127+
}
128+
129+
private static class Submarine extends AbstractWaterVehicle {
130+
}
131+
132+
@Test
133+
public void subclasses_of_subclasses_test() throws IOException {
134+
// GIVEN
135+
final Schema vehicleItselfSchema = MAPPER.schemaFrom(VEHICLE_ITSELF_SCHEMA_STR).getAvroSchema();
136+
final Schema landVehicleItselfSchema = MAPPER.schemaFrom(LAND_VEHICLE_ITSELF_SCHEMA_STR).getAvroSchema();
137+
final Schema carSchema = MAPPER.schemaFor(Car.class).getAvroSchema();
138+
final Schema motorCycleSchema = MAPPER.schemaFor(MotorCycle.class).getAvroSchema();
139+
final Schema boatSchema = MAPPER.schemaFor(Boat.class).getAvroSchema();
140+
final Schema submarineSchema = MAPPER.schemaFor(Submarine.class).getAvroSchema();
141+
142+
// WHEN
143+
Schema actualSchema = MAPPER.schemaFor(Vehicle.class).getAvroSchema();
144+
145+
System.out.println("Vehicle schema:\n" + actualSchema.toString(true));
146+
147+
// THEN
148+
assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
149+
assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(
150+
vehicleItselfSchema,
151+
landVehicleItselfSchema, carSchema, motorCycleSchema,
152+
// AbstractWaterVehicle is not here, because it is abstract
153+
boatSchema, submarineSchema);
154+
}
155+
156+
}

0 commit comments

Comments
 (0)