Skip to content

Commit

Permalink
Fix #1850 using logic from PR #3242 (with minor modifications)
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Sep 27, 2021
1 parent c100ed5 commit c7a4b0c
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 49 deletions.
5 changes: 4 additions & 1 deletion 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.13.0 (not yet released)

#1850: `@JsonValue` with integer for enum does not deserialize correctly
(reported by tgolden-andplus@github)
(fix contributed by limengning@github)
#2509: `AnnotatedMethod.getValue()/setValue()` doesn't have useful exception message
(reported by henryptung@github)
(fix contributed by Stephan S)
Expand Down Expand Up @@ -60,7 +63,7 @@ Project: jackson-databind
(suggested by Nick B)
#3214: For an absent property Jackson injects `NullNode` instead of `null` to a
JsonNode-typed constructor argument of a `@ConstructorProperties`-annotated constructor
(repored by robvarga@github)
(reported by robvarga@github)
#3217: `XMLGregorianCalendar` doesn't work with default typing
(reported by Xinzhe Y)
#3227: Content `null` handling not working for root values
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ public class EnumDeserializer

protected final Boolean _caseInsensitive;

/**
* Marker flag for cases where we expect actual integral value for Enum,
* based on {@code @JsonValue} (and equivalent) annotated accessor.
*
* @since 2.13
*/
protected final boolean _isFromIntValue;

/**
* @since 2.9
*/
Expand All @@ -63,6 +71,7 @@ public EnumDeserializer(EnumResolver byNameResolver, Boolean caseInsensitive)
_enumsByIndex = byNameResolver.getRawEnums();
_enumDefaultValue = byNameResolver.getDefaultValue();
_caseInsensitive = caseInsensitive;
_isFromIntValue = byNameResolver.isFromIntValue();
}

/**
Expand All @@ -75,6 +84,7 @@ protected EnumDeserializer(EnumDeserializer base, Boolean caseInsensitive)
_enumsByIndex = base._enumsByIndex;
_enumDefaultValue = base._enumDefaultValue;
_caseInsensitive = caseInsensitive;
_isFromIntValue = base._isFromIntValue;
}

/**
Expand All @@ -84,7 +94,7 @@ protected EnumDeserializer(EnumDeserializer base, Boolean caseInsensitive)
public EnumDeserializer(EnumResolver byNameResolver) {
this(byNameResolver, null);
}

/**
* @deprecated Since 2.8
*/
Expand Down Expand Up @@ -190,6 +200,13 @@ public Object deserialize(JsonParser p, DeserializationContext ctxt) throws IOEx

// But let's consider int acceptable as well (if within ordinal range)
if (p.hasToken(JsonToken.VALUE_NUMBER_INT)) {
// 26-Sep-2021, tatu: [databind#1850] Special case where we get "true" integer
// enumeration and should avoid use of {@code Enum.index()}
if (_isFromIntValue) {
// ... whether to rely on "getText()" returning String, or get number, convert?
// For now assume all format backends can produce String:
return _fromString(p, ctxt, p.getText());
}
return _fromInteger(p, ctxt, p.getIntValue());
}

Expand Down Expand Up @@ -309,7 +326,8 @@ private final Object _deserializeAltString(JsonParser p, DeserializationContext
if (match != null) {
return match;
}
} else if (!ctxt.isEnabled(DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS)) {
} else if (!ctxt.isEnabled(DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS)
&& !_isFromIntValue) {
// [databind#149]: Allow use of 'String' indexes as well -- unless prohibited (as per above)
char c = name.charAt(0);
if (c >= '0' && c <= '9') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,9 @@ public static Object defaultValue(Class<?> cls)

/**
* Helper method for finding wrapper type for given primitive type (why isn't
* there one in JDK?)
* there one in JDK?).
* NOTE: throws {@link IllegalArgumentException} if given type is NOT primitive
* type (caller has to check).
*/
public static Class<?> wrapperType(Class<?> primitiveType)
{
Expand Down Expand Up @@ -908,7 +910,7 @@ public static Class<?> wrapperType(Class<?> primitiveType)

/**
* Method that can be used to find primitive type for given class if (but only if)
* it is either wrapper type or primitive type; returns `null` if type is neither.
* it is either wrapper type or primitive type; returns {@code null} if type is neither.
*
* @since 2.7
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,47 @@ public class EnumResolver implements java.io.Serializable
protected final Enum<?> _defaultValue;

/**
* Marker for case-insensitive handling
*
* @since 2.12
*/
protected final boolean _isIgnoreCase;

/**
* Marker for case where value may come from {@code @JsonValue} annotated
* accessor and is expected/likely to come from actual integral number
* value (and not String).
*<p>
* Special case is needed since this specifically means that {@code Enum.index()}
* should NOT be used or default to.
*
* @since 2.13
*/
protected final boolean _isFromIntValue;

/**
* @since 2.12
*/
protected EnumResolver(Class<Enum<?>> enumClass, Enum<?>[] enums,
HashMap<String, Enum<?>> map, Enum<?> defaultValue,
boolean isIgnoreCase)
boolean isIgnoreCase, boolean isFromIntValue)
{
_enumClass = enumClass;
_enums = enums;
_enumsById = map;
_defaultValue = defaultValue;
_isIgnoreCase = isIgnoreCase;
_isFromIntValue = isFromIntValue;
}

/**
* @deprecated Since 2.13
*/
@Deprecated // since 2.13
protected EnumResolver(Class<Enum<?>> enumClass, Enum<?>[] enums,
HashMap<String, Enum<?>> map, Enum<?> defaultValue,
boolean isIgnoreCase) {
this(enumClass, enums, map, defaultValue, isIgnoreCase, false);
}

/**
Expand Down Expand Up @@ -85,7 +110,8 @@ protected static EnumResolver _constructFor(Class<?> enumCls0,
}
}
return new EnumResolver(enumCls, enumConstants, map,
_enumDefault(ai, enumCls), isIgnoreCase);
_enumDefault(ai, enumCls), isIgnoreCase,
false);
}

/**
Expand Down Expand Up @@ -130,7 +156,7 @@ protected static EnumResolver _constructUsingToString(Class<?> enumCls0,
}
}
return new EnumResolver(enumCls, enumConstants, map,
_enumDefault(ai, enumCls), isIgnoreCase);
_enumDefault(ai, enumCls), isIgnoreCase, false);
}

/**
Expand Down Expand Up @@ -167,7 +193,10 @@ protected static EnumResolver _constructUsingMethod(Class<?> enumCls0,
}
}
return new EnumResolver(enumCls, enumConstants, map,
_enumDefault(ai, enumCls), isIgnoreCase);
_enumDefault(ai, enumCls), isIgnoreCase,
// 26-Sep-2021, tatu: [databind#1850] Need to consider "from int" case
_isIntType(accessor.getRawType())
);
}

public CompactStringObjectMap constructLookup() {
Expand All @@ -191,6 +220,17 @@ protected static Enum<?> _enumDefault(AnnotationIntrospector intr, Class<?> enum
return (intr != null) ? intr.findDefaultEnumValue(_enumClass(enumCls)) : null;
}

protected static boolean _isIntType(Class<?> erasedType) {
if (erasedType.isPrimitive()) {
erasedType = ClassUtil.wrapperType(erasedType);
}
return (erasedType == Long.class)
|| (erasedType == Integer.class)
|| (erasedType == Short.class)
|| (erasedType == Byte.class)
;
}

/*
/**********************************************************************
/* Deprecated constructors, factory methods
Expand All @@ -203,7 +243,7 @@ protected static Enum<?> _enumDefault(AnnotationIntrospector intr, Class<?> enum
@Deprecated // since 2.12
protected EnumResolver(Class<Enum<?>> enumClass, Enum<?>[] enums,
HashMap<String, Enum<?>> map, Enum<?> defaultValue) {
this(enumClass, enums, map, defaultValue, false);
this(enumClass, enums, map, defaultValue, false, false);
}

/**
Expand Down Expand Up @@ -327,5 +367,17 @@ public Collection<String> getEnumIds() {
public Class<Enum<?>> getEnumClass() { return _enumClass; }

public int lastValidIndex() { return _enums.length-1; }

/**
* Accessor for checking if we have a special case in which value to map
* is from {@code @JsonValue} annotated accessor with integral type: this
* matters for cases where incoming content value is of integral type
* and should be mapped to specific value and NOT to {@code Enum.index()}.
*
* @since 2.13
*/
public boolean isFromIntValue() {
return _isFromIntValue;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package com.fasterxml.jackson.databind.deser.jdk;

import com.fasterxml.jackson.annotation.JsonValue;

import com.fasterxml.jackson.databind.*;

public class EnumDeserFromIntJsonValueTest extends BaseMapTest
{
// [databind#1850]

enum Bean1850IntMethod {
A(101);
final int x;
Bean1850IntMethod(int x) { this.x = x; }
@JsonValue
public int code() { return x; }
}

enum Bean1850IntField {
A(202);
@JsonValue
public final int x;
Bean1850IntField(int x) { this.x = x; }
}

enum Bean1850LongMethod {
A(-13L);
final long x;
Bean1850LongMethod(long x) { this.x = x; }
@JsonValue
public long code() { return x; }
}

enum Bean1850LongField {
A(29L);
@JsonValue
public final long x;
Bean1850LongField(long x) { this.x = x; }
}

private final ObjectMapper MAPPER = newJsonMapper();

// [databind#1850] pass tests

public void testEnumFromInt1850Method() throws Exception
{
String json = MAPPER.writeValueAsString(Bean1850IntMethod.A);
Bean1850IntMethod e1 = MAPPER.readValue(json, Bean1850IntMethod.class);
assertEquals(Bean1850IntMethod.A, e1);
}

public void testEnumFromInt1850Field() throws Exception
{
String json = MAPPER.writeValueAsString(Bean1850IntField.A);
Bean1850IntField e2 = MAPPER.readValue(json, Bean1850IntField.class);
assertEquals(Bean1850IntField.A, e2);
}

public void testEnumFromLong1850Method() throws Exception
{
String json = MAPPER.writeValueAsString(Bean1850LongMethod.A);
Bean1850LongMethod e1 = MAPPER.readValue(json, Bean1850LongMethod.class);
assertEquals(Bean1850LongMethod.A, e1);
}

public void testEnumFromLong1850Field() throws Exception
{
String json = MAPPER.writeValueAsString(Bean1850LongField.A);
Bean1850LongField e2 = MAPPER.readValue(json, Bean1850LongField.class);
assertEquals(Bean1850LongField.A, e2);
}
}

This file was deleted.

0 comments on commit c7a4b0c

Please sign in to comment.