(WIP) Fix #4589: remove MapperFeature.SORT_CREATOR_PROPERTIES_BY_DECLARATION_ORDER, change default sorting logic (#4590)

Author: cowtowncoder

src/main/java/tools/jackson/databind/MapperFeature.java

@@ -302,23 +302,6 @@ public enum MapperFeature
      */
     SORT_CREATOR_PROPERTIES_FIRST(true),
 
-    /**
-     * Feature that defines whether Creator properties (ones passed through
-     * constructor or static factory method) should be sorted in their declaration
-     * order if {@link #SORT_CREATOR_PROPERTIES_FIRST} is also enabled.
-     * This is usually used to prevent alphabetic sorting for
-     * Creator properties even if {@link #SORT_PROPERTIES_ALPHABETICALLY} is
-     * enabled for other types of properties.
-     *<p>
-     * NOTE: if {@link #SORT_CREATOR_PROPERTIES_FIRST} is disabled, this feature
-     * has no effect.
-     *<p>
-     * Feature is disabled by default (for backwards compatibility)
-     *
-     * @since 2.18
-     */
-    SORT_CREATOR_PROPERTIES_BY_DECLARATION_ORDER(false),
-
     /*
     /**********************************************************************
     /* Name-related features

src/main/java/tools/jackson/databind/introspect/POJOPropertiesCollector.java

@@ -1530,24 +1530,9 @@ protected void _sortProperties(Map<String, POJOPropertyBuilder> props)
              * order. Related question of creator vs non-creator is punted for now,
              * so creator properties still fully predate non-creator ones.
              */
-            Collection<POJOPropertyBuilder> cr;
-            // 18-Jun-2024, tatu: [databind#4580] We may want to retain declaration
-            //    order regardless
-            boolean sortCreatorPropsByAlpha = sortAlpha
-                    && !_config.isEnabled(MapperFeature.SORT_CREATOR_PROPERTIES_BY_DECLARATION_ORDER);
-            if (sortCreatorPropsByAlpha) {
-                TreeMap<String, POJOPropertyBuilder> sorted =
-                        new TreeMap<String,POJOPropertyBuilder>();
-                for (POJOPropertyBuilder prop : _creatorProperties) {
-                    if (prop != null) {
-                        sorted.put(prop.getName(), prop);
-                    }
-                }
-                cr = sorted.values();
-            } else {
-                cr = _creatorProperties;
-            }
-            for (POJOPropertyBuilder prop : cr) {
+            // 18-Jun-2024, tatu: Except in Jackson 3.0, we do NOT sort creator properties
+            //   alphabetically if they are to be sorted before other properties.
+            for (POJOPropertyBuilder prop : _creatorProperties) {
                 if (prop == null) {
                     continue;
                 }

src/test-jdk17/java/tools/jackson/databind/records/RecordCreatorSerialization4452Test.java

@@ -9,12 +9,13 @@
 
 import tools.jackson.databind.ObjectMapper;
 import tools.jackson.databind.json.JsonMapper;
+import tools.jackson.databind.testutil.DatabindTestUtil;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 // [databind#4452] : JsonProperty not serializing field names properly on JsonCreator in record #4452
-class RecordCreatorSerialization4452Test {
-
+class RecordCreatorSerialization4452Test extends DatabindTestUtil
+{
     public record PlainTestObject(
             @JsonProperty("strField") String testFieldName,
             @JsonProperty("intField") Integer testOtherField
@@ -43,7 +44,7 @@ public CreatorTestObject(
     public void testPlain() throws Exception
     {
         String result = OBJECT_MAPPER.writeValueAsString(new PlainTestObject("test", 1));
-        assertEquals("{\"intField\":1,\"strField\":\"test\"}", result);
+        assertEquals(a2q("{'strField':'test','intField':1}"), result);
     }
 
     @Test

src/test-jdk17/java/tools/jackson/databind/records/RecordExplicitCreatorsTest.java

@@ -153,7 +153,7 @@ public void testDeserializeUsingCanonicalConstructor_WhenJsonPropertyConstructor
         } catch (DatabindException e) {
             verifyException(e, "Unrecognized property \"id\"");
             verifyException(e, "RecordWithTwoJsonPropertyWithoutJsonCreator");
-            verifyException(e, "2 known properties: \"the_email\", \"the_id\"");
+            verifyException(e, "2 known properties: \"the_id\", \"the_email\"");
         }
     }
 

src/test-jdk17/java/tools/jackson/databind/records/RecordSerializationOrderTest.java

@@ -42,7 +42,7 @@ public void testSerializationOrder() throws Exception {
         NestedRecordTwo nestedRecordTwo = new NestedRecordTwo("2", "111110");
         NestedRecordOne nestedRecordOne = new NestedRecordOne("1", "[email protected]", nestedRecordTwo);
         final String output = MAPPER.writeValueAsString(nestedRecordOne);
-        final String expected = "{\"email\":\"[email protected]\",\"id\":\"1\",\"nestedRecordTwo\":{\"id\":\"2\",\"passport\":\"111110\"}}";
+        final String expected = a2q("{'id':'1','email':'[email protected]','nestedRecordTwo':{'id':'2','passport':'111110'}}");
         assertEquals(expected, output);
     }
 
@@ -52,7 +52,7 @@ public void testSerializationOrderWithJsonProperty() throws Exception {
         NestedRecordOneWithJsonProperty nestedRecordOne =
                 new NestedRecordOneWithJsonProperty("1", "[email protected]", nestedRecordTwo);
         final String output = MAPPER.writeValueAsString(nestedRecordOne);
-        final String expected = "{\"email\":\"[email protected]\",\"id\":\"1\",\"nestedProperty\":{\"id\":\"2\",\"passport\":\"111110\"}}";
+        final String expected = a2q("{'id':'1','email':'[email protected]','nestedProperty':{'id':'2','passport':'111110'}}");
         assertEquals(expected, output);
     }
 
@@ -79,21 +79,14 @@ public void testSerializationOrderWithJsonPropertyOrder() throws Exception {
     // [databind#4580]
     @Test
     public void testSerializationOrderWrtCreatorAlphabetic() throws Exception {
-        // In 3.0, sorting by Alphabetic enabled by default so
-        assertEquals(a2q("{'a':'a','b':'b','c':'c'}"),
-                MAPPER.writeValueAsString(new CABRecord("c", "a", "b")));
-        // But can disable
+        // In 3.0, sorting by Alphabetic enabled by default BUT it won't affect Creator props
         assertEquals(a2q("{'c':'c','a':'a','b':'b'}"),
+                MAPPER.writeValueAsString(new CABRecord("c", "a", "b")));
+        // Unless we disable Creator-props-first setting:
+        assertEquals(a2q("{'a':'a','b':'b','c':'c'}"),
                 jsonMapperBuilder()
-                    .disable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY)
+                    .disable(MapperFeature.SORT_CREATOR_PROPERTIES_FIRST)
                     .build()
                     .writeValueAsString(new CABRecord("c", "a", "b")));
-        // Except if we tell it not to:
-        assertEquals(a2q("{'c':'c','a':'a','b':'b'}"),
-                jsonMapperBuilder()
-                .enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY)
-                .enable(MapperFeature.SORT_CREATOR_PROPERTIES_BY_DECLARATION_ORDER)
-                .build()
-                .writeValueAsString(new CABRecord("c", "a", "b")));
     }
 }

src/test-jdk17/java/tools/jackson/databind/records/RecordWithView4085Test.java

@@ -22,7 +22,7 @@ public record Record4085(int total, @JsonView(View4085Field.class) int current)
     public void testRecordWithView4085() throws Exception
     {
         final Record4085 input = new Record4085(1, 2);
-        final String EXP = a2q("{'current':2,'total':1}");
+        final String EXP = a2q("{'total':1,'current':2}");
         final ObjectWriter w = newJsonMapper().writer();
 
         // by default, all properties included, without view

src/test/java/tools/jackson/databind/ser/SerializationOrderTest.java

@@ -22,7 +22,8 @@ static class BeanWithCreator
         public int b;
         public int c;
 
-        @JsonCreator public BeanWithCreator(@JsonProperty("c") int c, @JsonProperty("a") int a) {
+        @JsonCreator
+        public BeanWithCreator(@JsonProperty("c") int c, @JsonProperty("a") int a) {
             this.a = a;
             this.c = c;
         }
@@ -150,7 +151,7 @@ public BeanForStrictOrdering(@JsonProperty("c") int c, @JsonProperty("a") int a)
 
     @Test
     public void testImplicitOrderByCreator() throws Exception {
-        assertEquals("{\"a\":2,\"c\":1,\"b\":0}",
+        assertEquals("{\"c\":1,\"a\":2,\"b\":0}",
                 MAPPER.writeValueAsString(new BeanWithCreator(1, 2)));
     }
 
@@ -205,19 +206,12 @@ public void testCreatorVsExplicitOrdering() throws Exception
     @Test
     public void testAlphaAndCreatorOrdering() throws Exception
     {
-        assertEquals(a2q("{'a':1,'b':2}"),
+        assertEquals(a2q("{'b':2,'a':1}"),
                 ALPHA_MAPPER.writeValueAsString(new BeanForGH311(2, 1)));
-    }
-
-    // [databind#4580]
-    @Test
-    public void testAlphaAndCreatorDeclarationOrdering() throws Exception
-    {
         final ObjectMapper mapper = jsonMapperBuilder()
-                .enable(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY)
-                .enable(MapperFeature.SORT_CREATOR_PROPERTIES_BY_DECLARATION_ORDER)
+                .disable(MapperFeature.SORT_CREATOR_PROPERTIES_FIRST)
                 .build();
-        assertEquals(a2q("{'b':2,'a':1}"),
+        assertEquals(a2q("{'a':1,'b':2}"),
                 mapper.writeValueAsString(new BeanForGH311(2, 1)));
     }
 
@@ -240,7 +234,7 @@ public void testStrictAlphaAndCreatorOrdering() throws Exception
         // BUT are sorted within their own category
         assertTrue(ALPHA_MAPPER.isEnabled(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY));
         assertTrue(ALPHA_MAPPER.isEnabled(MapperFeature.SORT_CREATOR_PROPERTIES_FIRST));
-        assertEquals(a2q("{'a':3,'c':2,'b':0}"),
+        assertEquals(a2q("{'c':2,'a':3,'b':0}"),
                 ALPHA_MAPPER.writeValueAsString(new BeanForStrictOrdering(2, 3)));
 
         // but can change that