From 3ed2fe3468f6c491c064470054ab89b4e8fc94e4 Mon Sep 17 00:00:00 2001 From: Subin Kim Date: Sat, 6 Sep 2025 02:02:04 +0900 Subject: [PATCH] Fix: findByIdOrNull ignores AOP annotations on overridden methods. The findByIdOrNull Kotlin extension was not declared as an inline function. This caused the call to bind to the generic CrudRepository.findById(Object) method at compile time, bypassing any AOP proxies on user-overridden repository methods with specific types like findById(String). By declaring the extension as inline, the function body is inlined at the call site. This ensures that the compiler resolves the call to the most specific, user-defined findById method, allowing AOP proxies for features like caching or entity graphs to be correctly applied. Signed-off-by: Subin Kim --- .../repository/CrudRepositoryExtensions.kt | 3 ++- .../CrudRepositoryExtensionsTests.kt | 26 ++++++++++++++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/main/kotlin/org/springframework/data/repository/CrudRepositoryExtensions.kt b/src/main/kotlin/org/springframework/data/repository/CrudRepositoryExtensions.kt index f6eb459740..7835560ec9 100644 --- a/src/main/kotlin/org/springframework/data/repository/CrudRepositoryExtensions.kt +++ b/src/main/kotlin/org/springframework/data/repository/CrudRepositoryExtensions.kt @@ -22,6 +22,7 @@ package org.springframework.data.repository * @return the entity with the given id or `null` if none found. * @author Sebastien Deleuze * @author Oscar Hernandez + * @author Subin Kim * @since 2.1.4 */ -fun CrudRepository.findByIdOrNull(id: ID): T? = findById(id).orElse(null) +inline fun CrudRepository.findByIdOrNull(id: ID): T? = findById(id).orElse(null) diff --git a/src/test/kotlin/org/springframework/data/repository/CrudRepositoryExtensionsTests.kt b/src/test/kotlin/org/springframework/data/repository/CrudRepositoryExtensionsTests.kt index bcf68f92a1..ecf1f49445 100644 --- a/src/test/kotlin/org/springframework/data/repository/CrudRepositoryExtensionsTests.kt +++ b/src/test/kotlin/org/springframework/data/repository/CrudRepositoryExtensionsTests.kt @@ -18,8 +18,9 @@ package org.springframework.data.repository import io.mockk.every import io.mockk.mockk import io.mockk.verify -import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.* import org.junit.jupiter.api.Test +import org.springframework.aop.framework.ProxyFactory import org.springframework.data.repository.sample.User import java.util.* @@ -28,9 +29,14 @@ import java.util.* * * @author Sebastien Deleuze * @author Mark Paluch + * @author Subin Kim */ class CrudRepositoryExtensionsTests { + private interface UserRepository : CrudRepository { + override fun findById(id: String): Optional + } + var repository = mockk>() @Test // DATACMNS-1346 @@ -44,4 +50,22 @@ class CrudRepositoryExtensionsTests { assertThat(repository.findByIdOrNull("foo")).isNull() verify(exactly = 2) { repository.findById("foo") } } + + @Test // GH-3326 + fun `findByIdOrNull should trigger AOP proxy on overridden method`() { + + val mockTarget = mockk() + val user = User() + every { mockTarget.findById("1") } returns Optional.of(user) + + val factory = ProxyFactory() + factory.setTarget(mockTarget) + factory.addInterface(UserRepository::class.java) + val proxy = factory.proxy as UserRepository + + val result = proxy.findByIdOrNull("1") + assertThat(result).isEqualTo(user) + + verify(exactly = 1) { mockTarget.findById("1") } + } }