From 89029a029bab0668014043f888ad449cfe91a9af Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sat, 20 Jul 2019 17:03:31 +0100 Subject: [PATCH 1/6] Add missing variants of getBeanNamesForType Update `ListableBeanFactory` and `BeanFactoryUtils` to add the missing `getBeanNamesForType` methods that accept a `ResolvableType` rather than a `Class`. This completes the work started in 778a01943b. Closes gh-23335 --- .../beans/factory/BeanFactoryUtils.java | 40 ++++++++++++++++++- .../beans/factory/ListableBeanFactory.java | 34 ++++++++++++++++ .../support/DefaultListableBeanFactory.java | 9 ++++- .../support/StaticListableBeanFactory.java | 34 +++++++++------- .../DefaultListableBeanFactoryTests.java | 2 + .../support/AbstractApplicationContext.java | 6 +++ .../setup/StubWebApplicationContext.java | 5 +++ 7 files changed, 113 insertions(+), 17 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/BeanFactoryUtils.java b/spring-beans/src/main/java/org/springframework/beans/factory/BeanFactoryUtils.java index 1b5682d25da1..dfc60e36c480 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/BeanFactoryUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/BeanFactoryUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * 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. @@ -172,6 +172,44 @@ public static String[] beanNamesForTypeIncludingAncestors(ListableBeanFactory lb return result; } + /** + * Get all bean names for the given type, including those defined in ancestor + * factories. Will return unique names in case of overridden bean definitions. + *

Does consider objects created by FactoryBeans if the "allowEagerInit" + * flag is set, which means that FactoryBeans will get initialized. If the + * object created by the FactoryBean doesn't match, the raw FactoryBean itself + * will be matched against the type. If "allowEagerInit" is not set, + * only raw FactoryBeans will be checked (which doesn't require initialization + * of each FactoryBean). + * @param lbf the bean factory + * @param includeNonSingletons whether to include prototype or scoped beans too + * or just singletons (also applies to FactoryBeans) + * @param allowEagerInit whether to initialize lazy-init singletons and + * objects created by FactoryBeans (or by factory methods with a + * "factory-bean" reference) for the type check. Note that FactoryBeans need to be + * eagerly initialized to determine their type: So be aware that passing in "true" + * for this flag will initialize FactoryBeans and "factory-bean" references. + * @param type the type that beans must match (as a {@code ResolvableType}) + * @return the array of matching bean names, or an empty array if none + * @since 5.2 + * @see ListableBeanFactory#getBeanNamesForType(ResolvableType, boolean, boolean) + */ + public static String[] beanNamesForTypeIncludingAncestors( + ListableBeanFactory lbf, ResolvableType type, boolean includeNonSingletons, boolean allowEagerInit) { + + Assert.notNull(lbf, "ListableBeanFactory must not be null"); + String[] result = lbf.getBeanNamesForType(type, includeNonSingletons, allowEagerInit); + if (lbf instanceof HierarchicalBeanFactory) { + HierarchicalBeanFactory hbf = (HierarchicalBeanFactory) lbf; + if (hbf.getParentBeanFactory() instanceof ListableBeanFactory) { + String[] parentResult = beanNamesForTypeIncludingAncestors( + (ListableBeanFactory) hbf.getParentBeanFactory(), type, includeNonSingletons, allowEagerInit); + result = mergeNamesWithParent(result, parentResult, hbf); + } + } + return result; + } + /** * Get all bean names for the given type, including those defined in ancestor * factories. Will return unique names in case of overridden bean definitions. diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/ListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/ListableBeanFactory.java index 0fc8d08686f2..09f0f5dade56 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/ListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/ListableBeanFactory.java @@ -116,6 +116,40 @@ public interface ListableBeanFactory extends BeanFactory { */ String[] getBeanNamesForType(ResolvableType type); + /** + * Return the names of beans matching the given type (including subclasses), + * judging from either bean definitions or the value of {@code getObjectType} + * in the case of FactoryBeans. + *

NOTE: This method introspects top-level beans only. It does not + * check nested beans which might match the specified type as well. + *

Does consider objects created by FactoryBeans if the "allowEagerInit" flag is set, + * which means that FactoryBeans will get initialized. If the object created by the + * FactoryBean doesn't match, the raw FactoryBean itself will be matched against the + * type. If "allowEagerInit" is not set, only raw FactoryBeans will be checked + * (which doesn't require initialization of each FactoryBean). + *

Does not consider any hierarchy this factory may participate in. + * Use BeanFactoryUtils' {@code beanNamesForTypeIncludingAncestors} + * to include beans in ancestor factories too. + *

Note: Does not ignore singleton beans that have been registered + * by other means than bean definitions. + *

Bean names returned by this method should always return bean names in the + * order of definition in the backend configuration, as far as possible. + * @param type the generically typed class or interface to match + * @param includeNonSingletons whether to include prototype or scoped beans too + * or just singletons (also applies to FactoryBeans) + * @param allowEagerInit whether to initialize lazy-init singletons and + * objects created by FactoryBeans (or by factory methods with a + * "factory-bean" reference) for the type check. Note that FactoryBeans need to be + * eagerly initialized to determine their type: So be aware that passing in "true" + * for this flag will initialize FactoryBeans and "factory-bean" references. + * @return the names of beans (or objects created by FactoryBeans) matching + * the given object type (including subclasses), or an empty array if none + * @since 5.2 + * @see FactoryBean#getObjectType + * @see BeanFactoryUtils#beanNamesForTypeIncludingAncestors(ListableBeanFactory, ResolvableType, boolean, boolean) + */ + String[] getBeanNamesForType(ResolvableType type, boolean includeNonSingletons, boolean allowEagerInit); + /** * Return the names of beans matching the given type (including subclasses), * judging from either bean definitions or the value of {@code getObjectType} 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 8a6052bc7696..c94ae2cfde72 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 @@ -463,12 +463,17 @@ public String[] getBeanDefinitionNames() { @Override public String[] getBeanNamesForType(ResolvableType type) { + return getBeanNamesForType(type, true, true); + } + + @Override + public String[] getBeanNamesForType(ResolvableType type, boolean includeNonSingletons, boolean allowEagerInit) { Class resolved = type.resolve(); if (resolved != null && !type.hasGenerics()) { - return getBeanNamesForType(resolved, true, true); + return getBeanNamesForType(resolved, includeNonSingletons, includeNonSingletons); } else { - return doGetBeanNamesForType(type, true, true); + return doGetBeanNamesForType(type, includeNonSingletons, includeNonSingletons); } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java index 2d894a6c3081..ee364317e355 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * 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. @@ -329,26 +329,32 @@ public String[] getBeanDefinitionNames() { @Override public String[] getBeanNamesForType(@Nullable ResolvableType type) { - boolean isFactoryType = false; - if (type != null) { - Class resolved = type.resolve(); - if (resolved != null && FactoryBean.class.isAssignableFrom(resolved)) { - isFactoryType = true; - } - } + return getBeanNamesForType(type, true, true); + } + + + @Override + public String[] getBeanNamesForType(@Nullable ResolvableType type, + boolean includeNonSingletons, boolean allowEagerInit) { + + Class resolved = (type != null ? type.resolve() : null); + boolean isFactoryType = resolved != null && FactoryBean.class.isAssignableFrom(resolved); List matches = new ArrayList<>(); + for (Map.Entry entry : this.beans.entrySet()) { - String name = entry.getKey(); + String beanName = entry.getKey(); Object beanInstance = entry.getValue(); if (beanInstance instanceof FactoryBean && !isFactoryType) { - Class objectType = ((FactoryBean) beanInstance).getObjectType(); - if (objectType != null && (type == null || type.isAssignableFrom(objectType))) { - matches.add(name); + FactoryBean factoryBean = (FactoryBean) beanInstance; + Class objectType = factoryBean.getObjectType(); + if ((includeNonSingletons || factoryBean.isSingleton()) && + objectType != null && (type == null || type.isAssignableFrom(objectType))) { + matches.add(beanName); } } else { if (type == null || type.isInstance(beanInstance)) { - matches.add(name); + matches.add(beanName); } } } @@ -362,7 +368,7 @@ public String[] getBeanNamesForType(@Nullable Class type) { @Override public String[] getBeanNamesForType(@Nullable Class type, boolean includeNonSingletons, boolean allowEagerInit) { - return getBeanNamesForType(ResolvableType.forClass(type)); + return getBeanNamesForType(ResolvableType.forClass(type), includeNonSingletons, allowEagerInit); } @Override diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index e24565011012..e23dc6b243ee 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java @@ -1751,6 +1751,8 @@ public void testGetBeanWithArgsNotCreatedForFactoryBeanChecking() { assertThat(lbf.getBeanNamesForType(ConstructorDependencyFactoryBean.class).length).isEqualTo(1); assertThat(lbf.getBeanNamesForType(ResolvableType.forClassWithGenerics(FactoryBean.class, Object.class)).length).isEqualTo(1); assertThat(lbf.getBeanNamesForType(ResolvableType.forClassWithGenerics(FactoryBean.class, String.class)).length).isEqualTo(0); + assertThat(lbf.getBeanNamesForType(ResolvableType.forClassWithGenerics(FactoryBean.class, Object.class), true, true).length).isEqualTo(1); + assertThat(lbf.getBeanNamesForType(ResolvableType.forClassWithGenerics(FactoryBean.class, String.class), true, true).length).isEqualTo(0); } private RootBeanDefinition createConstructorDependencyBeanDefinition(int age) { diff --git a/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java b/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java index bcb11ef4b6f7..aa46bb8653ef 100644 --- a/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java +++ b/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java @@ -1208,6 +1208,12 @@ public String[] getBeanNamesForType(ResolvableType type) { return getBeanFactory().getBeanNamesForType(type); } + @Override + public String[] getBeanNamesForType(ResolvableType type, boolean includeNonSingletons, boolean allowEagerInit) { + assertBeanFactoryActive(); + return getBeanFactory().getBeanNamesForType(type, includeNonSingletons, allowEagerInit); + } + @Override public String[] getBeanNamesForType(@Nullable Class type) { assertBeanFactoryActive(); diff --git a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StubWebApplicationContext.java b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StubWebApplicationContext.java index 53c0298dbfc5..e86bd75ed57e 100644 --- a/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StubWebApplicationContext.java +++ b/spring-test/src/main/java/org/springframework/test/web/servlet/setup/StubWebApplicationContext.java @@ -251,6 +251,11 @@ public String[] getBeanNamesForType(@Nullable ResolvableType type) { return this.beanFactory.getBeanNamesForType(type); } + @Override + public String[] getBeanNamesForType(@Nullable ResolvableType type, boolean includeNonSingletons, boolean allowEagerInit) { + return this.beanFactory.getBeanNamesForType(type, includeNonSingletons, allowEagerInit); + } + @Override public String[] getBeanNamesForType(@Nullable Class type) { return this.beanFactory.getBeanNamesForType(type); From 3ac8de23f503e8ae9badbc55a3db5f433c3289c5 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 17 Jul 2019 13:45:09 +0100 Subject: [PATCH 2/6] Retain merged bean definition caches when possible Update the logic in `AbstractBeanFactory` so that caches from merged bean definitions remain whenever possible. Prior to this commit, all merged bean definitions would be completely removed after bean post processing in case a processor changed the bean type. It's fairly unlikely these days that the bean type will actually change, so instead we now compare a subset of the old cached properties against the newly created definition. Only if key properties have changed do we now discard the older cached values. Closes gh-23336 --- .../AbstractAutowireCapableBeanFactory.java | 7 ++-- .../factory/support/AbstractBeanFactory.java | 36 ++++++++++++++++--- .../factory/support/RootBeanDefinition.java | 3 ++ 3 files changed, 38 insertions(+), 8 deletions(-) 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 0c314a5b944d..a84f675f0ea7 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 @@ -114,6 +114,7 @@ * @author Costin Leau * @author Chris Beams * @author Sam Brannen + * @author Phillip Webb * @since 13.02.2004 * @see RootBeanDefinition * @see DefaultListableBeanFactory @@ -646,16 +647,16 @@ else if (!this.allowRawInjectionDespiteWrapping && hasDependentBean(beanName)) { @Nullable protected Class predictBeanType(String beanName, RootBeanDefinition mbd, Class... typesToMatch) { Class targetType = determineTargetType(beanName, mbd, typesToMatch); - // Apply SmartInstantiationAwareBeanPostProcessors to predict the // eventual type after a before-instantiation shortcut. if (targetType != null && !mbd.isSynthetic() && hasInstantiationAwareBeanPostProcessors()) { + boolean matchingOnlyFactoryBean = typesToMatch.length == 1 && typesToMatch[0] == FactoryBean.class; for (BeanPostProcessor bp : getBeanPostProcessors()) { if (bp instanceof SmartInstantiationAwareBeanPostProcessor) { SmartInstantiationAwareBeanPostProcessor ibp = (SmartInstantiationAwareBeanPostProcessor) bp; Class predicted = ibp.predictBeanType(targetType, beanName); - if (predicted != null && (typesToMatch.length != 1 || FactoryBean.class != typesToMatch[0] || - FactoryBean.class.isAssignableFrom(predicted))) { + if (predicted != null && + (!matchingOnlyFactoryBean || FactoryBean.class.isAssignableFrom(predicted))) { return predicted; } } 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 e6fcceb9e494..73f0f95c0108 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 @@ -103,6 +103,7 @@ * @author Juergen Hoeller * @author Costin Leau * @author Chris Beams + * @author Phillip Webb * @since 15 April 2001 * @see #getBeanDefinition * @see #createBean @@ -1215,7 +1216,7 @@ protected void registerCustomEditors(PropertyEditorRegistry registry) { protected RootBeanDefinition getMergedLocalBeanDefinition(String beanName) throws BeansException { // Quick check on the concurrent map first, with minimal locking. RootBeanDefinition mbd = this.mergedBeanDefinitions.get(beanName); - if (mbd != null) { + if (mbd != null && !mbd.stale) { return mbd; } return getMergedBeanDefinition(beanName, getBeanDefinition(beanName)); @@ -1251,13 +1252,16 @@ protected RootBeanDefinition getMergedBeanDefinition( synchronized (this.mergedBeanDefinitions) { RootBeanDefinition mbd = null; + RootBeanDefinition previous = null; // Check with full lock now in order to enforce the same merged instance. if (containingBd == null) { mbd = this.mergedBeanDefinitions.get(beanName); } - if (mbd == null) { + if (mbd == null || mbd.stale) { + previous = mbd; + mbd = null; if (bd.getParentName() == null) { // Use copy of given root bean definition. if (bd instanceof RootBeanDefinition) { @@ -1315,11 +1319,26 @@ protected RootBeanDefinition getMergedBeanDefinition( this.mergedBeanDefinitions.put(beanName, mbd); } } - + if (previous != null) { + copyRelevantMergedBeanDefinitionCaches(previous, mbd); + } return mbd; } } + private void copyRelevantMergedBeanDefinitionCaches(RootBeanDefinition previous, + RootBeanDefinition mbd) { + if (ObjectUtils.nullSafeEquals(mbd.getBeanClassName(), previous.getBeanClassName()) && + ObjectUtils.nullSafeEquals(mbd.getFactoryBeanName(), previous.getFactoryBeanName()) && + ObjectUtils.nullSafeEquals(mbd.getFactoryMethodName(), previous.getFactoryMethodName()) && + (mbd.targetType == null || mbd.targetType.equals(previous.targetType))) { + mbd.targetType = previous.targetType; + mbd.resolvedTargetType = previous.resolvedTargetType; + mbd.factoryMethodReturnType = previous.factoryMethodReturnType; + mbd.factoryMethodToIntrospect = previous.factoryMethodToIntrospect; + } + } + /** * Check the given merged bean definition, * potentially throwing validation exceptions. @@ -1342,7 +1361,10 @@ protected void checkMergedBeanDefinition(RootBeanDefinition mbd, String beanName * @param beanName the bean name to clear the merged definition for */ protected void clearMergedBeanDefinition(String beanName) { - this.mergedBeanDefinitions.remove(beanName); + RootBeanDefinition bd = this.mergedBeanDefinitions.get(beanName); + if (bd != null) { + bd.stale = true; + } } /** @@ -1354,7 +1376,11 @@ protected void clearMergedBeanDefinition(String beanName) { * @since 4.2 */ public void clearMetadataCache() { - this.mergedBeanDefinitions.keySet().removeIf(bean -> !isBeanEligibleForMetadataCaching(bean)); + this.mergedBeanDefinitions.forEach((beanName, bd) -> { + if (!isBeanEligibleForMetadataCaching(beanName)) { + bd.stale = true; + } + }); } /** diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java index 19fdb8314fc5..f393829e7472 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java @@ -60,6 +60,9 @@ public class RootBeanDefinition extends AbstractBeanDefinition { @Nullable private AnnotatedElement qualifiedElement; + /** Determines if the definition needs to be re-merged. */ + volatile boolean stale; + boolean allowCaching = true; boolean isFactoryMethodUnique = false; From 26997380c775dddcc1aa5a0875b4002c1022b4f8 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 18 Jul 2019 11:48:32 +0100 Subject: [PATCH 3/6] Cache AbstractBeanFactory.isFactoryBean results Add an additional cache to the `RootBeanDefinition` to save recalculating the result of `isFactoryBean`. Closes gh-23337 --- .../beans/factory/support/AbstractBeanFactory.java | 10 ++++++++-- .../beans/factory/support/RootBeanDefinition.java | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-) 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 73f0f95c0108..1f48fedc2faa 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 @@ -1333,6 +1333,7 @@ private void copyRelevantMergedBeanDefinitionCaches(RootBeanDefinition previous, ObjectUtils.nullSafeEquals(mbd.getFactoryMethodName(), previous.getFactoryMethodName()) && (mbd.targetType == null || mbd.targetType.equals(previous.targetType))) { mbd.targetType = previous.targetType; + mbd.isFactoryBean = previous.isFactoryBean; mbd.resolvedTargetType = previous.resolvedTargetType; mbd.factoryMethodReturnType = previous.factoryMethodReturnType; mbd.factoryMethodToIntrospect = previous.factoryMethodToIntrospect; @@ -1541,8 +1542,13 @@ protected Class predictBeanType(String beanName, RootBeanDefinition mbd, Clas * @param mbd the corresponding bean definition */ protected boolean isFactoryBean(String beanName, RootBeanDefinition mbd) { - Class beanType = predictBeanType(beanName, mbd, FactoryBean.class); - return (beanType != null && FactoryBean.class.isAssignableFrom(beanType)); + Boolean result = mbd.isFactoryBean; + if (result == null) { + Class beanType = predictBeanType(beanName, mbd, FactoryBean.class); + result = beanType != null && FactoryBean.class.isAssignableFrom(beanType); + mbd.isFactoryBean = result; + } + return result; } /** diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java index f393829e7472..8422ad309ebf 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java @@ -74,6 +74,10 @@ public class RootBeanDefinition extends AbstractBeanDefinition { @Nullable volatile Class resolvedTargetType; + /** Package-visible field for caching if the bean is a factory bean. */ + @Nullable + volatile Boolean isFactoryBean; + /** Package-visible field for caching the return type of a generically typed factory method. */ @Nullable volatile ResolvableType factoryMethodReturnType; From a3031ece3a4f154ce10ae64b04f927ed8336a7e9 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 18 Jul 2019 18:43:48 +0100 Subject: [PATCH 4/6] 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 | 184 +++++++++++----- .../support/DefaultListableBeanFactory.java | 60 +++--- ...ithFactoryBeanBeanEarlyDeductionTests.java | 172 +++++++++++++++ 4 files changed, 458 insertions(+), 160 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 a84f675f0ea7..8602dd93606c 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.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.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 @@ protected Class getTypeForFactoryMethod(String beanName, RootBeanDefinition m * 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 @@ protected Class getTypeForFactoryBean(String beanName, RootBeanDefinition mbd // 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 @@ protected Class getTypeForFactoryBean(String beanName, RootBeanDefinition mbd * @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); - - // 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); + Class factoryBeanClass = ClassUtils.getUserClass(beanClass); + FactoryBeanMethodTypeFinder finder = new FactoryBeanMethodTypeFinder(factoryMethodName); + ReflectionUtils.doWithMethods(factoryBeanClass, finder, ReflectionUtils.USER_DECLARED_METHODS); + return finder.getResult(); + } - 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 String getDependencyName() { } } + /** + * {@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 1f48fedc2faa..1eff7b50ff0c 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.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 boolean isPrototype(String name) throws NoSuchBeanDefinitionException { @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 boolean isTypeMatch(String name, ResolvableType typeToMatch) throws NoSuc 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 @@ else if (containsSingleton(beanName) && !containsBeanDefinition(beanName)) { // 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 @@ else if (containsSingleton(beanName) && !containsBeanDefinition(beanName)) { 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); + + // Attempt to predict the bean type + Class predictedType = null; + + // 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; + } } } - Class beanType = predictBeanType(beanName, mbd, typesToMatch); - if (beanType == null) { - return false; + // If we couldn't use the target type, try regular prediction. + if (predictedType == null) { + predictedType = predictBeanType(beanName, mbd, typesToMatch); + if (predictedType == null) { + return false; + } } - // 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) { + // 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; } } } - else if (BeanFactoryUtils.isFactoryDereference(name)) { + 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. - beanType = predictBeanType(beanName, mbd, FactoryBean.class); - if (beanType == null || !FactoryBean.class.isAssignableFrom(beanType)) { + predictedType = predictBeanType(beanName, mbd, FactoryBean.class); + if (predictedType == null || !FactoryBean.class.isAssignableFrom(predictedType)) { return false; } } - ResolvableType resolvableType = mbd.targetType; - if (resolvableType == null) { - resolvableType = mbd.factoryMethodReturnType; + // 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 (resolvableType != null && resolvableType.resolve() == beanType) { - return typeToMatch.isAssignableFrom(resolvableType); + + // If we have a bean type use it so that generics are considered + if (beanType != null) { + return typeToMatch.isAssignableFrom(beanType); } - 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 Class getType(String name) throws NoSuchBeanDefinitionException { 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 @@ protected boolean isFactoryBean(String beanName, RootBeanDefinition mbd) { 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 @@ protected boolean isFactoryBean(String beanName, RootBeanDefinition mbd) { * @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 c94ae2cfde72..92048dc6bb0e 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.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 @@ private String[] doGetBeanNamesForType(ResolvableType type, boolean includeNonSi 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 @@ private String[] doGetBeanNamesForType(ResolvableType type, boolean includeNonSi } 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 000000000000..8c88c19a10c6 --- /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()); + } + + } + +} From 45d1a62057685845615b0c087b6364e6581b0f0c Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 23 Jul 2019 08:21:42 +0100 Subject: [PATCH 5/6] Support FactoryBean bean definition attributes Update `getTypeForFactoryBean` detection so that a bean definition attribute can be used to supply the result. This commit allows projects such as Spring Data to provide the result that would be supplied by `getObjectType` early so that we don't need to initialize the `FactoryBean` unnecessarily. Closes gh-23338 --- .../beans/factory/FactoryBean.java | 13 ++++- .../AbstractAutowireCapableBeanFactory.java | 6 ++- .../factory/support/AbstractBeanFactory.java | 30 ++++++++++- ...ithFactoryBeanBeanEarlyDeductionTests.java | 54 ++++++++++++++++++- 4 files changed, 98 insertions(+), 5 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/FactoryBean.java b/spring-beans/src/main/java/org/springframework/beans/factory/FactoryBean.java index b4b847373ff1..0c806cef7b92 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/FactoryBean.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/FactoryBean.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * 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. @@ -16,6 +16,7 @@ package org.springframework.beans.factory; +import org.springframework.core.AttributeAccessor; import org.springframework.lang.Nullable; /** @@ -58,6 +59,16 @@ */ public interface FactoryBean { + /** + * The name of an attribute that can be + * {@link AttributeAccessor#setAttribute set} on a + * {@link org.springframework.beans.factory.config.BeanDefinition} so that + * factory beans can signal their object type when it can't be deduced from + * the factory bean class. + */ + public static final String OBJECT_TYPE_ATTRIBUTE = "factoryBeanObjectType"; + + /** * Return an instance (possibly shared or independent) of the object * managed by this factory. 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 8602dd93606c..e2660677389e 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 @@ -826,7 +826,11 @@ protected Class getTypeForFactoryMethod(String beanName, RootBeanDefinition m protected ResolvableType getTypeForFactoryBean(String beanName, RootBeanDefinition mbd, boolean allowInit) { - ResolvableType result = ResolvableType.NONE; + // Check it the the bean definition itself has defined the type with an attribute + ResolvableType result = getTypeForFactoryBeanFromAttributes(mbd); + if (result != ResolvableType.NONE) { + return result; + } ResolvableType beanType = mbd.hasBeanClass() ? ResolvableType.forClass(mbd.getBeanClass()) : 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 1eff7b50ff0c..5a1b92c75c76 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 @@ -65,6 +65,7 @@ import org.springframework.beans.factory.config.DestructionAwareBeanPostProcessor; import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessor; import org.springframework.beans.factory.config.Scope; +import org.springframework.core.AttributeAccessor; import org.springframework.core.DecoratingClassLoader; import org.springframework.core.NamedThreadLocal; import org.springframework.core.ResolvableType; @@ -1607,7 +1608,8 @@ protected boolean isFactoryBean(String beanName, RootBeanDefinition mbd) { * 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 + *

If no {@link FactoryBean#OBJECT_TYPE_ATTRIBUTE} if set on the bean definition + * and {@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 @@ -1617,7 +1619,7 @@ protected boolean isFactoryBean(String beanName, RootBeanDefinition mbd) { * 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 + * @param allowInit if initialization of the FactoryBean is permitted * @return the type for the bean if determinable, otherwise {@code ResolvableType.NONE} * @since 5.2 * @see org.springframework.beans.factory.FactoryBean#getObjectType() @@ -1626,6 +1628,11 @@ protected boolean isFactoryBean(String beanName, RootBeanDefinition mbd) { protected ResolvableType getTypeForFactoryBean(String beanName, RootBeanDefinition mbd, boolean allowInit) { + ResolvableType result = getTypeForFactoryBeanFromAttributes(mbd); + if (result != ResolvableType.NONE) { + return result; + } + if (allowInit && mbd.isSingleton()) { try { FactoryBean factoryBean = doGetBean(FACTORY_BEAN_PREFIX + beanName, FactoryBean.class, null, true); @@ -1648,6 +1655,25 @@ else if (mbd.isLazyInit()) { return ResolvableType.NONE; } + /** + * Determine the bean type for a FactoryBean by inspecting its attributes for a + * {@link FactoryBean#OBJECT_TYPE_ATTRIBUTE} value. + * @param attributes the attributes to inspect + * @return a {@link ResolvableType} extracted from the attributes or + * {@code ResolvableType.NONE} + * @since 5.2 + */ + ResolvableType getTypeForFactoryBeanFromAttributes(AttributeAccessor attributes) { + Object attribute = attributes.getAttribute(FactoryBean.OBJECT_TYPE_ATTRIBUTE); + if (attribute instanceof ResolvableType) { + return (ResolvableType) attribute; + } + if (attribute instanceof Class) { + return ResolvableType.forClass((Class) attribute); + } + 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. 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 index 8c88c19a10c6..24c7cfb558b6 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanBeanEarlyDeductionTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationWithFactoryBeanBeanEarlyDeductionTests.java @@ -22,9 +22,13 @@ import org.springframework.beans.BeansException; import org.springframework.beans.factory.FactoryBean; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.support.AbstractBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionBuilder; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.core.type.AnnotationMetadata; import static org.assertj.core.api.Assertions.assertThat; @@ -66,6 +70,16 @@ public void postFreezeGenericClass() { assertPostFreeze(GenericClassConfiguration.class); } + @Test + public void preFreezeAttribute() { + assertPreFreeze(AttributeClassConfiguration.class); + } + + @Test + public void postFreezeAttribute() { + assertPostFreeze(AttributeClassConfiguration.class); + } + private void assertPostFreeze(Class configurationClass) { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( configurationClass); @@ -138,7 +152,29 @@ MyFactoryBean myBean() { } - static class MyBean { + @Configuration + @Import(AttributeClassRegistrar.class) + static class AttributeClassConfiguration { + + } + + static class AttributeClassRegistrar implements ImportBeanDefinitionRegistrar { + + @Override + public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, + BeanDefinitionRegistry registry) { + BeanDefinition definition = BeanDefinitionBuilder.genericBeanDefinition( + RawWithAbstractObjectTypeFactoryBean.class).getBeanDefinition(); + definition.setAttribute(FactoryBean.OBJECT_TYPE_ATTRIBUTE, MyBean.class); + registry.registerBeanDefinition("myBean", definition); + } + + } + + abstract static class AbstractMyBean { + } + + static class MyBean extends AbstractMyBean { } static class TestFactoryBean implements FactoryBean { @@ -169,4 +205,20 @@ public MyFactoryBean() { } + static class RawWithAbstractObjectTypeFactoryBean implements FactoryBean { + + private final Object object = new MyBean(); + + @Override + public Object getObject() throws Exception { + return object; + } + + @Override + public Class getObjectType() { + return MyBean.class; + } + + } + } From 116c35712ee76731546b688fe9d4f4bafb95ebbd Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sat, 20 Jul 2019 20:50:02 +0100 Subject: [PATCH 6/6] Use shared zero length array constants Update code that's often called so that zero length array results use a single shared static constant, rather than a new instance for each call. Closes gh-23340 --- .../core/annotation/AnnotationTypeMapping.java | 8 ++++++-- .../java/org/springframework/util/MethodInvoker.java | 7 +++++-- .../java/org/springframework/util/ObjectUtils.java | 5 +++-- .../java/org/springframework/util/StringUtils.java | 10 ++++++---- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java index b8dc512c993f..eebb9f6e3df3 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationTypeMapping.java @@ -48,6 +48,10 @@ */ final class AnnotationTypeMapping { + + private static final MirrorSet[] EMPTY_MIRROR_SETS = new MirrorSet[0]; + + @Nullable private final AnnotationTypeMapping source; @@ -550,7 +554,7 @@ class MirrorSets { MirrorSets() { this.assigned = new MirrorSet[attributes.size()]; - this.mirrorSets = new MirrorSet[0]; + this.mirrorSets = EMPTY_MIRROR_SETS; } void updateFrom(Collection aliases) { @@ -575,7 +579,7 @@ void updateFrom(Collection aliases) { mirrorSet.update(); Set unique = new LinkedHashSet<>(Arrays.asList(this.assigned)); unique.remove(null); - this.mirrorSets = unique.toArray(new MirrorSet[0]); + this.mirrorSets = unique.toArray(EMPTY_MIRROR_SETS); } } diff --git a/spring-core/src/main/java/org/springframework/util/MethodInvoker.java b/spring-core/src/main/java/org/springframework/util/MethodInvoker.java index 41d5742d9d0f..e7b78ec5661f 100644 --- a/spring-core/src/main/java/org/springframework/util/MethodInvoker.java +++ b/spring-core/src/main/java/org/springframework/util/MethodInvoker.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * 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. @@ -38,6 +38,9 @@ */ public class MethodInvoker { + private static final Object[] EMPTY_ARGUMENTS = new Object[0]; + + @Nullable protected Class targetClass; @@ -141,7 +144,7 @@ public void setArguments(Object... arguments) { * Return the arguments for the method invocation. */ public Object[] getArguments() { - return (this.arguments != null ? this.arguments : new Object[0]); + return (this.arguments != null ? this.arguments : EMPTY_ARGUMENTS); } diff --git a/spring-core/src/main/java/org/springframework/util/ObjectUtils.java b/spring-core/src/main/java/org/springframework/util/ObjectUtils.java index f8497cd46c20..6592abd2a34b 100644 --- a/spring-core/src/main/java/org/springframework/util/ObjectUtils.java +++ b/spring-core/src/main/java/org/springframework/util/ObjectUtils.java @@ -54,6 +54,7 @@ public abstract class ObjectUtils { private static final String ARRAY_END = "}"; private static final String EMPTY_ARRAY = ARRAY_START + ARRAY_END; private static final String ARRAY_ELEMENT_SEPARATOR = ", "; + private static final Object[] EMPTY_OBJECT_ARRAY = new Object[0]; /** @@ -282,14 +283,14 @@ public static Object[] toObjectArray(@Nullable Object source) { return (Object[]) source; } if (source == null) { - return new Object[0]; + return EMPTY_OBJECT_ARRAY; } if (!source.getClass().isArray()) { throw new IllegalArgumentException("Source is not an array: " + source); } int length = Array.getLength(source); if (length == 0) { - return new Object[0]; + return EMPTY_OBJECT_ARRAY; } Class wrapperType = Array.get(source, 0).getClass(); Object[] newArray = (Object[]) Array.newInstance(wrapperType, length); diff --git a/spring-core/src/main/java/org/springframework/util/StringUtils.java b/spring-core/src/main/java/org/springframework/util/StringUtils.java index 7a9233ea4822..9062fba98198 100644 --- a/spring-core/src/main/java/org/springframework/util/StringUtils.java +++ b/spring-core/src/main/java/org/springframework/util/StringUtils.java @@ -60,6 +60,8 @@ */ public abstract class StringUtils { + private static final String[] EMPTY_STRING_ARRAY = {}; + private static final String FOLDER_SEPARATOR = "/"; private static final String WINDOWS_FOLDER_SEPARATOR = "\\"; @@ -898,7 +900,7 @@ public static TimeZone parseTimeZoneString(String timeZoneString) { * @return the resulting {@code String} array */ public static String[] toStringArray(@Nullable Collection collection) { - return (collection != null ? collection.toArray(new String[0]) : new String[0]); + return (collection != null || collection.isEmpty() ? collection.toArray(EMPTY_STRING_ARRAY) : EMPTY_STRING_ARRAY); } /** @@ -909,7 +911,7 @@ public static String[] toStringArray(@Nullable Collection collection) { * @return the resulting {@code String} array */ public static String[] toStringArray(@Nullable Enumeration enumeration) { - return (enumeration != null ? toStringArray(Collections.list(enumeration)) : new String[0]); + return (enumeration != null ? toStringArray(Collections.list(enumeration)) : EMPTY_STRING_ARRAY); } /** @@ -1151,7 +1153,7 @@ public static String[] tokenizeToStringArray( @Nullable String str, String delimiters, boolean trimTokens, boolean ignoreEmptyTokens) { if (str == null) { - return new String[0]; + return EMPTY_STRING_ARRAY; } StringTokenizer st = new StringTokenizer(str, delimiters); @@ -1204,7 +1206,7 @@ public static String[] delimitedListToStringArray( @Nullable String str, @Nullable String delimiter, @Nullable String charsToDelete) { if (str == null) { - return new String[0]; + return EMPTY_STRING_ARRAY; } if (delimiter == null) { return new String[] {str};