Support Kotlin parameter default values in handler methods

Allows using default parameter values for any method parameters resolved
via AbstractNamedValueMethodArgumentResolver, similar to defaultValue
parameter of e.g. @RequestParam annotation, but skipping the converter
chain.

Issues: SPR-16598

Author: diesieben07

spring-core/src/main/java/org/springframework/core/MethodParameter.java

@@ -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
@@ -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).
@@ -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;
+				}
+			}
+		}
 	}
 
 }

spring-core/src/test/kotlin/org/springframework/core/KotlinMethodParameterTests.kt

@@ -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)
 	}
 
 
@@ -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
+
 }

spring-web/src/main/java/org/springframework/web/method/annotation/AbstractNamedValueMethodArgumentResolver.java

@@ -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);
@@ -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.");

spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java

@@ -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;
@@ -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}.
@@ -205,7 +217,13 @@ private Object resolveProvidedArgument(MethodParameter parameter, @Nullable Obje
 	 */
 	protected Object doInvoke(Object... args) throws Exception {
 		ReflectionUtils.makeAccessible(getBridgedMethod());
+		if (kotlinFunction == KOTLIN_NOT_CHECKED) {
+			initKotlinFunction();
+		}
 		try {
+			if (kotlinFunction != null) {
+				return KotlinDelegate.doKotlinCall(this, kotlinFunction, args);
+			}
 			return getBridgedMethod().invoke(getBean(), args);
 		}
 		catch (IllegalArgumentException ex) {
@@ -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-
@@ -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);
+		}
+	}
+
 }

spring-web/src/test/kotlin/org/springframework/web/method/annotation/RequestParamMethodArgumentResolverKotlinTests.kt

@@ -62,6 +62,8 @@ class RequestParamMethodArgumentResolverKotlinTests {
 	lateinit var nonNullableMultipartParamRequired: MethodParameter
 	lateinit var nonNullableMultipartParamNotRequired: MethodParameter
 
+	lateinit var defaultParam: MethodParameter
+
 
 	@Before
 	fun setup() {
@@ -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)
@@ -86,6 +89,8 @@ class RequestParamMethodArgumentResolverKotlinTests {
 		nullableMultipartParamNotRequired = SynthesizingMethodParameter(method, 5)
 		nonNullableMultipartParamRequired = SynthesizingMethodParameter(method, 6)
 		nonNullableMultipartParamNotRequired = SynthesizingMethodParameter(method, 7)
+
+		defaultParam = SynthesizingMethodParameter(method, 8)
 	}
 
 	@Test
@@ -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() {
@@ -215,7 +232,6 @@ class RequestParamMethodArgumentResolverKotlinTests {
 		resolver.resolveArgument(nonNullableMultipartParamNotRequired, null, webRequest, binderFactory) as MultipartFile
 	}
 
-
 	@Suppress("unused_parameter")
 	fun handle(
 			@RequestParam("name") nullableParamRequired: String?,
@@ -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") {
 	}
 
 }