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

CDI OpenWebBeans implementation incompatibility #711

Open
jeanouii opened this issue Sep 26, 2022 · 2 comments
Open

CDI OpenWebBeans implementation incompatibility #711

jeanouii opened this issue Sep 26, 2022 · 2 comments

Comments

@jeanouii
Copy link

Hi,

I was looking to use SmallRye Fault Tolerance to Apache TomEE for MicroProfile Fault Tolerance.

Unfortunatly I found that it does not work. I wanted to provide a TCK runner within a profile in SmallRye Fault Tolerance to show that it does not work in OpenWebBeans.

I checked the reason and this is because the approach used in SmallRye seems to be not fully supported by OpenWebBeans. At this moment, it is not fully clear if this is fully described in the specification. But definitely I will try to push some TCK tests and submit a PR to clarify.

Interceptor bindings are transitive—an interceptor binding declared by an interceptor binding type is inherited by all components and other interceptor binding types that declare that interceptor binding type.

An interceptor binding type can only be applied to an interceptor binding type defining a subset of its target types. For example, interceptor binding types declared Target(TYPE) may not be applied to interceptor binding types declared Target({TYPE, METHOD}).

Looks like there is a blury area between Interceptor Spec and CDI Spec. Weld supports applying InterceptorBinding on another InterceptorBinding.

It seems to be also supported in OpenWebBeans with Reflection but the specification isn't clear if annotated types should be also supported. See OpenWebbeans code https://github.com/apache/openwebbeans/blob/master/webbeans-impl/src/main/java/org/apache/webbeans/annotation/AnnotationManager.java#L231

Long story short, I used a quick hack in TomEE and I was wondering if it would make sense to use the same approach in SmallRye so that it works out of the box with Weld and OpenWebBeans or if I keep it this way in TomEE.

https://github.com/apache/tomee/blob/main/tomee/tomee-microprofile/mp-common/src/main/java/org/apache/tomee/microprofile/faulttolerance/MPFaultToleranceCDIExtension.java

Thoughts?

If you guys agree to reverse the approach then I can create a PR with a TCK runner also for OpenWebBeans.

Thanks

@Ladicek
Copy link
Contributor

Ladicek commented Sep 27, 2022

I take it the problematic thing is how our FaultToleranceExtension registers MP Fault Tolerance interceptor bindings via a custom implementation of AnnotatedType which pretends that each MP Fault Tolerance interceptor binding also has @FaultToleranceBinding?

Your solution adds @FaultToleranceBinding to all classes and methods that have any MP Fault Tolerance annotation, which requires observing PAT for all annotated types in the deployment, which has negative performance implications. I don't think we want to do that in SmallRye Fault Tolerance.

When it comes to specification clarity, I think your quote from the Interceptors spec is pretty clear: transitive interceptor bindings must work. It seems OWB's implementation of BBD.addInterceptorBinding(AnnotatedType) just doesn't search that AnnotatedType for [transitive] interceptor bindings, which in my opinion is a bug. Perhaps this TODO needs fixing? https://github.com/apache/openwebbeans/blob/master/webbeans-impl/src/main/java/org/apache/webbeans/portable/events/discovery/BeforeBeanDiscoveryImpl.java#L175

One thing we could do is switch from BBD.addInterceptorBinding(AnnotatedType) to BBD.addInterceptorBinding(Class, Annotation...). That would probably be more elegant and, looking at OWB code, have a higher chance of working there too. Do you wanna take a stab at it?

@jeanouii
Copy link
Author

@Ladicek Thanks for the additional thoughts.

Yes, I can add an OpenWebBeans runner and give it a try with the other approach.

It was not clear to me whereas it was a bug or not based on the specification. But I'm happy to also fix the annotated type in OpenWebBeans anyways.

Thanks again for the additional thoughts and the peace of information.

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

No branches or pull requests

2 participants