Defensively check expected type for qualified bean

Closes gh-34187
This commit is contained in:
Juergen Hoeller 2025-01-13 13:03:25 +01:00
parent a1503a59ee
commit ff72652890
3 changed files with 76 additions and 29 deletions

View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -95,7 +95,7 @@ public abstract class BeanFactoryAnnotationUtils {
// Full qualifier matching supported. // Full qualifier matching supported.
return qualifiedBeanOfType(lbf, beanType, qualifier); 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. // Fallback: target bean at least found by bean name.
return beanFactory.getBean(qualifier, beanType); return beanFactory.getBean(qualifier, beanType);
} }
@ -110,16 +110,16 @@ public abstract class BeanFactoryAnnotationUtils {
/** /**
* Obtain a bean of type {@code T} from the given {@code BeanFactory} declaring a 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). * (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 beanType the type of bean to retrieve
* @param qualifier the qualifier for selecting between multiple bean matches * @param qualifier the qualifier for selecting between multiple bean matches
* @return the matching bean of type {@code T} (never {@code null}) * @return the matching bean of type {@code T} (never {@code null})
*/ */
private static <T> T qualifiedBeanOfType(ListableBeanFactory bf, Class<T> beanType, String qualifier) { private static <T> T qualifiedBeanOfType(ListableBeanFactory beanFactory, Class<T> beanType, String qualifier) {
String[] candidateBeans = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(bf, beanType); String[] candidateBeans = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(beanFactory, beanType);
String matchingBean = null; String matchingBean = null;
for (String beanName : candidateBeans) { for (String beanName : candidateBeans) {
if (isQualifierMatch(qualifier::equals, beanName, bf)) { if (isQualifierMatch(qualifier::equals, beanName, beanFactory)) {
if (matchingBean != null) { if (matchingBean != null) {
throw new NoUniqueBeanDefinitionException(beanType, matchingBean, beanName); throw new NoUniqueBeanDefinitionException(beanType, matchingBean, beanName);
} }
@ -127,11 +127,11 @@ public abstract class BeanFactoryAnnotationUtils {
} }
} }
if (matchingBean != null) { 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. // 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 { else {
throw new NoSuchBeanDefinitionException(qualifier, "No matching " + beanType.getSimpleName() + throw new NoSuchBeanDefinitionException(qualifier, "No matching " + beanType.getSimpleName() +

View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -58,6 +58,7 @@ import static org.springframework.transaction.annotation.RollbackOn.ALL_EXCEPTIO
* @author Juergen Hoeller * @author Juergen Hoeller
* @author Stephane Nicoll * @author Stephane Nicoll
* @author Sam Brannen * @author Sam Brannen
* @author Yanming Zhou
* @since 3.1 * @since 3.1
*/ */
class EnableTransactionManagementTests { class EnableTransactionManagementTests {
@ -243,8 +244,8 @@ class EnableTransactionManagementTests {
} }
@Test @Test
void spr11915TransactionManagerAsManualSingleton() { void transactionManagerAsManualSingleton() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Spr11915Config.class); AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ManualSingletonConfig.class);
TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class); TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class);
CallCountingTransactionManager txManager = ctx.getBean("qualifiedTransactionManager", CallCountingTransactionManager.class); CallCountingTransactionManager txManager = ctx.getBean("qualifiedTransactionManager", CallCountingTransactionManager.class);
@ -264,25 +265,49 @@ class EnableTransactionManagementTests {
} }
@Test @Test
void gh24291TransactionManagerViaQualifierAnnotation() { void transactionManagerViaQualifierAnnotation() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Gh24291Config.class); AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(QualifiedTransactionConfig.class);
TransactionalTestBean bean = ctx.getBean(TransactionalTestBean.class);
CallCountingTransactionManager txManager = ctx.getBean("qualifiedTransactionManager", CallCountingTransactionManager.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(); bean.saveQualifiedFoo();
assertThat(txManager.begun).isEqualTo(1); assertThat(qualified.begun).isEqualTo(1);
assertThat(txManager.commits).isEqualTo(1); assertThat(qualified.commits).isEqualTo(1);
assertThat(txManager.rollbacks).isEqualTo(0); assertThat(qualified.rollbacks).isEqualTo(0);
bean.saveQualifiedFooWithAttributeAlias(); bean.saveQualifiedFooWithAttributeAlias();
assertThat(txManager.begun).isEqualTo(2); assertThat(qualified.begun).isEqualTo(2);
assertThat(txManager.commits).isEqualTo(2); assertThat(qualified.commits).isEqualTo(2);
assertThat(txManager.rollbacks).isEqualTo(0); assertThat(qualified.rollbacks).isEqualTo(0);
bean.findAllFoos(); bean.findAllFoos();
assertThat(txManager.begun).isEqualTo(3); assertThat(qualified.begun).isEqualTo(3);
assertThat(txManager.commits).isEqualTo(3); assertThat(qualified.commits).isEqualTo(3);
assertThat(txManager.rollbacks).isEqualTo(0); 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(); ctx.close();
} }
@ -386,6 +411,16 @@ class EnableTransactionManagementTests {
public static class TransactionalTestBeanSubclass extends TransactionalTestBean { 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 @Configuration
static class PlaceholderConfig { static class PlaceholderConfig {
@ -558,7 +593,7 @@ class EnableTransactionManagementTests {
@Configuration @Configuration
@EnableTransactionManagement @EnableTransactionManagement
@Import(PlaceholderConfig.class) @Import(PlaceholderConfig.class)
static class Spr11915Config { static class ManualSingletonConfig {
@Autowired @Autowired
public void initializeApp(ConfigurableApplicationContext applicationContext) { public void initializeApp(ConfigurableApplicationContext applicationContext) {
@ -581,7 +616,7 @@ class EnableTransactionManagementTests {
@Configuration @Configuration
@EnableTransactionManagement @EnableTransactionManagement
@Import(PlaceholderConfig.class) @Import(PlaceholderConfig.class)
static class Gh24291Config { static class QualifiedTransactionConfig {
@Autowired @Autowired
public void initializeApp(ConfigurableApplicationContext applicationContext) { public void initializeApp(ConfigurableApplicationContext applicationContext) {
@ -596,7 +631,18 @@ class EnableTransactionManagementTests {
} }
@Bean @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(); return new CallCountingTransactionManager();
} }
} }

View File

@ -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"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -296,6 +296,7 @@ class TransactionInterceptorTests extends AbstractTransactionAspectTests {
private PlatformTransactionManager associateTransactionManager(BeanFactory beanFactory, String name) { private PlatformTransactionManager associateTransactionManager(BeanFactory beanFactory, String name) {
PlatformTransactionManager transactionManager = mock(); PlatformTransactionManager transactionManager = mock();
given(beanFactory.containsBean(name)).willReturn(true); given(beanFactory.containsBean(name)).willReturn(true);
given(beanFactory.isTypeMatch(name, TransactionManager.class)).willReturn(true);
given(beanFactory.getBean(name, TransactionManager.class)).willReturn(transactionManager); given(beanFactory.getBean(name, TransactionManager.class)).willReturn(transactionManager);
return transactionManager; return transactionManager;
} }