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 Thu, 15 Feb 2007 09:49:15 GMT
Hi I think I am convinced by Polar eventually as there are some cases the onComlete schema
can not handle very elegantly. Can we finalize the proposal before we switch the topic to
sth else like fault handling, better naming of phases etc? So we've had four proposals so
far, thanks for the contributions of DanD :-). 

1. OnComplete()
2. No OnComplete(), the chain is a flat one way traverse, developers need to write an ending
interceptor when a terminal action is needed
3. A variation of subchain proposed by Dan D.
4. XFire Serializer

I would veto Option 3 first as it still uses subchain, and I really don't like the idea of
subchain. Between option 2 and option 4, I personally prefer 2, as I found 2 is easier to
understand than 4 in terms of execution path and semantics. Also 2's semantics is very close
to CORBA interceptor, guess most guys from the old CORBA world would feel very comfortable
with this. Thoughts? 


> -----Original Message-----
> From: Polar Humenn [mailto:phumenn@iona.com]
> Sent: 2007?2?15? 3:03
> To: cxf-dev@incubator.apache.org
> Subject: Re: Proposal for chaning CXF Interceptor APIs. WAS: 
> RE: When should we close the handlers in CXF?
> 
> 
> Dan Diephouse wrote:
> > Alrighty, I think I'm convinced given that Dan pointed out 
> some better
> > syntax. I think we'd need to some convenience methods to
> > AbstractPhaseInterceptor with a signatures like this:
> >
> > void addInterceptor(String phase, Interceptor i)
> > void addInterceptor(String phase, String before, String after, 
> > Interceptor
> > i)
> >
> > Then it becomes
> >
> > addInterceptor(Phase.MARSHAL, new Interceptor() {
> > ...
> > });
> >
> > Now I think the important thing is to define some better phases. I 
> > need to
> > spend a little bit of time thinking about Andrea's email 
> and digging 
> > through
> > code before I really commit to anything though.
> >
> > Is everyone still ok with removing handleFault as well?
> >
> Does that mean you are going to get rid of the unwind?
> 
> I'm not in favor of removing handleFault();
> 
> Not that I'm a committer or anything :-[ , but I do use CXF. 8-)
> 
> I actually think handleFault may be improved. When the 
> PhaseInterceptorChain implementation gets a fault from a 
> handleMessage, 
> before it does the unwind, it places the thrown Fault on the Message 
> using message.setContent(Exception.class, fault).
> 
> Why is the interceptor chain modifying the message? Again, this seems 
> like a violation of a separation of concerns/control issue.
> 
> The handleFault method should explicitly be given the fault, such as:
> 
>     void handleFault(Message msg, Fault fault);
> 
> Cheers,
> -Polar
> 
> 
> > - Dan
> >
> > On 2/14/07, Polar Humenn <phumenn@iona.com> wrote:
> >>
> >>
> >> Hi Dan,
> >>
> >> I'm glad I convinced you on one point that was rockin' my boat.  :)
> >> Thanks.
> >>
> >> My only objection to the current and proposed scheme is 
> the "reentrant"
> >> interceptor chain execution scheme. This seems like a 
> violation of a
> >> control principle. Why should an interceptor actively control the
> >> mechanism that is controlling the interceptors?
> >>
> >> I guess it's like a disk device driver deciding bus access 
> affecting
> >> other devices it doesn't know about. (maybe a bad 
> example). Or a user
> >> process manipulating the operating system process table 
> deciding what
> >> should processes to run next.
> >>
> >> Furthermore, I think the sole purpose of the "reentrant" 
> model is so
> >> that a particular interceptor in Phase A, can perform an 
> operation X
> >> after Phase B further down the chain, which seems 
> confusing to me. I
> >> would think action X would be executed in some phase 
> *after* B, not in
> >> Phase A.
> >>
> >> So, I have three points:
> >>
> >> Point 1: Lets say, if I put an interceptor in the position 
> of the last
> >> phase, shouldn't I expect that I am the last interceptor 
> point, and the
> >> last actions that are done are mine?
> >>
> >> Point 2: fault handling. Granted, the doIntercept() call 
> returns false
> >> instead of true if the chain was aborted or not completed 
> yet, and maybe
> >> the actions after that can take that into account, but *why* is it
> >> *allowed* to do this? This interceptor should have *already* had
> >> handleFault called, yet this interceptor is still in the 
> handleMessage()
> >> call, technically 'handling" the message. This gives me a 
> big "Huh?".
> >>
> >> Point 3: What if you call doIntercept() in a handleFault() 
> operation?
> >> Nothing can prevent it. If you look at the code for the
> >> PhaseInterceptorChain, the chain is not yet ABORTED, and 
> this can reek
> >> havoc, by starting down the the rest of the chain when it should be
> >> unwinding!
> >>
> >>    [There's also another problem is an interceptor removes 
> itself from
> >> the chain, or something else removes the currently executing
> >> interceptor, but may get into that later :) ]
> >>
> >> Thanks for listening to my concerns.
> >>
> >> Cheers,
> >> -Polar
> >>
> >> Dan Diephouse wrote:
> >> > Hiya,
> >> >
> >> > On 2/14/07, Polar Humenn <phumenn@iona.com> wrote:
> >> >>
> >> >>
> >> >> > And the idea that onTermination() is called after the 
> chain has 
> >> been
> >> >> > invoked
> >> >> > is just as simple to understand. Are you telling me that a 
> >> developer
> >> >> > can't
> >> >> > understand that a method is executed at the end of the chain?
> >> >>
> >> >> But I'm confused, onTermination() isn't called at the 
> "end" of the
> >> >> chain. The first call to an onTermination() method is 
> in the "middle"
> >> of
> >> >> the chain. That's because you are still "handling" the 
> message in
> >> >> onTermination() calls of the interceptors. For instance, in the
> >> outgoing
> >> >> interceptor phase chain, the message isn't guaranteed 
> to pop-out 
> >> at the
> >> >> end of the chain, it pops out at the beginning of the chain!
> >> >>
> >> >> Let's take the MessageSenderInterceptor. This interceptor is 
> >> placed at
> >> >> the "PREPARE_SEND" Phase. Looking at its current 
> implementation there
> >> is
> >> >> a Chain.doIntercept() call. I will assume that it is 
> the split where
> >> >> handleMessage and onTermination() will now exist.
> >> >>
> >> >> handleMessage will call conduit.send(message);
> >> >>           (which basically readies the message to get 
> handled by 
> >> other
> >> >> interceptors,
> >> >>            in the PRE_STREAM, etc. phases.)
> >> >>
> >> >> onTermination will call conduit.close(message);
> >> >>           (which closes the OutputStream, i.e. flushes the final
> >> buffers
> >> >> writing data to the wire, guaranteeing that the message 
> has been 
> >> sent.)
> >> >>
> >> >> So, my basic question is, and what is confusing to me, is why is
> >> >> conduit.close(message) called in the PREPARE_SEND 
> phase? Shouldn't it
> >> >> logically be called in the "SEND" Phase, which is the 
> *last* outgoing
> >> >> phase?
> >> >
> >> >
> >> > Our phases aren't very well named at the moment. Its 
> been on my todo
> >> list
> >> > for forever to clean them up.
> >> >
> >> > And what if that onTermination() call got an IOException on the
> >> >> OutputStream.close(). (The message really cant be said 
> it made it out
> >> to
> >> >> the wire. None of the interceptors after the 
> PREPARE_SEND phase will
> >> get
> >> >> notified.
> >> >
> >> >
> >> > Ahhhh, I see your point now. You're right, that does get 
> sticky. I
> >> > think you
> >> > may be right - this may make that proposal a non 
> starter. (Sorry it
> >> > took me
> >> > a while.)
> >> >
> >> > I hope no one minds if I float two more proposals that 
> we've talked
> >> about
> >> > earlier for the sake of completeness:
> >> >
> >> > The first idea that we've floated before is supporting reentrant
> >> > interceptor
> >> > chains in a slightly different manner. The idea being 
> that instead of
> >> > doInterceptInSubChain we would support something like this:
> >> >
> >> > public void handleMessage(Message m)
> >> > startSend();
> >> > message.getChain().doIntercept(message, endPhase)
> >> > close();
> >> > }
> >> >
> >> > This is basically just a variation on the current theme, 
> but making it
> >> > cleaner. The main advantage of this over the current API 
> would be that
> >> > databinding interceptors wouldn't need to call 
> chain.endSubChain().
> >> >
> >> > The second idea would be to rework writing so that it was more
> >> > encapsulated.
> >> > In XFire we had the idea of a Serializer. One of the final
> >> > interceptors was
> >> > responsible for opening the connection, doing
> >> > serializer.write(message), and
> >> > finally closing the connection. The idea here is that as we move
> >> > through the
> >> > chain we can replace/modify the serializer as we feel 
> necessary. The
> >> > thing I
> >> > like about this is that it completely removes the need 
> for reentrant
> >> > interceptorchains/onTermination.  Here's an example of 
> how it might
> >> work:
> >> > 1. BareOutInterceptor sets the serializer to something 
> which writes 
> >> the
> >> > message in a doc/lit style
> >> > 2. Outgoing SAAJ interceptor uses this serializer to 
> write the message
> >> > to a
> >> > SAAJ tree. The serializer is then replaced with a SAAJ serializer
> >> > 3. Attachment out interceptor wraps the current 
> serializer with one
> >> which
> >> > starts writing the attachments, then delegates to the 
> SAAJ serializer,
> >> > and
> >> > then finishes writing the attachments
> >> > 4. A MessageSenderInterceptor opens the connection, executes the 
> >> current
> >> > serializer (outputting the message to the wire), and 
> then closes the
> >> > connection.
> >> >
> >> > The thing that I don't like about this is that it 
> introduces a new
> >> > concept,
> >> > but it does become much more straightforward how to do 
> output IMO.
> >> >
> >> > And then there is the standing proposal to just completely remove
> >> > doInterceptInSubChain() :-) Thoughts? At the moment I have a 
> >> preference
> >> > toward continuing our support for reentrant interceptors 
> a la the 
> >> first
> >> > proposal that I floated, but I could be swayed either way yet.
> >> >
> >> > Thanks Polar,
> >> >
> >> > - Dan
> >> >
> >>
> >>
> >
> >
> 
> 

Mime
View raw message