Skip to content

Commit

Permalink
Fix #2804
Browse files Browse the repository at this point in the history
  • Loading branch information
cowtowncoder committed Oct 15, 2020
1 parent af78f0a commit 29f7447
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 30 deletions.
3 changes: 3 additions & 0 deletions release-notes/VERSION-2.x
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ Project: jackson-databind
of `ObjectMapper.treeToValue()` regarding exceptions
#2880: Revert removal of 2.7-deprecated `PropertyNamingStrategy` constants
(reported by brettkail-wk@github)
#2804: Throw `InvalidFormatException` instead of `MismatchedInputException`

This comment has been minimized.

Copy link
@mjustin

mjustin Oct 15, 2020

Am I seeing correctly in the code that the actual change is broader than just the ACCEPT_FLOAT_AS_INT case? Is that something you would typically include in the release notes? For instance, I'm seeing empty/blank string conversions including the additional information, which wouldn't be the float-to-int issue, right?.

    protected CoercionAction _checkFromStringCoercion(DeserializationContext ctxt, String value,
            LogicalType logicalType, Class<?> rawTargetType)
        throws IOException
    {
        final CoercionAction act;
        if (value.length() == 0) {
            act = ctxt.findCoercionAction(logicalType, rawTargetType,
                    CoercionInputShape.EmptyString);
            return _checkCoercionActionFail(ctxt, act, "empty String (\"\")");
            return _checkCoercionFail(ctxt, act, rawTargetType, value,
                    "empty String (\"\")");
        } else if (_isBlank(value)) {
            act = ctxt.findCoercionFromBlankString(logicalType, rawTargetType, CoercionAction.Fail);
            return _checkCoercionActionFail(ctxt, act, "blank String (all whitespace)");
            return _checkCoercionFail(ctxt, act, rawTargetType, value,
                    "blank String (all whitespace)");
        }

This comment has been minimized.

Copy link
@cowtowncoder

cowtowncoder Oct 15, 2020

Author Member

That change was part of earlier CoercionConfig changes #2113, for the most part. Change here is to use subtype of an extension (but without further change to exception message), so bit of judgment call on what to call out and where.

for ACCEPT_FLOAT_AS_INT coercion failures
(requested by mjustin@github)

2.12.0-rc1 (12-Oct-2020)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1654,6 +1654,22 @@ public <T> T reportPropertyInputMismatch(JavaType targetType, String propertyNam
return reportPropertyInputMismatch(targetType.getRawClass(), propertyName, msg, msgArgs);
}

/**
* Helper method used to indicate a problem with input in cases where specific
* input coercion was not allowed.
*
* @since 2.12
*/
public <T> T reportBadCoercion(JsonDeserializer<?> src,
Class<?> targetType, Object inputValue,
String msg, Object... msgArgs) throws JsonMappingException
{
msg = _format(msg, msgArgs);
InvalidFormatException e = InvalidFormatException.from(getParser(),
msg, inputValue, targetType);
throw e;
}

public <T> T reportTrailingTokens(Class<?> targetType,
JsonParser p, JsonToken trailingToken) throws JsonMappingException
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ protected static String translateLowerCaseWithSeparator(final String input, fina
*/

/**
* @deprecated Since 2.12 use {@link PropertyNamingStrategies#SnakeCaseStrategy} instead
* @deprecated Since 2.12 use {@link PropertyNamingStrategies.SnakeCaseStrategy} instead
* (see
* <a href="https://github.com/FasterXML/jackson-databind/issues/2715">databind#2715</a>
* for reason for deprecation)
Expand Down Expand Up @@ -310,7 +310,7 @@ public String translate(String input)
}

/**
* @deprecated Since 2.12 use {@link PropertyNamingStrategies#UpperCamelCaseStrategy} instead
* @deprecated Since 2.12 use {@link PropertyNamingStrategies.UpperCamelCaseStrategy} instead
* (see
* <a href="https://github.com/FasterXML/jackson-databind/issues/2715">databind#2715</a>
* for reason for deprecation)
Expand Down Expand Up @@ -345,7 +345,7 @@ public String translate(String input) {
}

/**
* @deprecated Since 2.12 use {@link PropertyNamingStrategies#LowerCaseStrategy} instead
* @deprecated Since 2.12 use {@link PropertyNamingStrategies.LowerCaseStrategy} instead
* (see
* <a href="https://github.com/FasterXML/jackson-databind/issues/2715">databind#2715</a>
* for reason for deprecation)
Expand All @@ -360,7 +360,7 @@ public String translate(String input) {
}

/**
* @deprecated Since 2.12 use {@link PropertyNamingStrategies#KebabCaseStrategy} instead
* @deprecated Since 2.12 use {@link PropertyNamingStrategies.KebabCaseStrategy} instead
* (see
* <a href="https://github.com/FasterXML/jackson-databind/issues/2715">databind#2715</a>
* for reason for deprecation)
Expand All @@ -375,7 +375,7 @@ public String translate(String input) {
}

/**
* @deprecated Since 2.12 use {@link PropertyNamingStrategies#LowerDotCaseStrategy} instead
* @deprecated Since 2.12 use {@link PropertyNamingStrategies.LowerDotCaseStrategy} instead
* (see
* <a href="https://github.com/FasterXML/jackson-databind/issues/2715">databind#2715</a>
* for reason for deprecation)
Expand Down Expand Up @@ -407,13 +407,13 @@ public String translate(String input){
public static final PropertyNamingStrategy PASCAL_CASE_TO_CAMEL_CASE = UPPER_CAMEL_CASE;

/**
* @deprecated In 2.7 use {@link PropertyNamingStrategies#SnakeCaseStrategy} instead
* @deprecated In 2.7 use {@link PropertyNamingStrategies.SnakeCaseStrategy} instead
*/
@Deprecated // since 2.7
public static class LowerCaseWithUnderscoresStrategy extends SnakeCaseStrategy {}

/**
* @deprecated In 2.7 use {@link PropertyNamingStrategies#UpperCamelCaseStrategy} instead
* @deprecated In 2.7 use {@link PropertyNamingStrategies.UpperCamelCaseStrategy} instead
*/
@Deprecated // since 2.7
public static class PascalCaseStrategy extends UpperCamelCaseStrategy { }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public final class ConstructorDetector
*<p>
* Default choice is {@code HEURISTIC} (which is Jackson pre-2.12 always uses)
*<p>
* NOTE: does NOT have any effect if explicit {@link @JsonCreator}} annotation
* NOTE: does NOT have any effect if explicit {@code @JsonCreator}} annotation
* is required.
*
* @since 2.12
Expand Down Expand Up @@ -187,9 +187,10 @@ public boolean singleArgCreatorDefaultsToProperties() {
}

/**
* Accessor that combines calls to {@link #allowImplicitCreators} and
* {@link #allowJDKTypeConstructors} to determine whether implicit constructor
* detection should be enabled or not.
* Accessor that combines checks for whether implicit creators are allowed
* and, if so, whether JDK type constructors are allowed (if type is JDK type)
* to determine whether implicit constructor
* detection should be enabled for given type or not.
*
* @param rawType Value type to consider
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,9 @@ public Character deserialize(JsonParser p, DeserializationContext ctxt)
CoercionAction act = ctxt.findCoercionAction(logicalType(), _valueClass, CoercionInputShape.Integer);
switch (act) {
case Fail:
_checkCoercionActionFail(ctxt, act, "Integer value ("+p.getText()+")");
break;
_checkCoercionFail(ctxt, act, _valueClass, p.getNumberValue(),
"Integer value ("+p.getText()+")");
// fall-through in unlikely case of returning
case AsNull:
return getNullValue(ctxt);
case AsEmpty:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,10 +377,6 @@ protected final boolean _parseBooleanPrimitive(DeserializationContext ctxt,
/**
* @param ctxt Deserialization context for accessing configuration
* @param p Underlying parser
* @param targetType Actual type that is being deserialized, typically
* same as {@link #handledType}, and not necessarily {@code boolean}
* (may be {@code boolean[]} or {@code AtomicBoolean} for example);
* used for coercion config access
*/
protected final boolean _parseBooleanPrimitive(JsonParser p, DeserializationContext ctxt)
throws IOException
Expand Down Expand Up @@ -1284,10 +1280,12 @@ protected CoercionAction _checkFromStringCoercion(DeserializationContext ctxt, S
if (value.length() == 0) {
act = ctxt.findCoercionAction(logicalType, rawTargetType,
CoercionInputShape.EmptyString);
return _checkCoercionActionFail(ctxt, act, "empty String (\"\")");
return _checkCoercionFail(ctxt, act, rawTargetType, value,
"empty String (\"\")");
} else if (_isBlank(value)) {
act = ctxt.findCoercionFromBlankString(logicalType, rawTargetType, CoercionAction.Fail);
return _checkCoercionActionFail(ctxt, act, "blank String (all whitespace)");
return _checkCoercionFail(ctxt, act, rawTargetType, value,
"blank String (all whitespace)");
} else {
act = ctxt.findCoercionAction(logicalType, rawTargetType, CoercionInputShape.String);
if (act == CoercionAction.Fail) {
Expand All @@ -1310,7 +1308,8 @@ protected CoercionAction _checkFloatToIntCoercion(JsonParser p, DeserializationC
final CoercionAction act = ctxt.findCoercionAction(LogicalType.Integer,
rawTargetType, CoercionInputShape.Float);
if (act == CoercionAction.Fail) {
_checkCoercionActionFail(ctxt, act, "Floating-point value ("+p.getText()+")");
return _checkCoercionFail(ctxt, act, rawTargetType, p.getNumberValue(),
"Floating-point value ("+p.getText()+")");
}
return act;
}
Expand All @@ -1325,8 +1324,9 @@ protected Boolean _coerceBooleanFromInt(JsonParser p, DeserializationContext ctx
CoercionAction act = ctxt.findCoercionAction(LogicalType.Boolean, rawTargetType, CoercionInputShape.Integer);
switch (act) {
case Fail:
_checkCoercionActionFail(ctxt, act, "Integer value ("+p.getText()+")");
break;
_checkCoercionFail(ctxt, act, rawTargetType, p.getNumberValue(),
"Integer value ("+p.getText()+")");
return Boolean.FALSE;
case AsNull:
return null;
case AsEmpty:
Expand All @@ -1343,11 +1343,16 @@ protected Boolean _coerceBooleanFromInt(JsonParser p, DeserializationContext ctx
return !"0".equals(p.getText());
}

protected CoercionAction _checkCoercionActionFail(DeserializationContext ctxt,
CoercionAction act, String inputDesc) throws IOException
/**
* @since 2.12
*/
protected CoercionAction _checkCoercionFail(DeserializationContext ctxt,
CoercionAction act, Class<?> targetType, Object inputValue,
String inputDesc)
throws IOException
{
if (act == CoercionAction.Fail) {
ctxt.reportInputMismatch(this,
ctxt.reportBadCoercion(this, targetType, inputValue,
"Cannot coerce %s to %s (but could if coercion was enabled using `CoercionConfig`)",
inputDesc, _coercedTypeDesc());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ public JavaType constructType(TypeReference<?> typeRef)
* This is typically used only by code in databind itself.
*
* @param type Type of a {@link java.lang.reflect.Member} to resolve
* @param bindings Type bindings from the context, often class in which
* @param contextBindings Type bindings from the context, often class in which
* member declared but may be subtype of that type (to bind actual bound
* type parametrers). Not used if {@code type} is of type {@code Class<?>}.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
package com.fasterxml.jackson.databind.convert;

import java.math.BigInteger;
import java.util.*;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.cfg.CoercionAction;
import com.fasterxml.jackson.databind.cfg.CoercionInputShape;
import com.fasterxml.jackson.databind.exc.InvalidFormatException;
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
import com.fasterxml.jackson.databind.type.LogicalType;

public class CoerceFloatToIntTest extends BaseMapTest
{
private final ObjectMapper DEFAULT_MAPPER = sharedMapper();
private final ObjectMapper DEFAULT_MAPPER = newJsonMapper();
private final ObjectReader READER_LEGACY_FAIL = DEFAULT_MAPPER.reader()
.without(DeserializationFeature.ACCEPT_FLOAT_AS_INT);

Expand Down Expand Up @@ -104,6 +107,45 @@ public void testLegacyFailDoubleToOther() throws Exception
// _verifyCoerceFail(READER_LEGACY_FAIL, AtomicLong.class, "25236.256");
}

/*
/********************************************************
/* Test methods, legacy, correct exception type
/********************************************************
*/

// [databind#2804]
public void testLegacyFail2804() throws Exception
{
_testLegacyFail2804("5.5", Integer.class);
_testLegacyFail2804("5.0", Long.class);
_testLegacyFail2804("1234567890123456789.0", BigInteger.class);
_testLegacyFail2804("[4, 5.5, 6]", "5.5",
new TypeReference<List<Integer>>() {});
_testLegacyFail2804("{\"key1\": 4, \"key2\": 5.5}", "5.5",
new TypeReference<Map<String, Integer>>() {});
}

private void _testLegacyFail2804(String value, Class<?> type) throws Exception {
_testLegacyFail2804(value, DEFAULT_MAPPER.constructType(type), value);
}

private void _testLegacyFail2804(String doc, String probValue,
TypeReference<?> type) throws Exception {
_testLegacyFail2804(doc, DEFAULT_MAPPER.constructType(type), probValue);
}

private void _testLegacyFail2804(String doc, JavaType targetType,
String probValue) throws Exception {
try {
READER_LEGACY_FAIL.forType(targetType).readValue(doc);
fail("Should not pass");
} catch (InvalidFormatException ex) {
verifyException(ex, probValue);
} catch (MismatchedInputException ex) {
fail("Should get subtype, got: "+ex);
}
}

/*
/********************************************************
/* Test methods, CoerceConfig, to null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public void setUp() throws Exception

/**
* Unit test to verify translations of
* {@link PropertyNamingStrategy#SNAKE_CASE}
* {@link PropertyNamingStrategies#SNAKE_CASE}
* outside the context of an ObjectMapper.
*/
public void testLowerCaseStrategyStandAlone()
Expand Down Expand Up @@ -265,7 +265,7 @@ public void testLowerCaseUnchangedNames() throws Exception

/**
* Unit test to verify translations of
* {@link PropertyNamingStrategy#UPPER_CAMEL_CASE }
* {@link PropertyNamingStrategies#UPPER_CAMEL_CASE }
* outside the context of an ObjectMapper.
*/
public void testPascalCaseStandAlone()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ public void testConstructorIntrospection()
{
// Need this call to ensure there is a synthetic constructor being generated
// (not really needed otherwise)
@SuppressWarnings("synthetic-access")
Bean1005 bean = new Bean1005(13);
SerializationConfig config = MAPPER.getSerializationConfig();
JavaType t = MAPPER.constructType(bean.getClass());
Expand Down

0 comments on commit 29f7447

Please sign in to comment.