mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-10-01 03:39:05 +00:00
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 gh-37191 See gh-37155 Signed-off-by: Dmitry Sulman <dmitry.sulman@gmail.com>
This commit is contained in:
committed by
Sébastien Deleuze
parent
c70c434486
commit
c08fd24de3
@@ -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 @@ class CglibAopProxy implements AopProxy, Serializable {
|
||||
* 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 @@ class CglibAopProxy implements AopProxy, Serializable {
|
||||
"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;
|
||||
}
|
||||
|
||||
@@ -237,8 +237,14 @@ final class JdkDynamicAopProxy implements AopProxy, InvocationHandler, Serializa
|
||||
"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;
|
||||
}
|
||||
|
||||
+130
@@ -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<String> {
|
||||
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")
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+174
@@ -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<String>
|
||||
|
||||
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<String> {
|
||||
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")
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user