Skip to content

Commit a419584

Browse files
author
Phillip Webb
committed
Merge pull request #406 from aclement/fix-SPR-9194
* fix-SPR-9194: Change SpEL equality operators to use .equals
2 parents 7c3cdf8 + 2a05e6a commit a419584

File tree

6 files changed

+69
-52
lines changed

6 files changed

+69
-52
lines changed

spring-expression/src/main/java/org/springframework/expression/spel/ast/OpEQ.java

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
*/
2929
public class OpEQ extends Operator {
3030

31+
3132
public OpEQ(int pos, SpelNodeImpl... operands) {
3233
super("==", pos, operands);
3334
}
@@ -38,29 +39,7 @@ public BooleanTypedValue getValueInternal(ExpressionState state)
3839
throws EvaluationException {
3940
Object left = getLeftOperand().getValueInternal(state).getValue();
4041
Object right = getRightOperand().getValueInternal(state).getValue();
41-
if (left instanceof Number && right instanceof Number) {
42-
Number op1 = (Number) left;
43-
Number op2 = (Number) right;
44-
if (op1 instanceof Double || op2 instanceof Double) {
45-
return BooleanTypedValue.forValue(op1.doubleValue() == op2.doubleValue());
46-
}
47-
else if (op1 instanceof Float || op2 instanceof Float) {
48-
return BooleanTypedValue.forValue(op1.floatValue() == op2.floatValue());
49-
}
50-
else if (op1 instanceof Long || op2 instanceof Long) {
51-
return BooleanTypedValue.forValue(op1.longValue() == op2.longValue());
52-
}
53-
else {
54-
return BooleanTypedValue.forValue(op1.intValue() == op2.intValue());
55-
}
56-
}
57-
if (left != null && (left instanceof Comparable)) {
58-
return BooleanTypedValue.forValue(state.getTypeComparator().compare(left,
59-
right) == 0);
60-
}
61-
else {
62-
return BooleanTypedValue.forValue(left == right);
63-
}
42+
return BooleanTypedValue.forValue(equalityCheck(state, left, right));
6443
}
6544

6645
}

spring-expression/src/main/java/org/springframework/expression/spel/ast/OpGT.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
*/
2929
public class OpGT extends Operator {
3030

31+
3132
public OpGT(int pos, SpelNodeImpl... operands) {
3233
super(">", pos, operands);
3334
}
@@ -43,6 +44,9 @@ public BooleanTypedValue getValueInternal(ExpressionState state) throws Evaluati
4344
if (leftNumber instanceof Double || rightNumber instanceof Double) {
4445
return BooleanTypedValue.forValue(leftNumber.doubleValue() > rightNumber.doubleValue());
4546
}
47+
else if (leftNumber instanceof Float || rightNumber instanceof Float) {
48+
return BooleanTypedValue.forValue(leftNumber.floatValue() > rightNumber.floatValue());
49+
}
4650
else if (leftNumber instanceof Long || rightNumber instanceof Long) {
4751
return BooleanTypedValue.forValue(leftNumber.longValue() > rightNumber.longValue());
4852
}

spring-expression/src/main/java/org/springframework/expression/spel/ast/OpLT.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
*/
2929
public class OpLT extends Operator {
3030

31+
3132
public OpLT(int pos, SpelNodeImpl... operands) {
3233
super("<", pos, operands);
3334
}
@@ -38,7 +39,7 @@ public BooleanTypedValue getValueInternal(ExpressionState state)
3839
throws EvaluationException {
3940
Object left = getLeftOperand().getValueInternal(state).getValue();
4041
Object right = getRightOperand().getValueInternal(state).getValue();
41-
// TODO could leave all of these to the comparator - just seems quicker to do some here
42+
4243
if (left instanceof Number && right instanceof Number) {
4344
Number leftNumber = (Number) left;
4445
Number rightNumber = (Number) right;

spring-expression/src/main/java/org/springframework/expression/spel/ast/OpNE.java

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,42 +28,17 @@
2828
*/
2929
public class OpNE extends Operator {
3030

31+
3132
public OpNE(int pos, SpelNodeImpl... operands) {
3233
super("!=", pos, operands);
3334
}
3435

3536

3637
@Override
3738
public BooleanTypedValue getValueInternal(ExpressionState state) throws EvaluationException {
38-
3939
Object left = getLeftOperand().getValueInternal(state).getValue();
4040
Object right = getRightOperand().getValueInternal(state).getValue();
41-
42-
if (left instanceof Number && right instanceof Number) {
43-
Number op1 = (Number) left;
44-
Number op2 = (Number) right;
45-
46-
if (op1 instanceof Double || op2 instanceof Double) {
47-
return BooleanTypedValue.forValue(op1.doubleValue() != op2.doubleValue());
48-
}
49-
50-
if (op1 instanceof Float || op2 instanceof Float) {
51-
return BooleanTypedValue.forValue(op1.floatValue() != op2.floatValue());
52-
}
53-
54-
if (op1 instanceof Long || op2 instanceof Long) {
55-
return BooleanTypedValue.forValue(op1.longValue() != op2.longValue());
56-
}
57-
58-
return BooleanTypedValue.forValue(op1.intValue() != op2.intValue());
59-
}
60-
61-
if (left != null && (left instanceof Comparable)) {
62-
return BooleanTypedValue.forValue(state.getTypeComparator().compare(left,
63-
right) != 0);
64-
}
65-
66-
return BooleanTypedValue.forValue(left != right);
41+
return BooleanTypedValue.forValue(!equalityCheck(state, left, right));
6742
}
6843

6944
}

spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.expression.spel.ast;
1818

19+
import org.springframework.expression.spel.ExpressionState;
20+
1921
/**
2022
* Common supertype for operators that operate on either one or two operands. In the case
2123
* of multiply or divide there would be two operands, but for unary plus or minus, there
@@ -26,7 +28,7 @@
2628
*/
2729
public abstract class Operator extends SpelNodeImpl {
2830

29-
String operatorName;
31+
private final String operatorName;
3032

3133

3234
public Operator(String payload,int pos,SpelNodeImpl... operands) {
@@ -63,4 +65,31 @@ public String toStringAST() {
6365
return sb.toString();
6466
}
6567

68+
protected boolean equalityCheck(ExpressionState state, Object left, Object right) {
69+
if (left instanceof Number && right instanceof Number) {
70+
Number op1 = (Number) left;
71+
Number op2 = (Number) right;
72+
73+
if (op1 instanceof Double || op2 instanceof Double) {
74+
return (op1.doubleValue() == op2.doubleValue());
75+
}
76+
77+
if (op1 instanceof Float || op2 instanceof Float) {
78+
return (op1.floatValue() == op2.floatValue());
79+
}
80+
81+
if (op1 instanceof Long || op2 instanceof Long) {
82+
return (op1.longValue() == op2.longValue());
83+
}
84+
85+
return (op1.intValue() == op2.intValue());
86+
}
87+
88+
if (left != null && (left instanceof Comparable)) {
89+
return (state.getTypeComparator().compare(left, right) == 0);
90+
}
91+
92+
return (left == null ? right == null : left.equals(right));
93+
}
94+
6695
}

spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1839,6 +1839,19 @@ public void SPR_10486() throws Exception {
18391839
equalTo((Object) "name"));
18401840
}
18411841

1842+
@Test
1843+
public void testOperatorEq_SPR9194() {
1844+
TestClass2 one = new TestClass2("abc");
1845+
TestClass2 two = new TestClass2("abc");
1846+
Map<String,TestClass2> map = new HashMap<String,TestClass2>();
1847+
map.put("one",one);
1848+
map.put("two",two);
1849+
1850+
SpelExpressionParser parser = new SpelExpressionParser();
1851+
Expression classNameExpression = parser.parseExpression("['one'] == ['two']");
1852+
assertTrue(classNameExpression.getValue(map,Boolean.class));
1853+
}
1854+
18421855

18431856
private static enum ABC {A, B, C}
18441857

@@ -1922,4 +1935,20 @@ public void setName(String name) {
19221935
}
19231936

19241937
}
1938+
1939+
static class TestClass2 { // SPR-9194
1940+
String string;
1941+
1942+
public TestClass2(String string) {
1943+
this.string = string;
1944+
}
1945+
1946+
public boolean equals(Object o) {
1947+
if (o instanceof TestClass2) {
1948+
return string.equals(((TestClass2)o).string);
1949+
}
1950+
return false;
1951+
}
1952+
1953+
}
19251954
}

0 commit comments

Comments
 (0)