Defensively check expected type for qualified bean

Closes gh-34187

Author: jhoeller

spring-beans/src/main/java/org/springframework/beans/factory/annotation/BeanFactoryAnnotationUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2024 the original author or authors.
+ * Copyright 2002-2025 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.
@@ -95,7 +95,7 @@ public static <T> T qualifiedBeanOfType(BeanFactory beanFactory, Class<T> beanTy
 			// Full qualifier matching supported.
 			return qualifiedBeanOfType(lbf, beanType, qualifier);
 		}
-		else if (beanFactory.containsBean(qualifier)) {
+		else if (beanFactory.containsBean(qualifier) && beanFactory.isTypeMatch(qualifier, beanType)) {
 			// Fallback: target bean at least found by bean name.
 			return beanFactory.getBean(qualifier, beanType);
 		}
@@ -110,28 +110,28 @@ else if (beanFactory.containsBean(qualifier)) {
 	/**
 	 * Obtain a bean of type {@code T} from the given {@code BeanFactory} declaring a qualifier
 	 * (for example, {@code <qualifier>} or {@code @Qualifier}) matching the given qualifier).
-	 * @param bf the factory to get the target bean from
+	 * @param beanFactory the factory to get the target bean from
 	 * @param beanType the type of bean to retrieve
 	 * @param qualifier the qualifier for selecting between multiple bean matches
 	 * @return the matching bean of type {@code T} (never {@code null})
 	 */
-	private static <T> T qualifiedBeanOfType(ListableBeanFactory bf, Class<T> beanType, String qualifier) {
-		String[] candidateBeans = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(bf, beanType);
+	private static <T> T qualifiedBeanOfType(ListableBeanFactory beanFactory, Class<T> beanType, String qualifier) {
+		String[] candidateBeans = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(beanFactory, beanType);
 		String matchingBean = null;
 		for (String beanName : candidateBeans) {
-			if (isQualifierMatch(qualifier::equals, beanName, bf)) {
+			if (isQualifierMatch(qualifier::equals, beanName, beanFactory)) {
 				if (matchingBean != null) {
 					throw new NoUniqueBeanDefinitionException(beanType, matchingBean, beanName);
 				}
 				matchingBean = beanName;
 			}
 		}
 		if (matchingBean != null) {
-			return bf.getBean(matchingBean, beanType);
+			return beanFactory.getBean(matchingBean, beanType);
 		}
-		else if (bf.containsBean(qualifier)) {
+		else if (beanFactory.containsBean(qualifier) && beanFactory.isTypeMatch(qualifier, beanType)) {
 			// Fallback: target bean at least found by bean name - probably a manually registered singleton.
-			return bf.getBean(qualifier, beanType);
+			return beanFactory.getBean(qualifier, beanType);
 		}
 		else {
 			throw new NoSuchBeanDefinitionException(qualifier, "No matching " + beanType.getSimpleName() +

spring-tx/src/test/java/org/springframework/transaction/annotation/EnableTransactionManagementTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2024 the original author or authors.
+ * Copyright 2002-2025 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.
@@ -58,6 +58,7 @@
  * @author Juergen Hoeller
  * @author Stephane Nicoll
  * @author Sam Brannen
+ * @author Yanming Zhou
  * @since 3.1
  */
 class EnableTransactionManagementTests {
@@ -243,8 +244,8 @@ void transactionalEventListenerRegisteredProperly() {
 	}
 
 	@Test
-	void spr11915TransactionManagerAsManualSingleton() {
-		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Spr11915Config.class);
+	void transactionManagerAsManualSingleton() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ManualSingletonConfig.class);
 		TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class);
 		CallCountingTransactionManager txManager = ctx.getBean("qualifiedTransactionManager", CallCountingTransactionManager.class);
 
@@ -264,25 +265,49 @@ void spr11915TransactionManagerAsManualSingleton() {
 	}
 
 	@Test
-	void gh24291TransactionManagerViaQualifierAnnotation() {
-		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Gh24291Config.class);
-		TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class);
-		CallCountingTransactionManager txManager = ctx.getBean("qualifiedTransactionManager", CallCountingTransactionManager.class);
+	void transactionManagerViaQualifierAnnotation() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(QualifiedTransactionConfig.class);
+
+		TransactionalTestBean bean = ctx.getBean("testBean", TransactionalTestBean.class);
+		TransactionalTestBeanWithNonExistentQualifier beanWithNonExistentQualifier = ctx.getBean(
+				"testBeanWithNonExistentQualifier", TransactionalTestBeanWithNonExistentQualifier.class);
+		TransactionalTestBeanWithInvalidQualifier beanWithInvalidQualifier = ctx.getBean(
+				"testBeanWithInvalidQualifier", TransactionalTestBeanWithInvalidQualifier.class);
+
+		CallCountingTransactionManager qualified = ctx.getBean("qualifiedTransactionManager",
+				CallCountingTransactionManager.class);
+		CallCountingTransactionManager primary = ctx.getBean("primaryTransactionManager",
+				CallCountingTransactionManager.class);
 
 		bean.saveQualifiedFoo();
-		assertThat(txManager.begun).isEqualTo(1);
-		assertThat(txManager.commits).isEqualTo(1);
-		assertThat(txManager.rollbacks).isEqualTo(0);
+		assertThat(qualified.begun).isEqualTo(1);
+		assertThat(qualified.commits).isEqualTo(1);
+		assertThat(qualified.rollbacks).isEqualTo(0);
 
 		bean.saveQualifiedFooWithAttributeAlias();
-		assertThat(txManager.begun).isEqualTo(2);
-		assertThat(txManager.commits).isEqualTo(2);
-		assertThat(txManager.rollbacks).isEqualTo(0);
+		assertThat(qualified.begun).isEqualTo(2);
+		assertThat(qualified.commits).isEqualTo(2);
+		assertThat(qualified.rollbacks).isEqualTo(0);
 
 		bean.findAllFoos();
-		assertThat(txManager.begun).isEqualTo(3);
-		assertThat(txManager.commits).isEqualTo(3);
-		assertThat(txManager.rollbacks).isEqualTo(0);
+		assertThat(qualified.begun).isEqualTo(3);
+		assertThat(qualified.commits).isEqualTo(3);
+		assertThat(qualified.rollbacks).isEqualTo(0);
+
+		beanWithNonExistentQualifier.findAllFoos();
+		assertThat(primary.begun).isEqualTo(1);
+		assertThat(primary.commits).isEqualTo(1);
+		assertThat(primary.rollbacks).isEqualTo(0);
+
+		beanWithInvalidQualifier.findAllFoos();
+		assertThat(primary.begun).isEqualTo(2);
+		assertThat(primary.commits).isEqualTo(2);
+		assertThat(primary.rollbacks).isEqualTo(0);
+
+		// no further access to qualified transaction manager
+		assertThat(qualified.begun).isEqualTo(3);
+		assertThat(qualified.commits).isEqualTo(3);
+		assertThat(qualified.rollbacks).isEqualTo(0);
 
 		ctx.close();
 	}
@@ -386,6 +411,16 @@ public void saveQualifiedFooWithAttributeAlias() {
 	public static class TransactionalTestBeanSubclass extends TransactionalTestBean {
 	}
 
+	@Service
+	@Qualifier("nonExistentBean")
+	public static class TransactionalTestBeanWithNonExistentQualifier extends TransactionalTestBean {
+	}
+
+	@Service
+	@Qualifier("transactionalTestBeanWithInvalidQualifier")
+	public static class TransactionalTestBeanWithInvalidQualifier extends TransactionalTestBean {
+	}
+
 
 	@Configuration
 	static class PlaceholderConfig {
@@ -558,7 +593,7 @@ public PlatformTransactionManager annotationDrivenTransactionManager() {
 	@Configuration
 	@EnableTransactionManagement
 	@Import(PlaceholderConfig.class)
-	static class Spr11915Config {
+	static class ManualSingletonConfig {
 
 		@Autowired
 		public void initializeApp(ConfigurableApplicationContext applicationContext) {
@@ -581,7 +616,7 @@ public CallCountingTransactionManager otherTxManager() {
 	@Configuration
 	@EnableTransactionManagement
 	@Import(PlaceholderConfig.class)
-	static class Gh24291Config {
+	static class QualifiedTransactionConfig {
 
 		@Autowired
 		public void initializeApp(ConfigurableApplicationContext applicationContext) {
@@ -596,7 +631,18 @@ public TransactionalTestBeanSubclass testBean() {
 		}
 
 		@Bean
-		public CallCountingTransactionManager otherTxManager() {
+		public TransactionalTestBeanWithNonExistentQualifier testBeanWithNonExistentQualifier() {
+			return new TransactionalTestBeanWithNonExistentQualifier();
+		}
+
+		@Bean
+		public TransactionalTestBeanWithInvalidQualifier testBeanWithInvalidQualifier() {
+			return new TransactionalTestBeanWithInvalidQualifier();
+		}
+
+		@Bean
+		@Primary
+		public CallCountingTransactionManager primaryTransactionManager() {
 			return new CallCountingTransactionManager();
 		}
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2024 the original author or authors.
+ * Copyright 2002-2025 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.
@@ -296,6 +296,7 @@ private TransactionInterceptor simpleTransactionInterceptor(BeanFactory beanFact
 	private PlatformTransactionManager associateTransactionManager(BeanFactory beanFactory, String name) {
 		PlatformTransactionManager transactionManager = mock();
 		given(beanFactory.containsBean(name)).willReturn(true);
+		given(beanFactory.isTypeMatch(name, TransactionManager.class)).willReturn(true);
 		given(beanFactory.getBean(name, TransactionManager.class)).willReturn(transactionManager);
 		return transactionManager;
 	}