Consider factory beans when finding candidates

Previously, if a bean name was a factory dereference its definition would not be found. When the definition wasn't found it was assumed that the bean was an autowire candidate and a default candidate. If this, in fact, was not the case, @ConditionalOnMissingBean would not match when it should have done and @ConditionalOnBean would match when it should not had done. This commit updates the bean-based conditions to correctly consider factory beans so that whether or not they are a candidate can be evaluated correctly. Fixes gh-42970
spring-projects · Nov 1, 2024 · 2b3c93f · 2b3c93f
1 parent 4a9da78
commit 2b3c93f
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 22 deletions.
diff --git a/...igure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java b/...igure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java
@@ -36,6 +36,7 @@
 
 import org.springframework.aop.scope.ScopedProxyUtils;
 import org.springframework.beans.factory.BeanFactory;
+import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.HierarchicalBeanFactory;
 import org.springframework.beans.factory.ListableBeanFactory;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
@@ -496,12 +497,7 @@ private static Map<String, BeanDefinition> putAll(Map<String, BeanDefinition> re
 		}
 		for (String beanName : beanNames) {
 			if (beanFactory instanceof ConfigurableListableBeanFactory clbf) {
-				try {
-					result.put(beanName, clbf.getBeanDefinition(beanName));
-				}
-				catch (NoSuchBeanDefinitionException ex) {
-					result.put(beanName, null);
-				}
+				result.put(beanName, getBeanDefinition(beanName, clbf));
 			}
 			else {
 				result.put(beanName, null);
@@ -510,6 +506,18 @@ private static Map<String, BeanDefinition> putAll(Map<String, BeanDefinition> re
 		return result;
 	}
 
+	private static BeanDefinition getBeanDefinition(String beanName, ConfigurableListableBeanFactory beanFactory) {
+		try {
+			return beanFactory.getBeanDefinition(beanName);
+		}
+		catch (NoSuchBeanDefinitionException ex) {
+			if (BeanFactoryUtils.isFactoryDereference(beanName)) {
+				return getBeanDefinition(BeanFactoryUtils.transformedBeanName(beanName), beanFactory);
+			}
+		}
+		return null;
+	}
+
 	/**
 	 * A search specification extracted from the underlying annotation.
 	 */

diff --git a/...rc/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java b/...rc/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java
@@ -267,6 +267,14 @@ void conditionalOnBeanTypeIgnoresNotDefaultCandidateBean() {
 			.run((context) -> assertThat(context).doesNotHaveBean("bar"));
 	}
 
+	@Test
+	void conditionalOnBeanTypeIgnoresNotDefaultCandidateFactoryBean() {
+		this.contextRunner
+			.withUserConfiguration(NotDefaultCandidateFactoryBeanConfiguration.class,
+					OnBeanClassWithFactoryBeanConfiguration.class)
+			.run((context) -> assertThat(context).doesNotHaveBean("bar"));
+	}
+
 	@Test
 	void conditionalOnBeanNameMatchesNotDefaultCandidateBean() {
 		this.contextRunner.withUserConfiguration(NotDefaultCandidateConfiguration.class, OnBeanNameConfiguration.class)
@@ -332,6 +340,17 @@ String bar() {
 
 	}
 
+	@Configuration(proxyBeanMethods = false)
+	@ConditionalOnBean(ExampleFactoryBean.class)
+	static class OnBeanClassWithFactoryBeanConfiguration {
+
+		@Bean
+		String bar() {
+			return "bar";
+		}
+
+	}
+
 	@Configuration(proxyBeanMethods = false)
 	@ConditionalOnBean(type = "java.lang.String")
 	static class OnBeanClassNameConfiguration {
@@ -385,6 +404,16 @@ String foo() {
 
 	}
 
+	@Configuration(proxyBeanMethods = false)
+	static class NotDefaultCandidateFactoryBeanConfiguration {
+
+		@Bean(defaultCandidate = false)
+		ExampleFactoryBean exampleBeanFactoryBean() {
+			return new ExampleFactoryBean();
+		}
+
+	}
+
 	@Configuration(proxyBeanMethods = false)
 	@ImportResource("org/springframework/boot/autoconfigure/condition/foo.xml")
 	static class XmlConfiguration {

diff --git a/.../java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java b/.../java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java
@@ -160,7 +160,7 @@ void testOnMissingBeanConditionOutputShouldNotContainConditionalOnBeanClassInMes
 	@Test
 	void testOnMissingBeanConditionWithFactoryBean() {
 		this.contextRunner
-			.withUserConfiguration(FactoryBeanConfiguration.class, ConditionalOnFactoryBean.class,
+			.withUserConfiguration(FactoryBeanConfiguration.class, ConditionalOnMissingBeanProducedByFactoryBean.class,
 					PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context.getBean(ExampleBean.class)).hasToString("fromFactory"));
 	}
@@ -169,73 +169,73 @@ void testOnMissingBeanConditionWithFactoryBean() {
 	void testOnMissingBeanConditionWithComponentScannedFactoryBean() {
 		this.contextRunner
 			.withUserConfiguration(ComponentScannedFactoryBeanBeanMethodConfiguration.class,
-					ConditionalOnFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context.getBean(ScanBean.class)).hasToString("fromFactory"));
 	}
 
 	@Test
 	void testOnMissingBeanConditionWithComponentScannedFactoryBeanWithBeanMethodArguments() {
 		this.contextRunner
 			.withUserConfiguration(ComponentScannedFactoryBeanBeanMethodWithArgumentsConfiguration.class,
-					ConditionalOnFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context.getBean(ScanBean.class)).hasToString("fromFactory"));
 	}
 
 	@Test
 	void testOnMissingBeanConditionWithFactoryBeanWithBeanMethodArguments() {
 		this.contextRunner
 			.withUserConfiguration(FactoryBeanWithBeanMethodArgumentsConfiguration.class,
-					ConditionalOnFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.withPropertyValues("theValue=foo")
 			.run((context) -> assertThat(context.getBean(ExampleBean.class)).hasToString("fromFactory"));
 	}
 
 	@Test
 	void testOnMissingBeanConditionWithConcreteFactoryBean() {
 		this.contextRunner
-			.withUserConfiguration(ConcreteFactoryBeanConfiguration.class, ConditionalOnFactoryBean.class,
-					PropertyPlaceholderAutoConfiguration.class)
+			.withUserConfiguration(ConcreteFactoryBeanConfiguration.class,
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context.getBean(ExampleBean.class)).hasToString("fromFactory"));
 	}
 
 	@Test
 	void testOnMissingBeanConditionWithUnhelpfulFactoryBean() {
 		// We could not tell that the FactoryBean would ultimately create an ExampleBean
 		this.contextRunner
-			.withUserConfiguration(UnhelpfulFactoryBeanConfiguration.class, ConditionalOnFactoryBean.class,
-					PropertyPlaceholderAutoConfiguration.class)
+			.withUserConfiguration(UnhelpfulFactoryBeanConfiguration.class,
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context).getBeans(ExampleBean.class).hasSize(2));
 	}
 
 	@Test
 	void testOnMissingBeanConditionWithRegisteredFactoryBean() {
 		this.contextRunner
-			.withUserConfiguration(RegisteredFactoryBeanConfiguration.class, ConditionalOnFactoryBean.class,
-					PropertyPlaceholderAutoConfiguration.class)
+			.withUserConfiguration(RegisteredFactoryBeanConfiguration.class,
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context.getBean(ExampleBean.class)).hasToString("fromFactory"));
 	}
 
 	@Test
 	void testOnMissingBeanConditionWithNonspecificFactoryBeanWithClassAttribute() {
 		this.contextRunner
 			.withUserConfiguration(NonspecificFactoryBeanClassAttributeConfiguration.class,
-					ConditionalOnFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context.getBean(ExampleBean.class)).hasToString("fromFactory"));
 	}
 
 	@Test
 	void testOnMissingBeanConditionWithNonspecificFactoryBeanWithStringAttribute() {
 		this.contextRunner
 			.withUserConfiguration(NonspecificFactoryBeanStringAttributeConfiguration.class,
-					ConditionalOnFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context.getBean(ExampleBean.class)).hasToString("fromFactory"));
 	}
 
 	@Test
 	void testOnMissingBeanConditionWithFactoryBeanInXml() {
 		this.contextRunner
-			.withUserConfiguration(FactoryBeanXmlConfiguration.class, ConditionalOnFactoryBean.class,
-					PropertyPlaceholderAutoConfiguration.class)
+			.withUserConfiguration(FactoryBeanXmlConfiguration.class,
+					ConditionalOnMissingBeanProducedByFactoryBean.class, PropertyPlaceholderAutoConfiguration.class)
 			.run((context) -> assertThat(context.getBean(ExampleBean.class)).hasToString("fromFactory"));
 	}
 
@@ -377,6 +377,15 @@ void typeBasedMatchingIgnoresBeanThatIsNotDefaultCandidate() {
 			.run((context) -> assertThat(context).hasBean("bar"));
 	}
 
+	@Test
+	void typeBasedMatchingIgnoresFactoryBeanThatIsNotDefaultCandidate() {
+		this.contextRunner
+			.withUserConfiguration(NotDefaultCandidateFactoryBeanConfiguration.class,
+					ConditionalOnMissingFactoryBean.class)
+			.run((context) -> assertThat(context).hasBean("&exampleFactoryBean")
+				.hasBean("&additionalExampleFactoryBean"));
+	}
+
 	@Test
 	void nameBasedMatchingConsidersBeanThatIsNotDefaultCandidate() {
 		this.contextRunner.withUserConfiguration(NotDefaultCandidateConfig.class, OnBeanNameConfiguration.class)
@@ -447,7 +456,17 @@ String bar() {
 	static class FactoryBeanConfiguration {
 
 		@Bean
-		FactoryBean<ExampleBean> exampleBeanFactoryBean() {
+		ExampleFactoryBean exampleBeanFactoryBean() {
+			return new ExampleFactoryBean("foo");
+		}
+
+	}
+
+	@Configuration(proxyBeanMethods = false)
+	static class NotDefaultCandidateFactoryBeanConfiguration {
+
+		@Bean(defaultCandidate = false)
+		ExampleFactoryBean exampleFactoryBean() {
 			return new ExampleFactoryBean("foo");
 		}
 
@@ -548,7 +567,7 @@ static class FactoryBeanXmlConfiguration {
 	}
 
 	@Configuration(proxyBeanMethods = false)
-	static class ConditionalOnFactoryBean {
+	static class ConditionalOnMissingBeanProducedByFactoryBean {
 
 		@Bean
 		@ConditionalOnMissingBean
@@ -558,6 +577,17 @@ ExampleBean createExampleBean() {
 
 	}
 
+	@Configuration(proxyBeanMethods = false)
+	static class ConditionalOnMissingFactoryBean {
+
+		@Bean
+		@ConditionalOnMissingBean
+		ExampleFactoryBean additionalExampleFactoryBean() {
+			return new ExampleFactoryBean("factory");
+		}
+
+	}
+
 	@Configuration(proxyBeanMethods = false)
 	static class ConditionalOnIgnoredSubclass {