Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EXTERNAL_PROPERTY doesn't work with @JsonIgnoreProperties #2610

Closed
ashlanderr opened this issue Jan 22, 2020 · 7 comments
Closed

EXTERNAL_PROPERTY doesn't work with @JsonIgnoreProperties #2610

ashlanderr opened this issue Jan 22, 2020 · 7 comments
Milestone

Comments

@ashlanderr
Copy link

I am using polymorphic deserialization with Kotlin data classes like this:

@JsonIgnoreProperties(ignoreUnknown = true)
data class Value(
    val type: String,

    @JsonTypeInfo(
        use = JsonTypeInfo.Id.NAME,
        include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
        property = "type",
        defaultImpl = Default::class
    )
    val data: Default
)

object Default

trying to deserialize:

val data = """[{"type": "test","data": {},"additional": {}}]"""

val mapper = ObjectMapper()
    .registerModule(KotlinModule())

val result = mapper.readValue<List<Value>>(data)

and I get this error:

Exception in thread "main" com.fasterxml.jackson.module.kotlin.MissingKotlinParameterException: Instantiation of [simple type, class app.Value] value failed for JSON property type due to missing (therefore NULL) value for creator parameter type which is a non-nullable type
 at [Source: (String)"[{"type": "test","data": {},"additional": {}}]"; line: 1, column: 45] (through reference chain: java.util.ArrayList[1]->app.Value["type"])
	at com.fasterxml.jackson.module.kotlin.KotlinValueInstantiator.createFromObjectWith(KotlinValueInstantiator.kt:112)
	at com.fasterxml.jackson.databind.deser.impl.PropertyBasedCreator.build(PropertyBasedCreator.java:198)
	at com.fasterxml.jackson.databind.deser.impl.ExternalTypeHandler.complete(ExternalTypeHandler.java:316)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeUsingPropertyBasedWithExternalTypeId(BeanDeserializer.java:1001)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeWithExternalTypeId(BeanDeserializer.java:853)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:324)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:194)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:161)
	at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:286)
	at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:245)
	at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:27)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4202)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3205)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3188)
	at app.ApplicationKt.main(Application.kt:52)

If I will remove "additional" property from serialized JSON, add "additional" property to the Value class, or remove polymorphic deserialization, then the error will go away. I am using @JsonIgnoreProperties(ignoreUnknown = true) annotation, so I expected that the original case would work.

Jackson Version: 2.10.2

@cowtowncoder
Copy link
Member

cowtowncoder commented Jan 27, 2020

I think this is Kotlin-specific, as data classes handling of creators, null checks, is quite customized.
Will transfer.

May be transferred back if Java-only reproduction exists (if so let me know and I can move it).

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-databind Jan 27, 2020
@ashlanderr
Copy link
Author

Yes, it is not reproducible with Java. At least with my test code:

@JsonIgnoreProperties(ignoreUnknown = true)
class Value {
    String type;

    @JsonTypeInfo(
            use = JsonTypeInfo.Id.NAME,
            include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
            property = "type",
            defaultImpl = Default.class
    )
    Default data;
}

class Default {}

String data = "[{\"type\": \"test\",\"data\": {},\"additional\": {}}]";
ObjectMapper mapper = new ObjectMapper();
List<Value> result = mapper.readValue(data, new TypeReference<List<Value>>() {});

@cowtowncoder
Copy link
Member

Thank you for verifying: this should help in pinpointing the problem.

@ashlanderr
Copy link
Author

I think the problem is here. This forEach loop reads only fields required for constructor invocation. So if an object contains other fields, then they will not be read, and JsonParser will be at END_OBJECT token. The CollectionDeserializer higher in the call stack (here) don't expect that and tries to read another object.

I'll try to submit a pull request that fixes it.

@ashlanderr
Copy link
Author

I was wrong. It the KotlinValueInstantiator there is no way to read the object to the end. That must happen here, but it is not checking for ignoreAllUnknown value.

@ashlanderr
Copy link
Author

ashlanderr commented Jan 29, 2020

With this new knowledge I was able to reproduce the issue with Java:

@JsonIgnoreProperties(ignoreUnknown = true)
class Value {
    private String type;

    @JsonTypeInfo(
        use = JsonTypeInfo.Id.NAME,
        include = JsonTypeInfo.As.EXTERNAL_PROPERTY,
        property = "type",
        defaultImpl = Default.class
    )
    private Default data;

    @JsonCreator
    public Value(
        @JsonProperty(value = "type", required = true) String type,
        @JsonProperty(value = "data", required = true) Default data
    ) {
        this.type = type;
        this.data = data;
    }
}

class Default {}

String data = "[{\"type\": \"test\",\"data\": {},\"additional\": {}}]";
ObjectMapper mapper = new ObjectMapper();
List<Value> result = mapper.readValue(data, new TypeReference<List<Value>>() {});

The key difference here is to use the constructor for deserialization and require all properties. If properties are not required then the deserializer will put the second empty Value object in the result list, that still is an error.

So it's a Java issue. @cowtowncoder could you transfer it back?

@cowtowncoder
Copy link
Member

@ashlanderr Yes, thank you for reproduction, will transfer.

@cowtowncoder cowtowncoder transferred this issue from FasterXML/jackson-module-kotlin Jan 29, 2020
ashlanderr added a commit to ashlanderr/jackson-databind that referenced this issue Jan 30, 2020
ashlanderr added a commit to ashlanderr/jackson-databind that referenced this issue Jan 30, 2020
@ashlanderr ashlanderr mentioned this issue Jan 31, 2020
@cowtowncoder cowtowncoder added this to the 2.10.3 milestone Feb 1, 2020
cowtowncoder added a commit that referenced this issue Feb 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants