diff --git a/integration-tests/src/test/java/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests.java b/integration-tests/src/test/java/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests.java index d4a659f824f8..da32ff120181 100644 --- a/integration-tests/src/test/java/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests.java +++ b/integration-tests/src/test/java/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests.java @@ -19,6 +19,7 @@ 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 @@ void afterAdviceIsInvokedFirst(@Autowired Echo echo, @Autowired EchoAspect aspec 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 @@ Object echo(Object obj) throws Exception { } } - static class EchoAspect { + static class InvocationTrackingAspect { List 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 before() { + invocations.add("before"); } - void succeeded() { + void afterReturning() { invocations.add("after returning"); } - void failed() { + void afterThrowing() { invocations.add("after throwing"); } 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 f088cc102469..6e67a881fad0 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 @@ -19,10 +19,13 @@ 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 @@ */ class AspectJAutoProxyAdviceOrderIntegrationTests { + /** + * {@link After @After} advice declared as first after 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 after advice type method + * 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 + * {@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 AfterAdviceLastTests { 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 @@ void after() { } @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 @@ static class AfterAdviceLastAspect { 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"); } diff --git a/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterFirst.xml b/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterFirst.xml index 6293b2198270..fd98a0c1b407 100644 --- a/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterFirst.xml +++ b/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterFirst.xml @@ -7,14 +7,16 @@ - + - + + + - - + + diff --git a/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterLast.xml b/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterLast.xml index 376896533abd..bcab0df678a4 100644 --- a/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterLast.xml +++ b/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterLast.xml @@ -7,13 +7,15 @@ - + - + - - + + + + 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 67a7f0aa7890..c7bad575019e 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 @@ -455,7 +455,7 @@ void aspectMethodThrowsExceptionLegalOnSignature() { TestBean target = new TestBean(); UnsupportedOperationException expectedException = new UnsupportedOperationException(); List advisors = getFixture().getAdvisors( - new SingletonMetadataAwareAspectInstanceFactory(new ExceptionAspect(expectedException), "someBean")); + new SingletonMetadataAwareAspectInstanceFactory(new ExceptionThrowingAspect(expectedException), "someBean")); assertThat(advisors.size()).as("One advice method was found").isEqualTo(1); ITestBean itb = (ITestBean) createProxy(target, advisors, ITestBean.class); assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy( @@ -469,7 +469,7 @@ void aspectMethodThrowsExceptionIllegalOnSignature() { TestBean target = new TestBean(); RemoteException expectedException = new RemoteException(); List advisors = getFixture().getAdvisors( - new SingletonMetadataAwareAspectInstanceFactory(new ExceptionAspect(expectedException), "someBean")); + new SingletonMetadataAwareAspectInstanceFactory(new ExceptionThrowingAspect(expectedException), "someBean")); assertThat(advisors.size()).as("One advice method was found").isEqualTo(1); ITestBean itb = (ITestBean) createProxy(target, advisors, ITestBean.class); assertThatExceptionOfType(UndeclaredThrowableException.class).isThrownBy( @@ -510,18 +510,18 @@ void twoAdvicesOnOneAspect() { @Test void afterAdviceTypes() throws Exception { - ExceptionHandling aspect = new ExceptionHandling(); + InvocationTrackingAspect aspect = new InvocationTrackingAspect(); List advisors = getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(aspect, "exceptionHandlingAspect")); Echo echo = (Echo) createProxy(new Echo(), advisors, Echo.class); assertThat(aspect.invocations).isEmpty(); assertThat(echo.echo(42)).isEqualTo(42); - assertThat(aspect.invocations).containsExactly("after returning", "after"); + assertThat(aspect.invocations).containsExactly("around - start", "before", "after returning", "after", "around - end"); aspect.invocations.clear(); assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(() -> echo.echo(new FileNotFoundException())); - assertThat(aspect.invocations).containsExactly("after throwing", "after"); + assertThat(aspect.invocations).containsExactly("around - start", "before", "after throwing", "after", "around - end"); } @Test @@ -742,11 +742,11 @@ String reverseAdvice(ProceedingJoinPoint pjp, int b, int c, String d, StringBuff @Aspect - static class ExceptionAspect { + static class ExceptionThrowingAspect { private final Exception ex; - ExceptionAspect(Exception ex) { + ExceptionThrowingAspect(Exception ex) { this.ex = ex; } @@ -769,7 +769,7 @@ Object echo(Object o) throws Exception { @Aspect - static class ExceptionHandling { + private static class InvocationTrackingAspect { List invocations = new ArrayList<>(); @@ -778,13 +778,29 @@ static class ExceptionHandling { 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"); } diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java index b944da483e0b..806f30587bbe 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectJPointcutAdvisorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -22,6 +22,7 @@ import org.springframework.aop.Pointcut; import org.springframework.aop.aspectj.AspectJExpressionPointcut; import org.springframework.aop.aspectj.AspectJExpressionPointcutTests; +import org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.ExceptionThrowingAspect; import org.springframework.aop.framework.AopConfigException; import org.springframework.beans.testfixture.beans.TestBean; @@ -44,7 +45,7 @@ public void testSingleton() throws SecurityException, NoSuchMethodException { InstantiationModelAwarePointcutAdvisorImpl ajpa = new InstantiationModelAwarePointcutAdvisorImpl( ajexp, TestBean.class.getMethod("getAge"), af, - new SingletonMetadataAwareAspectInstanceFactory(new AbstractAspectJAdvisorFactoryTests.ExceptionAspect(null), "someBean"), + new SingletonMetadataAwareAspectInstanceFactory(new ExceptionThrowingAspect(null), "someBean"), 1, "someBean"); assertThat(ajpa.getAspectMetadata().getPerClausePointcut()).isSameAs(Pointcut.TRUE); diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java index 55fa481ecea7..637baa2450a8 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectMetadataTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 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. @@ -22,7 +22,7 @@ import org.springframework.aop.Pointcut; import org.springframework.aop.aspectj.AspectJExpressionPointcut; -import org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.ExceptionAspect; +import org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.ExceptionThrowingAspect; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -42,7 +42,7 @@ void notAnAspect() { @Test void singletonAspect() { - AspectMetadata am = new AspectMetadata(ExceptionAspect.class, "someBean"); + AspectMetadata am = new AspectMetadata(ExceptionThrowingAspect.class, "someBean"); assertThat(am.isPerThisOrPerTarget()).isFalse(); assertThat(am.getPerClausePointcut()).isSameAs(Pointcut.TRUE); assertThat(am.getAjType().getPerClause().getKind()).isEqualTo(PerClauseKind.SINGLETON);