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

Make @ResponseBody method return type available to message converters [SPR-12811] #17408

Closed
spring-projects-issues opened this issue Mar 12, 2015 · 8 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 12, 2015

Miroslav Senaj opened SPR-12811 and commented

In Spring MVC I have following object hierarchy.

@JsonTypeInfo( use = JsonTypeInfo.Id.NAME, 
     include = JsonTypeInfo.As.PROPERTY, 
     property = "type") 
public abstract class ParentClass {
}
@JsonTypeName("foo") 
public class Foo extends ParentClass {
    private String person;
}
@JsonTypeName("bar") 
public class Bar extends ParentClass {
    private String item;
}

ACTUAL:
When I generate response from method as List<ParentClass> it generates JSON without "type" : "foo" or "type" : "bar".

@ResponseBody
public List<ParentClass> method() {
    List<ParentClass> result =  // generate classes foo and bar
    return result;
}
[{ "person" : "John Doe" }, { "item" : "rifle" }]

EXPECTED:
In List<T> response type it should generate "type" as well.

@ResponseBody
public List<ParentClass> method() {
    List<ParentClass> result =  // generate classes foo and bar
    return result;
}
[{ "type" : "foo", "person" : "John Doe" }, { "type" : "bar", "item" : "rifle" }]

TRIVIA:
When I return from @Controller in @ResponseBody object which contains List<ParentClass> as

public class AnotherClass { 
    public List<ParentClass> values; 
}

It generates proper response with proper "type" in it.

@ResponseBody
public AnotherClass method() {
    AnotherClass result =  // generate Another class and list of foo and bar
    return result;
}
{ "values" :[
     { "type" : "foo", "person" : "John Doe" }, 
     { "type" : "bar", "item" : "rifle" } ] 
}

See also stackoverflow issue

What I found so far that in
org.springframework.http.converter.json.AbstractJackson2HttpMessageConverter:228
this.objectMapper.writerWithView(serializationView).writeValue(generator, value);

Is serializing data without its type and base on this topic it should have writerWithType in order to write type.


Affects: 4.1.1

Issue Links:

Referenced from: commits 31a5434, 3bff7bd

1 votes, 6 watchers

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Sébastien Deleuze, I suppose we could change the else block to:

this.objectMapper.writerWithType(value.getClass()).writeValue(generator, value);

Would that have any negative side effects? Should it rather be conditional?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Hmm, this seems to be what Jackson is doing without a writerWithType call anyway... Where does Jackson evaluate @JsonTypeInfo? Do we have to manually pass this in?

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Miroslav Senaj commented

This is strange because If I manually convert my List of objects using ObjectMapper like:

List<ParentClass> result = // generate foo and bar
ObjectMapper mapper = new ObjectMapper();
mapper.writerWithType(new TypeReference<List<ParentClass>>() {}).writeValue(response.getOutputStream(), result);

It generates correct output with "type" in it.
This indicates that Jackson is not doing writeWithType if you have not specified it.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

I am not sure this is an issue on Spring side.

When using List<ParentClass> return type, Jackson just see a List<Object> due to type erasure, and a BeanSerializer is used to write Foo and Bar and it seems to not support writing the type info.

When using a wrapper like AnotherClass, Jackson see a List<ParentClass>, an AsPropertyTypeSerializer that supports writing the type info is used.

Maybe we could create an issue on jackson-databind bugtracker, asking to improve Jackson BeanSerializer to write the type information based on each List element type ? This could have some performance impact, but it could be enabled by a Jackson feature.

Please also notice that Jackson bugtracker contains a lot of issues related to TypeInfo.

Any thoughts ?

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

A framework cannot use the TypeReference approach, unfortunately, since that little trick is effectively based on a user-declared inline subclass for each specific type, which we cannot mimic in generic framework code.

I suppose we could introspect the @ResponseBody-declaring method's return type and try to pass a corresponding JavaType to Jackson, assuming that the method declaration preserves generic type information which the returned object would otherwise lose. From my understanding, that is the only approach we could actually implement.

In any case, this is a rather involved improvement then, so I'm turning this into a 4.2-only issue along those lines.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

I suppose we could introspect the @ResponseBody-declaring method's return type and try to pass a corresponding JavaType to Jackson, assuming that the method declaration preserves generic type information which the returned object would otherwise lose. From my understanding, that is the only approach we could actually implement.

Note that we support this on the reading side, see GenericHttpMessageConverter, since otherwise Jackson would have no way of knowing how to read the body to something like List<Foo>. We didn't do the same for the writing side where the starting point is an actual object and the assumption is it should be possible to obtain all necessary information. Perhaps we can extend GenericHttpMessageConverter to include the writing side. Otherwise Jackson would have to do something to look at the List elements.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 22, 2015

Sébastien Deleuze commented

This commit adds canWrite() and write() methods to the
GenericHttpMessageConverter interface. These are type aware variants
of the methods available in HttpMessageConverter, in order to keep
parametrized type information when serializing objects.

AbstractMessageConverterMethodProcessor now calls those type aware
methods when the message converter implements GenericHttpMessageConverter.

AbstractJackson2HttpMessageConverter and GsonHttpMessageConverter uses
these new methods to make @ResponseBody method return type available
for type resolution instead of just letting the JSON serializer trying
to guess the type to use from the object to serialize.

See also #17745 for the same improvement applied to request body serialization.

@spring-projects-issues
Copy link
Collaborator Author

Sébastien Deleuze commented

This commit enables serialization with type only with Jackson 2.6+ since previous versions do not serialize polymorphic collections correctly when the type is specified. See FasterXML/jackson-databind#727 for the related Jackson fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants