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 e01e1e1403..1e88cebc96 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 @@ -18,6 +18,7 @@ package org.springframework.expression.spel.ast; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.springframework.core.convert.TypeDescriptor; @@ -45,7 +46,7 @@ public class MethodReference extends SpelNodeImpl { private final boolean nullSafe; - private volatile MethodExecutor cachedExecutor; + private volatile CachedMethodExecutor cachedExecutor; public MethodReference(boolean nullSafe, String methodName, int pos, SpelNodeImpl... arguments) { @@ -101,17 +102,18 @@ public class MethodReference extends SpelNodeImpl { state.popActiveContextObject(); } } + List argumentTypes = getTypes(arguments); if (currentContext.getValue() == null) { if (this.nullSafe) { return TypedValue.NULL; } else { throw new SpelEvaluationException(getStartPosition(), SpelMessage.METHOD_CALL_ON_NULL_OBJECT_NOT_ALLOWED, - FormatHelper.formatMethodForMessage(this.name, getTypes(arguments))); + FormatHelper.formatMethodForMessage(this.name, argumentTypes)); } } - MethodExecutor executorToUse = this.cachedExecutor; + MethodExecutor executorToUse = getCachedExecutor(argumentTypes); if (executorToUse != null) { try { return executorToUse.execute(state.getEvaluationContext(), @@ -136,8 +138,8 @@ public class MethodReference extends SpelNodeImpl { } // either there was no accessor or it no longer existed - executorToUse = findAccessorForMethod(this.name, getTypes(arguments), state); - this.cachedExecutor = executorToUse; + executorToUse = findAccessorForMethod(this.name, argumentTypes, state); + this.cachedExecutor = new CachedMethodExecutor(executorToUse, argumentTypes); try { return executorToUse.execute(state.getEvaluationContext(), state.getActiveContextObject().getValue(), arguments); @@ -172,7 +174,7 @@ public class MethodReference extends SpelNodeImpl { for (Object argument : arguments) { descriptors.add(TypeDescriptor.forObject(argument)); } - return descriptors; + return Collections.unmodifiableList(descriptors); } @Override @@ -225,6 +227,14 @@ public class MethodReference extends SpelNodeImpl { : contextObject.getClass())); } + private MethodExecutor getCachedExecutor(List argumentTypes) { + if (this.cachedExecutor == null || !this.cachedExecutor.isSuitable(argumentTypes)) { + this.cachedExecutor = null; + return null; + } + return this.cachedExecutor.get(); + } + private class MethodValueRef implements ValueRef { @@ -236,18 +246,21 @@ public class MethodReference extends SpelNodeImpl { private final Object[] arguments; + private List argumentTypes; + MethodValueRef(ExpressionState state, EvaluationContext evaluationContext, Object object, Object[] arguments) { this.state = state; this.evaluationContext = evaluationContext; this.target = object; this.arguments = arguments; + this.argumentTypes = getTypes(this.arguments); } @Override public TypedValue getValue() { - MethodExecutor executorToUse = MethodReference.this.cachedExecutor; + MethodExecutor executorToUse = getCachedExecutor(this.argumentTypes); if (executorToUse != null) { try { return executorToUse.execute(this.evaluationContext, this.target, this.arguments); @@ -271,8 +284,8 @@ public class MethodReference extends SpelNodeImpl { } // either there was no accessor or it no longer existed - executorToUse = findAccessorForMethod(MethodReference.this.name, getTypes(this.arguments), this.target, this.evaluationContext); - MethodReference.this.cachedExecutor = executorToUse; + executorToUse = findAccessorForMethod(MethodReference.this.name, argumentTypes, this.target, this.evaluationContext); + MethodReference.this.cachedExecutor = new CachedMethodExecutor(executorToUse, this.argumentTypes); try { return executorToUse.execute(this.evaluationContext, this.target, this.arguments); } @@ -297,4 +310,27 @@ public class MethodReference extends SpelNodeImpl { } } + + private static class CachedMethodExecutor { + + private final MethodExecutor methodExecutor; + + private final List argumentTypes; + + + public CachedMethodExecutor(MethodExecutor methodExecutor, + List argumentTypes) { + this.methodExecutor = methodExecutor; + this.argumentTypes = argumentTypes; + } + + + public boolean isSuitable(List argumentTypes) { + return (this.methodExecutor != null && this.argumentTypes.equals(argumentTypes)); + } + + public MethodExecutor get() { + return this.methodExecutor; + } + } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java new file mode 100644 index 0000000000..75e76a5903 --- /dev/null +++ b/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java @@ -0,0 +1,75 @@ +/* + * Copyright 2002-2012 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; + +import org.junit.Before; +import org.junit.Test; +import org.springframework.expression.Expression; +import org.springframework.expression.ExpressionParser; +import org.springframework.expression.spel.ast.MethodReference; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.support.StandardEvaluationContext; + +import static org.junit.Assert.*; + +/** + * Test for caching in {@link MethodReference} (SPR-10657). + * + * @author Oliver Becker + */ +public class CachedMethodExecutorTests { + + private final ExpressionParser parser = new SpelExpressionParser(); + + private StandardEvaluationContext context; + + + @Before + public void setUp() throws Exception { + this.context = new StandardEvaluationContext(new RootObject()); + } + + + @Test + public void testCachedExecution() throws Exception { + Expression expression = this.parser.parseExpression("echo(#something)"); + + assertMethodExecution(expression, 42, "int: 42"); + assertMethodExecution(expression, 42, "int: 42"); + assertMethodExecution(expression, "Deep Thought", "String: Deep Thought"); + assertMethodExecution(expression, 42, "int: 42"); + } + + private void assertMethodExecution(Expression expression, Object var, String expected) { + this.context.setVariable("something", var); + assertEquals(expected, expression.getValue(this.context)); + } + + + public static class RootObject { + + public String echo(String value) { + return "String: " + value; + } + + public String echo(int value) { + return "int: " + value; + } + + } + +}