Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReflectionUtils.USER_DECLARED_METHODS does not filter methods declared in java.lang.Object #27970

Closed
kse-music opened this issue Jan 23, 2022 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@kse-music
Copy link

kse-music commented Jan 23, 2022

When I saw the AOP source code, I found that the getAdvisorMethods method in ReflectiveAspectJAdvisorFactory introspects the methods of Object, and I am confused about this.

The sample as follow,

@Aspect
@EnableAspectJAutoProxy
@Slf4j
public class AopConfig {

    @Around("execution(* cn.hiboot.framework.research.spring.research.SimpleStart.*(..))")
    public Object around(ProceedingJoinPoint p) throws Throwable {
        log.info("proceed before");
        Object obj = p.proceed();
        log.info("proceed after);
        return obj;
    }

}
@Slf4j
@Import(AopConfig.class)
public class SimpleStart {

    public SimpleStart() {
        log.info("execute constructor");
    }

    public String test(){
        return "test method return value";
    }

}
  @Test
  public void research(){
      AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(SimpleStart.class);
      SimpleStart bean = context.getBean(SimpleStart.class);
      bean.test();
  }

When a superclass is Object, it continues recursively, because in ReflectiveAspectJAdvisorFactory.adviceMethodFilter the mf = ReflectionUtils.USER_DECLARED_METHODS.and(method -> (AnnotationUtils.getAnnotation(method, Pointcut.class) == null)).

The source code position:

https://github.com/spring-projects/spring-framework/blob/main/spring-core/src/main/java/org/springframework/util/ReflectionUtils.java#L369

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 23, 2022
@sbrannen
Copy link
Member

I've edited your comment to improve the formatting. You might want to check out this Mastering Markdown guide for future reference.

@sbrannen sbrannen changed the title In ReflectionUtils.doWithMethods,when a class super class is object and mf != USER_DECLARED_METHODS,it's continues recusive In ReflectionUtils.doWithMethods, when superclass is Object and mf != USER_DECLARED_METHODS, it continues recursively Jan 23, 2022
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Jan 23, 2022
@sbrannen
Copy link
Member

sbrannen commented Jan 23, 2022

With regard to the behavior of ReflectiveAspectJAdvisorFactory, this is likely a regression introduced in c419ea7 due to the composition of two MethodFilter implementations using .and(...).

I introduced that change, and I assume that I believed the Javadoc for ReflectionUtils.USER_DECLARED_METHODS which is unfortunately incorrect.

The USER_DECLARED_METHODS MethodFilter in fact does not filter out methods declared on java.lang.Object. Rather, the implementation of ReflectionUtils.doWithMethods(Class<?>, MethodCallback, MethodFilter) performs that filtering. The catch is that the MethodFilter instance must be the exact USER_DECLARED_METHODS instance, and when filters are composed via .and(...) that equality check (==) will never be true.

I see two ways to address this issue.

  1. Revert the changes in c419ea7.
  2. Modify ReflectionUtils.USER_DECLARED_METHODS so that it actually does what the Javadoc claims it does.

The latter can be achieved as follows.

public static final MethodFilter USER_DECLARED_METHODS =
		(method -> !method.isBridge() && !method.isSynthetic() && (method.getDeclaringClass() != Object.class));

@sbrannen sbrannen self-assigned this Jan 23, 2022
@sbrannen sbrannen added type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 23, 2022
@sbrannen sbrannen added this to the 5.3.16 milestone Jan 23, 2022
@sbrannen sbrannen changed the title In ReflectionUtils.doWithMethods, when superclass is Object and mf != USER_DECLARED_METHODS, it continues recursively ReflectiveAspectJAdvisorFactory needlessly introspects methods declared in java.lang.Object Jan 23, 2022
@sbrannen sbrannen changed the title ReflectiveAspectJAdvisorFactory needlessly introspects methods declared in java.lang.Object ReflectionUtils.USER_DECLARED_METHODS does not filter methods declared in java.lang.Object Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

3 participants