From 70b3de4afecd58ca248c872de7aa8bcfcfbe72c8 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Thu, 4 Jun 2020 14:07:35 +0200 Subject: [PATCH] Polish AspectJAdvisorFactory tests --- .../AbstractAspectJAdvisorFactoryTests.java | 191 +++++++++--------- .../ReflectiveAspectJAdvisorFactoryTests.java | 9 +- 2 files changed, 101 insertions(+), 99 deletions(-) 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 88bfdfb30b94..8b9675c8afe8 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. @@ -63,14 +63,16 @@ 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 +82,7 @@ public abstract class AbstractAspectJAdvisorFactoryTests { @Test - public void testRejectsPerCflowAspect() { + void rejectsPerCflowAspect() { assertThatExceptionOfType(AopConfigException.class).isThrownBy(() -> getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new PerCflowAspect(), "someBean"))) @@ -88,7 +90,7 @@ public void testRejectsPerCflowAspect() { } @Test - public void testRejectsPerCflowBelowAspect() { + void rejectsPerCflowBelowAspect() { assertThatExceptionOfType(AopConfigException.class).isThrownBy(() -> getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new PerCflowBelowAspect(), "someBean"))) @@ -96,7 +98,7 @@ public void testRejectsPerCflowBelowAspect() { } @Test - public void testPerTargetAspect() throws SecurityException, NoSuchMethodException { + void perTargetAspect() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -128,7 +130,7 @@ public void testPerTargetAspect() throws SecurityException, NoSuchMethodExceptio } @Test - public void testMultiplePerTargetAspects() throws SecurityException, NoSuchMethodException { + void multiplePerTargetAspects() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -156,7 +158,7 @@ public void testMultiplePerTargetAspects() throws SecurityException, NoSuchMetho } @Test - public void testMultiplePerTargetAspectsWithOrderAnnotation() throws SecurityException, NoSuchMethodException { + void multiplePerTargetAspectsWithOrderAnnotation() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -182,7 +184,7 @@ public void testMultiplePerTargetAspectsWithOrderAnnotation() throws SecurityExc } @Test - public void testPerThisAspect() throws SecurityException, NoSuchMethodException { + void perThisAspect() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -218,7 +220,7 @@ public void testPerThisAspect() throws SecurityException, NoSuchMethodException } @Test - public void testPerTypeWithinAspect() throws SecurityException, NoSuchMethodException { + void perTypeWithinAspect() throws SecurityException, NoSuchMethodException { TestBean target = new TestBean(); int realAge = 65; target.setAge(realAge); @@ -259,22 +261,22 @@ public void testPerTypeWithinAspect() throws SecurityException, NoSuchMethodExce } @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 +287,7 @@ public void testNamedPointcutFromAspectLibraryWithBinding() { 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 +299,7 @@ private void testNamedPointcuts(Object aspectInstance) { } @Test - public void testBindingWithSingleArg() { + void bindingWithSingleArg() { TestBean target = new TestBean(); ITestBean itb = (ITestBean) createProxy(target, getFixture().getAdvisors( @@ -309,7 +311,7 @@ public void testBindingWithSingleArg() { } @Test - public void testBindingWithMultipleArgsDifferentlyOrdered() { + void bindingWithMultipleArgsDifferentlyOrdered() { ManyValuedArgs target = new ManyValuedArgs(); ManyValuedArgs mva = (ManyValuedArgs) createProxy(target, getFixture().getAdvisors( @@ -329,7 +331,7 @@ public void testBindingWithMultipleArgsDifferentlyOrdered() { * 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 +360,7 @@ public void testIntroductionOnTargetNotImplementingInterface() { } @Test - public void testIntroductionAdvisorExcludedFromTargetImplementingInterface() { + void introductionAdvisorExcludedFromTargetImplementingInterface() { assertThat(AopUtils.findAdvisorsThatCanApply( getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new MakeLockable(), "someBean")), @@ -368,7 +370,7 @@ public void testIntroductionAdvisorExcludedFromTargetImplementingInterface() { } @Test - public void testIntroductionOnTargetImplementingInterface() { + void introductionOnTargetImplementingInterface() { CannotBeUnlocked target = new CannotBeUnlocked(); Lockable proxy = (Lockable) createProxy(target, // Ensure that we exclude @@ -388,7 +390,7 @@ public void testIntroductionOnTargetImplementingInterface() { } @Test - public void testIntroductionOnTargetExcludedByTypePattern() { + void introductionOnTargetExcludedByTypePattern() { LinkedList target = new LinkedList<>(); List proxy = (List) createProxy(target, AopUtils.findAdvisorsThatCanApply( @@ -400,7 +402,7 @@ public void testIntroductionOnTargetExcludedByTypePattern() { } @Test - public void testIntroductionBasedOnAnnotationMatch_SPR5307() { + void introductionBasedOnAnnotationMatch_SPR5307() { AnnotatedTarget target = new AnnotatedTargetImpl(); List advisors = getFixture().getAdvisors( new SingletonMetadataAwareAspectInstanceFactory(new MakeAnnotatedTypeModifiable(), "someBean")); @@ -414,7 +416,7 @@ public void testIntroductionBasedOnAnnotationMatch_SPR5307() { // 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 +450,7 @@ public void testIntroductionWithArgumentBinding() { } @Test - public void testAspectMethodThrowsExceptionLegalOnSignature() { + void aspectMethodThrowsExceptionLegalOnSignature() { TestBean target = new TestBean(); UnsupportedOperationException expectedException = new UnsupportedOperationException(); List advisors = getFixture().getAdvisors( @@ -462,7 +464,7 @@ public void testAspectMethodThrowsExceptionLegalOnSignature() { // 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 +493,7 @@ protected Object createProxy(Object target, List advisors, Class... } @Test - public void testTwoAdvicesOnOneAspect() { + void twoAdvicesOnOneAspect() { TestBean target = new TestBean(); TwoAdviceAspect twoAdviceAspect = new TwoAdviceAspect(); List advisors = getFixture().getAdvisors( @@ -506,7 +508,7 @@ public void testTwoAdvicesOnOneAspect() { } @Test - public void testAfterAdviceTypes() throws Exception { + void afterAdviceTypes() throws Exception { Echo target = new Echo(); ExceptionHandling afterReturningAspect = new ExceptionHandling(); List advisors = getFixture().getAdvisors( @@ -524,7 +526,7 @@ public void testAfterAdviceTypes() throws Exception { } @Test - public void testFailureWithoutExplicitDeclarePrecedence() { + void failureWithoutExplicitDeclarePrecedence() { TestBean target = new TestBean(); MetadataAwareAspectInstanceFactory aspectInstanceFactory = new SingletonMetadataAwareAspectInstanceFactory( new NoDeclarePrecedenceShouldFail(), "someBean"); @@ -534,7 +536,7 @@ public void testFailureWithoutExplicitDeclarePrecedence() { } @Test - public void testDeclarePrecedenceNotSupported() { + void declarePrecedenceNotSupported() { TestBean target = new TestBean(); assertThatIllegalArgumentException().isThrownBy(() -> { MetadataAwareAspectInstanceFactory aspectInstanceFactory = new SingletonMetadataAwareAspectInstanceFactory( @@ -545,28 +547,28 @@ public void testDeclarePrecedenceNotSupported() { @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 +576,34 @@ public void countSetter() { @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 +613,7 @@ private class PerTypeWithinAspectInstanceFactory implements MetadataAwareAspectI private int count; - public int getInstantiationCount() { + int getInstantiationCount() { return this.count; } @@ -644,97 +646,96 @@ public int getOrder() { @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 +743,24 @@ public String reverseAdvice(ProceedingJoinPoint pjp, int b, int c, String d, Str @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,7 +770,7 @@ public Object echo(Object o) throws Exception { @Aspect - public static class ExceptionHandling { + static class ExceptionHandling { public int successCount; @@ -795,19 +796,19 @@ public void invoked() { @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 int preventExecution(ProceedingJoinPoint pjp) { @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 int preventExecution(ProceedingJoinPoint pjp) { @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 @@ public void markDirty() { } @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 @@ public void setIntValue(int intValue) { @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 3eac9fac5225..d649ea019642 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() {