Le lun. 17 juin 2019 à 16:17, Rémy Maucherat <r...@apache.org> a écrit :
> On Mon, Jun 17, 2019 at 10:52 AM Romain Manni-Bucau <rmannibu...@gmail.com>
> wrote:
>
>> Hi Rémy,
>>
>> Great progression! Congrats!
>>
>> I have a few (details) notes - guess i'm opening an open door but just to
>> ensure:
>>
>> 1. Is it intended to bind cxf to /rest/* in any case? I tend to see it
>> bound on on /api but generally thanks to an Application
>> (+ @ApplicationPath("api")) so cxf is bound on /*. Wonder if the listener
>> shouldn't host that config and we would link it through a context listener
>> or servletcontextinitializer (both will work and rely on tomcat internals)
>>
>
> I am not comfortable mapping a servlet on /* by default, it would be
> annoying, so I changed the mapping to /api/*. I used a web-fragment for now
> since it's a decent Servlet API feature and has plenty of configuration
> flexibility.
> Adding additional CXF integration would be good but is work, a filter
> could useful. Moving CXF to a container integration is even more work and
> doesn't looks that practical to me.
>
> The CDI context listener configuring CXF if is present sounds possible (it
> can programmatically add the CDI CXF servlet, no problem), it does sound a
> bit weird conceptually but it's a good option.
>
>
>> 2. In beans.xml, rather than annotated mode - which skips a lot of
>> classes which can be used in CDI - we tend to prefer discovery mode
>> "full" + trim:
>> https://github.com/apache/geronimo-health/blob/master/geronimo-health/src/main/resources/META-INF/beans.xml
>>
>
> I don't understand how that works, if I add scanning to tomcat-cxf.jar, it
> loads all classes (which fails). So that's why I'm not doing it.
>
If you can detail that, I can surely help.
If you can change the code, adding @Vetoed on classes you don't want to be
CDI beans can help too.
The annotated mode - default in cxf - was here to "work out of the box",
however the defaults were not that great and it is easy to fall in a case
where "it should work". All mode is more reliable and aligned on the "old"
(1.0) scanning where all beans are here, the <trim/> addition was there to
drop the useless types and ensure the deployment stays "light".
So long story short, full+trim is likely the less error prone and
priviledged mode today.
>
>
>> 3. Why not merging SPI files thanks to maven? (shade part, end of
>> http://openwebbeans.apache.org/meecrowave/meecrowave-maven/index.html)
>> 4. You have a spring config (cxf.xml), not sure it is intended?
>> 5. jettison is not needed since it is not the one used for json
>> serialization (guess you can drop several cxf deps)
>>
>
> Very good idea, I made changes and fixed these items.
>
👍
BTW, is the assembly available?
>
> Rémy
>
>
