mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-09-17 16:30:28 +00:00
Suppress CGLIB validation WARN for lifecycle callbacks
When CglibAopProxy validates the target class, it logs a WARN-level message for each public final method that implements an interface, suggesting to use interface-based JDK proxies instead. For final methods inherited from Spring's configuration callback interfaces (InitializingBean, DisposableBean, Aware sub-interfaces, Closeable, AutoCloseable) that recommendation is misleading: those methods are container-driven, are not advised by typical application pointcuts, and the user usually cannot make them non-final. The validation now only emits the WARN-level message when at least one user-defined interface declares the method. Methods inherited exclusively from configuration callback interfaces fall back to the existing DEBUG diagnostic. In addition, the isConfigurationCallbackInterface() method has been extracted from ProxyProcessorSupport into a static package-private method in AopProxyUtils with the same signature, and ProxyProcessorSupport and CglibAopProxy now delegate to the new shared static utility in AopProxyUtils. See gh-35365 Closes gh-36935 Signed-off-by: seonwoo_jung <laborlawseon@kap.kr> Signed-off-by: seonwooj0810 <seonwooj0810@gmail.com>
This commit is contained in:
@@ -16,6 +16,7 @@
|
||||
|
||||
package org.springframework.aop.framework;
|
||||
|
||||
import java.io.Closeable;
|
||||
import java.lang.reflect.Array;
|
||||
import java.lang.reflect.Method;
|
||||
import java.lang.reflect.Proxy;
|
||||
@@ -30,6 +31,9 @@ import org.springframework.aop.TargetClassAware;
|
||||
import org.springframework.aop.TargetSource;
|
||||
import org.springframework.aop.support.AopUtils;
|
||||
import org.springframework.aop.target.SingletonTargetSource;
|
||||
import org.springframework.beans.factory.Aware;
|
||||
import org.springframework.beans.factory.DisposableBean;
|
||||
import org.springframework.beans.factory.InitializingBean;
|
||||
import org.springframework.core.DecoratingProxy;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.ClassUtils;
|
||||
@@ -278,4 +282,15 @@ public abstract class AopProxyUtils {
|
||||
return arguments;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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).
|
||||
*/
|
||||
static boolean isConfigurationCallbackInterface(Class<?> ifc) {
|
||||
return (InitializingBean.class == ifc || DisposableBean.class == ifc ||
|
||||
Closeable.class == ifc || AutoCloseable.class == ifc ||
|
||||
ObjectUtils.containsElement(ifc.getInterfaces(), Aware.class));
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
|
||||
package org.springframework.aop.framework;
|
||||
|
||||
import java.io.Closeable;
|
||||
import java.io.Serializable;
|
||||
import java.lang.reflect.Method;
|
||||
import java.lang.reflect.Modifier;
|
||||
@@ -37,6 +38,9 @@ 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;
|
||||
@@ -292,8 +296,10 @@ class CglibAopProxy implements AopProxy, Serializable {
|
||||
if (Modifier.isFinal(mod)) {
|
||||
if (logger.isWarnEnabled() && Modifier.isPublic(mod)) {
|
||||
if (implementsInterface(method, ifcs)) {
|
||||
logger.warn("Unable to proxy interface-implementing method [" + method + "] because " +
|
||||
"it is marked as final, consider using interface-based JDK proxies instead.");
|
||||
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.");
|
||||
}
|
||||
}
|
||||
else {
|
||||
logger.warn("Public final method [" + method + "] cannot get proxied via CGLIB, " +
|
||||
@@ -415,6 +421,28 @@ class CglibAopProxy implements AopProxy, Serializable {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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).
|
||||
*/
|
||||
static boolean implementsOnlyConfigurationCallbackInterfaces(Method method, Set<Class<?>> ifcs) {
|
||||
boolean matched = false;
|
||||
for (Class<?> ifc : ifcs) {
|
||||
if (ClassUtils.hasMethod(ifc, method)) {
|
||||
if (!AopProxyUtils.isConfigurationCallbackInterface(ifc)) {
|
||||
return false;
|
||||
}
|
||||
matched = true;
|
||||
}
|
||||
}
|
||||
return matched;
|
||||
}
|
||||
|
||||
/**
|
||||
* Process a return value. Wraps a return of {@code this} if necessary to be the
|
||||
* {@code proxy} and also verifies that {@code null} is not returned as a primitive.
|
||||
|
||||
+1
-8
@@ -16,17 +16,11 @@
|
||||
|
||||
package org.springframework.aop.framework;
|
||||
|
||||
import java.io.Closeable;
|
||||
|
||||
import org.jspecify.annotations.Nullable;
|
||||
|
||||
import org.springframework.beans.factory.Aware;
|
||||
import org.springframework.beans.factory.BeanClassLoaderAware;
|
||||
import org.springframework.beans.factory.DisposableBean;
|
||||
import org.springframework.beans.factory.InitializingBean;
|
||||
import org.springframework.core.Ordered;
|
||||
import org.springframework.util.ClassUtils;
|
||||
import org.springframework.util.ObjectUtils;
|
||||
|
||||
/**
|
||||
* Base class with common functionality for proxy processors, in particular
|
||||
@@ -130,8 +124,7 @@ public class ProxyProcessorSupport extends ProxyConfig implements Ordered, BeanC
|
||||
* @return whether the given interface is just a container callback
|
||||
*/
|
||||
protected boolean isConfigurationCallbackInterface(Class<?> ifc) {
|
||||
return (InitializingBean.class == ifc || DisposableBean.class == ifc || Closeable.class == ifc ||
|
||||
AutoCloseable.class == ifc || ObjectUtils.containsElement(ifc.getInterfaces(), Aware.class));
|
||||
return AopProxyUtils.isConfigurationCallbackInterface(ifc);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
+161
@@ -0,0 +1,161 @@
|
||||
/*
|
||||
* Copyright 2002-present 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.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
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.BeanFactoryAware;
|
||||
import org.springframework.beans.factory.DisposableBean;
|
||||
import org.springframework.beans.factory.InitializingBean;
|
||||
import org.springframework.util.ClassUtils;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* Tests for {@link CglibAopProxy#implementsOnlyConfigurationCallbackInterfaces}.
|
||||
*
|
||||
* <p>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).
|
||||
*/
|
||||
class CglibAopProxyConfigurationCallbackTests {
|
||||
|
||||
@Test
|
||||
void finalAfterPropertiesSetIsRecognisedAsCallback() throws NoSuchMethodException {
|
||||
Method method = WithFinalAfterPropertiesSet.class.getDeclaredMethod("afterPropertiesSet");
|
||||
Set<Class<?>> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalAfterPropertiesSet.class);
|
||||
|
||||
assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void finalDestroyIsRecognisedAsCallback() throws NoSuchMethodException {
|
||||
Method method = WithFinalDestroy.class.getDeclaredMethod("destroy");
|
||||
Set<Class<?>> 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<Class<?>> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalBeanFactoryAware.class);
|
||||
|
||||
assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void finalCloseIsRecognisedAsCallback() throws NoSuchMethodException {
|
||||
Method method = WithFinalClose.class.getDeclaredMethod("close");
|
||||
Set<Class<?>> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalClose.class);
|
||||
|
||||
assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void finalUserInterfaceMethodIsNotSuppressed() throws NoSuchMethodException {
|
||||
Method method = WithFinalUserApi.class.getDeclaredMethod("execute");
|
||||
Set<Class<?>> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithFinalUserApi.class);
|
||||
|
||||
assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void methodSharedBetweenCallbackAndUserInterfaceIsNotSuppressed() throws NoSuchMethodException {
|
||||
Method method = WithSharedSignature.class.getDeclaredMethod("afterPropertiesSet");
|
||||
Set<Class<?>> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithSharedSignature.class);
|
||||
|
||||
// Even though InitializingBean declares afterPropertiesSet(), a user
|
||||
// interface (CustomLifecycle) declares the same signature, so the
|
||||
// warning must still fire.
|
||||
assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void finalMethodWithoutInterfaceMatchIsNotSuppressed() throws NoSuchMethodException {
|
||||
Method method = WithStandaloneFinal.class.getDeclaredMethod("doSomething");
|
||||
Set<Class<?>> interfaces = ClassUtils.getAllInterfacesForClassAsSet(WithStandaloneFinal.class);
|
||||
|
||||
assertThat(CglibAopProxy.implementsOnlyConfigurationCallbackInterfaces(method, interfaces)).isFalse();
|
||||
}
|
||||
|
||||
|
||||
static class WithFinalAfterPropertiesSet implements InitializingBean {
|
||||
|
||||
@Override
|
||||
public final void afterPropertiesSet() {
|
||||
}
|
||||
}
|
||||
|
||||
static class WithFinalDestroy implements DisposableBean {
|
||||
|
||||
@Override
|
||||
public final void destroy() {
|
||||
}
|
||||
}
|
||||
|
||||
static class WithFinalBeanFactoryAware implements BeanFactoryAware {
|
||||
|
||||
@Override
|
||||
public final void setBeanFactory(org.springframework.beans.factory.BeanFactory beanFactory) {
|
||||
}
|
||||
}
|
||||
|
||||
static class WithFinalClose implements Closeable {
|
||||
|
||||
@Override
|
||||
public final void close() {
|
||||
}
|
||||
}
|
||||
|
||||
interface UserApi {
|
||||
|
||||
void execute();
|
||||
}
|
||||
|
||||
static class WithFinalUserApi implements UserApi {
|
||||
|
||||
@Override
|
||||
public final void execute() {
|
||||
}
|
||||
}
|
||||
|
||||
interface CustomLifecycle {
|
||||
|
||||
void afterPropertiesSet();
|
||||
}
|
||||
|
||||
static class WithSharedSignature implements InitializingBean, CustomLifecycle {
|
||||
|
||||
@Override
|
||||
public final void afterPropertiesSet() {
|
||||
}
|
||||
}
|
||||
|
||||
static class WithStandaloneFinal {
|
||||
|
||||
public final void doSomething() {
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user