Change SpEL equality operators to use .equals

Prior to this commit the SpEL operators `==` and `!=` were using the
Java `==` comparison operator as part of their equality checking. It is
more flexible to use the equals() method on Object.

Under this commit the change to .equals() has been made and the equality
checking code has been pushed into a common method in the Operator
superclass. This commit also makes some tweaks to the other operator
classes - the Float case was missing from OpGT.

Issue: SPR-9194
master
Andy Clement 11 years ago committed by Phillip Webb
parent 7c3cdf82cc
commit 2a05e6afa1
  1. 25
      spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java
  2. 4
      spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java
  3. 3
      spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java
  4. 29
      spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java
  5. 31
      spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java
  6. 29
      spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.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));
}
}

@ -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());
}

@ -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;

@ -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));
}
}

@ -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));
}
}

@ -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<String,TestClass2> map = new HashMap<String,TestClass2>();
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;
}
}
}

Loading…
Cancel
Save