cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Glynn, Eoghan" <eoghan.gl...@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 15:47:44 GMT


Yep, a CXF interceptor has a *lot* more flexibility than a CORBA
PortableInterceptor (or even a ORB-native Interceptor, at least in the
case of the ORBs I've worked on).

Also the message exchange patterns are much less regimented than in the
CORBA world ... for example, we have a CXF interceptor that kicks off an
outgoing partial response from within a in-chain traversal, before
continueing with the rest of the inbound chain. This sort of thing
doesn't really have an analogue in the CORBA world AFAIK.

As far as matching up out/in messages is concerned, this is done via the
Exchange.getInMessage()/getOutMessage() etc. Normally in the server-side
in-chain, the setting up of the out & fault messages is done in the
OutgoingChainSetupInterceptor.handleMessage(). But any other interceptor
in the in-chain is free to get in there first, and just call
Endpoint.getBinding().createMessage() to create the out message (as the
ChallengeInterceptor does). 

On the client-side, the correlation is also accessible to any
interceptor via the Exchange, with the priviso that in the decoupled
case, the correlation does not occur until the WS-A protocol interceptor
(MAPCodec) is traversed.

BTW I gave some detailed reasoning in an earlier post as to why I reckon
a "short circuit" 401 is more appropriate than a fault.

Cheers,
Eoghan

> -----Original Message-----
> From: Polar Humenn [mailto:phumenn@iona.com] 
> Sent: 15 February 2007 15:21
> To: cxf-dev@incubator.apache.org
> Subject: Re: Proposal for chaning CXF Interceptor APIs. WAS: 
> RE: When should we close the handlers in CXF?
> 
> Liu, Jervis wrote:
> > 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? 
> >
> >   
> I do have some thoughts, being an old CORBA guy, myself.  =-O 
> I'm the guy who started the CORBA interceptor standard 
> rolling (one of those bad 
> security guys trying to tell the ORB guys what to do!).   O:-)
> 
> Here is as least something to think about.
> 
> Eoghan brought up an interesting example the other day when 
> we were talking about issuing a 401 status code in HTTP on 
> the server side.
> 
> My solution was to throw a Fault in the password checker 
> interceptor. I found that throwing a fault would magically 
> generate a Fault Message with status 500, and I would 
> intercept that on the Out Fault Chain and change the status 
> code to a 401 and provide the WWW-Authentication header.
> 
> Eoghan presented an example whereby the password checker 
> interceptor, retrieved the Exchange, created a correlated 
> outgoing message containing the 401, and then pulled a 
> Conduit.send() and Conduit.close() on it (completely skipping 
> the out interceptor chain). And he had to do an
> InterceptorChain.abort() to prevent the message from going on 
> to the application.
> 
> I'm not sure which approach is better, because I think the 
> message internals are more unwieldy for CXF.
> 
> In CORBA, it's a little more defined. Every incoming message 
> is a Request for which there is always a Reply, whether that 
> be a real reply or an exception. If the reply or exception 
> came back from the application, it would unwind through the 
> interceptors with a
> send_reply() or send_exception() call, respectively. Also, if 
> a Request Interceptor threw an Exception, that exception 
> would be turned into a CORBA Exception Reply and that reply 
> would unwind back  through the interceptors that got it 
> there, through send_exception() or send_other() depending on 
> whether it was a CORBA typed exception or other exception.
> 
> On the client side, it is the same, requests go out 
> send_request, and returns through receive_reply(), 
> receive_exception(), receive_other(). 
> If a send_request throws an Exception, it unwinds back 
> through one of the receive calls.
> 
> In CXF, it doesn't appear that an in/out Message has the 
> notion of a matching out/in message. (at least for the 
> purpose of interceptors).
> 
> Interceptors do all the "replying" on the server side, and on 
> the client side Interceptors do the correlation of incoming 
> messages with previously sent outgoing messages.
> 
> So, therefore an interceptor needs to be able to reverse the 
> chain as in throwing a Fault, or at least stop it, as in 
> Eoghan's example when it decides to "short-circuit" an intended reply.
> 
> Cheers,
> -Polar
> 
> >   
> >> -----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