From e088892fc122875479c01bdc5561085f2044869c Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Fri, 12 Jul 2024 16:25:43 +0200 Subject: [PATCH] Support MethodHandle invocation with primitive varargs array in SpEL 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 --- .../spel/support/ReflectionHelper.java | 32 +++++++---- .../spel/MethodInvocationTests.java | 23 ++++++++ .../expression/spel/TestScenarioCreator.java | 19 +++++++ .../spel/VariableAndFunctionTests.java | 57 ++++++++++++++++++- .../spel/testresources/Inventor.java | 8 +++ 5 files changed, 124 insertions(+), 15 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java index 0d918214524..a5cdea8bbb2 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectionHelper.java @@ -384,11 +384,15 @@ public abstract class ReflectionHelper { 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) { @@ -396,17 +400,21 @@ public abstract class ReflectionHelper { 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. @@ -424,7 +432,7 @@ public abstract class ReflectionHelper { 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]); } } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java index b84a57498ee..e54d175c45e 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/MethodInvocationTests.java @@ -245,6 +245,7 @@ class MethodInvocationTests extends AbstractExpressionTests { 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); @@ -320,6 +321,7 @@ class MethodInvocationTests extends AbstractExpressionTests { // 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); @@ -333,6 +335,27 @@ class MethodInvocationTests extends AbstractExpressionTests { 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... values)' diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java index 4c3442a4d84..895952f62a4 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/TestScenarioCreator.java @@ -66,6 +66,8 @@ class TestScenarioCreator { 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); @@ -106,6 +108,11 @@ class TestScenarioCreator { "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)); @@ -160,6 +167,10 @@ class TestScenarioCreator { 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); } @@ -168,6 +179,14 @@ class TestScenarioCreator { 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; } diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java index a725b95306e..9cd3e21f3da 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/VariableAndFunctionTests.java @@ -80,9 +80,11 @@ class VariableAndFunctionTests extends AbstractExpressionTests { 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); @@ -103,6 +105,21 @@ class VariableAndFunctionTests extends AbstractExpressionTests { // 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 @@ -110,17 +127,25 @@ class VariableAndFunctionTests extends AbstractExpressionTests { // 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); @@ -131,9 +156,12 @@ class VariableAndFunctionTests extends AbstractExpressionTests { 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); @@ -147,6 +175,29 @@ class VariableAndFunctionTests extends AbstractExpressionTests { 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(); diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java index a40106d2124..99c5eaa4ccc 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/testresources/Inventor.java @@ -221,6 +221,14 @@ public class Inventor { 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) {