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

Fix SpEL compilation of constructor invocation 

The argument processing for compiling constructor references
was very basic and this fix removes that and ensures the
comprehensive logic written for method argument processing
(under SPR-12328) is now used for both method and constructor
argument handling. This fixes the reported issue and ensures
varargs constructor references can be compiled.

This also includes a couple of small fixes for the secondary
testcase reported in SPR-12326. The first is to ensure the
right root context object is used when it is passed
to getValue() indirectly through the evaluation context.
The final fix is to ensure correct boxing of primitives is
done when a method is called upon a primitive.

Issue: SPR-12326
This commit is contained in:
Andy Clement 2014-10-17 10:26:54 -07:00
parent c5e360d886
commit aae221cb15
7 changed files with 252 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java
View File

@ -17,6 +17,7 @@
package org.springframework.expression.spel;
import java.lang.reflect.Constructor;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
@ -837,12 +838,23 @@ public class CodeFlow implements Opcodes {
* packaged into an array.
* @param mv the method visitor where code should be generated
* @param cf the current codeflow
* @param method the method for which arguments are being setup
* @param member the method or constructor for which arguments are being setup
* @param arguments the expression nodes for the expression supplied argument values
*/
public static void generateCodeForArguments(MethodVisitor mv, CodeFlow cf, Method method, SpelNodeImpl[] arguments) {
String[] paramDescriptors = CodeFlow.toParamDescriptors(method);
if (method.isVarArgs()) {
public static void generateCodeForArguments(MethodVisitor mv, CodeFlow cf, Member member, SpelNodeImpl[] arguments) {
String[] paramDescriptors = null;
boolean isVarargs = false;
if (member instanceof Constructor) {
Constructor<?> ctor = (Constructor<?>)member;
paramDescriptors = toDescriptors(ctor.getParameterTypes());
isVarargs = ctor.isVarArgs();
}
else { // Method
Method method = (Method)member;
paramDescriptors = toDescriptors(method.getParameterTypes());
isVarargs = method.isVarArgs();
}
if (isVarargs) {
// The final parameter may or may not need packaging into an array, or nothing may
// have been passed to satisfy the varargs and so something needs to be built.
int p = 0; // Current supplied argument being processed

20
spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java
View File

@ -437,29 +437,21 @@ public class ConstructorReference extends SpelNodeImpl {
ReflectiveConstructorExecutor executor = (ReflectiveConstructorExecutor) this.cachedExecutor;
Constructor<?> constructor = executor.getConstructor();
return (!constructor.isVarArgs() && Modifier.isPublic(constructor.getModifiers()) &&
return (Modifier.isPublic(constructor.getModifiers()) &&
Modifier.isPublic(constructor.getDeclaringClass().getModifiers()));
}
@Override
public void generateCode(MethodVisitor mv, CodeFlow cf) {
ReflectiveConstructorExecutor executor = ((ReflectiveConstructorExecutor) this.cachedExecutor);
Constructor<?> constructor = executor.getConstructor();
Constructor<?> constructor = executor.getConstructor();
String classSlashedDescriptor = constructor.getDeclaringClass().getName().replace('.', '/');
String[] paramDescriptors = CodeFlow.toParamDescriptors(constructor);
mv.visitTypeInsn(NEW, classSlashedDescriptor);
mv.visitInsn(DUP);
for (int c = 1; c < this.children.length; c++) { // children[0] is the type of the constructor
SpelNodeImpl child = this.children[c];
cf.enterCompilationScope();
child.generateCode(mv, cf);
// Check if need to box it for the method reference?
if (CodeFlow.isPrimitive(cf.lastDescriptor()) && paramDescriptors[c-1].charAt(0) == 'L') {
CodeFlow.insertBoxIfNecessary(mv, cf.lastDescriptor().charAt(0));
}
cf.exitCompilationScope();
}
// children[0] is the type of the constructor, don't want to include that in argument processing
SpelNodeImpl[] arguments = new SpelNodeImpl[children.length-1];
System.arraycopy(children, 1, arguments, 0, children.length-1);
CodeFlow.generateCodeForArguments(mv, cf, constructor, arguments);
mv.visitMethodInsn(INVOKESPECIAL, classSlashedDescriptor, "<init>",
CodeFlow.createSignatureDescriptor(constructor), false);
cf.pushDescriptor(this.exitTypeDescriptor);

4
spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java
View File

@ -295,6 +295,10 @@ public class MethodReference extends SpelNodeImpl {
if (descriptor == null && !isStaticMethod) {
cf.loadTarget(mv);
}
if (CodeFlow.isPrimitive(descriptor)) {
CodeFlow.insertBoxIfNecessary(mv, descriptor.charAt(0));
}
boolean itf = method.getDeclaringClass().isInterface();
String methodDeclaringClassSlashedDescriptor = null;

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

@ -221,7 +221,8 @@ public class SpelExpression implements Expression {
Assert.notNull(context, "The EvaluationContext is required");
if (compiledAst!= null) {
try {
Object result = this.compiledAst.getValue(null,context);
TypedValue contextRoot = context == null ? null : context.getRootObject();
Object result = this.compiledAst.getValue(contextRoot==null?null:contextRoot.getValue(),context);
return result;
}
catch (Throwable ex) {
@ -272,7 +273,8 @@ public class SpelExpression implements Expression {
public <T> T getValue(EvaluationContext context, Class<T> expectedResultType) throws EvaluationException {
if (this.compiledAst != null) {
try {
Object result = this.compiledAst.getValue(null,context);
TypedValue contextRoot = context == null ? null : context.getRootObject();
Object result = this.compiledAst.getValue(contextRoot==null?null:contextRoot.getValue(),context);
if (expectedResultType != null) {
return ExpressionUtils.convertTypedValue(context, new TypedValue(result), expectedResultType);
}

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

@ -38,6 +38,7 @@ import org.springframework.expression.spel.standard.SpelCompiler;
import org.springframework.expression.spel.standard.SpelExpression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.expression.spel.testdata.PersonInOtherPackage;
import static org.junit.Assert.*;
@ -1683,6 +1684,128 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertEquals(1.0f,expression.getValue());
}
@Test
public void constructorReference_SPR12326() {
String type = this.getClass().getName();
String prefix = "new "+type+".Obj";
expression = parser.parseExpression(prefix+"([0])");
assertEquals("test", ((Obj) expression.getValue(new Object[] { "test" })).param1);
assertCanCompile(expression);
assertEquals("test", ((Obj) expression.getValue(new Object[] { "test" })).param1);
expression = parser.parseExpression(prefix+"2('foo','bar').output");
assertEquals("foobar", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("foobar", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"2('foo').output");
assertEquals("foo", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("foo", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"2().output");
assertEquals("", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"3(1,2,3).output");
assertEquals("123", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("123", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"3(1).output");
assertEquals("1", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("1", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"3().output");
assertEquals("", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"3('abc',5.0f,1,2,3).output");
assertEquals("abc:5.0:123", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("abc:5.0:123", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"3('abc',5.0f,1).output");
assertEquals("abc:5.0:1", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("abc:5.0:1", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"3('abc',5.0f).output");
assertEquals("abc:5.0:", expression.getValue(String.class));
assertCanCompile(expression);
assertEquals("abc:5.0:", expression.getValue(String.class));
expression = parser.parseExpression(prefix+"4(#root).output");
assertEquals("123", expression.getValue(new int[]{1,2,3},String.class));
assertCanCompile(expression);
assertEquals("123", expression.getValue(new int[]{1,2,3},String.class));
}
@Test
public void methodReferenceMissingCastAndRootObjectAccessing_SPR12326() {
// Need boxing code on the 1 so that toString() can be called
expression = parser.parseExpression("1.toString()");
assertEquals("1", expression.getValue());
assertCanCompile(expression);
assertEquals("1", expression.getValue());
expression = parser.parseExpression("#it?.age.equals([0])");
Person person = new Person(1);
StandardEvaluationContext context =
new StandardEvaluationContext(new Object[] { person.getAge() });
context.setVariable("it", person);
assertTrue(expression.getValue(context, Boolean.class));
assertCanCompile(expression);
assertTrue(expression.getValue(context, Boolean.class));
// Variant of above more like what was in the bug report:
SpelExpressionParser parser = new SpelExpressionParser(
new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE,
this.getClass().getClassLoader()));
SpelExpression ex = parser.parseRaw("#it?.age.equals([0])");
context = new StandardEvaluationContext(new Object[] { person.getAge() });
context.setVariable("it", person);
assertTrue(ex.getValue(context, Boolean.class));
assertTrue(ex.getValue(context, Boolean.class));
PersonInOtherPackage person2 = new PersonInOtherPackage(1);
ex = parser.parseRaw("#it?.age.equals([0])");
context =
new StandardEvaluationContext(new Object[] { person2.getAge() });
context.setVariable("it", person2);
assertTrue(ex.getValue(context, Boolean.class));
assertTrue(ex.getValue(context, Boolean.class));
ex = parser.parseRaw("#it?.age.equals([0])");
context =
new StandardEvaluationContext(new Object[] { person2.getAge() });
context.setVariable("it", person2);
assertTrue((Boolean)ex.getValue(context));
assertTrue((Boolean)ex.getValue(context));
}
public class Person {
private int age;
public Person(int age) {
this.age = age;
}
public int getAge() {
return age;
}
public void setAge(int age) {
this.age = age;
}
}
@Test
public void constructorReference() throws Exception {
// simple ctor
@ -3440,6 +3563,66 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
this.s = a+b;
}
}
public static class Obj {
private final String param1;
public Obj(String param1){
this.param1 = param1;
}
}
public static class Obj2 {
public final String output;
public Obj2(String... params){
StringBuilder b = new StringBuilder();
for (String param: params) {
b.append(param);
}
output = b.toString();
}
}
public static class Obj3 {
public final String output;
public Obj3(int... params) {
StringBuilder b = new StringBuilder();
for (int param: params) {
b.append(Integer.toString(param));
}
output = b.toString();
}
public Obj3(String s, Float f, int... ints) {
StringBuilder b = new StringBuilder();
b.append(s);
b.append(":");
b.append(Float.toString(f));
b.append(":");
for (int param: ints) {
b.append(Integer.toString(param));
}
output = b.toString();
}
}
public static class Obj4 {
public final String output;
public Obj4(int[] params) {
StringBuilder b = new StringBuilder();
for (int param: params) {
b.append(Integer.toString(param));
}
output = b.toString();
}
}
@SuppressWarnings("unused")
private static class TestClass9 {

2
spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationPerformanceTests.java
View File

@ -156,7 +156,7 @@ public class SpelCompilationPerformanceTests extends AbstractExpressionTests {
Object o = expression.getValue(g);
assertEquals("helloworld spring", o);
System.out.println("Performance check for SpEL expression: '{'abcde','ijklm'}[0].substring({1,3,4}[0],{1,3,4}[1])'");
System.out.println("Performance check for SpEL expression: 'hello' + getWorld() + ' spring'");
long stime = System.currentTimeMillis();
for (int i=0;i<1000000;i++) {
o = expression.getValue(g);

38
spring-expression/src/test/java/org/springframework/expression/spel/testdata/PersonInOtherPackage.java vendored Normal file
View File

@ -0,0 +1,38 @@
/*
* Copyright 2014 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.expression.spel.testdata;
/**
*
* @author Andy Clement
* @since 4.1.2
*/
public class PersonInOtherPackage {
private int age;
public PersonInOtherPackage(int age) {
this.age = age;
}
public int getAge() {
return age;
}
public void setAge(int age) {
this.age = age;
}
}