Skip to content

Implement new SerializationFeature.IGNORE_FAILURE_TO_ORDER_MAP_ENTRIES_BY_INCOMPARABLE_KEYS #4778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Nov 4, 2024
4 changes: 4 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package com.fasterxml.jackson.databind.tofix;
package com.fasterxml.jackson.databind.deser;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed this one: this is serialization, not deserialization test. Will move.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ughh how did I miss that 😂 thank you!


import org.junit.jupiter.api.Test;

Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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]");
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down