-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Register resource classes for reflection when ContainerResponseFilter exists #42754
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure your comment is accurate, the original issue is not about setEntityStream but obtaining annotations on the filtered resource/method.
Also, the original issue uses ContainerRequestFilter
which must go through reflection to obtain annotations, while our docs advise @ServerRequestFilter
which can probably obtain method annotations without registering the entire class for reflection (to be checked). So, is this new test taking this into account and only adding reflection for ContainerRequestFilter
and not @ServerRequestFilter
?
Or are they equally broken ATM?
Last, adding a test would probably save us a future headache :)
This comment has been minimized.
This comment has been minimized.
Yes and no. It's correct as the condition under which the error happens only occurs when
Both the title and the reproducer use
Sure, I'll add one. |
… exists This is needed because those filters can call setEntityStream which then forces the use of the slow path for calling writers Closes: quarkusio#42537
PR updated with a test |
My question was badly formulated, but I meant to ask whether |
Status for workflow
|
Oh, OK, I thought it was only due to using |
👍🏽 |
This is needed because those filters can call
setEntityStream
which then forces the use of theslow path for calling writers