From 172c8b2c3527f5bb8e2a0cb48d0d35302a42766e Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sun, 1 Dec 2024 15:32:03 +0100 Subject: [PATCH] Polish AOP tests --- .../AspectJAutoProxyCreatorTests.java | 61 +++++----- .../BeanFactoryTransactionTests.java | 111 +++++++++--------- 2 files changed, 88 insertions(+), 84 deletions(-) diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java index 16da2ef070..bc6f45bfc4 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java @@ -85,7 +85,7 @@ class AspectJAutoProxyCreatorTests { void aspectsAreApplied() { ClassPathXmlApplicationContext bf = newContext("aspects.xml"); - ITestBean tb = (ITestBean) bf.getBean("adrian"); + ITestBean tb = bf.getBean("adrian", ITestBean.class); assertThat(tb.getAge()).isEqualTo(68); MethodInvokingFactoryBean factoryBean = (MethodInvokingFactoryBean) bf.getBean("&factoryBean"); assertThat(AopUtils.isAopProxy(factoryBean.getTargetObject())).isTrue(); @@ -96,7 +96,7 @@ class AspectJAutoProxyCreatorTests { void multipleAspectsWithParameterApplied() { ClassPathXmlApplicationContext bf = newContext("aspects.xml"); - ITestBean tb = (ITestBean) bf.getBean("adrian"); + ITestBean tb = bf.getBean("adrian", ITestBean.class); tb.setAge(10); assertThat(tb.getAge()).isEqualTo(20); } @@ -105,7 +105,7 @@ class AspectJAutoProxyCreatorTests { void aspectsAreAppliedInDefinedOrder() { ClassPathXmlApplicationContext bf = newContext("aspectsWithOrdering.xml"); - ITestBean tb = (ITestBean) bf.getBean("adrian"); + ITestBean tb = bf.getBean("adrian", ITestBean.class); assertThat(tb.getAge()).isEqualTo(71); } @@ -113,8 +113,8 @@ class AspectJAutoProxyCreatorTests { void aspectsAndAdvisorAreApplied() { ClassPathXmlApplicationContext ac = newContext("aspectsPlusAdvisor.xml"); - ITestBean shouldBeWeaved = (ITestBean) ac.getBean("adrian"); - doTestAspectsAndAdvisorAreApplied(ac, shouldBeWeaved); + ITestBean shouldBeWoven = ac.getBean("adrian", ITestBean.class); + assertAspectsAndAdvisorAreApplied(ac, shouldBeWoven); } @Test @@ -124,20 +124,22 @@ class AspectJAutoProxyCreatorTests { GenericApplicationContext childAc = new GenericApplicationContext(ac); // Create a child factory with a bean that should be woven RootBeanDefinition bd = new RootBeanDefinition(TestBean.class); - bd.getPropertyValues().addPropertyValue(new PropertyValue("name", "Adrian")) + bd.getPropertyValues() + .addPropertyValue(new PropertyValue("name", "Adrian")) .addPropertyValue(new PropertyValue("age", 34)); childAc.registerBeanDefinition("adrian2", bd); // Register the advisor auto proxy creator with subclass - childAc.registerBeanDefinition(AnnotationAwareAspectJAutoProxyCreator.class.getName(), new RootBeanDefinition( - AnnotationAwareAspectJAutoProxyCreator.class)); + childAc.registerBeanDefinition(AnnotationAwareAspectJAutoProxyCreator.class.getName(), + new RootBeanDefinition(AnnotationAwareAspectJAutoProxyCreator.class)); childAc.refresh(); - ITestBean beanFromChildContextThatShouldBeWeaved = (ITestBean) childAc.getBean("adrian2"); - //testAspectsAndAdvisorAreApplied(childAc, (ITestBean) ac.getBean("adrian")); - doTestAspectsAndAdvisorAreApplied(childAc, beanFromChildContextThatShouldBeWeaved); + ITestBean beanFromParentContextThatShouldBeWoven = ac.getBean("adrian", ITestBean.class); + ITestBean beanFromChildContextThatShouldBeWoven = childAc.getBean("adrian2", ITestBean.class); + assertAspectsAndAdvisorAreApplied(childAc, beanFromParentContextThatShouldBeWoven); + assertAspectsAndAdvisorAreApplied(childAc, beanFromChildContextThatShouldBeWoven); } - protected void doTestAspectsAndAdvisorAreApplied(ApplicationContext ac, ITestBean shouldBeWeaved) { + protected void assertAspectsAndAdvisorAreApplied(ApplicationContext ac, ITestBean shouldBeWoven) { TestBeanAdvisor tba = (TestBeanAdvisor) ac.getBean("advisor"); MultiplyReturnValue mrv = (MultiplyReturnValue) ac.getBean("aspect"); @@ -146,10 +148,10 @@ class AspectJAutoProxyCreatorTests { tba.count = 0; mrv.invocations = 0; - assertThat(AopUtils.isAopProxy(shouldBeWeaved)).as("Autoproxying must apply from @AspectJ aspect").isTrue(); - assertThat(shouldBeWeaved.getName()).isEqualTo("Adrian"); + assertThat(AopUtils.isAopProxy(shouldBeWoven)).as("Autoproxying must apply from @AspectJ aspect").isTrue(); + assertThat(shouldBeWoven.getName()).isEqualTo("Adrian"); assertThat(mrv.invocations).isEqualTo(0); - assertThat(shouldBeWeaved.getAge()).isEqualTo((34 * mrv.getMultiple())); + assertThat(shouldBeWoven.getAge()).isEqualTo((34 * mrv.getMultiple())); assertThat(tba.count).as("Spring advisor must be invoked").isEqualTo(2); assertThat(mrv.invocations).as("Must be able to hold state in aspect").isEqualTo(1); } @@ -158,13 +160,13 @@ class AspectJAutoProxyCreatorTests { void perThisAspect() { ClassPathXmlApplicationContext bf = newContext("perthis.xml"); - ITestBean adrian1 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian1 = bf.getBean("adrian", ITestBean.class); assertThat(AopUtils.isAopProxy(adrian1)).isTrue(); assertThat(adrian1.getAge()).isEqualTo(0); assertThat(adrian1.getAge()).isEqualTo(1); - ITestBean adrian2 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian2 = bf.getBean("adrian", ITestBean.class); assertThat(adrian2).isNotSameAs(adrian1); assertThat(AopUtils.isAopProxy(adrian1)).isTrue(); assertThat(adrian2.getAge()).isEqualTo(0); @@ -178,7 +180,7 @@ class AspectJAutoProxyCreatorTests { void perTargetAspect() throws SecurityException, NoSuchMethodException { ClassPathXmlApplicationContext bf = newContext("pertarget.xml"); - ITestBean adrian1 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian1 = bf.getBean("adrian", ITestBean.class); assertThat(AopUtils.isAopProxy(adrian1)).isTrue(); // Does not trigger advice or count @@ -199,7 +201,7 @@ class AspectJAutoProxyCreatorTests { adrian1.setName("Adrian"); //assertEquals("Any other setter does not increment", 2, adrian1.getAge()); - ITestBean adrian2 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian2 = bf.getBean("adrian", ITestBean.class); assertThat(adrian2).isNotSameAs(adrian1); assertThat(AopUtils.isAopProxy(adrian1)).isTrue(); assertThat(adrian2.getAge()).isEqualTo(34); @@ -239,7 +241,7 @@ class AspectJAutoProxyCreatorTests { void twoAdviceAspect() { ClassPathXmlApplicationContext bf = newContext("twoAdviceAspect.xml"); - ITestBean adrian1 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian1 = bf.getBean("adrian", ITestBean.class); testAgeAspect(adrian1, 0, 2); } @@ -247,9 +249,9 @@ class AspectJAutoProxyCreatorTests { void twoAdviceAspectSingleton() { ClassPathXmlApplicationContext bf = newContext("twoAdviceAspectSingleton.xml"); - ITestBean adrian1 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian1 = bf.getBean("adrian", ITestBean.class); testAgeAspect(adrian1, 0, 1); - ITestBean adrian2 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian2 = bf.getBean("adrian", ITestBean.class); assertThat(adrian2).isNotSameAs(adrian1); testAgeAspect(adrian2, 2, 1); } @@ -258,9 +260,9 @@ class AspectJAutoProxyCreatorTests { void twoAdviceAspectPrototype() { ClassPathXmlApplicationContext bf = newContext("twoAdviceAspectPrototype.xml"); - ITestBean adrian1 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian1 = bf.getBean("adrian", ITestBean.class); testAgeAspect(adrian1, 0, 1); - ITestBean adrian2 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian2 = bf.getBean("adrian", ITestBean.class); assertThat(adrian2).isNotSameAs(adrian1); testAgeAspect(adrian2, 0, 1); } @@ -280,7 +282,7 @@ class AspectJAutoProxyCreatorTests { void adviceUsingJoinPoint() { ClassPathXmlApplicationContext bf = newContext("usesJoinPointAspect.xml"); - ITestBean adrian1 = (ITestBean) bf.getBean("adrian"); + ITestBean adrian1 = bf.getBean("adrian", ITestBean.class); adrian1.getAge(); AdviceUsingThisJoinPoint aspectInstance = (AdviceUsingThisJoinPoint) bf.getBean("aspect"); //(AdviceUsingThisJoinPoint) Aspects.aspectOf(AdviceUsingThisJoinPoint.class); @@ -292,7 +294,7 @@ class AspectJAutoProxyCreatorTests { void includeMechanism() { ClassPathXmlApplicationContext bf = newContext("usesInclude.xml"); - ITestBean adrian = (ITestBean) bf.getBean("adrian"); + ITestBean adrian = bf.getBean("adrian", ITestBean.class); assertThat(AopUtils.isAopProxy(adrian)).isTrue(); assertThat(adrian.getAge()).isEqualTo(68); } @@ -310,7 +312,7 @@ class AspectJAutoProxyCreatorTests { void withAbstractFactoryBeanAreApplied() { ClassPathXmlApplicationContext bf = newContext("aspectsWithAbstractBean.xml"); - ITestBean adrian = (ITestBean) bf.getBean("adrian"); + ITestBean adrian = bf.getBean("adrian", ITestBean.class); assertThat(AopUtils.isAopProxy(adrian)).isTrue(); assertThat(adrian.getAge()).isEqualTo(68); } @@ -321,8 +323,7 @@ class AspectJAutoProxyCreatorTests { UnreliableBean bean = (UnreliableBean) bf.getBean("unreliableBean"); RetryAspect aspect = (RetryAspect) bf.getBean("retryAspect"); - int attempts = bean.unreliable(); - assertThat(attempts).isEqualTo(2); + assertThat(bean.unreliable()).isEqualTo(2); assertThat(aspect.getBeginCalls()).isEqualTo(2); assertThat(aspect.getRollbackCalls()).isEqualTo(1); assertThat(aspect.getCommitCalls()).isEqualTo(1); @@ -332,7 +333,7 @@ class AspectJAutoProxyCreatorTests { void withBeanNameAutoProxyCreator() { ClassPathXmlApplicationContext bf = newContext("withBeanNameAutoProxyCreator.xml"); - ITestBean tb = (ITestBean) bf.getBean("adrian"); + ITestBean tb = bf.getBean("adrian", ITestBean.class); assertThat(tb.getAge()).isEqualTo(68); } diff --git a/spring-tx/src/test/java/org/springframework/transaction/interceptor/BeanFactoryTransactionTests.java b/spring-tx/src/test/java/org/springframework/transaction/interceptor/BeanFactoryTransactionTests.java index f8fd380f6f..169ac138ed 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/interceptor/BeanFactoryTransactionTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/interceptor/BeanFactoryTransactionTests.java @@ -56,59 +56,54 @@ import static org.mockito.Mockito.verifyNoInteractions; */ class BeanFactoryTransactionTests { - private DefaultListableBeanFactory factory; + private final DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); @BeforeEach - void setUp() { - this.factory = new DefaultListableBeanFactory(); + void loadBeanDefinitions() { new XmlBeanDefinitionReader(this.factory).loadBeanDefinitions( new ClassPathResource("transactionalBeanFactory.xml", getClass())); } @Test - void testGetsAreNotTransactionalWithProxyFactory1() { - ITestBean testBean = (ITestBean) factory.getBean("proxyFactory1"); + void getsAreNotTransactionalWithProxyFactory1() { + ITestBean testBean = factory.getBean("proxyFactory1", ITestBean.class); assertThat(Proxy.isProxyClass(testBean.getClass())).as("testBean is a dynamic proxy").isTrue(); - boolean condition = testBean instanceof TransactionalProxy; - assertThat(condition).isFalse(); - doTestGetsAreNotTransactional(testBean); + assertThat(testBean).isNotInstanceOf(TransactionalProxy.class); + assertGetsAreNotTransactional(testBean); } @Test - void testGetsAreNotTransactionalWithProxyFactory2DynamicProxy() { + void getsAreNotTransactionalWithProxyFactory2DynamicProxy() { this.factory.preInstantiateSingletons(); - ITestBean testBean = (ITestBean) factory.getBean("proxyFactory2DynamicProxy"); + ITestBean testBean = factory.getBean("proxyFactory2DynamicProxy", ITestBean.class); assertThat(Proxy.isProxyClass(testBean.getClass())).as("testBean is a dynamic proxy").isTrue(); - boolean condition = testBean instanceof TransactionalProxy; - assertThat(condition).isTrue(); - doTestGetsAreNotTransactional(testBean); + assertThat(testBean).isInstanceOf(TransactionalProxy.class); + assertGetsAreNotTransactional(testBean); } @Test - void testGetsAreNotTransactionalWithProxyFactory2Cglib() { - ITestBean testBean = (ITestBean) factory.getBean("proxyFactory2Cglib"); + void getsAreNotTransactionalWithProxyFactory2Cglib() { + ITestBean testBean = factory.getBean("proxyFactory2Cglib", ITestBean.class); assertThat(AopUtils.isCglibProxy(testBean)).as("testBean is CGLIB advised").isTrue(); - boolean condition = testBean instanceof TransactionalProxy; - assertThat(condition).isTrue(); - doTestGetsAreNotTransactional(testBean); + assertThat(testBean).isInstanceOf(TransactionalProxy.class); + assertGetsAreNotTransactional(testBean); } @Test - void testProxyFactory2Lazy() { - ITestBean testBean = (ITestBean) factory.getBean("proxyFactory2Lazy"); + void proxyFactory2Lazy() { + ITestBean testBean = factory.getBean("proxyFactory2Lazy", ITestBean.class); assertThat(factory.containsSingleton("target")).isFalse(); assertThat(testBean.getAge()).isEqualTo(666); assertThat(factory.containsSingleton("target")).isTrue(); } @Test - void testCglibTransactionProxyImplementsNoInterfaces() { - ImplementsNoInterfaces ini = (ImplementsNoInterfaces) factory.getBean("cglibNoInterfaces"); + void cglibTransactionProxyImplementsNoInterfaces() { + ImplementsNoInterfaces ini = factory.getBean("cglibNoInterfaces", ImplementsNoInterfaces.class); assertThat(AopUtils.isCglibProxy(ini)).as("testBean is CGLIB advised").isTrue(); - boolean condition = ini instanceof TransactionalProxy; - assertThat(condition).isTrue(); + assertThat(ini).isInstanceOf(TransactionalProxy.class); String newName = "Gordon"; // Install facade @@ -121,31 +116,41 @@ class BeanFactoryTransactionTests { } @Test - void testGetsAreNotTransactionalWithProxyFactory3() { - ITestBean testBean = (ITestBean) factory.getBean("proxyFactory3"); - boolean condition = testBean instanceof DerivedTestBean; - assertThat(condition).as("testBean is a full proxy").isTrue(); - boolean condition1 = testBean instanceof TransactionalProxy; - assertThat(condition1).isTrue(); - InvocationCounterPointcut txnCounter = (InvocationCounterPointcut) factory.getBean("txnInvocationCounterPointcut"); - InvocationCounterInterceptor preCounter = (InvocationCounterInterceptor) factory.getBean("preInvocationCounterInterceptor"); - InvocationCounterInterceptor postCounter = (InvocationCounterInterceptor) factory.getBean("postInvocationCounterInterceptor"); - txnCounter.counter = 0; - preCounter.counter = 0; - postCounter.counter = 0; - doTestGetsAreNotTransactional(testBean); - // Can't assert it's equal to 4 as the pointcut may be optimized and only invoked once - assertThat(0 < txnCounter.counter && txnCounter.counter <= 4).isTrue(); - assertThat(preCounter.counter).isEqualTo(4); - assertThat(postCounter.counter).isEqualTo(4); + void getsAreNotTransactionalWithProxyFactory3() { + ITestBean testBean = factory.getBean("proxyFactory3", ITestBean.class); + assertThat(testBean).as("testBean is a full proxy") + .isInstanceOf(DerivedTestBean.class) + .isInstanceOf(TransactionalProxy.class); + + InvocationCounterPointcut txnPointcut = factory.getBean("txnInvocationCounterPointcut", InvocationCounterPointcut.class); + InvocationCounterInterceptor preInterceptor = factory.getBean("preInvocationCounterInterceptor", InvocationCounterInterceptor.class); + InvocationCounterInterceptor postInterceptor = factory.getBean("postInvocationCounterInterceptor", InvocationCounterInterceptor.class); + assertThat(txnPointcut.counter).as("txnPointcut").isGreaterThan(0); + assertThat(preInterceptor.counter).as("preInterceptor").isZero(); + assertThat(postInterceptor.counter).as("postInterceptor").isZero(); + + // Reset counters + txnPointcut.counter = 0; + preInterceptor.counter = 0; + postInterceptor.counter = 0; + + // Invokes: getAge() * 2 and setAge() * 1 --> 2 + 1 = 3 method invocations. + assertGetsAreNotTransactional(testBean); + + // The transaction pointcut is currently asked if it matches() for all method + // invocations, but we cannot assert it's equal to 3 since the pointcut may be + // optimized and only invoked once. + assertThat(txnPointcut.counter).as("txnPointcut").isGreaterThanOrEqualTo(1).isLessThanOrEqualTo(3); + assertThat(preInterceptor.counter).as("preInterceptor").isEqualTo(3); + assertThat(postInterceptor.counter).as("postInterceptor").isEqualTo(3); } - private void doTestGetsAreNotTransactional(final ITestBean testBean) { + private void assertGetsAreNotTransactional(final ITestBean testBean) { // Install facade PlatformTransactionManager ptm = mock(); PlatformTransactionManagerFacade.delegate = ptm; - assertThat(testBean.getAge()).as("Age should not be " + testBean.getAge()).isEqualTo(666); + assertThat(testBean.getAge()).as("Age").isEqualTo(666); // Expect no methods verifyNoInteractions(ptm); @@ -177,14 +182,14 @@ class BeanFactoryTransactionTests { }; PlatformTransactionManagerFacade.delegate = ptm; - // TODO same as old age to avoid ordering effect for now + // same as old age to avoid ordering effect for now int age = 666; testBean.setAge(age); assertThat(testBean.getAge()).isEqualTo(age); } @Test - void testGetBeansOfTypeWithAbstract() { + void getBeansOfTypeWithAbstract() { Map beansOfType = factory.getBeansOfType(ITestBean.class, true, true); assertThat(beansOfType).isNotNull(); } @@ -193,24 +198,22 @@ class BeanFactoryTransactionTests { * Check that we fail gracefully if the user doesn't set any transaction attributes. */ @Test - void testNoTransactionAttributeSource() { - assertThatExceptionOfType(FatalBeanException.class).isThrownBy(() -> { - DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); - new XmlBeanDefinitionReader(bf).loadBeanDefinitions(new ClassPathResource("noTransactionAttributeSource.xml", getClass())); - bf.getBean("noTransactionAttributeSource"); - }); + void noTransactionAttributeSource() { + DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); + new XmlBeanDefinitionReader(bf).loadBeanDefinitions(new ClassPathResource("noTransactionAttributeSource.xml", getClass())); + assertThatExceptionOfType(FatalBeanException.class).isThrownBy(() -> bf.getBean("noTransactionAttributeSource")); } /** * Test that we can set the target to a dynamic TargetSource. */ @Test - void testDynamicTargetSource() { + void dynamicTargetSource() { // Install facade CallCountingTransactionManager txMan = new CallCountingTransactionManager(); PlatformTransactionManagerFacade.delegate = txMan; - TestBean tb = (TestBean) factory.getBean("hotSwapped"); + TestBean tb = factory.getBean("hotSwapped", TestBean.class); assertThat(tb.getAge()).isEqualTo(666); int newAge = 557; tb.setAge(newAge); @@ -218,7 +221,7 @@ class BeanFactoryTransactionTests { TestBean target2 = new TestBean(); target2.setAge(65); - HotSwappableTargetSource ts = (HotSwappableTargetSource) factory.getBean("swapper"); + HotSwappableTargetSource ts = factory.getBean("swapper", HotSwappableTargetSource.class); ts.swap(target2); assertThat(tb.getAge()).isEqualTo(target2.getAge()); tb.setAge(newAge);