Fix #2555 (use @JsonProperty.index for serialization ordering)

Author: cowtowncoder

release-notes/VERSION-2.x

@@ -22,6 +22,7 @@ Project: jackson-databind
 #2522: `DeserializationContext.handleMissingInstantiator()` throws `MismatchedInputException`
   for non-static inner classes
 #2525: Incorrect `JsonStreamContext` for `TokenBuffer` and `TreeTraversingParser`
+#2555: Use `@JsonProperty(index)` for sorting properties on serialization
 - Add `SerializerProvider.findContentValueSerializer()` methods
 
 2.10.2 (not yet released)

src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java

@@ -336,17 +336,14 @@ protected void collectAll()
             _renameUsing(props, naming);
         }
 
-        /* Sort by visibility (explicit over implicit); drop all but first
-         * of member type (getter, setter etc) if there is visibility
-         * difference
-         */
+        // Sort by visibility (explicit over implicit); drop all but first of member
+        // type (getter, setter etc) if there is visibility difference
         for (POJOPropertyBuilder property : props.values()) {
             property.trimByVisibility();
         }
 
-        /* and, if required, apply wrapper name: note, MUST be done after
-         * annotations are merged.
-         */
+        // and, if required, apply wrapper name: note, MUST be done after
+        // annotations are merged.
         if (_config.isEnabled(MapperFeature.USE_WRAPPER_NAME_AS_PROPERTY_NAME)) {
             _renameWithWrappers(props);
         }
@@ -933,25 +930,24 @@ protected void _renameWithWrappers(Map<String, POJOPropertyBuilder> props)
     /**********************************************************
      */
 
-    /* First, order by [JACKSON-90] (explicit ordering and/or alphabetic)
-     * and then for [JACKSON-170] (implicitly order creator properties before others)
-     */
+    // First, order by(explicit ordering and/or alphabetic),
+    // then by (optional) index (if any)
+    // and then implicitly order creator properties before others)
+
     protected void _sortProperties(Map<String, POJOPropertyBuilder> props)
     {
         // Then how about explicit ordering?
-        AnnotationIntrospector intr = _annotationIntrospector;
+        final AnnotationIntrospector intr = _annotationIntrospector;
         Boolean alpha = intr.findSerializationSortAlphabetically((Annotated) _classDef);
-        boolean sort;
-        
-        if (alpha == null) {
-            sort = _config.shouldSortPropertiesAlphabetically();
-        } else {
-            sort = alpha.booleanValue();
-        }
+        final boolean sort = (alpha == null)
+                ? _config.shouldSortPropertiesAlphabetically()
+                : alpha.booleanValue();
+        final boolean indexed = _anyIndexed(props.values());
+
         String[] propertyOrder = intr.findSerializationPropertyOrder(_classDef);
 
         // no sorting? no need to shuffle, then
-        if (!sort && (_creatorProperties == null) && (propertyOrder == null)) {
+        if (!sort && !indexed && (_creatorProperties == null) && (propertyOrder == null)) {
             return;
         }
         int size = props.size();
@@ -966,11 +962,11 @@ protected void _sortProperties(Map<String, POJOPropertyBuilder> props)
         for (POJOPropertyBuilder prop : props.values()) {
             all.put(prop.getName(), prop);
         }
-        Map<String,POJOPropertyBuilder> ordered = new LinkedHashMap<String,POJOPropertyBuilder>(size+size);
+        Map<String,POJOPropertyBuilder> ordered = new LinkedHashMap<>(size+size);
         // Ok: primarily by explicit order
         if (propertyOrder != null) {
             for (String name : propertyOrder) {
-                POJOPropertyBuilder w = all.get(name);
+                POJOPropertyBuilder w = all.remove(name);
                 if (w == null) { // will also allow use of "implicit" names for sorting
                     for (POJOPropertyBuilder prop : props.values()) {
                         if (name.equals(prop.getInternalName())) {
@@ -986,7 +982,26 @@ protected void _sortProperties(Map<String, POJOPropertyBuilder> props)
                 }
             }
         }
-        // And secondly by sorting Creator properties before other unordered properties
+
+        // Second (starting with 2.11): index, if any:
+        if (indexed) {
+            Map<Integer,POJOPropertyBuilder> byIndex = new TreeMap<>();
+            Iterator<Map.Entry<String,POJOPropertyBuilder>> it = all.entrySet().iterator();
+            while (it.hasNext()) {
+                Map.Entry<String,POJOPropertyBuilder> entry = it.next();
+                POJOPropertyBuilder prop = entry.getValue();
+                Integer index = prop.getMetadata().getIndex();
+                if (index != null) {
+                    byIndex.put(index, prop);
+                    it.remove();
+                }
+            }
+            for (POJOPropertyBuilder prop : byIndex.values()) {
+                ordered.put(prop.getName(), prop);
+            }
+        }
+
+        // Third by sorting Creator properties before other unordered properties
         if (_creatorProperties != null) {
             /* As per [databind#311], this is bit delicate; but if alphabetic ordering
              * is mandated, at least ensure creator properties are in alphabetic
@@ -1008,6 +1023,8 @@ protected void _sortProperties(Map<String, POJOPropertyBuilder> props)
                 // 16-Jan-2016, tatu: Related to [databind#1317], make sure not to accidentally
                 //    add back pruned creator properties!
                 String name = prop.getName();
+                // 27-Nov-2019, tatu: Not sure why, but we should NOT remove it from `all` tho:
+//                if (all.remove(name) != null) {
                 if (all.containsKey(name)) {
                     ordered.put(name, prop);
                 }
@@ -1017,7 +1034,16 @@ protected void _sortProperties(Map<String, POJOPropertyBuilder> props)
         ordered.putAll(all);
         props.clear();
         props.putAll(ordered);
-    }        
+    }
+
+    private boolean _anyIndexed(Collection<POJOPropertyBuilder> props) {
+        for (POJOPropertyBuilder prop : props) {
+            if (prop.getMetadata().hasIndex()) {
+                return true;
+            }
+        }
+        return false;
+    }
 
     /*
     /**********************************************************

src/test/java/com/fasterxml/jackson/databind/ser/TestSerializationOrder.java renamed to src/test/java/com/fasterxml/jackson/databind/ser/SerializationOrderTest.java

@@ -8,7 +8,7 @@
  * Unit tests for verifying that constraints on ordering of serialized
  * properties are held.
  */
-public class TestSerializationOrder
+public class SerializationOrderTest
     extends BaseMapTest
 {
     static class BeanWithCreator
@@ -68,7 +68,7 @@ static class BeanFor459 {
 
     // For [databind#311]
     @JsonPropertyOrder(alphabetic = true)
-    public class BeanForGH311 {
+    static class BeanForGH311 {
         private final int a;
         private final int b;
 
@@ -82,6 +82,21 @@ public BeanForGH311(@JsonProperty("b") int b, @JsonProperty("a") int a) { //b an
         public int getB() { return b; }
     }
 
+    // We'll expect ordering of "FUBAR"
+    @JsonPropertyOrder({ "f"  })
+    static class OrderingByIndexBean {
+        public int r;
+        public int a;
+
+        @JsonProperty(index = 1)
+        public int b;
+
+        @JsonProperty(index = 0)
+        public int u;
+
+        public int f;
+    }
+
     /*
     /*********************************************
     /* Unit tests
@@ -90,19 +105,23 @@ public BeanForGH311(@JsonProperty("b") int b, @JsonProperty("a") int a) { //b an
 
     private final ObjectMapper MAPPER = newJsonMapper();
 
-    public void testImplicitOrderByCreator() throws Exception
-    {
-        assertEquals("{\"c\":1,\"a\":2,\"b\":0}", MAPPER.writeValueAsString(new BeanWithCreator(1, 2)));
+    private final ObjectMapper ALPHA_MAPPER = jsonMapperBuilder()
+            .configure(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY, true)
+            .build();
+
+    public void testImplicitOrderByCreator() throws Exception {
+        assertEquals("{\"c\":1,\"a\":2,\"b\":0}",
+                MAPPER.writeValueAsString(new BeanWithCreator(1, 2)));
     }
 
-    public void testExplicitOrder() throws Exception
-    {
-        assertEquals("{\"c\":3,\"a\":1,\"b\":2,\"d\":4}", MAPPER.writeValueAsString(new BeanWithOrder(1, 2, 3, 4)));
+    public void testExplicitOrder() throws Exception {
+        assertEquals("{\"c\":3,\"a\":1,\"b\":2,\"d\":4}",
+                MAPPER.writeValueAsString(new BeanWithOrder(1, 2, 3, 4)));
     }
 
-    public void testAlphabeticOrder() throws Exception
-    {
-        assertEquals("{\"d\":4,\"a\":1,\"b\":2,\"c\":3}", MAPPER.writeValueAsString(new SubBeanWithOrder(1, 2, 3, 4)));
+    public void testAlphabeticOrder() throws Exception {
+        assertEquals("{\"d\":4,\"a\":1,\"b\":2,\"c\":3}",
+                MAPPER.writeValueAsString(new SubBeanWithOrder(1, 2, 3, 4)));
     }
 
     public void testOrderWithMixins() throws Exception
@@ -122,20 +141,23 @@ public void testOrderWrt268() throws Exception
 
     public void testOrderWithFeature() throws Exception
     {
-        ObjectMapper m = jsonMapperBuilder()
-                .configure(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY, true)
-                .build();
         assertEquals("{\"a\":1,\"b\":2,\"c\":3,\"d\":4}",
-                m.writeValueAsString(new BeanFor459()));
+                ALPHA_MAPPER.writeValueAsString(new BeanFor459()));
     }
 
     // [databind#311]
     public void testAlphaAndCreatorOrdering() throws Exception
     {
-        ObjectMapper m = jsonMapperBuilder()
-                .configure(MapperFeature.SORT_PROPERTIES_ALPHABETICALLY, true)
-                .build();
-        String json = m.writeValueAsString(new BeanForGH311(2, 1));
+        String json = ALPHA_MAPPER.writeValueAsString(new BeanForGH311(2, 1));
         assertEquals("{\"a\":1,\"b\":2}", json);
     }
+
+    // [databind#2555]
+    public void testOrderByIndexEtc() throws Exception
+    {
+        // since "default" order can actually vary with later JDKs, only verify
+        // case of alphabetic-as-default
+        assertEquals(aposToQuotes("{'f':0,'u':0,'b':0,'a':0,'r':0}"),
+                ALPHA_MAPPER.writeValueAsString(new OrderingByIndexBean()));
+    }
 }