Unwrap Kotlin inline value classes return values

The result returned by Kotlin reflective invocation of a function returning an inline value class is wrapped, which makes sense from Kotlin POV but from a JVM perspective the associated value and type should be unwrapped to be consistent with what would happen with a reflective invocation done by Java. This commit unwraps such result. Closes spring-projectsgh-33026
sdeleuze · Jul 10, 2024 · 7617a01 · 7617a01
1 parent 82c5aa4
commit 7617a01
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 3 deletions.
diff --git a/spring-core/src/main/java/org/springframework/core/CoroutinesUtils.java b/spring-core/src/main/java/org/springframework/core/CoroutinesUtils.java
@@ -44,6 +44,7 @@
 import org.reactivestreams.Publisher;
 import reactor.core.publisher.Flux;
 import reactor.core.publisher.Mono;
+import reactor.core.publisher.SynchronousSink;
 
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
@@ -145,7 +146,7 @@ public static Publisher<?> invokeSuspendingFunction(
 					}
 					return KCallables.callSuspendBy(function, argMap, continuation);
 				})
-				.filter(result -> result != Unit.INSTANCE)
+				.handle(CoroutinesUtils::handleResult)
 				.onErrorMap(InvocationTargetException.class, InvocationTargetException::getTargetException);
 
 		KType returnType = function.getReturnType();
@@ -165,4 +166,22 @@ private static Flux<?> asFlux(Object flow) {
 		return ReactorFlowKt.asFlux(((Flow<?>) flow));
 	}
 
+	private static void handleResult(Object result, SynchronousSink<Object> sink) {
+		if (result == Unit.INSTANCE) {
+			sink.complete();
+		}
+		else if (KotlinDetector.isInlineClass(result.getClass())) {
+			try {
+				sink.next(result.getClass().getDeclaredMethod("unbox-impl").invoke(result));
+				sink.complete();
+			}
+			catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException ex) {
+				sink.error(ex);
+			}
+		}
+		else {
+			sink.next(result);
+			sink.complete();
+		}
+	}
 }
diff --git a/spring-core/src/test/kotlin/org/springframework/core/CoroutinesUtilsTests.kt b/spring-core/src/test/kotlin/org/springframework/core/CoroutinesUtilsTests.kt
@@ -199,6 +199,15 @@ class CoroutinesUtilsTests {
 		}
 	}
 
+	@Test
+	fun invokeSuspendingFunctionWithValueClassReturnValue() {
+		val method = CoroutinesUtilsTests::class.java.declaredMethods.first { it.name.startsWith("suspendingFunctionWithValueClassReturnValue") }
+		val mono = CoroutinesUtils.invokeSuspendingFunction(method, this, null) as Mono
+		runBlocking {
+			Assertions.assertThat(mono.awaitSingle()).isEqualTo("foo")
+		}
+	}
+
 	@Test
 	fun invokeSuspendingFunctionWithValueClassWithInitParameter() {
 		val method = CoroutinesUtilsTests::class.java.declaredMethods.first { it.name.startsWith("suspendingFunctionWithValueClassWithInit") }
@@ -310,6 +319,11 @@ class CoroutinesUtilsTests {
 		return value.value
 	}
 
+	suspend fun suspendingFunctionWithValueClassReturnValue(): ValueClass {
+		delay(1)
+		return ValueClass("foo")
+	}
+
 	suspend fun suspendingFunctionWithValueClassWithInit(value: ValueClassWithInit): String {
 		delay(1)
 		return value.value

diff --git a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java
@@ -299,7 +299,7 @@ private static class KotlinDelegate {
 
 		@Nullable
 		@SuppressWarnings({"deprecation", "DataFlowIssue"})
-		public static Object invokeFunction(Method method, Object target, Object[] args) throws InvocationTargetException, IllegalAccessException {
+		public static Object invokeFunction(Method method, Object target, Object[] args) throws InvocationTargetException, IllegalAccessException, NoSuchMethodException {
 			KFunction<?> function = ReflectJvmMapping.getKotlinFunction(method);
 			// For property accessors
 			if (function == null) {
@@ -332,6 +332,9 @@ public static Object invokeFunction(Method method, Object target, Object[] args)
 				}
 			}
 			Object result = function.callBy(argMap);
+			if (result != null && KotlinDetector.isInlineClass(result.getClass())) {
+				return result.getClass().getDeclaredMethod("unbox-impl").invoke(result);
+			}
 			return (result == Unit.INSTANCE ? null : result);
 		}
 	}

diff --git a/...c/test/kotlin/org/springframework/web/method/support/InvocableHandlerMethodKotlinTests.kt b/...c/test/kotlin/org/springframework/web/method/support/InvocableHandlerMethodKotlinTests.kt
@@ -104,6 +104,12 @@ class InvocableHandlerMethodKotlinTests {
 		Assertions.assertThat(value).isEqualTo(1L)
 	}
 
+	@Test
+	fun valueClassReturnValue() {
+		val value = getInvocable(ValueClassHandler::valueClassReturnValue.javaMethod!!).invokeForRequest(request, null)
+		Assertions.assertThat(value).isEqualTo("foo")
+	}
+
 	@Test
 	fun valueClassDefaultValue() {
 		composite.addResolver(StubArgumentResolver(Double::class.java))
@@ -200,6 +206,9 @@ class InvocableHandlerMethodKotlinTests {
 
 	private class ValueClassHandler {
 
+		fun valueClassReturnValue() =
+			StringValueClass("foo")
+
 		fun longValueClass(limit: LongValueClass) =
 			limit.value
 
@@ -246,6 +255,9 @@ class InvocableHandlerMethodKotlinTests {
 
 	data class Animal(override val name: String) : Named
 
+	@JvmInline
+	value class StringValueClass(val value: String)
+
 	@JvmInline
 	value class LongValueClass(val value: Long)
 

diff --git a/.../src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java b/.../src/main/java/org/springframework/web/reactive/result/method/InvocableHandlerMethod.java
@@ -325,7 +325,7 @@ private static class KotlinDelegate {
 		@Nullable
 		@SuppressWarnings({"deprecation", "DataFlowIssue"})
 		public static Object invokeFunction(Method method, Object target, Object[] args, boolean isSuspendingFunction,
-				ServerWebExchange exchange) throws InvocationTargetException, IllegalAccessException {
+				ServerWebExchange exchange) throws InvocationTargetException, IllegalAccessException, NoSuchMethodException {
 
 			if (isSuspendingFunction) {
 				Object coroutineContext = exchange.getAttribute(COROUTINE_CONTEXT_ATTRIBUTE);
@@ -369,6 +369,9 @@ public static Object invokeFunction(Method method, Object target, Object[] args,
 					}
 				}
 				Object result = function.callBy(argMap);
+				if (result != null && KotlinDetector.isInlineClass(result.getClass())) {
+					return result.getClass().getDeclaredMethod("unbox-impl").invoke(result);
+				}
 				return (result == Unit.INSTANCE ? null : result);
 			}
 		}

diff --git a/.../test/kotlin/org/springframework/web/reactive/result/InvocableHandlerMethodKotlinTests.kt b/.../test/kotlin/org/springframework/web/reactive/result/InvocableHandlerMethodKotlinTests.kt
@@ -205,6 +205,13 @@ class InvocableHandlerMethodKotlinTests {
 		assertHandlerResultValue(result, "1")
 	}
 
+	@Test
+	fun valueClassReturnValue() {
+		val method = ValueClassController::valueClassReturnValue.javaMethod!!
+		val result = invoke(ValueClassController(), method,)
+		assertHandlerResultValue(result, "foo")
+	}
+
 	@Test
 	fun valueClassWithDefaultValue() {
 		this.resolvers.add(stubResolver(null, Double::class.java))
@@ -376,6 +383,9 @@ class InvocableHandlerMethodKotlinTests {
 		fun valueClass(limit: LongValueClass) =
 			"${limit.value}"
 
+		fun valueClassReturnValue() =
+			StringValueClass("foo")
+
 		fun valueClassWithDefault(limit: DoubleValueClass = DoubleValueClass(3.1)) =
 			"${limit.value}"
 
@@ -420,6 +430,9 @@ class InvocableHandlerMethodKotlinTests {
 
 	data class Animal(override val name: String) : Named
 
+	@JvmInline
+	value class StringValueClass(val value: String)
+
 	@JvmInline
 	value class LongValueClass(val value: Long)