From 37ce2be974fc8c1a328127b01231329810aafead Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Sat, 5 Jul 2025 01:37:01 +0900 Subject: [PATCH 1/7] 5215 fixer PoC --- .../introspect/POJOPropertiesCollector.java | 27 +++++++- ...AnyGetterSerializationOrderChangeTest.java | 67 +++++++++++++++++++ .../POJOPropertiesCollectorTest.java | 2 +- 3 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 2e1bc148c0..d1f1fdd0a7 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -501,6 +501,28 @@ protected void collectAll() _collected = true; } + /* + Put anyGetter in the end + */ + private Map _putAnyGettersInTheEnd(Map all, Map props) { + for (POJOPropertyBuilder prop : props.values()) { + all.put(prop.getName(), prop); + } + Map newAll = new LinkedHashMap(props.size()+props.size()); + POJOPropertyBuilder anyGetterProp = null; + for (POJOPropertyBuilder prop : all.values()) { + if (prop.getAccessor() != null && Boolean.TRUE.equals(this._config.getAnnotationIntrospector().hasAnyGetter(prop.getAccessor()))) { + anyGetterProp = prop; + } else { + newAll.put(prop.getName(), prop); + } + } + if (anyGetterProp != null) { + newAll.put(anyGetterProp.getName(), anyGetterProp); + } + return newAll; + } + /* /********************************************************************** /* Property introspection: Fields @@ -1593,9 +1615,8 @@ protected void _sortProperties(Map props) all = new LinkedHashMap(size+size); } - for (POJOPropertyBuilder prop : props.values()) { - all.put(prop.getName(), prop); - } + all = _putAnyGettersInTheEnd(all, props); + Map ordered = new LinkedHashMap<>(size+size); // Ok: primarily by explicit order if (propertyOrder != null) { diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java new file mode 100644 index 0000000000..662efa612b --- /dev/null +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java @@ -0,0 +1,67 @@ +package com.fasterxml.jackson.databind.records; + +import com.fasterxml.jackson.annotation.JsonAnyGetter; +import com.fasterxml.jackson.annotation.JsonAnySetter; +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; + +import org.junit.jupiter.api.Test; + +import java.util.LinkedHashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class AnyGetterSerializationOrderChangeTest + extends DatabindTestUtil +{ + + static class DynaBean { + public String l; + public String j; + public String a; + + protected Map extensions = new LinkedHashMap<>(); + + @JsonAnyGetter + public Map getExtensions() { + return extensions; + } + + @JsonAnySetter + public void addExtension(String name, Object value) { + extensions.put(name, value); + } + } + + /* + /********************************************************** + /* Test cases + /********************************************************** + */ + + private final ObjectMapper MAPPER = JsonMapper.builder() + .enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY) + .configure(SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS, true) + .build(); + + @Test + public void testDynaBean() throws Exception + { + DynaBean b = new DynaBean(); + b.a = "1"; + b.j = "2"; + b.l = "3"; + b.addExtension("z", "5"); + b.addExtension("b", "4"); + assertEquals(a2q("{" + + "'a':'1'," + + "'j':'2'," + + "'l':'3'," + + "'b':'4'," + + "'z':'5'}"), MAPPER.writeValueAsString(b)); + } +} diff --git a/src/test/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollectorTest.java b/src/test/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollectorTest.java index 0c9b67e918..117fc80bc4 100644 --- a/src/test/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollectorTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollectorTest.java @@ -502,7 +502,7 @@ public void testDuplicateGetters() throws Exception assertTrue(prop.getGetter().hasAnnotation(B.class)); } - @Test +// @Test public void testDuplicateGettersCreator() throws Exception { POJOPropertiesCollector coll = collector(MAPPER, DuplicateGetterCreatorBean.class, true); From cc4d11b3dcf6617c0ef854235e5c109fc6724992 Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Sat, 5 Jul 2025 01:44:18 +0900 Subject: [PATCH 2/7] Add JavaDoc --- .../jackson/databind/introspect/POJOPropertiesCollector.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index d1f1fdd0a7..dc71212791 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -501,8 +501,8 @@ protected void collectAll() _collected = true; } - /* - Put anyGetter in the end + /** + * Put anyGetter in the end, before actual sorting further down {@link POJOPropertiesCollector#_sortProperties(Map)} */ private Map _putAnyGettersInTheEnd(Map all, Map props) { for (POJOPropertyBuilder prop : props.values()) { From da60f7bb52304e8d0066caef51f04e8556777be7 Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Sat, 5 Jul 2025 01:45:06 +0900 Subject: [PATCH 3/7] Add issue number --- .../jackson/databind/introspect/POJOPropertiesCollector.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index dc71212791..1289ce104a 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -502,6 +502,7 @@ protected void collectAll() } /** + * [databind#5215] JsonAnyGetter Serializer behavior change from 2.18.4 to 2.19.0 * Put anyGetter in the end, before actual sorting further down {@link POJOPropertiesCollector#_sortProperties(Map)} */ private Map _putAnyGettersInTheEnd(Map all, Map props) { From 59339ab5e9e0b827f5e60c2986e310ac72f68968 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 8 Jul 2025 18:30:08 -0700 Subject: [PATCH 4/7] Minor clean up --- .../databind/introspect/POJOPropertiesCollector.java | 8 +++++--- .../records/AnyGetterSerializationOrderChangeTest.java | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 1289ce104a..2510192e7c 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -505,14 +505,16 @@ protected void collectAll() * [databind#5215] JsonAnyGetter Serializer behavior change from 2.18.4 to 2.19.0 * Put anyGetter in the end, before actual sorting further down {@link POJOPropertiesCollector#_sortProperties(Map)} */ - private Map _putAnyGettersInTheEnd(Map all, Map props) { + private Map _putAnyGettersInTheEnd(Map all, + Map props) + { for (POJOPropertyBuilder prop : props.values()) { all.put(prop.getName(), prop); } - Map newAll = new LinkedHashMap(props.size()+props.size()); + Map newAll = new LinkedHashMap<>(props.size() * 2); POJOPropertyBuilder anyGetterProp = null; for (POJOPropertyBuilder prop : all.values()) { - if (prop.getAccessor() != null && Boolean.TRUE.equals(this._config.getAnnotationIntrospector().hasAnyGetter(prop.getAccessor()))) { + if (prop.getAccessor() != null && Boolean.TRUE.equals(_config.getAnnotationIntrospector().hasAnyGetter(prop.getAccessor()))) { anyGetterProp = prop; } else { newAll.put(prop.getName(), prop); diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java index 662efa612b..7d86778a76 100644 --- a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java @@ -15,10 +15,10 @@ import static org.junit.jupiter.api.Assertions.assertEquals; +// For [databind#5215] public class AnyGetterSerializationOrderChangeTest extends DatabindTestUtil { - static class DynaBean { public String l; public String j; From 8a1d9bf31b52f0186fb0245839dc7c8c9c33bf92 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 8 Jul 2025 18:41:03 -0700 Subject: [PATCH 5/7] More clean up --- .../introspect/POJOPropertiesCollector.java | 20 ++++++++++++------- .../POJOPropertiesCollectorTest.java | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 2510192e7c..a73b1f3e5d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -505,15 +505,21 @@ protected void collectAll() * [databind#5215] JsonAnyGetter Serializer behavior change from 2.18.4 to 2.19.0 * Put anyGetter in the end, before actual sorting further down {@link POJOPropertiesCollector#_sortProperties(Map)} */ - private Map _putAnyGettersInTheEnd(Map all, - Map props) + private Map _putAnyGettersInTheEnd( + Map allProps, + Map sortedProps) { - for (POJOPropertyBuilder prop : props.values()) { - all.put(prop.getName(), prop); + // First: see if we have any any-getters + if (_anyGetters == null && _anyGetterField == null) { + return allProps; } - Map newAll = new LinkedHashMap<>(props.size() * 2); + + for (POJOPropertyBuilder prop : allProps.values()) { + sortedProps.put(prop.getName(), prop); + } + Map newAll = new LinkedHashMap<>(allProps.size() * 2); POJOPropertyBuilder anyGetterProp = null; - for (POJOPropertyBuilder prop : all.values()) { + for (POJOPropertyBuilder prop : sortedProps.values()) { if (prop.getAccessor() != null && Boolean.TRUE.equals(_config.getAnnotationIntrospector().hasAnyGetter(prop.getAccessor()))) { anyGetterProp = prop; } else { @@ -1618,7 +1624,7 @@ protected void _sortProperties(Map props) all = new LinkedHashMap(size+size); } - all = _putAnyGettersInTheEnd(all, props); + all = _putAnyGettersInTheEnd(props, all); Map ordered = new LinkedHashMap<>(size+size); // Ok: primarily by explicit order diff --git a/src/test/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollectorTest.java b/src/test/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollectorTest.java index 117fc80bc4..0c9b67e918 100644 --- a/src/test/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollectorTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollectorTest.java @@ -502,7 +502,7 @@ public void testDuplicateGetters() throws Exception assertTrue(prop.getGetter().hasAnnotation(B.class)); } -// @Test + @Test public void testDuplicateGettersCreator() throws Exception { POJOPropertiesCollector coll = collector(MAPPER, DuplicateGetterCreatorBean.class, true); From cd366f366974baca2147f76e31d8f6dff8040450 Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 8 Jul 2025 18:48:00 -0700 Subject: [PATCH 6/7] Fix prev commit --- .../databind/introspect/POJOPropertiesCollector.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index a73b1f3e5d..83dd5c81c6 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -509,14 +509,14 @@ private Map _putAnyGettersInTheEnd( Map allProps, Map sortedProps) { - // First: see if we have any any-getters - if (_anyGetters == null && _anyGetterField == null) { - return allProps; - } - + // First, handle sorting caller expects: for (POJOPropertyBuilder prop : allProps.values()) { sortedProps.put(prop.getName(), prop); } + // Then re-order if needed + if (_anyGetters == null && _anyGetterField == null) { + return sortedProps; + } Map newAll = new LinkedHashMap<>(allProps.size() * 2); POJOPropertyBuilder anyGetterProp = null; for (POJOPropertyBuilder prop : sortedProps.values()) { From d0c99da7f10a16f8637cc4f68dfc7e613c1974ff Mon Sep 17 00:00:00 2001 From: Tatu Saloranta Date: Tue, 8 Jul 2025 19:44:18 -0700 Subject: [PATCH 7/7] Final touches... --- .../introspect/POJOPropertiesCollector.java | 32 +++++++++++-------- .../introspect/POJOPropertyBuilder.java | 27 +++++++++++++--- .../records/RecordJsonSerDeser188Test.java | 1 - .../ser/AnyGetterOrdering5215Test.java} | 12 +++---- 4 files changed, 48 insertions(+), 24 deletions(-) rename src/{test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java => test/java/com/fasterxml/jackson/databind/ser/AnyGetterOrdering5215Test.java} (84%) diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 83dd5c81c6..8ad07a53a8 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -506,21 +506,24 @@ protected void collectAll() * Put anyGetter in the end, before actual sorting further down {@link POJOPropertiesCollector#_sortProperties(Map)} */ private Map _putAnyGettersInTheEnd( - Map allProps, Map sortedProps) { - // First, handle sorting caller expects: - for (POJOPropertyBuilder prop : allProps.values()) { - sortedProps.put(prop.getName(), prop); - } - // Then re-order if needed - if (_anyGetters == null && _anyGetterField == null) { + AnnotatedMember anyAccessor; + + if (_anyGetters != null) { + anyAccessor = _anyGetters.getFirst(); + } else if (_anyGetterField != null) { + anyAccessor = _anyGetterField.getFirst(); + } else { return sortedProps; } - Map newAll = new LinkedHashMap<>(allProps.size() * 2); + + // Here we'll use insertion-order preserving map, since possible alphabetic + // sorting already done earlier + Map newAll = new LinkedHashMap<>(sortedProps.size() * 2); POJOPropertyBuilder anyGetterProp = null; for (POJOPropertyBuilder prop : sortedProps.values()) { - if (prop.getAccessor() != null && Boolean.TRUE.equals(_config.getAnnotationIntrospector().hasAnyGetter(prop.getAccessor()))) { + if (prop.hasFieldOrGetter(anyAccessor)) { anyGetterProp = prop; } else { newAll.put(prop.getName(), prop); @@ -1619,12 +1622,15 @@ protected void _sortProperties(Map props) Map all; // Need to (re)sort alphabetically? if (sortAlpha) { - all = new TreeMap(); + all = new TreeMap<>(); } else { - all = new LinkedHashMap(size+size); + all = new LinkedHashMap<>(size+size); } - - all = _putAnyGettersInTheEnd(props, all); + // First, handle sorting caller expects: + for (POJOPropertyBuilder prop : props.values()) { + all.put(prop.getName(), prop); + } + all = _putAnyGettersInTheEnd(all); Map ordered = new LinkedHashMap<>(size+size); // Ok: primarily by explicit order diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java index e0c6f497df..088a7f8b30 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertyBuilder.java @@ -1,5 +1,6 @@ package com.fasterxml.jackson.databind.introspect; +import java.lang.reflect.Member; import java.util.*; import java.util.stream.Collectors; @@ -764,6 +765,24 @@ protected int _setterPriority(AnnotatedMethod m) return 2; } + // @since 2.19.2 + public boolean hasFieldOrGetter(AnnotatedMember member) { + return _hasAccessor(_fields, member) || _hasAccessor(_getters, member); + } + + private boolean _hasAccessor(Linked node, + AnnotatedMember memberToMatch) + { + // AnnotatedXxx are not canonical, but underlying JDK Members are: + final Member rawMemberToMatch = memberToMatch.getMember(); + for (; node != null; node = node.next) { + if (node.value.getMember() == rawMemberToMatch) { + return true; + } + } + return false; + } + /* /********************************************************** /* Implementations of refinement accessors @@ -877,19 +896,19 @@ public JsonProperty.Access withMember(AnnotatedMember member) { */ public void addField(AnnotatedField a, PropertyName name, boolean explName, boolean visible, boolean ignored) { - _fields = new Linked(a, _fields, name, explName, visible, ignored); + _fields = new Linked<>(a, _fields, name, explName, visible, ignored); } public void addCtor(AnnotatedParameter a, PropertyName name, boolean explName, boolean visible, boolean ignored) { - _ctorParameters = new Linked(a, _ctorParameters, name, explName, visible, ignored); + _ctorParameters = new Linked<>(a, _ctorParameters, name, explName, visible, ignored); } public void addGetter(AnnotatedMethod a, PropertyName name, boolean explName, boolean visible, boolean ignored) { - _getters = new Linked(a, _getters, name, explName, visible, ignored); + _getters = new Linked<>(a, _getters, name, explName, visible, ignored); } public void addSetter(AnnotatedMethod a, PropertyName name, boolean explName, boolean visible, boolean ignored) { - _setters = new Linked(a, _setters, name, explName, visible, ignored); + _setters = new Linked<>(a, _setters, name, explName, visible, ignored); } /** diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordJsonSerDeser188Test.java b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordJsonSerDeser188Test.java index 73e6c14d3d..6407074c59 100644 --- a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordJsonSerDeser188Test.java +++ b/src/test-jdk17/java/com/fasterxml/jackson/databind/records/RecordJsonSerDeser188Test.java @@ -48,7 +48,6 @@ public void serialize(String value, JsonGenerator jgen, SerializerProvider provi } } - @SuppressWarnings("serial") static class PrefixStringDeserializer extends StdScalarDeserializer { private static final long serialVersionUID = 1L; diff --git a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java b/src/test/java/com/fasterxml/jackson/databind/ser/AnyGetterOrdering5215Test.java similarity index 84% rename from src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java rename to src/test/java/com/fasterxml/jackson/databind/ser/AnyGetterOrdering5215Test.java index 7d86778a76..0e38fca74a 100644 --- a/src/test-jdk17/java/com/fasterxml/jackson/databind/records/AnyGetterSerializationOrderChangeTest.java +++ b/src/test/java/com/fasterxml/jackson/databind/ser/AnyGetterOrdering5215Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.databind.records; +package com.fasterxml.jackson.databind.ser; import com.fasterxml.jackson.annotation.JsonAnyGetter; import com.fasterxml.jackson.annotation.JsonAnySetter; @@ -15,8 +15,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; -// For [databind#5215] -public class AnyGetterSerializationOrderChangeTest +// For [databind#5215]: Any-getter should be sorted last, by default +public class AnyGetterOrdering5215Test extends DatabindTestUtil { static class DynaBean { @@ -38,9 +38,9 @@ public void addExtension(String name, Object value) { } /* - /********************************************************** - /* Test cases - /********************************************************** + /********************************************************************** + /* Test methods + /********************************************************************** */ private final ObjectMapper MAPPER = JsonMapper.builder()