cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Liu, Jervis" <j...@iona.com>
Subject RE: When should we close the handlers in CXF?
Date Mon, 22 Jan 2007 07:31:36 GMT
Comments in-line

________________________________

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?


[Jervis]. I am actually proposing that we use the close() method to remove the use of chain
reentrance, thus we do not need to jump back from a sub-chain to the main chain anymore. Currently,
this jump happens at the end of sub-chain or by calling PhaseInterceptorChain.finishSubChain()
explicitly.


> Cheers,
> Eoghan

[1]
http://mail-archives.apache.org/mod_mbox/incubator-cxf-dev/200611.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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message