From b4e3b7e61551fbc5fea430373376fe0962d9e2f4 Mon Sep 17 00:00:00 2001 From: Dmitry Sulman Date: Sat, 22 Aug 2026 19:11:15 +0300 Subject: [PATCH] Fix value class return handling for suspending AOP methods Unbox Kotlin value class results returned from proxied suspending methods before returning them to the caller. Spring AOP interceptor chains expose return values as Object, which causes Kotlin value class results to be boxed. For suspending functions, the direct return path expects the unboxed value class representation. Update both CglibAopProxy and JdkDynamicAopProxy to detect value class return types and unbox boxed results after coroutine adaptation. Add tests for suspending methods returning value classes. See #37155 Signed-off-by: Dmitry Sulman --- .../aop/framework/CglibAopProxy.java | 15 +- .../aop/framework/JdkDynamicAopProxy.java | 10 +- .../aop/framework/CglibAopProxyKotlinTests.kt | 130 +++++++++++++ .../JdkDynamicAopProxyKotlinTests.kt | 174 ++++++++++++++++++ 4 files changed, 323 insertions(+), 6 deletions(-) create mode 100644 spring-aop/src/test/kotlin/org/springframework/aop/framework/JdkDynamicAopProxyKotlinTests.kt diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java index 1efed81dec82..10edfb439286 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java @@ -17,6 +17,7 @@ package org.springframework.aop.framework; import java.io.Serializable; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.lang.reflect.UndeclaredThrowableException; @@ -421,7 +422,8 @@ private static boolean implementsInterface(Method method, Set> ifcs) { * Also takes care of the conversion from {@code Mono} to Kotlin Coroutines if needed. */ private static @Nullable Object processReturnType( - Object proxy, @Nullable Object target, Method method, Object[] arguments, @Nullable Object returnValue) { + Object proxy, @Nullable Object target, Method method, Object[] arguments, @Nullable Object returnValue) throws + NoSuchMethodException, InvocationTargetException, IllegalAccessException { // Massage return value if necessary if (returnValue != null && returnValue == target && @@ -436,9 +438,14 @@ private static boolean implementsInterface(Method method, Set> ifcs) { "Null return value from advice does not match primitive return type for: " + method); } if (COROUTINES_REACTOR_PRESENT && KotlinDetector.isSuspendingFunction(method)) { - return COROUTINES_FLOW_CLASS_NAME.equals(new MethodParameter(method, -1).getParameterType().getName()) ? - CoroutinesUtils.asFlow(returnValue) : - CoroutinesUtils.awaitSingleOrNull(returnValue, arguments[arguments.length - 1]); + Class returnParameterType = new MethodParameter(method, -1).getParameterType(); + if (COROUTINES_FLOW_CLASS_NAME.equals(returnParameterType.getName())) { + return CoroutinesUtils.asFlow(returnValue); + } + Object awaitResult = CoroutinesUtils.awaitSingleOrNull(returnValue, arguments[arguments.length - 1]); + return KotlinDetector.isInlineClass(returnParameterType) && + awaitResult != null && KotlinDetector.isInlineClass(awaitResult.getClass()) ? + awaitResult.getClass().getDeclaredMethod("unbox-impl").invoke(awaitResult) : awaitResult; } return returnValue; } diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java index b0016b00c039..234843407753 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java @@ -237,8 +237,14 @@ else if (retVal == null && returnType != void.class && returnType.isPrimitive()) "Null return value from advice does not match primitive return type for: " + method); } if (COROUTINES_REACTOR_PRESENT && KotlinDetector.isSuspendingFunction(method)) { - return COROUTINES_FLOW_CLASS_NAME.equals(new MethodParameter(method, -1).getParameterType().getName()) ? - CoroutinesUtils.asFlow(retVal) : CoroutinesUtils.awaitSingleOrNull(retVal, args[args.length - 1]); + Class returnParameterType = new MethodParameter(method, -1).getParameterType(); + if (COROUTINES_FLOW_CLASS_NAME.equals(returnParameterType.getName())) { + return CoroutinesUtils.asFlow(retVal); + } + Object awaitResult = CoroutinesUtils.awaitSingleOrNull(retVal, args[args.length - 1]); + return KotlinDetector.isInlineClass(returnParameterType) && + awaitResult != null && KotlinDetector.isInlineClass(awaitResult.getClass()) ? + awaitResult.getClass().getDeclaredMethod("unbox-impl").invoke(awaitResult) : awaitResult; } return retVal; } diff --git a/spring-aop/src/test/kotlin/org/springframework/aop/framework/CglibAopProxyKotlinTests.kt b/spring-aop/src/test/kotlin/org/springframework/aop/framework/CglibAopProxyKotlinTests.kt index 0489d68bb0c3..55f8a1944ef5 100644 --- a/spring-aop/src/test/kotlin/org/springframework/aop/framework/CglibAopProxyKotlinTests.kt +++ b/spring-aop/src/test/kotlin/org/springframework/aop/framework/CglibAopProxyKotlinTests.kt @@ -16,10 +16,13 @@ package org.springframework.aop.framework +import kotlinx.coroutines.delay +import org.aopalliance.intercept.MethodInterceptor import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.Assertions.assertThatThrownBy import org.junit.jupiter.api.Test import java.time.LocalDateTime +import kotlin.time.Duration.Companion.milliseconds /** * Tests for Kotlin support in [CglibAopProxy]. @@ -56,6 +59,95 @@ class CglibAopProxyKotlinTests { proxyFactory.proxy } + @Test + suspend fun proxiedSuspendedInvocationValueClass() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClass("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnValueClass()).isEqualTo(ValueClass("bar")) + } + + @Test + suspend fun proxiedSuspendedInvocationValueClassProceed() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + it.proceed() + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnValueClass()).isEqualTo(ValueClass("foo")) + } + + @Test + suspend fun proxiedSuspendedInvocationNullableValueClass() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClass("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnNullableValueClass()).isEqualTo(ValueClass("bar")) + } + + @Test + suspend fun proxiedSuspendedInvocationNullableValueClassNull() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + null + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnNullableValueClass()).isNull() + } + + @Test + suspend fun proxiedSuspendedInvocationValueClassNullableValue() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClassNullableValue("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnValueClassNullableValue()).isEqualTo(ValueClassNullableValue("bar")) + } + + @Test + suspend fun proxiedSuspendedInvocationValueClassNullableValueNull() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClassNullableValue(null) + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnValueClassNullableValue()).isEqualTo(ValueClassNullableValue(null)) + } + + @Test + suspend fun proxiedSuspendedInvocationResult() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + Result.success("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnResult().getOrNull()).isEqualTo("bar") + } + + @Test + suspend fun proxiedSuspendedInvocationString() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + "bar" + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnString()).isEqualTo("bar") + } + + @Test + suspend fun proxiedSuspendedInvocationAnyAdviceReturnValueClass() { + val proxyFactory = ProxyFactory(TestBean()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClass("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnAny()).isEqualTo(ValueClass("bar")) + } open class MyKotlinBean { @@ -91,4 +183,42 @@ class CglibAopProxyKotlinTests { val updatedAt: LocalDateTime? = null, ) + @JvmInline + value class ValueClass(val value: String) + + @JvmInline + value class ValueClassNullableValue(val value: String?) + + open class TestBean { + open suspend fun returnValueClass(): ValueClass { + delay(1000.milliseconds) + return ValueClass("foo") + } + + open suspend fun returnNullableValueClass(): ValueClass? { + delay(1000.milliseconds) + return null + } + + open suspend fun returnValueClassNullableValue(): ValueClassNullableValue { + delay(1000.milliseconds) + return ValueClassNullableValue(null) + } + + open suspend fun returnResult(): Result { + delay(1000.milliseconds) + return Result.success("foo") + } + + open suspend fun returnString(): String { + delay(1000.milliseconds) + return "foo" + } + + open suspend fun returnAny(): Any { + delay(1000.milliseconds) + return ValueClass("foo") + } + } + } diff --git a/spring-aop/src/test/kotlin/org/springframework/aop/framework/JdkDynamicAopProxyKotlinTests.kt b/spring-aop/src/test/kotlin/org/springframework/aop/framework/JdkDynamicAopProxyKotlinTests.kt new file mode 100644 index 000000000000..75d4e69eb4db --- /dev/null +++ b/spring-aop/src/test/kotlin/org/springframework/aop/framework/JdkDynamicAopProxyKotlinTests.kt @@ -0,0 +1,174 @@ +/* + * 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.aop.framework + +import kotlinx.coroutines.delay +import org.aopalliance.intercept.MethodInterceptor +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import kotlin.time.Duration.Companion.milliseconds + +/** + * Tests for Kotlin support in [JdkDynamicAopProxy]. + * + * @author Dmitry Sulman + */ +class JdkDynamicAopProxyKotlinTests { + + @Test + suspend fun proxiedSuspendedInvocationValueClass() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClass("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnValueClass()).isEqualTo(ValueClass("bar")) + } + + @Test + suspend fun proxiedSuspendedInvocationValueClassProceed() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + it.proceed() + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnValueClass()).isEqualTo(ValueClass("foo")) + } + + @Test + suspend fun proxiedSuspendedInvocationNullableValueClass() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClass("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnNullableValueClass()).isEqualTo(ValueClass("bar")) + } + + @Test + suspend fun proxiedSuspendedInvocationNullableValueClassNull() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + null + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnNullableValueClass()).isNull() + } + + @Test + suspend fun proxiedSuspendedInvocationValueClassNullableValue() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClassNullableValue("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnValueClassNullableValue()).isEqualTo(ValueClassNullableValue("bar")) + } + + @Test + suspend fun proxiedSuspendedInvocationValueClassNullableValueNull() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClassNullableValue(null) + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnValueClassNullableValue()).isEqualTo(ValueClassNullableValue(null)) + } + + @Test + suspend fun proxiedSuspendedInvocationResult() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + Result.success("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnResult().getOrNull()).isEqualTo("bar") + } + + @Test + suspend fun proxiedSuspendedInvocationString() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + "bar" + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnString()).isEqualTo("bar") + } + + @Test + suspend fun proxiedSuspendedInvocationAnyAdviceReturnValueClass() { + val proxyFactory = ProxyFactory(TestBeanImpl()) + proxyFactory.addAdvice(MethodInterceptor { + ValueClass("bar") + }) + val proxy = proxyFactory.proxy as TestBean + assertThat(proxy.returnAny()).isEqualTo(ValueClass("bar")) + } + + @JvmInline + value class ValueClass(val value: String) + + @JvmInline + value class ValueClassNullableValue(val value: String?) + + interface TestBean { + suspend fun returnValueClass(): ValueClass + + suspend fun returnNullableValueClass(): ValueClass? + + suspend fun returnValueClassNullableValue(): ValueClassNullableValue + + suspend fun returnResult(): Result + + suspend fun returnString(): String + + suspend fun returnAny(): Any + } + + class TestBeanImpl : TestBean { + override suspend fun returnValueClass(): ValueClass { + delay(1000.milliseconds) + return ValueClass("foo") + } + + override suspend fun returnNullableValueClass(): ValueClass? { + delay(1000.milliseconds) + return null + } + + override suspend fun returnValueClassNullableValue(): ValueClassNullableValue { + delay(1000.milliseconds) + return ValueClassNullableValue(null) + } + + override suspend fun returnResult(): Result { + delay(1000.milliseconds) + return Result.success("foo") + } + + override suspend fun returnString(): String { + delay(1000.milliseconds) + return "foo" + } + + override suspend fun returnAny(): Any { + delay(1000.milliseconds) + return ValueClass("foo") + } + } + +} \ No newline at end of file