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 new file mode 100644 index 00000000000..dd19236ee4a --- /dev/null +++ b/integration-tests/src/test/java/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests.java @@ -0,0 +1,107 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.aop.config; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +/** + * Integration tests for advice invocation order for advice configured via the + * AOP namespace. + * + * @author Sam Brannen + * @since 5.0.18 + * @see org.springframework.aop.framework.autoproxy.AspectJAutoProxyAdviceOrderIntegrationTests + */ +class AopNamespaceHandlerAdviceOrderIntegrationTests { + + @Nested + @SpringJUnitConfig(locations = "AopNamespaceHandlerAdviceOrderIntegrationTests-afterFirst.xml") + @DirtiesContext + class AfterAdviceFirstTests { + + @Test + void afterAdviceIsInvokedFirst(@Autowired Echo echo, @Autowired EchoAspect aspect) throws Exception { + assertThat(aspect.invocations).isEmpty(); + assertThat(echo.echo(42)).isEqualTo(42); + assertThat(aspect.invocations).containsExactly("after", "after returning"); + + aspect.invocations.clear(); + assertThatExceptionOfType(Exception.class).isThrownBy(() -> echo.echo(new Exception())); + assertThat(aspect.invocations).containsExactly("after", "after throwing"); + } + } + + @Nested + @SpringJUnitConfig(locations = "AopNamespaceHandlerAdviceOrderIntegrationTests-afterLast.xml") + @DirtiesContext + class AfterAdviceLastTests { + + @Test + void afterAdviceIsInvokedLast(@Autowired Echo echo, @Autowired EchoAspect aspect) throws Exception { + assertThat(aspect.invocations).isEmpty(); + assertThat(echo.echo(42)).isEqualTo(42); + assertThat(aspect.invocations).containsExactly("after returning", "after"); + + aspect.invocations.clear(); + assertThatExceptionOfType(Exception.class).isThrownBy(() -> echo.echo(new Exception())); + assertThat(aspect.invocations).containsExactly("after throwing", "after"); + } + } + + + static class Echo { + + Object echo(Object obj) throws Exception { + if (obj instanceof Exception) { + throw (Exception) obj; + } + return obj; + } + } + + static class EchoAspect { + + List invocations = new ArrayList<>(); + + void echo() { + } + + void succeeded() { + invocations.add("after returning"); + } + + void failed() { + invocations.add("after throwing"); + } + + void after() { + invocations.add("after"); + } + } + +} 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 new file mode 100644 index 00000000000..67563c19505 --- /dev/null +++ b/integration-tests/src/test/java/org/springframework/aop/framework/autoproxy/AspectJAutoProxyAdviceOrderIntegrationTests.java @@ -0,0 +1,186 @@ +/* + * 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.aop.framework.autoproxy; + +import java.util.ArrayList; +import java.util.List; + +import org.aspectj.lang.annotation.After; +import org.aspectj.lang.annotation.AfterReturning; +import org.aspectj.lang.annotation.AfterThrowing; +import org.aspectj.lang.annotation.Aspect; +import org.aspectj.lang.annotation.Pointcut; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +/** + * Integration tests for advice invocation order for advice configured via + * AspectJ auto-proxy support. + * + * @author Sam Brannen + * @since 5.0.18 + * @see org.springframework.aop.config.AopNamespaceHandlerAdviceOrderIntegrationTests + */ +class AspectJAutoProxyAdviceOrderIntegrationTests { + + @Nested + @SpringJUnitConfig(AfterAdviceFirstConfig.class) + @DirtiesContext + class AfterAdviceFirstTests { + + @Test + void afterAdviceIsInvokedFirst(@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"); + + aspect.invocations.clear(); + assertThatExceptionOfType(Exception.class).isThrownBy( + () -> echo.echo(new Exception())); + assertThat(aspect.invocations).containsExactly("after", "after throwing"); + } + } + + + @Nested + @SpringJUnitConfig(AfterAdviceLastConfig.class) + @DirtiesContext + class AfterAdviceLastTests { + + @Test + 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"); + + 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"); + } + } + + + @Configuration + @EnableAspectJAutoProxy(proxyTargetClass = true) + static class AfterAdviceFirstConfig { + + @Bean + AfterAdviceFirstAspect echoAspect() { + return new AfterAdviceFirstAspect(); + } + + @Bean + Echo echo() { + return new Echo(); + } + } + + @Configuration + @EnableAspectJAutoProxy(proxyTargetClass = true) + static class AfterAdviceLastConfig { + + @Bean + AfterAdviceLastAspect echoAspect() { + return new AfterAdviceLastAspect(); + } + + @Bean + Echo echo() { + return new Echo(); + } + } + + static class Echo { + + Object echo(Object obj) throws Exception { + if (obj instanceof Exception) { + throw (Exception) obj; + } + return obj; + } + } + + /** + * {@link After @After} advice declared as first after method in source code. + */ + @Aspect + static class AfterAdviceFirstAspect { + + List invocations = new ArrayList<>(); + + @Pointcut("execution(* echo(*))") + void echo() { + } + + @After("echo()") + void after() { + invocations.add("after"); + } + + @AfterReturning("echo()") + void succeeded() { + invocations.add("after returning"); + } + + @AfterThrowing("echo()") + void failed() { + invocations.add("after throwing"); + } + } + + /** + * {@link After @After} advice declared as last after method in source code. + */ + @Aspect + static class AfterAdviceLastAspect { + + List invocations = new ArrayList<>(); + + @Pointcut("execution(* echo(*))") + void echo() { + } + + @AfterReturning("echo()") + void succeeded() { + invocations.add("after returning"); + } + + @AfterThrowing("echo()") + void failed() { + invocations.add("after throwing"); + } + + @After("echo()") + void after() { + invocations.add("after"); + } + } + +} 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 new file mode 100644 index 00000000000..6293b219827 --- /dev/null +++ b/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterFirst.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + 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 new file mode 100644 index 00000000000..376896533ab --- /dev/null +++ b/integration-tests/src/test/resources/org/springframework/aop/config/AopNamespaceHandlerAdviceOrderIntegrationTests-afterLast.xml @@ -0,0 +1,21 @@ + + + + + + + + + + + + + + + + + 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 88bfdfb30b9..67a7f0aa789 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-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 java.lang.annotation.RetentionPolicy; import java.lang.reflect.Method; import java.lang.reflect.UndeclaredThrowableException; import java.rmi.RemoteException; +import java.util.ArrayList; import java.util.LinkedList; import java.util.List; @@ -63,14 +64,16 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException import static org.assertj.core.api.Assertions.assertThatIllegalStateException; /** - * Abstract tests for AspectJAdvisorFactory. - * See subclasses for tests of concrete factories. + * Abstract tests for {@link AspectJAdvisorFactory} implementations. + * + *

See subclasses for tests of concrete factories. * * @author Rod Johnson * @author Chris Beams * @author Phillip Webb + * @author Sam Brannen */ -public abstract class AbstractAspectJAdvisorFactoryTests { +abstract class AbstractAspectJAdvisorFactoryTests { /** * To be overridden by concrete test subclasses. @@ -80,7 +83,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Test - public void testRejectsPerCflowAspect() { + void rejectsPerCflowAspect() { assertThatExceptionOfType(AopConfigException.class).isThrownBy(() -> getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new PerCflowAspect(), "someBean"))) @@ -88,7 +91,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testRejectsPerCflowBelowAspect() { + void rejectsPerCflowBelowAspect() { assertThatExceptionOfType(AopConfigException.class).isThrownBy(() -> getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new PerCflowBelowAspect(), "someBean"))) @@ -96,7 +99,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testPerTargetAspect() throws SecurityException, NoSuchMethodException { + void perTargetAspect() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -128,7 +131,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testMultiplePerTargetAspects() throws SecurityException, NoSuchMethodException { + void multiplePerTargetAspects() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -156,7 +159,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testMultiplePerTargetAspectsWithOrderAnnotation() throws SecurityException, NoSuchMethodException { + void multiplePerTargetAspectsWithOrderAnnotation() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -182,7 +185,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testPerThisAspect() throws SecurityException, NoSuchMethodException { + void perThisAspect() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -218,7 +221,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testPerTypeWithinAspect() throws SecurityException, NoSuchMethodException { + void perTypeWithinAspect() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -259,22 +262,22 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testNamedPointcutAspectWithFQN() { - testNamedPointcuts(new NamedPointcutAspectWithFQN()); + void namedPointcutAspectWithFQN() { + namedPointcuts(new NamedPointcutAspectWithFQN()); } @Test - public void testNamedPointcutAspectWithoutFQN() { - testNamedPointcuts(new NamedPointcutAspectWithoutFQN()); + void namedPointcutAspectWithoutFQN() { + namedPointcuts(new NamedPointcutAspectWithoutFQN()); } @Test - public void testNamedPointcutFromAspectLibrary() { - testNamedPointcuts(new NamedPointcutAspectFromLibrary()); + void namedPointcutFromAspectLibrary() { + namedPointcuts(new NamedPointcutAspectFromLibrary()); } @Test - public void testNamedPointcutFromAspectLibraryWithBinding() { + void namedPointcutFromAspectLibraryWithBinding() { TestBean target = new TestBean(); ITestBean itb = (ITestBean) createProxy(target, getFixture().getAdvisors(new SingletonMetadataAwareAspectInstanceFactory( @@ -285,7 +288,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { assertThat(target.getAge()).isEqualTo(20); } - private void testNamedPointcuts(Object aspectInstance) { + private void namedPointcuts(Object aspectInstance) { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -297,7 +300,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testBindingWithSingleArg() { + void bindingWithSingleArg() { TestBean target = new TestBean(); ITestBean itb = (ITestBean) createProxy(target, getFixture().getAdvisors( @@ -309,7 +312,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testBindingWithMultipleArgsDifferentlyOrdered() { + void bindingWithMultipleArgsDifferentlyOrdered() { ManyValuedArgs target = new ManyValuedArgs(); ManyValuedArgs mva = (ManyValuedArgs) createProxy(target, getFixture().getAdvisors( @@ -329,7 +332,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { * In this case the introduction will be made. */ @Test - public void testIntroductionOnTargetNotImplementingInterface() { + void introductionOnTargetNotImplementingInterface() { NotLockable notLockableTarget = new NotLockable(); assertThat(notLockableTarget instanceof Lockable).isFalse(); NotLockable notLockable1 = (NotLockable) createProxy(notLockableTarget, @@ -358,7 +361,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testIntroductionAdvisorExcludedFromTargetImplementingInterface() { + void introductionAdvisorExcludedFromTargetImplementingInterface() { assertThat(AopUtils.findAdvisorsThatCanApply( getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new MakeLockable(), "someBean")), @@ -368,7 +371,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testIntroductionOnTargetImplementingInterface() { + void introductionOnTargetImplementingInterface() { CannotBeUnlocked target = new CannotBeUnlocked(); Lockable proxy = (Lockable) createProxy(target, // Ensure that we exclude @@ -388,7 +391,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testIntroductionOnTargetExcludedByTypePattern() { + void introductionOnTargetExcludedByTypePattern() { LinkedList target = new LinkedList<>(); List proxy = (List) createProxy(target, AopUtils.findAdvisorsThatCanApply( @@ -400,7 +403,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testIntroductionBasedOnAnnotationMatch_SPR5307() { + void introductionBasedOnAnnotationMatch_SPR5307() { AnnotatedTarget target = new AnnotatedTargetImpl(); List advisors = getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new MakeAnnotatedTypeModifiable(), "someBean")); @@ -414,7 +417,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { // TODO: Why does this test fail? It hasn't been run before, so it maybe never actually passed... @Test @Disabled - public void testIntroductionWithArgumentBinding() { + void introductionWithArgumentBinding() { TestBean target = new TestBean(); List advisors = getFixture().getAdvisors( @@ -448,7 +451,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testAspectMethodThrowsExceptionLegalOnSignature() { + void aspectMethodThrowsExceptionLegalOnSignature() { TestBean target = new TestBean(); UnsupportedOperationException expectedException = new UnsupportedOperationException(); List advisors = getFixture().getAdvisors( @@ -462,7 +465,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { // TODO document this behaviour. // Is it different AspectJ behaviour, at least for checked exceptions? @Test - public void testAspectMethodThrowsExceptionIllegalOnSignature() { + void aspectMethodThrowsExceptionIllegalOnSignature() { TestBean target = new TestBean(); RemoteException expectedException = new RemoteException(); List advisors = getFixture().getAdvisors( @@ -491,7 +494,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testTwoAdvicesOnOneAspect() { + void twoAdvicesOnOneAspect() { TestBean target = new TestBean(); TwoAdviceAspect twoAdviceAspect = new TwoAdviceAspect(); List advisors = getFixture().getAdvisors( @@ -506,25 +509,23 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testAfterAdviceTypes() throws Exception { - Echo target = new Echo(); - ExceptionHandling afterReturningAspect = new ExceptionHandling(); + void afterAdviceTypes() throws Exception { + ExceptionHandling aspect = new ExceptionHandling(); List advisors = getFixture().getAdvisors( - new SingletonMetadataAwareAspectInstanceFactory(afterReturningAspect, "someBean")); - Echo echo = (Echo) createProxy(target, advisors, Echo.class); - assertThat(afterReturningAspect.successCount).isEqualTo(0); - assertThat(echo.echo("")).isEqualTo(""); - assertThat(afterReturningAspect.successCount).isEqualTo(1); - assertThat(afterReturningAspect.failureCount).isEqualTo(0); - assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(() -> - echo.echo(new FileNotFoundException())); - assertThat(afterReturningAspect.successCount).isEqualTo(1); - assertThat(afterReturningAspect.failureCount).isEqualTo(1); - assertThat(afterReturningAspect.afterCount).isEqualTo(afterReturningAspect.failureCount + afterReturningAspect.successCount); + 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"); + + aspect.invocations.clear(); + assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(() -> echo.echo(new FileNotFoundException())); + assertThat(aspect.invocations).containsExactly("after throwing", "after"); } @Test - public void testFailureWithoutExplicitDeclarePrecedence() { + void failureWithoutExplicitDeclarePrecedence() { TestBean target = new TestBean(); MetadataAwareAspectInstanceFactory aspectInstanceFactory = new SingletonMetadataAwareAspectInstanceFactory( new NoDeclarePrecedenceShouldFail(), "someBean"); @@ -534,7 +535,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { } @Test - public void testDeclarePrecedenceNotSupported() { + void declarePrecedenceNotSupported() { TestBean target = new TestBean(); assertThatIllegalArgumentException().isThrownBy(() -> { MetadataAwareAspectInstanceFactory aspectInstanceFactory = new SingletonMetadataAwareAspectInstanceFactory( @@ -545,28 +546,28 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Aspect("percflow(execution(* *(..)))") - public static class PerCflowAspect { + static class PerCflowAspect { } @Aspect("percflowbelow(execution(* *(..)))") - public static class PerCflowBelowAspect { + static class PerCflowBelowAspect { } @Aspect("pertarget(execution(* *.getSpouse()))") @Order(10) - public static class PerTargetAspectWithOrderAnnotation10 { + static class PerTargetAspectWithOrderAnnotation10 { - public int count; + int count; @Around("execution(int *.getAge())") - public int returnCountAsAge() { + int returnCountAsAge() { return count++; } @Before("execution(void *.set*(int))") - public void countSetter() { + void countSetter() { ++count; } } @@ -574,34 +575,34 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Aspect("pertarget(execution(* *.getSpouse()))") @Order(5) - public static class PerTargetAspectWithOrderAnnotation5 { + static class PerTargetAspectWithOrderAnnotation5 { - public int count; + int count; @Around("execution(int *.getAge())") - public int returnCountAsAge() { + int returnCountAsAge() { return count++; } @Before("execution(void *.set*(int))") - public void countSetter() { + void countSetter() { ++count; } } @Aspect("pertypewithin(org.springframework.beans.testfixture.beans.IOther+)") - public static class PerTypeWithinAspect { + static class PerTypeWithinAspect { - public int count; + int count; @Around("execution(int *.getAge())") - public int returnCountAsAge() { + int returnCountAsAge() { return count++; } @Before("execution(void *.*(..))") - public void countAnythingVoid() { + void countAnythingVoid() { ++count; } } @@ -611,7 +612,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { private int count; - public int getInstantiationCount() { + int getInstantiationCount() { return this.count; } @@ -644,97 +645,96 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Aspect - public static class NamedPointcutAspectWithFQN { + static class NamedPointcutAspectWithFQN { @SuppressWarnings("unused") private ITestBean fieldThatShouldBeIgnoredBySpringAtAspectJProcessing = new TestBean(); @Pointcut("execution(* getAge())") - public void getAge() { + void getAge() { } @Around("org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.NamedPointcutAspectWithFQN.getAge()") - public int changeReturnValue(ProceedingJoinPoint pjp) { + int changeReturnValue(ProceedingJoinPoint pjp) { return -1; } } @Aspect - public static class NamedPointcutAspectWithoutFQN { + static class NamedPointcutAspectWithoutFQN { @Pointcut("execution(* getAge())") - public void getAge() { + void getAge() { } @Around("getAge()") - public int changeReturnValue(ProceedingJoinPoint pjp) { + int changeReturnValue(ProceedingJoinPoint pjp) { return -1; } } @Aspect - public static class NamedPointcutAspectFromLibrary { + static class NamedPointcutAspectFromLibrary { @Around("org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.Library.propertyAccess()") - public int changeReturnType(ProceedingJoinPoint pjp) { + int changeReturnType(ProceedingJoinPoint pjp) { return -1; } @Around(value="org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.Library.integerArgOperation(x)", argNames="x") - public void doubleArg(ProceedingJoinPoint pjp, int x) throws Throwable { + void doubleArg(ProceedingJoinPoint pjp, int x) throws Throwable { pjp.proceed(new Object[] {x*2}); } } @Aspect - public static class Library { + static class Library { @Pointcut("execution(!void get*())") - public void propertyAccess() {} + void propertyAccess() {} @Pointcut("execution(* *(..)) && args(i)") - public void integerArgOperation(int i) {} - + void integerArgOperation(int i) {} } @Aspect - public static class NamedPointcutAspectFromLibraryWithBinding { + static class NamedPointcutAspectFromLibraryWithBinding { @Around(value="org.springframework.aop.aspectj.annotation.AbstractAspectJAdvisorFactoryTests.Library.integerArgOperation(x)", argNames="x") - public void doubleArg(ProceedingJoinPoint pjp, int x) throws Throwable { + void doubleArg(ProceedingJoinPoint pjp, int x) throws Throwable { pjp.proceed(new Object[] {x*2}); } } @Aspect - public static class BindingAspectWithSingleArg { + static class BindingAspectWithSingleArg { @Pointcut(value="args(a)", argNames="a") - public void setAge(int a) {} + void setAge(int a) {} @Around(value="setAge(age)",argNames="age") // @ArgNames({"age"}) // AMC needs more work here? ignoring pjp arg... ok?? // argNames should be supported in Around as it is in Pointcut - public void changeReturnType(ProceedingJoinPoint pjp, int age) throws Throwable { + void changeReturnType(ProceedingJoinPoint pjp, int age) throws Throwable { pjp.proceed(new Object[] {age*2}); } } @Aspect - public static class ManyValuedArgs { + static class ManyValuedArgs { - public String mungeArgs(String a, int b, int c, String d, StringBuffer e) { + String mungeArgs(String a, int b, int c, String d, StringBuffer e) { return a + b + c + d + e; } @Around(value="execution(String mungeArgs(..)) && args(a, b, c, d, e)", argNames="b,c,d,e,a") - public String reverseAdvice(ProceedingJoinPoint pjp, int b, int c, String d, StringBuffer e, String a) throws Throwable { + String reverseAdvice(ProceedingJoinPoint pjp, int b, int c, String d, StringBuffer e, String a) throws Throwable { assertThat(pjp.proceed()).isEqualTo(a + b+ c+ d+ e); return a + b + c + d + e; } @@ -742,24 +742,24 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Aspect - public static class ExceptionAspect { + static class ExceptionAspect { private final Exception ex; - public ExceptionAspect(Exception ex) { + ExceptionAspect(Exception ex) { this.ex = ex; } @Before("execution(* getAge())") - public void throwException() throws Exception { + void throwException() throws Exception { throw ex; } } - public static class Echo { + static class Echo { - public Object echo(Object o) throws Exception { + Object echo(Object o) throws Exception { if (o instanceof Exception) { throw (Exception) o; } @@ -769,45 +769,46 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Aspect - public static class ExceptionHandling { + static class ExceptionHandling { - public int successCount; + List invocations = new ArrayList<>(); - public int failureCount; - public int afterCount; + @Pointcut("execution(* echo(*))") + void echo() { + } - @AfterReturning("execution(* echo(*))") - public void succeeded() { - ++successCount; + @AfterReturning("echo()") + void succeeded() { + invocations.add("after returning"); } - @AfterThrowing("execution(* echo(*))") - public void failed() { - ++failureCount; + @AfterThrowing("echo()") + void failed() { + invocations.add("after throwing"); } - @After("execution(* echo(*))") - public void invoked() { - ++afterCount; + @After("echo()") + void after() { + invocations.add("after"); } } @Aspect - public static class NoDeclarePrecedenceShouldFail { + static class NoDeclarePrecedenceShouldFail { @Pointcut("execution(int *.getAge())") - public void getAge() { + void getAge() { } @Before("getAge()") - public void blowUpButDoesntMatterBecauseAroundAdviceWontLetThisBeInvoked() { + void blowUpButDoesntMatterBecauseAroundAdviceWontLetThisBeInvoked() { throw new IllegalStateException(); } @Around("getAge()") - public int preventExecution(ProceedingJoinPoint pjp) { + int preventExecution(ProceedingJoinPoint pjp) { return 666; } } @@ -815,19 +816,19 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Aspect @DeclarePrecedence("test..*") - public static class DeclarePrecedenceShouldSucceed { + static class DeclarePrecedenceShouldSucceed { @Pointcut("execution(int *.getAge())") - public void getAge() { + void getAge() { } @Before("getAge()") - public void blowUpButDoesntMatterBecauseAroundAdviceWontLetThisBeInvoked() { + void blowUpButDoesntMatterBecauseAroundAdviceWontLetThisBeInvoked() { throw new IllegalStateException(); } @Around("getAge()") - public int preventExecution(ProceedingJoinPoint pjp) { + int preventExecution(ProceedingJoinPoint pjp) { return 666; } } @@ -845,12 +846,12 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Aspect abstract class AbstractMakeModifiable { - public interface MutableModifiable extends Modifiable { + interface MutableModifiable extends Modifiable { void markDirty(); } - public static class ModifiableImpl implements MutableModifiable { + static class ModifiableImpl implements MutableModifiable { private boolean modified; @@ -871,7 +872,7 @@ abstract class AbstractMakeModifiable { } @Before(value="execution(void set*(*)) && this(modifiable) && args(newValue)", argNames="modifiable,newValue") - public void recordModificationIfSetterArgumentDiffersFromOldValue( + void recordModificationIfSetterArgumentDiffersFromOldValue( JoinPoint jp, MutableModifiable mixin, Object newValue) { /* @@ -933,7 +934,7 @@ class MakeITestBeanModifiable extends AbstractMakeModifiable { @DeclareParents(value = "org.springframework.beans.testfixture.beans.ITestBean+", defaultImpl=ModifiableImpl.class) - public static MutableModifiable mixin; + static MutableModifiable mixin; } @@ -948,7 +949,7 @@ class MakeAnnotatedTypeModifiable extends AbstractMakeModifiable { @DeclareParents(value = "(@org.springframework.aop.aspectj.annotation.Measured *)", defaultImpl = DefaultLockable.class) - public static Lockable mixin; + static Lockable mixin; } @@ -960,10 +961,10 @@ class MakeAnnotatedTypeModifiable extends AbstractMakeModifiable { class MakeLockable { @DeclareParents(value = "org.springframework..*", defaultImpl = DefaultLockable.class) - public static Lockable mixin; + static Lockable mixin; @Before(value="execution(void set*(*)) && this(mixin)", argNames="mixin") - public void checkNotLocked( Lockable mixin) { + void checkNotLocked( Lockable mixin) { // Can also obtain the mixin (this) this way //Lockable mixin = (Lockable) jp.getThis(); if (mixin.locked()) { @@ -1032,11 +1033,11 @@ class NotLockable { private int intValue; - public int getIntValue() { + int getIntValue() { return intValue; } - public void setIntValue(int intValue) { + void setIntValue(int intValue) { this.intValue = intValue; } @@ -1046,19 +1047,19 @@ class NotLockable { @Aspect("perthis(execution(* *.getSpouse()))") class PerThisAspect { - public int count; + int count; // Just to check that this doesn't cause problems with introduction processing @SuppressWarnings("unused") private ITestBean fieldThatShouldBeIgnoredBySpringAtAspectJProcessing = new TestBean(); @Around("execution(int *.getAge())") - public int returnCountAsAge() { + int returnCountAsAge() { return count++; } @Before("execution(void *.set*(int))") - public void countSetter() { + void countSetter() { ++count; } diff --git a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactoryTests.java b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactoryTests.java index 3eac9fac522..d649ea01964 100644 --- a/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactoryTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 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. @@ -17,13 +17,14 @@ package org.springframework.aop.aspectj.annotation; /** - * Tests for ReflectiveAtAspectJAdvisorFactory. - * Tests are inherited: we only set the test fixture here. + * Tests for {@link ReflectiveAspectJAdvisorFactory}. + * + *

Tests are inherited: we only set the test fixture here. * * @author Rod Johnson * @since 2.0 */ -public class ReflectiveAspectJAdvisorFactoryTests extends AbstractAspectJAdvisorFactoryTests { +class ReflectiveAspectJAdvisorFactoryTests extends AbstractAspectJAdvisorFactoryTests { @Override protected AspectJAdvisorFactory getFixture() {