Support RunOnVirtualThread annotations on beans implementing a JAX-RS interface #45202
Conversation
danielbobbert
force-pushed
the
main
branch
4 times, most recently
from
0552b70 to
862e204
Compare
There was a problem hiding this comment.
Thanks for this PR! I will let @cescoffier check this PR but I asked for a small change to improve clarity.
|
|
||
| private boolean testMethod(MethodInfo method) { |
There was a problem hiding this comment.
Could we rename it isJaxrsResourceMethod() or something similar?
There was a problem hiding this comment.
Sure, can do. @cescoffier Any other comments from your side?
Besides that, I was wondering about the "shortcut" that was already present in the original code: If there is a @Path annotation on the class, all methods pass the test. But a JAX-RS could contain JAX-RS methods (annotated with @GET etc.) and also some (un-annotated) "helper" methods that are used internally by the JAX-RS methods. Now, @RunOnVirtualThread only makes sense on the JAX-RS methods and not on the helper methods, so I was wondering what the reasoning of the "shortcut" was and whether it is actually valid?
On the other hand, I don't know how "exact" this whole test needs to be. It seems to be checking method annotations only (and not class annotations) at this time anyways, so maybe it's "close enough" to prevent people from putting @RunOnVirtualThread in absurd places?!
There was a problem hiding this comment.
I have renamed the method and rebased the branch
This comment has been minimized.
This comment has been minimized.
danielbobbert
force-pushed
the
main
branch
5 times, most recently
from
136d539 to
fedbd80
Compare
danielbobbert
force-pushed
the
main
branch
2 times, most recently
from
2fa906a to
c5a2d35
Compare
|
Thanks, looks good to me! About your comment, @geoand any idea why we have this shortcut? |
Status for workflow
|
|
No pressure, but the question about the "shortcut" is of general nature and independent from my actual PR. So I would really love to see this merged, since the bug heavily blocks our development right now. |
Unfortunately not |
|
Is there anything else I could do to help get this merged? |
Fixes: #45186
This PR is about support for beans that implement a JAX-RS interface.
It does four things:
EndpointIndexersuch that explicit annotations on the bean overwrite the defaultBlockingJaxRsMethodProcessorsuch that theExecutionModelAnnotationsProcessorwill not complain about RunOnVirtualThread annotations on the beanResteasyReactiveScannerto allow @RunOnVirualThread annotations on the JAX-RS Application to set the defaultBlocking