From 4f8b347aa014e814c35d8448f85aaf119a133552 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Fri, 12 Jul 2019 15:35:52 +0200 Subject: [PATCH] Do not trigger transactional event listener if no transaction is active This commit fixes the behaviour of not triggering a transactional event listener if no transaction is active. Previously, a transaction boundary was all that was necessary to trigger the listener regardless of the fact there was an active transaction. This commit now prevents `Propagation.NOT_SUPPORTED` and `Propagation.SUPPORTS` without an active transaction to trigger the listener. Closes gh-23276 --- ...ionListenerMethodTransactionalAdapter.java | 5 +- .../event/TransactionalEventListener.java | 8 +- .../TransactionalEventListenerTests.java | 98 +++++++++++++++---- 3 files changed, 84 insertions(+), 27 deletions(-) diff --git a/spring-tx/src/main/java/org/springframework/transaction/event/ApplicationListenerMethodTransactionalAdapter.java b/spring-tx/src/main/java/org/springframework/transaction/event/ApplicationListenerMethodTransactionalAdapter.java index 7444ec5b395..a4f32cca962 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/event/ApplicationListenerMethodTransactionalAdapter.java +++ b/spring-tx/src/main/java/org/springframework/transaction/event/ApplicationListenerMethodTransactionalAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 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. @@ -60,7 +60,8 @@ class ApplicationListenerMethodTransactionalAdapter extends ApplicationListenerM @Override public void onApplicationEvent(ApplicationEvent event) { - if (TransactionSynchronizationManager.isSynchronizationActive()) { + if (TransactionSynchronizationManager.isSynchronizationActive() + && TransactionSynchronizationManager.isActualTransactionActive()) { TransactionSynchronization transactionSynchronization = createTransactionSynchronization(event); TransactionSynchronizationManager.registerSynchronization(transactionSynchronization); } diff --git a/spring-tx/src/main/java/org/springframework/transaction/event/TransactionalEventListener.java b/spring-tx/src/main/java/org/springframework/transaction/event/TransactionalEventListener.java index 0fb6fc6047c..3339af2721c 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/event/TransactionalEventListener.java +++ b/spring-tx/src/main/java/org/springframework/transaction/event/TransactionalEventListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2019 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. @@ -28,9 +28,9 @@ import org.springframework.core.annotation.AliasFor; /** * An {@link EventListener} that is invoked according to a {@link TransactionPhase}. * - *

If the event is not published within the boundaries of a managed transaction, the - * event is discarded unless the {@link #fallbackExecution} flag is explicitly set. If a - * transaction is running, the event is processed according to its {@code TransactionPhase}. + *

If the event is not published within an active transaction, the event is discarded + * unless the {@link #fallbackExecution} flag is explicitly set. If a transaction is + * running, the event is processed according to its {@code TransactionPhase}. * *

Adding {@link org.springframework.core.annotation.Order @Order} to your annotated * method allows you to prioritize that listener amongst other listeners running before diff --git a/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java b/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java index 85cb6ed3066..230450f5d90 100644 --- a/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java +++ b/spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java @@ -30,6 +30,7 @@ import org.junit.After; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -39,6 +40,7 @@ import org.springframework.core.annotation.Order; import org.springframework.stereotype.Component; import org.springframework.tests.transaction.CallCountingTransactionManager; import org.springframework.transaction.annotation.EnableTransactionManagement; +import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.support.TransactionSynchronizationAdapter; import org.springframework.transaction.support.TransactionSynchronizationManager; @@ -46,12 +48,8 @@ import org.springframework.transaction.support.TransactionTemplate; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatIllegalStateException; -import static org.springframework.transaction.event.TransactionPhase.AFTER_COMMIT; -import static org.springframework.transaction.event.TransactionPhase.AFTER_COMPLETION; -import static org.springframework.transaction.event.TransactionPhase.AFTER_ROLLBACK; -import static org.springframework.transaction.event.TransactionPhase.BEFORE_COMMIT; +import static org.assertj.core.api.Assertions.*; +import static org.springframework.transaction.event.TransactionPhase.*; /** * Integration tests for {@link TransactionalEventListener} support @@ -66,7 +64,7 @@ public class TransactionalEventListenerTests { private EventCollector eventCollector; - private TransactionTemplate transactionTemplate = new TransactionTemplate(new CallCountingTransactionManager()); + private TransactionTemplate transactionTemplate; @After @@ -148,7 +146,7 @@ public class TransactionalEventListenerTests { @Test public void afterCommitWithTransactionalComponentListenerProxiedViaDynamicProxy() { - load(TransactionalConfiguration.class, TransactionalComponentTestListener.class); + load(TransactionalComponentTestListener.class); this.transactionTemplate.execute(status -> { getContext().publishEvent("SKIP"); getEventCollector().assertNoEventReceived(); @@ -250,6 +248,38 @@ public class TransactionalEventListenerTests { getEventCollector().assertTotalEventsCount(0); } + @Test + public void transactionDemarcationWithNotSupportedPropagation() { + load(BeforeCommitTestListener.class, AfterCompletionTestListener.class); + getContext().getBean(TestBean.class).notSupported(); + getEventCollector().assertTotalEventsCount(0); + } + + @Test + public void transactionDemarcationWithSupportsPropagationAndNoTransaction() { + load(BeforeCommitTestListener.class, AfterCompletionTestListener.class); + getContext().getBean(TestBean.class).supports(); + getEventCollector().assertTotalEventsCount(0); + } + + @Test + public void transactionDemarcationWithSupportsPropagationAndExistingTransaction() { + load(BeforeCommitTestListener.class, AfterCompletionTestListener.class); + this.transactionTemplate.execute(status -> { + getContext().getBean(TestBean.class).supports(); + getEventCollector().assertNoEventReceived(); + return null; + }); + getEventCollector().assertTotalEventsCount(2); + } + + @Test + public void transactionDemarcationWithRequiredPropagation() { + load(BeforeCommitTestListener.class, AfterCompletionTestListener.class); + getContext().getBean(TestBean.class).required(); + getEventCollector().assertTotalEventsCount(2); + } + @Test public void noTransactionWithFallbackExecution() { load(FallbackExecutionTestListener.class); @@ -299,49 +329,50 @@ public class TransactionalEventListenerTests { protected EventCollector getEventCollector() { - return eventCollector; + return this.eventCollector; } protected ConfigurableApplicationContext getContext() { - return context; + return this.context; } private void load(Class... classes) { List> allClasses = new ArrayList<>(); allClasses.add(BasicConfiguration.class); allClasses.addAll(Arrays.asList(classes)); - doLoad(allClasses.toArray(new Class[allClasses.size()])); + doLoad(allClasses.toArray(new Class[0])); } private void doLoad(Class... classes) { this.context = new AnnotationConfigApplicationContext(classes); this.eventCollector = this.context.getBean(EventCollector.class); + this.transactionTemplate = this.context.getBean(TransactionTemplate.class); } @Configuration + @EnableTransactionManagement static class BasicConfiguration { - @Bean // set automatically with tx management - public TransactionalEventListenerFactory transactionalEventListenerFactory() { - return new TransactionalEventListenerFactory(); - } - @Bean public EventCollector eventCollector() { return new EventCollector(); } - } - - @EnableTransactionManagement - @Configuration - static class TransactionalConfiguration { + @Bean + public TestBean testBean(ApplicationEventPublisher eventPublisher) { + return new TestBean(eventPublisher); + } @Bean public CallCountingTransactionManager transactionManager() { return new CallCountingTransactionManager(); } + + @Bean + public TransactionTemplate transactionTemplate() { + return new TransactionTemplate(transactionManager()); + } } @@ -399,6 +430,31 @@ public class TransactionalEventListenerTests { } + static class TestBean { + + private final ApplicationEventPublisher eventPublisher; + + TestBean(ApplicationEventPublisher eventPublisher) { + this.eventPublisher = eventPublisher; + } + + @Transactional(propagation = Propagation.NOT_SUPPORTED) + public void notSupported() { + this.eventPublisher.publishEvent("test"); + } + + @Transactional(propagation = Propagation.SUPPORTS) + public void supports() { + this.eventPublisher.publishEvent("test"); + } + + @Transactional(propagation = Propagation.REQUIRED) + public void required() { + this.eventPublisher.publishEvent("test"); + } + } + + static abstract class BaseTransactionalTestListener { static final String FAIL_MSG = "FAIL";