root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Fix SpEL varargs handling and usage of other getValue() methods 

Building on the initial work for SPR-12326, this commit
addresses three problems:

Firstly the ReflectiveMethodResolver is modified to consider
a direct parameter match more important than a varargs match.
Also in that same type when there are a number of close
matches, the first one is taken rather than the last one.

Secondly more testcases and better support have been added
for the case of passing a single parameter to a varargs
accepting method.

Finally it is possible to set the root context object
indirectly and not pass it on getValue() calls to the
expression objects but not all variants of getValue()
were handling that. This is now fixed.

Issue: SPR-12326
This commit is contained in:
Andy Clement 2014-10-21 08:43:51 -07:00
parent 369cabf064
commit bc8e4d3680
5 changed files with 346 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java
View File

@ -109,7 +109,8 @@ public class SpelExpression implements Expression {
Object result;
if (this.compiledAst != null) {
try {
return this.compiledAst.getValue(null,null);
TypedValue contextRoot = evaluationContext == null ? null : evaluationContext.getRootObject();
return this.compiledAst.getValue(contextRoot == null ? null : contextRoot.getValue(), evaluationContext);
}
catch (Throwable ex) {
// If running in mixed mode, revert to interpreted
@ -134,7 +135,7 @@ public class SpelExpression implements Expression {
Object result;
if (this.compiledAst != null) {
try {
return this.compiledAst.getValue(rootObject,null);
return this.compiledAst.getValue(rootObject, evaluationContext);
}
catch (Throwable ex) {
// If running in mixed mode, revert to interpreted
@ -159,7 +160,8 @@ public class SpelExpression implements Expression {
public <T> T getValue(Class<T> expectedResultType) throws EvaluationException {
if (this.compiledAst != null) {
try {
Object result = this.compiledAst.getValue(null,null);
TypedValue contextRoot = evaluationContext == null ? null : evaluationContext.getRootObject();
Object result = this.compiledAst.getValue(contextRoot == null ? null : contextRoot.getValue(), evaluationContext);
if (expectedResultType == null) {
return (T)result;
}

81
spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java
View File

@ -253,8 +253,11 @@ public class ReflectionHelper {
if (varargsPosition == arguments.length - 1) {
TypeDescriptor targetType = new TypeDescriptor(methodParam);
Object argument = arguments[varargsPosition];
arguments[varargsPosition] = converter.convertValue(argument, TypeDescriptor.forObject(argument), targetType);
conversionOccurred |= (argument != arguments[varargsPosition]);
TypeDescriptor sourceType = TypeDescriptor.forObject(argument);
arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
if (!looksLikeSimpleArrayPackaging(sourceType, targetType)) {
conversionOccurred |= (argument != arguments[varargsPosition]);
}
}
else {
TypeDescriptor targetType = new TypeDescriptor(methodParam).getElementTypeDescriptor();
@ -268,6 +271,80 @@ public class ReflectionHelper {
return conversionOccurred;
}
/**
* Check if the target type simply represents the array (possibly boxed/unboxed) form of sourceType.
* @param sourceType the type of the original argument
* @param actualType the type of the converted argument
* @return
*/
private static boolean looksLikeSimpleArrayPackaging(TypeDescriptor sourceType, TypeDescriptor targetType) {
TypeDescriptor td = targetType.getElementTypeDescriptor();
if (td != null) {
if (td.equals(sourceType)) {
return true;
}
else { // check for boxing
if (td.isPrimitive() || sourceType.isPrimitive()) {
Class<?> targetElementClass = td.getType();
Class<?> sourceElementClass = sourceType.getType();
if (targetElementClass.isPrimitive()) {
if (targetElementClass == Boolean.TYPE) {
return sourceElementClass == Boolean.class;
}
else if (targetElementClass == Double.TYPE) {
return sourceElementClass == Double.class;
}
else if (targetElementClass == Float.TYPE) {
return sourceElementClass == Float.class;
}
else if (targetElementClass == Integer.TYPE) {
return sourceElementClass == Integer.class;
}
else if (targetElementClass == Long.TYPE) {
return sourceElementClass == Long.class;
}
else if (targetElementClass == Short.TYPE) {
return sourceElementClass == Short.class;
}
else if (targetElementClass == Character.TYPE) {
return sourceElementClass == Character.class;
}
else if (targetElementClass == Byte.TYPE) {
return sourceElementClass == Byte.class;
}
}
else if (sourceElementClass.isPrimitive()) {
if (sourceElementClass == Boolean.TYPE) {
return targetElementClass == Boolean.class;
}
else if (sourceElementClass == Double.TYPE) {
return targetElementClass == Double.class;
}
else if (sourceElementClass == Float.TYPE) {
return targetElementClass == Float.class;
}
else if (sourceElementClass == Integer.TYPE) {
return targetElementClass == Integer.class;
}
else if (sourceElementClass == Long.TYPE) {
return targetElementClass == Long.class;
}
else if (sourceElementClass == Character.TYPE) {
return targetElementClass == Character.class;
}
else if (sourceElementClass == Short.TYPE) {
return targetElementClass == Short.class;
}
else if (sourceElementClass == Byte.TYPE) {
return targetElementClass == Byte.class;
}
}
}
}
}
return false;
}
/**
* Convert a supplied set of arguments into the requested types. If the parameterTypes are related to
* a varargs method then the final entry in the parameterTypes array is going to be an array itself whose

17
spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java
View File

@ -123,6 +123,18 @@ public class ReflectiveMethodResolver implements MethodResolver {
public int compare(Method m1, Method m2) {
int m1pl = m1.getParameterTypes().length;
int m2pl = m2.getParameterTypes().length;
// varargs methods go last
if (m1pl == m2pl) {
if (!m1.isVarArgs() && m2.isVarArgs()) {
return -1;
}
else if (m1.isVarArgs() && !m2.isVarArgs()) {
return 1;
}
else {
return 0;
}
}
return (new Integer(m1pl)).compareTo(m2pl);
}
});
@ -163,7 +175,10 @@ public class ReflectiveMethodResolver implements MethodResolver {
}
else if (matchInfo.isCloseMatch()) {
if (!this.useDistance) {
closeMatch = method;
// Take this as a close match if there isn't one already
if (closeMatch == null) {
closeMatch = method;
}
}
else {
int matchDistance = ReflectionHelper.getTypeDifferenceWeight(paramDescriptors, argumentTypes);

30
spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java
View File

@ -18,6 +18,7 @@ package org.springframework.expression.spel;
import java.lang.reflect.Method;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@ -464,6 +465,35 @@ public class EvaluationTests extends AbstractExpressionTests {
assertEquals("T(java.lang.String)", expr.toStringAST());
assertEquals(String.class, expr.getValue(Class.class));
}
@Test
public void operatorVariants() throws Exception {
SpelExpression expr = (SpelExpression)parser.parseExpression("#a < #b");
EvaluationContext ctx = new StandardEvaluationContext();
ctx.setVariable("a", (short)3);
ctx.setVariable("b", (short)6);
assertTrue(expr.getValue(ctx, Boolean.class));
ctx.setVariable("b", (byte)6);
assertTrue(expr.getValue(ctx, Boolean.class));
ctx.setVariable("a", (byte)9);
ctx.setVariable("b", (byte)6);
assertFalse(expr.getValue(ctx, Boolean.class));
ctx.setVariable("a", 10L);
ctx.setVariable("b", (short)30);
assertTrue(expr.getValue(ctx, Boolean.class));
ctx.setVariable("a", (byte)3);
ctx.setVariable("b", (short)30);
assertTrue(expr.getValue(ctx, Boolean.class));
ctx.setVariable("a", (byte)3);
ctx.setVariable("b", 30L);
assertTrue(expr.getValue(ctx, Boolean.class));
ctx.setVariable("a", (byte)3);
ctx.setVariable("b", 30f);
assertTrue(expr.getValue(ctx, Boolean.class));
ctx.setVariable("a", new BigInteger("10"));
ctx.setVariable("b", new BigInteger("20"));
assertTrue(expr.getValue(ctx, Boolean.class));
}
@Test
public void testTypeReferencesPrimitive() {

218
spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java
View File

@ -1685,6 +1685,21 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertEquals(1.0f,expression.getValue());
}
@Test
public void failsWhenSettingContextForExpression_SPR12326() {
SpelExpressionParser parser = new SpelExpressionParser(new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, this
.getClass().getClassLoader()));
Person3 person = new Person3("foo", 1);
SpelExpression expression = parser.parseRaw("#it?.age?.equals([0])");
StandardEvaluationContext context = new StandardEvaluationContext(new Object[] { 1 });
context.setVariable("it", person);
expression.setEvaluationContext(context);
assertTrue(expression.getValue(Boolean.class));
assertTrue(expression.getValue(Boolean.class));
assertCanCompile(expression);
assertTrue(expression.getValue(Boolean.class));
}
@Test
public void constructorReference_SPR12326() {
String type = this.getClass().getName();
@ -1806,6 +1821,23 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
this.age = age;
}
}
public class Person3 {
private int age;
public Person3(String name, int age) {
this.age = age;
}
public int getAge() {
return age;
}
public void setAge(int age) {
this.age = age;
}
}
@Test
public void constructorReference() throws Exception {
@ -1851,6 +1883,57 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertCantCompile(expression);
}
@Test
public void methodReferenceReflectiveMethodSelectionWithVarargs() throws Exception {
TestClass10 tc = new TestClass10();
// Should call the non varargs version of concat
// (which causes the '::' prefix in test output)
expression = parser.parseExpression("concat('test')");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("::test",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("::test",tc.s);
tc.reset();
// This will call the varargs concat with an empty array
expression = parser.parseExpression("concat()");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("",tc.s);
tc.reset();
// Should call the non varargs version of concat
// (which causes the '::' prefix in test output)
expression = parser.parseExpression("concat2('test')");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("::test",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("::test",tc.s);
tc.reset();
// This will call the varargs concat with an empty array
expression = parser.parseExpression("concat2()");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("",tc.s);
tc.reset();
}
@Test
public void methodReferenceVarargs() throws Exception {
TestClass5 tc = new TestClass5();
@ -1866,6 +1949,17 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertEquals("",tc.s);
tc.reset();
// varargs string
expression = parser.parseExpression("eleven('aaa')");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("aaa",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("aaa",tc.s);
tc.reset();
// varargs string
expression = parser.parseExpression("eleven(stringArray)");
assertCantCompile(expression);
@ -1898,6 +1992,16 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
expression.getValue(tc);
assertEquals(6,tc.i);
tc.reset();
expression = parser.parseExpression("twelve(1)");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals(1,tc.i);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals(1,tc.i);
tc.reset();
// one string then varargs string
expression = parser.parseExpression("thirteen('aaa','bbb','ccc')");
@ -1954,6 +2058,16 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertEquals("truetruefalse",tc.s);
tc.reset();
expression = parser.parseExpression("arrayz(true)");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("true",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("true",tc.s);
tc.reset();
// varargs short
expression = parser.parseExpression("arrays(s1,s2,s3)");
assertCantCompile(expression);
@ -1965,6 +2079,16 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertEquals("123",tc.s);
tc.reset();
expression = parser.parseExpression("arrays(s1)");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("1",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("1",tc.s);
tc.reset();
// varargs double
expression = parser.parseExpression("arrayd(1.0d,2.0d,3.0d)");
assertCantCompile(expression);
@ -1976,6 +2100,16 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertEquals("1.02.03.0",tc.s);
tc.reset();
expression = parser.parseExpression("arrayd(1.0d)");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("1.0",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("1.0",tc.s);
tc.reset();
// varargs long
expression = parser.parseExpression("arrayj(l1,l2,l3)");
assertCantCompile(expression);
@ -1986,7 +2120,17 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
expression.getValue(tc);
assertEquals("123",tc.s);
tc.reset();
expression = parser.parseExpression("arrayj(l1)");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("1",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("1",tc.s);
tc.reset();
// varargs char
expression = parser.parseExpression("arrayc(c1,c2,c3)");
assertCantCompile(expression);
@ -1997,7 +2141,17 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
expression.getValue(tc);
assertEquals("abc",tc.s);
tc.reset();
expression = parser.parseExpression("arrayc(c1)");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("a",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("a",tc.s);
tc.reset();
// varargs byte
expression = parser.parseExpression("arrayb(b1,b2,b3)");
assertCantCompile(expression);
@ -2008,6 +2162,16 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
expression.getValue(tc);
assertEquals("656667",tc.s);
tc.reset();
expression = parser.parseExpression("arrayb(b1)");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("65",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("65",tc.s);
tc.reset();
// varargs float
expression = parser.parseExpression("arrayf(f1,f2,f3)");
@ -2019,6 +2183,16 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
expression.getValue(tc);
assertEquals("1.02.03.0",tc.s);
tc.reset();
expression = parser.parseExpression("arrayf(f1)");
assertCantCompile(expression);
expression.getValue(tc);
assertEquals("1.0",tc.s);
assertCanCompile(expression);
tc.reset();
expression.getValue(tc);
assertEquals("1.0",tc.s);
tc.reset();
}
@Test
@ -3307,6 +3481,46 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
public boolean getB() { return b; }
}
public static class TestClass10 {
public String s = null;
public void reset() {
s = null;
}
public void concat(String arg) {
s = "::"+arg;
}
public void concat(String... vargs) {
if (vargs==null) {
s = "";
}
else {
s = "";
for (String varg: vargs) {
s+=varg;
}
}
}
public void concat2(Object arg) {
s = "::"+arg;
}
public void concat2(Object... vargs) {
if (vargs==null) {
s = "";
}
else {
s = "";
for (Object varg: vargs) {
s+=varg;
}
}
}
}
public static class TestClass5 {
public int i = 0;
public String s = null;