Skip to content

Commit

Permalink
Support nullable Kotlin value class arguments
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sdeleuze committed Mar 1, 2024
1 parent de828e9 commit 227e75d
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -281,8 +282,12 @@ private static Object convertIfNecessary(
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(),
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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
Expand All @@ -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].
Expand Down Expand Up @@ -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() {
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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)

}

Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -197,8 +198,12 @@ private Object applyConversion(@Nullable Object value, NamedValueInfo namedValue
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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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].
Expand All @@ -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() {
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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)

}

0 comments on commit 227e75d

Please sign in to comment.