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 c1c10c946ed6..ce682a75774e 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-2020 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 847ef4f38044..97d74dd6f4fe 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 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. @@ -37,7 +37,6 @@ import org.aspectj.lang.annotation.DeclarePrecedence; import org.aspectj.lang.annotation.Pointcut; import org.aspectj.lang.reflect.MethodSignature; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import test.aop.DefaultLockable; import test.aop.Lockable; @@ -76,25 +75,24 @@ abstract class AbstractAspectJAdvisorFactoryTests { /** * To be overridden by concrete test subclasses. - * @return the fixture */ protected abstract AspectJAdvisorFactory getFixture(); @Test void rejectsPerCflowAspect() { - assertThatExceptionOfType(AopConfigException.class).isThrownBy(() -> - getFixture().getAdvisors( + assertThatExceptionOfType(AopConfigException.class) + .isThrownBy(() -> getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new PerCflowAspect(), "someBean"))) - .withMessageContaining("PERCFLOW"); + .withMessageContaining("PERCFLOW"); } @Test void rejectsPerCflowBelowAspect() { - assertThatExceptionOfType(AopConfigException.class).isThrownBy(() -> - getFixture().getAdvisors( - new SingletonMetadataAwareAspectInstanceFactory(new PerCflowBelowAspect(), "someBean"))) - .withMessageContaining("PERCFLOWBELOW"); + assertThatExceptionOfType(AopConfigException.class) + .isThrownBy(() -> getFixture().getAdvisors( + new SingletonMetadataAwareAspectInstanceFactory(new PerCflowBelowAspect(), "someBean"))) + .withMessageContaining("PERCFLOWBELOW"); } @Test @@ -385,8 +383,7 @@ void introductionOnTargetImplementingInterface() { assertThat(lockable.locked()).as("Already locked").isTrue(); lockable.lock(); assertThat(lockable.locked()).as("Real target ignores locking").isTrue(); - assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> - lockable.unlock()); + assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(lockable::unlock); } @Test @@ -413,9 +410,7 @@ void introductionBasedOnAnnotationMatch_SPR5307() { lockable.locked(); } - // TODO: Why does this test fail? It hasn't been run before, so it maybe never actually passed... @Test - @Disabled void introductionWithArgumentBinding() { TestBean target = new TestBean(); @@ -523,6 +518,16 @@ void afterAdviceTypes() throws Exception { assertThat(aspect.invocations).containsExactly("around - start", "before", "after throwing", "after", "around - end"); } + @Test + void parentAspect() { + TestBean target = new TestBean("Jane", 42); + MetadataAwareAspectInstanceFactory aspectInstanceFactory = new SingletonMetadataAwareAspectInstanceFactory( + new IncrementingAspect(), "incrementingAspect"); + ITestBean proxy = (ITestBean) createProxy(target, + getFixture().getAdvisors(aspectInstanceFactory), ITestBean.class); + assertThat(proxy.getAge()).isEqualTo(86); // (42 + 1) * 2 + } + @Test void failureWithoutExplicitDeclarePrecedence() { TestBean target = new TestBean(); @@ -647,7 +652,7 @@ public int getOrder() { static class NamedPointcutAspectWithFQN { @SuppressWarnings("unused") - private ITestBean fieldThatShouldBeIgnoredBySpringAtAspectJProcessing = new TestBean(); + private final ITestBean fieldThatShouldBeIgnoredBySpringAtAspectJProcessing = new TestBean(); @Pointcut("execution(* getAge())") void getAge() { @@ -767,6 +772,31 @@ Object echo(Object o) throws Exception { } + @Aspect + abstract static class DoublingAspect { + + @Around("execution(* getAge())") + public Object doubleAge(ProceedingJoinPoint pjp) throws Throwable { + return ((int) pjp.proceed()) * 2; + } + } + + + @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; + } + } + + @Aspect private static class InvocationTrackingAspect { @@ -824,7 +854,7 @@ void blowUpButDoesntMatterBecauseAroundAdviceWontLetThisBeInvoked() { @Around("getAge()") int preventExecution(ProceedingJoinPoint pjp) { - return 666; + return 42; } } @@ -844,7 +874,7 @@ void blowUpButDoesntMatterBecauseAroundAdviceWontLetThisBeInvoked() { @Around("getAge()") int preventExecution(ProceedingJoinPoint pjp) { - return 666; + return 42; } } @@ -1066,7 +1096,7 @@ class PerThisAspect { // Just to check that this doesn't cause problems with introduction processing @SuppressWarnings("unused") - private ITestBean fieldThatShouldBeIgnoredBySpringAtAspectJProcessing = new TestBean(); + private final ITestBean fieldThatShouldBeIgnoredBySpringAtAspectJProcessing = new TestBean(); @Around("execution(int *.getAge())") int returnCountAsAge() {