Fix #4589: remove MapperFeature.SORT_CREATOR_PROPERTIES_BY_DECLARATION_ORDER, change default sorting logic

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/RecordSerializationOrderTest.java

@@ -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/java/tools/jackson/databind/ser/SerializationOrderTest.java

@@ -205,19 +205,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)));
     }