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

Introduce config override for includeAsProperty (#1522) #1564

Closed
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 @@ -279,7 +279,7 @@ public JsonFormat.Value findPropertyFormat(MapperConfig<?> config, Class<?> base
@Override
public JsonInclude.Value findPropertyInclusion(MapperConfig<?> config, Class<?> baseType)
{
JsonInclude.Value v0 = config.getDefaultPropertyInclusion(baseType);
JsonInclude.Value v0 = config.getDefaultPropertyInclusion(baseType, _type.getRawClass());
AnnotationIntrospector intr = config.getAnnotationIntrospector();
if ((intr == null) || (_member == null)) {
return v0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,26 @@ public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType) {
return EMPTY_INCLUDE;
}

@Override
public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
Class<?> propertyType) {
ConfigOverride propertyTypeOverrides = findConfigOverride(propertyType);
if (propertyTypeOverrides != null) {
JsonInclude.Value v0 = propertyTypeOverrides.getIncludeAsProperty();
if (v0 != null) {
return v0;
}
}
ConfigOverride baseTypeOverrides = findConfigOverride(baseType);
if (baseTypeOverrides != null) {
JsonInclude.Value v1 = baseTypeOverrides.getInclude();
if (v1 != null) {
return v1;
}
}
return EMPTY_INCLUDE;
}

@Override
public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
JsonInclude.Value defaultIncl)
Expand All @@ -810,6 +830,26 @@ public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
return defaultIncl;
}

@Override
public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
Class<?> propertyType, JsonInclude.Value defaultIncl) {
ConfigOverride propertyTypeOverrides = findConfigOverride(propertyType);
if (propertyTypeOverrides != null) {
JsonInclude.Value v0 = propertyTypeOverrides.getIncludeAsProperty();
if (v0 != null) {
return v0;
}
}
ConfigOverride baseTypeOverrides = findConfigOverride(baseType);
if (baseTypeOverrides != null) {
JsonInclude.Value v1 = baseTypeOverrides.getInclude();
if (v1 != null) {
return v1;
}
}
return defaultIncl;
}

/*
/**********************************************************
/* MapperConfig implementation/overrides: other
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,26 @@ public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType) {
return _serializationInclusion;
}

@Override
public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
Class<?> propertyType) {
ConfigOverride propertyTypeOverrides = findConfigOverride(propertyType);
if (propertyTypeOverrides != null) {
JsonInclude.Value v0 = propertyTypeOverrides.getIncludeAsProperty();
if (v0 != null) {
return v0;
}
}
ConfigOverride baseTypeOverrides = findConfigOverride(baseType);
if (baseTypeOverrides != null) {
JsonInclude.Value v1 = baseTypeOverrides.getInclude();
if (v1 != null) {
return v1;
}
}
return _serializationInclusion;
}

@Override
public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
JsonInclude.Value defaultIncl)
Expand All @@ -913,6 +933,26 @@ public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
return defaultIncl;
}

@Override
public JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
Class<?> propertyType, JsonInclude.Value defaultIncl) {
ConfigOverride propertyTypeOverrides = findConfigOverride(propertyType);
if (propertyTypeOverrides != null) {
JsonInclude.Value v0 = propertyTypeOverrides.getIncludeAsProperty();
if (v0 != null) {
return v0;
}
}
ConfigOverride baseTypeOverrides = findConfigOverride(baseType);
if (baseTypeOverrides != null) {
JsonInclude.Value v1 = baseTypeOverrides.getInclude();
if (v1 != null) {
return v1;
}
}
return defaultIncl;
}

/*
/**********************************************************
/* Configuration: other
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ public abstract class ConfigOverride
*/
protected JsonInclude.Value _include;

/**
* Definitions of inclusion overrides for properties of configured type, if any.
*/
protected JsonInclude.Value _includeAsProperty;

/**
* Definitions of property ignoral (whether to serialize, deserialize
* given logical property) overrides, if any.
Expand All @@ -43,12 +48,14 @@ protected ConfigOverride() { }
protected ConfigOverride(ConfigOverride src) {
_format = src._format;
_include = src._include;
_includeAsProperty = src._includeAsProperty;
_ignorals = src._ignorals;
_isIgnoredType = src._isIgnoredType;
}

public JsonFormat.Value getFormat() { return _format; }
public JsonInclude.Value getInclude() { return _include; }
public JsonInclude.Value getIncludeAsProperty() { return _includeAsProperty; }
public JsonIgnoreProperties.Value getIgnorals() { return _ignorals; }

public Boolean getIsIgnoredType() {
Expand Down
25 changes: 25 additions & 0 deletions src/main/java/com/fasterxml/jackson/databind/cfg/MapperConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,18 @@ public BeanDescription introspectDirectClassAnnotations(Class<?> cls) {
*/
public abstract JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType);

/**
* Accessor for default property inclusion to use for serialization,
* considering possible per-type override for given base type and
* possible per-type override for given property type.<br>
* NOTE: if no override found, defaults to value returned by
* {@link #getDefaultPropertyInclusion()}.
*
* @since 2.8.8
*/
public abstract JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
Copy link
Member

Choose a reason for hiding this comment

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

This is a breaking change, so could not introduce in a patch for sure, and not really in a patch either. Could of course add a new method, deprecate old one, add bridge method.
But what is the meaning of additional type argument here? Why is baseType not sufficient? I thought it was meant specifically as the type for lookups.

Copy link
Contributor Author

@CarstenWickner CarstenWickner Mar 18, 2017

Choose a reason for hiding this comment

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

Agreed, that it shouldn't be merged as-is. As mentioned on the initial PR comment, this is just intended to be a proof-of-concept.

As far I understood the usage of these methods, the baseType refers to the type of bean containing the property and the additional type argument refers to the type of the property itself. The existing methods cannot look up the override configuration for a single property as they are not aware of its type.

Class<?> propertyType);

/**
* Accessor for default property inclusion to use for serialization,
* considering possible per-type override for given base type; but
Expand All @@ -377,6 +389,19 @@ public BeanDescription introspectDirectClassAnnotations(Class<?> cls) {
public abstract JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
JsonInclude.Value defaultIncl);

/**
* Accessor for default property inclusion to use for serialization,
* considering possible per-type override for given base type and
* possible per-type override for given property type; but
* if none found, returning given <code>defaultIncl</code>
*
* @param defaultIncl Inclusion setting to return if no overrides found.
*
* @since 2.8.8
*/
public abstract JsonInclude.Value getDefaultPropertyInclusion(Class<?> baseType,
Class<?> propertyType, JsonInclude.Value defaultIncl);

/**
* Accessor for default format settings to use for serialization (and, to a degree
* deserialization), considering baseline settings and per-type defaults
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ public MutableConfigOverride setInclude(JsonInclude.Value v) {
return this;
}

public MutableConfigOverride setIncludeAsProperty(JsonInclude.Value v) {
_includeAsProperty = v;
return this;
}

public MutableConfigOverride setIgnorals(JsonIgnoreProperties.Value v) {
_ignorals = v;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.fasterxml.jackson.annotation.JsonInclude;
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.annotation.JsonSerialize;
import com.fasterxml.jackson.databind.cfg.ConfigOverride;
import com.fasterxml.jackson.databind.introspect.*;
import com.fasterxml.jackson.databind.jsontype.TypeSerializer;
import com.fasterxml.jackson.databind.util.*;
Expand Down Expand Up @@ -56,8 +57,9 @@ public PropertyBuilder(SerializationConfig config, BeanDescription beanDesc)
// 08-Sep-2016, tatu: This gets tricky, with 3 levels of definitions:
// (a) global default inclusion
// (b) per-type default inclusion (from annotation or config overrides;
// latter having precedence
// Cc) per-property override
// config override having precedence)
// (c) per-property override (from annotation or config overrides;
// annotation having precedence)
//
// and not only requiring merging, but also considering special handling
// for NON_DEFAULT in case of (b) (vs (a) or (c))
Expand Down Expand Up @@ -127,11 +129,23 @@ protected BeanPropertyWriter buildWriter(SerializerProvider prov,
// 12-Jul-2016, tatu: [databind#1256] Need to make sure we consider type refinement
JavaType actualType = (serializationType == null) ? declaredType : serializationType;

// 17-Mar-2017: [databind#1522] Allow config override per property type
Class<?> rawPropertyType;
if (propDef.hasField()) {
rawPropertyType = propDef.getField().getRawType();
} else if (propDef.hasGetter()) {
rawPropertyType = propDef.getGetter().getRawReturnType();
} else {
// neither Setter nor ConstructorParameter are expected here
return prov.reportBadPropertyDefinition(_beanDesc, propDef,
"could not determine property type");
}

// 17-Aug-2016, tatu: Default inclusion covers global default (for all types), as well
// as type-default for enclosing POJO. What we need, then, is per-type default (if any)
// for declared property type... and finally property annotation overrides
JsonInclude.Value inclV = _config.getDefaultPropertyInclusion(actualType.getRawClass(),
_defaultInclusion);
rawPropertyType, _defaultInclusion);

// property annotation override

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,32 @@ static class EmptyListMapBean
public Map<String,String> map = Collections.emptyMap();
}

@JsonInclude(JsonInclude.Include.ALWAYS)
@JsonPropertyOrder({"num", "annotated", "plain"})
static class MixedTypeBean
{
@JsonInclude(JsonInclude.Include.USE_DEFAULTS)
public Integer num = null;

@JsonInclude(JsonInclude.Include.NON_NULL)
public String annotated = null;

public String plain = null;
}

@JsonInclude(JsonInclude.Include.NON_NULL)
@JsonPropertyOrder({"num", "annotated", "plain"})
static class MixedTypeNonNullBean
{
@JsonInclude(JsonInclude.Include.USE_DEFAULTS)
public Integer num = null;

@JsonInclude(JsonInclude.Include.ALWAYS)
public String annotated = null;

public String plain = null;
}

// [databind#1351]

static class Issue1351Bean
Expand Down Expand Up @@ -221,7 +247,7 @@ public void testNonDefaultByClassNoCtor() throws IOException
String json = MAPPER.writeValueAsString(bean);
assertEquals(aposToQuotes("{'x':1,'y':2}"), json);
}

public void testMixedMethod() throws IOException
{
MixedBean bean = new MixedBean();
Expand Down Expand Up @@ -309,6 +335,54 @@ public void testPropConfigOverridesForInclude() throws IOException
mapper.writeValueAsString(empty));
}

public void testPropConfigOverrideForIncludeAsPropertyNonNull() throws Exception
{
// First, with defaults, all but NON_NULL annotated included
MixedTypeBean nullValues = new MixedTypeBean();
assertEquals(aposToQuotes("{'num':null,'plain':null}"),
MAPPER.writeValueAsString(nullValues));
ObjectMapper mapper;

// and then change inclusion as property criteria for either
mapper = new ObjectMapper();
mapper.configOverride(String.class)
.setIncludeAsProperty(JsonInclude.Value
.construct(JsonInclude.Include.NON_NULL, null));
assertEquals("{\"num\":null}",
mapper.writeValueAsString(nullValues));

mapper = new ObjectMapper();
mapper.configOverride(Integer.class)
.setIncludeAsProperty(JsonInclude.Value
.construct(JsonInclude.Include.NON_NULL, null));
assertEquals("{\"plain\":null}",
mapper.writeValueAsString(nullValues));
}

public void testPropConfigOverrideForIncludeAsPropertyAlways() throws Exception
{
// First, with defaults, only ALWAYS annotated included
MixedTypeNonNullBean nullValues = new MixedTypeNonNullBean();
assertEquals("{\"annotated\":null}",
MAPPER.writeValueAsString(nullValues));
ObjectMapper mapper;

// and then change inclusion as property criteria for either
mapper = new ObjectMapper();
mapper.configOverride(String.class)
.setIncludeAsProperty(JsonInclude.Value
.construct(JsonInclude.Include.ALWAYS, null));
assertEquals(aposToQuotes("{'annotated':null,'plain':null}"),
mapper.writeValueAsString(nullValues));

mapper = new ObjectMapper();
mapper.configOverride(Integer.class)
.setIncludeAsProperty(JsonInclude.Value
.construct(JsonInclude.Include.ALWAYS, null));
assertEquals(aposToQuotes("{'num':null,'annotated':null}"),
mapper.writeValueAsString(nullValues));
}

// [databind#1351], [databind#1417]
public void testIssue1351() throws Exception
{
Expand Down