Treat kotlin.Unit as void in web controller

This commit fix a regression introduced by spring-projectsgh-21139
via the usage of Kotlin reflection to invoke HTTP
handler methods. It ensures that kotlin.Unit is treated
as void by returning null.

It also polish CoroutinesUtils to have a consistent
handling with the regular case, and add related
tests to prevent future regressions.

Closes spring-projectsgh-31648

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

@@ -123,7 +123,7 @@ public static Publisher<?> invokeSuspendingFunction(CoroutineContext context, Me
 					}
 					return KCallables.callSuspendBy(function, argMap, continuation);
 				})
-				.filter(result -> !Objects.equals(result, Unit.INSTANCE))
+				.filter(result -> result != Unit.INSTANCE)
 				.onErrorMap(InvocationTargetException.class, InvocationTargetException::getTargetException);
 
 		KClassifier returnType = function.getReturnType().getClassifier();

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

@@ -20,6 +20,7 @@ import kotlinx.coroutines.*
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.reactor.awaitSingle
+import kotlinx.coroutines.reactor.awaitSingleOrNull
 import org.assertj.core.api.Assertions
 import org.junit.jupiter.api.Test
 import reactor.core.publisher.Flux
@@ -126,6 +127,24 @@ class CoroutinesUtilsTests {
 		Assertions.assertThatIllegalArgumentException().isThrownBy { CoroutinesUtils.invokeSuspendingFunction(context, method, this, "foo") }
 	}
 
+	@Test
+	fun invokeSuspendingFunctionReturningUnit() {
+		val method = CoroutinesUtilsTests::class.java.getDeclaredMethod("suspendingUnit", Continuation::class.java)
+		val mono = CoroutinesUtils.invokeSuspendingFunction(method, this) as Mono
+		runBlocking {
+			Assertions.assertThat(mono.awaitSingleOrNull()).isNull()
+		}
+	}
+
+	@Test
+	fun invokeSuspendingFunctionReturningNull() {
+		val method = CoroutinesUtilsTests::class.java.getDeclaredMethod("suspendingNullable", Continuation::class.java)
+		val mono = CoroutinesUtils.invokeSuspendingFunction(method, this) as Mono
+		runBlocking {
+			Assertions.assertThat(mono.awaitSingleOrNull()).isNull()
+		}
+	}
+
 	suspend fun suspendingFunction(value: String): String {
 		delay(1)
 		return value
@@ -146,4 +165,11 @@ class CoroutinesUtilsTests {
 		return value
 	}
 
+	suspend fun suspendingUnit() {
+	}
+
+	suspend fun suspendingNullable(): String? {
+		return null
+	}
+
 }

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

@@ -22,6 +22,7 @@
 import java.util.Map;
 import java.util.Objects;
 
+import kotlin.Unit;
 import kotlin.reflect.KFunction;
 import kotlin.reflect.KParameter;
 import kotlin.reflect.jvm.KCallablesJvm;
@@ -315,7 +316,8 @@ public static Object invokeFunction(Method method, Object target, Object[] args)
 					}
 				}
 			}
-			return function.callBy(argMap);
+			Object result = function.callBy(argMap);
+			return (result == Unit.INSTANCE ? null : result);
 		}
 	}
 

spring-web/src/test/kotlin/org/springframework/web/method/support/InvocableHandlerMethodKotlinTests.kt

@@ -71,6 +71,19 @@ class InvocableHandlerMethodKotlinTests {
 		Assertions.assertThat(value).isEqualTo("true")
 	}
 
+	@Test
+	fun unitReturnValue() {
+		val value = getInvocable().invokeForRequest(request, null)
+		Assertions.assertThat(value).isNull()
+	}
+
+	@Test
+	fun nullReturnValue() {
+		composite.addResolver(StubArgumentResolver(String::class.java, null))
+		val value = getInvocable(String::class.java).invokeForRequest(request, null)
+		Assertions.assertThat(value).isNull()
+	}
+
 	private fun getInvocable(vararg argTypes: Class<*>): InvocableHandlerMethod {
 		val method = ResolvableMethod.on(Handler::class.java).argTypes(*argTypes).resolveMethod()
 		val handlerMethod = InvocableHandlerMethod(Handler(), method)
@@ -95,6 +108,14 @@ class InvocableHandlerMethodKotlinTests {
 
 		fun nullableBooleanDefaultValue(status: Boolean? = true) =
 			status.toString()
+
+		fun unit(): Unit {
+		}
+
+		@Suppress("UNUSED_PARAMETER")
+		fun nullable(arg: String?): String? {
+			return null
+		}
 	}
 
 }

spring-webflux/src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java

@@ -26,6 +26,7 @@
 import java.util.Objects;
 import java.util.stream.Stream;
 
+import kotlin.Unit;
 import kotlin.coroutines.CoroutineContext;
 import kotlin.reflect.KFunction;
 import kotlin.reflect.KParameter;
@@ -326,7 +327,8 @@ public static Object invokeFunction(Method method, Object target, Object[] args,
 						}
 					}
 				}
-				return function.callBy(argMap);
+				Object result = function.callBy(argMap);
+				return (result == Unit.INSTANCE ? null : result);
 			}
 		}
 	}

spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/InvocableHandlerMethodKotlinTests.kt

@@ -164,6 +164,20 @@ class InvocableHandlerMethodKotlinTests {
 		assertHandlerResultValue(result, "override")
 	}
 
+	@Test
+	fun unitReturnValue() {
+		val method = NullResultController::unit.javaMethod!!
+		val result = invoke(NullResultController(), method)
+		assertHandlerResultValue(result, null)
+	}
+
+	@Test
+	fun nullReturnValue() {
+		val method = NullResultController::nullable.javaMethod!!
+		val result = invoke(NullResultController(), method)
+		assertHandlerResultValue(result, null)
+	}
+
 
 	private fun invokeForResult(handler: Any, method: Method, vararg providedArgs: Any): HandlerResult? {
 		return invoke(handler, method, *providedArgs).block(Duration.ofSeconds(5))
@@ -186,7 +200,7 @@ class InvocableHandlerMethodKotlinTests {
 		return resolver
 	}
 
-	private fun assertHandlerResultValue(mono: Mono<HandlerResult>, expected: String) {
+	private fun assertHandlerResultValue(mono: Mono<HandlerResult>, expected: String?) {
 		StepVerifier.create(mono)
 				.consumeNextWith {
 					if (it.returnValue is Mono<*>) {
@@ -242,4 +256,14 @@ class InvocableHandlerMethodKotlinTests {
 		@Suppress("RedundantSuspendModifier")
 		suspend fun handleSuspending(@RequestParam value: String = "default") = value
 	}
+
+	class NullResultController {
+
+		fun unit() {
+		}
+
+		fun nullable(): String? {
+			return null
+		}
+	}
 }