SPR-6968: indexing via square brackets can now treat the index as an attempt at property access

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/AstUtils.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright 2010 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.ast;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.springframework.expression.PropertyAccessor;
+import org.springframework.expression.spel.ExpressionState;
+
+/**
+ * Utilities methods for use in the Ast classes.
+ *
+ * @author Andy Clement
+ * @since 3.0.2
+ */
+public class AstUtils {
+	
+	/**
+	 * Determines the set of property resolvers that should be used to try and access a property on the specified target
+	 * type. The resolvers are considered to be in an ordered list, however in the returned list any that are exact
+	 * matches for the input target type (as opposed to 'general' resolvers that could work for any type) are placed at
+	 * the start of the list. In addition, there are specific resolvers that exactly name the class in question and
+	 * resolvers that name a specific class but it is a supertype of the class we have. These are put at the end of the
+	 * specific resolvers set and will be tried after exactly matching accessors but before generic accessors.
+	 * 
+	 * @param targetType the type upon which property access is being attempted
+	 * @return a list of resolvers that should be tried in order to access the property
+	 */
+	public static List<PropertyAccessor> getPropertyAccessorsToTry(Class<?> targetType, ExpressionState state) {
+		List<PropertyAccessor> specificAccessors = new ArrayList<PropertyAccessor>();
+		List<PropertyAccessor> generalAccessors = new ArrayList<PropertyAccessor>();
+		for (PropertyAccessor resolver : state.getPropertyAccessors()) {
+			Class<?>[] targets = resolver.getSpecificTargetClasses();
+			if (targets == null) { // generic resolver that says it can be used for any type
+				generalAccessors.add(resolver);
+			}
+			else {
+				if (targetType != null) {
+					int pos = 0;
+					for (Class<?> clazz : targets) {
+						if (clazz == targetType) { // put exact matches on the front to be tried first?
+							specificAccessors.add(pos++, resolver);
+						}
+						else if (clazz.isAssignableFrom(targetType)) { // put supertype matches at the end of the
+							// specificAccessor list
+							generalAccessors.add(resolver);
+						}
+					}
+				}
+			}
+		}
+		List<PropertyAccessor> resolvers = new ArrayList<PropertyAccessor>();
+		resolvers.addAll(specificAccessors);
+		resolvers.addAll(generalAccessors);
+		return resolvers;
+	}
+}

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java

@@ -21,11 +21,15 @@ import java.util.List;
 import java.util.Map;
 
 import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.expression.AccessException;
+import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.EvaluationException;
+import org.springframework.expression.PropertyAccessor;
 import org.springframework.expression.TypedValue;
 import org.springframework.expression.spel.ExpressionState;
 import org.springframework.expression.spel.SpelEvaluationException;
 import org.springframework.expression.spel.SpelMessage;
+import org.springframework.expression.spel.support.ReflectivePropertyAccessor;
 
 /**
  * An Indexer can index into some proceeding structure to access a particular piece of it. Supported structures are:
@@ -38,6 +42,21 @@ import org.springframework.expression.spel.SpelMessage;
 // TODO support correct syntax for multidimensional [][][] and not [,,,]
 public class Indexer extends SpelNodeImpl {
 
+	// These fields are used when the indexer is being used as a property read accessor. If the name and 
+	// target type match these cached values then the cachedReadAccessor is used to read the property.
+	// If they do not match, the correct accessor is discovered and then cached for later use.
+	private String cachedReadName;
+	private Class<?> cachedReadTargetType;
+	private PropertyAccessor cachedReadAccessor;
+
+	// These fields are used when the indexer is being used as a property write accessor.  If the name and 
+	// target type match these cached values then the cachedWriteAccessor is used to write the property.
+	// If they do not match, the correct accessor is discovered and then cached for later use.
+	private String cachedWriteName;
+	private Class<?> cachedWriteTargetType;
+	private PropertyAccessor cachedWriteAccessor;
+	
+
 	public Indexer(int pos, SpelNodeImpl expr) {
 		super(pos, expr);
 	}
@@ -87,61 +106,96 @@ public class Indexer extends SpelNodeImpl {
 				return new TypedValue(o);
 			}
 		}
-
+		
-		int idx = (Integer)state.convertValue(index, TypeDescriptor.valueOf(Integer.class));
-
 		if (targetObject == null) {
 			throw new SpelEvaluationException(getStartPosition(),SpelMessage.CANNOT_INDEX_INTO_NULL_VALUE);
 		}
 		
-		if (targetObject.getClass().isArray()) {
+		// if the object is something that looks indexable by an integer, attempt to treat the index value as a number
-			return new TypedValue(accessArrayElement(targetObject, idx),TypeDescriptor.valueOf(targetObjectTypeDescriptor.getElementType()));
+		if ((targetObject instanceof Collection ) || targetObject.getClass().isArray() || targetObject instanceof String) {
-		} else if (targetObject instanceof Collection) {
+			int idx = (Integer)state.convertValue(index, TypeDescriptor.valueOf(Integer.class));		
-			Collection c = (Collection) targetObject;
+			if (targetObject.getClass().isArray()) {
-			if (idx >= c.size()) {
+				return new TypedValue(accessArrayElement(targetObject, idx),TypeDescriptor.valueOf(targetObjectTypeDescriptor.getElementType()));
-				if (state.getConfiguration().isAutoGrowCollections()) {
+			} else if (targetObject instanceof Collection) {
-					// Grow the collection
+				Collection c = (Collection) targetObject;
-					Object newCollectionElement = null;
+				if (idx >= c.size()) {
-					try {
+					if (state.getConfiguration().isAutoGrowCollections()) {
-						int newElements = idx-c.size();
+						// Grow the collection
-						Class elementClass = targetObjectTypeDescriptor.getElementType();
+						Object newCollectionElement = null;
-						if (elementClass == null) {
+						try {
-							throw new SpelEvaluationException(getStartPosition(), SpelMessage.UNABLE_TO_GROW_COLLECTION_UNKNOWN_ELEMENT_TYPE);	
+							int newElements = idx-c.size();
+							Class elementClass = targetObjectTypeDescriptor.getElementType();
+							if (elementClass == null) {
+								throw new SpelEvaluationException(getStartPosition(), SpelMessage.UNABLE_TO_GROW_COLLECTION_UNKNOWN_ELEMENT_TYPE);	
+							}
+							while (newElements>0) {
+								c.add(elementClass.newInstance());
+								newElements--;
+							}
+							newCollectionElement = targetObjectTypeDescriptor.getElementType().newInstance();
 						}
-						while (newElements>0) {
+						catch (Exception ex) {
-							c.add(elementClass.newInstance());
+							throw new SpelEvaluationException(getStartPosition(), ex, SpelMessage.UNABLE_TO_GROW_COLLECTION);
-							newElements--;
 						}
-						newCollectionElement = targetObjectTypeDescriptor.getElementType().newInstance();
+						c.add(newCollectionElement);
+						return new TypedValue(newCollectionElement,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getElementType()));
 					}
-					catch (Exception ex) {
+					else {
-						throw new SpelEvaluationException(getStartPosition(), ex, SpelMessage.UNABLE_TO_GROW_COLLECTION);
+						throw new SpelEvaluationException(getStartPosition(),SpelMessage.COLLECTION_INDEX_OUT_OF_BOUNDS, c.size(), idx);
 					}
-					c.add(newCollectionElement);
-					return new TypedValue(newCollectionElement,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getElementType()));
 				}
-				else {
+				int pos = 0;
-					throw new SpelEvaluationException(getStartPosition(),SpelMessage.COLLECTION_INDEX_OUT_OF_BOUNDS, c.size(), idx);
+				for (Object o : c) {
+					if (pos == idx) {
+						return new TypedValue(o,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getElementType()));
+					}
+					pos++;
 				}
-			}
+			} else if (targetObject instanceof String) {
-			int pos = 0;
+				String ctxString = (String) targetObject;
-			for (Object o : c) {
+				if (idx >= ctxString.length()) {
-				if (pos == idx) {
+					throw new SpelEvaluationException(getStartPosition(),SpelMessage.STRING_INDEX_OUT_OF_BOUNDS, ctxString.length(), idx);
-					return new TypedValue(o,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getElementType()));
 				}
-				pos++;
+				return new TypedValue(String.valueOf(ctxString.charAt(idx)));
 			}
-		} else if (targetObject instanceof String) {
+		}
-			String ctxString = (String) targetObject;
+		
-			if (idx >= ctxString.length()) {
+		// Try and treat the index value as a property of the context object
-				throw new SpelEvaluationException(getStartPosition(),SpelMessage.STRING_INDEX_OUT_OF_BOUNDS, ctxString.length(), idx);
+		// TODO could call the conversion service to convert the value to a String		
+		if (indexValue.getTypeDescriptor().getType()==String.class) {
+			Class<?> targetObjectRuntimeClass = getObjectClass(targetObject);
+			String name = (String)indexValue.getValue();
+			EvaluationContext eContext = state.getEvaluationContext();
+
+			try {
+				if (cachedReadName!=null && cachedReadName.equals(name) && cachedReadTargetType!=null && cachedReadTargetType.equals(targetObjectRuntimeClass)) {
+					// it is OK to use the cached accessor
+					return cachedReadAccessor.read(eContext, targetObject, name);
+				}
+				
+				List<PropertyAccessor> accessorsToTry = AstUtils.getPropertyAccessorsToTry(targetObjectRuntimeClass, state);
+		
+				if (accessorsToTry != null) {			
+					for (PropertyAccessor accessor : accessorsToTry) {
+							if (accessor.canRead(eContext, targetObject, name)) {
+								if (accessor instanceof ReflectivePropertyAccessor) {
+									accessor = ((ReflectivePropertyAccessor)accessor).createOptimalAccessor(eContext, targetObject, name);
+								}
+								this.cachedReadAccessor = accessor;
+								this.cachedReadName = name;
+								this.cachedReadTargetType = targetObjectRuntimeClass;
+								return accessor.read(eContext, targetObject, name);
+							}
+					}
+				}
+			} catch (AccessException e) {
+				throw new SpelEvaluationException(getStartPosition(), e, SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, targetObjectTypeDescriptor.asString());
 			}
-			return new TypedValue(String.valueOf(ctxString.charAt(idx)));
 		}
+			
 		throw new SpelEvaluationException(getStartPosition(),SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, targetObjectTypeDescriptor.asString());
 	}
-
+	
-
 	@Override
 	public boolean isWritable(ExpressionState expressionState) throws SpelEvaluationException {
 		return true;
@@ -174,6 +228,7 @@ public class Indexer extends SpelNodeImpl {
 		if (targetObjectTypeDescriptor.isArray()) {
 			int idx = (Integer)state.convertValue(index, TypeDescriptor.valueOf(Integer.class));
 			setArrayElement(state, contextObject.getValue(), idx, newValue, targetObjectTypeDescriptor.getElementType());
+			return;
 		}
 		else if (targetObjectTypeDescriptor.isCollection()) {
 			int idx = (Integer)state.convertValue(index, TypeDescriptor.valueOf(Integer.class));
@@ -185,13 +240,46 @@ public class Indexer extends SpelNodeImpl {
 				List list = (List)targetObject;
 				Object possiblyConvertedValue = state.convertValue(newValue,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getElementType()));
 				list.set(idx,possiblyConvertedValue);
+				return;
 			}
 			else {
-				throw new SpelEvaluationException(getStartPosition(),SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, contextObject.getClass().getName());
+				throw new SpelEvaluationException(getStartPosition(),SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, targetObjectTypeDescriptor.asString());
 			}
-		} else {
-			throw new SpelEvaluationException(getStartPosition(),SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, contextObject.getClass().getName());
 		}
+		
+		// Try and treat the index value as a property of the context object		
+		// TODO could call the conversion service to convert the value to a String		
+		if (index.getTypeDescriptor().getType()==String.class) {
+			Class<?> contextObjectClass = getObjectClass(contextObject.getValue());
+			String name = (String)index.getValue();
+			EvaluationContext eContext = state.getEvaluationContext();
+			try {
+				if (cachedWriteName!=null && cachedWriteName.equals(name) && cachedWriteTargetType!=null && cachedWriteTargetType.equals(contextObjectClass)) {
+					// it is OK to use the cached accessor
+					cachedWriteAccessor.write(eContext, targetObject, name,newValue);
+					return;
+				}
+	
+				List<PropertyAccessor> accessorsToTry = AstUtils.getPropertyAccessorsToTry(contextObjectClass, state);
+				if (accessorsToTry != null) {
+						for (PropertyAccessor accessor : accessorsToTry) {
+							if (accessor.canWrite(eContext, contextObject.getValue(), name)) {
+								this.cachedWriteName = name;
+								this.cachedWriteTargetType = contextObjectClass;
+								this.cachedWriteAccessor = accessor;
+								accessor.write(eContext, contextObject.getValue(), name, newValue);
+								return;
+							}
+						}
+				}
+			} catch (AccessException ae) {
+				throw new SpelEvaluationException(getStartPosition(), ae, SpelMessage.EXCEPTION_DURING_PROPERTY_WRITE,
+						name, ae.getMessage());
+			}
+
+		}
+		
+		throw new SpelEvaluationException(getStartPosition(),SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, targetObjectTypeDescriptor.asString());
 	}
 	
 	@Override

org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java

@@ -249,6 +249,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl {
 		return false;
 	}
 
+	// TODO when there is more time, remove this and use the version in AstUtils
 	/**
 	 * Determines the set of property resolvers that should be used to try and access a property on the specified target
 	 * type. The resolvers are considered to be in an ordered list, however in the returned list any that are exact

org.springframework.expression/src/test/java/org/springframework/expression/spel/SpringEL300Tests.java

@@ -291,6 +291,125 @@ public class SpringEL300Tests extends ExpressionTestCase {
 		}
 	}
 	
+
+	@Test
+	public void testNestedProperties_SPR6923() {
+		StandardEvaluationContext eContext = new StandardEvaluationContext(new Foo());
+		String name = null;
+		Expression expr = null;
+		
+		expr = new SpelExpressionParser().parseRaw("resource.resource.server");
+		name = expr.getValue(eContext,String.class);
+		Assert.assertEquals("abc",name);
+	}
+	
+	static class Foo {
+		public ResourceSummary resource = new ResourceSummary();
+	}
+	
+	static class ResourceSummary {
+		ResourceSummary() {
+			this.resource = new Resource();
+		}
+		private final Resource resource;
+		public Resource getResource() {
+	        return resource;
+	    }
+	}
+	
+	static class Resource {
+		public String getServer() {
+			return "abc";
+		}
+	}
+
+	/** Should be accessing Goo.getKey because 'bar' field evaluates to "key" */
+	@Test
+	public void testIndexingAsAPropertyAccess_SPR6968_1() {
+		StandardEvaluationContext eContext = new StandardEvaluationContext(new Goo());
+		String name = null;
+		Expression expr = null;
+		expr = new SpelExpressionParser().parseRaw("instance[bar]");
+		name = expr.getValue(eContext,String.class);
+		Assert.assertEquals("hello",name);
+		name = expr.getValue(eContext,String.class); // will be using the cached accessor this time
+		Assert.assertEquals("hello",name);
+	}
+
+	/** Should be accessing Goo.getKey because 'bar' variable evaluates to "key" */
+	@Test
+	public void testIndexingAsAPropertyAccess_SPR6968_2() {
+		StandardEvaluationContext eContext = new StandardEvaluationContext(new Goo());
+		eContext.setVariable("bar","key");
+		String name = null;
+		Expression expr = null;
+		expr = new SpelExpressionParser().parseRaw("instance[#bar]");
+		name = expr.getValue(eContext,String.class);
+		Assert.assertEquals("hello",name);
+		name = expr.getValue(eContext,String.class); // will be using the cached accessor this time
+		Assert.assertEquals("hello",name);
+	}
+
+	/** Should be accessing Goo.wibble field because 'bar' variable evaluates to "wibble" */
+	@Test
+	public void testIndexingAsAPropertyAccess_SPR6968_3() {
+		StandardEvaluationContext eContext = new StandardEvaluationContext(new Goo());
+		eContext.setVariable("bar","wibble");
+		String name = null;
+		Expression expr = null;
+		expr = new SpelExpressionParser().parseRaw("instance[#bar]");
+		// will access the field 'wibble' and not use a getter
+		name = expr.getValue(eContext,String.class);
+		Assert.assertEquals("wobble",name);
+		name = expr.getValue(eContext,String.class); // will be using the cached accessor this time
+		Assert.assertEquals("wobble",name);
+	}
+	
+	/** Should be accessing (setting) Goo.wibble field because 'bar' variable evaluates to "wibble" */
+	@Test
+	public void testIndexingAsAPropertyAccess_SPR6968_4() {
+		Goo g = Goo.instance;
+		StandardEvaluationContext eContext = new StandardEvaluationContext(g);
+		eContext.setVariable("bar","wibble");
+		Expression expr = null;
+		expr = new SpelExpressionParser().parseRaw("instance[#bar]='world'");
+		// will access the field 'wibble' and not use a getter
+		expr.getValue(eContext,String.class);
+		Assert.assertEquals("world",g.wibble);
+		expr.getValue(eContext,String.class); // will be using the cached accessor this time
+		Assert.assertEquals("world",g.wibble);
+	}
+
+	/** Should be accessing Goo.setKey field because 'bar' variable evaluates to "key" */
+	@Test
+	public void testIndexingAsAPropertyAccess_SPR6968_5() {
+		Goo g = Goo.instance;
+		StandardEvaluationContext eContext = new StandardEvaluationContext(g);
+		Expression expr = null;
+		expr = new SpelExpressionParser().parseRaw("instance[bar]='world'");
+		expr.getValue(eContext,String.class);
+		Assert.assertEquals("world",g.value);
+		expr.getValue(eContext,String.class); // will be using the cached accessor this time
+		Assert.assertEquals("world",g.value);
+	}
+
+	static class Goo {
+		
+		public static Goo instance = new Goo();
+		public String bar = "key";
+		public String value = null;
+
+		public String wibble = "wobble";
+		
+		public String getKey() {
+			return "hello";
+		}
+		
+		public void setKey(String s) {
+			value = s;
+		}
+		
+	}
 	
 	// ---