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 using @JsonPropertyOrder with any-property (@JsonAnyGetter) #4396

Open
wants to merge 19 commits into
base: 2.18
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -550,8 +550,8 @@ protected void _addFields(Map<String, POJOPropertyBuilder> props)
_anySetterField = new LinkedList<>();
}
_anySetterField.add(f);
continue;
}
continue;
}
String implName = ai.findImplicitPropertyName(f);
if (implName == null) {
Expand Down Expand Up @@ -1049,7 +1049,6 @@ protected void _addGetterMethod(Map<String, POJOPropertyBuilder> props,
_anyGetters = new LinkedList<>();
}
_anyGetters.add(m);
return;
}
// @JsonKey?
if (Boolean.TRUE.equals(ai.hasAsKey(_config, m))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* for serializing {@link com.fasterxml.jackson.annotation.JsonAnyGetter} annotated
* (Map) properties
*/
public class AnyGetterWriter
public class AnyGetterWriter extends BeanPropertyWriter
{
protected final BeanProperty _property;

Expand All @@ -25,10 +25,14 @@ public class AnyGetterWriter

protected MapSerializer _mapSerializer;

/**
* @since 2.19
*/
@SuppressWarnings("unchecked")
public AnyGetterWriter(BeanProperty property,
public AnyGetterWriter(BeanPropertyWriter parent, BeanProperty property,
AnnotatedMember accessor, JsonSerializer<?> serializer)
{
super(parent);
_accessor = accessor;
_property = property;
_serializer = (JsonSerializer<Object>) serializer;
Expand All @@ -37,6 +41,16 @@ public AnyGetterWriter(BeanProperty property,
}
}

/**
* @deprecated Since 2.19, use one that takes {@link BeanPropertyWriter} instead.
*/
@SuppressWarnings("unchecked")
public AnyGetterWriter(BeanProperty property,
AnnotatedMember accessor, JsonSerializer<?> serializer)
{
this(null, property, accessor, serializer);
}

/**
* @since 2.8.3
*/
Expand Down Expand Up @@ -65,6 +79,11 @@ public void getAndSerialize(Object bean, JsonGenerator gen, SerializerProvider p
_serializer.serialize(value, gen, provider);
}

@Override
public void serializeAsField(Object bean, JsonGenerator gen, SerializerProvider prov) throws Exception {
getAndSerialize(bean, gen, prov);
}

/**
* @since 2.3
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ protected BeanSerializerBase asArraySerializer()
* - have per-property filters
*/
if ((_objectIdWriter == null)
&& (_anyGetterWriter == null)
&& (_propertyFilterId == null)
) {
return new BeanAsArraySerializer(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@
import com.fasterxml.jackson.databind.ser.std.StdDelegatingSerializer;
import com.fasterxml.jackson.databind.ser.std.ToEmptyObjectSerializer;
import com.fasterxml.jackson.databind.type.ReferenceType;
import com.fasterxml.jackson.databind.util.BeanUtil;
import com.fasterxml.jackson.databind.util.ClassUtil;
import com.fasterxml.jackson.databind.util.Converter;
import com.fasterxml.jackson.databind.util.IgnorePropertiesUtil;
import com.fasterxml.jackson.databind.util.NativeImageUtil;
import com.fasterxml.jackson.databind.util.*;

/**
* Factory class that can provide serializers for any regular Java beans
Expand Down Expand Up @@ -457,7 +453,33 @@ protected JsonSerializer<Object> constructBeanOrAddOnSerializer(SerializerProvid
PropertyName name = PropertyName.construct(anyGetter.getName());
BeanProperty.Std anyProp = new BeanProperty.Std(name, valueType, null,
anyGetter, PropertyMetadata.STD_OPTIONAL);
builder.setAnyGetter(new AnyGetterWriter(anyProp, anyGetter, anySer));

JooHyukKim marked this conversation as resolved.
Show resolved Hide resolved
// Check if there is a accessor exposed for the anyGetter
BeanPropertyWriter anyGetterProp = null;
int anyGetterIndex = -1;
for (int i = 0; i < props.size(); i++) {
BeanPropertyWriter prop = props.get(i);
// Either any-getter as field...
if (Objects.equals(prop.getName(), anyGetter.getName())
// or as method
|| Objects.equals(prop.getMember().getMember(), anyGetter.getMember()))
{
anyGetterProp = prop;
anyGetterIndex = i;
break;
}
}
if (anyGetterIndex != -1) {
// There is prop is already in place, just need to replace it
AnyGetterWriter anyGetterWriter = new AnyGetterWriter(anyGetterProp, anyProp, anyGetter, anySer);
props.set(anyGetterIndex, anyGetterWriter);
} else {
// Otherwise just add it at the end, but won't be sorted...
// This is case where JsonAnyGetter is private/protected,
BeanPropertyDefinition anyGetterPropDef = SimpleBeanPropertyDefinition.construct(config, anyGetter, name);
BeanPropertyWriter anyPropWriter = _constructWriter(prov, anyGetterPropDef, new PropertyBuilder(config, beanDesc), staticTyping, anyGetter);
props.add(new AnyGetterWriter(anyPropWriter, anyProp, anyGetter, anySer));
}
}
// Next: need to gather view information, if any:
processViews(config, builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,10 +215,6 @@ protected final void serializeAsArray(Object bean, JsonGenerator gen, Serializer
prop.serializeAsElement(bean, gen, provider);
}
}
// NOTE: any getters cannot be supported either
//if (_anyGetterWriter != null) {
// _anyGetterWriter.getAndSerialize(bean, gen, provider);
//}
} catch (Exception e) {
wrapAndThrow(provider, e, bean, props[i].getName());
} catch (StackOverflowError e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ public void serializeAsField(Object pojo, JsonGenerator jgen,
writer.serializeAsField(pojo, jgen, provider);
} else if (!jgen.canOmitFields()) { // since 2.3
writer.serializeAsOmittedField(pojo, jgen, provider);
} else if (writer instanceof AnyGetterWriter) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WIP : Contemplating on how should we optimize this check...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried referring to other writer/serializers and it turns out instanceof here check seems sort of reasonable.

((AnyGetterWriter) writer).getAndFilter(pojo, jgen, provider, this);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,6 @@ public abstract class BeanSerializerBase
*/
final protected BeanPropertyWriter[] _filteredProps;

/**
* Handler for {@link com.fasterxml.jackson.annotation.JsonAnyGetter}
* annotated properties
*/
final protected AnyGetterWriter _anyGetterWriter;

/**
* Id of the bean property filter to use, if any; null if none.
*/
Expand Down Expand Up @@ -119,13 +113,11 @@ protected BeanSerializerBase(JavaType type, BeanSerializerBuilder builder,
// 20-Sep-2019, tatu: Actually not just that but also "dummy" serializer for
// case of no bean properties, too
_typeId = null;
_anyGetterWriter = null;
_propertyFilterId = null;
_objectIdWriter = null;
_serializationShape = null;
} else {
_typeId = builder.getTypeId();
_anyGetterWriter = builder.getAnyGetter();
_propertyFilterId = builder.getFilterId();
_objectIdWriter = builder.getObjectIdWriter();
final JsonFormat.Value format = builder.getBeanDescription().findExpectedFormat();
Expand All @@ -142,7 +134,6 @@ protected BeanSerializerBase(BeanSerializerBase src,
_filteredProps = filteredProperties;

_typeId = src._typeId;
_anyGetterWriter = src._anyGetterWriter;
_objectIdWriter = src._objectIdWriter;
_propertyFilterId = src._propertyFilterId;
_serializationShape = src._serializationShape;
Expand All @@ -166,7 +157,6 @@ protected BeanSerializerBase(BeanSerializerBase src,
_filteredProps = src._filteredProps;

_typeId = src._typeId;
_anyGetterWriter = src._anyGetterWriter;
_objectIdWriter = objectIdWriter;
_propertyFilterId = filterId;
_serializationShape = src._serializationShape;
Expand Down Expand Up @@ -210,7 +200,6 @@ protected BeanSerializerBase(BeanSerializerBase src, Set<String> toIgnore, Set<S
_filteredProps = (fpropsOut == null) ? null : fpropsOut.toArray(new BeanPropertyWriter[fpropsOut.size()]);

_typeId = src._typeId;
_anyGetterWriter = src._anyGetterWriter;
_objectIdWriter = src._objectIdWriter;
_propertyFilterId = src._propertyFilterId;
_serializationShape = src._serializationShape;
Expand Down Expand Up @@ -402,9 +391,11 @@ public void resolve(SerializerProvider provider)
}

// also, any-getter may need to be resolved
if (_anyGetterWriter != null) {
// 23-Feb-2015, tatu: Misleading, as this actually triggers call to contextualization...
_anyGetterWriter.resolve(provider);
for (int i = 0; i < _props.length; i++) {
BeanPropertyWriter prop = _props[i];
if (prop instanceof AnyGetterWriter) {
((AnyGetterWriter) prop).resolve(provider);
cowtowncoder marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

Expand Down Expand Up @@ -770,9 +761,6 @@ protected void serializeFields(Object bean, JsonGenerator gen, SerializerProvide
prop.serializeAsField(bean, gen, provider);
}
}
if (_anyGetterWriter != null) {
_anyGetterWriter.getAndSerialize(bean, gen, provider);
}
} catch (Exception e) {
String name = (i == props.length) ? "[anySetter]" : props[i].getName();
wrapAndThrow(provider, e, bean, name);
Expand Down Expand Up @@ -821,9 +809,6 @@ protected void serializeFieldsFiltered(Object bean, JsonGenerator gen,
filter.serializeAsField(bean, gen, provider, prop);
}
}
if (_anyGetterWriter != null) {
_anyGetterWriter.getAndFilter(bean, gen, provider, filter);
}
} catch (Exception e) {
String name = (i == props.length) ? "[anySetter]" : props[i].getName();
wrapAndThrow(provider, e, bean, name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,24 @@ public void set(String name, String value) {
}
}

// [databind#1458]: Allow `@JsonAnyGetter` on fields too
static class DynaFieldOrderedBean {
public int id = 123;

@JsonPropertyOrder(alphabetic = true)
@JsonAnyGetter
@JsonAnySetter
private HashMap<String,String> other = new LinkedHashMap<>();

public Map<String,String> any() {
return other;
}

public void set(String name, String value) {
other.put(name, value);
}
}

// [databind#1458]: Allow `@JsonAnyGetter` on fields too
@Test
public void testDynaFieldBean() throws Exception
Expand All @@ -199,6 +217,18 @@ public void testDynaFieldBean() throws Exception
assertEquals("Joe", result.other.get("name"));
}

// [databind#4388]: Allow `@JsonPropertyOrder` AND `@JsonAnyGetter` on fields too
@Test
public void testDynaFieldOrderedBean() throws Exception
{
DynaFieldOrderedBean b = new DynaFieldOrderedBean();
b.set("nameC", "Cilly");
b.set("nameB", "Billy");
b.set("nameA", "Ailly");

assertEquals("{\"id\":123,\"nameA\":\"Ailly\",\"nameB\":\"Billy\",\"nameC\":\"Cilly\"}", MAPPER.writeValueAsString(b));
}

/*
/**********************************************************
/* Test methods
Expand Down Expand Up @@ -299,8 +329,7 @@ public void testAnyGetterWithMapperDefaultIncludeNonEmptyAndFilterOnBean() throw
input.add("empty", "");
input.add("null", null);
String json = mapper.writeValueAsString(input);
// Unfortunately path for bean with filter is different. It still skips nulls.
assertEquals("{\"non-empty\":\"property\",\"empty\":\"\"}", json);
assertEquals("{\"non-empty\":\"property\"}", json);
}

// [databind#2592]
Expand Down