From a4b7ce168c16c499642fdebf09044bdec74c766d Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Tue, 7 Jul 2009 16:08:10 +0000 Subject: [PATCH] SPR-5804: problems with map access if it is the root object; SPR-5847: alternative map referencing strategies: a.b == a[b] == a['b'] --- .../expression/spel/ast/Indexer.java | 31 +++- .../spel/ast/PropertyOrFieldReference.java | 13 +- .../expression/spel/SpringEL300Tests.java | 141 +++++++++--------- .../spel/testresources/Inventor.java | 4 + 4 files changed, 113 insertions(+), 76 deletions(-) diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 5b6158c8c89..7640ae573db 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/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; } diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java index 283c21b80e1..1b2942ad425 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/PropertyOrFieldReference.java +++ b/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; + } } diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/SpringEL300Tests.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/SpringEL300Tests.java index 0858c3880d7..21a4c67f8e3 100644 --- a/org.springframework.expression/src/test/java/org/springframework/expression/spel/SpringEL300Tests.java +++ b/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 { diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java index 4239336637e..e266a32c495 100644 --- a/org.springframework.expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java +++ b/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;