From 79a75a5762115809bc4dd8e96de39b36e21187a5 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Mon, 24 Aug 2026 12:36:28 +0200 Subject: [PATCH] Polish contribution See gh-36935 --- .../aop/framework/AopProxyUtils.java | 1 + .../aop/framework/CglibAopProxy.java | 18 ++--- ...libAopProxyConfigurationCallbackTests.java | 72 +++++++++++-------- 3 files changed, 52 insertions(+), 39 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java b/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java index eccb75a119d..c53037d0ec4 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java @@ -286,6 +286,7 @@ public abstract class AopProxyUtils { * Determine whether the given interface is a Spring configuration callback * interface (i.e. {@link InitializingBean}, {@link DisposableBean}, * {@link Closeable}/{@link AutoCloseable}, or an {@link Aware} sub-interface). + * @since 7.1 */ static boolean isConfigurationCallbackInterface(Class ifc) { return (InitializingBean.class == ifc || DisposableBean.class == ifc || 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 0840f281737..cf93cafb418 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 @@ -16,7 +16,6 @@ package org.springframework.aop.framework; -import java.io.Closeable; import java.io.Serializable; import java.lang.reflect.Method; import java.lang.reflect.Modifier; @@ -38,9 +37,6 @@ import org.springframework.aop.RawTargetAccess; import org.springframework.aop.TargetSource; import org.springframework.aop.support.AopUtils; import org.springframework.aot.AotDetector; -import org.springframework.beans.factory.Aware; -import org.springframework.beans.factory.DisposableBean; -import org.springframework.beans.factory.InitializingBean; import org.springframework.cglib.core.ClassLoaderAwareGeneratorStrategy; import org.springframework.cglib.core.CodeGenerationException; import org.springframework.cglib.core.GeneratorStrategy; @@ -296,6 +292,9 @@ class CglibAopProxy implements AopProxy, Serializable { if (Modifier.isFinal(mod)) { if (logger.isWarnEnabled() && Modifier.isPublic(mod)) { if (implementsInterface(method, ifcs)) { + // Final methods inherited from configuration callback interfaces are + // typically driven by the container itself rather than by user code, so + // logging a warning about CGLIB being unable to advise them is misleading noise. if (!implementsOnlyConfigurationCallbackInterfaces(method, ifcs)) { logger.warn("Unable to proxy interface-implementing method [" + method + "] because " + "it is marked as final, consider using interface-based JDK proxies instead."); @@ -422,13 +421,10 @@ class CglibAopProxy implements AopProxy, Serializable { } /** - * Check whether every interface that declares the given method is a Spring - * configuration callback interface, such as {@link InitializingBean}, - * {@link DisposableBean}, an {@link Aware} sub-interface, or - * {@link Closeable}/{@link AutoCloseable}. Final methods inherited from such - * interfaces are typically driven by the container itself rather than by user - * code, so logging a WARN about CGLIB being unable to advise them is - * misleading noise (gh-35365). + * Check whether every interface that declares the given method is a + * configuration callback interface. + * @since 7.1 + * @see AopProxyUtils#isConfigurationCallbackInterface(Class) */ static boolean implementsOnlyConfigurationCallbackInterfaces(Method method, Set> ifcs) { boolean matched = false; diff --git a/spring-aop/src/test/java/org/springframework/aop/framework/CglibAopProxyConfigurationCallbackTests.java b/spring-aop/src/test/java/org/springframework/aop/framework/CglibAopProxyConfigurationCallbackTests.java index 79065babfec..b20569571b0 100644 --- a/spring-aop/src/test/java/org/springframework/aop/framework/CglibAopProxyConfigurationCallbackTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/framework/CglibAopProxyConfigurationCallbackTests.java @@ -17,11 +17,10 @@ package org.springframework.aop.framework; import java.io.Closeable; -import java.lang.reflect.Method; -import java.util.Set; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; @@ -34,56 +33,66 @@ import static org.assertj.core.api.Assertions.assertThat; * *

Verifies that final methods inherited from Spring's configuration callback * interfaces (InitializingBean, DisposableBean, Aware sub-interfaces, - * Closeable/AutoCloseable) are recognised so that the CGLIB validation warning - * can be suppressed for those container-driven methods (gh-35365). + * Closeable/AutoCloseable) are recognized so that the CGLIB validation warning + * can be suppressed for those container-driven methods. + * + * @since 7.1 + * @see > interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalAfterPropertiesSet.class); + void finalAfterPropertiesSetIsRecognisedAsCallback() { + var method = ClassUtils.getMethod(WithFinalAfterPropertiesSet.class, "afterPropertiesSet"); + var interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalAfterPropertiesSet.class); assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isTrue(); } @Test - void finalDestroyIsRecognisedAsCallback() throws NoSuchMethodException { - Method method = WithFinalDestroy.class.getDeclaredMethod("destroy"); - Set> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalDestroy.class); + void finalDestroyIsRecognisedAsCallback() { + var method = ClassUtils.getMethod(WithFinalDestroy.class, "destroy"); + var interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalDestroy.class); assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isTrue(); } @Test - void finalAwareCallbackIsRecognisedAsCallback() throws NoSuchMethodException { - Method method = WithFinalBeanFactoryAware.class.getDeclaredMethod("setBeanFactory", - org.springframework.beans.factory.BeanFactory.class); - Set> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalBeanFactoryAware.class); + void finalAwareCallbackIsRecognisedAsCallback() { + var method = ClassUtils.getMethod(WithFinalBeanFactoryAware.class, "setBeanFactory", BeanFactory.class); + var interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalBeanFactoryAware.class); assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isTrue(); } @Test - void finalCloseIsRecognisedAsCallback() throws NoSuchMethodException { - Method method = WithFinalClose.class.getDeclaredMethod("close"); - Set> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalClose.class); + void finalCloseableCloseIsRecognisedAsCallback() { + var method = ClassUtils.getMethod(WithFinalCloseableClose.class, "close"); + var interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalCloseableClose.class); assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isTrue(); } @Test - void finalUserInterfaceMethodIsNotSuppressed() throws NoSuchMethodException { - Method method = WithFinalUserApi.class.getDeclaredMethod("execute"); - Set> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalUserApi.class); + void finalAutoCloseableCloseIsRecognisedAsCallback() { + var method = ClassUtils.getMethod(WithFinalAutoCloseableClose.class, "close"); + var interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalAutoCloseableClose.class); + + assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isTrue(); + } + + @Test + void finalUserInterfaceMethodIsNotSuppressed() { + var method = ClassUtils.getMethod(WithFinalUserApi.class, "execute"); + var interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalUserApi.class); assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isFalse(); } @Test - void methodSharedBetweenCallbackAndUserInterfaceIsNotSuppressed() throws NoSuchMethodException { - Method method = WithSharedSignature.class.getDeclaredMethod("afterPropertiesSet"); - Set> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithSharedSignature.class); + void methodSharedBetweenCallbackAndUserInterfaceIsNotSuppressed() { + var method = ClassUtils.getMethod(WithSharedSignature.class, "afterPropertiesSet"); + var interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithSharedSignature.class); // Even though InitializingBean declares afterPropertiesSet(), a user // interface (CustomLifecycle) declares the same signature, so the @@ -92,9 +101,9 @@ class CglibAopProxyConfigurationCallbackTests { } @Test - void finalMethodWithoutInterfaceMatchIsNotSuppressed() throws NoSuchMethodException { - Method method = WithStandaloneFinal.class.getDeclaredMethod("doSomething"); - Set> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithStandaloneFinal.class); + void finalMethodWithoutInterfaceMatchIsNotSuppressed() { + var method = ClassUtils.getMethod(WithStandaloneFinal.class, "doSomething"); + var interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithStandaloneFinal.class); assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isFalse(); } @@ -117,11 +126,18 @@ class CglibAopProxyConfigurationCallbackTests { static class WithFinalBeanFactoryAware implements BeanFactoryAware { @Override - public final void setBeanFactory(org.springframework.beans.factory.BeanFactory beanFactory) { + public final void setBeanFactory(BeanFactory beanFactory) { } } - static class WithFinalClose implements Closeable { + static class WithFinalCloseableClose implements Closeable { + + @Override + public final void close() { + } + } + + static class WithFinalAutoCloseableClose implements AutoCloseable { @Override public final void close() {