diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java index acdef4d84f7..8b98c97f396 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/CodeFlow.java @@ -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 diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java index 7b72627195d..27319ad60ae 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java @@ -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, "", CodeFlow.createSignatureDescriptor(constructor), false); cf.pushDescriptor(this.exitTypeDescriptor); diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java index d86a71df5e4..addce1f6f3c 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java @@ -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; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java index 9c5012902bf..eba325e4158 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java @@ -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 getValue(EvaluationContext context, Class 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); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 956d9e7911e..e69c52a7ed6 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -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 { diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationPerformanceTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationPerformanceTests.java index 6b3a821570c..87f37e394fa 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationPerformanceTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationPerformanceTests.java @@ -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); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/testdata/PersonInOtherPackage.java b/spring-expression/src/test/java/org/springframework/expression/spel/testdata/PersonInOtherPackage.java new file mode 100644 index 00000000000..add606359f8 --- /dev/null +++ b/spring-expression/src/test/java/org/springframework/expression/spel/testdata/PersonInOtherPackage.java @@ -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; + } +} \ No newline at end of file