From 320831b18ad7e8f0d0f0e9072f287ba699e96a21 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sun, 1 Dec 2024 15:38:15 +0100 Subject: [PATCH] Test status quo for StaticMethodMatcherPointcut#matches invocations This commit introduces a test which verifies how many times the matches() method of a StaticMethodMatcherPointcut is invoked during ApplicationContext startup as well as during actual method invocations via the advice chain, which also indirectly tests the behavior of the equals() implementation in AdvisedSupport.MethodCacheKey. In addition, this commit revises BeanFactoryTransactionTests to assert that a transaction is started for the setAge() method. See gh-33915 --- .../DefaultAdvisorAutoProxyCreatorTests.java | 95 +++++++++++++++++++ .../BeanFactoryTransactionTests.java | 29 +++--- 2 files changed, 108 insertions(+), 16 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/aop/framework/autoproxy/DefaultAdvisorAutoProxyCreatorTests.java diff --git a/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/DefaultAdvisorAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/DefaultAdvisorAutoProxyCreatorTests.java new file mode 100644 index 000000000000..a9c723a04fa4 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/aop/framework/autoproxy/DefaultAdvisorAutoProxyCreatorTests.java @@ -0,0 +1,95 @@ +/* + * Copyright 2002-2024 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.lang.reflect.Method; + +import org.aopalliance.aop.Advice; +import org.aopalliance.intercept.MethodInterceptor; +import org.junit.jupiter.api.Test; + +import org.springframework.aop.Pointcut; +import org.springframework.aop.support.AbstractPointcutAdvisor; +import org.springframework.aop.support.RootClassFilter; +import org.springframework.aop.support.StaticMethodMatcherPointcut; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests for {@link DefaultAdvisorAutoProxyCreator}. + * + * @author Sam Brannen + * @since 6.2.1 + */ +class DefaultAdvisorAutoProxyCreatorTests { + + /** + * Indirectly tests behavior of {@link org.springframework.aop.framework.AdvisedSupport.MethodCacheKey}. + * @see StaticMethodMatcherPointcut#matches(Method, Class) + */ + @Test // gh-33915 + void staticMethodMatcherPointcutMatchesMethodIsInvokedAgainForActualMethodInvocation() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + DemoBean.class, DemoPointcutAdvisor.class, DefaultAdvisorAutoProxyCreator.class); + DemoPointcutAdvisor demoPointcutAdvisor = context.getBean(DemoPointcutAdvisor.class); + DemoBean demoBean = context.getBean(DemoBean.class); + + assertThat(demoPointcutAdvisor.matchesInvocationCount).as("matches() invocations before").isEqualTo(2); + assertThat(demoBean.sayHello()).isEqualTo("Advised: Hello!"); + assertThat(demoPointcutAdvisor.matchesInvocationCount).as("matches() invocations after").isEqualTo(3); + + context.close(); + } + + + static class DemoBean { + + public String sayHello() { + return "Hello!"; + } + } + + @SuppressWarnings("serial") + static class DemoPointcutAdvisor extends AbstractPointcutAdvisor { + + int matchesInvocationCount = 0; + + @Override + public Pointcut getPointcut() { + StaticMethodMatcherPointcut pointcut = new StaticMethodMatcherPointcut() { + + @Override + public boolean matches(Method method, Class targetClass) { + if (method.getName().equals("sayHello")) { + matchesInvocationCount++; + return true; + } + return false; + } + }; + pointcut.setClassFilter(new RootClassFilter(DemoBean.class)); + return pointcut; + } + + @Override + public Advice getAdvice() { + return (MethodInterceptor) invocation -> "Advised: " + invocation.proceed(); + } + } + +} 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 169ac138ed03..0fe3bef54a6a 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 @@ -19,6 +19,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; @@ -52,6 +53,7 @@ * * @author Rod Johnson * @author Juergen Hoeller + * @author Sam Brannen * @since 23.04.2003 */ class BeanFactoryTransactionTests { @@ -145,30 +147,25 @@ void getsAreNotTransactionalWithProxyFactory3() { assertThat(postInterceptor.counter).as("postInterceptor").isEqualTo(3); } - private void assertGetsAreNotTransactional(final ITestBean testBean) { + private void assertGetsAreNotTransactional(ITestBean testBean) { // Install facade PlatformTransactionManager ptm = mock(); PlatformTransactionManagerFacade.delegate = ptm; assertThat(testBean.getAge()).as("Age").isEqualTo(666); - // Expect no methods + // Expect no interactions with the transaction manager. verifyNoInteractions(ptm); // Install facade expecting a call - final TransactionStatus ts = mock(); + AtomicBoolean invoked = new AtomicBoolean(); + TransactionStatus ts = mock(); ptm = new PlatformTransactionManager() { - private boolean invoked; @Override public TransactionStatus getTransaction(@Nullable TransactionDefinition def) throws TransactionException { - if (invoked) { - throw new IllegalStateException("getTransaction should not get invoked more than once"); - } - invoked = true; - if (!(def.getName().contains(DerivedTestBean.class.getName()) && def.getName().contains("setAge"))) { - throw new IllegalStateException( - "transaction name should contain class and method name: " + def.getName()); - } + assertThat(invoked.compareAndSet(false, true)) + .as("getTransaction() should not get invoked more than once").isTrue(); + assertThat(def.getName()).as("transaction name").contains(DerivedTestBean.class.getName(), "setAge"); return ts; } @Override @@ -182,10 +179,10 @@ public void rollback(TransactionStatus status) throws TransactionException { }; PlatformTransactionManagerFacade.delegate = ptm; - // same as old age to avoid ordering effect for now - int age = 666; - testBean.setAge(age); - assertThat(testBean.getAge()).isEqualTo(age); + assertThat(invoked).as("getTransaction() invoked before setAge()").isFalse(); + testBean.setAge(42); + assertThat(invoked).as("getTransaction() invoked after setAge()").isTrue(); + assertThat(testBean.getAge()).as("Age").isEqualTo(42); } @Test