From 227e75dae48ee84a65c466161cd648df7f2873ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Fri, 1 Mar 2024 14:44:37 +0100 Subject: [PATCH] Support nullable Kotlin value class arguments This commit refines WebMVC and WebFlux argument resolution in order to convert properly Kotlin value class arguments by using the type of the value instead of the type of the value class. Closes gh-32353 --- ...tractNamedValueMethodArgumentResolver.java | 7 +++- ...tParamMethodArgumentResolverKotlinTests.kt | 38 ++++++++++++++--- .../AbstractNamedValueArgumentResolver.java | 7 +++- ...tParamMethodArgumentResolverKotlinTests.kt | 42 +++++++++++++++---- 4 files changed, 78 insertions(+), 16 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java b/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java index 18f6c805f48..332852691ee 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java @@ -26,6 +26,7 @@ import kotlin.reflect.KFunction; import kotlin.reflect.KParameter; import kotlin.reflect.jvm.ReflectJvmMapping; +import org.springframework.beans.BeanUtils; import org.springframework.beans.ConversionNotSupportedException; import org.springframework.beans.TypeMismatchException; import org.springframework.beans.factory.config.BeanExpressionContext; @@ -281,8 +282,12 @@ public abstract class AbstractNamedValueMethodArgumentResolver implements Handle NamedValueInfo namedValueInfo, @Nullable Object arg) throws Exception { WebDataBinder binder = binderFactory.createBinder(webRequest, null, namedValueInfo.name); + Class parameterType = parameter.getParameterType(); + if (KotlinDetector.isKotlinPresent() && KotlinDetector.isInlineClass(parameterType)) { + parameterType = BeanUtils.findPrimaryConstructor(parameterType).getParameterTypes()[0]; + } try { - arg = binder.convertIfNecessary(arg, parameter.getParameterType(), parameter); + arg = binder.convertIfNecessary(arg, parameterType, parameter); } catch (ConversionNotSupportedException ex) { throw new MethodArgumentConversionNotSupportedException(arg, ex.getRequiredType(), diff --git a/spring-web/src/test/kotlin/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt b/spring-web/src/test/kotlin/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt index f08c997bb57..d6e50b9f389 100644 --- a/spring-web/src/test/kotlin/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt +++ b/spring-web/src/test/kotlin/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -25,7 +25,6 @@ import org.springframework.core.annotation.SynthesizingMethodParameter import org.springframework.core.convert.support.DefaultConversionService import org.springframework.http.HttpMethod import org.springframework.http.MediaType -import org.springframework.util.ReflectionUtils import org.springframework.web.bind.MissingServletRequestParameterException import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.support.ConfigurableWebBindingInitializer @@ -39,6 +38,7 @@ import org.springframework.web.testfixture.servlet.MockHttpServletRequest import org.springframework.web.testfixture.servlet.MockHttpServletResponse import org.springframework.web.testfixture.servlet.MockMultipartFile import org.springframework.web.testfixture.servlet.MockMultipartHttpServletRequest +import kotlin.reflect.jvm.javaMethod /** * Kotlin test fixture for [RequestParamMethodArgumentResolver]. @@ -70,6 +70,9 @@ class RequestParamMethodArgumentResolverKotlinTests { lateinit var nonNullableMultipartParamRequired: MethodParameter lateinit var nonNullableMultipartParamNotRequired: MethodParameter + lateinit var nonNullableValueClassParam: MethodParameter + lateinit var nullableValueClassParam: MethodParameter + @BeforeEach fun setup() { @@ -80,10 +83,8 @@ class RequestParamMethodArgumentResolverKotlinTests { binderFactory = DefaultDataBinderFactory(initializer) webRequest = ServletWebRequest(request, MockHttpServletResponse()) - val method = ReflectionUtils.findMethod(javaClass, "handle", - String::class.java, String::class.java, String::class.java, String::class.java, - Boolean::class.java, Boolean::class.java, Int::class.java, Int::class.java, String::class.java, String::class.java, - MultipartFile::class.java, MultipartFile::class.java, MultipartFile::class.java, MultipartFile::class.java)!! + val method = RequestParamMethodArgumentResolverKotlinTests::handle.javaMethod!! + val valueClassMethod = RequestParamMethodArgumentResolverKotlinTests::handleValueClass.javaMethod!! nullableParamRequired = SynthesizingMethodParameter(method, 0) nullableParamNotRequired = SynthesizingMethodParameter(method, 1) @@ -101,6 +102,9 @@ class RequestParamMethodArgumentResolverKotlinTests { nullableMultipartParamNotRequired = SynthesizingMethodParameter(method, 11) nonNullableMultipartParamRequired = SynthesizingMethodParameter(method, 12) nonNullableMultipartParamNotRequired = SynthesizingMethodParameter(method, 13) + + nonNullableValueClassParam = SynthesizingMethodParameter(valueClassMethod, 0) + nullableValueClassParam = SynthesizingMethodParameter(valueClassMethod, 1) } @Test @@ -317,6 +321,20 @@ class RequestParamMethodArgumentResolverKotlinTests { } } + @Test + fun resolveNonNullableValueClass() { + request.addParameter("value", "123") + var result = resolver.resolveArgument(nonNullableValueClassParam, null, webRequest, binderFactory) + assertThat(result).isEqualTo(123) + } + + @Test + fun resolveNullableValueClass() { + request.addParameter("value", "123") + var result = resolver.resolveArgument(nullableValueClassParam, null, webRequest, binderFactory) + assertThat(result).isEqualTo(123) + } + @Suppress("unused_parameter") fun handle( @@ -338,5 +356,13 @@ class RequestParamMethodArgumentResolverKotlinTests { @RequestParam("mfile", required = false) nonNullableMultipartParamNotRequired: MultipartFile) { } + @Suppress("unused_parameter") + fun handleValueClass( + @RequestParam("value") nonNullable: ValueClass, + @RequestParam("value") nullable: ValueClass?) { + } + + @JvmInline value class ValueClass(val value: Int) + } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java index 6535158825c..4aff5cfd2c6 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/AbstractNamedValueArgumentResolver.java @@ -26,6 +26,7 @@ import kotlin.reflect.KParameter; import kotlin.reflect.jvm.ReflectJvmMapping; import reactor.core.publisher.Mono; +import org.springframework.beans.BeanUtils; import org.springframework.beans.ConversionNotSupportedException; import org.springframework.beans.TypeMismatchException; import org.springframework.beans.factory.config.BeanExpressionContext; @@ -197,8 +198,12 @@ public abstract class AbstractNamedValueArgumentResolver extends HandlerMethodAr BindingContext bindingContext, ServerWebExchange exchange) { WebDataBinder binder = bindingContext.createDataBinder(exchange, namedValueInfo.name); + Class parameterType = parameter.getParameterType(); + if (KotlinDetector.isKotlinPresent() && KotlinDetector.isInlineClass(parameterType)) { + parameterType = BeanUtils.findPrimaryConstructor(parameterType).getParameterTypes()[0]; + } try { - value = binder.convertIfNecessary(value, parameter.getParameterType(), parameter); + value = binder.convertIfNecessary(value, parameterType, parameter); } catch (ConversionNotSupportedException ex) { throw new ServerErrorException("Conversion not supported.", parameter, ex); diff --git a/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt b/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt index d9a088d91f6..68b11f9b2df 100644 --- a/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt +++ b/spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -22,14 +22,14 @@ import org.springframework.core.MethodParameter import org.springframework.core.ReactiveAdapterRegistry import org.springframework.core.annotation.SynthesizingMethodParameter import org.springframework.format.support.DefaultFormattingConversionService -import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest -import org.springframework.web.testfixture.server.MockServerWebExchange -import org.springframework.util.ReflectionUtils import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.support.ConfigurableWebBindingInitializer import org.springframework.web.reactive.BindingContext import org.springframework.web.server.ServerWebInputException +import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest +import org.springframework.web.testfixture.server.MockServerWebExchange import reactor.test.StepVerifier +import kotlin.reflect.jvm.javaMethod /** * Kotlin test fixture for [RequestParamMethodArgumentResolver]. @@ -53,6 +53,9 @@ class RequestParamMethodArgumentResolverKotlinTests { lateinit var defaultValueStringParamRequired: MethodParameter lateinit var defaultValueStringParamNotRequired: MethodParameter + lateinit var nonNullableValueClassParam: MethodParameter + lateinit var nullableValueClassParam: MethodParameter + @BeforeEach fun setup() { @@ -61,10 +64,8 @@ class RequestParamMethodArgumentResolverKotlinTests { initializer.conversionService = DefaultFormattingConversionService() bindingContext = BindingContext(initializer) - val method = ReflectionUtils.findMethod(javaClass, "handle", - String::class.java, String::class.java, String::class.java, String::class.java, - Boolean::class.java, Boolean::class.java, Int::class.java, Int::class.java, - String::class.java, String::class.java)!! + val method = RequestParamMethodArgumentResolverKotlinTests::handle.javaMethod!! + val valueClassMethod = RequestParamMethodArgumentResolverKotlinTests::handleValueClass.javaMethod!! nullableParamRequired = SynthesizingMethodParameter(method, 0) nullableParamNotRequired = SynthesizingMethodParameter(method, 1) @@ -77,6 +78,9 @@ class RequestParamMethodArgumentResolverKotlinTests { defaultValueIntParamNotRequired = SynthesizingMethodParameter(method, 7) defaultValueStringParamRequired = SynthesizingMethodParameter(method, 8) defaultValueStringParamNotRequired = SynthesizingMethodParameter(method, 9) + + nonNullableValueClassParam = SynthesizingMethodParameter(valueClassMethod, 0) + nullableValueClassParam = SynthesizingMethodParameter(valueClassMethod, 1) } @Test @@ -219,6 +223,20 @@ class RequestParamMethodArgumentResolverKotlinTests { StepVerifier.create(result).expectComplete().verify() } + @Test + fun resolveNonNullableValueClass() { + var exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path?value=123")) + var result = resolver.resolveArgument(nonNullableValueClassParam, bindingContext, exchange) + StepVerifier.create(result).expectNext(123).expectComplete().verify() + } + + @Test + fun resolveNullableValueClass() { + var exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/path?value=123")) + var result = resolver.resolveArgument(nullableValueClassParam, bindingContext, exchange) + StepVerifier.create(result).expectNext(123).expectComplete().verify() + } + @Suppress("unused_parameter") fun handle( @@ -235,5 +253,13 @@ class RequestParamMethodArgumentResolverKotlinTests { @RequestParam("value", required = false) withDefaultValueStringParamNotRequired: String = "default") { } + @Suppress("unused_parameter") + fun handleValueClass( + @RequestParam("value") nonNullable: ValueClass, + @RequestParam("value") nullable: ValueClass?) { + } + + @JvmInline value class ValueClass(val value: Int) + }