Fix transaction aspect's incorrect state after creating new context

integration-tests/integration-tests.gradle

@@ -1,5 +1,16 @@
 description = "Spring Integration Tests"
 
+apply plugin: "io.freefair.aspectj"
+apply plugin: "org.springframework.build.test-sources"
+
+sourceSets.main.aspectj.srcDir "src/main/java"
+sourceSets.main.java.srcDirs = files()
+
+sourceSets.test.aspectj.srcDir "src/test/java"
+sourceSets.test.java.srcDirs = files()
+
+aspectj.version = dependencyManagement.managedVersions['org.aspectj:aspectjweaver']
+
 dependencies {
 	testCompile(project(":spring-aop"))
 	testCompile(project(":spring-beans"))
@@ -15,6 +26,7 @@ dependencies {
 	testCompile(project(":spring-test"))
 	testCompile(project(":spring-tx"))
 	testCompile(project(":spring-web"))
+	aspect(project(":spring-aspects"))
 	testCompile("javax.inject:javax.inject")
 	testCompile("javax.resource:javax.resource-api")
 	testCompile("javax.servlet:javax.servlet-api")

integration-tests/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementIntegrationTests.java

@@ -41,6 +41,7 @@
 import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
 import org.springframework.stereotype.Repository;
 import org.springframework.transaction.PlatformTransactionManager;
+import org.springframework.transaction.aspectj.AspectJJtaTransactionManagementConfiguration;
 import org.springframework.transaction.interceptor.BeanFactoryTransactionAttributeSourceAdvisor;
 import org.springframework.transaction.testfixture.CallCountingTransactionManager;
 
@@ -95,12 +96,9 @@ void repositoryIsClassBasedTxProxy() {
 	void repositoryUsesAspectJAdviceMode() {
 		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
 		ctx.register(Config.class, AspectJTxConfig.class);
-		// this test is a bit fragile, but gets the job done, proving that an
-		// attempt was made to look up the AJ aspect. It's due to classpath issues
-		// in .integration-tests that it's not found.
-		assertThatExceptionOfType(Exception.class)
-			.isThrownBy(ctx::refresh)
-			.withMessageContaining("AspectJJtaTransactionManagementConfiguration");
+		ctx.refresh();
+		final AspectJJtaTransactionManagementConfiguration config = (AspectJJtaTransactionManagementConfiguration) ctx.getBean("org.springframework.transaction.aspectj.AspectJJtaTransactionManagementConfiguration");
+		assertThat(config).isNotNull();
 	}
 
 	@Test

integration-tests/src/test/java/org/springframework/transaction/interceptor/AnnotationTransactionAspectTest.java

@@ -0,0 +1,201 @@
+/*
+ * 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.
+ * 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.interceptor;
+
+import org.junit.jupiter.api.Test;
+import org.springframework.context.annotation.AdviceMode;
+import org.springframework.context.annotation.AnnotationConfigApplicationContext;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.jdbc.datasource.DataSourceTransactionManager;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseBuilder;
+import org.springframework.jdbc.datasource.embedded.EmbeddedDatabaseType;
+import org.springframework.stereotype.Repository;
+import org.springframework.transaction.PlatformTransactionManager;
+import org.springframework.transaction.TransactionManager;
+import org.springframework.transaction.annotation.EnableTransactionManagement;
+import org.springframework.transaction.annotation.TransactionManagementConfigurer;
+
+import javax.sql.DataSource;
+import java.util.Collections;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Tests proving that after the creation of new context, transaction aspects are cleared of
+ * transaction managers created in previous context.
+ *
+ * @author Semyon Danilov
+ */
+class AnnotationTransactionAspectTest {
+
+	private static final String TRANSACTION_MANAGER_KEY = "org.springframework.transaction.config.internalTransactionAspect";
+	private static final String JAVAX_TRANSACTION_MANAGER_KEY = "org.springframework.transaction.config.internalJtaTransactionAspect";
+
+	@Test
+	void testAspectTxManagerCacheCleanInNewContext() {
+		// stub txAttr to get txManager
+		TransactionAttribute txAttr = attr();
+
+		// create first context
+		AnnotationConfigApplicationContext ctx1 = new AnnotationConfigApplicationContext();
+		ctx1.register(Config.class, AspectJTxConfig.class);
+		ctx1.refresh();
+
+		// invoke transactional method thus initializing txManagerCache in aspect
+		final JdbcFooRepository repository1 = ctx1.getBean(JdbcFooRepository.class);
+		repository1.findAll();
+		repository1.findAllJta();
+
+		// get aspect bean (it's a real singleton, it's one per jvm)
+		TransactionAspectSupport supportBean = ctx1.getBeansOfType(TransactionAspectSupport.class).get(TRANSACTION_MANAGER_KEY);
+		TransactionAspectSupport supportBeanJavax = ctx1.getBeansOfType(TransactionAspectSupport.class).get(JAVAX_TRANSACTION_MANAGER_KEY);
+
+		// get txManager
+		final PlatformTransactionManager firstManager = supportBean.determineTransactionManager(txAttr);
+		final PlatformTransactionManager firstManagerJavax = supportBeanJavax.determineTransactionManager(txAttr);
+
+		// create second context, little different from first
+		AnnotationConfigApplicationContext ctx2 = new AnnotationConfigApplicationContext();
+		ctx2.register(Config.class, AspectJTxConfig.class, SomeOtherConfig.class);
+		ctx2.refresh();
+
+		// invoke transactional method thus initializing txManagerCache in aspect
+		final JdbcFooRepository repository2 = ctx2.getBean(JdbcFooRepository.class);
+		repository2.findAll();
+		repository2.findAllJta();
+
+		// get new txManager
+		final PlatformTransactionManager secondManager = supportBean.determineTransactionManager(txAttr);
+		final PlatformTransactionManager secondManagerJavax = supportBeanJavax.determineTransactionManager(txAttr);
+
+		// check that txManager was NOT cached
+		assertThat(firstManager).isNotEqualTo(secondManager);
+		assertThat(firstManagerJavax).isNotEqualTo(secondManagerJavax);
+	}
+
+
+	@Test
+	void testAspectTxManagerFieldCleanInNewContext() {
+		// stub txAttr to get txManager
+		TransactionAttribute txAttr = attr();
+
+		// create first context
+		AnnotationConfigApplicationContext ctx1 = new AnnotationConfigApplicationContext();
+
+		// this config will create default transaction manager
+		ctx1.register(Config.class, AspectJTxConfig.class, ConfigWithTxManagementConfigurer.class);
+		ctx1.refresh();
+
+		// get aspect bean (it's a real singleton, it's one per jvm)
+		TransactionAspectSupport supportBean = ctx1.getBeansOfType(TransactionAspectSupport.class).get(TRANSACTION_MANAGER_KEY);
+		TransactionAspectSupport supportBeanJavax = ctx1.getBeansOfType(TransactionAspectSupport.class).get(JAVAX_TRANSACTION_MANAGER_KEY);
+
+		// get txManager
+		final PlatformTransactionManager firstManager = supportBean.determineTransactionManager(txAttr);
+		final PlatformTransactionManager firstManagerJavax = supportBeanJavax.determineTransactionManager(txAttr);
+
+		// create second context, little different from first
+		AnnotationConfigApplicationContext ctx2 = new AnnotationConfigApplicationContext();
+
+		// this config is without default transaction manager, so it shouldn't be in aspect
+		ctx2.register(Config.class, AspectJTxConfig.class, SomeOtherConfig.class);
+		ctx2.refresh();
+
+		// get new txManager
+		final PlatformTransactionManager secondManager = supportBean.determineTransactionManager(txAttr);
+		final PlatformTransactionManager secondManagerJavax = supportBeanJavax.determineTransactionManager(txAttr);
+
+		// check that txManager is not the same
+		assertThat(firstManager).isNotEqualTo(secondManager);
+		assertThat(firstManagerJavax).isNotEqualTo(secondManagerJavax);
+	}
+
+	@Configuration
+	@EnableTransactionManagement(mode = AdviceMode.ASPECTJ)
+	static class Config {
+
+		@Bean
+		DataSource dataSource() {
+			return new EmbeddedDatabaseBuilder()
+					.setType(EmbeddedDatabaseType.HSQL)
+					.build();
+		}
+
+		@Bean
+		JdbcFooRepository repository() {
+			return new JdbcFooRepository();
+		}
+	}
+
+	@Configuration
+	static class AspectJTxConfig {
+
+		@Bean
+		PlatformTransactionManager transactionManager(DataSource dataSource) {
+			return new DataSourceTransactionManager(dataSource);
+		}
+	}
+
+	@Configuration
+	static class SomeOtherConfig {
+
+	}
+
+	static class ConfigWithTxManagementConfigurer {
+		@Bean
+		TransactionManagementConfigurer configurer(DataSource dataSource) {
+			return new TransactionManagementConfigurer() {
+				@Override
+				public TransactionManager annotationDrivenTransactionManager() {
+					return new DataSourceTransactionManager(dataSource);
+				}
+			};
+		}
+	}
+
+	@Repository
+	static class JdbcFooRepository {
+
+		@org.springframework.transaction.annotation.Transactional
+		public List<Object> findAll() {
+			return Collections.emptyList();
+		}
+
+		@javax.transaction.Transactional
+		public List<Object> findAllJta() {
+			return Collections.emptyList();
+		}
+	}
+
+	private TransactionAttribute attr() {
+		return new TransactionAttribute() {
+
+			@Override
+			public String getQualifier() {
+				return "";
+			}
+
+			@Override
+			public boolean rollbackOn(Throwable ex) {
+				return false;
+			}
+		};
+	}
+
+}

spring-aspects/src/main/java/org/springframework/transaction/aspectj/AspectJJtaTransactionManagementConfiguration.java

@@ -31,6 +31,7 @@
  * {@link org.springframework.transaction.annotation.Transactional} annotation.
  *
  * @author Juergen Hoeller
+ * @author Semyon Danilov
  * @since 5.1
  * @see EnableTransactionManagement
  * @see TransactionManagementConfigurationSelector
@@ -43,9 +44,9 @@ public class AspectJJtaTransactionManagementConfiguration extends AspectJTransac
 	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
 	public JtaAnnotationTransactionAspect jtaTransactionAspect() {
 		JtaAnnotationTransactionAspect txAspect = JtaAnnotationTransactionAspect.aspectOf();
-		if (this.txManager != null) {
-			txAspect.setTransactionManager(this.txManager);
-		}
+		// aspect is a singleton, so it must be cleared every time bean is created
+		txAspect.clearTransactionManagerCache();
+		txAspect.setTransactionManager(this.txManager);
 		return txAspect;
 	}
 

spring-aspects/src/main/java/org/springframework/transaction/aspectj/AspectJTransactionManagementConfiguration.java

@@ -32,6 +32,7 @@
  *
  * @author Chris Beams
  * @author Juergen Hoeller
+ * @author Semyon Danilov
  * @since 3.1
  * @see EnableTransactionManagement
  * @see TransactionManagementConfigurationSelector
@@ -45,9 +46,9 @@ public class AspectJTransactionManagementConfiguration extends AbstractTransacti
 	@Role(BeanDefinition.ROLE_INFRASTRUCTURE)
 	public AnnotationTransactionAspect transactionAspect() {
 		AnnotationTransactionAspect txAspect = AnnotationTransactionAspect.aspectOf();
-		if (this.txManager != null) {
-			txAspect.setTransactionManager(this.txManager);
-		}
+		// aspect is a singleton, so it must be cleared every time bean is created
+		txAspect.clearTransactionManagerCache();
+		txAspect.setTransactionManager(this.txManager);
 		return txAspect;
 	}
 

spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java

@@ -78,6 +78,7 @@
  * @author Stéphane Nicoll
  * @author Sam Brannen
  * @author Mark Paluch
+ * @author Semyon Danilov
  * @since 1.1
  * @see PlatformTransactionManager
  * @see ReactiveTransactionManager
@@ -451,7 +452,7 @@ protected Object invokeWithinTransaction(Method method, @Nullable Class<?> targe
 	/**
 	 * Clear the cache.
 	 */
-	protected void clearTransactionManagerCache() {
+	public void clearTransactionManagerCache() {
 		this.transactionManagerCache.clear();
 		this.beanFactory = null;
 	}