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

[JBWS-4430]:Sever throws IllegalStateException when call a handler wi… #557

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

jimma
Copy link
Member

@jimma jimma commented Oct 21, 2024

…th the CDI bean invocation

@jimma jimma added the WIP label Oct 21, 2024
@jimma jimma requested a review from a team as a code owner October 21, 2024 12:24
Copy link
Member

@asoldano asoldano left a comment

Choose a reason for hiding this comment

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

hi @jimma , when this will be ready (in general, not just wrt my other comments here), please be sure to include a testcase to cover this new scenario. Thanks!

@jimma
Copy link
Member Author

jimma commented Oct 22, 2024

hi @jimma , when this will be ready (in general, not just wrt my other comments here), please be sure to include a testcase to cover this new scenario. Thanks!

Yes. That is on my todo list.

Copy link
Member

@asoldano asoldano left a comment

Choose a reason for hiding this comment

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

Thanks Jim, this looks ok, I've added a couple of minor comments.
WRT the AbstractTCCLPhaseInterceptor, I'm wondering if you explored the possibility of configuring that automatically by interjecting the creation of the interceptor chain in a modified PhaseInterceptorChain which could decide to wrap configured interceptors. Just a thought though, I haven't really verified / tested anything ;-)

@jimma
Copy link
Member Author

jimma commented Nov 1, 2024

Thanks Jim, this looks ok, I've added a couple of minor comments. WRT the AbstractTCCLPhaseInterceptor, I'm wondering if you explored the possibility of configuring that automatically by interjecting the creation of the interceptor chain in a modified PhaseInterceptorChain which could decide to wrap configured interceptors. Just a thought though, I haven't really verified / tested anything ;-)

@asoldano Yes. I tried with checking the classloader and by adding set TCCL with interceptor.getClass().getClassLoader() in PhaseInterceptorChain for each interceptor before the interceptor call, but this will slows down the performance. The current approach is more efficient as we need to set TCCL only for the interceptor which has CDI access.

@jimma
Copy link
Member Author

jimma commented Nov 1, 2024

@asoldano I rename the HandlerAuthInterceptor to HandlerConfigInterceptor. Is it a better name ?

@jimma jimma merged commit f890d15 into jbossws:main Nov 5, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants