Fix Kotlin value class handling for suspending AOP methods

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<Class<?>> 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<Class<?>> 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;
 	}

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;
 		}

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<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")
+		}
+	}
+
 }

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<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")
+		}
+	}
+
+}