Comments in-line ________________________________
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? [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/%> [EMAIL PROTECTED] 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> > > >
