Add targetIsClass to SpEL property cache key

Phillip Webb · Phillip Webb · commit 5854d519a11f · 2013-10-30T13:02:07.000-07:00
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
(cherry picked from commit 6d882b14)
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
@@ -29,6 +29,7 @@
 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;
@@ -71,7 +72,7 @@ public boolean canRead(EvaluationContext context, Object target, String name) th
 		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;
 		}
@@ -110,7 +111,7 @@ public TypedValue read(EvaluationContext context, Object target, String name) th
 			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) {
@@ -168,7 +169,7 @@ public boolean canWrite(EvaluationContext context, Object target, String name) t
 			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;
 		}
@@ -209,7 +210,7 @@ public void write(EvaluationContext context, Object target, String name, Object
 				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) {
@@ -266,7 +267,7 @@ private TypeDescriptor getTypeDescriptor(EvaluationContext context, Object targe
 		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
@@ -417,7 +418,7 @@ public PropertyAccessor createOptimalAccessor(EvaluationContext eContext, Object
 			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) {
@@ -476,9 +477,12 @@ private static class CacheKey {
 
 		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
@@ -490,13 +494,23 @@ public boolean equals(Object other) {
 				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
@@ -1818,6 +1818,19 @@ public TypedValue execute(EvaluationContext context, Object target, Object... ar
 		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}
 
@@ -1885,4 +1898,20 @@ public static class StaticFinalImpl1 extends AbstractStaticFinal implements Stat
 	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;
+		}
+
+	}
 }
