You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2021/04/11 15:46:08 UTC

[GitHub] [cxf] reta opened a new pull request #775: CXF-8344: org.apache.cxf.jaxrs.utils.AnnotationUtils#getNameBindings does not use ClassUnwrapper

reta opened a new pull request #775:
URL: https://github.com/apache/cxf/pull/775


   I also added a couple of tests for `@NameBinding` and custom CDI scopes (which would be using `PerRequestResourceProvider` invocation), it turned out we had none.
   
   @rmannibucau mind taking a look please?  thanks a lot!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on pull request #775: CXF-8344: org.apache.cxf.jaxrs.utils.AnnotationUtils#getNameBindings does not use ClassUnwrapper

Posted by GitBox <gi...@apache.org>.
reta commented on pull request #775:
URL: https://github.com/apache/cxf/pull/775#issuecomment-817361168


   @rmannibucau gotcha, yeah sure, I will add yet another test case to make sure proxies are involved, thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta merged pull request #775: CXF-8344: org.apache.cxf.jaxrs.utils.AnnotationUtils#getNameBindings does not use ClassUnwrapper

Posted by GitBox <gi...@apache.org>.
reta merged pull request #775:
URL: https://github.com/apache/cxf/pull/775


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] rmannibucau commented on pull request #775: CXF-8344: org.apache.cxf.jaxrs.utils.AnnotationUtils#getNameBindings does not use ClassUnwrapper

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #775:
URL: https://github.com/apache/cxf/pull/775#issuecomment-817344925


   @reta not sure I get the custom scope impl and need (@ApplicationScoped or @RequestScoped should do he same and properly impl get(bean)) but otherwise looks good


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] rmannibucau commented on pull request #775: CXF-8344: org.apache.cxf.jaxrs.utils.AnnotationUtils#getNameBindings does not use ClassUnwrapper

Posted by GitBox <gi...@apache.org>.
rmannibucau commented on pull request #775:
URL: https://github.com/apache/cxf/pull/775#issuecomment-817360640


   @reta hmm, both case are interesting but my point was that normal scoped beans are required to be proxied whereas others not. That said it also depends how CDI beans are proxies so maybe saner to test with a custom fake proxy not copying annotation since I suspect several CDI impl will copy them to not break frameworks, wdyt?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta commented on pull request #775: CXF-8344: org.apache.cxf.jaxrs.utils.AnnotationUtils#getNameBindings does not use ClassUnwrapper

Posted by GitBox <gi...@apache.org>.
reta commented on pull request #775:
URL: https://github.com/apache/cxf/pull/775#issuecomment-817359964


   > ... @ApplicationScoped or @RequestScoped should do he same and properly impl get(bean)) but otherwise looks good
   
   Aha, so thought but here are some findings, in `JAXRSCdiResourceExtension`:
   - we create `PerRequestResourceProvider` when `isCxfSingleton(beanManager, bean)` is `false`
   -  in turn, `isCxfSingleton` checks the scope
       ```
       boolean isCxfSingleton(final BeanManager beanManager, final Bean<?> bean) {
           return beanManager.isNormalScope(bean.getScope()) || isConsideredSingleton(bean.getScope());
       }
       ```
   - and `beanManager.isNormalScope(bean.getScope())` is `true` for all pre-predefined `@XxxScoped` scopes  because each of those has `@NormalScope` annotation as well
   
   So I introduced the custom scope to trigger the `PerRequestResourceProvider`  flow. What do you think, see any issues with `isCxfSingleton` implementation?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [cxf] reta edited a comment on pull request #775: CXF-8344: org.apache.cxf.jaxrs.utils.AnnotationUtils#getNameBindings does not use ClassUnwrapper

Posted by GitBox <gi...@apache.org>.
reta edited a comment on pull request #775:
URL: https://github.com/apache/cxf/pull/775#issuecomment-817359964


   > ... @ApplicationScoped or @RequestScoped should do he same and properly impl get(bean)) but otherwise looks good
   
   @rmannibucau thanks a lot for the feedback, so thought but here are some findings, in `JAXRSCdiResourceExtension`:
   - we create `PerRequestResourceProvider` when `isCxfSingleton(beanManager, bean)` is `false`
   -  in turn, `isCxfSingleton` checks the scope
       ```
       boolean isCxfSingleton(final BeanManager beanManager, final Bean<?> bean) {
           return beanManager.isNormalScope(bean.getScope()) || isConsideredSingleton(bean.getScope());
       }
       ```
   - and `beanManager.isNormalScope(bean.getScope())` is `true` for all pre-predefined `@XxxScoped` scopes  because each of those has `@NormalScope` annotation as well
   
   So I introduced the custom scope to trigger the `PerRequestResourceProvider`  flow. What do you think, see any issues with `isCxfSingleton` implementation?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org