From 3150681307b73960d173dda28ffc6de698099346 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Mon, 6 Jul 2009 19:21:26 +0000 Subject: [PATCH] SPR-5899: Invoking methods or constructors passing null (including varargs support) git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@1478 50f2f4bb-b051-0410-bef5-90022cba6387 --- .../spel/ast/ConstructorReference.java | 2 +- .../expression/spel/ast/MethodReference.java | 2 +- .../spel/support/ReflectionHelper.java | 91 ++++++----- .../expression/spel/SpringEL300Tests.java | 145 ++++++++++++++++++ 4 files changed, 201 insertions(+), 39 deletions(-) diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java index 0552d028862..90a9763ca75 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java +++ b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/ConstructorReference.java @@ -81,7 +81,7 @@ public class ConstructorReference extends SpelNodeImpl { TypedValue childValue = children[i + 1].getValueInternal(state); Object value = childValue.getValue(); arguments[i] = value; - argumentTypes[i] = (value==null?Object.class:value.getClass()); + argumentTypes[i] = (value==null?null:value.getClass()); } ConstructorExecutor executorToUse = this.cachedExecutor; diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java index 77be08d6044..500f22e25d4 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java +++ b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/MethodReference.java @@ -92,7 +92,7 @@ public class MethodReference extends SpelNodeImpl { private Class[] getTypes(Object... arguments) { Class[] argumentTypes = new Class[arguments.length]; for (int i = 0; i < arguments.length; i++) { - argumentTypes[i] = (arguments[i]==null?Object.class:arguments[i].getClass()); + argumentTypes[i] = (arguments[i]==null?null:arguments[i].getClass()); } return argumentTypes; } diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index ed379952652..b839c6742bb 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java @@ -57,19 +57,26 @@ public class ReflectionHelper { Class suppliedArg = suppliedArgTypes[i]; Class expectedArg = expectedArgTypes[i]; if (expectedArg != suppliedArg) { - if (ClassUtils.isAssignable(expectedArg, suppliedArg) - /* || isWidenableTo(expectedArg, suppliedArg) */) { - if (match != ArgsMatchKind.REQUIRES_CONVERSION) { - match = ArgsMatchKind.CLOSE; - } - } else if (typeConverter.canConvert(suppliedArg, expectedArg)) { - if (argsRequiringConversion == null) { - argsRequiringConversion = new ArrayList(); + // The user may supply null - and that will be ok unless a primitive is expected + if (suppliedArg==null) { + if (expectedArg.isPrimitive()) { + match=null; } - argsRequiringConversion.add(i); - match = ArgsMatchKind.REQUIRES_CONVERSION; } else { - match = null; + if (ClassUtils.isAssignable(expectedArg, suppliedArg) + /* || isWidenableTo(expectedArg, suppliedArg) */) { + if (match != ArgsMatchKind.REQUIRES_CONVERSION) { + match = ArgsMatchKind.CLOSE; + } + } else if (typeConverter.canConvert(suppliedArg, expectedArg)) { + if (argsRequiringConversion == null) { + argsRequiringConversion = new ArrayList(); + } + argsRequiringConversion.add(i); + match = ArgsMatchKind.REQUIRES_CONVERSION; + } else { + match = null; + } } } } @@ -113,23 +120,29 @@ public class ReflectionHelper { for (int i = 0; i < argCountUpToVarargs && match != null; i++) { Class suppliedArg = suppliedArgTypes[i]; Class expectedArg = expectedArgTypes[i]; - if (expectedArg != suppliedArg) { - if (expectedArg.isAssignableFrom(suppliedArg) || ClassUtils.isAssignableValue(expectedArg, suppliedArg)) { - if (match != ArgsMatchKind.REQUIRES_CONVERSION) { - match = ArgsMatchKind.CLOSE; + if (suppliedArg==null) { + if (expectedArg.isPrimitive()) { + match=null; + } + } else { + if (expectedArg != suppliedArg) { + if (expectedArg.isAssignableFrom(suppliedArg) || ClassUtils.isAssignableValue(expectedArg, suppliedArg)) { + if (match != ArgsMatchKind.REQUIRES_CONVERSION) { + match = ArgsMatchKind.CLOSE; + } + } else if (typeConverter.canConvert(suppliedArg, expectedArg)) { + if (argsRequiringConversion == null) { + argsRequiringConversion = new ArrayList(); + } + argsRequiringConversion.add(i); + match = ArgsMatchKind.REQUIRES_CONVERSION; + } else { + match = null; } - } else if (typeConverter.canConvert(suppliedArg, expectedArg)) { - if (argsRequiringConversion == null) { - argsRequiringConversion = new ArrayList(); - } - argsRequiringConversion.add(i); - match = ArgsMatchKind.REQUIRES_CONVERSION; - } else { - match = null; } } } - // If already confirmed it cannot be a match, then returnW + // If already confirmed it cannot be a match, then return if (match == null) { return null; } @@ -147,19 +160,24 @@ public class ReflectionHelper { for (int i = expectedArgTypes.length - 1; i < suppliedArgTypes.length; i++) { Class suppliedArg = suppliedArgTypes[i]; if (varargsParameterType != suppliedArg) { - if (ClassUtils.isAssignable(varargsParameterType, suppliedArg)) { - if (match != ArgsMatchKind.REQUIRES_CONVERSION) { - match = ArgsMatchKind.CLOSE; + if (suppliedArg==null) { + if (varargsParameterType.isPrimitive()) { + match=null; } - } else if (typeConverter.canConvert(suppliedArg, varargsParameterType)) { - if (argsRequiringConversion == null) { - argsRequiringConversion = new ArrayList(); + } else { + if (ClassUtils.isAssignable(varargsParameterType, suppliedArg)) { + if (match != ArgsMatchKind.REQUIRES_CONVERSION) { + match = ArgsMatchKind.CLOSE; + } + } else if (typeConverter.canConvert(suppliedArg, varargsParameterType)) { + if (argsRequiringConversion == null) { + argsRequiringConversion = new ArrayList(); + } + argsRequiringConversion.add(i); + match = ArgsMatchKind.REQUIRES_CONVERSION; + } else { + match = null; } - argsRequiringConversion.add(i); - match = ArgsMatchKind.REQUIRES_CONVERSION; - } - else { - match = null; } } } @@ -167,8 +185,7 @@ public class ReflectionHelper { if (match == null) { return null; - } - else { + } else { if (match == ArgsMatchKind.REQUIRES_CONVERSION) { int[] argsArray = new int[argsRequiringConversion.size()]; for (int i = 0; i < argsRequiringConversion.size(); i++) { 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 2829b26f46f..0858c3880d7 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 @@ -21,11 +21,14 @@ import junit.framework.Assert; import org.junit.Test; import org.springframework.expression.AccessException; import org.springframework.expression.EvaluationContext; +import org.springframework.expression.EvaluationException; import org.springframework.expression.Expression; import org.springframework.expression.ParserContext; import org.springframework.expression.PropertyAccessor; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.ReflectivePropertyResolver; +import org.springframework.expression.spel.support.StandardEvaluationContext; +import org.springframework.expression.spel.support.StandardTypeLocator; /** * Tests based on Jiras up to the release of Spring 3.0.0 @@ -39,6 +42,148 @@ public class SpringEL300Tests extends ExpressionTestCase { evaluate("joinThreeStrings('a',null,'c')", "anullc", String.class); } + @Test + public void testSPR5899() throws Exception { + StandardEvaluationContext eContext = new StandardEvaluationContext(new Spr5899Class()); + Expression expr = new SpelExpressionParser().parse("tryToInvokeWithNull(12)"); + Assert.assertEquals(12,expr.getValue(eContext)); + expr = new SpelExpressionParser().parse("tryToInvokeWithNull(null)"); + Assert.assertEquals(null,expr.getValue(eContext)); + try { + expr = new SpelExpressionParser().parse("tryToInvokeWithNull2(null)"); + expr.getValue(); + Assert.fail("Should have failed to find a method to which it could pass null"); + } catch (EvaluationException see) { + // success + } + eContext.setTypeLocator(new MyTypeLocator()); + + // varargs + expr = new SpelExpressionParser().parse("tryToInvokeWithNull3(null,'a','b')"); + Assert.assertEquals("ab",expr.getValue(eContext)); + + // varargs 2 - null is packed into the varargs + expr = new SpelExpressionParser().parse("tryToInvokeWithNull3(12,'a',null,'c')"); + Assert.assertEquals("anullc",expr.getValue(eContext)); + + // check we can find the ctor ok + expr = new SpelExpressionParser().parse("new Spr5899Class().toString()"); + Assert.assertEquals("instance",expr.getValue(eContext)); + + expr = new SpelExpressionParser().parse("new Spr5899Class(null).toString()"); + Assert.assertEquals("instance",expr.getValue(eContext)); + + // ctor varargs + expr = new SpelExpressionParser().parse("new Spr5899Class(null,'a','b').toString()"); + Assert.assertEquals("instance",expr.getValue(eContext)); + + // ctor varargs 2 + expr = new SpelExpressionParser().parse("new Spr5899Class(null,'a', null, 'b').toString()"); + Assert.assertEquals("instance",expr.getValue(eContext)); + } + + static class MyTypeLocator extends StandardTypeLocator { + + public Class findType(String typename) throws EvaluationException { + if (typename.equals("Spr5899Class")) { + return Spr5899Class.class; + } + return super.findType(typename); + } + } + + static class Spr5899Class { + public Spr5899Class() {} + public Spr5899Class(Integer i) { } + public Spr5899Class(Integer i, String... s) { } + + public Integer tryToInvokeWithNull(Integer value) { return value; } + public Integer tryToInvokeWithNull2(int i) { return new Integer(i); } + public String tryToInvokeWithNull3(Integer value,String... strings) { + StringBuilder sb = new StringBuilder(); + for (int i=0;i