Skip to content

Commit

Permalink
Support MethodHandle invocation with primitive varargs array in SpEL
Browse files Browse the repository at this point in the history 
Prior to this commit, the Spring Expression Language (SpEL) could not
invoke a varargs MethodHandle function with a primitive array
containing the variable arguments, although that is supported for a
varargs Method function. Attempting to do so resulted in the first
element of the primitive array being supplied as a single argument to
the MethodHandle, effectively ignoring any variable arguments after the
first one.

This commit addresses this by updating the
convertAllMethodHandleArguments(...) method in ReflectionHelper as
follows when the user supplies the varargs already packaged in a
primitive array.

- Regarding conversion, use the wrapper type for a primitive varargs
  array, since we eventually need an Object array in order to invoke
  the MethodHandle in FunctionReference#executeFunctionViaMethodHandle().

- When deciding whether to convert a single element passed as varargs,
  we now check if the argument is an array that is assignable to the
  varargs array type.

- When converting an array supplied as the varargs, we now convert that
  array to the varargs array type instead of the varargs component type.

Note, however, that a SpEL expression cannot provide a primitive array
for an Object[] varargs target. This is due to the fact that the
ArrayToArrayConverter used by Spring's ConversionService does not
support conversion from a primitive array to Object[] -- for example,
from int[] to Object[].

See gh-33191
Closes gh-33198
  • Loading branch information
@sbrannen
sbrannen committed Jul 12, 2024
1 parent 152914a commit e088892
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 15 deletions.
32 changes: 20 additions & 12 deletions ...xpression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,29 +384,37 @@ public static boolean convertAllMethodHandleArguments(TypeConverter converter, O
conversionOccurred |= (argument != arguments[i]);
}

Class<?> varArgClass = methodHandleType.lastParameterType();
ResolvableType varArgResolvableType = ResolvableType.forClass(varArgClass);
TypeDescriptor targetType = new TypeDescriptor(varArgResolvableType, varArgClass.componentType(), null);
TypeDescriptor componentTypeDesc = targetType.getElementTypeDescriptor();
Assert.state(componentTypeDesc != null, "Component type must not be null for a varargs array");
Class<?> varargsArrayClass = methodHandleType.lastParameterType();
// We use the wrapper type for a primitive varargs array, since we eventually
// need an Object array in order to invoke the MethodHandle in
// FunctionReference#executeFunctionViaMethodHandle().
Class<?> varargsComponentClass = ClassUtils.resolvePrimitiveIfNecessary(varargsArrayClass.componentType());
TypeDescriptor varargsArrayType = TypeDescriptor.array(TypeDescriptor.valueOf(varargsComponentClass));
Assert.state(varargsArrayType != null, "Array type must not be null for a varargs array");
TypeDescriptor varargsComponentType = varargsArrayType.getElementTypeDescriptor();
Assert.state(varargsComponentType != null, "Component type must not be null for a varargs array");

// If the target is varargs and there is just one more argument, then convert it here.
if (varargsPosition == arguments.length - 1) {
Object argument = arguments[varargsPosition];
TypeDescriptor sourceType = TypeDescriptor.forObject(argument);
if (argument == null) {
// Perform the equivalent of GenericConversionService.convertNullSource() for a single argument.
if (componentTypeDesc.getObjectType() == Optional.class) {
if (varargsComponentType.getObjectType() == Optional.class) {
arguments[varargsPosition] = Optional.empty();
conversionOccurred = true;
}
}
// If the argument type is assignable to the varargs component type, there is no need to
// convert it or wrap it in an array. For example, using StringToArrayConverter to
// convert a String containing a comma would result in the String being split and
// repackaged in an array when it should be used as-is.
else if (!sourceType.isAssignableTo(componentTypeDesc)) {
arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetType);
// convert it. For example, using StringToArrayConverter to convert a String containing a
// comma would result in the String being split and repackaged in an array when it should
// be used as-is. Similarly, if the argument is an array that is assignable to the varargs
// array type, there is no need to convert it.
else if (!sourceType.isAssignableTo(varargsComponentType) ||
(sourceType.isArray() && !sourceType.isAssignableTo(varargsArrayType))) {

TypeDescriptor targetTypeToUse = (sourceType.isArray() ? varargsArrayType : varargsComponentType);
arguments[varargsPosition] = converter.convertValue(argument, sourceType, targetTypeToUse);
}
// Possible outcomes of the above if-else block:
// 1) the input argument was null, and nothing was done.
Expand All @@ -424,7 +432,7 @@ else if (!sourceType.isAssignableTo(componentTypeDesc)) {
for (int i = varargsPosition; i < arguments.length; i++) {
Object argument = arguments[i];
TypeDescriptor sourceType = TypeDescriptor.forObject(argument);
arguments[i] = converter.convertValue(argument, sourceType, componentTypeDesc);
arguments[i] = converter.convertValue(argument, sourceType, varargsComponentType);
conversionOccurred |= (argument != arguments[i]);
}
}
Expand Down
23 changes: 23 additions & 0 deletions ...g-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ void testVarargsInvocation01() {
evaluate("aVarargsMethod(1,'a',3.0d)", "[1, a, 3.0]", String.class); // first and last need conversion
evaluate("aVarargsMethod(new String[]{'a','b','c'})", "[a, b, c]", String.class);
evaluate("aVarargsMethod(new String[]{})", "[]", String.class);
evaluate("aVarargsMethod(new int[]{1, 2, 3})", "[1, 2, 3]", String.class); // needs int[] to String[] conversion
evaluate("aVarargsMethod(null)", "[null]", String.class);
evaluate("aVarargsMethod(null,'a')", "[null, a]", String.class);
evaluate("aVarargsMethod('a',null,'b')", "[a, null, b]", String.class);
Expand Down Expand Up @@ -320,6 +321,7 @@ void testVarargsWithObjectArrayType() {
// Conversion necessary
evaluate("formatObjectVarargs('x -> %s %s', 2, 3)", "x -> 2 3", String.class);
evaluate("formatObjectVarargs('x -> %s %s', 'a', 3.0d)", "x -> a 3.0", String.class);
evaluate("formatObjectVarargs('x -> %s %s %s', new Integer[]{1, 2, 3})", "x -> 1 2 3", String.class);

// Individual string contains a comma with multiple varargs arguments
evaluate("formatObjectVarargs('foo -> %s %s', ',', 'baz')", "foo -> , baz", String.class);
Expand All @@ -333,6 +335,27 @@ void testVarargsWithObjectArrayType() {
evaluate("formatObjectVarargs('foo -> %s', 'bar,baz')", "foo -> bar,baz", String.class);
}

@Test
void testVarargsWithPrimitiveArrayType() {
// Calling 'public String formatPrimitiveVarargs(String format, int... nums)' -> effectively String.format(format, args)

// No var-args and no conversion necessary
evaluate("formatPrimitiveVarargs(9)", "9", String.class);

// No var-args but conversion necessary
evaluate("formatPrimitiveVarargs('7')", "7", String.class);

// No conversion necessary
evaluate("formatPrimitiveVarargs('x -> %s', 9)", "x -> 9", String.class);
evaluate("formatPrimitiveVarargs('x -> %s %s %s', 1, 2, 3)", "x -> 1 2 3", String.class);
evaluate("formatPrimitiveVarargs('x -> %s', new int[]{1})", "x -> 1", String.class);
evaluate("formatPrimitiveVarargs('x -> %s %s %s', new int[]{1, 2, 3})", "x -> 1 2 3", String.class);

// Conversion necessary
evaluate("formatPrimitiveVarargs('x -> %s %s', '2', '3')", "x -> 2 3", String.class);
evaluate("formatPrimitiveVarargs('x -> %s %s', '2', 3.0d)", "x -> 2 3", String.class);
}

@Test
void testVarargsOptionalInvocation() {
// Calling 'public String optionalVarargsMethod(Optional<String>... values)'
Expand Down
19 changes: 19 additions & 0 deletions spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ private static void populateFunctions(StandardEvaluationContext testContext) {
TestScenarioCreator.class.getDeclaredMethod("varargsFunction", String[].class));
testContext.registerFunction("varargsFunction2",
TestScenarioCreator.class.getDeclaredMethod("varargsFunction2", int.class, String[].class));
testContext.registerFunction("varargsObjectFunction",
TestScenarioCreator.class.getDeclaredMethod("varargsObjectFunction", Object[].class));
}
catch (Exception ex) {
throw new IllegalStateException(ex);
Expand Down Expand Up @@ -106,6 +108,11 @@ private static void populateMethodHandles(StandardEvaluationContext testContext)
"formatObjectVarargs", MethodType.methodType(String.class, String.class, Object[].class));
testContext.registerFunction("formatObjectVarargs", formatObjectVarargs);

// #formatObjectVarargs(format, args...)
MethodHandle formatPrimitiveVarargs = MethodHandles.lookup().findStatic(TestScenarioCreator.class,
"formatPrimitiveVarargs", MethodType.methodType(String.class, String.class, int[].class));
testContext.registerFunction("formatPrimitiveVarargs", formatPrimitiveVarargs);

// #add(int, int)
MethodHandle add = MethodHandles.lookup().findStatic(TestScenarioCreator.class,
"add", MethodType.methodType(int.class, int.class, int.class));
Expand Down Expand Up @@ -160,6 +167,10 @@ public static String varargsFunction2(int i, String... strings) {
return i + "-" + Arrays.toString(strings);
}

public static String varargsObjectFunction(Object... args) {
return Arrays.toString(args);
}

public static String message(String template, String... args) {
return template.formatted((Object[]) args);
}
Expand All @@ -168,6 +179,14 @@ public static String formatObjectVarargs(String format, Object... args) {
return String.format(format, args);
}

public static String formatPrimitiveVarargs(String format, int... nums) {
Object[] args = new Object[nums.length];
for (int i = 0; i < nums.length; i++) {
args[i] = nums[i];
}
return String.format(format, args);
}

public static int add(int x, int y) {
return x + y;
}
Expand Down
57 changes: 54 additions & 3 deletions ...xpression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,11 @@ void functionWithVarargs() {
evaluate("#varargsFunction(new String[0])", "[]", String.class);
evaluate("#varargsFunction('a')", "[a]", String.class);
evaluate("#varargsFunction('a','b','c')", "[a, b, c]", String.class);
evaluate("#varargsFunction(new String[]{'a','b','c'})", "[a, b, c]", String.class);
// Conversion from int to String
evaluate("#varargsFunction(25)", "[25]", String.class);
evaluate("#varargsFunction('b',25)", "[b, 25]", String.class);
evaluate("#varargsFunction(new int[]{1, 2, 3})", "[1, 2, 3]", String.class);
// Strings that contain a comma
evaluate("#varargsFunction('a,b')", "[a,b]", String.class);
evaluate("#varargsFunction('a', 'x,y', 'd')", "[a, x,y, d]", String.class);
Expand All @@ -103,24 +105,47 @@ void functionWithVarargs() {
// null values
evaluate("#varargsFunction2(9,null)", "9-[null]", String.class);
evaluate("#varargsFunction2(9,'a',null,'b')", "9-[a, null, b]", String.class);

evaluate("#varargsObjectFunction()", "[]", String.class);
evaluate("#varargsObjectFunction(new String[0])", "[]", String.class);
evaluate("#varargsObjectFunction('a')", "[a]", String.class);
evaluate("#varargsObjectFunction('a','b','c')", "[a, b, c]", String.class);
evaluate("#varargsObjectFunction(new String[]{'a','b','c'})", "[a, b, c]", String.class);
// Conversion from int to String
evaluate("#varargsObjectFunction(25)", "[25]", String.class);
evaluate("#varargsObjectFunction('b',25)", "[b, 25]", String.class);
// Strings that contain a comma
evaluate("#varargsObjectFunction('a,b')", "[a,b]", String.class);
evaluate("#varargsObjectFunction('a', 'x,y', 'd')", "[a, x,y, d]", String.class);
// null values
evaluate("#varargsObjectFunction(null)", "[null]", String.class);
evaluate("#varargsObjectFunction('a',null,'b')", "[a, null, b]", String.class);
}

@Test // gh-33013
void functionWithVarargsViaMethodHandle() {
// Calling 'public static String formatObjectVarargs(String format, Object... args)' -> String.format(format, args)

// No var-args and no conversion necessary
evaluate("#message('x')", "x", String.class);
evaluate("#formatObjectVarargs('x')", "x", String.class);

// No var-args but conversion necessary
evaluate("#message(9)", "9", String.class);
evaluate("#formatObjectVarargs(9)", "9", String.class);

// No conversion necessary
evaluate("#add(3, 4)", 7, Integer.class);
evaluate("#message('x -> %s %s %s', 'a', 'b', 'c')", "x -> a b c", String.class);
evaluate("#formatObjectVarargs('x -> %s', '')", "x -> ", String.class);
evaluate("#formatObjectVarargs('x -> %s', ' ')", "x -> ", String.class);
evaluate("#formatObjectVarargs('x -> %s', 'a')", "x -> a", String.class);
evaluate("#formatObjectVarargs('x -> %s %s %s', 'a', 'b', 'c')", "x -> a b c", String.class);
evaluate("#message('x -> %s %s %s', new Object[]{'a', 'b', 'c'})", "x -> a b c", String.class); // Object[] instanceof Object[]
evaluate("#message('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class); // String[] instanceof Object[]
evaluate("#message('x -> %s %s %s', new Integer[]{1, 2, 3})", "x -> 1 2 3", String.class); // Integer[] instanceof Object[]
evaluate("#formatObjectVarargs('x -> %s %s', 2, 3)", "x -> 2 3", String.class); // Integer instanceof Object
evaluate("#formatObjectVarargs('x -> %s %s', 'a', 3.0F)", "x -> a 3.0", String.class); // String/Float instanceof Object
evaluate("#formatObjectVarargs('x -> %s', new Object[]{''})", "x -> ", String.class);
evaluate("#formatObjectVarargs('x -> %s', new String[]{''})", "x -> ", String.class);
evaluate("#formatObjectVarargs('x -> %s', new Object[]{' '})", "x -> ", String.class);
Expand All @@ -131,9 +156,12 @@ void functionWithVarargsViaMethodHandle() {
evaluate("#formatObjectVarargs('x -> %s %s %s', new String[]{'a', 'b', 'c'})", "x -> a b c", String.class);

// Conversion necessary
evaluate("#add('2', 5.0)", 7, Integer.class);
evaluate("#formatObjectVarargs('x -> %s %s', 2, 3)", "x -> 2 3", String.class);
evaluate("#formatObjectVarargs('x -> %s %s', 'a', 3.0d)", "x -> a 3.0", String.class);
evaluate("#add('2', 5.0)", 7, Integer.class); // String/Double to Integer
evaluate("#messageStatic('x -> %s %s %s', 1, 2, 3)", "x -> 1 2 3", String.class); // Integer to String
evaluate("#messageStatic('x -> %s %s %s', new Integer[]{1, 2, 3})", "x -> 1 2 3", String.class); // Integer[] to String[]
evaluate("#messageStatic('x -> %s %s %s', new int[]{1, 2, 3})", "x -> 1 2 3", String.class); // int[] to String[]
evaluate("#messageStatic('x -> %s %s %s', new short[]{1, 2, 3})", "x -> 1 2 3", String.class); // short[] to String[]
evaluate("#formatObjectVarargs('x -> %s %s %s', new Integer[]{1, 2, 3})", "x -> 1 2 3", String.class); // Integer[] to String[]

// Individual string contains a comma with multiple varargs arguments
evaluate("#formatObjectVarargs('foo -> %s %s', ',', 'baz')", "foo -> , baz", String.class);
Expand All @@ -147,6 +175,29 @@ void functionWithVarargsViaMethodHandle() {
evaluate("#formatObjectVarargs('foo -> %s', 'bar,baz')", "foo -> bar,baz", String.class);
}

@Test
void functionWithPrimitiveVarargsViaMethodHandle() {
// Calling 'public String formatPrimitiveVarargs(String format, int... nums)' -> effectively String.format(format, args)

// No var-args and no conversion necessary
evaluate("#formatPrimitiveVarargs(9)", "9", String.class);

// No var-args but conversion necessary
evaluate("#formatPrimitiveVarargs('7')", "7", String.class);

// No conversion necessary
evaluate("#formatPrimitiveVarargs('x -> %s', 9)", "x -> 9", String.class);
evaluate("#formatPrimitiveVarargs('x -> %s %s %s', 1, 2, 3)", "x -> 1 2 3", String.class);
evaluate("#formatPrimitiveVarargs('x -> %s', new int[]{1})", "x -> 1", String.class);
evaluate("#formatPrimitiveVarargs('x -> %s %s %s', new int[]{1, 2, 3})", "x -> 1 2 3", String.class);

// Conversion necessary
evaluate("#formatPrimitiveVarargs('x -> %s %s', '2', '3')", "x -> 2 3", String.class); // String to int
evaluate("#formatPrimitiveVarargs('x -> %s %s', '2', 3.0F)", "x -> 2 3", String.class); // String/Float to int
evaluate("#formatPrimitiveVarargs('x -> %s %s %s', new Integer[]{1, 2, 3})", "x -> 1 2 3", String.class); // Integer[] to int[]
evaluate("#formatPrimitiveVarargs('x -> %s %s %s', new String[]{'1', '2', '3'})", "x -> 1 2 3", String.class); // String[] to int[]
}

@Test
void functionMethodMustBeStatic() throws Exception {
SpelExpressionParser parser = new SpelExpressionParser();
Expand Down
8 changes: 8 additions & 0 deletions ...-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,14 @@ public String formatObjectVarargs(String format, Object... args) {
return String.format(format, args);
}

public String formatPrimitiveVarargs(String format, int... nums) {
Object[] args = new Object[nums.length];
for (int i = 0; i < nums.length; i++) {
args[i] = nums[i];
}
return String.format(format, args);
}


public Inventor(String... strings) {
if (strings.length > 0) {
Expand Down

0 comments on commit e088892

Please sign in to comment.