diff --git a/integration-tests/integration-tests.gradle b/integration-tests/integration-tests.gradle index 71e23b906049..0282fa49a109 100644 --- a/integration-tests/integration-tests.gradle +++ b/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") diff --git a/integration-tests/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementIntegrationTests.java b/integration-tests/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementIntegrationTests.java index 8ba437386b04..385be918d265 100644 --- a/integration-tests/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementIntegrationTests.java +++ b/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 diff --git a/integration-tests/src/test/java/org/springframework/transaction/interceptor/AnnotationTransactionAspectTest.java b/integration-tests/src/test/java/org/springframework/transaction/interceptor/AnnotationTransactionAspectTest.java new file mode 100644 index 000000000000..4724ecf5ced1 --- /dev/null +++ b/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 findAll() { + return Collections.emptyList(); + } + + @javax.transaction.Transactional + public List findAllJta() { + return Collections.emptyList(); + } + } + + private TransactionAttribute attr() { + return new TransactionAttribute() { + + @Override + public String getQualifier() { + return ""; + } + + @Override + public boolean rollbackOn(Throwable ex) { + return false; + } + }; + } + +} diff --git a/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AspectJJtaTransactionManagementConfiguration.java b/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AspectJJtaTransactionManagementConfiguration.java index ec733d3cf2b9..0f6d3af1ef56 100644 --- a/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AspectJJtaTransactionManagementConfiguration.java +++ b/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; } diff --git a/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AspectJTransactionManagementConfiguration.java b/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AspectJTransactionManagementConfiguration.java index 2c99c3050744..802abbf17aae 100644 --- a/spring-aspects/src/main/java/org/springframework/transaction/aspectj/AspectJTransactionManagementConfiguration.java +++ b/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; } diff --git a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java b/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java index ebc56af8f109..ffdb481c61ad 100644 --- a/spring-tx/src/main/java/org/springframework/transaction/interceptor/TransactionAspectSupport.java +++ b/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; }