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

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

@@ -28,6 +28,7 @@
  */
 public class OpEQ extends Operator {
 
+
 	public OpEQ(int pos, SpelNodeImpl... operands) {
 		super("==", pos, operands);
 	}
@@ -38,29 +39,7 @@ 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());
-			}
-			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));
 	}
 
 }

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

@@ -28,6 +28,7 @@
  */
 public class OpGT extends Operator {
 
+
 	public OpGT(int pos, SpelNodeImpl... operands) {
 		super(">", pos, operands);
 	}
@@ -43,6 +44,9 @@ public BooleanTypedValue getValueInternal(ExpressionState state) throws Evaluati
 			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());
 			}

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

@@ -28,6 +28,7 @@
  */
 public class OpLT extends Operator {
 
+
 	public OpLT(int pos, SpelNodeImpl... operands) {
 		super("<", pos, operands);
 	}
@@ -38,7 +39,7 @@ public BooleanTypedValue getValueInternal(ExpressionState state)
 			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;

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

@@ -28,42 +28,17 @@
  */
 public class OpNE extends Operator {
 
+
 	public OpNE(int pos, SpelNodeImpl... operands) {
 		super("!=", pos, operands);
 	}
 
 
 	@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));
 	}
 
 }

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 @@
  */
 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 String toStringAST() {
 		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));
+	}
+
 }

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

@@ -1839,6 +1839,19 @@ public void SPR_10486() throws Exception {
 				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 void setName(String name) {
 		}
 
 	}
+
+	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;
+		}
+
+	}
 }