Skip to content

Commit

Permalink
Use ConversionService to convert POJO to array for SpEL varargs invoc…
Browse files Browse the repository at this point in the history
…ations

Prior to this commit, if an appropriate Converter was registered with
the ConversionService that converts from a POJO to an array and that
ConversionService was registered with the Spring Expression Language
(SpEL) TypeConverter, an attempt to invoke a varargs method in a SpEL
expression with such a POJO would fail because the ConversionService
was not used to convert the POJO to an array suitable for the varargs
method invocation.

This commit revises the implementations of convertArguments(...) and
convertAllMethodHandleArguments(...) in ReflectionHelper to support
such use cases.

Closes gh-34371
  • Loading branch information
sbrannen committed Feb 11, 2025
1 parent aa7e84c commit b07217a
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
Expand Down Expand Up @@ -319,8 +319,8 @@ else if (!sourceType.isAssignableTo(componentTypeDesc) ||
(sourceType.isArray() && !sourceType.isAssignableTo(targetType)) ||
(argument instanceof List)) {

TypeDescriptor targetTypeToUse =
(sourceType.isArray() || argument instanceof List ? targetType : componentTypeDesc);
TypeDescriptor targetTypeToUse = (sourceType.isArray() || argument instanceof List ||
converter.canConvert(sourceType, targetType) ? targetType : componentTypeDesc);
arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetTypeToUse);
}
// Possible outcomes of the above if-else block:
Expand Down Expand Up @@ -420,8 +420,8 @@ else if (!sourceType.isAssignableTo(varargsComponentType) ||
(sourceType.isArray() && !sourceType.isAssignableTo(varargsArrayType)) ||
(argument instanceof List)) {

TypeDescriptor targetTypeToUse =
(sourceType.isArray() || argument instanceof List ? varargsArrayType : varargsComponentType);
TypeDescriptor targetTypeToUse = (sourceType.isArray() || argument instanceof List ||
converter.canConvert(sourceType, varargsArrayType) ? varargsArrayType : varargsComponentType);
arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetTypeToUse);
}
// Possible outcomes of the above if-else block:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
Expand Down Expand Up @@ -118,6 +118,11 @@ private static void populateMethodHandles(StandardEvaluationContext testContext)
"varargsFunction", MethodType.methodType(String.class, String[].class));
testContext.registerFunction("varargsFunctionHandle", varargsFunctionHandle);

// #varargsObjectFunctionHandle(args...)
MethodHandle varargsObjectFunctionHandle = MethodHandles.lookup().findStatic(TestScenarioCreator.class,
"varargsObjectFunction", MethodType.methodType(String.class, Object[].class));
testContext.registerFunction("varargsObjectFunctionHandle", varargsObjectFunctionHandle);

// #add(int, int)
MethodHandle add = MethodHandles.lookup().findStatic(TestScenarioCreator.class,
"add", MethodType.methodType(int.class, int.class, int.class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,21 @@

package org.springframework.expression.spel;

import java.util.Set;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.GenericConverter;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.expression.Expression;
import org.springframework.expression.spel.standard.SpelExpression;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.expression.spel.support.StandardTypeConverter;
import org.springframework.expression.spel.support.StandardTypeLocator;
import org.springframework.lang.Nullable;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
Expand Down Expand Up @@ -279,4 +288,75 @@ void functionMethodMustBeStatic() throws Exception {
public void nonStatic() {
}


@Nested // gh-34371
class VarargsAndPojoToArrayConversionTests {

private final StandardEvaluationContext context = TestScenarioCreator.getTestEvaluationContext();

private final ArrayHolder arrayHolder = new ArrayHolder("a", "b", "c");


@BeforeEach
void setUp() {
DefaultConversionService conversionService = new DefaultConversionService();
conversionService.addConverter(new ArrayHolderConverter());
context.setTypeConverter(new StandardTypeConverter(conversionService));
context.setVariable("arrayHolder", arrayHolder);
}

@Test
void functionWithVarargsAndPojoToArrayConversion() {
// #varargsFunction: static String varargsFunction(String... strings) -> Arrays.toString(strings)
evaluate("#varargsFunction(#arrayHolder)", "[a, b, c]");

// #varargsObjectFunction: static String varargsObjectFunction(Object... args) -> Arrays.toString(args)
//
// Since ArrayHolder is an "instanceof Object" and Object is the varargs component type,
// we expect the ArrayHolder not to be converted to an array but rather to be passed
// "as is" as a single argument to the varargs method.
evaluate("#varargsObjectFunction(#arrayHolder)", "[" + arrayHolder.toString() + "]");
}

@Test
void functionWithVarargsAndPojoToArrayConversionViaMethodHandle() {
// #varargsFunctionHandle: static String varargsFunction(String... strings) -> Arrays.toString(strings)
evaluate("#varargsFunctionHandle(#arrayHolder)", "[a, b, c]");

// #varargsObjectFunctionHandle: static String varargsObjectFunction(Object... args) -> Arrays.toString(args)
//
// Since ArrayHolder is an "instanceof Object" and Object is the varargs component type,
// we expect the ArrayHolder not to be converted to an array but rather to be passed
// "as is" as a single argument to the varargs method.
evaluate("#varargsObjectFunctionHandle(#arrayHolder)", "[" + arrayHolder.toString() + "]");
}

private void evaluate(String expression, Object expectedValue) {
Expression expr = parser.parseExpression(expression);
assertThat(expr).as("expression").isNotNull();
Object value = expr.getValue(context);
assertThat(value).as("expression '" + expression + "'").isEqualTo(expectedValue);
}


record ArrayHolder(String... array) {
}

static class ArrayHolderConverter implements GenericConverter {

@Nullable
@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return Set.of(new ConvertiblePair(ArrayHolder.class, Object[].class));
}

@Nullable
@Override
public String[] convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
return ((ArrayHolder) source).array();
}
}

}

}

0 comments on commit b07217a

Please sign in to comment.