From bf4563e204f1695fa8d4d356432bdd6860b1efac Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 28 Jun 2013 13:14:23 +0200 Subject: [PATCH] Include target types in MethodReference cache Update the cached MethodExecutor in MethodReference to include the target type. Prevents the incorrect use of the cache when the SpEL expression refers to a different target object. Issue: SPR-10657 --- .../expression/spel/ast/MethodReference.java | 39 +++++++++++-------- .../spel/CachedMethodExecutorTests.java | 22 +++++++++-- 2 files changed, 41 insertions(+), 20 deletions(-) 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 1e88cebc96..595bddad9d 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 @@ -48,14 +48,12 @@ public class MethodReference extends SpelNodeImpl { private volatile CachedMethodExecutor cachedExecutor; - public MethodReference(boolean nullSafe, String methodName, int pos, SpelNodeImpl... arguments) { super(pos, arguments); this.name = methodName; this.nullSafe = nullSafe; } - public final String getName() { return this.name; } @@ -102,6 +100,9 @@ public class MethodReference extends SpelNodeImpl { state.popActiveContextObject(); } } + TypedValue activeContextObject = state.getActiveContextObject(); + TypeDescriptor target = (activeContextObject == null ? null + : activeContextObject.getTypeDescriptor()); List argumentTypes = getTypes(arguments); if (currentContext.getValue() == null) { if (this.nullSafe) { @@ -113,7 +114,7 @@ public class MethodReference extends SpelNodeImpl { } } - MethodExecutor executorToUse = getCachedExecutor(argumentTypes); + MethodExecutor executorToUse = getCachedExecutor(target, argumentTypes); if (executorToUse != null) { try { return executorToUse.execute(state.getEvaluationContext(), @@ -139,7 +140,7 @@ public class MethodReference extends SpelNodeImpl { // either there was no accessor or it no longer existed executorToUse = findAccessorForMethod(this.name, argumentTypes, state); - this.cachedExecutor = new CachedMethodExecutor(executorToUse, argumentTypes); + this.cachedExecutor = new CachedMethodExecutor(executorToUse, target, argumentTypes); try { return executorToUse.execute(state.getEvaluationContext(), state.getActiveContextObject().getValue(), arguments); @@ -227,15 +228,15 @@ public class MethodReference extends SpelNodeImpl { : contextObject.getClass())); } - private MethodExecutor getCachedExecutor(List argumentTypes) { - if (this.cachedExecutor == null || !this.cachedExecutor.isSuitable(argumentTypes)) { + private MethodExecutor getCachedExecutor(TypeDescriptor target, + List argumentTypes) { + if (this.cachedExecutor == null || !this.cachedExecutor.isSuitable(target, argumentTypes)) { this.cachedExecutor = null; return null; } return this.cachedExecutor.get(); } - private class MethodValueRef implements ValueRef { private final ExpressionState state; @@ -244,15 +245,19 @@ public class MethodReference extends SpelNodeImpl { private final Object target; + private TypeDescriptor targetType; + private final Object[] arguments; private List argumentTypes; - MethodValueRef(ExpressionState state, EvaluationContext evaluationContext, Object object, Object[] arguments) { + MethodValueRef(ExpressionState state, EvaluationContext evaluationContext, + Object object, Object[] arguments) { this.state = state; this.evaluationContext = evaluationContext; this.target = object; + this.targetType = TypeDescriptor.valueOf(target.getClass()); this.arguments = arguments; this.argumentTypes = getTypes(this.arguments); } @@ -260,7 +265,8 @@ public class MethodReference extends SpelNodeImpl { @Override public TypedValue getValue() { - MethodExecutor executorToUse = getCachedExecutor(this.argumentTypes); + MethodExecutor executorToUse = getCachedExecutor(this.targetType, + this.argumentTypes); if (executorToUse != null) { try { return executorToUse.execute(this.evaluationContext, this.target, this.arguments); @@ -285,7 +291,7 @@ public class MethodReference extends SpelNodeImpl { // either there was no accessor or it no longer existed executorToUse = findAccessorForMethod(MethodReference.this.name, argumentTypes, this.target, this.evaluationContext); - MethodReference.this.cachedExecutor = new CachedMethodExecutor(executorToUse, this.argumentTypes); + MethodReference.this.cachedExecutor = new CachedMethodExecutor(executorToUse, this.targetType, this.argumentTypes); try { return executorToUse.execute(this.evaluationContext, this.target, this.arguments); } @@ -310,23 +316,24 @@ public class MethodReference extends SpelNodeImpl { } } - private static class CachedMethodExecutor { private final MethodExecutor methodExecutor; + private final TypeDescriptor target; + private final List argumentTypes; - - public CachedMethodExecutor(MethodExecutor methodExecutor, + public CachedMethodExecutor(MethodExecutor methodExecutor, TypeDescriptor target, List argumentTypes) { this.methodExecutor = methodExecutor; + this.target = target; this.argumentTypes = argumentTypes; } - - public boolean isSuitable(List argumentTypes) { - return (this.methodExecutor != null && this.argumentTypes.equals(argumentTypes)); + public boolean isSuitable(TypeDescriptor target, List argumentTypes) { + return (this.methodExecutor != null && this.target != null + && this.target.equals(target) && this.argumentTypes.equals(argumentTypes)); } public MethodExecutor get() { 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 index 75e76a5903..b23197aa6d 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/CachedMethodExecutorTests.java @@ -45,8 +45,8 @@ public class CachedMethodExecutorTests { @Test - public void testCachedExecution() throws Exception { - Expression expression = this.parser.parseExpression("echo(#something)"); + public void testCachedExecutionForParameters() throws Exception { + Expression expression = this.parser.parseExpression("echo(#var)"); assertMethodExecution(expression, 42, "int: 42"); assertMethodExecution(expression, 42, "int: 42"); @@ -54,18 +54,32 @@ public class CachedMethodExecutorTests { assertMethodExecution(expression, 42, "int: 42"); } + @Test + public void testCachedExecutionForTarget() throws Exception { + Expression expression = this.parser.parseExpression("#var.echo(42)"); + + assertMethodExecution(expression, new RootObject(), "int: 42"); + assertMethodExecution(expression, new RootObject(), "int: 42"); + assertMethodExecution(expression, new BaseObject(), "String: 42"); + assertMethodExecution(expression, new RootObject(), "int: 42"); + } + private void assertMethodExecution(Expression expression, Object var, String expected) { - this.context.setVariable("something", var); + this.context.setVariable("var", var); assertEquals(expected, expression.getValue(this.context)); } - public static class RootObject { + public static class BaseObject { public String echo(String value) { return "String: " + value; } + } + + public static class RootObject extends BaseObject { + public String echo(int value) { return "int: " + value; }