From e9d594c6930fd11561d12f1048f5b22161e7970a Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 21 May 2014 12:47:53 +0100 Subject: [PATCH] Add conditional processing option for unhelpful FactoryBeans OnBeanCondition has some issues with FactoryBean object types where the FactoryBean is not generic (i.e. you have to instantiate it to get its object type). This is a known issue (see tests in ConditionalOnMissingBeanTests), but we can provide some help for library authors who know the type in advance. The approach we have taken here is to check the BeanDefinition for an attribute called "factoryBeanObjectType" (OnBeanCondition.FACTORY_BEAN_OBJECT_TYPE) which, if it exists, can be used as a tie-breaker. Its value should be a Class instance. Fixes gh-921 --- .../condition/OnBeanCondition.java | 41 ++++++--- .../ConditionalOnMissingBeanTests.java | 89 +++++++++++++++++++ 2 files changed, 118 insertions(+), 12 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java index 1490d1c162..45d52d4193 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java @@ -56,6 +56,12 @@ import org.springframework.util.StringUtils; */ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondition { + /** + * Bean definition attribute name for factory beans to signal their product type (if + * known and it can't be deduced from the factory bean class). + */ + public static final String FACTORY_BEAN_OBJECT_TYPE = "factoryBeanObjectType"; + private static final String[] NO_BEANS = {}; @Override @@ -179,7 +185,7 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit for (String name : names) { name = BeanFactoryUtils.transformedBeanName(name); BeanDefinition beanDefinition = beanFactory.getBeanDefinition(name); - Class generic = getFactoryBeanGeneric(beanFactory, beanDefinition); + Class generic = getFactoryBeanGeneric(beanFactory, beanDefinition, name); if (generic != null && ClassUtils.isAssignable(type, generic)) { result.add(name); } @@ -187,14 +193,15 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit } private Class getFactoryBeanGeneric(ConfigurableListableBeanFactory beanFactory, - BeanDefinition definition) { + BeanDefinition definition, String name) { try { if (StringUtils.hasLength(definition.getFactoryBeanName()) && StringUtils.hasLength(definition.getFactoryMethodName())) { - return getConfigurationClassFactoryBeanGeneric(beanFactory, definition); + return getConfigurationClassFactoryBeanGeneric(beanFactory, definition, + name); } if (StringUtils.hasLength(definition.getBeanClassName())) { - return getDirectFactoryBeanGeneric(beanFactory, definition); + return getDirectFactoryBeanGeneric(beanFactory, definition, name); } } catch (Exception ex) { @@ -203,25 +210,35 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit } private Class getConfigurationClassFactoryBeanGeneric( - ConfigurableListableBeanFactory beanFactory, BeanDefinition definition) - throws Exception { + ConfigurableListableBeanFactory beanFactory, BeanDefinition definition, + String name) throws Exception { BeanDefinition factoryDefinition = beanFactory.getBeanDefinition(definition .getFactoryBeanName()); Class factoryClass = ClassUtils.forName(factoryDefinition.getBeanClassName(), beanFactory.getBeanClassLoader()); Method method = ReflectionUtils.findMethod(factoryClass, definition.getFactoryMethodName()); - return ResolvableType.forMethodReturnType(method).as(FactoryBean.class) - .resolveGeneric(); + Class generic = ResolvableType.forMethodReturnType(method) + .as(FactoryBean.class).resolveGeneric(); + if ((generic == null || generic.equals(Object.class)) + && definition.hasAttribute(FACTORY_BEAN_OBJECT_TYPE)) { + generic = (Class) definition.getAttribute(FACTORY_BEAN_OBJECT_TYPE); + } + return generic; } private Class getDirectFactoryBeanGeneric( - ConfigurableListableBeanFactory beanFactory, BeanDefinition definition) - throws ClassNotFoundException, LinkageError { + ConfigurableListableBeanFactory beanFactory, BeanDefinition definition, + String name) throws ClassNotFoundException, LinkageError { Class factoryBeanClass = ClassUtils.forName(definition.getBeanClassName(), beanFactory.getBeanClassLoader()); - return ResolvableType.forClass(factoryBeanClass).as(FactoryBean.class) - .resolveGeneric(); + Class generic = ResolvableType.forClass(factoryBeanClass) + .as(FactoryBean.class).resolveGeneric(); + if ((generic == null || generic.equals(Object.class)) + && definition.hasAttribute(FACTORY_BEAN_OBJECT_TYPE)) { + generic = (Class) definition.getAttribute(FACTORY_BEAN_OBJECT_TYPE); + } + return generic; } private String[] getBeanNamesForAnnotation( diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java index 50ab687e5d..f333546492 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnMissingBeanTests.java @@ -18,11 +18,16 @@ package org.springframework.boot.autoconfigure.condition; import org.junit.Test; import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.ImportBeanDefinitionRegistrar; import org.springframework.context.annotation.ImportResource; +import org.springframework.core.type.AnnotationMetadata; import org.springframework.scheduling.annotation.EnableScheduling; import org.springframework.util.Assert; @@ -141,6 +146,26 @@ public class ConditionalOnMissingBeanTests { equalTo(2)); } + @Test + public void testOnMissingBeanConditionWithRegisteredFactoryBean() { + this.context.register(RegisteredFactoryBeanConfiguration.class, + ConditionalOnFactoryBean.class, + PropertyPlaceholderAutoConfiguration.class); + this.context.refresh(); + assertThat(this.context.getBean(ExampleBean.class).toString(), + equalTo("fromFactory")); + } + + @Test + public void testOnMissingBeanConditionWithNonspecificFactoryBean() { + this.context.register(NonspecificFactoryBeanConfiguration.class, + ConditionalOnFactoryBean.class, + PropertyPlaceholderAutoConfiguration.class); + this.context.refresh(); + assertThat(this.context.getBean(ExampleBean.class).toString(), + equalTo("fromFactory")); + } + @Test public void testOnMissingBeanConditionWithFactoryBeanInXml() { this.context.register(FactoryBeanXmlConfiguration.class, @@ -185,6 +210,47 @@ public class ConditionalOnMissingBeanTests { } } + @Configuration + @Import(NonspecificFactoryBeanRegistrar.class) + protected static class NonspecificFactoryBeanConfiguration { + } + + protected static class NonspecificFactoryBeanRegistrar implements + ImportBeanDefinitionRegistrar { + + @Override + public void registerBeanDefinitions(AnnotationMetadata meta, + BeanDefinitionRegistry registry) { + BeanDefinitionBuilder builder = BeanDefinitionBuilder + .genericBeanDefinition(NonspecificFactoryBean.class); + builder.addConstructorArgValue("foo"); + builder.getBeanDefinition().setAttribute( + OnBeanCondition.FACTORY_BEAN_OBJECT_TYPE, ExampleBean.class); + registry.registerBeanDefinition("exampleBeanFactoryBean", + builder.getBeanDefinition()); + } + + } + + @Configuration + @Import(FactoryBeanRegistrar.class) + protected static class RegisteredFactoryBeanConfiguration { + } + + protected static class FactoryBeanRegistrar implements ImportBeanDefinitionRegistrar { + + @Override + public void registerBeanDefinitions(AnnotationMetadata meta, + BeanDefinitionRegistry registry) { + BeanDefinitionBuilder builder = BeanDefinitionBuilder + .genericBeanDefinition(ExampleFactoryBean.class); + builder.addConstructorArgValue("foo"); + registry.registerBeanDefinition("exampleBeanFactoryBean", + builder.getBeanDefinition()); + } + + } + @Configuration @ImportResource("org/springframework/boot/autoconfigure/condition/factorybean.xml") protected static class FactoryBeanXmlConfiguration { @@ -291,4 +357,27 @@ public class ConditionalOnMissingBeanTests { } } + + public static class NonspecificFactoryBean implements FactoryBean { + + public NonspecificFactoryBean(String value) { + Assert.state(!value.contains("$")); + } + + @Override + public ExampleBean getObject() throws Exception { + return new ExampleBean("fromFactory"); + } + + @Override + public Class getObjectType() { + return ExampleBean.class; + } + + @Override + public boolean isSingleton() { + return false; + } + + } }