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:[EMAIL PROTECTED]
> Sent: 22 January 2007 07:16
> To: [email protected]; [email protected]
> 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/[EMAIL PROTECTED]
> ublin.emea.iona.com%3e
>
> ________________________________
>
> From: Glynn, Eoghan [mailto:[EMAIL PROTECTED]
> Sent: Fri 1/19/2007 6:41 PM
> To: [email protected]
> Subject: RE: When should we close the handlers in CXF?
>
>
>
>
>
> > -----Original Message-----
> > From: Liu, Jervis [mailto:[EMAIL PROTECTED]
> > Sent: 19 January 2007 07:34
> > To: [email protected]; [email protected]
> > 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/%>
> [EMAIL PROTECTED]
> .iona.com%
> 3e
>
> > Cheers,
> > Jervis
> >
> > ________________________________
> >
> > From: Dan Diephouse [mailto:[EMAIL PROTECTED]
> > Sent: Thu 1/18/2007 9:55 PM
> > To: [email protected]
> > 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 <[EMAIL PROTECTED]> wrote:
> > >
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Unreal Jiang [mailto:[EMAIL PROTECTED]
> > > > Sent: 18 January 2007 11:44
> > > > To: [email protected]
> > > > 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" <[EMAIL PROTECTED]> 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:[EMAIL PROTECTED]
> > > > Sent: Wed 1/17/2007 9:42 PM
> > > > To: [email protected]
> > > > Subject: RE: When should we close the handlers in CXF?
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Glynn, Eoghan [mailto:[EMAIL PROTECTED]
> > > > > Sent: 17 January 2007 12:34
> > > > > To: [email protected]
> > > > > 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>
> >
> >
> >
>
>
>
