From 35c183d6346d68f443b6d06757f749a3d415bd88 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 26 Apr 2024 16:13:05 +0300 Subject: [PATCH] Introduce ReflectiveIndexAccessor in SpEL This commit introduces ReflectiveIndexAccessor for the Spring Expression Language (SpEL) which is somewhat analogous to the ReflectivePropertyAccessor implementation of PropertyAccessor. ReflectiveIndexAccessor is a flexible IndexAccessor implementation that uses reflection to read from and optionally write to an indexed structure of a target object. ReflectiveIndexAccessor also implements CompilableIndexAccessor in order to support compilation to bytecode for read access. For example, the following creates a read-write IndexAccessor for a FruitMap type that is indexed by Color, including built-in support for compilation to bytecode for read access. IndexAccessor indexAccessor = new ReflectiveIndexAccessor( FruitMap.class, Color.class, "getFruit", "setFruit"); Closes gh-32714 --- .../spel/support/ReflectiveIndexAccessor.java | 283 ++++++++++++++++++ .../expression/spel/PublicInterface.java | 2 + .../expression/spel/PublicSuperclass.java | 8 + .../spel/SpelCompilationCoverageTests.java | 272 ++++++++--------- .../support/ReflectiveIndexAccessorTests.java | 129 ++++++++ 5 files changed, 541 insertions(+), 153 deletions(-) create mode 100644 spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java create mode 100644 spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectiveIndexAccessorTests.java diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java new file mode 100644 index 00000000000..abf0ec7505a --- /dev/null +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectiveIndexAccessor.java @@ -0,0 +1,283 @@ +/* + * Copyright 2002-2024 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.lang.reflect.Method; +import java.lang.reflect.Modifier; + +import org.springframework.asm.MethodVisitor; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.IndexAccessor; +import org.springframework.expression.TypedValue; +import org.springframework.expression.spel.CodeFlow; +import org.springframework.expression.spel.CompilableIndexAccessor; +import org.springframework.expression.spel.SpelNode; +import org.springframework.lang.Nullable; +import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; + +/** + * A flexible {@link org.springframework.expression.IndexAccessor IndexAccessor} + * that uses reflection to read from and optionally write to an indexed structure + * of a target object. + * + *

The indexed structure can be accessed through a public read-method (when + * being read) or a public write-method (when being written). The relationship + * between the read-method and write-method is based on a convention that is + * applicable for typical implementations of indexed structures. See the example + * below for details. + * + *

{@code ReflectiveIndexAccessor} also implements {@link CompilableIndexAccessor} + * in order to support compilation to bytecode for read access. Note, however, + * that the configured read-method must be invokable via a public class or public + * interface for compilation to succeed. + * + *

Example

+ * + *

The {@code FruitMap} class (the {@code targetType}) represents a structure + * that is indexed via the {@code Color} enum (the {@code indexType}). The name + * of the read-method is {@code "getFruit"}, and that method returns a + * {@code String} (the {@code indexedValueType}). The name of the write-method + * is {@code "setFruit"}, and that method accepts a {@code Color} enum (the + * {@code indexType}) and a {@code String} (the {@code indexedValueType} which + * must match the return type of the read-method). + * + *

A read-only {@code IndexAccessor} for {@code FruitMap} can be created via + * {@code new ReflectiveIndexAccessor(FruitMap.class, Color.class, "getFruit")}. + * With that accessor registered and a {@code FruitMap} registered as a variable + * named {@code #fruitMap}, the SpEL expression {@code #fruitMap[T(example.Color).RED]} + * will evaluate to {@code "cherry"}. + * + *

A read-write {@code IndexAccessor} for {@code FruitMap} can be created via + * {@code new ReflectiveIndexAccessor(FruitMap.class, Color.class, "getFruit", "setFruit")}. + * With that accessor registered and a {@code FruitMap} registered as a variable + * named {@code #fruitMap}, the SpEL expression + * {@code #fruitMap[T(example.Color).RED] = 'strawberry'} can be used to change + * the fruit mapping for the color red from {@code "cherry"} to {@code "strawberry"}. + * + *

+ * package example;
+ *
+ * public enum Color {
+ *     RED, ORANGE, YELLOW
+ * }
+ * + *
+ * public class FruitMap {
+ *
+ *     private final Map<Color, String> map = new HashMap<>();
+ *
+ *     public FruitMap() {
+ *         this.map.put(Color.RED, "cherry");
+ *         this.map.put(Color.ORANGE, "orange");
+ *         this.map.put(Color.YELLOW, "banana");
+ *     }
+ *
+ *     public String getFruit(Color color) {
+ *         return this.map.get(color);
+ *     }
+ *
+ *     public void setFruit(Color color, String fruit) {
+ *         this.map.put(color, fruit);
+ *     }
+ * }
+ * + * @author Sam Brannen + * @since 6.2 + * @see IndexAccessor + * @see CompilableIndexAccessor + * @see StandardEvaluationContext + * @see SimpleEvaluationContext + */ +public class ReflectiveIndexAccessor implements CompilableIndexAccessor { + + private final Class targetType; + + private final Class indexType; + + private final Method readMethod; + + private final Method readMethodToInvoke; + + @Nullable + private final Method writeMethodToInvoke; + + + /** + * Construct a new {@code ReflectiveIndexAccessor} for read-only access. + *

See {@linkplain ReflectiveIndexAccessor class-level documentation} for + * further details and an example. + * @param targetType the type of indexed structure which serves as the target + * of index operations + * @param indexType the type of index used to read from the indexed structure + * @param readMethodName the name of the method used to read from the indexed + * structure + */ + public ReflectiveIndexAccessor(Class targetType, Class indexType, String readMethodName) { + this(targetType, indexType, readMethodName, null); + } + + /** + * Construct a new {@code ReflectiveIndexAccessor} for read-write access. + *

See {@linkplain ReflectiveIndexAccessor class-level documentation} for + * further details and an example. + * @param targetType the type of indexed structure which serves as the target + * of index operations + * @param indexType the type of index used to read from or write to the indexed + * structure + * @param readMethodName the name of the method used to read from the indexed + * structure + * @param writeMethodName the name of the method used to write to the indexed + * structure, or {@code null} if writing is not supported + */ + public ReflectiveIndexAccessor(Class targetType, Class indexType, String readMethodName, + @Nullable String writeMethodName) { + + this.targetType = targetType; + this.indexType = indexType; + + try { + this.readMethod = targetType.getMethod(readMethodName, indexType); + } + catch (Exception ex) { + throw new IllegalArgumentException("Failed to find public read-method '%s(%s)' in class '%s'." + .formatted(readMethodName, getName(indexType), getName(targetType))); + } + + this.readMethodToInvoke = ClassUtils.getInterfaceMethodIfPossible(this.readMethod, targetType); + ReflectionUtils.makeAccessible(this.readMethodToInvoke); + + if (writeMethodName != null) { + Class indexedValueType = this.readMethod.getReturnType(); + Method writeMethod; + try { + writeMethod = targetType.getMethod(writeMethodName, indexType, indexedValueType); + } + catch (Exception ex) { + throw new IllegalArgumentException("Failed to find public write-method '%s(%s, %s)' in class '%s'." + .formatted(writeMethodName, getName(indexType), getName(indexedValueType), + getName(targetType))); + } + this.writeMethodToInvoke = ClassUtils.getInterfaceMethodIfPossible(writeMethod, targetType); + ReflectionUtils.makeAccessible(this.writeMethodToInvoke); + } + else { + this.writeMethodToInvoke = null; + } + } + + + /** + * Return an array containing the {@code targetType} configured via the constructor. + */ + @Override + public Class[] getSpecificTargetClasses() { + return new Class[] { this.targetType }; + } + + /** + * Return {@code true} if the supplied {@code target} and {@code index} can + * be assigned to the {@code targetType} and {@code indexType} configured + * via the constructor. + *

Considers primitive wrapper classes as assignable to the corresponding + * primitive types. + */ + @Override + public boolean canRead(EvaluationContext context, Object target, Object index) { + return (ClassUtils.isAssignableValue(this.targetType, target) && + ClassUtils.isAssignableValue(this.indexType, index)); + } + + /** + * Invoke the configured read-method via reflection and return the result + * wrapped in a {@link TypedValue}. + */ + @Override + public TypedValue read(EvaluationContext context, Object target, Object index) { + Object value = ReflectionUtils.invokeMethod(this.readMethodToInvoke, target, index); + return new TypedValue(value); + } + + /** + * Return {@code true} if a write-method has been configured and + * {@link #canRead} returns {@code true} for the same arguments. + */ + @Override + public boolean canWrite(EvaluationContext context, Object target, Object index) { + return (this.writeMethodToInvoke != null && canRead(context, target, index)); + } + + /** + * Invoke the configured write-method via reflection. + *

Should only be invoked if {@link #canWrite} returns {@code true} for the + * same arguments. + */ + @Override + public void write(EvaluationContext context, Object target, Object index, @Nullable Object newValue) { + Assert.state(this.writeMethodToInvoke != null, "Write-method cannot be null"); + ReflectionUtils.invokeMethod(this.writeMethodToInvoke, target, index, newValue); + } + + @Override + public boolean isCompilable() { + return true; + } + + /** + * Get the return type of the configured read-method. + */ + @Override + public Class getIndexedValueType() { + return this.readMethod.getReturnType(); + } + + @Override + public void generateCode(SpelNode index, MethodVisitor mv, CodeFlow cf) { + // Find the public declaring class. + Class publicDeclaringClass = this.readMethodToInvoke.getDeclaringClass(); + if (!Modifier.isPublic(publicDeclaringClass.getModifiers())) { + publicDeclaringClass = CodeFlow.findPublicDeclaringClass(this.readMethod); + } + Assert.state(publicDeclaringClass != null && Modifier.isPublic(publicDeclaringClass.getModifiers()), + () -> "Failed to find public declaring class for read-method: " + this.readMethod); + String classDesc = publicDeclaringClass.getName().replace('.', '/'); + + // Ensure the current object on the stack is the required type. + String lastDesc = cf.lastDescriptor(); + if (lastDesc == null || !classDesc.equals(lastDesc.substring(1))) { + mv.visitTypeInsn(CHECKCAST, classDesc); + } + + // Push the index onto the stack. + cf.generateCodeForArgument(mv, index, this.indexType); + + // Invoke the read-method. + String methodName = this.readMethod.getName(); + String methodDescr = CodeFlow.createSignatureDescriptor(this.readMethod); + boolean isInterface = publicDeclaringClass.isInterface(); + int opcode = (isInterface ? INVOKEINTERFACE : INVOKEVIRTUAL); + mv.visitMethodInsn(opcode, classDesc, methodName, methodDescr, isInterface); + } + + + private static String getName(Class clazz) { + String canonicalName = clazz.getCanonicalName(); + return (canonicalName != null ? canonicalName : clazz.getName()); + } + +} diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/PublicInterface.java b/spring-expression/src/test/java/org/springframework/expression/spel/PublicInterface.java index 587a86769b0..6272b9b172c 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/PublicInterface.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/PublicInterface.java @@ -23,4 +23,6 @@ public interface PublicInterface { String getText(); + String getFruit(int index); + } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/PublicSuperclass.java b/spring-expression/src/test/java/org/springframework/expression/spel/PublicSuperclass.java index 0c8a2f8d54b..f6b7517e03e 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/PublicSuperclass.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/PublicSuperclass.java @@ -37,4 +37,12 @@ public class PublicSuperclass { return "Super, " + name; } + public String getIndex(int index) { + return "value-" + index; + } + + public String getIndex2(int index) { + return "value-" + (2 * index); + } + } 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 9b714f6ed1f..d664d1f7974 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 @@ -54,12 +54,11 @@ 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.ReflectiveIndexAccessor; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.testdata.PersonInOtherPackage; import org.springframework.expression.spel.testresources.Person; import org.springframework.lang.Nullable; -import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; import static java.util.stream.Collectors.joining; @@ -131,6 +130,7 @@ import static org.springframework.expression.spel.standard.SpelExpressionTestUti * @author Sam Brannen * @since 4.1 * @see org.springframework.expression.spel.standard.SpelCompilerTests + * @see org.springframework.expression.spel.support.ReflectiveIndexAccessorTests */ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @@ -823,6 +823,12 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertCanCompile(expression); assertThat(expression.getValue(context, colors)).isEqualTo(Color.ORANGE); assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor); + + // Set color at index 3. + expression.setValue(context, colors, Color.RED); + assertCanCompile(expression); + assertThat(expression.getValue(context, colors)).isEqualTo(Color.RED); + assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor); } @Test @@ -1321,29 +1327,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { result = expression.getValue(context, privateSubclass, Integer.class); assertThat(result).isEqualTo(2); } - - private interface PrivateInterface { - - String getMessage(); - } - - private static class PrivateSubclass extends PublicSuperclass implements PublicInterface, PrivateInterface { - - @Override - public int getNumber() { - return 2; - } - - @Override - public String getText() { - return "enigma"; - } - - @Override - public String getMessage() { - return "hello"; - } - } } @Nested @@ -1374,7 +1357,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @Test void packagePrivateSubclassOverridesMethodInPrivateInterface() { expression = parser.parseExpression("greet('Jane')"); - PrivateSubclass privateSubclass = new PrivateSubclass(); + LocalPrivateSubclass privateSubclass = new LocalPrivateSubclass(); // Prerequisite: type must not be public for this use case. assertNotPublic(privateSubclass.getClass()); @@ -1390,7 +1373,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @Test void privateSubclassOverridesMethodInPublicSuperclass() { expression = parser.parseExpression("process(2)"); - PrivateSubclass privateSubclass = new PrivateSubclass(); + LocalPrivateSubclass privateSubclass = new LocalPrivateSubclass(); // Prerequisite: type must not be public for this use case. assertNotPublic(privateSubclass.getClass()); @@ -1403,12 +1386,14 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertThat(result).isEqualTo(2 * 2); } - private interface PrivateInterface { + // Cannot be named PrivateInterface due to issues with the Kotlin compiler. + private interface LocalPrivateInterface { String greet(String name); } - private static class PrivateSubclass extends PublicSuperclass implements PrivateInterface { + // Cannot be named PrivateSubclass due to issues with the Kotlin compiler. + private static class LocalPrivateSubclass extends PublicSuperclass implements LocalPrivateInterface { @Override public int process(int num) { @@ -1422,6 +1407,61 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } } + @Nested + class ReflectiveIndexAccessorVisibilityTests { + + @Test + void privateSubclassOverridesIndexReadMethodInPublicInterface() { + PrivateSubclass privateSubclass = new PrivateSubclass(); + Class targetType = privateSubclass.getClass(); + assertNotPublic(targetType); + + context.addIndexAccessor(new ReflectiveIndexAccessor(targetType, int.class, "getFruit")); + expression = parser.parseExpression("[1]"); + + String result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("fruit-1"); + + assertCanCompile(expression); + result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("fruit-1"); + } + + @Test + void privateSubclassOverridesIndexReadMethodInPrivateInterface() { + PrivateSubclass privateSubclass = new PrivateSubclass(); + Class targetType = privateSubclass.getClass(); + assertNotPublic(targetType); + + context.addIndexAccessor(new ReflectiveIndexAccessor(targetType, int.class, "getIndex")); + expression = parser.parseExpression("[1]"); + + String result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("value-1"); + + assertCanCompile(expression); + result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("value-1"); + } + + @Test + void privateSubclassOverridesIndexReadMethodInPublicSuperclass() { + PrivateSubclass privateSubclass = new PrivateSubclass(); + Class targetType = privateSubclass.getClass(); + assertNotPublic(targetType); + + context.addIndexAccessor(new ReflectiveIndexAccessor(targetType, int.class, "getIndex2")); + expression = parser.parseExpression("[2]"); + + String result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("sub-4"); // 2 * 2 + + assertCanCompile(expression); + result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("sub-4"); // 2 * 2 + } + } + @Test void typeReference() { @@ -7234,129 +7274,50 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { } } - // Must be public with public fields/properties. - public static class RootContextWithIndexedProperties { - public int[] intArray; - public Number[] numberArray; - public List list; - public Set set; - public String string; - public Map map; - public Person person; - } - - /** - * {@link CompilableIndexAccessor} that uses reflection to invoke the - * configured read-method for index access operations. - */ - static class ReflectiveIndexAccessor implements CompilableIndexAccessor { - - private final Class targetType; - - private final Class indexType; - - private final Method readMethod; - - private final Method readMethodToInvoke; - - @Nullable - private final Method writeMethodToInvoke; - - private final String targetTypeDesc; - - private final String methodDescr; - - - public ReflectiveIndexAccessor(Class targetType, Class indexType, String readMethodName, - @Nullable String writeMethodName) { - - this.targetType = targetType; - this.indexType = indexType; - this.readMethod = ReflectionUtils.findMethod(targetType, readMethodName, indexType); - Assert.notNull(this.readMethod, () -> "Failed to find read-method '%s(%s)' in class '%s'." - .formatted(readMethodName, indexType.getTypeName(), targetType.getTypeName())); - this.readMethodToInvoke = ClassUtils.getInterfaceMethodIfPossible(this.readMethod, targetType); - if (writeMethodName != null) { - Class indexedValueType = this.readMethod.getReturnType(); - Method writeMethod = ReflectionUtils.findMethod(targetType, writeMethodName, indexType, indexedValueType); - Assert.notNull(writeMethod, () -> "Failed to find write-method '%s(%s, %s)' in class '%s'." - .formatted(writeMethodName, indexType.getTypeName(), indexedValueType.getTypeName(), - targetType.getTypeName())); - this.writeMethodToInvoke = ClassUtils.getInterfaceMethodIfPossible(writeMethod, targetType); - } - else { - this.writeMethodToInvoke = null; - } - this.targetTypeDesc = CodeFlow.toDescriptor(targetType); - this.methodDescr = CodeFlow.createSignatureDescriptor(this.readMethod); - } + private interface PrivateInterface { + String getMessage(); - @Override - public Class[] getSpecificTargetClasses() { - return new Class[] { this.targetType }; - } + String getIndex(int index); + } - @Override - public boolean canRead(EvaluationContext context, Object target, Object index) { - return (ClassUtils.isAssignableValue(this.targetType, target) && - ClassUtils.isAssignableValue(this.indexType, index)); - } + private static class PrivateSubclass extends PublicSuperclass implements PublicInterface, PrivateInterface { @Override - public TypedValue read(EvaluationContext context, Object target, Object index) { - ReflectionUtils.makeAccessible(this.readMethodToInvoke); - Object value = ReflectionUtils.invokeMethod(this.readMethodToInvoke, target, index); - return new TypedValue(value); + public int getNumber() { + return 2; } @Override - public boolean canWrite(EvaluationContext context, Object target, Object index) { - return (this.writeMethodToInvoke != null && canRead(context, target, index)); + public String getText() { + return "enigma"; } @Override - public void write(EvaluationContext context, Object target, Object index, @Nullable Object newValue) { - Assert.state(this.writeMethodToInvoke != null, "Write-method cannot be null"); - ReflectionUtils.invokeMethod(this.writeMethodToInvoke, target, index, newValue); + public String getMessage() { + return "hello"; } @Override - public boolean isCompilable() { - return true; + public String getIndex2(int index) { + return "sub-" + (2 * index); } @Override - public Class getIndexedValueType() { - return this.readMethod.getReturnType(); + public String getFruit(int index) { + return "fruit-" + index; } + } - @Override - public void generateCode(SpelNode index, MethodVisitor mv, CodeFlow cf) { - // Determine the public declaring class. - Class publicDeclaringClass = this.readMethodToInvoke.getDeclaringClass(); - if (!Modifier.isPublic(publicDeclaringClass.getModifiers()) && this.readMethod != null) { - publicDeclaringClass = CodeFlow.findPublicDeclaringClass(this.readMethod); - } - Assert.state(publicDeclaringClass != null && Modifier.isPublic(publicDeclaringClass.getModifiers()), - () -> "Failed to find public declaring class for: " + this.readMethod); - - // Ensure the current object on the stack is the target type. - String lastDesc = cf.lastDescriptor(); - if (lastDesc == null || !lastDesc.equals(this.targetTypeDesc)) { - CodeFlow.insertCheckCast(mv, this.targetTypeDesc); - } - - // Push the index onto the stack. - cf.generateCodeForArgument(mv, index, this.indexType); - - // Invoke the read method. - String classDesc = publicDeclaringClass.getName().replace('.', '/'); - boolean isStatic = Modifier.isStatic(this.readMethod.getModifiers()); - boolean isInterface = publicDeclaringClass.isInterface(); - int opcode = (isStatic ? INVOKESTATIC : isInterface ? INVOKEINTERFACE : INVOKEVIRTUAL); - mv.visitMethodInsn(opcode, classDesc, this.readMethod.getName(), this.methodDescr, isInterface); - } + // Must be public with public fields/properties. + public static class RootContextWithIndexedProperties { + public int[] intArray; + public Number[] numberArray; + public List list; + public Set set; + public String string; + public Map map; + public Person person; } /** @@ -7365,14 +7326,24 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { */ public static class Colors { + private final Map map = new HashMap<>(); + + { + this.map.put(1, Color.BLUE); + this.map.put(2, Color.GREEN); + this.map.put(3, Color.ORANGE); + this.map.put(42, Color.PURPLE); + } + public Color get(int index) { - return switch (index) { - case 1 -> Color.BLUE; - case 2 -> Color.GREEN; - case 3 -> Color.ORANGE; - case 42 -> Color.PURPLE; - default -> throw new IndexOutOfBoundsException("No color for index " + index); - }; + if (!this.map.containsKey(index)) { + throw new IndexOutOfBoundsException("No color for index " + index); + } + return this.map.get(index); + } + + public void set(int index, Color color) { + this.map.put(index, color); } } @@ -7382,7 +7353,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { private static class ColorsIndexAccessor extends ReflectiveIndexAccessor { ColorsIndexAccessor() { - super(Colors.class, int.class, "get", null); + super(Colors.class, int.class, "get", "set"); } } @@ -7403,23 +7374,19 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { private static class ColorOrdinalsIndexAccessor extends ReflectiveIndexAccessor { ColorOrdinalsIndexAccessor() { - super(ColorOrdinals.class, Color.class, "get", null); + super(ColorOrdinals.class, Color.class, "get"); } } /** * Manually implemented {@link CompilableIndexAccessor} that knows how to - * index into {@link FruitMap}. + * index into {@link FruitMap} for reading, writing, and compilation. */ private static class FruitMapIndexAccessor implements CompilableIndexAccessor { - private final Class targetType = FruitMap.class; - - private final Class indexType = Color.class; + private final Method method = ReflectionUtils.findMethod(FruitMap.class, "getFruit", Color.class); - private final Method method = ReflectionUtils.findMethod(this.targetType, "getFruit", this.indexType); - - private final String targetTypeDesc = CodeFlow.toDescriptor(this.targetType); + private final String targetTypeDesc = CodeFlow.toDescriptor(FruitMap.class); private final String classDesc = this.targetTypeDesc.substring(1); @@ -7428,12 +7395,12 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @Override public Class[] getSpecificTargetClasses() { - return new Class[] { this.targetType }; + return new Class[] { FruitMap.class }; } @Override public boolean canRead(EvaluationContext context, Object target, Object index) { - return (this.targetType.isInstance(target) && this.indexType.isInstance(index)); + return (target instanceof FruitMap && index instanceof Color); } @Override @@ -7463,7 +7430,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { @Override public Class getIndexedValueType() { - return this.method.getReturnType(); + return String.class; } @Override @@ -7478,7 +7445,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { // Invoke the read-method. mv.visitMethodInsn(INVOKEVIRTUAL, this.classDesc, this.method.getName(), this.methodDescr, false); } - } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectiveIndexAccessorTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectiveIndexAccessorTests.java new file mode 100644 index 00000000000..ec210268270 --- /dev/null +++ b/spring-expression/src/test/java/org/springframework/expression/spel/support/ReflectiveIndexAccessorTests.java @@ -0,0 +1,129 @@ +/* + * Copyright 2002-2024 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.lang.reflect.Method; + +import example.Color; +import example.FruitMap; +import org.junit.jupiter.api.Test; + +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.TypedValue; +import org.springframework.util.ReflectionUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.assertThatNoException; +import static org.mockito.Mockito.mock; + +/** + * Unit tests for {@link ReflectiveIndexAccessor}. + * + * @author Sam Brannen + * @since 6.2 + * @see org.springframework.expression.spel.SpelCompilationCoverageTests + */ +class ReflectiveIndexAccessorTests { + + @Test + void nonexistentReadMethod() { + Class targetType = getClass(); + assertThatIllegalArgumentException() + .isThrownBy(() -> new ReflectiveIndexAccessor(targetType, int.class, "bogus")) + .withMessage("Failed to find public read-method 'bogus(int)' in class '%s'.", targetType.getCanonicalName()); + } + + @Test + void nonPublicReadMethod() { + Class targetType = PrivateReadMethod.class; + assertThatIllegalArgumentException() + .isThrownBy(() -> new ReflectiveIndexAccessor(targetType, int.class, "get")) + .withMessage("Failed to find public read-method 'get(int)' in class '%s'.", targetType.getCanonicalName()); + } + + @Test + void nonPublicWriteMethod() { + Class targetType = PrivateWriteMethod.class; + assertThatIllegalArgumentException() + .isThrownBy(() -> new ReflectiveIndexAccessor(targetType, int.class, "get", "set")) + .withMessage("Failed to find public write-method 'set(int, java.lang.Object)' in class '%s'.", + targetType.getCanonicalName()); + } + + @Test + void nonPublicDeclaringClass() { + Class targetType = NonPublicTargetType.class; + Method readMethod = ReflectionUtils.findMethod(targetType, "get", int.class); + ReflectiveIndexAccessor accessor = new ReflectiveIndexAccessor(targetType, int.class, "get"); + + assertThatIllegalStateException() + .isThrownBy(() -> accessor.generateCode(mock(), mock(), mock())) + .withMessage("Failed to find public declaring class for read-method: %s", readMethod); + } + + @Test + void publicReadAndWriteMethods() { + FruitMap fruitMap = new FruitMap(); + EvaluationContext context = mock(); + ReflectiveIndexAccessor accessor = + new ReflectiveIndexAccessor(FruitMap.class, Color.class, "getFruit", "setFruit"); + + assertThat(accessor.getSpecificTargetClasses()).containsOnly(FruitMap.class); + + assertThat(accessor.canRead(context, this, Color.RED)).isFalse(); + assertThat(accessor.canRead(context, fruitMap, this)).isFalse(); + assertThat(accessor.canRead(context, fruitMap, Color.RED)).isTrue(); + assertThat(accessor.read(context, fruitMap, Color.RED)).extracting(TypedValue::getValue).isEqualTo("cherry"); + + assertThat(accessor.canWrite(context, this, Color.RED)).isFalse(); + assertThat(accessor.canWrite(context, fruitMap, this)).isFalse(); + assertThat(accessor.canWrite(context, fruitMap, Color.RED)).isTrue(); + accessor.write(context, fruitMap, Color.RED, "strawberry"); + assertThat(fruitMap.getFruit(Color.RED)).isEqualTo("strawberry"); + assertThat(accessor.read(context, fruitMap, Color.RED)).extracting(TypedValue::getValue).isEqualTo("strawberry"); + + assertThat(accessor.isCompilable()).isTrue(); + assertThat(accessor.getIndexedValueType()).isEqualTo(String.class); + assertThatNoException().isThrownBy(() -> accessor.generateCode(mock(), mock(), mock())); + } + + + public static class PrivateReadMethod { + Object get(int i) { + return "foo"; + } + } + + public static class PrivateWriteMethod { + public Object get(int i) { + return "foo"; + } + + void set(int i, String value) { + // no-op + } + } + + static class NonPublicTargetType { + public Object get(int i) { + return "foo"; + } + } + +}