Skip to content

Commit d16420f

Browse files
JooHyukKimJooHyukKim
authored
Fix #4938 Allow @JsonCreator to return null (#4948)
1 parent bd7adfb commit d16420f

File tree

5 files changed

+237
-42
lines changed

5 files changed

+237
-42
lines changed

‎release-notes/VERSION-2.x‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,15 @@ Project: jackson-databind
5252
#4896: Coercion shouldn't be necessary for Enums specifying an empty string
5353
(reported by @joaocanaverde-blue)
5454
#4915: Cannot access attributes from `Converter`
55-
(requested by @jakub-bochenski
55+
(requested by @jakub-bochenski)
5656
(fixed by Joo-Hyuk K)
5757
#4934: `DeserializationContext.readTreeAsValue()` handles null nodes
5858
differently from `ObjectMapper.treeToValue()`
5959
(reported by Floris W)
60-
#4953: Allow clearing all caches to avoid classloader leaks
60+
#4938: Allow `@JsonCreator` annotated Creator to return `null`
61+
(reported by @f-aubert)
62+
(fixed by Joo-Hyuk K)
63+
4953: Allow clearing all caches to avoid classloader leaks
6164
(contributed by Joren I)
6265
#4955: Add more remove methods for `ArrayNode`, `ObjectNode` [STEP-3]
6366
#4959: Add explicit deserializer for `ThreadGroup`

‎src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java‎

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,6 @@ public class BeanDeserializer
2323
{
2424
private static final long serialVersionUID = 1L;
2525

26-
/**
27-
* Lazily constructed exception used as root cause if reporting problem
28-
* with creator method that returns <code>null</code> (which is not allowed)
29-
*
30-
* @since 2.8
31-
*/
32-
protected transient Exception _nullFromCreator;
33-
3426
/**
3527
* State marker we need in order to avoid infinite recursion for some cases
3628
* (not very clean, alas, but has to do for now)
@@ -455,14 +447,15 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri
455447
} catch (Exception e) {
456448
bean = wrapInstantiationProblem(e, ctxt);
457449
}
458-
if (bean == null) {
459-
return ctxt.handleInstantiationProblem(handledType(), null,
460-
_creatorReturnedNullException());
461-
}
462450
// [databind#631]: Assign current value, to be accessible by custom serializers
463451
p.assignCurrentValue(bean);
452+
// [databind#4938] Since 2.19, allow returning `null` from creator,
453+
// but if so, need to skip all possibly relevant content
454+
if (bean == null) {
455+
_handleNullFromPropsBasedCreator(p, ctxt, unknown, referrings);
456+
return null;
457+
}
464458

465-
// polymorphic?
466459
if (bean.getClass() != _beanType.getRawClass()) {
467460
return handlePolymorphic(p, ctxt, p.streamReadConstraints(), bean, unknown);
468461
}
@@ -543,6 +536,14 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri
543536
} catch (Exception e) {
544537
return wrapInstantiationProblem(e, ctxt);
545538
}
539+
p.assignCurrentValue(bean);
540+
// [databind#4938] Since 2.19, allow returning `null` from creator,
541+
// but if so, need to skip all possibly relevant content
542+
if (bean == null) {
543+
_handleNullFromPropsBasedCreator(null, ctxt, unknown, referrings);
544+
return null;
545+
}
546+
546547
// 13-Apr-2020, tatu: [databind#2678] need to handle injection here
547548
if (_injectables != null) {
548549
injectValues(ctxt, bean);
@@ -887,6 +888,16 @@ protected Object deserializeUsingPropertyBasedWithUnwrapped(JsonParser p, Deseri
887888
}
888889
// [databind#631]: Assign current value, to be accessible by custom serializers
889890
p.assignCurrentValue(bean);
891+
// [databind#4938] Since 2.19, allow returning `null` from creator,
892+
// but if so, need to skip all possibly relevant content
893+
if (bean == null) {
894+
// 13-Mar-2025, tatu: We don't have "referrings" here for some reason...
895+
// Nor "unknown" since unwrapping makes it impossible to tell unwrapped
896+
// and unknown apart
897+
_handleNullFromPropsBasedCreator(p, ctxt, null, null);
898+
return null;
899+
}
900+
890901
// if so, need to copy all remaining tokens into buffer
891902
while (t == JsonToken.FIELD_NAME) {
892903
// NOTE: do NOT skip name as it needs to be copied; `copyCurrentStructure` does that
@@ -957,6 +968,14 @@ protected Object deserializeUsingPropertyBasedWithUnwrapped(JsonParser p, Deseri
957968
} catch (Exception e) {
958969
return wrapInstantiationProblem(e, ctxt);
959970
}
971+
// [databind#4938] Since 2.19, allow returning `null` from creator,
972+
// but if so, need to skip all possibly relevant content
973+
if (bean == null) {
974+
// no "referrings" here either:
975+
_handleNullFromPropsBasedCreator(null, ctxt, null, null);
976+
return null;
977+
}
978+
960979
return _unwrappedPropertyHandler.processUnwrapped(p, ctxt, bean, tokens);
961980
}
962981

@@ -1134,17 +1153,27 @@ protected Object deserializeUsingPropertyBasedWithExternalTypeId(JsonParser p,
11341153
}
11351154
}
11361155

1137-
/**
1138-
* Helper method for getting a lazily construct exception to be reported
1139-
* to {@link DeserializationContext#handleInstantiationProblem(Class, Object, Throwable)}.
1140-
*
1141-
* @since 2.8
1142-
*/
1143-
protected Exception _creatorReturnedNullException() {
1144-
if (_nullFromCreator == null) {
1145-
_nullFromCreator = new NullPointerException("JSON Creator returned null");
1156+
// @since 2.19
1157+
protected void _handleNullFromPropsBasedCreator(JsonParser p, DeserializationContext ctxt,
1158+
TokenBuffer unknown, List<BeanReferring> referrings)
1159+
throws IOException
1160+
{
1161+
if (p != null) {
1162+
JsonToken t = p.currentToken();
1163+
while (t == JsonToken.FIELD_NAME) {
1164+
p.nextToken();
1165+
p.skipChildren();
1166+
t = p.nextToken();
1167+
}
1168+
}
1169+
if (unknown != null) { // nope, just extra unknown stuff...
1170+
handleUnknownProperties(ctxt, null, unknown);
1171+
}
1172+
if (referrings != null) {
1173+
for (BeanReferring referring : referrings) {
1174+
referring.setBean(null);
1175+
}
11461176
}
1147-
return _nullFromCreator;
11481177
}
11491178

11501179
/**

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,10 +1969,9 @@ public <T> T wrapAndThrow(Throwable t, Object bean, String fieldName, Deserializ
19691969
private Throwable throwOrReturnThrowable(Throwable t, DeserializationContext ctxt)
19701970
throws IOException
19711971
{
1972-
/* 05-Mar-2009, tatu: But one nasty edge is when we get
1973-
* StackOverflow: usually due to infinite loop. But that
1974-
* often gets hidden within an InvocationTargetException...
1975-
*/
1972+
// 05-Mar-2009, tatu: But one nasty edge is when we get
1973+
// StackOverflow: usually due to infinite loop. But that
1974+
// often gets hidden within an InvocationTargetException...
19761975
while (t instanceof InvocationTargetException && t.getCause() != null) {
19771976
t = t.getCause();
19781977
}

‎src/test/java/com/fasterxml/jackson/databind/deser/creators/JsonCreatorReturningNull4938Test.java‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
package com.fasterxml.jackson.databind.deser.creators;
2+
3+
import java.util.*;
4+
5+
import org.junit.jupiter.api.Test;
6+
7+
import com.fasterxml.jackson.annotation.*;
8+
import com.fasterxml.jackson.core.JsonParseException;
9+
import com.fasterxml.jackson.databind.*;
10+
import com.fasterxml.jackson.databind.exc.UnrecognizedPropertyException;
11+
import com.fasterxml.jackson.databind.testutil.DatabindTestUtil;
12+
13+
import static org.junit.jupiter.api.Assertions.*;
14+
15+
// [databind#4938] Allow JsonCreator factory method to return `null`
16+
public class JsonCreatorReturningNull4938Test
17+
extends DatabindTestUtil
18+
{
19+
static class Localized3 {
20+
public final String en;
21+
public final String de;
22+
public final String fr;
23+
24+
@JsonCreator
25+
public static Localized3 of(@JsonProperty("en") String en,
26+
@JsonProperty("de") String de, @JsonProperty("fr") String fr) {
27+
if (en == null && de == null && fr == null) {
28+
return null; // Explicitly return null when all arguments are null
29+
}
30+
return new Localized3(en, de, fr);
31+
}
32+
33+
// This is how users would normally create instances, I think...?
34+
private Localized3(String en, String de, String fr) {
35+
this.en = en;
36+
this.de = de;
37+
this.fr = fr;
38+
}
39+
}
40+
41+
static class Localized4 {
42+
public final String en;
43+
public final String de;
44+
public final String fr;
45+
46+
@JsonCreator
47+
public static Localized4 of(@JsonProperty("en") String en,
48+
@JsonProperty("de") String de, @JsonProperty("fr") String fr) {
49+
if (en == null && de == null && fr == null) {
50+
return null; // Explicitly return null when all arguments are null
51+
}
52+
throw new IllegalStateException("Should not be called");
53+
}
54+
55+
// This is how users would normally create instances, I think...?
56+
private Localized4(String en, String de, String fr) {
57+
this.en = en;
58+
this.de = de;
59+
this.fr = fr;
60+
}
61+
}
62+
63+
// Test with AnySetter when creator returns null
64+
static class Localized5 {
65+
public final String en;
66+
public final String de;
67+
public final String fr;
68+
public final Map<String, Object> props = new HashMap<>();
69+
70+
@JsonCreator
71+
public static Localized5 of(@JsonProperty("en") String en,
72+
@JsonProperty("de") String de, @JsonProperty("fr") String fr) {
73+
if (en == null && de == null && fr == null) {
74+
return null; // Explicitly return null when all arguments are null
75+
}
76+
throw new IllegalStateException("Should not be called");
77+
}
78+
79+
// This is how users would normally create instances, I think...?
80+
private Localized5(String en, String de, String fr) {
81+
this.en = en;
82+
this.de = de;
83+
this.fr = fr;
84+
}
85+
86+
@JsonAnySetter
87+
public void addProperty(String key, Object value) {
88+
props.put(key, value);
89+
}
90+
}
91+
92+
93+
private final ObjectMapper MAPPER = newJsonMapper();
94+
95+
@Test
96+
void testDeserializeToNullWhenAllPropertiesAreNull()
97+
throws Exception
98+
{
99+
Localized3 result = MAPPER.readValue(
100+
"{ \"en\": null, \"de\": null, \"fr\": null }",
101+
Localized3.class);
102+
103+
assertNull(result);
104+
}
105+
106+
@Test
107+
void testDeserializeToNonNullWhenAnyPropertyIsNonNull()
108+
throws Exception
109+
{
110+
Localized3 result = MAPPER.readValue(
111+
"{ \"en\": \"Hello\", \"de\": null, \"fr\": null }",
112+
Localized3.class);
113+
114+
assertNotNull(result);
115+
assertEquals("Hello", result.en);
116+
}
117+
118+
@Test
119+
void testDeserializeReadingAfterCreatorProps()
120+
throws Exception
121+
{
122+
// Should all fail...
123+
ObjectReader enabled = MAPPER.readerFor(Localized4.class)
124+
.with(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
125+
// ...with unknown properties in front
126+
try {
127+
enabled.readValue("{ \"unknown\": null, \"en\": null, \"de\": null, \"fr\": null, \"unknown2\": \"hello\" }");
128+
fail("Should not pass");
129+
} catch (UnrecognizedPropertyException e) {
130+
// We fail with the FIRST unknown property
131+
verifyException(e, "Unrecognized field \"unknown\"");
132+
}
133+
}
134+
135+
// Test to verify we are reading till the end of the OBJECT
136+
@Test
137+
void testDeserializeReadingUntilEndObject()
138+
throws Exception
139+
{
140+
// Should all fail...
141+
ObjectReader enabled = MAPPER.readerFor(Localized4.class)
142+
// We don't stop in the middle
143+
.without(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
144+
// This will trigger after...
145+
// ONLY AFTER we have read the whole object
146+
.with(DeserializationFeature.FAIL_ON_TRAILING_TOKENS);
147+
// ...with unknown properties in front
148+
try {
149+
enabled.readValue( "{ \"en\": null, \"de\": null, \"fr\": null, \"unknown\": null, \"unknown2\": \"hello\" }" +
150+
"!!!!!!!!!!!!BOOM!!!!!!!!!!!!!!");
151+
fail("Should not pass");
152+
} catch (JsonParseException e) {
153+
verifyException(e, "Unexpected character ('!'");
154+
}
155+
}
156+
157+
@Test
158+
void testJsonCreatorNullWithAnySetter()
159+
throws Exception
160+
{
161+
String JSON = "{ \"en\": null, \"de\": null, \"fr\": null, " +
162+
// These two properties are unknown
163+
"\"unknown\": null, \"unknown2\": \"hello\" }";
164+
165+
MAPPER.readerFor(Localized5.class)
166+
.without(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
167+
.readValue(JSON);
168+
}
169+
}

‎src/test/java/com/fasterxml/jackson/databind/deser/creators/NullValueViaCreatorTest.java‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,12 @@
1010
import com.fasterxml.jackson.core.*;
1111
import com.fasterxml.jackson.databind.*;
1212
import com.fasterxml.jackson.databind.deser.*;
13-
import com.fasterxml.jackson.databind.exc.ValueInstantiationException;
13+
import com.fasterxml.jackson.databind.testutil.DatabindTestUtil;
1414

15-
import static org.junit.jupiter.api.Assertions.assertEquals;
16-
import static org.junit.jupiter.api.Assertions.fail;
17-
18-
import static com.fasterxml.jackson.databind.testutil.DatabindTestUtil.verifyException;
15+
import static org.junit.jupiter.api.Assertions.*;
1916

2017
public class NullValueViaCreatorTest
18+
extends DatabindTestUtil
2119
{
2220
protected static class Container {
2321
Contained<String> contained;
@@ -112,14 +110,11 @@ public void testUsesDeserializersNullValue() throws Exception {
112110

113111
// [databind#597]: ensure that a useful exception is thrown
114112
@Test
115-
public void testCreatorReturningNull() throws IOException {
116-
ObjectMapper objectMapper = new ObjectMapper();
113+
public void testCreatorReturningNull() throws Exception {
114+
ObjectMapper objectMapper = newJsonMapper();
115+
117116
String json = "{ \"type\" : \" \", \"id\" : \"000c0ffb-a0d6-4d2e-a379-4aeaaf283599\" }";
118-
try {
119-
objectMapper.readValue(json, JsonEntity.class);
120-
fail("Should not have succeeded");
121-
} catch (ValueInstantiationException e) {
122-
verifyException(e, "JSON creator returned null");
123-
}
117+
118+
assertNull(objectMapper.readValue(json, JsonEntity.class));
124119
}
125120
}

0 commit comments

Comments
 (0)