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

Improve SpEL inline collection caching

Browse Source 
This commit fixes SpEL inline collection caching with
with negative keys or values.

Closes gh-25921
This commit is contained in:
Semyon Danilov
2020-05-11 14:59:23 +03:00
committed by Sébastien Deleuze
parent 6548cee20a
commit 6f9546722a
5 changed files with 218 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineList.java
View File

@@ -27,6 +27,7 @@ import org.springframework.expression.TypedValue;
import org.springframework.expression.spel.CodeFlow;
import org.springframework.expression.spel.ExpressionState;
import org.springframework.expression.spel.SpelNode;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
@@ -36,6 +37,7 @@ import org.springframework.util.Assert;
* @author Andy Clement
* @author Sam Brannen
* @author Harry Yang
* @author Semyon Danilov
* @since 3.0.4
*/
public class InlineList extends SpelNodeImpl {
@@ -66,7 +68,7 @@ public class InlineList extends SpelNodeImpl {
return null;
}
}
else {
else if (!(child instanceof OpMinus) || !((OpMinus) child).isNegativeNumber()) {
return null;
}
}
@@ -74,6 +76,7 @@ public class InlineList extends SpelNodeImpl {
List<Object> constantList = new ArrayList<>();
int childcount = getChildCount();
ExpressionState expressionState = new ExpressionState(new StandardEvaluationContext());
for (int c = 0; c < childcount; c++) {
SpelNode child = getChild(c);
if (child instanceof Literal literal) {
@@ -82,6 +85,9 @@ public class InlineList extends SpelNodeImpl {
else if (child instanceof InlineList inlineList) {
constantList.add(inlineList.getConstantValue());
}
else if (child instanceof OpMinus) {
constantList.add(child.getValue(expressionState));
}
}
return new TypedValue(Collections.unmodifiableList(constantList));
}

13
spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineMap.java
View File

@@ -24,6 +24,7 @@ import org.springframework.expression.EvaluationException;
import org.springframework.expression.TypedValue;
import org.springframework.expression.spel.ExpressionState;
import org.springframework.expression.spel.SpelNode;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
@@ -33,6 +34,7 @@ import org.springframework.util.Assert;
* @author Andy Clement
* @author Sam Brannen
* @author Harry Yang
* @author Semyon Danilov
* @since 4.1
*/
public class InlineMap extends SpelNodeImpl {
@@ -69,13 +71,16 @@ public class InlineMap extends SpelNodeImpl {
}
}
else if (!(c % 2 == 0 && child instanceof PropertyOrFieldReference)) {
return null;
if (!(child instanceof OpMinus) || !((OpMinus) child).isNegativeNumber()) {
return null;
}
}
}
}
Map<Object, Object> constantMap = new LinkedHashMap<>();
int childCount = getChildCount();
ExpressionState expressionState = new ExpressionState(new StandardEvaluationContext());
for (int c = 0; c < childCount; c++) {
SpelNode keyChild = getChild(c++);
Object key;
@@ -85,6 +90,9 @@ public class InlineMap extends SpelNodeImpl {
else if (keyChild instanceof PropertyOrFieldReference propertyOrFieldReference) {
key = propertyOrFieldReference.getName();
}
else if (keyChild instanceof OpMinus) {
key = keyChild.getValue(expressionState);
}
else {
return null;
}
@@ -100,6 +108,9 @@ public class InlineMap extends SpelNodeImpl {
else if (valueChild instanceof InlineMap inlineMap) {
value = inlineMap.getConstantValue();
}
else if (valueChild instanceof OpMinus) {
value = valueChild.getValue(expressionState);
}
constantMap.put(key, value);
}
return new TypedValue(Collections.unmodifiableMap(constantMap));

12
spring-expression/src/main/java/org/springframework/expression/spel/ast/Literal.java
View File

@@ -29,6 +29,7 @@ import org.springframework.lang.Nullable;
*
* @author Andy Clement
* @author Juergen Hoeller
* @author Semyon Danilov
*/
public abstract class Literal extends SpelNodeImpl {
@@ -110,4 +111,15 @@ public abstract class Literal extends SpelNodeImpl {
}
}
/**
* Check whether this literal is a number.
* @return true if this is a number
*/
public boolean isNumberLiteral() {
return this instanceof IntLiteral ||
this instanceof LongLiteral ||
this instanceof FloatLiteral ||
this instanceof RealLiteral;
}
}

12
spring-expression/src/main/java/org/springframework/expression/spel/ast/OpMinus.java
View File

@@ -44,6 +44,7 @@ import org.springframework.util.NumberUtils;
* @author Juergen Hoeller
* @author Giovanni Dall'Oglio Risso
* @author Sam Brannen
* @author Semyon Danilov
* @since 3.0
*/
public class OpMinus extends Operator {
@@ -205,4 +206,15 @@ public class OpMinus extends Operator {
cf.pushDescriptor(this.exitTypeDescriptor);
}
/**
* Check whether this operator is an unary minus and it's child is a number.
* @return true if it is a negative number
*/
public boolean isNegativeNumber() {
if (children.length == 1 && children[0] instanceof Literal) {
return ((Literal) children[0]).isNumberLiteral();
}
return false;
}
}

175
spring-expression/src/test/java/org/springframework/expression/spel/ast/InlineCollectionTests.java Normal file
View File

@@ -0,0 +1,175 @@
/*
* Copyright 2002-2020 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
*
* https://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.ast;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.springframework.expression.ExpressionParser;
import org.springframework.expression.spel.ExpressionState;
import org.springframework.expression.spel.standard.SpelCompiler;
import org.springframework.expression.spel.standard.SpelExpression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import static org.assertj.core.api.Assertions.assertThat;
/**
* @author Semyon Danilov
*/
public class InlineCollectionTests {
@Test
public void testListCached() {
InlineList list = parseList("{1, -2, 3, 4}");
assertThat(list.isConstant()).isTrue();
assertThat(list.getConstantValue()).isEqualTo(Arrays.asList(1, -2, 3, 4));
}
@Test
public void testDynamicListNotCached() {
InlineList list = parseList("{1, 5-2, 3, 4}");
assertThat(list.isConstant()).isFalse();
assertThat(list.getValue(null)).isEqualTo(Arrays.asList(1, 3, 3, 4));
}
@Test
public void testListWithVariableNotCached() {
InlineList list = parseList("{1, -a, 3, 4}");
assertThat(list.isConstant()).isFalse();
final StandardEvaluationContext standardEvaluationContext = new StandardEvaluationContext(new AHolder());
standardEvaluationContext.setVariable("a", 2);
assertThat(list.getValue(new ExpressionState(standardEvaluationContext))).isEqualTo(Arrays.asList(1, -2, 3, 4));
}
@Test
public void testListCanBeCompiled() {
SpelExpression listExpression = parseExpression("{1, -2, 3, 4}");
assertThat(((SpelNodeImpl) listExpression.getAST()).isCompilable()).isTrue();
assertThat(SpelCompiler.compile(listExpression)).isTrue();
}
@Test
public void testDynamicListCantBeCompiled() {
SpelExpression listExpression = parseExpression("{1, 5-2, 3, 4}");
assertThat(((SpelNodeImpl) listExpression.getAST()).isCompilable()).isFalse();
assertThat(SpelCompiler.compile(listExpression)).isFalse();
}
@Test
public void testMapCached() {
InlineMap map = parseMap("{1 : 2, 3 : 4}");
assertThat(map.isConstant()).isTrue();
final Map<Integer, Integer> expected = new HashMap<>();
expected.put(1, 2);
expected.put(3, 4);
assertThat(map.getValue(null)).isEqualTo(expected);
}
@Test
public void testMapWithNegativeKeyCached() {
InlineMap map = parseMap("{-1 : 2, -3 : 4}");
assertThat(map.isConstant()).isTrue();
final Map<Integer, Integer> expected = new HashMap<>();
expected.put(-1, 2);
expected.put(-3, 4);
assertThat(map.getValue(null)).isEqualTo(expected);
}
@Test
public void testMapWithNegativeValueCached() {
InlineMap map = parseMap("{1 : -2, 3 : -4}");
assertThat(map.isConstant()).isTrue();
final Map<Integer, Integer> expected = new HashMap<>();
expected.put(1, -2);
expected.put(3, -4);
assertThat(map.getValue(null)).isEqualTo(expected);
}
@Test
public void testMapWithNegativeLongTypesCached() {
InlineMap map = parseMap("{1L : -2L, 3L : -4L}");
assertThat(map.isConstant()).isTrue();
final Map<Long, Long> expected = new HashMap<>();
expected.put(1L, -2L);
expected.put(3L, -4L);
assertThat(map.getValue(null)).isEqualTo(expected);
}
@Test
public void testMapWithNegativeFloatTypesCached() {
InlineMap map = parseMap("{-1.0f : -2.0f, -3.0f : -4.0f}");
assertThat(map.isConstant()).isTrue();
final Map<Float, Float> expected = new HashMap<>();
expected.put(-1.0f, -2.0f);
expected.put(-3.0f, -4.0f);
assertThat(map.getValue(null)).isEqualTo(expected);
}
@Test
public void testMapWithNegativeRealTypesCached() {
InlineMap map = parseMap("{-1.0 : -2.0, -3.0 : -4.0}");
assertThat(map.isConstant()).isTrue();
final Map<Double, Double> expected = new HashMap<>();
expected.put(-1.0, -2.0);
expected.put(-3.0, -4.0);
assertThat(map.getValue(null)).isEqualTo(expected);
}
@Test
public void testMapWithNegativeKeyAndValueCached() {
InlineMap map = parseMap("{-1 : -2, -3 : -4}");
assertThat(map.isConstant()).isTrue();
final Map<Integer, Integer> expected = new HashMap<>();
expected.put(-1, -2);
expected.put(-3, -4);
assertThat(map.getValue(null)).isEqualTo(expected);
}
@Test
public void testMapWithDynamicNotCached() {
InlineMap map = parseMap("{-1 : 2, -3+1 : -4}");
assertThat(map.isConstant()).isFalse();
final Map<Integer, Integer> expected = new HashMap<>();
expected.put(-1, 2);
expected.put(-2, -4);
assertThat(map.getValue(null)).isEqualTo(expected);
}
private InlineMap parseMap(String s) {
SpelExpression expression = parseExpression(s);
return (InlineMap) expression.getAST();
}
private InlineList parseList(String s) {
SpelExpression expression = parseExpression(s);
return (InlineList) expression.getAST();
}
private SpelExpression parseExpression(final String s) {
ExpressionParser parser = new SpelExpressionParser();
return (SpelExpression) parser.parseExpression(s);
}
private static class AHolder {
public int a = 2;
}
}