From 051adf267fcbabbeb101c3c0f072a54c55820170 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 24 Sep 2025 16:52:19 +0200 Subject: [PATCH] Introduce MapAccessor in SpEL and deprecate existing implementation Prior to this commit, the MapAccessor for SpEL resided in the org.springframework.context.expression package in the spring-context module; however, it arguably should reside in the spring-expression module so that it can be used whenever SpEL is used (without requiring spring-context). This commit therefore officially deprecates the MapAccessor in spring-context for removal in favor of a new copy of the implementation in the org.springframework.expression.spel.support package in the spring-expression module. Closes gh-35537 --- .../context/expression/MapAccessor.java | 101 +--------- .../StandardBeanExpressionResolver.java | 2 +- .../context/expression/MapAccessorTests.java | 1 + .../expression/spel/support/MapAccessor.java} | 24 +-- .../spel/SpelCompilationCoverageTests.java | 11 +- .../spel/support/MapAccessorTests.java | 179 ++++++++++++++++++ .../support/SimpleEvaluationContextTests.java | 11 +- .../web/servlet/tags/EvalTag.java | 2 +- 8 files changed, 210 insertions(+), 121 deletions(-) rename spring-expression/src/{test/java/org/springframework/expression/spel/CompilableMapAccessor.java => main/java/org/springframework/expression/spel/support/MapAccessor.java} (84%) create mode 100644 spring-expression/src/test/java/org/springframework/expression/spel/support/MapAccessorTests.java diff --git a/spring-context/src/main/java/org/springframework/context/expression/MapAccessor.java b/spring-context/src/main/java/org/springframework/context/expression/MapAccessor.java index 3d08f3345b0..c8f19b0a1c8 100644 --- a/spring-context/src/main/java/org/springframework/context/expression/MapAccessor.java +++ b/spring-context/src/main/java/org/springframework/context/expression/MapAccessor.java @@ -16,18 +16,7 @@ package org.springframework.context.expression; -import java.util.Map; - -import org.jspecify.annotations.Nullable; - -import org.springframework.asm.MethodVisitor; -import org.springframework.expression.AccessException; -import org.springframework.expression.EvaluationContext; import org.springframework.expression.PropertyAccessor; -import org.springframework.expression.TypedValue; -import org.springframework.expression.spel.CodeFlow; -import org.springframework.expression.spel.CompilablePropertyAccessor; -import org.springframework.util.Assert; /** * SpEL {@link PropertyAccessor} that knows how to access the keys of a standard @@ -36,11 +25,10 @@ import org.springframework.util.Assert; * @author Juergen Hoeller * @author Andy Clement * @since 3.0 + * @deprecated as of Spring Framework 7.0 in favor of {@link org.springframework.expression.spel.support.MapAccessor}. */ -public class MapAccessor implements CompilablePropertyAccessor { - - private final boolean allowWrite; - +@Deprecated(since = "7.0", forRemoval = true) +public class MapAccessor extends org.springframework.expression.spel.support.MapAccessor { /** * Create a new {@code MapAccessor} for reading as well as writing. @@ -57,88 +45,7 @@ public class MapAccessor implements CompilablePropertyAccessor { * @see #canWrite */ public MapAccessor(boolean allowWrite) { - this.allowWrite = allowWrite; - } - - - @Override - public Class[] getSpecificTargetClasses() { - return new Class[] {Map.class}; - } - - @Override - public boolean canRead(EvaluationContext context, @Nullable Object target, String name) throws AccessException { - return (target instanceof Map map && map.containsKey(name)); - } - - @Override - public TypedValue read(EvaluationContext context, @Nullable Object target, String name) throws AccessException { - Assert.state(target instanceof Map, "Target must be of type Map"); - Map map = (Map) target; - Object value = map.get(name); - if (value == null && !map.containsKey(name)) { - throw new MapAccessException(name); - } - return new TypedValue(value); - } - - @Override - public boolean canWrite(EvaluationContext context, @Nullable Object target, String name) throws AccessException { - return (this.allowWrite && target instanceof Map); - } - - @Override - @SuppressWarnings("unchecked") - public void write(EvaluationContext context, @Nullable Object target, String name, @Nullable Object newValue) - throws AccessException { - - Assert.state(target instanceof Map, "Target must be of type Map"); - Map map = (Map) target; - map.put(name, newValue); - } - - @Override - public boolean isCompilable() { - return true; - } - - @Override - public Class getPropertyType() { - return Object.class; - } - - @Override - public void generateCode(String propertyName, MethodVisitor mv, CodeFlow cf) { - String descriptor = cf.lastDescriptor(); - if (descriptor == null || !descriptor.equals("Ljava/util/Map")) { - if (descriptor == null) { - cf.loadTarget(mv); - } - CodeFlow.insertCheckCast(mv, "Ljava/util/Map"); - } - mv.visitLdcInsn(propertyName); - mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Map", "get", "(Ljava/lang/Object;)Ljava/lang/Object;", true); - } - - - /** - * Exception thrown from {@code read} in order to reset a cached - * PropertyAccessor, allowing other accessors to have a try. - */ - @SuppressWarnings("serial") - private static class MapAccessException extends AccessException { - - private final String key; - - public MapAccessException(String key) { - super(""); - this.key = key; - } - - @Override - public String getMessage() { - return "Map does not contain a value for key '" + this.key + "'"; - } + super(allowWrite); } } diff --git a/spring-context/src/main/java/org/springframework/context/expression/StandardBeanExpressionResolver.java b/spring-context/src/main/java/org/springframework/context/expression/StandardBeanExpressionResolver.java index 20f46c91691..798d954e577 100644 --- a/spring-context/src/main/java/org/springframework/context/expression/StandardBeanExpressionResolver.java +++ b/spring-context/src/main/java/org/springframework/context/expression/StandardBeanExpressionResolver.java @@ -168,7 +168,7 @@ public class StandardBeanExpressionResolver implements BeanExpressionResolver { StandardEvaluationContext sec = new StandardEvaluationContext(bec); sec.addPropertyAccessor(new BeanExpressionContextAccessor()); sec.addPropertyAccessor(new BeanFactoryAccessor()); - sec.addPropertyAccessor(new MapAccessor()); + sec.addPropertyAccessor(new org.springframework.expression.spel.support.MapAccessor()); sec.addPropertyAccessor(new EnvironmentAccessor()); sec.setBeanResolver(new BeanFactoryResolver(beanFactory)); sec.setTypeLocator(new StandardTypeLocator(beanFactory.getBeanClassLoader())); 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 a9670c94dad..db873108a7b 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 @@ -36,6 +36,7 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType; * @author Andy Clement * @author Sam Brannen */ +@SuppressWarnings("removal") class MapAccessorTests { private final StandardEvaluationContext context = new StandardEvaluationContext(); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/CompilableMapAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/MapAccessor.java similarity index 84% rename from spring-expression/src/test/java/org/springframework/expression/spel/CompilableMapAccessor.java rename to spring-expression/src/main/java/org/springframework/expression/spel/support/MapAccessor.java index b0616336045..dec6f7db949 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/CompilableMapAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/MapAccessor.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package org.springframework.expression.spel; +package org.springframework.expression.spel.support; import java.util.Map; @@ -23,37 +23,39 @@ import org.jspecify.annotations.Nullable; import org.springframework.asm.MethodVisitor; import org.springframework.expression.AccessException; import org.springframework.expression.EvaluationContext; +import org.springframework.expression.PropertyAccessor; import org.springframework.expression.TypedValue; +import org.springframework.expression.spel.CodeFlow; +import org.springframework.expression.spel.CompilablePropertyAccessor; import org.springframework.util.Assert; /** - * This is a local COPY of {@link org.springframework.context.expression.MapAccessor}. + * SpEL {@link PropertyAccessor} that knows how to access the keys of a standard + * {@link java.util.Map}. * * @author Juergen Hoeller * @author Andy Clement - * @since 4.1 + * @since 7.0 */ -public class CompilableMapAccessor implements CompilablePropertyAccessor { +public class MapAccessor implements CompilablePropertyAccessor { private final boolean allowWrite; /** - * Create a new {@code CompilableMapAccessor} for reading as well as writing. - * @since 6.2 - * @see #CompilableMapAccessor(boolean) + * Create a new {@code MapAccessor} for reading as well as writing. + * @see #MapAccessor(boolean) */ - public CompilableMapAccessor() { + public MapAccessor() { this(true); } /** - * Create a new {@code CompilableMapAccessor} for reading and possibly also writing. + * Create a new {@code MapAccessor} for reading and possibly also writing. * @param allowWrite whether to allow write operations on a target instance - * @since 6.2 * @see #canWrite */ - public CompilableMapAccessor(boolean allowWrite) { + public MapAccessor(boolean allowWrite) { this.allowWrite = allowWrite; } 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 150897c5325..f0ab0fe7eb7 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 @@ -57,6 +57,7 @@ import org.springframework.expression.spel.ast.Ternary; 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.MapAccessor; import org.springframework.expression.spel.support.ReflectiveIndexAccessor; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.testdata.PersonInOtherPackage; @@ -618,9 +619,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } @Test // gh-32356 - void indexIntoMapOfPrimitiveIntArrayWithCompilableMapAccessor() { + void indexIntoMapOfPrimitiveIntArrayWithMapAccessor() { StandardEvaluationContext context = new StandardEvaluationContext(); - context.addPropertyAccessor(new CompilableMapAccessor()); + context.addPropertyAccessor(new MapAccessor()); Map map = Map.of("foo", new int[] { 1, 2, 3 }); @@ -635,21 +636,21 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertThat(stringify(expression.getValue(context, map))).isEqualTo("1 2 3"); assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object"); - // custom CompilableMapAccessor via implicit #root & array index + // custom MapAccessor via implicit #root & array index expression = parser.parseExpression("foo[1]"); assertThat(expression.getValue(context, map)).isEqualTo(2); assertCanCompile(expression); assertThat(expression.getValue(context, map)).isEqualTo(2); - // custom CompilableMapAccessor via explicit #root & array index + // custom MapAccessor via explicit #root & array index expression = parser.parseExpression("#root.foo[1]"); assertThat(expression.getValue(context, map)).isEqualTo(2); assertCanCompile(expression); assertThat(expression.getValue(context, map)).isEqualTo(2); - // custom CompilableMapAccessor via explicit #this & array index + // custom MapAccessor via explicit #this & array index expression = parser.parseExpression("#this.foo[1]"); assertThat(expression.getValue(context, map)).isEqualTo(2); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/support/MapAccessorTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/support/MapAccessorTests.java new file mode 100644 index 00000000000..6aa73efc18a --- /dev/null +++ b/spring-expression/src/test/java/org/springframework/expression/spel/support/MapAccessorTests.java @@ -0,0 +1,179 @@ +/* + * Copyright 2002-present 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.support; + +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import org.springframework.expression.AccessException; +import org.springframework.expression.spel.standard.SpelCompiler; +import org.springframework.expression.spel.standard.SpelExpressionParser; + +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() { + context.addPropertyAccessor(new MapAccessor()); + + var parser = new SpelExpressionParser(); + var testMap = getSimpleTestMap(); + var nestedMap = getNestedTestMap(); + + // basic + 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 + 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 + 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 + expression = parser.parseExpression("getMap().foo"); + MapGetter mapGetter = new MapGetter(); + assertThat(expression.getValue(context, mapGetter)).isEqualTo("bar"); + assertThat(SpelCompiler.compile(expression)).isTrue(); + assertThat(expression.getValue(context, mapGetter)).isEqualTo("bar"); + + // basic isWritable + expression = parser.parseExpression("foo"); + assertThat(expression.isWritable(context, testMap)).isTrue(); + + // basic write + 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 AccessException { + var mapAccessor = new MapAccessor(); + var testMap = getSimpleTestMap(); + + 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. + assertThat(mapAccessor.canWrite(context, Map.of(), "x")).isTrue(); + + mapAccessor = new MapAccessor(false); + assertThat(mapAccessor.canWrite(context, new Object(), "foo")).isFalse(); + assertThat(mapAccessor.canWrite(context, testMap, "foo")).isFalse(); + } + + @Test + void isWritable() { + var testMap = getSimpleTestMap(); + var parser = new SpelExpressionParser(); + var expression = parser.parseExpression("foo"); + + assertThat(expression.isWritable(context, testMap)).isFalse(); + + context.setPropertyAccessors(List.of(new MapAccessor(true))); + assertThat(expression.isWritable(context, testMap)).isTrue(); + + context.setPropertyAccessors(List.of(new MapAccessor(false))); + assertThat(expression.isWritable(context, testMap)).isFalse(); + } + + + @SuppressWarnings({"rawtypes", "unchecked"}) + public static class MapGetter { + Map map = new HashMap<>(); + + public MapGetter() { + this.map.put("foo", "bar"); + } + + public Map getMap() { + return this.map; + } + } + + 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<>(); + map2.put("aaa", map); + return map2; + } + +} diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java index 29e89fb2843..f3e5a2cfa3a 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/support/SimpleEvaluationContextTests.java @@ -26,7 +26,6 @@ import org.junit.jupiter.api.Test; import org.springframework.expression.Expression; import org.springframework.expression.IndexAccessor; -import org.springframework.expression.spel.CompilableMapAccessor; import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.expression.spel.SpelMessage; import org.springframework.expression.spel.standard.SpelExpressionParser; @@ -112,13 +111,13 @@ class SimpleEvaluationContextTests { @Test void forPropertyAccessorsInReadWriteMode() { SimpleEvaluationContext context = SimpleEvaluationContext - .forPropertyAccessors(new CompilableMapAccessor(), DataBindingPropertyAccessor.forReadWriteAccess()) + .forPropertyAccessors(new MapAccessor(), DataBindingPropertyAccessor.forReadWriteAccess()) .withIndexAccessors(colorsIndexAccessor) .build(); assertReadWriteMode(context); - // Map -- with key as property name supported by CompilableMapAccessor + // Map -- with key as property name supported by MapAccessor Expression expression; expression = parser.parseExpression("map.yellow"); @@ -138,13 +137,13 @@ class SimpleEvaluationContextTests { @Test void forPropertyAccessorsInMixedReadOnlyMode() { SimpleEvaluationContext context = SimpleEvaluationContext - .forPropertyAccessors(new CompilableMapAccessor(true), DataBindingPropertyAccessor.forReadOnlyAccess()) + .forPropertyAccessors(new MapAccessor(true), DataBindingPropertyAccessor.forReadOnlyAccess()) .withIndexAccessors(colorsIndexAccessor) .build(); assertCommonReadOnlyModeBehavior(context); - // Map -- with key as property name supported by CompilableMapAccessor with allowWrite = true. + // Map -- with key as property name supported by MapAccessor with allowWrite = true. Expression expression; expression = parser.parseExpression("map.yellow"); @@ -214,7 +213,7 @@ class SimpleEvaluationContextTests { @Test void forPropertyAccessorsWithAssignmentDisabled() { SimpleEvaluationContext context = SimpleEvaluationContext - .forPropertyAccessors(new CompilableMapAccessor(false), DataBindingPropertyAccessor.forReadOnlyAccess()) + .forPropertyAccessors(new MapAccessor(false), DataBindingPropertyAccessor.forReadOnlyAccess()) .withIndexAccessors(colorsIndexAccessor) .withAssignmentDisabled() .build(); diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/EvalTag.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/EvalTag.java index 6b6b3590d7f..faeb9baefe1 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/EvalTag.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/tags/EvalTag.java @@ -25,7 +25,6 @@ import org.jspecify.annotations.Nullable; import org.springframework.context.expression.BeanFactoryResolver; import org.springframework.context.expression.EnvironmentAccessor; -import org.springframework.context.expression.MapAccessor; import org.springframework.core.convert.ConversionService; import org.springframework.expression.AccessException; import org.springframework.expression.EvaluationContext; @@ -34,6 +33,7 @@ import org.springframework.expression.ExpressionParser; import org.springframework.expression.PropertyAccessor; import org.springframework.expression.TypedValue; import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.support.MapAccessor; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.support.StandardTypeConverter; import org.springframework.util.ObjectUtils;