Skip to content

[databind#4849] Allow standard defaultTyping with EnumSet<E> #4857

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

Merged
merged 13 commits into from
Dec 19, 2024
6 changes: 4 additions & 2 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,17 @@ Project: jackson-databind
(reported by Eduard G)
#4773: `SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS` should not apply to Maps
with uncomparable keys
(requested by @nathanukey)
(requested by @nathanukey
#4849 Not able to deserialize Enum with default typing after upgrading 2.15.4 -> 2.17.1
(reported by Kornel Zemla)

2.18.3 (not yet released)

#4827: Subclassed Throwable deserialization fails since v2.18.0 - no creator
index for property 'cause'
(reported by @nilswieber)
(fix by Joo-Hyuk K)
#4844: Fix wrapped array hanlding wrt `null` by `StdDeserializer`
#4844: Fix wrapped array handling wrt `null` by `StdDeserializer`
(fix by Stanislav S)

2.18.2 (27-Nov-2024)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -788,8 +788,7 @@ public JsonDeserializer<?> createCollectionDeserializer(DeserializationContext c
if (contentDeser == null) { // not defined by annotation
// One special type: EnumSet:
if (EnumSet.class.isAssignableFrom(collectionClass)) {
deser = new EnumSetDeserializer(contentType, null,
contentTypeDeser);
deser = new EnumSetDeserializer(contentType, null);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,6 @@ public class EnumSetDeserializer

protected JsonDeserializer<Enum<?>> _enumDeserializer;

/**
* If element instances have polymorphic type information, this
* is the type deserializer that can handle it.
*<p>
* NOTE: only added in 2.17 due to new {@code DefaultType} choices
* that allow polymorphic deserialization of {@code Enum} types.
*
* @since 2.17
*/
protected final TypeDeserializer _valueTypeDeserializer;

/**
* Handler we need for dealing with nulls.
*
Expand Down Expand Up @@ -74,11 +63,12 @@ public class EnumSetDeserializer
*/

/**
* @since 2.17
* Main constructor for the deserializer.
*<p>
* NOTE: was temporarily deprecated in 2.17 - 2.18, restored in 2.19
*/
@SuppressWarnings("unchecked" )
public EnumSetDeserializer(JavaType enumType, JsonDeserializer<?> deser,
TypeDeserializer valueTypeDeser)
public EnumSetDeserializer(JavaType enumType, JsonDeserializer<?> deser)
{
super(EnumSet.class);
_enumType = enumType;
Expand All @@ -87,29 +77,19 @@ public EnumSetDeserializer(JavaType enumType, JsonDeserializer<?> deser,
throw new IllegalArgumentException("Type "+enumType+" not Java Enum type");
}
_enumDeserializer = (JsonDeserializer<Enum<?>>) deser;
_valueTypeDeserializer = valueTypeDeser;
_unwrapSingle = null;
_nullProvider = null;
_skipNullValues = false;
}

/**
* @deprecated Since 2.17
* @deprecated Since 2.19 (was added in 2.17)
*/
@Deprecated
public EnumSetDeserializer(JavaType enumType, JsonDeserializer<?> deser)
public EnumSetDeserializer(JavaType enumType, JsonDeserializer<?> deser,
TypeDeserializer valueTypeDeser)
{
this(enumType, deser, null);
}

/**
* @since 2.7
* @deprecated Since 2.10.1
*/
@Deprecated
protected EnumSetDeserializer(EnumSetDeserializer base,
JsonDeserializer<?> deser, Boolean unwrapSingle) {
this(base, deser, base._nullProvider, unwrapSingle);
this(enumType, deser);
}

/**
Expand All @@ -121,7 +101,6 @@ protected EnumSetDeserializer(EnumSetDeserializer base,
super(base);
_enumType = base._enumType;
_enumDeserializer = (JsonDeserializer<Enum<?>>) deser;
_valueTypeDeserializer = base._valueTypeDeserializer;
_nullProvider = nuller;
_skipNullValues = NullsConstantProvider.isSkipper(nuller);
_unwrapSingle = unwrapSingle;
Expand All @@ -135,29 +114,18 @@ public EnumSetDeserializer withDeserializer(JsonDeserializer<?> deser) {
}

/**
* @since 2.10.1
* @since 2.19
*/
public EnumSetDeserializer withResolved(JsonDeserializer<?> deser,
TypeDeserializer valueTypeDeser,
NullValueProvider nuller, Boolean unwrapSingle) {
if ((Objects.equals(_unwrapSingle, unwrapSingle))
&& (_enumDeserializer == deser)
&& (_valueTypeDeserializer == valueTypeDeser)
&& (_nullProvider == deser)) {
return this;
}
return new EnumSetDeserializer(this, deser, nuller, unwrapSingle);
}

/**
* @deprecated Since 2.17
*/
@Deprecated
public EnumSetDeserializer withResolved(JsonDeserializer<?> deser,
NullValueProvider nuller, Boolean unwrapSingle) {
return withResolved(deser, _valueTypeDeserializer, nuller, unwrapSingle);
}

/*
/**********************************************************
/* Basic metadata
Expand All @@ -171,9 +139,7 @@ public EnumSetDeserializer withResolved(JsonDeserializer<?> deser,
@Override
public boolean isCachable() {
// One caveat: content deserializer should prevent caching
if ((_enumType.getValueHandler() != null)
// Another: polymorphic deserialization
|| (_valueTypeDeserializer != null)) {
if (_enumType.getValueHandler() != null) {
return false;
}
return true;
Expand Down Expand Up @@ -220,12 +186,7 @@ public JsonDeserializer<?> createContextual(DeserializationContext ctxt,
} else { // if directly assigned, probably not yet contextual, so:
deser = ctxt.handleSecondaryContextualization(deser, property, _enumType);
}
// and finally, type deserializer needs context as well
TypeDeserializer valueTypeDeser = _valueTypeDeserializer;
if (valueTypeDeser != null) {
valueTypeDeser = valueTypeDeser.forProperty(property);
}
return withResolved(deser, valueTypeDeser,
return withResolved(deser,
findContentNullProvider(ctxt, property, deser), unwrapSingle);
}

Expand Down Expand Up @@ -261,10 +222,8 @@ public EnumSet<?> deserialize(JsonParser p, DeserializationContext ctxt,
protected final EnumSet<?> _deserialize(JsonParser p, DeserializationContext ctxt,
EnumSet result) throws IOException
{
JsonToken t;
final TypeDeserializer typeDeser = _valueTypeDeserializer;

try {
JsonToken t;
while ((t = p.nextToken()) != JsonToken.END_ARRAY) {
// What to do with nulls? Fail or ignore? Fail, for now (note: would fail if we
// passed it to EnumDeserializer, too, but in general nulls should never be passed
Expand All @@ -275,10 +234,8 @@ protected final EnumSet<?> _deserialize(JsonParser p, DeserializationContext ctx
continue;
}
value = (Enum<?>) _nullProvider.getNullValue(ctxt);
} else if (typeDeser == null) {
value = _enumDeserializer.deserialize(p, ctxt);
} else {
value = (Enum<?>) _enumDeserializer.deserializeWithType(p, ctxt, typeDeser);
value = _enumDeserializer.deserialize(p, ctxt);
}
if (value != null) {
result.add(value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ protected String _locateTypeId(JsonParser p, DeserializationContext ctxt) throws
}
return id;
}
ctxt.reportWrongTokenException(baseType(), JsonToken.START_ARRAY,
"need Array value to contain `As.WRAPPER_ARRAY` type information for class "+baseTypeName());
return null;
ctxt.reportWrongTokenException(baseType(), JsonToken.START_ARRAY,
"need Array value to contain `As.WRAPPER_ARRAY` type information for class "+baseTypeName());
return null;
}
// And then type id as a String
JsonToken t = p.nextToken();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.io.IOException;
import java.util.Collection;
import java.util.EnumSet;
import java.util.Iterator;

import com.fasterxml.jackson.core.*;
Expand All @@ -26,6 +27,15 @@ public class CollectionSerializer
{
private static final long serialVersionUID = 1L;

/**
* Flag that indicates that we may need to check for EnumSet dynamically
* during serialization: problem being that we can't always do it statically.
* But we can figure out when there is a possibility wrt type signature we get.
*
* @since 2.18.3
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
* @since 2.18.3
* @since 2.19

*/
private final boolean _maybeEnumSet;

/*
/**********************************************************
/* Life-cycle
Expand All @@ -38,22 +48,17 @@ public class CollectionSerializer
public CollectionSerializer(JavaType elemType, boolean staticTyping, TypeSerializer vts,
JsonSerializer<Object> valueSerializer) {
super(Collection.class, elemType, staticTyping, vts, valueSerializer);
}

/**
* @deprecated since 2.6
*/
@Deprecated // since 2.6
public CollectionSerializer(JavaType elemType, boolean staticTyping, TypeSerializer vts,
BeanProperty property, JsonSerializer<Object> valueSerializer) {
// note: assumption is 'property' is always passed as null
this(elemType, staticTyping, vts, valueSerializer);
// Unfortunately we can't check for EnumSet statically (if type indicated it,
// we'd have constructed `EnumSetSerializer` instead). But we can check that
// element type could possibly be an Enum.
_maybeEnumSet = elemType.isEnumType() || elemType.isJavaLangObject();
Comment on lines +51 to +54
Copy link
Member Author

Choose a reason for hiding this comment

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

For 3.x version, would it be worth the effort trying to find other way instead of this?
And also parts with below code

// [databind#4849]/[databind#4214]: need to check for EnumSet
        final TypeSerializer typeSer = (_maybeEnumSet && value instanceof EnumSet<?>)
                ? null : _valueTypeSerializer;
                

Copy link
Member

Choose a reason for hiding this comment

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

If there is a way, sure, it would be great to get rid of this work-around.
But I am not sure how, considering the problem itself and how it occurs.
That is: I do not know how this could be avoided at all, regardless of compatibility concerns.

}

public CollectionSerializer(CollectionSerializer src,
BeanProperty property, TypeSerializer vts, JsonSerializer<?> valueSerializer,
Boolean unwrapSingle) {
super(src, property, vts, valueSerializer, unwrapSingle);
_maybeEnumSet = src._maybeEnumSet;
}

@Override
Expand Down Expand Up @@ -120,7 +125,9 @@ public void serializeContents(Collection<?> value, JsonGenerator g, SerializerPr
return;
}
PropertySerializerMap serializers = _dynamicSerializers;
final TypeSerializer typeSer = _valueTypeSerializer;
// [databind#4849]/[databind#4214]: need to check for EnumSet
final TypeSerializer typeSer = (_maybeEnumSet && value instanceof EnumSet<?>)
? null : _valueTypeSerializer;

int i = 0;
try {
Expand Down Expand Up @@ -158,7 +165,9 @@ public void serializeContentsUsing(Collection<?> value, JsonGenerator g, Seriali
{
Iterator<?> it = value.iterator();
if (it.hasNext()) {
TypeSerializer typeSer = _valueTypeSerializer;
// [databind#4849]/[databind#4214]: need to check for EnumSet
final TypeSerializer typeSer = (_maybeEnumSet && value instanceof EnumSet<?>)
? null : _valueTypeSerializer;
int i = 0;
do {
Object elem = it.next();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public EnumSetSerializer(EnumSetSerializer src,

@Override
public EnumSetSerializer _withValueTypeSerializer(TypeSerializer vts) {
// no typing for enums (always "hard" type)
// no typing for enum elements (always strongly typed), so don't change
return this;
}

Expand All @@ -48,7 +48,7 @@ public boolean hasSingleElement(EnumSet<? extends Enum<?>> value) {
}

@Override
public final void serialize(EnumSet<? extends Enum<?>> value, JsonGenerator gen,
public void serialize(EnumSet<? extends Enum<?>> value, JsonGenerator gen,
SerializerProvider provider) throws IOException
{
final int len = value.size();
Expand All @@ -70,15 +70,13 @@ public void serializeContents(EnumSet<? extends Enum<?>> value, JsonGenerator ge
SerializerProvider provider)
throws IOException
{
gen.assignCurrentValue(value);
JsonSerializer<Object> enumSer = _elementSerializer;
/* Need to dynamically find instance serializer; unfortunately
* that seems to be the only way to figure out type (no accessors
* to the enum class that set knows)
*/
// Need to dynamically find instance serializer; unfortunately
// that seems to be the only way to figure out type (no accessors
// to the enum class that set knows)
for (Enum<?> en : value) {
if (enumSer == null) {
// 12-Jan-2010, tatu: Since enums cannot be polymorphic, let's
// not bother with typed serializer variant here
enumSer = provider.findContentValueSerializer(en.getDeclaringClass(), _property);
}
enumSer.serialize(en, gen, provider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ public void jsonMapperRebuildTest()

JsonMapper m3 = m2.rebuild()
.propertyNamingStrategy(PropertyNamingStrategies.SNAKE_CASE)
.enumNamingStrategy(EnumNamingStrategies.CamelCaseStrategy.INSTANCE)
.enumNamingStrategy(EnumNamingStrategies.UPPER_CAMEL_CASE)
.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_ENUMS)
.enable(EnumFeature.WRITE_ENUMS_TO_LOWERCASE)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import com.fasterxml.jackson.databind.exc.InvalidDefinitionException;
import com.fasterxml.jackson.databind.introspect.AnnotatedWithParams;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.databind.type.TypeFactory;

import static org.junit.jupiter.api.Assertions.*;

Expand Down
Loading