Almost completing #2903, add checks for Integer->Enum coercion

Author: cowtowncoder

src/main/java/com/fasterxml/jackson/databind/cfg/CoercionConfigs.java

@@ -191,16 +191,26 @@ public CoercionAction findCoercion(DeserializationConfig config,
         }
 
         // Otherwise there are some legacy features that can provide answer
-        if (inputShape == CoercionInputShape.EmptyArray) {
+        switch (inputShape) {
+        case EmptyArray:
             // Default for setting is false
             return config.isEnabled(DeserializationFeature.ACCEPT_EMPTY_ARRAY_AS_NULL_OBJECT) ?
                     CoercionAction.AsNull : CoercionAction.Fail;
-        }
-        if ((inputShape == CoercionInputShape.Float)
-                && (targetType == LogicalType.Integer)) {
-            // Default for setting in 2.x is true
-            return config.isEnabled(DeserializationFeature.ACCEPT_FLOAT_AS_INT) ?
-                    CoercionAction.TryConvert : CoercionAction.Fail;
+        case Float:
+            if (targetType == LogicalType.Integer) {
+                // Default for setting in 2.x is true
+                return config.isEnabled(DeserializationFeature.ACCEPT_FLOAT_AS_INT) ?
+                        CoercionAction.TryConvert : CoercionAction.Fail;
+            }
+            break;
+        case Integer:
+            if (targetType == LogicalType.Enum) {
+                if (config.isEnabled(DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS)) {
+                    return CoercionAction.Fail;
+                }
+            }
+            break;
+        default:
         }
 
         // classic scalars are numbers, booleans; but date/time also considered

src/main/java/com/fasterxml/jackson/databind/deser/std/EnumDeserializer.java

@@ -9,6 +9,7 @@
 import com.fasterxml.jackson.databind.*;
 import com.fasterxml.jackson.databind.annotation.JacksonStdImpl;
 import com.fasterxml.jackson.databind.cfg.CoercionAction;
+import com.fasterxml.jackson.databind.cfg.CoercionInputShape;
 import com.fasterxml.jackson.databind.deser.ContextualDeserializer;
 import com.fasterxml.jackson.databind.deser.SettableBeanProperty;
 import com.fasterxml.jackson.databind.deser.ValueInstantiator;
@@ -188,26 +189,7 @@ 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)) {
-            // ... unless told not to do that
-            int index = p.getIntValue();
-            if (ctxt.isEnabled(DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS)) {
-                return ctxt.handleWeirdNumberValue(_enumClass(), index,
-                        "not allowed to deserialize Enum value out of number: disable DeserializationConfig.DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS to allow"
-                        );
-            }
-            if (index >= 0 && index < _enumsByIndex.length) {
-                return _enumsByIndex[index];
-            }
-            if ((_enumDefaultValue != null)
-                    && ctxt.isEnabled(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE)) {
-                return _enumDefaultValue;
-            }
-            if (!ctxt.isEnabled(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL)) {
-                return ctxt.handleWeirdNumberValue(_enumClass(), index,
-                        "index value outside legal index range [0..%s]",
-                        _enumsByIndex.length-1);
-            }
-            return null;
+            return _fromInteger(p, ctxt, p.getIntValue());
         }
 
         // 29-Jun-2020, tatu: New! "Scalar from Object" (mostly for XML)
@@ -234,6 +216,51 @@ protected Object _fromString(JsonParser p, DeserializationContext ctxt,
         return result;
     }
 
+    protected Object _fromInteger(JsonParser p, DeserializationContext ctxt,
+            int index)
+        throws IOException
+    {
+        final CoercionAction act = ctxt.findCoercionAction(logicalType(), handledType(),
+                CoercionInputShape.Integer);
+
+        // First, check legacy setting for slightly different message
+        if (act == CoercionAction.Fail) {
+            if (ctxt.isEnabled(DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS)) {
+                return ctxt.handleWeirdNumberValue(_enumClass(), index,
+                        "not allowed to deserialize Enum value out of number: disable DeserializationConfig.DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS to allow"
+                        );
+            }
+            // otherwise this will force failure with new setting
+            _checkCoercionFail(ctxt, act, handledType(), index,
+                    "Integer value ("+index+")");
+        }
+        switch (act) {
+        case AsNull:
+            return null;
+        case AsEmpty:
+            return getEmptyValue(ctxt);
+        case TryConvert:
+        default:
+        }
+        if (index >= 0 && index < _enumsByIndex.length) {
+            return _enumsByIndex[index];
+        }
+        if ((_enumDefaultValue != null)
+                && ctxt.isEnabled(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE)) {
+            return _enumDefaultValue;
+        }
+        if (!ctxt.isEnabled(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_AS_NULL)) {
+            return ctxt.handleWeirdNumberValue(_enumClass(), index,
+                    "index value outside legal index range [0..%s]",
+                    _enumsByIndex.length-1);
+        }
+        return null;
+    }
+        /*
+    return _checkCoercionFail(ctxt, act, rawTargetType, value,
+            "empty String (\"\")");
+            */
+    
     /*
     /**********************************************************
     /* Internal helper methods

src/test/java/com/fasterxml/jackson/databind/BaseMapTest.java

@@ -371,16 +371,4 @@ protected byte[] utf8Bytes(String str) {
             throw new IllegalArgumentException(e);
         }
     }
-
-    protected static String aposToQuotes(String json) {
-        return json.replace("'", "\"");
-    }
-
-    protected static String a2q(String json) {
-        return json.replace("'", "\"");
-    }
-    
-    protected static String quotesToApos(String json) {
-        return json.replace("\"", "'");
-    }
 }

src/test/java/com/fasterxml/jackson/databind/BaseTest.java

@@ -401,7 +401,19 @@ protected byte[] encodeInUTF32BE(String input)
         return result;
     }
 
-    public String quote(String str) {
+    public String q(String str) {
         return '"'+str+'"';
     }
+
+    public String quote(String str) {
+        return q(str);
+    }
+
+    protected static String a2q(String json) {
+        return json.replace("'", "\"");
+    }
+
+    protected static String aposToQuotes(String json) {
+        return a2q(json);
+    }
 }

src/test/java/com/fasterxml/jackson/databind/convert/CoerceEnumTest.java

@@ -76,6 +76,84 @@ public void testEnumFromBlankPhysicalTypeConfig() throws Exception {
     /********************************************************
      */
 
+    public void testEnumFromIntFailLegacy() throws Exception
+    {
+        final ObjectReader r = MAPPER.readerFor(EnumCoerce.class);
+
+        // by default, should be ok
+        EnumCoerce result = r.readValue("1");
+        assertEquals(EnumCoerce.values()[1], result);
+
+        try {
+            r.with(DeserializationFeature.FAIL_ON_NUMBERS_FOR_ENUMS)
+                .readValue("1");
+            fail("Should not pass");
+        } catch (Exception e) {
+            verifyException(e, "not allowed to deserialize Enum value out of");
+        }
+    }
+
+    public void testEnumFromIntAsNull() throws Exception
+    {
+        final String json = "1";
+        ObjectMapper mapper;
+
+        mapper = _globMapper(CoercionInputShape.Integer, CoercionAction.AsNull, false);
+        assertNull(_readEnumPass(mapper, json));
+        mapper = _logMapper(LogicalType.Enum,
+                CoercionInputShape.Integer, CoercionAction.AsNull, false);
+        assertNull(_readEnumPass(mapper, json));
+        mapper = _physMapper(EnumCoerce.class,
+                CoercionInputShape.Integer, CoercionAction.AsNull, false);
+        assertNull(_readEnumPass(mapper, json));
+    }
+
+    public void testEnumFromIntAsEmpty() throws Exception
+    {
+        final String json = "1";
+        ObjectMapper mapper;
+
+        mapper = _globMapper(CoercionInputShape.Integer, CoercionAction.AsEmpty, false);
+        assertEquals(ENUM_DEFAULT, _readEnumPass(mapper, json));
+        mapper = _logMapper(LogicalType.Enum,
+                CoercionInputShape.Integer, CoercionAction.AsEmpty, false);
+        assertEquals(ENUM_DEFAULT, _readEnumPass(mapper, json));
+        mapper = _physMapper(EnumCoerce.class,
+                CoercionInputShape.Integer, CoercionAction.AsEmpty, false);
+        assertEquals(ENUM_DEFAULT, _readEnumPass(mapper, json));
+    }
+
+    public void testEnumFromIntCoerce() throws Exception
+    {
+        final String json = "1";
+        ObjectMapper mapper;
+        EnumCoerce exp = EnumCoerce.B; // entry[1]
+
+        mapper = _globMapper(CoercionInputShape.Integer, CoercionAction.TryConvert, false);
+        assertEquals(exp, _readEnumPass(mapper, json));
+        mapper = _logMapper(LogicalType.Enum,
+                CoercionInputShape.Integer, CoercionAction.TryConvert, false);
+        assertEquals(exp, _readEnumPass(mapper, json));
+        mapper = _physMapper(EnumCoerce.class,
+                CoercionInputShape.Integer, CoercionAction.TryConvert, false);
+        assertEquals(exp, _readEnumPass(mapper, json));
+    }
+
+    public void testEnumFromIntFailCoercionConfig() throws Exception
+    {
+        final String json = "1";
+        ObjectMapper mapper;
+
+        mapper = _globMapper(CoercionInputShape.Integer, CoercionAction.Fail, false);
+        _verifyFromIntegerFail(mapper, json);
+        mapper = _logMapper(LogicalType.Enum,
+                CoercionInputShape.Integer, CoercionAction.Fail, false);
+        _verifyFromIntegerFail(mapper, json);
+        mapper = _physMapper(EnumCoerce.class,
+                CoercionInputShape.Integer, CoercionAction.Fail, false);
+        _verifyFromIntegerFail(mapper, json);
+    }
+
     /*
     /********************************************************
     /* Second-level helper methods
@@ -90,16 +168,16 @@ private void _testEnumFromEmptyGlobalConfig(final CoercionInputShape shape, fina
 
         // First, coerce to null
         mapper = _globMapper(shape, CoercionAction.AsNull, allowEmpty);
-        assertNull(_verifyFromEmptyPass(mapper, json));
+        assertNull(_readEnumPass(mapper, json));
 
         // Then coerce as empty
         mapper = _globMapper(shape, CoercionAction.AsEmpty, allowEmpty);
-        EnumCoerce b = _verifyFromEmptyPass(mapper, json);
+        EnumCoerce b = _readEnumPass(mapper, json);
         assertEquals(ENUM_DEFAULT, b);
 
         // and finally, "try convert", which for Enums is same as "empty" (default)
         mapper = _globMapper(shape, CoercionAction.TryConvert, allowEmpty);
-        assertEquals(ENUM_DEFAULT, _verifyFromEmptyPass(mapper, json));
+        assertEquals(ENUM_DEFAULT, _readEnumPass(mapper, json));
     }
 
     private void _testEnumFromEmptyLogicalTypeConfig(final CoercionInputShape shape, final String json,
@@ -111,17 +189,17 @@ private void _testEnumFromEmptyLogicalTypeConfig(final CoercionInputShape shape,
 
         // First, coerce to null
         mapper = _logMapper(LogicalType.Enum, shape, CoercionAction.AsNull, allowEmpty);
-        b = _verifyFromEmptyPass(mapper, json);
+        b = _readEnumPass(mapper, json);
         assertNull(b);
 
         // Then coerce as empty
         mapper = _logMapper(LogicalType.Enum, shape, CoercionAction.AsEmpty, allowEmpty);
-        b = _verifyFromEmptyPass(mapper, json);
+        b = _readEnumPass(mapper, json);
         assertEquals(ENUM_DEFAULT, b);
 
         // and with TryConvert (for enums same as empty)
         mapper = _logMapper(LogicalType.Enum, shape, CoercionAction.TryConvert, allowEmpty);
-        b = _verifyFromEmptyPass(mapper, json);
+        b = _readEnumPass(mapper, json);
         assertEquals(ENUM_DEFAULT, b);
 
         // But also make fail again with 2-level settings
@@ -142,16 +220,16 @@ private void _testEnumFromEmptyPhysicalTypeConfig(final CoercionInputShape shape
 
         // First, coerce to null
         mapper = _physMapper(EnumCoerce.class, shape, CoercionAction.AsNull, allowEmpty);
-        b = _verifyFromEmptyPass(mapper, json);
+        b = _readEnumPass(mapper, json);
         assertNull(b);
 
         // Then coerce as empty
         mapper = _physMapper(EnumCoerce.class, shape, CoercionAction.AsEmpty, allowEmpty);
-        b = _verifyFromEmptyPass(mapper, json);
+        b = _readEnumPass(mapper, json);
         assertEquals(ENUM_DEFAULT, b);
 
         mapper = _physMapper(EnumCoerce.class, shape, CoercionAction.TryConvert, allowEmpty);
-        b = _verifyFromEmptyPass(mapper, json);
+        b = _readEnumPass(mapper, json);
         assertEquals(ENUM_DEFAULT, b);
 
         // But also make fail again with 2-level settings, with physical having precedence
@@ -201,11 +279,11 @@ private ObjectMapper _physMapper(Class<?> type, CoercionInputShape shape, Coerci
     /********************************************************
      */
 
-    private EnumCoerce _verifyFromEmptyPass(ObjectMapper m, String json) throws Exception {
-        return _verifyFromEmptyPass(m.reader(), json);
+    private EnumCoerce _readEnumPass(ObjectMapper m, String json) throws Exception {
+        return _readEnumPass(m.reader(), json);
     }
 
-    private EnumCoerce _verifyFromEmptyPass(ObjectReader r, String json) throws Exception
+    private EnumCoerce _readEnumPass(ObjectReader r, String json) throws Exception
     {
         return r.forType(EnumCoerce.class)
                 .readValue(json);
@@ -217,14 +295,21 @@ private void _verifyFromEmptyFail(ObjectMapper m, String json) throws Exception
             m.readValue(json, EnumCoerce.class);
             fail("Should not accept Empty/Blank String for Enum with passed settings");
         } catch (MismatchedInputException e) {
-            _verifyFailMessage(e);
+            verifyException(e, "Cannot coerce ");
+            verifyException(e, " empty String ", " blank String ");
+            assertValidLocation(e.getLocation());
         }
     }
 
-    private void _verifyFailMessage(JsonProcessingException e)
+    private void _verifyFromIntegerFail(ObjectMapper m, String json) throws Exception
     {
-        verifyException(e, "Cannot coerce ");
-        verifyException(e, " empty String ", " blank String ");
-        assertValidLocation(e.getLocation());
+        try {
+            m.readValue(json, EnumCoerce.class);
+            fail("Should not accept Empty/Blank String for Enum with passed settings");
+        } catch (MismatchedInputException e) {
+            verifyException(e, "Cannot coerce Integer value (");
+            verifyException(e, "but could if coercion was enabled");
+            assertValidLocation(e.getLocation());
+        }
     }
 }

src/test/java/com/fasterxml/jackson/databind/jsonschema/TestGenerateJsonSchema.java

@@ -247,4 +247,8 @@ public void testNumberTypes()  throws Exception
                 +"'bigInt':{'type':'integer'}}}";
         assertEquals(EXP, json);
     }
+
+    protected static String quotesToApos(String json) {
+        return json.replace("\"", "'");
+    }
 }