Mailing-List: contact cxf-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: cxf-dev@incubator.apache.org
Received-SPF: pass (herse.apache.org: domain of eoghan.glynn@iona.com
 designates 62.221.12.33 as permitted sender)
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="US-ASCII"
Content-Transfer-Encoding: quoted-printable
Subject: RE: Proposal for chaning CXF Interceptor APIs. WAS: RE: When should
 we close the handlers in CXF?
Date: Mon, 22 Jan 2007 15:59:17 -0000
Message-ID: <FA1787F64A095C4090E76EBAF8B183E071FB9A@owa-emea.iona.com>
Thread-Topic: Proposal for chaning CXF Interceptor APIs. WAS: RE: When should
 we close the handlers in CXF?
Thread-Index: Acc7CGe4BvJVV7StT6K/xViw/LolVQAjkYclAAdkJaAAjcwabAATioTA
From: "Glynn, Eoghan" <eoghan.glynn@iona.com>
To: <cxf-dev@incubator.apache.org>


Obviously I'd be +1 on clearing up the potential for confusion between
the faultMessage() semantics on JAX-WS Handler and CXF Interceptor.

However, I'm not 100% sure that the new Interceptor.close() method will
give us exactly the same semantics as the sub-chain stuff.

Consider the following sequence of interceptors:

A.handleMessage() -> action A
B.handleMessage() -> action B1
                     InterceptorChain.doInterceptInSubChain()
                     action B2
C.handleMessage() -> action C
D.handleMessage() -> action D
                     InterceptorChain.finishSubChain()
E.handleMessage() -> action E

So the sequence of actions would be {A, B1, C, D, B2, E}, right?

If we remodelled this logic using close() instead:

A.handleMessage() -> action A
B.handleMessage() -> action B1
B.close()         -> action B2
C.handleMessage() -> action C
D.handleMessage() -> action D
E.handleMessage() -> action E

Then the sequence is {A, B1, C, D, E, B2} which is obviously slightly
different ... i.e. B.close() is only invoked after the complete chain
has been traversed, whereas finishSubChain() could be called in a
non-terminal interceptor (i.e. D in the example above).

I'm not overly familiar with the sub-chain logic though, so I'm open to
correction on this.

Cheers,
Eoghan

> -----Original Message-----
> From: Liu, Jervis [mailto:jliu@iona.com]=20
> Sent: 22 January 2007 07:16
> To: cxf-dev@incubator.apache.org; cxf-dev@incubator.apache.org
> Subject: Proposal for chaning CXF Interceptor APIs. WAS: RE:=20
> When should we close the handlers in CXF?
>=20
> Hi, I would like to summarize what we have been discussed in=20
> this thread (including Eoghan's proposal posted last Oct [1])=20
> regarding Interceptor API changes. Any comments would be appreciated.=20
> =20
> Currently our Interceptor APIs look like below:
> =20
>  public interface Interceptor<T extends Message> {
>      void handleMessage(T message) throws Fault;
>      void handleFault(T message);
> }
> =20
> Also in the interceptor chain, we have a notion of sub-chain=20
> or interceptor chain reentrance by calling=20
> message.getInterceptorChain().doIntercept(message) or=20
> message.getInterceptorChain().doInterceptInSubChain(message).=20
> =20
> The main issues we have with the current implementation are:
> =20
> 1. Fault handling. See Eoghag's email [1]
> =20
> 2. Sub-chain reentrance. See previous discussion in this thread.
> =20
> We propose to change Interceptor API as below:
> =20
>  public interface Interceptor<T extends Message> {
>      void handleMessage(T message) throws Fault;
>      void handleFault(T message);
>      void close(T message);  =20
> }
> =20
> handleFault(T message) method is used to process fault=20
> message (which is done by handleMessage() in fault-chain currently).
> =20
> close(T message) method is called on a reverse direction at=20
> the end of interceptor chain or when a fault or exception=20
> occurs. Take the fault handling case as an example, below is=20
> how handleFault and close work together
> =20
> when a fault occurs on the in-chain, we unwind the current=20
> chain by calling close() on each previously traversed=20
> interceptor and then jump to the out-fault-chain, calling=20
> handleFault() on each interceptor with the fault message.=20
> =20
> Close method is also used to remove the sub-chain reentrance.=20
> See the SOAPHandlerInterceptor example I posted previously.
> =20
> Cheers,
> Jervis=20
> =20
> [1]=20
> http://mail-archives.apache.org/mod_mbox/incubator-cxf-dev/200
> 611.mbox/%3cFA1787F64A095C4090E76EBAF8B183E071FADE@emea-ems1.d
> ublin.emea.iona.com%3e
>=20
> ________________________________
>=20
> From: Glynn, Eoghan [mailto:eoghan.glynn@iona.com]
> Sent: Fri 1/19/2007 6:41 PM
> To: cxf-dev@incubator.apache.org
> Subject: RE: When should we close the handlers in CXF?
>=20
>=20
>=20
>=20
>=20
> > -----Original Message-----
> > From: Liu, Jervis [mailto:jliu@iona.com]
> > Sent: 19 January 2007 07:34
> > To: cxf-dev@incubator.apache.org; cxf-dev@incubator.apache.org
> > Subject: RE: When should we close the handlers in CXF?
> >
> > Hi,
> >=20
> > Did we have any previous discussions on why we wont want a=20
> JAX-WS like=20
> > APIs for interceptor chain?
>=20
> Yes, I've brought this up a few times before, e.g. see point=20
> #4 in this cxf-dev post [1] from Nov 10th last.
>=20
> My problem with the CXF Interceptor.handleFault() method is=20
> that it actually has semantics closer to the JAX-WS=20
> Handler.close(), and is gratuitously different from the=20
> similarly-named Handler.handleFault() method .
>=20
> I've argued that is needlessly confusing for developers who=20
> use both the CXF Interceptor and JAX-WS Handler APIs, and we=20
> should in fact model the former on the latter.
>=20
> > I.e., an Interceptor
> > interface should look like below:
> >=20
> > public interface Interceptor<T extends Message> {
> >     void handleMessage(T message) throws Fault;
> >     void handleFault(T message);
> >     void close(T message);  =20
> > }
>=20
> If we were to include *both* handleFault() and close() on=20
> Interceptor, then the current Interceptor.handleFault()=20
> semantics would have to change.
>=20
> My proposal in the above referenced mail was to change=20
> handleFault() semantics as follows:
>=20
> "[currently] when a fault occurs on the in-chain, we unwind=20
> the current chain by calling
> handleFault() on each previously traversed interceptor and=20
> then jump to the out-fault-chain, calling handleMessage() on=20
> each interceptor with the fault message. I'd propose changing=20
> this to drop the unwind, instead call faultMessage() on each=20
> interceptor in the out-fault-chain."
>=20
> I suppose the unwind wouldn't be totally dropped, just achieved using
> Interceptor.close() instead of handleFault().
>=20
> > I see the possibility of removing subchains(interceptor chain
> > re-entrance) from the interceptor chain by using close method. I am=20
> > not saying sub-chain and the reentrance is that bad, but it=20
> does bring=20
> > some confusion to understand the interceptor flow and it=20
> also brings=20
> > unnecessary complexity such as handling exceptions thrown from a=20
> > subchain and how to return back from subchain. Take the=20
> > SOAPHandlerInterceptor as example, it looks like below now with a=20
> > close method:
> >=20
> >     private OutputStream oldStream;
> >     ......
> >     public void handleMessage(SoapMessage message) {
> >         if (getInvoker(message).getProtocolHandlers().isEmpty()) {
> >             return;
> >         }
> >         if (getInvoker(message).isOutbound()) {
> >             oldStream =3D message.getContent(OutputStream.class);
> >             CachedStream cs =3D new CachedStream();
> >             message.setContent(OutputStream.class, cs);
> >         } else {
> >            .....
> >         }
> >     }
> >
> >     public void close(SoapMessage message) {
> >         if (getInvoker(message).isOutbound()) {
> >             super.handleMessage(message);
> >             try {
> >                 CachedStream cs =3D
> > (CachedStream)message.getContent(OutputStream.class);
> >                 // Stream SOAPMessage back to output stream if=20
> > necessary
> >                 SOAPMessage soapMessage =3D=20
> > message.getContent(SOAPMessage.class);
> >                 if (soapMessage !=3D null) {
> >                     soapMessage.writeTo(oldStream);
> >                 } else {
> >                     cs.flush();
> >                     AbstractCachedOutputStream csnew =3D
> > (AbstractCachedOutputStream) message
> >                             .getContent(OutputStream.class);
> >                     AbstractCachedOutputStream.copyStream(csnew
> >                             .getInputStream(), oldStream,=20
> 64 * 1024);
> >                     cs.close();
> >                 }
> >                 oldStream.flush();
> >                 message.setContent(OutputStream.class, oldStream);
> >             } catch (IOException ioe) {
> >                 throw new SoapFault(new=20
> > org.apache.cxf.common.i18n.Message(
> >                         "SOAPHANDLERINTERCEPTOR_EXCEPTION",
> > BUNDLE), ioe,
> >                         message.getVersion().getSender());
> >             } catch (SOAPException soape) {
> >                 throw new SoapFault(new=20
> > org.apache.cxf.common.i18n.Message(
> >                         "SOAPHANDLERINTERCEPTOR_EXCEPTION",
> > BUNDLE), soape,
> >                         message.getVersion().getSender());
> >             }
> >         }
> >     }
> >
> >=20
> > We can do the same thing for MessageSenderInterceptor,=20
> > StaxOutInteceptor and SoapOutInterceptor etc.
> >=20
> > Does this look good to everyone, any thoughts?
>=20
> Just so I'm sure I understand correctly, are you proposing to use
> close() to trigger a jump from the sub-chain back up to the=20
> main chain?
>=20
> If so, where is this jump done currently?
>=20
> Cheers,
> Eoghan
>=20
> [1]
> http://mail-archives.apache.org/mod_mbox/incubator-cxf-dev/200
> 611.mbox/%=20
> <https://bosgate2.iona.com/http/0/mail-archives.apache.org/mod
> _mbox/incubator-cxf-dev/200611.mbox/%>
> 3cFA1787F64A095C4090E76EBAF8B183E071FADE@emea-ems1.dublin.emea
> .iona.com%
> 3e
>=20
> > Cheers,
> > Jervis
> >
> > ________________________________
> >
> > From: Dan Diephouse [mailto:dan@envoisolutions.com]
> > Sent: Thu 1/18/2007 9:55 PM
> > To: cxf-dev@incubator.apache.org
> > Subject: Re: When should we close the handlers in CXF?
> >
> >
> >
> > I think the registering of actions to be run at the end of=20
> the chain=20
> > is good.
> >
> > Another possibility is to add a close(Message) method to the=20
> > Interceptor which gets called at the end of the chain. If=20
> we did that=20
> > I would think we might want to get rid of the handleFault method as=20
> > cleanup could be done in close().
> > (Eoghan - I'm actually suggesting we move closer to the=20
> JAX-WS APIs!=20
> > ;-))
> >
> > Thoughts?
> >
> > - Dan
> >
> > On 1/18/07, Glynn, Eoghan <eoghan.glynn@iona.com> wrote:
> > >
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Unreal Jiang [mailto:sinbad_jia@yahoo.com]
> > > > Sent: 18 January 2007 11:44
> > > > To: cxf-dev@incubator.apache.org
> > > > Subject: RE: When should we close the handlers in CXF?
> > > >
> > > > Hi Eoghan,
> > > >   I think those two approach are work fine.
> > > >
> > > >   The first approach is only for handlers process,
> > > >   The second approach can do some clean-up works not only for=20
> > > > handlers  but interceptors,  but if we use runnable object for=20
> > > > TerminalAction,  the order of handlers or interceptors
> > will be  hard
> > > > to ensure.
> > >
> > > In most cases, a FIFO ordering of the terminal actions
> > would be fine,
> > > i.e. the order in which the terminal actions are executed would=20
> > > reflect the order in which the interceptors were=20
> traversed. If you=20
> > > needed more fine-grained control, maybe
> > > InterceptorChain.addTerminalAction() could be replaced by
> > something like ...
> > >
> > > interface InterceptorChain {
> > >     List<Runnbale> getTerminalActions(); }
> > >
> > > ... so that the code submitting the terminal action could control=20
> > > ordering with respect to previously submitted terminal
> > actions. Going
> > > even further that this (e.g. something akin to the
> > > PhaseInterceptor.getBefore/After() business) would I think
> > be overkill
> > > without a specific ordering-sensitive usecase.
> > >
> > > However, a closer look at the JAX-WS HandlerChainInvoker
> > code suggests
> > > that only one of the LogicalHandlerInterceptor and=20
> > > SOAPHandlerInterceptor will actually need to submit a
> > terminal action.
> > > So with only a *single* terminal action concerned with closing=20
> > > handlers, ordering shouldn't be an issue here.
> > >
> > > This is because the same HandlerChainInvoker instance is=20
> shared by=20
> > > these two interceptors, and the code that calls
> > handleMessage/Fault()
> > > on the individual Handlers also adds each of these to a
> > separate list
> > > (closeHandlers) of handlers for which close() should be called.
> > >
> > > Only a single call to HandlerChainInvoker.mepComplete() is then=20
> > > actually required to ensure that *all* the traversed handlers are=20
> > > close()d in the correct order.
> > >
> > > So maybe the simplest approach would be be to submit the terminal=20
> > > action in AbstractJAXWSHandlerInterceptor.getInvoker(), i.e.:
> > >
> > >     protected HandlerChainInvoker getInvoker(final T message) {
> > >         HandlerChainInvoker invoker =3D
> > >             message.getExchange().get(HandlerChainInvoker.class);
> > >         if (null =3D=3D invoker) {
> > >             invoker =3D new
> > HandlerChainInvoker(binding.getHandlerChain(),
> > >                                              =20
> isOutbound(message));
> > >             message.getExchange().put(HandlerChainInvoker.class,
> > > invoker);
> > >
> > >             // submit a *single* terminal action for=20
> entire handler=20
> > > chain
> > >             message.getInterceptorChain().addTerminalAction(new
> > > Runnable() {
> > >                 public void run() {
> > >                     mepComplete(message);
> > >                 }
> > >             }
> > >         }
> > >         //...
> > >     }
> > >
> > > Cheers,
> > > Eoghan
> > >
> > > >   So I incline to  the second approach, but we should use
> > some other
> > > > way to instead of runnable object.
> > > >
> > > >   Regards
> > > >   Unreal
> > > >
> > > > "Liu, Jervis" <jliu@iona.com> wrote:I  would vote for=20
> the second=20
> > > > approach. When its there, we can probably use  the
> > similiar approach
> > > > to remove the sub-chain (interceptor chain
> > > > reentrance) wherever it is possible.
> > > >
> > > > ________________________________
> > > >
> > > > From: Glynn, Eoghan [mailto:eoghan.glynn@iona.com]
> > > > Sent: Wed 1/17/2007 9:42 PM
> > > > To: cxf-dev@incubator.apache.org
> > > > Subject: RE: When should we close the handlers in CXF?
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Glynn, Eoghan [mailto:eoghan.glynn@iona.com]
> > > > > Sent: 17 January 2007 12:34
> > > > > To: cxf-dev@incubator.apache.org
> > > > > Subject: RE: When should we close the handlers in CXF?
> > > > >
> > > > >
> > > > > Hi Unreal,
> > > > >
> > > > > One point to note is that all the other JAX-WS Handler touch=20
> > > > > points are driven through a set of interceptors, each=20
> wrapping a
> > > > chain of a
> > > > > particular Handler type (logical, protocol etc.).
> > > > >
> > > > > So for example the SOAPHanderInterceptor takes care of
> > calling the
> > > > > handleMessage/Fault() methods of any SOAPHandlers in=20
> the chain.
> > > > > Similarly there's a *separate* LogicalHandlerInterceptor that=20
> > > > > traverses the chain of LogicalHandlers. I'm guessing you
> > > > already know
> > > > > all this ...
> > > > >
> > > > > But the point is that it would be a good idea to maintain
> > > > the pattern
> > > > > of wrapper-interceptor calling out to JAX-WS Handler, and
> > > > obviously it
> > > > > would be badness to for example put this JAXWS-specific
> > > > logic into the
> > > > > ClientImpl code.
> > > > >
> > > > > However, because Handler.close() should only be called
> > at the very
> > > > > *end* of the interceptor chain tarversal, and because
> > we currently
> > > > > have
> > > > > *multiple* interceptors wrapping the JAX-WS Handler chains
> > > > of various
> > > > > types, the close() call should not be made from within the=20
> > > > > existing wrapper interceptors. Otherwise we'd end up with for
> > > > example close()
> > > > > called prematurely on the SOAPHandlers *before* the=20
> > > > > LogicalHandlers have even been traversed (inbound on
> > the client-side).
> > > > >
> > > > > So we'd need a *single* new wrapper interceptor, positioned
> > > > at the end
> > > > > of the in & fault-in interceptor chains, that's=20
> responsible for=20
> > > > > calling
> > > > > close() on all types of handler. This could be driven via a=20
> > > > > pattern similar to the
> > > > > LogicalHandlerInterceptor.onCompletion() method (e.g. the new=20
> > > > > interceptor walks back along the chain to find the=20
> > > > > LogicalHandlerInterceptor & SOAPHandlerInterceptor and calls
> > > > > onCompletion() on these).
> > > >
> > > > On second thoughts, maybe a cleaner may of doing this
> > would be allow
> > > > an interceptor to register some sort of terminal action=20
> with the=20
> > > > InterceptorChain to be executed when the chain traversal is=20
> > > > complete, e.g.
> > > >
> > > > public interface InterceptorChain {
> > > >     void addTerminalAction(Runnable r);
> > > >
> > > >     //...
> > > > }
> > > >
> > > > Or alternatively take the Runnable as a return value from=20
> > > > Interceptor.handleMessage/Fault().
> > > >
> > > > Then in the InterceptorChain impl, run all the
> > > > TerminalAction(s) from a finally block, e.g.
> > > >
> > > > public class PhaseInterceptorChain {
> > > >    public boolean doIntercept(Message m) {
> > > >        try {
> > > >            while (interceptorIterator.hasNext()) {
> > > >                interceptorIterator.next().handleMessage(m);
> > > >            }
> > > >        } finally {
> > > >            for (Runnable r : terminalActions) {
> > > >                r.run();
> > > >            }
> > > >        }
> > > >    }
> > > > }
> > > >
> > > > Then for example the
> > > > LogicalHandlerInterceptor.handleMessage() would end with
> > some logic
> > > > like:
> > > >
> > > >    if (isRequestor(message) && (isOneway(message) ||
> > > > !isOutbound(message))) {
> > > >       message.getInterceptorChain().addTerminalAction(new
> > Runnable() {
> > > >           public void run() {
> > > >               getInvoker(message).mepComplete(message);
> > > >           }
> > > >       }
> > > >    }
> > > >
> > > > Similarly for SOAPHandlerInterceptor etc.
> > > >
> > > > Cheers,
> > > > Eoghan
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > ---------------------------------
> > > > Cheap Talk? Check out Yahoo! Messenger's low PC-to-Phone
> > call rates.
> > > >
> > >
> >
> >
> >
> > --
> > Dan Diephouse
> > Envoi Solutions
> > http://envoisolutions.com=20
> > <https://bosgate2.iona.com/http/0/envoisolutions.com>
> > <https://bosgate2.iona.com/http/0/envoisolutions.com>  |=20
> > http://netzooid.com/blog=20
> > <https://bosgate2.iona.com/http/0/netzooid.com/blog>
> > <https://bosgate2.iona.com/http/0/netzooid.com/blog>
> >
> >
> >
>=20
>=20
>=20