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

Discriminator is not written polymorphic object is assigned to generic property #4422

Open
1 task done
Tillerino opened this issue Mar 8, 2024 · 7 comments
Open
1 task done
Labels
to-evaluate Issue that has been received but not yet evaluated

Comments

@Tillerino
Copy link
Contributor

Tillerino commented Mar 8, 2024

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

Sorry, I struggled to find a good title. So basically, Jackson always writes and requires the discriminator of a polymorphic type P, even if the extending type C of P is known. However, if an instance of C is assigned to a generic property, then the discriminator is not written. This is impossible to describe, just look at the example 😆

Version Information

2.16.1

Reproduction

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "valueType")
@JsonSubTypes({
        @JsonSubTypes.Type(value = PolyParent.PolyChild.class, name = "CHILD")
})
public sealed interface PolyParent {
    record PolyChild() implements PolyParent { }

    record Proper(PolyChild c) { }

    record Generic<T>(T c) { }

    static void main(String[] args) throws JsonProcessingException {
        ObjectMapper jackson = new ObjectMapper();

        // roundtrip works just fine in the standard case
        Proper p = new Proper(new PolyChild());
        String ps = jackson.writeValueAsString(p);
        System.out.println(ps);
        Proper pd = jackson.readValue(ps, new TypeReference<>() {
        });
        System.out.println(pd.equals(p));

        // roundtrip does not work for generic case
        Generic<PolyChild> g = new Generic<>(new PolyChild());
        String gs = jackson.writeValueAsString(g);
        System.out.println(gs);
        // next line crashes
        Generic<PolyChild> gd = jackson.readValue(gs, new TypeReference<>() {
        });
        System.out.println(gd.equals(g));
    }
}

So there is a polymorphic type PolyParent with one child type PolyChild, the type info is set up in a pretty standard fashion.

If PolyChild is a property of a regular object (where the property has the concrete type PolyChild), serialization works as expected - a round trip works just fine.

If an instance of PolyChild is assigned to a generic property, then the serialization will not include the descriminator and deserialization will fail.

Expected behavior

The serialization-deserialization-roundtrip works in both cases.

Additional context

You can see that the output of System.out.println(gs); doesn't contain the discriminator, I guess that's the issue.

@Tillerino Tillerino added the to-evaluate Issue that has been received but not yet evaluated label Mar 8, 2024
@cowtowncoder
Copy link
Member

Couple of things:

  1. What is Proper? (definition missing)
  2. When showing polymorphic (de)serialization issues it is usually best to do round-tripping: serialize, then try to deserialize -- that should work. Just checking for serialization itself is not always indicative of an issue
  3. Please do not include "System.out.println(...) -- see what happens" style step(s) -- there should be an assertion like in test, otherwise I have no idea how I should react to what I see. And to test for discriminator (type id), deserialization will fail if one is expected

@Tillerino
Copy link
Contributor Author

  1. What do you mean? It is included. Line 8.
  2. What do you mean? This is a round-trip-test. It's two round-trip tests!
  3. The code crashes, that's why there's no assertion. (as remarked in the code and in the description)

Can you just run the code, please :D

@Tillerino
Copy link
Contributor Author

This is the same thing without context, just the crash:

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "valueType")
@JsonSubTypes({
        @JsonSubTypes.Type(value = PolyParent.PolyChild.class, name = "CHILD")
})
public sealed interface PolyParent {
    record PolyChild() implements PolyParent { }

    record Generic<T>(T c) { }

    static void main(String[] args) throws JsonProcessingException {
        ObjectMapper jackson = new ObjectMapper();

        String gs = jackson.writeValueAsString(new Generic<PolyChild>(new PolyChild()));
        jackson.readValue(gs, new TypeReference<Generic<PolyChild>>() {
        });
    }
}

To be clear: There is no assertion here, because the code simply crashes.

@cowtowncoder
Copy link
Member

Ah. I did indeed miss inline Proper. My bad.

And you are right, it is indeed round-trip test :)

Looking into this now (without running code yet), this is almost certainly due to good old Java Type Erasure. You cannot use generic types as root values without work-arounds since there is no runtime type information about T.
Work-arounds include:

  1. Avoid root-level generics by adding wrapper classes
  2. Use throw-away subtypes (not possible with Record types)
  3. Force type information with ObjectWriter on serialization (for deserialization you typically already specify target type so there's no problem)

So for (3) you would do something like:

String gs = jackson.writerFor(new TypeReference<Generic<Polychild>>() { })
   writeValueAsString(...)

and that would then include type id.

Without doing so, all Jackson sees is an instace of Generic<?>, due to Type Erasure.

Hope this helps (this is a VFAQ).

@Tillerino
Copy link
Contributor Author

Oh man, I could swear had actually tried (3) - which I wouldn't even consider a workaround. But yeah, it works just fine with the toy example.
I'll go back to my actual thing and try it once again.

Thanks for your quick response!

@cowtowncoder
Copy link
Member

@Tillerino ah no problem & apologies for mis-reading your issue here. I hope it works out.

@Tillerino
Copy link
Contributor Author

Hi there, just to confirm the workaround: Writing with a TypeReference works just fine. Getting this to work was a bit awkward, because I was actually serializing to a JsonNode and the ObjectWriter does not have valueToTree method. If I tried the workaround previously, I can see myself stopping there. Anyway, it works.

I am a little confused about the reason, though. You're saying that type erasure is the issue here. But it looks weirder. I would expect type erasure to lead to the object being serialized as Generic<Object>. But that's not what the output looks like.

In my example above, this doesn't come across - I tried to keep it minimal after all. So consider the following, modified example, where I added a field to the PolyChild class. I renamed the fields to make the output clearer.

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, property = "valueType")
@JsonSubTypes({
        @JsonSubTypes.Type(value = PolyParent.PolyChild.class, name = "CHILD")
})
public sealed interface PolyParent {
    record PolyChild(String polyChildField) implements PolyParent { }

    record Generic<T>(T genericField) { }

    static void main(String[] args) throws JsonProcessingException {
        ObjectMapper jackson = new ObjectMapper();

        String gs = jackson.writeValueAsString(new Generic<>(new PolyChild("S")));
        System.out.println(gs);
        jackson.readValue(gs, new TypeReference<Generic<PolyParent>>() {
        });
    }
}

(The same crash as before, obviously, nevermind that). The output is

{"genericField":{"polyChildField":"S"}}

so clearly the fields of PolyChild are written. But if the output were serialized as Generic<Object>, i.e. the PolyChild instance were serialized as Object, then I would expect its fields to be missing as well. I would expect the following output:

{"genericField":{}}

because I expect Object to be serialized as {}. This seems inconsistent.

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

No branches or pull requests

2 participants