diff --git a/integration-tests/src/test/java/org/springframework/aop/framework/autoproxy/AspectJAutoProxyAdviceOrderIntegrationTests.java b/integration-tests/src/test/java/org/springframework/aop/framework/autoproxy/AspectJAutoProxyAdviceOrderIntegrationTests.java index 6e67a881fad0..78d45922be0c 100644 --- a/integration-tests/src/test/java/org/springframework/aop/framework/autoproxy/AspectJAutoProxyAdviceOrderIntegrationTests.java +++ b/integration-tests/src/test/java/org/springframework/aop/framework/autoproxy/AspectJAutoProxyAdviceOrderIntegrationTests.java @@ -59,15 +59,15 @@ class AspectJAutoProxyAdviceOrderIntegrationTests { class AfterAdviceFirstTests { @Test - void afterAdviceIsNotInvokedLast(@Autowired Echo echo, @Autowired AfterAdviceFirstAspect aspect) throws Exception { + void afterAdviceIsInvokedLast(@Autowired Echo echo, @Autowired AfterAdviceFirstAspect aspect) throws Exception { assertThat(aspect.invocations).isEmpty(); assertThat(echo.echo(42)).isEqualTo(42); - assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after returning"); + assertThat(aspect.invocations).containsExactly("around - start", "before", "after returning", "after", "around - end"); aspect.invocations.clear(); assertThatExceptionOfType(Exception.class).isThrownBy( () -> echo.echo(new Exception())); - assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after throwing"); + assertThat(aspect.invocations).containsExactly("around - start", "before", "after throwing", "after", "around - end"); } } @@ -78,7 +78,7 @@ void afterAdviceIsNotInvokedLast(@Autowired Echo echo, @Autowired AfterAdviceFir * in its source code. * *

On Java versions prior to JDK 7, we would have expected the {@code @After} - * advice method to be invoked after {@code @AfterThrowing} and + * advice method to be invoked before {@code @AfterThrowing} and * {@code @AfterReturning} advice methods due to the AspectJ precedence * rules implemented in * {@link org.springframework.aop.aspectj.autoproxy.AspectJPrecedenceComparator}. @@ -89,15 +89,15 @@ void afterAdviceIsNotInvokedLast(@Autowired Echo echo, @Autowired AfterAdviceFir class AfterAdviceLastTests { @Test - void afterAdviceIsNotInvokedLast(@Autowired Echo echo, @Autowired AfterAdviceLastAspect aspect) throws Exception { + void afterAdviceIsInvokedLast(@Autowired Echo echo, @Autowired AfterAdviceLastAspect aspect) throws Exception { assertThat(aspect.invocations).isEmpty(); assertThat(echo.echo(42)).isEqualTo(42); - assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after returning"); + assertThat(aspect.invocations).containsExactly("around - start", "before", "after returning", "after", "around - end"); aspect.invocations.clear(); assertThatExceptionOfType(Exception.class).isThrownBy( () -> echo.echo(new Exception())); - assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after throwing"); + assertThat(aspect.invocations).containsExactly("around - start", "before", "after throwing", "after", "around - end"); } } 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 da5a9070a245..5355b2bbb379 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 @@ -64,6 +64,7 @@ * @author Juergen Hoeller * @author Ramnivas Laddad * @author Phillip Webb + * @author Sam Brannen * @since 2.0 */ @SuppressWarnings("serial") @@ -72,6 +73,11 @@ public class ReflectiveAspectJAdvisorFactory extends AbstractAspectJAdvisorFacto private static final Comparator METHOD_COMPARATOR; static { + // Note: although @After is ordered before @AfterReturning and @AfterThrowing, + // an @After advice method will actually be invoked after @AfterReturning and + // @AfterThrowing methods due to the fact that AspectJAfterAdvice.invoke(MethodInvocation) + // invokes proceed() in a `try` block and only invokes the @After advice method + // in a corresponding `finally` block. Comparator adviceKindComparator = new ConvertingComparator<>( new InstanceComparator<>( Around.class, Before.class, After.class, AfterReturning.class, AfterThrowing.class), @@ -122,7 +128,15 @@ public List getAdvisors(MetadataAwareAspectInstanceFactory aspectInstan List advisors = new ArrayList<>(); for (Method method : getAdvisorMethods(aspectClass)) { - Advisor advisor = getAdvisor(method, lazySingletonAspectInstanceFactory, advisors.size(), aspectName); + // 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); }