Le mer. 19 juin 2019 à 11:16, Rémy Maucherat <r...@apache.org> a écrit :
> On Tue, Jun 18, 2019 at 10:26 PM Romain Manni-Bucau <rmannibu...@gmail.com > > > wrote: > > > +1 to merge (replace) it in owb module since tomcat adherance is reduced > > thanks your refactos. > > > > The few weird things I saw - but guess it is inherited from our owb > module > > and cdi 1 times are: > > > > 1: dont start cdi if there is no beans.xml in webinf (in cdi 2 it should > be > > anyway) > > > > I don't see the bug there. The tomcat7 code seems to do that and it appears > to work for me (no CDI logging without a /WEB-INF/beans.xml): > > https://github.com/apache/openwebbeans/blob/trunk/webbeans-tomcat7/src/main/java/org/apache/webbeans/web/tomcat7/ContextLifecycleListener.java#L63 > > This is the bug, CDI must start without a beans.xml (annotated mode is implicit) > > > 2: security service is thread local based instead of using cdi request > bean > > and it does not support enriching principal api (needed for mp jwt auth > for > > instance). > > > > > https://github.com/apache/meecrowave/blob/trunk/meecrowave-core/src/main/java/org/apache/meecrowave/openwebbeans/MeecrowaveSecurityService.java > > does that and should work in tomcat. > > > > Ok, I imported this class. > > > > > > Except that it looks good to me. > > > > Side question: any interest to try to make meecrowave converging too? > > I can envision a v2 with a nicer tomcat embedded api used as raw base of > > the project and all other features being plugged on top of it but not > sure > > it makes sense for tomcat community. > > Every feature would just be initializers (with @priority support if > > possible to be fully deterministic and user friendly). > > > > After having some problems for a while with some uses of embedding, I made > some changes to embedded to make it more flexible about processing > server.xml and other configuration files (context.xml, default web.xml, > certificates, etc). This is to allow avoiding reinventing server.xml when > using embedded (as SSL configuration in particular became more complex it's > not a very good plan). I did not add any new fancier embedded API, and Mark > seems to be happy enough with the current one as well. > Fair enough. > > Rémy > > Le mar. 18 juin 2019 à 22:07, Rémy Maucherat <r...@apache.org> a écrit : > > > > > Hi, > > > > > > I have made some improvements and refactorings to the Tomcat OWB > > > integration. The code is available here: > > > https://github.com/rmaucher/tomcat-owb > > > > > > The changes are roughly: > > > - I expected this would be a Tomcat external module (like JDBC pool), > so > > > the code was changed to have the Tomcat code style, i18n, package > rename > > > (it can be reverted), refactorings, etc. > > > - Works as a listener on the Server element, which would CDI enable the > > > container. The listener can be a good location for configuration in the > > > future. Various listener event timing issues prevented the tomcat7 > > > integration from working this way. It still has a listener at the > context > > > level, so the integration can be used at the individual webapp level as > > > well. > > > - Requires Tomcat 9.0.21+ due to the said event changes plus some > utility > > > class to avoid code duplication. > > > - Using this integration code, the pom builds a semi fat jar which can > be > > > put in the Tomcat lib folder, then the listener should be added to > > > server.xml. I have verified CDI extension support is functional. > > > - I have tested that CXF (with the right packaging), and the Geronimo > MP > > > impl JARs (without needing a "right" packaging ;) ) work well with this > > OWB > > > support using said CDI extension support. > > > > > > The shade packaging pom would be somewhere in Tomcat (like the > packaging > > > I'm using for container and graal test bed: > > > https://github.com/apache/tomcat/tree/master/res/tomcat-maven ), but > the > > > code needs to be somewhere. As I said I planned to host it at > > > https://github.com/apache/tomcat/tree/master/modules but, > unexpectedly, > > > OWB > > > community members said it should be in OWB. > > > > > > If the code is picked up in OWB, I'd recommend: > > > - Package renaming back to org.apache.webbeans.web.tomcat (I'm not a > fan > > of > > > "tomcat9", it ends up looking bad and/or outdated in the configuration, > > > like "tomcat7" does these days). > > > - Keeping "OpenWebBeans" in class names. Although it's not the usual > > > convention here, the classnames do appear in Tomcat configuration, its > > > monitoring with JMX, and it's more obvious this way. > > > "org.apache.webbeans.web.tomcat9.TomcatListener" in server.xml works I > > > guess, but "org.apache.webbeans.web.tomcat.OpenWebBeansListener" looks > > > better. Overall, only OpenWebBeansPlugin and > OpenWebBeansSecurityService > > > could lose the "OpenWebBeans" name in favor of "Tomcat" (or whatever > > name). > > > - Keep the Tomcat i18n since it doesn't hurt. > > > - No idea about the code formatting, I'm ok if it is changed back. > > > > > > Rémy > > > > > >
