From aa7e84c89fde09c9563ede41d73f74ec83093c16 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 11 Feb 2025 15:48:00 +0100 Subject: [PATCH 1/2] Polishing --- .../expression/spel/VariableAndFunctionTests.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 88f8b0b8b39d..3cdccda00739 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 @@ -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. @@ -19,6 +19,7 @@ import org.junit.jupiter.api.Test; import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.expression.spel.standard.SpelExpression; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.support.StandardTypeLocator; @@ -266,11 +267,10 @@ void functionViaMethodHandleForStaticMethodThatAcceptsOnlyVarargs() { @Test void functionMethodMustBeStatic() throws Exception { - SpelExpressionParser parser = new SpelExpressionParser(); - StandardEvaluationContext ctx = new StandardEvaluationContext(); - ctx.setVariable("notStatic", this.getClass().getMethod("nonStatic")); + context.registerFunction("nonStatic", this.getClass().getMethod("nonStatic")); + SpelExpression expression = parser.parseRaw("#nonStatic()"); assertThatExceptionOfType(SpelEvaluationException.class) - .isThrownBy(() -> parser.parseRaw("#notStatic()").getValue(ctx)) + .isThrownBy(() -> expression.getValue(context)) .satisfies(ex -> assertThat(ex.getMessageCode()).isEqualTo(FUNCTION_MUST_BE_STATIC)); } From b07217ab677d56b0f392fda4cdfe8c0785f0eff5 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Tue, 11 Feb 2025 16:00:32 +0100 Subject: [PATCH 2/2] Use ConversionService to convert POJO to array for SpEL varargs invocations 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 --- .../spel/support/ReflectionHelper.java | 10 +-- .../expression/spel/TestScenarioCreator.java | 7 +- .../spel/VariableAndFunctionTests.java | 82 ++++++++++++++++++- 3 files changed, 92 insertions(+), 7 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 c45464e5451b..bb230b6dd9f5 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 @@ -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. @@ -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: @@ -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: 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 d9280f5be4bf..a99d3f5aea3f 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 @@ -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. @@ -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)); 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 3cdccda00739..4e5e22df0c59 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 @@ -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; @@ -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 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(); + } + } + + } + }