Fix #2336

release-notes/CREDITS-2.x

@@ -896,16 +896,20 @@ Manuel Hegner (manuel-hegner@github)
   * Suggested #2311: Unnecessary MultiView creation for property writers
    (2.10.0)
 
+Chris Mercer (cmercer@github)
+  * Reported #2331: `JsonMappingException` through nested getter with generic wildcard return type
+   (2.10.0)
+
+Robert Greig (rgreig@github)
+  * Reported #2336: `MapDeserializer` can not merge `Map`s with polymorphic values
+   (2.10.0)
+
 Victor Noël (victornoel@github)
   * Reported #2338: Suboptimal return type for `JsonNode.withArray()`
    (2.10.0)
   * Reported #2339: Suboptimal return type for `ObjectNode.set()`
    (2.10.0)
 
-Chris Mercer (cmercer@github)
-  * Reported #2331: `JsonMappingException` through nested getter with generic wildcard return type
-   (2.10.0)
-
 Vladimir Tsanev (tsachev@github)
   * Contributed #2415: Builder-based POJO deserializer should pass builder instance, not type,
     to `handleUnknownVanilla()` to fix earlier #822

release-notes/VERSION-2.x

@@ -8,6 +8,8 @@ Project: jackson-databind
 
 #2331: `JsonMappingException` through nested getter with generic wildcard return type
  (reported by sunchezz89@github)
+#2336: `MapDeserializer` can not merge `Map`s with polymorphic values
+ (reported by Robert G)
 #2390: `Iterable` serialization breaks when adding `@JsonFilter` annotation
  (reported by Chris M)
 #2392: `BeanDeserializerModifier.modifyDeserializer()` not applied to custom bean deserializers

src/main/java/com/fasterxml/jackson/databind/DeserializationContext.java

@@ -1219,6 +1219,26 @@ public JavaType handleMissingTypeId(JavaType baseType,
         throw missingTypeIdException(baseType, extraDesc);
     }
 
+    /**
+     * Method that deserializer may call if it is called to do an update ("merge")
+     * but deserializer operates on a non-mergeable type. Although this should
+     * usually be caught earlier, sometimes it may only be caught during operation
+     * and if so this is the method to call.
+     * Note that if {@link MapperFeature#IGNORE_MERGE_FOR_UNMERGEABLE} is enabled,
+     * this method will simply return null; otherwise {@link InvalidDefinitionException}
+     * will be thrown.
+     *
+     * @since 2.10
+     */
+    public void handleBadMerge(JsonDeserializer<?> deser) throws JsonMappingException
+    {
+        if (!isEnabled(MapperFeature.IGNORE_MERGE_FOR_UNMERGEABLE)) {
+            JavaType type = constructType(deser.handledType());
+            String msg = String.format("Invalid configuration: values of type %s cannot be merged", type);
+            throw InvalidDefinitionException.from(getParser(), msg, type);
+        }
+    }
+
     /**
      * @since 2.9.2
      */
@@ -1454,24 +1474,12 @@ public <T> T reportBadDefinition(JavaType type, String msg) throws JsonMappingEx
     }
 
     /**
-     * Method that deserializer may call if it is called to do an update ("merge")
-     * but deserializer operates on a non-mergeable type. Although this should
-     * usually be caught earlier, sometimes it may only be caught during operation
-     * and if so this is the method to call.
-     * Note that if {@link MapperFeature#IGNORE_MERGE_FOR_UNMERGEABLE} is enabled,
-     * this method will simply return null; otherwise {@link InvalidDefinitionException}
-     * will be thrown.
-     *
-     * @since 2.9
+     * @deprecated Since 2.10 use {@link #handleBadMerge} instead
      */
-    public <T> T reportBadMerge(JsonDeserializer<?> deser) throws JsonMappingException
-    {
-        if (isEnabled(MapperFeature.IGNORE_MERGE_FOR_UNMERGEABLE)) {
-            return null;
-        }
-        JavaType type = constructType(deser.handledType());
-        String msg = String.format("Invalid configuration: values of type %s cannot be merged", type);
-        throw InvalidDefinitionException.from(getParser(), msg, type);
+    @Deprecated // since 2.10
+    public <T> T reportBadMerge(JsonDeserializer<?> deser) throws JsonMappingException {
+        handleBadMerge(deser);
+        return null;
     }
 
     /*

src/main/java/com/fasterxml/jackson/databind/JsonDeserializer.java

@@ -125,7 +125,7 @@ public abstract T deserialize(JsonParser p, DeserializationContext ctxt)
     public T deserialize(JsonParser p, DeserializationContext ctxt, T intoValue)
         throws IOException
     {
-        ctxt.reportBadMerge(this);
+        ctxt.handleBadMerge(this);
         return deserialize(p, ctxt);
     }
 
@@ -153,16 +153,18 @@ public Object deserializeWithType(JsonParser p, DeserializationContext ctxt,
 
     /**
      * Method similar to {@link #deserializeWithType(JsonParser,DeserializationContext,TypeDeserializer)}
-     * but called when merging value.
+     * but called when merging value. Considered "bad merge" by default implementation,
+     * but if {@link MapperFeature#IGNORE_MERGE_FOR_UNMERGEABLE} is enabled will simple delegate to
+     * {@link #deserializeWithType(JsonParser, DeserializationContext, TypeDeserializer)}.
      *
      * @since 2.10
      */
     public Object deserializeWithType(JsonParser p, DeserializationContext ctxt,
             TypeDeserializer typeDeserializer, T intoValue)
         throws IOException
     {
-        ctxt.reportBadMerge(this);
-        return typeDeserializer.deserializeTypedFromAny(p, ctxt);
+        ctxt.handleBadMerge(this);
+        return deserializeWithType(p, ctxt, typeDeserializer);
     }
 
     /*

src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializerBase.java

@@ -899,9 +899,9 @@ protected SettableBeanProperty _resolveMergeAndNullSettings(DeserializationConte
                 }
             } else if (!mayMerge.booleanValue()) { // prevented
                 if (!merge.fromDefaults) {
-                    // If attempts was made via explicit annotation/per-type config override,
+                    // If attempt was made via explicit annotation/per-type config override,
                     // should be reported; may or may not result in exception
-                    ctxt.reportBadMerge(valueDeser);
+                    ctxt.handleBadMerge(valueDeser);
                 }
                 return prop;
             }

src/main/java/com/fasterxml/jackson/databind/deser/std/StdScalarDeserializer.java

@@ -33,8 +33,8 @@ public Object deserializeWithType(JsonParser p, DeserializationContext ctxt, Typ
     @Override // since 2.9
     public T deserialize(JsonParser p, DeserializationContext ctxt, T intoValue) throws IOException {
         // 25-Oct-2016, tatu: And if attempt is made, see if we are to complain...
-        ctxt.reportBadMerge(this);
-        // except that it is possible to suppress this; and if so...
+        ctxt.handleBadMerge(this);
+        // if that does not report an exception we can just delegate
         return deserialize(p, ctxt);
     }
 

src/test/java/com/fasterxml/jackson/failing/MapPolymorphicMerge2336Test.java → src/test/java/com/fasterxml/jackson/databind/deser/merge/MapPolymorphicMerge2336Test.java

@@ -1,4 +1,4 @@
-package com.fasterxml.jackson.failing;
+package com.fasterxml.jackson.databind.deser.merge;
 
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -11,9 +11,7 @@ public class MapPolymorphicMerge2336Test extends BaseMapTest
 {
     private final ObjectMapper MAPPER = sharedMapper();
 
-    @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "discriminator"
-            //, visible = true
-    )
+    @JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property = "discriminator")
     @JsonSubTypes({@JsonSubTypes.Type(value = SomeClassA.class, name = "FirstConcreteImpl")})
     @JsonInclude(JsonInclude.Include.NON_NULL)
     public static abstract class SomeBaseClass {
@@ -71,7 +69,7 @@ public SomeOtherClass(@JsonProperty("someprop") String someprop) {
         }
 
         @JsonMerge
-        private Map<String, SomeBaseClass> data = new LinkedHashMap<>();
+        Map<String, SomeBaseClass> data = new LinkedHashMap<>();
 
         public void addValue(String key, SomeBaseClass value) {
             data.put(key, value);
@@ -87,24 +85,35 @@ public void setData(
         }
     }
 
+    // [databind#2336]
     public void testPolymorphicMapMerge() throws Exception
     {
         // first let's just get some valid JSON
-        SomeOtherClass test = new SomeOtherClass("house");
-        test.addValue("SOMEKEY", new SomeClassA("fred", 1, null));
-//        String serializedValue = MAPPER.writeValueAsString(test);
-
-//System.out.println("Serialized value: " + serializedValue);
+        SomeOtherClass baseValue = new SomeOtherClass("house");
+        baseValue.addValue("SOMEKEY", new SomeClassA("fred", 1, null));
 
         // now create a reader specifically for merging
-        ObjectReader reader = MAPPER.readerForUpdating(test);
+        ObjectReader reader = MAPPER.readerForUpdating(baseValue);
 
+
         SomeOtherClass toBeMerged = new SomeOtherClass("house");
         toBeMerged.addValue("SOMEKEY", new SomeClassA("jim", null, 2));
         String jsonForMerging = MAPPER.writeValueAsString(toBeMerged);
-System.out.println("JSON to be merged: " + jsonForMerging);
+
+        assertEquals("fred", baseValue.data.get("SOMEKEY").getName());
+
         // now try to do the merge and it blows up
         SomeOtherClass mergedResult = reader.readValue(jsonForMerging);
-System.out.println("Serialized value: " + MAPPER.writeValueAsString(mergedResult));
+
+        // First of all, should update main POJO (since it's "value to update")
+        assertSame(baseValue, mergedResult);
+        // as well as Map within
+        assertSame(baseValue.data, mergedResult.data);
+
+        assertEquals(1, mergedResult.data.size());
+        // but entry value has changed by necessity
+        assertEquals(SomeClassA.class, mergedResult.data.get("SOMEKEY").getClass());
+
+        assertEquals("jim", mergedResult.data.get("SOMEKEY").getName());
     }
 }