cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Glynn, Eoghan" <eoghan.gl...@iona.com>
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 GMT

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] 
> 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: 
> When should we close the handlers in CXF?
> 
> Hi, I would like to summarize what we have been discussed in 
> this thread (including Eoghan's proposal posted last Oct [1]) 
> regarding Interceptor API changes. Any comments would be appreciated. 
>  
> Currently our Interceptor APIs look like below:
>  
>  public interface Interceptor<T extends Message> {
>      void handleMessage(T message) throws Fault;
>      void handleFault(T message);
> }
>  
> Also in the interceptor chain, we have a notion of sub-chain 
> or interceptor chain reentrance by calling 
> message.getInterceptorChain().doIntercept(message) or 
> message.getInterceptorChain().doInterceptInSubChain(message). 
>  
> The main issues we have with the current implementation are:
>  
> 1. Fault handling. See Eoghag's email [1]
>  
> 2. Sub-chain reentrance. See previous discussion in this thread.
>  
> We propose to change Interceptor API as below:
>  
>  public interface Interceptor<T extends Message> {
>      void handleMessage(T message) throws Fault;
>      void handleFault(T message);
>      void close(T message);   
> }
>  
> handleFault(T message) method is used to process fault 
> message (which is done by handleMessage() in fault-chain currently).
>  
> close(T message) method is called on a reverse direction at 
> the end of interceptor chain or when a fault or exception 
> occurs. Take the fault handling case as an example, below is 
> how handleFault and close work together
>  
> when a fault occurs on the in-chain, we unwind the current 
> chain by calling close() on each previously traversed 
> interceptor and then jump to the out-fault-chain, calling 
> handleFault() on each interceptor with the fault message. 
>  
> Close method is also used to remove the sub-chain reentrance. 
> See the SOAPHandlerInterceptor example I posted previously.
>  
> Cheers,
> Jervis 
>  
> [1] 
> http://mail-archives.apache.org/mod_mbox/incubator-cxf-dev/200
> 611.mbox/%3cFA1787F64A095C4090E76EBAF8B183E071FADE@emea-ems1.d
> ublin.emea.iona.com%3e
> 
> ________________________________
> 
> 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?
> 
> 
> 
> 
> 
> > -----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,
> > 
> > Did we have any previous discussions on why we wont want a 
> JAX-WS like 
> > APIs for interceptor chain?
> 
> Yes, I've brought this up a few times before, e.g. see point 
> #4 in this cxf-dev post [1] from Nov 10th last.
> 
> My problem with the CXF Interceptor.handleFault() method is 
> that it actually has semantics closer to the JAX-WS 
> Handler.close(), and is gratuitously different from the 
> similarly-named Handler.handleFault() method .
> 
> I've argued that is needlessly confusing for developers who 
> use both the CXF Interceptor and JAX-WS Handler APIs, and we 
> should in fact model the former on the latter.
> 
> > I.e., an Interceptor
> > interface should look like below:
> > 
> > public interface Interceptor<T extends Message> {
> >     void handleMessage(T message) throws Fault;
> >     void handleFault(T message);
> >     void close(T message);   
> > }
> 
> If we were to include *both* handleFault() and close() on 
> Interceptor, then the current Interceptor.handleFault() 
> semantics would have to change.
> 
> My proposal in the above referenced mail was to change 
> handleFault() semantics as follows:
> 
> "[currently] when a fault occurs on the in-chain, we unwind 
> the current chain by calling
> handleFault() on each previously traversed interceptor and 
> then jump to the out-fault-chain, calling handleMessage() on 
> each interceptor with the fault message. I'd propose changing 
> this to drop the unwind, instead call faultMessage() on each 
> interceptor in the out-fault-chain."
> 
> I suppose the unwind wouldn't be totally dropped, just achieved using
> Interceptor.close() instead of handleFault().
> 
> > I see the possibility of removing subchains(interceptor chain
> > re-entrance) from the interceptor chain by using close method. I am 
> > not saying sub-chain and the reentrance is that bad, but it 
> does bring 
> > some confusion to understand the interceptor flow and it 
> also brings 
> > unnecessary complexity such as handling exceptions thrown from a 
> > subchain and how to return back from subchain. Take the 
> > SOAPHandlerInterceptor as example, it looks like below now with a 
> > close method:
> > 
> >     private OutputStream oldStream;
> >     ......
> >     public void handleMessage(SoapMessage message) {
> >         if (getInvoker(message).getProtocolHandlers().isEmpty()) {
> >             return;
> >         }
> >         if (getInvoker(message).isOutbound()) {
> >             oldStream = message.getContent(OutputStream.class);
> >             CachedStream cs = new CachedStream();
> >             message.setContent(OutputStream.class, cs);
> >         } else {
> >            .....
> >         }
> >     }
> >
> >     public void close(SoapMessage message) {
> >         if (getInvoker(message).isOutbound()) {
> >             super.handleMessage(message);
> >             try {
> >                 CachedStream cs =
> > (CachedStream)message.getContent(OutputStream.class);
> >                 // Stream SOAPMessage back to output stream if 
> > necessary
> >                 SOAPMessage soapMessage = 
> > message.getContent(SOAPMessage.class);
> >                 if (soapMessage != null) {
> >                     soapMessage.writeTo(oldStream);
> >                 } else {
> >                     cs.flush();
> >                     AbstractCachedOutputStream csnew =
> > (AbstractCachedOutputStream) message
> >                             .getContent(OutputStream.class);
> >                     AbstractCachedOutputStream.copyStream(csnew
> >                             .getInputStream(), oldStream, 
> 64 * 1024);
> >                     cs.close();
> >                 }
> >                 oldStream.flush();
> >                 message.setContent(OutputStream.class, oldStream);
> >             } catch (IOException ioe) {
> >                 throw new SoapFault(new 
> > org.apache.cxf.common.i18n.Message(
> >                         "SOAPHANDLERINTERCEPTOR_EXCEPTION",
> > BUNDLE), ioe,
> >                         message.getVersion().getSender());
> >             } catch (SOAPException soape) {
> >                 throw new SoapFault(new 
> > org.apache.cxf.common.i18n.Message(
> >                         "SOAPHANDLERINTERCEPTOR_EXCEPTION",
> > BUNDLE), soape,
> >                         message.getVersion().getSender());
> >             }
> >         }
> >     }
> >
> > 
> > We can do the same thing for MessageSenderInterceptor, 
> > StaxOutInteceptor and SoapOutInterceptor etc.
> > 
> > Does this look good to everyone, any thoughts?
> 
> 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 
> main chain?
> 
> If so, where is this jump done currently?
> 
> Cheers,
> Eoghan
> 
> [1]
> http://mail-archives.apache.org/mod_mbox/incubator-cxf-dev/200
> 611.mbox/% 
> <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
> 
> > 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 
> the chain 
> > is good.
> >
> > Another possibility is to add a close(Message) method to the 
> > Interceptor which gets called at the end of the chain. If 
> we did that 
> > I would think we might want to get rid of the handleFault method as 
> > cleanup could be done in close().
> > (Eoghan - I'm actually suggesting we move closer to the 
> JAX-WS APIs! 
> > ;-))
> >
> > 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 
> > > > handlers  but interceptors,  but if we use runnable object for 
> > > > 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 
> > > reflect the order in which the interceptors were 
> traversed. If you 
> > > 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 
> > > 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 
> > > SOAPHandlerInterceptor will actually need to submit a
> > terminal action.
> > > So with only a *single* terminal action concerned with closing 
> > > handlers, ordering shouldn't be an issue here.
> > >
> > > This is because the same HandlerChainInvoker instance is 
> shared by 
> > > 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 
> > > actually required to ensure that *all* the traversed handlers are 
> > > close()d in the correct order.
> > >
> > > So maybe the simplest approach would be be to submit the terminal 
> > > action in AbstractJAXWSHandlerInterceptor.getInvoker(), i.e.:
> > >
> > >     protected HandlerChainInvoker getInvoker(final T message) {
> > >         HandlerChainInvoker invoker =
> > >             message.getExchange().get(HandlerChainInvoker.class);
> > >         if (null == invoker) {
> > >             invoker = new
> > HandlerChainInvoker(binding.getHandlerChain(),
> > >                                               
> isOutbound(message));
> > >             message.getExchange().put(HandlerChainInvoker.class,
> > > invoker);
> > >
> > >             // submit a *single* terminal action for 
> entire handler 
> > > 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 
> the second 
> > > > 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 
> > > > > points are driven through a set of interceptors, each 
> 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 
> the chain.
> > > > > Similarly there's a *separate* LogicalHandlerInterceptor that 
> > > > > 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 
> > > > > existing wrapper interceptors. Otherwise we'd end up with for
> > > > example close()
> > > > > called prematurely on the SOAPHandlers *before* the 
> > > > > 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 
> responsible for 
> > > > > calling
> > > > > close() on all types of handler. This could be driven via a 
> > > > > pattern similar to the
> > > > > LogicalHandlerInterceptor.onCompletion() method (e.g. the new 
> > > > > interceptor walks back along the chain to find the 
> > > > > 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 
> with the 
> > > > InterceptorChain to be executed when the chain traversal is 
> > > > complete, e.g.
> > > >
> > > > public interface InterceptorChain {
> > > >     void addTerminalAction(Runnable r);
> > > >
> > > >     //...
> > > > }
> > > >
> > > > Or alternatively take the Runnable as a return value from 
> > > > 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 
> > <https://bosgate2.iona.com/http/0/envoisolutions.com>
> > <https://bosgate2.iona.com/http/0/envoisolutions.com>  | 
> > http://netzooid.com/blog 
> > <https://bosgate2.iona.com/http/0/netzooid.com/blog>
> > <https://bosgate2.iona.com/http/0/netzooid.com/blog>
> >
> >
> >
> 
> 
> 

Mime
View raw message