MethodBasedEvaluationContext reliably exposes varargs

Issue: SPR-14554

spring-context/src/main/java/org/springframework/cache/interceptor/CacheEvaluationContext.java

@@ -17,8 +17,8 @@
 package org.springframework.cache.interceptor;
 
 import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Set;
 
 import org.springframework.context.expression.MethodBasedEvaluationContext;
 import org.springframework.core.ParameterNameDiscoverer;
@@ -38,25 +38,27 @@ import org.springframework.core.ParameterNameDiscoverer;
  *
  * @author Costin Leau
  * @author Stephane Nicoll
+ * @author Juergen Hoeller
  * @since 3.1
  */
 class CacheEvaluationContext extends MethodBasedEvaluationContext {
 
-	private final List<String> unavailableVariables;
+	private final Set<String> unavailableVariables = new HashSet<>(1);
 
-	CacheEvaluationContext(Object rootObject, Method method, Object[] args,
-			ParameterNameDiscoverer paramDiscoverer) {
 
-		super(rootObject, method, args, paramDiscoverer);
-		this.unavailableVariables = new ArrayList<>();
+	CacheEvaluationContext(Object rootObject, Method method, Object[] arguments,
+			ParameterNameDiscoverer parameterNameDiscoverer) {
+
+		super(rootObject, method, arguments, parameterNameDiscoverer);
 	}
 
+
 	/**
-	 * Add the specified variable name as unavailable for that context. Any expression trying
-	 * to access this variable should lead to an exception.
-	 * <p>This permits the validation of expressions that could potentially a variable even
-	 * when such variable isn't available yet. Any expression trying to use that variable should
-	 * therefore fail to evaluate.
+	 * Add the specified variable name as unavailable for that context.
+	 * Any expression trying to access this variable should lead to an exception.
+	 * <p>This permits the validation of expressions that could potentially a
+	 * variable even when such variable isn't available yet. Any expression
+	 * trying to use that variable should therefore fail to evaluate.
 	 */
 	public void addUnavailableVariable(String name) {
 		this.unavailableVariables.add(name);

spring-context/src/main/java/org/springframework/context/expression/MethodBasedEvaluationContext.java

@@ -17,6 +17,7 @@
 package org.springframework.context.expression;
 
 import java.lang.reflect.Method;
+import java.util.Arrays;
 
 import org.springframework.core.ParameterNameDiscoverer;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
@@ -34,27 +35,27 @@ import org.springframework.util.ObjectUtils;
  * </ol>
  *
  * @author Stephane Nicoll
- * @author Sergey Podgurskiy
+ * @author Juergen Hoeller
  * @since 4.2
  */
 public class MethodBasedEvaluationContext extends StandardEvaluationContext {
 
 	private final Method method;
 
-	private final Object[] args;
+	private final Object[] arguments;
 
-	private final ParameterNameDiscoverer paramDiscoverer;
+	private final ParameterNameDiscoverer parameterNameDiscoverer;
 
-	private boolean paramLoaded = false;
+	private boolean argumentsLoaded = false;
 
 
-	public MethodBasedEvaluationContext(Object rootObject, Method method, Object[] args,
-			ParameterNameDiscoverer paramDiscoverer) {
+	public MethodBasedEvaluationContext(Object rootObject, Method method, Object[] arguments,
+			ParameterNameDiscoverer parameterNameDiscoverer) {
 
 		super(rootObject);
 		this.method = method;
-		this.args = args;
-		this.paramDiscoverer = paramDiscoverer;
+		this.arguments = arguments;
+		this.parameterNameDiscoverer = parameterNameDiscoverer;
 	}
 
 
@@ -64,9 +65,9 @@ public class MethodBasedEvaluationContext extends StandardEvaluationContext {
 		if (variable != null) {
 			return variable;
 		}
-		if (!this.paramLoaded) {
+		if (!this.argumentsLoaded) {
 			lazyLoadArguments();
-			this.paramLoaded = true;
+			this.argumentsLoaded = true;
 			variable = super.lookupVariable(name);
 		}
 		return variable;
@@ -76,22 +77,30 @@ public class MethodBasedEvaluationContext extends StandardEvaluationContext {
 	 * Load the param information only when needed.
 	 */
 	protected void lazyLoadArguments() {
-		// shortcut if no args need to be loaded
-		if (ObjectUtils.isEmpty(this.args)) {
+		// Shortcut if no args need to be loaded
+		if (ObjectUtils.isEmpty(this.arguments)) {
 			return;
 		}
 
-		// save arguments as indexed variables
-		for (int i = 0; i < this.args.length; i++) {
-			setVariable("a" + i, this.args[i]);
-			setVariable("p" + i, this.args[i]);
-		}
+		// Expose indexed variables as well as parameter names (if discoverable)
+		String[] paramNames = this.parameterNameDiscoverer.getParameterNames(this.method);
+		int paramCount = (paramNames != null ? paramNames.length : this.method.getParameterCount());
+		int argsCount = this.arguments.length;
 
-		String[] parameterNames = this.paramDiscoverer.getParameterNames(this.method);
-		// save parameter names (if discovered)
-		if (parameterNames != null) {
-			for (int i = 0; i < this.args.length; i++) {
-				setVariable(parameterNames[i], this.args[i]);
+		for (int i = 0; i < paramCount; i++) {
+			Object value = null;
+			if (argsCount > paramCount && i == paramCount - 1) {
+				// Expose remaining arguments as vararg array for last parameter
+				value = Arrays.copyOfRange(this.arguments, i, argsCount);
+			}
+			else if (argsCount > i) {
+				// Actual argument found - otherwise left as null
+				value = this.arguments[i];
+			}
+			setVariable("a" + i, value);
+			setVariable("p" + i, value);
+			if (paramNames != null) {
+				setVariable(paramNames[i], value);
 			}
 		}
 	}

spring-context/src/test/java/org/springframework/context/expression/MethodBasedEvaluationContextTests.java

@@ -30,17 +30,18 @@ import static org.junit.Assert.*;
  * Unit tests for {@link MethodBasedEvaluationContext}.
  *
  * @author Stephane Nicoll
+ * @author Juergen Hoeller
  * @author Sergey Podgurskiy
  */
 public class MethodBasedEvaluationContextTests {
 
 	private final ParameterNameDiscoverer paramDiscover = new DefaultParameterNameDiscoverer();
 
+
 	@Test
 	public void simpleArguments() {
-		Method method = ReflectionUtils.findMethod(SampleMethods.class, "hello",
-				String.class, Boolean.class);
-		MethodBasedEvaluationContext context = createEvaluationContext(method, new Object[] {"test", true});
+		Method method = ReflectionUtils.findMethod(SampleMethods.class, "hello", String.class, Boolean.class);
+		MethodBasedEvaluationContext context = createEvaluationContext(method, "test", true);
 
 		assertEquals("test", context.lookupVariable("a0"));
 		assertEquals("test", context.lookupVariable("p0"));
@@ -51,16 +52,21 @@ public class MethodBasedEvaluationContextTests {
 		assertEquals(true, context.lookupVariable("flag"));
 
 		assertNull(context.lookupVariable("a2"));
+		assertNull(context.lookupVariable("p2"));
 	}
 
 	@Test
 	public void nullArgument() {
-		Method method = ReflectionUtils.findMethod(SampleMethods.class, "hello",
-				String.class, Boolean.class);
-		MethodBasedEvaluationContext context = createEvaluationContext(method, new Object[] {null, null});
+		Method method = ReflectionUtils.findMethod(SampleMethods.class, "hello", String.class, Boolean.class);
+		MethodBasedEvaluationContext context = createEvaluationContext(method, null, null);
 
 		assertNull(context.lookupVariable("a0"));
 		assertNull(context.lookupVariable("p0"));
+		assertNull(context.lookupVariable("foo"));
+
+		assertNull(context.lookupVariable("a1"));
+		assertNull(context.lookupVariable("p1"));
+		assertNull(context.lookupVariable("flag"));
 	}
 
 	@Test
@@ -68,39 +74,58 @@ public class MethodBasedEvaluationContextTests {
 		Method method = ReflectionUtils.findMethod(SampleMethods.class, "hello", Boolean.class, String[].class);
 		MethodBasedEvaluationContext context = createEvaluationContext(method, new Object[] {null});
 
+		assertNull(context.lookupVariable("a0"));
 		assertNull(context.lookupVariable("p0"));
+		assertNull(context.lookupVariable("flag"));
+
+		assertNull(context.lookupVariable("a1"));
 		assertNull(context.lookupVariable("p1"));
+		assertNull(context.lookupVariable("vararg"));
 	}
 
 	@Test
 	public void varArgNull() {
 		Method method = ReflectionUtils.findMethod(SampleMethods.class, "hello", Boolean.class, String[].class);
-		MethodBasedEvaluationContext context = createEvaluationContext(method, new Object[] {null, null});
+		MethodBasedEvaluationContext context = createEvaluationContext(method, null, null);
 
+		assertNull(context.lookupVariable("a0"));
 		assertNull(context.lookupVariable("p0"));
+		assertNull(context.lookupVariable("flag"));
+
+		assertNull(context.lookupVariable("a1"));
 		assertNull(context.lookupVariable("p1"));
+		assertNull(context.lookupVariable("vararg"));
 	}
 
 	@Test
 	public void varArgSingle() {
 		Method method = ReflectionUtils.findMethod(SampleMethods.class, "hello", Boolean.class, String[].class);
-		MethodBasedEvaluationContext context = createEvaluationContext(method, new Object[] {null, "hello"});
+		MethodBasedEvaluationContext context = createEvaluationContext(method, null, "hello");
 
+		assertNull(context.lookupVariable("a0"));
 		assertNull(context.lookupVariable("p0"));
+		assertNull(context.lookupVariable("flag"));
+
+		assertEquals("hello", context.lookupVariable("a1"));
 		assertEquals("hello", context.lookupVariable("p1"));
+		assertEquals("hello", context.lookupVariable("vararg"));
 	}
 
 	@Test
 	public void varArgMultiple() {
 		Method method = ReflectionUtils.findMethod(SampleMethods.class, "hello", Boolean.class, String[].class);
-		MethodBasedEvaluationContext context = createEvaluationContext(method,
-				new Object[] {null, new String[]{"hello", "hi"}});
+		MethodBasedEvaluationContext context = createEvaluationContext(method, null, "hello", "hi");
 
+		assertNull(context.lookupVariable("a0"));
 		assertNull(context.lookupVariable("p0"));
-		assertArrayEquals(new String[]{"hello", "hi"}, (String[]) context.lookupVariable("p1"));
+		assertNull(context.lookupVariable("flag"));
+
+		assertArrayEquals(new Object[] {"hello", "hi"}, (Object[]) context.lookupVariable("a1"));
+		assertArrayEquals(new Object[] {"hello", "hi"}, (Object[]) context.lookupVariable("p1"));
+		assertArrayEquals(new Object[] {"hello", "hi"}, (Object[]) context.lookupVariable("vararg"));
 	}
 
-	private MethodBasedEvaluationContext createEvaluationContext(Method method, Object[] args) {
+	private MethodBasedEvaluationContext createEvaluationContext(Method method, Object... args) {
 		return new MethodBasedEvaluationContext(this, method, args, this.paramDiscover);
 	}
 
@@ -112,9 +137,7 @@ public class MethodBasedEvaluationContextTests {
 		}
 
 		private void hello(Boolean flag, String... vararg){
-
 		}
-
 	}
 
-}
+}