On Tue, May 28, 2019 at 10:47 PM Mark Thomas <ma...@apache.org> wrote:

> On 28/05/2019 15:40, r...@apache.org wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > remm pushed a commit to branch master
> > in repository https://gitbox.apache.org/repos/asf/tomcat.git
> >
> >
> > The following commit(s) were added to refs/heads/master by this push:
> >      new c5fbb15  Add utility Server listener
> > c5fbb15 is described below
> >
> > commit c5fbb158f7e91306a010dd95f4f13996991dd8fd
> > Author: remm <r...@apache.org>
> > AuthorDate: Tue May 28 16:40:30 2019 +0200
> >
> >     Add utility Server listener
> >
> >     Its purpose is to replicate adding a Listener in context.xml. Also
> add
> >     new container events to notify container add and remove before start
> and
> >     after stop (respectively) so that containers can actually be
> configured
> >     before a possible lifecycle change.
>
> <snip/>
>
> > ---
> >  java/org/apache/catalina/Container.java            |  14 +++
> >  java/org/apache/catalina/core/ContainerBase.java   |   4 +
> >  .../apache/catalina/core/FrameworkListener.java    | 114
> +++++++++++++++++++++
> >  webapps/docs/changelog.xml                         |  10 ++
> >  4 files changed, 142 insertions(+)
> >
> > diff --git a/java/org/apache/catalina/Container.java
> b/java/org/apache/catalina/Container.java
> > index 177b2d2..0b30247 100644
> > --- a/java/org/apache/catalina/Container.java
> > +++ b/java/org/apache/catalina/Container.java
> > @@ -84,6 +84,13 @@ public interface Container extends Lifecycle {
> >
> >      /**
> >       * The ContainerEvent event type sent when a child container is
> added
> > +     * by <code>addChild()</code>, but before it is started.
> > +     */
> > +    public static final String ADD_CHILD_BEFORE_START_EVENT =
> "addChildBeforeStart";
>
> Is this necessary? The listener can check the status of the parent if
> that matters can't it?
>

I found it necessary (to be honest, it's more like the add/remove child
events are misplaced, but I'd rather not move them).

I'm now trying to improve CDI support (this was in my hackaton todo) and
this feature needs to replace the instance manager with a wrapped one. We
could previously replace the instance listener just fine, but CDI is OTOH a
simpler framework that would otherwise use all the Tomcat infrastructure
(naming, configuration, whatever). To do this in a clean way with CDI, the
instance manager replacement needs to happen after the naming listener gets
the start event, and before the actual default instance manager is created.
So I decided it would be nice to add a new hook for such frameworks, along
with a "new" utility server listener (100% ripped off
ThreadLocalLeakPreventionListener).

WIP: https://github.com/rmaucher/tomcat-owb


>
> > +    /**
> > +     * The ContainerEvent event type sent when a child container is
> added
> >       * by <code>addChild()</code>.
> >       */
> >      public static final String ADD_CHILD_EVENT = "addChild";
> > @@ -98,6 +105,13 @@ public interface Container extends Lifecycle {
> >
> >      /**
> >       * The ContainerEvent event type sent when a child container is
> removed
> > +     * by <code>removeChild()</code>, but before it is stopped.
> > +     */
> > +    public static final String REMOVE_CHILD_BEFORE_STOP_EVENT =
> "removeChildBeforeStop";
>
> Same here.
>
> > +    /**
> > +     * The ContainerEvent event type sent when a child container is
> removed
> >       * by <code>removeChild()</code>.
> >       */
> >      public static final String REMOVE_CHILD_EVENT = "removeChild";
>
> <snip/>
>
> > diff --git a/java/org/apache/catalina/core/FrameworkListener.java
> b/java/org/apache/catalina/core/FrameworkListener.java
> > new file mode 100644
> > index 0000000..4927e49
>
> <snip/>
>
> > +    private void processContainerAddChild(Container parent, Container
> child) {
>
> parent is unused. Can it be removed?
>
> > +        if (child instanceof Context) {
> > +            registerContextListener((Context) child);
> > +        } else if (child instanceof Engine) {
> > +            registerListenersForEngine((Engine) child);
> > +        } else if (child instanceof Host) {
> > +            registerListenersForHost((Host) child);
> > +        }
> > +    }
> > +
> > +    private void processContainerRemoveChild(Container parent,
> Container child) {
>
> Same here.
>

These two were only used in a debug log, which I found obnoxious and I
trashed it. So the parameter is indeed useless now and I'll remove it.

Rémy

Reply via email to