The main problem with Alternative 2 is the performance impact it will have.
The PhaseInterceptorChain (and it's iterator) is the FUNDAMENTAL loop and
adding any processing in there would occur many many times for each invoke.
By then end of processing a message, it's no uncommon for the chain to have
20-30+ interceptors. Thus, those checks would have occured that many times
(plus another 20-30 for the response message). That would definitely have a
least some measurable performance impact.
In general, I've preferred:
1) If an interceptor is "generic" keep it generic Message.
2) If it has to be specific (like the SoapMessage things), then make sure it's
a soap binding it's being added to.
One option might be to catch ClassCastException in the interceptor chain and
only do your "check" if the ClassCastException was thrown to then log a more
appropriate error/warning. BTW: since it would be moved to an exception
case and not on performance critical path, we can "auto detect" the type it's
supposed to be:
Class cls = currentInterceptor.getClass();
for (Method m : cls.getMethods()) {
if (!m.isSynthetic() && m.getName().equals("handleMessage")) {
Class typeTheyWant = m.getParameters()[0];
}
}
Dan
On Fri August 21 2009 8:17:11 am Benson Margulies wrote:
> Interceptors implement a generic interface that is parameterized over
> a subclass of Message. Each interceptor class implements
> handleMessage(T), where 'T extends Message'.
>
> This removes one type cast from the interceptors, but it creates
> certain risks. I hit one of them, and I've been thinking about whether
> we want to change anything.
>
> Nothing checks to ensure that the actual message being processed is
> type-compatible with each of the interceptors. If someone puts, for
> example, an Interceptor<SoapMessage> into a chain, and an XMLMessage
> comes along, the result will be a ClassCastException. It will be an
> especially confusing ClassCastException, due to type erasure.
>
> Interceptor declares handleMessage(T), but by the time type erasure is
> done, that's just handleMessage(Message). When a mistyped message
> comes along, this mismatch is detected when code tries to assign it to
> the parameter, and the backtrace claims that the exception is being
> thrown by line one of the overall source file.
>
> I could think of two alternatives (other than leaving well enough alone).
>
> Alternative 1: get rid of the whole generic structure in interceptors.
> Everybody handles Message, and casts as they need to based on their
> assumptions or knowledge of the situation.
>
> Alternative 2: the chain filters out mis-types interceptors, making
> the type of the message a condition on whether the interceptor fires
> or not.
>
> While #1 might have something to be said for it in pure hindsight, it
> would be quite disruptive to all those user-written interceptors out
> there, so let's forget it.
>
> #2 has some attractive characteristics. It would work like this, I think.
>
> Interceptor<T> would gain a method:
>
> Class<? extends Message> getMessageClass();
>
> Each interceptor would return the message type it's prepared to handle.
>
> The chain would skip messages that didn't qualify.
>
> Does anyone think this is worth doing?
--
Daniel Kulp
dk...@apache.org
http://www.dankulp.com/blog
