jackson-databind

jackson-databind copied to clipboard

When attempting to deserialize to a concrete class that is part of a polymorphic type hierarchy, an InvalidTypeIdException is thrown if the JSON does not contain the type id field. Example:

@JsonTypeInfo(
    use = JsonTypeInfo.Id.NAME,
    property = "type")
@JsonSubTypes({
    @JsonSubTypes.Type(value = Foo.class, name = "foo"),
    @JsonSubTypes.Type(value = Bar.class, name = "bar")})
public interface Base {}

public class Foo implements Base {}

public class Bar implements Base {}

ObjectMapper mapper = new ObjectMapper();

mapper.readerFor(Foo.class).readValue("{}"); // throws InvalidTypeIdException
mapper.readValue("{}", Foo.class); // throws InvalidTypeIdException

While I understand why this happens, as Jackson is finding the JsonTypeInfo / JsonSubTypes annotations on the interface, it is counterintuitive to me. In this instance, I am instructing the mapper as to the specific concrete class to deserialize to, so consulting those annotations seems unnecessary. Perhaps checking if the class / type supplied to readerFor / readValue matches exactly one of the classes listed in JsonSubType could be a fallback if the type id property is not found?

So far, the only workaround I've found is to do something like this:

@JsonTypeInfo(use = JsonTypeInfo.Id.NONE)
public class Foo implements Base {}

but then that means serializing a Foo instance would not get the type id property. Perhaps a custom TypeIdResolver or SubTypeResolver could also be used, but having the described behavior baked in seems like a sensible default to me. Thoughts?

I'd be interested in more about the particular use case for this, but setting that aside for the moment, I could possibly see one benefit, of a small performance gain, i.e. bypass the annotations check if there's nothing supplied.

Perhaps checking if the class / type supplied to readerFor / readValue matches exactly one of the classes listed in JsonSubType could be a fallback if the type id property is not found?

Assuming this is a reasonable improvement, given the above, do you have a PR to show your thinking on this? Also (going back to the use case), not clear on how big of a hit this would be, i.e. how often would this apply? This seems to be a very narrow scenario.

Ok, I think there already an issue for improving handling in this case (will see if I can find it). It would be nice to make type id optional for concrete case but it seems rather difficult to make work reliably due to delegating nature of things.

Here's an example. We have a tool that generates Java / Kotlin classes from GraphQL schemas, so given an example trivial schema:

interface Animal {
  name: String
}

type Dog implements Animal {
  name: String
  barkVolume: Int
}
        
type Cat implements Animal {
  name: String
  lives: Int
}

type Query {
  animals: [Animal]
  dogs: [Dog]
  cats: [Cat]
}
       

We might generate the following for the Animal interface so that query for animals works correctly:

@JsonTypeInfo(
    use = JsonTypeInfo.Id.NAME,
    include = JsonTypeInfo.As.PROPERTY,
    property = "__typename"
)
@JsonSubTypes({
    @JsonSubTypes.Type(value = Dog.class, name = "Dog"),
    @JsonSubTypes.Type(value = Cat.class, name = "Cat")
})
public interface Animal {
  String getName();
}

Now when queries are also built using our codegen tool, we can always arrange for the __typename meta field to be returned, but some users may opt to only use the generated types and build their own queries by hand. To us the generated class even when querying for a specific type like Dog, they need to remember to also always ask for __typename, which seems a little strange to have to do if they are indicating at time of deserialization they know the concrete type based on the query they're performing.

I can also think of non-GraphQL scenarios, where the client is using generated code and doing similar queries for REST endpoints like /animals, /dogs, and the server component is perhaps not using the same classes at all or is written in a different language. Yes, the /dogs endpoint could return JSON with type ids, but it seems redundant to have to do that when the client is specifically asking for a "concrete type" in this case.

Assuming this is a reasonable improvement, given the above, do you have a PR to show your thinking on this? Also (going back to the use case), not clear on how big of a hit this would be, i.e. how often would this apply? This seems to be a very narrow scenario.

I have not implemented a fix for this yet, but did begin digging around to see if it's possible. I tried to give some more context on the use case in my previous comment.

Could not find clear example, but #2063 is similar.

I would be open to a PR that would accept missing type id in case of concrete subtype being requested, and have been planning on working on making this possible (but it did not make it in 2.12).

@kupci Annotation processing would not be optimized: they are checked when constructing (de)serializers, once, and never during runtime.

Since I cannot quite find a more up-to-date issue, I will mark this one as "most-wanted" one -- and may also close older ones that seem like they would be solved with this one.

Another related use case may be the following:

@JsonTypeInfo(
    use = JsonTypeInfo.Id.NAME,
    include = JsonTypeInfo.As.PROPERTY)
abstract class MyGeneralDomainObject {
    String id; //property
}

class A extends MyGeneralDomainObject {
    MyGeneralDomainObject obj; //here, we need type information
}

class B extends MyGeneralDomainObject {
    A propertyWithExplicitType; //here, we know the type is A, so if A doesn't have subclasses, why should we require type information in the JSON?
}

I would also find this feature very useful.

I want to share a workaround which seems to work fine in my case: to annotate the subclasses with the same @JsonTypeInfo, but including a defaultImpl. For instance:

// Top-level interface

@JsonTypeInfo(
    use = JsonTypeInfo.Id.NAME,
    include = JsonTypeInfo.As.PROPERTY,
    property = "__typename"
)
@JsonSubTypes({
    @JsonSubTypes.Type(value = Dog.class, name = "Dog"),
    @JsonSubTypes.Type(value = Cat.class, name = "Cat")
})
public interface Animal {
  String getName();
}

// Subclasses

@JsonTypeInfo(
    use = JsonTypeInfo.Id.NAME,
    include = JsonTypeInfo.As.PROPERTY,
    property = "__typename",
    defaultImpl = Dog.class
)
@JsonSubTypes({
    @JsonSubTypes.Type(value = Dog.class, name = "Dog")
})
public class Dog implements Animal {
    // ...
}

@JsonTypeInfo(
    use = JsonTypeInfo.Id.NAME,
    include = JsonTypeInfo.As.PROPERTY,
    property = "__typename",
    defaultImpl = Cat.class
)
@JsonSubTypes({
    @JsonSubTypes.Type(value = Cat.class, name = "Cat")
})
public class Cat implements Animal {
   // ...
}

It is not exactly pretty, but it seems to be working as expected.

This feature would also help me a lot.

Let me shortly explain my use case. We have the interface Animal and two implementing classes Cat and Dog.

@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, property = "_class")
public abstract class Animal {
  public String name;
}

public class Cat implements Animal {}

public class Dog implements Animal {}

I have a HTTP Interface, where the client can request multiple Animals (GET /animals), which would result in a response like this:

[
{"_class": "Cat.class", "name": "foo"},
{"_class": "Cat.class", "name": "bar"},
{"_class": "Dog.class", "name": "bark"},
]

Here it is very important to include _class, because only with this the client can distinguish the two classes.

I also have HTTP endpoints to add a Cat (POST /cat), and one to add a Dog (POST /dog). When using the POST /cat endpoint, I do not need to know about the _class attribute, because I expect the sender to send a Cat. I want the client to be able to send the body {"name": "meow"} when doing the POST /cat request, instead of having to send {"_class": "Cat.class", "name": "meow"}

As a workaround, I currently do this (thanks to the comment of @wetneb ), which is not the end of the world for me, but I definitly do not like it.

@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, property = "_class")
public abstract class Animal {
  public String name;
}

@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, property = "_class", defaultImpl = Cat.class)
public class Cat implements Animal {}

@JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, property = "_class", defaultImpl = Dog.class)
public class Dog implements Animal {}

Hi all, any news on this issue? Is marked as most-wanted for version 2.14 but is still open. Thanks

@paulux84 No, unfortunately I haven't had any time to look into this particular issue. PRs welcome; will add label.

• #3803

I started working on this issue on PR #3803 But it turns out there is quite a number of tests proving otherwise, testing that even deserialization of value as explicit concrete subtype should fail.

So to prevent myself from modifying loads of tests just to abort 😅, may I ask you to check if the change at PR #3803 is viable, @cowtowncoder ? 🙏🏼

@JooHyukKim Unfortunately I don't think that approach is correct; determination cannot rely on only abstract types using TypeDeserializers -- intermediate concrete (or even base) types are perfectly legal for polymorphic handling.

PR Status

• #3803
• Sharing here just In case anybody is interested in reviewing the solution with us. 🙏🏼 Let's solve this issue together
• LATEST STATUS : waiting for PR review.
• ~~About the third, attempt (currently the last), seems hopeful, but seemingly with performance issue.~~
  • ~~more details below.~~
  • Fix. PR #3804 did solve the problem I thought to be performance issue like below. ✌️

~~About the third attempt~~

• ~~Took the approach of checking whether deserialization target class has TypeResolver attached to it, before throwing InvalidTypeIdException.~~
• ~~About last solution, passes all tests only "locally" but fails on the CI 🥲🥲. (Below photos are the CI error log)~~
• ~~LATEST UPDATE : current problem might be fixed by #3804 . I am waiting for the mentioned PR to be merged. Then I will try again.~~

~~Further plan~~

1. ~~Check if we can fix issue with the third attempt.~~
2. ~~If failed, try setting deserializer closer to the initialization phase.~~

Ok, yay -- this is now fixed for 2.15, and will be in the first 2.15.0 release candidate! Big Thank You to @JooHyukKim for the fix!

One future challenge: I was not able to merge the fix cleanly in 3.0 (master) -- handling of polymorphic type resolution was refactored significantly and I could not see simple 1-to-1 translation. For now, then, tests wrt this issue are under failing in master. I will create a new follow-up issue for that.

NOTE: as per #3853 this is OPT-IN for 2.15, so to allow deserialization you HAVE TO enable

MapperFeature.REQUIRE_TYPE_ID_FOR_SUBTYPES

so that the behavior is changed.