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, 13 Feb 2007 03:28:23 GMT


> -----Original Message-----
> From: Polar Humenn [mailto:phumenn@iona.com]
> Sent: 2007?2?13? 3:33
> To: cxf-dev@incubator.apache.org
> Subject: Re: Proposal for chaning CXF Interceptor APIs. WAS: 
> RE: When should we close the handlers in CXF?
> 
> 
> So, if handleFault is removed, then there is no way to discern if an 
> interceptor threw a fault during processing?
> 
> So, you are saying that a Fault may not be thrown in onTermination()?
> 
> I don't agree. The purpose of onTermination in this proposal 
> is still to 
> "handle" the message, which allows semantically for throwing 
> a handling 
> fault. Fault handling should unwind through *all* the operations that 
> got it there, so that things may indeed be *unwound*.
> 
> I'll admit my interleaving example is a bit contrived, but never the 
> less, in a system as dynamically configurable as this interceptor 
> architecture is now, anything is possible, and  I submit that the 
> process to get it done is counter intuitive and complex in this 
> proposal. It is better to have a simple and clean semantics..
> 

I agree that it would be a simpler semantic if we can have a chain that only contains the
calling of handleMessage(message) and only traverses on one direction. But a second thought
makes me to think about the trade off we have to make in order to achieve this: First we have
to write corresponding ending interceptors for these need a terminal action, agreed it is
not a bit deal to write more interceptors and more phases.  Secondly and more importantly,
we have to make sure the ending interceptors are put in a proper place, this is where trick
plays. Its not that straightforward and easy to make mistakes to find a proper place in the
chain for ending interceptors. Things can get even worse if we have too many interceptors
in a same phase, I dont think addBefore and addAfter can handle complicate ordering cases.
As a side note, currently the way how interceptors order themselves is not ideal, as you already
noticed:  "As far as the phases go, as far as I can tell so far, that the amount of phases
and names of the phases are configurable to the particular interceptor chain, and don't really
have much "meaning" associated with them at the moment, except for the implicit ordering and
where some of the CXF internal interceptors are actually placed." Someone wants to refactor
this? ;-) Anyway, that will be another story. 

To summarize, without onComplete(or onTerminate) the semantics does look simpler as the chain
only flows on one direction, but it leaves the burden of choosing a proper phase for ending
interceptors to developers, thus subjects to human errors and obviously more work to do for
developers. The semantic itself can not guarantee the terminal action in an ending interceptors
is called in a place that is symmetric to its corresponding starting interceptors. E.g., the
chain below

Interceptor A -> Interceptor B.Staring -> Interceptor C.Staring -> Interceptor C.Ending
-> Interceptor B.Ending

Developers are free to put interceptor C.Ending and Interceptor B.Ending anywhere, the semantic
does not enforce the ending interceptors are called in a symmetric position in the chain corresponding
to their starting interceptors. For example, they can end up of having a chain:

Interceptor A -> Interceptor B.Staring -> Interceptor C.Staring -> Interceptor B.Ending
-> Interceptor C.Ending

This can be very wrong. Using onComplete released this burden, and I don't see a chain with
a traverse back phase on complete is that complicate in terms of semantics.

 

> As far as the phases go, as far as I can tell so far, that 
> the amount of 
> phases and names of the phases are configurable to the particular 
> interceptor chain, and don't really have much "meaning" 
> associated with 
> them at the moment, except for the implicit ordering and 
> where some of 
> the CXF internal interceptors are actually placed.  So, what would it 
> matter to have a couple more phases that are explicitly 
> created for the 
> CXF internal interceptors, such as 
> MessageSenderStartIntercpetor and a 
> MessageSenderEndInterceptor?
> 
> Cheers,
> -Polar
> 
> Dan Diephouse wrote:
> > Hi Polar, comments inline...
> >
> > On 2/12/07, Polar Humenn <phumenn@iona.com> wrote:
> >>
> >>
> >> >>
> >> > Agreed. It could be done via another interceptor, but 
> its a common
> >> enough
> >> > case that we'd like to make it simpler.
> >> > On a related note I would like to see the method named 
> >> onTermination() -
> >> > this would imply "on termination of the chain take this 
> action..." 
> >> which
> >> > would give interceptors a chance to close resources 
> associated with 
> >> the
> >> > message. I'm -1 on the current "postHandleMessage" method name.
> >> >
> >> I would argue that you may make some of the "common" cases 
> "simpler" to
> >> a degree in the sense that both operations will be in the 
> same class,
> >> but it make the semantics much more complex in general, and less
> >> efficient:
> >>
> >>      1. Many interceptors will have to implement 
> onTermination() without
> >> a need for it.
> >>      2. It will get always get called.
> >>
> >> The only advantage of this approach is that interceptors 
> may be able to
> >> save some instance state between the two calls, like a 
> reference to an
> >> object. However, that can be done merely by two subclasses 
> implementing
> >> the interceptor interface inside a single class.
> >>
> >> Also, it complicates the fault handing, which hasn't yet 
> been addressed.
> >>
> >> For instance, what happens if a Fault is thrown in onTermination()?
> >>     Does it unwind through handleFault()?
> >
> >
> > No, handleFault is removed.
> >
> >    If so, in what direction?
> >
> >
> > In reverse.
> >
> >    How many times? Once or twice? If possibly twice, which 
> first call
> >> to handleFault called?
> >
> >
> > handleFault is gone, and onTermination is called just once.
> >
> >    Does it unwind through the interceptor's handleFault() operation
> >> twice? On what run was it when it did?
> >
> >
> > See above.
> >
> > I surmise that the current interceptor interface {handleMessage,
> >> handleFault) is adequate, and it was the doIntercept() and
> >> doInterceptInSubChain() calls that kind of mucked up the 
> cleanliness and
> >> simplicity of the approach.
> >>
> >> Given that the proposal includes the eliminatation 
> doIntercept() and
> >> doInterceptInSubChain() you are going to have to the same 
> amount of work
> >> to current interceptors that use doIntercept and 
> doInterceptInSubChain:
> >>
> >> You will have to split the single handleMessage() that into a "save
> >> state on the message" so that handleMessage and onTermination() may
> >> communicate properly. However, this is the same amount of 
> work you need
> >> to do to create two separate interceptors using 
> handleMessage calls.
> >>
> >> Also, for example. let's say you require functionality 
> that needs to be
> >> interleaved between the handleMessage and onTermination() 
> calls of one
> >> interceptor (call it A). You will need two interceptors B 
> and C as you
> >> will not be able to get by with one. For example, 
> interceptor B will
> >> have a potent handleMessage() that goes AFTER interceptor 
> A, and limp
> >> onTermination() call. Inteceptor C must get installed 
> BEFORE interceptor
> >> A with a limp handleMessage() and a potent onTermination() 
> call. I say
> >> installing interceptor C before interceptor A is a counter 
> intuitive
> >> approach.
> >
> >
> > I'm not sure I follow your scenario. Do you have a concrete 
> use case 
> > in mind
> > here? I think all the current uses of 
> doIntercept(inSubChain) can be 
> > handled
> > by the simple flow of calling handleMessage when moving 
> forward and then
> > calling onTermination in reverse on all the interceptors 
> that have been
> > executed.
> >
> > A simple linear installation of interceptors is clearer, 
> more efficient,
> >> and has simple already defined fault handling.
> >
> >
> > This isn't simply about creating a new class, its about 
> creating new 
> > phases
> > as well. Lets take the MessageSenderInterceptor for 
> example. Following 
> > your
> > approach we need two classes, but we also need additional phases at 
> > the end
> > of the chain which mirror the start phases. This is very not ideal 
> > IMO. Its
> > much simpler to call handleMessage() throughout the chain, 
> and then call
> > onTermination() in reverse order once the chain is done. If a fault 
> > occurs
> > in the chain, onTermination is only called from that point back.
> >
> > - Dan
> >
> 
> 

Mime
View raw message