From dce025e40f4d4435314147eaf2cda355098c2e2c Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Mon, 13 Apr 2009 20:51:52 +0000 Subject: [PATCH] more tests - about 82% coverage now. focused on testing reflection helper since it is rather complex. some asserts also added. git-svn-id: https://src.springframework.org/svn/spring-framework/trunk@989 50f2f4bb-b051-0410-bef5-90022cba6387 --- .../spel/ast/FunctionReference.java | 2 +- .../spel/support/ReflectionHelper.java | 114 ++++++++---- .../support/ReflectiveMethodResolver.java | 3 +- .../expression/spel/ExpressionStateTests.java | 4 + .../expression/spel/HelperTests.java | 175 +++++++++++++++++- .../spel/ScenariosForSpringSecurity.java | 2 +- 6 files changed, 248 insertions(+), 52 deletions(-) diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java index df8268889d2..9721153c8db 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java +++ b/org.springframework.expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java @@ -95,7 +95,7 @@ public class FunctionReference extends SpelNodeImpl { // Convert arguments if necessary and remap them for varargs if required if (functionArgs != null) { TypeConverter converter = state.getEvaluationContext().getTypeConverter(); - ReflectionHelper.convertArguments(m.getParameterTypes(), m.isVarArgs(), converter, functionArgs); + ReflectionHelper.convertAllArguments(m.getParameterTypes(), m.isVarArgs(), converter, functionArgs); } if (m.isVarArgs()) { functionArgs = ReflectionHelper.setupArgumentsForVarargsInvocation(m.getParameterTypes(), functionArgs); 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 5961cba5f5e..1828c5814ea 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 @@ -24,6 +24,7 @@ import org.springframework.expression.EvaluationException; import org.springframework.expression.TypeConverter; import org.springframework.expression.spel.SpelException; import org.springframework.expression.spel.SpelMessages; +import org.springframework.util.Assert; import org.springframework.util.ClassUtils; /** @@ -45,9 +46,11 @@ public class ReflectionHelper { * @param typeConverter a registered type converter * @return a MatchInfo object indicating what kind of match it was or null if it was not a match */ - public static ArgumentsMatchInfo compareArguments( + public static ArgumentsMatchInfo compareArguments( Class[] expectedArgTypes, Class[] suppliedArgTypes, TypeConverter typeConverter) { + Assert.isTrue(expectedArgTypes.length==suppliedArgTypes.length, "Expected argument types and supplied argument types should be arrays of same length"); + ArgsMatchKind match = ArgsMatchKind.EXACT; List argsRequiringConversion = null; for (int i = 0; i < expectedArgTypes.length && match != null; i++) { @@ -94,9 +97,12 @@ public class ReflectionHelper { * @param typeConverter a registered type converter * @return a MatchInfo object indicating what kind of match it was or null if it was not a match */ - static ArgumentsMatchInfo compareArgumentsVarargs( + public static ArgumentsMatchInfo compareArgumentsVarargs( Class[] expectedArgTypes, Class[] suppliedArgTypes, TypeConverter typeConverter) { + Assert.isTrue(expectedArgTypes!=null && expectedArgTypes.length>0, "Expected arguments must at least include one array (the vargargs parameter)"); + Assert.isTrue(expectedArgTypes[expectedArgTypes.length-1].isArray(), "Final expected argument should be array type (the varargs parameter)"); + ArgsMatchKind match = ArgsMatchKind.EXACT; List argsRequiringConversion = null; @@ -145,8 +151,7 @@ public class ReflectionHelper { if (match != ArgsMatchKind.REQUIRES_CONVERSION) { match = ArgsMatchKind.CLOSE; } - } - else if (typeConverter.canConvert(suppliedArg, varargsParameterType)) { + } else if (typeConverter.canConvert(suppliedArg, varargsParameterType)) { if (argsRequiringConversion == null) { argsRequiringConversion = new ArrayList(); } @@ -177,34 +182,60 @@ public class ReflectionHelper { } } - public static void convertArguments(Class[] parameterTypes, boolean isVarargs, TypeConverter converter, - int[] argsRequiringConversion, Object... arguments) throws EvaluationException { + /** + * Takes an input set of argument values and, following the positions specified in the int array, it converts + * them to the types specified as the required parameter types. The arguments are converted 'in-place' in the + * input array. + * @param requiredParameterTypes the types that the caller would like to have + * @param isVarargs whether the requiredParameterTypes is a varargs list + * @param converter the type converter to use for attempting conversions + * @param argumentsRequiringConversion details which of the input arguments need conversion + * @param arguments the actual arguments that need conversion + * @throws EvaluationException if a problem occurs during conversion + */ + public static void convertArguments(Class[] requiredParameterTypes, boolean isVarargs, TypeConverter converter, + int[] argumentsRequiringConversion, Object[] arguments) throws EvaluationException { + + Assert.notNull(argumentsRequiringConversion,"should not be called if no conversions required"); + Assert.notNull(arguments,"should not be called if no conversions required"); + Class varargsType = null; if (isVarargs) { - varargsType = parameterTypes[parameterTypes.length - 1].getComponentType(); + Assert.isTrue(requiredParameterTypes[requiredParameterTypes.length-1].isArray(),"if varargs then last parameter type must be array"); + varargsType = requiredParameterTypes[requiredParameterTypes.length - 1].getComponentType(); } - for (Integer argPosition : argsRequiringConversion) { + for (Integer argPosition : argumentsRequiringConversion) { Class targetType = null; - if (isVarargs && argPosition >= (parameterTypes.length - 1)) { + if (isVarargs && argPosition >= (requiredParameterTypes.length - 1)) { targetType = varargsType; + } else { + targetType = requiredParameterTypes[argPosition]; } - else { - targetType = parameterTypes[argPosition]; - } - // try { arguments[argPosition] = converter.convertValue(arguments[argPosition], targetType); - // } catch (EvaluationException e) { - // throw new SpelException(e, SpelMessages.PROBLEM_DURING_TYPE_CONVERSION, "Converter failed to convert '" - // + arguments[argPosition] + " to type '" + targetType + "'"); - // } } } - public static void convertArguments(Class[] parameterTypes, boolean isVarargs, TypeConverter converter, - Object... arguments) throws EvaluationException { + /** + * Convert a supplied set of arguments into the requested types. If the parameterTypes are related to + * a varargs method then the final entry in the parameterTypes array is going to be an array itself whose + * component type should be used as the conversion target for extraneous arguments. (For example, if the + * parameterTypes are {Integer, String[]} and the input arguments are {Integer, boolean, float} then both + * the boolean and float must be converted to strings). This method does not repackage the arguments + * into a form suitable for the varargs invocation + * @param parameterTypes the types to be converted to + * @param isVarargs whether parameterTypes relates to a varargs method + * @param converter the converter to use for type conversions + * @param arguments the arguments to convert to the requested parameter types + * @throws SpelException if there is a problem with conversion + */ + public static void convertAllArguments(Class[] parameterTypes, boolean isVarargs, TypeConverter converter, + Object[] arguments) throws SpelException { + Assert.notNull(arguments,"should not be called if nothing to convert"); + Class varargsType = null; if (isVarargs) { + Assert.isTrue(parameterTypes[parameterTypes.length-1].isArray(),"if varargs then last parameter type must be array"); varargsType = parameterTypes[parameterTypes.length - 1].getComponentType(); } for (int i = 0; i < arguments.length; i++) { @@ -214,23 +245,19 @@ public class ReflectionHelper { } else { targetType = parameterTypes[i]; } - if (converter == null) { - throw new SpelException(SpelMessages.PROBLEM_DURING_TYPE_CONVERSION, - "No converter available to convert '" + arguments[i] + " to type '" + targetType + "'"); - } try { if (arguments[i] != null && arguments[i].getClass() != targetType) { + if (converter == null) { + throw new SpelException(SpelMessages.TYPE_CONVERSION_ERROR, arguments[i].getClass().getName(),targetType); + } arguments[i] = converter.convertValue(arguments[i], targetType); } - } - catch (EvaluationException ex) { + } catch (EvaluationException ex) { // allows for another type converter throwing a different kind of EvaluationException if (ex instanceof SpelException) { - throw ex; - } - else { - throw new SpelException(ex, SpelMessages.PROBLEM_DURING_TYPE_CONVERSION, - "Converter failed to convert '" + arguments[i].getClass().getName() + "' to type '" + targetType + "'"); + throw (SpelException)ex; + } else { + throw new SpelException(ex, SpelMessages.TYPE_CONVERSION_ERROR,arguments[i].getClass().getName(),targetType); } } } @@ -241,30 +268,30 @@ public class ReflectionHelper { * parameterTypes is (int, String[]) because the second parameter was declared String... then if arguments is * [1,"a","b"] then it must be repackaged as [1,new String[]{"a","b"}] in order to match the expected * parameterTypes. - * @param paramTypes the types of the parameters for the invocation + * @param requiredParameterTypes the types of the parameters for the invocation * @param args the arguments to be setup ready for the invocation * @return a repackaged array of arguments where any varargs setup has been done */ - public static Object[] setupArgumentsForVarargsInvocation(Class[] paramTypes, Object... args) { + public static Object[] setupArgumentsForVarargsInvocation(Class[] requiredParameterTypes, Object... args) { // Check if array already built for final argument - int nParams = paramTypes.length; - int nArgs = args.length; + int parameterCount = requiredParameterTypes.length; + int argumentCount = args.length; // Check if repackaging is needed: - if (nParams != args.length || paramTypes[nParams - 1] != (args[nArgs - 1] == null ? null : args[nArgs - 1].getClass())) { + if (parameterCount != args.length || requiredParameterTypes[parameterCount - 1] != (args[argumentCount - 1] == null ? null : args[argumentCount - 1].getClass())) { int arraySize = 0; // zero size array if nothing to pass as the varargs parameter - if (nArgs >= nParams) { - arraySize = nArgs - (nParams - 1); + if (argumentCount >= parameterCount) { + arraySize = argumentCount - (parameterCount - 1); } - Object[] repackagedArguments = (Object[]) Array.newInstance(paramTypes[nParams - 1].getComponentType(), + Object[] repackagedArguments = (Object[]) Array.newInstance(requiredParameterTypes[parameterCount - 1].getComponentType(), arraySize); // Copy all but the varargs arguments for (int i = 0; i < arraySize; i++) { - repackagedArguments[i] = args[nParams + i - 1]; + repackagedArguments[i] = args[parameterCount + i - 1]; } // Create an array for the varargs arguments - Object[] newArgs = new Object[nParams]; + Object[] newArgs = new Object[parameterCount]; for (int i = 0; i < newArgs.length - 1; i++) { newArgs[i] = args[i]; } @@ -276,7 +303,12 @@ public class ReflectionHelper { public static enum ArgsMatchKind { - EXACT, CLOSE, REQUIRES_CONVERSION + // An exact match is where the parameter types exactly match what the method/constructor being invoked is expecting + EXACT, + // A close match is where the parameter types either exactly match or are assignment compatible with the method/constructor being invoked + CLOSE, + // A conversion match is where the type converter must be used to transform some of the parameter types + REQUIRES_CONVERSION } diff --git a/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java b/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java index 92c80cedfaa..97de92d00d7 100644 --- a/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java +++ b/org.springframework.expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java @@ -62,8 +62,7 @@ public class ReflectiveMethodResolver implements MethodResolver { if (method.isVarArgs() && argumentTypes.length >= (method.getParameterTypes().length - 1)) { // *sigh* complicated matchInfo = ReflectionHelper.compareArgumentsVarargs(method.getParameterTypes(), argumentTypes, typeConverter); - } - else if (method.getParameterTypes().length == argumentTypes.length) { + } else if (method.getParameterTypes().length == argumentTypes.length) { // name and parameter number match, check the arguments matchInfo = ReflectionHelper.compareArguments(method.getParameterTypes(), argumentTypes, typeConverter); } diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/ExpressionStateTests.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/ExpressionStateTests.java index 0f4f61b5849..90ccf781059 100644 --- a/org.springframework.expression/src/test/java/org/springframework/expression/spel/ExpressionStateTests.java +++ b/org.springframework.expression/src/test/java/org/springframework/expression/spel/ExpressionStateTests.java @@ -115,6 +115,10 @@ public class ExpressionStateTests extends ExpressionTestCase { state = new ExpressionState(new StandardEvaluationContext()); assertEquals(TypedValue.NULL_TYPED_VALUE,state.getRootContextObject()); + + + ((StandardEvaluationContext)state.getEvaluationContext()).setRootObject(null,TypeDescriptor.NULL_TYPE_DESCRIPTOR); + assertEquals(null,state.getRootContextObject().getValue()); } public void testActiveContextObject() { diff --git a/org.springframework.expression/src/test/java/org/springframework/expression/spel/HelperTests.java b/org.springframework.expression/src/test/java/org/springframework/expression/spel/HelperTests.java index 33f827ca68a..3d004bfb6b6 100644 --- a/org.springframework.expression/src/test/java/org/springframework/expression/spel/HelperTests.java +++ b/org.springframework.expression/src/test/java/org/springframework/expression/spel/HelperTests.java @@ -20,6 +20,7 @@ import java.io.PrintStream; import java.util.ArrayList; import java.util.List; +import org.springframework.expression.EvaluationException; import org.springframework.expression.ParseException; import org.springframework.expression.TypedValue; import org.springframework.expression.spel.ast.FormatHelper; @@ -94,13 +95,6 @@ public class HelperTests extends ExpressionTestCase { checkMatch(new Class[]{String.class,Integer.class},new Class[]{String.class,Integer.class},typeConverter,ArgsMatchKind.EXACT); } - public void testReflectionHelperCompareArguments_Varargs_ExactMatching() { - StandardTypeConverter tc = new StandardTypeConverter(); - - // Calling foo(String) with (String) is exact match - checkMatch(new Class[]{String.class},new Class[]{String.class},tc,ArgsMatchKind.EXACT); - } - public void testReflectionHelperCompareArguments_CloseMatching() { StandardTypeConverter typeConverter = new StandardTypeConverter(); @@ -136,6 +130,131 @@ public class HelperTests extends ExpressionTestCase { // Passing (Super,String) on call to foo(Sub,String) is not a match checkMatch(new Class[]{Super.class,String.class},new Class[]{Sub.class,String.class},typeConverter,null); } + + public void testReflectionHelperCompareArguments_Varargs_ExactMatching() { + StandardTypeConverter tc = new StandardTypeConverter(); + Class stringArrayClass = new String[0].getClass(); + Class integerArrayClass = new Integer[0].getClass(); + + // Passing (String[]) on call to (String[]) is exact match + checkMatch2(new Class[]{stringArrayClass},new Class[]{stringArrayClass},tc,ArgsMatchKind.EXACT); + + // Passing (Integer, String[]) on call to (Integer, String[]) is exact match + checkMatch2(new Class[]{Integer.class,stringArrayClass},new Class[]{Integer.class,stringArrayClass},tc,ArgsMatchKind.EXACT); + + // Passing (String, Integer, String[]) on call to (String, String, String[]) is exact match + checkMatch2(new Class[]{String.class,Integer.class,stringArrayClass},new Class[]{String.class,Integer.class,stringArrayClass},tc,ArgsMatchKind.EXACT); + + // Passing (Sub, String[]) on call to (Super, String[]) is exact match + checkMatch2(new Class[]{Sub.class,stringArrayClass},new Class[]{Super.class,stringArrayClass},tc,ArgsMatchKind.CLOSE); + + // Passing (Integer, String[]) on call to (String, String[]) is exact match + checkMatch2(new Class[]{Integer.class,stringArrayClass},new Class[]{String.class,stringArrayClass},tc,ArgsMatchKind.REQUIRES_CONVERSION,0); + + // Passing (Integer, Sub, String[]) on call to (String, Super, String[]) is exact match + checkMatch2(new Class[]{Integer.class,Sub.class,String[].class},new Class[]{String.class,Super.class,String[].class},tc,ArgsMatchKind.REQUIRES_CONVERSION,0); + + // Passing (String) on call to (String[]) is exact match + checkMatch2(new Class[]{String.class},new Class[]{stringArrayClass},tc,ArgsMatchKind.EXACT); + + // Passing (Integer,String) on call to (Integer,String[]) is exact match + checkMatch2(new Class[]{Integer.class,String.class},new Class[]{Integer.class,stringArrayClass},tc,ArgsMatchKind.EXACT); + + // Passing (String) on call to (Integer[]) is conversion match (String to Integer) + checkMatch2(new Class[]{String.class},new Class[]{integerArrayClass},tc,ArgsMatchKind.REQUIRES_CONVERSION,0); + + // Passing (Sub) on call to (Super[]) is close match + checkMatch2(new Class[]{Sub.class},new Class[]{new Super[0].getClass()},tc,ArgsMatchKind.CLOSE); + + // Passing (Super) on call to (Sub[]) is not a match + checkMatch2(new Class[]{Super.class},new Class[]{new Sub[0].getClass()},tc,null); + + checkMatch2(new Class[]{Unconvertable.class,String.class},new Class[]{Sub.class,Super[].class},tc,null); + + checkMatch2(new Class[]{Integer.class,Integer.class,String.class},new Class[]{String.class,String.class,Super[].class},tc,null); + + checkMatch2(new Class[]{Unconvertable.class,String.class},new Class[]{Sub.class,Super[].class},tc,null); + + checkMatch2(new Class[]{Integer.class,Integer.class,String.class},new Class[]{String.class,String.class,Super[].class},tc,null); + + checkMatch2(new Class[]{Integer.class,Integer.class,Sub.class},new Class[]{String.class,String.class,Super[].class},tc,ArgsMatchKind.REQUIRES_CONVERSION,0,1); + + checkMatch2(new Class[]{Integer.class,Integer.class,Integer.class},new Class[]{Integer.class,String[].class},tc,ArgsMatchKind.REQUIRES_CONVERSION,1,2); + // what happens on (Integer,String) passed to (Integer[]) ? + } + + public void testConvertArguments() throws Exception { + StandardTypeConverter tc = new StandardTypeConverter(); + + // basic conversion int>String + Object[] args = new Object[]{3}; + ReflectionHelper.convertArguments(new Class[]{String.class}, false, tc, new int[]{0}, args); + checkArguments(args, "3"); + + // varargs but nothing to convert + args = new Object[]{3}; + ReflectionHelper.convertArguments(new Class[]{String.class,String[].class}, false, tc, new int[]{0}, args); + checkArguments(args, "3"); + + // varargs with nothing needing conversion + args = new Object[]{3,"abc","abc"}; + ReflectionHelper.convertArguments(new Class[]{String.class,String[].class}, true, tc, new int[]{0,1,2}, args); + checkArguments(args, "3","abc","abc"); + + // varargs with conversion required + args = new Object[]{3,false,3.0d}; + ReflectionHelper.convertArguments(new Class[]{String.class,String[].class}, true, tc, new int[]{0,1,2}, args); + checkArguments(args, "3","false","3.0"); +} + + public void testConvertArguments2() throws EvaluationException { + StandardTypeConverter tc = new StandardTypeConverter(); + + // Simple conversion: int to string + Object[] args = new Object[]{3}; + ReflectionHelper.convertAllArguments(new Class[]{String.class}, false, tc, args); + checkArguments(args,"3"); + + // varargs conversion + args = new Object[]{3,false,3.0f}; + ReflectionHelper.convertAllArguments(new Class[]{String.class,String[].class}, true, tc, args); + checkArguments(args,"3","false","3.0"); + + // varargs conversion but no varargs + args = new Object[]{3}; + ReflectionHelper.convertAllArguments(new Class[]{String.class,String[].class}, true, tc, args); + checkArguments(args,"3"); + + // missing converter + args = new Object[]{3,false,3.0f}; + try { + ReflectionHelper.convertAllArguments(new Class[]{String.class,String[].class}, true, null, args); + fail("Should have failed because no converter supplied"); + } catch (SpelException se) { + assertEquals(SpelMessages.TYPE_CONVERSION_ERROR,se.getMessageUnformatted()); + } + + // null value + args = new Object[]{3,null,3.0f}; + ReflectionHelper.convertAllArguments(new Class[]{String.class,String[].class}, true, tc, args); + checkArguments(args,"3",null,"3.0"); + } + + + public void testSetupArguments() { + Object[] newArray = ReflectionHelper.setupArgumentsForVarargsInvocation(new Class[]{new String[0].getClass()},"a","b","c"); + + assertEquals(1,newArray.length); + Object firstParam = newArray[0]; + assertEquals(String.class,firstParam.getClass().getComponentType()); + Object[] firstParamArray = (Object[])firstParam; + assertEquals(3,firstParamArray.length); + assertEquals("a",firstParamArray[0]); + assertEquals("b",firstParamArray[1]); + assertEquals("c",firstParamArray[2]); + } + + // test classes static class Super { } @@ -143,6 +262,8 @@ public class HelperTests extends ExpressionTestCase { static class Sub extends Super { } + static class Unconvertable {} + // --- /** @@ -173,4 +294,44 @@ public class HelperTests extends ExpressionTestCase { } } } + + /** + * Used to validate the match returned from a compareArguments call. + */ + private void checkMatch2(Class[] inputTypes, Class[] expectedTypes, StandardTypeConverter typeConverter,ArgsMatchKind expectedMatchKind,int... argsForConversion) { + ReflectionHelper.ArgumentsMatchInfo matchInfo = ReflectionHelper.compareArgumentsVarargs(expectedTypes, inputTypes, typeConverter); + if (expectedMatchKind==null) { + assertNull("Did not expect them to match in any way: "+matchInfo, matchInfo); + } else { + assertNotNull("Should not be a null match", matchInfo); + } + + if (expectedMatchKind==ArgsMatchKind.EXACT) { + assertTrue(matchInfo.isExactMatch()); + assertNull(matchInfo.argsRequiringConversion); + } else if (expectedMatchKind==ArgsMatchKind.CLOSE) { + assertTrue(matchInfo.isCloseMatch()); + assertNull(matchInfo.argsRequiringConversion); + } else if (expectedMatchKind==ArgsMatchKind.REQUIRES_CONVERSION) { + assertTrue("expected to be a match requiring conversion, but was "+matchInfo,matchInfo.isMatchRequiringConversion()); + if (argsForConversion==null) { + fail("there are arguments that need conversion"); + } + assertEquals("The array of args that need conversion is different length to that expected",argsForConversion.length, matchInfo.argsRequiringConversion.length); + for (int a=0;a