Follow-up to #745: fix BigDecimal union regression, root union-to-map (#753)

Author: cowtowncoder

avro/src/main/java/tools/jackson/dataformat/avro/ser/AvroWriteContext.java

@@ -4,6 +4,7 @@
 import java.math.BigDecimal;
 import java.util.*;
 
+import org.apache.avro.LogicalTypes;
 import org.apache.avro.Schema;
 import org.apache.avro.Schema.Type;
 import org.apache.avro.UnresolvedUnionException;
@@ -434,26 +435,41 @@ private static int _findNotNullIndex(List<Schema> types)
 
     private static int _resolveBigDecimalIndex(Schema unionSchema, List<Schema> types,
             BigDecimal value) {
-        int match = -1;
+        // Branches are considered in order of how well they retain the value,
+        // regardless of declaration order: "decimal" first, then String, then Double
+        int stringMatch = -1;
+        int doubleMatch = -1;
 
         for (int i = 0, size = types.size(); i < size; ++i) {
             Schema schema = types.get(i);
             Schema.Type t = schema.getType();
 
-            // Prefer String or Bytes with logical type info for BigDecimal;
-            // fall back to DOUBLE if nothing better is found
-            if (t == Type.STRING || t == Type.BYTES) {
-                return i;
-            }
-            if (t == Type.DOUBLE) {
-                match = i;
-                continue;
+            if (t == Type.BYTES || t == Type.FIXED) {
+                // Best match: retains both scale and type.
+                // NOTE: plain `bytes`/`fixed` must NOT be chosen, since conversion
+                // requires the "decimal" logical type to exist
+                if (schema.getLogicalType() instanceof LogicalTypes.Decimal) {
+                    return i;
+                }
+            } else if (t == Type.STRING) {
+                // Second best: retains all digits, but reads back as String
+                if (stringMatch < 0) {
+                    stringMatch = i;
+                }
+            } else if (t == Type.DOUBLE) {
+                // Last resort: lossy
+                if (doubleMatch < 0) {
+                    doubleMatch = i;
+                }
             }
         }
-        if (match < 0) {
-            match = ReflectData.get().resolveUnion(unionSchema, value);
+        if (stringMatch >= 0) {
+            return stringMatch;
         }
-        return match;
+        if (doubleMatch >= 0) {
+            return doubleMatch;
+        }
+        return ReflectData.get().resolveUnion(unionSchema, value);
     }
 
     private static int _resolveMapIndex(Schema unionSchema, List<Schema> types,

avro/src/main/java/tools/jackson/dataformat/avro/ser/RootContext.java

@@ -59,12 +59,17 @@ public final AvroWriteContext createChildObjectContext(Object currValue) {
         // verify that root type is record (or compatible)
         switch (_schema.getType()) {
         case RECORD:
-        case UNION: // maybe
             {
                 GenericRecord rec = _createRecord(_schema, currValue);
                 _rootValue = rec;
                 return new ObjectWriteContext(this, _generator, rec, currValue);
             }
+        case UNION: // maybe: may resolve to either Record or Map
+            {
+                AvroWriteContext child = _createObjectContext(_schema, currValue);
+                _rootValue = child.rawValue();
+                return child;
+            }
         case MAP: // used to not be supported
             {
                 MapWriteContext ctxt = new MapWriteContext(this, _generator, _schema, currValue);

avro/src/test/java/tools/jackson/dataformat/avro/BigDecimalSerializationAndDeserializationTest.java

@@ -233,4 +233,110 @@ public void testBigDecimalUnionPrefersBytesOverDouble() throws Exception
         assertThat((BigDecimal) result.get("value")).isEqualByComparingTo(input);
     }
 
+    // Verify that plain BYTES (without "decimal" logical type) is NOT chosen for
+    // BigDecimal: conversion would fail, so DOUBLE must be used instead
+    @Test
+    public void testBigDecimalUnionSkipsPlainBytes() throws Exception
+    {
+        String schemaJson = a2q("{" +
+                "'type':'record'," +
+                "'name':'Test'," +
+                "'fields':[" +
+                "  {'name':'value', 'type':['null','bytes','double']}" +
+                "]" +
+                "}");
+        AvroSchema schema = MAPPER.schemaFrom(schemaJson);
+
+        BigDecimal input = BigDecimal.valueOf(123456789, 6);
+        Map<String, Object> data = Map.of("value", input);
+        byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(data);
+
+        Map<String, Object> result = MAPPER.readerFor(Map.class)
+                .with(schema)
+                .readValue(bytes);
+        // Written as DOUBLE (index 2) since plain `bytes` cannot hold a BigDecimal
+        assertThat(result.get("value")).isInstanceOf(Double.class);
+        assertThat((Double) result.get("value")).isEqualTo(input.doubleValue());
+    }
+
+    // Verify that "decimal" BYTES wins over STRING even when declared later:
+    // preference is by type fidelity, not by declaration order
+    @Test
+    public void testBigDecimalUnionPrefersBytesOverString() throws Exception
+    {
+        String schemaJson = a2q("{" +
+                "'type':'record'," +
+                "'name':'Test'," +
+                "'fields':[" +
+                "  {'name':'value', 'type':['null','string'," +
+                "    {'type':'bytes','logicalType':'decimal','precision':20,'scale':6}," +
+                "    'double']}" +
+                "]" +
+                "}");
+        AvroSchema schema = MAPPER.schemaFrom(schemaJson);
+
+        BigDecimal input = BigDecimal.valueOf(123456789, 6);
+        Map<String, Object> data = Map.of("value", input);
+        byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(data);
+
+        Map<String, Object> result = MAPPER.readerFor(Map.class)
+                .with(schema)
+                .readValue(bytes);
+        assertThat(result.get("value")).isInstanceOf(BigDecimal.class);
+        assertThat((BigDecimal) result.get("value")).isEqualByComparingTo(input);
+    }
+
+    // Verify that FIXED with "decimal" logical type is also usable for BigDecimal
+    @Test
+    public void testBigDecimalUnionPrefersFixedOverString() throws Exception
+    {
+        String schemaJson = a2q("{" +
+                "'type':'record'," +
+                "'name':'Test'," +
+                "'fields':[" +
+                "  {'name':'value', 'type':['null','string'," +
+                "    {'type':'fixed','name':'Dec','size':16," +
+                "     'logicalType':'decimal','precision':20,'scale':6}," +
+                "    'double']}" +
+                "]" +
+                "}");
+        AvroSchema schema = MAPPER.schemaFrom(schemaJson);
+
+        BigDecimal input = BigDecimal.valueOf(123456789, 6);
+        Map<String, Object> data = Map.of("value", input);
+        byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(data);
+
+        Map<String, Object> result = MAPPER.readerFor(Map.class)
+                .with(schema)
+                .readValue(bytes);
+        assertThat(result.get("value")).isInstanceOf(BigDecimal.class);
+        assertThat((BigDecimal) result.get("value")).isEqualByComparingTo(input);
+    }
+
+    // Verify that plain FIXED (without "decimal" logical type) is not chosen either
+    @Test
+    public void testBigDecimalUnionSkipsPlainFixed() throws Exception
+    {
+        String schemaJson = a2q("{" +
+                "'type':'record'," +
+                "'name':'Test'," +
+                "'fields':[" +
+                "  {'name':'value', 'type':['null'," +
+                "    {'type':'fixed','name':'Raw','size':16}," +
+                "    'double']}" +
+                "]" +
+                "}");
+        AvroSchema schema = MAPPER.schemaFrom(schemaJson);
+
+        BigDecimal input = BigDecimal.valueOf(123456789, 6);
+        Map<String, Object> data = Map.of("value", input);
+        byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(data);
+
+        Map<String, Object> result = MAPPER.readerFor(Map.class)
+                .with(schema)
+                .readValue(bytes);
+        assertThat(result.get("value")).isInstanceOf(Double.class);
+        assertThat((Double) result.get("value")).isEqualTo(input.doubleValue());
+    }
+
 }

avro/src/test/java/tools/jackson/dataformat/avro/MapWithUnionTest.java

@@ -131,7 +131,26 @@ public void testMapContainerWithNested() throws Exception
         assertEquals("bing", m.get("zap"));
     }
 
-    // Verify that a union resolving to MAP is handled correctly in _createRecord
+    // Verify that a root-level union resolving to MAP is written as a Map,
+    // and not passed to Record handling
+    @SuppressWarnings("unchecked")
+    @Test
+    public void testRootUnionResolvingToMapType() throws Exception
+    {
+        String schemaJson = a2q("['null',{'type':'map','values':'string'}]");
+        AvroSchema schema = MAPPER.schemaFrom(schemaJson);
+
+        Map<String, Object> input = Map.of("key1", "val1", "key2", "val2");
+        byte[] bytes = MAPPER.writer(schema).writeValueAsBytes(input);
+
+        Map<String, String> result = MAPPER.readerFor(Map.class)
+                .with(schema)
+                .readValue(bytes);
+        assertEquals("val1", result.get("key1"));
+        assertEquals("val2", result.get("key2"));
+    }
+
+    // Verify that a union resolving to MAP is handled correctly for Record properties
     @SuppressWarnings("unchecked")
     @Test
     public void testUnionResolvingToMapType() throws Exception