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

Allow @JsonAnySetter on Creators #4558

Merged
merged 28 commits into from
Jun 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
10e60b9
Implement all
JooHyukKim Jun 1, 2024
b9b43f3
Order methods in PropertyBasedCreator
JooHyukKim Jun 1, 2024
2ec9140
Clean up anySetter discovery during building beanDeserialier.addBeanP…
JooHyukKim Jun 1, 2024
ee9afc0
Implement separate constructCreatorPropertyAnySetter()
JooHyukKim Jun 1, 2024
adb5d1e
refactor: fix indents
JooHyukKim Jun 1, 2024
fb36d8e
feature: Add index field to SettableAnyProperty so PropertyValueBuffe…
JooHyukKim Jun 1, 2024
40aaf88
test : Add record tests
JooHyukKim Jun 1, 2024
c66ca73
Add support for ObjectNode
JooHyukKim Jun 1, 2024
326e491
Clean up rest of the code
JooHyukKim Jun 1, 2024
c076683
Merge branch '2.18' into 562-version2
JooHyukKim Jun 1, 2024
bf91015
Refactor : simplify implementations
JooHyukKim Jun 1, 2024
7d40a43
Another clean up
JooHyukKim Jun 1, 2024
b96725c
Update SettableAnyProperty.java
JooHyukKim Jun 1, 2024
0792312
Merge branch '2.18' into 562-version2
cowtowncoder Jun 3, 2024
346ff29
Merge branch '2.18' into 562-version2
cowtowncoder Jun 4, 2024
a0da86f
Merge branch '2.18' into 562-version2
cowtowncoder Jun 8, 2024
0e11a15
Revise comment and throw UnsupportedOperationException in SettableAny…
JooHyukKim Jun 8, 2024
d678cb4
Replace _consumedFlag, by actually using ProeprtyValue class
JooHyukKim Jun 8, 2024
e910875
Minor comment addition
cowtowncoder Jun 8, 2024
93ad870
Minor renaming, reordering, to unify handling (same order of type che…
cowtowncoder Jun 8, 2024
b3a37ef
One more comment add
cowtowncoder Jun 8, 2024
b5ef0d3
minor comment improvement
cowtowncoder Jun 8, 2024
6cefc8e
Update tests slightly: no 1 failure as behavior not quite correct (wr…
cowtowncoder Jun 8, 2024
e05336d
Extend tests, fix an issue with combined buffering of non-creator vs …
cowtowncoder Jun 9, 2024
b482bd8
Update release notes
cowtowncoder Jun 9, 2024
22952bb
Fix the failing test
cowtowncoder Jun 9, 2024
01c0cc5
Minor error reporting improvement
cowtowncoder Jun 9, 2024
34c0411
Test improvement
cowtowncoder Jun 9, 2024
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
2 changes: 2 additions & 0 deletions release-notes/CREDITS-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ Chris Cleveland:
Benson Margulies:
* Reported #467: Unwanted POJO's embedded in tree via serialization to tree
(2.4.0)
* Reported #562: Allow `@JsonAnySetter` to flow through Creators
(2.18.0)
* Reported #601: ClassCastException for a custom serializer for enum key in `EnumMap`
(2.4.4)
* Contributed 944: Failure to use custom deserializer for key deserializer
Expand Down
3 changes: 3 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ Project: jackson-databind

2.18.0 (not yet released)

#562: Allow `@JsonAnySetter` to flow through Creators
(reported by Benson M)
(fix by Joo-Hyuk K)
#806: Problem with `NamingStrategy`, creator methods with implicit names
#2977: Incompatible `FAIL_ON_MISSING_PRIMITIVE_PROPERTIES` and
field level `@JsonProperty`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -508,12 +508,22 @@ private void _addSelectedPropertiesBasedCreator(DeserializationContext ctxt,
{
final int paramCount = candidate.paramCount();
SettableBeanProperty[] properties = new SettableBeanProperty[paramCount];
int anySetterIx = -1;

for (int i = 0; i < paramCount; ++i) {
JacksonInject.Value injectId = candidate.injection(i);
AnnotatedParameter param = candidate.parameter(i);
PropertyName name = candidate.paramName(i);
if (name == null) {
boolean isAnySetter = Boolean.TRUE.equals(ctxt.getAnnotationIntrospector().hasAnySetter(param));
if (isAnySetter) {
if (anySetterIx >= 0) {
ctxt.reportBadTypeDefinition(beanDesc,
"More than one 'any-setter' specified (parameter #%d vs #%d)",
anySetterIx, i);
} else {
anySetterIx = i;
}
} else if (name == null) {
// 21-Sep-2017, tatu: Looks like we want to block accidental use of Unwrapped,
// as that will not work with Creators well at all
NameTransformer unwrapper = ctxt.getAnnotationIntrospector().findUnwrappingNameTransformer(param);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,9 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri
throws IOException
{
final PropertyBasedCreator creator = _propertyBasedCreator;
PropertyValueBuffer buffer = creator.startBuilding(p, ctxt, _objectIdReader);
PropertyValueBuffer buffer = (_anySetter != null)
? creator.startBuildingWithAnySetter(p, ctxt, _objectIdReader, _anySetter)
: creator.startBuilding(p, ctxt, _objectIdReader);
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
TokenBuffer unknown = null;
final Class<?> activeView = _needViewProcesing ? ctxt.getActiveView() : null;

Expand All @@ -429,15 +431,15 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri
if (buffer.readIdProperty(propName) && creatorProp == null) {
continue;
}
// creator property?
// Creator property?
if (creatorProp != null) {
// Last creator property to set?
Object value;
if ((activeView != null) && !creatorProp.visibleInView(activeView)) {
p.skipChildren();
continue;
}
value = _deserializeWithErrorWrapping(p, ctxt, creatorProp);
// Last creator property to set?
if (buffer.assignParameter(creatorProp, value)) {
p.nextToken(); // to move to following FIELD_NAME/END_OBJECT
Object bean;
Expand Down Expand Up @@ -497,7 +499,7 @@ protected Object _deserializeUsingPropertyBased(final JsonParser p, final Deseri
// "any property"?
if (_anySetter != null) {
try {
buffer.bufferAnyProperty(_anySetter, propName, _anySetter.deserialize(p, ctxt));
buffer.bufferAnyParameterProperty(_anySetter, propName, _anySetter.deserialize(p, ctxt));
} catch (Exception e) {
wrapAndThrow(e, _beanType.getRawClass(), propName, ctxt);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -543,9 +543,9 @@ protected void addBeanProps(DeserializationContext ctxt,
}

// Also, do we have a fallback "any" setter?
AnnotatedMember anySetter = beanDesc.findAnySetterAccessor();
SettableAnyProperty anySetter = _resolveAnySetter(ctxt, beanDesc, creatorProps);
if (anySetter != null) {
builder.setAnySetter(constructAnySetter(ctxt, beanDesc, anySetter));
builder.setAnySetter(anySetter);
} else {
// 23-Jan-2018, tatu: although [databind#1805] would suggest we should block
// properties regardless, for now only consider unless there's any setter...
Expand Down Expand Up @@ -661,6 +661,29 @@ protected void addBeanProps(DeserializationContext ctxt,
}
}

// since 2.18
private SettableAnyProperty _resolveAnySetter(DeserializationContext ctxt,
BeanDescription beanDesc, SettableBeanProperty[] creatorProps)
throws JsonMappingException
{
// Find the regular method/field level any-setter
AnnotatedMember anySetter = beanDesc.findAnySetterAccessor();
if (anySetter != null) {
return constructAnySetter(ctxt, beanDesc, anySetter);
}
// else look for any-setter via @JsonCreator
if (creatorProps != null) {
for (SettableBeanProperty prop : creatorProps) {
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
AnnotatedMember member = prop.getMember();
if (member != null && Boolean.TRUE.equals(ctxt.getAnnotationIntrospector().hasAnySetter(member))) {
return constructAnySetter(ctxt, beanDesc, member);
}
}
}
// not found, that's fine, too
return null;
}

private boolean _isSetterlessType(Class<?> rawType) {
// May also need to consider getters
// for Map/Collection properties; but with lowest precedence
Expand Down Expand Up @@ -795,25 +818,30 @@ protected void addInjectables(DeserializationContext ctxt,
* for handling unknown bean properties, given a method that
* has been designated as such setter.
*
* @param mutator Either 2-argument method (setter, with key and value), or Field
* that contains Map; either way accessor used for passing "any values"
* @param mutator Either a 2-argument method (setter, with key and value),
* or a Field or (as of 2.18) Constructor Parameter of type Map or JsonNode/Object;
* either way accessor used for passing "any values"
*/
@SuppressWarnings("unchecked")
protected SettableAnyProperty constructAnySetter(DeserializationContext ctxt,
BeanDescription beanDesc, AnnotatedMember mutator)
throws JsonMappingException
{
//find the java type based on the annotated setter method or setter field
// find the java type based on the annotated setter method or setter field
BeanProperty prop;
JavaType keyType;
JavaType valueType;
final boolean isField = mutator instanceof AnnotatedField;
// [databind#562] Allow @JsonAnySetter on Creator constructor
final boolean isParameter = mutator instanceof AnnotatedParameter;
int parameterIndex = -1;

if (mutator instanceof AnnotatedMethod) {
// we know it's a 2-arg method, second arg is the value
AnnotatedMethod am = (AnnotatedMethod) mutator;
keyType = am.getParameterType(0);
valueType = am.getParameterType(1);
// Need to resolve for possible generic types (like Maps, Collections)
valueType = resolveMemberAndTypeAnnotations(ctxt, mutator, valueType);
prop = new BeanProperty.Std(PropertyName.construct(mutator.getName()),
valueType, null, mutator,
Expand Down Expand Up @@ -848,11 +876,43 @@ protected SettableAnyProperty constructAnySetter(DeserializationContext ctxt,
"Unsupported type for any-setter: %s -- only support `Map`s, `JsonNode` and `ObjectNode` ",
ClassUtil.getTypeDescription(fieldType)));
}
} else if (isParameter) {
AnnotatedParameter af = (AnnotatedParameter) mutator;
JavaType paramType = af.getType();
parameterIndex = af.getIndex();

if (paramType.isMapLikeType()) {
paramType = resolveMemberAndTypeAnnotations(ctxt, mutator, paramType);
keyType = paramType.getKeyType();
valueType = paramType.getContentType();
prop = new BeanProperty.Std(PropertyName.construct(mutator.getName()),
paramType, null, mutator, PropertyMetadata.STD_OPTIONAL);
} else if (paramType.hasRawClass(JsonNode.class) || paramType.hasRawClass(ObjectNode.class)) {
paramType = resolveMemberAndTypeAnnotations(ctxt, mutator, paramType);
// Deserialize is individual values of ObjectNode, not full ObjectNode, so:
valueType = ctxt.constructType(JsonNode.class);
prop = new BeanProperty.Std(PropertyName.construct(mutator.getName()),
paramType, null, mutator, PropertyMetadata.STD_OPTIONAL);

// Unlike with more complicated types, here we do not allow any annotation
// overrides etc but instead short-cut handling:
return SettableAnyProperty.constructForJsonNodeParameter(ctxt, prop, mutator, valueType,
ctxt.findRootValueDeserializer(valueType), parameterIndex);
} else {
return ctxt.reportBadDefinition(beanDesc.getType(), String.format(
"Unsupported type for any-setter: %s -- only support `Map`s, `JsonNode` and `ObjectNode` ",
ClassUtil.getTypeDescription(paramType)));
}
} else {
return ctxt.reportBadDefinition(beanDesc.getType(), String.format(
"Unrecognized mutator type for any-setter: %s",
ClassUtil.nameOf(mutator.getClass())));
}

// NOTE: code from now on is for `Map` valued Any properties (JsonNode/ObjectNode
// already returned; unsupported types threw Exception), if we have Field/Ctor-Parameter
// any-setter -- or, basically Any supported type (if Method)

// First: see if there are explicitly specified
// and then possible direct deserializer override on accessor
KeyDeserializer keyDeser = findKeyDeserializerFromAnnotation(ctxt, mutator);
Expand Down Expand Up @@ -880,6 +940,10 @@ protected SettableAnyProperty constructAnySetter(DeserializationContext ctxt,
return SettableAnyProperty.constructForMapField(ctxt,
prop, mutator, valueType, keyDeser, deser, typeDeser);
}
if (isParameter) {
return SettableAnyProperty.constructForMapParameter(ctxt,
prop, mutator, valueType, keyDeser, deser, typeDeser, parameterIndex);
}
return SettableAnyProperty.constructForMethod(ctxt,
prop, mutator, valueType, keyDeser, deser, typeDeser);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package com.fasterxml.jackson.databind.deser;

import java.io.IOException;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;

import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.databind.*;
Expand Down Expand Up @@ -117,6 +119,31 @@ public static SettableAnyProperty constructForJsonNodeField(DeserializationConte
ctxt.getNodeFactory());
}

/**
* @since 2.18
*/
public static SettableAnyProperty constructForMapParameter(DeserializationContext ctxt,
BeanProperty property, AnnotatedMember field, JavaType valueType, KeyDeserializer keyDeser,
JsonDeserializer<Object> valueDeser, TypeDeserializer typeDeser, int parameterIndex
) {
Class<?> mapType = field.getRawType();
// 02-Aug-2022, tatu: Ideally would be resolved to a concrete type by caller but
// alas doesn't appear to happen. Nor does `BasicDeserializerFactory` expose method
// for finding default or explicit mappings.
if (mapType == Map.class) {
mapType = LinkedHashMap.class;
}
ValueInstantiator vi = JDKValueInstantiators.findStdValueInstantiator(ctxt.getConfig(), mapType);
return new MapParameterAnyProperty(property, field, valueType, keyDeser, valueDeser, typeDeser, vi, parameterIndex);
}

public static SettableAnyProperty constructForJsonNodeParameter(DeserializationContext ctxt, BeanProperty prop,
AnnotatedMember mutator, JavaType valueType, JsonDeserializer<Object> valueDeser, int parameterIndex
) {
return new JsonNodeParameterAnyProperty(prop, mutator, valueType, valueDeser, ctxt.getNodeFactory(), parameterIndex);
}


// Abstract @since 2.14
public abstract SettableAnyProperty withValueDeserializer(JsonDeserializer<Object> deser);

Expand Down Expand Up @@ -159,6 +186,23 @@ Object readResolve() {
*/
public String getPropertyName() { return _property.getName(); }

/**
* Accessor for parameterIndex.
* @return -1 if not a parameterized setter, otherwise index of parameter
*
* @since 2.18
*/
public int getParameterIndex() { return -1; }

/**
* Create an instance of value to pass through Creator parameter.
*
* @since 2.18
*/
public Object createParameterObject() {
throw new UnsupportedOperationException("Cannot call createParameterObject() on " + getClass().getName());
}

/*
/**********************************************************
/* Public API, deserialization
Expand Down Expand Up @@ -437,4 +481,102 @@ public SettableAnyProperty withValueDeserializer(JsonDeserializer<Object> deser)
return this;
}
}


/**
* [databind#562] Allow @JsonAnySetter on Creator constructor
*
* @since 2.18
*/
protected static class MapParameterAnyProperty extends SettableAnyProperty
implements java.io.Serializable
{
private static final long serialVersionUID = 1L;

protected final ValueInstantiator _valueInstantiator;

protected final int _parameterIndex;

public MapParameterAnyProperty(BeanProperty property, AnnotatedMember field, JavaType valueType,
KeyDeserializer keyDeser, JsonDeserializer<Object> valueDeser, TypeDeserializer typeDeser,
ValueInstantiator inst, int parameterIndex)
{
super(property, field, valueType, keyDeser, valueDeser, typeDeser);
_valueInstantiator = Objects.requireNonNull(inst, "ValueInstantiator for MapParameterAnyProperty cannot be `null`");
_parameterIndex = parameterIndex;
}

@Override
public SettableAnyProperty withValueDeserializer(JsonDeserializer<Object> deser)
{
return new MapParameterAnyProperty(_property, _setter, _type, _keyDeserializer, deser,
_valueTypeDeserializer, _valueInstantiator, _parameterIndex);
}

@SuppressWarnings("unchecked")
@Override
protected void _set(Object instance, Object propName, Object value)
{
((Map<Object, Object>) instance).put(propName, value);
}

@Override
public int getParameterIndex() { return _parameterIndex; }

@Override
public Object createParameterObject() { return new HashMap<>(); }

}

/**
* [databind#562] Allow @JsonAnySetter on Creator constructor
*
* @since 2.18
*/
protected static class JsonNodeParameterAnyProperty extends SettableAnyProperty
implements java.io.Serializable
{
private static final long serialVersionUID = 1L;

protected final JsonNodeFactory _nodeFactory;

protected final int _parameterIndex;

public JsonNodeParameterAnyProperty(BeanProperty property, AnnotatedMember field, JavaType valueType,
JsonDeserializer<Object> valueDeser, JsonNodeFactory nodeFactory, int parameterIndex)
{
super(property, field, valueType, null, valueDeser, null);
_nodeFactory = nodeFactory;
_parameterIndex = parameterIndex;
}

// Let's override since this is much simpler with JsonNodes
@Override
public Object deserialize(JsonParser p, DeserializationContext ctxt)
throws IOException
{
return _valueDeserializer.deserialize(p, ctxt);
}

@Override
protected void _set(Object instance, Object propName, Object value)
throws Exception
{
((ObjectNode) instance).set((String) propName, (JsonNode) value);
}

// Should not get called but...
@Override
public SettableAnyProperty withValueDeserializer(JsonDeserializer<Object> deser) {
throw new UnsupportedOperationException("Cannot call withValueDeserializer() on " + getClass().getName());
}

@Override
public int getParameterIndex() { return _parameterIndex; }

@Override
public Object createParameterObject() { return _nodeFactory.objectNode(); }

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ public void addPropertyCreator(AnnotatedWithParams creator,
String name = properties[i].getName();
// Need to consider Injectables, which may not have
// a name at all, and need to be skipped
// (same for possible AnySetter)
if (name.isEmpty() && (properties[i].getInjectableValueId() != null)) {
continue;
}
Expand Down
Loading