minor changes to go with #2922 (improved exception messages for failed coercion from empty String)

Author: cowtowncoder

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

@@ -248,23 +248,7 @@ public Collection<Object> deserialize(JsonParser p, DeserializationContext ctxt)
         // there is also possibility of "auto-wrapping" of single-element arrays.
         // Hence we only accept empty String here.
         if (p.hasToken(JsonToken.VALUE_STRING)) {
-            // 05-Nov-2020, ckozak: As per [jackson-databind#2922] string values may be handled
-            // using handleNonArray, however empty strings may result in a null or empty collection
-            // depending on configuration.
-            final CoercionAction act = ctxt.findCoercionAction(logicalType(), handledType(),
-                    CoercionInputShape.EmptyString);
-            // 05-Nov-2020, ckozak: Unclear if TryConvert should return the default
-            // conversion (null) or fall through to handleNonArray.
-            if (act != null
-                    // handleNonArray may successfully deserialize the result (if
-                    // ACCEPT_SINGLE_VALUE_AS_ARRAY is enabled, for example) otherwise it
-                    // is capable of failing just as well as _deserializeFromEmptyString.
-                    && act != CoercionAction.Fail
-                    // getValueAsString call is ordered last to avoid unnecessarily building a string value.
-                    && p.getValueAsString().isEmpty()) {
-                return (Collection<Object>) _deserializeFromEmptyString(
-                        p, ctxt, act, handledType(), "empty String (\"\")");
-            }
+            return _deserializeFromString(p, ctxt, p.getText());
         }
         return handleNonArray(p, ctxt, createDefaultInstance(ctxt));
     }
@@ -300,6 +284,41 @@ public Object deserializeWithType(JsonParser p, DeserializationContext ctxt,
         return typeDeserializer.deserializeTypedFromArray(p, ctxt);
     }
 
+    /**
+     * Logic extracted to deal with incoming String value.
+     *
+     * @since 2.12
+     */
+    @SuppressWarnings("unchecked")
+    protected Collection<Object> _deserializeFromString(JsonParser p, DeserializationContext ctxt,
+            String value)
+        throws IOException
+    {
+        final Class<?> rawTargetType = handledType();
+
+        // 05-Nov-2020, ckozak: As per [jackson-databind#2922] string values may be handled
+        // using handleNonArray, however empty strings may result in a null or empty collection
+        // depending on configuration.
+
+        // Start by verifying if we got empty/blank string since accessing
+        // CoercionAction may be costlier than String value we'll almost certainly
+        // need anyway
+        if (value.isEmpty()) { // ... in future may want to allow blank, too?
+            CoercionAction act = ctxt.findCoercionAction(logicalType(), rawTargetType,
+                    CoercionInputShape.EmptyString);
+            act = _checkCoercionFail(ctxt, act, rawTargetType, value,
+                    "empty String (\"\")");
+            if (act != null) {
+                    // handleNonArray may successfully deserialize the result (if
+                    // ACCEPT_SINGLE_VALUE_AS_ARRAY is enabled, for example) otherwise it
+                    // is capable of failing just as well as _deserializeFromEmptyString.
+                return (Collection<Object>) _deserializeFromEmptyString(
+                        p, ctxt, act, rawTargetType, "empty String (\"\")");
+            }
+        }
+        return handleNonArray(p, ctxt, createDefaultInstance(ctxt));
+    }
+
     /**
      * @since 2.12
      */

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

@@ -316,23 +316,32 @@ protected T _deserializeFromString(JsonParser p, DeserializationContext ctxt)
     protected Object _deserializeFromEmptyString(JsonParser p, DeserializationContext ctxt,
             CoercionAction act, Class<?> rawTargetType, String desc) throws IOException
     {
+
         switch (act) {
         case AsEmpty:
             return getEmptyValue(ctxt);
+        case Fail:
+            // This will throw an exception
+            _checkCoercionFail(ctxt, act, rawTargetType, "", "empty String (\"\")");
+            // ... so will never fall through
         case TryConvert:
             // hmmmh... empty or null, typically? Assume "as null" for now
         case AsNull:
+        default:
             return null;
-        case Fail:
-            break;
         }
+
+        // 06-Nov-2020, tatu: This was behavior pre-2.12, giving less useful
+        //    exception
+        /*
         final ValueInstantiator inst = getValueInstantiator();
 
         // 03-Jun-2020, tatu: Should ideally call `handleUnexpectedToken()` instead, but
         //    since this call was already made, use it.
         return ctxt.handleMissingInstantiator(rawTargetType, inst, p,
 "Cannot deserialize value of type %s from %s (no String-argument constructor/factory method; coercion not enabled)",
                 ClassUtil.getTypeDescription(getValueType(ctxt)), desc);
+                */
     }
 
     /**

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

@@ -160,8 +160,11 @@ private void _verifyNoCoercion(JavaType targetType) throws Exception {
             VANILLA_MAPPER.readerFor(targetType).readValue(JSON_EMPTY);
             fail("Should not pass");
         } catch (Exception e) {
-            verifyException(e, "Cannot deserialize value of type");
-            verifyException(e, "from empty String", "from String value (token `JsonToken.VALUE_STRING`)");
+            // 06-Nov-2020, tatu: tests for failure get rather fragile unfortunately,
+            //   but this seems to be what we should be getting
+
+            verifyException(e, "Cannot coerce empty String");
+//            verifyException(e, "Cannot deserialize value of type");
         }
     }
 

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

@@ -181,8 +181,13 @@ private void _verifyFromEmptyFail(ObjectMapper m, String json) throws Exception
 
     private void _verifyFailMessage(JsonProcessingException e)
     {
-        verifyException(e, "Cannot deserialize value of type ");
-        verifyException(e, " from empty String ", " from blank String ");
+        // 06-Nov-2020, tatu: tests for failure get rather fragile unfortunately,
+        //   but this seems to be what we should be getting
+
+        verifyException(e, "Cannot coerce empty String");
+//        verifyException(e, "Cannot deserialize value of type ");
+//        verifyException(e, " from empty String ", " from blank String ");
+
         assertValidLocation(e.getLocation());
     }
 }