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 class with @JsonUnwrapped throwing "Unrecognized field" after upgrade to 2.18 #4792

Open
1 task done
martin-frydl opened this issue Nov 11, 2024 · 9 comments
Open
1 task done
Labels
2.18 to-evaluate Issue that has been received but not yet evaluated

Comments

@martin-frydl
Copy link

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

I have following code:

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonSubTypes.Type;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.annotation.JsonTypeInfo.As;
import com.fasterxml.jackson.annotation.JsonTypeInfo.Id;
import com.fasterxml.jackson.annotation.JsonUnwrapped;
import com.fasterxml.jackson.databind.ObjectMapper;

import static com.fasterxml.jackson.annotation.JsonProperty.Access.READ_ONLY;

class Test {
    @JsonTypeInfo(use = Id.NAME, include = As.PROPERTY, property = "name")
    @JsonSubTypes({
            @Type(value = SubA.class, name = "A"),
    })
    interface Parent {}

    static class SubA implements Parent {
        @JsonUnwrapped
        @JsonProperty(access = READ_ONLY)
        private Model model;

        @JsonCreator
        public SubA(@JsonProperty("model") Model model) {
            this.model = model;
        }
    }

    static class Model {
        @JsonProperty
        private double x;
    }

    public static class Wrapper {
        @JsonProperty
        private Parent model;
    }

    public static void main(String[] args) throws Exception {
        Wrapper w = objectMapper.readValue("{\"model\":{\"name\":\"A\",\"x\":10.0}}", Wrapper.class);
        System.out.println(((SubA)w.model).model.x);
    }
}

When run in 2.17.3, it works fine. However, in 2.18 it started to fail:

Unrecognized field "x" (class UnwrappedTest$SubA), not marked as ignorable (0 known properties: ])

Version Information

2.18.0, 2.18.1

Reproduction

<-- Any of the following

  1. Brief code sample/snippet: include here in preformatted/code section
  2. Longer example stored somewhere else (diff repo, snippet), add a link
  3. Textual explanation: include here
    -->
// Your code here

Expected behavior

No response

Additional context

Could be related to issue #1497 but is a bit different and worked before.

@martin-frydl martin-frydl added the to-evaluate Issue that has been received but not yet evaluated label Nov 11, 2024
@JooHyukKim
Copy link
Member

JooHyukKim commented Nov 11, 2024

Thank you for reporting @martin-frydl 🙏🏼 There is a chance this is regression caused by #4515. As always, will submit failing test against 2.18.

PS

I tried digging into it by comparing 2.17 vs 2.18 version. It seems Creator detection flow changed like this: In 2.17 class SubA uses valueInstantiator with _delegateType and _delegateCreator whereas 2.18 never has delegate such information.

@cowtowncoder cowtowncoder changed the title JsonUnwrapped throwing "Unrecognized field" after upgrade to 2.18 Polymorphic class with @JsonUnwrapped throwing "Unrecognized field" after upgrade to 2.18 Nov 13, 2024
@JooHyukKim
Copy link
Member

JooHyukKim commented Nov 16, 2024

UPDATE : Did some hard digging into the difference made. So..

  • In 2.17, Creator is registered via BasicDeserializerFactory._constructDefaultValueInstantiator() then _handleSingleArgumentCreator(). Later on the creator gets proper delegate deserializer to use.
  • In 2.18, Creator is registered as propertiesBased but is neither implicit or explicit and not used. Also has no delegate deser.
    • Attempt 1 : I tried to modify _addExplicitDelegatingCreators to register PotentialCreators.propertiesBased as explicit on if no other one is registered, but this would fail a large bunch of tests, so nay.

This is taking longer than expected, but getting the hang of it.

@cowtowncoder
Copy link
Member

I wonder if the issue are the annotation combination used:

        @JsonUnwrapped
        @JsonProperty(access = READ_ONLY)
        private Model model;

        @JsonCreator
        public SubA(@JsonProperty("model") Model model) {
            this.model = model;
        }

I am suspicious of READ_ONLY in there; and placement of @JsonUnwrapped.
I am not 100% sure what the intent with READ_ONLY is. But moving @JsonUnwrapped to constructor parameter would seem to make sense ... in what none should be put on Field, I think.

@cowtowncoder
Copy link
Member

@JooHyukKim Single-arg constructor definitely should be considered properties-based (due to @JsonProperty), and explicit (for that plus @JsonCreator).
But it is definitely NOT delegating-one, so should not be added with _addExplicitDelegatingCreators().

@JooHyukKim
Copy link
Member

Btw, changing SubA declaration like below works.

    static class SubA implements Parent {
        private Model model;
        @JsonCreator
        public SubA(@JsonUnwrapped Model model) {
            this.model = model;
        }
    }

I think you are right about ...

I am suspicious of READ_ONLY in there; and placement of @JsonUnwrapped.
I am not 100% sure what the intent with READ_ONLY is. But moving @JsonUnwrapped to constructor parameter would seem to make sense ... in what none should be put on Field, I think.

... placement and @JsonProperty and all.

We can also advise users to follow new usage if the usage is off or intended as long as we provide more documentation, then it would be fine.

@cowtowncoder
Copy link
Member

Ohhhhhhh. Now I get it -- because @JsonUnwrapped won't work until 2.19 (support is added via #1467), that's why DELEGATING is intended for deserialization.
But that can't work with @JsonProperty UNLESS @JsonCreator(mode=Mode.DELEGATING) specified. So that's one thing.

And then @JsonUnwrapped is indeed only meant for serialization.
So handling is asymmetric, if so.

So maybe what is attempted would work better with:

        @JsonUnwrapped
        private Model model;

        // Optionally can use @JsonCreator but not required
        public SubA() { }

if I finally understood intent correctly.

@JooHyukKim
Copy link
Member

So yeah @martin-frydl. What do u think?

@JooHyukKim
Copy link
Member

FYI @cowtowncoder , what I said above about working is in version 2.18. Below declaration succeeds deserialization

    static class SubA implements Parent {
        private Model model;
        @JsonCreator
        public SubA(@JsonUnwrapped Model model) {
            this.model = model;
        }
    }

@JooHyukKim
Copy link
Member

Anyway @martin-frydl if you need "exactly same" declaration with JsonProperty(READ_ONLY) and all in 2.18, you need to modify SubA JsonCreator like below... just add Mode.DELEGATING.

    static class SubA implements Parent {
        @JsonUnwrapped
        @JsonProperty(access = READ_ONLY)
        private Model model;

        @JsonCreator(mode = JsonCreator.Mode.DELEGATING)
        public SubA(@JsonProperty("model") Model model) {
            this.model = model;
        }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.18 to-evaluate Issue that has been received but not yet evaluated
Projects
None yet
Development

No branches or pull requests

3 participants