Improve value class handling after #36449

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

@@ -134,9 +134,10 @@ public static Publisher<?> invokeSuspendingFunction(
 								Object arg = args[index];
 								if (!(parameter.isOptional() && arg == null)) {
 									KType type = parameter.getType();
-									if (!type.isMarkedNullable() &&
+									if (!(type.isMarkedNullable() && arg == null) &&
 											type.getClassifier() instanceof KClass<?> kClass &&
-											KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(kClass))) {
+											KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(kClass)) &&
+											!JvmClassMappingKt.getJavaClass(kClass).isInstance(arg)) {
 										arg = box(kClass, arg);
 									}
 									argMap.put(parameter, arg);
@@ -166,9 +167,10 @@ public static Publisher<?> invokeSuspendingFunction(
 	private static Object box(KClass<?> kClass, @Nullable Object arg) {
 		KFunction<?> constructor = Objects.requireNonNull(KClasses.getPrimaryConstructor(kClass));
 		KType type = constructor.getParameters().get(0).getType();
-		if (!type.isMarkedNullable() &&
+		if (!(type.isMarkedNullable() && arg == null) &&
 				type.getClassifier() instanceof KClass<?> parameterClass &&
-				KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(parameterClass))) {
+				KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(parameterClass)) &&
+				!JvmClassMappingKt.getJavaClass(parameterClass).isInstance(arg)) {
 			arg = box(parameterClass, arg);
 		}
 		if (!KCallablesJvm.isAccessible(constructor)) {

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

@@ -236,6 +236,13 @@ class CoroutinesUtilsTests {
 		Assertions.assertThat(mono.awaitSingleOrNull()).isEqualTo("foo")
 	}
 
+	@Test
+	suspend fun invokeSuspendingFunctionWithNullableValueClassParameterAndUnderlyingValue() {
+		val method = CoroutinesUtilsTests::class.java.declaredMethods.first { it.name.startsWith("suspendingFunctionWithNullableValueClass") }
+		val mono = CoroutinesUtils.invokeSuspendingFunction(method, this, "foo", null) as Mono
+		Assertions.assertThat(mono.awaitSingleOrNull()).isEqualTo("foo")
+	}
+
 	@Test
 	suspend fun invokeSuspendingFunctionWithNullableValueClassParameter() {
 		val method = CoroutinesUtilsTests::class.java.declaredMethods.first { it.name.startsWith("suspendingFunctionWithNullableValueClass") }

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

@@ -316,9 +316,10 @@ private static class KotlinDelegate {
 						Object arg = args[index];
 						if (!(parameter.isOptional() && arg == null)) {
 							KType type = parameter.getType();
-							if (!type.isMarkedNullable() &&
+							if (!(type.isMarkedNullable() && arg == null) &&
 									type.getClassifier() instanceof KClass<?> kClass &&
-									KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(kClass))) {
+									KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(kClass)) &&
+									!JvmClassMappingKt.getJavaClass(kClass).isInstance(arg)) {
 								arg = box(kClass, arg);
 							}
 							argMap.put(parameter, arg);
@@ -337,9 +338,10 @@ private static class KotlinDelegate {
 		private static Object box(KClass<?> kClass, @Nullable Object arg) {
 			KFunction<?> constructor = Objects.requireNonNull(KClasses.getPrimaryConstructor(kClass));
 			KType type = constructor.getParameters().get(0).getType();
-			if (!type.isMarkedNullable() &&
+			if (!(type.isMarkedNullable() && arg == null) &&
 					type.getClassifier() instanceof KClass<?> parameterClass &&
-					KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(parameterClass))) {
+					KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(parameterClass)) &&
+					!JvmClassMappingKt.getJavaClass(parameterClass).isInstance(arg)) {
 				arg = box(parameterClass, arg);
 			}
 			if (!KCallablesJvm.isAccessible(constructor)) {

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

@@ -155,6 +155,13 @@ class InvocableHandlerMethodKotlinTests {
 		Assertions.assertThat(value).isEqualTo(1L)
 	}
 
+	@Test
+	fun valueClassWithNullableAndUnderlyingValue() {
+		composite.addResolver(StubArgumentResolver(LongValueClass::class.java, 1L))
+		val value = getInvocable(ValueClassHandler::valueClassWithNullable.javaMethod!!).invokeForRequest(request, null)
+		Assertions.assertThat(value).isEqualTo(1L)
+	}
+
 	@Test
 	fun valueClassWithNullable() {
 		composite.addResolver(StubArgumentResolver(LongValueClass::class.java, null))
@@ -215,6 +222,14 @@ class InvocableHandlerMethodKotlinTests {
 		StepVerifier.create(value as Mono<Long>).verifyComplete()
 	}
 
+	@Test
+	fun suspendingValueClassWithNullableAndUnderlyingValue() {
+		composite.addResolver(ContinuationHandlerMethodArgumentResolver())
+		composite.addResolver(StubArgumentResolver(LongValueClass::class.java, 1L))
+		val value = getInvocable(SuspendingValueClassHandler::valueClassWithNullable.javaMethod!!).invokeForRequest(request, null)
+		StepVerifier.create(value as Mono<Long>).expectNext(1L).verifyComplete()
+	}
+
 	@Test
 	fun suspendingValueClassWithPrivateConstructor() {
 		composite.addResolver(ContinuationHandlerMethodArgumentResolver())

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

@@ -356,9 +356,10 @@ private static class KotlinDelegate {
 							Object arg = args[index];
 							if (!(parameter.isOptional() && arg == null)) {
 								KType type = parameter.getType();
-								if (!type.isMarkedNullable() &&
+								if (!(type.isMarkedNullable() && arg == null) &&
 										type.getClassifier() instanceof KClass<?> kClass &&
-										KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(kClass))) {
+										KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(kClass)) &&
+										!JvmClassMappingKt.getJavaClass(kClass).isInstance(arg)) {
 									arg = box(kClass, arg);
 								}
 								argMap.put(parameter, arg);
@@ -378,9 +379,10 @@ private static class KotlinDelegate {
 		private static Object box(KClass<?> kClass, @Nullable Object arg) {
 			KFunction<?> constructor = Objects.requireNonNull(KClasses.getPrimaryConstructor(kClass));
 			KType type = constructor.getParameters().get(0).getType();
-			if (!type.isMarkedNullable() &&
+			if (!(type.isMarkedNullable() && arg == null) &&
 					type.getClassifier() instanceof KClass<?> parameterClass &&
-					KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(parameterClass))) {
+					KotlinDetector.isInlineClass(JvmClassMappingKt.getJavaClass(parameterClass)) &&
+					!JvmClassMappingKt.getJavaClass(parameterClass).isInstance(arg)) {
 				arg = box(parameterClass, arg);
 			}
 			if (!KCallablesJvm.isAccessible(constructor)) {

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

@@ -258,6 +258,14 @@ class InvocableHandlerMethodKotlinTests {
 		assertHandlerResultValue(result, "1")
 	}
 
+	@Test
+	fun valueClassWithNullableAndUnderlyingValue() {
+		this.resolvers.add(stubResolver(1L, LongValueClass::class.java))
+		val method = ValueClassController::valueClassWithNullable.javaMethod!!
+		val result = invoke(ValueClassController(), method)
+		assertHandlerResultValue(result, "1")
+	}
+
 	@Test
 	fun valueClassWithNullable() {
 		this.resolvers.add(stubResolver(null, LongValueClass::class.java))
@@ -320,6 +328,14 @@ class InvocableHandlerMethodKotlinTests {
 		assertHandlerResultValue(result, "null")
 	}
 
+	@Test
+	fun suspendingValueClassWithNullableAndUnderlyingValue() {
+		this.resolvers.add(stubResolver(1L, LongValueClass::class.java))
+		val method = SuspendingValueClassController::valueClassWithNullable.javaMethod!!
+		val result = invoke(SuspendingValueClassController(), method)
+		assertHandlerResultValue(result, "1")
+	}
+
 	@Test
 	fun suspendingValueClassWithPrivateConstructor() {
 		this.resolvers.add(stubResolver(1L, Long::class.java))
@@ -590,4 +606,4 @@ class InvocableHandlerMethodKotlinTests {
 	}
 
 	class CustomException(message: String) : Throwable(message)
-}
+}

spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/CoroutinesIntegrationTests.kt

@@ -131,7 +131,6 @@ class CoroutinesIntegrationTests : AbstractRequestMappingIntegrationTests() {
 		assertThat(entity.body).isEqualTo("foobar")
 	}
 
-
 	@Configuration
 	@EnableWebFlux
 	@ComponentScan(resourcePattern = "**/CoroutinesIntegrationTests*")
@@ -207,7 +206,6 @@ class CoroutinesIntegrationTests : AbstractRequestMappingIntegrationTests() {
 						}
 			return ResponseEntity.ok().body(strings)
 		}
-
 	}
 
 

spring-webflux/src/test/kotlin/org/springframework/web/reactive/result/method/annotation/CoroutinesValueClassIntegrationTest.kt

@@ -0,0 +1,125 @@
+/*
+ * Copyright 2002-present 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.web.reactive.result.method.annotation
+
+import kotlinx.coroutines.delay
+import org.assertj.core.api.Assertions.assertThat
+import org.springframework.context.ApplicationContext
+import org.springframework.context.annotation.AnnotationConfigApplicationContext
+import org.springframework.context.annotation.ComponentScan
+import org.springframework.context.annotation.Configuration
+import org.springframework.http.HttpHeaders
+import org.springframework.http.HttpStatus
+import org.springframework.web.bind.annotation.GetMapping
+import org.springframework.web.bind.annotation.RequestParam
+import org.springframework.web.bind.annotation.RestController
+import org.springframework.web.reactive.config.EnableWebFlux
+import org.springframework.web.testfixture.http.server.reactive.bootstrap.HttpServer
+import java.util.UUID
+
+class CoroutinesValueClassIntegrationTest : AbstractRequestMappingIntegrationTests() {
+
+	override fun initApplicationContext(): ApplicationContext {
+		val context = AnnotationConfigApplicationContext()
+		context.register(WebConfig::class.java)
+		context.refresh()
+		return context
+	}
+
+
+	@ParameterizedHttpServerTest
+	fun `Suspending handler method with nullable value class request param`(httpServer: HttpServer) {
+		startServer(httpServer)
+
+		val entity = performGet("/suspend-value-class?value=550e8400-e29b-41d4-a716-446655440000", HttpHeaders.EMPTY, String::class.java)
+		assertThat(entity.statusCode).isEqualTo(HttpStatus.OK)
+		assertThat(entity.body).isEqualTo("550e8400-e29b-41d4-a716-446655440000")
+	}
+
+	@ParameterizedHttpServerTest
+	fun `Suspending handler method with nullable value class request param omitted`(httpServer: HttpServer) {
+		startServer(httpServer)
+
+		val entity = performGet("/suspend-value-class", HttpHeaders.EMPTY, String::class.java)
+		assertThat(entity.statusCode).isEqualTo(HttpStatus.OK)
+		assertThat(entity.body).isEqualTo("outer-null")
+	}
+
+	@ParameterizedHttpServerTest
+	fun `Suspending handler method with non-optional nullable inner value class request param`(httpServer: HttpServer) {
+		startServer(httpServer)
+
+		val entity = performGet("/suspend-nullable-inner-value-class", HttpHeaders.EMPTY, String::class.java)
+		assertThat(entity.statusCode).isEqualTo(HttpStatus.OK)
+		assertThat(entity.body).isEqualTo("inner-null")
+	}
+
+	@ParameterizedHttpServerTest
+	fun `Suspending handler method with optional nullable inner value class request param`(httpServer: HttpServer) {
+		startServer(httpServer)
+
+		val entity = performGet("/suspend-nullable-inner-value-class-optional", HttpHeaders.EMPTY, String::class.java)
+		assertThat(entity.statusCode).isEqualTo(HttpStatus.OK)
+		assertThat(entity.body).isEqualTo("outer-null")
+	}
+
+
+	@Configuration
+	@EnableWebFlux
+	@ComponentScan(resourcePattern = "**/CoroutinesValueClassIntegrationTest*")
+	open class WebConfig
+
+	@RestController
+	class CoroutinesController {
+
+		@GetMapping("/suspend-value-class")
+		suspend fun suspendingValueClassEndpoint(@RequestParam value: ValueClass?): String {
+			delay(1)
+			return when (value) {
+				null -> "outer-null"
+				else -> value.value.toString()
+			}
+		}
+
+		@GetMapping("/suspend-nullable-inner-value-class")
+		suspend fun suspendingNullableInnerValueClassEndpoint(
+			@RequestParam(required = false) value: NullableInnerValueClass
+		): String {
+			delay(1)
+			return if (value.value == null) "inner-null" else value.value.toString()
+		}
+
+		@GetMapping("/suspend-nullable-inner-value-class-optional")
+		suspend fun suspendingOptionalNullableInnerValueClassEndpoint(
+			@RequestParam(required = false) value: NullableInnerValueClass?
+		): String {
+			delay(1)
+			return when {
+				value == null -> "outer-null"
+				value.value == null -> "inner-null"
+				else -> value.value.toString()
+			}
+		}
+	}
+
+	@JvmInline
+	value class ValueClass(val value: UUID)
+
+	@JvmInline
+	value class NullableInnerValueClass(val value: UUID?)
+
+}