diff --git a/spring-aop/src/main/java/org/springframework/aop/ProxyMethodInvocation.java b/spring-aop/src/main/java/org/springframework/aop/ProxyMethodInvocation.java index 935f745de4..bee8192567 100644 --- a/spring-aop/src/main/java/org/springframework/aop/ProxyMethodInvocation.java +++ b/spring-aop/src/main/java/org/springframework/aop/ProxyMethodInvocation.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,14 +57,14 @@ public interface ProxyMethodInvocation extends MethodInvocation { * @return an invocable clone of this invocation. * {@code proceed()} can be called once per clone. */ - MethodInvocation invocableClone(Object[] arguments); + MethodInvocation invocableClone(Object... arguments); /** * Set the arguments to be used on subsequent invocations in the any advice * in this chain. * @param arguments the argument array */ - void setArguments(Object[] arguments); + void setArguments(Object... arguments); /** * Add the specified user attribute with the given value to this invocation. diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java b/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java index d7b965da42..4e063d2bc0 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java @@ -16,6 +16,8 @@ package org.springframework.aop.framework; +import java.lang.reflect.Array; +import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.Arrays; @@ -25,6 +27,7 @@ import org.springframework.aop.TargetSource; import org.springframework.aop.support.AopUtils; import org.springframework.aop.target.SingletonTargetSource; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * Utility methods for AOP proxy factories. @@ -161,4 +164,38 @@ public abstract class AopProxyUtils { return Arrays.equals(a.getAdvisors(), b.getAdvisors()); } + + /** + * Adapt the given arguments to the target signature in the given method, + * if necessary: in particular, if a given vararg argument array does not + * match the array type of the declared vararg parameter in the method. + * @param method the target method + * @param arguments the given arguments + * @return a cloned argument array, or the original if no adaptation is needed + * @since 4.2.3 + */ + static Object[] adaptArgumentsIfNecessary(Method method, Object... arguments) { + if (method.isVarArgs() && !ObjectUtils.isEmpty(arguments)) { + Class[] paramTypes = method.getParameterTypes(); + if (paramTypes.length == arguments.length) { + int varargIndex = paramTypes.length - 1; + Class varargType = paramTypes[varargIndex]; + if (varargType.isArray()) { + Object varargArray = arguments[varargIndex]; + if (varargArray instanceof Object[] && !varargType.isInstance(varargArray)) { + Object[] newArguments = new Object[arguments.length]; + System.arraycopy(arguments, 0, newArguments, 0, varargIndex); + Class targetElementType = varargType.getComponentType(); + int varargLength = Array.getLength(varargArray); + Object newVarargArray = Array.newInstance(targetElementType, varargLength); + System.arraycopy(varargArray, 0, newVarargArray, 0, varargLength); + newArguments[varargIndex] = newVarargArray; + return newArguments; + } + } + } + } + return arguments; + } + } diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java index 64a5d7083a..7f6e0c0042 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java @@ -304,13 +304,13 @@ class CglibAopProxy implements AopProxy, Serializable { Callback targetDispatcher = isStatic ? new StaticDispatcher(this.advised.getTargetSource().getTarget()) : new SerializableNoOp(); - Callback[] mainCallbacks = new Callback[]{ - aopInterceptor, // for normal advice - targetInterceptor, // invoke target without considering advice, if optimized - new SerializableNoOp(), // no override for methods mapped to this - targetDispatcher, this.advisedDispatcher, - new EqualsInterceptor(this.advised), - new HashCodeInterceptor(this.advised) + Callback[] mainCallbacks = new Callback[] { + aopInterceptor, // for normal advice + targetInterceptor, // invoke target without considering advice, if optimized + new SerializableNoOp(), // no override for methods mapped to this + targetDispatcher, this.advisedDispatcher, + new EqualsInterceptor(this.advised), + new HashCodeInterceptor(this.advised) }; Callback[] callbacks; @@ -646,7 +646,8 @@ class CglibAopProxy implements AopProxy, Serializable { // Note that the final invoker must be an InvokerInterceptor, so we know // it does nothing but a reflective operation on the target, and no hot // swapping or fancy proxying. - retVal = methodProxy.invoke(target, args); + Object[] argsToUse = AopProxyUtils.adaptArgumentsIfNecessary(method, args); + retVal = methodProxy.invoke(target, argsToUse); } else { // We need to create a method invocation... diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java index e93aa9e5cb..e98c3085c2 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java @@ -198,7 +198,8 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa // We can skip creating a MethodInvocation: just invoke the target directly // Note that the final invoker must be an InvokerInterceptor so we know it does // nothing but a reflective operation on the target, and no hot swapping or fancy proxying. - retVal = AopUtils.invokeJoinpointUsingReflection(target, method, args); + Object[] argsToUse = AopProxyUtils.adaptArgumentsIfNecessary(method, args); + retVal = AopUtils.invokeJoinpointUsingReflection(target, method, argsToUse); } else { // We need to create a method invocation... diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/ReflectiveMethodInvocation.java b/spring-aop/src/main/java/org/springframework/aop/framework/ReflectiveMethodInvocation.java index 62efcb77ae..4a223afe27 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/ReflectiveMethodInvocation.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/ReflectiveMethodInvocation.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2015 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -109,7 +109,7 @@ public class ReflectiveMethodInvocation implements ProxyMethodInvocation, Clonea this.target = target; this.targetClass = targetClass; this.method = BridgeMethodResolver.findBridgedMethod(method); - this.arguments = arguments; + this.arguments = AopProxyUtils.adaptArgumentsIfNecessary(method, arguments); this.interceptorsAndDynamicMethodMatchers = interceptorsAndDynamicMethodMatchers; } @@ -145,7 +145,7 @@ public class ReflectiveMethodInvocation implements ProxyMethodInvocation, Clonea } @Override - public void setArguments(Object[] arguments) { + public void setArguments(Object... arguments) { this.arguments = arguments; } @@ -219,7 +219,7 @@ public class ReflectiveMethodInvocation implements ProxyMethodInvocation, Clonea * @see java.lang.Object#clone() */ @Override - public MethodInvocation invocableClone(Object[] arguments) { + public MethodInvocation invocableClone(Object... arguments) { // Force initialization of the user attributes Map, // for having a shared Map reference in the clone. if (this.userAttributes == null) { diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java index 87b295b908..42b4746ec2 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java @@ -19,18 +19,15 @@ package org.springframework.aop.aspectj.annotation; import java.util.Arrays; import org.apache.commons.logging.LogFactory; - import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; import org.aspectj.lang.annotation.Aspect; - import org.junit.Ignore; import org.junit.Test; +import test.aop.PerThisAspect; import org.springframework.util.SerializationTestUtils; -import test.aop.PerThisAspect; - import static org.junit.Assert.*; /** @@ -109,18 +106,27 @@ public class AspectProxyFactoryTests { } @Test // SPR-13328 - public void testVarargsWithEnumArray() throws Exception { + public void testProxiedVarargsWithEnumArray() throws Exception { AspectJProxyFactory proxyFactory = new AspectJProxyFactory(new TestBean()); - proxyFactory.addAspect(LoggingAspect.class); - proxyFactory.setProxyTargetClass(true); - TestBean proxy = proxyFactory.getProxy(); - assertTrue(proxy.doWithVarargs(MyEnum.A, MyEnum.B)); + proxyFactory.addAspect(LoggingAspectOnVarargs.class); + ITestBean proxy = proxyFactory.getProxy(); + assertTrue(proxy.doWithVarargs(MyEnum.A, MyOtherEnum.C)); + } + + @Test // SPR-13328 + public void testUnproxiedVarargsWithEnumArray() throws Exception { + AspectJProxyFactory proxyFactory = new AspectJProxyFactory(new TestBean()); + proxyFactory.addAspect(LoggingAspectOnSetter.class); + ITestBean proxy = proxyFactory.getProxy(); + assertTrue(proxy.doWithVarargs(MyEnum.A, MyOtherEnum.C)); } public interface ITestBean { int getAge(); + + boolean doWithVarargs(V... args); } @@ -138,6 +144,7 @@ public class AspectProxyFactoryTests { } @SuppressWarnings("unchecked") + @Override public boolean doWithVarargs(V... args) { return true; } @@ -154,12 +161,29 @@ public class AspectProxyFactoryTests { } + public enum MyOtherEnum implements MyInterface { + + C, D; + } + + @Aspect - public static class LoggingAspect { + public static class LoggingAspectOnVarargs { @Around("execution(* doWithVarargs(*))") public Object doLog(ProceedingJoinPoint pjp) throws Throwable { - LogFactory.getLog(LoggingAspect.class).debug(Arrays.asList(pjp.getArgs())); + LogFactory.getLog(LoggingAspectOnVarargs.class).debug(Arrays.asList(pjp.getArgs())); + return pjp.proceed(); + } + } + + + @Aspect + public static class LoggingAspectOnSetter { + + @Around("execution(* setAge(*))") + public Object doLog(ProceedingJoinPoint pjp) throws Throwable { + LogFactory.getLog(LoggingAspectOnSetter.class).debug(Arrays.asList(pjp.getArgs())); return pjp.proceed(); } } diff --git a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java index e129bbc523..1052a2599c 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java @@ -21,7 +21,6 @@ import java.io.Serializable; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.junit.Test; - import test.mixin.LockMixinAdvisor; import org.springframework.aop.ClassFilter; @@ -395,7 +394,7 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab public void testVarargsWithEnumArray() throws Exception { ProxyFactory proxyFactory = new ProxyFactory(new MyBean()); MyBean proxy = (MyBean) proxyFactory.getProxy(); - assertTrue(proxy.doWithVarargs(MyEnum.A, MyEnum.B)); + assertTrue(proxy.doWithVarargs(MyEnum.A, MyOtherEnum.C)); } @@ -432,6 +431,12 @@ public class CglibProxyTests extends AbstractAopProxyTests implements Serializab } + public enum MyOtherEnum implements MyInterface { + + C, D; + } + + public static class ExceptionThrower { private boolean catchInvoked; diff --git a/spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java index 7a194d9a43..cb5b07e908 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java @@ -138,6 +138,13 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria assertEquals("hashCode()", proxy.hashCode(), named.hashCode()); } + @Test // SPR-13328 + public void testVarargsWithEnumArray() throws Exception { + ProxyFactory proxyFactory = new ProxyFactory(new VarargTestBean()); + VarargTestInterface proxy = (VarargTestInterface) proxyFactory.getProxy(); + assertTrue(proxy.doWithVarargs(MyEnum.A, MyOtherEnum.C)); + } + public interface Foo { @@ -201,4 +208,36 @@ public class JdkDynamicProxyTests extends AbstractAopProxyTests implements Seria } } + + public interface VarargTestInterface { + + boolean doWithVarargs(V... args); + } + + + public static class VarargTestBean implements VarargTestInterface { + + @SuppressWarnings("unchecked") + @Override + public boolean doWithVarargs(V... args) { + return true; + } + } + + + public interface MyInterface { + } + + + public enum MyEnum implements MyInterface { + + A, B; + } + + + public enum MyOtherEnum implements MyInterface { + + C, D; + } + }