-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Deserialization of @JsonTypeInfo
annotated type fails with missing type id even for explicit concrete subtypes
#2968
Comments
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.
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 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. |
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. |
@JsonTypeInfo
annotated type fails with missing type id even for explicit concrete subtypes
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:
|
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 // 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.
I have a HTTP Interface, where the client can request multiple Animals (
Here it is very important to include I also have HTTP endpoints to add a 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.
|
Hi all, any news on this issue? Is marked as most-wanted for version 2.14 but is still open. |
@paulux84 No, unfortunately I haven't had any time to look into this particular issue. PRs welcome; will add label. |
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 |
PR Status
|
…ortable as-is due to refactoring.
Ok, yay -- this is now fixed for 2.15, and will be in the first 2.15.0 release candidate! One future challenge: I was not able to merge the fix cleanly in 3.0 ( |
NOTE: as per #3853 this is OPT-IN for 2.15, so to allow deserialization you HAVE TO enable
so that the behavior is changed. |
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:
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:
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?
The text was updated successfully, but these errors were encountered: