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

Serialising generic value classes via Reference Types (like Optional) fails to include type information #1673

Closed
pwhissell opened this issue Jun 22, 2017 · 20 comments
Milestone

Comments

@pwhissell
Copy link

After going through a couple issues, I came to understand that Jackson always uses declared type of value to figure out whether to include type information. This is done to ensure that type settings are identical between serialization and deserialization.

However isn't that statement false for templated class?. This robustness would be better ensured if the type id settings would be bound to declared types for all untemplated classes but were bound to both declaration and component classes for templated classes. This would imply obtaining the type information at runtime, thus serialisation/deserialisation of templated classes would be a little bit less efficient but that drawback would be negligeable at least in my case.

This would allow for serialisation/deserialisation of Templated classes who's component class are abstract ex. List

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = As.PROPERTY, property = "type")
@JsonSubTypes({
	@JsonSubTypes.Type(name = "ItemADto", value = ItemADto.class)
	@JsonSubTypes.Type(name = "ItemBDto", value = ItemBDto.class)
})
public abstract class AbstractItemDto {...}

@JsonTypeName("ItemADto")
public class ItemADto {...}

@JsonTypeName("ItemBDto")
public class ItemBDto {...}
@cowtowncoder
Copy link
Member

I am not sure I know what "templated" class here means.

@pwhissell
Copy link
Author

@cowtowncoder I meant any generic class or interface that is parameterized over types. More specifically, my problem occurs on instances of classes/interfaces whos GenericType is abstract
example myGenericClass {}
or List

@cowtowncoder
Copy link
Member

@pwhissell Ok.

I guess I am not quite sure what is being asked then. If this is about ability to handle nested generic type information, answer is likely "no, this goes beyond what Jackson can do".

Example itself seems something that should already work, so I am probably missing something: AbstractItemDto is declared to be polymorphic, so all elements within List that contains them will have type information.

@pwhissell
Copy link
Author

pwhissell commented Jun 27, 2017

Given the example
I have a list of item's who's classes are part of a polymorphic hierarchy

List<AbstractItemDto> items = new ArrayList<>();
items.add(new ItemADto());
items.add(new ItemBDto());

When I serialize this list,

ObjectMapper mapper = new ObjectMapper();
String itemsJson = mapper.writeValueAsString(items);

then the type information concerning ItemADto and ItemBDto are not serialized and therefor lost.

/* 
itemsJson == 
"[
    {},
    {}
]"
*/

Since AbstractItemDto specifies the usage of a "type" attribute for JsonTypeInfo
@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = As.PROPERTY, property = "type")

I would expect the serialized string to contain the "type" attribute necessary for the list's deserialization

/* 
itemsJson == 
"[
    {type: "ItemADto"},
    {type: "ItemBDto"}
]"
*/

this would allow me to effectively deserialise itemsJson into a list containing one instance of ItemADto and one instance of ItemBDto

@pwhissell pwhissell changed the title Serialising templated whos component class uses @JsonTypeInfo for its class hiarchy Serialising Generic Classes whos parameterized class is polymorphic and uses @JsonTypeInfo for its class hierarchy Jun 27, 2017
@cowtowncoder
Copy link
Member

@pwhissell Ah. No, it's not because this is not supported, but due to Java Type Erasure. This is a FAQ.
(type erasure meaning that runtime type can not be detected as anything but List<?>)

My main suggestion is really to avoid ever using generic types as root value (that is, value directly passed to ObjectMapper/ObjectWriter). They are fine everywhere else, when reached via property, since generic signature is actually retained in class definition, and thereby accessible.

Instead, either:

  1. Use temporary sub-class that binds type parameters (like class MyList extends ArrayList<Pojo> { }
  2. Use wrapper POJO that has generic type.

But if you really really want to use Lists, Maps or generic POJOs as root values, you need to pass type parameter to work around type erasure:

mapper.writerFor(new TypeReference<List<AbstractDTO.>>() { })
    .writeValueAsBytes(value);

As things are, there may be issues here too, unfortunately, as this forces type for serializer to use AbstractDTO, and not just as base type. This may or may be a problem, depending on exactly where properties for subtypes exist.
There is a filed issue to allow specifying just the base type (distinct from full value types), but it has not been implemented.

@pwhissell
Copy link
Author

@cowtowncoder thank you for your detailed answer.
I understand that type erasure is the fact that the java compiler removes all metadata pertaining to Genericness of a class. In effect the generic parameters are replaced with their bound classes which is, by default Object.class.
So if I have an instance of a generic class:

public MyGenericClass<T> {
    public T genericProperty;
    ...
}

public MyParameterClass {
    ...
}

MyGenericClass<MyParameterClass> myGenericClassInstance = new MyGenericClass<MyParameterClass>();
myGenericClassInstance.genericProperty = new MyParameterClass();

Because of type erasure, the code rather looks something like this after compilation:

public MyGenericClass {
    public Object genericProperty;
    ...
}

public MyParameterClass {
    ...
}

MyGenericClass myGenericClassInstance = new MyGenericClass();

This is why there is no way to use reflection with MyGenericClass to find the parameter class of myGenericClassInstance

ParameterizedType parameterizedType = (ParameterizedType)myGenericClassInstance.getClass().getGenericSuperclass();
parameterizedType.getActualTypeArguments()[0]; // returns the Bound class (in this example it would be java.lang.Object)

That being said, the instance of the property which's type is parameterized (in my case genericProperty) still has metadata pertaining to its own class. And so i could use reflection on that particular instance to get the information I need on the class.

for (Field field : myGenericClassInstance.getClass().getFields()) {
	field.get(myGenericClassInstance).getClass();// this would return the actual Class for the property instance. In this example MyParameterClass
}

This is true for any generic class including List s which can be iterated over to get the information of each object inside. Using my List example from my previous posts one could do something like:

for (Object parameterizedObject : items) {
items.getClass()// this would return ItemADto.class and ItemBDto.class for my list example
// from these classes you can get the superclass information therefore the annotations needed for Jackson's JsonTypeInfo
}

All that being said, would it be possible for jackson to implement such logic in a future version?

@cowtowncoder
Copy link
Member

@pwhissell Jackson does indeed resolve any and all generic type information that is retained by field, method, super-type declarations.

But this is not where the problem occurs: rather, your List does not retain type information. And since Jackson does require homogenous base type (for purpose of generic type handling for elements), this is problematic. It is also a fundamental part of how serialization/deserialization works, and will not be changed (to, for example, try to determine specific polymorphic handling for each and every element -- polymorphoc type handler is created once for the whole Collection/array/Map).

@pwhissell
Copy link
Author

@cowtowncoder I hadn't realized how the problem only occurs in lists and other iterables, this actually fixes half of my problem.

What are the reasons for which the polymorphic type handler is only defined once for Collention/Array/Map/etc. instead of each underliing instance? Is it done for performance reasons?

@cowtowncoder
Copy link
Member

@pwhissell It's due to multiple reasons, including performance but not as the main reason.
I think biggest thing was that common base type really must be known for deserialization and so while it'd be possible for it to vary (or, rather, that no such concept exists) for serialization, things would not work for deserialization when using type name (it would work with class name as type id -- except for "minimal" class); or that definitions could be inconsistent. It just really seemed like proper handling would rely on commonly determined base type (design for polymorphic types took rather long, for Jackson 1.5, but it has been a while and I did not write down reasons -- I'd probably bump into problems I solved back then if I tried to change handling).

Come to think of this through again, I think a real show-stopper would be that there really is no way to reliably figure out polymorphic type handling (@JsonTypeInfo) without starting from some base type. Looking solely at JSON content does not give reliable information -- after all, any and all JSON content, structures, are legal representation of some Java types -- considering that inclusion can be using wrapper array, wrapper object, property of an Object, or even property of parent object.

So I am actually not sure how polymorphic deserialization could be implemented on per-element basis without mandating either additional metadata, or limitation on kinds of type ids that were allowed for Collection, array and Map values.

@pwhissell
Copy link
Author

@cowtowncoder You are absolutely right with a JSON string there is no way of knowing what Type is represented by the JSON object. However is limitation stil applies when polymorphism isn't in play. I'm not sure what the implementation is but deserializing already works with jackson out of the box for example, if I wrap my earlier polymorphic list in a Foo class like so

public class Foo {
    public List<AbstractItemDto> itemList();

    public Foo() {
         list.add(new ItemADto());
         list.add(new ItemBDto());
    }
}

and I force the definition of the type attribute in my earlier classes like so in order to work around the serialisation problem:

public class ItemADto {
    public String type = "ItemADto";
...
}

public class ItemBDto {
    public String type = "ItemBDto";
...
}

then deserialization with jackson works out of the box:

Foo initialFoo = new Foo();
Foo deserializedFoo = null;

ObjectMapper objMapper = new ObjectMapper();
String json = objMapper.writeValueAsString(initialFoo);

deserializedFoo = objMapper.readValue(json, Foo.class); // works well

As for Lists that aren't wrapped in an object, we could use jackson's overloaded implementation of objMapper.readValue and specify a type argument for the destination class like so:

ObjectMapper objMapper = new ObjectMapper();
String jsonList = objMapper.writeValueAsString(items);

CollectionType destinationType = mapper.getTypeFactory().constructCollectionType(List.class, AbstractItemDto.class);
List<MyDto> deserializedItems = mapper.readValue(jsonList, destinationType); // works well

@cowtowncoder
Copy link
Member

I am not sure why you think this is against what I am saying -- your root value is NOT generic type.
It is perfectly fine to use generics that are reached via properties, since generic type info is retained.
It is only problematic for root value where Class is only info available, unless caller explicitly specifies type.

And specifying type does work as you suggested as long as you use it ALSO for serialization -- this is where base type comes into play. Deserialization side is fine.
For serialization it is necessary to use ObjectWriter methods:

mapper.writerFor(typeToUse).writeValue(destination, value);

@brharrington
Copy link

It is perfectly fine to use generics that are reached via properties, since generic type info is retained.

Does it matter how many levels it goes down? For example, if a non generic type has a field that is an AtomicReference to a List of a polymorphic type. Currently that case does not work with 2.9 (FasterXML/jackson-module-scala#346 (comment)).

@cowtowncoder
Copy link
Member

@brharrington Hmmh. I can not actually answer that definitely: nesting of types would work fine for, say, parameterized elements of a Collection or AtomicReference. But it is possible that nesting of types like Collection and AtomicReference, both of which require more elaborate handling, can cause complications.

I think I'd need to see specific case, with Java reproduction. Scala module, further, has its own issues that could be problematic.

@brharrington
Copy link

I think I'd need to see specific case, with Java reproduction.

See this test case: brharrington@1549473

@cowtowncoder
Copy link
Member

Thanks.

@cowtowncoder
Copy link
Member

I can see the failure and yes, looks like it is missing type information it should include.

@cowtowncoder cowtowncoder added this to the 2.9.4 milestone Jan 15, 2018
@cowtowncoder cowtowncoder changed the title Serialising Generic Classes whos parameterized class is polymorphic and uses @JsonTypeInfo for its class hierarchy Serialising generic value classes via Reference Types (like Optional) fails to include type information Jan 15, 2018
@cowtowncoder
Copy link
Member

Ok that took a while from beginning to end: thank you for your patience. In the end it makes sense; one piece in elaborate dance was missing from ReferenceTypeSerializer; something that existed for, say, CollectionSerializer, so I was able to go scavenge and reuse it.

@pwhissell
Copy link
Author

pwhissell commented Jan 16, 2018

thanks guys, I havent had the chance to try the implementation but you juste made my day :)

@david-convey
Copy link

Thank you for your work on this! Very happy to see the jackson-module-scala issue unblocked.

@cowtowncoder
Copy link
Member

:)

I hope patch to Scala module to base ReferenceType serializer/deserializer can now proceed, and solve/prevent many problems... use of Option is so common in Scala that issues are more acute than on Java side.

brharrington added a commit to brharrington/jackson-module-scala that referenced this issue Jan 28, 2018
Fixes FasterXML#346. Note this also update the jackson version
to 2.9.4 as it has a fix that is needed to avoid breaking
other tests. See FasterXML/jackson-databind#1673 for more
details.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants