Comments

The recent changes to EmptyTargetSource have broken Spring Security's tests.

Spring Security relies on checking to see if a MethodInvocation.getThis() is null to determine if the method is static. This seems to be a safe assumption based on the javadoc which states:

Returns:
the object (can be null if the accessible object is static).

What's more is it would seem non-obvious for me to use the EmptyTargetSource.EMPTY_TARGET (which is not accessible, so I need to use EmptyTargetSource.INSTANCE.getTarget() from Spring Framework to compare against the result of MethodInvocation.getThis() from aopalliance.

I think it makes sense to ensure that the construction of ReflectiveMethodInvocation contains target=null and targetClass=null for static methods.

This comment has been minimized.

Good point, Rob. This change was largely meant to be internal since we're effectively just using such an EmptyTargetSource as a dummy, in particular for client-side remoting proxies where there is no "real" target object (it's all handled by the interceptor) for interface-defined service methods (which are of course non-static). A null target artificially complicates the contract and the proxy implementations there and arguably also gives a false indication of a static method.

In any case, as you said, we should fix this towards MethodInvocation.getThis() returning null for actual static invocations. How did you actually arrive that, since we're only really intercepting instance-based methods to begin with? Are you just validating against static methods there, or are you trying to do anything specific with them?

This comment has been minimized.

I've largely reverted the EmptyTargetSource change to return null from getTarget and therefore also from MethodInvocation.getThis(). This is not so much for actual static methods but rather for interceptor-based scenarios without an actual target (like in your HTTP Invoker case) where we need to retain compatibility with our previous MethodInvocation semantics.