[CXF-7610] - Adding SPI to handle customizations to the server bean. #369
Conversation
| } | ||
|
|
||
| private static void customize(JAXRSServerFactoryBean bean) { | ||
| ServiceLoader<JAXRSServerFactoryCustomizationExtension> extensions |
There was a problem hiding this comment.
shouldn't it be loaded from the bus as other extensions?
There was a problem hiding this comment.
Not sure what you mean.
There was a problem hiding this comment.
bus.getExtension
main issue is ServiceLoader doesnt work accross environments cxf works so cxf has another "portable" abstraction
There was a problem hiding this comment.
If you're talking about ExtensionRegistry, the javadoc on that indicates OSGi preference.
There was a problem hiding this comment.
META-INF/cxf/bus-extensions.txt / Extension (check ExtensionManagerImpl). Thinking about it we already have events on the bus so it can be used to customize the server without requiring any new API or specific kind of SPI
There was a problem hiding this comment.
@johnament That's the thing, right, if you leave it here it should become a public instance method of the JAXRSServerFactoryBean. Than the question would be, it is there but never being called inside JAXRSServerFactoryBean, what the purpose of this method? I think we could always move it back if the use case pops up. Make sense?
There was a problem hiding this comment.
Yes meecrowave uses the extension.
The fact to not use a SPI but a native cxf loading makes the feature generic IMHO. If we dont care about it as a feature then just testing if sse jar is here (loadClass is fine) and set the sse transportid is enough and doesnt require any SPI.
There was a problem hiding this comment.
@rmannibucau I'm planning to use this same approach for adding MP OpenTracing support as well; TBD though if that's going to be part of Apache CXF.
@reta the method right now is on ResourceUtils. I was going to pull this out into something like a JAXRSServerFactoryCustomizationUtils, so the question is whether this should sit in the CDI module or the jaxrs-frontend module. Obviously my vote is jaxrs-frontend
There was a problem hiding this comment.
@johnament then ensure there is an OSGi test and it is deactivable through a bus or factory property please (dont think it is in this PR, is it?). Goal is to ensure the code is portable and not limited to MP which would dedeat the SPI and that the module can be provided as a container service but not activated if not desired in the app. Also opentracing is already integrated in brave alone or cxf extension so probably doesnt need much SPI but more a wrapper.
There was a problem hiding this comment.
JAXRSServerFactoryCustomizationUtils in jaxrs-frontend sounds good, thanks
johnament
force-pushed
the
cxf-testing-customization
branch
from
becd594 to
886dcb6
Compare
| <beans xmlns="http://java.sun.com/xml/ns/javaee" | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://java.sun.com/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/beans_1_1.xsd" | ||
| bean-discovery-mode="all"> |
There was a problem hiding this comment.
Agree that all was not correct here, but I would like SSE to be discoverable with no efforts when run in CDI container. I think we need only one single class to be discoverable , SseFeature, so @johnament could we add some exclusion / inclusion rules instead of just cutting everything off?
There was a problem hiding this comment.
Since SseFeature is a core CXF feature, not a JAX-RS feature, I believe its autoloaded that way. Isn't it?
There was a problem hiding this comment.
It is core CXF feature, right, and is it automatically discovered by CDI extension (as an annotated type). But I believe if we set bean-discovery-mode="none", it won't be automatically discovered anymore, no?
There was a problem hiding this comment.
I'll test it out and get back to you. If it doesn't work, I'd actually prefer to move it into the extension.
There was a problem hiding this comment.
I ended up going the route of adding the feature's behavior into the extension. Thoughts?
Use the core instance to look up objects since they will satisfy the instance check.
johnament
force-pushed
the
cxf-testing-customization
branch
from
886dcb6 to
89fcf2a
Compare
…pache#369) - Register the existing SSE event handling as an extension
…pache#369) - Register the existing SSE event handling as an extension
…pache#369) - Register the existing SSE event handling as an extension
3 participants
I'm only marking this as a WIP for now, to make sure this makes sense first. Still need to add some tests and port the existing SSE code to use this.