CXF-8188: Inject into ClientHeadersFactory

[andymc12]

Implements CXF-8188 - injecting @Context and @Inject into custom ClientHeadersFactory implementations.

This tests JAX-RS injection. The MP Rest Client 2.0 TCK will test CDI injection.

[reta]

Debug leftovers? ;-)

[andymc12]

Good catch - I'll remove that. Thanks!

[reta]

@andymc12 please correct me if I am wrong, if we consider CDI-based deployments, the instances of the MicroProfileClientProxyImpl (and others) should be created as CDI beans, right?

[andymc12]

if we consider CDI-based deployments, the instances of the MicroProfileClientProxyImpl (and others) should be created as CDI beans, right?

@reta yes, it is done indirectly by creating the client proxy instance using the CxfTypeSafeClientBuilder from the RestClientBean / RestClientExtension.

[reta]

Excellent, thanks @andymc12 , so here is my follow up question / concern: why won't we make CxfTypeSafeClientBuilder CDI-aware (using extension or alike) so the proxies are going to be managed by BeanManager and as such, could use @Inject annotation directly? In this case, the integration should work in all scenarios (managed, standalone, ...). Does it make sense?

[andymc12]

@reta I think CDI is already managing the proxy instances. The extension manages the creation of the proxy (it has to provide additional config via the MP annotations, like @RegisterProvider, @RegisterClientHeaders, etc.), but once it is created, then it turns the proxy over to CDI to manage.

The problem with the ClientHeadersFactory instance is that it is associated with the client interface proxy by annotation, not injected. The other problem is that we want the ClientHeadersFactory to be injected with any JAX-RS contextual info too.

Does that makes sense? Thanks again for your review!

[reta]

hey @andymc12, sorry for the delay, finally found the time to finish the review:

• CDIUtils and CDIFacade: we already use reflection-base BeanManager detection in CDIInterceptorWrapper::createWrapper, it think we should stick to one way to do it
• may be MicroProfileClientFactoryBean is a better place for dealing with CDI & ClientHeadersFactory, which could be passed to MicroProfileClientProxyImpl in a similar way as CDIInterceptorWrapper. The BeanManager detection logic (whichever we pick), could be reused for both of them.

What do you think?

[andymc12]

@reta

CDIUtils and CDIFacade: we already use reflection-base BeanManager detection in CDIInterceptorWrapper::createWrapper, it think we should stick to one way to do it

Agreed. I updated the CDIInterceptorWrapper to use CDIFacade/CDIUtils.

may be MicroProfileClientFactoryBean is a better place for dealing with CDI & ClientHeadersFactory, which could be passed to MicroProfileClientProxyImpl in a similar way as CDIInterceptorWrapper. The BeanManager detection logic (whichever we pick), could be reused for both of them.

My latest changes reuse the BeanManager detection logic. I'm not sure if moving the ClientHeadersFactory logic into MicroProfileClientFactoryBean will work since that is when the client proxy is being created. It's possible the the rest client instance is created in one JAX-RS context (or outside of a JAX-RS request entirely) and then used in a different JAX-RS context. So then the injection would need to occur on each request, right?

Thanks again!

[reta]

Thanks @andymc12 , regarding

My latest changes reuse the BeanManager detection logic. I'm not sure if moving the ClientHeadersFactory logic into MicroProfileClientFactoryBean will work since that is when the client proxy is being created. It's possible the the rest client instance is created in one JAX-RS context (or outside of a JAX-RS request entirely) and then used in a different JAX-RS context. So then the injection would need to occur on each request, right?

you are right, it might happen, but probably the same would apply to CDIInterceptorWrapper: it will pick up the interceptors from the different context. I don't know what is the best option to be fair, just an opinion if we have a single place to make the decisions (the factory bean), it would be much easier to reason about the outcome (proxy) and maintain it. The uncertainty with contexts may cause issues but I would leave the decision to you, I think you have a better view of the picture.