diff --git a/spring-expression/src/main/java/org/springframework/expression/common/ExpressionUtils.java b/spring-expression/src/main/java/org/springframework/expression/common/ExpressionUtils.java index b2f00b30cd..cc959c799e 100644 --- a/spring-expression/src/main/java/org/springframework/expression/common/ExpressionUtils.java +++ b/spring-expression/src/main/java/org/springframework/expression/common/ExpressionUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2010 the original author or authors. + * Copyright 2002-2012 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. @@ -60,7 +60,7 @@ public abstract class ExpressionUtils { @SuppressWarnings("unchecked") public static T convertTypedValue(EvaluationContext context, TypedValue typedValue, Class targetType) { Object value = typedValue.getValue(); - if (targetType == null || ClassUtils.isAssignableValue(targetType, value)) { + if ((targetType == null) || (value != null && ClassUtils.isAssignableValue(targetType, value))) { return (T) value; } if (context != null) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java index 4c33d9b9ca..b2140d11e0 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpAnd.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -16,7 +16,6 @@ package org.springframework.expression.spel.ast; -import org.springframework.core.convert.TypeDescriptor; import org.springframework.expression.EvaluationException; import org.springframework.expression.TypedValue; import org.springframework.expression.spel.ExpressionState; @@ -29,6 +28,7 @@ import org.springframework.expression.spel.support.BooleanTypedValue; * * @author Andy Clement * @author Mark Fisher + * @author Oliver Becker * @since 3.0 */ public class OpAnd extends Operator { @@ -39,38 +39,27 @@ public class OpAnd extends Operator { @Override public TypedValue getValueInternal(ExpressionState state) throws EvaluationException { - boolean leftValue; - boolean rightValue; - - try { - TypedValue typedValue = getLeftOperand().getValueInternal(state); - this.assertTypedValueNotNull(typedValue); - leftValue = (Boolean)state.convertValue(typedValue, TypeDescriptor.valueOf(Boolean.class)); + if (getBooleanValue(state, getLeftOperand()) == false) { + // no need to evaluate right operand + return BooleanTypedValue.FALSE; } - catch (SpelEvaluationException ee) { - ee.setPosition(getLeftOperand().getStartPosition()); - throw ee; - } - - if (leftValue == false) { - return BooleanTypedValue.forValue(false); // no need to evaluate right operand - } - - try { - TypedValue typedValue = getRightOperand().getValueInternal(state); - this.assertTypedValueNotNull(typedValue); - rightValue = (Boolean)state.convertValue(typedValue, TypeDescriptor.valueOf(Boolean.class)); - } - catch (SpelEvaluationException ee) { - ee.setPosition(getRightOperand().getStartPosition()); - throw ee; - } - - return /* leftValue && */BooleanTypedValue.forValue(rightValue); + return BooleanTypedValue.forValue(getBooleanValue(state, getRightOperand())); } - private void assertTypedValueNotNull(TypedValue typedValue) { - if (TypedValue.NULL.equals(typedValue)) { + private boolean getBooleanValue(ExpressionState state, SpelNodeImpl operand) { + try { + Boolean value = operand.getValue(state, Boolean.class); + assertValueNotNull(value); + return value; + } + catch (SpelEvaluationException ee) { + ee.setPosition(operand.getStartPosition()); + throw ee; + } + } + + private void assertValueNotNull(Boolean value) { + if (value == null) { throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean"); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java index c96f757685..c4d3cd4cf4 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpOr.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -16,9 +16,7 @@ package org.springframework.expression.spel.ast; -import org.springframework.core.convert.TypeDescriptor; import org.springframework.expression.EvaluationException; -import org.springframework.expression.TypedValue; import org.springframework.expression.spel.ExpressionState; import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.expression.spel.SpelMessage; @@ -29,6 +27,7 @@ import org.springframework.expression.spel.support.BooleanTypedValue; * * @author Andy Clement * @author Mark Fisher + * @author Oliver Becker * @since 3.0 */ public class OpOr extends Operator { @@ -39,37 +38,27 @@ public class OpOr extends Operator { @Override public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { - boolean leftValue; - boolean rightValue; - try { - TypedValue typedValue = getLeftOperand().getValueInternal(state); - this.assertTypedValueNotNull(typedValue); - leftValue = (Boolean)state.convertValue(typedValue, TypeDescriptor.valueOf(Boolean.class)); + if (getBooleanValue(state, getLeftOperand())) { + // no need to evaluate right operand + return BooleanTypedValue.TRUE; } - catch (SpelEvaluationException see) { - see.setPosition(getLeftOperand().getStartPosition()); - throw see; - } - - if (leftValue == true) { - return BooleanTypedValue.TRUE; // no need to evaluate right operand - } - - try { - TypedValue typedValue = getRightOperand().getValueInternal(state); - this.assertTypedValueNotNull(typedValue); - rightValue = (Boolean)state.convertValue(typedValue, TypeDescriptor.valueOf(Boolean.class)); - } - catch (SpelEvaluationException see) { - see.setPosition(getRightOperand().getStartPosition()); // TODO end positions here and in similar situations - throw see; - } - - return BooleanTypedValue.forValue(leftValue || rightValue); + return BooleanTypedValue.forValue(getBooleanValue(state, getRightOperand())); } - private void assertTypedValueNotNull(TypedValue typedValue) { - if (TypedValue.NULL.equals(typedValue)) { + private boolean getBooleanValue(ExpressionState state, SpelNodeImpl operand) { + try { + Boolean value = operand.getValue(state, Boolean.class); + assertValueNotNull(value); + return value; + } + catch (SpelEvaluationException ee) { + ee.setPosition(operand.getStartPosition()); + throw ee; + } + } + + private void assertValueNotNull(Boolean value) { + if (value == null) { throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean"); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java index 835d522b4b..33ae9e0d97 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OperatorNot.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -16,9 +16,7 @@ package org.springframework.expression.spel.ast; -import org.springframework.core.convert.TypeDescriptor; import org.springframework.expression.EvaluationException; -import org.springframework.expression.TypedValue; import org.springframework.expression.spel.ExpressionState; import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.expression.spel.SpelMessage; @@ -29,6 +27,7 @@ import org.springframework.expression.spel.support.BooleanTypedValue; * * @author Andy Clement * @author Mark Fisher + * @author Oliver Becker * @since 3.0 */ public class OperatorNot extends SpelNodeImpl { // Not is a unary operator so do not extend BinaryOperator @@ -40,11 +39,10 @@ public class OperatorNot extends SpelNodeImpl { // Not is a unary operator so do @Override public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { try { - TypedValue typedValue = children[0].getValueInternal(state); - if (TypedValue.NULL.equals(typedValue)) { + Boolean value = children[0].getValue(state, Boolean.class); + if (value == null) { throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean"); } - boolean value = (Boolean) state.convertValue(typedValue, TypeDescriptor.valueOf(Boolean.class)); return BooleanTypedValue.forValue(!value); } catch (SpelEvaluationException see) { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java index 93cc0794bc..109de5fdc1 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/SpelNodeImpl.java @@ -130,18 +130,8 @@ public abstract class SpelNodeImpl implements SpelNode { return (obj instanceof Class ? ((Class) obj) : obj.getClass()); } - @SuppressWarnings("unchecked") protected final T getValue(ExpressionState state, Class desiredReturnType) throws EvaluationException { - Object result = getValueInternal(state).getValue(); - if (result != null && desiredReturnType != null) { - Class resultType = result.getClass(); - if (desiredReturnType.isAssignableFrom(resultType)) { - return (T) result; - } - // Attempt conversion to the requested type, may throw an exception - return ExpressionUtils.convert(state.getEvaluationContext(), result, desiredReturnType); - } - return (T) result; + return ExpressionUtils.convertTypedValue(state.getEvaluationContext(), getValueInternal(state), desiredReturnType); } public abstract TypedValue getValueInternal(ExpressionState expressionState) throws EvaluationException; diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/BooleanExpressionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/BooleanExpressionTests.java index 30b3210b8f..f84a53b0e4 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/BooleanExpressionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/BooleanExpressionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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. @@ -17,11 +17,15 @@ package org.springframework.expression.spel; import org.junit.Test; +import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.support.GenericConversionService; +import org.springframework.expression.spel.support.StandardTypeConverter; /** * Tests the evaluation of real boolean expressions, these use AND, OR, NOT, TRUE, FALSE - * + * * @author Andy Clement + * @author Oliver Becker */ public class BooleanExpressionTests extends ExpressionTestCase { @@ -83,4 +87,27 @@ public class BooleanExpressionTests extends ExpressionTestCase { evaluateAndCheckError("!35.2", SpelMessage.TYPE_CONVERSION_ERROR, 1); evaluateAndCheckError("! 'foob'", SpelMessage.TYPE_CONVERSION_ERROR, 2); } + + @Test + public void testConvertAndHandleNull() { // SPR-9445 + // without null conversion + evaluateAndCheckError("null or true", SpelMessage.TYPE_CONVERSION_ERROR, 0, "null", "boolean"); + evaluateAndCheckError("null and true", SpelMessage.TYPE_CONVERSION_ERROR, 0, "null", "boolean"); + evaluateAndCheckError("!null", SpelMessage.TYPE_CONVERSION_ERROR, 1, "null", "boolean"); + evaluateAndCheckError("null ? 'foo' : 'bar'", SpelMessage.TYPE_CONVERSION_ERROR, 0, "null", "boolean"); + + // with null conversion (null -> false) + GenericConversionService conversionService = new GenericConversionService() { + @Override + protected Object convertNullSource(TypeDescriptor sourceType, TypeDescriptor targetType) { + return targetType.getType() == Boolean.class ? false : null; + } + }; + eContext.setTypeConverter(new StandardTypeConverter(conversionService)); + + evaluate("null or true", Boolean.TRUE, Boolean.class, false); + evaluate("null and true", Boolean.FALSE, Boolean.class, false); + evaluate("!null", Boolean.TRUE, Boolean.class, false); + evaluate("null ? 'foo' : 'bar'", "bar", String.class, false); + } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionTestCase.java b/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionTestCase.java index 3090c68225..9433568d9c 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionTestCase.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/ExpressionTestCase.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2009 the original author or authors. + * Copyright 2002-2012 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,8 +40,8 @@ public abstract class ExpressionTestCase { protected final static boolean SHOULD_BE_WRITABLE = true; protected final static boolean SHOULD_NOT_BE_WRITABLE = false; - protected final static ExpressionParser parser = new SpelExpressionParser(); - protected final static StandardEvaluationContext eContext = TestScenarioCreator.getTestEvaluationContext(); + 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