Problems with case class deserializer

[virusdave]

The finatra/jackson case class deserializer is incredibly brittle. Lots of reasonable use cases just plain don't work, and worse, fail with runtime failures.

Examples (apologies if distilling them has resulted in actually-working examples):

Case classes inside 2-layer nested objects

object Outer { object Inner {
  case class Foo(...)
} }

won't work for Foo

Case classes with members that are 2-layer nested case classes

object Outer { object Inner {
  case class Foo(...)
} }
case class Bar(foo: Foo)

won't work for Bar

Case classes with members that are type aliases for non-reflectable classes, even if there's a jackson deserializer for those classes.

type Foo = (something)
/*
  jackson ser/deser modules for Foo exist, are wired in, and work on Foo directly
*/
case class Bar(foo: Foo)

won't work for Bar.

Currently (but obsoleted by PR 386 if accepted): Case classes defined in objects which are themselves companion objects don't work.

Overall, the implementation is extremely brittle. The last example shows this pretty clearly. The somewhat magical use of classnames, being split, attempting to reflectively traverse parent types, etc, is highly likely to only work reliably for exactly the cases for which there are unit tests. The logic touched in 386 to identify classes or parent objects is very likely to fail in many real-world use cases that are present in codebases before a decision to use Finatra, making migration painful. The penultimate example demonstrates that the case class implementation doesn't play nicely with the rest of the bindings in jackson for other types, which is itself severely restrictive.

Basically my experience was that every case i ran into in our codebase that wasn't exactly covered by a unit test scenario broke the system with a runtime exception.

I suggest that a rewrite, that does less introspective reflective magic, would result in much higher reliability.

For example, play's Json library uses (well, allows use of) some compiler macro magic to do compile-time wiring of ser/deserializers. Might this type of strategy be an option?

[ryanoneill]

Thanks for sharing your thoughts @virusdave. I've added a ticket internally for us to discuss the direction we'd like to take with this moving forward.

[ryanb93]

Another example here I've found when trying to use the refined project in case classes.

case class Request(a: Int Refined Positive, b: Int Refined Positive)

Positive is defined within the refined library as:

type Positive = Greater[_0]

Then when Finatra tries to parse the JSON into the case class using

com.twitter.finatra.json.internal.caseclass.reflection.CaseClassSigParser$.parseConstructorParams(CaseClassSigParser.scala:31)

It results in:

java.lang.ClassNotFoundException: eu.timepit.refined.numeric.Positive

[lssilva]

@cacoco this was closed with won't fix or fixed? any update here?