Cache CGLIB proxy classes properly again

The introduction of AdvisedSupport.AdvisorKeyEntry in Spring Framework
6.0.10 resulted in a regression regarding caching of CGLIB generated
proxy classes. Specifically, equality checks for the proxy class cache
became based partially on identity rather than equivalence. For
example, if an ApplicationContext was configured to create a
class-based @transactional proxy, a second attempt to create the
ApplicationContext resulted in a duplicate proxy class for the same
@transactional component.

On the JVM this went unnoticed; however, when running Spring
integration tests within a native image, if a test made use of
@⁠DirtiesContext, a second attempt to create the test
ApplicationContext resulted in an exception stating, "CGLIB runtime
enhancement not supported on native image." This is because Test AOT
processing only refreshes a test ApplicationContext once, and the
duplicate CGLIB proxy classes are only requested in subsequent
refreshes of the same ApplicationContext which means that duplicate
proxy classes are not tracked during AOT processing and consequently
not included in a native image.

This commit addresses this regression as follows.

- AdvisedSupport.AdvisorKeyEntry is now based on the toString()
  representations of the ClassFilter and MethodMatcher in the
  corresponding Pointcut instead of the filter's and matcher's
  identities.

- Due to the above changes to AdvisorKeyEntry, ClassFilter and
  MethodMatcher implementations are now required to implement equals(),
  hashCode(), AND toString().

- Consequently, the following now include proper equals(), hashCode(),
  and toString() implementations.

  - CacheOperationSourcePointcut
  - TransactionAttributeSourcePointcut
  - PerTargetInstantiationModelPointcut

Closes gh-31238

Author: sbrannen

spring-aop/src/main/java/org/springframework/aop/ClassFilter.java

@@ -23,12 +23,17 @@
  * <p>Can be used as part of a {@link Pointcut} or for the entire targeting of
  * an {@link IntroductionAdvisor}.
  *
- * <p>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 &mdash; for
- * example, in proxies generated by CGLIB.
+ * <p><strong>WARNING</strong>: 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 &mdash; 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
  */

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.
  *
- * <p>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 &mdash; for
- * example, in proxies generated by CGLIB.
+ * <p><strong>WARNING</strong>: 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 &mdash; 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

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();
+		}
+
 	}
 
 }

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;

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

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();

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

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

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(