From 06411831e856b446942323d150a902f6f8a46264 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 8 Jun 2023 18:31:26 +0200 Subject: [PATCH] Consistent ProxyCallbackFilter#equals/hashCode methods Opaque check in equals instead; no consideration of optimize flag. Closes gh-30616 --- .../aop/framework/CglibAopProxy.java | 13 +++-- .../aop/framework/CglibProxyTests.java | 51 ++++++++++++++++--- 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java index 87fa84d6b98a..1af6ae029aaf 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java +++ b/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; } } diff --git a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java index b1d1cd59438b..1281410e49cd 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java +++ b/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,6 +28,7 @@ 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; @@ -35,6 +36,8 @@ 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();