-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
Option field deserialization failure #382
Comments
I've traced the error to this line: https://github.com/FasterXML/jackson-module-scala/blob/master/src/main/scala/com/fasterxml/jackson/module/scala/deser/OptionDeserializerModule.scala#L71 |
I don't have any information to add, but wanted to make sure this occurs with the latest release (2.9.6); or if not, that version it occurs with is included here. That helps verify the problem. |
Yes, it does occur with 2.9.6. |
A quick note: instead of a quick-and-hacky fix like in my patch linked above, it might be a better idea to rewrite OptionDeserializer to extend ReferenceTypeDeserializer (similar to OptionSerializer that extends ReferenceTypeSerializer), which seems to include some handling for this kind of issue. |
Ah, interesting idea. I am going to get to this before the next jackson release or the scala-lang 2.13.0-M5 release (whichever comes first). Thanks for the patch and extra thanks for the test. I want to look a little more closely at the before/after effects before merging (I just want to be certain this only affects the default typing feature or to see if there are other tests that should be added). |
Yes, ReferenceType[De]Serializer is designed to be used as the skeletal implementation here; it is the basis for Java 8 Optional, Guava Optional, JDK AtomicReference handling, and I think Kotlin's equivalent. |
@cowtowncoder this is still an issue - would you have a suggestion about how com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer could be told about the Option wrapper class as well as the inner class (Int)? |
@pjfanning I think the way databind at least sees reference types is to consider them mostly invisibile, so that type info only ever relates to value contained. Or put another way: serialization of contents (polymorphic typing or no) should be same as serialization of one wrapped in But maybe the problem here is due to Type Erasure: it is generally bad idea to use generic types as root values -- that can be made to work (but user MUST provide type via |
@milenkovicm that link you made is clearly merged to the 2.13 branch |
sorry @pjfanning i've realised that few moments later after i've submitted the comment hence comment removal |
in v2.13.0-rc2 release |
Given:
When:
Line
1
prints{"x":{"_t":"test.A","a":1}}
and line2
crashes:The text was updated successfully, but these errors were encountered: