Implement new SerializationFeature.IGNORE_FAILURE_TO_ORDER_MAP_ENTRIES_BY_INCOMPARABLE_KEYS (#4778)

Author: JooHyukKim

release-notes/VERSION-2.x

@@ -20,6 +20,10 @@ Project: jackson-databind
 #4676: Support other enum naming strategies than camelCase
  (requested by @hajdamak)
  (contributed by Lars B)
+#4773: `SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS` should not apply to Maps
+  with uncomparable keys
+ (requested by @nathanukey)
+ (fix by Joo-Hyuk K)
 
 2.18.1 (28-Oct-2024)
 

src/main/java/com/fasterxml/jackson/databind/SerializationFeature.java

@@ -434,6 +434,23 @@ public enum SerializationFeature implements ConfigFeature
      */
     ORDER_MAP_ENTRIES_BY_KEYS(false),
 
+    /**
+     * Feature that determines whether to intentionally fail when the mapper attempts to
+     * order map entries with incomparable keys by accessing the first key of the map.
+     * So depending on the Map implementation, this may not be the same key every time.
+     * <p>
+     * If enabled, will simply fail by throwing an exception.
+     * If disabled, will not throw an exception and instead simply return the original map.
+     * <p>
+     * Note that this feature will apply only when configured to order map entries by keys, either
+     * through annotation or enabling {@link #ORDER_MAP_ENTRIES_BY_KEYS}.
+     * <p>
+     * Feature is enabled by default and will default false in Jackson 3 and later.
+     *
+     * @since 2.19
+     */
+    FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY(true),
+
     /*
     /******************************************************
     /* Other

src/main/java/com/fasterxml/jackson/databind/ser/std/MapSerializer.java

@@ -1166,6 +1166,26 @@ protected Map<?,?> _orderEntries(Map<?,?> input, JsonGenerator gen,
         if (input instanceof SortedMap<?,?>) {
             return input;
         }
+        // or is it empty? then no need to sort either
+        if (input.isEmpty()) {
+            return input;
+        }
+        // [databind#4773] Since 2.19: We should not try sorting Maps with uncomparable keys
+        // And first key is a good enough sample for now.
+        Object firstKey = input.keySet().iterator().next();
+        if (!Comparable.class.isInstance(firstKey)) {
+            // We cannot sort incomparable keys, should we fail or just skip sorting?
+            if (!provider.isEnabled(SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY)) {
+                return input;
+            } else {
+                Class<?> clazz = firstKey == null ? Object.class : firstKey.getClass();
+                provider.reportBadDefinition(clazz,
+                    String.format("Cannot order Map entries by key of incomparable type %s, consider disabling " +
+                                "`SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY` to simply skip sorting",
+                                ClassUtil.classNameOf(firstKey)));
+            }
+        }
+
         // [databind#1411]: TreeMap does not like null key... (although note that
         //   check above should prevent this code from being called in that case)
         // [databind#153]: but, apparently, some custom Maps do manage hit this

src/test/java/com/fasterxml/jackson/databind/tofix/OrderMapEntriesByKeysSerializationFeature4773Test.java renamed to src/test/java/com/fasterxml/jackson/databind/deser/OrderMapEntriesByKeysSerializationFeature4773Test.java

@@ -1,4 +1,4 @@
-package com.fasterxml.jackson.databind.tofix;
+package com.fasterxml.jackson.databind.deser;
 
 import org.junit.jupiter.api.Test;
 
@@ -8,11 +8,10 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
 import com.fasterxml.jackson.databind.testutil.DatabindTestUtil;
-import com.fasterxml.jackson.databind.testutil.failure.JacksonTestFailureExpected;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 // [databind#4773] Test to verify `SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS` behavior
 //     when serializing `Map` instances with un-comparable keys.
@@ -32,9 +31,8 @@ public static class ObjectContainer4773 {
             .configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true)
             .build();
 
-    @JacksonTestFailureExpected
     @Test
-    void testSerializationWithIncomparableKeys()
+    void testSerializationFailureWhenEnabledWithIncomparableKeys()
             throws Exception
     {
         // Given
@@ -44,11 +42,15 @@ void testSerializationWithIncomparableKeys()
 
         // When : Throws exception
         // com.fasterxml.jackson.databind.JsonMappingException: class java.util.Currency cannot be cast to class java.lang.Comparable
-        String jsonResult = objectMapper.writeValueAsString(entity);
-
-        // Then : Order should not matter, just plain old serialize
-        assertTrue(jsonResult.contains("GBP"));
-        assertTrue(jsonResult.contains("AUD"));
+        try {
+            objectMapper.writer()
+                .with(SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY)
+                .writeValueAsString(entity);
+            fail("Should not pass");
+        } catch (InvalidDefinitionException e) {
+            // Then
+            verifyException(e, "Cannot order Map entries by key of incomparable type");
+        }
     }
 
     @Test
@@ -75,4 +77,24 @@ void testSerializationWithGenericObjectKeys()
                 "'5':'N_TEXT'}}"), jsonResult);
     }
 
+    @Test
+    void testSerWithNullType()
+            throws Exception
+    {
+        // Given : Mixed keys with incomparable `Currency` and comparable `Integer`
+        ObjectContainer4773 entity = new ObjectContainer4773();
+        entity.exampleMap.put(null, "AUD_TEXT");
+
+        // When : Throws exception
+        try {
+            objectMapper.writer()
+                .with(SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY)
+                .writeValueAsString(entity);
+            fail("Should not pass");
+        } catch (InvalidDefinitionException e) {
+            // Then
+            verifyException(e, "Cannot order Map entries by key of incomparable type [null]");
+        }
+    }
+
 }

src/test/java/com/fasterxml/jackson/databind/ser/jdk/MapSerializationTest.java

@@ -164,7 +164,8 @@ public void testOrderByKey() throws IOException
     @Test
     public void testOrderByWithNulls() throws IOException
     {
-        ObjectWriter sortingW = MAPPER.writer(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS);
+        ObjectWriter sortingW = MAPPER.writer(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS)
+                .without(SerializationFeature.FAIL_ON_ORDER_MAP_BY_INCOMPARABLE_KEY);
         // 16-Oct-2016, tatu: but mind the null key, if any
         Map<String,Integer> mapWithNullKey = new LinkedHashMap<String,Integer>();
         mapWithNullKey.put(null, 1);