Browse Source

Fix SpEL comparison operator for comparable types

Prior to this commit, the SpEL relational operators for comparison
would have the following problem:

* their implementation would claim that incompatible types
  could be compared and later fail during comparison
* not delegate the comparison to the actual `TypeComparator`
  implementation but rely on operator specifics

This commit ensures that the `TypeComparator` implementation is used for
both `canCompare` and `compare` calls in the operators.

See gh-1581
pull/27735/head
Tin Pavlinic 7 years ago committed by Brian Clozel
parent
commit
d178eafc11
  1. 7
      spring-expression/src/main/java/org/springframework/expression/spel/ast/Operator.java
  2. 6
      spring-expression/src/main/java/org/springframework/expression/spel/support/StandardTypeComparator.java
  3. 62
      spring-expression/src/test/java/org/springframework/expression/spel/AlternativeComparatorTests.java
  4. 2
      spring-expression/src/test/java/org/springframework/expression/spel/DefaultComparatorUnitTests.java

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

@ -309,11 +309,8 @@ public abstract class Operator extends SpelNodeImpl { @@ -309,11 +309,8 @@ public abstract class Operator extends SpelNodeImpl {
return true;
}
if (left instanceof Comparable && right instanceof Comparable) {
Class<?> ancestor = ClassUtils.determineCommonAncestor(left.getClass(), right.getClass());
if (ancestor != null && Comparable.class.isAssignableFrom(ancestor)) {
return (context.getTypeComparator().compare(left, right) == 0);
}
if (context.getTypeComparator().canCompare(left, right)) {
return context.getTypeComparator().compare(left, right) == 0;
}
return false;

6
spring-expression/src/main/java/org/springframework/expression/spel/support/StandardTypeComparator.java

@ -23,6 +23,7 @@ import org.springframework.expression.TypeComparator; @@ -23,6 +23,7 @@ import org.springframework.expression.TypeComparator;
import org.springframework.expression.spel.SpelEvaluationException;
import org.springframework.expression.spel.SpelMessage;
import org.springframework.lang.Nullable;
import org.springframework.util.ClassUtils;
import org.springframework.util.NumberUtils;
/**
@ -44,8 +45,9 @@ public class StandardTypeComparator implements TypeComparator { @@ -44,8 +45,9 @@ public class StandardTypeComparator implements TypeComparator {
if (left instanceof Number && right instanceof Number) {
return true;
}
if (left instanceof Comparable) {
return true;
if (left instanceof Comparable && right instanceof Comparable) {
Class<?> ancestor = ClassUtils.determineCommonAncestor(left.getClass(), right.getClass());
return ancestor != null && Comparable.class.isAssignableFrom(ancestor);
}
return false;
}

62
spring-expression/src/test/java/org/springframework/expression/spel/AlternativeComparatorTests.java

@ -0,0 +1,62 @@ @@ -0,0 +1,62 @@
/*
* Copyright 2002-2014 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.expression.spel;
import org.junit.Test;
import org.springframework.lang.Nullable;
import org.springframework.expression.Expression;
import org.springframework.expression.TypeComparator;
import org.springframework.expression.EvaluationException;
import org.springframework.expression.ExpressionParser;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import static org.junit.Assert.*;
/**
* Test construction of arrays.
*
* @author Andy Clement
*/
public class AlternativeComparatorTests {
private ExpressionParser parser = new SpelExpressionParser();
// A silly comparator declaring everything to be equal
private TypeComparator customComparator = new TypeComparator() {
@Override
public boolean canCompare(@Nullable Object firstObject, @Nullable Object secondObject) {
return true;
}
@Override
public int compare(@Nullable Object firstObject, @Nullable Object secondObject) throws EvaluationException {
return 0;
}
};
@Test
public void customComparatorWorksWithEquality() {
final StandardEvaluationContext ctx = new StandardEvaluationContext();
ctx.setTypeComparator(customComparator);
Expression expr = parser.parseExpression("'1' == 1");
assertEquals(true, expr.getValue(ctx, Boolean.class));
}
}

2
spring-expression/src/test/java/org/springframework/expression/spel/DefaultComparatorUnitTests.java

@ -116,7 +116,7 @@ class DefaultComparatorUnitTests { @@ -116,7 +116,7 @@ class DefaultComparatorUnitTests {
assertThat(comparator.canCompare(2,1)).isTrue();
assertThat(comparator.canCompare("abc","def")).isTrue();
assertThat(comparator.canCompare("abc",3)).isTrue();
assertThat(comparator.canCompare("abc",3)).isFalse();
assertThat(comparator.canCompare(String.class,3)).isFalse();
}

Loading…
Cancel
Save