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:[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
