Refactor CollectionDeserializer (take 2) to solve FasterXML/jackson-dataformats-text#199

Author: cowtowncoder

release-notes/VERSION-2.x

@@ -20,6 +20,7 @@ Project: jackson-databind
   registered custom (de)serializers
 #2707: Improve description included in by `DeserializationContext.handleUnexpectedToken()`
 - Add `BeanDeserializerBase.isCaseInsensitive()`
+- Some refactoring of `CollectionDeserializer` to solve CSV array handling issues
 
 2.11.1 (not yet released)
 

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

@@ -85,18 +85,16 @@ protected Collection<Object> createDefaultInstance(DeserializationContext ctxt)
         return null;
     }
 
+    // NOTE: implementation changed between 2.11 and 2.12
     @Override
-    public Collection<Object> deserialize(JsonParser p, DeserializationContext ctxt,
-            Collection<Object> result0) throws IOException
+    protected Collection<Object> _deserializeFromArray(JsonParser p, DeserializationContext ctxt,
+            Collection<Object> result0)
+        throws IOException
     {
-        if (result0 != null) {
-            return super.deserialize(p, ctxt, result0);
-        }
-        // Ok: must point to START_ARRAY (or equivalent)
-        if (!p.isExpectedStartArrayToken()) {
-            return handleNonArray(p, ctxt, new ArrayBlockingQueue<>(1));
+        if (result0 == null) { // usual case
+            result0 = new ArrayList<>();
         }
-        result0 = super.deserialize(p, ctxt, new ArrayList<>());
+        result0 = super._deserializeFromArray(p, ctxt, result0);
         if (result0.isEmpty()) {
             return new ArrayBlockingQueue<>(1, false);
         }

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

@@ -232,22 +232,21 @@ public Collection<Object> deserialize(JsonParser p, DeserializationContext ctxt)
             return (Collection<Object>) _valueInstantiator.createUsingDelegate(ctxt,
                     _delegateDeserializer.deserialize(p, ctxt));
         }
+        // 16-May-2020, tatu: As per [dataformats-text#199] need to first check for
+        //   possible Array-coercion and only after that String coercion
+        if (p.isExpectedStartArrayToken()) {
+            return _deserializeFromArray(p, ctxt, createDefaultInstance(ctxt));
+        }
         // Empty String may be ok; bit tricky to check, however, since
         // there is also possibility of "auto-wrapping" of single-element arrays.
         // Hence we only accept empty String here.
         if (p.hasToken(JsonToken.VALUE_STRING)) {
-            // 16-May-2020, tatu: As [dataformats-text#199] need to avoid blocking
-            //    check to `isExpectedStartArrayToken()` (needed for CSV in-field array/list logic)
-            // ... alas, trying to do this here leads to 2 unit test regressions so will
-            //   need to figure out safer mechanism.
-//            if (_valueInstantiator.canCreateFromString()) {
-                String str = p.getText();
-                if (str.length() == 0) {
-                    return (Collection<Object>) _valueInstantiator.createFromString(ctxt, str);
-//                }
+            String str = p.getText();
+            if (str.length() == 0) {
+                return (Collection<Object>) _valueInstantiator.createFromString(ctxt, str);
             }
         }
-        return deserialize(p, ctxt, createDefaultInstance(ctxt));
+        return handleNonArray(p, ctxt, createDefaultInstance(ctxt));
     }
 
     /**
@@ -266,9 +265,28 @@ public Collection<Object> deserialize(JsonParser p, DeserializationContext ctxt,
         throws IOException
     {
         // Ok: must point to START_ARRAY (or equivalent)
-        if (!p.isExpectedStartArrayToken()) {
-            return handleNonArray(p, ctxt, result);
+        if (p.isExpectedStartArrayToken()) {
+            return _deserializeFromArray(p, ctxt, result);
         }
+        return handleNonArray(p, ctxt, result);
+    }
+
+    @Override
+    public Object deserializeWithType(JsonParser p, DeserializationContext ctxt,
+            TypeDeserializer typeDeserializer)
+        throws IOException
+    {
+        // In future could check current token... for now this should be enough:
+        return typeDeserializer.deserializeTypedFromArray(p, ctxt);
+    }
+
+    /**
+     * @since 2.12
+     */
+    protected Collection<Object> _deserializeFromArray(JsonParser p, DeserializationContext ctxt,
+            Collection<Object> result)
+        throws IOException
+    {
         // [databind#631]: Assign current value, to be accessible by custom serializers
         p.setCurrentValue(result);
 
@@ -310,15 +328,6 @@ public Collection<Object> deserialize(JsonParser p, DeserializationContext ctxt,
         return result;
     }
 
-    @Override
-    public Object deserializeWithType(JsonParser p, DeserializationContext ctxt,
-            TypeDeserializer typeDeserializer)
-        throws IOException
-    {
-        // In future could check current token... for now this should be enough:
-        return typeDeserializer.deserializeTypedFromArray(p, ctxt);
-    }
-
     /**
      * Helper method called when current token is no START_ARRAY. Will either
      * throw an exception, or try to handle value as if member of implicit