From 466c8d8f23a982cb53f1cfad0143afd28e9c27ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Sun, 15 Oct 2023 19:11:13 +0200 Subject: [PATCH] Add Coroutines support for `@Cacheable` This commit adds Coroutines support for `@Cacheable`. It also refines SimpleKeyGenerator to ignore Continuation parameters (Kotlin does not allow to have the same method signature with both suspending and non-suspending variants) and refines org.springframework.aop.framework.CoroutinesUtils.awaitSingleOrNull in order to wrap plain value to Mono. Closes gh-31412 --- .../aop/framework/CoroutinesUtils.java | 7 +- spring-context/spring-context.gradle | 1 + .../cache/interceptor/CacheAspectSupport.java | 5 + .../cache/interceptor/CacheInterceptor.java | 24 ++- .../cache/interceptor/SimpleKeyGenerator.java | 9 +- .../MethodBasedEvaluationContext.java | 5 +- .../interceptor/SimpleKeyGeneratorTests.java | 9 +- .../cache/KotlinCacheReproTests.kt | 148 ++++++++++++++++++ .../KotlinSimpleKeyGeneratorTests.kt | 71 +++++++++ 9 files changed, 270 insertions(+), 9 deletions(-) create mode 100644 spring-context/src/test/kotlin/org/springframework/cache/KotlinCacheReproTests.kt create mode 100644 spring-context/src/test/kotlin/org/springframework/cache/interceptor/KotlinSimpleKeyGeneratorTests.kt diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/CoroutinesUtils.java b/spring-aop/src/main/java/org/springframework/aop/framework/CoroutinesUtils.java index dea2fe6408..092099d2e6 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/CoroutinesUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/CoroutinesUtils.java @@ -36,10 +36,11 @@ abstract class CoroutinesUtils { return ReactiveFlowKt.asFlow((Publisher) publisher); } - @SuppressWarnings("unchecked") @Nullable - static Object awaitSingleOrNull(Object mono, Object continuation) { - return MonoKt.awaitSingleOrNull((Mono) mono, (Continuation) continuation); + @SuppressWarnings({"unchecked", "rawtypes"}) + static Object awaitSingleOrNull(Object value, Object continuation) { + return MonoKt.awaitSingleOrNull(value instanceof Mono mono ? mono : Mono.just(value), + (Continuation) continuation); } } diff --git a/spring-context/spring-context.gradle b/spring-context/spring-context.gradle index 1efab5bc24..e8299dcc63 100644 --- a/spring-context/spring-context.gradle +++ b/spring-context/spring-context.gradle @@ -30,6 +30,7 @@ dependencies { optional("org.hibernate:hibernate-validator") optional("org.jetbrains.kotlin:kotlin-reflect") optional("org.jetbrains.kotlin:kotlin-stdlib") + optional("org.jetbrains.kotlinx:kotlinx-coroutines-core") optional("org.reactivestreams:reactive-streams") testFixturesApi("org.junit.jupiter:junit-jupiter-api") testFixturesImplementation(testFixtures(project(":spring-beans"))) diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java index 695c53471c..338dc7a1cb 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheAspectSupport.java @@ -48,6 +48,7 @@ import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; import org.springframework.context.expression.AnnotatedElementKey; import org.springframework.core.BridgeMethodResolver; +import org.springframework.core.KotlinDetector; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.expression.EvaluationContext; @@ -85,6 +86,7 @@ import org.springframework.util.function.SupplierUtils; * @author Phillip Webb * @author Sam Brannen * @author Stephane Nicoll + * @author Sebastien Deleuze * @since 3.1 */ public abstract class CacheAspectSupport extends AbstractCacheInvoker @@ -1024,6 +1026,9 @@ public abstract class CacheAspectSupport extends AbstractCacheInvoker () -> Mono.from(adapter.toPublisher(invokeOperation(invoker))).toFuture()))); } } + if (KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isSuspendingFunction(method)) { + return Mono.fromFuture(cache.retrieve(key, () -> ((Mono) invokeOperation(invoker)).toFuture())); + } return NOT_HANDLED; } diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheInterceptor.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheInterceptor.java index 2488fa4c54..50ea0e870a 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheInterceptor.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 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. @@ -19,9 +19,15 @@ package org.springframework.cache.interceptor; import java.io.Serializable; import java.lang.reflect.Method; +import kotlin.coroutines.Continuation; +import kotlin.coroutines.CoroutineContext; +import kotlinx.coroutines.Job; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; +import org.reactivestreams.Publisher; +import org.springframework.core.CoroutinesUtils; +import org.springframework.core.KotlinDetector; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -39,6 +45,7 @@ import org.springframework.util.Assert; * * @author Costin Leau * @author Juergen Hoeller + * @author Sebastien Deleuze * @since 3.1 */ @SuppressWarnings("serial") @@ -51,6 +58,9 @@ public class CacheInterceptor extends CacheAspectSupport implements MethodInterc CacheOperationInvoker aopAllianceInvoker = () -> { try { + if (KotlinDetector.isKotlinReflectPresent() && KotlinDetector.isSuspendingFunction(method)) { + return KotlinDelegate.invokeSuspendingFunction(method, invocation.getThis(), invocation.getArguments()); + } return invocation.proceed(); } catch (Throwable ex) { @@ -68,4 +78,16 @@ public class CacheInterceptor extends CacheAspectSupport implements MethodInterc } } + /** + * Inner class to avoid a hard dependency on Kotlin at runtime. + */ + private static class KotlinDelegate { + + public static Publisher invokeSuspendingFunction(Method method, Object target, Object... args) { + Continuation continuation = (Continuation) args[args.length - 1]; + CoroutineContext coroutineContext = continuation.getContext().minusKey(Job.Key); + return CoroutinesUtils.invokeSuspendingFunction(coroutineContext, method, target, args); + } + } + } diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.java b/spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.java index f443ea91a5..c2365ad9e6 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/SimpleKeyGenerator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2023 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. @@ -17,6 +17,9 @@ package org.springframework.cache.interceptor; import java.lang.reflect.Method; +import java.util.Arrays; + +import org.springframework.core.KotlinDetector; /** * Simple key generator. Returns the parameter itself if a single non-null @@ -30,6 +33,7 @@ import java.lang.reflect.Method; * * @author Phillip Webb * @author Juergen Hoeller + * @author Sebastien Deleuze * @since 4.0 * @see SimpleKey * @see org.springframework.cache.annotation.CachingConfigurer @@ -38,7 +42,8 @@ public class SimpleKeyGenerator implements KeyGenerator { @Override public Object generate(Object target, Method method, Object... params) { - return generateKey(params); + return generateKey((KotlinDetector.isSuspendingFunction(method) ? + Arrays.copyOf(params, params.length - 1) : params)); } /** diff --git a/spring-context/src/main/java/org/springframework/context/expression/MethodBasedEvaluationContext.java b/spring-context/src/main/java/org/springframework/context/expression/MethodBasedEvaluationContext.java index 11d29e30d7..3cc6613e1a 100644 --- a/spring-context/src/main/java/org/springframework/context/expression/MethodBasedEvaluationContext.java +++ b/spring-context/src/main/java/org/springframework/context/expression/MethodBasedEvaluationContext.java @@ -19,6 +19,7 @@ package org.springframework.context.expression; import java.lang.reflect.Method; import java.util.Arrays; +import org.springframework.core.KotlinDetector; import org.springframework.core.ParameterNameDiscoverer; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.lang.Nullable; @@ -37,6 +38,7 @@ import org.springframework.util.ObjectUtils; * * @author Stephane Nicoll * @author Juergen Hoeller + * @author Sebastien Deleuze * @since 4.2 */ public class MethodBasedEvaluationContext extends StandardEvaluationContext { @@ -55,7 +57,8 @@ public class MethodBasedEvaluationContext extends StandardEvaluationContext { super(rootObject); this.method = method; - this.arguments = arguments; + this.arguments = (KotlinDetector.isSuspendingFunction(method) ? + Arrays.copyOf(arguments, arguments.length - 1) : arguments); this.parameterNameDiscoverer = parameterNameDiscoverer; } diff --git a/spring-context/src/test/java/org/springframework/cache/interceptor/SimpleKeyGeneratorTests.java b/spring-context/src/test/java/org/springframework/cache/interceptor/SimpleKeyGeneratorTests.java index 2d9398ccb1..c29ecbcd8a 100644 --- a/spring-context/src/test/java/org/springframework/cache/interceptor/SimpleKeyGeneratorTests.java +++ b/spring-context/src/test/java/org/springframework/cache/interceptor/SimpleKeyGeneratorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 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. @@ -16,9 +16,12 @@ package org.springframework.cache.interceptor; +import java.lang.reflect.Method; + import org.junit.jupiter.api.Test; import org.springframework.core.testfixture.io.SerializationTestUtils; +import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -28,6 +31,7 @@ import static org.assertj.core.api.Assertions.assertThat; * @author Phillip Webb * @author Stephane Nicoll * @author Juergen Hoeller + * @author Sebastien Deleuze */ public class SimpleKeyGeneratorTests { @@ -126,7 +130,8 @@ public class SimpleKeyGeneratorTests { private Object generateKey(Object[] arguments) { - return this.generator.generate(null, null, arguments); + Method method = ReflectionUtils.findMethod(this.getClass(), "generateKey", Object[].class); + return this.generator.generate(this, method, arguments); } } diff --git a/spring-context/src/test/kotlin/org/springframework/cache/KotlinCacheReproTests.kt b/spring-context/src/test/kotlin/org/springframework/cache/KotlinCacheReproTests.kt new file mode 100644 index 0000000000..26e6b1e20a --- /dev/null +++ b/spring-context/src/test/kotlin/org/springframework/cache/KotlinCacheReproTests.kt @@ -0,0 +1,148 @@ +/* + * Copyright 2002-2023 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.cache + +import kotlinx.coroutines.runBlocking +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.beans.testfixture.beans.TestBean +import org.springframework.cache.CacheReproTests.* +import org.springframework.cache.annotation.CacheEvict +import org.springframework.cache.annotation.CachePut +import org.springframework.cache.annotation.Cacheable +import org.springframework.cache.annotation.EnableCaching +import org.springframework.cache.concurrent.ConcurrentMapCacheManager +import org.springframework.context.annotation.AnnotationConfigApplicationContext +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration + +class KotlinCacheReproTests { + + @Test + fun spr14235AdaptsToSuspendingFunction() { + runBlocking { + val context = AnnotationConfigApplicationContext( + Spr14235Config::class.java, + Spr14235SuspendingService::class.java + ) + val bean = context.getBean(Spr14235SuspendingService::class.java) + val cache = context.getBean(CacheManager::class.java).getCache("itemCache")!! + val tb: TestBean = bean.findById("tb1") + assertThat(bean.findById("tb1")).isSameAs(tb) + assertThat(cache["tb1"]!!.get()).isSameAs(tb) + bean.clear() + val tb2: TestBean = bean.findById("tb1") + assertThat(tb2).isNotSameAs(tb) + assertThat(cache["tb1"]!!.get()).isSameAs(tb2) + bean.clear() + bean.insertItem(tb) + assertThat(bean.findById("tb1")).isSameAs(tb) + assertThat(cache["tb1"]!!.get()).isSameAs(tb) + context.close() + } + } + + @Test + fun spr14235AdaptsToSuspendingFunctionWithSync() { + runBlocking { + val context = AnnotationConfigApplicationContext( + Spr14235Config::class.java, + Spr14235SuspendingServiceSync::class.java + ) + val bean = context.getBean(Spr14235SuspendingServiceSync::class.java) + val cache = context.getBean(CacheManager::class.java).getCache("itemCache")!! + val tb = bean.findById("tb1") + assertThat(bean.findById("tb1")).isSameAs(tb) + assertThat(cache["tb1"]!!.get()).isSameAs(tb) + cache.clear() + val tb2 = bean.findById("tb1") + assertThat(tb2).isNotSameAs(tb) + assertThat(cache["tb1"]!!.get()).isSameAs(tb2) + cache.clear() + bean.insertItem(tb) + assertThat(bean.findById("tb1")).isSameAs(tb) + assertThat(cache["tb1"]!!.get()).isSameAs(tb) + context.close() + } + } + + @Test + fun spr15271FindsOnInterfaceWithInterfaceProxy() { + val context = AnnotationConfigApplicationContext(Spr15271ConfigA::class.java) + val bean = context.getBean(Spr15271Interface::class.java) + val cache = context.getBean(CacheManager::class.java).getCache("itemCache")!! + val tb = TestBean("tb1") + bean.insertItem(tb) + assertThat(bean.findById("tb1").get()).isSameAs(tb) + assertThat(cache["tb1"]!!.get()).isSameAs(tb) + context.close() + } + + @Test + fun spr15271FindsOnInterfaceWithCglibProxy() { + val context = AnnotationConfigApplicationContext(Spr15271ConfigB::class.java) + val bean = context.getBean(Spr15271Interface::class.java) + val cache = context.getBean(CacheManager::class.java).getCache("itemCache")!! + val tb = TestBean("tb1") + bean.insertItem(tb) + assertThat(bean.findById("tb1").get()).isSameAs(tb) + assertThat(cache["tb1"]!!.get()).isSameAs(tb) + context.close() + } + + + open class Spr14235SuspendingService { + + @Cacheable(value = ["itemCache"]) + open suspend fun findById(id: String): TestBean { + return TestBean(id) + } + + @CachePut(cacheNames = ["itemCache"], key = "#item.name") + open suspend fun insertItem(item: TestBean): TestBean { + return item + } + + @CacheEvict(cacheNames = ["itemCache"], allEntries = true) + open suspend fun clear() { + } + } + + + open class Spr14235SuspendingServiceSync { + @Cacheable(value = ["itemCache"], sync = true) + open suspend fun findById(id: String): TestBean { + return TestBean(id) + } + + @CachePut(cacheNames = ["itemCache"], key = "#item.name") + open suspend fun insertItem(item: TestBean): TestBean { + return item + } + } + + + @Configuration(proxyBeanMethods = false) + @EnableCaching + class Spr14235Config { + @Bean + fun cacheManager(): CacheManager { + return ConcurrentMapCacheManager() + } + } + +} diff --git a/spring-context/src/test/kotlin/org/springframework/cache/interceptor/KotlinSimpleKeyGeneratorTests.kt b/spring-context/src/test/kotlin/org/springframework/cache/interceptor/KotlinSimpleKeyGeneratorTests.kt new file mode 100644 index 0000000000..c8afdbe75e --- /dev/null +++ b/spring-context/src/test/kotlin/org/springframework/cache/interceptor/KotlinSimpleKeyGeneratorTests.kt @@ -0,0 +1,71 @@ +/* + * Copyright 2002-2023 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.cache.interceptor + +import io.mockk.mockk +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import org.springframework.util.ReflectionUtils +import kotlin.coroutines.Continuation + +/** + * Tests for [SimpleKeyGenerator] and [SimpleKey]. + * + * @author Sebastien Deleuze + */ +class KotlinSimpleKeyGeneratorTests { + + private val generator = SimpleKeyGenerator() + + @Test + fun ignoreContinuationArgumentWithNoParameter() { + val method = ReflectionUtils.findMethod(javaClass, "suspendingMethod", Continuation::class.java)!! + val continuation = mockk>() + val key = generator.generate(this, method, continuation) + assertThat(key).isEqualTo(SimpleKey.EMPTY) + } + + @Test + fun ignoreContinuationArgumentWithOneParameter() { + val method = ReflectionUtils.findMethod(javaClass, "suspendingMethod", String::class.java, Continuation::class.java)!! + val continuation = mockk>() + val key = generator.generate(this, method, "arg", continuation) + assertThat(key).isEqualTo("arg") + } + + @Test + fun ignoreContinuationArgumentWithMultipleParameters() { + val method = ReflectionUtils.findMethod(javaClass, "suspendingMethod", String::class.java, String::class.java, Continuation::class.java)!! + val continuation = mockk>() + val key = generator.generate(this, method, "arg1", "arg2", continuation) + assertThat(key).isEqualTo(SimpleKey("arg1", "arg2")) + } + + + @Suppress("unused", "RedundantSuspendModifier") + suspend fun suspendingMethod() { + } + + @Suppress("unused", "UNUSED_PARAMETER", "RedundantSuspendModifier") + suspend fun suspendingMethod(param: String) { + } + + @Suppress("unused", "UNUSED_PARAMETER", "RedundantSuspendModifier") + suspend fun suspendingMethod(param1: String, param2: String) { + } + +}