Skip to content

Commit cec26e9

Browse files
snicollsnicoll
committed
Rework determineTransactionManager condition
SPR-11954 introduced a regression that when the "default" transaction manager is cached, qualified transaction managers are not taken into account anymore. This commit rework the "determineTransactionManager" condition to favor qualifier and "named" transaction managers. If none of these apply, the default transaction manager is used as it should. Also reworked the caching infrastructure so that a single cache holds all transaction manager instances. Issue: SPR-12541
1 parent 4308a04 commit cec26e9

File tree

2 files changed

+105
-41
lines changed

2 files changed

+105
-41
lines changed

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

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@
6767
*/
6868
public abstract class TransactionAspectSupport implements BeanFactoryAware, InitializingBean {
6969

70+
/**
71+
* Key to use to store the default transaction manager.
72+
*/
73+
private final Object DEFAULT_TRANSACTION_MANAGER_KEY = new Object();
74+
7075
// NOTE: This class must not implement Serializable because it serves as base
7176
// class for AspectJ aspects (which are not allowed to implement Serializable)!
7277

@@ -80,8 +85,8 @@ public abstract class TransactionAspectSupport implements BeanFactoryAware, Init
8085
new NamedThreadLocal<TransactionInfo>("Current aspect-driven transaction");
8186

8287

83-
private final ConcurrentHashMap<String, PlatformTransactionManager> transactionManagerCache =
84-
new ConcurrentHashMap<String, PlatformTransactionManager>();
88+
private final ConcurrentHashMap<Object, PlatformTransactionManager> transactionManagerCache =
89+
new ConcurrentHashMap<Object, PlatformTransactionManager>();
8590

8691
/**
8792
* Subclasses can use this to return the current TransactionInfo.
@@ -127,11 +132,6 @@ public static TransactionStatus currentTransactionStatus() throws NoTransactionE
127132
*/
128133
private String transactionManagerBeanName;
129134

130-
/**
131-
* Default transaction manager.
132-
*/
133-
private PlatformTransactionManager transactionManager;
134-
135135
private TransactionAttributeSource transactionAttributeSource;
136136

137137
private BeanFactory beanFactory;
@@ -159,14 +159,16 @@ protected final String getTransactionManagerBeanName() {
159159
* @see #setTransactionManagerBeanName
160160
*/
161161
public void setTransactionManager(PlatformTransactionManager transactionManager) {
162-
this.transactionManager = transactionManager;
162+
if (transactionManager != null) {
163+
this.transactionManagerCache.put(DEFAULT_TRANSACTION_MANAGER_KEY, transactionManager);
164+
}
163165
}
164166

165167
/**
166168
* Return the default transaction manager, or {@code null} if unknown.
167169
*/
168170
public PlatformTransactionManager getTransactionManager() {
169-
return this.transactionManager;
171+
return this.transactionManagerCache.get(DEFAULT_TRANSACTION_MANAGER_KEY);
170172
}
171173

172174
/**
@@ -239,7 +241,7 @@ protected final BeanFactory getBeanFactory() {
239241
*/
240242
@Override
241243
public void afterPropertiesSet() {
242-
if (this.transactionManager == null && this.beanFactory == null) {
244+
if (getTransactionManager() == null && this.beanFactory == null) {
243245
throw new IllegalStateException(
244246
"Setting the property 'transactionManager' or running in a ListableBeanFactory is required");
245247
}
@@ -340,33 +342,36 @@ public Object doInTransaction(TransactionStatus status) {
340342
* Determine the specific transaction manager to use for the given transaction.
341343
*/
342344
protected PlatformTransactionManager determineTransactionManager(TransactionAttribute txAttr) {
343-
if (this.transactionManager != null || this.beanFactory == null || txAttr == null) {
344-
return this.transactionManager;
345-
}
346-
String qualifier = txAttr.getQualifier();
347-
if (StringUtils.hasText(qualifier)) {
348-
PlatformTransactionManager txManager = this.transactionManagerCache.get(qualifier);
349-
if (txManager == null) {
350-
txManager = BeanFactoryAnnotationUtils.qualifiedBeanOfType(
351-
this.beanFactory, PlatformTransactionManager.class, qualifier);
352-
this.transactionManagerCache.putIfAbsent(qualifier, txManager);
345+
if (this.beanFactory != null) {
346+
String qualifier = txAttr != null ? txAttr.getQualifier() : null;
347+
if (StringUtils.hasText(qualifier)) {
348+
PlatformTransactionManager txManager = this.transactionManagerCache.get(qualifier);
349+
if (txManager == null) {
350+
txManager = BeanFactoryAnnotationUtils.qualifiedBeanOfType(
351+
this.beanFactory, PlatformTransactionManager.class, qualifier);
352+
this.transactionManagerCache.putIfAbsent(qualifier, txManager);
353+
}
354+
return txManager;
353355
}
354-
return txManager;
355-
}
356-
else if (StringUtils.hasText(this.transactionManagerBeanName)) {
357-
PlatformTransactionManager txManager = this.transactionManagerCache.get(this.transactionManagerBeanName);
358-
if (txManager == null) {
359-
txManager = this.beanFactory.getBean(
360-
this.transactionManagerBeanName, PlatformTransactionManager.class);
361-
this.transactionManagerCache.putIfAbsent(this.transactionManagerBeanName, txManager);
356+
else if (StringUtils.hasText(this.transactionManagerBeanName)) {
357+
PlatformTransactionManager txManager = this.transactionManagerCache.get(this.transactionManagerBeanName);
358+
if (txManager == null) {
359+
txManager = this.beanFactory.getBean(
360+
this.transactionManagerBeanName, PlatformTransactionManager.class);
361+
this.transactionManagerCache.putIfAbsent(this.transactionManagerBeanName, txManager);
362+
}
363+
return txManager;
364+
} else {
365+
PlatformTransactionManager defaultTransactionManager = getTransactionManager();
366+
if (defaultTransactionManager == null) {
367+
defaultTransactionManager = this.beanFactory.getBean(PlatformTransactionManager.class);
368+
this.transactionManagerCache.putIfAbsent(
369+
DEFAULT_TRANSACTION_MANAGER_KEY, defaultTransactionManager);
370+
}
371+
return defaultTransactionManager;
362372
}
363-
return txManager;
364-
}
365-
else {
366-
// Look up the default transaction manager and cache it for subsequent calls
367-
this.transactionManager = this.beanFactory.getBean(PlatformTransactionManager.class);
368-
return this.transactionManager;
369373
}
374+
return getTransactionManager();
370375
}
371376

372377
/**

‎spring-tx/src/test/java/org/springframework/transaction/interceptor/TransactionInterceptorTests.java

Lines changed: 66 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,22 @@ public void serializableWithCompositeSource() throws Exception {
123123
assertTrue(ctas.getTransactionAttributeSources()[1] instanceof NameMatchTransactionAttributeSource);
124124
}
125125

126+
@Test
127+
public void determineTransactionManagerWithNoBeanFactory() {
128+
PlatformTransactionManager transactionManager = mock(PlatformTransactionManager.class);
129+
TransactionInterceptor ti = createTestTransactionInterceptor(null, transactionManager);
130+
131+
assertSame(transactionManager, ti.determineTransactionManager(new DefaultTransactionAttribute()));
132+
}
133+
134+
@Test
135+
public void determineTransactionManagerWithNoBeanFactoryAndNoTransactionAttribute() {
136+
PlatformTransactionManager transactionManager = mock(PlatformTransactionManager.class);
137+
TransactionInterceptor ti = createTestTransactionInterceptor(null, transactionManager);
138+
139+
assertSame(transactionManager, ti.determineTransactionManager(null));
140+
}
141+
126142
@Test
127143
public void determineTransactionManagerWithQualifierUnknown() {
128144
BeanFactory beanFactory = mock(BeanFactory.class);
@@ -135,14 +151,41 @@ public void determineTransactionManagerWithQualifierUnknown() {
135151
ti.determineTransactionManager(attribute);
136152
}
137153

154+
@Test
155+
public void determineTransactionManagerWithQualifierAndDefault() {
156+
BeanFactory beanFactory = mock(BeanFactory.class);
157+
PlatformTransactionManager transactionManager = mock(PlatformTransactionManager.class);
158+
TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory, transactionManager);
159+
PlatformTransactionManager fooTransactionManager =
160+
associateTransactionManager(beanFactory, "fooTransactionManager");
161+
162+
DefaultTransactionAttribute attribute = new DefaultTransactionAttribute();
163+
attribute.setQualifier("fooTransactionManager");
164+
165+
assertSame(fooTransactionManager, ti.determineTransactionManager(attribute));
166+
}
167+
168+
@Test
169+
public void determineTransactionManagerWithQualifierAndDefaultName() {
170+
BeanFactory beanFactory = mock(BeanFactory.class);
171+
associateTransactionManager(beanFactory, "defaultTransactionManager");
172+
TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory);
173+
ti.setTransactionManagerBeanName("defaultTransactionManager");
174+
175+
PlatformTransactionManager fooTransactionManager =
176+
associateTransactionManager(beanFactory, "fooTransactionManager");
177+
DefaultTransactionAttribute attribute = new DefaultTransactionAttribute();
178+
attribute.setQualifier("fooTransactionManager");
179+
180+
assertSame(fooTransactionManager, ti.determineTransactionManager(attribute));
181+
}
182+
138183
@Test
139184
public void determineTransactionManagerWithQualifierSeveralTimes() {
140185
BeanFactory beanFactory = mock(BeanFactory.class);
141186
TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory);
142187

143-
PlatformTransactionManager txManager = mock(PlatformTransactionManager.class);
144-
given(beanFactory.containsBean("fooTransactionManager")).willReturn(true);
145-
given(beanFactory.getBean("fooTransactionManager", PlatformTransactionManager.class)).willReturn(txManager);
188+
PlatformTransactionManager txManager = associateTransactionManager(beanFactory, "fooTransactionManager");
146189

147190
DefaultTransactionAttribute attribute = new DefaultTransactionAttribute();
148191
attribute.setQualifier("fooTransactionManager");
@@ -162,8 +205,7 @@ public void determineTransactionManagerWithBeanNameSeveralTimes() {
162205
TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory);
163206
ti.setTransactionManagerBeanName("fooTransactionManager");
164207

165-
PlatformTransactionManager txManager = mock(PlatformTransactionManager.class);
166-
given(beanFactory.getBean("fooTransactionManager", PlatformTransactionManager.class)).willReturn(txManager);
208+
PlatformTransactionManager txManager = associateTransactionManager(beanFactory, "fooTransactionManager");
167209

168210
DefaultTransactionAttribute attribute = new DefaultTransactionAttribute();
169211
PlatformTransactionManager actual = ti.determineTransactionManager(attribute);
@@ -193,14 +235,31 @@ public void determineTransactionManagerDefaultSeveralTimes() {
193235
verify(beanFactory, times(1)).getBean(PlatformTransactionManager.class);
194236
}
195237

196-
private TransactionInterceptor createTestTransactionInterceptor(BeanFactory beanFactory) {
238+
private TransactionInterceptor createTestTransactionInterceptor(BeanFactory beanFactory,
239+
PlatformTransactionManager transactionManager) {
197240
TransactionInterceptor ti = new TransactionInterceptor();
198-
ti.setBeanFactory(beanFactory);
241+
if (beanFactory != null) {
242+
ti.setBeanFactory(beanFactory);
243+
}
244+
if (transactionManager != null) {
245+
ti.setTransactionManager(transactionManager);
246+
}
199247
ti.setTransactionAttributeSource(new NameMatchTransactionAttributeSource());
200248
ti.afterPropertiesSet();
201249
return ti;
202250
}
203251

252+
private TransactionInterceptor createTestTransactionInterceptor(BeanFactory beanFactory) {
253+
return createTestTransactionInterceptor(beanFactory, null);
254+
}
255+
256+
private PlatformTransactionManager associateTransactionManager(BeanFactory beanFactory, String name) {
257+
PlatformTransactionManager transactionManager = mock(PlatformTransactionManager.class);
258+
given(beanFactory.containsBean(name)).willReturn(true);
259+
given(beanFactory.getBean(name, PlatformTransactionManager.class)).willReturn(transactionManager);
260+
return transactionManager;
261+
}
262+
204263

205264
/**
206265
* We won't use this: we just want to know it's serializable.

0 commit comments

Comments
 (0)