From ea596aa211ea17b814099a73bb5a091a9af1cadd Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 22 May 2024 10:00:31 +0200 Subject: [PATCH] Select most specific advice method in case of override Closes gh-32865 --- .../ReflectiveAspectJAdvisorFactory.java | 27 ++++++++++--------- .../AbstractAspectJAdvisorFactoryTests.java | 15 +++++++---- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactory.java b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactory.java index 4e2f5c3bd801..90cc3880cb84 100644 --- a/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactory.java +++ b/spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 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. @@ -50,6 +50,7 @@ import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConvertingComparator; import org.springframework.lang.Nullable; +import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.ReflectionUtils.MethodFilter; import org.springframework.util.StringUtils; @@ -133,17 +134,19 @@ public List getAdvisors(MetadataAwareAspectInstanceFactory aspectInstan List advisors = new ArrayList<>(); for (Method method : getAdvisorMethods(aspectClass)) { - // Prior to Spring Framework 5.2.7, advisors.size() was supplied as the declarationOrderInAspect - // to getAdvisor(...) to represent the "current position" in the declared methods list. - // However, since Java 7 the "current position" is not valid since the JDK no longer - // returns declared methods in the order in which they are declared in the source code. - // Thus, we now hard code the declarationOrderInAspect to 0 for all advice methods - // discovered via reflection in order to support reliable advice ordering across JVM launches. - // Specifically, a value of 0 aligns with the default value used in - // AspectJPrecedenceComparator.getAspectDeclarationOrder(Advisor). - Advisor advisor = getAdvisor(method, lazySingletonAspectInstanceFactory, 0, aspectName); - if (advisor != null) { - advisors.add(advisor); + if (method.equals(ClassUtils.getMostSpecificMethod(method, aspectClass))) { + // Prior to Spring Framework 5.2.7, advisors.size() was supplied as the declarationOrderInAspect + // to getAdvisor(...) to represent the "current position" in the declared methods list. + // However, since Java 7 the "current position" is not valid since the JDK no longer + // returns declared methods in the order in which they are declared in the source code. + // Thus, we now hard code the declarationOrderInAspect to 0 for all advice methods + // discovered via reflection in order to support reliable advice ordering across JVM launches. + // Specifically, a value of 0 aligns with the default value used in + // AspectJPrecedenceComparator.getAspectDeclarationOrder(Advisor). + Advisor advisor = getAdvisor(method, lazySingletonAspectInstanceFactory, 0, aspectName); + if (advisor != null) { + advisors.add(advisor); + } } } diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java index ff92d110afc3..02d968212d53 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java @@ -83,15 +83,15 @@ abstract class AbstractAspectJAdvisorFactoryTests { @Test void rejectsPerCflowAspect() { assertThatExceptionOfType(AopConfigException.class) - .isThrownBy(() -> getAdvisorFactory().getAdvisors(aspectInstanceFactory(new PerCflowAspect(), "someBean"))) - .withMessageContaining("PERCFLOW"); + .isThrownBy(() -> getAdvisorFactory().getAdvisors(aspectInstanceFactory(new PerCflowAspect(), "someBean"))) + .withMessageContaining("PERCFLOW"); } @Test void rejectsPerCflowBelowAspect() { assertThatExceptionOfType(AopConfigException.class) - .isThrownBy(() -> getAdvisorFactory().getAdvisors(aspectInstanceFactory(new PerCflowBelowAspect(), "someBean"))) - .withMessageContaining("PERCFLOWBELOW"); + .isThrownBy(() -> getAdvisorFactory().getAdvisors(aspectInstanceFactory(new PerCflowBelowAspect(), "someBean"))) + .withMessageContaining("PERCFLOWBELOW"); } @Test @@ -770,9 +770,15 @@ public Object doubleAge(ProceedingJoinPoint pjp) throws Throwable { } } + @Aspect static class IncrementingAspect extends DoublingAspect { + @Override + public Object doubleAge(ProceedingJoinPoint pjp) throws Throwable { + return ((int) pjp.proceed()) * 2; + } + @Around("execution(* getAge())") public int incrementAge(ProceedingJoinPoint pjp) throws Throwable { return ((int) pjp.proceed()) + 1; @@ -780,7 +786,6 @@ public int incrementAge(ProceedingJoinPoint pjp) throws Throwable { } - @Aspect private static class InvocationTrackingAspect {