Skip to content

Commit c641863

Browse files
committed
Fix FasterXML#4688 2.18.0-rc1 Regression deserializing with no-arg delegating JsonCreator
The refactor to bean property introspection in FasterXML#4515 caused existing no-arg delegatring constructors to fail deserialization. Example from this branch where we test RC releases: palantir/conjure-java#2349 specifically this test: https://github.com/palantir/conjure-java/blob/19d7f54eb0001c49b5d8a4bb897b9ad4cb5a28de/conjure-java-core/src/test/java/com/palantir/conjure/java/types/NoFieldBeanTests.java#L36-L39 Using this type: https://github.com/palantir/conjure-java/blob/19d7f54eb0001c49b5d8a4bb897b9ad4cb5a28de/conjure-java-core/src/integrationInput/java/com/palantir/product/EmptyObjectExample.java ``` InvalidDefinitionException: Invalid type definition for type `com.palantir.product.EmptyObjectExample`: No argument left as delegating for Creator [method com.palantir.product.EmptyObjectExample#of()]: exactly one required at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 1] at app//com.fasterxml.jackson.databind.exc.InvalidDefinitionException.from(InvalidDefinitionException.java:62) at app//com.fasterxml.jackson.databind.DeserializationContext.reportBadTypeDefinition(DeserializationContext.java:1866) at app//com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._addExplicitDelegatingCreator(BasicDeserializerFactory.java:489) at app//com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._addExplicitDelegatingCreators(BasicDeserializerFactory.java:365) at app//com.fasterxml.jackson.databind.deser.BasicDeserializerFactory._constructDefaultValueInstantiator(BasicDeserializerFactory.java:270) at app//com.fasterxml.jackson.databind.deser.BasicDeserializerFactory.findValueInstantiator(BasicDeserializerFactory.java:219) at app//com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.buildBeanDeserializer(BeanDeserializerFactory.java:262) at app//com.fasterxml.jackson.databind.deser.BeanDeserializerFactory.createBeanDeserializer(BeanDeserializerFactory.java:151) at app//com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer2(DeserializerCache.java:471) at app//com.fasterxml.jackson.databind.deser.DeserializerCache._createDeserializer(DeserializerCache.java:415) at app//com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:317) at app//com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:284) at app//com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:174) at app//com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:669) at app//com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:5048) at app//com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4918) at app//com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3860) at app//com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3828) at app//com.palantir.conjure.java.types.NoFieldBeanTests.testDeserializeUsesSingleton(NoFieldBeanTests.java:38) ``` It's not entirely clear that we are correctly using the `DELEGATING` type here, perhaps the `PROPERTIES` type would be a better fit, however neither necessarily document support for a no-arg constructor. In general we prefer `DELEGATING` when possible to avoid ambiguity with potential property sources -- we want the ability to fail deserialization in this case when any properties are included in the incoming object. In 2.17.2, this branch allowed the code to functiona as we desired: https://github.com/FasterXML/jackson-databind/blob/091d968751ef00150d22a788fe7f45b7cdcb337a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java#L658-L662 Is there any chance we could allow the previous DELEGATING behavior to continue to handle the no-arg case? If we'd prefer to move away from `DELEGATING` for that case, is there any chance we could emit a deprecation warning for some time? I've included a potential patch which recovers the previous behavior.
1 parent 88f4671 commit c641863

File tree

2 files changed

+82
-0
lines changed

2 files changed

+82
-0
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,10 @@ private boolean _addExplicitDelegatingCreator(DeserializationContext ctxt,
468468
int ix = -1;
469469
final int argCount = candidate.paramCount();
470470
SettableBeanProperty[] properties = new SettableBeanProperty[argCount];
471+
if (argCount == 0) {
472+
creators.addPropertyCreator(candidate.creator(), true, properties);
473+
return true;
474+
}
471475
for (int i = 0; i < argCount; ++i) {
472476
AnnotatedParameter param = candidate.parameter(i);
473477
JacksonInject.Value injectId = candidate.injection(i);
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package com.fasterxml.jackson.databind.deser.creators;
2+
3+
import com.fasterxml.jackson.annotation.JsonCreator;
4+
import com.fasterxml.jackson.databind.ObjectMapper;
5+
import org.junit.jupiter.api.Test;
6+
7+
import static com.fasterxml.jackson.databind.testutil.DatabindTestUtil.newJsonMapper;
8+
import static org.junit.jupiter.api.Assertions.assertSame;
9+
10+
// [databind#4688]
11+
public class SingletonDelegatingCreatorTest
12+
{
13+
14+
static final class NoFieldSingletonWithDelegatingCreator {
15+
private static final NoFieldSingletonWithDelegatingCreator INSTANCE = new NoFieldSingletonWithDelegatingCreator();
16+
17+
private NoFieldSingletonWithDelegatingCreator() {}
18+
19+
@JsonCreator(mode = JsonCreator.Mode.DELEGATING)
20+
static NoFieldSingletonWithDelegatingCreator of() {
21+
return INSTANCE;
22+
}
23+
}
24+
25+
static final class NoFieldSingletonWithPropertiesCreator {
26+
private static final NoFieldSingletonWithPropertiesCreator INSTANCE = new NoFieldSingletonWithPropertiesCreator();
27+
28+
private NoFieldSingletonWithPropertiesCreator() {}
29+
30+
@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
31+
static NoFieldSingletonWithPropertiesCreator of() {
32+
return INSTANCE;
33+
}
34+
}
35+
36+
static final class NoFieldSingletonWithDefaultCreator {
37+
private static final NoFieldSingletonWithDefaultCreator INSTANCE = new NoFieldSingletonWithDefaultCreator();
38+
39+
private NoFieldSingletonWithDefaultCreator() {}
40+
41+
@JsonCreator
42+
static NoFieldSingletonWithDefaultCreator of() {
43+
return INSTANCE;
44+
}
45+
}
46+
47+
/*
48+
/**********************************************************************
49+
/* Test methods
50+
/**********************************************************************
51+
*/
52+
53+
private final ObjectMapper MAPPER = newJsonMapper();
54+
55+
@Test
56+
public void testNoFieldSingletonWithDelegatingCreator() throws Exception
57+
{
58+
NoFieldSingletonWithDelegatingCreator deserialized = MAPPER.readValue("{}",
59+
NoFieldSingletonWithDelegatingCreator.class);
60+
assertSame(NoFieldSingletonWithDelegatingCreator.INSTANCE, deserialized);
61+
}
62+
63+
@Test
64+
public void testNoFieldSingletonWithPropertiesCreator() throws Exception
65+
{
66+
NoFieldSingletonWithPropertiesCreator deserialized = MAPPER.readValue("{}",
67+
NoFieldSingletonWithPropertiesCreator.class);
68+
assertSame(NoFieldSingletonWithPropertiesCreator.INSTANCE, deserialized);
69+
}
70+
71+
@Test
72+
public void testNoFieldSingletonWithDefaultCreator() throws Exception
73+
{
74+
NoFieldSingletonWithDefaultCreator deserialized = MAPPER.readValue("{}",
75+
NoFieldSingletonWithDefaultCreator.class);
76+
assertSame(NoFieldSingletonWithDefaultCreator.INSTANCE, deserialized);
77+
}
78+
}

0 commit comments

Comments
 (0)