Skip to content
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

Closed
ypoluektovich opened this issue Jul 4, 2018 · 11 comments
Closed

Option field deserialization failure #382

ypoluektovich opened this issue Jul 4, 2018 · 11 comments
Labels

Comments

@ypoluektovich
Copy link

ypoluektovich commented Jul 4, 2018

Given:

package test
trait T
case class A(a:Int) extends T
case class X(x: Option[T])

When:

    val om = new ObjectMapper with ScalaObjectMapper {
      registerModule(DefaultScalaModule)
      enableDefaultTypingAsProperty(ObjectMapper.DefaultTyping.OBJECT_AND_NON_CONCRETE, "_t")
    }
    val v = X(Some(A(1)))
    val str = om.writeValueAsString(v)
    println(str) // 1
    val d = om.readValue[X](str) // 2
    println(d)

Line 1 prints {"x":{"_t":"test.A","a":1}} and line 2 crashes:

com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id 'test.A' as a subtype of [reference type, class scala.Option<test.T<[simple type, class test.T]>]: Not a subtype
 at [Source: (String)"{"x":{"_t":"test.A","a":1}}"; line: 1, column: 12] (through reference chain: test.X["x"])
	at com.fasterxml.jackson.databind.exc.InvalidTypeIdException.from(InvalidTypeIdException.java:43)
	at com.fasterxml.jackson.databind.DeserializationContext.invalidTypeIdException(DeserializationContext.java:1628)
	at com.fasterxml.jackson.databind.DatabindContext.resolveSubType(DatabindContext.java:200)
	at com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver._typeFromId(ClassNameIdResolver.java:49)
	at com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver.typeFromId(ClassNameIdResolver.java:44)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._findDeserializer(TypeDeserializerBase.java:156)
	at com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer._deserializeTypedForId(AsPropertyTypeDeserializer.java:113)
	at com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer.deserializeTypedFromObject(AsPropertyTypeDeserializer.java:97)
	at com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer.deserializeTypedFromAny(AsPropertyTypeDeserializer.java:193)
	at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.deserializeWithType(OptionDeserializerModule.scala:71)
	at com.fasterxml.jackson.module.scala.deser.OptionDeserializer.deserializeWithType(OptionDeserializerModule.scala:11)
	at com.fasterxml.jackson.databind.deser.SettableBeanProperty.deserialize(SettableBeanProperty.java:527)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeWithErrorWrapping(BeanDeserializer.java:528)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeUsingPropertyBased(BeanDeserializer.java:417)
	at com.fasterxml.jackson.databind.deser.BeanDeserializerBase.deserializeFromObjectUsingNonDefault(BeanDeserializerBase.java:1280)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserializeFromObject(BeanDeserializer.java:326)
	at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:159)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4001)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3030)
	at com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper$class.readValue(ScalaObjectMapper.scala:190)
@ypoluektovich
Copy link
Author

ypoluektovich commented Jul 4, 2018

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
In it, typeDeserializer is a AsPropertyTypeDeserializer that references the Option-typed property. It then goes on to detect the type hint from the actual JSON, and the mismatch between the expected Option[T] and the actual A crashes the program.

@cowtowncoder
Copy link
Member

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.

@ypoluektovich
Copy link
Author

ypoluektovich commented Jul 10, 2018

Yes, it does occur with 2.9.6.

@ypoluektovich
Copy link
Author

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.

@nbauernfeind
Copy link
Member

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).

@cowtowncoder
Copy link
Member

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.
It was added relatively recently (2.7 or 2.8) but does implement things according to current understanding of how things should resolve, including handling of polymorphic types.

@pjfanning
Copy link
Member

@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)?

@cowtowncoder
Copy link
Member

@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 Option.

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 ObjectWriter w = mapper.writerFor(TypeReferenceOrJavaType)), but add to problem.

@pjfanning
Copy link
Member

@milenkovicm that link you made is clearly merged to the 2.13 branch

@pjfanning pjfanning added the 2.13 label Sep 3, 2021
@milenkovicm
Copy link

sorry @pjfanning i've realised that few moments later after i've submitted the comment hence comment removal

@pjfanning
Copy link
Member

in v2.13.0-rc2 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants