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

Author: snicoll

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 @@ public ApplicationListenerMethodTransactionalAdapter(String beanName, Class<?> t
 
 	@Override
 	public void onApplicationEvent(ApplicationEvent event) {
-		if (TransactionSynchronizationManager.isSynchronizationActive()) {
+		if (TransactionSynchronizationManager.isSynchronizationActive()
+				&& TransactionSynchronizationManager.isActualTransactionActive()) {
 			TransactionSynchronization transactionSynchronization = createTransactionSynchronization(event);
 			TransactionSynchronizationManager.registerSynchronization(transactionSynchronization);
 		}

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 @@
 /**
  * An {@link EventListener} that is invoked according to a {@link TransactionPhase}.
  *
- * <p>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}.
+ * <p>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}.
  *
  * <p>Adding {@link org.springframework.core.annotation.Order @Order} to your annotated
  * method allows you to prioritize that listener amongst other listeners running before

spring-tx/src/test/java/org/springframework/transaction/event/TransactionalEventListenerTests.java

@@ -30,6 +30,7 @@
 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,19 +40,16 @@
 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;
 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 void afterCommit() {
 
 	@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 void noTransaction() {
 		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 void conditionFoundOnMetaAnnotation() {
 
 
 	protected EventCollector getEventCollector() {
-		return eventCollector;
+		return this.eventCollector;
 	}
 
 	protected ConfigurableApplicationContext getContext() {
-		return context;
+		return this.context;
 	}
 
 	private void load(Class<?>... classes) {
 		List<Class<?>> 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 void assertTotalEventsCount(int number) {
 	}
 
 
+	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";