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

Null coercion with @JsonSetter does not work with java.lang.Record #2974

Closed
cowtowncoder opened this issue Dec 6, 2020 · 2 comments · Fixed by #3724
Closed

Null coercion with @JsonSetter does not work with java.lang.Record #2974

cowtowncoder opened this issue Dec 6, 2020 · 2 comments · Fixed by #3724
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue Record Issue related to JDK17 java.lang.Record support
Milestone

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 6, 2020

Looks like use of @JsonSetter(nulls=...) does not work with Records in 2.12.0.

Note that earlier report (#2970) turned out to be valid, but since I closed it earlier decided to file a separate one.

cowtowncoder added a commit that referenced this issue Dec 6, 2020
@cowtowncoder cowtowncoder added the has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue label Dec 6, 2020
@cowtowncoder
Copy link
Member Author

Oh boy. This gets deeper and deeper. But long and short of this is that there IS one specific work-around: name your constructor parameters and linkage works. For example:

    record RecordWithNonNullDefs(List<String> names,Map<String, Integer> agesByNames)
    {
        @JsonCreator
        public RecordWithNonNullDefs(
            @JsonProperty("name") @JsonSetter(nulls=Nulls.AS_EMPTY) List<String> names,
            @JsonProperty("agesByNames") @JsonSetter(nulls=Nulls.FAIL) Map<String, Integer> agesByNames)
        {
            this.names = names;
            this.agesByNames = agesByNames;
        }
    }

Not pretty but works. I'll see if this can be fixed more generally but this is the workaround for now.

@cowtowncoder cowtowncoder added 2.13 and removed 2.12 labels Dec 15, 2020
@cowtowncoder
Copy link
Member Author

Ok so the problem is as follows:

  1. During initial POJOPropertiesCollector scan, only explicitly-annotated constructor parameters are located (no attention paid to Creator (constructor/factory) methods themselves, linked to method/field accessors
  2. BasicBeanDescription is constructed
  3. BeanDeserializerFactory (and/or BasicDeserializerFactory) goes over Creator candidates, selects what to included, but does not (can not?) link CreatorProperty instances found using this method; only ones that were already located are

Now: there are couple of ways things could be improved. For example, either

  • Try to link not-explicitly-named properties from step (3), after the fact (also merging annotations)
  • Move Creator discovery to an earlier stage (POJOPropertiesCollector); at least explicitly located ones

of these, first option seems more likely for 2.x; second better overall (and hence for 3.x).
But both are big enough changes that they probably should NOT be attempted for 2.12.x patch release; esp. since there are workarounds.

So marking this as 2.13, unlikely to be attempted for 2.12.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue Record Issue related to JDK17 java.lang.Record support
Projects
None yet
1 participant