From 759c9b35cd4d74aff7b912324751b334bf598f47 Mon Sep 17 00:00:00 2001 From: Oliver Becker Date: Wed, 21 Nov 2012 22:53:29 -0800 Subject: [PATCH] Call ConversionService for null SpEL values Update SpEL boolean operators to always call the ConversionService for null values. Primarily to allow null values to be treated as false by overriding GenericConversionService.convertNullSource(). Issue: SPR-9445 --- .../expression/common/ExpressionUtils.java | 4 +- .../expression/spel/ast/OpAnd.java | 51 ++++++++----------- .../expression/spel/ast/OpOr.java | 51 ++++++++----------- .../expression/spel/ast/OperatorNot.java | 10 ++-- .../expression/spel/ast/SpelNodeImpl.java | 12 +---- .../spel/BooleanExpressionTests.java | 31 ++++++++++- .../expression/spel/ExpressionTestCase.java | 6 +-- 7 files changed, 79 insertions(+), 86 deletions(-) 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