diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index 17c0e5064a..2a8d807993 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -40,13 +40,14 @@ import org.springframework.util.MethodInvoker; public class ReflectionHelper { /** - * Compare argument arrays and return information about whether they match. A supplied - * type converter and conversionAllowed flag allow for matches to take into account - * that a type may be transformed into a different type by the converter. - * @param expectedArgTypes the array of types the method/constructor is expecting - * @param suppliedArgTypes the array of types that are being supplied at the point of invocation + * Compare argument arrays and return information about whether they match. + * A supplied type converter and conversionAllowed flag allow for matches to take + * into account that a type may be transformed into a different type by the converter. + * @param expectedArgTypes the types the method/constructor is expecting + * @param suppliedArgTypes the types that are being supplied at the point of invocation * @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 + * @return a MatchInfo object indicating what kind of match it was, + * or {@code null} if it was not a match */ static ArgumentsMatchInfo compareArguments( List expectedArgTypes, List suppliedArgTypes, TypeConverter typeConverter) { @@ -90,7 +91,7 @@ public class ReflectionHelper { int result = 0; for (int i = 0; i < paramTypes.size(); i++) { TypeDescriptor paramType = paramTypes.get(i); - TypeDescriptor argType = argTypes.get(i); + TypeDescriptor argType = (i < argTypes.size() ? argTypes.get(i) : null); if (argType == null) { if (paramType.isPrimitive()) { return Integer.MAX_VALUE; @@ -127,13 +128,15 @@ public class ReflectionHelper { } /** - * Compare argument arrays and return information about whether they match. A supplied type converter and - * conversionAllowed flag allow for matches to take into account that a type may be transformed into a different - * type by the converter. This variant of compareArguments also allows for a varargs match. - * @param expectedArgTypes the array of types the method/constructor is expecting - * @param suppliedArgTypes the array of types that are being supplied at the point of invocation + * Compare argument arrays and return information about whether they match. + * A supplied type converter and conversionAllowed flag allow for matches to + * take into account that a type may be transformed into a different type by the + * converter. This variant of compareArguments also allows for a varargs match. + * @param expectedArgTypes the types the method/constructor is expecting + * @param suppliedArgTypes the types that are being supplied at the point of invocation * @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 + * @return a MatchInfo object indicating what kind of match it was, + * or {@code null} if it was not a match */ static ArgumentsMatchInfo compareArgumentsVarargs( List expectedArgTypes, List suppliedArgTypes, TypeConverter typeConverter) { @@ -246,12 +249,14 @@ public class ReflectionHelper { * @param converter the type converter to use for attempting conversions * @param arguments the actual arguments that need conversion * @param methodOrCtor the target Method or Constructor - * @param varargsPosition the known position of the varargs argument, if any (null if not varargs) - * @return true if some kind of conversion occurred on an argument + * @param varargsPosition the known position of the varargs argument, if any + * ({@code null} if not varargs) + * @return {@code true} if some kind of conversion occurred on an argument * @throws EvaluationException if a problem occurs during conversion */ static boolean convertArguments(TypeConverter converter, Object[] arguments, Object methodOrCtor, Integer varargsPosition) throws EvaluationException { + boolean conversionOccurred = false; if (varargsPosition == null) { for (int i = 0; i < arguments.length; i++) { @@ -320,9 +325,9 @@ public class ReflectionHelper { /** * Package up the arguments so that they correctly match what is expected in parameterTypes. - * For example, if 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. + * For example, if parameterTypes is {@code (int, String[])} because the second parameter + * was declared {@code String...}, then if arguments is {@code [1,"a","b"]} then it must be + * repackaged as {@code [1,new String[]{"a","b"}]} in order to match the expected types. * @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 @@ -374,10 +379,11 @@ public class ReflectionHelper { /** - * An instance of ArgumentsMatchInfo describes what kind of match was achieved between two sets of arguments - - * the set that a method/constructor is expecting and the set that are being supplied at the point of invocation. - * If the kind indicates that conversion is required for some of the arguments then the arguments that require - * conversion are listed in the argsRequiringConversion array. + * An instance of ArgumentsMatchInfo describes what kind of match was achieved + * between two sets of arguments - the set that a method/constructor is expecting + * and the set that are being supplied at the point of invocation. If the kind + * indicates that conversion is required for some of the arguments then the arguments + * that require conversion are listed in the argsRequiringConversion array. */ static class ArgumentsMatchInfo { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java index cb6f5301df..b78f66e9de 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveMethodResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -62,17 +62,18 @@ public class ReflectiveMethodResolver implements MethodResolver { public ReflectiveMethodResolver() { - this.useDistance = false; + this.useDistance = true; } /** - * This constructors allows the ReflectiveMethodResolver to be configured such that it will - * use a distance computation to check which is the better of two close matches (when there - * are multiple matches). Using the distance computation is intended to ensure matches - * are more closely representative of what a Java compiler would do when taking into - * account boxing/unboxing and whether the method candidates are declared to handle a - * supertype of the type (of the argument) being passed in. - * @param useDistance true if distance computation should be used when calculating matches + * This constructor allows the ReflectiveMethodResolver to be configured such that it + * will use a distance computation to check which is the better of two close matches + * (when there are multiple matches). Using the distance computation is intended to + * ensure matches are more closely representative of what a Java compiler would do + * when taking into account boxing/unboxing and whether the method candidates are + * declared to handle a supertype of the type (of the argument) being passed in. + * @param useDistance {@code true} if distance computation should be used when + * calculating matches; {@code false} otherwise */ public ReflectiveMethodResolver(boolean useDistance) { this.useDistance = useDistance; @@ -175,17 +176,17 @@ public class ReflectiveMethodResolver implements MethodResolver { return new ReflectiveMethodExecutor(method); } else if (matchInfo.isCloseMatch()) { - if (!this.useDistance) { - // Take this as a close match if there isn't one already - if (closeMatch == null) { + if (this.useDistance) { + int matchDistance = ReflectionHelper.getTypeDifferenceWeight(paramDescriptors, argumentTypes); + if (closeMatch == null || matchDistance < closeMatchDistance) { + // This is a better match... closeMatch = method; + closeMatchDistance = matchDistance; } } else { - int matchDistance = ReflectionHelper.getTypeDifferenceWeight(paramDescriptors, argumentTypes); - if (matchDistance < closeMatchDistance) { - // This is a better match... - closeMatchDistance = matchDistance; + // Take this as a close match if there isn't one already + if (closeMatch == null) { closeMatch = method; } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java index 23e82ee240..0ff6458b54 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/AbstractExpressionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -35,196 +35,146 @@ import static org.junit.Assert.*; */ public abstract class AbstractExpressionTests { - private final static boolean DEBUG = false; + private static final boolean DEBUG = false; + + protected static final boolean SHOULD_BE_WRITABLE = true; + + protected static final boolean SHOULD_NOT_BE_WRITABLE = false; - protected final static boolean SHOULD_BE_WRITABLE = true; - protected final static boolean SHOULD_NOT_BE_WRITABLE = false; protected final ExpressionParser parser = new SpelExpressionParser(); + protected final StandardEvaluationContext eContext = TestScenarioCreator.getTestEvaluationContext(); /** - * Evaluate an expression and check that the actual result matches the expectedValue and the class of the result - * matches the expectedClassOfResult. - * @param expression The expression to evaluate + * Evaluate an expression and check that the actual result matches the + * expectedValue and the class of the result matches the expectedClassOfResult. + * @param expression the expression to evaluate * @param expectedValue the expected result for evaluating the expression * @param expectedResultType the expected class of the evaluation result */ - protected void evaluate(String expression, Object expectedValue, Class expectedResultType) { - try { - Expression expr = parser.parseExpression(expression); - if (expr == null) { - fail("Parser returned null for expression"); + public void evaluate(String expression, Object expectedValue, Class expectedResultType) { + Expression expr = parser.parseExpression(expression); + if (expr == null) { + fail("Parser returned null for expression"); + } + if (DEBUG) { + SpelUtilities.printAbstractSyntaxTree(System.out, expr); + } + + Object value = expr.getValue(eContext); + + // Check the return value + if (value == null) { + if (expectedValue == null) { + return; // no point doing other checks } - if (DEBUG) { - SpelUtilities.printAbstractSyntaxTree(System.out, expr); - } - // Class expressionType = expr.getValueType(); - // assertEquals("Type of the expression is not as expected. Should be '" + - // expectedResultType + "' but is '" - // + expressionType + "'", expectedResultType, expressionType); + assertEquals("Expression returned null value, but expected '" + expectedValue + "'", expectedValue, null); + } - Object value = expr.getValue(eContext); + Class resultType = value.getClass(); + assertEquals("Type of the actual result was not as expected. Expected '" + expectedResultType + + "' but result was of type '" + resultType + "'", expectedResultType, resultType); - // Check the return value - if (value == null) { - if (expectedValue == null) { - return; // no point doing other checks - } - assertEquals("Expression returned null value, but expected '" + expectedValue + "'", expectedValue, - null); - } - - Class resultType = value.getClass(); - assertEquals("Type of the actual result was not as expected. Expected '" + expectedResultType - + "' but result was of type '" + resultType + "'", expectedResultType, resultType); - // .equals/* isAssignableFrom */(resultType), truers); - - // TODO isAssignableFrom would allow some room for compatibility - // in the above expression... - - if (expectedValue instanceof String) { - assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, + if (expectedValue instanceof String) { + assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, AbstractExpressionTests.stringValueOf(value)); - } - else { - assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, value); - } } - catch (EvaluationException ee) { - ee.printStackTrace(); - fail("Unexpected Exception: " + ee.getMessage()); - } - catch (ParseException pe) { - pe.printStackTrace(); - fail("Unexpected Exception: " + pe.getMessage()); + else { + assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, value); } } - protected void evaluateAndAskForReturnType(String expression, Object expectedValue, Class expectedResultType) { - try { - Expression expr = parser.parseExpression(expression); - if (expr == null) { - fail("Parser returned null for expression"); - } - if (DEBUG) { - SpelUtilities.printAbstractSyntaxTree(System.out, expr); - } - // Class expressionType = expr.getValueType(); - // assertEquals("Type of the expression is not as expected. Should be '" + - // expectedResultType + "' but is '" - // + expressionType + "'", expectedResultType, expressionType); + public void evaluateAndAskForReturnType(String expression, Object expectedValue, Class expectedResultType) { + Expression expr = parser.parseExpression(expression); + if (expr == null) { + fail("Parser returned null for expression"); + } + if (DEBUG) { + SpelUtilities.printAbstractSyntaxTree(System.out, expr); + } - Object value = expr.getValue(eContext, expectedResultType); - if (value == null) { - if (expectedValue == null) - return; // no point doing other checks - assertEquals("Expression returned null value, but expected '" + expectedValue + "'", expectedValue, - null); + Object value = expr.getValue(eContext, expectedResultType); + if (value == null) { + if (expectedValue == null) { + return; // no point doing other checks } + assertEquals("Expression returned null value, but expected '" + expectedValue + "'", expectedValue, null); + } - Class resultType = value.getClass(); - assertEquals("Type of the actual result was not as expected. Expected '" + expectedResultType - + "' but result was of type '" + resultType + "'", expectedResultType, resultType); - // .equals/* isAssignableFrom */(resultType), truers); - assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, value); - // isAssignableFrom would allow some room for compatibility - // in the above expression... - } - catch (EvaluationException ee) { - SpelEvaluationException ex = (SpelEvaluationException) ee; - ex.printStackTrace(); - fail("Unexpected EvaluationException: " + ex.getMessage()); - } - catch (ParseException pe) { - fail("Unexpected ParseException: " + pe.getMessage()); - } + Class resultType = value.getClass(); + assertEquals("Type of the actual result was not as expected. Expected '" + expectedResultType + + "' but result was of type '" + resultType + "'", expectedResultType, resultType); + assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, value); } /** - * Evaluate an expression and check that the actual result matches the expectedValue and the class of the result - * matches the expectedClassOfResult. This method can also check if the expression is writable (for example, it is a - * variable or property reference). - * - * @param expression The expression to evaluate + * Evaluate an expression and check that the actual result matches the + * expectedValue and the class of the result matches the expectedClassOfResult. + * This method can also check if the expression is writable (for example, + * it is a variable or property reference). + * @param expression the expression to evaluate * @param expectedValue the expected result for evaluating the expression * @param expectedClassOfResult the expected class of the evaluation result * @param shouldBeWritable should the parsed expression be writable? */ - protected void evaluate(String expression, Object expectedValue, Class expectedClassOfResult, - boolean shouldBeWritable) { - try { - Expression e = parser.parseExpression(expression); - if (e == null) { - fail("Parser returned null for expression"); + public void evaluate(String expression, Object expectedValue, Class expectedClassOfResult, boolean shouldBeWritable) { + Expression expr = parser.parseExpression(expression); + if (expr == null) { + fail("Parser returned null for expression"); + } + if (DEBUG) { + SpelUtilities.printAbstractSyntaxTree(System.out, expr); + } + Object value = expr.getValue(eContext); + if (value == null) { + if (expectedValue == null) { + return; // no point doing other checks } - if (DEBUG) { - SpelUtilities.printAbstractSyntaxTree(System.out, e); - } - Object value = e.getValue(eContext); - if (value == null) { - if (expectedValue == null) - return; // no point doing other - // checks - assertEquals("Expression returned null value, but expected '" + expectedValue + "'", expectedValue, - null); - } - Class resultType = value.getClass(); - if (expectedValue instanceof String) { - assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, + assertEquals("Expression returned null value, but expected '" + expectedValue + "'", expectedValue, null); + } + Class resultType = value.getClass(); + if (expectedValue instanceof String) { + assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, AbstractExpressionTests.stringValueOf(value)); - } - else { - assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, value); - } - // assertEquals("Did not get expected value for expression '" + expression + - // "'.", expectedValue, stringValueOf(value)); - assertEquals("Type of the result was not as expected. Expected '" + expectedClassOfResult - + "' but result was of type '" + resultType + "'", - expectedClassOfResult.equals/* isAssignableFrom */(resultType), true); - // TODO isAssignableFrom would allow some room for compatibility - // in the above expression... + } + else { + assertEquals("Did not get expected value for expression '" + expression + "'.", expectedValue, value); + } + assertEquals("Type of the result was not as expected. Expected '" + expectedClassOfResult + + "' but result was of type '" + resultType + "'", expectedClassOfResult.equals(resultType), true); - boolean isWritable = e.isWritable(eContext); - if (isWritable != shouldBeWritable) { - if (shouldBeWritable) - fail("Expected the expression to be writable but it is not"); - else - fail("Expected the expression to be readonly but it is not"); - } - } - catch (EvaluationException ee) { - ee.printStackTrace(); - fail("Unexpected Exception: " + ee.getMessage()); - } - catch (ParseException pe) { - pe.printStackTrace(); - fail("Unexpected Exception: " + pe.getMessage()); + boolean isWritable = expr.isWritable(eContext); + if (isWritable != shouldBeWritable) { + if (shouldBeWritable) + fail("Expected the expression to be writable but it is not"); + else + fail("Expected the expression to be readonly but it is not"); } } /** - * Evaluate the specified expression and ensure the expected message comes out. The message may have inserts and - * they will be checked if otherProperties is specified. The first entry in otherProperties should always be the - * position. - * @param expression The expression to evaluate - * @param expectedMessage The expected message - * @param otherProperties The expected inserts within the message + * Evaluate the specified expression and ensure the expected message comes out. + * The message may have inserts and they will be checked if otherProperties is specified. + * The first entry in otherProperties should always be the position. + * @param expression the expression to evaluate + * @param expectedMessage the expected message + * @param otherProperties the expected inserts within the message */ protected void evaluateAndCheckError(String expression, SpelMessage expectedMessage, Object... otherProperties) { evaluateAndCheckError(expression, null, expectedMessage, otherProperties); } /** - * Evaluate the specified expression and ensure the expected message comes out. The message may have inserts and - * they will be checked if otherProperties is specified. The first entry in otherProperties should always be the - * position. - * @param expression The expression to evaluate - * @param expectedReturnType Ask the expression return value to be of this type if possible (null indicates don't - * ask for conversion) - * @param expectedMessage The expected message - * @param otherProperties The expected inserts within the message + * Evaluate the specified expression and ensure the expected message comes out. + * The message may have inserts and they will be checked if otherProperties is specified. + * The first entry in otherProperties should always be the position. + * @param expression the expression to evaluate + * @param expectedReturnType ask the expression return value to be of this type if possible + * ({@code null} indicates don't ask for conversion) + * @param expectedMessage the expected message + * @param otherProperties the expected inserts within the message */ protected void evaluateAndCheckError(String expression, Class expectedReturnType, SpelMessage expectedMessage, Object... otherProperties) { @@ -234,19 +184,16 @@ public abstract class AbstractExpressionTests { fail("Parser returned null for expression"); } if (expectedReturnType != null) { - @SuppressWarnings("unused") - Object value = expr.getValue(eContext, expectedReturnType); + expr.getValue(eContext, expectedReturnType); } else { - @SuppressWarnings("unused") - Object value = expr.getValue(eContext); + expr.getValue(eContext); } fail("Should have failed with message " + expectedMessage); } catch (EvaluationException ee) { SpelEvaluationException ex = (SpelEvaluationException) ee; if (ex.getMessageCode() != expectedMessage) { - ex.printStackTrace(); assertEquals("Failed to get expected message", expectedMessage, ex.getMessageCode()); } if (otherProperties != null && otherProperties.length != 0) { @@ -260,47 +207,39 @@ public abstract class AbstractExpressionTests { inserts = new Object[0]; } if (inserts.length < otherProperties.length - 1) { - ex.printStackTrace(); - fail("Cannot check " + (otherProperties.length - 1) - + " properties of the exception, it only has " + inserts.length + " inserts"); + fail("Cannot check " + (otherProperties.length - 1) + + " properties of the exception, it only has " + inserts.length + " inserts"); } for (int i = 1; i < otherProperties.length; i++) { if (otherProperties[i] == null) { if (inserts[i - 1] != null) { - ex.printStackTrace(); - fail("Insert does not match, expected 'null' but insert value was '" + inserts[i - 1] - + "'"); + fail("Insert does not match, expected 'null' but insert value was '" + + inserts[i - 1] + "'"); } } else if (inserts[i - 1] == null) { if (otherProperties[i] != null) { - ex.printStackTrace(); - fail("Insert does not match, expected '" + otherProperties[i] - + "' but insert value was 'null'"); + fail("Insert does not match, expected '" + otherProperties[i] + + "' but insert value was 'null'"); } } else if (!inserts[i - 1].equals(otherProperties[i])) { - ex.printStackTrace(); - fail("Insert does not match, expected '" + otherProperties[i] + "' but insert value was '" - + inserts[i - 1] + "'"); + fail("Insert does not match, expected '" + otherProperties[i] + + "' but insert value was '" + inserts[i - 1] + "'"); } } } } } - catch (ParseException pe) { - pe.printStackTrace(); - fail("Unexpected Exception: " + pe.getMessage()); - } } /** - * Parse the specified expression and ensure the expected message comes out. The message may have inserts and they - * will be checked if otherProperties is specified. The first entry in otherProperties should always be the - * position. - * @param expression The expression to evaluate - * @param expectedMessage The expected message - * @param otherProperties The expected inserts within the message + * Parse the specified expression and ensure the expected message comes out. + * The message may have inserts and they will be checked if otherProperties is specified. + * The first entry in otherProperties should always be the position. + * @param expression the expression to evaluate + * @param expectedMessage the expected message + * @param otherProperties the expected inserts within the message */ protected void parseAndCheckError(String expression, SpelMessage expectedMessage, Object... otherProperties) { try { @@ -309,21 +248,8 @@ public abstract class AbstractExpressionTests { fail("Parsing should have failed!"); } catch (ParseException pe) { - // pe.printStackTrace(); - // Throwable t = pe.getCause(); - // if (t == null) { - // fail("ParseException caught with no defined cause"); - // } - // if (!(t instanceof SpelEvaluationException)) { - // t.printStackTrace(); - // fail("Cause of parse exception is not a SpelException"); - // } - // SpelEvaluationException ex = (SpelEvaluationException) t; - // pe.printStackTrace(); - SpelParseException ex = (SpelParseException) pe; + SpelParseException ex = (SpelParseException)pe; if (ex.getMessageCode() != expectedMessage) { - // System.out.println(ex.getMessage()); - ex.printStackTrace(); assertEquals("Failed to get expected message", expectedMessage, ex.getMessageCode()); } if (otherProperties != null && otherProperties.length != 0) { @@ -337,15 +263,13 @@ public abstract class AbstractExpressionTests { inserts = new Object[0]; } if (inserts.length < otherProperties.length - 1) { - ex.printStackTrace(); - fail("Cannot check " + (otherProperties.length - 1) - + " properties of the exception, it only has " + inserts.length + " inserts"); + fail("Cannot check " + (otherProperties.length - 1) + + " properties of the exception, it only has " + inserts.length + " inserts"); } for (int i = 1; i < otherProperties.length; i++) { if (!inserts[i - 1].equals(otherProperties[i])) { - ex.printStackTrace(); - fail("Insert does not match, expected '" + otherProperties[i] + "' but insert value was '" - + inserts[i - 1] + "'"); + fail("Insert does not match, expected '" + otherProperties[i] + + "' but insert value was '" + inserts[i - 1] + "'"); } } } @@ -353,13 +277,13 @@ public abstract class AbstractExpressionTests { } } + protected static String stringValueOf(Object value) { return stringValueOf(value, false); } /** * Produce a nice string representation of the input object. - * * @param value object to be formatted * @return a nice string */ @@ -395,8 +319,8 @@ public abstract class AbstractExpressionTests { sb.append("}"); } else { - throw new RuntimeException("Please implement support for type " + primitiveType.getName() - + " in ExpressionTestCase.stringValueOf()"); + throw new RuntimeException("Please implement support for type " + primitiveType.getName() + + " in ExpressionTestCase.stringValueOf()"); } } else if (value.getClass().getComponentType().isArray()) { diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index 4487ee8eb7..a09169f3e9 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -54,17 +54,6 @@ public class MethodInvocationTests extends AbstractExpressionTests { evaluate("getPlaceOfBirth().getCity()", "SmilJan", String.class); } - // public void testBuiltInProcessors() { - // evaluate("new int[]{1,2,3,4}.count()", 4, Integer.class); - // evaluate("new int[]{4,3,2,1}.sort()[3]", 4, Integer.class); - // evaluate("new int[]{4,3,2,1}.average()", 2, Integer.class); - // evaluate("new int[]{4,3,2,1}.max()", 4, Integer.class); - // evaluate("new int[]{4,3,2,1}.min()", 1, Integer.class); - // evaluate("new int[]{4,3,2,1,2,3}.distinct().count()", 4, Integer.class); - // evaluate("{1,2,3,null}.nonnull().count()", 3, Integer.class); - // evaluate("new int[]{4,3,2,1,2,3}.distinct().count()", 4, Integer.class); - // } - @Test public void testStringClass() { evaluate("new java.lang.String('hello').charAt(2)", 'l', Character.class); @@ -107,7 +96,7 @@ public class MethodInvocationTests extends AbstractExpressionTests { // Normal exit StandardEvaluationContext eContext = TestScenarioCreator.getTestEvaluationContext(); - eContext.setVariable("bar",3); + eContext.setVariable("bar", 3); Object o = expr.getValue(eContext); assertEquals(o, 3); assertEquals(1, parser.parseExpression("counter").getValue(eContext)); @@ -115,46 +104,44 @@ public class MethodInvocationTests extends AbstractExpressionTests { // Now the expression has cached that throwException(int) is the right thing to call // Let's change 'bar' to be a PlaceOfBirth which indicates the cached reference is // out of date. - eContext.setVariable("bar",new PlaceOfBirth("London")); + eContext.setVariable("bar", new PlaceOfBirth("London")); o = expr.getValue(eContext); assertEquals("London", o); // That confirms the logic to mark the cached reference stale and retry is working - - // Now let's cause the method to exit via exception and ensure it doesn't cause - // a retry. + // Now let's cause the method to exit via exception and ensure it doesn't cause a retry. // First, switch back to throwException(int) - eContext.setVariable("bar",3); + eContext.setVariable("bar", 3); o = expr.getValue(eContext); assertEquals(3, o); assertEquals(2, parser.parseExpression("counter").getValue(eContext)); // Now cause it to throw an exception: - eContext.setVariable("bar",1); + eContext.setVariable("bar", 1); try { o = expr.getValue(eContext); fail(); - } catch (Exception e) { - if (e instanceof SpelEvaluationException) { - e.printStackTrace(); - fail("Should not be a SpelEvaluationException"); + } + catch (Exception ex) { + if (ex instanceof SpelEvaluationException) { + fail("Should not be a SpelEvaluationException: " + ex); } // normal } // If counter is 4 then the method got called twice! assertEquals(3, parser.parseExpression("counter").getValue(eContext)); - eContext.setVariable("bar",4); + eContext.setVariable("bar", 4); try { o = expr.getValue(eContext); fail(); - } catch (Exception e) { + } + catch (Exception ex) { // 4 means it will throw a checked exception - this will be wrapped - if (!(e instanceof ExpressionInvocationTargetException)) { - e.printStackTrace(); - fail("Should have been wrapped"); + if (!(ex instanceof ExpressionInvocationTargetException)) { + fail("Should have been wrapped: " + ex); } // normal } @@ -176,14 +163,14 @@ public class MethodInvocationTests extends AbstractExpressionTests { SpelExpressionParser parser = new SpelExpressionParser(); Expression expr = parser.parseExpression("throwException(#bar)"); - eContext.setVariable("bar",2); + eContext.setVariable("bar", 2); try { expr.getValue(eContext); fail(); - } catch (Exception e) { - if (e instanceof SpelEvaluationException) { - e.printStackTrace(); - fail("Should not be a SpelEvaluationException"); + } + catch (Exception ex) { + if (ex instanceof SpelEvaluationException) { + fail("Should not be a SpelEvaluationException: " + ex); } // normal } @@ -200,18 +187,16 @@ public class MethodInvocationTests extends AbstractExpressionTests { SpelExpressionParser parser = new SpelExpressionParser(); Expression expr = parser.parseExpression("throwException(#bar)"); - eContext.setVariable("bar",4); + eContext.setVariable("bar", 4); try { expr.getValue(eContext); fail(); - } catch (ExpressionInvocationTargetException e) { - Throwable t = e.getCause(); - assertEquals( - "org.springframework.expression.spel.testresources.Inventor$TestException", - t.getClass().getName()); - return; } - fail("Should not be a SpelEvaluationException"); + catch (ExpressionInvocationTargetException ex) { + Throwable cause = ex.getCause(); + assertEquals("org.springframework.expression.spel.testresources.Inventor$TestException", + cause.getClass().getName()); + } } @Test @@ -224,7 +209,7 @@ public class MethodInvocationTests extends AbstractExpressionTests { // Filter will be called but not do anything, so first doit() will be invoked SpelExpression expr = (SpelExpression) parser.parseExpression("doit(1)"); - String result = expr.getValue(context,String.class); + String result = expr.getValue(context, String.class); assertEquals("1", result); assertTrue(filter.filterCalled); @@ -232,15 +217,15 @@ public class MethodInvocationTests extends AbstractExpressionTests { filter.removeIfNotAnnotated = true; filter.filterCalled = false; expr = (SpelExpression) parser.parseExpression("doit(1)"); - result = expr.getValue(context,String.class); + result = expr.getValue(context, String.class); assertEquals("double 1.0", result); assertTrue(filter.filterCalled); // check not called for other types - filter.filterCalled=false; + filter.filterCalled = false; context.setRootObject(new String("abc")); expr = (SpelExpression) parser.parseExpression("charAt(0)"); - result = expr.getValue(context,String.class); + result = expr.getValue(context, String.class); assertEquals("a", result); assertFalse(filter.filterCalled); @@ -249,64 +234,11 @@ public class MethodInvocationTests extends AbstractExpressionTests { context.registerMethodFilter(TestObject.class,null);//clear filter context.setRootObject(new TestObject()); expr = (SpelExpression) parser.parseExpression("doit(1)"); - result = expr.getValue(context,String.class); + result = expr.getValue(context, String.class); assertEquals("1", result); assertFalse(filter.filterCalled); } - // Simple filter - static class LocalFilter implements MethodFilter { - - public boolean removeIfNotAnnotated = false; - - public boolean filterCalled = false; - - private boolean isAnnotated(Method m) { - Annotation[] annos = m.getAnnotations(); - if (annos==null) { - return false; - } - for (Annotation anno: annos) { - String s = anno.annotationType().getName(); - if (s.endsWith("Anno")) { - return true; - } - } - return false; - } - - @Override - public List filter(List methods) { - filterCalled = true; - List forRemoval = new ArrayList(); - for (Method m: methods) { - if (removeIfNotAnnotated && !isAnnotated(m)) { - forRemoval.add(m); - } - } - for (Method m: forRemoval) { - methods.remove(m); - } - return methods; - } - - } - - @Retention(RetentionPolicy.RUNTIME) - @interface Anno {} - - class TestObject { - public int doit(int i) { - return i; - } - - @Anno - public String doit(double d) { - return "double "+d; - } - - } - @Test public void testAddingMethodResolvers() { StandardEvaluationContext ctx = new StandardEvaluationContext(); @@ -329,17 +261,6 @@ public class MethodInvocationTests extends AbstractExpressionTests { assertEquals(2, ctx.getMethodResolvers().size()); } - static class DummyMethodResolver implements MethodResolver { - - @Override - public MethodExecutor resolve(EvaluationContext context, Object targetObject, String name, - List argumentTypes) throws AccessException { - throw new UnsupportedOperationException("Auto-generated method stub"); - } - - } - - @Test public void testVarargsInvocation01() { // Calling 'public int aVarargsMethod(String... strings)' @@ -383,8 +304,7 @@ public class MethodInvocationTests extends AbstractExpressionTests { StandardEvaluationContext context = new StandardEvaluationContext(bytes); context.setBeanResolver(new BeanResolver() { @Override - public Object resolve(EvaluationContext context, String beanName) - throws AccessException { + public Object resolve(EvaluationContext context, String beanName) throws AccessException { if ("service".equals(beanName)) { return service; } @@ -396,10 +316,78 @@ public class MethodInvocationTests extends AbstractExpressionTests { assertSame(bytes, outBytes); } + + // Simple filter + static class LocalFilter implements MethodFilter { + + public boolean removeIfNotAnnotated = false; + + public boolean filterCalled = false; + + private boolean isAnnotated(Method method) { + Annotation[] anns = method.getAnnotations(); + if (anns == null) { + return false; + } + for (Annotation ann : anns) { + String name = ann.annotationType().getName(); + if (name.endsWith("Anno")) { + return true; + } + } + return false; + } + + @Override + public List filter(List methods) { + filterCalled = true; + List forRemoval = new ArrayList(); + for (Method method: methods) { + if (removeIfNotAnnotated && !isAnnotated(method)) { + forRemoval.add(method); + } + } + for (Method method: forRemoval) { + methods.remove(method); + } + return methods; + } + } + + + @Retention(RetentionPolicy.RUNTIME) + @interface Anno { + } + + + class TestObject { + + public int doit(int i) { + return i; + } + + @Anno + public String doit(double d) { + return "double "+d; + } + } + + + static class DummyMethodResolver implements MethodResolver { + + @Override + public MethodExecutor resolve(EvaluationContext context, Object targetObject, String name, + List argumentTypes) throws AccessException { + throw new UnsupportedOperationException(); + } + } + + public static class BytesService { public byte[] handleBytes(byte[] bytes) { return bytes; } } + } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java index c8b2b62cf5..3fc0db8dc1 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -1895,6 +1895,15 @@ public class SpelReproTests extends AbstractExpressionTests { assertTrue(((List) value).isEmpty()); } + @Test + public void SPR12803() throws Exception { + StandardEvaluationContext sec = new StandardEvaluationContext(); + sec.setVariable("iterable", Collections.emptyList()); + SpelExpressionParser parser = new SpelExpressionParser(); + Expression expression = parser.parseExpression("T(org.springframework.expression.spel.SpelReproTests.GuavaLists).newArrayList(#iterable)"); + assertTrue(expression.getValue(sec) instanceof ArrayList); + } + private static enum ABC { A, B, C } @@ -2180,4 +2189,16 @@ public class SpelReproTests extends AbstractExpressionTests { } } + + public static class GuavaLists { + + public static List newArrayList(Iterable iterable) { + return new ArrayList(); + } + + public static List newArrayList(Object... elements) { + throw new UnsupportedOperationException(); + } + } + } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java index 05327188c0..caea1aa457 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2015 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. @@ -22,9 +22,9 @@ import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.testresources.Inventor; import org.springframework.expression.spel.testresources.PlaceOfBirth; -///CLOVER:OFF /** - * Builds an evaluation context for test expressions. Features of the test evaluation context are: + * Builds an evaluation context for test expressions. + * Features of the test evaluation context are: *
    *
  • The root context object is an Inventor instance {@link Inventor} *
@@ -45,21 +45,19 @@ public class TestScenarioCreator { */ private static void populateFunctions(StandardEvaluationContext testContext) { try { - testContext.registerFunction("isEven", TestScenarioCreator.class.getDeclaredMethod("isEven", - new Class[] { Integer.TYPE })); - testContext.registerFunction("reverseInt", TestScenarioCreator.class.getDeclaredMethod("reverseInt", - new Class[] { Integer.TYPE, Integer.TYPE, Integer.TYPE })); - testContext.registerFunction("reverseString", TestScenarioCreator.class.getDeclaredMethod("reverseString", - new Class[] { String.class })); - testContext.registerFunction("varargsFunctionReverseStringsAndMerge", TestScenarioCreator.class - .getDeclaredMethod("varargsFunctionReverseStringsAndMerge", new Class[] { String[].class })); - testContext.registerFunction("varargsFunctionReverseStringsAndMerge2", TestScenarioCreator.class - .getDeclaredMethod("varargsFunctionReverseStringsAndMerge2", new Class[] { Integer.TYPE, - String[].class })); - } catch (SecurityException e) { - e.printStackTrace(); - } catch (NoSuchMethodException e) { - e.printStackTrace(); + testContext.registerFunction("isEven", + TestScenarioCreator.class.getDeclaredMethod("isEven", Integer.TYPE)); + testContext.registerFunction("reverseInt", + TestScenarioCreator.class.getDeclaredMethod("reverseInt", Integer.TYPE, Integer.TYPE, Integer.TYPE)); + testContext.registerFunction("reverseString", + TestScenarioCreator.class.getDeclaredMethod("reverseString", String.class)); + testContext.registerFunction("varargsFunctionReverseStringsAndMerge", + TestScenarioCreator.class.getDeclaredMethod("varargsFunctionReverseStringsAndMerge", String[].class)); + testContext.registerFunction("varargsFunctionReverseStringsAndMerge2", + TestScenarioCreator.class.getDeclaredMethod("varargsFunctionReverseStringsAndMerge2", Integer.TYPE, String[].class)); + } + catch (Exception ex) { + throw new IllegalStateException(ex); } } @@ -72,9 +70,8 @@ public class TestScenarioCreator { } /** - * Create the root context object, an Inventor instance. Non-qualified property and method references will be - * resolved against this context object. - * + * Create the root context object, an Inventor instance. Non-qualified property + * and method references will be resolved against this context object. * @param testContext the evaluation context in which to set the root object */ private static void setupRootContextObject(StandardEvaluationContext testContext) { @@ -88,12 +85,14 @@ public class TestScenarioCreator { testContext.setRootObject(tesla); } + // These methods are registered in the test context and therefore accessible through function calls // in test expressions public static String isEven(int i) { - if ((i % 2) == 0) + if ((i % 2) == 0) { return "y"; + } return "n"; } @@ -129,4 +128,5 @@ public class TestScenarioCreator { } return sb.toString(); } + }