From 06ed818f4c6b045f72bf792876fb516fed1e82fc Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 7 Sep 2018 13:12:53 +0200 Subject: [PATCH] Fix SpEL compilation for non trivial elvis operand Issue: SPR-17214 --- .../expression/spel/ast/Elvis.java | 6 +- .../spel/SpelCompilationCoverageTests.java | 155 +++++++++++++----- 2 files changed, 117 insertions(+), 44 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Elvis.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Elvis.java index 28c74aba2ea..10ec6b2bb41 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Elvis.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Elvis.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -79,10 +79,12 @@ public class Elvis extends SpelNodeImpl { public void generateCode(MethodVisitor mv, CodeFlow cf) { // exit type descriptor can be null if both components are literal expressions computeExitTypeDescriptor(); + cf.enterCompilationScope(); this.children[0].generateCode(mv, cf); String lastDesc = cf.lastDescriptor(); Assert.state(lastDesc != null, "No last descriptor"); CodeFlow.insertBoxIfNecessary(mv, lastDesc.charAt(0)); + cf.exitCompilationScope(); Label elseTarget = new Label(); Label endOfIf = new Label(); mv.visitInsn(DUP); @@ -95,12 +97,14 @@ public class Elvis extends SpelNodeImpl { mv.visitJumpInsn(IFEQ, endOfIf); // if not empty, drop through to elseTarget mv.visitLabel(elseTarget); mv.visitInsn(POP); + cf.enterCompilationScope(); this.children[1].generateCode(mv, cf); if (!CodeFlow.isPrimitive(this.exitTypeDescriptor)) { lastDesc = cf.lastDescriptor(); Assert.state(lastDesc != null, "No last descriptor"); CodeFlow.insertBoxIfNecessary(mv, lastDesc.charAt(0)); } + cf.exitCompilationScope(); mv.visitLabel(endOfIf); cf.pushDescriptor(this.exitTypeDescriptor); } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java index 03d9bda890f..2954e1ef3da 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java @@ -692,7 +692,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } @Test - public void ternaryWithBooleanReturn() { // SPR-12271 + public void ternaryWithBooleanReturn_SPR12271() { expression = parser.parseExpression("T(Boolean).TRUE?'abc':'def'"); assertEquals("abc", expression.getValue()); assertCanCompile(expression); @@ -703,7 +703,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(expression); assertEquals("def", expression.getValue()); } - + @Test public void nullsafeFieldPropertyDereferencing_SPR16489() throws Exception { FooObjectHolder foh = new FooObjectHolder(); @@ -715,7 +715,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals("hello",expression.getValue(context)); foh.foo = null; assertNull(expression.getValue(context)); - + // Now revert state of foh and try compiling it: foh.foo = new FooObject(); assertEquals("hello",expression.getValue(context)); @@ -723,9 +723,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals("hello",expression.getValue(context)); foh.foo = null; assertNull(expression.getValue(context)); - + // Static references - expression = (SpelExpression)parser.parseExpression("#var?.propertya"); + expression = (SpelExpression) parser.parseExpression("#var?.propertya"); context.setVariable("var", StaticsHelper.class); assertEquals("sh",expression.getValue(context).toString()); context.setVariable("var", null); @@ -737,7 +737,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertNull(expression.getValue(context)); // Single size primitive (boolean) - expression = (SpelExpression)parser.parseExpression("#var?.a"); + expression = (SpelExpression) parser.parseExpression("#var?.a"); context.setVariable("var", new TestClass4()); assertFalse((Boolean)expression.getValue(context)); context.setVariable("var", null); @@ -749,7 +749,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertNull(expression.getValue(context)); // Double slot primitives - expression = (SpelExpression)parser.parseExpression("#var?.four"); + expression = (SpelExpression) parser.parseExpression("#var?.four"); context.setVariable("var", new Three()); assertEquals("0.04",expression.getValue(context).toString()); context.setVariable("var", null); @@ -760,7 +760,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { context.setVariable("var", null); assertNull(expression.getValue(context)); } - + @Test public void nullsafeMethodChaining_SPR16489() throws Exception { FooObjectHolder foh = new FooObjectHolder(); @@ -777,9 +777,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals("hello",expression.getValue(context)); foh.foo = null; assertNull(expression.getValue(context)); - + // Static method references - expression = (SpelExpression)parser.parseExpression("#var?.methoda()"); + expression = (SpelExpression) parser.parseExpression("#var?.methoda()"); context.setVariable("var", StaticsHelper.class); assertEquals("sh",expression.getValue(context).toString()); context.setVariable("var", null); @@ -789,9 +789,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals("sh",expression.getValue(context).toString()); context.setVariable("var", null); assertNull(expression.getValue(context)); - + // Nullsafe guard on expression element evaluating to primitive/null - expression = (SpelExpression)parser.parseExpression("#var?.intValue()"); + expression = (SpelExpression) parser.parseExpression("#var?.intValue()"); context.setVariable("var", 4); assertEquals("4",expression.getValue(context).toString()); context.setVariable("var", null); @@ -802,9 +802,8 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { context.setVariable("var", null); assertNull(expression.getValue(context)); - // Nullsafe guard on expression element evaluating to primitive/null - expression = (SpelExpression)parser.parseExpression("#var?.booleanValue()"); + expression = (SpelExpression) parser.parseExpression("#var?.booleanValue()"); context.setVariable("var", false); assertEquals("false",expression.getValue(context).toString()); context.setVariable("var", null); @@ -816,7 +815,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertNull(expression.getValue(context)); // Nullsafe guard on expression element evaluating to primitive/null - expression = (SpelExpression)parser.parseExpression("#var?.booleanValue()"); + expression = (SpelExpression) parser.parseExpression("#var?.booleanValue()"); context.setVariable("var", true); assertEquals("true",expression.getValue(context).toString()); context.setVariable("var", null); @@ -828,7 +827,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertNull(expression.getValue(context)); // Nullsafe guard on expression element evaluating to primitive/null - expression = (SpelExpression)parser.parseExpression("#var?.longValue()"); + expression = (SpelExpression) parser.parseExpression("#var?.longValue()"); context.setVariable("var", 5L); assertEquals("5",expression.getValue(context).toString()); context.setVariable("var", null); @@ -840,7 +839,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertNull(expression.getValue(context)); // Nullsafe guard on expression element evaluating to primitive/null - expression = (SpelExpression)parser.parseExpression("#var?.floatValue()"); + expression = (SpelExpression) parser.parseExpression("#var?.floatValue()"); context.setVariable("var", 3f); assertEquals("3.0",expression.getValue(context).toString()); context.setVariable("var", null); @@ -852,7 +851,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertNull(expression.getValue(context)); // Nullsafe guard on expression element evaluating to primitive/null - expression = (SpelExpression)parser.parseExpression("#var?.shortValue()"); + expression = (SpelExpression) parser.parseExpression("#var?.shortValue()"); context.setVariable("var", (short)8); assertEquals("8",expression.getValue(context).toString()); context.setVariable("var", null); @@ -863,7 +862,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { context.setVariable("var", null); assertNull(expression.getValue(context)); } - + @Test public void elvis() throws Exception { Expression expression = parser.parseExpression("'a'?:'b'"); @@ -1575,7 +1574,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(expression); assertTrue((Boolean) expression.getValue(f)); - long l = 300l; + long l = 300L; expression = parse("#root==300l"); assertTrue((Boolean) expression.getValue(l)); assertCanCompile(expression); @@ -3236,15 +3235,15 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertTrue(expression.getValue(Boolean.class)); context.setVariable("it", null); assertNull(expression.getValue(Boolean.class)); - + assertCanCompile(expression); - + context.setVariable("it", 3); assertTrue(expression.getValue(Boolean.class)); context.setVariable("it", null); assertNull(expression.getValue(Boolean.class)); } - + @Test public void failsWhenSettingContextForExpression_SPR12326() { SpelExpressionParser parser = new SpelExpressionParser( @@ -3259,9 +3258,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertTrue(expression.getValue(Boolean.class)); context.setVariable("it", null); assertNull(expression.getValue(Boolean.class)); - + assertCanCompile(expression); - + context.setVariable("it", person); assertTrue(expression.getValue(Boolean.class)); context.setVariable("it", null); @@ -4199,7 +4198,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } @Test - public void propertyReferenceVisibility() { // SPR-12771 + public void propertyReferenceVisibility_SPR12771() { StandardEvaluationContext ctx = new StandardEvaluationContext(); ctx.setVariable("httpServletRequest", HttpServlet3RequestFactory.getOne()); // Without a fix compilation was inserting a checkcast to a private type @@ -4813,46 +4812,46 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertEquals(3, expression.getValue(root)); assertEquals(3, expression.getValue(root)); } - + @Test public void elvisOperator_SPR15192() { SpelParserConfiguration configuration = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); Expression exp; - + exp = new SpelExpressionParser(configuration).parseExpression("bar()"); assertEquals("BAR", exp.getValue(new Foo(), String.class)); assertCanCompile(exp); assertEquals("BAR", exp.getValue(new Foo(), String.class)); assertIsCompiled(exp); - + exp = new SpelExpressionParser(configuration).parseExpression("bar('baz')"); assertEquals("BAZ", exp.getValue(new Foo(), String.class)); assertCanCompile(exp); assertEquals("BAZ", exp.getValue(new Foo(), String.class)); assertIsCompiled(exp); - + StandardEvaluationContext context = new StandardEvaluationContext(); context.setVariable("map", Collections.singletonMap("foo", "qux")); - + exp = new SpelExpressionParser(configuration).parseExpression("bar(#map['foo'])"); assertEquals("QUX", exp.getValue(context, new Foo(), String.class)); assertCanCompile(exp); assertEquals("QUX", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + exp = new SpelExpressionParser(configuration).parseExpression("bar(#map['foo'] ?: 'qux')"); assertEquals("QUX", exp.getValue(context, new Foo(), String.class)); assertCanCompile(exp); assertEquals("QUX", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + // When the condition is a primitive exp = new SpelExpressionParser(configuration).parseExpression("3?:'foo'"); assertEquals("3", exp.getValue(context, new Foo(), String.class)); assertCanCompile(exp); assertEquals("3", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + // When the condition is a double slot primitive exp = new SpelExpressionParser(configuration).parseExpression("3L?:'foo'"); assertEquals("3", exp.getValue(context, new Foo(), String.class)); @@ -4866,7 +4865,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(exp); assertEquals("4", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + // null condition exp = new SpelExpressionParser(configuration).parseExpression("null?:4L"); assertEquals("4", exp.getValue(context, new Foo(), String.class)); @@ -4888,7 +4887,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(exp); assertEquals("foo", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + // variable access returning array exp = new SpelExpressionParser(configuration).parseExpression("#x?:'foo'"); context.setVariable("x",new int[]{1,2,3}); @@ -4898,19 +4897,67 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertIsCompiled(exp); } + @Test + public void elvisOperator_SPR17214() throws Exception { + SpelParserConfiguration spc = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); + SpelExpressionParser sep = new SpelExpressionParser(spc); + + RecordHolder rh = null; + + expression = sep.parseExpression("record.get('abc')?:record.put('abc',expression.someLong?.longValue())"); + rh = new RecordHolder(); + assertNull(expression.getValue(rh)); + assertEquals(3L,expression.getValue(rh)); + assertCanCompile(expression); + rh = new RecordHolder(); + assertNull(expression.getValue(rh)); + assertEquals(3L,expression.getValue(rh)); + + expression = sep.parseExpression("record.get('abc')?:record.put('abc',3L.longValue())"); + rh = new RecordHolder(); + assertNull(expression.getValue(rh)); + assertEquals(3L,expression.getValue(rh)); + assertCanCompile(expression); + rh = new RecordHolder(); + assertNull(expression.getValue(rh)); + assertEquals(3L,expression.getValue(rh)); + + expression = sep.parseExpression("record.get('abc')?:record.put('abc',3L.longValue())"); + rh = new RecordHolder(); + assertNull(expression.getValue(rh)); + assertEquals(3L,expression.getValue(rh)); + assertCanCompile(expression); + rh = new RecordHolder(); + assertNull(expression.getValue(rh)); + assertEquals(3L,expression.getValue(rh)); + + expression = sep.parseExpression("record.get('abc')==null?record.put('abc',expression.someLong?.longValue()):null"); + rh = new RecordHolder(); + rh.expression.someLong=6L; + assertNull(expression.getValue(rh)); + assertEquals(6L,rh.get("abc")); + assertNull(expression.getValue(rh)); + assertCanCompile(expression); + rh = new RecordHolder(); + rh.expression.someLong=6L; + assertNull(expression.getValue(rh)); + assertEquals(6L,rh.get("abc")); + assertNull(expression.getValue(rh)); + } + @Test public void ternaryOperator_SPR15192() { SpelParserConfiguration configuration = new SpelParserConfiguration(SpelCompilerMode.IMMEDIATE, null); Expression exp; StandardEvaluationContext context = new StandardEvaluationContext(); context.setVariable("map", Collections.singletonMap("foo", "qux")); - + exp = new SpelExpressionParser(configuration).parseExpression("bar(#map['foo'] != null ? #map['foo'] : 'qux')"); assertEquals("QUX", exp.getValue(context, new Foo(), String.class)); assertCanCompile(exp); assertEquals("QUX", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + exp = new SpelExpressionParser(configuration).parseExpression("3==3?3:'foo'"); assertEquals("3", exp.getValue(context, new Foo(), String.class)); assertCanCompile(exp); @@ -4921,7 +4968,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(exp); assertEquals("foo", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + // When the condition is a double slot primitive exp = new SpelExpressionParser(configuration).parseExpression("3==3?3L:'foo'"); assertEquals("3", exp.getValue(context, new Foo(), String.class)); @@ -4940,7 +4987,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(exp); assertEquals("abc", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + // null condition exp = new SpelExpressionParser(configuration).parseExpression("3==3?null:4L"); assertEquals(null, exp.getValue(context, new Foo(), String.class)); @@ -4962,7 +5009,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(exp); assertEquals("foo", exp.getValue(context, new Foo(), String.class)); assertIsCompiled(exp); - + // variable access returning array exp = new SpelExpressionParser(configuration).parseExpression("#x==#x?'1,2,3':'foo'"); context.setVariable("x",new int[]{1,2,3}); @@ -5289,9 +5336,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } public static class FooObjectHolder { - + private FooObject foo = new FooObject(); - + public FooObject getFoo() { return foo; } @@ -6060,4 +6107,26 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } } + + public static class RecordHolder { + + public Map record = new HashMap<>(); + + public LongHolder expression = new LongHolder(); + + public void add(String key, Long value) { + record.put(key, value); + } + + public long get(String key) { + return record.get(key); + } + } + + + public static class LongHolder { + + public Long someLong = 3L; + } + }