Skip to content

Commit

Permalink
Polishing
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbrannen
sbrannen committed Aug 20, 2023
1 parent 0e0c298 commit 1e30997
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 86 deletions.
8 changes: 3 additions & 5 deletions spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineList.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
*/
public class InlineList extends SpelNodeImpl {

// If the list is purely literals, it is a constant value and can be computed and cached
@Nullable
private final TypedValue constant;

Expand All @@ -52,10 +51,9 @@ public InlineList(int startPos, int endPos, SpelNodeImpl... args) {


/**
* If all the components of the list are constants, or lists
* that themselves contain constants, then a constant list
* can be built to represent this node. This will speed up
* later getValue calls and reduce the amount of garbage
* If all the components of the list are constants, or lists that themselves
* contain constants, then a constant list can be built to represent this node.
* <p>This will speed up later getValue calls and reduce the amount of garbage
* created.
*/
@Nullable
Expand Down
14 changes: 7 additions & 7 deletions spring-expression/src/main/java/org/springframework/expression/spel/ast/InlineMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,21 @@
*/
public class InlineMap extends SpelNodeImpl {

// If the map is purely literals, it is a constant value and can be computed and cached
@Nullable
private final TypedValue constant;


public InlineMap(int startPos, int endPos, SpelNodeImpl... args) {
super(startPos, endPos, args);
this.constant = computeConstantValue();
this.constant = computeConstantValue();
}


/**
* If all the components of the map are constants, or lists/maps that themselves
* contain constants, then a constant list can be built to represent this node.
* This will speed up later getValue calls and reduce the amount of garbage created.
* <p>This will speed up later getValue calls and reduce the amount of garbage
* created.
*/
@Nullable
private TypedValue computeConstantValue() {
Expand All @@ -78,9 +78,7 @@ else if (!(c % 2 == 0 && child instanceof PropertyOrFieldReference)) {
int childCount = getChildCount();
for (int c = 0; c < childCount; c++) {
SpelNode keyChild = getChild(c++);
SpelNode valueChild = getChild(c);
Object key;
Object value = null;
if (keyChild instanceof Literal literal) {
key = literal.getLiteralValue().getValue();
}
Expand All @@ -90,6 +88,9 @@ else if (keyChild instanceof PropertyOrFieldReference propertyOrFieldReference)
else {
return null;
}

SpelNode valueChild = getChild(c);
Object value = null;
if (valueChild instanceof Literal literal) {
value = literal.getLiteralValue().getValue();
}
Expand All @@ -113,7 +114,6 @@ public TypedValue getValueInternal(ExpressionState expressionState) throws Evalu
Map<Object, Object> returnValue = new LinkedHashMap<>();
int childcount = getChildCount();
for (int c = 0; c < childcount; c++) {
// TODO allow for key being PropertyOrFieldReference like Indexer on maps
SpelNode keyChild = getChild(c++);
Object key = null;
if (keyChild instanceof PropertyOrFieldReference reference) {
Expand Down Expand Up @@ -146,7 +146,7 @@ public String toStringAST() {
}

/**
* Return whether this list is a constant value.
* Return whether this map is a constant value.
*/
public boolean isConstant() {
return this.constant != null;
Expand Down
142 changes: 68 additions & 74 deletions spring-expression/src/test/java/org/springframework/expression/spel/MapTests.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2023 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.
Expand All @@ -24,6 +24,7 @@

import org.junit.jupiter.api.Test;

import org.springframework.expression.Expression;
import org.springframework.expression.spel.ast.InlineMap;
import org.springframework.expression.spel.standard.SpelExpression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
Expand All @@ -35,33 +36,26 @@
* Test usage of inline maps.
*
* @author Andy Clement
* @author Sam Brannen
* @since 4.1
*/
public class MapTests extends AbstractExpressionTests {
class MapTests extends AbstractExpressionTests {

// if the list is full of literals then it will be of the type unmodifiableClass
// if the list is full of literals then it will be of the type unmodifiableMapClass
// rather than HashMap (or similar)
Class<?> unmodifiableClass = Collections.unmodifiableMap(new LinkedHashMap<>()).getClass();
private static final Class<?> unmodifiableMapClass = Collections.unmodifiableMap(Map.of()).getClass();


@Test
public void testInlineMapCreation01() {
evaluate("{'a':1, 'b':2, 'c':3, 'd':4, 'e':5}", "{a=1, b=2, c=3, d=4, e=5}", unmodifiableClass);
evaluate("{'a':1}", "{a=1}", unmodifiableClass);
void inlineMapCreationForLiterals() {
evaluate("{'a':1, 'b':2, 'c':3, 'd':4, 'e':5}", "{a=1, b=2, c=3, d=4, e=5}", unmodifiableMapClass);
evaluate("{'a':1}", "{a=1}", unmodifiableMapClass);
evaluate("{'abc':'def', 'uvw':'xyz'}", "{abc=def, uvw=xyz}", unmodifiableMapClass);
evaluate("{:}", "{}", unmodifiableMapClass);
}

@Test
public void testInlineMapCreation02() {
evaluate("{'abc':'def', 'uvw':'xyz'}", "{abc=def, uvw=xyz}", unmodifiableClass);
}

@Test
public void testInlineMapCreation03() {
evaluate("{:}", "{}", unmodifiableClass);
}

@Test
public void testInlineMapCreation04() {
void inlineMapCreationForNonLiterals() {
evaluate("{'key':'abc'=='xyz'}", "{key=false}", LinkedHashMap.class);
evaluate("{key:'abc'=='xyz'}", "{key=false}", LinkedHashMap.class);
evaluate("{key:'abc'=='xyz',key2:true}[key]", "false", Boolean.class);
Expand All @@ -70,43 +64,48 @@ public void testInlineMapCreation04() {
}

@Test
public void testInlineMapAndNesting() {
evaluate("{a:{a:1,b:2,c:3},b:{d:4,e:5,f:6}}", "{a={a=1, b=2, c=3}, b={d=4, e=5, f=6}}", unmodifiableClass);
evaluate("{a:{x:1,y:'2',z:3},b:{u:4,v:{'a','b'},w:5,x:6}}", "{a={x=1, y=2, z=3}, b={u=4, v=[a, b], w=5, x=6}}", unmodifiableClass);
evaluate("{a:{1,2,3},b:{4,5,6}}", "{a=[1, 2, 3], b=[4, 5, 6]}", unmodifiableClass);
void inlineMapAndNesting() {
evaluate("{a:{a:1,b:2,c:3},b:{d:4,e:5,f:6}}", "{a={a=1, b=2, c=3}, b={d=4, e=5, f=6}}", unmodifiableMapClass);
evaluate("{a:{x:1,y:'2',z:3},b:{u:4,v:{'a','b'},w:5,x:6}}", "{a={x=1, y=2, z=3}, b={u=4, v=[a, b], w=5, x=6}}", unmodifiableMapClass);
evaluate("{a:{1,2,3},b:{4,5,6}}", "{a=[1, 2, 3], b=[4, 5, 6]}", unmodifiableMapClass);
}

@Test
public void testInlineMapWithFunkyKeys() {
evaluate("{#root.name:true}","{Nikola Tesla=true}",LinkedHashMap.class);
void inlineMapWithFunkyKeys() {
evaluate("{#root.name:true}", "{Nikola Tesla=true}", LinkedHashMap.class);
}

@Test
public void testInlineMapError() {
void inlineMapSyntaxError() {
parseAndCheckError("{key:'abc'", SpelMessage.OOD);
}

@Test
public void testRelOperatorsIs02() {
evaluate("{a:1, b:2, c:3, d:4, e:5} instanceof T(java.util.Map)", "true", Boolean.class);
void inelineMapIsInstanceOfMap() {
evaluate("{a:1, b:2} instanceof T(java.util.Map)", "true", Boolean.class);
}

@Test
public void testInlineMapAndProjectionSelection() {
evaluate("{a:1,b:2,c:3,d:4,e:5,f:6}.![value>3]", "[false, false, false, true, true, true]", ArrayList.class);
evaluate("{a:1,b:2,c:3,d:4,e:5,f:6}.?[value>3]", "{d=4, e=5, f=6}", HashMap.class);
evaluate("{a:1,b:2,c:3,d:4,e:5,f:6,g:7,h:8,i:9,j:10}.?[value%2==0]", "{b=2, d=4, f=6, h=8, j=10}", HashMap.class);
// TODO this looks like a serious issue (but not a new one): the context object against which arguments are evaluated seems wrong:
// evaluate("{a:1,b:2,c:3,d:4,e:5,f:6,g:7,h:8,i:9,j:10}.?[isEven(value) == 'y']", "[2, 4, 6, 8, 10]", ArrayList.class);
void inlineMapProjection() {
evaluate("{a:1,b:2,c:3,d:4}.![value > 2]", "[false, false, true, true]", ArrayList.class);
evaluate("{a:1,b:2,c:3,d:4}.![value % 2 == 0]", "[false, true, false, true]", ArrayList.class);
evaluate("{a:1,b:2,c:3,d:4}.![#isEven(value) == 'y']", "[false, true, false, true]", ArrayList.class);
}

@Test
public void testSetConstruction01() {
evaluate("new java.util.HashMap().putAll({a:'a',b:'b',c:'c'})", null, Object.class);
void inlineMapSelection() {
evaluate("{a:1,b:2,c:3,d:4}.?[value > 2]", "{c=3, d=4}", HashMap.class);
evaluate("{a:1,b:2,c:3,d:4,e:5,f:6}.?[value % 2 == 0]", "{b=2, d=4, f=6}", HashMap.class);
evaluate("{a:1,b:2,c:3,d:4,e:5,f:6}.?[#isEven(value) == 'y']", "{b=2, d=4, f=6}", HashMap.class);
}

@Test
public void testConstantRepresentation1() {
void mapConstruction() {
evaluate("new java.util.HashMap().putAll({a:'a',b:'b'})", null, Object.class);
}

@Test
void constantMaps() {
checkConstantMap("{f:{'a','b','c'}}", true);
checkConstantMap("{'a':1,'b':2,'c':3,'d':4,'e':5}", true);
checkConstantMap("{aaa:'abc'}", true);
Expand All @@ -117,43 +116,37 @@ public void testConstantRepresentation1() {
checkConstantMap("{#root.name:true}",false);
checkConstantMap("{a:1,b:2,c:{d:true,e:false}}", true);
checkConstantMap("{a:1,b:2,c:{d:{1,2,3},e:{4,5,6},f:{'a','b','c'}}}", true);
// for nested InlineMap
checkConstantMap("{a:{k:#d}}", false);
checkConstantMap("{a:{k:#d}}", false); // nested InlineMap
checkConstantMap("{@bean:@bean}", false);
}

private void checkConstantMap(String expressionText, boolean expectedToBeConstant) {
SpelExpressionParser parser = new SpelExpressionParser();
SpelExpression expression = (SpelExpression) parser.parseExpression(expressionText);
SpelNode node = expression.getAST();
boolean condition = node instanceof InlineMap;
assertThat(condition).isTrue();
InlineMap inlineMap = (InlineMap) node;
if (expectedToBeConstant) {
assertThat(inlineMap.isConstant()).isTrue();
}
else {
assertThat(inlineMap.isConstant()).isFalse();
}
assertThat(node).isInstanceOfSatisfying(InlineMap.class, inlineMap -> {
if (expectedToBeConstant) {
assertThat(inlineMap.isConstant()).isTrue();
}
else {
assertThat(inlineMap.isConstant()).isFalse();
}
});
}

@Test
public void testInlineMapWriting() {
// list should be unmodifiable
assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() ->
evaluate("{a:1, b:2, c:3, d:4, e:5}[a]=6", "[a:1,b: 2,c: 3,d: 4,e: 5]", unmodifiableClass));
void inlineMapIsUnmodifiable() {
Expression expr = parser.parseExpression("{a:1}[a] = 6");
assertThatExceptionOfType(UnsupportedOperationException.class)
.isThrownBy(() -> expr.getValue(context));
}

@Test
public void testMapKeysThatAreAlsoSpELKeywords() {
void mapKeysThatAreAlsoSpELKeywords() {
SpelExpressionParser parser = new SpelExpressionParser();
SpelExpression expression = null;
Object o = null;

// expression = (SpelExpression) parser.parseExpression("foo['NEW']");
// o = expression.getValue(new MapHolder());
// assertEquals("VALUE",o);

expression = (SpelExpression) parser.parseExpression("foo[T]");
o = expression.getValue(new MapHolder());
assertThat(o).isEqualTo("TV");
Expand All @@ -162,6 +155,10 @@ public void testMapKeysThatAreAlsoSpELKeywords() {
o = expression.getValue(new MapHolder());
assertThat(o).isEqualTo("tv");

expression = (SpelExpression) parser.parseExpression("foo['NEW']");
o = expression.getValue(new MapHolder());
assertThat(o).isEqualTo("VALUE");

expression = (SpelExpression) parser.parseExpression("foo[NEW]");
o = expression.getValue(new MapHolder());
assertThat(o).isEqualTo("VALUE");
Expand All @@ -174,35 +171,32 @@ public void testMapKeysThatAreAlsoSpELKeywords() {
o = expression.getValue(new MapHolder());
assertThat(o).isEqualTo("value");

expression = (SpelExpression)parser.parseExpression("foo[foo[NEW]]");
expression = (SpelExpression) parser.parseExpression("foo[foo[NEW]]");
o = expression.getValue(new MapHolder());
assertThat(o).isEqualTo("37");

expression = (SpelExpression)parser.parseExpression("foo[foo[new]]");
expression = (SpelExpression) parser.parseExpression("foo[foo[new]]");
o = expression.getValue(new MapHolder());
assertThat(o).isEqualTo("38");

expression = (SpelExpression)parser.parseExpression("foo[foo[foo[T]]]");
expression = (SpelExpression) parser.parseExpression("foo[foo[foo[T]]]");
o = expression.getValue(new MapHolder());
assertThat(o).isEqualTo("value");
}

@SuppressWarnings({ "rawtypes", "unchecked" })
public static class MapHolder {

public Map foo;

public MapHolder() {
foo = new HashMap();
foo.put("NEW", "VALUE");
foo.put("new", "value");
foo.put("T", "TV");
foo.put("t", "tv");
foo.put("abc.def", "value");
foo.put("VALUE","37");
foo.put("value","38");
foo.put("TV","new");
}
static class MapHolder {

public Map foo = Map.of(
"NEW", "VALUE",
"new", "value",
"T", "TV",
"t", "tv",
"abc.def", "value",
"VALUE","37",
"value","38",
"TV","new"
);
}

}

0 comments on commit 1e30997

Please sign in to comment.