diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 6d6a801880..45fa98894c 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -36,6 +36,9 @@ Project: jackson-databind #4733: Wrong serialization of Type Ids for certain types of Enum values (reported by @nlisker) +#4783 Possibly wrong behavior of @JsonMerge + (reported by @nlisker) + (fix by Joo-Hyuk K) #4787: Wrong `String.format()` in `StdDelegatingDeserializer` hides actual error (reported by @Horus1337) #4788: `EnumFeature.WRITE_ENUMS_TO_LOWERCASE` overrides `@JsonProperty` values diff --git a/src/main/java/tools/jackson/databind/deser/BasicDeserializerFactory.java b/src/main/java/tools/jackson/databind/deser/BasicDeserializerFactory.java index dc98389eb2..40ab76bd89 100644 --- a/src/main/java/tools/jackson/databind/deser/BasicDeserializerFactory.java +++ b/src/main/java/tools/jackson/databind/deser/BasicDeserializerFactory.java @@ -812,13 +812,7 @@ public ValueDeserializer createCollectionDeserializer(DeserializationContext if (deser == null) { if (type.isInterface() || type.isAbstract()) { CollectionType implType = _mapAbstractCollectionType(type, config); - if (implType == null) { - // [databind#292]: Actually, may be fine, but only if polymorphich deser enabled - if (type.getTypeHandler() == null) { - throw new IllegalArgumentException("Cannot find a deserializer for non-concrete Collection type "+type); - } - deser = AbstractDeserializer.constructForNonPOJO(beanDesc); - } else { + if (implType != null) { type = implType; // But if so, also need to re-check creators... beanDesc = ctxt.introspectBeanDescriptionForCreation(type); @@ -965,9 +959,9 @@ public ValueDeserializer createMapDeserializer(DeserializationContext ctxt, */ if (deser == null) { if (type.isInterface() || type.isAbstract()) { - MapType fallback = _mapAbstractMapType(type, config); - if (fallback != null) { - type = (MapType) fallback; + MapType implType = _mapAbstractMapType(type, config); + if (implType != null) { + type = (MapType) implType; mapClass = type.getRawClass(); // But if so, also need to re-check creators... beanDesc = ctxt.introspectBeanDescriptionForCreation(type); diff --git a/src/test/java/tools/jackson/databind/deser/merge/CustomCollectionMerge4783Test.java b/src/test/java/tools/jackson/databind/deser/merge/CustomCollectionMerge4783Test.java new file mode 100644 index 0000000000..9823f67452 --- /dev/null +++ b/src/test/java/tools/jackson/databind/deser/merge/CustomCollectionMerge4783Test.java @@ -0,0 +1,149 @@ +package tools.jackson.databind.deser.merge; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.annotation.JsonMerge; +import com.fasterxml.jackson.annotation.JsonProperty; + +import tools.jackson.databind.ObjectMapper; +import tools.jackson.databind.exc.InvalidDefinitionException; +import tools.jackson.databind.testutil.DatabindTestUtil; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + +// [databind#4783] Test to verify that JsonMerge also works for custom list +@SuppressWarnings("serial") +public class CustomCollectionMerge4783Test + extends DatabindTestUtil +{ + static class MyArrayListJDK extends ArrayList { } + + static class MergeListJDK { + @JsonMerge + @JsonProperty + public List values = new MyArrayListJDK<>(); + { values.add("a");} + } + + interface MyListCustom extends List { } + + static class MyArrayListCustom extends ArrayList implements MyListCustom { } + + static abstract class MyAbstractStringList extends ArrayList { + MyAbstractStringList() { super(); } + MyAbstractStringList(int i) { super(); } + } + + static class MergeCustomStringList { + @JsonMerge + @JsonProperty + public MyListCustom values = new MyArrayListCustom<>(); + { values.add("a"); } + } + + static class MergeMyCustomLongList { + @JsonMerge + @JsonProperty + public MyListCustom values = new MyArrayListCustom<>(); + { values.add(1L); } + } + + static class MergeMyCustomPojoList { + @JsonMerge + @JsonProperty + public MyListCustom values = new MyArrayListCustom<>(); + { + values.add(CustomPojo.create("a", 1)); + values.add(CustomPojo.create("b", 2)); + } + } + + // And then non-merging case too + static class NonMergeCustomStringList { + public MyListCustom values; + } + + public static class CustomPojo { + public String name; + public int age; + + public static CustomPojo create(String name, int age) { + CustomPojo pojo = new CustomPojo(); + pojo.name = name; + pojo.age = age; + return pojo; + } + } + + private final ObjectMapper MAPPER = newJsonMapper(); + + @Test + void testJDKMapperReading() throws Exception { + MergeListJDK result = MAPPER.readValue("{\"values\":[\"x\"]}", MergeListJDK.class); + + assertEquals(2, result.values.size()); + assertTrue(result.values.contains("x")); + assertTrue(result.values.contains("a")); + } + + @Test + void testCustomMapperReading() throws Exception { + MergeCustomStringList result = MAPPER.readValue("{\"values\":[\"x\"]}", + MergeCustomStringList.class); + + assertEquals(2, result.values.size()); + assertTrue(result.values.contains("x")); + assertTrue(result.values.contains("a")); + } + + @Test + void testCustomMapperReadingLongArrayList() throws Exception { + MergeMyCustomLongList result = MAPPER.readValue("{\"values\":[7]}", + MergeMyCustomLongList.class); + + assertEquals(2, result.values.size()); + assertTrue(result.values.contains(1L)); + assertTrue(result.values.contains(7L)); + } + + @Test + void testCustomMapperReadingPojoArrayList() throws Exception { + MergeMyCustomPojoList result = MAPPER.readValue("{\"values\":[{\"name\":\"c\",\"age\":3}]}", + MergeMyCustomPojoList.class); + + assertEquals(3, result.values.size()); + } + + // // // And then failure cases + + // Fail can't construct Collection interface unless there's maaping + @Test + void failNonMergeInterfaceList() throws Exception { + try { + MAPPER.readValue("{\"values\":[\"x\"]}", NonMergeCustomStringList.class); + fail("Should not pass"); + } catch (InvalidDefinitionException e) { + verifyException(e, String.format( + "Cannot construct instance of `%s` (no Creators", + MyListCustom.class.getName())); + } + } + + // Fail can't construct abstract types + @Test + void failNonMergeAbstractList() throws Exception { + try { + MAPPER.readValue("[]", MyAbstractStringList.class); + fail("Should not pass"); + } catch (InvalidDefinitionException e) { + verifyException(e, String.format( + "Cannot construct instance of `%s` (no Creators", + MyAbstractStringList.class.getName())); + } + } +}