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

Polymorphic deserialization should handle case-insensitive Type Id property name if MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES is enabled #1983

Closed
soundvibe opened this issue Mar 26, 2018 · 8 comments
Milestone

Comments

@soundvibe
Copy link

Consider this data class:

@JsonTypeInfo(use=JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.EXTERNAL_PROPERTY, property = "operation")
@JsonSubTypes({
        @JsonSubTypes.Type(value = Equal.class, name = "eq"),
        @JsonSubTypes.Type(value = NotEqual.class, name = "noteq"),
})
public abstract class Filter {
    public String operation;
}

MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES is enabled for ObjectMapper:
objectMapper.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES);

Trying to deserialize json { "operation": "EQ"} fails with:
com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id 'eq' into a subtype of...

but deserializing from { "operation": "eq" } works as expected.

It would be great if MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES would also work for this use case.

Tested with Jackson 2.8.0.

@cowtowncoder
Copy link
Member

Hmmh. Interesting. Yes, I think it would make sense. Thank you for reporting this.

@asambol
Copy link

asambol commented May 23, 2019

The same should be taken into account for actual property upper/lower case. In this example, both "operation" and "Operation" should be successfully deserialised. Currently, it doesn't work - we have to choose one.

@cowtowncoder
Copy link
Member

@asambol if MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES is enabled (globally, or as one of features of per-property @JsonFormat), that's how it should work. Is that not the case for you?

@cowtowncoder cowtowncoder added 2.10 and removed 2.9 labels Jun 4, 2019
@asambol
Copy link

asambol commented Jun 4, 2019

@cowtowncoder It's currently not the case. The following code works for lowercase "type" property, but not for "Type". I'm getting mixed case from an API and basically have to choose between uppercase and lowercase + ignoring exceptions for the other.

@JsonTypeInfo(
    use = Id.NAME,
    include = As.PROPERTY,
    property = "type"
)

@cowtowncoder
Copy link
Member

@asambol ah! I see what you mean. Did not read it carefully.

In fact, I think that (virtual) property name should match, but actual type id not necessarily -- that's not what feature actually controls.

At the same type I can see why accepting case-insensitive Type Id value could be useful: I am just not sure how the API should work: I do not like overloading or arbitrarily existing definitions. Type id value is not a property (name).

@cowtowncoder cowtowncoder changed the title Polymorphic deserialization should handle case-insensitive property names if MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES is enabled Polymorphic deserialization should handle case-insensitive Type Id proeprty name if MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES is enabled Oct 6, 2019
@cowtowncoder cowtowncoder changed the title Polymorphic deserialization should handle case-insensitive Type Id proeprty name if MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES is enabled Polymorphic deserialization should handle case-insensitive Type Id property name if MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES is enabled Oct 6, 2019
@cowtowncoder cowtowncoder added the good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project label Oct 6, 2019
@cowtowncoder
Copy link
Member

Note to possible contributors: the good first step would be to add a failing unit test under failing package (so it will not block builds). I can also help anyone who wants to know where handling of Type Id occurs.

If anyone is thinking of tackling this, just add a note here: I am not working on this, and will not do if someone else is. I may eventually have another look if no one works on it, but right now there are many other open issues for me to focus on.

@alevskyi
Copy link
Contributor

alevskyi commented Feb 5, 2020

I would like to pick this task

@cowtowncoder
Copy link
Member

@alevskyi That would be great! I think that this should go in 2.11 branch so may make sense to branch from 2.11 (I can handle merging into master)

alevskyi added a commit to alevskyi/jackson-databind that referenced this issue Feb 7, 2020
alevskyi added a commit to alevskyi/jackson-databind that referenced this issue Feb 7, 2020
@cowtowncoder cowtowncoder added this to the 2.11.0 milestone Feb 17, 2020
cowtowncoder pushed a commit that referenced this issue Feb 17, 2020
@cowtowncoder cowtowncoder added 2.11 and removed 2.10 good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project labels Feb 17, 2020
@cowtowncoder cowtowncoder removed the 2.11 label Apr 12, 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

4 participants