Skip to content

Commit 1d33114

Browse files
authored
Fixes #601: Jackson subtype schema unions are non-deterministic (#613)
1 parent 70e5a92 commit 1d33114

File tree

2 files changed

+59
-8
lines changed

2 files changed

+59
-8
lines changed

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,8 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
9898
// (see org.apache.avro.Schema.RecordSchema#computeHash).
9999
// Therefore, unionSchemas must not be HashSet (or any other type
100100
// using hashCode() for equality check).
101-
// Set ensures that each subType schema is once in resulting union.
102-
// IdentityHashMap is used because it is using reference-equality.
103-
final Set<Schema> unionSchemas = Collections.newSetFromMap(new IdentityHashMap<>());
101+
// ArrayList ensures that ordering of subTypes is preserved.
102+
final List<Schema> unionSchemas = new ArrayList<>();
104103
// Initialize with this schema
105104
if (_type.isConcrete()) {
106105
unionSchemas.add(_typeSchema);
@@ -126,14 +125,27 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
126125
} catch (JsonMappingException jme) {
127126
throw new RuntimeJsonMappingException("Failed to build schema", jme);
128127
}
129-
_avroSchema = Schema.createUnion(new ArrayList<>(unionSchemas));
128+
_avroSchema = Schema.createUnion(deduplicateByReference(unionSchemas));
130129
} else {
131130
_avroSchema = _typeSchema;
132131
}
133132
}
134133
_visitorWrapper.getSchemas().addSchema(type, _avroSchema);
135134
}
136135

136+
private static List<Schema> deduplicateByReference(List<Schema> schemas) {
137+
final List<Schema> result = new ArrayList<>();
138+
// Set based on IdentityHashMap is used because we need to deduplicate by reference.
139+
final Set<Schema> seenSchemas = Collections.newSetFromMap(new IdentityHashMap<>());
140+
141+
for(Schema s : schemas) {
142+
if(seenSchemas.add(s)) {
143+
result.add(s); // preserve order
144+
}
145+
}
146+
return result;
147+
}
148+
137149
@Override
138150
public Schema builtAvroSchema() {
139151
if (!_overridden) {

avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schema/PolymorphicTypeAnnotationsTest.java

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,9 +254,17 @@ public void base_class_explicitly_in_Union_annotation_test() throws Exception {
254254
}
255255

256256
@Union({
257-
// Interface being explicitly in @Union led to StackOverflowError exception.
258-
DocumentInterface.class,
259-
Word.class, Excel.class})
257+
// Interface being explicitly in @Union led to StackOverflowError exception.
258+
DocumentInterface.class,
259+
// We added a bunch of implementations to test deterministic ordering of the schemas' subtypes ordering.
260+
Word.class,
261+
Excel.class,
262+
Pdf.class,
263+
PowerPoint.class,
264+
TextDocument.class,
265+
Markdown.class,
266+
HtmlDocument.class
267+
})
260268
interface DocumentInterface {
261269
}
262270

@@ -266,11 +274,32 @@ static class Word implements DocumentInterface {
266274
static class Excel implements DocumentInterface {
267275
}
268276

277+
static class Pdf implements DocumentInterface {
278+
}
279+
280+
static class PowerPoint implements DocumentInterface {
281+
}
282+
283+
static class TextDocument implements DocumentInterface {
284+
}
285+
286+
static class Markdown implements DocumentInterface {
287+
}
288+
289+
static class HtmlDocument implements DocumentInterface {
290+
}
291+
292+
269293
@Test
270294
public void interface_explicitly_in_Union_annotation_test() throws Exception {
271295
// GIVEN
272296
final Schema wordSchema = MAPPER.schemaFor(Word.class).getAvroSchema();
273297
final Schema excelSchema = MAPPER.schemaFor(Excel.class).getAvroSchema();
298+
final Schema pdfSchema = MAPPER.schemaFor(Pdf.class).getAvroSchema();
299+
final Schema powerPointSchema = MAPPER.schemaFor(PowerPoint.class).getAvroSchema();
300+
final Schema textSchema = MAPPER.schemaFor(TextDocument.class).getAvroSchema();
301+
final Schema markdownSchema = MAPPER.schemaFor(Markdown.class).getAvroSchema();
302+
final Schema htmlSchema = MAPPER.schemaFor(HtmlDocument.class).getAvroSchema();
274303

275304
// WHEN
276305
Schema actualSchema = MAPPER.schemaFor(DocumentInterface.class).getAvroSchema();
@@ -279,6 +308,16 @@ public void interface_explicitly_in_Union_annotation_test() throws Exception {
279308

280309
// THEN
281310
assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
282-
assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(wordSchema, excelSchema);
311+
312+
// Deterministic order: exactly as declared in @Union (excluding the interface).
313+
assertThat(actualSchema.getTypes()).containsExactly(
314+
wordSchema,
315+
excelSchema,
316+
pdfSchema,
317+
powerPointSchema,
318+
textSchema,
319+
markdownSchema,
320+
htmlSchema
321+
);
283322
}
284323
}

0 commit comments

Comments
 (0)