From 27d2200058bffb022b43a8bba4399e2fe837ee97 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 15 Apr 2024 17:06:09 +0200 Subject: [PATCH] Introduce CompilableIndexAccessor SPI in SpEL This commit introduces a new CompilableIndexAccessor SPI for the Spring Expression Language (SpEL) which allows an IndexAccessor to support compilation to bytecode for operations that read an index. This analogous to the CompilablePropertyAccessor SPI. This commit also includes a prototype for a general purpose ReflectiveIndexAccessor in the tests. Closes gh-32613 --- .../spel/CompilableIndexAccessor.java | 69 +++ .../expression/spel/ast/Indexer.java | 21 + .../spel/SpelCompilationCoverageTests.java | 506 ++++++++++++++++++ 3 files changed, 596 insertions(+) create mode 100644 spring-expression/src/main/java/org/springframework/expression/spel/CompilableIndexAccessor.java diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/CompilableIndexAccessor.java b/spring-expression/src/main/java/org/springframework/expression/spel/CompilableIndexAccessor.java new file mode 100644 index 0000000000..bc6607bebf --- /dev/null +++ b/spring-expression/src/main/java/org/springframework/expression/spel/CompilableIndexAccessor.java @@ -0,0 +1,69 @@ +/* + * 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; + +import org.springframework.asm.MethodVisitor; +import org.springframework.asm.Opcodes; +import org.springframework.expression.IndexAccessor; + +/** + * A compilable {@link IndexAccessor} is able to generate bytecode that represents + * the operation for reading the index, facilitating compilation to bytecode of + * expressions that use the accessor. + * + * @author Sam Brannen + * @since 6.2 + */ +public interface CompilableIndexAccessor extends IndexAccessor, Opcodes { + + /** + * Determine if this {@code IndexAccessor} is currently suitable for compilation. + *
May only be known once the index has been read. + * @see #read(org.springframework.expression.EvaluationContext, Object, Object) + */ + boolean isCompilable(); + + /** + * Get the type of the indexed value. + *
For example, given the expression {@code book.authors[0]}, the indexed + * value type represents the result of {@code authors[0]} which may be an + * {@code Author} object, a {@code String} representing the author's name, etc. + *
May only be known once the index has been read. + * @see #read(org.springframework.expression.EvaluationContext, Object, Object) + */ + Class> getIndexedValueType(); + + /** + * Generate bytecode that performs the operation for reading the index. + *
Bytecode should be generated into the supplied {@link MethodVisitor} + * using context information from the {@link CodeFlow} where necessary. + *
The supplied {@code indexNode} should be used to generate the + * appropriate bytecode to load the index onto the stack. For example, given + * the expression {@code book.authors[0]}, invoking + * {@code codeFlow.generateCodeForArgument(methodVisitor, indexNode, int.class)} + * will ensure that the index ({@code 0}) is available on the stack as a + * primitive {@code int}. + *
Will only be invoked if {@link #isCompilable()} returns {@code true}.
+ * @param indexNode the {@link SpelNode} that represents the index being
+ * accessed
+ * @param methodVisitor the ASM {@link MethodVisitor} into which code should
+ * be generated
+ * @param codeFlow the current state of the expression compiler
+ */
+ void generateCode(SpelNode indexNode, 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 be4b91d718..287a4f5697 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
@@ -33,6 +33,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.CompilableIndexAccessor;
import org.springframework.expression.spel.CompilablePropertyAccessor;
import org.springframework.expression.spel.ExpressionState;
import org.springframework.expression.spel.SpelEvaluationException;
@@ -314,6 +315,12 @@ public class Indexer extends SpelNodeImpl {
cachedPropertyReadState.accessor instanceof CompilablePropertyAccessor cpa &&
cpa.isCompilable());
}
+ else if (this.indexedType == IndexedType.CUSTOM) {
+ CachedIndexState cachedIndexReadState = this.cachedIndexReadState;
+ return (cachedIndexReadState != null &&
+ cachedIndexReadState.accessor instanceof CompilableIndexAccessor cia &&
+ cia.isCompilable() && index.isCompilable());
+ }
return false;
}
@@ -398,6 +405,17 @@ public class Indexer extends SpelNodeImpl {
compilablePropertyAccessor.generateCode(propertyName, mv, cf);
}
+ else if (this.indexedType == IndexedType.CUSTOM) {
+ CachedIndexState cachedIndexReadState = this.cachedIndexReadState;
+ Assert.state(cachedIndexReadState != null, "No cached IndexAccessor for reading");
+ if (!(cachedIndexReadState.accessor instanceof CompilableIndexAccessor compilableIndexAccessor)) {
+ throw new IllegalStateException(
+ "Cached IndexAccessor must be a CompilableIndexAccessor, but was: " +
+ cachedIndexReadState.accessor.getClass().getName());
+ }
+ compilableIndexAccessor.generateCode(index, mv, cf);
+ }
+
cf.pushDescriptor(exitTypeDescriptor);
if (skipIfNull != null) {
@@ -987,6 +1005,9 @@ public class Indexer extends SpelNodeImpl {
if (indexAccessor.canRead(this.evaluationContext, this.target, this.index)) {
TypedValue result = indexAccessor.read(this.evaluationContext, this.target, this.index);
Indexer.this.cachedIndexReadState = new CachedIndexState(indexAccessor, targetType, this.index);
+ if (indexAccessor instanceof CompilableIndexAccessor compilableIndexAccessor) {
+ setExitTypeDescriptor(CodeFlow.toDescriptor(compilableIndexAccessor.getIndexedValueType()));
+ }
return result;
}
}
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 e5a7b3692b..2701ad68de 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
@@ -32,14 +32,18 @@ import java.util.Set;
import java.util.StringTokenizer;
import java.util.stream.Stream;
+import example.Color;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.asm.MethodVisitor;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.Expression;
+import org.springframework.expression.IndexAccessor;
import org.springframework.expression.TypedValue;
import org.springframework.expression.spel.ast.CompoundExpression;
import org.springframework.expression.spel.ast.InlineList;
@@ -52,12 +56,19 @@ import org.springframework.expression.spel.standard.SpelExpressionParser;
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;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.within;
import static org.assertj.core.api.InstanceOfAssertFactories.BOOLEAN;
+import static org.junit.jupiter.api.Named.named;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
+import static org.springframework.expression.spel.SpelMessage.EXCEPTION_DURING_INDEX_READ;
import static org.springframework.expression.spel.standard.SpelExpressionTestUtils.assertIsCompiled;
/**
@@ -749,6 +760,198 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
return stream.map(Object::toString).collect(joining(" "));
}
+ @Nested
+ class IndexAccessorTests {
+
+ @Test
+ void indexWithPrimitiveIndexTypeAndReferenceValueTypeAccessedViaRoot() {
+ String exitTypeDescriptor = CodeFlow.toDescriptor(Color.class);
+ Colors colors = new Colors();
+
+ StandardEvaluationContext context = new StandardEvaluationContext();
+ context.addIndexAccessor(new ColorsIndexAccessor());
+
+ expression = parser.parseExpression("[0]");
+ assertCannotCompile(expression);
+
+ assertThatExceptionOfType(SpelEvaluationException.class)
+ .isThrownBy(() -> expression.getValue(context, colors))
+ .withMessageEndingWith("A problem occurred while attempting to read index '%s' in '%s'",
+ 0, Colors.class.getName())
+ .withCauseInstanceOf(IndexOutOfBoundsException.class)
+ .extracting(SpelEvaluationException::getMessageCode).isEqualTo(EXCEPTION_DURING_INDEX_READ);
+ assertCannotCompile(expression);
+
+ // IntLiteral as index --> represented as an int in compiled bytecode,
+ // which does not require unboxing since get(int) method expects an int.
+ // Falls in range [ICONST_0, ICONST_5]
+ expression = parser.parseExpression("[1]");
+ assertCannotCompile(expression);
+
+ assertThat(expression.getValue(context, colors)).isEqualTo(Color.BLUE);
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context, colors)).isEqualTo(Color.BLUE);
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+
+ // IntLiteral as index --> represented as an int in compiled bytecode,
+ // which does not require unboxing since get(int) method expects an int.
+ // Does not fall in range [ICONST_0, ICONST_5]
+ expression = parser.parseExpression("[42]");
+ assertCannotCompile(expression);
+
+ assertThat(expression.getValue(context, colors)).isEqualTo(Color.PURPLE);
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context, colors)).isEqualTo(Color.PURPLE);
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+
+ // Integer variable as index --> represented as an Integer in compiled bytecode,
+ // which requires unboxing from Integer to int since get(int) method expects an int.
+ context.setVariable("colorIndex", 2);
+ expression = parser.parseExpression("[#colorIndex]");
+ assertCannotCompile(expression);
+
+ assertThat(expression.getValue(context, colors)).isEqualTo(Color.GREEN);
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context, colors)).isEqualTo(Color.GREEN);
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+
+ // Reuse expression but change value of colorIndex.
+ context.setVariable("colorIndex", 3);
+
+ assertThat(expression.getValue(context, colors)).isEqualTo(Color.ORANGE);
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context, colors)).isEqualTo(Color.ORANGE);
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+ }
+
+ @Test
+ void indexWithPrimitiveIndexTypeAndReferenceValueTypeAccessedViaList() {
+ String exitTypeDescriptor = CodeFlow.toDescriptor(Color.class);
+
+ StandardEvaluationContext context = new StandardEvaluationContext();
+ context.addIndexAccessor(new ColorsIndexAccessor());
+ context.setVariable("list", List.of(new Colors()));
+
+ expression = parser.parseExpression("#list.get(0)[0]");
+ assertCannotCompile(expression);
+
+ assertThatExceptionOfType(SpelEvaluationException.class)
+ .isThrownBy(() -> expression.getValue(context))
+ .withMessageEndingWith("A problem occurred while attempting to read index '%s' in '%s'",
+ 0, Colors.class.getName())
+ .withCauseInstanceOf(IndexOutOfBoundsException.class)
+ .extracting(SpelEvaluationException::getMessageCode).isEqualTo(EXCEPTION_DURING_INDEX_READ);
+ assertCannotCompile(expression);
+
+ // IntLiteral as index --> represented as an int in compiled bytecode,
+ // which does not require unboxing since get(int) method expects an int.
+ expression = parser.parseExpression("#list.get(0)[1]");
+ assertCannotCompile(expression);
+
+ assertThat(expression.getValue(context)).isEqualTo(Color.BLUE);
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context)).isEqualTo(Color.BLUE);
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+
+ // Integer variable as index --> represented as an Integer in compiled bytecode,
+ // which requires unboxing from Integer to int since get(int) method expects an int.
+ context.setVariable("colorIndex", 2);
+ expression = parser.parseExpression("#list.get(0)[#colorIndex]");
+ assertCannotCompile(expression);
+
+ assertThat(expression.getValue(context)).isEqualTo(Color.GREEN);
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context)).isEqualTo(Color.GREEN);
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+
+ // Reuse expression but change value of colorIndex.
+ context.setVariable("colorIndex", 3);
+
+ assertThat(expression.getValue(context)).isEqualTo(Color.ORANGE);
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context)).isEqualTo(Color.ORANGE);
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+ }
+
+ @Test
+ void indexWithReferenceIndexTypeAndPrimitiveValueType() {
+ String exitTypeDescriptor = CodeFlow.toDescriptor(int.class);
+
+ StandardEvaluationContext context = new StandardEvaluationContext();
+ context.addIndexAccessor(new ColorOrdinalsIndexAccessor());
+ context.setVariable("colorOrdinals", new ColorOrdinals());
+ context.setVariable("color", Color.GREEN);
+
+ expression = parser.parseExpression("#colorOrdinals[#color]");
+ assertCannotCompile(expression);
+
+ assertThat(expression.getValue(context)).isEqualTo(Color.GREEN.ordinal());
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context)).isEqualTo(Color.GREEN.ordinal());
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+
+ // Reuse expression but change value of color.
+ context.setVariable("color", Color.BLUE);
+
+ assertThat(expression.getValue(context)).isEqualTo(Color.BLUE.ordinal());
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context)).isEqualTo(Color.BLUE.ordinal());
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+ }
+
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("fruitsIndexAccessors")
+ void indexWithReferenceIndexTypeAndReferenceValueType(IndexAccessor indexAccessor) {
+ String exitTypeDescriptor = CodeFlow.toDescriptor(String.class);
+
+ StandardEvaluationContext context = new StandardEvaluationContext();
+ context.addIndexAccessor(indexAccessor);
+ context.setVariable("list", List.of(new Fruits()));
+
+ expression = parser.parseExpression("#list.get(0)[T(example.Color).PURPLE]");
+ assertCannotCompile(expression);
+
+ assertThatExceptionOfType(SpelEvaluationException.class)
+ .isThrownBy(() -> expression.getValue(context))
+ .withMessageEndingWith("A problem occurred while attempting to read index '%s' in '%s'",
+ Color.PURPLE, Fruits.class.getName())
+ .withCauseInstanceOf(IndexOutOfBoundsException.class)
+ .extracting(SpelEvaluationException::getMessageCode).isEqualTo(EXCEPTION_DURING_INDEX_READ);
+ assertCannotCompile(expression);
+
+ expression = parser.parseExpression("#list[0][T(example.Color).RED]");
+ assertCannotCompile(expression);
+
+ assertThat(expression.getValue(context)).isEqualTo("cherry");
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context)).isEqualTo("cherry");
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+
+ context.setVariable("color", Color.GREEN);
+ expression = parser.parseExpression("#list[0][#color]");
+ assertCannotCompile(expression);
+
+ assertThat(expression.getValue(context)).isEqualTo("kiwi");
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context)).isEqualTo("kiwi");
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+
+ // Reuse expression but change value of color.
+ context.setVariable("color", Color.BLUE);
+
+ assertThat(expression.getValue(context)).isEqualTo("blueberry");
+ assertCanCompile(expression);
+ assertThat(expression.getValue(context)).isEqualTo("blueberry");
+ assertThat(getAst().getExitDescriptor()).isEqualTo(exitTypeDescriptor);
+ }
+
+ static Stream