Skip to content

Commit bc8e4d3

Browse files
aclementaclement
committed
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
1 parent 369cabf commit bc8e4d3

File tree

5 files changed

+346
-8
lines changed

5 files changed

+346
-8
lines changed

‎spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ public Object getValue() throws EvaluationException {
109109
Object result;
110110
if (this.compiledAst != null) {
111111
try {
112-
return this.compiledAst.getValue(null,null);
112+
TypedValue contextRoot = evaluationContext == null ? null : evaluationContext.getRootObject();
113+
return this.compiledAst.getValue(contextRoot == null ? null : contextRoot.getValue(), evaluationContext);
113114
}
114115
catch (Throwable ex) {
115116
// If running in mixed mode, revert to interpreted
@@ -134,7 +135,7 @@ public Object getValue(Object rootObject) throws EvaluationException {
134135
Object result;
135136
if (this.compiledAst != null) {
136137
try {
137-
return this.compiledAst.getValue(rootObject,null);
138+
return this.compiledAst.getValue(rootObject, evaluationContext);
138139
}
139140
catch (Throwable ex) {
140141
// If running in mixed mode, revert to interpreted
@@ -159,7 +160,8 @@ public Object getValue(Object rootObject) throws EvaluationException {
159160
public <T> T getValue(Class<T> expectedResultType) throws EvaluationException {
160161
if (this.compiledAst != null) {
161162
try {
162-
Object result = this.compiledAst.getValue(null,null);
163+
TypedValue contextRoot = evaluationContext == null ? null : evaluationContext.getRootObject();
164+
Object result = this.compiledAst.getValue(contextRoot == null ? null : contextRoot.getValue(), evaluationContext);
163165
if (expectedResultType == null) {
164166
return (T)result;
165167
}

‎spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,11 @@ static boolean convertArguments(TypeConverter converter, Object[] arguments, Obj
253253
if (varargsPosition == arguments.length - 1) {
254254
TypeDescriptor targetType = new TypeDescriptor(methodParam);
255255
Object argument = arguments[varargsPosition];
256-
arguments[varargsPosition] = converter.convertValue(argument, TypeDescriptor.forObject(argument), targetType);
257-
conversionOccurred |= (argument != arguments[varargsPosition]);
256+
TypeDescriptor sourceType = TypeDescriptor.forObject(argument);
257+
arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
258+
if (!looksLikeSimpleArrayPackaging(sourceType, targetType)) {
259+
conversionOccurred |= (argument != arguments[varargsPosition]);
260+
}
258261
}
259262
else {
260263
TypeDescriptor targetType = new TypeDescriptor(methodParam).getElementTypeDescriptor();
@@ -268,6 +271,80 @@ static boolean convertArguments(TypeConverter converter, Object[] arguments, Obj
268271
return conversionOccurred;
269272
}
270273

274+
/**
275+
* Check if the target type simply represents the array (possibly boxed/unboxed) form of sourceType.
276+
* @param sourceType the type of the original argument
277+
* @param actualType the type of the converted argument
278+
* @return
279+
*/
280+
private static boolean looksLikeSimpleArrayPackaging(TypeDescriptor sourceType, TypeDescriptor targetType) {
281+
TypeDescriptor td = targetType.getElementTypeDescriptor();
282+
if (td != null) {
283+
if (td.equals(sourceType)) {
284+
return true;
285+
}
286+
else { // check for boxing
287+
if (td.isPrimitive() || sourceType.isPrimitive()) {
288+
Class<?> targetElementClass = td.getType();
289+
Class<?> sourceElementClass = sourceType.getType();
290+
if (targetElementClass.isPrimitive()) {
291+
if (targetElementClass == Boolean.TYPE) {
292+
return sourceElementClass == Boolean.class;
293+
}
294+
else if (targetElementClass == Double.TYPE) {
295+
return sourceElementClass == Double.class;
296+
}
297+
else if (targetElementClass == Float.TYPE) {
298+
return sourceElementClass == Float.class;
299+
}
300+
else if (targetElementClass == Integer.TYPE) {
301+
return sourceElementClass == Integer.class;
302+
}
303+
else if (targetElementClass == Long.TYPE) {
304+
return sourceElementClass == Long.class;
305+
}
306+
else if (targetElementClass == Short.TYPE) {
307+
return sourceElementClass == Short.class;
308+
}
309+
else if (targetElementClass == Character.TYPE) {
310+
return sourceElementClass == Character.class;
311+
}
312+
else if (targetElementClass == Byte.TYPE) {
313+
return sourceElementClass == Byte.class;
314+
}
315+
}
316+
else if (sourceElementClass.isPrimitive()) {
317+
if (sourceElementClass == Boolean.TYPE) {
318+
return targetElementClass == Boolean.class;
319+
}
320+
else if (sourceElementClass == Double.TYPE) {
321+
return targetElementClass == Double.class;
322+
}
323+
else if (sourceElementClass == Float.TYPE) {
324+
return targetElementClass == Float.class;
325+
}
326+
else if (sourceElementClass == Integer.TYPE) {
327+
return targetElementClass == Integer.class;
328+
}
329+
else if (sourceElementClass == Long.TYPE) {
330+
return targetElementClass == Long.class;
331+
}
332+
else if (sourceElementClass == Character.TYPE) {
333+
return targetElementClass == Character.class;
334+
}
335+
else if (sourceElementClass == Short.TYPE) {
336+
return targetElementClass == Short.class;
337+
}
338+
else if (sourceElementClass == Byte.TYPE) {
339+
return targetElementClass == Byte.class;
340+
}
341+
}
342+
}
343+
}
344+
}
345+
return false;
346+
}
347+
271348
/**
272349
* Convert a supplied set of arguments into the requested types. If the parameterTypes are related to
273350
* a varargs method then the final entry in the parameterTypes array is going to be an array itself whose

‎spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,18 @@ public MethodExecutor resolve(EvaluationContext context, Object targetObject, St
123123
public int compare(Method m1, Method m2) {
124124
int m1pl = m1.getParameterTypes().length;
125125
int m2pl = m2.getParameterTypes().length;
126+
// varargs methods go last
127+
if (m1pl == m2pl) {
128+
if (!m1.isVarArgs() && m2.isVarArgs()) {
129+
return -1;
130+
}
131+
else if (m1.isVarArgs() && !m2.isVarArgs()) {
132+
return 1;
133+
}
134+
else {
135+
return 0;
136+
}
137+
}
126138
return (new Integer(m1pl)).compareTo(m2pl);
127139
}
128140
});
@@ -163,7 +175,10 @@ else if (paramTypes.length == argumentTypes.size()) {
163175
}
164176
else if (matchInfo.isCloseMatch()) {
165177
if (!this.useDistance) {
166-
closeMatch = method;
178+
// Take this as a close match if there isn't one already
179+
if (closeMatch == null) {
180+
closeMatch = method;
181+
}
167182
}
168183
else {
169184
int matchDistance = ReflectionHelper.getTypeDifferenceWeight(paramDescriptors, argumentTypes);

‎spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.lang.reflect.Method;
2020
import java.math.BigDecimal;
21+
import java.math.BigInteger;
2122
import java.util.ArrayList;
2223
import java.util.List;
2324
import java.util.Map;
@@ -464,6 +465,35 @@ public void testTypeReferencesAndQualifiedIdentifierCaching() throws Exception {
464465
assertEquals("T(java.lang.String)", expr.toStringAST());
465466
assertEquals(String.class, expr.getValue(Class.class));
466467
}
468+
469+
@Test
470+
public void operatorVariants() throws Exception {
471+
SpelExpression expr = (SpelExpression)parser.parseExpression("#a < #b");
472+
EvaluationContext ctx = new StandardEvaluationContext();
473+
ctx.setVariable("a", (short)3);
474+
ctx.setVariable("b", (short)6);
475+
assertTrue(expr.getValue(ctx, Boolean.class));
476+
ctx.setVariable("b", (byte)6);
477+
assertTrue(expr.getValue(ctx, Boolean.class));
478+
ctx.setVariable("a", (byte)9);
479+
ctx.setVariable("b", (byte)6);
480+
assertFalse(expr.getValue(ctx, Boolean.class));
481+
ctx.setVariable("a", 10L);
482+
ctx.setVariable("b", (short)30);
483+
assertTrue(expr.getValue(ctx, Boolean.class));
484+
ctx.setVariable("a", (byte)3);
485+
ctx.setVariable("b", (short)30);
486+
assertTrue(expr.getValue(ctx, Boolean.class));
487+
ctx.setVariable("a", (byte)3);
488+
ctx.setVariable("b", 30L);
489+
assertTrue(expr.getValue(ctx, Boolean.class));
490+
ctx.setVariable("a", (byte)3);
491+
ctx.setVariable("b", 30f);
492+
assertTrue(expr.getValue(ctx, Boolean.class));
493+
ctx.setVariable("a", new BigInteger("10"));
494+
ctx.setVariable("b", new BigInteger("20"));
495+
assertTrue(expr.getValue(ctx, Boolean.class));
496+
}
467497

468498
@Test
469499
public void testTypeReferencesPrimitive() {

0 commit comments

Comments
 (0)