From 9614817e884367da0c20539da5391968d74a0b51 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 24 Aug 2018 00:49:01 +0200 Subject: [PATCH] Do not proxy test instances based on "original instance" convention Issue: SPR-17137 --- .../autoproxy/AbstractAutoProxyCreator.java | 9 +- ...BeanFactoryAwareAdvisingPostProcessor.java | 8 +- .../framework/autoproxy/AutoProxyUtils.java | 34 ++++- .../config/AutowireCapableBeanFactory.java | 27 +++- ...endencyInjectionTestExecutionListener.java | 13 +- .../groovy/GroovySpringContextTests.java | 49 +++--- ...TransactionalJUnit4SpringContextTests.java | 107 ++++++------- .../SpringJUnit4ClassRunnerAppCtxTests.java | 81 +++++----- ...TransactionalTestNGSpringContextTests.java | 142 +++++++++--------- .../PrimaryTransactionManagerTests.java | 4 +- 10 files changed, 268 insertions(+), 206 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java b/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java index ebd7cf5372..05eb217803 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractAutoProxyCreator.java @@ -385,14 +385,17 @@ public abstract class AbstractAutoProxyCreator extends ProxyProcessorSupport /** * Subclasses should override this method to return {@code true} if the * given bean should not be considered for auto-proxying by this post-processor. - *

Sometimes we need to be able to avoid this happening if it will lead to - * a circular reference. This implementation returns {@code false}. + *

Sometimes we need to be able to avoid this happening, e.g. if it will lead to + * a circular reference or if the existing target instance needs to be preserved. + * This implementation returns {@code false} unless the bean name indicates an + * "original instance" according to {@code AutowireCapableBeanFactory} conventions. * @param beanClass the class of the bean * @param beanName the name of the bean * @return whether to skip the given bean + * @see org.springframework.beans.factory.config.AutowireCapableBeanFactory#ORIGINAL_INSTANCE_SUFFIX */ protected boolean shouldSkip(Class beanClass, String beanName) { - return false; + return AutoProxyUtils.isOriginalInstance(beanName, beanClass); } /** diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractBeanFactoryAwareAdvisingPostProcessor.java b/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractBeanFactoryAwareAdvisingPostProcessor.java index c17cdc05eb..b8a90d052a 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractBeanFactoryAwareAdvisingPostProcessor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AbstractBeanFactoryAwareAdvisingPostProcessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -64,4 +64,10 @@ public abstract class AbstractBeanFactoryAwareAdvisingPostProcessor extends Abst return proxyFactory; } + @Override + protected boolean isEligible(Object bean, String beanName) { + return (!AutoProxyUtils.isOriginalInstance(beanName, bean.getClass()) && + super.isEligible(bean, beanName)); + } + } diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AutoProxyUtils.java b/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AutoProxyUtils.java index 2cf44715ce..d90552b910 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AutoProxyUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/autoproxy/AutoProxyUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -16,10 +16,12 @@ package org.springframework.aop.framework.autoproxy; +import org.springframework.beans.factory.config.AutowireCapableBeanFactory; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.core.Conventions; import org.springframework.lang.Nullable; +import org.springframework.util.StringUtils; /** * Utilities for auto-proxy aware components. @@ -63,7 +65,9 @@ public abstract class AutoProxyUtils { * @param beanName the name of the bean * @return whether the given bean should be proxied with its target class */ - public static boolean shouldProxyTargetClass(ConfigurableListableBeanFactory beanFactory, @Nullable String beanName) { + public static boolean shouldProxyTargetClass( + ConfigurableListableBeanFactory beanFactory, @Nullable String beanName) { + if (beanName != null && beanFactory.containsBeanDefinition(beanName)) { BeanDefinition bd = beanFactory.getBeanDefinition(beanName); return Boolean.TRUE.equals(bd.getAttribute(PRESERVE_TARGET_CLASS_ATTRIBUTE)); @@ -81,7 +85,9 @@ public abstract class AutoProxyUtils { * @see org.springframework.beans.factory.BeanFactory#getType(String) */ @Nullable - public static Class determineTargetClass(ConfigurableListableBeanFactory beanFactory, @Nullable String beanName) { + public static Class determineTargetClass( + ConfigurableListableBeanFactory beanFactory, @Nullable String beanName) { + if (beanName == null) { return null; } @@ -102,12 +108,30 @@ public abstract class AutoProxyUtils { * @param targetClass the corresponding target class * @since 4.2.3 */ - static void exposeTargetClass(ConfigurableListableBeanFactory beanFactory, @Nullable String beanName, - Class targetClass) { + static void exposeTargetClass( + ConfigurableListableBeanFactory beanFactory, @Nullable String beanName, Class targetClass) { if (beanName != null && beanFactory.containsBeanDefinition(beanName)) { beanFactory.getMergedBeanDefinition(beanName).setAttribute(ORIGINAL_TARGET_CLASS_ATTRIBUTE, targetClass); } } + /** + * Determine whether the given bean name indicates an "original instance" + * according to {@link AutowireCapableBeanFactory#ORIGINAL_INSTANCE_SUFFIX}, + * skipping any proxy attempts for it. + * @param beanName the name of the bean + * @param beanClass the corresponding bean class + * @since 5.1 + * @see AutowireCapableBeanFactory#ORIGINAL_INSTANCE_SUFFIX + */ + static boolean isOriginalInstance(String beanName, Class beanClass) { + if (!StringUtils.hasLength(beanName) || beanName.length() != + beanClass.getName().length() + AutowireCapableBeanFactory.ORIGINAL_INSTANCE_SUFFIX.length()) { + return false; + } + return (beanName.startsWith(beanClass.getName()) && + beanName.endsWith(AutowireCapableBeanFactory.ORIGINAL_INSTANCE_SUFFIX)); + } + } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/config/AutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/config/AutowireCapableBeanFactory.java index 96c0f6d2b9..49b3abf9a7 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/config/AutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/config/AutowireCapableBeanFactory.java @@ -107,6 +107,18 @@ public interface AutowireCapableBeanFactory extends BeanFactory { @Deprecated int AUTOWIRE_AUTODETECT = 4; + /** + * Suffix for the "original instance" convention when initializing an existing + * bean instance: to be appended to the fully-qualified bean class name, + * e.g. "com.mypackage.MyClass.ORIGINAL", in order to enforce the given instance + * to be returned, i.e. no proxies etc. + * @since 5.1 + * @see #initializeBean(Object, String) + * @see #applyBeanPostProcessorsBeforeInitialization(Object, String) + * @see #applyBeanPostProcessorsAfterInitialization(Object, String) + */ + String ORIGINAL_INSTANCE_SUFFIX = ".ORIGINAL"; + //------------------------------------------------------------------------- // Typical methods for creating and populating external bean instances @@ -264,9 +276,12 @@ public interface AutowireCapableBeanFactory extends BeanFactory { * for callbacks but not checked against the registered bean definitions. * @param existingBean the existing bean instance * @param beanName the name of the bean, to be passed to it if necessary - * (only passed to {@link BeanPostProcessor BeanPostProcessors}) + * (only passed to {@link BeanPostProcessor BeanPostProcessors}; + * can follow the {@link #ORIGINAL_INSTANCE_SUFFIX} convention in order to + * enforce the given instance to be returned, i.e. no proxies etc) * @return the bean instance to use, either the original or a wrapped one * @throws BeansException if the initialization failed + * @see #ORIGINAL_INSTANCE_SUFFIX */ Object initializeBean(Object existingBean, String beanName) throws BeansException; @@ -275,10 +290,13 @@ public interface AutowireCapableBeanFactory extends BeanFactory { * instance, invoking their {@code postProcessBeforeInitialization} methods. * The returned bean instance may be a wrapper around the original. * @param existingBean the new bean instance - * @param beanName the name of the bean + * (only passed to {@link BeanPostProcessor BeanPostProcessors}; + * can follow the {@link #ORIGINAL_INSTANCE_SUFFIX} convention in order to + * enforce the given instance to be returned, i.e. no proxies etc) * @return the bean instance to use, either the original or a wrapped one * @throws BeansException if any post-processing failed * @see BeanPostProcessor#postProcessBeforeInitialization + * @see #ORIGINAL_INSTANCE_SUFFIX */ Object applyBeanPostProcessorsBeforeInitialization(Object existingBean, String beanName) throws BeansException; @@ -288,10 +306,13 @@ public interface AutowireCapableBeanFactory extends BeanFactory { * instance, invoking their {@code postProcessAfterInitialization} methods. * The returned bean instance may be a wrapper around the original. * @param existingBean the new bean instance - * @param beanName the name of the bean + * (only passed to {@link BeanPostProcessor BeanPostProcessors}; + * can follow the {@link #ORIGINAL_INSTANCE_SUFFIX} convention in order to + * enforce the given instance to be returned, i.e. no proxies etc) * @return the bean instance to use, either the original or a wrapped one * @throws BeansException if any post-processing failed * @see BeanPostProcessor#postProcessAfterInitialization + * @see #ORIGINAL_INSTANCE_SUFFIX */ Object applyBeanPostProcessorsAfterInitialization(Object existingBean, String beanName) throws BeansException; diff --git a/spring-test/src/main/java/org/springframework/test/context/support/DependencyInjectionTestExecutionListener.java b/spring-test/src/main/java/org/springframework/test/context/support/DependencyInjectionTestExecutionListener.java index 1c2eac6c83..3f91a4f645 100644 --- a/spring-test/src/main/java/org/springframework/test/context/support/DependencyInjectionTestExecutionListener.java +++ b/spring-test/src/main/java/org/springframework/test/context/support/DependencyInjectionTestExecutionListener.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -49,7 +49,7 @@ public class DependencyInjectionTestExecutionListener extends AbstractTestExecut *

Permissible values include {@link Boolean#TRUE} and {@link Boolean#FALSE}. */ public static final String REINJECT_DEPENDENCIES_ATTRIBUTE = Conventions.getQualifiedAttributeName( - DependencyInjectionTestExecutionListener.class, "reinjectDependencies"); + DependencyInjectionTestExecutionListener.class, "reinjectDependencies"); private static final Log logger = LogFactory.getLog(DependencyInjectionTestExecutionListener.class); @@ -76,7 +76,7 @@ public class DependencyInjectionTestExecutionListener extends AbstractTestExecut * from the test context, regardless of its value. */ @Override - public void prepareTestInstance(final TestContext testContext) throws Exception { + public void prepareTestInstance(TestContext testContext) throws Exception { if (logger.isDebugEnabled()) { logger.debug("Performing dependency injection for test context [" + testContext + "]."); } @@ -91,7 +91,7 @@ public class DependencyInjectionTestExecutionListener extends AbstractTestExecut * otherwise, this method will have no effect. */ @Override - public void beforeTestMethod(final TestContext testContext) throws Exception { + public void beforeTestMethod(TestContext testContext) throws Exception { if (Boolean.TRUE.equals(testContext.getAttribute(REINJECT_DEPENDENCIES_ATTRIBUTE))) { if (logger.isDebugEnabled()) { logger.debug("Reinjecting dependencies for test context [" + testContext + "]."); @@ -112,11 +112,12 @@ public class DependencyInjectionTestExecutionListener extends AbstractTestExecut * @see #prepareTestInstance(TestContext) * @see #beforeTestMethod(TestContext) */ - protected void injectDependencies(final TestContext testContext) throws Exception { + protected void injectDependencies(TestContext testContext) throws Exception { Object bean = testContext.getTestInstance(); + Class clazz = testContext.getTestClass(); AutowireCapableBeanFactory beanFactory = testContext.getApplicationContext().getAutowireCapableBeanFactory(); beanFactory.autowireBeanProperties(bean, AutowireCapableBeanFactory.AUTOWIRE_NO, false); - beanFactory.initializeBean(bean, testContext.getTestClass().getName()); + beanFactory.initializeBean(bean, clazz.getName() + AutowireCapableBeanFactory.ORIGINAL_INSTANCE_SUFFIX); testContext.removeAttribute(REINJECT_DEPENDENCIES_ATTRIBUTE); } diff --git a/spring-test/src/test/java/org/springframework/test/context/groovy/GroovySpringContextTests.java b/spring-test/src/test/java/org/springframework/test/context/groovy/GroovySpringContextTests.java index 4dcebe0ecc..0bdf0499a9 100644 --- a/spring-test/src/test/java/org/springframework/test/context/groovy/GroovySpringContextTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/groovy/GroovySpringContextTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2018 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. @@ -43,13 +43,6 @@ import static org.junit.Assert.*; @ContextConfiguration("context.groovy") public class GroovySpringContextTests implements BeanNameAware, InitializingBean { - private boolean beanInitialized = false; - - private String beanName = "replace me with [" + getClass().getName() + "]"; - - @Autowired - private ApplicationContext applicationContext; - private Employee employee; @Autowired @@ -63,41 +56,49 @@ public class GroovySpringContextTests implements BeanNameAware, InitializingBean protected String bar; + @Autowired + private ApplicationContext applicationContext; + + private String beanName; + + private boolean beanInitialized = false; + @Autowired - protected final void setEmployee(final Employee employee) { + protected void setEmployee(Employee employee) { this.employee = employee; } @Resource - protected final void setBar(final String bar) { + protected void setBar(String bar) { this.bar = bar; } @Override - public final void setBeanName(final String beanName) { + public void setBeanName(String beanName) { this.beanName = beanName; } @Override - public final void afterPropertiesSet() throws Exception { + public void afterPropertiesSet() { this.beanInitialized = true; } + @Test - public final void verifyBeanInitialized() { + public void verifyBeanNameSet() { + assertTrue("The bean name of this test instance should have been set to the fully qualified class name " + + "due to BeanNameAware semantics.", this.beanName.startsWith(getClass().getName())); + } + + @Test + public void verifyBeanInitialized() { assertTrue("This test bean should have been initialized due to InitializingBean semantics.", - this.beanInitialized); + this.beanInitialized); } @Test - public final void verifyBeanNameSet() { - assertEquals("The bean name of this test instance should have been set to the fully qualified class name " - + "due to BeanNameAware semantics.", getClass().getName(), this.beanName); - } - - @Test - public final void verifyAnnotationAutowiredFields() { + public void verifyAnnotationAutowiredFields() { assertNull("The nonrequiredLong property should NOT have been autowired.", this.nonrequiredLong); assertNotNull("The application context should have been autowired.", this.applicationContext); assertNotNull("The pet field should have been autowired.", this.pet); @@ -105,18 +106,18 @@ public class GroovySpringContextTests implements BeanNameAware, InitializingBean } @Test - public final void verifyAnnotationAutowiredMethods() { + public void verifyAnnotationAutowiredMethods() { assertNotNull("The employee setter method should have been autowired.", this.employee); assertEquals("Dilbert", this.employee.getName()); } @Test - public final void verifyResourceAnnotationWiredFields() { + public void verifyResourceAnnotationWiredFields() { assertEquals("The foo field should have been wired via @Resource.", "Foo", this.foo); } @Test - public final void verifyResourceAnnotationWiredMethods() { + public void verifyResourceAnnotationWiredMethods() { assertEquals("The bar method should have been wired via @Resource.", "Bar", this.bar); } diff --git a/spring-test/src/test/java/org/springframework/test/context/junit4/ConcreteTransactionalJUnit4SpringContextTests.java b/spring-test/src/test/java/org/springframework/test/context/junit4/ConcreteTransactionalJUnit4SpringContextTests.java index ecf14e33b8..7bd3b9fef8 100644 --- a/spring-test/src/test/java/org/springframework/test/context/junit4/ConcreteTransactionalJUnit4SpringContextTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/junit4/ConcreteTransactionalJUnit4SpringContextTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2018 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. @@ -51,10 +51,6 @@ public class ConcreteTransactionalJUnit4SpringContextTests extends AbstractTrans private static final String SUE = "sue"; private static final String YODA = "yoda"; - private boolean beanInitialized = false; - - private String beanName = "replace me with [" + getClass().getName() + "]"; - private Employee employee; @Autowired @@ -68,30 +64,70 @@ public class ConcreteTransactionalJUnit4SpringContextTests extends AbstractTrans private String bar; + private String beanName; + + private boolean beanInitialized = false; + @Autowired - private final void setEmployee(final Employee employee) { + private void setEmployee(Employee employee) { this.employee = employee; } @Resource - private final void setBar(final String bar) { + private void setBar(String bar) { this.bar = bar; } @Override - public final void setBeanName(final String beanName) { + public void setBeanName(String beanName) { this.beanName = beanName; } @Override - public final void afterPropertiesSet() throws Exception { + public void afterPropertiesSet() { this.beanInitialized = true; } + + @Before + public void setUp() { + assertEquals("Verifying the number of rows in the person table before a test method.", + (inTransaction() ? 2 : 1), countRowsInPersonTable()); + } + + @After + public void tearDown() { + assertEquals("Verifying the number of rows in the person table after a test method.", + (inTransaction() ? 4 : 1), countRowsInPersonTable()); + } + + @BeforeTransaction + public void beforeTransaction() { + assertEquals("Verifying the number of rows in the person table before a transactional test method.", + 1, countRowsInPersonTable()); + assertEquals("Adding yoda", 1, addPerson(YODA)); + } + + @AfterTransaction + public void afterTransaction() { + assertEquals("Deleting yoda", 1, deletePerson(YODA)); + assertEquals("Verifying the number of rows in the person table after a transactional test method.", + 1, countRowsInPersonTable()); + } + + + @Test + @Transactional(propagation = Propagation.NOT_SUPPORTED) + public void verifyBeanNameSet() { + assertInTransaction(false); + assertTrue("The bean name of this test instance should have been set to the fully qualified class name " + + "due to BeanNameAware semantics.", this.beanName.startsWith(getClass().getName())); + } + @Test @Transactional(propagation = Propagation.NOT_SUPPORTED) - public final void verifyApplicationContext() { + public void verifyApplicationContext() { assertInTransaction(false); assertNotNull("The application context should have been set due to ApplicationContextAware semantics.", super.applicationContext); @@ -99,7 +135,7 @@ public class ConcreteTransactionalJUnit4SpringContextTests extends AbstractTrans @Test @Transactional(propagation = Propagation.NOT_SUPPORTED) - public final void verifyBeanInitialized() { + public void verifyBeanInitialized() { assertInTransaction(false); assertTrue("This test bean should have been initialized due to InitializingBean semantics.", this.beanInitialized); @@ -107,15 +143,7 @@ public class ConcreteTransactionalJUnit4SpringContextTests extends AbstractTrans @Test @Transactional(propagation = Propagation.NOT_SUPPORTED) - public final void verifyBeanNameSet() { - assertInTransaction(false); - assertEquals("The bean name of this test instance should have been set to the fully qualified class name " - + "due to BeanNameAware semantics.", getClass().getName(), this.beanName); - } - - @Test - @Transactional(propagation = Propagation.NOT_SUPPORTED) - public final void verifyAnnotationAutowiredFields() { + public void verifyAnnotationAutowiredFields() { assertInTransaction(false); assertNull("The nonrequiredLong property should NOT have been autowired.", this.nonrequiredLong); assertNotNull("The pet field should have been autowired.", this.pet); @@ -124,7 +152,7 @@ public class ConcreteTransactionalJUnit4SpringContextTests extends AbstractTrans @Test @Transactional(propagation = Propagation.NOT_SUPPORTED) - public final void verifyAnnotationAutowiredMethods() { + public void verifyAnnotationAutowiredMethods() { assertInTransaction(false); assertNotNull("The employee setter method should have been autowired.", this.employee); assertEquals("John Smith", this.employee.getName()); @@ -132,58 +160,33 @@ public class ConcreteTransactionalJUnit4SpringContextTests extends AbstractTrans @Test @Transactional(propagation = Propagation.NOT_SUPPORTED) - public final void verifyResourceAnnotationWiredFields() { + public void verifyResourceAnnotationWiredFields() { assertInTransaction(false); assertEquals("The foo field should have been wired via @Resource.", "Foo", this.foo); } @Test @Transactional(propagation = Propagation.NOT_SUPPORTED) - public final void verifyResourceAnnotationWiredMethods() { + public void verifyResourceAnnotationWiredMethods() { assertInTransaction(false); assertEquals("The bar method should have been wired via @Resource.", "Bar", this.bar); } - @BeforeTransaction - public void beforeTransaction() { - assertEquals("Verifying the number of rows in the person table before a transactional test method.", 1, - countRowsInPersonTable()); - assertEquals("Adding yoda", 1, addPerson(YODA)); - } - - @Before - public void setUp() throws Exception { - assertEquals("Verifying the number of rows in the person table before a test method.", - (inTransaction() ? 2 : 1), countRowsInPersonTable()); - } - @Test public void modifyTestDataWithinTransaction() { assertInTransaction(true); assertEquals("Adding jane", 1, addPerson(JANE)); assertEquals("Adding sue", 1, addPerson(SUE)); - assertEquals("Verifying the number of rows in the person table in modifyTestDataWithinTransaction().", 4, - countRowsInPersonTable()); + assertEquals("Verifying the number of rows in the person table in modifyTestDataWithinTransaction().", + 4, countRowsInPersonTable()); } - @After - public void tearDown() throws Exception { - assertEquals("Verifying the number of rows in the person table after a test method.", - (inTransaction() ? 4 : 1), countRowsInPersonTable()); - } - @AfterTransaction - public void afterTransaction() { - assertEquals("Deleting yoda", 1, deletePerson(YODA)); - assertEquals("Verifying the number of rows in the person table after a transactional test method.", 1, - countRowsInPersonTable()); - } - - private int addPerson(final String name) { + private int addPerson(String name) { return super.jdbcTemplate.update("INSERT INTO person VALUES(?)", name); } - private int deletePerson(final String name) { + private int deletePerson(String name) { return super.jdbcTemplate.update("DELETE FROM person WHERE name=?", name); } diff --git a/spring-test/src/test/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunnerAppCtxTests.java b/spring-test/src/test/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunnerAppCtxTests.java index 057db58af7..71a448cd4c 100644 --- a/spring-test/src/test/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunnerAppCtxTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/junit4/SpringJUnit4ClassRunnerAppCtxTests.java @@ -23,7 +23,6 @@ import javax.inject.Named; import org.junit.Test; import org.junit.runner.RunWith; -import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanNameAware; import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.annotation.Autowired; @@ -82,13 +81,9 @@ public class SpringJUnit4ClassRunnerAppCtxTests implements ApplicationContextAwa * Default resource path for the application context configuration for * {@link SpringJUnit4ClassRunnerAppCtxTests}: {@value #DEFAULT_CONTEXT_RESOURCE_PATH} */ - public static final String DEFAULT_CONTEXT_RESOURCE_PATH = "/org/springframework/test/context/junit4/SpringJUnit4ClassRunnerAppCtxTests-context.xml"; + public static final String DEFAULT_CONTEXT_RESOURCE_PATH = + "/org/springframework/test/context/junit4/SpringJUnit4ClassRunnerAppCtxTests-context.xml"; - private ApplicationContext applicationContext; - - private boolean beanInitialized = false; - - private String beanName = "replace me with [" + getClass().getName() + "]"; private Employee employee; @@ -124,31 +119,20 @@ public class SpringJUnit4ClassRunnerAppCtxTests implements ApplicationContextAwa @Named("quux") protected String namedQuux; + private String beanName; - // ------------------------------------------------------------------------| + private ApplicationContext applicationContext; - @Override - public final void afterPropertiesSet() throws Exception { - this.beanInitialized = true; - } + private boolean beanInitialized = false; - @Override - public final void setApplicationContext(final ApplicationContext applicationContext) throws BeansException { - this.applicationContext = applicationContext; - } - - @Override - public final void setBeanName(final String beanName) { - this.beanName = beanName; - } @Autowired - protected final void setEmployee(final Employee employee) { + protected void setEmployee(Employee employee) { this.employee = employee; } @Resource - protected final void setBar(final String bar) { + protected void setBar(String bar) { this.bar = bar; } @@ -162,33 +146,46 @@ public class SpringJUnit4ClassRunnerAppCtxTests implements ApplicationContextAwa this.spelParameterValue = spelParameterValue; } - // ------------------------------------------------------------------------| + @Override + public void setBeanName(String beanName) { + this.beanName = beanName; + } + + @Override + public void setApplicationContext(ApplicationContext applicationContext) { + this.applicationContext = applicationContext; + } + + @Override + public void afterPropertiesSet() { + this.beanInitialized = true; + } + @Test - public final void verifyApplicationContextSet() { + public void verifyBeanNameSet() { + assertTrue("The bean name of this test instance should have been set due to BeanNameAware semantics.", + this.beanName.startsWith(getClass().getName())); + } + + @Test + public void verifyApplicationContextSet() { assertNotNull("The application context should have been set due to ApplicationContextAware semantics.", - this.applicationContext); + this.applicationContext); } @Test - public final void verifyBeanInitialized() { + public void verifyBeanInitialized() { assertTrue("This test bean should have been initialized due to InitializingBean semantics.", - this.beanInitialized); + this.beanInitialized); } @Test - public final void verifyBeanNameSet() { - assertEquals("The bean name of this test instance should have been set due to BeanNameAware semantics.", - getClass().getName(), this.beanName); - } - - @Test - public final void verifyAnnotationAutowiredAndInjectedFields() { + public void verifyAnnotationAutowiredAndInjectedFields() { assertNull("The nonrequiredLong field should NOT have been autowired.", this.nonrequiredLong); assertEquals("The quux field should have been autowired via @Autowired and @Qualifier.", "Quux", this.quux); assertEquals("The namedFoo field should have been injected via @Inject and @Named.", "Quux", this.namedQuux); - assertSame("@Autowired/@Qualifier and @Inject/@Named quux should be the same object.", this.quux, - this.namedQuux); + assertSame("@Autowired/@Qualifier and @Inject/@Named quux should be the same object.", this.quux, this.namedQuux); assertNotNull("The pet field should have been autowired.", this.autowiredPet); assertNotNull("The pet field should have been injected.", this.injectedPet); @@ -198,13 +195,13 @@ public class SpringJUnit4ClassRunnerAppCtxTests implements ApplicationContextAwa } @Test - public final void verifyAnnotationAutowiredMethods() { + public void verifyAnnotationAutowiredMethods() { assertNotNull("The employee setter method should have been autowired.", this.employee); assertEquals("John Smith", this.employee.getName()); } @Test - public final void verifyAutowiredAtValueFields() { + public void verifyAutowiredAtValueFields() { assertNotNull("Literal @Value field should have been autowired", this.literalFieldValue); assertNotNull("SpEL @Value field should have been autowired.", this.spelFieldValue); assertEquals("enigma", this.literalFieldValue); @@ -212,7 +209,7 @@ public class SpringJUnit4ClassRunnerAppCtxTests implements ApplicationContextAwa } @Test - public final void verifyAutowiredAtValueMethods() { + public void verifyAutowiredAtValueMethods() { assertNotNull("Literal @Value method parameter should have been autowired.", this.literalParameterValue); assertNotNull("SpEL @Value method parameter should have been autowired.", this.spelParameterValue); assertEquals("enigma", this.literalParameterValue); @@ -220,12 +217,12 @@ public class SpringJUnit4ClassRunnerAppCtxTests implements ApplicationContextAwa } @Test - public final void verifyResourceAnnotationInjectedFields() { + public void verifyResourceAnnotationInjectedFields() { assertEquals("The foo field should have been injected via @Resource.", "Foo", this.foo); } @Test - public final void verifyResourceAnnotationInjectedMethods() { + public void verifyResourceAnnotationInjectedMethods() { assertEquals("The bar method should have been wired via @Resource.", "Bar", this.bar); } diff --git a/spring-test/src/test/java/org/springframework/test/context/testng/ConcreteTransactionalTestNGSpringContextTests.java b/spring-test/src/test/java/org/springframework/test/context/testng/ConcreteTransactionalTestNGSpringContextTests.java index d249fc38d9..2405d1ef4d 100644 --- a/spring-test/src/test/java/org/springframework/test/context/testng/ConcreteTransactionalTestNGSpringContextTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/testng/ConcreteTransactionalTestNGSpringContextTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -61,9 +61,6 @@ public class ConcreteTransactionalTestNGSpringContextTests extends AbstractTrans private static int numTearDownCalls = 0; private static int numTearDownCallsInTransaction = 0; - private boolean beanInitialized = false; - - private String beanName = "replace me with [" + getClass().getName() + "]"; private Employee employee; @@ -71,25 +68,26 @@ public class ConcreteTransactionalTestNGSpringContextTests extends AbstractTrans private Pet pet; @Autowired(required = false) - protected Long nonrequiredLong; + private Long nonrequiredLong; - @Resource() - protected String foo; + @Resource + private String foo; - protected String bar; + private String bar; + + private String beanName; + + private boolean beanInitialized = false; - private int createPerson(String name) { - return jdbcTemplate.update("INSERT INTO person VALUES(?)", name); + @Autowired + private void setEmployee(Employee employee) { + this.employee = employee; } - private int deletePerson(String name) { - return jdbcTemplate.update("DELETE FROM person WHERE name=?", name); - } - - @Override - public void afterPropertiesSet() throws Exception { - this.beanInitialized = true; + @Resource + private void setBar(String bar) { + this.bar = bar; } @Override @@ -97,24 +95,11 @@ public class ConcreteTransactionalTestNGSpringContextTests extends AbstractTrans this.beanName = beanName; } - @Autowired - protected void setEmployee(Employee employee) { - this.employee = employee; + @Override + public void afterPropertiesSet() { + this.beanInitialized = true; } - @Resource - protected void setBar(String bar) { - this.bar = bar; - } - - private void assertNumRowsInPersonTable(int expectedNumRows, String testState) { - assertEquals(countRowsInTable("person"), expectedNumRows, "the number of rows in the person table (" - + testState + ")."); - } - - private void assertAddPerson(final String name) { - assertEquals(createPerson(name), 1, "Adding '" + name + "'"); - } @BeforeClass void beforeClass() { @@ -132,12 +117,51 @@ public class ConcreteTransactionalTestNGSpringContextTests extends AbstractTrans assertEquals(numTearDownCallsInTransaction, NUM_TX_TESTS, "number of calls to tearDown() within a transaction."); } + @BeforeMethod + void setUp() { + numSetUpCalls++; + if (inTransaction()) { + numSetUpCallsInTransaction++; + } + assertNumRowsInPersonTable((inTransaction() ? 2 : 1), "before a test method"); + } + + @AfterMethod + void tearDown() { + numTearDownCalls++; + if (inTransaction()) { + numTearDownCallsInTransaction++; + } + assertNumRowsInPersonTable((inTransaction() ? 4 : 1), "after a test method"); + } + + @BeforeTransaction + void beforeTransaction() { + assertNumRowsInPersonTable(1, "before a transactional test method"); + assertAddPerson(YODA); + } + + @AfterTransaction + void afterTransaction() { + assertEquals(deletePerson(YODA), 1, "Deleting yoda"); + assertNumRowsInPersonTable(1, "after a transactional test method"); + } + + + @Test + @Transactional(propagation = Propagation.NOT_SUPPORTED) + void verifyBeanNameSet() { + assertInTransaction(false); + assertTrue(this.beanName.startsWith(getClass().getName()), "The bean name of this test instance " + + "should have been set to the fully qualified class name due to BeanNameAware semantics."); + } + @Test @Transactional(propagation = Propagation.NOT_SUPPORTED) void verifyApplicationContextSet() { assertInTransaction(false); assertNotNull(super.applicationContext, - "The application context should have been set due to ApplicationContextAware semantics."); + "The application context should have been set due to ApplicationContextAware semantics."); Employee employeeBean = (Employee) super.applicationContext.getBean("employee"); assertEquals(employeeBean.getName(), "John Smith", "employee's name."); } @@ -147,15 +171,7 @@ public class ConcreteTransactionalTestNGSpringContextTests extends AbstractTrans void verifyBeanInitialized() { assertInTransaction(false); assertTrue(beanInitialized, - "This test instance should have been initialized due to InitializingBean semantics."); - } - - @Test - @Transactional(propagation = Propagation.NOT_SUPPORTED) - void verifyBeanNameSet() { - assertInTransaction(false); - assertEquals(beanName, getClass().getName(), - "The bean name of this test instance should have been set due to BeanNameAware semantics."); + "This test instance should have been initialized due to InitializingBean semantics."); } @Test @@ -189,21 +205,6 @@ public class ConcreteTransactionalTestNGSpringContextTests extends AbstractTrans assertEquals(bar, "Bar", "The setBar() method should have been injected via @Resource."); } - @BeforeTransaction - void beforeTransaction() { - assertNumRowsInPersonTable(1, "before a transactional test method"); - assertAddPerson(YODA); - } - - @BeforeMethod - void setUp() throws Exception { - numSetUpCalls++; - if (inTransaction()) { - numSetUpCallsInTransaction++; - } - assertNumRowsInPersonTable((inTransaction() ? 2 : 1), "before a test method"); - } - @Test void modifyTestDataWithinTransaction() { assertInTransaction(true); @@ -212,19 +213,22 @@ public class ConcreteTransactionalTestNGSpringContextTests extends AbstractTrans assertNumRowsInPersonTable(4, "in modifyTestDataWithinTransaction()"); } - @AfterMethod - void tearDown() throws Exception { - numTearDownCalls++; - if (inTransaction()) { - numTearDownCallsInTransaction++; - } - assertNumRowsInPersonTable((inTransaction() ? 4 : 1), "after a test method"); + + private int createPerson(String name) { + return jdbcTemplate.update("INSERT INTO person VALUES(?)", name); } - @AfterTransaction - void afterTransaction() { - assertEquals(deletePerson(YODA), 1, "Deleting yoda"); - assertNumRowsInPersonTable(1, "after a transactional test method"); + private int deletePerson(String name) { + return jdbcTemplate.update("DELETE FROM person WHERE name=?", name); + } + + private void assertNumRowsInPersonTable(int expectedNumRows, String testState) { + assertEquals(countRowsInTable("person"), expectedNumRows, + "the number of rows in the person table (" + testState + ")."); + } + + private void assertAddPerson(String name) { + assertEquals(createPerson(name), 1, "Adding '" + name + "'"); } } diff --git a/spring-test/src/test/java/org/springframework/test/context/transaction/PrimaryTransactionManagerTests.java b/spring-test/src/test/java/org/springframework/test/context/transaction/PrimaryTransactionManagerTests.java index dabedc902e..7df2aca374 100644 --- a/spring-test/src/test/java/org/springframework/test/context/transaction/PrimaryTransactionManagerTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/transaction/PrimaryTransactionManagerTests.java @@ -36,6 +36,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.jdbc.JdbcTestUtils; import org.springframework.test.transaction.TransactionTestUtils; import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.annotation.EnableTransactionManagement; import org.springframework.transaction.annotation.Transactional; import static org.junit.Assert.*; @@ -51,7 +52,7 @@ import static org.junit.Assert.*; @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration @DirtiesContext -public class PrimaryTransactionManagerTests { +public final class PrimaryTransactionManagerTests { private JdbcTemplate jdbcTemplate; @@ -90,6 +91,7 @@ public class PrimaryTransactionManagerTests { @Configuration + @EnableTransactionManagement // SPR-17137: should not break trying to proxy the final test class static class Config { @Primary