Fixes FasterXML#601: Jackson subtype schema unions are non-deterministic (FasterXML#613)

Author: beseder1

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

@@ -101,9 +101,8 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
                 // (see org.apache.avro.Schema.RecordSchema#computeHash).
                 // Therefore, unionSchemas must not be HashSet (or any other type
                 // using hashCode() for equality check).
-                // Set ensures that each subType schema is once in resulting union.
-                // IdentityHashMap is used because it is using reference-equality.
-                final Set<Schema> unionSchemas = Collections.newSetFromMap(new IdentityHashMap<>());
+                // ArrayList ensures that ordering of subTypes is preserved.
+                final List<Schema> unionSchemas = new ArrayList<>();
                 // Initialize with this schema
                 if (_type.isConcrete()) {
                     unionSchemas.add(_typeSchema);
@@ -126,7 +125,7 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
                             unionSchemas.add(subTypeSchema);
                         }
                     }
-                    _avroSchema = Schema.createUnion(new ArrayList<>(unionSchemas));
+                    _avroSchema = Schema.createUnion(deduplicateByReference(unionSchemas));
                 } catch (JsonMappingException jme) {
                     throw new RuntimeJsonMappingException("Failed to build schema", jme);
                 }
@@ -135,6 +134,19 @@ public RecordVisitor(SerializerProvider p, JavaType type, VisitorFormatWrapperIm
         _visitorWrapper.getSchemas().addSchema(type, _avroSchema);
     }
 
+    private static List<Schema> deduplicateByReference(List<Schema> schemas) {
+        final List<Schema> result = new ArrayList<>();
+        // Set based on IdentityHashMap is used because we need to deduplicate by reference.
+        final Set<Schema> seenSchemas = Collections.newSetFromMap(new IdentityHashMap<>());
+
+        for(Schema s : schemas) {
+            if(seenSchemas.add(s)) {
+                result.add(s);             // preserve order
+            }
+        }
+        return result;
+    }
+
     @Override
     public Schema builtAvroSchema() {
         if (!_overridden) {

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

@@ -264,9 +264,17 @@ public void base_class_explicitly_in_Union_annotation_test() throws Exception {
     }
 
     @Union({
-            // Interface being explicitly in @Union led to StackOverflowError exception.
-            DocumentInterface.class,
-            Word.class, Excel.class})
+      // Interface being explicitly in @Union led to StackOverflowError exception.
+      DocumentInterface.class,
+      // We added a bunch of implementations to test deterministic ordering of the schemas' subtypes ordering.
+      Word.class,
+      Excel.class,
+      Pdf.class,
+      PowerPoint.class,
+      TextDocument.class,
+      Markdown.class,
+      HtmlDocument.class
+    })
     interface DocumentInterface {
     }
 
@@ -276,11 +284,32 @@ static class Word implements DocumentInterface {
     static class Excel implements DocumentInterface {
     }
 
+    static class Pdf implements DocumentInterface {
+    }
+
+    static class PowerPoint implements DocumentInterface {
+    }
+
+    static class TextDocument implements DocumentInterface {
+    }
+
+    static class Markdown implements DocumentInterface {
+    }
+
+    static class HtmlDocument implements DocumentInterface {
+    }
+
+
     @Test
     public void interface_explicitly_in_Union_annotation_test() throws Exception {
         // GIVEN
         final Schema wordSchema = MAPPER.schemaFor(Word.class).getAvroSchema();
         final Schema excelSchema = MAPPER.schemaFor(Excel.class).getAvroSchema();
+        final Schema pdfSchema = MAPPER.schemaFor(Pdf.class).getAvroSchema();
+        final Schema powerPointSchema = MAPPER.schemaFor(PowerPoint.class).getAvroSchema();
+        final Schema textSchema = MAPPER.schemaFor(TextDocument.class).getAvroSchema();
+        final Schema markdownSchema = MAPPER.schemaFor(Markdown.class).getAvroSchema();
+        final Schema htmlSchema = MAPPER.schemaFor(HtmlDocument.class).getAvroSchema();
 
         // WHEN
         Schema actualSchema = MAPPER.schemaFor(DocumentInterface.class).getAvroSchema();
@@ -289,6 +318,16 @@ public void interface_explicitly_in_Union_annotation_test() throws Exception {
 
         // THEN
         assertThat(actualSchema.getType()).isEqualTo(Schema.Type.UNION);
-        assertThat(actualSchema.getTypes()).containsExactlyInAnyOrder(wordSchema, excelSchema);
+
+        // Deterministic order: exactly as declared in @Union (excluding the interface).
+        assertThat(actualSchema.getTypes()).containsExactly(
+          wordSchema,
+          excelSchema,
+          pdfSchema,
+          powerPointSchema,
+          textSchema,
+          markdownSchema,
+          htmlSchema
+        );
     }
 }