twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 405 forks source link

Deserialisation of nested generic case classes fails #548

Closed aatasiei closed 4 years ago

aatasiei commented 4 years ago

When deserialising generic case classes which, in turn, contain fields with generic case classes, parsing fails with a JsonMappingException.

Expected behavior

import com.twitter.finatra.jackson.ScalaObjectMapper

val mapper = ScalaObjectMapper()

case class B(value: String)
case class A[T](b: T)
val a = mapper.parse[A[B]]("""{"b":{"value":"string"}}""")
// a: A[B] = A(B(string))

case class C[T](a: A[T])
val c = mapper.parse[C[B]]("""{"a":{"b":{"value":"string"}}}""")
// c: C[B] = C(A(B(string)))

Actual behavior

import com.twitter.finatra.jackson.ScalaObjectMapper

val mapper = ScalaObjectMapper()

case class B(value: String)
case class A[T](b: T)
val a = mapper.parse[A[B]]("""{"b":{"value":"string"}}""")
// a: A[B] = A(B(string))

case class C[T](a: A[T])
val c = mapper.parse[C[B]]("""{"a":{"b":{"value":"string"}}}""")
/*
com.fasterxml.jackson.databind.JsonMappingException: Cannot create TypeBindings for class A with 2 type parameters: class expects 1
 at [Source: (String)"{"a":{"b":{"value":"string"}}}"; line: 1, column: 1]
  at com.fasterxml.jackson.databind.JsonMappingException.from(JsonMappingException.java:306)
  at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCache2(DeserializerCache.java:268)
  at com.fasterxml.jackson.databind.deser.DeserializerCache._createAndCacheValueDeserializer(DeserializerCache.java:244)
  at com.fasterxml.jackson.databind.deser.DeserializerCache.findValueDeserializer(DeserializerCache.java:142)
  at com.fasterxml.jackson.databind.DeserializationContext.findRootValueDeserializer(DeserializationContext.java:477)
  at com.fasterxml.jackson.databind.ObjectMapper._findRootDeserializer(ObjectMapper.java:4191)
  at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4010)
  at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3043)
  at com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper.readValue(ScalaObjectMapper.scala:191)
  at com.fasterxml.jackson.module.scala.experimental.ScalaObjectMapper.readValue$(ScalaObjectMapper.scala:190)
  at com.twitter.finatra.jackson.ScalaObjectMapper$Builder$$anon$1.readValue(ScalaObjectMapper.scala:260)
  at com.twitter.finatra.jackson.ScalaObjectMapper.parse(ScalaObjectMapper.scala:608)
  ... 40 elided
Caused by: java.lang.IllegalArgumentException: Cannot create TypeBindings for class A with 2 type parameters: class expects 1
  at com.fasterxml.jackson.databind.type.TypeBindings.create(TypeBindings.java:139)
  at com.fasterxml.jackson.databind.type.TypeBindings.create(TypeBindings.java:98)
  at com.fasterxml.jackson.databind.type.TypeFactory.constructParametricType(TypeFactory.java:984)
  at com.twitter.finatra.jackson.caseclass.Types$.javaType(Types.scala:118)
  at com.twitter.finatra.jackson.caseclass.CaseClassDeserializer.$anonfun$getBeanPropertyDefinitions$2(CaseClassDeserializer.scala:690)
*/

Steps to reproduce the behavior

See above.

Debugging led me to this line in Types.scala:

      val types: Seq[JavaType] =
        for (idx <- 0 to typeBindings.size()) yield typeBindings.getBoundType(idx)
      typeFactory.constructParametricType(scalaType.erasure, types: _*)

It seems that idx goes all the way to typeBindings.size() and that results in always appending an extra null to types.

cacoco commented 4 years ago

Thanks for the issue, we'll see if we can address it.

vkostyukov commented 4 years ago

Hey @aatasiei, thanks for such a thorough bug report that was actually containing a bug fix! You made my job of fixing it very easy: 0a3803ff9ae3e975c4dbe32538f7907fe7f6d9f1

aatasiei commented 4 years ago

@vkostyukov thanks for the prompt fix! 🔝