diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/CompilablePropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/CompilablePropertyAccessor.java index 6651bd68887..0792cb1d506 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/CompilablePropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/CompilablePropertyAccessor.java @@ -19,6 +19,7 @@ package org.springframework.expression.spel; import org.springframework.asm.MethodVisitor; import org.springframework.asm.Opcodes; import org.springframework.expression.PropertyAccessor; +import org.springframework.lang.Nullable; /** * A compilable {@link PropertyAccessor} is able to generate bytecode that represents @@ -41,13 +42,17 @@ public interface CompilablePropertyAccessor extends PropertyAccessor, Opcodes { Class getPropertyType(); /** - * Generate the bytecode the performs the access operation into the specified + * Generate the bytecode that performs the access operation into the specified * {@link MethodVisitor} using context information from the {@link CodeFlow} * where necessary. - * @param propertyName the name of the property + *

Concrete implementations of {@code CompilablePropertyAccessor} typically + * have access to the property name via other means (for example, supplied as + * an argument when they were instantiated). Thus, the {@code propertyName} + * supplied to this method may be {@code null}. + * @param propertyName the name of the property, or {@code null} if not available * @param methodVisitor the ASM method visitor into which code should be generated * @param codeFlow the current state of the expression compiler */ - void generateCode(String propertyName, MethodVisitor methodVisitor, CodeFlow codeFlow); + void generateCode(@Nullable String propertyName, MethodVisitor methodVisitor, CodeFlow codeFlow); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index 5205b24eb12..85665adf501 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -17,10 +17,6 @@ package org.springframework.expression.spel.ast; import java.lang.reflect.Constructor; -import java.lang.reflect.Field; -import java.lang.reflect.Member; -import java.lang.reflect.Method; -import java.lang.reflect.Modifier; import java.util.Collection; import java.util.List; import java.util.Map; @@ -35,6 +31,7 @@ import org.springframework.expression.PropertyAccessor; import org.springframework.expression.TypeConverter; import org.springframework.expression.TypedValue; import org.springframework.expression.spel.CodeFlow; +import org.springframework.expression.spel.CompilablePropertyAccessor; import org.springframework.expression.spel.ExpressionState; import org.springframework.expression.spel.SpelEvaluationException; import org.springframework.expression.spel.SpelMessage; @@ -223,10 +220,11 @@ public class Indexer extends SpelNodeImpl { return (this.children[0] instanceof PropertyOrFieldReference || this.children[0].isCompilable()); } else if (this.indexedType == IndexedType.OBJECT) { - // If the string name is changing, the accessor is clearly going to change (so no compilation possible) - return (this.cachedReadAccessor != null && - this.cachedReadAccessor instanceof ReflectivePropertyAccessor.OptimalPropertyAccessor && - getChild(0) instanceof StringLiteral); + // If the string name is changing, the accessor is clearly going to change. + // So compilation is only possible if the index expression is a StringLiteral. + return (getChild(0) instanceof StringLiteral && + this.cachedReadAccessor instanceof CompilablePropertyAccessor compilablePropertyAccessor && + compilablePropertyAccessor.isCompilable()); } return false; } @@ -315,30 +313,9 @@ public class Indexer extends SpelNodeImpl { } else if (this.indexedType == IndexedType.OBJECT) { - ReflectivePropertyAccessor.OptimalPropertyAccessor accessor = - (ReflectivePropertyAccessor.OptimalPropertyAccessor) this.cachedReadAccessor; - Assert.state(accessor != null, "No cached read accessor"); - Member member = accessor.member; - boolean isStatic = Modifier.isStatic(member.getModifiers()); - String classDesc = member.getDeclaringClass().getName().replace('.', '/'); - - if (!isStatic) { - if (descriptor == null) { - cf.loadTarget(mv); - } - if (descriptor == null || !classDesc.equals(descriptor.substring(1))) { - mv.visitTypeInsn(CHECKCAST, classDesc); - } - } - - if (member instanceof Method method) { - mv.visitMethodInsn((isStatic? INVOKESTATIC : INVOKEVIRTUAL), classDesc, member.getName(), - CodeFlow.createSignatureDescriptor(method), false); - } - else { - mv.visitFieldInsn((isStatic ? GETSTATIC : GETFIELD), classDesc, member.getName(), - CodeFlow.toJvmDescriptor(((Field) member).getType())); - } + CompilablePropertyAccessor compilablePropertyAccessor = (CompilablePropertyAccessor) this.cachedReadAccessor; + Assert.state(compilablePropertyAccessor != null, "No cached read accessor"); + compilablePropertyAccessor.generateCode(null, mv, cf); } cf.pushDescriptor(this.exitTypeDescriptor); @@ -600,10 +577,8 @@ public class Indexer extends SpelNodeImpl { Indexer.this.cachedReadAccessor = accessor; Indexer.this.cachedReadName = this.name; Indexer.this.cachedReadTargetType = targetObjectRuntimeClass; - if (accessor instanceof ReflectivePropertyAccessor.OptimalPropertyAccessor optimalAccessor) { - Member member = optimalAccessor.member; - Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(member instanceof Method method ? - method.getReturnType() : ((Field) member).getType()); + if (accessor instanceof CompilablePropertyAccessor compilablePropertyAccessor) { + Indexer.this.exitTypeDescriptor = CodeFlow.toDescriptor(compilablePropertyAccessor.getPropertyType()); } return accessor.read(this.evaluationContext, this.targetObject, this.name); } diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java index 322a275c62b..307984d7e73 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java @@ -712,7 +712,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor { } @Override - public void generateCode(String propertyName, MethodVisitor mv, CodeFlow cf) { + public void generateCode(@Nullable String propertyName, MethodVisitor mv, CodeFlow cf) { Class publicDeclaringClass = this.member.getDeclaringClass(); if (!Modifier.isPublic(publicDeclaringClass.getModifiers()) && this.originalMethod != null) { publicDeclaringClass = ReflectionHelper.findPublicDeclaringClass(this.originalMethod); 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 c7eced339d9..d19f50f00e6 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 @@ -764,6 +764,54 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests { assertThat(result).isEqualTo(2); } + @Test + void indexIntoPropertyInPrivateSubclassThatOverridesPropertyInPublicInterface() { + expression = parser.parseExpression("#root['text']"); + PrivateSubclass privateSubclass = new PrivateSubclass(); + + // Prerequisite: type must not be public for this use case. + assertNotPublic(privateSubclass.getClass()); + + String result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("enigma"); + + assertCanCompile(expression); + result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("enigma"); + } + + @Test + void indexIntoPropertyInPrivateSubclassThatOverridesPropertyInPrivateInterface() { + expression = parser.parseExpression("#root['message']"); + PrivateSubclass privateSubclass = new PrivateSubclass(); + + // Prerequisite: type must not be public for this use case. + assertNotPublic(privateSubclass.getClass()); + + String result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("hello"); + + assertCanCompile(expression); + result = expression.getValue(context, privateSubclass, String.class); + assertThat(result).isEqualTo("hello"); + } + + @Test + void indexIntoPropertyInPrivateSubclassThatOverridesPropertyInPublicSuperclass() { + expression = parser.parseExpression("#root['number']"); + PrivateSubclass privateSubclass = new PrivateSubclass(); + + // Prerequisite: type must not be public for this use case. + assertNotPublic(privateSubclass.getClass()); + + Integer result = expression.getValue(context, privateSubclass, Integer.class); + assertThat(result).isEqualTo(2); + + assertCanCompile(expression); + result = expression.getValue(context, privateSubclass, Integer.class); + assertThat(result).isEqualTo(2); + } + private interface PrivateInterface { String getMessage();