Re: [gatein-dev] CDI Injection for GateIn

From: "Julien Viet" <julien(a)julienviet.com&gt; To: "Ken Finnigan" <kfinniga(a)redhat.com&gt; 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 > > > > > > > > > > > > > > > > > > > > >