Change Records deserialization to use the same mechanism as POJO deserialization. (#3724)

Author: yihtserns

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<String> 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<String> 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;

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) {

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<String, POJOPropertyBuilder> props)
     protected void _addCreators(Map<String, POJOPropertyBuilder> props)
     {
         // can be null if annotation processing is disabled...
-        if (!_useAnnotations) {
-            return;
-        }
-        for (AnnotatedConstructor ctor : _classDef.getConstructors()) {
-            if (_creatorProperties == null) {
-                _creatorProperties = new LinkedList<POJOPropertyBuilder>();
+        if (_useAnnotations) {
+            for (AnnotatedConstructor ctor : _classDef.getConstructors()) {
+                if (_creatorProperties == null) {
+                    _creatorProperties = new LinkedList<POJOPropertyBuilder>();
+                }
+                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<POJOPropertyBuilder>();
+                }
+                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<POJOPropertyBuilder>();
-            }
-            for (int i = 0, len = factory.getParameterCount(); i < len; ++i) {
-                _addCreatorParam(props, factory.getParameter(i));
+        if (_classDef.getType().isRecordType()) {
+            List<String> recordComponentNames = new ArrayList<String>();
+            AnnotatedConstructor canonicalCtor = JDK14Util.findRecordConstructor(
+                    _classDef, _annotationIntrospector, _config, recordComponentNames);
+
+            if (canonicalCtor != null) {
+                if (_creatorProperties == null) {
+                    _creatorProperties = new LinkedList<POJOPropertyBuilder>();
+                }
+
+                Set<AnnotatedParameter> registeredParams = new HashSet<AnnotatedParameter>();
+                for (POJOPropertyBuilder creatorProperty : _creatorProperties) {
+                    Iterator<AnnotatedParameter> 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<String, POJOPropertyBuilder> props)
     protected void _addCreatorParam(Map<String, POJOPropertyBuilder> 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<String, POJOPropertyBuilder> 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<String, POJOPropertyBuilder> 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);
@@ -902,6 +948,17 @@ protected void _removeUnwantedProperties(Map<String, POJOPropertyBuilder> 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

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<String> 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<String> 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<AnnotatedConstructor> _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;
             }

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;
+    }
+}