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 11:00:23 GMT


Conversely, I think there are scenarios that approach #2 can't handle at
all, elegantly or otherwise.

Specifically I'm thinking of the case of an interceptor wanting to kill
the in-chain traversal stone dead, currently achievable via
InterceptorChain.abort(). As the traversal has been aborted, we'd never
get to any extra "ending interceptors" dynamically added to the in-chain
to do cleanup on behalf of already-traversed interceptors.

However it would be a simple matter for the InterceptorChain.abort()
semantics to involve unwinding the in-chain with
onComplete()/onTermination() calls to the already traversed
interceptors.

The specific user-case I have in mind is a simple mechanism to allow an
interceptor cause an incoming message be rejected with a HTTP/BA 401
challenge. See my last post on the "HTTP Basic Authentication Is there
hope?" thread for more detail.

Cheers,
Eoghan 

> -----Original Message-----
> From: Liu, Jervis [mailto:jliu@iona.com] 
> Sent: 15 February 2007 09:49
> To: cxf-dev@incubator.apache.org
> Subject: RE: Proposal for chaning CXF Interceptor APIs. WAS: 
> RE: When should we close the handlers in CXF?
> 
> 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