Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Select most specific advice method in case of override

Browse Source 
Closes gh-32865

(cherry picked from commit ea596aa211)
This commit is contained in:
Juergen Hoeller
2024-05-22 10:07:31 +02:00
parent f1fed9c174
commit eff9f5b92a
2 changed files with 63 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactory.java
View File

@@ -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.annotation.AnnotationUtils;
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 class ReflectiveAspectJAdvisorFactory extends AbstractAspectJAdvisorFacto
List<Advisor> 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);
}
}
}

66
spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AbstractAspectJAdvisorFactoryTests.java
View File

@@ -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.DeclareParents;
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 @@ abstract class AbstractAspectJAdvisorFactoryTests {
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 @@ abstract class AbstractAspectJAdvisorFactoryTests {
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 @@ abstract class AbstractAspectJAdvisorFactoryTests {
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 @@ abstract class AbstractAspectJAdvisorFactoryTests {
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 @@ abstract class AbstractAspectJAdvisorFactoryTests {
}
@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 @@ abstract class AbstractAspectJAdvisorFactoryTests {
@Around("getAge()")
int preventExecution(ProceedingJoinPoint pjp) {
return 666;
return 42;
}
}
@@ -844,7 +874,7 @@ abstract class AbstractAspectJAdvisorFactoryTests {
@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() {