Consistent ProxyCallbackFilter#equals/hashCode methods

Opaque check in equals instead; no consideration of optimize flag.

Closes gh-30616

Author: jhoeller

spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.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.
@@ -283,8 +283,8 @@ else if (logger.isDebugEnabled() && !Modifier.isPublic(mod) && !Modifier.isProte
 
 	private Callback[] getCallbacks(Class<?> rootClass) throws Exception {
 		// Parameters used for optimization choices...
-		boolean exposeProxy = this.advised.isExposeProxy();
 		boolean isFrozen = this.advised.isFrozen();
+		boolean exposeProxy = this.advised.isExposeProxy();
 		boolean isStatic = this.advised.getTargetSource().isStatic();
 
 		// Choose an "aop" interceptor (used for AOP calls).
@@ -899,9 +899,9 @@ public int accept(Method method) {
 			// Proxy is not yet available, but that shouldn't matter.
 			List<?> chain = this.advised.getInterceptorsAndDynamicInterceptionAdvice(method, targetClass);
 			boolean haveAdvice = !chain.isEmpty();
+			boolean isFrozen = this.advised.isFrozen();
 			boolean exposeProxy = this.advised.isExposeProxy();
 			boolean isStatic = this.advised.getTargetSource().isStatic();
-			boolean isFrozen = this.advised.isFrozen();
 			if (haveAdvice || !isFrozen) {
 				// If exposing the proxy, then AOP_PROXY must be used.
 				if (exposeProxy) {
@@ -970,6 +970,9 @@ public boolean equals(@Nullable Object other) {
 			if (this.advised.isExposeProxy() != otherAdvised.isExposeProxy()) {
 				return false;
 			}
+			if (this.advised.isOpaque() != otherAdvised.isOpaque()) {
+				return false;
+			}
 			if (this.advised.getTargetSource().isStatic() != otherAdvised.getTargetSource().isStatic()) {
 				return false;
 			}
@@ -1016,10 +1019,6 @@ public int hashCode() {
 				Advice advice = advisor.getAdvice();
 				hashCode = 13 * hashCode + advice.getClass().hashCode();
 			}
-			hashCode = 13 * hashCode + (this.advised.isFrozen() ? 1 : 0);
-			hashCode = 13 * hashCode + (this.advised.isExposeProxy() ? 1 : 0);
-			hashCode = 13 * hashCode + (this.advised.isOptimize() ? 1 : 0);
-			hashCode = 13 * hashCode + (this.advised.isOpaque() ? 1 : 0);
 			return hashCode;
 		}
 	}

spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.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.
@@ -28,13 +28,16 @@
 import org.springframework.aop.Pointcut;
 import org.springframework.aop.support.AopUtils;
 import org.springframework.aop.support.DefaultPointcutAdvisor;
+import org.springframework.aop.support.annotation.AnnotationMatchingPointcut;
 import org.springframework.aop.testfixture.advice.CountingBeforeAdvice;
 import org.springframework.aop.testfixture.interceptor.NopInterceptor;
 import org.springframework.beans.testfixture.beans.ITestBean;
 import org.springframework.beans.testfixture.beans.TestBean;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextException;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
+import org.springframework.lang.NonNull;
+import org.springframework.lang.Nullable;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -87,8 +90,7 @@ public void testNoTarget() {
 		AdvisedSupport pc = new AdvisedSupport(ITestBean.class);
 		pc.addAdvice(new NopInterceptor());
 		AopProxy aop = createAopProxy(pc);
-		assertThatExceptionOfType(AopConfigException.class).isThrownBy(
-				aop::getProxy);
+		assertThatExceptionOfType(AopConfigException.class).isThrownBy(aop::getProxy);
 	}
 
 	@Test
@@ -136,8 +138,8 @@ public void testProxyCanBeClassNotInterface() {
 
 		Object proxy = aop.getProxy();
 		assertThat(AopUtils.isCglibProxy(proxy)).isTrue();
-		assertThat(proxy instanceof ITestBean).isTrue();
-		assertThat(proxy instanceof TestBean).isTrue();
+		assertThat(proxy).isInstanceOf(ITestBean.class);
+		assertThat(proxy).isInstanceOf(TestBean.class);
 
 		TestBean tb = (TestBean) proxy;
 		assertThat(tb.getAge()).isEqualTo(32);
@@ -304,6 +306,8 @@ public void testProxyAProxyWithAdditionalInterface() {
 		CglibAopProxy cglib = new CglibAopProxy(as);
 
 		ITestBean proxy1 = (ITestBean) cglib.getProxy();
+		ITestBean proxy1a = (ITestBean) cglib.getProxy();
+		assertThat(proxy1a.getClass()).isSameAs(proxy1.getClass());
 
 		mockTargetSource.setTarget(proxy1);
 		as = new AdvisedSupport(new Class<?>[]{});
@@ -312,7 +316,40 @@ public void testProxyAProxyWithAdditionalInterface() {
 		cglib = new CglibAopProxy(as);
 
 		ITestBean proxy2 = (ITestBean) cglib.getProxy();
-		assertThat(proxy2 instanceof Serializable).isTrue();
+		assertThat(proxy2).isInstanceOf(Serializable.class);
+		assertThat(proxy2.getClass()).isNotSameAs(proxy1.getClass());
+
+		ITestBean proxy2a = (ITestBean) cglib.getProxy();
+		assertThat(proxy2a).isInstanceOf(Serializable.class);
+		assertThat(proxy2a.getClass()).isSameAs(proxy2.getClass());
+
+		mockTargetSource.setTarget(proxy1);
+		as = new AdvisedSupport(new Class<?>[]{});
+		as.setTargetSource(mockTargetSource);
+		as.addAdvisor(new DefaultPointcutAdvisor(new AnnotationMatchingPointcut(Nullable.class), new NopInterceptor()));
+		cglib = new CglibAopProxy(as);
+
+		ITestBean proxy3 = (ITestBean) cglib.getProxy();
+		assertThat(proxy3).isInstanceOf(Serializable.class);
+		assertThat(proxy3.getClass()).isNotSameAs(proxy2.getClass());
+
+		ITestBean proxy3a = (ITestBean) cglib.getProxy();
+		assertThat(proxy3a).isInstanceOf(Serializable.class);
+		assertThat(proxy3a.getClass()).isSameAs(proxy3.getClass());
+
+		mockTargetSource.setTarget(proxy1);
+		as = new AdvisedSupport(new Class<?>[]{});
+		as.setTargetSource(mockTargetSource);
+		as.addAdvisor(new DefaultPointcutAdvisor(new AnnotationMatchingPointcut(NonNull.class), new NopInterceptor()));
+		cglib = new CglibAopProxy(as);
+
+		ITestBean proxy4 = (ITestBean) cglib.getProxy();
+		assertThat(proxy4).isInstanceOf(Serializable.class);
+		assertThat(proxy4.getClass()).isNotSameAs(proxy3.getClass());
+
+		ITestBean proxy4a = (ITestBean) cglib.getProxy();
+		assertThat(proxy4a).isInstanceOf(Serializable.class);
+		assertThat(proxy4a.getClass()).isSameAs(proxy4.getClass());
 	}
 
 	@Test
@@ -331,7 +368,7 @@ public void testExceptionHandling() {
 			proxy.doTest();
 		}
 		catch (Exception ex) {
-			assertThat(ex instanceof ApplicationContextException).as("Invalid exception class").isTrue();
+			assertThat(ex).as("Invalid exception class").isInstanceOf(ApplicationContextException.class);
 		}
 
 		assertThat(proxy.isCatchInvoked()).as("Catch was not invoked").isTrue();