Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Change IndexAccessor#read()'s return type to TypedValue

Browse Source 
Prior to this commit, the read() method in the IndexAccessor SPI
declared a return type of ValueRef which introduced a package cycle.

This commit addresses this by aligning with the PropertyAccess SPI and
returning TypedValue from IndexAccessor's read() method. This commit
also reworks the internals of Indexer based on a new, local
IndexAccessorValueRef implementation.

See gh-26409
See gh-26478
This commit is contained in:
Sam Brannen
2024-03-27 10:45:21 +01:00
parent b7c3833732
commit 5c6b82a947
3 changed files with 68 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
spring-expression/src/main/java/org/springframework/expression/IndexAccessor.java
View File

@@ -16,7 +16,6 @@
package org.springframework.expression;
import org.springframework.expression.spel.ast.ValueRef;
import org.springframework.lang.Nullable;
/**
@@ -80,8 +79,7 @@ public interface IndexAccessor extends TargetedAccessor {
* descriptor for it
* @throws AccessException if there is any problem reading the index value
*/
// TODO Change return type to TypedValue to avoid package cycle.
ValueRef read(EvaluationContext context, Object target, Object index) throws AccessException;
TypedValue read(EvaluationContext context, Object target, Object index) throws AccessException;
/**
* Called to determine if this index accessor is able to write to a specified

63
spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java
View File

@@ -158,14 +158,14 @@ public class Indexer extends SpelNodeImpl {
throws EvaluationException {
TypedValue typedValue = valueSupplier.get();
// TODO Set value for IndexAccessor via its write() method, NOT via the ValueRef returned from its read() method.
// TODO Query IndexAccessor's canWrite() method before invoking its write() method.
getValueRef(state).setValue(typedValue.getValue());
return typedValue;
}
@Override
public boolean isWritable(ExpressionState expressionState) throws SpelEvaluationException {
return true;
return getValueRef(expressionState).isWritable();
}
@@ -254,13 +254,13 @@ public class Indexer extends SpelNodeImpl {
try {
for (IndexAccessor indexAccessor : accessorsToTry) {
if (indexAccessor.canRead(evalContext, target, index)) {
// TODO Introduce local IndexAccessorValueRef.
return indexAccessor.read(evalContext, target, index);
return new IndexAccessorValueRef(indexAccessor, target, index, evalContext, targetDescriptor);
}
}
}
catch (Exception ex) {
// TODO throw SpelEvaluationException for "exception during index access"
// TODO throw SpelEvaluationException for "exception during index access",
// analogous to SpelMessage.EXCEPTION_DURING_PROPERTY_READ.
}
throw new SpelEvaluationException(
@@ -874,4 +874,57 @@ public class Indexer extends SpelNodeImpl {
}
}
private class IndexAccessorValueRef implements ValueRef {
private final IndexAccessor indexAccessor;
private final Object target;
private final Object index;
private final EvaluationContext evaluationContext;
private final TypeDescriptor typeDescriptor;
IndexAccessorValueRef(IndexAccessor indexAccessor, Object target, Object index,
EvaluationContext evaluationContext, TypeDescriptor typeDescriptor) {
this.indexAccessor = indexAccessor;
this.target = target;
this.index = index;
this.evaluationContext = evaluationContext;
this.typeDescriptor = typeDescriptor;
}
@Override
public TypedValue getValue() {
try {
return this.indexAccessor.read(this.evaluationContext, this.target, this.index);
}
catch (AccessException ex) {
throw new SpelEvaluationException(getStartPosition(), ex,
SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, this.typeDescriptor.toString());
}
}
@Override
public void setValue(@Nullable Object newValue) {
try {
this.indexAccessor.write(this.evaluationContext, this.target, this.index, newValue);
}
catch (AccessException ex) {
throw new SpelEvaluationException(getStartPosition(), ex,
SpelMessage.INDEXING_NOT_SUPPORTED_FOR_TYPE, this.typeDescriptor.toString());
}
}
@Override
public boolean isWritable() {
return true;
}
}
}

40
spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java
View File

@@ -41,10 +41,10 @@ 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;
import org.springframework.lang.Nullable;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -543,45 +543,23 @@ class IndexingTests {
}
@Override
public ValueRef read(EvaluationContext context, Object target, Object index) {
return new ArrayNodeValueRef((ArrayNode) target, (Integer) index, this.objectMapper);
public TypedValue read(EvaluationContext context, Object target, Object index) {
ArrayNode arrayNode = (ArrayNode) target;
Integer intIndex = (Integer) index;
return new TypedValue(arrayNode.get(intIndex));
}
@Override
public boolean canWrite(EvaluationContext context, Object target, Object index) {
return (target instanceof ArrayNode && index instanceof Integer);
return canRead(context, target, index);
}
@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());
}
public void write(EvaluationContext context, Object target, Object index, @Nullable Object newValue) {
ArrayNode arrayNode = (ArrayNode) target;
Integer intIndex = (Integer) index;
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;
}
}
}
}