TargetSource.getTarget() is nullable again (for compatibility with MethodInvocation.getThis)

Issue: SPR-15651

Author: jhoeller

spring-aop/src/main/java/org/springframework/aop/TargetSource.java

@@ -16,6 +16,8 @@
 
 package org.springframework.aop;
 
+import org.springframework.lang.Nullable;
+
 /**
  * A {@code TargetSource} is used to obtain the current "target" of
  * an AOP invocation, which will be invoked via reflection if no around
@@ -54,14 +56,16 @@ public interface TargetSource extends TargetClassAware {
 	/**
 	 * Return a target instance. Invoked immediately before the
 	 * AOP framework calls the "target" of an AOP method invocation.
-	 * @return the target object, which contains the joinpoint
+	 * @return the target object which contains the joinpoint,
+	 * or {@code null} if there is no actual target instance
 	 * @throws Exception if the target object can't be resolved
 	 */
+	@Nullable
 	Object getTarget() throws Exception;
 
 	/**
 	 * Release the given target object obtained from the
-	 * {@link #getTarget()} method.
+	 * {@link #getTarget()} method, if any.
 	 * @param target object obtained from a call to {@link #getTarget()}
 	 * @throws Exception if the object can't be released
 	 */

spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java

@@ -326,12 +326,8 @@ private Callback[] getCallbacks(Class<?> rootClass) throws Exception {
 			// TODO: small memory optimization here (can skip creation for methods with no advice)
 			for (int x = 0; x < methods.length; x++) {
 				List<Object> chain = this.advised.getInterceptorsAndDynamicInterceptionAdvice(methods[x], rootClass);
-				Object target = this.advised.getTargetSource().getTarget();
-				Class<?> targetClass = this.advised.getTargetClass();
-				if (targetClass == null) {
-					targetClass = target.getClass();
-				}
-				fixedCallbacks[x] = new FixedChainStaticTargetInterceptor(chain, target, targetClass);
+				fixedCallbacks[x] = new FixedChainStaticTargetInterceptor(
+						chain, this.advised.getTargetSource().getTarget(), this.advised.getTargetClass());
 				this.fixedInterceptorMap.put(methods[x].toString(), x);
 			}
 
@@ -378,20 +374,22 @@ private static boolean implementsInterface(Method method, Set<Class<?>> ifcs) {
 	 * {@code proxy} and also verifies that {@code null} is not returned as a primitive.
 	 */
 	@Nullable
-	private static Object processReturnType(Object proxy, Object target, Method method, @Nullable Object retVal) {
+	private static Object processReturnType(
+			Object proxy, @Nullable Object target, Method method, @Nullable Object returnValue) {
+
 		// Massage return value if necessary
-		if (retVal != null && retVal == target &&
+		if (returnValue != null && returnValue == target &&
 				!RawTargetAccess.class.isAssignableFrom(method.getDeclaringClass())) {
 			// Special case: it returned "this". Note that we can't help
 			// if the target sets a reference to itself in another returned object.
-			retVal = proxy;
+			returnValue = proxy;
 		}
 		Class<?> returnType = method.getReturnType();
-		if (retVal == null && returnType != Void.TYPE && returnType.isPrimitive()) {
+		if (returnValue == null && returnType != Void.TYPE && returnType.isPrimitive()) {
 			throw new AopInvocationException(
 					"Null return value from advice does not match primitive return type for: " + method);
 		}
-		return retVal;
+		return returnValue;
 	}
 
 
@@ -413,7 +411,7 @@ private static class StaticUnadvisedInterceptor implements MethodInterceptor, Se
 
 		private final Object target;
 
-		public StaticUnadvisedInterceptor(Object target) {
+		public StaticUnadvisedInterceptor(@Nullable Object target) {
 			this.target = target;
 		}
 
@@ -434,7 +432,7 @@ private static class StaticUnadvisedExposedInterceptor implements MethodIntercep
 
 		private final Object target;
 
-		public StaticUnadvisedExposedInterceptor(Object target) {
+		public StaticUnadvisedExposedInterceptor(@Nullable Object target) {
 			this.target = target;
 		}
 
@@ -476,7 +474,9 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
 				return processReturnType(proxy, target, method, retVal);
 			}
 			finally {
-				this.targetSource.releaseTarget(target);
+				if (target != null) {
+					this.targetSource.releaseTarget(target);
+				}
 			}
 		}
 	}
@@ -505,7 +505,9 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
 			}
 			finally {
 				AopContext.setCurrentProxy(oldProxy);
-				this.targetSource.releaseTarget(target);
+				if (target != null) {
+					this.targetSource.releaseTarget(target);
+				}
 			}
 		}
 	}
@@ -612,7 +614,9 @@ private static class FixedChainStaticTargetInterceptor implements MethodIntercep
 
 		private final Class<?> targetClass;
 
-		public FixedChainStaticTargetInterceptor(List<Object> adviceChain, Object target, Class<?> targetClass) {
+		public FixedChainStaticTargetInterceptor(
+				List<Object> adviceChain, @Nullable Object target, @Nullable Class<?> targetClass) {
+
 			this.adviceChain = adviceChain;
 			this.target = target;
 			this.targetClass = targetClass;
@@ -649,15 +653,16 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
 			Object oldProxy = null;
 			boolean setProxyContext = false;
 			Object target = null;
+			TargetSource targetSource = this.advised.getTargetSource();
 			try {
 				if (this.advised.exposeProxy) {
 					// Make invocation available if necessary.
 					oldProxy = AopContext.setCurrentProxy(proxy);
 					setProxyContext = true;
 				}
 				// Get as late as possible to minimize the time we "own" the target, in case it comes from a pool...
-				target = getTarget();
-				Class<?> targetClass = target.getClass();
+				target = targetSource.getTarget();
+				Class<?> targetClass = (target != null ? target.getClass() : null);
 				List<Object> chain = this.advised.getInterceptorsAndDynamicInterceptionAdvice(method, targetClass);
 				Object retVal;
 				// Check whether we only have one InvokerInterceptor: that is,
@@ -678,8 +683,8 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
 				return retVal;
 			}
 			finally {
-				if (target != null) {
-					releaseTarget(target);
+				if (target != null && !targetSource.isStatic()) {
+					targetSource.releaseTarget(target);
 				}
 				if (setProxyContext) {
 					// Restore old proxy.
@@ -702,14 +707,6 @@ public boolean equals(Object other) {
 		public int hashCode() {
 			return this.advised.hashCode();
 		}
-
-		protected Object getTarget() throws Exception {
-			return this.advised.getTargetSource().getTarget();
-		}
-
-		protected void releaseTarget(Object target) throws Exception {
-			this.advised.getTargetSource().releaseTarget(target);
-		}
 	}
 
 
@@ -722,8 +719,9 @@ private static class CglibMethodInvocation extends ReflectiveMethodInvocation {
 
 		private final boolean publicMethod;
 
-		public CglibMethodInvocation(Object proxy, Object target, Method method, Object[] arguments,
-				Class<?> targetClass, List<Object> interceptorsAndDynamicMethodMatchers, MethodProxy methodProxy) {
+		public CglibMethodInvocation(Object proxy, @Nullable Object target, Method method,
+				Object[] arguments, @Nullable Class<?> targetClass,
+				List<Object> interceptorsAndDynamicMethodMatchers, MethodProxy methodProxy) {
 
 			super(proxy, target, method, arguments, targetClass, interceptorsAndDynamicMethodMatchers);
 			this.methodProxy = methodProxy;

spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java

@@ -191,7 +191,7 @@ else if (!this.advised.opaque && method.getDeclaringClass().isInterface() &&
 			// Get as late as possible to minimize the time we "own" the target,
 			// in case it comes from a pool.
 			target = targetSource.getTarget();
-			Class<?> targetClass = target.getClass();
+			Class<?> targetClass = (target != null ? target.getClass() : null);
 
 			// Get the interception chain for this method.
 			List<Object> chain = this.advised.getInterceptorsAndDynamicInterceptionAdvice(method, targetClass);

spring-aop/src/main/java/org/springframework/aop/framework/ReflectiveMethodInvocation.java

@@ -103,8 +103,8 @@ public class ReflectiveMethodInvocation implements ProxyMethodInvocation, Clonea
 	 * but would complicate the code. And it would work only for static pointcuts.
 	 */
 	protected ReflectiveMethodInvocation(
-			Object proxy, Object target, Method method, Object[] arguments,
-			Class<?> targetClass, List<Object> interceptorsAndDynamicMethodMatchers) {
+			Object proxy, @Nullable Object target, Method method, Object[] arguments,
+			@Nullable Class<?> targetClass, List<Object> interceptorsAndDynamicMethodMatchers) {
 
 		this.proxy = proxy;
 		this.target = target;
@@ -121,6 +121,7 @@ public final Object getProxy() {
 	}
 
 	@Override
+	@Nullable
 	public final Object getThis() {
 		return this.target;
 	}

spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java

@@ -329,7 +329,7 @@ public static List<Advisor> findAdvisorsThatCanApply(List<Advisor> candidateAdvi
 	 * @throws org.springframework.aop.AopInvocationException in case of a reflection error
 	 */
 	@Nullable
-	public static Object invokeJoinpointUsingReflection(Object target, Method method, Object[] args)
+	public static Object invokeJoinpointUsingReflection(@Nullable Object target, Method method, Object[] args)
 			throws Throwable {
 
 		// Use reflection to invoke the method.

spring-aop/src/main/java/org/springframework/aop/target/EmptyTargetSource.java

@@ -32,13 +32,6 @@
  */
 public class EmptyTargetSource implements TargetSource, Serializable {
 
-	/**
-	 * The canonical (Singleton) instance of this {@link EmptyTargetSource}.
-	 */
-	public static final EmptyTargetSource INSTANCE = new EmptyTargetSource(null, true);
-
-	private static final Object EMPTY_TARGET = new Object();
-
 	/** use serialVersionUID from Spring 1.2 for interoperability */
 	private static final long serialVersionUID = 3680494563553489691L;
 
@@ -47,6 +40,12 @@ public class EmptyTargetSource implements TargetSource, Serializable {
 	// Static factory methods
 	//---------------------------------------------------------------------
 
+	/**
+	 * The canonical (Singleton) instance of this {@link EmptyTargetSource}.
+	 */
+	public static final EmptyTargetSource INSTANCE = new EmptyTargetSource(null, true);
+
+
 	/**
 	 * Return an EmptyTargetSource for the given target Class.
 	 * @param targetClass the target Class (may be {@code null})
@@ -106,11 +105,11 @@ public boolean isStatic() {
 	}
 
 	/**
-	 * Always returns {@code DUMMY_TARGET}.
+	 * Always returns {@code null}.
 	 */
 	@Override
 	public Object getTarget() {
-		return EMPTY_TARGET;
+		return null;
 	}
 
 	/**

spring-aop/src/test/java/org/springframework/aop/framework/ProxyFactoryTests.java

@@ -35,6 +35,7 @@
 import org.springframework.aop.support.DelegatingIntroductionInterceptor;
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 import org.springframework.core.annotation.Order;
+import org.springframework.lang.Nullable;
 import org.springframework.tests.TimeStamped;
 import org.springframework.tests.aop.advice.CountingBeforeAdvice;
 import org.springframework.tests.aop.interceptor.NopInterceptor;
@@ -369,6 +370,16 @@ public void testTargetClassProxiesCanBeOrderedThroughAnnotations() {
 		assertSame(proxy1, list.get(1));
 	}
 
+	@Test
+	public void testInterceptorWithoutJoinpoint() {
+		final TestBean target = new TestBean("tb");
+		ITestBean proxy = ProxyFactory.getProxy(ITestBean.class, (MethodInterceptor) invocation -> {
+			assertNull(invocation.getThis());
+			return invocation.getMethod().invoke(target, invocation.getArguments());
+		});
+		assertEquals("tb", proxy.getName());
+	}
+
 
 	@SuppressWarnings("serial")
 	private static class TimestampIntroductionInterceptor extends DelegatingIntroductionInterceptor

spring-test/src/main/java/org/springframework/test/util/AopTestUtils.java

@@ -29,6 +29,7 @@
  * {@link org.springframework.aop.framework.AopProxyUtils AopProxyUtils}.
  *
  * @author Sam Brannen
+ * @author Juergen Hoeller
  * @since 4.2
  * @see org.springframework.aop.support.AopUtils
  * @see org.springframework.aop.framework.AopProxyUtils
@@ -54,7 +55,10 @@ public static <T> T getTargetObject(Object candidate) {
 		Assert.notNull(candidate, "Candidate must not be null");
 		try {
 			if (AopUtils.isAopProxy(candidate) && candidate instanceof Advised) {
-				return (T) ((Advised) candidate).getTargetSource().getTarget();
+				Object target = ((Advised) candidate).getTargetSource().getTarget();
+				if (target != null) {
+					return (T) target;
+				}
 			}
 		}
 		catch (Throwable ex) {
@@ -83,7 +87,10 @@ public static <T> T getUltimateTargetObject(Object candidate) {
 		Assert.notNull(candidate, "Candidate must not be null");
 		try {
 			if (AopUtils.isAopProxy(candidate) && candidate instanceof Advised) {
-				return (T) getUltimateTargetObject(((Advised) candidate).getTargetSource().getTarget());
+				Object target = ((Advised) candidate).getTargetSource().getTarget();
+				if (target != null) {
+					return (T) getUltimateTargetObject(target);
+				}
 			}
 		}
 		catch (Throwable ex) {