Can be used as part of a {@link Pointcut} or for the entire targeting of * an {@link IntroductionAdvisor}. * - *
Concrete implementations of this interface typically should provide proper - * implementations of {@link Object#equals(Object)} and {@link Object#hashCode()} - * in order to allow the filter to be used in caching scenarios — for - * example, in proxies generated by CGLIB. + *
WARNING: Concrete implementations of this interface must + * provide proper implementations of {@link Object#equals(Object)}, + * {@link Object#hashCode()}, and {@link Object#toString()} in order to allow the + * filter to be used in caching scenarios — for example, in proxies generated + * by CGLIB. As of Spring Framework 6.0.13, the {@code toString()} implementation + * must generate a unique string representation that aligns with the logic used + * to implement {@code equals()}. See concrete implementations of this interface + * within the framework for examples. * * @author Rod Johnson + * @author Sam Brannen * @see Pointcut * @see MethodMatcher */ diff --git a/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java b/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java index 71f19e97dded..9e04831a0150 100644 --- a/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java +++ b/spring-aop/src/main/java/org/springframework/aop/MethodMatcher.java @@ -41,12 +41,17 @@ * state changes they have produced in parameters or {@code ThreadLocal} state will * be available at the time of evaluation. * - *
Concrete implementations of this interface typically should provide proper - * implementations of {@link Object#equals(Object)} and {@link Object#hashCode()} - * in order to allow the matcher to be used in caching scenarios — for - * example, in proxies generated by CGLIB. + *
WARNING: Concrete implementations of this interface must + * provide proper implementations of {@link Object#equals(Object)}, + * {@link Object#hashCode()}, and {@link Object#toString()} in order to allow the + * matcher to be used in caching scenarios — for example, in proxies generated + * by CGLIB. As of Spring Framework 6.0.13, the {@code toString()} implementation + * must generate a unique string representation that aligns with the logic used + * to implement {@code equals()}. See concrete implementations of this interface + * within the framework for examples. * * @author Rod Johnson + * @author Sam Brannen * @since 11.11.2003 * @see Pointcut * @see ClassFilter diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/InstantiationModelAwarePointcutAdvisorImpl.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/InstantiationModelAwarePointcutAdvisorImpl.java index 74a2e1b3c435..f4d150d64d4d 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/InstantiationModelAwarePointcutAdvisorImpl.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/InstantiationModelAwarePointcutAdvisorImpl.java @@ -32,6 +32,7 @@ import org.springframework.aop.support.DynamicMethodMatcherPointcut; import org.springframework.aop.support.Pointcuts; import org.springframework.lang.Nullable; +import org.springframework.util.ObjectUtils; /** * Internal implementation of AspectJPointcutAdvisor. @@ -40,6 +41,7 @@ * * @author Rod Johnson * @author Juergen Hoeller + * @author Sam Brannen * @since 2.0 */ @SuppressWarnings("serial") @@ -297,6 +299,23 @@ public boolean matches(Method method, Class targetClass, Object... args) { private boolean isAspectMaterialized() { return (this.aspectInstanceFactory == null || this.aspectInstanceFactory.isMaterialized()); } + + @Override + public boolean equals(@Nullable Object other) { + return (this == other || (other instanceof PerTargetInstantiationModelPointcut that && + ObjectUtils.nullSafeEquals(this.declaredPointcut.getExpression(), that.declaredPointcut.getExpression()))); + } + + @Override + public int hashCode() { + return ObjectUtils.nullSafeHashCode(this.declaredPointcut.getExpression()); + } + + @Override + public String toString() { + return PerTargetInstantiationModelPointcut.class.getName() + ": " + this.declaredPointcut.getExpression(); + } + } } diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java b/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java index 6020cbbd99c7..41c5b1d21c1d 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/AdvisedSupport.java @@ -62,6 +62,7 @@ * * @author Rod Johnson * @author Juergen Hoeller + * @author Sam Brannen * @see org.springframework.aop.framework.AopProxy */ public class AdvisedSupport extends ProxyConfig implements Advised { @@ -653,8 +654,8 @@ public AdvisorKeyEntry(Advisor advisor) { this.adviceType = advisor.getAdvice().getClass(); if (advisor instanceof PointcutAdvisor pointcutAdvisor) { Pointcut pointcut = pointcutAdvisor.getPointcut(); - this.classFilterKey = ObjectUtils.identityToString(pointcut.getClassFilter()); - this.methodMatcherKey = ObjectUtils.identityToString(pointcut.getMethodMatcher()); + this.classFilterKey = pointcut.getClassFilter().toString(); + this.methodMatcherKey = pointcut.getMethodMatcher().toString(); } else { this.classFilterKey = null; diff --git a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java index e7988d5e12ca..e70275aeaed7 100644 --- a/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java +++ b/spring-context/src/main/java/org/springframework/cache/interceptor/CacheOperationSourcePointcut.java @@ -32,6 +32,7 @@ * * @author Costin Leau * @author Juergen Hoeller + * @author Sam Brannen * @since 3.1 */ @SuppressWarnings("serial") @@ -86,6 +87,27 @@ public boolean matches(Class clazz) { } return (cacheOperationSource == null || cacheOperationSource.isCandidateClass(clazz)); } + + private CacheOperationSource getCacheOperationSource() { + return cacheOperationSource; + } + + @Override + public boolean equals(@Nullable Object other) { + return (this == other || (other instanceof CacheOperationSourceClassFilter that && + ObjectUtils.nullSafeEquals(cacheOperationSource, that.getCacheOperationSource()))); + } + + @Override + public int hashCode() { + return CacheOperationSourceClassFilter.class.hashCode(); + } + + @Override + public String toString() { + return CacheOperationSourceClassFilter.class.getName() + ": " + cacheOperationSource; + } + } } diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java index 8a7e0b50116f..69f5f4bd888e 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java @@ -53,12 +53,14 @@ import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.testfixture.beans.ITestBean; import org.springframework.beans.testfixture.beans.TestBean; +import org.springframework.cglib.proxy.Factory; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.context.annotation.Scope; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.DecoratingProxy; @@ -208,6 +210,31 @@ void perTargetAspect() throws SecurityException, NoSuchMethodException { assertThat(adrian1.getAge()).isEqualTo(3); } + @Test // gh-31238 + void cglibProxyClassIsCachedAcrossApplicationContextsForPerTargetAspect() { + Class configClass = PerTargetProxyTargetClassTrueConfig.class; + TestBean testBean1; + TestBean testBean2; + + // Round #1 + try (ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(configClass)) { + testBean1 = context.getBean(TestBean.class); + assertThat(AopUtils.isCglibProxy(testBean1)).as("CGLIB proxy").isTrue(); + assertThat(testBean1.getClass().getInterfaces()) + .containsExactlyInAnyOrder(Factory.class, SpringProxy.class, Advised.class); + } + + // Round #2 + try (ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(configClass)) { + testBean2 = context.getBean(TestBean.class); + assertThat(AopUtils.isCglibProxy(testBean2)).as("CGLIB proxy").isTrue(); + assertThat(testBean2.getClass().getInterfaces()) + .containsExactlyInAnyOrder(Factory.class, SpringProxy.class, Advised.class); + } + + assertThat(testBean1.getClass()).isSameAs(testBean2.getClass()); + } + @Test void twoAdviceAspect() { ClassPathXmlApplicationContext bf = newContext("twoAdviceAspect.xml"); @@ -615,6 +642,23 @@ class ProxyTargetClassFalseConfig extends AbstractProxyTargetClassConfig { class ProxyTargetClassTrueConfig extends AbstractProxyTargetClassConfig { } +@Configuration +@EnableAspectJAutoProxy(proxyTargetClass = true) +class PerTargetProxyTargetClassTrueConfig { + + @Bean + @Scope("prototype") + TestBean testBean() { + return new TestBean("Jane", 34); + } + + @Bean + @Scope("prototype") + PerTargetAspect perTargetAspect() { + return new PerTargetAspect(); + } +} + @FunctionalInterface interface MessageGenerator { String generateMessage(); diff --git a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingIntegrationTests.java b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingIntegrationTests.java index 9d371d23a187..f4e25b70caf8 100644 --- a/spring-context/src/test/java/org/springframework/cache/config/EnableCachingIntegrationTests.java +++ b/spring-context/src/test/java/org/springframework/cache/config/EnableCachingIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 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. @@ -21,6 +21,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.cache.Cache; import org.springframework.cache.CacheManager; @@ -45,6 +46,7 @@ * Tests that represent real use cases with advanced configuration. * * @author Stephane Nicoll + * @author Sam Brannen */ class EnableCachingIntegrationTests { @@ -83,6 +85,25 @@ private void fooGetSimple(FooService service) { assertCacheHit(key, value, cache); } + @Test // gh-31238 + public void cglibProxyClassIsCachedAcrossApplicationContexts() { + ConfigurableApplicationContext ctx; + + // Round #1 + ctx = new AnnotationConfigApplicationContext(FooConfigCglib.class); + FooService service1 = ctx.getBean(FooService.class); + assertThat(AopUtils.isCglibProxy(service1)).as("FooService #1 is not a CGLIB proxy").isTrue(); + ctx.close(); + + // Round #2 + ctx = new AnnotationConfigApplicationContext(FooConfigCglib.class); + FooService service2 = ctx.getBean(FooService.class); + assertThat(AopUtils.isCglibProxy(service2)).as("FooService #2 is not a CGLIB proxy").isTrue(); + ctx.close(); + + assertThat(service1.getClass()).isSameAs(service2.getClass()); + } + @Test void barServiceWithCacheableInterfaceCglib() { this.context = new AnnotationConfigApplicationContext(BarConfigCglib.class); diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java index 9917ab7b2ed9..10ac08147ae3 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java +++ b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAttributeSourcePointcut.java @@ -27,14 +27,15 @@ import org.springframework.util.ObjectUtils; /** - * Abstract class that implements a {@code Pointcut} that matches if the underlying + * Internal class that implements a {@code Pointcut} that matches if the underlying * {@link TransactionAttributeSource} has an attribute for a given method. * * @author Juergen Hoeller + * @author Sam Brannen * @since 2.5.5 */ @SuppressWarnings("serial") -class TransactionAttributeSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { +final class TransactionAttributeSourcePointcut extends StaticMethodMatcherPointcut implements Serializable { @Nullable private TransactionAttributeSource transactionAttributeSource; @@ -87,6 +88,27 @@ public boolean matches(Class clazz) { } return (transactionAttributeSource == null || transactionAttributeSource.isCandidateClass(clazz)); } + + private TransactionAttributeSource getTransactionAttributeSource() { + return transactionAttributeSource; + } + + @Override + public boolean equals(@Nullable Object other) { + return (this == other || (other instanceof TransactionAttributeSourceClassFilter that && + ObjectUtils.nullSafeEquals(transactionAttributeSource, that.getTransactionAttributeSource()))); + } + + @Override + public int hashCode() { + return TransactionAttributeSourceClassFilter.class.hashCode(); + } + + @Override + public String toString() { + return TransactionAttributeSourceClassFilter.class.getName() + ": " + transactionAttributeSource; + } + } } diff --git a/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java b/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java index 8f33660d36ff..b9d70ee6a6b3 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 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. @@ -69,6 +69,25 @@ public void transactionProxyIsCreated() { ctx.close(); } + @Test // gh-31238 + public void cglibProxyClassIsCachedAcrossApplicationContexts() { + ConfigurableApplicationContext ctx; + + // Round #1 + ctx = new AnnotationConfigApplicationContext(EnableTxConfig.class, TxManagerConfig.class); + TransactionalTestBean bean1 = ctx.getBean(TransactionalTestBean.class); + assertThat(AopUtils.isCglibProxy(bean1)).as("testBean #1 is not a CGLIB proxy").isTrue(); + ctx.close(); + + // Round #2 + ctx = new AnnotationConfigApplicationContext(EnableTxConfig.class, TxManagerConfig.class); + TransactionalTestBean bean2 = ctx.getBean(TransactionalTestBean.class); + assertThat(AopUtils.isCglibProxy(bean2)).as("testBean #2 is not a CGLIB proxy").isTrue(); + ctx.close(); + + assertThat(bean1.getClass()).isSameAs(bean2.getClass()); + } + @Test public void transactionProxyIsCreatedWithEnableOnSuperclass() { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(