From d9bd2e19a2ec128588ca36080e3b32964936dc85 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 21 Nov 2012 22:52:11 -0800 Subject: [PATCH 1/2] Polish whitespace --- .../expression/spel/ast/OperatorNot.java | 2 +- .../expression/spel/ast/SpelNodeImpl.java | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) 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 1d2910bf0c..835d522b4b 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 @@ -36,7 +36,7 @@ public class OperatorNot extends SpelNodeImpl { // Not is a unary operator so do public OperatorNot(int pos, SpelNodeImpl operand) { super(pos, operand); } - + @Override public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { try { 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 159f732ed4..93cc0794bc 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 @@ -35,11 +35,11 @@ import org.springframework.util.Assert; public abstract class SpelNodeImpl implements SpelNode { private static SpelNodeImpl[] NO_CHILDREN = new SpelNodeImpl[0]; - + protected int pos; // start = top 16bits, end = bottom 16bits protected SpelNodeImpl[] children = SpelNodeImpl.NO_CHILDREN; private SpelNodeImpl parent; - + public SpelNodeImpl(int pos, SpelNodeImpl... operands) { this.pos = pos; // pos combines start and end so can never be zero because tokens cannot be zero length @@ -51,7 +51,7 @@ public abstract class SpelNodeImpl implements SpelNode { } } } - + protected SpelNodeImpl getPreviousChild() { SpelNodeImpl result = null; if (parent != null) { @@ -62,7 +62,7 @@ public abstract class SpelNodeImpl implements SpelNode { } return result; } - + /** * @return true if the next child is one of the specified classes */ @@ -96,7 +96,7 @@ public abstract class SpelNodeImpl implements SpelNode { return getValue(new ExpressionState(new StandardEvaluationContext())); } } - + public final TypedValue getTypedValue(ExpressionState expressionState) throws EvaluationException { if (expressionState != null) { return getValueInternal(expressionState); @@ -118,7 +118,7 @@ public abstract class SpelNodeImpl implements SpelNode { public SpelNode getChild(int index) { return children[index]; } - + public int getChildCount() { return children.length; } From 759c9b35cd4d74aff7b912324751b334bf598f47 Mon Sep 17 00:00:00 2001 From: Oliver Becker Date: Wed, 21 Nov 2012 22:53:29 -0800 Subject: [PATCH 2/2] 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