cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Liu, Jervis" <j...@iona.com>
Subject RE: Proposal for chaning CXF Interceptor APIs. WAS: RE: When should we close the handlers in CXF?
Date Tue, 23 Jan 2007 07:53:34 GMT
Hi Eoghan, you are right in saying that we can not achieve exact same execution path by using
close() as opposed to using doInterceptInSubChain(), i.e. "close() can only be invoked after
the complete chain has been traversed, whereas finishSubChain() could be called in a non-terminal
interceptor". However, this extra feature provided by the subChain is never used in CXF, and
I believe it would be a bad design if we really have to go this far to use this feature. Lets
take a closer look into the reason why we need to use the sub-chain reentrance in the first
place. 
 
Currently we have 5 interceptors that using subchain
 
A. MessageSenderInterceptor:  call conduit.close(message) after the complete chain has been
traversed
 
B. AttachementOutInterceptor: write http header, attachement boundary etc, then complete the
rest of the chain. After subchain invocation, flush out attachement.
 
C. SoapHandlerInterceptor: replace outputStream with a cached outputstream to avoid any flush
to the socket prior to the end of SoapHandlerInterceptor, then enter into subchain. After
subchain invocation, create a SAAJ SoapMessage from the cached outputstrem, call registered
JAX-WS SoapHandlers.
 
D. StaxOutInterceptor: close XMLStreamWriter after the complete chain has been traversed
 
E. SoapOutInterceptor: Write the end element of Soap Body and Envelope after the complete
chain has been traversed
 
As far as I can see, I do not see a case that we need to do any clean up job right after a
certain non-terminal interceptor has been called, instead, all the post-subchain logic executed
by interceptors listed above can be fit into a method (a name called postHandlerMessage()
probably make more sense, see below) that can be guaranteed to called after the complete chain
has been traversed.
 
Please note all CXF interceptor are designed to be stateless, in this case, a close() method
is meaningless as there is no resource needs to be released. Thus I suggest we name this method
to postHandlerMessage() to accommodate what we need to do for any post-subchain logic. I will
start another thread to discuss whether or not we still want a close() method. 
 
Cheers,
Jervis
 
 
________________________________

From: Glynn, Eoghan [mailto:eoghan.glynn@iona.com]
Sent: Mon 1/22/2007 11:59 PM
To: cxf-dev@incubator.apache.org
Subject: RE: Proposal for chaning CXF Interceptor APIs. WAS: RE: When should we close the
handlers in CXF?




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 <https://bosgate2.iona.com/http/0/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 <https://bosgate2.iona.com/http/0/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>
> > <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>
> > <https://bosgate2.iona.com/http/0/netzooid.com/blog>
> >
> >
> >
>
>
>



Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message