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

Support RunOnVirtualThread annotations on beans implementing a JAX-RS interface #45202

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

danielbobbert
Copy link
Contributor

Fixes: #45186

This PR is about support for beans that implement a JAX-RS interface.

It does four things:

  • fix the resteasy EndpointIndexer such that explicit annotations on the bean overwrite the defaultBlocking
  • Extend the JaxRsMethodProcessor such that the ExecutionModelAnnotationsProcessor will not complain about RunOnVirtualThread annotations on the bean
  • Extend the ResteasyReactiveScanner to allow @RunOnVirualThread annotations on the JAX-RS Application to set the defaultBlocking
  • Add integration tests for beans implementing a JAX-RS interface with @RunOnVirtualThread annotations either on the method or on the bean class

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we rename it isJaxrsResourceMethod() or something similar?

Copy link
Contributor Author

@danielbobbert danielbobbert Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have renamed the method and rebased the branch

This comment has been minimized.

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

Successfully merging this pull request may close these issues.

VirtualThreads cannot be used with JAX-RS interfaces
2 participants