Fix #3484

cowtowncoder · cowtowncoder · commit 3ebd44421209 · 2022-10-05T19:11:29.000-07:00
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -45,6 +45,7 @@ Project: jackson-databind
 #3481: Filter method only got called once if the field is null when using
   `@JsonInclude(value = JsonInclude.Include.CUSTOM, valueFilter = SomeFieldFilter.class)`
  (contributed by AmiDavidW@github)
+#3484: Update `MapDeserializer` to support `StreamReadCapability.DUPLICATE_PROPERTIES`
 #3497: Deserialization of Throwables with PropertyNamingStrategy does not work
 #3500: Add optional explicit `JsonSubTypes` repeated names check
  (contributed by Igor S)
diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/std/MapDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/std/MapDeserializer.java
@@ -101,6 +101,15 @@ public class MapDeserializer
      */
     protected IgnorePropertiesUtil.Checker _inclusionChecker;
 
+
+    /**
+     * Flag used to check, whether the {@link com.fasterxml.jackson.core.StreamReadCapability#DUPLICATE_PROPERTIES}
+     * can be applied, because the Map has declared value type of {@code java.lang.Object}.
+     *
+     * @since 2.14
+     */
+    protected boolean _checkDupSquash;
+
     /*
     /**********************************************************
     /* Life-cycle
@@ -121,6 +130,7 @@ public MapDeserializer(JavaType mapType, ValueInstantiator valueInstantiator,
         _propertyBasedCreator = null;
         _standardStringKey = _isStdKeyDeser(mapType, keyDeser);
         _inclusionChecker = null;
+        _checkDupSquash = mapType.getContentType().hasRawClass(Object.class);
     }
 
     /**
@@ -143,6 +153,7 @@ protected MapDeserializer(MapDeserializer src)
         _inclusionChecker = src._inclusionChecker;
 
         _standardStringKey = src._standardStringKey;
+        _checkDupSquash = src._checkDupSquash;
     }
 
     protected MapDeserializer(MapDeserializer src,
@@ -177,6 +188,7 @@ protected MapDeserializer(MapDeserializer src,
         _inclusionChecker = IgnorePropertiesUtil.buildCheckerIfNeeded(ignorable, includable);
 
         _standardStringKey = _isStdKeyDeser(_containerType, keyDeser);
+        _checkDupSquash = src._checkDupSquash;
     }
 
     /**
@@ -434,11 +446,9 @@ public Map<Object,Object> deserialize(JsonParser p, DeserializationContext ctxt)
         case JsonTokenId.ID_FIELD_NAME:
             final Map<Object,Object> result = (Map<Object,Object>) _valueInstantiator.createUsingDefault(ctxt);
             if (_standardStringKey) {
-                _readAndBindStringKeyMap(p, ctxt, result);
-                return result;
+                return _readAndBindStringKeyMap(p, ctxt, result);
             }
-            _readAndBind(p, ctxt, result);
-            return result;
+            return _readAndBind(p, ctxt, result);
         case JsonTokenId.ID_STRING:
             // (empty) String may be ok however; or single-String-arg ctor
             return _deserializeFromString(p, ctxt);
@@ -499,7 +509,7 @@ public Object deserializeWithType(JsonParser p, DeserializationContext ctxt,
     /**********************************************************
      */
 
-    protected final void _readAndBind(JsonParser p, DeserializationContext ctxt,
+    protected final Map<Object,Object> _readAndBind(JsonParser p, DeserializationContext ctxt,
             Map<Object,Object> result) throws IOException
     {
         final KeyDeserializer keyDes = _keyDeserializer;
@@ -520,7 +530,7 @@ protected final void _readAndBind(JsonParser p, DeserializationContext ctxt,
             JsonToken t = p.currentToken();
             if (t != JsonToken.FIELD_NAME) {
                 if (t == JsonToken.END_OBJECT) {
-                    return;
+                    return result;
                 }
                 ctxt.reportWrongTokenException(this, JsonToken.FIELD_NAME, null);
             }
@@ -551,22 +561,26 @@ protected final void _readAndBind(JsonParser p, DeserializationContext ctxt,
                 if (useObjectId) {
                     referringAccumulator.put(key, value);
                 } else {
-                    result.put(key, value);
+                    Object oldValue = result.put(key, value);
+                    if (oldValue != null) {
+                        _squashDups(ctxt, result, key, oldValue, value);
+                    }
                 }
             } catch (UnresolvedForwardReference reference) {
                 handleUnresolvedReference(ctxt, referringAccumulator, key, reference);
             } catch (Exception e) {
                 wrapAndThrow(ctxt, e, result, keyStr);
             }
         }
+        return result;
     }
 
     /**
      * Optimized method used when keys can be deserialized as plain old
      * {@link java.lang.String}s, and there is no custom deserialized
      * specified.
      */
-    protected final void _readAndBindStringKeyMap(JsonParser p, DeserializationContext ctxt,
+    protected final Map<Object,Object> _readAndBindStringKeyMap(JsonParser p, DeserializationContext ctxt,
             Map<Object,Object> result) throws IOException
     {
         final JsonDeserializer<Object> valueDes = _valueDeserializer;
@@ -583,7 +597,7 @@ protected final void _readAndBindStringKeyMap(JsonParser p, DeserializationConte
         } else {
             JsonToken t = p.currentToken();
             if (t == JsonToken.END_OBJECT) {
-                return;
+                return result;
             }
             if (t != JsonToken.FIELD_NAME) {
                 ctxt.reportWrongTokenException(this, JsonToken.FIELD_NAME, null);
@@ -613,7 +627,10 @@ protected final void _readAndBindStringKeyMap(JsonParser p, DeserializationConte
                 if (useObjectId) {
                     referringAccumulator.put(key, value);
                 } else {
-                    result.put(key, value);
+                    Object oldValue = result.put(key, value);
+                    if (oldValue != null) {
+                        _squashDups(ctxt, result, key, oldValue, value);
+                    }
                 }
             } catch (UnresolvedForwardReference reference) {
                 handleUnresolvedReference(ctxt, referringAccumulator, key, reference);
@@ -622,6 +639,8 @@ protected final void _readAndBindStringKeyMap(JsonParser p, DeserializationConte
             }
         }
         // 23-Mar-2015, tatu: TODO: verify we got END_OBJECT?
+
+        return result;
     }
     
     @SuppressWarnings("unchecked") 
@@ -661,8 +680,7 @@ public Map<Object,Object> _deserializeUsingCreator(JsonParser p, Deserialization
                     } catch (Exception e) {
                         return wrapAndThrow(ctxt, e, _containerType.getRawClass(), key);
                     }
-                    _readAndBind(p, ctxt, result);
-                    return result;
+                    return _readAndBind(p, ctxt, result);
                 }
                 continue;
             }
@@ -836,6 +854,27 @@ protected final void _readAndUpdateStringKeyMap(JsonParser p, DeserializationCon
         }
     }
 
+    /**
+     * @since 2.14
+     */
+    @SuppressWarnings("unchecked")
+    protected void _squashDups(final DeserializationContext ctxt,
+            final Map<Object, Object> result,
+            final Object key, final Object oldValue, final Object newValue)
+    {
+        if (_checkDupSquash && ctxt.isEnabled(StreamReadCapability.DUPLICATE_PROPERTIES)) {
+            if (oldValue instanceof List<?>) {
+                ((List<Object>) oldValue).add(newValue);
+                result.put(key, oldValue);
+            } else {
+                ArrayList<Object> l = new ArrayList<>();
+                l.add(oldValue);
+                l.add(newValue);
+                result.put(key, l);
+            }
+        }
+    }
+
     /*
     /**********************************************************
     /* Internal methods, other
diff --git a/src/test/java/com/fasterxml/jackson/databind/interop/UntypedObjectWithDupsTest.java b/src/test/java/com/fasterxml/jackson/databind/interop/UntypedObjectWithDupsTest.java
@@ -1,6 +1,6 @@
 package com.fasterxml.jackson.databind.interop;
 
-import java.util.Map;
+import java.util.*;
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.core.StreamReadCapability;
@@ -13,42 +13,91 @@ public class UntypedObjectWithDupsTest extends BaseMapTest
 {
     private final ObjectMapper JSON_MAPPER = newJsonMapper();
 
+    @SuppressWarnings("serial")
+    static class StringStringMap extends LinkedHashMap<String,String> { };
+
+    private final String DOC_WITH_DUPS = a2q(
+            "{'hello': 'world',\n"
+            + "'lists' : 1,\n"
+            + "'lists' : 2,\n"
+            + "'lists' : {\n"
+            + "  'inner' : 'internal',\n"
+            + "  'time' : 123\n"
+            + "},\n"
+            + "'lists' : 3,\n"
+            + "'single' : 'one'\n"
+            + "}");
+
+    // Testing the baseline non-merging behavior
+    public void testDocWithDupsNoMerging() throws Exception
+    {
+        _verifyDupsNoMerging(Object.class);
+        _verifyDupsNoMerging(Map.class);
+    }
+
     // For [dataformat-xml#???]
     public void testDocWithDupsAsUntyped() throws Exception
     {
-        _readWriteDocAs(Object.class);
+        _verifyDupsAreMerged(Object.class);
     }
 
     // For [dataformat-xml#498] / [databind#3484]
     public void testDocWithDupsAsMap() throws Exception
     {
-//        _readWriteDocAs(Map.class);
+        _verifyDupsAreMerged(Map.class);
     }
 
-    private <T> void _readWriteDocAs(Class<T> cls) throws Exception
+    // And also verify that Maps with values other than `Object` will
+    // NOT try merging no matter what
+    public void testDocWithDupsAsNonUntypedMap() throws Exception
+    {
+        final String DOC = a2q("{'key':'a','key':'b'}");
+        assertEquals(a2q("{'key':'b'}"),
+                _readWriteDupDoc(DOC, StringStringMap.class));
+    }
+
+    /*
+    ///////////////////////////////////////////////////////////////////////
+    // Helper methods
+    ///////////////////////////////////////////////////////////////////////
+     */
+    
+    /* Method that will verify default JSON behavior of overwriting value
+     * (no merging).
+     */
+    private <T> void _verifyDupsNoMerging(Class<T> cls) throws Exception
     {
-        final String doc = a2q(
-                "{'hello': 'world',\n"
-                + "'lists' : 1,\n"
-                + "'lists' : 2,\n"
-                + "'lists' : {\n"
-                + "  'inner' : 'internal',\n"
-                + "  'time' : 123\n"
-                + "},\n"
-                + "'lists' : 3,\n"
-                + "'single' : 'one'\n"
-                + "}");
         // This is where need some trickery
         T value;
-        try (JsonParser p = new WithDupsParser(JSON_MAPPER.createParser(doc))) {
+        try (JsonParser p = JSON_MAPPER.createParser(DOC_WITH_DUPS)) {
             value = JSON_MAPPER.readValue(p, cls);
         }
 
         String json = JSON_MAPPER.writeValueAsString(value);
         assertEquals(a2q(
+"{'hello':'world','lists':3,'single':'one'}"),
+                json);
+    }
+
+    /* Method that will verify alternate behavior (used by XML module f.ex)
+     * in which duplicate "properties" are merged into `List`s as necessary
+     */
+    private void _verifyDupsAreMerged(Class<?> cls) throws Exception
+    {
+        assertEquals(a2q(
 "{'hello':'world','lists':[1,2,"
 +"{'inner':'internal','time':123},3],'single':'one'}"),
-                json);
+                _readWriteDupDoc(DOC_WITH_DUPS, cls));
+    }
+
+    private String _readWriteDupDoc(String doc, Class<?> cls) throws Exception
+    {
+        // This is where need some trickery
+        Object value;
+        try (JsonParser p = new WithDupsParser(JSON_MAPPER.createParser(doc))) {
+            value = JSON_MAPPER.readValue(p, cls);
+        }
+        return JSON_MAPPER.writeValueAsString(value);
     }
 
     /**
