-
-
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
Allow introspection and polymorphic type handling to use JsonTypeInfo.Value
#3942
Allow introspection and polymorphic type handling to use JsonTypeInfo.Value
#3942
Conversation
src/main/java/com/fasterxml/jackson/databind/introspect/JacksonAnnotationIntrospector.java
Outdated
Show resolved
Hide resolved
Should we deprecate below methods in 2.16? --they will be gone in 3.0.
|
@cowtowncoder may I ask for a brief review? Just to make sure we are heading to the right direction.😄 If I were to take this version further, I will probably just add some test cases. Thank you so much in advance 🙏🏼🙏🏼 |
JsonTypeInfo.Value
.JsonTypeInfo.Value
@@ -1498,27 +1509,30 @@ protected PropertyName _findConstructorName(Annotated a) | |||
protected TypeResolverBuilder<?> _findTypeResolver(MapperConfig<?> config, | |||
Annotated ann, JavaType baseType) | |||
{ | |||
// since 2.16 : backporting {@link JsonTypeInfo.Value} from 3.0 | |||
final AnnotationIntrospector ai = config.getAnnotationIntrospector(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are in AnnotationIntrospector
, should not look for config here (in fact that'd be wrong with AI pair, I think)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. Changed to plan findPolymorphicTypeInfo(config, ann);
👍🏻
* | ||
* @since 2.16 | ||
*/ | ||
public T init(JsonTypeInfo.Value settings, TypeIdResolver res); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would require default implementation, I think, to avoid breakage wrt backwards-compatibility (existing implementations can't really implement it). Not sure if that impl should do nothing or throw an exception...
@@ -256,6 +256,28 @@ public void testUnWrappedMapWithDefaultType() throws Exception{ | |||
assertEquals("{\"@type\":\"HashMap\",\"xxxB\":\"bar\"}", json); | |||
} | |||
|
|||
// [databind#838] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably not the right issue? But also, does this belong here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, should not have copied the issue label 😅.
I was struggling to figure out (I am still 🥲) what should be tested and wrote a sample replacing the old behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. No problem, was just confused :)
Ok, how about we split this further? At first only add new method in And then the meaningful changes can follow. I think XML module ( |
Sounds like a plan👍🏻. I will turn this PR into draft, create a new one with only Thank you for your suggestion @cowtowncoder 🙏🏼🙏🏼 |
src/main/java/com/fasterxml/jackson/databind/jsontype/TypeResolverBuilder.java
Show resolved
Hide resolved
src/main/java/com/fasterxml/jackson/databind/jsontype/TypeResolverBuilder.java
Outdated
Show resolved
Hide resolved
7f116b5
to
57b740e
Compare
/** | ||
* @since 2.16 (backported from Jackson 3.0) | ||
*/ | ||
protected Boolean _requireTypeIdForSubtypes; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this field should be included in this PR, but #3891
…ype (deprecated in 2.7)
…HyukKim/jackson-databind into Introduce-JsonTypeInfo.Value
….com/JooHyukKim/jackson-databind into Introduce-JsonTypeInfo.Value" This reverts commit 93983d3, reversing changes made to c30d8a6.
….com/JooHyukKim/jackson-databind into Introduce-JsonTypeInfo.Value" This reverts commit 93983d3, reversing changes made to c30d8a6.
…HyukKim/jackson-databind into Introduce-JsonTypeInfo.Value
CLOSEDWill close this PR and open #3953 due to severe Git conflicts. |
part of #3943