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 > 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. 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 > > >
