DataBindingPropertyAccessor with factory methods (forReadOnlyAccess etc)

Includes configurable write support at ReflectivePropertyAccessor level.

Issue: SPR-16588

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

@@ -18,13 +18,9 @@ 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.
+ * A {@link org.springframework.expression.PropertyAccessor} variant for data binding
+ * purposes, using reflection to access properties for reading and possibly 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.
@@ -35,52 +31,41 @@ import org.springframework.lang.Nullable;
  *
  * @author Juergen Hoeller
  * @since 4.3.15
+ * @see #forReadOnlyAccess()
+ * @see #forReadWriteAccess()
  * @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;
-	}
+public class DataBindingPropertyAccessor extends ReflectivePropertyAccessor {
 
 	/**
 	 * 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;
+	private DataBindingPropertyAccessor(boolean allowWrite) {
+		super(allowWrite);
 	}
 
-
 	@Override
-	public boolean canWrite(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
-		return (this.allowWrite && super.canWrite(context, target, name));
+	protected boolean isCandidateForProperty(Method method) {
+		return (method.getDeclaringClass() != Object.class);
 	}
 
-	@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);
+	/**
+	 * Create a new data-binding property accessor for read-only access.
+	 */
+	public static DataBindingPropertyAccessor forReadOnlyAccess() {
+		return new DataBindingPropertyAccessor(false);
 	}
 
-	@Override
-	protected boolean isCandidateForProperty(Method method) {
-		return (method.getDeclaringClass() != Object.class);
+	/**
+	 * Create a new data-binding property accessor for read-write access.
+	 */
+	public static DataBindingPropertyAccessor forReadWriteAccess() {
+		return new DataBindingPropertyAccessor(true);
 	}
 
 }

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

@@ -46,7 +46,7 @@ import org.springframework.util.StringUtils;
 
 /**
  * A powerful {@link PropertyAccessor} that uses reflection to access properties
- * for reading and writing.
+ * for reading and possibly also for 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.
@@ -57,7 +57,7 @@ import org.springframework.util.StringUtils;
  * @since 3.0
  * @see StandardEvaluationContext
  * @see SimpleEvaluationContext
- * @see SimplePropertyAccessor
+ * @see DataBindingPropertyAccessor
  */
 public class ReflectivePropertyAccessor implements PropertyAccessor {
 
@@ -73,6 +73,8 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	}
 
 
+	private final boolean allowWrite;
+
 	private final Map<PropertyCacheKey, InvokerPair> readerCache = new ConcurrentHashMap<>(64);
 
 	private final Map<PropertyCacheKey, Member> writerCache = new ConcurrentHashMap<>(64);
@@ -83,6 +85,25 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	private volatile InvokerPair lastReadInvokerPair;
 
 
+	/**
+	 * Create a new property accessor for reading as well writing.
+	 * @see #ReflectivePropertyAccessor(boolean)
+	 */
+	public ReflectivePropertyAccessor() {
+		this.allowWrite = true;
+	}
+
+	/**
+	 * Create a new property accessor for reading and possibly writing.
+	 * @param allowWrite whether to also allow for write operations
+	 * @since 4.3.15
+	 * @see #canWrite
+	 */
+	public ReflectivePropertyAccessor(boolean allowWrite) {
+		this.allowWrite = allowWrite;
+	}
+
+
 	/**
 	 * Returns {@code null} which means this is a general purpose accessor.
 	 */
@@ -200,7 +221,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 
 	@Override
 	public boolean canWrite(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
-		if (target == null) {
+		if (!this.allowWrite || target == null) {
 			return false;
 		}
 
@@ -235,6 +256,11 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	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");
+		}
+
 		Assert.state(target != null, "Target must not be null");
 		Class<?> type = (target instanceof Class ? (Class<?>) target : target.getClass());
 
@@ -477,14 +503,18 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 	}
 
 	/**
-	 * Attempt to create an optimized property accessor tailored for a property of a particular name on
-	 * a particular class. The general ReflectivePropertyAccessor will always work but is not optimal
-	 * due to the need to lookup which reflective member (method/field) to use each time read() is called.
-	 * This method will just return the ReflectivePropertyAccessor instance if it is unable to build
-	 * something more optimal.
+	 * Attempt to create an optimized property accessor tailored for a property of a
+	 * particular name on a particular class. The general ReflectivePropertyAccessor
+	 * will always work but is not optimal due to the need to lookup which reflective
+	 * member (method/field) to use each time read() is called. This method will just
+	 * return the ReflectivePropertyAccessor instance if it is unable to build a more
+	 * optimal accessor.
+	 * <p>Note: An optimal accessor is currently only usable for read attempts.
+	 * Do not call this method if you need a read-write accessor.
+	 * @see OptimalPropertyAccessor
 	 */
 	public PropertyAccessor createOptimalAccessor(EvaluationContext context, @Nullable Object target, String name) {
-		// Don't be clever for arrays or null target
+		// Don't be clever for arrays or a null target...
 		if (target == null) {
 			return this;
 		}
@@ -506,7 +536,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
 					this.readerCache.put(cacheKey, invocationTarget);
 				}
 			}
-			if (method != null) {
+			if (method != null && isCandidateForProperty(method)) {
 				return new OptimalPropertyAccessor(invocationTarget);
 			}
 		}

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

@@ -32,7 +32,7 @@ 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.DataBindingPropertyAccessor;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
 import org.springframework.expression.spel.testresources.Person;
 
@@ -194,14 +194,14 @@ public class PropertyAccessTests extends AbstractExpressionTests {
 	public void noGetClassAccess() {
 		Expression expr = parser.parseExpression("'a'.class.getName()");
 		StandardEvaluationContext context = new StandardEvaluationContext();
-		context.setPropertyAccessors(Collections.singletonList(new SimplePropertyAccessor()));
+		context.setPropertyAccessors(Collections.singletonList(DataBindingPropertyAccessor.forReadWriteAccess()));
 		expr.getValue(context);
 	}
 
 	@Test
 	public void propertyReadWrite() {
 		StandardEvaluationContext context = new StandardEvaluationContext();
-		context.setPropertyAccessors(Collections.singletonList(new SimplePropertyAccessor()));
+		context.setPropertyAccessors(Collections.singletonList(DataBindingPropertyAccessor.forReadWriteAccess()));
 
 		Expression expr = parser.parseExpression("name");
 		Person target = new Person("p1");
@@ -216,7 +216,7 @@ public class PropertyAccessTests extends AbstractExpressionTests {
 	@Test(expected = SpelEvaluationException.class)
 	public void propertyReadOnly() {
 		StandardEvaluationContext context = new StandardEvaluationContext();
-		context.setPropertyAccessors(Collections.singletonList(new SimplePropertyAccessor(false)));
+		context.setPropertyAccessors(Collections.singletonList(DataBindingPropertyAccessor.forReadOnlyAccess()));
 
 		Expression expr = parser.parseExpression("name");
 		Person target = new Person("p1");