From a0e462581fe1d27d5926809fbb21441a6685ef45 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 18 Jul 2019 18:43:48 +0100 Subject: [PATCH] Consider generics for predicting FactoryBean types Update the `FactoryBean` type prediction logic (primarily in the `DefaultListableBeanFactory`) so that generic type information is considered when calling `getBeanNamesForType` on a non-frozen configuration. Calling `getBeanNamesForType` with `allowEagerInit` disabled will now detect `FactoryBean` variants as long as generic type information is available in either the class or the factory method return type. Closes gh-23338 --- .../AbstractAutowireCapableBeanFactory.java | 202 +++++++++++------- .../factory/support/AbstractBeanFactory.java | 198 +++++++++++------ .../support/DefaultListableBeanFactory.java | 60 +++--- ...ithFactoryBeanBeanEarlyDeductionTests.java | 172 +++++++++++++++ 4 files changed, 465 insertions(+), 167 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanBeanEarlyDeductionTests.java diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index a84f675f0ea..8602dd93606 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -71,7 +71,6 @@ import org.springframework.beans.factory.config.InstantiationAwareBeanPostProces import org.springframework.beans.factory.config.SmartInstantiationAwareBeanPostProcessor; import org.springframework.beans.factory.config.TypedStringValue; import org.springframework.core.DefaultParameterNameDiscoverer; -import org.springframework.core.GenericTypeResolver; import org.springframework.core.MethodParameter; import org.springframework.core.NamedThreadLocal; import org.springframework.core.ParameterNameDiscoverer; @@ -82,6 +81,7 @@ import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ObjectUtils; import org.springframework.util.ReflectionUtils; +import org.springframework.util.ReflectionUtils.MethodCallback; import org.springframework.util.StringUtils; /** @@ -815,46 +815,51 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac * if present to determine the object type. If not present, i.e. the FactoryBean is * declared as a raw type, checks the FactoryBean's {@code getObjectType} method * on a plain instance of the FactoryBean, without bean properties applied yet. - * If this doesn't return a type yet, a full creation of the FactoryBean is - * used as fallback (through delegation to the superclass's implementation). + * If this doesn't return a type yet, and {@code allowInit} is {@code true} a + * full creation of the FactoryBean is used as fallback (through delegation to the + * superclass's implementation). *

The shortcut check for a FactoryBean is only applied in case of a singleton * FactoryBean. If the FactoryBean instance itself is not kept as singleton, * it will be fully created to check the type of its exposed object. */ @Override - @Nullable - protected Class getTypeForFactoryBean(String beanName, RootBeanDefinition mbd) { + protected ResolvableType getTypeForFactoryBean(String beanName, + RootBeanDefinition mbd, boolean allowInit) { + + ResolvableType result = ResolvableType.NONE; + + ResolvableType beanType = mbd.hasBeanClass() ? + ResolvableType.forClass(mbd.getBeanClass()) : + ResolvableType.NONE; + + // For instance supplied beans try the target type and bean class if (mbd.getInstanceSupplier() != null) { - ResolvableType targetType = mbd.targetType; - if (targetType != null) { - Class result = targetType.as(FactoryBean.class).getGeneric().resolve(); - if (result != null) { - return result; - } + result = getFactoryBeanGeneric(mbd.targetType); + if (result.resolve() != null) { + return result; } - if (mbd.hasBeanClass()) { - Class result = GenericTypeResolver.resolveTypeArgument(mbd.getBeanClass(), FactoryBean.class); - if (result != null) { - return result; - } + result = getFactoryBeanGeneric(beanType); + if (result.resolve() != null) { + return result; } } + // Consider factory methods String factoryBeanName = mbd.getFactoryBeanName(); String factoryMethodName = mbd.getFactoryMethodName(); + // Scan the factory bean methods if (factoryBeanName != null) { if (factoryMethodName != null) { - // Try to obtain the FactoryBean's object type from its factory method declaration - // without instantiating the containing bean at all. - BeanDefinition fbDef = getBeanDefinition(factoryBeanName); - if (fbDef instanceof AbstractBeanDefinition) { - AbstractBeanDefinition afbDef = (AbstractBeanDefinition) fbDef; - if (afbDef.hasBeanClass()) { - Class result = getTypeForFactoryBeanFromMethod(afbDef.getBeanClass(), factoryMethodName); - if (result != null) { - return result; - } + // Try to obtain the FactoryBean's object type from its factory method + // declaration without instantiating the containing bean at all. + BeanDefinition factoryBeanDefinition = getBeanDefinition(factoryBeanName); + if (factoryBeanDefinition instanceof AbstractBeanDefinition && + ((AbstractBeanDefinition) factoryBeanDefinition).hasBeanClass()) { + Class factoryBeanClass = ((AbstractBeanDefinition) factoryBeanDefinition).getBeanClass(); + result = getTypeForFactoryBeanFromMethod(factoryBeanClass, factoryMethodName); + if (result.resolve() != null) { + return result; } } } @@ -862,40 +867,44 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac // exit here - we don't want to force the creation of another bean just to // obtain a FactoryBean's object type... if (!isBeanEligibleForMetadataCaching(factoryBeanName)) { - return null; + return ResolvableType.NONE; } } - // Let's obtain a shortcut instance for an early getObjectType() call... - FactoryBean fb = (mbd.isSingleton() ? - getSingletonFactoryBeanForTypeCheck(beanName, mbd) : - getNonSingletonFactoryBeanForTypeCheck(beanName, mbd)); - - if (fb != null) { - // Try to obtain the FactoryBean's object type from this early stage of the instance. - Class result = getTypeForFactoryBean(fb); - if (result != null) { - return result; - } - else { + // If we're allowed, we can create the factory bean and call getObjectType() early + if (allowInit) { + FactoryBean factoryBean = (mbd.isSingleton() ? + getSingletonFactoryBeanForTypeCheck(beanName, mbd) : + getNonSingletonFactoryBeanForTypeCheck(beanName, mbd)); + if (factoryBean != null) { + // Try to obtain the FactoryBean's object type from this early stage of the instance. + Class type = getTypeForFactoryBean(factoryBean); + if (type != null) { + return ResolvableType.forClass(type); + } // No type found for shortcut FactoryBean instance: // fall back to full creation of the FactoryBean instance. - return super.getTypeForFactoryBean(beanName, mbd); + return super.getTypeForFactoryBean(beanName, mbd, allowInit); } } - if (factoryBeanName == null && mbd.hasBeanClass()) { + if (factoryBeanName == null && mbd.hasBeanClass() && factoryMethodName != null) { // No early bean instantiation possible: determine FactoryBean's type from // static factory method signature or from class inheritance hierarchy... - if (factoryMethodName != null) { - return getTypeForFactoryBeanFromMethod(mbd.getBeanClass(), factoryMethodName); - } - else { - return GenericTypeResolver.resolveTypeArgument(mbd.getBeanClass(), FactoryBean.class); - } + return getTypeForFactoryBeanFromMethod(mbd.getBeanClass(), factoryMethodName); } + result = getFactoryBeanGeneric(beanType); + if (result.resolve() != null) { + return result; + } + return ResolvableType.NONE; + } - return null; + private ResolvableType getFactoryBeanGeneric(@Nullable ResolvableType type) { + if (type == null) { + return ResolvableType.NONE; + } + return type.as(FactoryBean.class).getGeneric(); } /** @@ -905,36 +914,30 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac * @param factoryMethodName the name of the factory method * @return the common {@code FactoryBean} object type, or {@code null} if none */ - @Nullable - private Class getTypeForFactoryBeanFromMethod(Class beanClass, final String factoryMethodName) { - - /** - * Holder used to keep a reference to a {@code Class} value. - */ - class Holder { - - @Nullable - Class value = null; - } - - final Holder objectType = new Holder(); - + private ResolvableType getTypeForFactoryBeanFromMethod(Class beanClass, String factoryMethodName) { // CGLIB subclass methods hide generic parameters; look at the original user class. - Class fbClass = ClassUtils.getUserClass(beanClass); + Class factoryBeanClass = ClassUtils.getUserClass(beanClass); + FactoryBeanMethodTypeFinder finder = new FactoryBeanMethodTypeFinder(factoryMethodName); + ReflectionUtils.doWithMethods(factoryBeanClass, finder, ReflectionUtils.USER_DECLARED_METHODS); + return finder.getResult(); + } - // Find the given factory method, taking into account that in the case of - // @Bean methods, there may be parameters present. - ReflectionUtils.doWithMethods(fbClass, method -> { - if (method.getName().equals(factoryMethodName) && - FactoryBean.class.isAssignableFrom(method.getReturnType())) { - Class currentType = GenericTypeResolver.resolveReturnTypeArgument(method, FactoryBean.class); - if (currentType != null) { - objectType.value = ClassUtils.determineCommonAncestor(currentType, objectType.value); - } - } - }, ReflectionUtils.USER_DECLARED_METHODS); - - return (objectType.value != null && Object.class != objectType.value ? objectType.value : null); + /** + * This implementation attempts to query the FactoryBean's generic parameter metadata + * if present to determine the object type. If not present, i.e. the FactoryBean is + * declared as a raw type, checks the FactoryBean's {@code getObjectType} method + * on a plain instance of the FactoryBean, without bean properties applied yet. + * If this doesn't return a type yet, a full creation of the FactoryBean is + * used as fallback (through delegation to the superclass's implementation). + *

The shortcut check for a FactoryBean is only applied in case of a singleton + * FactoryBean. If the FactoryBean instance itself is not kept as singleton, + * it will be fully created to check the type of its exposed object. + */ + @Override + @Deprecated + @Nullable + protected Class getTypeForFactoryBean(String beanName, RootBeanDefinition mbd) { + return getTypeForFactoryBean(beanName, mbd, true).resolve(); } /** @@ -1983,4 +1986,51 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac } } + /** + * {@link MethodCallback} used to find {@link FactoryBean} type information. + */ + private static class FactoryBeanMethodTypeFinder implements MethodCallback { + + private final String factoryMethodName; + + private ResolvableType result = ResolvableType.NONE; + + + FactoryBeanMethodTypeFinder(String factoryMethodName) { + this.factoryMethodName = factoryMethodName; + } + + + @Override + public void doWith(Method method) throws IllegalArgumentException, IllegalAccessException { + if (isFactoryBeanMethod(method)) { + ResolvableType returnType = ResolvableType.forMethodReturnType(method); + ResolvableType candidate = returnType.as(FactoryBean.class).getGeneric(); + if (this.result == ResolvableType.NONE) { + this.result = candidate; + } + else { + Class resolvedResult = this.result.resolve(); + Class commonAncestor = ClassUtils.determineCommonAncestor(candidate.resolve(), resolvedResult); + if (!ObjectUtils.nullSafeEquals(resolvedResult, commonAncestor)) { + this.result = ResolvableType.forClass(commonAncestor); + } + } + } + } + + private boolean isFactoryBeanMethod(Method method) { + return method.getName().equals(this.factoryMethodName) && + FactoryBean.class.isAssignableFrom(method.getReturnType()); + } + + + ResolvableType getResult() { + Class resolved = this.result.resolve(); + boolean foundResult = resolved != null && resolved != Object.class; + return (foundResult ? this.result : ResolvableType.NONE); + } + + } + } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index 1f48fedc2fa..1eff7b50ff0 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -69,6 +69,7 @@ import org.springframework.core.DecoratingClassLoader; import org.springframework.core.NamedThreadLocal; import org.springframework.core.ResolvableType; import org.springframework.core.convert.ConversionService; +import org.springframework.core.log.LogMessage; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -488,13 +489,35 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp @Override public boolean isTypeMatch(String name, ResolvableType typeToMatch) throws NoSuchBeanDefinitionException { + return isTypeMatch(name, typeToMatch, true); + } + + /** + * Internal extended variant of {@link #isTypeMatch(String, ResolvableType)} + * to check whether the bean with the given name matches the specified type. Allow + * additional constraints to be applied to ensure that beans are not created early. + * @param name the name of the bean to query + * @param typeToMatch the type to match against (as a + * {@code ResolvableType}) + * @return {@code true} if the bean type matches, {@code false} if it + * doesn't match or cannot be determined yet + * @throws NoSuchBeanDefinitionException if there is no bean with the given + * name + * @since 5.2 + * @see #getBean + * @see #getType + */ + boolean isTypeMatch(String name, ResolvableType typeToMatch, + boolean allowFactoryBeanInit) throws NoSuchBeanDefinitionException { + String beanName = transformedBeanName(name); + boolean isFactoryDereference = BeanFactoryUtils.isFactoryDereference(name); // Check manually registered singletons. Object beanInstance = getSingleton(beanName, false); if (beanInstance != null && beanInstance.getClass() != NullBean.class) { if (beanInstance instanceof FactoryBean) { - if (!BeanFactoryUtils.isFactoryDereference(name)) { + if (!isFactoryDereference) { Class type = getTypeForFactoryBean((FactoryBean) beanInstance); return (type != null && typeToMatch.isAssignableFrom(type)); } @@ -502,7 +525,7 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp return typeToMatch.isInstance(beanInstance); } } - else if (!BeanFactoryUtils.isFactoryDereference(name)) { + else if (!isFactoryDereference) { if (typeToMatch.isInstance(beanInstance)) { // Direct match for exposed instance? return true; @@ -544,7 +567,9 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp // Retrieve corresponding bean definition. RootBeanDefinition mbd = getMergedLocalBeanDefinition(beanName); + BeanDefinitionHolder dbd = mbd.getDecoratedDefinition(); + // Setup the types that we want to match against Class classToMatch = typeToMatch.resolve(); if (classToMatch == null) { classToMatch = FactoryBean.class; @@ -552,50 +577,75 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp Class[] typesToMatch = (FactoryBean.class == classToMatch ? new Class[] {classToMatch} : new Class[] {FactoryBean.class, classToMatch}); - // Check decorated bean definition, if any: We assume it'll be easier - // to determine the decorated bean's type than the proxy's type. - BeanDefinitionHolder dbd = mbd.getDecoratedDefinition(); - if (dbd != null && !BeanFactoryUtils.isFactoryDereference(name)) { - RootBeanDefinition tbd = getMergedBeanDefinition(dbd.getBeanName(), dbd.getBeanDefinition(), mbd); - Class targetClass = predictBeanType(dbd.getBeanName(), tbd, typesToMatch); - if (targetClass != null && !FactoryBean.class.isAssignableFrom(targetClass)) { - return typeToMatch.isAssignableFrom(targetClass); - } - } - Class beanType = predictBeanType(beanName, mbd, typesToMatch); - if (beanType == null) { - return false; - } + // Attempt to predict the bean type + Class predictedType = null; - // Check bean class whether we're dealing with a FactoryBean. - if (FactoryBean.class.isAssignableFrom(beanType)) { - if (!BeanFactoryUtils.isFactoryDereference(name) && beanInstance == null) { - // If it's a FactoryBean, we want to look at what it creates, not the factory class. - beanType = getTypeForFactoryBean(beanName, mbd); - if (beanType == null) { - return false; + // We're looking for a regular reference but we're a factory bean that has + // a decorated bean definition. The target bean should be the same type + // as FactoryBean would ultimately return. + if (!isFactoryDereference && dbd != null && isFactoryBean(beanName, mbd)) { + // We should only attempt if the user explicitly set lazy-init to true + // and we know the merged bean definition is for a factory bean. + if (!mbd.isLazyInit() || allowFactoryBeanInit) { + RootBeanDefinition tbd = getMergedBeanDefinition(dbd.getBeanName(), dbd.getBeanDefinition(), mbd); + Class targetType = predictBeanType(dbd.getBeanName(), tbd, typesToMatch); + if (targetType != null && !FactoryBean.class.isAssignableFrom(targetType)) { + predictedType = targetType; } } } - else if (BeanFactoryUtils.isFactoryDereference(name)) { - // Special case: A SmartInstantiationAwareBeanPostProcessor returned a non-FactoryBean - // type but we nevertheless are being asked to dereference a FactoryBean... - // Let's check the original bean class and proceed with it if it is a FactoryBean. - beanType = predictBeanType(beanName, mbd, FactoryBean.class); - if (beanType == null || !FactoryBean.class.isAssignableFrom(beanType)) { + + // If we couldn't use the target type, try regular prediction. + if (predictedType == null) { + predictedType = predictBeanType(beanName, mbd, typesToMatch); + if (predictedType == null) { return false; } } - ResolvableType resolvableType = mbd.targetType; - if (resolvableType == null) { - resolvableType = mbd.factoryMethodReturnType; + // Attempt to get the actual ResolvableType for the bean. + ResolvableType beanType = null; + + // If it's a FactoryBean, we want to look at what it creates, not the factory class. + if (FactoryBean.class.isAssignableFrom(predictedType)) { + if (beanInstance == null && !isFactoryDereference) { + beanType = getTypeForFactoryBean(beanName, mbd, allowFactoryBeanInit); + predictedType = (beanType != null) ? beanType.resolve() : null; + if (predictedType == null) { + return false; + } + } } - if (resolvableType != null && resolvableType.resolve() == beanType) { - return typeToMatch.isAssignableFrom(resolvableType); + else if (isFactoryDereference) { + // Special case: A SmartInstantiationAwareBeanPostProcessor returned a non-FactoryBean + // type but we nevertheless are being asked to dereference a FactoryBean... + // Let's check the original bean class and proceed with it if it is a FactoryBean. + predictedType = predictBeanType(beanName, mbd, FactoryBean.class); + if (predictedType == null || !FactoryBean.class.isAssignableFrom(predictedType)) { + return false; + } } - return typeToMatch.isAssignableFrom(beanType); + + // We don't have an exact type but if bean definition target type or the factory + // method return type matches the predicted type then we can use that. + if (beanType == null) { + ResolvableType definedType = mbd.targetType; + if (definedType == null) { + definedType = mbd.factoryMethodReturnType; + } + if (definedType != null && definedType.resolve() == predictedType) { + beanType = definedType; + } + } + + // If we have a bean type use it so that generics are considered + if (beanType != null) { + return typeToMatch.isAssignableFrom(beanType); + } + + // If we don't have a bean type, fallback to the predicted type + return typeToMatch.isAssignableFrom(predictedType); } @Override @@ -645,7 +695,7 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp if (beanClass != null && FactoryBean.class.isAssignableFrom(beanClass)) { if (!BeanFactoryUtils.isFactoryDereference(name)) { // If it's a FactoryBean, we want to look at what it creates, not at the factory class. - return getTypeForFactoryBean(beanName, mbd); + return getTypeForFactoryBean(beanName, mbd, true).resolve(); } else { return beanClass; @@ -1551,6 +1601,53 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp return result; } + /** + * Determine the bean type for the given FactoryBean definition, as far as possible. + * Only called if there is no singleton instance registered for the target bean + * already. Implementations are only allowed to instantiate the factory bean if + * {@code allowInit} is {@code true}, otherwise they should try to determine the + * result through other means. + *

If {@code allowInit} is {@code true}, the default implementation will create + * the FactoryBean via {@code getBean} to call its {@code getObjectType} method. + * Subclasses are encouraged to optimize this, typically by inspecting the generic + * signature of the factory bean class or the factory method that creates it. If + * subclasses do instantiate the FactoryBean, they should consider trying the + * {@code getObjectType} method without fully populating the bean. If this fails, a + * full FactoryBean creation as performed by this implementation should be used as + * fallback. + * @param beanName the name of the bean + * @param mbd the merged bean definition for the bean + * @param allowInit if initialization of the bean is permitted + * @return the type for the bean if determinable, otherwise {@code ResolvableType.NONE} + * @since 5.2 + * @see org.springframework.beans.factory.FactoryBean#getObjectType() + * @see #getBean(String) + */ + protected ResolvableType getTypeForFactoryBean(String beanName, + RootBeanDefinition mbd, boolean allowInit) { + + if (allowInit && mbd.isSingleton()) { + try { + FactoryBean factoryBean = doGetBean(FACTORY_BEAN_PREFIX + beanName, FactoryBean.class, null, true); + Class objectType = getTypeForFactoryBean(factoryBean); + return (objectType != null) ? ResolvableType.forClass(objectType) : ResolvableType.NONE; + } + catch (BeanCreationException ex) { + if (ex.contains(BeanCurrentlyInCreationException.class)) { + logger.trace(LogMessage.format("Bean currently in creation on FactoryBean type check: %s", ex)); + } + else if (mbd.isLazyInit()) { + logger.trace(LogMessage.format("Bean creation exception on lazy FactoryBean type check: %s", ex)); + } + else { + logger.debug(LogMessage.format("Bean creation exception on non-lazy FactoryBean type check: %s", ex)); + } + onSuppressedException(ex); + } + } + return ResolvableType.NONE; + } + /** * Determine the bean type for the given FactoryBean definition, as far as possible. * Only called if there is no singleton instance registered for the target bean already. @@ -1565,35 +1662,12 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp * @return the type for the bean if determinable, or {@code null} otherwise * @see org.springframework.beans.factory.FactoryBean#getObjectType() * @see #getBean(String) + * @deprecated since 5.2 in favor of {@link #getTypeForFactoryBean(String, RootBeanDefinition, boolean)} */ @Nullable + @Deprecated protected Class getTypeForFactoryBean(String beanName, RootBeanDefinition mbd) { - if (!mbd.isSingleton()) { - return null; - } - try { - FactoryBean factoryBean = doGetBean(FACTORY_BEAN_PREFIX + beanName, FactoryBean.class, null, true); - return getTypeForFactoryBean(factoryBean); - } - catch (BeanCreationException ex) { - if (ex.contains(BeanCurrentlyInCreationException.class)) { - if (logger.isTraceEnabled()) { - logger.trace("Bean currently in creation on FactoryBean type check: " + ex); - } - } - else if (mbd.isLazyInit()) { - if (logger.isTraceEnabled()) { - logger.trace("Bean creation exception on lazy FactoryBean type check: " + ex); - } - } - else { - if (logger.isDebugEnabled()) { - logger.debug("Bean creation exception on non-lazy FactoryBean type check: " + ex); - } - } - onSuppressedException(ex); - return null; - } + return getTypeForFactoryBean(beanName, mbd, true).resolve(); } /** diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java index c94ae2cfde7..92048dc6bb0 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java @@ -77,6 +77,7 @@ import org.springframework.core.ResolvableType; import org.springframework.core.annotation.MergedAnnotation; import org.springframework.core.annotation.MergedAnnotations; import org.springframework.core.annotation.MergedAnnotations.SearchStrategy; +import org.springframework.core.log.LogMessage; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -514,48 +515,47 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto if (!mbd.isAbstract() && (allowEagerInit || (mbd.hasBeanClass() || !mbd.isLazyInit() || isAllowEagerClassLoading()) && !requiresEagerInitForType(mbd.getFactoryBeanName()))) { - // In case of FactoryBean, match object created by FactoryBean. boolean isFactoryBean = isFactoryBean(beanName, mbd); BeanDefinitionHolder dbd = mbd.getDecoratedDefinition(); - boolean matchFound = - (allowEagerInit || !isFactoryBean || - (dbd != null && !mbd.isLazyInit()) || containsSingleton(beanName)) && - (includeNonSingletons || - (dbd != null ? mbd.isSingleton() : isSingleton(beanName))) && - isTypeMatch(beanName, type); - if (!matchFound && isFactoryBean) { - // In case of FactoryBean, try to match FactoryBean instance itself next. - beanName = FACTORY_BEAN_PREFIX + beanName; - matchFound = (includeNonSingletons || mbd.isSingleton()) && isTypeMatch(beanName, type); + boolean matchFound = false; + boolean allowFactoryBeanInit = allowEagerInit || containsSingleton(beanName); + boolean isNonLazyDecorated = dbd != null && !mbd.isLazyInit(); + if (!isFactoryBean) { + if (includeNonSingletons || isSingleton(beanName, mbd, dbd)) { + matchFound = isTypeMatch(beanName, type, allowFactoryBeanInit); + } + } + else { + if (includeNonSingletons || isNonLazyDecorated || + (allowFactoryBeanInit && isSingleton(beanName, mbd, dbd))) { + matchFound = isTypeMatch(beanName, type, allowFactoryBeanInit); + } + if (!matchFound) { + // In case of FactoryBean, try to match FactoryBean instance itself next. + beanName = FACTORY_BEAN_PREFIX + beanName; + matchFound = isTypeMatch(beanName, type, allowFactoryBeanInit); + } } if (matchFound) { result.add(beanName); } } } - catch (CannotLoadBeanClassException ex) { + catch (CannotLoadBeanClassException | BeanDefinitionStoreException ex) { if (allowEagerInit) { throw ex; } - // Probably a class name with a placeholder: let's ignore it for type matching purposes. - if (logger.isTraceEnabled()) { - logger.trace("Ignoring bean class loading failure for bean '" + beanName + "'", ex); - } - onSuppressedException(ex); - } - catch (BeanDefinitionStoreException ex) { - if (allowEagerInit) { - throw ex; - } - // Probably some metadata with a placeholder: let's ignore it for type matching purposes. - if (logger.isTraceEnabled()) { - logger.trace("Ignoring unresolvable metadata in bean definition '" + beanName + "'", ex); - } + // Probably a placeholder: let's ignore it for type matching purposes. + LogMessage message = (ex instanceof CannotLoadBeanClassException) ? + LogMessage.format("Ignoring bean class loading failure for bean '%s'", beanName) : + LogMessage.format("Ignoring unresolvable metadata in bean definition '%s'", beanName); + logger.trace(message, ex); onSuppressedException(ex); } } } + // Check manually registered singletons too. for (String beanName : this.manualSingletonNames) { try { @@ -576,15 +576,17 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } catch (NoSuchBeanDefinitionException ex) { // Shouldn't happen - probably a result of circular reference resolution... - if (logger.isTraceEnabled()) { - logger.trace("Failed to check manually registered singleton with name '" + beanName + "'", ex); - } + logger.trace(LogMessage.format("Failed to check manually registered singleton with name '%s'", beanName), ex); } } return StringUtils.toStringArray(result); } + private boolean isSingleton(String beanName, RootBeanDefinition mbd, BeanDefinitionHolder dbd) { + return (dbd != null) ? mbd.isSingleton() : isSingleton(beanName); + } + /** * Check whether the specified bean would need to be eagerly initialized * in order to determine its type. diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanBeanEarlyDeductionTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanBeanEarlyDeductionTests.java new file mode 100644 index 00000000000..8c88c19a10c --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanBeanEarlyDeductionTests.java @@ -0,0 +1,172 @@ +/* + * Copyright 2002-2019 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.context.annotation; + +import java.util.Arrays; + +import org.junit.Test; + +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.config.BeanFactoryPostProcessor; +import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.AbstractBeanFactory; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Test for {@link AbstractBeanFactory} type inference from + * {@link FactoryBean FactoryBeans} defined in the configuration. + * + * @author Phillip Webb + */ +public class ConfigurationWithFactoryBeanBeanEarlyDeductionTests { + + @Test + public void preFreezeDirect() { + assertPreFreeze(DirectConfiguration.class); + } + + @Test + public void postFreezeDirect() { + assertPostFreeze(DirectConfiguration.class); + } + + @Test + public void preFreezeGenericMethod() { + assertPreFreeze(GenericMethodConfiguration.class); + } + + @Test + public void postFreezeGenericMethod() { + assertPostFreeze(GenericMethodConfiguration.class); + } + + @Test + public void preFreezeGenericClass() { + assertPreFreeze(GenericClassConfiguration.class); + } + + @Test + public void postFreezeGenericClass() { + assertPostFreeze(GenericClassConfiguration.class); + } + + private void assertPostFreeze(Class configurationClass) { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( + configurationClass); + assertContainsMyBeanName(context); + } + + private void assertPreFreeze(Class configurationClass, + BeanFactoryPostProcessor... postProcessors) { + NameCollectingBeanFactoryPostProcessor postProcessor = new NameCollectingBeanFactoryPostProcessor(); + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + Arrays.stream(postProcessors).forEach(context::addBeanFactoryPostProcessor); + context.addBeanFactoryPostProcessor(postProcessor); + context.register(configurationClass); + context.refresh(); + assertContainsMyBeanName(postProcessor.getNames()); + } + + private void assertContainsMyBeanName(AnnotationConfigApplicationContext context) { + assertContainsMyBeanName(context.getBeanNamesForType(MyBean.class, true, false)); + } + + private void assertContainsMyBeanName(String[] names) { + assertThat(names).containsExactly("myBean"); + } + + private static class NameCollectingBeanFactoryPostProcessor + implements BeanFactoryPostProcessor { + + private String[] names; + + @Override + public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) + throws BeansException { + this.names = beanFactory.getBeanNamesForType(MyBean.class, true, false); + } + + public String[] getNames() { + return this.names; + } + + } + + @Configuration + static class DirectConfiguration { + + @Bean + MyBean myBean() { + return new MyBean(); + } + + } + + @Configuration + static class GenericMethodConfiguration { + + @Bean + FactoryBean myBean() { + return new TestFactoryBean<>(new MyBean()); + } + + } + + @Configuration + static class GenericClassConfiguration { + + @Bean + MyFactoryBean myBean() { + return new MyFactoryBean(); + } + + } + + static class MyBean { + } + + static class TestFactoryBean implements FactoryBean { + + private final T instance; + + public TestFactoryBean(T instance) { + this.instance = instance; + } + + @Override + public T getObject() throws Exception { + return this.instance; + } + + @Override + public Class getObjectType() { + return this.instance.getClass(); + } + + } + + static class MyFactoryBean extends TestFactoryBean { + + public MyFactoryBean() { + super(new MyBean()); + } + + } + +}