Accept JsonTypeInfo.As.WRAPPER_ARRAY with no second argument to deserialize as "null value"

[colltoaction]

Hi!

At RSK we're trying to map Ethereum PUB-SUB JSON-RPC API requests.

We got stuck trying to map the eth_subscribe parameters, here are two examples:

{"id": 1, "method": "eth_subscribe", "params": ["newHeads"]}

{"id": 1, "method": "eth_subscribe", "params": ["logs", {"address": "...", ...]}]}

As you can see, the params type is given by the first parameter, so we thought we could do something like this:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.WRAPPER_ARRAY)
@JsonSubTypes({
        @JsonSubTypes.Type(value = EthSubscribeNewHeadsParams.class, name = "newHeads"),
        @JsonSubTypes.Type(value = EthSubscribeLogsParams.class, name = "logs"),
})
public class EthSubscribeParams {

While everything worked perfectly for logs, the AsArrayTypeDeserializer wasn't able to deserialize newHeads because it doesn't have an object with extra parameters. It fails on:

jackson-databind/src/main/java/com/fasterxml/jackson/databind/jsontype/impl/AsArrayTypeDeserializer.java

Line 110 in bf00c2d

Object value = deser.deserialize(p, ctxt);

com.fasterxml.jackson.databind.JsonMappingException: Can not deserialize instance of co.rsk.rpc.modules.eth.subscribe.EthSubscribeNewHeadsParams out of END_ARRAY token
 at [Source: java.io.ByteArrayInputStream@cb0ed20; line: 1, column: 72] (through reference chain: co.rsk.rpc.modules.eth.subscribe.EthSubscribeRequest["params"])
    at com.fasterxml.jackson.databind.JsonMappingException.from(JsonMappingException.java:270)
    at com.fasterxml.jackson.databind.DeserializationContext.reportMappingException(DeserializationContext.java:1234)
    at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1122)
    at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1075)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:186)
    at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:150)
    at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:116)
    at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromObject(AsArrayTypeDeserializer.java:61)

We then tried to implement DeserializationProblemHandler#handleUnexpectedToken and build the expected instance ourselves, but the workaround doesn't work because the code expects p.nextToken() != JsonToken.END_ARRAY even if the instance was created successfully.

Now we'll try a different approach without JsonTypeInfo.As.WRAPPER_ARRAY, but it would be amazing if this simple case could be handled. I would suggest treating no value as {}.

If you would be interested in me contributing this feature, please advise how I could approach it. I'd be happy to send a PR.

Cheers!

[cowtowncoder]

Hmmh. Interesting. I think I would consider improvement here, and PR would be welcome.

As to value to pass, I don't think { } should be passed, but... hmmh. Tough call: my first instinct would be to pass null (that is, add JsonToken.VALUE_NULL in TokenBuffer to pass), although in theory passing empty buffer could work too -- but result in likely more complex handling later on.
The concern here is simply that it'd be good to be able to optionally distinguish different cases.

But I think that taking

[ "id" ]

to be equivalent to

[ "id", null ]

is reasonable and I'm ok accepting possible down-the-line complications that might arise (esp. as I can't think of what those would be)

[colltoaction]

Thanks for your quick reply and predisposition to include this change!

I tried passing a JsonToken.VALUE_NULL in the buffer, but as the documentation says, the deserialize method doesn't expect it:

jackson-databind/src/main/java/com/fasterxml/jackson/databind/JsonDeserializer.java

Lines 100 to 104 in 55e8d23

	* Pre-condition for this method is that the parser points to the
	* first event that is part of value to deserializer (and which
	* is never JSON 'null' literal, more on this below): for simple
	* types it may be the only value; and for structured types the
	* Object start marker or a FIELD_NAME.

On the other hand, I think calling getNullValue was really concise and clear :). Here's my change set, what do you think?:

https://github.com/FasterXML/jackson-databind/compare/2.10...colltoaction:wrapper_array_one_value?expand=1

I added a couple of tests but I'm not sure if they fit your structure. I also haven't checked for failure cases.

Let me know!

[cowtowncoder]

Good point on null not being typically expected. That is absolutely true!
And you followed this to logical conclusion, I like it. Well done sir!

Only one other thing, then: if we do not yet have a CLA for you (one is enough), we would need one; it can be found here:

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

and usual way is to print, fill & sign, scan, email to info at fasterxml dot com.
With that (and PR of course), I am happy to merge the fix.

[colltoaction]

Thank you, sir!

The CLA is sent, and the PR to the 2.10 branch is here: #2468.