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

Author: sbrannen

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();
+		}
+	}
+
+}

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