Changed so that existing getters/setters are used before direct field access is attempted

git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@99 50f2f4bb-b051-0410-bef5-90022cba6387

org.springframework.expression/src/main/java/org/springframework/expression/spel/reflection/ReflectionPropertyReaderExecutor.java

@@ -12,35 +12,24 @@ public class ReflectionPropertyReaderExecutor implements PropertyReaderExecutor
 
 	private Method methodToAccessProperty;
 	private Field fieldToAccessProperty;
-	private String propertyName;
+	private final String propertyName;
 
 	public ReflectionPropertyReaderExecutor(String propertyName, Method method) {
 		this.propertyName = propertyName;
-		this.methodToAccessProperty = method;
+		methodToAccessProperty = method;
 	}
 
 	public ReflectionPropertyReaderExecutor(String propertyName, Field field) {
 		this.propertyName = propertyName;
-		this.fieldToAccessProperty = field;
+		fieldToAccessProperty = field;
 	}
 
 	public Object execute(EvaluationContext context, Object target) throws AccessException {
-		if (fieldToAccessProperty != null) {
-			try {
-				if (!fieldToAccessProperty.isAccessible()) {
-					fieldToAccessProperty.setAccessible(true);
-				}
-				return fieldToAccessProperty.get(target);
-			} catch (IllegalArgumentException e) {
-				throw new AccessException("Unable to access field: " + propertyName, e);
-			} catch (IllegalAccessException e) {
-				throw new AccessException("Unable to access field: " + propertyName, e);
-			}
-		}
 		if (methodToAccessProperty != null) {
 			try {
-				if (!methodToAccessProperty.isAccessible())
+				if (!methodToAccessProperty.isAccessible()) {
 					methodToAccessProperty.setAccessible(true);
+				}
 				return methodToAccessProperty.invoke(target);
 			} catch (IllegalArgumentException e) {
 				throw new AccessException("Unable to access property '" + propertyName + "' through getter", e);
@@ -50,6 +39,18 @@ public class ReflectionPropertyReaderExecutor implements PropertyReaderExecutor
 				throw new AccessException("Unable to access property '" + propertyName + "' through getter", e);
 			}
 		}
+		if (fieldToAccessProperty != null) {
+			try {
+				if (!fieldToAccessProperty.isAccessible()) {
+					fieldToAccessProperty.setAccessible(true);
+				}
+				return fieldToAccessProperty.get(target);
+			} catch (IllegalArgumentException e) {
+				throw new AccessException("Unable to access field: " + propertyName, e);
+			} catch (IllegalAccessException e) {
+				throw new AccessException("Unable to access field: " + propertyName, e);
+			}
+		}
 		throw new AccessException("No method or field accessor found for property '" + propertyName + "'");
 	}
 

org.springframework.expression/src/main/java/org/springframework/expression/spel/reflection/ReflectionPropertyResolver.java

@@ -63,14 +63,14 @@ public class ReflectionPropertyResolver extends CacheablePropertyAccessor {
 		if (relevantClass.isArray() && propertyName.equals("length")) {
 			return new ReflectionPropertyReaderExecutorForArrayLength();
 		}
-		Field field = ReflectionUtils.findField(propertyName, relevantClass);
+		Method m = ReflectionUtils.findGetterForProperty(propertyName, relevantClass, target instanceof Class);
-		if (field != null) {
-			return new ReflectionPropertyReaderExecutor(propertyName, field);
-		}
-		Method m = ReflectionUtils.findGetterForProperty(propertyName, relevantClass);
 		if (m != null) {
 			return new ReflectionPropertyReaderExecutor(propertyName, m);
 		}
+		Field field = ReflectionUtils.findField(propertyName, relevantClass, target instanceof Class);
+		if (field != null) {
+			return new ReflectionPropertyReaderExecutor(propertyName, field);
+		}
 		return null;
 	}
 
@@ -93,14 +93,14 @@ public class ReflectionPropertyResolver extends CacheablePropertyAccessor {
 			// A property not found exception will occur if the reflection finder was supposed to find it
 			return null;
 		}
-		Field field = ReflectionUtils.findField((String) name, relevantClass);
+		Method m = ReflectionUtils.findSetterForProperty((String) name, relevantClass, target instanceof Class);
-		if (field != null) {
-			return new ReflectionPropertyWriterExecutor((String) name, field);
-		}
-		Method m = ReflectionUtils.findSetterForProperty((String) name, relevantClass);
 		if (m != null) {
 			return new ReflectionPropertyWriterExecutor((String) name, m);
 		}
+		Field field = ReflectionUtils.findField((String) name, relevantClass, target instanceof Class);
+		if (field != null) {
+			return new ReflectionPropertyWriterExecutor((String) name, field);
+		}
 		return null;
 	}
 

org.springframework.expression/src/main/java/org/springframework/expression/spel/reflection/ReflectionPropertyWriterExecutor.java

@@ -12,33 +12,20 @@ public class ReflectionPropertyWriterExecutor implements PropertyWriterExecutor
 
 	private Method methodToAccessProperty;
 	private Field fieldToAccessProperty;
-	private String propertyName;
+	private final String propertyName;
 
 	public ReflectionPropertyWriterExecutor(String propertyName, Method method) {
 		this.propertyName = propertyName;
-		this.methodToAccessProperty = method;
+		methodToAccessProperty = method;
 	}
 
 	public ReflectionPropertyWriterExecutor(String propertyName, Field field) {
 		this.propertyName = propertyName;
-		this.fieldToAccessProperty = field;
+		fieldToAccessProperty = field;
 	}
 
 	// public Object execute(EvaluationContext context, Object target) throws AccessException {
 	public void execute(EvaluationContext evaluationContext, Object target, Object newValue) throws AccessException {
-		if (fieldToAccessProperty != null) {
-			try {
-				if (!fieldToAccessProperty.isAccessible()) {
-					fieldToAccessProperty.setAccessible(true);
-				}
-				fieldToAccessProperty.set(target, newValue);
-				return;
-			} catch (IllegalArgumentException e) {
-				throw new AccessException("Unable to access field: " + propertyName, e);
-			} catch (IllegalAccessException e) {
-				throw new AccessException("Unable to access field: " + propertyName, e);
-			}
-		}
 		if (methodToAccessProperty != null) {
 			try {
 				if (!methodToAccessProperty.isAccessible())
@@ -53,6 +40,19 @@ public class ReflectionPropertyWriterExecutor implements PropertyWriterExecutor
 				throw new AccessException("Unable to access property '" + propertyName + "' through setter", e);
 			}
 		}
+		if (fieldToAccessProperty != null) {
+			try {
+				if (!fieldToAccessProperty.isAccessible()) {
+					fieldToAccessProperty.setAccessible(true);
+				}
+				fieldToAccessProperty.set(target, newValue);
+				return;
+			} catch (IllegalArgumentException e) {
+				throw new AccessException("Unable to access field: " + propertyName, e);
+			} catch (IllegalAccessException e) {
+				throw new AccessException("Unable to access field: " + propertyName, e);
+			}
+		}
 		throw new AccessException("No method or field accessor found for property '" + propertyName + "'");
 	}
 }

org.springframework.expression/src/main/java/org/springframework/expression/spel/reflection/ReflectionUtils.java

@@ -19,6 +19,7 @@ import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -347,11 +348,11 @@ public class ReflectionUtils {
 	/**
 	 * Find a field of a certain name on a specified class
 	 */
-	public final static Field findField(String name, Class<?> clazz) {
+	public final static Field findField(String name, Class<?> clazz, boolean mustBeStatic) {
-		Field[] fields = clazz.getFields(); // TODO what about inherited fields? try getFields() too?
+		Field[] fields = clazz.getFields(); // TODO use getDeclaredFields() and search up hierarchy?
 		for (int i = 0; i < fields.length; i++) {
 			Field field = fields[i];
-			if (field.getName().equals(name)) {
+			if (field.getName().equals(name) && (mustBeStatic ? Modifier.isStatic(field.getModifiers()) : true)) {
 				return field;
 			}
 		}
@@ -362,14 +363,16 @@ public class ReflectionUtils {
 	 * Find a getter method for the specified property. A getter is defined as a method whose name start with the prefix
 	 * 'get' and the rest of the name is the same as the property name (with the first character uppercased).
 	 */
-	public static Method findGetterForProperty(String propertyName, Class<?> clazz) {
+	public static Method findGetterForProperty(String propertyName, Class<?> clazz, boolean mustBeStatic) {
-		Method[] ms = clazz.getMethods();
+		Method[] ms = clazz.getMethods();// TODO use getDeclaredMethods() and search up hierarchy?
 		StringBuilder sb = new StringBuilder();
 		sb.append("get").append(propertyName.substring(0, 1).toUpperCase()).append(propertyName.substring(1));
 		String expectedGetterName = sb.toString();
 		for (int i = 0; i < ms.length; i++) {
 			Method method = ms[i];
-			if (method.getParameterTypes().length == 0 && method.getName().equals(expectedGetterName)) {
+			if (method.getParameterTypes().length == 0
+					&& (mustBeStatic ? Modifier.isStatic(method.getModifiers()) : true)
+					&& method.getName().equals(expectedGetterName)) {
 				return method;
 			}
 		}
@@ -379,14 +382,16 @@ public class ReflectionUtils {
 	/**
 	 * Find a setter method for the specified property
 	 */
-	public static Method findSetterForProperty(String propertyName, Class<?> clazz) {
+	public static Method findSetterForProperty(String propertyName, Class<?> clazz, boolean mustBeStatic) {
-		Method[] ms = clazz.getMethods();
+		Method[] ms = clazz.getMethods(); // TODO use getDeclaredMethods() and search up hierarchy?
 		StringBuilder sb = new StringBuilder();
 		sb.append("set").append(propertyName.substring(0, 1).toUpperCase()).append(propertyName.substring(1));
 		String setterName = sb.toString();
 		for (int i = 0; i < ms.length; i++) {
 			Method method = ms[i];
-			if (method.getParameterTypes().length == 1 && method.getName().equals(setterName)) {
+			if (method.getParameterTypes().length == 1
+					&& (mustBeStatic ? Modifier.isStatic(method.getModifiers()) : true)
+					&& method.getName().equals(setterName)) {
 				return method;
 			}
 		}