From bd89187ff3cb82e8c952718f597c6d8498430e95 Mon Sep 17 00:00:00 2001 From: Yih Tsern Date: Mon, 9 Jan 2023 03:16:12 +0800 Subject: [PATCH 1/2] Change Records deserialization to use the same mechanism as POJO deserialization. --- .../deser/BasicDeserializerFactory.java | 17 +- .../introspect/POJOPropertiesCollector.java | 90 +++-- .../jackson/databind/jdk14/JDK14Util.java | 32 +- ...orParameterNameAnnotationIntrospector.java | 28 ++ .../RecordBasicsTest.java | 102 +++++- .../databind/records/RecordCreatorsTest.java | 11 + .../records/RecordExplicitCreatorsTest.java | 326 ++++++++++++++++++ .../records/RecordImplicitCreatorsTest.java | 241 +++++++++++++ .../RecordNamingStrategy2992Test.java | 3 +- .../records/RecordTypeInfo3342Test.java | 58 ++++ .../RecordWithJsonNaming3102Test.java | 4 +- .../RecordWithJsonSetter2974Test.java | 2 +- 12 files changed, 859 insertions(+), 55 deletions(-) create mode 100644 src/test-jdk14/java/com/fasterxml/jackson/databind/records/Jdk8ConstructorParameterNameAnnotationIntrospector.java rename src/test-jdk14/java/com/fasterxml/jackson/databind/{failing => records}/RecordBasicsTest.java (61%) create mode 100644 src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordExplicitCreatorsTest.java create mode 100644 src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordImplicitCreatorsTest.java rename src/test-jdk14/java/com/fasterxml/jackson/databind/{failing => records}/RecordNamingStrategy2992Test.java (90%) create mode 100644 src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordTypeInfo3342Test.java rename src/test-jdk14/java/com/fasterxml/jackson/databind/{failing => records}/RecordWithJsonNaming3102Test.java (90%) rename src/test-jdk14/java/com/fasterxml/jackson/databind/{failing => records}/RecordWithJsonSetter2974Test.java (97%) 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 ae7206381a..e4d04aacb3 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BasicDeserializerFactory.java @@ -280,16 +280,6 @@ protected ValueInstantiator _constructDefaultValueInstantiator(DeserializationCo // constructors only usable on concrete types: if (beanDesc.getType().isConcrete()) { - // [databind#2709]: Record support - if (beanDesc.getType().isRecordType()) { - final List names = new ArrayList<>(); - // NOTE: this does verify that there is no explicitly annotated alternatives - AnnotatedConstructor canonical = JDK14Util.findRecordConstructor(ctxt, beanDesc, names); - if (canonical != null) { - _addRecordConstructor(ctxt, ccState, canonical, names); - return ccState.creators.constructValueInstantiator(ctxt); - } - } // 25-Jan-2017, tatu: As per [databind#1501], [databind#1502], [databind#1503], best // for now to skip attempts at using anything but no-args constructor (see // `InnerClassProperty` construction for that) @@ -400,7 +390,10 @@ public ValueInstantiator _valueInstantiatorInstance(DeserializationConfig config * constructor is to be used: if so, we have implicit names to consider. * * @since 2.12 + * @deprecated since 2.15 - no longer used, but kept because this protected method might have been overridden/used + * elsewhere */ + @Deprecated protected void _addRecordConstructor(DeserializationContext ctxt, CreatorCollectionState ccState, AnnotatedConstructor canonical, List implicitNames) throws JsonMappingException @@ -547,7 +540,9 @@ protected void _addImplicitConstructorCreators(DeserializationContext ctxt, JacksonInject.Value injectable = intr.findInjectableValue(param); final PropertyName name = (propDef == null) ? null : propDef.getFullName(); - if (propDef != null && propDef.isExplicitlyNamed()) { + if (propDef != null + // NOTE: Record canonical constructor will have implicitly named propDef + && (propDef.isExplicitlyNamed() || beanDesc.getType().isRecordType())) { ++explicitNameCount; properties[i] = constructCreatorProperty(ctxt, beanDesc, name, i, param, injectable); continue; diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index dc104fbd99..7611795a3d 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -9,6 +9,7 @@ import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.cfg.HandlerInstantiator; import com.fasterxml.jackson.databind.cfg.MapperConfig; +import com.fasterxml.jackson.databind.jdk14.JDK14Util; import com.fasterxml.jackson.databind.util.ClassUtil; /** @@ -610,23 +611,53 @@ protected void _addFields(Map props) protected void _addCreators(Map props) { // can be null if annotation processing is disabled... - if (!_useAnnotations) { - return; - } - for (AnnotatedConstructor ctor : _classDef.getConstructors()) { - if (_creatorProperties == null) { - _creatorProperties = new LinkedList(); + if (_useAnnotations) { + for (AnnotatedConstructor ctor : _classDef.getConstructors()) { + if (_creatorProperties == null) { + _creatorProperties = new LinkedList(); + } + for (int i = 0, len = ctor.getParameterCount(); i < len; ++i) { + _addCreatorParam(props, ctor.getParameter(i)); + } } - for (int i = 0, len = ctor.getParameterCount(); i < len; ++i) { - _addCreatorParam(props, ctor.getParameter(i)); + for (AnnotatedMethod factory : _classDef.getFactoryMethods()) { + if (_creatorProperties == null) { + _creatorProperties = new LinkedList(); + } + for (int i = 0, len = factory.getParameterCount(); i < len; ++i) { + _addCreatorParam(props, factory.getParameter(i)); + } } } - for (AnnotatedMethod factory : _classDef.getFactoryMethods()) { - if (_creatorProperties == null) { - _creatorProperties = new LinkedList(); - } - for (int i = 0, len = factory.getParameterCount(); i < len; ++i) { - _addCreatorParam(props, factory.getParameter(i)); + if (_classDef.getType().isRecordType()) { + List recordComponentNames = new ArrayList(); + AnnotatedConstructor canonicalCtor = JDK14Util.findRecordConstructor( + _classDef, _annotationIntrospector, _config, recordComponentNames); + + if (canonicalCtor != null) { + if (_creatorProperties == null) { + _creatorProperties = new LinkedList(); + } + + Set registeredParams = new HashSet(); + for (POJOPropertyBuilder creatorProperty : _creatorProperties) { + Iterator iter = creatorProperty.getConstructorParameters(); + while (iter.hasNext()) { + AnnotatedParameter param = iter.next(); + if (param.getOwner().equals(canonicalCtor)) { + registeredParams.add(param); + } + } + } + + if (_creatorProperties.isEmpty() || !registeredParams.isEmpty()) { + for (int i = 0; i < canonicalCtor.getParameterCount(); i++) { + AnnotatedParameter param = canonicalCtor.getParameter(i); + if (!registeredParams.contains(param)) { + _addCreatorParam(props, param, recordComponentNames.get(i)); + } + } + } } } } @@ -637,11 +668,23 @@ protected void _addCreators(Map props) protected void _addCreatorParam(Map props, AnnotatedParameter param) { - // JDK 8, paranamer, Scala can give implicit name - String impl = _annotationIntrospector.findImplicitPropertyName(param); - if (impl == null) { - impl = ""; + _addCreatorParam(props, param, null); + } + + private void _addCreatorParam(Map props, + AnnotatedParameter param, String recordComponentName) + { + String impl; + if (recordComponentName != null) { + impl = recordComponentName; + } else { + // JDK 8, paranamer, Scala can give implicit name + impl = _annotationIntrospector.findImplicitPropertyName(param); + if (impl == null) { + impl = ""; + } } + PropertyName pn = _annotationIntrospector.findNameForDeserialization(param); boolean expl = (pn != null && !pn.isEmpty()); if (!expl) { @@ -650,10 +693,13 @@ protected void _addCreatorParam(Map props, // this creator parameter -- may or may not be a problem, verified at a later point. return; } - // Also: if this occurs, there MUST be explicit annotation on creator itself - JsonCreator.Mode creatorMode = _annotationIntrospector.findCreatorAnnotation(_config, - param.getOwner()); - if ((creatorMode == null) || (creatorMode == JsonCreator.Mode.DISABLED)) { + + // Also: if this occurs, there MUST be explicit annotation on creator itself... + JsonCreator.Mode creatorMode = _annotationIntrospector.findCreatorAnnotation(_config, param.getOwner()); + // ...or is a Records canonical constructor + boolean isCanonicalConstructor = recordComponentName != null; + + if ((creatorMode == null || creatorMode == JsonCreator.Mode.DISABLED) && !isCanonicalConstructor) { return; } pn = PropertyName.construct(impl); diff --git a/src/main/java/com/fasterxml/jackson/databind/jdk14/JDK14Util.java b/src/main/java/com/fasterxml/jackson/databind/jdk14/JDK14Util.java index 604e05be60..640935f9f8 100644 --- a/src/main/java/com/fasterxml/jackson/databind/jdk14/JDK14Util.java +++ b/src/main/java/com/fasterxml/jackson/databind/jdk14/JDK14Util.java @@ -8,8 +8,9 @@ import com.fasterxml.jackson.annotation.JsonCreator.Mode; import com.fasterxml.jackson.databind.AnnotationIntrospector; import com.fasterxml.jackson.databind.BeanDescription; -import com.fasterxml.jackson.databind.DeserializationConfig; import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.cfg.MapperConfig; +import com.fasterxml.jackson.databind.introspect.AnnotatedClass; import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor; import com.fasterxml.jackson.databind.util.ClassUtil; import com.fasterxml.jackson.databind.util.NativeImageUtil; @@ -30,7 +31,12 @@ public static String[] getRecordFieldNames(Class recordType) { public static AnnotatedConstructor findRecordConstructor(DeserializationContext ctxt, BeanDescription beanDesc, List names) { - return new CreatorLocator(ctxt, beanDesc) + return findRecordConstructor(beanDesc.getClassInfo(), ctxt.getAnnotationIntrospector(), ctxt.getConfig(), names); + } + + public static AnnotatedConstructor findRecordConstructor(AnnotatedClass recordClass, + AnnotationIntrospector intr, MapperConfig config, List names) { + return new CreatorLocator(recordClass, intr, config) .locate(names); } @@ -150,25 +156,25 @@ public RawTypeName(Class rt, String n) { } static class CreatorLocator { - protected final BeanDescription _beanDesc; - protected final DeserializationConfig _config; + protected final AnnotatedClass _recordClass; + protected final MapperConfig _config; protected final AnnotationIntrospector _intr; protected final List _constructors; protected final AnnotatedConstructor _primaryConstructor; protected final RawTypeName[] _recordFields; - CreatorLocator(DeserializationContext ctxt, BeanDescription beanDesc) + CreatorLocator(AnnotatedClass recordClass, AnnotationIntrospector intr, MapperConfig config) { - _beanDesc = beanDesc; + _recordClass = recordClass; - _intr = ctxt.getAnnotationIntrospector(); - _config = ctxt.getConfig(); + _intr = intr; + _config = config; - _recordFields = RecordAccessor.instance().getRecordFields(beanDesc.getBeanClass()); + _recordFields = RecordAccessor.instance().getRecordFields(recordClass.getRawType()); if (_recordFields == null) { // not a record, or no reflective access on native image - _constructors = beanDesc.getConstructors(); + _constructors = recordClass.getConstructors(); _primaryConstructor = null; } else { final int argCount = _recordFields.length; @@ -179,10 +185,10 @@ static class CreatorLocator { // One special case: empty Records, empty constructor is separate case if (argCount == 0) { - primary = beanDesc.findDefaultConstructor(); + primary = recordClass.getDefaultConstructor(); _constructors = Collections.singletonList(primary); } else { - _constructors = beanDesc.getConstructors(); + _constructors = recordClass.getConstructors(); main_loop: for (AnnotatedConstructor ctor : _constructors) { if (ctor.getParameterCount() != argCount) { @@ -199,7 +205,7 @@ static class CreatorLocator { } if (primary == null) { throw new IllegalArgumentException("Failed to find the canonical Record constructor of type " - +ClassUtil.getTypeDescription(_beanDesc.getType())); + +ClassUtil.getTypeDescription(_recordClass.getType())); } _primaryConstructor = primary; } diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/Jdk8ConstructorParameterNameAnnotationIntrospector.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/Jdk8ConstructorParameterNameAnnotationIntrospector.java new file mode 100644 index 0000000000..d8200fcd8a --- /dev/null +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/Jdk8ConstructorParameterNameAnnotationIntrospector.java @@ -0,0 +1,28 @@ +package com.fasterxml.jackson.databind.records; + +import com.fasterxml.jackson.databind.introspect.AnnotatedConstructor; +import com.fasterxml.jackson.databind.introspect.AnnotatedMember; +import com.fasterxml.jackson.databind.introspect.AnnotatedParameter; +import com.fasterxml.jackson.databind.introspect.JacksonAnnotationIntrospector; + +class Jdk8ConstructorParameterNameAnnotationIntrospector extends JacksonAnnotationIntrospector { + + @Override + public String findImplicitPropertyName(AnnotatedMember member) { + if (!(member instanceof AnnotatedParameter)) { + return null; + } + AnnotatedParameter parameter = (AnnotatedParameter) member; + if (!(parameter.getOwner() instanceof AnnotatedConstructor)) { + return null; + } + AnnotatedConstructor constructor = (AnnotatedConstructor) parameter.getOwner(); + String parameterName = constructor.getAnnotated().getParameters()[parameter.getIndex()].getName(); + + if (parameterName == null || parameterName.isBlank()) { + throw new IllegalArgumentException("Unable to extract constructor parameter name for: " + member); + } + + return parameterName; + } +} diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordBasicsTest.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java similarity index 61% rename from src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordBasicsTest.java rename to src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java index 2f4ceb49e4..d16b9d7e0b 100644 --- a/src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordBasicsTest.java +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java @@ -1,17 +1,20 @@ -package com.fasterxml.jackson.databind.failing; +package com.fasterxml.jackson.databind.records; import com.fasterxml.jackson.annotation.*; import com.fasterxml.jackson.databind.*; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonNaming; +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.type.TypeFactory; import com.fasterxml.jackson.databind.util.ClassUtil; +import com.fasterxml.jackson.databind.util.Converter; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; -// 01-Dec-2022, tatu: Alas, fails on JDK 17 public class RecordBasicsTest extends BaseMapTest { record EmptyRecord() { } @@ -24,10 +27,22 @@ record RecordWithIgnore(int id, @JsonIgnore String name) { } record RecordWithRename(int id, @JsonProperty("rename")String name) { } + record RecordWithHeaderInject(int id, @JacksonInject String name) { } + + record RecordWithConstructorInject(int id, String name) { + + RecordWithConstructorInject(int id, @JacksonInject String name) { + this.id = id; + this.name = name; + } + } + // [databind#2992] @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) record SnakeRecord(String myId, String myValue){} + record RecordWithJsonDeserialize(int id, @JsonDeserialize(converter = StringTrimmer.class) String name) { } + private final ObjectMapper MAPPER = newJsonMapper(); /* @@ -116,7 +131,7 @@ public void testDeserializeSimpleRecord_DisableAnnotationIntrospector() throws E /* /********************************************************************** - /* Test methods, ignorals, renames + /* Test methods, ignorals, renames, injects /********************************************************************** */ @@ -125,6 +140,28 @@ public void testSerializeJsonIgnoreRecord() throws Exception { assertEquals("{\"id\":123}", json); } + /** + * This test-case is just for documentation purpose: + * Because unlike JavaBean where the setter can be ignored, the Record's constructor argument must + * have value. + *

+ * You can make a constructor parameter optional by {@link JacksonInject}-ing a value, or by creating an alternative + * JsonCreator.mode=PROPERTIES constructor that excludes the ignored parameter. + * + * @see #testDeserializeConstructorInjectRecord() + * @see RecordCreatorsTest#testDeserializeWithAltCtor() + */ + public void testDeserializeJsonIgnoreRecord_WillFail() throws Exception { + try { + MAPPER.readValue("{\"id\":123,\"name\":\"Bob\"}", RecordWithIgnore.class); + + fail("should not pass"); + } catch (InvalidDefinitionException e) { + verifyException(e, "Argument #1 of constructor"); + verifyException(e, "must have name when multiple-parameter constructor annotated as Creator"); + } + } + public void testSerializeJsonRename() throws Exception { String json = MAPPER.writeValueAsString(new RecordWithRename(123, "Bob")); final Object EXP = map("id", Integer.valueOf(123), "rename", "Bob"); @@ -137,6 +174,32 @@ public void testDeserializeJsonRename() throws Exception { assertEquals(new RecordWithRename(123, "Bob"), value); } + /** + * This test-case is just for documentation purpose: + * GOTCHA: Annotations on header will be propagated to the field, leading to this failure. + * + * @see #testDeserializeConstructorInjectRecord() + */ + public void testDeserializeHeaderInjectRecord_WillFail() throws Exception { + MAPPER.setInjectableValues(new InjectableValues.Std().addValue(String.class, "Bob")); + + try { + MAPPER.readValue("{\"id\":123}", RecordWithHeaderInject.class); + + fail("should not pass"); + } catch (IllegalArgumentException e) { + verifyException(e, "RecordWithHeaderInject#name"); + verifyException(e, "Can not set final java.lang.String field"); + } + } + + public void testDeserializeConstructorInjectRecord() throws Exception { + MAPPER.setInjectableValues(new InjectableValues.Std().addValue(String.class, "Bob")); + + RecordWithConstructorInject value = MAPPER.readValue("{\"id\":123}", RecordWithConstructorInject.class); + assertEquals(new RecordWithConstructorInject(123, "Bob"), value); + } + /* /********************************************************************** /* Test methods, naming strategy @@ -147,11 +210,26 @@ public void testDeserializeJsonRename() throws Exception { public void testNamingStrategy() throws Exception { SnakeRecord input = new SnakeRecord("123", "value"); + String json = MAPPER.writeValueAsString(input); + assertEquals("{\"my_id\":\"123\",\"my_value\":\"value\"}", json); + SnakeRecord output = MAPPER.readValue(json, SnakeRecord.class); assertEquals(input, output); } + /* + /********************************************************************** + /* Test methods, JsonDeserialize + /********************************************************************** + */ + + public void testDeserializeJsonDeserializeRecord() throws Exception { + RecordWithJsonDeserialize value = MAPPER.readValue("{\"id\":123,\"name\":\" Bob \"}", RecordWithJsonDeserialize.class); + + assertEquals(new RecordWithJsonDeserialize(123, "Bob"), value); + } + /* /********************************************************************** /* Internal helper methods @@ -165,4 +243,22 @@ private Map map(String key1, Object value1, result.put(key2, value2); return result; } + + public static class StringTrimmer implements Converter { + + @Override + public String convert(String value) { + return value.trim(); + } + + @Override + public JavaType getInputType(TypeFactory typeFactory) { + return typeFactory.constructType(String.class); + } + + @Override + public JavaType getOutputType(TypeFactory typeFactory) { + return typeFactory.constructType(String.class); + } + } } diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordCreatorsTest.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordCreatorsTest.java index 9cfda5749b..225db2274e 100644 --- a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordCreatorsTest.java +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordCreatorsTest.java @@ -57,6 +57,17 @@ public void testDeserializeWithAltCtor() throws Exception { RecordWithAltCtor.class); assertEquals(2812, value.id()); assertEquals("name2", value.name()); + + // "Implicit" canonical constructor can no longer be used when there's explicit constructor + try { + MAPPER.readValue("{\"id\":2812,\"name\":\"Bob\"}", + RecordWithAltCtor.class); + + fail("should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Can not set final java.lang.String field"); + verifyException(e, "RecordWithAltCtor.name"); + } } // [databind#2980] diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordExplicitCreatorsTest.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordExplicitCreatorsTest.java new file mode 100644 index 0000000000..18110d5176 --- /dev/null +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordExplicitCreatorsTest.java @@ -0,0 +1,326 @@ +package com.fasterxml.jackson.databind.records; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.BaseMapTest; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; + +import java.math.BigDecimal; + +public class RecordExplicitCreatorsTest extends BaseMapTest +{ + record RecordWithOneJsonPropertyWithoutJsonCreator(int id, String name) { + + public RecordWithOneJsonPropertyWithoutJsonCreator(@JsonProperty("id_only") int id) { + this(id, "JsonPropertyConstructor"); + } + + public static RecordWithOneJsonPropertyWithoutJsonCreator valueOf(int id) { + return new RecordWithOneJsonPropertyWithoutJsonCreator(id); + } + } + + record RecordWithTwoJsonPropertyWithoutJsonCreator(int id, String name, String email) { + + public RecordWithTwoJsonPropertyWithoutJsonCreator(@JsonProperty("the_id") int id, @JsonProperty("the_email") String email) { + this(id, "TwoJsonPropertyConstructor", email); + } + + public static RecordWithTwoJsonPropertyWithoutJsonCreator valueOf(int id) { + return new RecordWithTwoJsonPropertyWithoutJsonCreator(id, "factory@example.com"); + } + } + + record RecordWithJsonPropertyAndImplicitPropertyWithoutJsonCreator(int id, String name, String email) { + + public RecordWithJsonPropertyAndImplicitPropertyWithoutJsonCreator(@JsonProperty("id_only") int id, String email) { + this(id, "JsonPropertyConstructor", email); + } + } + + record RecordWithJsonPropertyWithJsonCreator(int id, String name) { + + @JsonCreator + public RecordWithJsonPropertyWithJsonCreator(@JsonProperty("id_only") int id) { + this(id, "JsonCreatorConstructor"); + } + + public static RecordWithJsonPropertyWithJsonCreator valueOf(int id) { + return new RecordWithJsonPropertyWithJsonCreator(id); + } + } + + record RecordWithJsonPropertyAndImplicitPropertyWithJsonCreator(int id, String name, String email) { + + @JsonCreator + public RecordWithJsonPropertyAndImplicitPropertyWithJsonCreator(@JsonProperty("id_only") int id, String email) { + this(id, "JsonPropertyConstructor", email); + } + } + + record RecordWithMultiExplicitDelegatingConstructor(int id, String name) { + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public RecordWithMultiExplicitDelegatingConstructor(int id) { + this(id, "IntConstructor"); + } + + @JsonCreator(mode = JsonCreator.Mode.DELEGATING) + public RecordWithMultiExplicitDelegatingConstructor(String id) { + this(Integer.parseInt(id), "StringConstructor"); + } + } + + record RecordWithDisabledJsonCreator(int id, String name) { + + @JsonCreator(mode = JsonCreator.Mode.DISABLED) + RecordWithDisabledJsonCreator { + } + } + + record RecordWithExplicitFactoryMethod(BigDecimal id, String name) { + + @JsonCreator + public static RecordWithExplicitFactoryMethod valueOf(int id) { + return new RecordWithExplicitFactoryMethod(BigDecimal.valueOf(id), "IntFactoryMethod"); + } + + public static RecordWithExplicitFactoryMethod valueOf(double id) { + return new RecordWithExplicitFactoryMethod(BigDecimal.valueOf(id), "DoubleFactoryMethod"); + } + + @JsonCreator + public static RecordWithExplicitFactoryMethod valueOf(String id) { + return new RecordWithExplicitFactoryMethod(BigDecimal.valueOf(Double.parseDouble(id)), "StringFactoryMethod"); + } + } + + private final ObjectMapper MAPPER = jsonMapperBuilder() + .disable(MapperFeature.ALLOW_FINAL_FIELDS_AS_MUTATORS) // So that test cases don't have to assert weird error messages + .build(); + + /* + /********************************************************************** + /* Test methods, "implicit" (?) constructor with JsonProperty without JsonCreator + /********************************************************************** + */ + + public void testDeserializeUsingJsonPropertyConstructor_WithoutJsonCreator() throws Exception { + RecordWithOneJsonPropertyWithoutJsonCreator oneJsonPropertyValue = MAPPER.readValue( + "{\"id_only\":123}", + RecordWithOneJsonPropertyWithoutJsonCreator.class); + assertEquals(new RecordWithOneJsonPropertyWithoutJsonCreator(123), oneJsonPropertyValue); + + RecordWithTwoJsonPropertyWithoutJsonCreator twoJsonPropertyValue = MAPPER.readValue( + "{\"the_id\":123,\"the_email\":\"bob@example.com\"}", + RecordWithTwoJsonPropertyWithoutJsonCreator.class); + assertEquals(new RecordWithTwoJsonPropertyWithoutJsonCreator(123, "bob@example.com"), twoJsonPropertyValue); + } + + /** + * Only 1 properties-based creator allowed, so can no longer use the (un-annotated) canonical constructor. + */ + public void testDeserializeUsingCanonicalConstructor_WhenJsonPropertyConstructorExists_WillFail() throws Exception { + try { + MAPPER.readValue( + "{\"id\":123,\"name\":\"Bobby\"}", + RecordWithOneJsonPropertyWithoutJsonCreator.class); + + fail("should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Unrecognized field \"id\""); + verifyException(e, "RecordWithOneJsonPropertyWithoutJsonCreator"); + verifyException(e, "one known property: \"id_only\""); + } + + try { + MAPPER.readValue( + "{\"id\":123,\"name\":\"Bobby\",\"email\":\"bobby@example.com\"}", + RecordWithTwoJsonPropertyWithoutJsonCreator.class); + + fail("should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Unrecognized field \"id\""); + verifyException(e, "RecordWithTwoJsonPropertyWithoutJsonCreator"); + verifyException(e, "2 known properties: \"the_id\", \"the_email\""); + } + } + + public void testDeserializeUsingImplicitFactoryMethod_WhenJsonPropertyConstructorExists() throws Exception { + RecordWithOneJsonPropertyWithoutJsonCreator oneJsonPropertyValue = MAPPER.readValue( + "123", + RecordWithOneJsonPropertyWithoutJsonCreator.class); + assertEquals(RecordWithOneJsonPropertyWithoutJsonCreator.valueOf(123), oneJsonPropertyValue); + + RecordWithTwoJsonPropertyWithoutJsonCreator twoJsonPropertyValue = MAPPER.readValue( + "123", + RecordWithTwoJsonPropertyWithoutJsonCreator.class); + assertEquals(RecordWithTwoJsonPropertyWithoutJsonCreator.valueOf(123), twoJsonPropertyValue); + } + + /* + /********************************************************************** + /* Test methods, explicit constructor with JsonProperty with JsonCreator + /********************************************************************** + */ + + public void testDeserializeUsingJsonCreatorConstructor() throws Exception { + RecordWithJsonPropertyWithJsonCreator value = MAPPER.readValue("{\"id_only\":123}", RecordWithJsonPropertyWithJsonCreator.class); + + assertEquals(new RecordWithJsonPropertyWithJsonCreator(123), value); + } + + /** + * Only 1 properties-based creator allowed, so can no longer use the (un-annotated) canonical constructor + */ + public void testDeserializeUsingCanonicalConstructor_WhenJsonCreatorConstructorExists_WillFail() throws Exception { + try { + MAPPER.readValue("{\"id\":123,\"name\":\"Bobby\"}", RecordWithJsonPropertyWithJsonCreator.class); + + fail("should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Unrecognized field \"id\""); + verifyException(e, "RecordWithJsonPropertyWithJsonCreator"); + verifyException(e, "one known property: \"id_only\""); + } + } + + public void testDeserializeUsingImplicitFactoryMethod_WhenJsonCreatorConstructorExists_WillFail() throws Exception { + try { + MAPPER.readValue("123", RecordWithJsonPropertyWithJsonCreator.class); + + fail("should not pass"); + } catch (MismatchedInputException e) { + verifyException(e, "Cannot construct instance"); + verifyException(e, "RecordWithJsonPropertyWithJsonCreator"); + verifyException(e, "although at least one Creator exists"); + verifyException(e, "no int/Int-argument constructor/factory method"); + } + } + + /* + /********************************************************************** + /* Test methods, multiple explicit delegating constructors + /********************************************************************** + */ + + public void testDeserializeUsingExplicitDelegatingConstructors() throws Exception { + RecordWithMultiExplicitDelegatingConstructor intConstructorValue = MAPPER.readValue("123", RecordWithMultiExplicitDelegatingConstructor.class); + assertEquals(new RecordWithMultiExplicitDelegatingConstructor(123, "IntConstructor"), intConstructorValue); + + RecordWithMultiExplicitDelegatingConstructor stringConstructorValue = MAPPER.readValue("\"123\"", RecordWithMultiExplicitDelegatingConstructor.class); + assertEquals(new RecordWithMultiExplicitDelegatingConstructor(123, "StringConstructor"), stringConstructorValue); + } + + /* + /********************************************************************** + /* Test methods, JsonCreator.mode=DISABLED + /********************************************************************** + */ + + public void testDeserializeUsingDisabledConstructors_WillFail() throws Exception { + try { + MAPPER.readValue("{\"id\":123,\"name\":\"Bobby\"}", RecordWithDisabledJsonCreator.class); + + fail("should not pass"); + } catch (InvalidDefinitionException e) { + verifyException(e, "Cannot construct instance"); + verifyException(e, "RecordWithDisabledJsonCreator"); + verifyException(e, "no Creators, like default constructor, exist"); + verifyException(e, "cannot deserialize from Object value"); + } + + } + + /* + /********************************************************************** + /* Test methods, explicit factory methods + /********************************************************************** + */ + + public void testDeserializeUsingExplicitFactoryMethods() throws Exception { + RecordWithExplicitFactoryMethod intFactoryValue = MAPPER.readValue("123", RecordWithExplicitFactoryMethod.class); + assertEquals(new RecordWithExplicitFactoryMethod(BigDecimal.valueOf(123), "IntFactoryMethod"), intFactoryValue); + + RecordWithExplicitFactoryMethod stringFactoryValue = MAPPER.readValue("\"123.4\"", RecordWithExplicitFactoryMethod.class); + assertEquals(new RecordWithExplicitFactoryMethod(BigDecimal.valueOf(123.4), "StringFactoryMethod"), stringFactoryValue); + } + + /** + * Implicit factory methods detection is only activated when there's no explicit (i.e. annotated + * with {@link JsonCreator}) factory methods. + */ + public void testDeserializeUsingImplicitFactoryMethods_WhenExplicitFactoryMethodsExist_WillFail() throws Exception { + try { + MAPPER.readValue("123.4", RecordWithExplicitFactoryMethod.class); + + fail("should not pass"); + } catch (MismatchedInputException e) { + verifyException(e, "Cannot construct instance"); + verifyException(e, "RecordWithExplicitFactoryMethod"); + verifyException(e, "although at least one Creator exists"); + verifyException(e, "no double/Double-argument constructor/factory"); + } + } + + /** + * Just like how no-arg constructor + setters will still be used to deserialize JSON Object even when + * there's JsonCreator factory method(s) in the JavaBean class. + */ + public void testDeserializeUsingImplicitCanonicalConstructor_WhenFactoryMethodsExist() throws Exception { + RecordWithExplicitFactoryMethod value = MAPPER.readValue("{\"id\":123.4,\"name\":\"CanonicalConstructor\"}", RecordWithExplicitFactoryMethod.class); + + assertEquals(new RecordWithExplicitFactoryMethod(BigDecimal.valueOf(123.4), "CanonicalConstructor"), value); + } + + /* + /********************************************************************** + /* Test methods, implicit parameter names + /********************************************************************** + */ + + public void testDeserializeMultipleConstructorsRecord_WithExplicitAndImplicitParameterNames_WithJsonCreator() throws Exception { + MAPPER.setAnnotationIntrospector(new Jdk8ConstructorParameterNameAnnotationIntrospector()); + + RecordWithJsonPropertyAndImplicitPropertyWithJsonCreator value = MAPPER.readValue( + "{\"id_only\":123,\"email\":\"bob@example.com\"}", + RecordWithJsonPropertyAndImplicitPropertyWithJsonCreator.class); + assertEquals(new RecordWithJsonPropertyAndImplicitPropertyWithJsonCreator(123, "bob@example.com"), value); + } + + /** + * This test-case is just for documentation purpose: + * GOTCHA: The problem is there are two usable constructors: + *

    + *
  1. Canonical constructor
    2. + *
  2. Non-canonical constructor with JsonProperty parameter
    3. + *
+ * ...so Jackson-Databind decided NOT to choose any. To overcome this, annotate JsonCreator on the non-canonical + * constructor. + *

+ * Similar behaviour is observed if a JavaBean has two usable constructors. + * + * @see #testDeserializeUsingJsonCreatorConstructor() + * @see #testDeserializeUsingCanonicalConstructor_WhenJsonCreatorConstructorExists_WillFail() + */ + public void testDeserializeMultipleConstructorsRecord_WithExplicitAndImplicitParameterNames() throws Exception { + MAPPER.setAnnotationIntrospector(new Jdk8ConstructorParameterNameAnnotationIntrospector()); + + try { + MAPPER.readValue( + "{\"id_only\":123,\"email\":\"bob@example.com\"}", + RecordWithJsonPropertyAndImplicitPropertyWithoutJsonCreator.class); + + fail("should not pass"); + } catch (InvalidDefinitionException e) { + verifyException(e, "Cannot construct instance"); + verifyException(e, "RecordWithJsonPropertyAndImplicitPropertyWithoutJsonCreator"); + verifyException(e, "no Creators, like default constructor, exist"); + verifyException(e, "cannot deserialize from Object value"); + } + } +} diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordImplicitCreatorsTest.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordImplicitCreatorsTest.java new file mode 100644 index 0000000000..7918ef3369 --- /dev/null +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordImplicitCreatorsTest.java @@ -0,0 +1,241 @@ +package com.fasterxml.jackson.databind.records; + +import com.fasterxml.jackson.databind.BaseMapTest; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.cfg.ConstructorDetector; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; + +import java.math.BigDecimal; + +public class RecordImplicitCreatorsTest extends BaseMapTest +{ + record RecordWithImplicitFactoryMethods(BigDecimal id, String name) { + + public static RecordWithImplicitFactoryMethods valueOf(int id) { + return new RecordWithImplicitFactoryMethods(BigDecimal.valueOf(id), "IntFactoryMethod"); + } + + public static RecordWithImplicitFactoryMethods valueOf(double id) { + return new RecordWithImplicitFactoryMethods(BigDecimal.valueOf(id), "DoubleFactoryMethod"); + } + + public static RecordWithImplicitFactoryMethods valueOf(String id) { + return new RecordWithImplicitFactoryMethods(BigDecimal.valueOf(Double.parseDouble(id)), "StringFactoryMethod"); + } + } + + record RecordWithSingleValueConstructor(int id) { + } + + record RecordWithNonCanonicalConstructor(int id, String name, String email) { + + public RecordWithNonCanonicalConstructor(int id, String email) { + this(id, "NonCanonicalConstructor", email); + } + } + + /** + * Similar to: + * 

+     *   public class MyBean {
+     *       ...
+     *       // Single-arg constructor used by delegating creator.
+     *       public MyBean(int id) { ... }
+     *
+     *       // No-arg constructor used by properties-based creator.
+     *       public MyBean() {}
+     *
+     *       // Setters used by properties-based creator.
+     *       public void setId(int id) { ... }
+     *       public void setName(String name) { ... }
+     *   }
+     *
+ */ + record RecordWithAltSingleValueConstructor(int id, String name) { + + public RecordWithAltSingleValueConstructor(int id) { + this(id, "SingleValueConstructor"); + } + } + + private final ObjectMapper MAPPER = newJsonMapper(); + + /* + /********************************************************************** + /* Test methods, implicit factory methods & "implicit" canonical constructor + /********************************************************************** + */ + + public void testDeserializeUsingImplicitIntegerFactoryMethod() throws Exception { + RecordWithImplicitFactoryMethods factoryMethodValue = MAPPER.readValue("123", RecordWithImplicitFactoryMethods.class); + + assertEquals(new RecordWithImplicitFactoryMethods(BigDecimal.valueOf(123), "IntFactoryMethod"), factoryMethodValue); + } + + public void testDeserializeUsingImplicitDoubleFactoryMethod() throws Exception { + RecordWithImplicitFactoryMethods value = MAPPER.readValue("123.4", RecordWithImplicitFactoryMethods.class); + + assertEquals(new RecordWithImplicitFactoryMethods(BigDecimal.valueOf(123.4), "DoubleFactoryMethod"), value); + } + + public void testDeserializeUsingImplicitStringFactoryMethod() throws Exception { + RecordWithImplicitFactoryMethods value = MAPPER.readValue("\"123.4\"", RecordWithImplicitFactoryMethods.class); + + assertEquals(new RecordWithImplicitFactoryMethods(BigDecimal.valueOf(123.4), "StringFactoryMethod"), value); + } + + public void testDeserializeUsingImplicitCanonicalConstructor_WhenImplicitFactoryMethodsExist() throws Exception { + RecordWithImplicitFactoryMethods value = MAPPER.readValue( + "{\"id\":123.4,\"name\":\"CanonicalConstructor\"}", + RecordWithImplicitFactoryMethods.class); + + assertEquals(new RecordWithImplicitFactoryMethods(BigDecimal.valueOf(123.4), "CanonicalConstructor"), value); + } + + public void testDeserializeUsingImplicitFactoryMethod_WithAutoDetectCreatorsDisabled_WillFail() throws Exception { + MAPPER.disable(MapperFeature.AUTO_DETECT_CREATORS); + + try { + MAPPER.readValue("123", RecordWithImplicitFactoryMethods.class); + + fail("should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Cannot construct instance"); + verifyException(e, "RecordWithImplicitFactoryMethod"); + verifyException(e, "no int/Int-argument constructor/factory method"); + } + } + + /* + /********************************************************************** + /* Test methods, implicit single-value constructor + /********************************************************************** + */ + + /** + * This test-case is just for documentation purpose: + * GOTCHA: For JavaBean, only having single-value constructor results in implicit delegating creator. But for + * Records, the CANONICAL single-value constructor results in properties-based creator. + *

+ * Only when there's NON-CANONICAL single-value constructor will there be implicit delegating creator - see + * {@link #testDeserializeUsingImplicitDelegatingConstructor()}. + *

+ * yihtserns: maybe we can change this to adopt JavaBean's behaviour, but I prefer to not break existing behaviour + * until and unless there's a discussion on this. + */ + public void testDeserializeUsingImplicitSingleValueConstructor() throws Exception { + try { + // Cannot use delegating creator, unlike when dealing with JavaBean + MAPPER.readValue("123", RecordWithSingleValueConstructor.class); + + fail("should not pass"); + } catch (MismatchedInputException e) { + verifyException(e, "Cannot construct instance"); + verifyException(e, "RecordWithSingleValueConstructor"); + verifyException(e, "at least one Creator exists"); + verifyException(e, "no int/Int-argument constructor/factory method"); + } + + // Can only use properties-based creator + RecordWithSingleValueConstructor value = MAPPER.readValue("{\"id\":123}", RecordWithSingleValueConstructor.class); + assertEquals(new RecordWithSingleValueConstructor(123), value); + } + + /** + * This test-case is just for documentation purpose: + * See {@link #testDeserializeUsingImplicitSingleValueConstructor} + */ + public void testDeserializeSingleValueConstructor_WithDelegatingConstructorDetector_WillFail() throws Exception { + MAPPER.setConstructorDetector(ConstructorDetector.USE_DELEGATING); + + try { + // Fail, no delegating creator + MAPPER.readValue("123", RecordWithSingleValueConstructor.class); + + fail("should not pass"); + } catch (MismatchedInputException e) { + verifyException(e, "Cannot construct instance"); + verifyException(e, "RecordWithSingleValueConstructor"); + verifyException(e, "at least one Creator exists"); + verifyException(e, "no int/Int-argument constructor/factory method"); + } + + // Only have properties-based creator + RecordWithSingleValueConstructor value = MAPPER.readValue("{\"id\":123}", RecordWithSingleValueConstructor.class); + assertEquals(new RecordWithSingleValueConstructor(123), value); + } + + /** + * This is just to catch any potential regression. + */ + public void testDeserializeSingleValueConstructor_WithPropertiesBasedConstructorDetector_WillFail() throws Exception { + MAPPER.setConstructorDetector(ConstructorDetector.USE_PROPERTIES_BASED); + + try { + // This should fail + MAPPER.readValue("123", RecordWithSingleValueConstructor.class); + + fail("should not pass"); + } catch (MismatchedInputException e) { + verifyException(e, "Cannot construct instance"); + verifyException(e, "RecordWithSingleValueConstructor"); + verifyException(e, "at least one Creator exists"); + verifyException(e, "no int/Int-argument constructor/factory method"); + } + + // This should work + RecordWithSingleValueConstructor value = MAPPER.readValue("{\"id\":123}", RecordWithSingleValueConstructor.class); + assertEquals(new RecordWithSingleValueConstructor(123), value); + } + + /* + /********************************************************************** + /* Test methods, implicit properties-based + delegating constructor + /********************************************************************** + */ + + public void testDeserializeUsingImplicitPropertiesBasedConstructor() throws Exception { + RecordWithAltSingleValueConstructor value = MAPPER.readValue( + "{\"id\":123,\"name\":\"PropertiesBasedConstructor\"}", + RecordWithAltSingleValueConstructor.class); + + assertEquals(new RecordWithAltSingleValueConstructor(123, "PropertiesBasedConstructor"), value); + } + + /** + * @see #testDeserializeUsingImplicitSingleValueConstructor() + */ + public void testDeserializeUsingImplicitDelegatingConstructor() throws Exception { + RecordWithAltSingleValueConstructor value = MAPPER.readValue("123", RecordWithAltSingleValueConstructor.class); + + assertEquals(new RecordWithAltSingleValueConstructor(123, "SingleValueConstructor"), value); + } + + /* + /********************************************************************** + /* Test methods, implicit parameter names + /********************************************************************** + */ + + public void testDeserializeMultipleConstructorsRecord_WithImplicitParameterNames_WillUseCanonicalConstructor() throws Exception { + MAPPER.setAnnotationIntrospector(new Jdk8ConstructorParameterNameAnnotationIntrospector()); + + RecordWithNonCanonicalConstructor value = MAPPER.readValue( + "{\"id\":123,\"name\":\"Bob\",\"email\":\"bob@example.com\"}", + RecordWithNonCanonicalConstructor.class); + + assertEquals(new RecordWithNonCanonicalConstructor(123, "Bob", "bob@example.com"), value); + } + + public void testDeserializeMultipleConstructorsRecord_WithImplicitParameterNames_WillIgnoreNonCanonicalConstructor() throws Exception { + MAPPER.setAnnotationIntrospector(new Jdk8ConstructorParameterNameAnnotationIntrospector()); + + RecordWithNonCanonicalConstructor value = MAPPER.readValue( + "{\"id\":123,\"email\":\"bob@example.com\"}", + RecordWithNonCanonicalConstructor.class); + + assertEquals(new RecordWithNonCanonicalConstructor(123, null, "bob@example.com"), value); + } +} diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordNamingStrategy2992Test.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordNamingStrategy2992Test.java similarity index 90% rename from src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordNamingStrategy2992Test.java rename to src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordNamingStrategy2992Test.java index 711a199f9f..bf4d34990b 100644 --- a/src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordNamingStrategy2992Test.java +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordNamingStrategy2992Test.java @@ -1,11 +1,10 @@ -package com.fasterxml.jackson.databind.failing; +package com.fasterxml.jackson.databind.records; import com.fasterxml.jackson.databind.BaseMapTest; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.PropertyNamingStrategies; import com.fasterxml.jackson.databind.annotation.JsonNaming; -// 01-Dec-2022, tatu: Alas, fails on JDK 17 public class RecordNamingStrategy2992Test extends BaseMapTest { @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordTypeInfo3342Test.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordTypeInfo3342Test.java new file mode 100644 index 0000000000..c565832b40 --- /dev/null +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordTypeInfo3342Test.java @@ -0,0 +1,58 @@ +package com.fasterxml.jackson.databind.records; + +import com.fasterxml.jackson.annotation.JsonSubTypes; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.databind.BaseMapTest; +import com.fasterxml.jackson.databind.ObjectMapper; + +// [databind#3102] +public class RecordTypeInfo3342Test extends BaseMapTest +{ + public enum SpiceLevel { + LOW, + HIGH + } + + public interface SpiceTolerance { + } + + public record LowSpiceTolerance(String food) implements SpiceTolerance { + } + + public record HighSpiceTolerance(String food) implements SpiceTolerance { + } + + public record Example( + SpiceLevel level, + @JsonTypeInfo( + use = JsonTypeInfo.Id.NAME, + include = JsonTypeInfo.As.EXTERNAL_PROPERTY, + property = "level") + @JsonSubTypes({ + @JsonSubTypes.Type(value = LowSpiceTolerance.class, name = "LOW"), + @JsonSubTypes.Type(value = HighSpiceTolerance.class, name = "HIGH") + }) + SpiceTolerance tolerance) { } + + private final ObjectMapper MAPPER = newJsonMapper(); + + public void testSerializeDeserializeJsonSubType_LOW() throws Exception { + Example record = new Example(SpiceLevel.LOW, new LowSpiceTolerance("Tomato")); + + String json = MAPPER.writeValueAsString(record); + assertEquals("{\"level\":\"LOW\",\"tolerance\":{\"food\":\"Tomato\"}}", json); + + Example value = MAPPER.readValue(json, Example.class); + assertEquals(record, value); + } + + public void testSerializeDeserializeJsonSubType_HIGH() throws Exception { + Example record = new Example(SpiceLevel.HIGH, new HighSpiceTolerance("Chilli")); + + String json = MAPPER.writeValueAsString(record); + assertEquals("{\"level\":\"HIGH\",\"tolerance\":{\"food\":\"Chilli\"}}", json); + + Example value = MAPPER.readValue(json, Example.class); + assertEquals(record, value); + } +} diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordWithJsonNaming3102Test.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonNaming3102Test.java similarity index 90% rename from src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordWithJsonNaming3102Test.java rename to src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonNaming3102Test.java index f554450143..3533b6362b 100644 --- a/src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordWithJsonNaming3102Test.java +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonNaming3102Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.databind.failing; +package com.fasterxml.jackson.databind.records; import com.fasterxml.jackson.annotation.JsonCreator; @@ -8,8 +8,6 @@ import com.fasterxml.jackson.databind.PropertyNamingStrategies; import com.fasterxml.jackson.databind.annotation.JsonNaming; -// [databind#3102]: fails on JDK 16 which finally blocks mutation -// of Record fields. public class RecordWithJsonNaming3102Test extends BaseMapTest { @JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordWithJsonSetter2974Test.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonSetter2974Test.java similarity index 97% rename from src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordWithJsonSetter2974Test.java rename to src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonSetter2974Test.java index 06ee8e75c2..a54825b83e 100644 --- a/src/test-jdk14/java/com/fasterxml/jackson/databind/failing/RecordWithJsonSetter2974Test.java +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonSetter2974Test.java @@ -1,4 +1,4 @@ -package com.fasterxml.jackson.databind.failing; +package com.fasterxml.jackson.databind.records; import java.util.List; import java.util.Map; From eaf013a5003a5988b8c134851fc6f554a26727ba Mon Sep 17 00:00:00 2001 From: Yih Tsern Date: Sat, 14 Jan 2023 19:32:13 +0800 Subject: [PATCH 2/2] Special handling to support JsonIgnore for Records deserialization. --- .../databind/deser/BeanDeserializer.java | 7 +- .../introspect/POJOPropertiesCollector.java | 11 +++ .../databind/records/RecordBasicsTest.java | 34 +------ .../records/RecordWithJsonIgnoreTest.java | 93 +++++++++++++++++++ 4 files changed, 111 insertions(+), 34 deletions(-) create mode 100644 src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonIgnoreTest.java diff --git a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java index a7da130872..b883c382ec 100644 --- a/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java +++ b/src/main/java/com/fasterxml/jackson/databind/deser/BeanDeserializer.java @@ -466,7 +466,12 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri } // regular property? needs buffering SettableBeanProperty prop = _beanProperties.find(propName); - if (prop != null) { + // Special handling because Records' ignored creator props weren't removed (to help in creating + // constructor-backed PropertyCreator) so they ended up in _beanProperties, unlike POJO (whose ignored + // props are removed) + boolean isClassWithoutMutator = _beanType.isRecordType(); + + if (prop != null && !isClassWithoutMutator) { try { buffer.bufferProperty(prop, _deserializeWithErrorWrapping(p, ctxt, prop)); } catch (UnresolvedForwardReference reference) { diff --git a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java index 7611795a3d..0e83a8bd2f 100644 --- a/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java +++ b/src/main/java/com/fasterxml/jackson/databind/introspect/POJOPropertiesCollector.java @@ -948,6 +948,17 @@ protected void _removeUnwantedProperties(Map props) } // Otherwise, check ignorals if (prop.anyIgnorals()) { + // Special handling for Records, as they do not have mutators so relying on constructors with (mostly) + // implicitly-named parameters... + if (_classDef.getType().isRecordType()) { + // ...so can only remove ignored field and/or accessors, not constructor parameters that are needed + // for instantiation... + prop.removeIgnored(); + // ...which will then be ignored (the incoming property value) during deserialization + _collectIgnorals(prop.getName()); + continue; + } + // first: if one or more ignorals, and no explicit markers, remove the whole thing // 16-May-2022, tatu: NOTE! As per [databind#3357] need to consider // only explicit inclusion by accessors OTHER than ones with ignoral marker diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java index d16b9d7e0b..ca1e01faf1 100644 --- a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordBasicsTest.java @@ -5,7 +5,6 @@ import com.fasterxml.jackson.databind.*; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonNaming; -import com.fasterxml.jackson.databind.exc.InvalidDefinitionException; import com.fasterxml.jackson.databind.json.JsonMapper; import com.fasterxml.jackson.databind.type.TypeFactory; import com.fasterxml.jackson.databind.util.ClassUtil; @@ -23,8 +22,6 @@ record SimpleRecord(int id, String name) { } record RecordOfRecord(SimpleRecord record) { } - record RecordWithIgnore(int id, @JsonIgnore String name) { } - record RecordWithRename(int id, @JsonProperty("rename")String name) { } record RecordWithHeaderInject(int id, @JacksonInject String name) { } @@ -56,7 +53,6 @@ public void testClassUtil() { assertTrue(ClassUtil.isRecordType(SimpleRecord.class)); assertTrue(ClassUtil.isRecordType(RecordOfRecord.class)); - assertTrue(ClassUtil.isRecordType(RecordWithIgnore.class)); assertTrue(ClassUtil.isRecordType(RecordWithRename.class)); } @@ -65,7 +61,6 @@ public void testRecordJavaType() { assertTrue(MAPPER.constructType(SimpleRecord.class).isRecordType()); assertTrue(MAPPER.constructType(RecordOfRecord.class).isRecordType()); - assertTrue(MAPPER.constructType(RecordWithIgnore.class).isRecordType()); assertTrue(MAPPER.constructType(RecordWithRename.class).isRecordType()); } @@ -131,37 +126,10 @@ public void testDeserializeSimpleRecord_DisableAnnotationIntrospector() throws E /* /********************************************************************** - /* Test methods, ignorals, renames, injects + /* Test methods, renames, injects /********************************************************************** */ - public void testSerializeJsonIgnoreRecord() throws Exception { - String json = MAPPER.writeValueAsString(new RecordWithIgnore(123, "Bob")); - assertEquals("{\"id\":123}", json); - } - - /** - * This test-case is just for documentation purpose: - * Because unlike JavaBean where the setter can be ignored, the Record's constructor argument must - * have value. - *

- * You can make a constructor parameter optional by {@link JacksonInject}-ing a value, or by creating an alternative - * JsonCreator.mode=PROPERTIES constructor that excludes the ignored parameter. - * - * @see #testDeserializeConstructorInjectRecord() - * @see RecordCreatorsTest#testDeserializeWithAltCtor() - */ - public void testDeserializeJsonIgnoreRecord_WillFail() throws Exception { - try { - MAPPER.readValue("{\"id\":123,\"name\":\"Bob\"}", RecordWithIgnore.class); - - fail("should not pass"); - } catch (InvalidDefinitionException e) { - verifyException(e, "Argument #1 of constructor"); - verifyException(e, "must have name when multiple-parameter constructor annotated as Creator"); - } - } - public void testSerializeJsonRename() throws Exception { String json = MAPPER.writeValueAsString(new RecordWithRename(123, "Bob")); final Object EXP = map("id", Integer.valueOf(123), "rename", "Bob"); diff --git a/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonIgnoreTest.java b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonIgnoreTest.java new file mode 100644 index 0000000000..3004cbebf9 --- /dev/null +++ b/src/test-jdk14/java/com/fasterxml/jackson/databind/records/RecordWithJsonIgnoreTest.java @@ -0,0 +1,93 @@ +package com.fasterxml.jackson.databind.records; + +import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.BaseMapTest; +import com.fasterxml.jackson.databind.ObjectMapper; + +public class RecordWithJsonIgnoreTest extends BaseMapTest +{ + record RecordWithIgnore(int id, @JsonIgnore String name) { + } + + record RecordWithIgnoreJsonProperty(int id, @JsonIgnore @JsonProperty("name") String name) { + } + + record RecordWithIgnoreAccessor(int id, String name) { + + @JsonIgnore + @Override + public String name() { + return name; + } + } + + record RecordWithIgnorePrimitiveType(@JsonIgnore int id, String name) { + } + + private final ObjectMapper MAPPER = newJsonMapper(); + + /* + /********************************************************************** + /* Test methods, JsonIgnore + /********************************************************************** + */ + + public void testSerializeJsonIgnoreRecord() throws Exception { + String json = MAPPER.writeValueAsString(new RecordWithIgnore(123, "Bob")); + assertEquals("{\"id\":123}", json); + } + + public void testDeserializeJsonIgnoreRecord() throws Exception { + RecordWithIgnore value = MAPPER.readValue("{\"id\":123,\"name\":\"Bob\"}", RecordWithIgnore.class); + assertEquals(new RecordWithIgnore(123, null), value); + } + + /* + /********************************************************************** + /* Test methods, JsonIgnore + JsonProperty + /********************************************************************** + */ + + public void testSerializeJsonIgnoreAndJsonPropertyRecord() throws Exception { + String json = MAPPER.writeValueAsString(new RecordWithIgnoreJsonProperty(123, "Bob")); + assertEquals("{\"id\":123}", json); + } + + public void testDeserializeJsonIgnoreAndJsonPropertyRecord() throws Exception { + RecordWithIgnoreJsonProperty value = MAPPER.readValue("{\"id\":123,\"name\":\"Bob\"}", RecordWithIgnoreJsonProperty.class); + assertEquals(new RecordWithIgnoreJsonProperty(123, "Bob"), value); + } + + /* + /********************************************************************** + /* Test methods, JsonIgnore accessor + /********************************************************************** + */ + + public void testSerializeJsonIgnoreAccessorRecord() throws Exception { + String json = MAPPER.writeValueAsString(new RecordWithIgnoreAccessor(123, "Bob")); + assertEquals("{\"id\":123}", json); + } + + public void testDeserializeJsonIgnoreAccessorRecord() throws Exception { + RecordWithIgnoreAccessor value = MAPPER.readValue("{\"id\":123,\"name\":\"Bob\"}", RecordWithIgnoreAccessor.class); + assertEquals(new RecordWithIgnoreAccessor(123, null), value); + } + + /* + /********************************************************************** + /* Test methods, JsonIgnore parameter of primitive type + /********************************************************************** + */ + + public void testSerializeJsonIgnorePrimitiveTypeRecord() throws Exception { + String json = MAPPER.writeValueAsString(new RecordWithIgnorePrimitiveType(123, "Bob")); + assertEquals("{\"name\":\"Bob\"}", json); + } + + public void testDeserializeJsonIgnorePrimitiveTypeRecord() throws Exception { + RecordWithIgnorePrimitiveType value = MAPPER.readValue("{\"id\":123,\"name\":\"Bob\"}", RecordWithIgnorePrimitiveType.class); + assertEquals(new RecordWithIgnorePrimitiveType(0, "Bob"), value); + } +}