From 9cd0d2675cb5151fafef934112666914f18f9f97 Mon Sep 17 00:00:00 2001 From: "Kim, Joo Hyuk" Date: Tue, 12 Nov 2024 12:46:30 +0900 Subject: [PATCH] Allow `@JsonMerge` with Custom List (#4784) --- release-notes/VERSION-2.x | 3 + .../deser/BasicDeserializerFactory.java | 14 +- .../merge/CustomCollectionMerge4783Test.java | 148 ++++++++++++++++++ 3 files changed, 155 insertions(+), 10 deletions(-) create mode 100644 src/test/java/com/fasterxml/jackson/databind/deser/merge/CustomCollectionMerge4783Test.java diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 8099ad4279..5ad842e46e 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -16,6 +16,9 @@ Project: jackson-databind #4790: Fix `@JsonAnySetter` issue with "setter" method (related to #4639) (reported by @bsa01) (fix by Joo-Hyuk K) + #4783 Possibly wrong behavior of @JsonMerge + (reported by @nlisker) + (fix by Joo-Hyuk K) 2.18.1 (28-Oct-2024) diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java b/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java index 8c262cf406..93fae8e49d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java @@ -824,13 +824,7 @@ public JsonDeserializer createCollectionDeserializer(DeserializationContext c 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 = config.introspectForCreation(type); @@ -972,9 +966,9 @@ public JsonDeserializer 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 = config.introspectForCreation(type); diff --git a/src/test/java/com/fasterxml/jackson/databind/deser/merge/CustomCollectionMerge4783Test.java b/src/test/java/com/fasterxml/jackson/databind/deser/merge/CustomCollectionMerge4783Test.java new file mode 100644 index 0000000000..2d3f84ba7f --- /dev/null +++ b/src/test/java/com/fasterxml/jackson/databind/deser/merge/CustomCollectionMerge4783Test.java @@ -0,0 +1,148 @@ +package com.fasterxml.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 com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; +import com.fasterxml.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())); + } + } +}