9:11 a.m.
All,
The work to have CDI injection into portlets and filters is almost complete, but a
difference has been discovered between AS7 and Tomcat.
In essence, the ApplicationDispatcher for Tomcat never fires the ServletRequestEvent on
includes, but the ApplicationDispatcher in AS7 has been modified to fire the event when
the current TCCL is different to the classloader on the Context.
I'm proposing that in the Tomcat specific bits of WCI, I create an extended
CommandServlet that performs the same check and fires the events as desired.
Any concerns or better approaches are most welcome.
Ken
========================
Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
10:05 a.m.
On Apr 15, 2013, at 4:11 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
All,
The work to have CDI injection into portlets and filters is almost complete, but a
difference has been discovered between AS7 and Tomcat.
In essence, the ApplicationDispatcher for Tomcat never fires the ServletRequestEvent on
includes, but the ApplicationDispatcher in AS7 has been modified to fire the event when
the current TCCL is different to the classloader on the Context.
wouldn't it be better to do it when there is an include between context ? what is the
difference ?
I'm proposing that in the Tomcat specific bits of WCI, I create an extended
CommandServlet that performs the same check and fires the events as desired.
yes, can you add a test for it so we have guarantees that it will not break later ?
Any concerns or better approaches are most welcome.
one remark: I think we are reaching application spec boundaries. This kind of stuff should
be brought to the Portlet 3.0 EG so we can make it specified in the Servlet spec.
Ken
========================
Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
_______________________________________________
gatein-dev mailing list
gatein-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/gatein-dev
11:03 a.m.
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Monday, April 15, 2013 11:05:16 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 15, 2013, at 4:11 PM, Ken Finnigan < kfinniga(a)redhat.com
> wrote:
> All,
> The work to have CDI injection into portlets and filters is
almost
> complete,
> but a difference has been discovered between AS7 and Tomcat.
> In essence, the ApplicationDispatcher for Tomcat never fires
the
> ServletRequestEvent on includes, but the ApplicationDispatcher in AS7 has
> been modified to fire the event when the current TCCL is different to the
> classloader on the Context.
wouldn't it be better to do it when there is an include between
context ?
what is the difference ?
Pretty sure that's the intention, but for AS7 they use the Classloader to determine
whether the request is in the same context.
> I'm proposing that in the Tomcat specific bits of WCI, I
create an extended
> CommandServlet that performs the same check and fires the events as
> desired.
yes, can you add a test for it so we have guarantees that it will not
break
later ?
not a problem to add test for it.
However, putting together an implementation has hit a snag in terms of needing access to
the Context object from Tomcat to retrieve the application event listeners. We retain an
instance of it on TC7WebAppContext, but not sure whether there are issues in either
exposing that instance, or adding ability to retrieve the event listeners onto the
WebAppContext (and then whether that would be accessible to Dispatcher/Servlet).
Thoughts?
> Any concerns or better approaches are most welcome.
one remark: I think we are reaching application spec boundaries. This
kind of
stuff should be brought to the Portlet 3.0 EG so we can make it specified in
the Servlet spec.
> Ken
> ========================
> Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
> _______________________________________________
> gatein-dev mailing list
> gatein-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/gatein-dev
2:36 p.m.
Julien,
Here's the changes for WCI to enable servlet request events:
https://github.com/gatein/gatein-wci/pull/11
If everything looks ok I'll merge tomorrow and release a new WCI.
Ken
----- Original Message -----
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Monday, April 15, 2013 11:05:16 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 15, 2013, at 4:11 PM, Ken Finnigan < kfinniga(a)redhat.com
> wrote:
> All,
> The work to have CDI injection into portlets and filters is
almost
> complete,
> but a difference has been discovered between AS7 and Tomcat.
> In essence, the ApplicationDispatcher for Tomcat never fires
the
> ServletRequestEvent on includes, but the ApplicationDispatcher in AS7 has
> been modified to fire the event when the current TCCL is different to the
> classloader on the Context.
wouldn't it be better to do it when there is an include between
context ?
what is the difference ?
> I'm proposing that in the Tomcat specific bits of WCI, I
create an extended
> CommandServlet that performs the same check and fires the events as
> desired.
yes, can you add a test for it so we have guarantees that it will not
break
later ?
> Any concerns or better approaches are most welcome.
one remark: I think we are reaching application spec boundaries. This
kind of
stuff should be brought to the Portlet 3.0 EG so we can make it specified in
the Servlet spec.
> Ken
> ========================
> Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
> _______________________________________________
> gatein-dev mailing list
> gatein-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/gatein-dev
8:35 a.m.
Hi Ken,
there are a few things I would like to discuss about the PR.
The main thing I want to understand is when the "fire" event should be fired
when the dispatch occurs.
In the new TomcatCommandDispatcher I see it is done in the include method before the
dispatch is effectively done (i.e before the servlet container performs the dispatch).
What about having it done after the dispatch is really done ? (which would avoid to switch
thread context classloader as Tomcat would have done that already).
On Apr 16, 2013, at 9:36 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Julien,
Here's the changes for WCI to enable servlet request events:
https://github.com/gatein/gatein-wci/pull/11
If everything looks ok I'll merge tomorrow and release a new WCI.
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Monday, April 15, 2013 11:05:16 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 15, 2013, at 4:11 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
All,
The work to have CDI injection into portlets and filters is almost complete, but a
difference has been discovered between AS7 and Tomcat.
In essence, the ApplicationDispatcher for Tomcat never fires the ServletRequestEvent on
includes, but the ApplicationDispatcher in AS7 has been modified to fire the event when
the current TCCL is different to the classloader on the Context.
wouldn't it be better to do it when there is an include between context ? what is the
difference ?
I'm proposing that in the Tomcat specific bits of WCI, I create an extended
CommandServlet that performs the same check and fires the events as desired.
yes, can you add a test for it so we have guarantees that it will not break later ?
Any concerns or better approaches are most welcome.
one remark: I think we are reaching application spec boundaries. This kind of stuff
should be brought to the Portlet 3.0 EG so we can make it specified in the Servlet spec.
Ken
========================
Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
_______________________________________________
gatein-dev mailing list
gatein-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/gatein-dev
8:58 a.m.
Are you suggesting to fire the events inside CommandServlet.include() around the following
call?
switcher.include(request, response);
My original plan was to perform it within that method for Tomcat, but with the way
CommandServlet is written, I wasn't sure that it would be easy to extend it to only
replace the bits I needed, which is why I created a CommandDispatcher instead.
Ken
----- Original Message -----
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 9:35:32 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
Hi Ken,
there are a few things I would like to discuss about the PR.
The main thing I want to understand is when the "fire"
event should be fired
when the dispatch occurs.
In the new TomcatCommandDispatcher I see it is done in the include
method
before the dispatch is effectively done (i.e before the servlet container
performs the dispatch).
What about having it done after the dispatch is really done ? (which
would
avoid to switch thread context classloader as Tomcat would have done that
already).
On Apr 16, 2013, at 9:36 PM, Ken Finnigan < kfinniga(a)redhat.com
> wrote:
> Julien,
> Here's the changes for WCI to enable servlet request
events:
> https://github.com/gatein/gatein-wci/pull/11
> If everything looks ok I'll merge tomorrow and release a new
WCI.
> Ken
> ----- Original Message -----
> > From: "Julien Viet" < julien(a)julienviet.com
>
>
> > To: "Ken Finnigan" < kfinniga(a)redhat.com >
>
> > Cc: gatein-dev(a)lists.jboss.org
>
> > Sent: Monday, April 15, 2013 11:05:16 AM
>
> > Subject: Re: [gatein-dev] CDI Injection for GateIn
>
> > On Apr 15, 2013, at 4:11 PM, Ken Finnigan <
kfinniga(a)redhat.com > wrote:
>
> > > All,
> >
>
> > > The work to have CDI injection into portlets and
filters is almost
> > > complete,
> > > but a difference has been discovered between AS7 and Tomcat.
> >
>
> > > In essence, the ApplicationDispatcher for Tomcat never
fires the
> > > ServletRequestEvent on includes, but the ApplicationDispatcher in AS7
> > > has
> > > been modified to fire the event when the current TCCL is different to
> > > the
> > > classloader on the Context.
> >
>
> > wouldn't it be better to do it when there is an include
between context ?
> > what is the difference ?
>
> > > I'm proposing that in the Tomcat specific bits of
WCI, I create an
> > > extended
> > > CommandServlet that performs the same check and fires the events as
> > > desired.
> >
>
> > yes, can you add a test for it so we have guarantees that
it will not
> > break
> > later ?
>
> > > Any concerns or better approaches are most welcome.
> >
>
> > one remark: I think we are reaching application spec
boundaries. This
> > kind
> > of
> > stuff should be brought to the Portlet 3.0 EG so we can make it specified
> > in
> > the Servlet spec.
>
> > > Ken
> >
>
> > > ========================
> >
>
> > > Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
> >
>
> > > _______________________________________________
> >
>
> > > gatein-dev mailing list
> >
>
> > > gatein-dev(a)lists.jboss.org
> >
>
> > > https://lists.jboss.org/mailman/listinfo/gatein-dev
> >
>
9:08 a.m.
On Apr 17, 2013, at 3:58 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Are you suggesting to fire the events inside CommandServlet.include()
around the following call?
switcher.include(request, response);
yes
My original plan was to perform it within that method for Tomcat, but
with the way CommandServlet is written, I wasn't sure that it would be easy to extend
it to only replace the bits I needed, which is why I created a CommandDispatcher instead.
I was thinking at wrapping the callback with a call that perform the event firing:
RequestDispatchCallback wrapper = new RequestDispatchCallback() {
public Object doCallback(
ServletContext dispatchedServletContext,
HttpServletRequest dispatchedRequest,
HttpServletResponse dispatchedResponse,
Object handback) throws ServletException, IOException {
// Fire event before
Object ret = cmd.doCallback(dispatchedServletContext, dispatchedRequest,
dispatchedResponse, handback);
// Fire event after
return ret;
}
}
and change
result = CommandServlet.include(servletPath, req, resp, cmd,
targetServletContext);
to
result = CommandServlet.include(servletPath, req, resp, wrapper,
targetServletContext);
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 9:35:32 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
Hi Ken,
there are a few things I would like to discuss about the PR.
The main thing I want to understand is when the "fire" event should be fired
when the dispatch occurs.
In the new TomcatCommandDispatcher I see it is done in the include method before the
dispatch is effectively done (i.e before the servlet container performs the dispatch).
What about having it done after the dispatch is really done ? (which would avoid to
switch thread context classloader as Tomcat would have done that already).
On Apr 16, 2013, at 9:36 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Julien,
Here's the changes for WCI to enable servlet request events:
https://github.com/gatein/gatein-wci/pull/11
If everything looks ok I'll merge tomorrow and release a new WCI.
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Monday, April 15, 2013 11:05:16 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 15, 2013, at 4:11 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
All,
The work to have CDI injection into portlets and filters is almost complete, but a
difference has been discovered between AS7 and Tomcat.
In essence, the ApplicationDispatcher for Tomcat never fires the ServletRequestEvent on
includes, but the ApplicationDispatcher in AS7 has been modified to fire the event when
the current TCCL is different to the classloader on the Context.
wouldn't it be better to do it when there is an include between context ? what is the
difference ?
I'm proposing that in the Tomcat specific bits of WCI, I create an extended
CommandServlet that performs the same check and fires the events as desired.
yes, can you add a test for it so we have guarantees that it will not break later ?
Any concerns or better approaches are most welcome.
one remark: I think we are reaching application spec boundaries. This kind of stuff
should be brought to the Portlet 3.0 EG so we can make it specified in the Servlet spec.
Ken
========================
Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
_______________________________________________
gatein-dev mailing list
gatein-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/gatein-dev
9:44 a.m.
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 10:08:24 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 17, 2013, at 3:58 PM, Ken Finnigan < kfinniga(a)redhat.com
> wrote:
> Are you suggesting to fire the events inside
CommandServlet.include()
> around
> the following call?
> switcher.include(request, response);
yes
> My original plan was to perform it within that method for
Tomcat, but with
> the way CommandServlet is written, I wasn't sure that it would be easy to
> extend it to only replace the bits I needed, which is why I created a
> CommandDispatcher instead.
I was thinking at wrapping the callback with a call that perform the
event
firing:
RequestDispatchCallback wrapper = new RequestDispatchCallback() {
public Object doCallback(
ServletContext dispatchedServletContext,
HttpServletRequest dispatchedRequest,
HttpServletResponse dispatchedResponse,
Object handback) throws ServletException, IOException {
// Fire event before
Object ret = cmd.doCallback(dispatchedServletContext, dispatchedRequest,
dispatchedResponse, handback);
Is cmd the CallbackCommand that is currently created within CommandDispatcher.include()?
Do you mean that the RequestDispatcherCallback should be wrapped, and not the
CallbackCommand?
So we'd have as above except:
Object ret = callback.doCallback(...);
// Fire event after
return ret;
}
}
And the CallbackCommand would be created as:
new CallbackCommand(targetServletContext, wrapper, handback);
instead of
new CallbackCommand(targetServletContext, cmd, handback);
and change
result = CommandServlet.include(servletPath, req, resp, cmd,
targetServletContext);
and this would stay as is
to
result = CommandServlet.include(servletPath, req, resp, wrapper,
targetServletContext);
> Ken
> ----- Original Message -----
> > From: "Julien Viet" < julien(a)julienviet.com
>
>
> > To: "Ken Finnigan" < kfinniga(a)redhat.com >
>
> > Cc: gatein-dev(a)lists.jboss.org
>
> > Sent: Wednesday, April 17, 2013 9:35:32 AM
>
> > Subject: Re: [gatein-dev] CDI Injection for GateIn
>
> > Hi Ken,
>
> > there are a few things I would like to discuss about the
PR.
>
> > The main thing I want to understand is when the
"fire" event should be
> > fired
> > when the dispatch occurs.
>
> > In the new TomcatCommandDispatcher I see it is done in the
include method
> > before the dispatch is effectively done (i.e before the servlet container
> > performs the dispatch).
>
> > What about having it done after the dispatch is really done
? (which
> > would
> > avoid to switch thread context classloader as Tomcat would have done that
> > already).
>
> > On Apr 16, 2013, at 9:36 PM, Ken Finnigan <
kfinniga(a)redhat.com > wrote:
>
> > > Julien,
> >
>
> > > Here's the changes for WCI to enable servlet
request events:
> > > https://github.com/gatein/gatein-wci/pull/11
> >
>
> > > If everything looks ok I'll merge tomorrow and
release a new WCI.
> >
>
> > > Ken
> >
>
> > > ----- Original Message -----
> >
>
> > > > From: "Julien Viet" <
julien(a)julienviet.com >
> > >
> >
>
> > > > To: "Ken Finnigan" < kfinniga(a)redhat.com >
> > >
> >
>
> > > > Cc: gatein-dev(a)lists.jboss.org
> > >
> >
>
> > > > Sent: Monday, April 15, 2013 11:05:16 AM
> > >
> >
>
> > > > Subject: Re: [gatein-dev] CDI Injection for GateIn
> > >
> >
>
> > > > On Apr 15, 2013, at 4:11 PM, Ken Finnigan <
kfinniga(a)redhat.com >
> > > > wrote:
> > >
> >
>
> > > > > All,
> > > >
> > >
> >
>
> > > > > The work to have CDI injection into portlets
and filters is almost
> > > > > complete,
> > > > > but a difference has been discovered between AS7 and Tomcat.
> > > >
> > >
> >
>
> > > > > > In essence, the ApplicationDispatcher for Tomcat never fires
the
> > > > > > ServletRequestEvent on includes, but the
ApplicationDispatcher in
> > > > > > AS7
> > > > > > has
> > > > > > been modified to fire the event when the current TCCL is
different
> > > > > to
> > > > > >
the
> > > > > > classloader on the Context.
> > > > >
> > > >
> > >
> >
>
> > > > wouldn't it be better to do it when there is
an include between
> > > > context
> > > > ?
> > > > what is the difference ?
> > >
> >
>
> > > > > I'm proposing that in the Tomcat
specific bits of WCI, I create an
> > > > > extended
> > > > > CommandServlet that performs the same check and fires the events
as
> > > > > desired.
> > > >
> > >
> >
>
> > > > yes, can you add a test for it so we have
guarantees that it will not
> > > > break
> > > > later ?
> > >
> >
>
> > > > > Any concerns or better approaches are most
welcome.
> > > >
> > >
> >
>
> > > > one remark: I think we are reaching application
spec boundaries. This
> > > > kind
> > > > of
> > > > stuff should be brought to the Portlet 3.0 EG so we can make it
> > > > specified
> > > > in
> > > > the Servlet spec.
> > >
> >
>
> > > > > Ken
> > > >
> > >
> >
>
> > > > > ========================
> > > >
> > >
> >
>
> > > > > Senior Software Engineer / JBoss Enterprise Middleware Red Hat,
> > > > > Inc.
> > > >
> > >
> >
>
> > > > >
_______________________________________________
> > > >
> > >
> >
>
> > > > > gatein-dev mailing list
> > > >
> > >
> >
>
> > > > > gatein-dev(a)lists.jboss.org
> > > >
> > >
> >
>
> > > > > https://lists.jboss.org/mailman/listinfo/gatein-dev
> > > >
> > >
> >
>
10:22 a.m.
sorry I was not clear and I even confused myself :-) :
replacing
CallbackCommand cmd = new CallbackCommand(targetServletContext, callback,
handback);
by
CallbackCommand cmd = new CallbackCommand(targetServletContext, callback,
handback) {
@Override
public Object execute(HttpServletRequest req, HttpServletResponse resp) throws
ServletException, IOException
{
// Fire before
Object ret = super.execute(req, resp);
// Fire after
return ret;
}
};
is what I meant.
On Apr 17, 2013, at 4:44 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 10:08:24 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 17, 2013, at 3:58 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Are you suggesting to fire the events inside CommandServlet.include() around the
following call?
switcher.include(request, response);
yes
My original plan was to perform it within that method for Tomcat, but with the way
CommandServlet is written, I wasn't sure that it would be easy to extend it to only
replace the bits I needed, which is why I created a CommandDispatcher instead.
I was thinking at wrapping the callback with a call that perform the event firing:
RequestDispatchCallback wrapper = new RequestDispatchCallback() {
public Object doCallback(
ServletContext dispatchedServletContext,
HttpServletRequest dispatchedRequest,
HttpServletResponse dispatchedResponse,
Object handback) throws ServletException, IOException {
// Fire event before
Object ret = cmd.doCallback(dispatchedServletContext, dispatchedRequest,
dispatchedResponse, handback);
Is cmd the CallbackCommand that is currently created within CommandDispatcher.include()?
Do you mean that the RequestDispatcherCallback should be wrapped, and not the
CallbackCommand?
So we'd have as above except:
Object ret = callback.doCallback(...);
// Fire event after
return ret;
}
}
And the CallbackCommand would be created as:
new CallbackCommand(targetServletContext, wrapper, handback);
instead of
new CallbackCommand(targetServletContext, cmd, handback);
and change
result = CommandServlet.include(servletPath, req, resp, cmd,
targetServletContext);
and this would stay as is
to
result = CommandServlet.include(servletPath, req, resp, wrapper,
targetServletContext);
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 9:35:32 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
Hi Ken,
there are a few things I would like to discuss about the PR.
The main thing I want to understand is when the "fire" event should be fired
when the dispatch occurs.
In the new TomcatCommandDispatcher I see it is done in the include method before the
dispatch is effectively done (i.e before the servlet container performs the dispatch).
What about having it done after the dispatch is really done ? (which would avoid to
switch thread context classloader as Tomcat would have done that already).
On Apr 16, 2013, at 9:36 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Julien,
Here's the changes for WCI to enable servlet request events:
https://github.com/gatein/gatein-wci/pull/11
If everything looks ok I'll merge tomorrow and release a new WCI.
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Monday, April 15, 2013 11:05:16 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 15, 2013, at 4:11 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
All,
The work to have CDI injection into portlets and filters is almost complete, but a
difference has been discovered between AS7 and Tomcat.
In essence, the ApplicationDispatcher for Tomcat never fires the ServletRequestEvent on
includes, but the ApplicationDispatcher in AS7 has been modified to fire the event when
the current TCCL is different to the classloader on the Context.
wouldn't it be better to do it when there is an include between context ? what is the
difference ?
I'm proposing that in the Tomcat specific bits of WCI, I create an extended
CommandServlet that performs the same check and fires the events as desired.
yes, can you add a test for it so we have guarantees that it will not break later ?
Any concerns or better approaches are most welcome.
one remark: I think we are reaching application spec boundaries. This kind of stuff
should be brought to the Portlet 3.0 EG so we can make it specified in the Servlet spec.
Ken
========================
Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
_______________________________________________
gatein-dev mailing list
gatein-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/gatein-dev
10:23 a.m.
(in TomcatCommandDispatcher#include)
On Apr 17, 2013, at 5:22 PM, Julien Viet <julien(a)julienviet.com> wrote:
sorry I was not clear and I even confused myself :-) :
replacing
CallbackCommand cmd = new CallbackCommand(targetServletContext, callback,
handback);
by
CallbackCommand cmd = new CallbackCommand(targetServletContext, callback,
handback) {
@Override
public Object execute(HttpServletRequest req, HttpServletResponse resp) throws
ServletException, IOException
{
// Fire before
Object ret = super.execute(req, resp);
// Fire after
return ret;
}
};
is what I meant.
On Apr 17, 2013, at 4:44 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
> From: "Julien Viet" <julien(a)julienviet.com>
> To: "Ken Finnigan" <kfinniga(a)redhat.com>
> Cc: gatein-dev(a)lists.jboss.org
> Sent: Wednesday, April 17, 2013 10:08:24 AM
> Subject: Re: [gatein-dev] CDI Injection for GateIn
>
>
> On Apr 17, 2013, at 3:58 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
>
> Are you suggesting to fire the events inside CommandServlet.include() around the
following call?
> switcher.include(request, response);
>
>
> yes
>
> My original plan was to perform it within that method for Tomcat, but with the way
CommandServlet is written, I wasn't sure that it would be easy to extend it to only
replace the bits I needed, which is why I created a CommandDispatcher instead.
>
> I was thinking at wrapping the callback with a call that perform the event firing:
>
> RequestDispatchCallback wrapper = new RequestDispatchCallback() {
> public Object doCallback(
> ServletContext dispatchedServletContext,
> HttpServletRequest dispatchedRequest,
> HttpServletResponse dispatchedResponse,
> Object handback) throws ServletException, IOException {
>
> // Fire event before
> Object ret = cmd.doCallback(dispatchedServletContext, dispatchedRequest,
dispatchedResponse, handback);
>
> Is cmd the CallbackCommand that is currently created within
CommandDispatcher.include()?
>
> Do you mean that the RequestDispatcherCallback should be wrapped, and not the
CallbackCommand?
>
> So we'd have as above except:
>
> Object ret = callback.doCallback(...);
>
> // Fire event after
> return ret;
>
> }
> }
>
> And the CallbackCommand would be created as:
>
> new CallbackCommand(targetServletContext, wrapper, handback);
>
> instead of
>
> new CallbackCommand(targetServletContext, cmd, handback);
>
> and change
>
> result = CommandServlet.include(servletPath, req, resp, cmd,
targetServletContext);
>
> and this would stay as is
>
>
> to
>
> result = CommandServlet.include(servletPath, req, resp, wrapper,
targetServletContext);
>
>
>
> Ken
>
> From: "Julien Viet" <julien(a)julienviet.com>
> To: "Ken Finnigan" <kfinniga(a)redhat.com>
> Cc: gatein-dev(a)lists.jboss.org
> Sent: Wednesday, April 17, 2013 9:35:32 AM
> Subject: Re: [gatein-dev] CDI Injection for GateIn
>
> Hi Ken,
>
> there are a few things I would like to discuss about the PR.
>
> The main thing I want to understand is when the "fire" event should be
fired when the dispatch occurs.
>
> In the new TomcatCommandDispatcher I see it is done in the include method before the
dispatch is effectively done (i.e before the servlet container performs the dispatch).
>
> What about having it done after the dispatch is really done ? (which would avoid to
switch thread context classloader as Tomcat would have done that already).
>
> On Apr 16, 2013, at 9:36 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
>
> Julien,
>
> Here's the changes for WCI to enable servlet request events:
https://github.com/gatein/gatein-wci/pull/11
>
> If everything looks ok I'll merge tomorrow and release a new WCI.
>
> Ken
>
> From: "Julien Viet" <julien(a)julienviet.com>
> To: "Ken Finnigan" <kfinniga(a)redhat.com>
> Cc: gatein-dev(a)lists.jboss.org
> Sent: Monday, April 15, 2013 11:05:16 AM
> Subject: Re: [gatein-dev] CDI Injection for GateIn
>
>
> On Apr 15, 2013, at 4:11 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
>
> All,
>
> The work to have CDI injection into portlets and filters is almost complete, but a
difference has been discovered between AS7 and Tomcat.
>
> In essence, the ApplicationDispatcher for Tomcat never fires the ServletRequestEvent
on includes, but the ApplicationDispatcher in AS7 has been modified to fire the event when
the current TCCL is different to the classloader on the Context.
>
> wouldn't it be better to do it when there is an include between context ? what is
the difference ?
>
>
> I'm proposing that in the Tomcat specific bits of WCI, I create an extended
CommandServlet that performs the same check and fires the events as desired.
>
> yes, can you add a test for it so we have guarantees that it will not break later ?
>
>
> Any concerns or better approaches are most welcome.
>
> one remark: I think we are reaching application spec boundaries. This kind of stuff
should be brought to the Portlet 3.0 EG so we can make it specified in the Servlet spec.
>
>
> Ken
>
> ========================
> Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
>
> _______________________________________________
> gatein-dev mailing list
> gatein-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/gatein-dev
>
>
>
>
>
>
10:33 a.m.
Ok, see what you mean now.
Shouldn't be a problem to do that, will let you know if there are issues
Thanks
Ken
----- Original Message -----
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 11:22:37 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
sorry I was not clear and I even confused myself :-) :
replacing
CallbackCommand cmd = new CallbackCommand(targetServletContext,
callback,
handback);
by
CallbackCommand cmd = new CallbackCommand(targetServletContext,
callback,
handback) {
@Override
public Object execute(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException
{
// Fire before
Object ret = super.execute(req, resp);
// Fire after
return ret;
}
};
is what I meant.
On Apr 17, 2013, at 4:44 PM, Ken Finnigan < kfinniga(a)redhat.com
> wrote:
> > From: "Julien Viet" < julien(a)julienviet.com
>
>
> > To: "Ken Finnigan" < kfinniga(a)redhat.com >
>
> > Cc: gatein-dev(a)lists.jboss.org
>
> > Sent: Wednesday, April 17, 2013 10:08:24 AM
>
> > Subject: Re: [gatein-dev] CDI Injection for GateIn
>
> > On Apr 17, 2013, at 3:58 PM, Ken Finnigan <
kfinniga(a)redhat.com > wrote:
>
> > > Are you suggesting to fire the events inside
CommandServlet.include()
> > > around
> > > the following call?
> >
>
> > > switcher.include(request, response);
> >
>
> > yes
>
> > > My original plan was to perform it within that method
for Tomcat, but
> > > with
> > > the way CommandServlet is written, I wasn't sure that it would be
easy
> > > to
> > > extend it to only replace the bits I needed, which is why I created a
> > > CommandDispatcher instead.
> >
>
> > I was thinking at wrapping the callback with a call that
perform the
> > event
> > firing:
>
> > RequestDispatchCallback wrapper = new
RequestDispatchCallback() {
>
> > public Object doCallback(
>
> > ServletContext dispatchedServletContext,
>
> > HttpServletRequest dispatchedRequest,
>
> > HttpServletResponse dispatchedResponse,
>
> > Object handback) throws ServletException, IOException {
>
> > // Fire event before
>
> > Object ret = cmd.doCallback(dispatchedServletContext, dispatchedRequest,
> > dispatchedResponse, handback);
>
> Is cmd the CallbackCommand that is currently created within
> CommandDispatcher.include()?
> Do you mean that the RequestDispatcherCallback should be
wrapped, and not
> the
> CallbackCommand?
> So we'd have as above except:
> Object ret = callback.doCallback(...);
> > // Fire event after
>
> > return ret;
>
> > }
>
> > }
>
> And the CallbackCommand would be created as:
> new CallbackCommand(targetServletContext, wrapper, handback);
> instead of
> new CallbackCommand(targetServletContext, cmd, handback);
> > and change
>
> > result = CommandServlet.include(servletPath, req, resp,
cmd,
> > targetServletContext);
>
> and this would stay as is
> > to
>
> > result = CommandServlet.include(servletPath, req, resp,
wrapper,
> > targetServletContext);
>
> > > Ken
> >
>
> > > ----- Original Message -----
> >
>
> > > > From: "Julien Viet" <
julien(a)julienviet.com >
> > >
> >
>
> > > > To: "Ken Finnigan" < kfinniga(a)redhat.com >
> > >
> >
>
> > > > Cc: gatein-dev(a)lists.jboss.org
> > >
> >
>
> > > > Sent: Wednesday, April 17, 2013 9:35:32 AM
> > >
> >
>
> > > > Subject: Re: [gatein-dev] CDI Injection for GateIn
> > >
> >
>
> > > > Hi Ken,
> > >
> >
>
> > > > there are a few things I would like to discuss
about the PR.
> > >
> >
>
> > > > The main thing I want to understand is when the
"fire" event should
> > > > be
> > > > fired
> > > > when the dispatch occurs.
> > >
> >
>
> > > > In the new TomcatCommandDispatcher I see it is
done in the include
> > > > method
> > > > before the dispatch is effectively done (i.e before the servlet
> > > > container
> > > > performs the dispatch).
> > >
> >
>
> > > > What about having it done after the dispatch is
really done ? (which
> > > > would
> > > > avoid to switch thread context classloader as Tomcat would have done
> > > > that
> > > > already).
> > >
> >
>
> > > > On Apr 16, 2013, at 9:36 PM, Ken Finnigan <
kfinniga(a)redhat.com >
> > > > wrote:
> > >
> >
>
> > > > > Julien,
> > > >
> > >
> >
>
> > > > > Here's the changes for WCI to enable
servlet request events:
> > > > > https://github.com/gatein/gatein-wci/pull/11
> > > >
> > >
> >
>
> > > > > If everything looks ok I'll merge
tomorrow and release a new WCI.
> > > >
> > >
> >
>
> > > > > Ken
> > > >
> > >
> >
>
> > > > > ----- Original Message -----
> > > >
> > >
> >
>
> > > > > > From: "Julien Viet" <
julien(a)julienviet.com >
> > > > >
> > > >
> > >
> >
>
> > > > > > To: "Ken Finnigan" < kfinniga(a)redhat.com >
> > > > >
> > > >
> > >
> >
>
> > > > > > Cc: gatein-dev(a)lists.jboss.org
> > > > >
> > > >
> > >
> >
>
> > > > > > Sent: Monday, April 15, 2013 11:05:16 AM
> > > > >
> > > >
> > >
> >
>
> > > > > > Subject: Re: [gatein-dev] CDI Injection for GateIn
> > > > >
> > > >
> > >
> >
>
> > > > > > On Apr 15, 2013, at 4:11 PM, Ken
Finnigan < kfinniga(a)redhat.com >
> > > > > > wrote:
> > > > >
> > > >
> > >
> >
>
> > > > > > > All,
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > The work to have CDI injection
into portlets and filters is
> > > > > > > almost
> > > > > > > complete,
> > > > > > > but a difference has been discovered between AS7 and
Tomcat.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > In essence, the
ApplicationDispatcher for Tomcat never fires
> > > > > > > the
> > > > > > > ServletRequestEvent on includes, but the
ApplicationDispatcher
> > > > > > > in
> > > > > > > AS7
> > > > > > > has
> > > > > > > been modified to fire the event when the current TCCL
is
> > > > > > > different
> > > > > > > to
> > > > > > > the
> > > > > > > classloader on the Context.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > wouldn't it be better to do it when
there is an include between
> > > > > > context
> > > > > > ?
> > > > > > what is the difference ?
> > > > >
> > > >
> > >
> >
>
> > > > > > > I'm proposing that in the
Tomcat specific bits of WCI, I create
> > > > > > > an
> > > > > > > extended
> > > > > > > CommandServlet that performs the same check and fires
the
> > > > > > > events
> > > > > > > as
> > > > > > > desired.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > yes, can you add a test for it so we
have guarantees that it will
> > > > > > not
> > > > > > break
> > > > > > later ?
> > > > >
> > > >
> > >
> >
>
> > > > > > > Any concerns or better approaches
are most welcome.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > one remark: I think we are reaching
application spec boundaries.
> > > > > > This
> > > > > > kind
> > > > > > of
> > > > > > stuff should be brought to the Portlet 3.0 EG so we can
make it
> > > > > > specified
> > > > > > in
> > > > > > the Servlet spec.
> > > > >
> > > >
> > >
> >
>
> > > > > > > Ken
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > ========================
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > Senior Software Engineer / JBoss Enterprise Middleware
Red Hat,
> > > > > > > Inc.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > >
_______________________________________________
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > gatein-dev mailing list
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > gatein-dev(a)lists.jboss.org
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > https://lists.jboss.org/mailman/listinfo/gatein-dev
> > > > > >
> > > > >
> > > >
> > >
> >
>
12:25 p.m.
Have updated the PR with the suggested changes:
https://github.com/gatein/gatein-wci/pull/11/files
If all is ok, will merge and do release
Thanks
Ken
----- Original Message -----
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 11:22:37 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
sorry I was not clear and I even confused myself :-) :
replacing
CallbackCommand cmd = new CallbackCommand(targetServletContext,
callback,
handback);
by
CallbackCommand cmd = new CallbackCommand(targetServletContext,
callback,
handback) {
@Override
public Object execute(HttpServletRequest req, HttpServletResponse resp)
throws ServletException, IOException
{
// Fire before
Object ret = super.execute(req, resp);
// Fire after
return ret;
}
};
is what I meant.
On Apr 17, 2013, at 4:44 PM, Ken Finnigan < kfinniga(a)redhat.com
> wrote:
> > From: "Julien Viet" < julien(a)julienviet.com
>
>
> > To: "Ken Finnigan" < kfinniga(a)redhat.com >
>
> > Cc: gatein-dev(a)lists.jboss.org
>
> > Sent: Wednesday, April 17, 2013 10:08:24 AM
>
> > Subject: Re: [gatein-dev] CDI Injection for GateIn
>
> > On Apr 17, 2013, at 3:58 PM, Ken Finnigan <
kfinniga(a)redhat.com > wrote:
>
> > > Are you suggesting to fire the events inside
CommandServlet.include()
> > > around
> > > the following call?
> >
>
> > > switcher.include(request, response);
> >
>
> > yes
>
> > > My original plan was to perform it within that method
for Tomcat, but
> > > with
> > > the way CommandServlet is written, I wasn't sure that it would be
easy
> > > to
> > > extend it to only replace the bits I needed, which is why I created a
> > > CommandDispatcher instead.
> >
>
> > I was thinking at wrapping the callback with a call that
perform the
> > event
> > firing:
>
> > RequestDispatchCallback wrapper = new
RequestDispatchCallback() {
>
> > public Object doCallback(
>
> > ServletContext dispatchedServletContext,
>
> > HttpServletRequest dispatchedRequest,
>
> > HttpServletResponse dispatchedResponse,
>
> > Object handback) throws ServletException, IOException {
>
> > // Fire event before
>
> > Object ret = cmd.doCallback(dispatchedServletContext, dispatchedRequest,
> > dispatchedResponse, handback);
>
> Is cmd the CallbackCommand that is currently created within
> CommandDispatcher.include()?
> Do you mean that the RequestDispatcherCallback should be
wrapped, and not
> the
> CallbackCommand?
> So we'd have as above except:
> Object ret = callback.doCallback(...);
> > // Fire event after
>
> > return ret;
>
> > }
>
> > }
>
> And the CallbackCommand would be created as:
> new CallbackCommand(targetServletContext, wrapper, handback);
> instead of
> new CallbackCommand(targetServletContext, cmd, handback);
> > and change
>
> > result = CommandServlet.include(servletPath, req, resp,
cmd,
> > targetServletContext);
>
> and this would stay as is
> > to
>
> > result = CommandServlet.include(servletPath, req, resp,
wrapper,
> > targetServletContext);
>
> > > Ken
> >
>
> > > ----- Original Message -----
> >
>
> > > > From: "Julien Viet" <
julien(a)julienviet.com >
> > >
> >
>
> > > > To: "Ken Finnigan" < kfinniga(a)redhat.com >
> > >
> >
>
> > > > Cc: gatein-dev(a)lists.jboss.org
> > >
> >
>
> > > > Sent: Wednesday, April 17, 2013 9:35:32 AM
> > >
> >
>
> > > > Subject: Re: [gatein-dev] CDI Injection for GateIn
> > >
> >
>
> > > > Hi Ken,
> > >
> >
>
> > > > there are a few things I would like to discuss
about the PR.
> > >
> >
>
> > > > The main thing I want to understand is when the
"fire" event should
> > > > be
> > > > fired
> > > > when the dispatch occurs.
> > >
> >
>
> > > > In the new TomcatCommandDispatcher I see it is
done in the include
> > > > method
> > > > before the dispatch is effectively done (i.e before the servlet
> > > > container
> > > > performs the dispatch).
> > >
> >
>
> > > > What about having it done after the dispatch is
really done ? (which
> > > > would
> > > > avoid to switch thread context classloader as Tomcat would have done
> > > > that
> > > > already).
> > >
> >
>
> > > > On Apr 16, 2013, at 9:36 PM, Ken Finnigan <
kfinniga(a)redhat.com >
> > > > wrote:
> > >
> >
>
> > > > > Julien,
> > > >
> > >
> >
>
> > > > > Here's the changes for WCI to enable
servlet request events:
> > > > > https://github.com/gatein/gatein-wci/pull/11
> > > >
> > >
> >
>
> > > > > If everything looks ok I'll merge
tomorrow and release a new WCI.
> > > >
> > >
> >
>
> > > > > Ken
> > > >
> > >
> >
>
> > > > > ----- Original Message -----
> > > >
> > >
> >
>
> > > > > > From: "Julien Viet" <
julien(a)julienviet.com >
> > > > >
> > > >
> > >
> >
>
> > > > > > To: "Ken Finnigan" < kfinniga(a)redhat.com >
> > > > >
> > > >
> > >
> >
>
> > > > > > Cc: gatein-dev(a)lists.jboss.org
> > > > >
> > > >
> > >
> >
>
> > > > > > Sent: Monday, April 15, 2013 11:05:16 AM
> > > > >
> > > >
> > >
> >
>
> > > > > > Subject: Re: [gatein-dev] CDI Injection for GateIn
> > > > >
> > > >
> > >
> >
>
> > > > > > On Apr 15, 2013, at 4:11 PM, Ken
Finnigan < kfinniga(a)redhat.com >
> > > > > > wrote:
> > > > >
> > > >
> > >
> >
>
> > > > > > > All,
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > The work to have CDI injection
into portlets and filters is
> > > > > > > almost
> > > > > > > complete,
> > > > > > > but a difference has been discovered between AS7 and
Tomcat.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > In essence, the
ApplicationDispatcher for Tomcat never fires
> > > > > > > the
> > > > > > > ServletRequestEvent on includes, but the
ApplicationDispatcher
> > > > > > > in
> > > > > > > AS7
> > > > > > > has
> > > > > > > been modified to fire the event when the current TCCL
is
> > > > > > > different
> > > > > > > to
> > > > > > > the
> > > > > > > classloader on the Context.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > wouldn't it be better to do it when
there is an include between
> > > > > > context
> > > > > > ?
> > > > > > what is the difference ?
> > > > >
> > > >
> > >
> >
>
> > > > > > > I'm proposing that in the
Tomcat specific bits of WCI, I create
> > > > > > > an
> > > > > > > extended
> > > > > > > CommandServlet that performs the same check and fires
the
> > > > > > > events
> > > > > > > as
> > > > > > > desired.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > yes, can you add a test for it so we
have guarantees that it will
> > > > > > not
> > > > > > break
> > > > > > later ?
> > > > >
> > > >
> > >
> >
>
> > > > > > > Any concerns or better approaches
are most welcome.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > one remark: I think we are reaching
application spec boundaries.
> > > > > > This
> > > > > > kind
> > > > > > of
> > > > > > stuff should be brought to the Portlet 3.0 EG so we can
make it
> > > > > > specified
> > > > > > in
> > > > > > the Servlet spec.
> > > > >
> > > >
> > >
> >
>
> > > > > > > Ken
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > ========================
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > Senior Software Engineer / JBoss Enterprise Middleware
Red Hat,
> > > > > > > Inc.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > >
_______________________________________________
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > gatein-dev mailing list
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > gatein-dev(a)lists.jboss.org
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > https://lists.jboss.org/mailman/listinfo/gatein-dev
> > > > > >
> > > > >
> > > >
> > >
> >
>
1:24 p.m.
cosmetic issue : you are not using the legacy formatting as WCI has not yet been migrated
to new code layout.
On Apr 17, 2013, at 7:25 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Have updated the PR with the suggested changes:
https://github.com/gatein/gatein-wci/pull/11/files
If all is ok, will merge and do release
Thanks
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 11:22:37 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
sorry I was not clear and I even confused myself :-) :
replacing
CallbackCommand cmd = new CallbackCommand(targetServletContext, callback,
handback);
by
CallbackCommand cmd = new CallbackCommand(targetServletContext, callback,
handback) {
@Override
public Object execute(HttpServletRequest req, HttpServletResponse resp) throws
ServletException, IOException
{
// Fire before
Object ret = super.execute(req, resp);
// Fire after
return ret;
}
};
is what I meant.
On Apr 17, 2013, at 4:44 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 10:08:24 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 17, 2013, at 3:58 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Are you suggesting to fire the events inside CommandServlet.include() around the
following call?
switcher.include(request, response);
yes
My original plan was to perform it within that method for Tomcat, but with the way
CommandServlet is written, I wasn't sure that it would be easy to extend it to only
replace the bits I needed, which is why I created a CommandDispatcher instead.
I was thinking at wrapping the callback with a call that perform the event firing:
RequestDispatchCallback wrapper = new RequestDispatchCallback() {
public Object doCallback(
ServletContext dispatchedServletContext,
HttpServletRequest dispatchedRequest,
HttpServletResponse dispatchedResponse,
Object handback) throws ServletException, IOException {
// Fire event before
Object ret = cmd.doCallback(dispatchedServletContext, dispatchedRequest,
dispatchedResponse, handback);
Is cmd the CallbackCommand that is currently created within CommandDispatcher.include()?
Do you mean that the RequestDispatcherCallback should be wrapped, and not the
CallbackCommand?
So we'd have as above except:
Object ret = callback.doCallback(...);
// Fire event after
return ret;
}
}
And the CallbackCommand would be created as:
new CallbackCommand(targetServletContext, wrapper, handback);
instead of
new CallbackCommand(targetServletContext, cmd, handback);
and change
result = CommandServlet.include(servletPath, req, resp, cmd,
targetServletContext);
and this would stay as is
to
result = CommandServlet.include(servletPath, req, resp, wrapper,
targetServletContext);
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 9:35:32 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
Hi Ken,
there are a few things I would like to discuss about the PR.
The main thing I want to understand is when the "fire" event should be fired
when the dispatch occurs.
In the new TomcatCommandDispatcher I see it is done in the include method before the
dispatch is effectively done (i.e before the servlet container performs the dispatch).
What about having it done after the dispatch is really done ? (which would avoid to
switch thread context classloader as Tomcat would have done that already).
On Apr 16, 2013, at 9:36 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Julien,
Here's the changes for WCI to enable servlet request events:
https://github.com/gatein/gatein-wci/pull/11
If everything looks ok I'll merge tomorrow and release a new WCI.
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Monday, April 15, 2013 11:05:16 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 15, 2013, at 4:11 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
All,
The work to have CDI injection into portlets and filters is almost complete, but a
difference has been discovered between AS7 and Tomcat.
In essence, the ApplicationDispatcher for Tomcat never fires the ServletRequestEvent on
includes, but the ApplicationDispatcher in AS7 has been modified to fire the event when
the current TCCL is different to the classloader on the Context.
wouldn't it be better to do it when there is an include between context ? what is the
difference ?
I'm proposing that in the Tomcat specific bits of WCI, I create an extended
CommandServlet that performs the same check and fires the events as desired.
yes, can you add a test for it so we have guarantees that it will not break later ?
Any concerns or better approaches are most welcome.
one remark: I think we are reaching application spec boundaries. This kind of stuff
should be brought to the Portlet 3.0 EG so we can make it specified in the Servlet spec.
Ken
========================
Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
_______________________________________________
gatein-dev mailing list
gatein-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/gatein-dev
2:02 p.m.
Updated the formatting.
Ken
----- Original Message -----
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 2:24:37 PM
Subject: Re: [gatein-dev] CDI Injection for GateIn
cosmetic issue : you are not using the legacy formatting as WCI has
not yet
been migrated to new code layout.
On Apr 17, 2013, at 7:25 PM, Ken Finnigan < kfinniga(a)redhat.com
> wrote:
> Have updated the PR with the suggested changes:
> https://github.com/gatein/gatein-wci/pull/11/files
> If all is ok, will merge and do release
> Thanks
> Ken
> ----- Original Message -----
> > From: "Julien Viet" < julien(a)julienviet.com
>
>
> > To: "Ken Finnigan" < kfinniga(a)redhat.com >
>
> > Cc: gatein-dev(a)lists.jboss.org
>
> > Sent: Wednesday, April 17, 2013 11:22:37 AM
>
> > Subject: Re: [gatein-dev] CDI Injection for GateIn
>
> > sorry I was not clear and I even confused myself :-) :
>
> > replacing
>
> > CallbackCommand cmd = new
CallbackCommand(targetServletContext, callback,
> > handback);
>
> > by
>
> > CallbackCommand cmd = new
CallbackCommand(targetServletContext, callback,
> > handback) {
>
> > @Override
>
> > public Object execute(HttpServletRequest req, HttpServletResponse resp)
> > throws ServletException, IOException
>
> > {
>
> > // Fire before
>
> > Object ret = super.execute(req, resp);
>
> > // Fire after
>
> > return ret;
>
> > }
>
> > };
>
> > is what I meant.
>
> > On Apr 17, 2013, at 4:44 PM, Ken Finnigan <
kfinniga(a)redhat.com > wrote:
>
> > > > From: "Julien Viet" <
julien(a)julienviet.com >
> > >
> >
>
> > > > To: "Ken Finnigan" < kfinniga(a)redhat.com >
> > >
> >
>
> > > > Cc: gatein-dev(a)lists.jboss.org
> > >
> >
>
> > > > Sent: Wednesday, April 17, 2013 10:08:24 AM
> > >
> >
>
> > > > Subject: Re: [gatein-dev] CDI Injection for GateIn
> > >
> >
>
> > > > On Apr 17, 2013, at 3:58 PM, Ken Finnigan <
kfinniga(a)redhat.com >
> > > > wrote:
> > >
> >
>
> > > > > Are you suggesting to fire the events
inside
> > > > > CommandServlet.include()
> > > > > around
> > > > > the following call?
> > > >
> > >
> >
>
> > > > > switcher.include(request, response);
> > > >
> > >
> >
>
> > > > yes
> > >
> >
>
> > > > > My original plan was to perform it within
that method for Tomcat,
> > > > > but
> > > > > with
> > > > > the way CommandServlet is written, I wasn't sure that it
would be
> > > > > easy
> > > > > to
> > > > > extend it to only replace the bits I needed, which is why I
created
> > > > > a
> > > > > CommandDispatcher instead.
> > > >
> > >
> >
>
> > > > I was thinking at wrapping the callback with a
call that perform the
> > > > event
> > > > firing:
> > >
> >
>
> > > > RequestDispatchCallback wrapper = new
RequestDispatchCallback() {
> > >
> >
>
> > > > public Object doCallback(
> > >
> >
>
> > > > ServletContext dispatchedServletContext,
> > >
> >
>
> > > > HttpServletRequest dispatchedRequest,
> > >
> >
>
> > > > HttpServletResponse dispatchedResponse,
> > >
> >
>
> > > > Object handback) throws ServletException, IOException {
> > >
> >
>
> > > > // Fire event before
> > >
> >
>
> > > > Object ret = cmd.doCallback(dispatchedServletContext,
> > > > dispatchedRequest,
> > > > dispatchedResponse, handback);
> > >
> >
>
> > > Is cmd the CallbackCommand that is currently created
within
> > > CommandDispatcher.include()?
> >
>
> > > Do you mean that the RequestDispatcherCallback should
be wrapped, and
> > > not
> > > the
> > > CallbackCommand?
> >
>
> > > So we'd have as above except:
> >
>
> > > Object ret = callback.doCallback(...);
> >
>
> > > > // Fire event after
> > >
> >
>
> > > > return ret;
> > >
> >
>
> > > > }
> > >
> >
>
> > > > }
> > >
> >
>
> > > And the CallbackCommand would be created as:
> >
>
> > > new CallbackCommand(targetServletContext, wrapper,
handback);
> >
>
> > > instead of
> >
>
> > > new CallbackCommand(targetServletContext, cmd,
handback);
> >
>
> > > > and change
> > >
> >
>
> > > > result = CommandServlet.include(servletPath, req,
resp, cmd,
> > > > targetServletContext);
> > >
> >
>
> > > and this would stay as is
> >
>
> > > > to
> > >
> >
>
> > > > result = CommandServlet.include(servletPath, req,
resp, wrapper,
> > > > targetServletContext);
> > >
> >
>
> > > > > Ken
> > > >
> > >
> >
>
> > > > > ----- Original Message -----
> > > >
> > >
> >
>
> > > > > > From: "Julien Viet" <
julien(a)julienviet.com >
> > > > >
> > > >
> > >
> >
>
> > > > > > To: "Ken Finnigan" < kfinniga(a)redhat.com >
> > > > >
> > > >
> > >
> >
>
> > > > > > Cc: gatein-dev(a)lists.jboss.org
> > > > >
> > > >
> > >
> >
>
> > > > > > Sent: Wednesday, April 17, 2013 9:35:32 AM
> > > > >
> > > >
> > >
> >
>
> > > > > > Subject: Re: [gatein-dev] CDI Injection for GateIn
> > > > >
> > > >
> > >
> >
>
> > > > > > Hi Ken,
> > > > >
> > > >
> > >
> >
>
> > > > > > there are a few things I would like to
discuss about the PR.
> > > > >
> > > >
> > >
> >
>
> > > > > > The main thing I want to understand is
when the "fire" event
> > > > > > should
> > > > > > be
> > > > > > fired
> > > > > > when the dispatch occurs.
> > > > >
> > > >
> > >
> >
>
> > > > > > In the new TomcatCommandDispatcher I
see it is done in the
> > > > > > include
> > > > > > method
> > > > > > before the dispatch is effectively done (i.e before the
servlet
> > > > > > container
> > > > > > performs the dispatch).
> > > > >
> > > >
> > >
> >
>
> > > > > > What about having it done after the
dispatch is really done ?
> > > > > > (which
> > > > > > would
> > > > > > avoid to switch thread context classloader as Tomcat would
have
> > > > > > done
> > > > > > that
> > > > > > already).
> > > > >
> > > >
> > >
> >
>
> > > > > > On Apr 16, 2013, at 9:36 PM, Ken
Finnigan < kfinniga(a)redhat.com >
> > > > > > wrote:
> > > > >
> > > >
> > >
> >
>
> > > > > > > Julien,
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > Here's the changes for WCI to
enable servlet request events:
> > > > > > > https://github.com/gatein/gatein-wci/pull/11
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > If everything looks ok I'll
merge tomorrow and release a new
> > > > > > > WCI.
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > Ken
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > ----- Original Message -----
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > From: "Julien Viet"
< julien(a)julienviet.com >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > To: "Ken Finnigan" <
kfinniga(a)redhat.com >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > Cc: gatein-dev(a)lists.jboss.org
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > Sent: Monday, April 15, 2013 11:05:16 AM
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > Subject: Re: [gatein-dev] CDI Injection for
GateIn
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > On Apr 15, 2013, at 4:11 PM,
Ken Finnigan <
> > > > > > > > kfinniga(a)redhat.com
> > > > > > > > >
> > > > > > > > wrote:
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > All,
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > The work to have CDI
injection into portlets and filters is
> > > > > > > > > almost
> > > > > > > > > complete,
> > > > > > > > > but a difference has been discovered between
AS7 and
> > > > > > > > > Tomcat.
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > In essence, the
ApplicationDispatcher for Tomcat never
> > > > > > > > > fires
> > > > > > > > > the
> > > > > > > > > ServletRequestEvent on includes, but the
> > > > > > > > > ApplicationDispatcher
> > > > > > > > > in
> > > > > > > > > AS7
> > > > > > > > > has
> > > > > > > > > been modified to fire the event when the
current TCCL is
> > > > > > > > > different
> > > > > > > > > to
> > > > > > > > > the
> > > > > > > > > classloader on the Context.
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > wouldn't it be better to
do it when there is an include
> > > > > > > > between
> > > > > > > > context
> > > > > > > > ?
> > > > > > > > what is the difference ?
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > I'm proposing that
in the Tomcat specific bits of WCI, I
> > > > > > > > > create
> > > > > > > > > an
> > > > > > > > > extended
> > > > > > > > > CommandServlet that performs the same check
and fires the
> > > > > > > > > events
> > > > > > > > > as
> > > > > > > > > desired.
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > yes, can you add a test for
it so we have guarantees that it
> > > > > > > > will
> > > > > > > > not
> > > > > > > > break
> > > > > > > > later ?
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > Any concerns or better
approaches are most welcome.
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > one remark: I think we are
reaching application spec
> > > > > > > > boundaries.
> > > > > > > > This
> > > > > > > > kind
> > > > > > > > of
> > > > > > > > stuff should be brought to the Portlet 3.0 EG so
we can make
> > > > > > > > it
> > > > > > > > specified
> > > > > > > > in
> > > > > > > > the Servlet spec.
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > Ken
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > >
========================
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > Senior Software Engineer / JBoss Enterprise
Middleware Red
> > > > > > > > > Hat,
> > > > > > > > > Inc.
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > >
_______________________________________________
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > gatein-dev mailing list
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > > gatein-dev(a)lists.jboss.org
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
> > > > > > > > >
https://lists.jboss.org/mailman/listinfo/gatein-dev
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
2:33 p.m.
fantastic
On Apr 17, 2013, at 9:02 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Updated the formatting.
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 2:24:37 PM
Subject: Re: [gatein-dev] CDI Injection for GateIn
cosmetic issue : you are not using the legacy formatting as WCI has not yet been migrated
to new code layout.
On Apr 17, 2013, at 7:25 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Have updated the PR with the suggested changes:
https://github.com/gatein/gatein-wci/pull/11/files
If all is ok, will merge and do release
Thanks
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 11:22:37 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
sorry I was not clear and I even confused myself :-) :
replacing
CallbackCommand cmd = new CallbackCommand(targetServletContext, callback,
handback);
by
CallbackCommand cmd = new CallbackCommand(targetServletContext, callback,
handback) {
@Override
public Object execute(HttpServletRequest req, HttpServletResponse resp) throws
ServletException, IOException
{
// Fire before
Object ret = super.execute(req, resp);
// Fire after
return ret;
}
};
is what I meant.
On Apr 17, 2013, at 4:44 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 10:08:24 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 17, 2013, at 3:58 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Are you suggesting to fire the events inside CommandServlet.include() around the
following call?
switcher.include(request, response);
yes
My original plan was to perform it within that method for Tomcat, but with the way
CommandServlet is written, I wasn't sure that it would be easy to extend it to only
replace the bits I needed, which is why I created a CommandDispatcher instead.
I was thinking at wrapping the callback with a call that perform the event firing:
RequestDispatchCallback wrapper = new RequestDispatchCallback() {
public Object doCallback(
ServletContext dispatchedServletContext,
HttpServletRequest dispatchedRequest,
HttpServletResponse dispatchedResponse,
Object handback) throws ServletException, IOException {
// Fire event before
Object ret = cmd.doCallback(dispatchedServletContext, dispatchedRequest,
dispatchedResponse, handback);
Is cmd the CallbackCommand that is currently created within CommandDispatcher.include()?
Do you mean that the RequestDispatcherCallback should be wrapped, and not the
CallbackCommand?
So we'd have as above except:
Object ret = callback.doCallback(...);
// Fire event after
return ret;
}
}
And the CallbackCommand would be created as:
new CallbackCommand(targetServletContext, wrapper, handback);
instead of
new CallbackCommand(targetServletContext, cmd, handback);
and change
result = CommandServlet.include(servletPath, req, resp, cmd,
targetServletContext);
and this would stay as is
to
result = CommandServlet.include(servletPath, req, resp, wrapper,
targetServletContext);
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Wednesday, April 17, 2013 9:35:32 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
Hi Ken,
there are a few things I would like to discuss about the PR.
The main thing I want to understand is when the "fire" event should be fired
when the dispatch occurs.
In the new TomcatCommandDispatcher I see it is done in the include method before the
dispatch is effectively done (i.e before the servlet container performs the dispatch).
What about having it done after the dispatch is really done ? (which would avoid to
switch thread context classloader as Tomcat would have done that already).
On Apr 16, 2013, at 9:36 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
Julien,
Here's the changes for WCI to enable servlet request events:
https://github.com/gatein/gatein-wci/pull/11
If everything looks ok I'll merge tomorrow and release a new WCI.
Ken
From: "Julien Viet" <julien(a)julienviet.com>
To: "Ken Finnigan" <kfinniga(a)redhat.com>
Cc: gatein-dev(a)lists.jboss.org
Sent: Monday, April 15, 2013 11:05:16 AM
Subject: Re: [gatein-dev] CDI Injection for GateIn
On Apr 15, 2013, at 4:11 PM, Ken Finnigan <kfinniga(a)redhat.com> wrote:
All,
The work to have CDI injection into portlets and filters is almost complete, but a
difference has been discovered between AS7 and Tomcat.
In essence, the ApplicationDispatcher for Tomcat never fires the ServletRequestEvent on
includes, but the ApplicationDispatcher in AS7 has been modified to fire the event when
the current TCCL is different to the classloader on the Context.
wouldn't it be better to do it when there is an include between context ? what is the
difference ?
I'm proposing that in the Tomcat specific bits of WCI, I create an extended
CommandServlet that performs the same check and fires the events as desired.
yes, can you add a test for it so we have guarantees that it will not break later ?
Any concerns or better approaches are most welcome.
one remark: I think we are reaching application spec boundaries. This kind of stuff
should be brought to the Portlet 3.0 EG so we can make it specified in the Servlet spec.
Ken
========================
Senior Software Engineer / JBoss Enterprise Middleware Red Hat, Inc.
_______________________________________________
gatein-dev mailing list
gatein-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/gatein-dev
4550
days inactive
4552
days old
14 comments
2 participants
participants (2)
-
Julien Viet -
Ken Finnigan