SPR-5804: problems with map access if it is the root object; SPR-5847: alternative map referencing strategies: a.b == a[b] == a['b']

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

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

@@ -48,14 +48,29 @@ public class Indexer extends SpelNodeImpl {
 		TypedValue context = state.getActiveContextObject();
 		Object targetObject = context.getValue();
 		TypeDescriptor targetObjectTypeDescriptor = context.getTypeDescriptor();
-		TypedValue indexValue =  children[0].getValueInternal(state);
-		Object index = indexValue.getValue();
+		TypedValue indexValue =  null;
+		Object index = null;
+		
+		// This first part of the if clause prevents a 'double dereference' of the property (SPR-5847)
+		if (targetObject instanceof Map && (children[0] instanceof PropertyOrFieldReference)) {
+			PropertyOrFieldReference reference = (PropertyOrFieldReference)children[0];
+			index = reference.getName();
+			indexValue = new TypedValue(index,CommonTypeDescriptors.STRING_TYPE_DESCRIPTOR);
+		} else {
+			indexValue = children[0].getValueInternal(state);
+			index = indexValue.getValue();
+		}
 
 		// Indexing into a Map
 		if (targetObject instanceof Map) {
-			Object possiblyConvertedKey = state.convertValue(indexValue,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getMapKeyType()));
+			Object possiblyConvertedKey = index;
+			if (targetObjectTypeDescriptor.isMapEntryTypeKnown()) {
+				possiblyConvertedKey = state.convertValue(index,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getMapKeyType()));
+			}
 			Object o = ((Map<?, ?>) targetObject).get(possiblyConvertedKey);
-			return new TypedValue(o,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getMapValueType()));
+			TypeDescriptor resultDescriptor = targetObjectTypeDescriptor.isMapEntryTypeKnown()?
+					TypeDescriptor.valueOf(targetObjectTypeDescriptor.getMapValueType()):CommonTypeDescriptors.OBJECT_TYPE_DESCRIPTOR;
+			return new TypedValue(o,resultDescriptor);
 		}
 
 		int idx = (Integer)state.convertValue(index, INTEGER_TYPE_DESCRIPTOR);
@@ -131,8 +146,12 @@ public class Indexer extends SpelNodeImpl {
 		// Indexing into a Map
 		if (targetObjectTypeDescriptor.isMap()) {
 			Map map = (Map)targetObject;
-			Object possiblyConvertedKey = state.convertValue(index.getValue(),TypeDescriptor.valueOf(targetObjectTypeDescriptor.getMapKeyType()));
-			Object possiblyConvertedValue = state.convertValue(newValue,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getMapValueType()));
+			Object possiblyConvertedKey = index;
+			Object possiblyConvertedValue = newValue;
+			if (targetObjectTypeDescriptor.isMapEntryTypeKnown()) {
+			  possiblyConvertedKey = state.convertValue(index.getValue(),TypeDescriptor.valueOf(targetObjectTypeDescriptor.getMapKeyType()));
+			  possiblyConvertedValue = state.convertValue(newValue,TypeDescriptor.valueOf(targetObjectTypeDescriptor.getMapValueType()));
+			}
 			map.put(possiblyConvertedKey,possiblyConvertedValue);
 			return;
 		}

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

@@ -18,10 +18,12 @@ package org.springframework.expression.spel.ast;
 
 import java.util.ArrayList;
 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;
@@ -52,7 +54,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl {
 	}
 
 	@Override
-	public TypedValue getValueInternal(ExpressionState state) throws SpelEvaluationException {
+	public TypedValue getValueInternal(ExpressionState state) throws EvaluationException {
 		TypedValue result = readProperty(state, this.name);
 		if (result.getValue()==null && state.configuredToCreateCollection() && result.getTypeDescriptor().getType().equals(List.class) && nextChildIs(Indexer.class)) {
 			// Create a new list ready for the indexer
@@ -93,11 +95,12 @@ public class PropertyOrFieldReference extends SpelNodeImpl {
 	 * @return the value of the property
 	 * @throws SpelEvaluationException if any problem accessing the property or it cannot be found
 	 */
-	private TypedValue readProperty(ExpressionState state, String name) throws SpelEvaluationException {
+	private TypedValue readProperty(ExpressionState state, String name) throws EvaluationException {
 		TypedValue contextObject = state.getActiveContextObject();
 		EvaluationContext eContext = state.getEvaluationContext();
+		Object targetObject = contextObject.getValue();
 
-		if (contextObject.getValue() == null && nullSafe) {
+		if (targetObject == null && nullSafe) {
 			return TypedValue.NULL_TYPED_VALUE;
 		}
 
@@ -245,5 +248,9 @@ public class PropertyOrFieldReference extends SpelNodeImpl {
 		resolvers.addAll(generalAccessors);
 		return resolvers;
 	}
+	
+	String getName() {
+		return name;
+	}
 
 }

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

@@ -16,6 +16,10 @@
 
 package org.springframework.expression.spel;
 
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+
 import junit.framework.Assert;
 
 import org.junit.Test;
@@ -25,6 +29,8 @@ import org.springframework.expression.EvaluationException;
 import org.springframework.expression.Expression;
 import org.springframework.expression.ParserContext;
 import org.springframework.expression.PropertyAccessor;
+import org.springframework.expression.TypedValue;
+import org.springframework.expression.spel.ast.CommonTypeDescriptors;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.expression.spel.support.ReflectivePropertyResolver;
 import org.springframework.expression.spel.support.StandardEvaluationContext;
@@ -41,6 +47,11 @@ public class SpringEL300Tests extends ExpressionTestCase {
 	public void testNPE_SPR5661() {
 		evaluate("joinThreeStrings('a',null,'c')", "anullc", String.class);
 	}
+
+//	@Test
+//	public void testSWF1086() {
+//		evaluate("printDouble(T(java.math.BigDecimal).valueOf(14.35))", "anullc", String.class);
+//	}
 	
 	@Test
 	public void testSPR5899() throws Exception {
@@ -116,73 +127,69 @@ public class SpringEL300Tests extends ExpressionTestCase {
 		 }
 	}
 
-	// work in progress tests:
-//	@Test
-//	public void testSPR5804() throws ParseException, EvaluationException {
-//		Map m  = new HashMap();
-//		m.put("foo",null);
-//		StandardEvaluationContext eContext = new StandardEvaluationContext(m);
-//		eContext.addPropertyAccessor(new MapAccessor());
-//		Expression expr = new SpelExpressionParser().parse("foo");
-//		Object o = expr.getValue(eContext);
-//	}
-//	
-////	jdbcProperties.username
-//	
-//	@Test
-//	public void testSPR5847() throws ParseException, EvaluationException {
-//		StandardEvaluationContext eContext = new StandardEvaluationContext(new TestProperties2());
-//		Expression expr = new SpelExpressionParser().parse("jdbcProperties['username']");
-//		eContext.addPropertyAccessor(new MapAccessor());
-//		String name = expr.getValue(eContext,String.class);
-//		Assert.assertEquals("Dave",name);
-////		System.out.println(o);
-//		
-//		
-////		Map m  = new HashMap();
-////		m.put("jdbcProperties",new TestProperties());
-////		StandardEvaluationContext eContext = new StandardEvaluationContext(m);
-//////		eContext.addPropertyAccessor(new MapAccessor());
-////		Expression expr = new SpelExpressionParser().parse("jdbcProperties.username");
-////		Object o = expr.getValue(eContext);
-////		System.out.println(o);
-//	}
-//	
-//	static class TestProperties {
-//		public String username = "Dave";
-//	}
-//	
-//	static class TestProperties2 {
-//		public Map jdbcProperties = new HashMap();
-//		TestProperties2() {
-//			jdbcProperties.put("username","Dave");
-//		}
-//	}
-//
-//	static class MapAccessor implements PropertyAccessor {
-//
-//		public boolean canRead(EvaluationContext context, Object target, String name) throws AccessException {
-//			return (((Map) target).containsKey(name));
-//		}
-//
-//		public TypedValue read(EvaluationContext context, Object target, String name) throws AccessException {
-//			return new TypedValue(((Map) target).get(name),CommonTypeDescriptors.OBJECT_TYPE_DESCRIPTOR);
-//		}
-//
-//		public boolean canWrite(EvaluationContext context, Object target, String name) throws AccessException {
-//			return true;
-//		}
-//
-//		@SuppressWarnings("unchecked")
-//		public void write(EvaluationContext context, Object target, String name, Object newValue) throws AccessException {
-//			((Map) target).put(name, newValue);
-//		}
-//
-//		public Class[] getSpecificTargetClasses() {
-//			return new Class[] {Map.class};
-//		}
-//		
-//	}
+	@SuppressWarnings("unchecked")
+	@Test
+	public void testSPR5804() throws Exception {
+		Map m = new HashMap();
+		m.put("foo", "bar");
+		StandardEvaluationContext eContext = new StandardEvaluationContext(m); // root is a map instance
+		eContext.addPropertyAccessor(new MapAccessor());
+		Expression expr = new SpelExpressionParser().parseExpression("['foo']");
+		Assert.assertEquals("bar", expr.getValue(eContext));
+	}
+	
+	@Test
+	public void testSPR5847() throws Exception {
+		StandardEvaluationContext eContext = new StandardEvaluationContext(new TestProperties());
+		String name = null;
+		Expression expr = null;
+		
+		expr = new SpelExpressionParser().parse("jdbcProperties['username']");
+		name = expr.getValue(eContext,String.class);
+		Assert.assertEquals("Dave",name);
+		
+		expr = new SpelExpressionParser().parse("jdbcProperties[username]");
+		name = expr.getValue(eContext,String.class);
+		Assert.assertEquals("Dave",name);
+
+		expr = new SpelExpressionParser().parse("jdbcProperties.username");
+		eContext.addPropertyAccessor(new MapAccessor());
+		name = expr.getValue(eContext,String.class);
+		Assert.assertEquals("Dave",name);
+	}
+	
+	static class TestProperties {
+		public Properties jdbcProperties = new Properties();
+		TestProperties() {
+			jdbcProperties.put("username","Dave");
+			jdbcProperties.put("foo.bar","Dave");
+		}
+	}
+
+	static class MapAccessor implements PropertyAccessor {
+
+		public boolean canRead(EvaluationContext context, Object target, String name) throws AccessException {
+			return (((Map) target).containsKey(name));
+		}
+
+		public TypedValue read(EvaluationContext context, Object target, String name) throws AccessException {
+			return new TypedValue(((Map) target).get(name),CommonTypeDescriptors.OBJECT_TYPE_DESCRIPTOR);
+		}
+
+		public boolean canWrite(EvaluationContext context, Object target, String name) throws AccessException {
+			return true;
+		}
+
+		@SuppressWarnings("unchecked")
+		public void write(EvaluationContext context, Object target, String name, Object newValue) throws AccessException {
+			((Map) target).put(name, newValue);
+		}
+
+		public Class[] getSpecificTargetClasses() {
+			return new Class[] {Map.class};
+		}
+		
+	}
 	
 	@Test
 	public void testNPE_SPR5673() throws Exception {

org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java

@@ -125,6 +125,10 @@ public class Inventor {
 	public String sayHelloTo(String person) {
 		return "hello " + person;
 	}
+	
+	public String printDouble(Double d) {
+		return d.toString();
+	}
 
 	public String joinThreeStrings(String a, String b, String c) {
 		return a + b + c;