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

MessageBodyWriter is called with genericType CompletionStage<T> when the resource method returns CompletionStage<T> #4640

Closed
everett1992 opened this issue Nov 19, 2020 · 8 comments · Fixed by #4885
Assignees

Comments

@everett1992
Copy link

After executing a ResourceMethod with return type CompletionStage<T> the MessageBodyWriter is called with an object of type T, a type of T, but a genericType of CompletionStage<T>.

The spec is a little vague on CompletionStage, but I think the genericType should unwrap the CompletionStage and just be T.

3.3.3
Results in an entity body mapped from the class of the returned instance or of its type parameter T if the return type isCompletionStage(see Section 8.2.2); if the class is an anonymous innerclass, its superclass is used instead. If the return value is notnulla 200 status code is used, anullreturn value results in a 204 status code.
https://download.oracle.com/otn-pub/jcp/jaxrs-2_1-final-eval-spec/jaxrs-2_1-final-spec.pdf?AuthParam=1605738947_546c3c96f63881e7441d4b95f55d00e7

The current behavior is incongruous with, say, a ResourceMethod that returns Response - where MessageBodyWriter is called with the Response's entity, the type of it's entity, and the generic type of it's entity. e.g. a method that responds with Resource.ok("test") causes MessageBodyWriter("test", String, String), not ("test", String, Response).

It looks like #3672 describes the same issue but the PR tries to fix it in jackson, when I think the fix should be be made in Jersey if my understanding of the spec is correct. I found the same issue using the GsonMessageBodyProvider from immutables and it seems like a foot gun for any code using the genericType.

@everett1992
Copy link
Author

I think this should be be false when invocableResponseType is assignable from CompletionStage, or invocableResponseType should be unwrapped to T if it is ``CompletionStage`.

https://github.com/eclipse-ee4j/jersey/blob/master/core-server/src/main/java/org/glassfish/jersey/server/model/ResourceMethodInvoker.java#L304-L308

@jansupol
Copy link
Contributor

This looks reasonable, but it changes the behaviour for message body providers. The <CompletionStage> is there for a long time, there would be a provider that expects CompletionStage which would have been broken by the changed behaviour.

There would be needed a configuration option to unwrap the CompletionStage.

@everett1992
Copy link
Author

That makes sense, and I assume the current behavior would have to be the default? What about Jersey 3.0 or a future version of 2.?

@jansupol
Copy link
Contributor

jansupol commented Dec 7, 2020

Yes, for Jersey 2.x the old behaviour should be the default. We already released Jersey 3.0.0, too.

@jansupol
Copy link
Contributor

jansupol commented Dec 7, 2020

We currently go with the following.

@everett1992
Copy link
Author

Any documentation changes I can review or help with?

@everett1992
Copy link
Author

everett1992 commented Dec 8, 2020

This issue seems related #4463 and the root cause seems to be lack of detail in the JAX-RS spec

These are the only relevant references to CompletionStage I could find and they are light on details, especially regarding exceptions.
Resource Method implementation and Resource Method return types

@jansupol
Copy link
Contributor

jansupol commented Dec 20, 2020

We will be able to change the default behaviour for Jakarta RESTful Web Services 3.1. implementation (possibly Jersey 3.1). It would be nice to mention it in the 3.x migration guide, then. Currently, the 3.x guide is under developement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging a pull request may close this issue.

2 participants