PrimitiveArrayDeserializers should deal with single String value if DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY enabled

[cowtowncoder]

Discussed in #4649

^{Originally posted by eeren-bm July 25, 2024}
I think there's a bit of asymmetry when the lib is used to deserialized into primitive array vs List of objects. Please see the below example

        String longPrim = "2247483647";
        List<String> list1 = List.of("2247483647", "2247483648");
        Long longObj = 2247483647L;
        List<Long> longList = List.of(2247483647L, 2247483648L);

        String longPrimJson = "\"2247483647\"";
        String listStringJson = "[\"2247483647\", \"2247483648\"]";
        String longObjJson = "2247483647";
        String longListJson = "[2247483647, 2247483648]";

        ObjectMapper om = new ObjectMapper();
        om.enable(DeserializationFeature.ACCEPT_SINGLE_VALUE_AS_ARRAY);

//long[] primResult = om.convertValue(longPrim, long[].class);
        long[] primResult2 = om.convertValue(list1, long[].class);
        long[] primResult3 = om.convertValue(longObj, long[].class);
        long[] primResult4 = om.convertValue(longList, long[].class);

//long[] primResultFromJson = om.readValue(longPrimJson, long[].class);
        long[] primResult2FromJson = om.readValue(listStringJson, long[].class);
        long[] primResult3FromJson = om.readValue(longObjJson, long[].class);
        long[] primResult4FromJson = om.readValue(longListJson, long[].class);

        List<Long> longListResult = om.convertValue(longPrim, List.class);
        List<Long> longListResult2 = om.convertValue(list1, List.class);
        List<Long> longListResult3 = om.convertValue(longObj, List.class);
        List<Long> longListResult4 = om.convertValue(longList, List.class);

        List<Long> longListResultFromJson = om.readValue(longPrimJson, List.class);
        List<Long> longListResult2FromJson = om.readValue(listStringJson, List.class);
        List<Long> longListResult3FromJson = om.readValue(longObjJson, List.class);
        List<Long> longListResult4FromJson = om.readValue(longListJson, List.class);

The commented lines are lines I would expect to generate the similar results as List without throwing exceptions.

I believe the simple fix is to remove below lines from PrimitiveArrayDeserializers class' handleNonArray() method

// Empty String can become null...
        if (p.hasToken(JsonToken.VALUE_STRING)) {
            return _deserializeFromString(p, ctxt);
        }

as they would almost certainly throw exception as getValueInstantiator() method is not being overriden to generate a sensible result. And a single string value will be dealt with correctly without above lines.

Thanks for looking ! Please kindly let me know your thoughts

[eeren-bm]

hi, just wondering if this is a sensible request for enhancement? I believe it'd be fine in terms of backwards compatibility too, I am happy to raise a PR if that's ok

[cowtowncoder]

@eeren-bm Yes, I think PR would make sense -- including tests to verify that handling for primitive arrays works as expected.

[cowtowncoder]

Ah. The challenge here is that it's not a single Number to wrap, but a JSON String that happens to contain number.

Hmmmh. If it works for Lists, would make sense for arrays. But needs to also work for non-wrapped case; allow coercion from empty String into null...

@eeren-bm If you are still interested, PR would be welcome.

[eeren-bm]

hi created #5022, hope that's good

[cowtowncoder]

Thank you @eeren-bm ! I added some tweaks but fundamentally we are good to go!

[JooHyukKim]

@cowtowncoder Have we received CLA from our contributor here? Seems new

[cowtowncoder]

@JooHyukKim Thank you for reminder (good catch!).

[cowtowncoder]

@eeren-bm I forgot to ask first, but we do need a CLA. It's 1-page document from:

https://github.com/FasterXML/jackson/blob/master/contributor-agreement.pdf

(just needed once, before merging the first PR -- good for all future contributions)

The usual way is to print it, fill & sign, scan/photo, email to cla at fasterxml dot com.

Apologies for forgetting to ask for this first: but we do need it. I won't revert the PR at this point, I assume we can get this resolved. :)

[JooHyukKim]

@cowtowncoder yeah probably no biggies. If we are not reverting this then let's at least ask and get physical answer?

@eeren-bm Are u okay to cover this merged PR to be in scope with the CLA u would be submitting? No biggies but procedural thing to stay open source :)

[cowtowncoder]

@JooHyukKim what I mean(t) by "not reverting" was just that let's see if we can get CLA first -- without it need to revert, but let's not do that unless we must. So "not revert yet".

[eeren-bm]

thanks, I have sent it across, let me know if it's all good =]

[cowtowncoder]

@eeren-bm Thank you -- I got it, all good

@JooHyukKim As per above we are good. Thank you for reminder!