Skip to content

Commit 40c6213

Browse files
jhoellerjhoeller
committed
Early resolution of unique factory methods in configuration classes
Includes consistent bean class resolution in the enhancement step as well as general reflection optimizations for user-declared methods. Closes gh-22420
1 parent a35adc6 commit 40c6213

File tree

10 files changed

+270
-141
lines changed

10 files changed

+270
-141
lines changed

‎spring-aop/src/main/java/org/springframework/aop/aspectj/annotation/ReflectiveAspectJAdvisorFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -153,7 +153,7 @@ private List<Method> getAdvisorMethods(Class<?> aspectClass) {
153153
if (AnnotationUtils.getAnnotation(method, Pointcut.class) == null) {
154154
methods.add(method);
155155
}
156-
});
156+
}, ReflectionUtils.USER_DECLARED_METHODS);
157157
methods.sort(METHOD_COMPARATOR);
158158
return methods;
159159
}

‎spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

Lines changed: 80 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -706,96 +706,100 @@ protected Class<?> getTypeForFactoryMethod(String beanName, RootBeanDefinition m
706706
return cachedReturnType.resolve();
707707
}
708708

709-
Class<?> factoryClass;
710-
boolean isStatic = true;
709+
Class<?> commonType = null;
710+
Method uniqueCandidate = mbd.factoryMethodToIntrospect;
711711

712-
String factoryBeanName = mbd.getFactoryBeanName();
713-
if (factoryBeanName != null) {
714-
if (factoryBeanName.equals(beanName)) {
715-
throw new BeanDefinitionStoreException(mbd.getResourceDescription(), beanName,
716-
"factory-bean reference points back to the same bean definition");
717-
}
718-
// Check declared factory method return type on factory class.
719-
factoryClass = getType(factoryBeanName);
720-
isStatic = false;
721-
}
722-
else {
723-
// Check declared factory method return type on bean class.
724-
factoryClass = resolveBeanClass(mbd, beanName, typesToMatch);
725-
}
712+
if (uniqueCandidate == null) {
713+
Class<?> factoryClass;
714+
boolean isStatic = true;
726715

727-
if (factoryClass == null) {
728-
return null;
729-
}
730-
factoryClass = ClassUtils.getUserClass(factoryClass);
716+
String factoryBeanName = mbd.getFactoryBeanName();
717+
if (factoryBeanName != null) {
718+
if (factoryBeanName.equals(beanName)) {
719+
throw new BeanDefinitionStoreException(mbd.getResourceDescription(), beanName,
720+
"factory-bean reference points back to the same bean definition");
721+
}
722+
// Check declared factory method return type on factory class.
723+
factoryClass = getType(factoryBeanName);
724+
isStatic = false;
725+
}
726+
else {
727+
// Check declared factory method return type on bean class.
728+
factoryClass = resolveBeanClass(mbd, beanName, typesToMatch);
729+
}
731730

732-
// If all factory methods have the same return type, return that type.
733-
// Can't clearly figure out exact method due to type converting / autowiring!
734-
Class<?> commonType = null;
735-
Method uniqueCandidate = null;
736-
int minNrOfArgs =
737-
(mbd.hasConstructorArgumentValues() ? mbd.getConstructorArgumentValues().getArgumentCount() : 0);
738-
Method[] candidates = this.factoryMethodCandidateCache.computeIfAbsent(
739-
factoryClass, ReflectionUtils::getUniqueDeclaredMethods);
740-
741-
for (Method candidate : candidates) {
742-
if (Modifier.isStatic(candidate.getModifiers()) == isStatic && mbd.isFactoryMethod(candidate) &&
743-
candidate.getParameterCount() >= minNrOfArgs) {
744-
// Declared type variables to inspect?
745-
if (candidate.getTypeParameters().length > 0) {
746-
try {
747-
// Fully resolve parameter names and argument values.
748-
Class<?>[] paramTypes = candidate.getParameterTypes();
749-
String[] paramNames = null;
750-
ParameterNameDiscoverer pnd = getParameterNameDiscoverer();
751-
if (pnd != null) {
752-
paramNames = pnd.getParameterNames(candidate);
753-
}
754-
ConstructorArgumentValues cav = mbd.getConstructorArgumentValues();
755-
Set<ConstructorArgumentValues.ValueHolder> usedValueHolders = new HashSet<>(paramTypes.length);
756-
Object[] args = new Object[paramTypes.length];
757-
for (int i = 0; i < args.length; i++) {
758-
ConstructorArgumentValues.ValueHolder valueHolder = cav.getArgumentValue(
759-
i, paramTypes[i], (paramNames != null ? paramNames[i] : null), usedValueHolders);
760-
if (valueHolder == null) {
761-
valueHolder = cav.getGenericArgumentValue(null, null, usedValueHolders);
731+
if (factoryClass == null) {
732+
return null;
733+
}
734+
factoryClass = ClassUtils.getUserClass(factoryClass);
735+
736+
// If all factory methods have the same return type, return that type.
737+
// Can't clearly figure out exact method due to type converting / autowiring!
738+
int minNrOfArgs =
739+
(mbd.hasConstructorArgumentValues() ? mbd.getConstructorArgumentValues().getArgumentCount() : 0);
740+
Method[] candidates = this.factoryMethodCandidateCache.computeIfAbsent(factoryClass,
741+
clazz -> ReflectionUtils.getUniqueDeclaredMethods(clazz, ReflectionUtils.USER_DECLARED_METHODS));
742+
743+
for (Method candidate : candidates) {
744+
if (Modifier.isStatic(candidate.getModifiers()) == isStatic && mbd.isFactoryMethod(candidate) &&
745+
candidate.getParameterCount() >= minNrOfArgs) {
746+
// Declared type variables to inspect?
747+
if (candidate.getTypeParameters().length > 0) {
748+
try {
749+
// Fully resolve parameter names and argument values.
750+
Class<?>[] paramTypes = candidate.getParameterTypes();
751+
String[] paramNames = null;
752+
ParameterNameDiscoverer pnd = getParameterNameDiscoverer();
753+
if (pnd != null) {
754+
paramNames = pnd.getParameterNames(candidate);
755+
}
756+
ConstructorArgumentValues cav = mbd.getConstructorArgumentValues();
757+
Set<ConstructorArgumentValues.ValueHolder> usedValueHolders = new HashSet<>(paramTypes.length);
758+
Object[] args = new Object[paramTypes.length];
759+
for (int i = 0; i < args.length; i++) {
760+
ConstructorArgumentValues.ValueHolder valueHolder = cav.getArgumentValue(
761+
i, paramTypes[i], (paramNames != null ? paramNames[i] : null), usedValueHolders);
762+
if (valueHolder == null) {
763+
valueHolder = cav.getGenericArgumentValue(null, null, usedValueHolders);
764+
}
765+
if (valueHolder != null) {
766+
args[i] = valueHolder.getValue();
767+
usedValueHolders.add(valueHolder);
768+
}
762769
}
763-
if (valueHolder != null) {
764-
args[i] = valueHolder.getValue();
765-
usedValueHolders.add(valueHolder);
770+
Class<?> returnType = AutowireUtils.resolveReturnTypeForFactoryMethod(
771+
candidate, args, getBeanClassLoader());
772+
uniqueCandidate = (commonType == null && returnType == candidate.getReturnType() ?
773+
candidate : null);
774+
commonType = ClassUtils.determineCommonAncestor(returnType, commonType);
775+
if (commonType == null) {
776+
// Ambiguous return types found: return null to indicate "not determinable".
777+
return null;
778+
}
779+
}
780+
catch (Throwable ex) {
781+
if (logger.isDebugEnabled()) {
782+
logger.debug("Failed to resolve generic return type for factory method: " + ex);
766783
}
767784
}
768-
Class<?> returnType = AutowireUtils.resolveReturnTypeForFactoryMethod(
769-
candidate, args, getBeanClassLoader());
770-
uniqueCandidate = (commonType == null && returnType == candidate.getReturnType() ?
771-
candidate : null);
772-
commonType = ClassUtils.determineCommonAncestor(returnType, commonType);
785+
}
786+
else {
787+
uniqueCandidate = (commonType == null ? candidate : null);
788+
commonType = ClassUtils.determineCommonAncestor(candidate.getReturnType(), commonType);
773789
if (commonType == null) {
774790
// Ambiguous return types found: return null to indicate "not determinable".
775791
return null;
776792
}
777793
}
778-
catch (Throwable ex) {
779-
if (logger.isDebugEnabled()) {
780-
logger.debug("Failed to resolve generic return type for factory method: " + ex);
781-
}
782-
}
783-
}
784-
else {
785-
uniqueCandidate = (commonType == null ? candidate : null);
786-
commonType = ClassUtils.determineCommonAncestor(candidate.getReturnType(), commonType);
787-
if (commonType == null) {
788-
// Ambiguous return types found: return null to indicate "not determinable".
789-
return null;
790-
}
791794
}
792795
}
793-
}
794796

795-
mbd.factoryMethodToIntrospect = uniqueCandidate;
796-
if (commonType == null) {
797-
return null;
797+
mbd.factoryMethodToIntrospect = uniqueCandidate;
798+
if (commonType == null) {
799+
return null;
800+
}
798801
}
802+
799803
// Common return type found: all factory methods return same type. For a non-parameterized
800804
// unique candidate, cache the full type declaration context of the target factory method.
801805
cachedReturnType = (uniqueCandidate != null ?
@@ -926,7 +930,7 @@ class Holder {
926930
objectType.value = ClassUtils.determineCommonAncestor(currentType, objectType.value);
927931
}
928932
}
929-
});
933+
}, ReflectionUtils.USER_DECLARED_METHODS);
930934

931935
return (objectType.value != null && Object.class != objectType.value ? objectType.value : null);
932936
}

‎spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
2626
import java.security.PrivilegedAction;
2727
import java.util.ArrayList;
2828
import java.util.Arrays;
29+
import java.util.Collections;
2930
import java.util.HashSet;
3031
import java.util.LinkedHashSet;
3132
import java.util.LinkedList;
@@ -436,11 +437,22 @@ public BeanWrapper instantiateUsingFactoryMethod(
436437
// Try all methods with this name to see if they match the given arguments.
437438
factoryClass = ClassUtils.getUserClass(factoryClass);
438439

439-
Method[] rawCandidates = getCandidateMethods(factoryClass, mbd);
440-
List<Method> candidateList = new ArrayList<>();
441-
for (Method candidate : rawCandidates) {
442-
if (Modifier.isStatic(candidate.getModifiers()) == isStatic && mbd.isFactoryMethod(candidate)) {
443-
candidateList.add(candidate);
440+
List<Method> candidateList = null;
441+
if (mbd.isFactoryMethodUnique) {
442+
if (factoryMethodToUse == null) {
443+
factoryMethodToUse = mbd.getResolvedFactoryMethod();
444+
}
445+
if (factoryMethodToUse != null) {
446+
candidateList = Collections.singletonList(factoryMethodToUse);
447+
}
448+
}
449+
if (candidateList == null) {
450+
candidateList = new ArrayList<>();
451+
Method[] rawCandidates = getCandidateMethods(factoryClass, mbd);
452+
for (Method candidate : rawCandidates) {
453+
if (Modifier.isStatic(candidate.getModifiers()) == isStatic && mbd.isFactoryMethod(candidate)) {
454+
candidateList.add(candidate);
455+
}
444456
}
445457
}
446458

‎spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -237,6 +237,7 @@ public RootBeanDefinition(RootBeanDefinition original) {
237237
this.allowCaching = original.allowCaching;
238238
this.isFactoryMethodUnique = original.isFactoryMethodUnique;
239239
this.targetType = original.targetType;
240+
this.factoryMethodToIntrospect = original.factoryMethodToIntrospect;
240241
}
241242

242243
/**
@@ -361,13 +362,32 @@ public void setUniqueFactoryMethodName(String name) {
361362
this.isFactoryMethodUnique = true;
362363
}
363364

365+
/**
366+
* Specify a factory method name that refers to an overloaded method.
367+
* @since 5.2
368+
*/
369+
public void setNonUniqueFactoryMethodName(String name) {
370+
Assert.hasText(name, "Factory method name must not be empty");
371+
setFactoryMethodName(name);
372+
this.isFactoryMethodUnique = false;
373+
}
374+
364375
/**
365376
* Check whether the given candidate qualifies as a factory method.
366377
*/
367378
public boolean isFactoryMethod(Method candidate) {
368379
return candidate.getName().equals(getFactoryMethodName());
369380
}
370381

382+
/**
383+
* Set a resolved Java Method for the factory method on this bean definition.
384+
* @param method the resolved factory method, or {@code null} to reset it
385+
* @since 5.2
386+
*/
387+
public void setResolvedFactoryMethod(@Nullable Method method) {
388+
this.factoryMethodToIntrospect = method;
389+
}
390+
371391
/**
372392
* Return the resolved factory method as a Java Method object, if available.
373393
* @return the factory method, or {@code null} if not found or not resolved yet

‎spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -50,6 +50,9 @@
5050
import org.springframework.core.io.ResourceLoader;
5151
import org.springframework.core.type.AnnotationMetadata;
5252
import org.springframework.core.type.MethodMetadata;
53+
import org.springframework.core.type.StandardAnnotationMetadata;
54+
import org.springframework.core.type.StandardMethodMetadata;
55+
import org.springframework.lang.NonNull;
5356
import org.springframework.util.Assert;
5457
import org.springframework.util.StringUtils;
5558

@@ -214,14 +217,24 @@ private void loadBeanDefinitionsForBeanMethod(BeanMethod beanMethod) {
214217

215218
if (metadata.isStatic()) {
216219
// static @Bean method
217-
beanDef.setBeanClassName(configClass.getMetadata().getClassName());
218-
beanDef.setFactoryMethodName(methodName);
220+
if (configClass.getMetadata() instanceof StandardAnnotationMetadata) {
221+
beanDef.setBeanClass(((StandardAnnotationMetadata) configClass.getMetadata()).getIntrospectedClass());
222+
}
223+
else {
224+
beanDef.setBeanClassName(configClass.getMetadata().getClassName());
225+
}
226+
beanDef.setUniqueFactoryMethodName(methodName);
219227
}
220228
else {
221229
// instance @Bean method
222230
beanDef.setFactoryBeanName(configClass.getBeanName());
223231
beanDef.setUniqueFactoryMethodName(methodName);
224232
}
233+
234+
if (metadata instanceof StandardMethodMetadata) {
235+
beanDef.setResolvedFactoryMethod(((StandardMethodMetadata) metadata).getIntrospectedMethod());
236+
}
237+
225238
beanDef.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_CONSTRUCTOR);
226239
beanDef.setAttribute(org.springframework.beans.factory.annotation.RequiredAnnotationBeanPostProcessor.
227240
SKIP_REQUIRED_CHECK_ATTRIBUTE, Boolean.TRUE);
@@ -286,8 +299,16 @@ protected boolean isOverriddenByExistingDefinition(BeanMethod beanMethod, String
286299
// preserve the existing bean definition.
287300
if (existingBeanDef instanceof ConfigurationClassBeanDefinition) {
288301
ConfigurationClassBeanDefinition ccbd = (ConfigurationClassBeanDefinition) existingBeanDef;
289-
return ccbd.getMetadata().getClassName().equals(
290-
beanMethod.getConfigurationClass().getMetadata().getClassName());
302+
if (ccbd.getMetadata().getClassName().equals(
303+
beanMethod.getConfigurationClass().getMetadata().getClassName())) {
304+
if (ccbd.getFactoryMethodMetadata().getMethodName().equals(ccbd.getFactoryMethodName())) {
305+
ccbd.setNonUniqueFactoryMethodName(ccbd.getFactoryMethodMetadata().getMethodName());
306+
}
307+
return true;
308+
}
309+
else {
310+
return false;
311+
}
291312
}
292313

293314
// A bean definition resulting from a component scan can be silently overridden
@@ -403,6 +424,7 @@ public AnnotationMetadata getMetadata() {
403424
}
404425

405426
@Override
427+
@NonNull
406428
public MethodMetadata getFactoryMethodMetadata() {
407429
return this.factoryMethodMetadata;
408430
}

0 commit comments

Comments
 (0)