Test status quo for canRead() and read() in SpEL MapAccessor

See gh-35534

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<String, Object> 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<String, Map<String, Object>> 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 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 Exception {
-		StandardEvaluationContext context = new StandardEvaluationContext();
-		Map<String, Object> testMap = getSimpleTestMap();
+	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<String, Object> 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<String,Object> getSimpleTestMap() {
-		Map<String,Object> map = new HashMap<>();
-		map.put("foo","bar");
+	private static Map<String, Object> getSimpleTestMap() {
+		Map<String, Object> map = new HashMap<>();
+		map.put("foo", "bar");
 		return map;
 	}
 
-	private static Map<String,Map<String,Object>> getNestedTestMap() {
-		Map<String,Object> map = new HashMap<>();
-		map.put("foo","bar");
-		Map<String,Map<String,Object>> map2 = new HashMap<>();
+	private static Map<String, Map<String, Object>> getNestedTestMap() {
+		Map<String, Object> map = new HashMap<>();
+		map.put("foo", "bar");
+		Map<String, Map<String, Object>> map2 = new HashMap<>();
 		map2.put("aaa", map);
 		return map2;
 	}