jackson-databind
jackson-databind copied to clipboard
FasterXML
Deserialise JSON array where an element is polymorphic
Search before asking
- [X] I searched in the issues and found nothing similar.
Describe the bug
I also rasied this as a SO question so far with no response but I believe it might be a bug.
When I try to deserialise a JSON array with an element containig the type of another element the polymorphic type doesn't seem to be handled correcly. The code can work if you change to use JsonTypeInfo.Id.DEDUCTION again suggesting the issue is in the type selection.
I beleive the source of the issue here is somehow the combination of @JsonFormat(shape = JsonFormat.Shape.ARRAY) with JsonTypeInfo.As.EXTERNAL_PROPERTY and a polymorphic array ellement.
Version Information
2.15.3
Reproduction
import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.annotation.JsonPropertyOrder;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.ObjectMapper;
class Scratch {
private static ObjectMapper objectMapper = new ObjectMapper();
@JsonFormat(shape = JsonFormat.Shape.ARRAY)
@JsonPropertyOrder({"type", "uniqueId", "animal"})
public static class Wrapper {
public String type;
public String uniqueId;
@JsonTypeInfo(
use = JsonTypeInfo.Id.NAME,
include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
property = "type"
)
@JsonSubTypes({
@JsonSubTypes.Type(value = Cat.class, name = "cat")
})
public Animal animal;
}
public interface Animal {
int getLegs();
}
public static class Cat implements Animal {
public int legs = 4;
@Override
public int getLegs() {
return legs;
}
}
public static void main(String[] args) throws Exception {
Wrapper wrapper = new Wrapper();
wrapper.type = "cat";
wrapper.uniqueId = "123";
wrapper.animal = new Cat();
String json = objectMapper.writeValueAsString(wrapper);
System.out.println(json);
Wrapper deserialized = objectMapper.readValue(json, Wrapper.class);
System.out.println(deserialized);
}
}
Expected behavior
Should deserialise correctly (and with more possible types)
Additional context
This example is kind of an extension of JsonTypeInfo.As.WRAPPER_ARRAY where the array also includes other elements in this example uniqueId.
Actualy realised by example might be slightly wrong as im setting wrapper.type = "cat" this should be automatically handled when serializing to JSON so actually maybe im wrong to say the serialisation works correctly.
Would be helpful if the reproduction had failing assertions against the output JSON string or values, instead of sout
Yeah, use of EXTERNAL_PROPERTY is not really supported with "POJO-as-Array" because that will introduce a new main-level property. Ideally an InvalidDefinitionException would be thrown, I think (unless of course we figured out a way to support this setup).
+1 for actual unit test -- test code that requires manual inspection of textual output is not automatable, and it typically ambiguous wrt meaning ("what is it supposed to be again?"). Should just require an assertion or two.
Ok think I understand this issue, i'm basically in an unsupported case, POJO-as-array where one polymorphic elements type is declared in another element. I'm thinking a better solution would be needed, something like an EXTERNAL_ARRAY_PROPERTY. Then I guess it throws up the question of if the type property should actually be a field of the POJO or not? If not declaring the order of POJO-as-array properties might be ambiguous but maybe that can be handled by @JsonPropertyOrder
@jamesmudd Yes, at least currently this is unsupported. I am not 100% sure it couldn't be solved, fwtw, but the whole As.EXTERNAL_PROPERTY is rather tricky to support even without as-array serialization (since Jackson does not create or hold actual in-memory representation of the logical output tree, but writes everything incrementally).
I am not sure that addition of new As.XXX option would help, the fundamental challenge probably remains.
Still, if you or anyone else wants to try to figure it out (and has time & interest), I'd be happy to help with PR. I don't have time to drive it, but do my best to try to help.
Could something like this perhaps be easy to implement? Using a fixed array index instead of a named property?
@JsonTypeInfo(
use = JsonTypeInfo.Id.NAME,
include = JsonTypeInfo.As.EXTERNAL_ARRAY_ITEM,
itemIndex = 2
)
The reason I'm asking is because I've run into the exact same issue. And we have an actual use case for this, namely the implementation of a protocol here called OCPP, which is an officially defined standard in the EV charging industry.
For those who like to know more about this protocol, it can be downloaded freely from https://openchargealliance.org/protocols/open-charge-point-protocol/#OCPP2.0.1
This standard defines certain WebSocket messages called RPC messages, which contain payloads of varying types. The standard mandates that these are in the form of JSON arrays, not objects. Ideally, I'd like to implement proper support for parsing OCPP messages purely with annotations, but this bug or limitation is preventing me from doing so.
The reason why JsonTypeInfo.Id.DEDUCTION won't work here, is because the OCPP standard defines multiple request payload types that have identical property signatures. (Multiple request types that have only a single property with name id and type integer.)
In the case of the RPC CALL type, the payload type is always the third item (index 2) of the incoming JSON array.
(RPC CALLRESULT types don't include a type property, so those will require a custom deserializer regardless, but that's beyond the scope of the issue I'm describing here.)
If anybody knows of another way to solve this problem without have to resort to programming a custom deserializer, I would be happy to read about it. Thanks!
To clarify the problem, I'm sharing a test below that reproduces the issue (with a simplified JSON model that implements a small part of the OCPP 2.0.1 spec). It's written in Kotlin, but that should not be relevant to this issue.
The test testOcppRpcCallJsonArrayDeserialization will fail, whereas the test testOcppRpcCallJsonObjectDeserialization will pass.
import com.fasterxml.jackson.annotation.JsonFormat
import com.fasterxml.jackson.annotation.JsonSubTypes
import com.fasterxml.jackson.annotation.JsonTypeInfo
import com.fasterxml.jackson.core.StreamReadFeature
import com.fasterxml.jackson.databind.json.JsonMapper
import com.fasterxml.jackson.module.kotlin.readValue
import com.fasterxml.jackson.module.kotlin.registerKotlinModule
import kotlin.test.Test
import kotlin.test.assertIs
/**
* Conforms to the OCPP-J 2.0.1 spec, but has the deserialization problem.
*/
@JsonFormat(shape = JsonFormat.Shape.ARRAY)
data class JsonArrayShapedOcppRpcRequest(
val messageTypeId: Int,
val messageId: String,
val action: String,
@JsonTypeInfo(
use = JsonTypeInfo.Id.NAME,
include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
property = "action",
visible = true
)
@JsonSubTypes(
JsonSubTypes.Type(value = BootNotificationRequest::class, name = "BootNotification"),
)
val payload: OcppRequestPayload,
)
/**
* Identical to [JsonArrayShapedOcppRpcRequest], except this variant doesn't have a [JsonFormat] annotation, so it
* defaults to [JsonFormat.Shape.OBJECT], since it's a data class.
*/
data class JsonObjectShapedOcppRpcRequest(
val messageTypeId: Int,
val messageId: String,
val action: String,
@JsonTypeInfo(
use = JsonTypeInfo.Id.NAME,
include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
property = "action",
visible = true
)
@JsonSubTypes(
JsonSubTypes.Type(value = BootNotificationRequest::class, name = "BootNotification"),
)
val payload: OcppRequestPayload,
)
/**
* Polymorphic request payload, common parent type
*/
interface OcppRequestPayload
/**
* One specific child type of request payload (there are many more in the OCPP spec)
*/
data class BootNotificationRequest(
val reason: String,
val chargingStation: BootNotificationChargingStation,
) : OcppRequestPayload
data class BootNotificationChargingStation(
val model: String,
val vendorName: String,
)
class ReproductionTest {
private val objectMapper =
JsonMapper.builder().enable(StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION).build().registerKotlinModule()
/**
* This test reproduces the exact problem: deserialization with a polymorphic field and type defined through
* JsonTypeInfo.As.EXTERNAL_PROPERTY does not work when deserializing JSON arrays.
*/
@Test
fun testOcppRpcCallJsonArrayDeserialization() {
/**
* Example source: OCPP-2.0.1_part4_ocpp-j-specification.pdf, chapter 4.2.1. CALL, page 11/25
* Document can be found at https://openchargealliance.org/protocols/open-charge-point-protocol/#OCPP2.0.1
*/
val incomingRpcRequestJsonArrayString = """
[
2,
"19223201",
"BootNotification",
{
"reason": "PowerUp",
"chargingStation": {
"model": "SingleSocketCharger",
"vendorName": "VendorX"
}
}
]
""".trimIndent()
val deserializedRequest: JsonArrayShapedOcppRpcRequest =
objectMapper.readValue(incomingRpcRequestJsonArrayString)
assertIs<BootNotificationRequest>(deserializedRequest.payload)
}
/**
* This test is just to show that the issue does not occur when deserializing JSON objects.
* It's not useful in practice, since the OCPP-J specification mandates that OCPP RPC messages are in JSON
* array form.
* For more information, see the spec in file OCPP-2.0.1_part4_ocpp-j-specification.pdf, which can be obtained
* at https://openchargealliance.org/protocols/open-charge-point-protocol/#OCPP2.0.1
*/
@Test
fun testOcppRpcCallJsonObjectDeserialization() {
val incomingRpcRequestJsonObjectString = """
{
"messageTypeId": 2,
"messageId": "19223201",
"action": "BootNotification",
"payload": {
"reason": "PowerUp",
"chargingStation": {
"model": "SingleSocketCharger",
"vendorName": "VendorX"
}
}
}
""".trimIndent()
val deserializedRequest: JsonObjectShapedOcppRpcRequest =
objectMapper.readValue(incomingRpcRequestJsonObjectString)
assertIs<BootNotificationRequest>(deserializedRequest.payload)
}
}
@jamesmudd I see that you work at Osprey Charging, so it's not surprising that you ran into the same issue. 😉
Indeed, OCPP was my use case as well. My solution was a custom de-serializer sorry. If this could be resolved that would be great, but i'm actually not really sure its possible the way Jackson works, and is a general problem with OCPP in any JSON deserialisation library. I consider this a poor choice/mistake in the OCPP spec because of this use of an array where an object would have been better IMO. So I actually think this is more of an OCPP bug than Jackson at this point.
So I actually think this is more of an OCPP bug than Jackson at this point.
@jamesmudd so the issue can be closed?
Well I guess technically it could be supported in Jackson, but its not really how its designed ATM. There isn't anything technically invalid with what OCPP have defined, its just a poor design choice IMO. So in that respect, it defines some valid JSON, which Jackson can't deserialize without a custom deserializer. Ideally it would be possible via Jackson annotations IMO but I think this is just too much of an edge case. I guess this is the reason to offer custom deserializers as a workaround for weird cases like this. For this reason I'm not going to work to solve this in Jackson but maybe others feel differently?
Indeed, OCPP was my use case as well. My solution was a custom de-serializer sorry. If this could be resolved that would be great, but i'm actually not really sure its possible the way Jackson works, and is a general problem with OCPP in any JSON deserialisation library. I consider this a poor choice/mistake in the OCPP spec because of this use of an array where an object would have been better IMO. So I actually think this is more of an OCPP bug than Jackson at this point.
@jamesmudd This is actually useful feedback to give to the OCA technical working group. It's probably too late for inclusion in the upcoming OCPP 2.1 spec, but they could perhaps consider it for improvement in future OCPP versions. They have meetings planned around Plugfest in September. Might be worth attending.
Well I guess technically it could be supported in Jackson, but its not really how its designed ATM. There isn't anything technically invalid with what OCPP have defined, its just a poor design choice IMO. So in that respect, it defines some valid JSON, which Jackson can't deserialize without a custom deserializer. Ideally it would be possible via Jackson annotations IMO but I think this is just too much of an edge case. I guess this is the reason to offer custom deserializers as a workaround for weird cases like this. For this reason I'm not going to work to solve this in Jackson but maybe others feel differently?
Fair point. There's also the matter of the CALLRESULT RPC messages, in which OCPP response payloads are wrapped. Those don't have any properties or array items that identify the payload type. For such RPC messages, the payload type has to be contextually determined by looking up the corresponding sent request with the same unique messageId (second array item). That's something that has to be processed programmatically. I don't see how that could ever be done with annotations. And I've been looking into alternatives to Jackson as well, and in my experience, you'll have this challenge with any programming language and/or SerDes framework.
So at best, resolving this specific issue in Jackson would only be useful for one of the three RPC JSON array types in the OCPP-J spec.
That being said, it would be nice if Jackson could be made smarter and more flexible to deal with this stuff. If the type identifier is always present in a JSON array and always at the same array index, and that index comes before the index of the actual polymorphic type, then I see no technical reason why Jackson couldn't intelligently handle this.
At least with a standard such as OCPP, there is at least one real-world use case for this, albeit for just one specific JSON structure in the spec.
@cowtowncoder @JooHyukKim I guess it's up to the Jackson developers/maintainers to decide whether that's worth the effort to fix or implement this.
@volkert-fastned indeed here is a comment from my code
// TODO this isn't great but OCPP is stateful. In order to deserialize the Call Result you
// need to know the message type, you would derive this from the outgoing call ID but that
// means if you didn't send the request you cant parse the response
which you rightly say will prevent OCPP from being fully parsed without custom implementation even if this issue is resolved.
@cowtowncoder @JooHyukKim Should you decide not to fix/implement/improve this, I would advise you to at least have Jackson return a clearer error message in this case, instead of claiming that the identifier property is missing or null, which it isn't. Because that error message was what made this problem so confusing to me.
An error message like JsonFormat.Shape.ARRAY cannot be used with JsonTypeInfo.Id.NAME would prevent this confusion. Thanks!
Yeah, I think supporting EXTERNAL_PROPERTY with "POJO-as-Array" combo would be up to how the discussion goes.
But improving error message sounds great. +1 on @volkert-fastned 's suggestion.
To be honest, I think that trying to actually implement support for such type identifier handling would be quite difficult. So although defining annotation setup to describe intent would be doable, actual wiring would get hairy pretty quickly, from what I understood.
There is no access to full JSON Array value by TypeDeserializer, for example, to extract type id.
And while everything is theoretically possible (i.e. this is not really impossible with enough effort), it seems like a rather big undertaking.
Having said that, if anyone wants to tackle this, I always try to help them (instead of standing in the way :) ) -- "The person who says it cannot be done should not interrupt the person who is doing it."