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 Creator constructors #4366

Closed
wants to merge 72 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
94540f8
temporarily remove other jdk versions
JooHyukKim Jan 27, 2024
9c6cace
Merge branch '2.17' into 562-jsonanysetter-test
JooHyukKim Jan 28, 2024
1faccf0
Working solution first draft
JooHyukKim Feb 3, 2024
8930a4e
Revert "temporarily remove other jdk versions"
JooHyukKim Feb 3, 2024
5280004
Delete JsonAnySetterThroughCreator562Test.java
JooHyukKim Feb 3, 2024
99a882b
Remove hard coded field names
JooHyukKim Feb 3, 2024
a840e56
Clean Up
JooHyukKim Feb 3, 2024
75c23f1
Clean up unused _anySetter
JooHyukKim Feb 3, 2024
2b7d22f
Add back required property _anySetter implementation
JooHyukKim Feb 3, 2024
2f49175
Merge branch '2.17' into 562-jsonanysetter-test
JooHyukKim Feb 3, 2024
1a9afab
Add `Boolean _ hasAnySetter` in
JooHyukKim Feb 4, 2024
2b2e0aa
Use hasAnySetter from CreatorProperty
JooHyukKim Feb 4, 2024
6a892f8
Remove unncessary anySetter construction
JooHyukKim Feb 4, 2024
765ed3a
Revert "Remove unncessary anySetter construction"
JooHyukKim Feb 4, 2024
57d467a
Put back isAnySetterXxx methods and fields
JooHyukKim Feb 4, 2024
2ae2dca
Fix JDK 8 error
JooHyukKim Feb 4, 2024
c4657cc
Merge branch '2.17' into 562-jsonanysetter-test
JooHyukKim Feb 9, 2024
7da980a
Move test dir
JooHyukKim Feb 9, 2024
09d4c82
Fix error
JooHyukKim Feb 9, 2024
9baad77
Add new SettableAnyProperty impl
JooHyukKim Feb 9, 2024
81a8803
Remove hard coded prop name
JooHyukKim Feb 9, 2024
742a589
Update javadoc and clean comments
JooHyukKim Feb 9, 2024
19182fa
Merge branch '2.17' into 562-jsonanysetter-test
cowtowncoder Feb 10, 2024
010849a
Minor clean up, streamlining
cowtowncoder Feb 10, 2024
10540a8
Add verification there's only one any-setter
cowtowncoder Feb 10, 2024
1458882
Test case for disabled(@JsonAnySetter) (rain check?
JooHyukKim Feb 10, 2024
bddb042
Require non-null _valueInstantiator during construction
JooHyukKim Feb 10, 2024
5c1f30d
Add test with DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES
JooHyukKim Feb 10, 2024
bdc2357
Fix tests
JooHyukKim Feb 10, 2024
47ac3f7
Update AnySetterForCreator562Test.java
JooHyukKim Feb 10, 2024
8612433
Implement version 2
JooHyukKim Feb 10, 2024
5d69465
Clean up changes
JooHyukKim Feb 10, 2024
551a001
Merge branch '2.17' into 562-jsonanysetter-test
JooHyukKim Feb 12, 2024
7d7a4c5
Merge branch '2.17' into 562-jsonanysetter-test
cowtowncoder Feb 19, 2024
778ddc3
Merge branch '2.17' into 562-jsonanysetter-test
JooHyukKim Apr 8, 2024
30c0f57
Merge branch '2.17' into 562-jsonanysetter-test
cowtowncoder May 31, 2024
7a55942
temporarily remove other jdk versions
JooHyukKim Jan 27, 2024
ff22500
Working solution first draft
JooHyukKim Feb 3, 2024
e2670e6
Revert "temporarily remove other jdk versions"
JooHyukKim Feb 3, 2024
05a4c0b
Delete JsonAnySetterThroughCreator562Test.java
JooHyukKim Feb 3, 2024
dd53092
Remove hard coded field names
JooHyukKim Feb 3, 2024
49f8b95
Clean Up
JooHyukKim Feb 3, 2024
b4d03e3
Clean up unused _anySetter
JooHyukKim Feb 3, 2024
426f303
Add back required property _anySetter implementation
JooHyukKim Feb 3, 2024
201c5ea
Add `Boolean _ hasAnySetter` in
JooHyukKim Feb 4, 2024
a247d02
Use hasAnySetter from CreatorProperty
JooHyukKim Feb 4, 2024
2ece9f9
Remove unncessary anySetter construction
JooHyukKim Feb 4, 2024
d3d8546
Revert "Remove unncessary anySetter construction"
JooHyukKim Feb 4, 2024
3d632f4
Put back isAnySetterXxx methods and fields
JooHyukKim Feb 4, 2024
ce58b14
Fix JDK 8 error
JooHyukKim Feb 4, 2024
21a0795
Move test dir
JooHyukKim Feb 9, 2024
09296d1
Fix error
JooHyukKim Feb 9, 2024
3445ddd
Add new SettableAnyProperty impl
JooHyukKim Feb 9, 2024
9e95155
Remove hard coded prop name
JooHyukKim Feb 9, 2024
1908532
Update javadoc and clean comments
JooHyukKim Feb 9, 2024
659bbf2
Minor clean up, streamlining
cowtowncoder Feb 10, 2024
fcfac63
Add verification there's only one any-setter
cowtowncoder Feb 10, 2024
f6a6c37
Test case for disabled(@JsonAnySetter) (rain check?
JooHyukKim Feb 10, 2024
93b8587
Require non-null _valueInstantiator during construction
JooHyukKim Feb 10, 2024
134d8d1
Add test with DeserializationFeature.FAIL_ON_NULL_CREATOR_PROPERTIES
JooHyukKim Feb 10, 2024
7c60905
Fix tests
JooHyukKim Feb 10, 2024
3e62a2b
Update AnySetterForCreator562Test.java
JooHyukKim Feb 10, 2024
60a2933
Implement version 2
JooHyukKim Feb 10, 2024
40061f9
Clean up changes
JooHyukKim Feb 10, 2024
cd86a74
Add missing imports
JooHyukKim May 31, 2024
6cbdeff
Merge branch '562-jsonanysetter-test' of https://github.com/JooHyukKi…
JooHyukKim May 31, 2024
8046794
Merge branch '2.18' into 562-jsonanysetter-test
cowtowncoder May 31, 2024
2de9beb
Clean up code, version, etc...
JooHyukKim May 31, 2024
825f31e
Merge branch '2.18' into 562-jsonanysetter-test
cowtowncoder May 31, 2024
cdcdf3e
Merge branch '2.18' into 562-jsonanysetter-test
cowtowncoder May 31, 2024
2cd2b9d
Apply review
JooHyukKim Jun 1, 2024
b616cb6
Change initMap() method to createAnyPropertyMap()
JooHyukKim Jun 1, 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
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ private void _addImplicitDelegatingConstructors(DeserializationContext ctxt,

if (injectable != null) {
++injectCount;
properties[i] = constructCreatorProperty(ctxt, beanDesc, null, i, param, injectable);
properties[i] = constructCreatorProperty(ctxt, beanDesc, null, i, param, injectable, false);
continue;
}
NameTransformer unwrapper = intr.findUnwrappingNameTransformer(param);
Expand Down Expand Up @@ -472,7 +472,7 @@ private boolean _addExplicitDelegatingCreator(DeserializationContext ctxt,
AnnotatedParameter param = candidate.parameter(i);
JacksonInject.Value injectId = candidate.injection(i);
if (injectId != null) {
properties[i] = constructCreatorProperty(ctxt, beanDesc, null, i, param, injectId);
properties[i] = constructCreatorProperty(ctxt, beanDesc, null, i, param, injectId, false);
continue;
}
if (ix < 0) {
Expand Down Expand Up @@ -508,6 +508,7 @@ private void _addSelectedPropertiesBasedCreator(DeserializationContext ctxt,
{
final int paramCount = candidate.paramCount();
SettableBeanProperty[] properties = new SettableBeanProperty[paramCount];
int anySetterIndex = -1;

for (int i = 0; i < paramCount; ++i) {
JacksonInject.Value injectId = candidate.injection(i);
Expand All @@ -520,14 +521,25 @@ private void _addSelectedPropertiesBasedCreator(DeserializationContext ctxt,
if (unwrapper != null) {
_reportUnwrappedCreatorProperty(ctxt, beanDesc, param);
}
// Must be injectable or have name; without either won't work
if ((name == null) && (injectId == null)) {
ctxt.reportBadTypeDefinition(beanDesc,
// [databind#562] Allow @JsonAnySetter in creators
if (Boolean.TRUE.equals(ctxt.getAnnotationIntrospector().hasAnySetter(param))) {
if (anySetterIndex >= 0) {
ctxt.reportBadTypeDefinition(beanDesc,
"More than one 'any-setter' specified (parameters #%d vs #%d)",
anySetterIndex, i);
}
anySetterIndex = i;
} else {
// Must be injectable or have name; without either won't work
if ((name == null) && (injectId == null)) {
ctxt.reportBadTypeDefinition(beanDesc,
"Argument #%d of Creator %s has no property name (and is not Injectable): can not use as property-based Creator",
i, candidate);
}
}
}
properties[i] = constructCreatorProperty(ctxt, beanDesc, name, i, param, injectId);
properties[i] = constructCreatorProperty(ctxt, beanDesc, name, i, param, injectId,
anySetterIndex == i);
}
creators.addPropertyCreator(candidate.creator(), true, properties);
}
Expand Down Expand Up @@ -600,11 +612,14 @@ private void _reportUnwrappedCreatorProperty(DeserializationContext ctxt,
* Method that will construct a property object that represents
* a logical property passed via Creator (constructor or static
* factory method)
*
* @since 2.18
*/
protected SettableBeanProperty constructCreatorProperty(DeserializationContext ctxt,
BeanDescription beanDesc, PropertyName name, int index,
AnnotatedParameter param,
JacksonInject.Value injectable)
JacksonInject.Value injectable,
boolean hasAnySetter)
throws JsonMappingException
{
final DeserializationConfig config = ctxt.getConfig();
Expand Down Expand Up @@ -642,7 +657,7 @@ protected SettableBeanProperty constructCreatorProperty(DeserializationContext c
// so it is not called directly here
SettableBeanProperty prop = CreatorProperty.construct(name, type, property.getWrapperName(),
typeDeser, beanDesc.getClassAnnotations(), param, index, injectable,
metadata);
metadata, hasAnySetter);
JsonDeserializer<?> deser = findDeserializerFromAnnotation(ctxt, param);
if (deser == null) {
deser = type.getValueHandler();
Expand All @@ -655,6 +670,20 @@ protected SettableBeanProperty constructCreatorProperty(DeserializationContext c
return prop;
}

/**
* @deprecated since 2.18, use {@link #constructCreatorProperty(DeserializationContext, BeanDescription,
* PropertyName, int, AnnotatedParameter, JacksonInject.Value, boolean)} instead.
*/
@Deprecated
protected SettableBeanProperty constructCreatorProperty(DeserializationContext ctxt,
BeanDescription beanDesc, PropertyName name, int index,
AnnotatedParameter param,
JacksonInject.Value injectable)
throws JsonMappingException
{
return constructCreatorProperty(ctxt, beanDesc, name, index, param, injectable, false);
}

/**
* Helper method copied from {@code POJOPropertyBuilder} since that won't be
* applied to creator parameters
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);
TokenBuffer unknown = null;
final Class<?> activeView = _needViewProcesing ? ctxt.getActiveView() : null;

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.bufferAnySetter(ctxt, p, creator, propName);
} catch (Exception e) {
wrapAndThrow(e, _beanType.getRawClass(), propName, ctxt);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,8 +544,11 @@ protected void addBeanProps(DeserializationContext ctxt,

// Also, do we have a fallback "any" setter?
AnnotatedMember anySetter = beanDesc.findAnySetterAccessor();
AnnotatedMember creatorPropWithAnySetter = _findCreatorPropWithAnySetter(creatorProps);
if (anySetter != null) {
builder.setAnySetter(constructAnySetter(ctxt, beanDesc, anySetter));
} else if (creatorPropWithAnySetter != null) {
builder.setAnySetter(constructAnySetter(ctxt, beanDesc, creatorPropWithAnySetter));
} 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 All @@ -558,6 +561,7 @@ protected void addBeanProps(DeserializationContext ctxt,
}
}
}

final boolean useGettersAsSetters = ctxt.isEnabled(MapperFeature.USE_GETTERS_AS_SETTERS)
&& ctxt.isEnabled(MapperFeature.AUTO_DETECT_GETTERS);

Expand Down Expand Up @@ -661,6 +665,18 @@ protected void addBeanProps(DeserializationContext ctxt,
}
}

// since 2.18
private AnnotatedMember _findCreatorPropWithAnySetter(SettableBeanProperty[] creatorProps) {
if (creatorProps != null) {
for (SettableBeanProperty prop : creatorProps) {
if (((CreatorProperty) prop).isAnySetter()) {
return prop.getMember();
}
}
}
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 @@ -808,6 +824,7 @@ protected SettableAnyProperty constructAnySetter(DeserializationContext ctxt,
JavaType keyType;
JavaType valueType;
final boolean isField = mutator instanceof AnnotatedField;
boolean isMapParam = false;

if (mutator instanceof AnnotatedMethod) {
// we know it's a 2-arg method, second arg is the value
Expand All @@ -819,6 +836,22 @@ protected SettableAnyProperty constructAnySetter(DeserializationContext ctxt,
valueType, null, mutator,
PropertyMetadata.STD_OPTIONAL);

// [databind#562] Allow @JsonAnySetter in creators
} else if (mutator instanceof AnnotatedParameter){
AnnotatedParameter af = (AnnotatedParameter) mutator;
// get the type from the content type of the map object
JavaType fieldType = af.getType();
if (!fieldType.isMapLikeType()) {
return ctxt.reportBadDefinition(beanDesc.getType(), String.format(
"Unsupported type for any-setter: %s -- only support `Map`s, `JsonNode` and `ObjectNode` ",
ClassUtil.getTypeDescription(fieldType)));
}
fieldType = resolveMemberAndTypeAnnotations(ctxt, mutator, fieldType);
keyType = fieldType.getKeyType();
valueType = fieldType.getContentType();
prop = new BeanProperty.Std(PropertyName.NO_NAME,
fieldType, null, mutator, PropertyMetadata.STD_OPTIONAL);
isMapParam = true;
} else if (isField) {
AnnotatedField af = (AnnotatedField) mutator;
// get the type from the content type of the map object
Expand Down Expand Up @@ -880,6 +913,11 @@ protected SettableAnyProperty constructAnySetter(DeserializationContext ctxt,
return SettableAnyProperty.constructForMapField(ctxt,
prop, mutator, valueType, keyDeser, deser, typeDeser);
}
// [databind#562] Allow @JsonAnySetter in creators
if (isMapParam) {
return SettableAnyProperty.constructForMapParameter(ctxt,
prop, mutator, valueType, keyDeser, deser, typeDeser);
}
return SettableAnyProperty.constructForMethod(ctxt,
prop, mutator, valueType, keyDeser, deser, typeDeser);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.io.IOException;
import java.lang.annotation.Annotation;
import java.util.Map;

import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.core.JsonParser;
Expand Down Expand Up @@ -76,19 +77,42 @@ public class CreatorProperty
protected boolean _ignorable;

/**
* @since 2.11
* Marker flag to indicate that current property is used to handle "any setter" via `@JsonAnySetter`.
*
* @since 2.18
*/
protected final boolean _isAnySetter;

/**
* @since 2.18
*/
protected CreatorProperty(PropertyName name, JavaType type, PropertyName wrapperName,
TypeDeserializer typeDeser,
Annotations contextAnnotations, AnnotatedParameter param,
int index, JacksonInject.Value injectable,
PropertyMetadata metadata)
PropertyMetadata metadata, boolean isAnySetter)
{
super(name, type, wrapperName, typeDeser, contextAnnotations, metadata);
_annotated = param;
_creatorIndex = index;
_injectableValue = injectable;
_fallbackSetter = null;
_isAnySetter = isAnySetter; // [databind#562] since 2.18
}

/**
* @since 2.11
* @deprecated since 2.18. use later version instead.
*/
@Deprecated
protected CreatorProperty(PropertyName name, JavaType type, PropertyName wrapperName,
TypeDeserializer typeDeser,
Annotations contextAnnotations, AnnotatedParameter param,
int index, JacksonInject.Value injectable,
PropertyMetadata metadata)
{
this(name, type, wrapperName, typeDeser, contextAnnotations, param, index, injectable,
metadata, false);
}

/**
Expand Down Expand Up @@ -122,17 +146,34 @@ public CreatorProperty(PropertyName name, JavaType type, PropertyName wrapperNam
* method parameter; used for accessing annotations of the property
* @param injectable Information about injectable value, if any
* @param index Index of this property within creator invocation
*
* @param isAnySetter Marker flag to indicate that current property is used to handle "any setter" via `@JsonAnySetter`.
*
* @since 2.18
*/
public static CreatorProperty construct(PropertyName name, JavaType type, PropertyName wrapperName,
TypeDeserializer typeDeser,
Annotations contextAnnotations, AnnotatedParameter param,
int index, JacksonInject.Value injectable,
PropertyMetadata metadata,
boolean isAnySetter)
{
return new CreatorProperty(name, type, wrapperName, typeDeser, contextAnnotations,
param, index, injectable, metadata, isAnySetter);
}

/**
* @since 2.11
* @deprecated since 2.18. use later version instead.
*/
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
@Deprecated
public static CreatorProperty construct(PropertyName name, JavaType type, PropertyName wrapperName,
TypeDeserializer typeDeser,
Annotations contextAnnotations, AnnotatedParameter param,
int index, JacksonInject.Value injectable,
PropertyMetadata metadata)
{
return new CreatorProperty(name, type, wrapperName, typeDeser, contextAnnotations,
param, index, injectable, metadata);
param, index, injectable, metadata, false);
}

/**
Expand All @@ -145,6 +186,7 @@ protected CreatorProperty(CreatorProperty src, PropertyName newName) {
_fallbackSetter = src._fallbackSetter;
_creatorIndex = src._creatorIndex;
_ignorable = src._ignorable;
_isAnySetter = src._isAnySetter; // [databind#562] since 2.18
}

protected CreatorProperty(CreatorProperty src, JsonDeserializer<?> deser,
Expand All @@ -155,6 +197,7 @@ protected CreatorProperty(CreatorProperty src, JsonDeserializer<?> deser,
_fallbackSetter = src._fallbackSetter;
_creatorIndex = src._creatorIndex;
_ignorable = src._ignorable;
_isAnySetter = src._isAnySetter; // [databind#562] since 2.18
}

@Override
Expand Down Expand Up @@ -320,6 +363,13 @@ public boolean isInjectionOnly() {

// public boolean isInjectionOnly() { return false; }

/**
* @since 2.18
*/
public boolean isAnySetter() {
return _isAnySetter;
}

/*
/**********************************************************
/* Overridden methods, other
Expand Down Expand Up @@ -354,4 +404,13 @@ private void _reportMissingSetter(JsonParser p, DeserializationContext ctxt) thr
throw InvalidDefinitionException.from(p, msg, getType());
}
}

/**
* @since 2.18
*/
public Map<Object, Object> createAnyPropertyMap(DeserializationContext context, SettableAnyProperty anySetter)
throws IOException
{
return ((SettableAnyProperty.MapParamAnyProperty) anySetter).createAnyPropertyMap(context);
}
}
Loading