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:[EMAIL PROTECTED] > Sent: 15 February 2007 09:49 > To: [email protected] > 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:[EMAIL PROTECTED] > > Sent: 2007?2?15? 3:03 > > To: [email protected] > > 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 > > >> > > > >> > > >> > > > > > > > > > > >
