Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jan 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
yihtserns marked this conversation as resolved.
Show resolved Hide resolved
protected void _addRecordConstructor(DeserializationContext ctxt, CreatorCollectionState ccState,
AnnotatedConstructor canonical, List<String> implicitNames)
throws JsonMappingException
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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) {
yihtserns marked this conversation as resolved.
Show resolved Hide resolved
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));
}
}
}
}
}
}
Expand All @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -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();
yihtserns marked this conversation as resolved.
Show resolved Hide resolved
// ...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
Expand Down
32 changes: 19 additions & 13 deletions src/main/java/com/fasterxml/jackson/databind/jdk14/JDK14Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -150,25 +156,25 @@ public RawTypeName(Class<?> rt, String n) {
}

static class CreatorLocator {
protected final BeanDescription _beanDesc;
yihtserns marked this conversation as resolved.
Show resolved Hide resolved
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;
Expand All @@ -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) {
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Loading