This commit extracts a Class.getName() invocation from String
concatenation in AbstractMonitoringInterceptor to avoid an issue
related to profile pollution.
Closes gh-25324
The AspectJPrecedenceComparator was designed to mimic the precedence
order enforced by the AspectJ compiler with regard to multiple 'after'
methods defined within the same aspect whose pointcuts match the same
joinpoint. Specifically, if an aspect declares multiple @After,
@AfterReturning, or @AfterThrowing advice methods whose pointcuts match
the same joinpoint, such 'after' advice methods should be invoked in
the reverse order in which they are declared in the source code.
When the AspectJPrecedenceComparator was introduced in Spring Framework
2.0, it achieved its goal of mimicking the AspectJ compiler since the
JDK at that time (i.e., Java 5) ensured that an invocation of
Class#geDeclaredMethods() returned an array of methods that matched the
order of declaration in the source code. However, Java 7 removed this
guarantee. Consequently, in Java 7 or higher,
AspectJPrecedenceComparator no longer works as it is documented or as
it was designed when sorting advice methods in a single @Aspect class.
Note, however, that AspectJPrecedenceComparator continues to work as
documented and designed when sorting advice configured via the
<aop:aspect> XML namespace element.
PR gh-24673 highlights a use case where AspectJPrecedenceComparator
fails to assign the highest precedence to an @After advice method
declared last in the source code. Note that an @After advice method
with a precedence higher than @AfterReturning and @AfterThrowing advice
methods in the same aspect will effectively be invoked last due to the
try-finally implementation in AspectJAfterAdvice.invoke() which invokes
proceed() in the try-block and invokeAdviceMethod() in the
finally-block.
Since Spring cannot reliably determine the source code declaration
order of annotated advice methods without using ASM to analyze the byte
code, this commit introduces reliable invocation order for advice
methods declared within a single @Aspect. Specifically, the
getAdvisors(...) method in ReflectiveAspectJAdvisorFactory now hard
codes the declarationOrderInAspect to `0` instead of using the index of
the current advice method. This is necessary since the index no longer
has any correlation to the method declaration order in the source code.
The result is that all advice methods discovered via reflection will
now be sorted only according to the precedence rules defined in the
ReflectiveAspectJAdvisorFactory.METHOD_COMPARATOR. Specifically, advice
methods within a single @Aspect will be sorted in the following order
(with @After advice methods effectively invoked after @AfterReturning
and @AfterThrowing advice methods): @Around, @Before, @After,
@AfterReturning, @AfterThrowing.
The modified assertions in AspectJAutoProxyAdviceOrderIntegrationTests
demonstrate the concrete effects of this change.
Closes gh-25186
Prior to this commit we did not have tests in place to verify the status
quo for the invocation order of all advice types when declared within
a single aspect, either via the <aop:aspect> XML namespace or AspectJ
auto-proxy support.
This commit introduces such tests that demonstrate where such ordering
is broken or suboptimal.
The only test for which the advice invocation order is correct or at
least as expected is the afterAdviceTypes() test method in
ReflectiveAspectJAdvisorFactoryTests, where an AOP proxy is hand crafted
using ReflectiveAspectJAdvisorFactory without the use of Spring's
AspectJPrecedenceComparator.
See gh-25186
