diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java index 1121cc840a..027d4e41cf 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java @@ -28,6 +28,7 @@ import org.springframework.expression.spel.support.BooleanTypedValue; */ public class OpEQ extends Operator { + public OpEQ(int pos, SpelNodeImpl... operands) { super("==", pos, operands); } @@ -38,29 +39,7 @@ public class OpEQ extends Operator { throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); - if (left instanceof Number && right instanceof Number) { - Number op1 = (Number) left; - Number op2 = (Number) right; - if (op1 instanceof Double || op2 instanceof Double) { - return BooleanTypedValue.forValue(op1.doubleValue() == op2.doubleValue()); - } - else if (op1 instanceof Float || op2 instanceof Float) { - return BooleanTypedValue.forValue(op1.floatValue() == op2.floatValue()); - } - else if (op1 instanceof Long || op2 instanceof Long) { - return BooleanTypedValue.forValue(op1.longValue() == op2.longValue()); - } - else { - return BooleanTypedValue.forValue(op1.intValue() == op2.intValue()); - } - } - if (left != null && (left instanceof Comparable)) { - return BooleanTypedValue.forValue(state.getTypeComparator().compare(left, - right) == 0); - } - else { - return BooleanTypedValue.forValue(left == right); - } + return BooleanTypedValue.forValue(equalityCheck(state, left, right)); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java index 3d751c976d..769cc8d11a 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java @@ -28,6 +28,7 @@ import org.springframework.expression.spel.support.BooleanTypedValue; */ public class OpGT extends Operator { + public OpGT(int pos, SpelNodeImpl... operands) { super(">", pos, operands); } @@ -43,6 +44,9 @@ public class OpGT extends Operator { if (leftNumber instanceof Double || rightNumber instanceof Double) { return BooleanTypedValue.forValue(leftNumber.doubleValue() > rightNumber.doubleValue()); } + else if (leftNumber instanceof Float || rightNumber instanceof Float) { + return BooleanTypedValue.forValue(leftNumber.floatValue() > rightNumber.floatValue()); + } else if (leftNumber instanceof Long || rightNumber instanceof Long) { return BooleanTypedValue.forValue(leftNumber.longValue() > rightNumber.longValue()); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java index dc8916dda8..1027a0c026 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java @@ -28,6 +28,7 @@ import org.springframework.expression.spel.support.BooleanTypedValue; */ public class OpLT extends Operator { + public OpLT(int pos, SpelNodeImpl... operands) { super("<", pos, operands); } @@ -38,7 +39,7 @@ public class OpLT extends Operator { throws EvaluationException { Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); - // TODO could leave all of these to the comparator - just seems quicker to do some here + if (left instanceof Number && right instanceof Number) { Number leftNumber = (Number) left; Number rightNumber = (Number) right; diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java index fdf65c24d7..70b2ce4b8e 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java @@ -28,6 +28,7 @@ import org.springframework.expression.spel.support.BooleanTypedValue; */ public class OpNE extends Operator { + public OpNE(int pos, SpelNodeImpl... operands) { super("!=", pos, operands); } @@ -35,35 +36,9 @@ public class OpNE extends Operator { @Override public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException { - Object left = getLeftOperand().getValueInternal(state).getValue(); Object right = getRightOperand().getValueInternal(state).getValue(); - - if (left instanceof Number && right instanceof Number) { - Number op1 = (Number) left; - Number op2 = (Number) right; - - if (op1 instanceof Double || op2 instanceof Double) { - return BooleanTypedValue.forValue(op1.doubleValue() != op2.doubleValue()); - } - - if (op1 instanceof Float || op2 instanceof Float) { - return BooleanTypedValue.forValue(op1.floatValue() != op2.floatValue()); - } - - if (op1 instanceof Long || op2 instanceof Long) { - return BooleanTypedValue.forValue(op1.longValue() != op2.longValue()); - } - - return BooleanTypedValue.forValue(op1.intValue() != op2.intValue()); - } - - if (left != null && (left instanceof Comparable)) { - return BooleanTypedValue.forValue(state.getTypeComparator().compare(left, - right) != 0); - } - - return BooleanTypedValue.forValue(left != right); + return BooleanTypedValue.forValue(!equalityCheck(state, left, right)); } } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java index 637ba71185..9f5e00364f 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java @@ -16,6 +16,8 @@ package org.springframework.expression.spel.ast; +import org.springframework.expression.spel.ExpressionState; + /** * Common supertype for operators that operate on either one or two operands. In the case * of multiply or divide there would be two operands, but for unary plus or minus, there @@ -26,7 +28,7 @@ package org.springframework.expression.spel.ast; */ public abstract class Operator extends SpelNodeImpl { - String operatorName; + private final String operatorName; public Operator(String payload,int pos,SpelNodeImpl... operands) { @@ -63,4 +65,31 @@ public abstract class Operator extends SpelNodeImpl { return sb.toString(); } + protected boolean equalityCheck(ExpressionState state, Object left, Object right) { + if (left instanceof Number && right instanceof Number) { + Number op1 = (Number) left; + Number op2 = (Number) right; + + if (op1 instanceof Double || op2 instanceof Double) { + return (op1.doubleValue() == op2.doubleValue()); + } + + if (op1 instanceof Float || op2 instanceof Float) { + return (op1.floatValue() == op2.floatValue()); + } + + if (op1 instanceof Long || op2 instanceof Long) { + return (op1.longValue() == op2.longValue()); + } + + return (op1.intValue() == op2.intValue()); + } + + if (left != null && (left instanceof Comparable)) { + return (state.getTypeComparator().compare(left, right) == 0); + } + + return (left == null ? right == null : left.equals(right)); + } + } 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 a68dfda68d..cc6a36af06 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 @@ -1839,6 +1839,19 @@ public class SpelReproTests extends ExpressionTestCase { equalTo((Object) "name")); } + @Test + public void testOperatorEq_SPR9194() { + TestClass2 one = new TestClass2("abc"); + TestClass2 two = new TestClass2("abc"); + Map map = new HashMap(); + map.put("one",one); + map.put("two",two); + + SpelExpressionParser parser = new SpelExpressionParser(); + Expression classNameExpression = parser.parseExpression("['one'] == ['two']"); + assertTrue(classNameExpression.getValue(map,Boolean.class)); + } + private static enum ABC {A, B, C} @@ -1922,4 +1935,20 @@ public class SpelReproTests extends ExpressionTestCase { } } + + static class TestClass2 { // SPR-9194 + String string; + + public TestClass2(String string) { + this.string = string; + } + + public boolean equals(Object o) { + if (o instanceof TestClass2) { + return string.equals(((TestClass2)o).string); + } + return false; + } + + } }