You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openwebbeans.apache.org by Rémy Maucherat <re...@apache.org> on 2019/06/18 20:06:51 UTC

Tomcat integration improvements

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

Re: Tomcat integration improvements

Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le mer. 19 juin 2019 à 20:23, Stephen Connolly <
stephen.alan.connolly@gmail.com> a écrit :

> On Wed, 19 Jun 2019 at 10:37, Romain Manni-Bucau <rm...@gmail.com>
> wrote:
>
> > Le mer. 19 juin 2019 à 11:16, Rémy Maucherat <re...@apache.org> a écrit :
> >
> > > On Tue, Jun 18, 2019 at 10:26 PM Romain Manni-Bucau <
> > rmannibucau@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)
> >
> >
> Ahh I'll need to remove that bug from the jetty module too.... I just
> copied it from Tomcat!
>


Can move to a ServletContextListener in web module and be delegated in
container api when needed, would avoid to fork it and duplicate bugs ;).

Will be harder for the scanning but for the boot it can make sense.



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

Re: Tomcat integration improvements

Posted by Stephen Connolly <st...@gmail.com>.
On Wed, 19 Jun 2019 at 10:37, Romain Manni-Bucau <rm...@gmail.com>
wrote:

> Le mer. 19 juin 2019 à 11:16, Rémy Maucherat <re...@apache.org> a écrit :
>
> > On Tue, Jun 18, 2019 at 10:26 PM Romain Manni-Bucau <
> rmannibucau@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)
>
>
Ahh I'll need to remove that bug from the jetty module too.... I just
copied it from Tomcat!


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

Re: Tomcat integration improvements

Posted by Rémy Maucherat <re...@apache.org>.
On Wed, Jun 19, 2019 at 11:37 AM Romain Manni-Bucau <rm...@gmail.com>
wrote:

> Le mer. 19 juin 2019 à 11:16, Rémy Maucherat <re...@apache.org> a écrit :
>
> > On Tue, Jun 18, 2019 at 10:26 PM Romain Manni-Bucau <
> rmannibucau@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)
>

Ok, no problem. This is a good justification to start adding some
configuration options, so it starts there.

I will create a jira with the patch for this tomcat9 integration code next.

Rémy

Re: Tomcat integration improvements

Posted by Romain Manni-Bucau <rm...@gmail.com>.
Le mer. 19 juin 2019 à 11:16, Rémy Maucherat <re...@apache.org> a écrit :

> On Tue, Jun 18, 2019 at 10:26 PM Romain Manni-Bucau <rmannibucau@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 <re...@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
> > >
> >
>

Re: Tomcat integration improvements

Posted by Rémy Maucherat <re...@apache.org>.
On Tue, Jun 18, 2019 at 10:26 PM Romain Manni-Bucau <rm...@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 <re...@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
> >
>

Re: Tomcat integration improvements

Posted by Romain Manni-Bucau <rm...@gmail.com>.
+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)
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.

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

Le mar. 18 juin 2019 à 22:07, Rémy Maucherat <re...@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
>