Skip to content

Support Kotlin parameter default values in handler methods #1741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,14 @@ public boolean isOptional() {
(KotlinDetector.isKotlinType(getContainingClass()) && KotlinDelegate.isOptional(this)));
}

/**
* Return whether this parameter declares a language-level default value,
* such as a default parameter in Kotlin.
*/
public boolean hasDefaultValue() {
return KotlinDetector.isKotlinType(getContainingClass()) && KotlinDelegate.hasDefaultValue(this);
}

/**
* Check whether this method parameter is annotated with any variant of a
* {@code Nullable} annotation, e.g. {@code javax.annotation.Nullable} or
Expand Down Expand Up @@ -745,6 +753,28 @@ private static int validateIndex(Executable executable, int parameterIndex) {
*/
private static class KotlinDelegate {

private static KParameter getKotlinParameter(KFunction<?> function, int index) {
List<KParameter> parameters = function.getParameters();
return parameters
.stream()
.filter(p -> KParameter.Kind.VALUE.equals(p.getKind()))
.collect(Collectors.toList())
.get(index);
}

@Nullable
private static KFunction<?> getKotlinFunction(@Nullable Method method, @Nullable Constructor<?> ctor) {
if (method != null) {
return ReflectJvmMapping.getKotlinFunction(method);
}
else if (ctor != null) {
return ReflectJvmMapping.getKotlinFunction(ctor);
}
else {
return null;
}
}

/**
* Check whether the specified {@link MethodParameter} represents a nullable Kotlin type
* or an optional parameter (with a default value in the Kotlin declaration).
Expand All @@ -758,25 +788,34 @@ public static boolean isOptional(MethodParameter param) {
return (function != null && function.getReturnType().isMarkedNullable());
}
else {
KFunction<?> function = null;
if (method != null) {
function = ReflectJvmMapping.getKotlinFunction(method);
}
else if (ctor != null) {
function = ReflectJvmMapping.getKotlinFunction(ctor);
}
KFunction<?> function = getKotlinFunction(method, ctor);
if (function != null) {
List<KParameter> parameters = function.getParameters();
KParameter parameter = parameters
.stream()
.filter(p -> KParameter.Kind.VALUE.equals(p.getKind()))
.collect(Collectors.toList())
.get(index);
KParameter parameter = getKotlinParameter(function, index);
return (parameter.getType().isMarkedNullable() || parameter.isOptional());
}
}
return false;
}

/**
* Check whether the specified {@link MethodParameter} has a default value in it's Kotlin declaration.
*/
public static boolean hasDefaultValue(MethodParameter param) {
int index = param.getParameterIndex();
if (index == -1) { // default value does not make sense for return types
return false;
}
else {
KFunction<?> function = getKotlinFunction(param.getMethod(), param.getConstructor());
if (function != null) {
KParameter parameter = getKotlinParameter(function, index);
return parameter.isOptional();
}
else {
return false;
}
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,18 @@ class KotlinMethodParameterTests {

lateinit var nonNullableMethod: Method

lateinit var withDefaultParameterMethod: Method

lateinit var withoutDefaultParameterMethod: Method


@Before
@Throws(NoSuchMethodException::class)
fun setup() {
nullableMethod = javaClass.getMethod("nullable", String::class.java)
nonNullableMethod = javaClass.getMethod("nonNullable", String::class.java)
withDefaultParameterMethod = javaClass.getMethod("withDefaultParameter", String::class.java)
withoutDefaultParameterMethod = javaClass.getMethod("withoutDefaultParameter", String::class.java)
}


Expand All @@ -57,11 +63,22 @@ class KotlinMethodParameterTests {
assertFalse(MethodParameter(nonNullableMethod, -1).isOptional())
}

@Test
fun `Method parameter default value`() {
assertTrue(MethodParameter(withDefaultParameterMethod, 0).hasDefaultValue())
assertFalse(MethodParameter(withoutDefaultParameterMethod, 0).hasDefaultValue())
}

@Suppress("unused", "unused_parameter")
fun nullable(p1: String?): Int? = 42

@Suppress("unused", "unused_parameter")
fun nonNullable(p1: String): Int = 42

@Suppress("unused", "unused_parameter")
fun withDefaultParameter(p1: String = "42") = 42

@Suppress("unused", "unused_parameter")
fun withoutDefaultParameter(p1: String) = 42

}
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,15 @@ public final Object resolveArgument(MethodParameter parameter, @Nullable ModelAn
else if (namedValueInfo.required && !nestedParameter.isOptional()) {
handleMissingValue(namedValueInfo.name, nestedParameter, webRequest);
}
arg = handleNullValue(namedValueInfo.name, arg, nestedParameter.getNestedParameterType());
arg = handleNullValue(namedValueInfo.name, arg, nestedParameter);
}
else if ("".equals(arg) && namedValueInfo.defaultValue != null) {
arg = resolveStringValue(namedValueInfo.defaultValue);
}

if (binderFactory != null) {
// if we have a resolved arg here (not null), we must convert in any case
// if we do not have a resolved arg here (null), only convert if we do not have a default value
if (binderFactory != null && (arg != null || !nestedParameter.hasDefaultValue())) {
WebDataBinder binder = binderFactory.createBinder(webRequest, null, namedValueInfo.name);
try {
arg = binder.convertIfNecessary(arg, parameter.getParameterType(), parameter);
Expand Down Expand Up @@ -235,12 +237,14 @@ protected void handleMissingValue(String name, MethodParameter parameter) throws
* A {@code null} results in a {@code false} value for {@code boolean}s or an exception for other primitives.
*/
@Nullable
private Object handleNullValue(String name, @Nullable Object value, Class<?> paramType) {
private Object handleNullValue(String name, @Nullable Object value, MethodParameter param) {
Class<?> paramType = param.getNestedParameterType();
if (value == null) {
if (Boolean.TYPE.equals(paramType)) {
return Boolean.FALSE;
}
else if (paramType.isPrimitive()) {
// primitive parameters may only be null if they have a default value
else if (paramType.isPrimitive() && !param.hasDefaultValue()) {
throw new IllegalStateException("Optional " + paramType.getSimpleName() + " parameter '" + name +
"' is present but cannot be translated into a null value due to being declared as a " +
"primitive type. Consider declaring it as object wrapper for the corresponding primitive type.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,15 @@
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;

import kotlin.reflect.KFunction;
import kotlin.reflect.KParameter;
import kotlin.reflect.jvm.ReflectJvmMapping;

import org.springframework.core.DefaultParameterNameDiscoverer;
import org.springframework.core.KotlinDetector;
import org.springframework.core.MethodParameter;
import org.springframework.core.ParameterNameDiscoverer;
import org.springframework.lang.Nullable;
Expand Down Expand Up @@ -55,6 +62,11 @@ public class InvocableHandlerMethod extends HandlerMethod {

private ParameterNameDiscoverer parameterNameDiscoverer = new DefaultParameterNameDiscoverer();

private static final Object KOTLIN_NOT_CHECKED = new Object();

// has type Object to avoid hard dependency on Kotlin, only ever holds KFunction instances
@Nullable
private Object kotlinFunction = KOTLIN_NOT_CHECKED;

/**
* Create an instance from a {@code HandlerMethod}.
Expand Down Expand Up @@ -205,7 +217,13 @@ private Object resolveProvidedArgument(MethodParameter parameter, @Nullable Obje
*/
protected Object doInvoke(Object... args) throws Exception {
ReflectionUtils.makeAccessible(getBridgedMethod());
if (KOTLIN_NOT_CHECKED.equals(kotlinFunction)) {
initKotlinFunction();
}
try {
if (kotlinFunction != null) {
return KotlinDelegate.doKotlinCall(this, kotlinFunction, args);
}
return getBridgedMethod().invoke(getBean(), args);
}
catch (IllegalArgumentException ex) {
Expand All @@ -232,6 +250,15 @@ else if (targetException instanceof Exception) {
}
}

private void initKotlinFunction() {
if (KotlinDetector.isKotlinPresent() && KotlinDetector.isKotlinType(getBeanType())) {
kotlinFunction = KotlinDelegate.initKotlinCache(this);
}
else {
kotlinFunction = null;
}
}

/**
* Assert that the target bean class is an instance of the class where the given
* method is declared. In some cases the actual controller instance at request-
Expand Down Expand Up @@ -279,4 +306,42 @@ protected String getDetailedErrorMessage(String text) {
return sb.toString();
}

private static class KotlinDelegate {

@Nullable
public static Object initKotlinCache(InvocableHandlerMethod method) {
KFunction<?> function = ReflectJvmMapping.getKotlinFunction(method.getBridgedMethod());
// we only need to do the call via Kotlin reflection if we have at least one optional parameter
if (function != null && function.getParameters().stream().anyMatch(KParameter::isOptional)) {
return function;
}
else {
return null;
}
}

public static Object doKotlinCall(InvocableHandlerMethod method, Object kotlinFunction, Object[] args) {
KFunction<?> function = (KFunction<?>) kotlinFunction;
Map<KParameter, Object> argMap = new HashMap<>();
int index = 0;
for (KParameter parameter : function.getParameters()) {
switch (parameter.getKind()) {
case INSTANCE:
argMap.put(parameter, method.getBean());
break;
case VALUE:
if (!parameter.isOptional() || args[index] != null) {
argMap.put(parameter, args[index]);
}
index++;
break;
case EXTENSION_RECEIVER:
default:
throw new UnsupportedOperationException("Unsupported Kotlin function parameter type: " + parameter.getKind());
}
}
return function.callBy(argMap);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class RequestParamMethodArgumentResolverKotlinTests {
lateinit var nonNullableMultipartParamRequired: MethodParameter
lateinit var nonNullableMultipartParamNotRequired: MethodParameter

lateinit var defaultParam: MethodParameter


@Before
fun setup() {
Expand All @@ -75,7 +77,8 @@ class RequestParamMethodArgumentResolverKotlinTests {
val method = ReflectionUtils.findMethod(javaClass, "handle", String::class.java,
String::class.java, String::class.java, String::class.java,
MultipartFile::class.java, MultipartFile::class.java,
MultipartFile::class.java, MultipartFile::class.java)!!
MultipartFile::class.java, MultipartFile::class.java,
String::class.java)!!

nullableParamRequired = SynthesizingMethodParameter(method, 0)
nullableParamNotRequired = SynthesizingMethodParameter(method, 1)
Expand All @@ -86,6 +89,8 @@ class RequestParamMethodArgumentResolverKotlinTests {
nullableMultipartParamNotRequired = SynthesizingMethodParameter(method, 5)
nonNullableMultipartParamRequired = SynthesizingMethodParameter(method, 6)
nonNullableMultipartParamNotRequired = SynthesizingMethodParameter(method, 7)

defaultParam = SynthesizingMethodParameter(method, 8)
}

@Test
Expand Down Expand Up @@ -138,6 +143,18 @@ class RequestParamMethodArgumentResolverKotlinTests {
resolver.resolveArgument(nonNullableParamNotRequired, null, webRequest, binderFactory) as String
}

@Test
fun resolveDefaultValueWithoutParameter() {
val result = resolver.resolveArgument(defaultParam, null, webRequest, binderFactory)
assertNull(result)
}

@Test
fun resolveDefaultValueWithParameter() {
request.addParameter("name", "123")
val result = resolver.resolveArgument(defaultParam, null, webRequest, binderFactory)
assertEquals("123", result)
}

@Test
fun resolveNullableRequiredWithMultipartParameter() {
Expand Down Expand Up @@ -215,7 +232,6 @@ class RequestParamMethodArgumentResolverKotlinTests {
resolver.resolveArgument(nonNullableMultipartParamNotRequired, null, webRequest, binderFactory) as MultipartFile
}


@Suppress("unused_parameter")
fun handle(
@RequestParam("name") nullableParamRequired: String?,
Expand All @@ -226,7 +242,9 @@ class RequestParamMethodArgumentResolverKotlinTests {
@RequestParam("mfile") nullableMultipartParamRequired: MultipartFile?,
@RequestParam("mfile", required = false) nullableMultipartParamNotRequired: MultipartFile?,
@RequestParam("mfile") nonNullableMultipartParamRequired: MultipartFile,
@RequestParam("mfile", required = false) nonNullableMultipartParamNotRequired: MultipartFile) {
@RequestParam("mfile", required = false) nonNullableMultipartParamNotRequired: MultipartFile,

@RequestParam("name") paramDefaultValue: String = "42") {
}

}
Expand Down