From 6d882b149d0b32bab116a49ac5486abeb819f4f1 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 21 Oct 2013 10:54:49 -0700 Subject: [PATCH] Add targetIsClass to SpEL property cache key Update the `CacheKey` class used by `ReflectivePropertyAccessor` to include if the target object is class. The prevents an incorrect cache hit from being returned when a property with the same name is read on both an object and its class. For example: #{class.name} #{name} Issue: SPR-10486 --- .../support/ReflectivePropertyAccessor.java | 30 ++++++++++++++----- .../expression/spel/SpelReproTests.java | 29 ++++++++++++++++++ 2 files changed, 51 insertions(+), 8 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java index d78f96085f..7443896b58 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java @@ -29,6 +29,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.springframework.core.MethodParameter; import org.springframework.core.convert.Property; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.style.ToStringCreator; import org.springframework.expression.AccessException; import org.springframework.expression.EvaluationContext; import org.springframework.expression.EvaluationException; @@ -73,7 +74,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { if (type.isArray() && name.equals("length")) { return true; } - CacheKey cacheKey = new CacheKey(type, name); + CacheKey cacheKey = new CacheKey(type, name, target instanceof Class); if (this.readerCache.containsKey(cacheKey)) { return true; } @@ -113,7 +114,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { return new TypedValue(Array.getLength(target)); } - CacheKey cacheKey = new CacheKey(type, name); + CacheKey cacheKey = new CacheKey(type, name, target instanceof Class); InvokerPair invoker = this.readerCache.get(cacheKey); if (invoker == null || invoker.member instanceof Method) { @@ -172,7 +173,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { return false; } Class type = (target instanceof Class ? (Class) target : target.getClass()); - CacheKey cacheKey = new CacheKey(type, name); + CacheKey cacheKey = new CacheKey(type, name, target instanceof Class); if (this.writerCache.containsKey(cacheKey)) { return true; } @@ -214,7 +215,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { throw new AccessException("Type conversion failure",evaluationException); } } - CacheKey cacheKey = new CacheKey(type, name); + CacheKey cacheKey = new CacheKey(type, name, target instanceof Class); Member cachedMember = this.writerCache.get(cacheKey); if (cachedMember == null || cachedMember instanceof Method) { @@ -271,7 +272,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { if (type.isArray() && name.equals("length")) { return TypeDescriptor.valueOf(Integer.TYPE); } - CacheKey cacheKey = new CacheKey(type, name); + CacheKey cacheKey = new CacheKey(type, name, target instanceof Class); TypeDescriptor typeDescriptor = this.typeDescriptorCache.get(cacheKey); if (typeDescriptor == null) { // attempt to populate the cache entry @@ -431,7 +432,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { return this; } - CacheKey cacheKey = new CacheKey(type, name); + CacheKey cacheKey = new CacheKey(type, name, target instanceof Class); InvokerPair invocationTarget = this.readerCache.get(cacheKey); if (invocationTarget == null || invocationTarget.member instanceof Method) { @@ -490,9 +491,12 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { private final String name; - public CacheKey(Class clazz, String name) { + private boolean targetIsClass; + + public CacheKey(Class clazz, String name, boolean targetIsClass) { this.clazz = clazz; this.name = name; + this.targetIsClass = targetIsClass; } @Override @@ -504,13 +508,23 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { return false; } CacheKey otherKey = (CacheKey) other; - return (this.clazz.equals(otherKey.clazz) && this.name.equals(otherKey.name)); + boolean rtn = true; + rtn &= this.clazz.equals(otherKey.clazz); + rtn &= this.name.equals(otherKey.name); + rtn &= this.targetIsClass == otherKey.targetIsClass; + return rtn; } @Override public int hashCode() { return this.clazz.hashCode() * 29 + this.name.hashCode(); } + + @Override + public String toString() { + return new ToStringCreator(this).append("clazz", this.clazz).append("name", + this.name).append("targetIsClass", this.targetIsClass).toString(); + } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java index 1fa9c6e3a3..391cbfe4f8 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java @@ -1820,6 +1820,19 @@ public class SpelReproTests extends ExpressionTestCase { assertEquals(XYZ.Z, Array.get(result, 2)); } + @Test + public void SPR_10486() throws Exception { + SpelExpressionParser parser = new SpelExpressionParser(); + StandardEvaluationContext context = new StandardEvaluationContext(); + SPR10486 rootObject = new SPR10486(); + Expression classNameExpression = parser.parseExpression("class.name"); + Expression nameExpression = parser.parseExpression("name"); + assertThat(classNameExpression.getValue(context, rootObject), + equalTo((Object) SPR10486.class.getName())); + assertThat(nameExpression.getValue(context, rootObject), + equalTo((Object) "name")); + } + private static enum ABC {A, B, C} @@ -1887,4 +1900,20 @@ public class SpelReproTests extends ExpressionTestCase { public static class StaticFinalImpl2 extends AbstractStaticFinal { } + /** + * The Class TestObject. + */ + public static class SPR10486 { + + private String name = "name"; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + } } -- GitLab