From 85b0bfff585bc3390924bcfa1282616f3e476bf7 Mon Sep 17 00:00:00 2001
From: Phillip Webb <pwebb@gopivotal.com>
Date: Tue, 3 Dec 2013 12:01:24 -0800
Subject: [PATCH] Only consider "is" methods with boolean returns

Fix regression introduced in b25e91a5 where ReflectivePropertyAccessor
does not consider the return type for "is" getters.

Issue: SPR-11142
---
 .../support/ReflectivePropertyAccessor.java   | 42 +++++++++++++------
 .../expression/spel/SpelReproTests.java       | 19 +++++++++
 2 files changed, 49 insertions(+), 12 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 f594808f12..b10e50cafb 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
@@ -22,8 +22,11 @@ import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
 import org.springframework.core.MethodParameter;
@@ -50,6 +53,17 @@ import org.springframework.util.StringUtils;
  */
 public class ReflectivePropertyAccessor implements PropertyAccessor {
 
+	private static final Set<Class<?>> BOOLEAN_TYPES;
+	static {
+		Set<Class<?>> booleanTypes = new HashSet<Class<?>>();
+		booleanTypes.add(Boolean.class);
+		booleanTypes.add(Boolean.TYPE);
+		BOOLEAN_TYPES = Collections.unmodifiableSet(booleanTypes);
+	}
+
+	private static final Set<Class<?>> ANY_TYPES = Collections.emptySet();
+
+
 	private final Map<CacheKey, InvokerPair> readerCache = new ConcurrentHashMap<CacheKey, InvokerPair>(64);
 
 	private final Map<CacheKey, Member> writerCache = new ConcurrentHashMap<CacheKey, Member>(64);
@@ -319,8 +333,13 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	 * Find a getter method for the specified property.
 	 */
 	protected Method findGetterForProperty(String propertyName, Class<?> clazz, boolean mustBeStatic) {
-		return findMethodForProperty(getPropertyMethodSuffixes(propertyName),
-				new String[] { "get", "is" }, clazz, mustBeStatic, 0);
+		Method method = findMethodForProperty(getPropertyMethodSuffixes(propertyName),
+				 "get", clazz, mustBeStatic, 0, ANY_TYPES);
+		if (method == null) {
+			method = findMethodForProperty(getPropertyMethodSuffixes(propertyName),
+					 "is", clazz, mustBeStatic, 0, BOOLEAN_TYPES);
+		}
+		return method;
 	}
 
 	/**
@@ -328,20 +347,19 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	 */
 	protected Method findSetterForProperty(String propertyName, Class<?> clazz, boolean mustBeStatic) {
 		return findMethodForProperty(getPropertyMethodSuffixes(propertyName),
-				new String[] { "set" }, clazz, mustBeStatic, 1);
+				"set", clazz, mustBeStatic, 1, ANY_TYPES);
 	}
 
-	private Method findMethodForProperty(String[] methodSuffixes, String[] prefixes, Class<?> clazz,
-			boolean mustBeStatic, int numberOfParams) {
+	private Method findMethodForProperty(String[] methodSuffixes, String prefix, Class<?> clazz,
+			boolean mustBeStatic, int numberOfParams, Set<Class<?>> requiredReturnTypes) {
 		Method[] methods = getSortedClassMethods(clazz);
 		for (String methodSuffix : methodSuffixes) {
-			for (String prefix : prefixes) {
-				for (Method method : methods) {
-					if (method.getName().equals(prefix + methodSuffix)
-							&& method.getParameterTypes().length == numberOfParams
-							&& (!mustBeStatic || Modifier.isStatic(method.getModifiers()))) {
-						return method;
-					}
+			for (Method method : methods) {
+				if (method.getName().equals(prefix + methodSuffix)
+						&& method.getParameterTypes().length == numberOfParams
+						&& (!mustBeStatic || Modifier.isStatic(method.getModifiers()))
+						&& (requiredReturnTypes.isEmpty() || requiredReturnTypes.contains(method.getReturnType()))) {
+					return method;
 				}
 			}
 		}
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 78ef848cc4..7abc5dd52a 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
@@ -1852,6 +1852,17 @@ public class SpelReproTests extends ExpressionTestCase {
 		assertTrue(classNameExpression.getValue(map,Boolean.class));
 	}
 
+	@Test
+	public void SPR_11142() throws Exception {
+		SpelExpressionParser parser = new SpelExpressionParser();
+		StandardEvaluationContext context = new StandardEvaluationContext();
+		SPR11142 rootObject = new SPR11142();
+		Expression expression = parser.parseExpression("something");
+		thrown.expect(SpelEvaluationException.class);
+		thrown.expectMessage("property 'something' cannot be found");
+		expression.getValue(context, rootObject);
+	}
+
 
 	private static enum ABC {A, B, C}
 
@@ -1956,4 +1967,12 @@ public class SpelReproTests extends ExpressionTestCase {
 		}
 
 	}
+
+	static class SPR11142 {
+
+		public String isSomething() {
+			return "";
+		}
+
+	}
 }