Skip to content

Commit 62fa3f1

Browse files
committed
Correctly request primitive conversion in SpEL's Indexer
Prior to this commit, SpEL's Indexer incorrectly requested conversion to wrappers instead of primitives when setting an element in a primitive array. This commit addresses this by requesting primitive conversion -- for example, conversion to `int.class` instead of `Integer.class` when setting a value in an `int[]` array. For greater clarity, this commit also switches from using `TYPE` constants in wrapper classes to primitive class literals -- for example, from `Integer.TYPE` to `int.class`. Closes gh-32147
1 parent 2e56361 commit 62fa3f1

File tree

2 files changed

+68
-33
lines changed

2 files changed

+68
-33
lines changed

Diff for: spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java

+17-17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -336,45 +336,45 @@ public String toStringAST() {
336336
private void setArrayElement(TypeConverter converter, Object ctx, int idx, @Nullable Object newValue,
337337
Class<?> arrayComponentType) throws EvaluationException {
338338

339-
if (arrayComponentType == Boolean.TYPE) {
339+
if (arrayComponentType == boolean.class) {
340340
boolean[] array = (boolean[]) ctx;
341341
checkAccess(array.length, idx);
342-
array[idx] = convertValue(converter, newValue, Boolean.class);
342+
array[idx] = convertValue(converter, newValue, boolean.class);
343343
}
344-
else if (arrayComponentType == Byte.TYPE) {
344+
else if (arrayComponentType == byte.class) {
345345
byte[] array = (byte[]) ctx;
346346
checkAccess(array.length, idx);
347-
array[idx] = convertValue(converter, newValue, Byte.class);
347+
array[idx] = convertValue(converter, newValue, byte.class);
348348
}
349-
else if (arrayComponentType == Character.TYPE) {
349+
else if (arrayComponentType == char.class) {
350350
char[] array = (char[]) ctx;
351351
checkAccess(array.length, idx);
352-
array[idx] = convertValue(converter, newValue, Character.class);
352+
array[idx] = convertValue(converter, newValue, char.class);
353353
}
354-
else if (arrayComponentType == Double.TYPE) {
354+
else if (arrayComponentType == double.class) {
355355
double[] array = (double[]) ctx;
356356
checkAccess(array.length, idx);
357-
array[idx] = convertValue(converter, newValue, Double.class);
357+
array[idx] = convertValue(converter, newValue, double.class);
358358
}
359-
else if (arrayComponentType == Float.TYPE) {
359+
else if (arrayComponentType == float.class) {
360360
float[] array = (float[]) ctx;
361361
checkAccess(array.length, idx);
362-
array[idx] = convertValue(converter, newValue, Float.class);
362+
array[idx] = convertValue(converter, newValue, float.class);
363363
}
364-
else if (arrayComponentType == Integer.TYPE) {
364+
else if (arrayComponentType == int.class) {
365365
int[] array = (int[]) ctx;
366366
checkAccess(array.length, idx);
367-
array[idx] = convertValue(converter, newValue, Integer.class);
367+
array[idx] = convertValue(converter, newValue, int.class);
368368
}
369-
else if (arrayComponentType == Long.TYPE) {
369+
else if (arrayComponentType == long.class) {
370370
long[] array = (long[]) ctx;
371371
checkAccess(array.length, idx);
372-
array[idx] = convertValue(converter, newValue, Long.class);
372+
array[idx] = convertValue(converter, newValue, long.class);
373373
}
374-
else if (arrayComponentType == Short.TYPE) {
374+
else if (arrayComponentType == short.class) {
375375
short[] array = (short[]) ctx;
376376
checkAccess(array.length, idx);
377-
array[idx] = convertValue(converter, newValue, Short.class);
377+
array[idx] = convertValue(converter, newValue, short.class);
378378
}
379379
else {
380380
Object[] array = (Object[]) ctx;

Diff for: spring-expression/src/test/java/org/springframework/expression/spel/SetValueTests.java

+51-16
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,19 @@
2121
import java.util.Set;
2222

2323
import org.assertj.core.api.ThrowableTypeAssert;
24-
import org.junit.jupiter.api.Disabled;
2524
import org.junit.jupiter.api.Test;
25+
import org.junit.jupiter.params.ParameterizedTest;
26+
import org.junit.jupiter.params.provider.ValueSource;
2627

2728
import org.springframework.expression.EvaluationException;
2829
import org.springframework.expression.Expression;
2930
import org.springframework.expression.ParseException;
3031
import org.springframework.expression.spel.testresources.PlaceOfBirth;
32+
import org.springframework.util.ObjectUtils;
3133

3234
import static org.assertj.core.api.Assertions.assertThat;
3335
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
36+
import static org.springframework.expression.spel.SpelMessage.TYPE_CONVERSION_ERROR;
3437

3538
/**
3639
* Tests for assignment, setValue(), and isWritable() expressions.
@@ -164,21 +167,19 @@ void setArrayElementToPrimitiveFromSingleElementWrapperList() {
164167
setValue("arrayContainer.doubles[1]", List.of(42D), 42D);
165168
}
166169

167-
@Disabled("Disabled due to bug in Indexer.setArrayElement() regarding primitive/wrapper types")
168-
@Test
169-
void setArrayElementToPrimitiveFromEmptyCollectionFails() {
170-
List<Object> emptyList = List.of();
171-
// TODO These fail because CollectionToObjectConverter returns null.
172-
// It currently throws: java.lang.IllegalStateException: Null conversion result for index [[]].
173-
// Whereas, it should throw a SpelEvaluationException.
174-
setValueAndExpectError("arrayContainer.booleans[1]", emptyList);
175-
setValueAndExpectError("arrayContainer.chars[1]", emptyList);
176-
setValueAndExpectError("arrayContainer.shorts[1]", emptyList);
177-
setValueAndExpectError("arrayContainer.bytes[1]", emptyList);
178-
setValueAndExpectError("arrayContainer.ints[1]", emptyList);
179-
setValueAndExpectError("arrayContainer.longs[1]", emptyList);
180-
setValueAndExpectError("arrayContainer.floats[1]", emptyList);
181-
setValueAndExpectError("arrayContainer.doubles[1]", emptyList);
170+
@ParameterizedTest
171+
@ValueSource(strings = {
172+
"arrayContainer.booleans[1]",
173+
"arrayContainer.chars[1]",
174+
"arrayContainer.shorts[1]",
175+
"arrayContainer.bytes[1]",
176+
"arrayContainer.ints[1]",
177+
"arrayContainer.longs[1]",
178+
"arrayContainer.floats[1]",
179+
"arrayContainer.doubles[1]"
180+
})
181+
void setArrayElementToPrimitiveFromEmptyCollectionFailsWithTypeConversionError(String expression) {
182+
setValueAndExpectError(expression, List.of(), TYPE_CONVERSION_ERROR);
182183
}
183184

184185
@Test
@@ -272,6 +273,40 @@ private void setValueAndExpectError(String expression, Object value) {
272273
assertThatSpelEvaluationException().isThrownBy(() -> e.setValue(context, value));
273274
}
274275

276+
/**
277+
* Call setValue() but expect it to fail.
278+
* @see #evaluateAndCheckError(org.springframework.expression.ExpressionParser, String, Class, SpelMessage, Object...)
279+
*/
280+
private void setValueAndExpectError(String expression, Object value, SpelMessage expectedMessage,
281+
Object... otherProperties) {
282+
283+
Expression expr = parser.parseExpression(expression);
284+
assertThat(expr).as("expression").isNotNull();
285+
286+
if (DEBUG) {
287+
SpelUtilities.printAbstractSyntaxTree(System.out, expr);
288+
}
289+
290+
assertThatSpelEvaluationException()
291+
.isThrownBy(() -> expr.setValue(context, value))
292+
.satisfies(ex -> {
293+
assertThat(ex.getMessageCode()).isEqualTo(expectedMessage);
294+
if (!ObjectUtils.isEmpty(otherProperties)) {
295+
// first one is expected position of the error within the string
296+
int pos = (Integer) otherProperties[0];
297+
assertThat(ex.getPosition()).as("position").isEqualTo(pos);
298+
if (otherProperties.length > 1) {
299+
// Check inserts match
300+
Object[] inserts = ex.getInserts();
301+
assertThat(inserts).as("inserts").hasSizeGreaterThanOrEqualTo(otherProperties.length - 1);
302+
Object[] expectedInserts = new Object[inserts.length];
303+
System.arraycopy(otherProperties, 1, expectedInserts, 0, expectedInserts.length);
304+
assertThat(inserts).as("inserts").containsExactly(expectedInserts);
305+
}
306+
}
307+
});
308+
}
309+
275310
private void setValue(String expression, Object value) {
276311
Class<?> expectedType = value.getClass();
277312
try {

0 commit comments

Comments
 (0)