Jackson 2.6: message converter should use type only for collections [SPR-13318]

[spring-projects-issues]

Markus Heiden opened SPR-13318 and commented

All our JSON entities implement an interface which is not annotated with Jackson annotations. Since using Jackson 2.6.0 the code snippet below of AbstractJackson2HttpMessageConverter#writeInternal(Object, Type, HttpOutputMessage) causes javaType always to be detected as the not annotated interface instead of the concrete JSON entity class. And Jackson later on complains about not finding any serializer for it.

if (jackson26Available && type != null && value != null && TypeUtils.isAssignable(type, value.getClass())) {
   javaType = getJavaType(type, null);
}

---

Affects: 4.2 GA

Issue Links:

• DATAREST-664 GET on single resource does not show link for association collection resource
• Spring response handler returning abstract object drops unique fields from concrete class [SPR-13429] #18008 Spring response handler returning abstract object drops unique fields from concrete class
• Full support for Jackson 2.7 [SPR-13483] #18062 Full support for Jackson 2.7
• Jackson 2.6: message converter should use type also for Iterable [SPR-13523] #18100 Jackson 2.6: message converter should use type also for Iterable
• Make @ResponseBody method return type available to message converters [SPR-12811] #17408 Make @ResponseBody method return type available to message converters
• Improve Jackson 2.7 compatibility [SPR-13853] #18426 Improve Jackson 2.7 compatibility

1 votes, 10 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

From a brief look it might be just a matter of passing value.getClass() as the contextClass, i.e. the second argument of getJavaType which is currently null.

[spring-projects-issues]

Oliver Drotbohm commented

This ticket in Spring Boot seems to be running into the same issue. The newly introduced clause in AbstractJackson2HttpMessageConverter causes the return type of the controller method to be favored over the actual value type.

I think the change is suboptimal as it might bring benefit in a very narrow case: en empty collection or map being returned as favoring the return type will allow to reason about the element type in that case which was impossible if only the value type was inspected. However, this completely breaks the rendering for value types being returned for more abstract declared return types, as Jackson will use that reference to look up the serializer to be used and thus not render any properties that only the actual value carries or even fail to find a serializer in the first place (as shown above).

Maybe we should restrict this special behavior the case of collections being returned as I think this is what the ticket the fix was introduced for was originally requesting.

I've created a test case that reproduces the error described in the Boot ticket by showing the additional properties of a Spring HATEOAS PagedResources don't get rendered if the controller method declares Resources as return type. The suggested workaround of handing value.getClass() as the context class unfortunately doesn't work as it only helps to resolve generics on the return type and thus would have to be the invoked controller's type (which I don't think we have access to at this point).

[spring-projects-issues]

Sébastien Deleuze commented

Indeed, I think the best we can do here is to add an additional check and use types only for collection, as detected by Jackson 2.6+ here.

This fix proposal makes Oliver test case green, and is likely to fix this issue reported by Markus.

Are you ok with these changes Oliver Drotbohm and Rossen Stoyanchev?

[spring-projects-issues]

Oliver Drotbohm commented

Thanks for taking the time so quickly, Sébastien. The fix generally looks good to me. I still think passing in value.getClass() for the context type is semantically incorrect, as it would need to be the concrete controller type as Jackson is using it solely to derive generic types. A short summary from our discussion in Slack yesterday:

Oliver Gierke: The context type is used for generics resolution. Jackson seems to try to derive the type variable map from it.
Oliver Gierke: So that you can have a method returning List<T> in a generic class Type<T> and then at runtime use the parameterized type of List<T> with a ConcreteType extends Type<Foo> to interpret List<T> as List<Foo>.

[spring-projects-issues]

Sébastien Deleuze commented

Indeed. I think I found what causes the confusion around this contextClass parameter, its name is misleading since it has the same purpose than the clazz parameter in HttpMessageConverter.canWrite(), it is the source object class, not a context class. This clazz parameter is needed for Jackson for example, since it provides only a ObjectMapper.canSerialize(Class<?>) method. I have added a polish commit to rename the parameter and improve the Javadoc.

I have also updated the original commit in order to contain only the additional javaType.isContainerType() check.

[spring-projects-issues]

Sébastien Deleuze commented

These changes are now in master and will be available in our next 4.2.1.BUILD-SNAPSHOT build (when https://build.spring.io/browse/SPR-PUB-2766 will be completed).

Oliver Drotbohm Markus Heiden Could you please validate that these changes fix your issue(s)?

[spring-projects-issues]

Oliver Drotbohm commented

I can confirm the explicitly introduced integration tests now producing correct serialization results in Spring HATEOAS.

[spring-projects-issues]

Sébastien Deleuze commented

Thanks for your feedback Olie.