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

Test status quo for invocation order of all advice types

Browse Source 
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
This commit is contained in:
Sam Brannen
2020-06-05 15:22:27 +02:00
parent 04c8de4e50
commit fbeecf3bf8
7 changed files with 124 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
integration-tests/src/test/java/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests.java
View File

@@ -19,6 +19,7 @@ package org.springframework.aop.config;
import java.util.ArrayList;
import java.util.List;
import org.aspectj.lang.ProceedingJoinPoint;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
@@ -45,14 +46,14 @@ class AopNamespaceHandlerAdviceOrderIntegrationTests {
class AfterAdviceFirstTests {
@Test
void afterAdviceIsInvokedFirst(@Autowired Echo echo, @Autowired EchoAspect aspect) throws Exception {
void afterAdviceIsInvokedFirst(@Autowired Echo echo, @Autowired InvocationTrackingAspect aspect) throws Exception {
assertThat(aspect.invocations).isEmpty();
assertThat(echo.echo(42)).isEqualTo(42);
assertThat(aspect.invocations).containsExactly("after", "after returning");
assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after returning");
aspect.invocations.clear();
assertThatExceptionOfType(Exception.class).isThrownBy(() -> echo.echo(new Exception()));
assertThat(aspect.invocations).containsExactly("after", "after throwing");
assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after throwing");
}
}
@@ -62,14 +63,14 @@ class AopNamespaceHandlerAdviceOrderIntegrationTests {
class AfterAdviceLastTests {
@Test
void afterAdviceIsInvokedLast(@Autowired Echo echo, @Autowired EchoAspect aspect) throws Exception {
void afterAdviceIsInvokedLast(@Autowired Echo echo, @Autowired InvocationTrackingAspect aspect) throws Exception {
assertThat(aspect.invocations).isEmpty();
assertThat(echo.echo(42)).isEqualTo(42);
assertThat(aspect.invocations).containsExactly("after returning", "after");
assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after returning", "after");
aspect.invocations.clear();
assertThatExceptionOfType(Exception.class).isThrownBy(() -> echo.echo(new Exception()));
assertThat(aspect.invocations).containsExactly("after throwing", "after");
assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after throwing", "after");
}
}
@@ -84,18 +85,29 @@ class AopNamespaceHandlerAdviceOrderIntegrationTests {
}
}
static class EchoAspect {
static class InvocationTrackingAspect {
List<String> invocations = new ArrayList<>();
void echo() {
Object around(ProceedingJoinPoint joinPoint) throws Throwable {
invocations.add("around - start");
try {
return joinPoint.proceed();
}
finally {
invocations.add("around - end");
}
}
void succeeded() {
void before() {
invocations.add("before");
}
void afterReturning() {
invocations.add("after returning");
}
void failed() {
void afterThrowing() {
invocations.add("after throwing");
}

69
integration-tests/src/test/java/org/springframework/aop/framework/autoproxy/AspectJAutoProxyAdviceOrderIntegrationTests.java
View File

@@ -19,10 +19,13 @@ package org.springframework.aop.framework.autoproxy;
import java.util.ArrayList;
import java.util.List;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.After;
import org.aspectj.lang.annotation.AfterReturning;
import org.aspectj.lang.annotation.AfterThrowing;
import org.aspectj.lang.annotation.Around;
import org.aspectj.lang.annotation.Aspect;
import org.aspectj.lang.annotation.Before;
import org.aspectj.lang.annotation.Pointcut;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
@@ -47,25 +50,39 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
*/
class AspectJAutoProxyAdviceOrderIntegrationTests {
/**
* {@link After @After} advice declared as first <em>after</em> method in source code.
*/
@Nested
@SpringJUnitConfig(AfterAdviceFirstConfig.class)
@DirtiesContext
class AfterAdviceFirstTests {
@Test
void afterAdviceIsInvokedFirst(@Autowired Echo echo, @Autowired AfterAdviceFirstAspect aspect) throws Exception {
void afterAdviceIsNotInvokedLast(@Autowired Echo echo, @Autowired AfterAdviceFirstAspect aspect) throws Exception {
assertThat(aspect.invocations).isEmpty();
assertThat(echo.echo(42)).isEqualTo(42);
assertThat(aspect.invocations).containsExactly("after", "after returning");
assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after returning");
aspect.invocations.clear();
assertThatExceptionOfType(Exception.class).isThrownBy(
() -> echo.echo(new Exception()));
assertThat(aspect.invocations).containsExactly("after", "after throwing");
assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after throwing");
}
}
/**
* This test class uses {@link AfterAdviceLastAspect} which declares its
* {@link After @After} advice as the last <em>after advice type</em> method
* in its source code.
*
* <p>On Java versions prior to JDK 7, we would have expected the {@code @After}
* advice method to be invoked after {@code @AfterThrowing} and
* {@code @AfterReturning} advice methods due to the AspectJ precedence
* rules implemented in
* {@link org.springframework.aop.aspectj.autoproxy.AspectJPrecedenceComparator}.
*/
@Nested
@SpringJUnitConfig(AfterAdviceLastConfig.class)
@DirtiesContext
@@ -75,14 +92,12 @@ class AspectJAutoProxyAdviceOrderIntegrationTests {
void afterAdviceIsNotInvokedLast(@Autowired Echo echo, @Autowired AfterAdviceLastAspect aspect) throws Exception {
assertThat(aspect.invocations).isEmpty();
assertThat(echo.echo(42)).isEqualTo(42);
// On Java versions prior to JDK 7, we would expect the @After advice to be invoked last.
assertThat(aspect.invocations).containsExactly("after", "after returning");
assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after returning");
aspect.invocations.clear();
assertThatExceptionOfType(Exception.class).isThrownBy(
() -> echo.echo(new Exception()));
// On Java versions prior to JDK 7, we would expect the @After advice to be invoked last.
assertThat(aspect.invocations).containsExactly("after", "after throwing");
assertThat(aspect.invocations).containsExactly("around - start", "before", "around - end", "after", "after throwing");
}
}
@@ -145,14 +160,30 @@ class AspectJAutoProxyAdviceOrderIntegrationTests {
}
@AfterReturning("echo()")
void succeeded() {
void afterReturning() {
invocations.add("after returning");
}
@AfterThrowing("echo()")
void failed() {
void afterThrowing() {
invocations.add("after throwing");
}
@Before("echo()")
void before() {
invocations.add("before");
}
@Around("echo()")
Object around(ProceedingJoinPoint joinPoint) throws Throwable {
invocations.add("around - start");
try {
return joinPoint.proceed();
}
finally {
invocations.add("around - end");
}
}
}
/**
@@ -167,13 +198,29 @@ class AspectJAutoProxyAdviceOrderIntegrationTests {
void echo() {
}
@Around("echo()")
Object around(ProceedingJoinPoint joinPoint) throws Throwable {
invocations.add("around - start");
try {
return joinPoint.proceed();
}
finally {
invocations.add("around - end");
}
}
@Before("echo()")
void before() {
invocations.add("before");
}
@AfterReturning("echo()")
void succeeded() {
void afterReturning() {
invocations.add("after returning");
}
@AfterThrowing("echo()")
void failed() {
void afterThrowing() {
invocations.add("after throwing");
}