Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow @JsonMerge with Custom List #4784

Merged
merged 22 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
36fc768
test : Add test to verify JsonMerge behavior with custom merge
JooHyukKim Nov 6, 2024
6289ba3
Modify test
JooHyukKim Nov 6, 2024
8667a7e
Possible implementation?
JooHyukKim Nov 8, 2024
8e019ea
Merge branch '2.18' into test-4783
JooHyukKim Nov 8, 2024
85c9b91
chore : rename method and comment
JooHyukKim Nov 8, 2024
8668a02
Merge branch 'test-4783' of https://github.com/JooHyukKim/jackson-dat…
JooHyukKim Nov 8, 2024
53c6455
test : clean up unnecessary parameterization
JooHyukKim Nov 9, 2024
434e418
feature : Straight up create AbstractDeserializer.constructForNonPOJO…
JooHyukKim Nov 9, 2024
3354b95
Merge branch '2.18' into test-4783
cowtowncoder Nov 9, 2024
897d27c
Minor clean up
cowtowncoder Nov 9, 2024
d372a72
Remove use of `AbstractDeserializer`
cowtowncoder Nov 9, 2024
bf7ec4e
Add test for error handling too
cowtowncoder Nov 9, 2024
3c848db
renaming
cowtowncoder Nov 9, 2024
70bee50
Merge branch '2.18' into test-4783
cowtowncoder Nov 9, 2024
02748f4
Change abstract Map handling similar to abstract Collection (defer fail)
cowtowncoder Nov 9, 2024
2d39bd2
Merge branch 'test-4783' of github.com:JooHyukKim/jackson-databind in…
cowtowncoder Nov 9, 2024
1f1f84c
Add release note
JooHyukKim Nov 10, 2024
d021792
Merge branch 'test-4783' of https://github.com/JooHyukKim/jackson-dat…
JooHyukKim Nov 10, 2024
b0860b3
Update release note
JooHyukKim Nov 10, 2024
8510175
Fix test name typo
JooHyukKim Nov 10, 2024
8efc34d
Merge branch '2.18' into test-4783
cowtowncoder Nov 12, 2024
7f985f0
Remove one unnecessary change (wrt Map defaulting), add a test case
cowtowncoder Nov 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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<T> extends ArrayList<T> { }

static class MergeListJDK {
@JsonMerge
@JsonProperty
public List<String> values = new MyArrayListJDK<>();
{ values.add("a");}
}

interface MyListCustom<T> extends List<T> { }

static class MyArrayListCustom<T> extends ArrayList<T> implements MyListCustom<T> { }

static abstract class MyAbstractStringList extends ArrayList<String> {
MyAbstractStringList() { super(); }
MyAbstractStringList(int i) { super(); }
}

static class MergeCustomStringList {
@JsonMerge
@JsonProperty
public MyListCustom<String> values = new MyArrayListCustom<>();
{ values.add("a"); }
}

static class MergeMyCustomLongList {
@JsonMerge
@JsonProperty
public MyListCustom<Long> values = new MyArrayListCustom<>();
{ values.add(1L); }
}

static class MergeMyCustomPojoList {
@JsonMerge
@JsonProperty
public MyListCustom<CustomPojo> 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<String> 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()));
}
}
}