From d91277095a3eee690f3e6766257cec6e1c2510f8 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sun, 24 Mar 2024 18:07:20 +0100 Subject: [PATCH] Initial review and polish of IndexAccessor support in SpEL See gh-26409 See gh-26478 --- spring-expression/spring-expression.gradle | 1 - .../expression/EvaluationContext.java | 10 +- .../expression/IndexAccessor.java | 103 +++++++------ .../expression/PropertyAccessor.java | 15 +- .../expression/TargetedAccessor.java | 56 ++++++++ .../expression/spel/ast/AstUtils.java | 81 ++++++++--- .../expression/spel/ast/Indexer.java | 91 ++++-------- .../spel/support/SimpleEvaluationContext.java | 6 - .../support/StandardEvaluationContext.java | 62 ++++++-- .../expression/spel/IndexingTests.java | 136 ++++++++++++++++++ .../expression/spel/PropertyAccessTests.java | 106 -------------- 11 files changed, 405 insertions(+), 262 deletions(-) create mode 100644 spring-expression/src/main/java/org/springframework/expression/TargetedAccessor.java diff --git a/spring-expression/spring-expression.gradle b/spring-expression/spring-expression.gradle index 5bc6aadf2ee2..d141d5309ab7 100644 --- a/spring-expression/spring-expression.gradle +++ b/spring-expression/spring-expression.gradle @@ -9,5 +9,4 @@ dependencies { testImplementation("org.jetbrains.kotlin:kotlin-reflect") testImplementation("org.jetbrains.kotlin:kotlin-stdlib") testImplementation("com.fasterxml.jackson.core:jackson-databind") - testImplementation("com.fasterxml.jackson.core:jackson-core") } diff --git a/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java b/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java index 217b84fb1b24..2746352d1ac2 100644 --- a/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java +++ b/spring-expression/src/main/java/org/springframework/expression/EvaluationContext.java @@ -16,6 +16,7 @@ package org.springframework.expression; +import java.util.Collections; import java.util.List; import java.util.function.Supplier; @@ -57,9 +58,14 @@ public interface EvaluationContext { List getPropertyAccessors(); /** - * Return a list of index accessors that will be asked in turn to read/write a property. + * Return a list of index accessors that will be asked in turn to access or + * set an indexed value. + *

The default implementation returns an empty list. + * @since 6.2 */ - List getIndexAccessors(); + default List getIndexAccessors() { + return Collections.emptyList(); + } /** * Return a list of resolvers that will be asked in turn to locate a constructor. diff --git a/spring-expression/src/main/java/org/springframework/expression/IndexAccessor.java b/spring-expression/src/main/java/org/springframework/expression/IndexAccessor.java index a5a76a9c769b..e26136f6822c 100644 --- a/spring-expression/src/main/java/org/springframework/expression/IndexAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/IndexAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * 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. @@ -20,78 +20,91 @@ import org.springframework.lang.Nullable; /** - * A index accessor is able to read from (and possibly write to) an array's elements. + * An index accessor is able to read from (and possibly write to) an indexed + * structure of an object. * - *

This interface places no restrictions, and so implementors are free to access elements - * directly as fields or through getters or in any other way they see as appropriate. + *

This interface places no restrictions on what constitutes an indexed + * structure. Implementors are therefore free to access indexed values any way + * they deem appropriate. * - *

A resolver can optionally specify an array of target classes for which it should be - * called. However, if it returns {@code null} from {@link #getSpecificTargetClasses()}, - * it will be called for all property references and given a chance to determine if it - * can read or write them. + *

An index accessor can optionally specify an array of target classes for + * which it should be called. However, if it returns {@code null} or an empty + * array from {@link #getSpecificTargetClasses()}, it will be called for all + * indexing operations and given a chance to determine if it can read from or + * write to the indexed structure. * - *

Property resolvers are considered to be ordered, and each will be called in turn. - * The only rule that affects the call order is that any resolver naming the target - * class directly in {@link #getSpecificTargetClasses()} will be called first, before - * the general resolvers. + *

Index accessors are considered to be ordered, and each will be called in + * turn. The only rule that affects the call order is that any index accessor + * which specifies explicit support for the target class via + * {@link #getSpecificTargetClasses()} will be called first, before other + * generic index accessors. * - * @author jackmiking lee - * @since 3.0 + * @author Jackmiking Lee + * @author Sam Brannen + * @since 6.2 + * @see PropertyAccessor */ -public interface IndexAccessor { +public interface IndexAccessor extends TargetedAccessor { + /** - * Return an array of classes for which this resolver should be called. - *

Returning {@code null} indicates this is a general resolver that - * can be called in an attempt to resolve a property on any type. - * @return an array of classes that this resolver is suitable for - * (or {@code null} if a general resolver) + * Get the set of classes for which this index accessor should be called. + *

Returning {@code null} or an empty array indicates this is a generic + * index accessor that can be called in an attempt to access an index on any + * type. + * @return an array of classes that this index accessor is suitable for + * (or {@code null} or an empty array if a generic index accessor) */ + @Override @Nullable Class[] getSpecificTargetClasses(); /** - * Called to determine if a resolver instance is able to access a specified property - * on a specified target object. + * Called to determine if this index accessor is able to read a specified + * index on a specified target object. * @param context the evaluation context in which the access is being attempted - * @param target the target object upon which the property is being accessed - * @param index the index of the array being accessed - * @return true if this resolver is able to read the property - * @throws AccessException if there is any problem determining whether the property can be read + * @param target the target object upon which the index is being accessed + * @param index the index being accessed + * @return true if this index accessor is able to read the index + * @throws AccessException if there is any problem determining whether the + * index can be read */ boolean canRead(EvaluationContext context, @Nullable Object target, Object index) throws AccessException; /** - * Called to read a property from a specified target object. - * Should only succeed if {@link #canRead} also returns {@code true}. + * Called to read an index from a specified target object. + *

Should only succeed if {@link #canRead} also returns {@code true}. * @param context the evaluation context in which the access is being attempted - * @param target the target object upon which the property is being accessed - * @param index the index of the array being accessed - * @return a TypedValue object wrapping the property value read and a type descriptor for it - * @throws AccessException if there is any problem accessing the property value + * @param target the target object upon which the index is being accessed + * @param index the index being accessed + * @return a TypedValue object wrapping the index value read and a type + * descriptor for it + * @throws AccessException if there is any problem reading the index value */ - ValueRef read(EvaluationContext context, @Nullable Object target,Object index) throws AccessException; + // TODO Change return type to TypedValue to avoid package cycle. + ValueRef read(EvaluationContext context, @Nullable Object target, Object index) throws AccessException; /** - * Called to determine if a resolver instance is able to write to a specified - * property on a specified target object. + * Called to determine if this index accessor is able to write to a specified + * index on a specified target object. * @param context the evaluation context in which the access is being attempted - * @param target the target object upon which the property is being accessed - * @param index the index of the array being accessed - * @return true if this resolver is able to write to the property + * @param target the target object upon which the index is being accessed + * @param index the index being accessed + * @return true if this index accessor is able to write to the index * @throws AccessException if there is any problem determining whether the - * property can be written to + * index can be written to */ boolean canWrite(EvaluationContext context, @Nullable Object target, Object index) throws AccessException; /** - * Called to write to a property on a specified target object. - * Should only succeed if {@link #canWrite} also returns {@code true}. + * Called to write to an index on a specified target object. + *

Should only succeed if {@link #canWrite} also returns {@code true}. * @param context the evaluation context in which the access is being attempted - * @param target the target object upon which the property is being accessed - * @param index the index of the array being accessed - * @param newValue the new value for the property - * @throws AccessException if there is any problem writing to the property value + * @param target the target object upon which the index is being accessed + * @param index the index being accessed + * @param newValue the new value for the index + * @throws AccessException if there is any problem writing to the index value */ void write(EvaluationContext context, @Nullable Object target, Object index, @Nullable Object newValue) throws AccessException; + } diff --git a/spring-expression/src/main/java/org/springframework/expression/PropertyAccessor.java b/spring-expression/src/main/java/org/springframework/expression/PropertyAccessor.java index c063750b1b12..a439046be2df 100644 --- a/spring-expression/src/main/java/org/springframework/expression/PropertyAccessor.java +++ b/spring-expression/src/main/java/org/springframework/expression/PropertyAccessor.java @@ -34,21 +34,24 @@ *

Property accessors are considered to be ordered, and each will be called in * turn. The only rule that affects the call order is that any property accessor * which specifies explicit support for the target class via - * {@link #getSpecificTargetClasses()} will be called first, before the general + * {@link #getSpecificTargetClasses()} will be called first, before the generic * property accessors. * * @author Andy Clement * @since 3.0 + * @see IndexAccessor */ -public interface PropertyAccessor { +public interface PropertyAccessor extends TargetedAccessor { /** - * Return an array of classes for which this property accessor should be called. - *

Returning {@code null} indicates this is a general property accessor that - * can be called in an attempt to access a property on any type. + * Get the set of classes for which this property accessor should be called. + *

Returning {@code null} or an empty array indicates this is a generic + * property accessor that can be called in an attempt to access a property on + * any type. * @return an array of classes that this property accessor is suitable for - * (or {@code null} if a general property accessor) + * (or {@code null} if a generic property accessor) */ + @Override @Nullable Class[] getSpecificTargetClasses(); diff --git a/spring-expression/src/main/java/org/springframework/expression/TargetedAccessor.java b/spring-expression/src/main/java/org/springframework/expression/TargetedAccessor.java new file mode 100644 index 000000000000..88b91055f5da --- /dev/null +++ b/spring-expression/src/main/java/org/springframework/expression/TargetedAccessor.java @@ -0,0 +1,56 @@ +/* + * 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; + +import org.springframework.lang.Nullable; + +/** + * Strategy for types that access elements of specific target classes. + * + *

This interface places no restrictions on what constitutes an element. + * + *

A targeted accessor can specify a set of target classes for which it should + * be called. However, if it returns {@code null} or an empty array from + * {@link #getSpecificTargetClasses()}, it will typically be called for all + * access operations and given a chance to determine if it supports a concrete + * access attempt. + * + *

Targeted accessors are considered to be ordered, and each will be called + * in turn. The only rule that affects the call order is that any accessor which + * specifies explicit support for a given target class via + * {@link #getSpecificTargetClasses()} will be called first, before other generic + * accessors that do not specify explicit support for the given target class. + * + * @author Sam Brannen + * @since 6.2 + * @see PropertyAccessor + * @see IndexAccessor + */ +public interface TargetedAccessor { + + /** + * Get the set of classes for which this accessor should be called. + *

Returning {@code null} or an empty array indicates this is a generic + * accessor that can be called in an attempt to access an element on any + * type. + * @return an array of classes that this accessor is suitable for + * (or {@code null} or an empty array if a generic accessor) + */ + @Nullable + Class[] getSpecificTargetClasses(); + +} diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/AstUtils.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/AstUtils.java index e4e710f7d611..665f1b6ee13c 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/AstUtils.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/AstUtils.java @@ -17,9 +17,11 @@ package org.springframework.expression.spel.ast; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import org.springframework.expression.PropertyAccessor; +import org.springframework.expression.TargetedAccessor; import org.springframework.lang.Nullable; import org.springframework.util.ObjectUtils; @@ -27,53 +29,92 @@ * Utility methods for use in the AST classes. * * @author Andy Clement + * @author Sam Brannen * @since 3.0.2 */ public abstract class AstUtils { /** - * Determine the set of property accessors that should be used to try to - * access a property on the specified target type. + * Determine the set of accessors that should be used to try to access an + * element on the specified target type. *

The accessors are considered to be in an ordered list; however, in the * returned list any accessors that are exact matches for the input target - * type (as opposed to 'general' accessors that could work for any type) are + * type (as opposed to 'generic' accessors that could work for any type) are * placed at the start of the list. In addition, if there are specific * accessors that exactly name the class in question and accessors that name * a specific class which is a supertype of the class in question, the latter * are put at the end of the specific accessors set and will be tried after * exactly matching accessors but before generic accessors. - * @param targetType the type upon which property access is being attempted - * @param propertyAccessors the list of property accessors to process - * @return a list of accessors that should be tried in order to access the property + * @param targetType the type upon which element access is being attempted + * @param accessors the list of element accessors to process + * @return a list of accessors that should be tried in order to access the + * element on the specified target type, or an empty list if no suitable + * accessor could be found + * @since 6.2 */ - public static List getPropertyAccessorsToTry( - @Nullable Class targetType, List propertyAccessors) { + public static List getAccessorsToTry( + @Nullable Class targetType, List accessors) { + + if (accessors.isEmpty()) { + return Collections.emptyList(); + } - List specificAccessors = new ArrayList<>(); - List generalAccessors = new ArrayList<>(); - for (PropertyAccessor accessor : propertyAccessors) { + List exactMatches = new ArrayList<>(); + List inexactMatches = new ArrayList<>(); + List genericMatches = new ArrayList<>(); + for (T accessor : accessors) { Class[] targets = accessor.getSpecificTargetClasses(); if (ObjectUtils.isEmpty(targets)) { // generic accessor that says it can be used for any type - generalAccessors.add(accessor); + genericMatches.add(accessor); } else if (targetType != null) { for (Class clazz : targets) { if (clazz == targetType) { - // add exact matches to the specificAccessors list - specificAccessors.add(accessor); + exactMatches.add(accessor); } else if (clazz.isAssignableFrom(targetType)) { - // add supertype matches to the front of the generalAccessors list - generalAccessors.add(0, accessor); + inexactMatches.add(accessor); } } } } - List accessors = new ArrayList<>(specificAccessors.size() + generalAccessors.size()); - accessors.addAll(specificAccessors); - accessors.addAll(generalAccessors); - return accessors; + + int size = exactMatches.size() + inexactMatches.size() + genericMatches.size(); + if (size == 0) { + return Collections.emptyList(); + } + else { + List result = new ArrayList<>(size); + result.addAll(exactMatches); + result.addAll(inexactMatches); + result.addAll(genericMatches); + return result; + } + } + + /** + * Determine the set of property accessors that should be used to try to + * access a property on the specified target type. + *

The accessors are considered to be in an ordered list; however, in the + * returned list any accessors that are exact matches for the input target + * type (as opposed to 'generic' accessors that could work for any type) are + * placed at the start of the list. In addition, if there are specific + * accessors that exactly name the class in question and accessors that name + * a specific class which is a supertype of the class in question, the latter + * are put at the end of the specific accessors set and will be tried after + * exactly matching accessors but before generic accessors. + * @param targetType the type upon which property access is being attempted + * @param propertyAccessors the list of property accessors to process + * @return a list of accessors that should be tried in order to access the + * property on the specified target type, or an empty list if no suitable + * accessor could be found + * @see #getAccessorsToTry(Class, List) + */ + public static List getPropertyAccessorsToTry( + @Nullable Class targetType, List propertyAccessors) { + + return getAccessorsToTry(targetType, propertyAccessors); } } 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 5a1c6f527dc1..88f534409e5e 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,11 +17,9 @@ package org.springframework.expression.spel.ast; import java.lang.reflect.Constructor; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.function.Supplier; import org.springframework.asm.Label; @@ -160,6 +158,7 @@ public TypedValue setValueInternal(ExpressionState state, Supplier v throws EvaluationException { TypedValue typedValue = valueSupplier.get(); + // TODO Set value for IndexAccessor via its write() method, NOT via the ValueRef returned from its read() method. getValueRef(state).setValue(typedValue.getValue()); return typedValue; } @@ -249,71 +248,23 @@ else if (target instanceof Collection collection) { return new PropertyIndexingValueRef( target, (String) index, state.getEvaluationContext(), targetDescriptor); } - Optional optional = tryIndexAccessor(state, index); - if (optional.isPresent()) { - return optional.get(); - } - throw new SpelEvaluationException( - getStartPosition(), SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, targetDescriptor); - } - private Optional tryIndexAccessor(ExpressionState state, Object index) { - EvaluationContext context = state.getEvaluationContext(); - Object target = state.getActiveContextObject().getValue(); - if (context != null) { - List list = context.getIndexAccessors(); - if (list != null) { - List availableAccessors = getIndexAccessorsToTry(state.getActiveContextObject(), list); - try { - for (IndexAccessor indexAccessor : availableAccessors) { - if (indexAccessor.canRead(context, target, index)) { - ValueRef valueRef = indexAccessor.read(context, target, index); - return Optional.of(valueRef); - } - } - } - catch (Exception ex) { + EvaluationContext evalContext = state.getEvaluationContext(); + List accessorsToTry = getIndexAccessorsToTry(target, evalContext.getIndexAccessors()); + try { + for (IndexAccessor indexAccessor : accessorsToTry) { + if (indexAccessor.canRead(evalContext, target, index)) { + // TODO Introduce local IndexAccessorValueRef. + return indexAccessor.read(evalContext, target, index); } } } - return Optional.empty(); - } - - private List getIndexAccessorsToTry( - @Nullable Object contextObject, List propertyAccessors) { - - Class targetType; - if (contextObject instanceof TypedValue) { - targetType = ((TypedValue) contextObject).getTypeDescriptor().getObjectType(); + catch (Exception ex) { + // TODO throw SpelEvaluationException for "exception during index access" } - else { - targetType = (contextObject != null ? contextObject.getClass() : null); - } - - List specificAccessors = new ArrayList<>(); - List generalAccessors = new ArrayList<>(); - for (IndexAccessor resolver : propertyAccessors) { - Class[] targets = resolver.getSpecificTargetClasses(); - if (targets == null) { - // generic resolver that says it can be used for any type - generalAccessors.add(resolver); - } - else if (targetType != null) { - for (Class clazz : targets) { - if (clazz == targetType) { - specificAccessors.add(resolver); - break; - } - else if (clazz.isAssignableFrom(targetType)) { - generalAccessors.add(resolver); - } - } - } - } - List resolvers = new ArrayList<>(specificAccessors); - generalAccessors.removeAll(specificAccessors); - resolvers.addAll(generalAccessors); - return resolvers; + + throw new SpelEvaluationException( + getStartPosition(), SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, targetDescriptor); } @Override @@ -451,6 +402,22 @@ private void setExitTypeDescriptor(String descriptor) { } } + /** + * Determine the set of index accessors that should be used to try to access + * an index on the specified context object. + *

Delegates to {@link AstUtils#getAccessorsToTry(Class, List)}. + * @param targetObject the object upon which index access is being attempted + * @param indexAccessors the list of index accessors to process + * @return a list of accessors that should be tried in order to access the + * index, or an empty list if no suitable accessor could be found + */ + private static List getIndexAccessorsToTry( + @Nullable Object targetObject, List indexAccessors) { + + Class targetType = (targetObject != null ? targetObject.getClass() : null); + return AstUtils.getAccessorsToTry(targetType, indexAccessors); + } + private class ArrayIndexingValueRef implements ValueRef { diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java index 24ab07b9ca71..f8cd8a1cce5a 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/SimpleEvaluationContext.java @@ -28,7 +28,6 @@ import org.springframework.expression.BeanResolver; import org.springframework.expression.ConstructorResolver; import org.springframework.expression.EvaluationContext; -import org.springframework.expression.IndexAccessor; import org.springframework.expression.MethodResolver; import org.springframework.expression.OperatorOverloader; import org.springframework.expression.PropertyAccessor; @@ -147,11 +146,6 @@ public List getPropertyAccessors() { return this.propertyAccessors; } - @Override - public List getIndexAccessors() { - return null; - } - /** * Return an empty list, always, since this context does not support the * use of type references. diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/StandardEvaluationContext.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/StandardEvaluationContext.java index ebe22a449a86..5d196b7e930e 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/StandardEvaluationContext.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/StandardEvaluationContext.java @@ -146,10 +146,6 @@ public void setPropertyAccessors(List propertyAccessors) { this.propertyAccessors = propertyAccessors; } - public void setIndexAccessors(ListindexAccessors){ - this.indexAccessors=indexAccessors; - } - @Override public List getPropertyAccessors() { return initPropertyAccessors(); @@ -163,11 +159,53 @@ public boolean removePropertyAccessor(PropertyAccessor accessor) { return initPropertyAccessors().remove(accessor); } - public void addIndexAccessor(IndexAccessor accessor){ - initIndexAccessors().add(accessor); + /** + * Set the list of index accessors to use in this evaluation context. + *

Replaces any previously configured index accessors. + * @since 6.2 + * @see #getIndexAccessors() + * @see #addIndexAccessor(IndexAccessor) + * @see #removeIndexAccessor(IndexAccessor) + */ + public void setIndexAccessors(List indexAccessors) { + this.indexAccessors = indexAccessors; + } + + /** + * Get the list of index accessors configured in this evaluation context. + * @since 6.2 + * @see #setIndexAccessors(List) + * @see #addIndexAccessor(IndexAccessor) + * @see #removeIndexAccessor(IndexAccessor) + */ + @Override + public List getIndexAccessors() { + return initIndexAccessors(); } - public boolean removeIndexAccessor(IndexAccessor indexAccessor){ + /** + * Add the supplied index accessor to this evaluation context. + * @param indexAccessor the index accessor to add + * @since 6.2 + * @see #getIndexAccessors() + * @see #setIndexAccessors(List) + * @see #removeIndexAccessor(IndexAccessor) + */ + public void addIndexAccessor(IndexAccessor indexAccessor) { + initIndexAccessors().add(indexAccessor); + } + + /** + * Remove the supplied index accessor from this evaluation context. + * @param indexAccessor the index accessor to remove + * @return {@code true} if the index accessor was removed, {@code false} if + * the index accessor was not configured in this evaluation context + * @since 6.2 + * @see #getIndexAccessors() + * @see #setIndexAccessors(List) + * @see #addIndexAccessor(IndexAccessor) + */ + public boolean removeIndexAccessor(IndexAccessor indexAccessor) { return initIndexAccessors().remove(indexAccessor); } @@ -400,6 +438,7 @@ public void applyDelegatesTo(StandardEvaluationContext evaluationContext) { evaluationContext.setConstructorResolvers(new ArrayList<>(getConstructorResolvers())); evaluationContext.setMethodResolvers(new ArrayList<>(getMethodResolvers())); evaluationContext.setPropertyAccessors(new ArrayList<>(getPropertyAccessors())); + evaluationContext.setIndexAccessors(new ArrayList<>(getIndexAccessors())); evaluationContext.setTypeLocator(getTypeLocator()); evaluationContext.setTypeConverter(getTypeConverter()); @@ -420,9 +459,9 @@ private List initPropertyAccessors() { return accessors; } - private ListinitIndexAccessors(){ + private List initIndexAccessors() { List accessors = this.indexAccessors; - if(accessors == null){ + if (accessors == null) { accessors = new ArrayList<>(5); this.indexAccessors = accessors; } @@ -454,9 +493,4 @@ private static void addBeforeDefault(List list, T element) { list.add(list.size() - 1, element); } - @Override - public List getIndexAccessors() { - return initIndexAccessors(); - } - } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java index ab424b3b53a4..22e7717444fb 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java @@ -28,13 +28,20 @@ import java.util.Map; import java.util.Set; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.NullNode; +import com.fasterxml.jackson.databind.node.TextNode; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; +import org.springframework.expression.IndexAccessor; import org.springframework.expression.PropertyAccessor; import org.springframework.expression.TypedValue; +import org.springframework.expression.spel.ast.ValueRef; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.testresources.Person; @@ -449,6 +456,135 @@ static class RootContextWithIndexedProperties { } + @Nested + class IndexAccessorTests { // gh-26478 + + @Test + void addingAndRemovingIndexAccessors() { + ObjectMapper objectMapper = new ObjectMapper(); + IndexAccessor accessor1 = new JacksonArrayNodeIndexAccessor(objectMapper); + IndexAccessor accessor2 = new JacksonArrayNodeIndexAccessor(objectMapper); + + StandardEvaluationContext context = new StandardEvaluationContext(); + List indexAccessors = context.getIndexAccessors(); + assertThat(indexAccessors).isEmpty(); + + context.addIndexAccessor(accessor1); + assertThat(indexAccessors).containsExactly(accessor1); + + context.addIndexAccessor(accessor2); + assertThat(indexAccessors).containsExactly(accessor1, accessor2); + + List copy = new ArrayList<>(indexAccessors); + assertThat(context.removeIndexAccessor(accessor1)).isTrue(); + assertThat(context.removeIndexAccessor(accessor1)).isFalse(); + assertThat(indexAccessors).containsExactly(accessor2); + + context.setIndexAccessors(copy); + assertThat(context.getIndexAccessors()).containsExactly(accessor1, accessor2); + } + + @Test + void indexReadWrite() { + StandardEvaluationContext context = new StandardEvaluationContext(); + + ObjectMapper objectMapper = new ObjectMapper(); + context.addIndexAccessor(new JacksonArrayNodeIndexAccessor(objectMapper)); + + TextNode node0 = new TextNode("node0"); + TextNode node1 = new TextNode("node1"); + ArrayNode arrayNode = objectMapper.createArrayNode(); + arrayNode.addAll(List.of(node0, node1)); + + SpelExpressionParser parser = new SpelExpressionParser(); + Expression expr = parser.parseExpression("[0]"); + assertThat(expr.getValue(context, arrayNode)).isSameAs(node0); + + TextNode nodeX = new TextNode("nodeX"); + expr.setValue(context, arrayNode, nodeX); + // We use isEqualTo() instead of isSameAs(), since ObjectMapper.convertValue() + // converts the supplied TextNode to an equivalent JsonNode. + assertThat(expr.getValue(context, arrayNode)).isEqualTo(nodeX); + + NullNode nullNode = NullNode.getInstance(); + expr.setValue(context, arrayNode, nullNode); + assertThat(expr.getValue(context, arrayNode)).isSameAs(nullNode); + + expr = parser.parseExpression("[1]"); + assertThat(expr.getValue(context, arrayNode)).isSameAs(node1); + + expr = parser.parseExpression("[-1]"); + // Jackson's ArrayNode returns null for a non-existent index instead + // of throwing an ArrayIndexOutOfBoundsException or similar. + assertThat(expr.getValue(context, arrayNode)).isNull(); + } + + + /** + * {@link IndexAccessor} that knows how to read and write indexes in a + * Jackson {@link ArrayNode}. + */ + private static class JacksonArrayNodeIndexAccessor implements IndexAccessor { + + private final ObjectMapper objectMapper; + + JacksonArrayNodeIndexAccessor(ObjectMapper objectMapper) { + this.objectMapper = objectMapper; + } + + @Override + public Class[] getSpecificTargetClasses() { + return new Class[] { ArrayNode.class }; + } + + @Override + public boolean canRead(EvaluationContext context, Object target, Object index) { + return (target instanceof ArrayNode && index instanceof Integer); + } + + @Override + public ValueRef read(EvaluationContext context, Object target, Object index) { + return new ArrayNodeValueRef((ArrayNode) target, (Integer) index, this.objectMapper); + } + + @Override + public boolean canWrite(EvaluationContext context, Object target, Object index) { + return (target instanceof ArrayNode && index instanceof Integer); + } + + @Override + public void write(EvaluationContext context, Object target, Object index, Object newValue) { + if (!(target instanceof ArrayNode arrayNode)) { + throw new IllegalStateException("target must be an ArrayNode: " + target.getClass().getName()); + } + if (!(index instanceof Integer intIndex)) { + throw new IllegalStateException("index must be an integer: " + target.getClass().getName()); + } + + arrayNode.set(intIndex, this.objectMapper.convertValue(newValue, JsonNode.class)); + } + + private record ArrayNodeValueRef(ArrayNode arrayNode, int index, ObjectMapper objectMapper) implements ValueRef { + + @Override + public TypedValue getValue() { + return new TypedValue(this.arrayNode.get(this.index)); + } + + @Override + public void setValue(Object newValue) { + // TODO throw new UnsupportedOperationException("setValue() is not supported"); + this.arrayNode.set(index, this.objectMapper.convertValue(newValue, JsonNode.class)); + } + + @Override + public boolean isWritable() { + return true; + } + } + } + } + @Target({ElementType.FIELD}) @Retention(RetentionPolicy.RUNTIME) diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java index 0557ab64cfc7..287644f84538 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java @@ -21,10 +21,6 @@ import java.util.List; import java.util.Map; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.TextNode; import org.junit.jupiter.api.Test; import org.springframework.core.convert.TypeDescriptor; @@ -32,10 +28,8 @@ import org.springframework.expression.EvaluationContext; import org.springframework.expression.EvaluationException; import org.springframework.expression.Expression; -import org.springframework.expression.IndexAccessor; import org.springframework.expression.PropertyAccessor; import org.springframework.expression.TypedValue; -import org.springframework.expression.spel.ast.ValueRef; import org.springframework.expression.spel.standard.SpelExpression; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.SimpleEvaluationContext; @@ -150,25 +144,6 @@ void addingAndRemovingAccessors() { assertThat(ctx.getPropertyAccessors()).hasSize(2); } - @Test - public void testAddingRemovingIndexAccessors() { - StandardEvaluationContext ctx = new StandardEvaluationContext(); - List indexAccessors = ctx.getIndexAccessors(); - assertThat(indexAccessors.size()).isEqualTo(0); - JsonIndexAccessor jsonIndexAccessor=new JsonIndexAccessor(); - ctx.addIndexAccessor(jsonIndexAccessor); - assertThat(indexAccessors.size()).isEqualTo(1); - JsonIndexAccessor jsonIndexAccessor1=new JsonIndexAccessor(); - ctx.addIndexAccessor(jsonIndexAccessor1); - assertThat(indexAccessors.size()).isEqualTo(2); - List copy=new ArrayList(indexAccessors); - assertThat(ctx.removeIndexAccessor(jsonIndexAccessor)).isTrue(); - assertThat(ctx.removeIndexAccessor(jsonIndexAccessor)).isFalse(); - assertThat(indexAccessors.size()).isEqualTo(1); - ctx.setIndexAccessors(copy); - assertThat(ctx.getIndexAccessors().size()).isEqualTo(2); - } - @Test void accessingPropertyOfClass() { Expression expression = parser.parseExpression("name"); @@ -248,24 +223,6 @@ void propertyReadWrite() { assertThat(target.getName()).isEqualTo("p4"); assertThat(expr.getValue(context, target)).isEqualTo("p4"); } - @Test - public void indexReadWrite(){ - StandardEvaluationContext context=new StandardEvaluationContext(); - JsonIndexAccessor indexAccessor=new JsonIndexAccessor(); - context.addIndexAccessor(indexAccessor); - ArrayNode arrayNode=indexAccessor.objectMapper.createArrayNode(); - arrayNode.add(new TextNode("node0")); - arrayNode.add(new TextNode("node1")); - Expression expr=parser.parseExpression("[0]"); - assertThat(new TextNode("node0").equals(expr.getValue(context,arrayNode))).isTrue(); - expr.setValue(context,arrayNode,new TextNode("nodeUpdate")); - assertThat(new TextNode("nodeUpdate").equals(expr.getValue(context,arrayNode))).isTrue(); - Expression expr1=parser.parseExpression("[1]"); - assertThat(new TextNode("node1").equals(expr1.getValue(context,arrayNode))).isTrue(); - - - } - @Test void propertyReadWriteWithRootObject() { @@ -406,67 +363,4 @@ public void write(EvaluationContext context, Object target, String name, Object } } - private static class JsonIndexAccessor implements IndexAccessor { - ObjectMapper objectMapper=new ObjectMapper(); - public class ArrayValueRef implements ValueRef { - ArrayNode arrayNode; - Integer index; - - @Override - public TypedValue getValue() { - return new TypedValue(arrayNode.get(index)); - } - - @Override - public void setValue(Object newValue) { - arrayNode.set(index,objectMapper.convertValue(newValue, JsonNode.class)); - } - public void setArrayNode(ArrayNode arrayNode){ - this.arrayNode=arrayNode; - } - - public void setIndex(Object index) { - if (index instanceof Integer) { - this.index = (Integer) index; - } - } - - @Override - public boolean isWritable() { - return false; - } - } - - @Override - public Class[] getSpecificTargetClasses() { - return new Class[]{ - ArrayNode.class - }; - } - - @Override - public boolean canRead(EvaluationContext context, Object target, Object index) throws AccessException { - return true; - } - - @Override - public ValueRef read(EvaluationContext context, Object target, Object index) throws AccessException { - ArrayValueRef valueRef = new ArrayValueRef(); - valueRef.setArrayNode((ArrayNode) target); - valueRef.setIndex(index); - return valueRef; - } - - @Override - public boolean canWrite(EvaluationContext context, Object target, Object index) throws AccessException { - return true; - } - - @Override - public void write(EvaluationContext context, Object target, Object index, Object newValue) throws AccessException { - ValueRef valueRef=read(context,target,index); - valueRef.setValue(newValue); - } - } - }