Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Rework compilation of OpNE/OpEQ SpEL operators

Browse Source 
For SPR-14863 we need to adjust the code generation for OpNE
to use !x.equals(y) rather than x!=y. There are also further
cases in the equalityCheck() code in Operator that were not
being handled in the compilation case (when comparators are
used for example). This latter issue also affects OpEQ.

Rather than add yet more bytecode generation, both OpNE and
OpEQ generateCode() methods have been simplified. The
generated code now delegates to equalityCheck() in Operator
which is exactly what the interpreted case does.

This ensures that the compiled code continues to behave just
like the interpreted case. It ensures changes to the interpreted
case are automatically picked up for the compiled case. It
makes the bytecode generation simpler.

The benefit of compilation of SpEL expressions is to avoid
slow reflective calls - that doesn't apply for a basic
(in)equality test so there is no need to go crazy in bytecode
gen.

Issue: SPR-14863
This commit is contained in:
Andy Clement
2016-11-01 13:42:23 -07:00
parent 99cacaa72d
commit 9000acd39d
5 changed files with 196 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files

120
spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java
View File

@@ -1335,8 +1335,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
public void opEq() throws Exception {
String tvar = "35";
expression = parse("#root == 35");
Boolean bb = (Boolean)expression.getValue(tvar);
System.out.println(bb);
assertFalse((Boolean)expression.getValue(tvar));
assertCanCompile(expression);
assertFalse((Boolean)expression.getValue(tvar));
@@ -1623,6 +1621,122 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertTrue((Boolean)expression.getValue());
}
@Test
public void opNe_SPR14863() throws Exception {
// First part is from the test case specified in the bug report
Map<String, String> data = new HashMap<>();
data.put("my-key", new String("my-value"));
StandardEvaluationContext context = new StandardEvaluationContext(new MyContext(data));
SpelParserConfiguration configuration = new SpelParserConfiguration(SpelCompilerMode.MIXED,
ClassLoader.getSystemClassLoader());
SpelExpressionParser parser = new SpelExpressionParser(configuration);
Expression expression = parser.parseExpression("data['my-key'] != 'my-value'");
assertFalse(expression.getValue(context, Boolean.class));
assertCanCompile(expression);
((SpelExpression) expression).compileExpression();
assertFalse(expression.getValue(context, Boolean.class));
List<String> ls = new ArrayList<String>();
ls.add(new String("foo"));
context = new StandardEvaluationContext(ls);
expression = parse("get(0) != 'foo'");
assertFalse(expression.getValue(context, Boolean.class));
assertCanCompile(expression);
assertFalse(expression.getValue(context, Boolean.class));
ls.remove(0);
ls.add("goo");
assertTrue(expression.getValue(context, Boolean.class));
}
@Test
public void opEq_SPR14863() throws Exception {
// Exercise the comparator invocation code that runs in
// equalityCheck() (called from interpreted and compiled code)
expression = parser.parseExpression("#aa==#bb");
StandardEvaluationContext sec = new StandardEvaluationContext();
Apple aa = new Apple(1);
Apple bb = new Apple(2);
sec.setVariable("aa",aa);
sec.setVariable("bb",bb);
boolean b = expression.getValue(sec,Boolean.class);
// Verify what the expression caused aa to be compared to
assertEquals(bb,aa.gotComparedTo);
assertFalse(b);
bb.setValue(1);
b = expression.getValue(sec,Boolean.class);
assertEquals(bb,aa.gotComparedTo);
assertTrue(b);
assertCanCompile(expression);
// Similar test with compiled expression
aa = new Apple(99);
bb = new Apple(100);
sec.setVariable("aa",aa);
sec.setVariable("bb",bb);
b = expression.getValue(sec,Boolean.class);
assertFalse(b);
assertEquals(bb,aa.gotComparedTo);
bb.setValue(99);
b = expression.getValue(sec,Boolean.class);
assertTrue(b);
assertEquals(bb,aa.gotComparedTo);
List<String> ls = new ArrayList<String>();
ls.add(new String("foo"));
StandardEvaluationContext context = new StandardEvaluationContext(ls);
expression = parse("get(0) == 'foo'");
assertTrue(expression.getValue(context, Boolean.class));
assertCanCompile(expression);
assertTrue(expression.getValue(context, Boolean.class));
ls.remove(0);
ls.add("goo");
assertFalse(expression.getValue(context, Boolean.class));
}
public static class Apple implements Comparable<Apple> {
public Object gotComparedTo = null;
public int i;
public Apple(int i) {
this.i = i;
}
public void setValue(int i) {
this.i = i;
}
@Override
public int compareTo(Apple that) {
this.gotComparedTo = that;
if (this.i<that.i) {
return -1;
} else if (this.i>that.i) {
return +1;
} else {
return 0;
}
}
}
// For opNe_SPR14863
public static class MyContext {
private final Map<String, String> data;
public MyContext(Map<String, String> data) {
this.data = data;
}
public Map<String, String> getData() {
return data;
}
}
@Test
public void opPlus() throws Exception {
expression = parse("2+2");
@@ -3901,7 +4015,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
m.put("andy","778");
expression = parse("['andy']==null?1:2");
System.out.println(expression.getValue(m));
assertEquals(2,expression.getValue(m));
assertCanCompile(expression);
assertEquals(2,expression.getValue(m));
m.remove("andy");