-
-
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
Problem with type specialization for Maps with @JsonDeserialize(as=subtype)
#1215
Comments
I think this is one of bugs fixed (#1186) for 2.7.4, so I think this will be resolved with 2.7.4. If so, the problem was that generic type parameters were not being properly resolved when creating specialized subtype: The only thing to make sure is that you declare
because otherwise type resolution can not properly propagate variable substitutions (earlier versions did not care; they forcible set parameters regardless -- but this is not possible any more). I don't know if it's possible for you to check 2.7.4-SNAPSHOT to verify, but I am fairly confident this is the root cause. |
Ya, I can easily check that out. I'll repost what I find. Thanks for the quick response. |
So I built the 2.7 branch of jackson-core and jackson-databind locally and updated my test project to use 2.7.4-SNAPSHOT and it doesn't fix the problem. I'll see if there's more investigation I can do and maybe create a unit test that fails within this project. I'm not sure how possible that will be because it uses annotations in my project but I can give it a try. |
Also, it appears like the issue is still present in master 2.8.0-SNAPSHOT as well. |
@brentryan thank you for checking this. Too bad problem is not fixed. I'll try to figure it out, probably tomorrow. Any help in diagnosing appreciated. But I thnik information you gave should be sufficient if I need to write a test. |
Ok, re-reading the description in more detail I think a reproduction is needed. It is quite possible that what happens here is more complex than previously added cases of how to specialize type parameters -- before 2.7, handling was much simpler and more naive, which has its problems as well as benefits. |
I'll setup a mini project with an example that reproduces the issue. Probably easier since it combines a few jackson libraries that aren't part of just 1 so unit tests are harder. |
@brentryan sounds good, thank you in advance. |
@cowtowncoder Here's an example project that shows it failing. Change the version to 2.6.6 and you'll see it work, while 2.7.3 fails. |
@brentryan Thank you; I can reproduce this locally, and have a lead to follow. |
@brentryan Ah. Actually, no, I think that the code as is can not work properly, and the reason Jackson 2.6 worked with it was due to a (lucky) flaw. The problem is as follows: class class MyHashMap<K, V extends HasUniqueId<K>>
extends LinkedHashMap<K, V>
implements AnotherMap<K, V> {
...
} which is fine as is, setting bounds. But the creator method is static: @JsonCreator
public static <K, V extends HasUniqueId<K>> MyHashMap<K, V> fromArray(V[] values) {
} and type variables Because of this, method only has bound of So why did this work with Jackson 2.6? Because Jackson type resolution got fooled by declarations and assumed |
Oh. So how to fix the code to work? Well, ideally you would be able to choose local types for creator method to work properly, so that instead of base of abstract @JsonCreator
public static <K, V extends HasUniqueId<K>> MyHashMap<K, V> fromArray(Item[] values) {
MyHashMap<K, V> map = new MyHashMap<>();
for (int i = 0; i < values.length; i++) {
Item v = values[i];
if (v.getId() == null) {
throw new RuntimeException("Failed to get id");
}
if (map.containsKey(v.getId())) {
throw new RuntimeException("Conflict on id");
}
map.put((K) v.getId(), (V) v);
}
return map;
} it's bit nasty what with forced casts, and has to hard-code @JsonCreator(mode=JsonCreator.Mode.DELEGATING) // probably fine without mod too
public MyHashMap(V[] values) {
for (int i = 0; i < values.length; i++) {
V v = values[i];
if (v.getId() == null) {
throw new RuntimeException("Failed to get id");
}
if (containsKey(v.getId())) {
throw new RuntimeException("Conflict on id");
}
put(v.getId(), v);
}
} but seems to have some issues, unlike static version above. I'll see if I can work around this issue. |
Ok. So there definitely is an issue with constructors for this case, so I will leave this issue open for that part. I modified the test case appropriately as well. After spending some time on this, I do think it is related to type refinements; will see if this can be fixed for 2.7.5, or only 2.8. |
@cowtowncoder I haven't tried yet, but was this issue covered in 2.7.5 or 2.8 latest? |
@brentryan issue with constructor type mismatch is not yet resolved, as per test; failing test is included to allow reproduction. |
Odd: first looked like generic type specialization problem; but turns out that |
@JsonDeserialize(as=subtype)
If I have json that looks like
And I have a java pojo with an annotation like this:
Where MyHashMap.java has some custom logic using generics that allow us to map the array json above into a Map where "id" is the key and everything else serializes into the value. We use generics on MyHashMap to enforce that every value implements a certain interface that respects the contract of returning an "id" property. In this example Foo.java implements this interface MyCustomIdInterface.java.
When using 2.6.6 this worked fine, but if I switch to 2.7.x then it breaks with the error:
Can not construct instance of MyCustomIdInterface, problem: abstract types either need to be mapped to concrete types, have custom deserializer, or be instantiated with additional type information
in 2.7.x, it looks like jackson resolves to using AbstractDeserializer based on MyCustomIdInterface but in 2.6.6 it resolves to using BeanDeserializer based on Foo.java.
Is this a bug or is there some default/feature flag that changed here?
The text was updated successfully, but these errors were encountered: