From 27b2243b43117e59ddc1d0f99e3f48e4ced632df Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 24 Sep 2025 16:06:37 +0200 Subject: [PATCH] Test status quo for canRead() and read() in SpEL MapAccessor See gh-35534 --- .../context/expression/MapAccessorTests.java | 134 +++++++++++------- 1 file changed, 85 insertions(+), 49 deletions(-) diff --git a/spring-context/src/test/java/org/springframework/context/expression/MapAccessorTests.java b/spring-context/src/test/java/org/springframework/context/expression/MapAccessorTests.java index a9a98f9c5aa..a9670c94dad 100644 --- a/spring-context/src/test/java/org/springframework/context/expression/MapAccessorTests.java +++ b/spring-context/src/test/java/org/springframework/context/expression/MapAccessorTests.java @@ -22,71 +22,108 @@ import java.util.Map; import org.junit.jupiter.api.Test; -import org.springframework.expression.Expression; +import org.springframework.expression.AccessException; import org.springframework.expression.spel.standard.SpelCompiler; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * Tests for {@link MapAccessor}. * * @author Andy Clement + * @author Sam Brannen */ class MapAccessorTests { + private final StandardEvaluationContext context = new StandardEvaluationContext(); + + @Test void compilationSupport() { - Map testMap = getSimpleTestMap(); - StandardEvaluationContext sec = new StandardEvaluationContext(); - sec.addPropertyAccessor(new MapAccessor()); - SpelExpressionParser sep = new SpelExpressionParser(); + context.addPropertyAccessor(new MapAccessor()); + + var parser = new SpelExpressionParser(); + var testMap = getSimpleTestMap(); + var nestedMap = getNestedTestMap(); // basic - Expression ex = sep.parseExpression("foo"); - assertThat(ex.getValue(sec, testMap)).isEqualTo("bar"); - assertThat(SpelCompiler.compile(ex)).isTrue(); - assertThat(ex.getValue(sec, testMap)).isEqualTo("bar"); + var expression = parser.parseExpression("foo"); + assertThat(expression.getValue(context, testMap)).isEqualTo("bar"); + assertThat(SpelCompiler.compile(expression)).isTrue(); + assertThat(expression.getValue(context, testMap)).isEqualTo("bar"); // compound expression - ex = sep.parseExpression("foo.toUpperCase()"); - assertThat(ex.getValue(sec, testMap)).isEqualTo("BAR"); - assertThat(SpelCompiler.compile(ex)).isTrue(); - assertThat(ex.getValue(sec, testMap)).isEqualTo("BAR"); + expression = parser.parseExpression("foo.toUpperCase()"); + assertThat(expression.getValue(context, testMap)).isEqualTo("BAR"); + assertThat(SpelCompiler.compile(expression)).isTrue(); + assertThat(expression.getValue(context, testMap)).isEqualTo("BAR"); // nested map - Map> nestedMap = getNestedTestMap(); - ex = sep.parseExpression("aaa.foo.toUpperCase()"); - assertThat(ex.getValue(sec, nestedMap)).isEqualTo("BAR"); - assertThat(SpelCompiler.compile(ex)).isTrue(); - assertThat(ex.getValue(sec, nestedMap)).isEqualTo("BAR"); + expression = parser.parseExpression("aaa.foo.toUpperCase()"); + assertThat(expression.getValue(context, nestedMap)).isEqualTo("BAR"); + assertThat(SpelCompiler.compile(expression)).isTrue(); + assertThat(expression.getValue(context, nestedMap)).isEqualTo("BAR"); // avoiding inserting checkcast because first part of expression returns a Map - ex = sep.parseExpression("getMap().foo"); + expression = parser.parseExpression("getMap().foo"); MapGetter mapGetter = new MapGetter(); - assertThat(ex.getValue(sec, mapGetter)).isEqualTo("bar"); - assertThat(SpelCompiler.compile(ex)).isTrue(); - assertThat(ex.getValue(sec, mapGetter)).isEqualTo("bar"); + assertThat(expression.getValue(context, mapGetter)).isEqualTo("bar"); + assertThat(SpelCompiler.compile(expression)).isTrue(); + assertThat(expression.getValue(context, mapGetter)).isEqualTo("bar"); // basic isWritable - ex = sep.parseExpression("foo"); - assertThat(ex.isWritable(sec, testMap)).isTrue(); + expression = parser.parseExpression("foo"); + assertThat(expression.isWritable(context, testMap)).isTrue(); // basic write - ex = sep.parseExpression("foo2"); - ex.setValue(sec, testMap, "bar2"); - assertThat(ex.getValue(sec, testMap)).isEqualTo("bar2"); - assertThat(SpelCompiler.compile(ex)).isTrue(); - assertThat(ex.getValue(sec, testMap)).isEqualTo("bar2"); + expression = parser.parseExpression("foo2"); + expression.setValue(context, testMap, "bar2"); + assertThat(expression.getValue(context, testMap)).isEqualTo("bar2"); + assertThat(SpelCompiler.compile(expression)).isTrue(); + assertThat(expression.getValue(context, testMap)).isEqualTo("bar2"); } @Test - void canWrite() throws Exception { - StandardEvaluationContext context = new StandardEvaluationContext(); - Map testMap = getSimpleTestMap(); + void canReadForNonMap() throws AccessException { + var mapAccessor = new MapAccessor(); + + assertThat(mapAccessor.canRead(context, new Object(), "foo")).isFalse(); + } + + @Test + void canReadAndReadForExistingKeys() throws AccessException { + var mapAccessor = new MapAccessor(); + var map = new HashMap<>(); + map.put("foo", null); + map.put("bar", "baz"); + + assertThat(mapAccessor.canRead(context, map, "foo")).isTrue(); + assertThat(mapAccessor.canRead(context, map, "bar")).isTrue(); + + assertThat(mapAccessor.read(context, map, "foo").getValue()).isNull(); + assertThat(mapAccessor.read(context, map, "bar").getValue()).isEqualTo("baz"); + } + + @Test + void canReadAndReadForNonexistentKeys() throws AccessException { + var mapAccessor = new MapAccessor(); + var map = Map.of(); + + assertThat(mapAccessor.canRead(context, map, "XXX")).isFalse(); + + assertThatExceptionOfType(AccessException.class) + .isThrownBy(() -> mapAccessor.read(context, map, "XXX").getValue()) + .withMessage("Map does not contain a value for key 'XXX'"); + } + + @Test + void canWrite() throws AccessException { + var mapAccessor = new MapAccessor(); + var testMap = getSimpleTestMap(); - MapAccessor mapAccessor = new MapAccessor(); assertThat(mapAccessor.canWrite(context, new Object(), "foo")).isFalse(); assertThat(mapAccessor.canWrite(context, testMap, "foo")).isTrue(); // Cannot actually write to an immutable Map, but MapAccessor cannot easily check for that. @@ -99,18 +136,17 @@ class MapAccessorTests { @Test void isWritable() { - Map testMap = getSimpleTestMap(); - StandardEvaluationContext sec = new StandardEvaluationContext(); - SpelExpressionParser sep = new SpelExpressionParser(); - Expression ex = sep.parseExpression("foo"); + var testMap = getSimpleTestMap(); + var parser = new SpelExpressionParser(); + var expression = parser.parseExpression("foo"); - assertThat(ex.isWritable(sec, testMap)).isFalse(); + assertThat(expression.isWritable(context, testMap)).isFalse(); - sec.setPropertyAccessors(List.of(new MapAccessor(true))); - assertThat(ex.isWritable(sec, testMap)).isTrue(); + context.setPropertyAccessors(List.of(new MapAccessor(true))); + assertThat(expression.isWritable(context, testMap)).isTrue(); - sec.setPropertyAccessors(List.of(new MapAccessor(false))); - assertThat(ex.isWritable(sec, testMap)).isFalse(); + context.setPropertyAccessors(List.of(new MapAccessor(false))); + assertThat(expression.isWritable(context, testMap)).isFalse(); } @@ -127,16 +163,16 @@ class MapAccessorTests { } } - private static Map getSimpleTestMap() { - Map map = new HashMap<>(); - map.put("foo","bar"); + private static Map getSimpleTestMap() { + Map map = new HashMap<>(); + map.put("foo", "bar"); return map; } - private static Map> getNestedTestMap() { - Map map = new HashMap<>(); - map.put("foo","bar"); - Map> map2 = new HashMap<>(); + private static Map> getNestedTestMap() { + Map map = new HashMap<>(); + map.put("foo", "bar"); + Map> map2 = new HashMap<>(); map2.put("aaa", map); return map2; }