JohnWeisz/TypedJSON

Feature : type resolver on a class

mullerch opened this issue · 3 comments

Actually, we can solve polymorphic deserialization using a custom typeResolver on the deserializer instance (TyperJSON).

When having nested classes where a root class has a polymorphic level 2 child, the typeResolver has to be added to the deserializer of the root element. You finally get the logic of type info of all the polymorphic children into the root class deserializer.

The idea here is adding a feature so that each class could define it's own typeResolver and that the serializer would then use the typeResolver of each class when deserializing it's childs.

  1. Add a typeResolver property to @JsonObject
  2. On deserialization, get the type of the child, check if it has a custom typeResolver. If yes, use this resolver to find the effective child type
  3. Instantiate the child

This way you can also use those polymorphic class in many different root objects using different deserializers without having the need of specifying the typeResolvers of all those deserializer.

Bonus: with adding a kind of 'id' or 'group' to the typeResolver, one could choose at the deserializer instanciation which typeResolvers to use, giving the flexibility of both approches (the existing one and the proposed one).

Hi @mullerch, this sounds like a nice concept. I implemented this in our 1.6.0-rc2 version, you can give it a spin, and if everything works correctly, I would be releasing that in a couple of days.

This was quick! It looks good, and very simple to use.

Just a last little problem. I would like to define the knownType on the base class (of the polymorphic child), but it looks like it makes an inter-dependency that causes problem at initialization. So actually I have to define the knownType on any class using the base class as child (or globally?). I don't understand the mechanism used below so I'm not sur if or how this can be solved.

Hi @mullerch, I already tried to fix that, but it turns out it is more complicated than it seems. I have created a separate issue to track that #139