From 85a781d5179910f97f762007568a13cd7faf9832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Fri, 1 Mar 2024 11:43:16 +0100 Subject: [PATCH] Instantiate value class parameters with Kotlin reflection In order to invoke the init block and to improve the maintainability. Closes gh-32324 --- .../springframework/core/CoroutinesUtils.java | 11 ++------ .../core/CoroutinesUtilsTests.kt | 25 +++++++++++++++++ .../support/InvocableHandlerMethod.java | 13 ++------- .../InvocableHandlerMethodKotlinTests.kt | 28 +++++++++++++++++-- .../result/method/InvocableHandlerMethod.java | 13 ++------- .../InvocableHandlerMethodKotlinTests.kt | 26 +++++++++++++++++ 6 files changed, 85 insertions(+), 31 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/CoroutinesUtils.java b/spring-core/src/main/java/org/springframework/core/CoroutinesUtils.java index b7c413da278..0599f824ead 100644 --- a/spring-core/src/main/java/org/springframework/core/CoroutinesUtils.java +++ b/spring-core/src/main/java/org/springframework/core/CoroutinesUtils.java @@ -18,7 +18,6 @@ package org.springframework.core; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Modifier; import java.util.Map; import java.util.Objects; @@ -30,6 +29,7 @@ import kotlin.reflect.KClassifier; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; import kotlin.reflect.full.KCallables; +import kotlin.reflect.full.KClasses; import kotlin.reflect.jvm.KCallablesJvm; import kotlin.reflect.jvm.ReflectJvmMapping; import kotlinx.coroutines.BuildersKt; @@ -46,7 +46,6 @@ import reactor.core.publisher.Mono; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; -import org.springframework.util.ReflectionUtils; /** * Utilities for working with Kotlin Coroutines. @@ -57,9 +56,6 @@ import org.springframework.util.ReflectionUtils; */ public abstract class CoroutinesUtils { - private static final ReflectionUtils.MethodFilter boxImplFilter = - (method -> method.isSynthetic() && Modifier.isStatic(method.getModifiers()) && method.getName().equals("box-impl")); - /** * Convert a {@link Deferred} instance to a {@link Mono}. */ @@ -123,10 +119,7 @@ public abstract class CoroutinesUtils { if (parameter.getType().getClassifier() instanceof KClass kClass) { Class javaClass = JvmClassMappingKt.getJavaClass(kClass); if (KotlinDetector.isInlineClass(javaClass)) { - Method[] methods = ReflectionUtils.getUniqueDeclaredMethods(javaClass, boxImplFilter); - Assert.state(methods.length == 1, - "Unable to find a single box-impl synthetic static method in " + javaClass.getName()); - argMap.put(parameter, ReflectionUtils.invokeMethod(methods[0], null, args[index])); + argMap.put(parameter, KClasses.getPrimaryConstructor(kClass).call(args[index])); } else { argMap.put(parameter, args[index]); diff --git a/spring-core/src/test/kotlin/org/springframework/core/CoroutinesUtilsTests.kt b/spring-core/src/test/kotlin/org/springframework/core/CoroutinesUtilsTests.kt index fdce5caf8d8..c92e6036619 100644 --- a/spring-core/src/test/kotlin/org/springframework/core/CoroutinesUtilsTests.kt +++ b/spring-core/src/test/kotlin/org/springframework/core/CoroutinesUtilsTests.kt @@ -154,6 +154,17 @@ class CoroutinesUtilsTests { } } + @Test + fun invokeSuspendingFunctionWithValueClassWithInitParameter() { + val method = CoroutinesUtilsTests::class.java.declaredMethods.first { it.name.startsWith("suspendingFunctionWithValueClassWithInit") } + val mono = CoroutinesUtils.invokeSuspendingFunction(method, this, "", null) as Mono + Assertions.assertThatIllegalArgumentException().isThrownBy { + runBlocking { + mono.awaitSingle() + } + } + } + @Test fun invokeSuspendingFunctionWithExtension() { val method = CoroutinesUtilsTests::class.java.getDeclaredMethod("suspendingFunctionWithExtension", @@ -206,6 +217,11 @@ class CoroutinesUtilsTests { return value.value } + suspend fun suspendingFunctionWithValueClassWithInit(value: ValueClassWithInit): String { + delay(1) + return value.value + } + suspend fun CustomException.suspendingFunctionWithExtension(): String { delay(1) return "${this.message}" @@ -219,6 +235,15 @@ class CoroutinesUtilsTests { @JvmInline value class ValueClass(val value: String) + @JvmInline + value class ValueClassWithInit(val value: String) { + init { + if (value.isEmpty()) { + throw IllegalArgumentException() + } + } + } + class CustomException(message: String) : Throwable(message) } diff --git a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java index bfa90a69120..caa6b85417b 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java @@ -18,7 +18,6 @@ package org.springframework.web.method.support; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Modifier; import java.util.Arrays; import java.util.Map; @@ -27,6 +26,7 @@ import kotlin.jvm.JvmClassMappingKt; import kotlin.reflect.KClass; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; +import kotlin.reflect.full.KClasses; import kotlin.reflect.jvm.KCallablesJvm; import kotlin.reflect.jvm.ReflectJvmMapping; @@ -37,10 +37,8 @@ import org.springframework.core.KotlinDetector; import org.springframework.core.MethodParameter; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.lang.Nullable; -import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; -import org.springframework.util.ReflectionUtils; import org.springframework.validation.method.MethodValidator; import org.springframework.web.bind.WebDataBinder; import org.springframework.web.bind.support.SessionStatus; @@ -64,9 +62,6 @@ public class InvocableHandlerMethod extends HandlerMethod { private static final Class[] EMPTY_GROUPS = new Class[0]; - private static final ReflectionUtils.MethodFilter boxImplFilter = - (method -> method.isSynthetic() && Modifier.isStatic(method.getModifiers()) && method.getName().equals("box-impl")); - private HandlerMethodArgumentResolverComposite resolvers = new HandlerMethodArgumentResolverComposite(); @@ -322,10 +317,7 @@ public class InvocableHandlerMethod extends HandlerMethod { if (parameter.getType().getClassifier() instanceof KClass kClass) { Class javaClass = JvmClassMappingKt.getJavaClass(kClass); if (KotlinDetector.isInlineClass(javaClass)) { - Method[] methods = ReflectionUtils.getUniqueDeclaredMethods(javaClass, boxImplFilter); - Assert.state(methods.length == 1, - "Unable to find a single box-impl synthetic static method in " + javaClass.getName()); - argMap.put(parameter, ReflectionUtils.invokeMethod(methods[0], null, args[index])); + argMap.put(parameter, KClasses.getPrimaryConstructor(kClass).call(args[index])); } else { argMap.put(parameter, args[index]); @@ -342,6 +334,7 @@ public class InvocableHandlerMethod extends HandlerMethod { Object result = function.callBy(argMap); return (result == Unit.INSTANCE ? null : result); } + } } diff --git a/spring-web/src/test/kotlin/org/springframework/web/method/support/InvocableHandlerMethodKotlinTests.kt b/spring-web/src/test/kotlin/org/springframework/web/method/support/InvocableHandlerMethodKotlinTests.kt index 2ba7f249d42..e23284d8c31 100644 --- a/spring-web/src/test/kotlin/org/springframework/web/method/support/InvocableHandlerMethodKotlinTests.kt +++ b/spring-web/src/test/kotlin/org/springframework/web/method/support/InvocableHandlerMethodKotlinTests.kt @@ -87,17 +87,24 @@ class InvocableHandlerMethodKotlinTests { @Test fun valueClass() { composite.addResolver(StubArgumentResolver(Long::class.java, 1L)) - val value = getInvocable(Handler::class.java, Long::class.java).invokeForRequest(request, null) + val value = getInvocable(ValueClassHandler::class.java, Long::class.java).invokeForRequest(request, null) Assertions.assertThat(value).isEqualTo(1L) } @Test fun valueClassDefaultValue() { composite.addResolver(StubArgumentResolver(Double::class.java)) - val value = getInvocable(Handler::class.java, Double::class.java).invokeForRequest(request, null) + val value = getInvocable(ValueClassHandler::class.java, Double::class.java).invokeForRequest(request, null) Assertions.assertThat(value).isEqualTo(3.1) } + @Test + fun valueClassWithInit() { + composite.addResolver(StubArgumentResolver(String::class.java, "")) + val invocable = getInvocable(ValueClassHandler::class.java, String::class.java) + Assertions.assertThatIllegalArgumentException().isThrownBy { invocable.invokeForRequest(request, null) } + } + @Test fun propertyAccessor() { val value = getInvocable(PropertyAccessorHandler::class.java).invokeForRequest(request, null) @@ -153,11 +160,19 @@ class InvocableHandlerMethodKotlinTests { return null } + } + + private class ValueClassHandler { + fun valueClass(limit: LongValueClass) = limit.value fun valueClass(limit: DoubleValueClass = DoubleValueClass(3.1)) = limit.value + + fun valueClassWithInit(valueClass: ValueClassWithInit) = + valueClass + } private class PropertyAccessorHandler { @@ -183,6 +198,15 @@ class InvocableHandlerMethodKotlinTests { @JvmInline value class DoubleValueClass(val value: Double) + @JvmInline + value class ValueClassWithInit(val value: String) { + init { + if (value.isEmpty()) { + throw IllegalArgumentException() + } + } + } + class CustomException(message: String) : Throwable(message) } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java index dbfa1585b20..1b9501c28d6 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java @@ -18,7 +18,6 @@ package org.springframework.web.reactive.result.method; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Modifier; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.ArrayList; @@ -32,6 +31,7 @@ import kotlin.jvm.JvmClassMappingKt; import kotlin.reflect.KClass; import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; +import kotlin.reflect.full.KClasses; import kotlin.reflect.jvm.KCallablesJvm; import kotlin.reflect.jvm.ReflectJvmMapping; import reactor.core.publisher.Mono; @@ -46,10 +46,8 @@ import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.http.HttpStatusCode; import org.springframework.http.server.reactive.ServerHttpResponse; import org.springframework.lang.Nullable; -import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; -import org.springframework.util.ReflectionUtils; import org.springframework.validation.method.MethodValidator; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.BindingContext; @@ -74,9 +72,6 @@ public class InvocableHandlerMethod extends HandlerMethod { private static final Object NO_ARG_VALUE = new Object(); - private static final ReflectionUtils.MethodFilter boxImplFilter = - (method -> method.isSynthetic() && Modifier.isStatic(method.getModifiers()) && method.getName().equals("box-impl")); - private final HandlerMethodArgumentResolverComposite resolvers = new HandlerMethodArgumentResolverComposite(); @@ -333,10 +328,7 @@ public class InvocableHandlerMethod extends HandlerMethod { if (parameter.getType().getClassifier() instanceof KClass kClass) { Class javaClass = JvmClassMappingKt.getJavaClass(kClass); if (KotlinDetector.isInlineClass(javaClass)) { - Method[] methods = ReflectionUtils.getUniqueDeclaredMethods(javaClass, boxImplFilter); - Assert.state(methods.length == 1, - "Unable to find a single box-impl synthetic static method in " + javaClass.getName()); - argMap.put(parameter, ReflectionUtils.invokeMethod(methods[0], null, args[index])); + argMap.put(parameter, KClasses.getPrimaryConstructor(kClass).call(args[index])); } else { argMap.put(parameter, args[index]); @@ -354,6 +346,7 @@ public class InvocableHandlerMethod extends HandlerMethod { return (result == Unit.INSTANCE ? null : result); } } + } } diff --git a/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/InvocableHandlerMethodKotlinTests.kt b/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/InvocableHandlerMethodKotlinTests.kt index 85c30a34159..d021c9a6d29 100644 --- a/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/InvocableHandlerMethodKotlinTests.kt +++ b/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/InvocableHandlerMethodKotlinTests.kt @@ -19,6 +19,7 @@ package org.springframework.web.reactive.result import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.delay +import org.assertj.core.api.Assertions import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.Test import org.springframework.core.MethodParameter @@ -41,6 +42,7 @@ import reactor.core.publisher.Mono import reactor.test.StepVerifier import java.lang.reflect.Method import java.time.Duration +import kotlin.reflect.KClass import kotlin.reflect.jvm.javaMethod /** @@ -197,6 +199,14 @@ class InvocableHandlerMethodKotlinTests { assertHandlerResultValue(result, "3.1") } + @Test + fun valueClassWithInit() { + this.resolvers.add(stubResolver("", String::class.java)) + val method = ValueClassController::valueClassWithInit.javaMethod!! + val result = invoke(ValueClassController(), method) + assertExceptionThrown(result, IllegalArgumentException::class) + } + @Test fun propertyAccessor() { this.resolvers.add(stubResolver(null, String::class.java)) @@ -256,6 +266,10 @@ class InvocableHandlerMethodKotlinTests { }.verifyComplete() } + private fun assertExceptionThrown(mono: Mono, exceptionClass: KClass) { + StepVerifier.create(mono).verifyError(exceptionClass.java) + } + class CoroutinesController { suspend fun singleArg(q: String?): String { @@ -320,6 +334,9 @@ class InvocableHandlerMethodKotlinTests { fun valueClassWithDefault(limit: DoubleValueClass = DoubleValueClass(3.1)) = "${limit.value}" + fun valueClassWithInit(valueClass: ValueClassWithInit) = + valueClass + } class PropertyAccessorController { @@ -346,5 +363,14 @@ class InvocableHandlerMethodKotlinTests { @JvmInline value class DoubleValueClass(val value: Double) + @JvmInline + value class ValueClassWithInit(val value: String) { + init { + if (value.isEmpty()) { + throw IllegalArgumentException() + } + } + } + class CustomException(message: String) : Throwable(message) } \ No newline at end of file