SimplePropertyAccessor with configurable write support

Issue: SPR-16588

spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -45,7 +45,7 @@ import org.springframework.util.ReflectionUtils;
 import org.springframework.util.StringUtils;
 
 /**
- * Simple {@link PropertyAccessor} that uses reflection to access properties
+ * A powerful {@link PropertyAccessor} that uses reflection to access properties
  * for reading and writing.
  *
  * <p>A property can be accessed through a public getter method (when being read)
@@ -55,6 +55,9 @@ import org.springframework.util.StringUtils;
  * @author Juergen Hoeller
  * @author Phillip Webb
  * @since 3.0
+ * @see StandardEvaluationContext
+ * @see SimpleEvaluationContext
+ * @see SimplePropertyAccessor
  */
 public class ReflectivePropertyAccessor implements PropertyAccessor {
 
@@ -63,7 +66,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	private static final Set<Class<?>> BOOLEAN_TYPES;
 
 	static {
-		Set<Class<?>> booleanTypes = new HashSet<>();
+		Set<Class<?>> booleanTypes = new HashSet<>(4);
 		booleanTypes.add(Boolean.class);
 		booleanTypes.add(Boolean.TYPE);
 		BOOLEAN_TYPES = Collections.unmodifiableSet(booleanTypes);
@@ -77,7 +80,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	private final Map<PropertyCacheKey, TypeDescriptor> typeDescriptorCache = new ConcurrentHashMap<>(64);
 
 	@Nullable
-	private InvokerPair lastReadInvokerPair;
+	private volatile InvokerPair lastReadInvokerPair;
 
 
 	/**
@@ -94,14 +97,17 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 		if (target == null) {
 			return false;
 		}
+
 		Class<?> type = (target instanceof Class ? (Class<?>) target : target.getClass());
 		if (type.isArray() && name.equals("length")) {
 			return true;
 		}
+
 		PropertyCacheKey cacheKey = new PropertyCacheKey(type, name, target instanceof Class);
 		if (this.readerCache.containsKey(cacheKey)) {
 			return true;
 		}
+
 		Method method = findGetterForProperty(name, type, target);
 		if (method != null) {
 			// Treat it like a property...
@@ -121,13 +127,8 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 				return true;
 			}
 		}
-		return false;
-	}
 
-	@Nullable
-	public Member getLastReadInvokerPair() {
-		InvokerPair lastReadInvoker = this.lastReadInvokerPair;
-		return (lastReadInvoker != null ? lastReadInvoker.member : null);
+		return false;
 	}
 
 	@Override
@@ -144,20 +145,19 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 
 		PropertyCacheKey cacheKey = new PropertyCacheKey(type, name, target instanceof Class);
 		InvokerPair invoker = this.readerCache.get(cacheKey);
-		lastReadInvokerPair = invoker;
+		this.lastReadInvokerPair = invoker;
 
 		if (invoker == null || invoker.member instanceof Method) {
 			Method method = (Method) (invoker != null ? invoker.member : null);
 			if (method == null) {
 				method = findGetterForProperty(name, type, target);
 				if (method != null) {
-					// TODO remove the duplication here between canRead and read
 					// Treat it like a property...
 					// The readerCache will only contain gettable properties (let's not worry about setters for now).
 					Property property = new Property(type, method, null);
 					TypeDescriptor typeDescriptor = new TypeDescriptor(property);
 					invoker = new InvokerPair(method, typeDescriptor);
-					lastReadInvokerPair = invoker;
+					this.lastReadInvokerPair = invoker;
 					this.readerCache.put(cacheKey, invoker);
 				}
 			}
@@ -179,7 +179,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 				field = findField(name, type, target);
 				if (field != null) {
 					invoker = new InvokerPair(field, new TypeDescriptor(field));
-					lastReadInvokerPair = invoker;
+					this.lastReadInvokerPair = invoker;
 					this.readerCache.put(cacheKey, invoker);
 				}
 			}
@@ -203,11 +203,13 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 		if (target == null) {
 			return false;
 		}
+
 		Class<?> type = (target instanceof Class ? (Class<?>) target : target.getClass());
 		PropertyCacheKey cacheKey = new PropertyCacheKey(type, name, target instanceof Class);
 		if (this.writerCache.containsKey(cacheKey)) {
 			return true;
 		}
+
 		Method method = findSetterForProperty(name, type, target);
 		if (method != null) {
 			// Treat it like a property
@@ -225,6 +227,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 				return true;
 			}
 		}
+
 		return false;
 	}
 
@@ -246,6 +249,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 				throw new AccessException("Type conversion failure", evaluationException);
 			}
 		}
+
 		PropertyCacheKey cacheKey = new PropertyCacheKey(type, name, target instanceof Class);
 		Member cachedMember = this.writerCache.get(cacheKey);
 
@@ -294,6 +298,17 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 		throw new AccessException("Neither setter method nor field found for property '" + name + "'");
 	}
 
+	/**
+	 * @deprecated as of 4.3.15 since it is not used within the framework anymore
+	 */
+	@Deprecated
+	@Nullable
+	public Member getLastReadInvokerPair() {
+		InvokerPair lastReadInvoker = this.lastReadInvokerPair;
+		return (lastReadInvoker != null ? lastReadInvoker.member : null);
+	}
+
+
 	@Nullable
 	private TypeDescriptor getTypeDescriptor(EvaluationContext context, Object target, String name) {
 		Class<?> type = (target instanceof Class ? (Class<?>) target : target.getClass());
@@ -304,7 +319,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 		PropertyCacheKey cacheKey = new PropertyCacheKey(type, name, target instanceof Class);
 		TypeDescriptor typeDescriptor = this.typeDescriptorCache.get(cacheKey);
 		if (typeDescriptor == null) {
-			// attempt to populate the cache entry
+			// Attempt to populate the cache entry
 			try {
 				if (canRead(context, target, name)) {
 					typeDescriptor = this.typeDescriptorCache.get(cacheKey);
@@ -314,7 +329,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 				}
 			}
 			catch (AccessException ex) {
-				// continue with null type descriptor
+				// Continue with null type descriptor
 			}
 		}
 		return typeDescriptor;
@@ -338,15 +353,6 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 		return method;
 	}
 
-	@Nullable
-	private Field findField(String name, Class<?> clazz, Object target) {
-		Field field = findField(name, clazz, target instanceof Class);
-		if (field == null && target instanceof Class) {
-			field = findField(name, target.getClass(), false);
-		}
-		return field;
-	}
-
 	/**
 	 * Find a getter method for the specified property.
 	 */
@@ -377,7 +383,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 		Method[] methods = getSortedClassMethods(clazz);
 		for (String methodSuffix : methodSuffixes) {
 			for (Method method : methods) {
-				if (method.getName().equals(prefix + methodSuffix) &&
+				if (isCandidateForProperty(method) && method.getName().equals(prefix + methodSuffix) &&
 						method.getParameterCount() == numberOfParams &&
 						(!mustBeStatic || Modifier.isStatic(method.getModifiers())) &&
 						(requiredReturnTypes.isEmpty() || requiredReturnTypes.contains(method.getReturnType()))) {
@@ -386,15 +392,25 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 			}
 		}
 		return null;
-
 	}
 
 	/**
-	 * Returns class methods ordered with non bridge methods appearing higher.
+	 * Determine whether the given {@code Method} is a candidate for property access.
+	 * <p>The default implementation considers any method as a candidate, even for
+	 * non-user-declared properties on the {@link Object} base class.
+	 * @param method the Method to evaluate
+	 * @since 4.3.15
+	 */
+	protected boolean isCandidateForProperty(Method method) {
+		return true;
+	}
+
+	/**
+	 * Return class methods ordered with non bridge methods appearing higher.
 	 */
 	private Method[] getSortedClassMethods(Class<?> clazz) {
 		Method[] methods = clazz.getMethods();
-		Arrays.sort(methods, (o1, o2) -> (o1.isBridge() == o2.isBridge()) ? 0 : (o1.isBridge() ? 1 : -1));
+		Arrays.sort(methods, (o1, o2) -> (o1.isBridge() == o2.isBridge() ? 0 : (o1.isBridge() ? 1 : -1)));
 		return methods;
 	}
 
@@ -407,9 +423,9 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	protected String[] getPropertyMethodSuffixes(String propertyName) {
 		String suffix = getPropertyMethodSuffix(propertyName);
 		if (suffix.length() > 0 && Character.isUpperCase(suffix.charAt(0))) {
-			return new String[] { suffix };
+			return new String[] {suffix};
 		}
-		return new String[] { suffix, StringUtils.capitalize(suffix) };
+		return new String[] {suffix, StringUtils.capitalize(suffix)};
 	}
 
 	/**
@@ -423,6 +439,15 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 		return StringUtils.capitalize(propertyName);
 	}
 
+	@Nullable
+	private Field findField(String name, Class<?> clazz, Object target) {
+		Field field = findField(name, clazz, target instanceof Class);
+		if (field == null && target instanceof Class) {
+			field = findField(name, target.getClass(), false);
+		}
+		return field;
+	}
+
 	/**
 	 * Find a field of a certain name on a specified class.
 	 */
@@ -458,7 +483,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	 * This method will just return the ReflectivePropertyAccessor instance if it is unable to build
 	 * something more optimal.
 	 */
-	public PropertyAccessor createOptimalAccessor(EvaluationContext evalContext, @Nullable Object target, String name) {
+	public PropertyAccessor createOptimalAccessor(EvaluationContext context, @Nullable Object target, String name) {
 		// Don't be clever for arrays or null target
 		if (target == null) {
 			return this;

spring-expression/src/main/java/org/springframework/expression/spel/support/SimplePropertyAccessor.java

@@ -0,0 +1,86 @@
+/*
+ * Copyright 2002-2018 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.expression.spel.support;
+
+import java.lang.reflect.Method;
+
+import org.springframework.expression.AccessException;
+import org.springframework.expression.EvaluationContext;
+import org.springframework.lang.Nullable;
+
+/**
+ * A simple {@link org.springframework.expression.PropertyAccessor} variant that
+ * uses reflection to access properties for reading and possibly also writing.
+ *
+ * <p>A property can be accessed through a public getter method (when being read)
+ * or a public setter method (when being written), and also as a public field.
+ *
+ * <p>This accessor is explicitly designed for user-level property evaluation
+ * and does not resolve technical properties on {@code java.lang.Object}.
+ * For more resolution power, choose {@link ReflectivePropertyAccessor} instead.
+ *
+ * @author Juergen Hoeller
+ * @since 4.3.15
+ * @see SimpleEvaluationContext
+ * @see StandardEvaluationContext
+ * @see ReflectivePropertyAccessor
+ */
+public class SimplePropertyAccessor extends ReflectivePropertyAccessor {
+
+	private final boolean allowWrite;
+
+
+	/**
+	 * Create a new property accessor for reading as well writing.
+	 * @see #SimplePropertyAccessor(boolean)
+	 */
+	public SimplePropertyAccessor() {
+		this.allowWrite = true;
+	}
+
+	/**
+	 * Create a new property accessor for reading and possibly also writing.
+	 * @param allowWrite whether to also allow for write operations
+	 * @see #canWrite
+	 */
+	public SimplePropertyAccessor(boolean allowWrite) {
+		this.allowWrite = allowWrite;
+	}
+
+
+	@Override
+	public boolean canWrite(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
+		return (this.allowWrite && super.canWrite(context, target, name));
+	}
+
+	@Override
+	public void write(EvaluationContext context, @Nullable Object target, String name, @Nullable Object newValue)
+			throws AccessException {
+
+		if (!this.allowWrite) {
+			throw new AccessException("PropertyAccessor for property '" + name +
+					"' on target [" + target + "] does not allow write operations");
+		}
+		super.write(context, target, name, newValue);
+	}
+
+	@Override
+	protected boolean isCandidateForProperty(Method method) {
+		return (method.getDeclaringClass() != Object.class);
+	}
+
+}

spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -32,7 +32,9 @@ import org.springframework.expression.PropertyAccessor;
 import org.springframework.expression.TypedValue;
 import org.springframework.expression.spel.standard.SpelExpression;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
+import org.springframework.expression.spel.support.SimplePropertyAccessor;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
+import org.springframework.expression.spel.testresources.Person;
 
 import static org.junit.Assert.*;
 
@@ -73,7 +75,7 @@ public class PropertyAccessTests extends AbstractExpressionTests {
 	 * supplied resolver might be able to - so null shouldn't crash the reflection resolver.
 	 */
 	@Test
-	public void testAccessingOnNullObject() throws Exception {
+	public void testAccessingOnNullObject() {
 		SpelExpression expr = (SpelExpression)parser.parseExpression("madeup");
 		EvaluationContext context = new StandardEvaluationContext(null);
 		try {
@@ -105,7 +107,7 @@ public class PropertyAccessTests extends AbstractExpressionTests {
 
 	@Test
 	// Adding a new property accessor just for a particular type
-	public void testAddingSpecificPropertyAccessor() throws Exception {
+	public void testAddingSpecificPropertyAccessor() {
 		SpelExpressionParser parser = new SpelExpressionParser();
 		StandardEvaluationContext ctx = new StandardEvaluationContext();
 
@@ -162,7 +164,7 @@ public class PropertyAccessTests extends AbstractExpressionTests {
 	}
 
 	@Test
-	public void testAccessingPropertyOfClass() throws Exception {
+	public void testAccessingPropertyOfClass() {
 		Expression expression = parser.parseExpression("name");
 		Object value = expression.getValue(new StandardEvaluationContext(String.class));
 		assertEquals("java.lang.String", value);
@@ -182,6 +184,49 @@ public class PropertyAccessTests extends AbstractExpressionTests {
 		assertEquals("Jens", expression.getValue(context));
 	}
 
+	@Test
+	public void standardGetClassAccess() {
+		Expression expr = parser.parseExpression("'a'.class.getName()");
+		assertEquals(String.class.getName(), expr.getValue());
+	}
+
+	@Test(expected = SpelEvaluationException.class)
+	public void noGetClassAccess() {
+		Expression expr = parser.parseExpression("'a'.class.getName()");
+		StandardEvaluationContext context = new StandardEvaluationContext();
+		context.setPropertyAccessors(Collections.singletonList(new SimplePropertyAccessor()));
+		expr.getValue(context);
+	}
+
+	@Test
+	public void propertyReadWrite() {
+		StandardEvaluationContext context = new StandardEvaluationContext();
+		context.setPropertyAccessors(Collections.singletonList(new SimplePropertyAccessor()));
+
+		Expression expr = parser.parseExpression("name");
+		Person target = new Person("p1");
+		assertEquals("p1", expr.getValue(context, target));
+		target.setName("p2");
+		assertEquals("p2", expr.getValue(context, target));
+
+		parser.parseExpression("name='p3'").getValue(context, target);
+		assertEquals("p3", expr.getValue(context, target));
+	}
+
+	@Test(expected = SpelEvaluationException.class)
+	public void propertyReadOnly() {
+		StandardEvaluationContext context = new StandardEvaluationContext();
+		context.setPropertyAccessors(Collections.singletonList(new SimplePropertyAccessor(false)));
+
+		Expression expr = parser.parseExpression("name");
+		Person target = new Person("p1");
+		assertEquals("p1", expr.getValue(context, target));
+		target.setName("p2");
+		assertEquals("p2", expr.getValue(context, target));
+
+		parser.parseExpression("name='p3'").getValue(context, target);
+	}
+
 
 	// This can resolve the property 'flibbles' on any String (very useful...)
 	private static class StringyPropertyAccessor implements PropertyAccessor {
@@ -223,7 +268,8 @@ public class PropertyAccessTests extends AbstractExpressionTests {
 				throw new RuntimeException("Assertion Failed! name should be flibbles");
 			}
 			try {
-				flibbles = (Integer) context.getTypeConverter().convertValue(newValue, TypeDescriptor.forObject(newValue), TypeDescriptor.valueOf(Integer.class));
+				flibbles = (Integer) context.getTypeConverter().convertValue(newValue,
+						TypeDescriptor.forObject(newValue), TypeDescriptor.valueOf(Integer.class));
 			}
 			catch (EvaluationException ex) {
 				throw new AccessException("Cannot set flibbles to an object of type '" + newValue.getClass() + "'");