From 415203479947fa3b757251fdc22e0da05d1c9f01 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 12 Jul 2023 19:09:48 +0200 Subject: [PATCH] Move transaction configuration check to transaction.annotation package AbstractTransactionManagementConfiguration.transactionalEventListenerFactory() creating a RestrictedTransactionalEventListenerFactory now. See gh-30679 --- ...actTransactionManagementConfiguration.java | 4 +- ...ctedTransactionalEventListenerFactory.java | 50 +++++++++++++++++++ ...ionalApplicationListenerMethodAdapter.java | 9 ---- ...ApplicationListenerMethodAdapterTests.java | 13 +++-- 4 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 spring-tx/src/main/java/org/springframework/transaction/annotation/RestrictedTransactionalEventListenerFactory.java diff --git a/spring-tx/src/main/java/org/springframework/transaction/annotation/AbstractTransactionManagementConfiguration.java b/spring-tx/src/main/java/org/springframework/transaction/annotation/AbstractTransactionManagementConfiguration.java index 0e885081fa..0bcfb3b707 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/annotation/AbstractTransactionManagementConfiguration.java +++ b/spring-tx/src/main/java/org/springframework/transaction/annotation/AbstractTransactionManagementConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 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. @@ -80,7 +80,7 @@ public abstract class AbstractTransactionManagementConfiguration implements Impo @Bean(name = TransactionManagementConfigUtils.TRANSACTIONAL_EVENT_LISTENER_FACTORY_BEAN_NAME) @Role(BeanDefinition.ROLE_INFRASTRUCTURE) public static TransactionalEventListenerFactory transactionalEventListenerFactory() { - return new TransactionalEventListenerFactory(); + return new RestrictedTransactionalEventListenerFactory(); } } diff --git a/spring-tx/src/main/java/org/springframework/transaction/annotation/RestrictedTransactionalEventListenerFactory.java b/spring-tx/src/main/java/org/springframework/transaction/annotation/RestrictedTransactionalEventListenerFactory.java new file mode 100644 index 0000000000..13f5d88e70 --- /dev/null +++ b/spring-tx/src/main/java/org/springframework/transaction/annotation/RestrictedTransactionalEventListenerFactory.java @@ -0,0 +1,50 @@ +/* + * Copyright 2002-2023 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.transaction.annotation; + +import java.lang.reflect.Method; + +import org.springframework.context.ApplicationListener; +import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.scheduling.annotation.Async; +import org.springframework.transaction.event.TransactionalEventListenerFactory; + +/** + * Extension of {@link TransactionalEventListenerFactory}, + * detecting invalid transaction configuration for transactional event listeners: + * {@link Transactional} only supported with {@link Propagation#REQUIRES_NEW} or + * {@link Async}. + * + * @author Juergen Hoeller + * @since 6.1 + * @see org.springframework.transaction.event.TransactionalEventListener + * @see Transactional + */ +public class RestrictedTransactionalEventListenerFactory extends TransactionalEventListenerFactory { + + @Override + public ApplicationListener createApplicationListener(String beanName, Class type, Method method) { + Transactional txAnn = AnnotatedElementUtils.findMergedAnnotation(method, Transactional.class); + if (txAnn != null && txAnn.propagation() != Propagation.REQUIRES_NEW && + !AnnotatedElementUtils.hasAnnotation(method, Async.class)) { + throw new IllegalStateException("@TransactionalEventListener method must not be annotated with " + + "@Transactional unless when marked as REQUIRES_NEW or declared as @Async: " + method); + } + return super.createApplicationListener(beanName, type, method); + } + +} diff --git a/spring-tx/src/main/java/org/springframework/transaction/event/TransactionalApplicationListenerMethodAdapter.java b/spring-tx/src/main/java/org/springframework/transaction/event/TransactionalApplicationListenerMethodAdapter.java index bca785fd90..a7830e4761 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/event/TransactionalApplicationListenerMethodAdapter.java +++ b/spring-tx/src/main/java/org/springframework/transaction/event/TransactionalApplicationListenerMethodAdapter.java @@ -25,9 +25,6 @@ import org.springframework.context.event.ApplicationListenerMethodAdapter; import org.springframework.context.event.EventListener; import org.springframework.context.event.GenericApplicationListener; import org.springframework.core.annotation.AnnotatedElementUtils; -import org.springframework.scheduling.annotation.Async; -import org.springframework.transaction.annotation.Propagation; -import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionSynchronizationManager; import org.springframework.util.Assert; @@ -71,12 +68,6 @@ public class TransactionalApplicationListenerMethodAdapter extends ApplicationLi if (eventAnn == null) { throw new IllegalStateException("No TransactionalEventListener annotation found on method: " + method); } - Transactional txAnn = AnnotatedElementUtils.findMergedAnnotation(method, Transactional.class); - if (txAnn != null && txAnn.propagation() != Propagation.REQUIRES_NEW && - !AnnotatedElementUtils.hasAnnotation(method, Async.class)) { - throw new IllegalStateException("@TransactionalEventListener method must not be annotated with " + - "@Transactional unless when marked as REQUIRES_NEW or declared as @Async: " + method); - } this.annotation = eventAnn; this.transactionPhase = eventAnn.phase(); } diff --git a/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalApplicationListenerMethodAdapterTests.java b/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalApplicationListenerMethodAdapterTests.java index 9e338b3630..cfbefa4687 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalApplicationListenerMethodAdapterTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalApplicationListenerMethodAdapterTests.java @@ -26,6 +26,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.scheduling.annotation.Async; import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.RestrictedTransactionalEventListenerFactory; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -34,6 +35,7 @@ import org.springframework.util.ReflectionUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatRuntimeException; /** @@ -129,22 +131,23 @@ public class TransactionalApplicationListenerMethodAdapterTests { @Test public void withTransactionalAnnotation() { + RestrictedTransactionalEventListenerFactory factory = new RestrictedTransactionalEventListenerFactory(); Method m = ReflectionUtils.findMethod(SampleEvents.class, "withTransactionalAnnotation", String.class); - assertThatIllegalStateException().isThrownBy(() -> createTestInstance(m)); + assertThatIllegalStateException().isThrownBy(() -> factory.createApplicationListener("test", SampleEvents.class, m)); } @Test public void withTransactionalRequiresNewAnnotation() { + RestrictedTransactionalEventListenerFactory factory = new RestrictedTransactionalEventListenerFactory(); Method m = ReflectionUtils.findMethod(SampleEvents.class, "withTransactionalRequiresNewAnnotation", String.class); - supportsEventType(true, m, createGenericEventType(String.class)); - supportsEventType(false, m, createGenericEventType(Double.class)); + assertThatNoException().isThrownBy(() -> factory.createApplicationListener("test", SampleEvents.class, m)); } @Test public void withAsyncTransactionalAnnotation() { + RestrictedTransactionalEventListenerFactory factory = new RestrictedTransactionalEventListenerFactory(); Method m = ReflectionUtils.findMethod(SampleEvents.class, "withAsyncTransactionalAnnotation", String.class); - supportsEventType(true, m, createGenericEventType(String.class)); - supportsEventType(false, m, createGenericEventType(Double.class)); + assertThatNoException().isThrownBy(() -> factory.createApplicationListener("test", SampleEvents.class, m)); }