Skip to content

Commit 94b2377

Browse files
author
Vincent Eigenberger
committed
Fixes #601: Jackson subtype schema unions are non-deterministic
1 parent 57eb753 commit 94b2377

File tree

2 files changed

+60
-8
lines changed

2 files changed

+60
-8
lines changed

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

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,8 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
101101
// (see org.apache.avro.Schema.RecordSchema#computeHash).
102102
// Therefore, unionSchemas must not be HashSet (or any other type
103103
// using hashCode() for equality check).
104-
// Set ensures that each subType schema is once in resulting union.
105-
// IdentityHashMap is used because it is using reference-equality.
106-
final Set<Schema> unionSchemas = Collections.newSetFromMap(new IdentityHashMap<>());
104+
// ArrayList ensures that ordering of subTypes is preserved.
105+
final List<Schema> unionSchemas = new ArrayList<>();
107106
// Initialize with this schema
108107
if (_type.isConcrete()) {
109108
unionSchemas.add(_typeSchema);
@@ -126,7 +125,7 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
126125
unionSchemas.add(subTypeSchema);
127126
}
128127
}
129-
_avroSchema = Schema.createUnion(new ArrayList<>(unionSchemas));
128+
_avroSchema = Schema.createUnion(deduplicateByReference(unionSchemas));
130129
} catch (JsonMappingException jme) {
131130
throw new RuntimeJsonMappingException("Failed to build schema", jme);
132131
}
@@ -135,6 +134,20 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
135134
_visitorWrapper.getSchemas().addSchema(type, _avroSchema);
136135
}
137136

137+
private static List<Schema> deduplicateByReference(List<Schema> schemas) {
138+
final List<Schema> result = new ArrayList<>();
139+
// Set based on IdentityHashMap is used because we need to deduplicate by reference.
140+
final Set<Schema> seenSchemas = Collections.newSetFromMap(new IdentityHashMap<>());
141+
142+
for(Schema s : schemas) {
143+
if(!seenSchemas.contains(s)) {
144+
seenSchemas.add(s); // marks as seen by reference
145+
result.add(s); // preserve order
146+
}
147+
}
148+
return result;
149+
}
150+
138151
@Override
139152
public Schema builtAvroSchema() {
140153
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
@@ -264,9 +264,17 @@ public void base_class_explicitly_in_Union_annotation_test() throws Exception {
264264
}
265265

266266
@Union({
267-
// Interface being explicitly in @Union led to StackOverflowError exception.
268-
DocumentInterface.class,
269-
Word.class, Excel.class})
267+
// Interface being explicitly in @Union led to StackOverflowError exception.
268+
DocumentInterface.class,
269+
// We added a bunch of implementations to test deterministic ordering of the schemas' subtypes ordering.
270+
Word.class,
271+
Excel.class,
272+
Pdf.class,
273+
PowerPoint.class,
274+
TextDocument.class,
275+
Markdown.class,
276+
HtmlDocument.class
277+
})
270278
interface DocumentInterface {
271279
}
272280

@@ -276,11 +284,32 @@ static class Word implements DocumentInterface {
276284
static class Excel implements DocumentInterface {
277285
}
278286

287+
static class Pdf implements DocumentInterface {
288+
}
289+
290+
static class PowerPoint implements DocumentInterface {
291+
}
292+
293+
static class TextDocument implements DocumentInterface {
294+
}
295+
296+
static class Markdown implements DocumentInterface {
297+
}
298+
299+
static class HtmlDocument implements DocumentInterface {
300+
}
301+
302+
279303
@Test
280304
public void interface_explicitly_in_Union_annotation_test() throws Exception {
281305
// GIVEN
282306
final Schema wordSchema = MAPPER.schemaFor(Word.class).getAvroSchema();
283307
final Schema excelSchema = MAPPER.schemaFor(Excel.class).getAvroSchema();
308+
final Schema pdfSchema = MAPPER.schemaFor(Pdf.class).getAvroSchema();
309+
final Schema powerPointSchema = MAPPER.schemaFor(PowerPoint.class).getAvroSchema();
310+
final Schema textSchema = MAPPER.schemaFor(TextDocument.class).getAvroSchema();
311+
final Schema markdownSchema = MAPPER.schemaFor(Markdown.class).getAvroSchema();
312+
final Schema htmlSchema = MAPPER.schemaFor(HtmlDocument.class).getAvroSchema();
284313

285314
// WHEN
286315
Schema actualSchema = MAPPER.schemaFor(DocumentInterface.class).getAvroSchema();
@@ -289,6 +318,16 @@ public void interface_explicitly_in_Union_annotation_test() throws Exception {
289318

290319
// THEN
291320
assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
292-
assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(wordSchema, excelSchema);
321+
322+
// Deterministic order: exactly as declared in @Union (excluding the interface).
323+
assertThat(actualSchema.getTypes()).containsExactly(
324+
wordSchema,
325+
excelSchema,
326+
pdfSchema,
327+
powerPointSchema,
328+
textSchema,
329+
markdownSchema,
330+
htmlSchema
331+
);
293332
}
294333
}

0 commit comments

Comments
 (0)