Restore default transaction manager by name lookup

Fix a regression introduced by 961574b that prevents a proper lookup
of the default transaction manager by name as the absence of a qualifier
is represented by an empty string (passing the faulty null check).

Issue: SPR-12577

Author: snicoll

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

@@ -353,16 +353,12 @@ protected PlatformTransactionManager determineTransactionManager(TransactionAttr
 		if (txAttr == null || this.beanFactory == null) {
 			return getTransactionManager();
 		}
-		String qualifier = (txAttr.getQualifier() != null ?
-				txAttr.getQualifier() : this.transactionManagerBeanName);
+		String qualifier = txAttr.getQualifier();
 		if (StringUtils.hasText(qualifier)) {
-			PlatformTransactionManager txManager = this.transactionManagerCache.get(qualifier);
-			if (txManager == null) {
-				txManager = BeanFactoryAnnotationUtils.qualifiedBeanOfType(
-						this.beanFactory, PlatformTransactionManager.class, qualifier);
-				this.transactionManagerCache.putIfAbsent(qualifier, txManager);
-			}
-			return txManager;
+			return determineQualifiedTransactionManager(qualifier);
+		}
+		else if (StringUtils.hasText(this.transactionManagerBeanName)) {
+			return determineQualifiedTransactionManager(this.transactionManagerBeanName);
 		}
 		else {
 			PlatformTransactionManager defaultTransactionManager = getTransactionManager();
@@ -375,6 +371,16 @@ protected PlatformTransactionManager determineTransactionManager(TransactionAttr
 		}
 	}
 
+	private PlatformTransactionManager determineQualifiedTransactionManager(String qualifier) {
+		PlatformTransactionManager txManager = this.transactionManagerCache.get(qualifier);
+		if (txManager == null) {
+			txManager = BeanFactoryAnnotationUtils.qualifiedBeanOfType(
+					this.beanFactory, PlatformTransactionManager.class, qualifier);
+			this.transactionManagerCache.putIfAbsent(qualifier, txManager);
+		}
+		return txManager;
+	}
+
 	/**
 	 * Convenience method to return a String representation of this Method
 	 * for use in logging. Can be overridden in subclasses to provide a

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

@@ -126,31 +126,31 @@ public void serializableWithCompositeSource() throws Exception {
 	@Test
 	public void determineTransactionManagerWithNoBeanFactory() {
 		PlatformTransactionManager transactionManager = mock(PlatformTransactionManager.class);
-		TransactionInterceptor ti = createTestTransactionInterceptor(null, transactionManager);
+		TransactionInterceptor ti = transactionInterceptorWithTransactionManager(transactionManager, null);
 
 		assertSame(transactionManager, ti.determineTransactionManager(new DefaultTransactionAttribute()));
 	}
 
 	@Test
 	public void determineTransactionManagerWithNoBeanFactoryAndNoTransactionAttribute() {
 		PlatformTransactionManager transactionManager = mock(PlatformTransactionManager.class);
-		TransactionInterceptor ti = createTestTransactionInterceptor(null, transactionManager);
+		TransactionInterceptor ti = transactionInterceptorWithTransactionManager(transactionManager, null);
 
 		assertSame(transactionManager, ti.determineTransactionManager(null));
 	}
 
 	@Test
 	public void determineTransactionManagerWithNoTransactionAttribute() {
 		BeanFactory beanFactory = mock(BeanFactory.class);
-		TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory, null);
+		TransactionInterceptor ti = simpleTransactionInterceptor(beanFactory);
 
 		assertNull(ti.determineTransactionManager(null));
 	}
 
 	@Test
 	public void determineTransactionManagerWithQualifierUnknown() {
 		BeanFactory beanFactory = mock(BeanFactory.class);
-		TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory);
+		TransactionInterceptor ti = simpleTransactionInterceptor(beanFactory);
 		DefaultTransactionAttribute attribute = new DefaultTransactionAttribute();
 		attribute.setQualifier("fooTransactionManager");
 
@@ -163,7 +163,7 @@ public void determineTransactionManagerWithQualifierUnknown() {
 	public void determineTransactionManagerWithQualifierAndDefault() {
 		BeanFactory beanFactory = mock(BeanFactory.class);
 		PlatformTransactionManager transactionManager = mock(PlatformTransactionManager.class);
-		TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory, transactionManager);
+		TransactionInterceptor ti = transactionInterceptorWithTransactionManager(transactionManager, beanFactory);
 		PlatformTransactionManager fooTransactionManager =
 				associateTransactionManager(beanFactory, "fooTransactionManager");
 
@@ -177,8 +177,8 @@ public void determineTransactionManagerWithQualifierAndDefault() {
 	public void determineTransactionManagerWithQualifierAndDefaultName() {
 		BeanFactory beanFactory = mock(BeanFactory.class);
 		associateTransactionManager(beanFactory, "defaultTransactionManager");
-		TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory);
-		ti.setTransactionManagerBeanName("defaultTransactionManager");
+		TransactionInterceptor ti = transactionInterceptorWithTransactionManagerName(
+				"defaultTransactionManager", beanFactory);
 
 		PlatformTransactionManager fooTransactionManager =
 				associateTransactionManager(beanFactory, "fooTransactionManager");
@@ -188,10 +188,24 @@ public void determineTransactionManagerWithQualifierAndDefaultName() {
 		assertSame(fooTransactionManager, ti.determineTransactionManager(attribute));
 	}
 
+	@Test
+	public void determineTransactionManagerWithEmptyQualifierAndDefaultName() {
+		BeanFactory beanFactory = mock(BeanFactory.class);
+		PlatformTransactionManager defaultTransactionManager
+				= associateTransactionManager(beanFactory, "defaultTransactionManager");
+		TransactionInterceptor ti = transactionInterceptorWithTransactionManagerName(
+				"defaultTransactionManager", beanFactory);
+
+		DefaultTransactionAttribute attribute = new DefaultTransactionAttribute();
+		attribute.setQualifier("");
+
+		assertSame(defaultTransactionManager, ti.determineTransactionManager(attribute));
+	}
+
 	@Test
 	public void determineTransactionManagerWithQualifierSeveralTimes() {
 		BeanFactory beanFactory = mock(BeanFactory.class);
-		TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory);
+		TransactionInterceptor ti = simpleTransactionInterceptor(beanFactory);
 
 		PlatformTransactionManager txManager = associateTransactionManager(beanFactory, "fooTransactionManager");
 
@@ -210,8 +224,8 @@ public void determineTransactionManagerWithQualifierSeveralTimes() {
 	@Test
 	public void determineTransactionManagerWithBeanNameSeveralTimes() {
 		BeanFactory beanFactory = mock(BeanFactory.class);
-		TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory);
-		ti.setTransactionManagerBeanName("fooTransactionManager");
+		TransactionInterceptor ti = transactionInterceptorWithTransactionManagerName(
+				"fooTransactionManager", beanFactory);
 
 		PlatformTransactionManager txManager = 	associateTransactionManager(beanFactory, "fooTransactionManager");
 
@@ -228,7 +242,7 @@ public void determineTransactionManagerWithBeanNameSeveralTimes() {
 	@Test
 	public void determineTransactionManagerDefaultSeveralTimes() {
 		BeanFactory beanFactory = mock(BeanFactory.class);
-		TransactionInterceptor ti = createTestTransactionInterceptor(beanFactory);
+		TransactionInterceptor ti = simpleTransactionInterceptor(beanFactory);
 
 		PlatformTransactionManager txManager = mock(PlatformTransactionManager.class);
 		given(beanFactory.getBean(PlatformTransactionManager.class)).willReturn(txManager);
@@ -243,12 +257,16 @@ public void determineTransactionManagerDefaultSeveralTimes() {
 		verify(beanFactory, times(1)).getBean(PlatformTransactionManager.class);
 	}
 
-	private TransactionInterceptor createTestTransactionInterceptor(BeanFactory beanFactory,
-			PlatformTransactionManager transactionManager) {
+	private TransactionInterceptor createTransactionInterceptor(BeanFactory beanFactory,
+			String transactionManagerName, PlatformTransactionManager transactionManager) {
 		TransactionInterceptor ti = new TransactionInterceptor();
 		if (beanFactory != null) {
 			ti.setBeanFactory(beanFactory);
 		}
+		if (transactionManagerName != null) {
+			ti.setTransactionManagerBeanName(transactionManagerName);
+
+		}
 		if (transactionManager != null) {
 			ti.setTransactionManager(transactionManager);
 		}
@@ -257,8 +275,20 @@ private TransactionInterceptor createTestTransactionInterceptor(BeanFactory bean
 		return ti;
 	}
 
-	private TransactionInterceptor createTestTransactionInterceptor(BeanFactory beanFactory) {
-		return createTestTransactionInterceptor(beanFactory, null);
+	private TransactionInterceptor transactionInterceptorWithTransactionManager(
+			PlatformTransactionManager transactionManager, BeanFactory beanFactory) {
+
+		return createTransactionInterceptor(beanFactory, null, transactionManager);
+	}
+
+	private TransactionInterceptor transactionInterceptorWithTransactionManagerName(
+			String transactionManagerName, BeanFactory beanFactory) {
+
+		return createTransactionInterceptor(beanFactory, transactionManagerName, null);
+	}
+
+	private TransactionInterceptor simpleTransactionInterceptor(BeanFactory beanFactory) {
+		return createTransactionInterceptor(beanFactory, null, null);
 	}
 
 	private PlatformTransactionManager associateTransactionManager(BeanFactory beanFactory, String name) {