Skip to content
This repository was archived by the owner on Feb 23, 2023. It is now read-only.

Commit ff70963

Browse files
snicollsnicoll
committed
Fix detection of constructor with multiple matches
This commit uses the resolved parameter types, including detection of inner bean definition, rather than query ConstructorArgumentValues if it can process a given type. The latter will transparently convert the value if necessary, leading to detecting a constructor that will not ultimately match. Closes gh-1035
1 parent 6d83951 commit ff70963

File tree

3 files changed

+124
-36
lines changed

3 files changed

+124
-36
lines changed

‎spring-aot/src/main/java/org/springframework/context/bootstrap/generator/bean/descriptor/BeanInstanceExecutableSupplier.java

+32-29
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.lang.reflect.Field;
2222
import java.lang.reflect.Method;
2323
import java.util.ArrayList;
24-
import java.util.Arrays;
2524
import java.util.List;
2625
import java.util.function.Supplier;
2726
import java.util.stream.Collectors;
@@ -61,9 +60,10 @@ class BeanInstanceExecutableSupplier {
6160
}
6261

6362
Executable detectBeanInstanceExecutable(BeanDefinition beanDefinition) {
64-
Supplier<Class<?>> beanClass = () -> getBeanClass(beanDefinition);
65-
List<Class<?>> parameterTypes = determineParameterTypes(beanDefinition);
66-
Method resolvedFactoryMethod = resolveFactoryMethod(beanDefinition, beanClass, parameterTypes);
63+
Supplier<ResolvableType> beanType = () -> getBeanType(beanDefinition);
64+
ConstructorArgumentValues constructorArgumentValues = beanDefinition.getConstructorArgumentValues();
65+
List<Class<?>> parameterTypes = determineParameterTypes(constructorArgumentValues);
66+
Method resolvedFactoryMethod = resolveFactoryMethod(beanDefinition, beanType, parameterTypes);
6767
if (resolvedFactoryMethod != null) {
6868
return resolvedFactoryMethod;
6969
}
@@ -73,14 +73,14 @@ Executable detectBeanInstanceExecutable(BeanDefinition beanDefinition) {
7373
boolean isCompatible = ResolvableType.forClass(factoryBeanClass).as(FactoryBean.class)
7474
.getGeneric(0).isAssignableFrom(resolvableType);
7575
if (isCompatible) {
76-
return resolveConstructor(() -> factoryBeanClass, beanDefinition.getConstructorArgumentValues());
76+
return resolveConstructor(() -> ResolvableType.forClass(factoryBeanClass), parameterTypes);
7777
}
7878
else {
7979
throw new IllegalStateException(String.format("Incompatible target type '%s' for factory bean '%s'",
8080
resolvableType.toClass().getName(), factoryBeanClass.getName()));
8181
}
8282
}
83-
Executable resolvedConstructor = resolveConstructor(beanClass, beanDefinition.getConstructorArgumentValues());
83+
Executable resolvedConstructor = resolveConstructor(beanType, parameterTypes);
8484
if (resolvedConstructor != null) {
8585
return resolvedConstructor;
8686
}
@@ -93,9 +93,8 @@ Executable detectBeanInstanceExecutable(BeanDefinition beanDefinition) {
9393
return null;
9494
}
9595

96-
private List<Class<?>> determineParameterTypes(BeanDefinition beanDefinition) {
96+
private List<Class<?>> determineParameterTypes(ConstructorArgumentValues constructorArgumentValues) {
9797
List<Class<?>> parameterTypes = new ArrayList<>();
98-
ConstructorArgumentValues constructorArgumentValues = beanDefinition.getConstructorArgumentValues();
9998
if (constructorArgumentValues.isEmpty()) {
10099
return parameterTypes;
101100
}
@@ -108,6 +107,9 @@ private List<Class<?>> determineParameterTypes(BeanDefinition beanDefinition) {
108107
if (value instanceof BeanReference) {
109108
parameterTypes.add(this.beanFactory.getType(((BeanReference) value).getBeanName(), false));
110109
}
110+
else if (value instanceof BeanDefinition) {
111+
parameterTypes.add(extractTypeFromBeanDefinition(getBeanType((BeanDefinition) value)));
112+
}
111113
else {
112114
parameterTypes.add(value.getClass());
113115
}
@@ -116,7 +118,15 @@ private List<Class<?>> determineParameterTypes(BeanDefinition beanDefinition) {
116118
return parameterTypes;
117119
}
118120

119-
private Method resolveFactoryMethod(BeanDefinition beanDefinition, Supplier<Class<?>> beanClass,
121+
private Class<?> extractTypeFromBeanDefinition(ResolvableType type) {
122+
Class<?> beanClass = type.toClass();
123+
if (FactoryBean.class.isAssignableFrom(beanClass)) {
124+
return type.as(FactoryBean.class).getGeneric(0).toClass();
125+
}
126+
return beanClass;
127+
}
128+
129+
private Method resolveFactoryMethod(BeanDefinition beanDefinition, Supplier<ResolvableType> beanType,
120130
List<Class<?>> parameterTypes) {
121131
if (beanDefinition instanceof RootBeanDefinition) {
122132
RootBeanDefinition rootBeanDefinition = (RootBeanDefinition) beanDefinition;
@@ -128,7 +138,7 @@ private Method resolveFactoryMethod(BeanDefinition beanDefinition, Supplier<Clas
128138
String factoryMethodName = beanDefinition.getFactoryMethodName();
129139
if (factoryMethodName != null) {
130140
List<Method> methods = new ArrayList<>();
131-
ReflectionUtils.doWithMethods(beanClass.get(), methods::add,
141+
ReflectionUtils.doWithMethods(beanType.get().toClass(), methods::add,
132142
(method) -> method.getName().equals(factoryMethodName));
133143
if (methods.size() >= 1) {
134144
return (Method) filter(methods, parameterTypes);
@@ -137,8 +147,8 @@ private Method resolveFactoryMethod(BeanDefinition beanDefinition, Supplier<Clas
137147
return null;
138148
}
139149

140-
private Executable resolveConstructor(Supplier<Class<?>> beanClass, ConstructorArgumentValues argumentValues) {
141-
Class<?> type = beanClass.get();
150+
private Executable resolveConstructor(Supplier<ResolvableType> beanType, List<Class<?>> parameterTypes) {
151+
Class<?> type = beanType.get().toClass();
142152
Constructor<?>[] constructors = type.getDeclaredConstructors();
143153
if (constructors.length == 1) {
144154
return constructors[0];
@@ -147,26 +157,13 @@ private Executable resolveConstructor(Supplier<Class<?>> beanClass, ConstructorA
147157
if (MergedAnnotations.from(constructor).isPresent(Autowired.class)) {
148158
return constructor;
149159
}
150-
if (constructorMatchesArguments(constructor, argumentValues)) {
160+
if (match(constructor, parameterTypes)) {
151161
return constructor;
152162
}
153163
}
154164
return null;
155165
}
156166

157-
private boolean constructorMatchesArguments(Constructor<?> constructor, ConstructorArgumentValues argumentValues) {
158-
if (constructor.getParameterTypes().length != argumentValues.getArgumentCount()) {
159-
return false;
160-
}
161-
return Arrays.stream(constructor.getParameterTypes()).allMatch(paramType -> {
162-
for (int index = 0; index < argumentValues.getArgumentCount(); index++) {
163-
if (argumentValues.getArgumentValue(index, paramType) != null)
164-
return true;
165-
}
166-
return false;
167-
});
168-
}
169-
170167
private Executable filter(List<? extends Executable> executables, List<Class<?>> parameterTypes) {
171168
List<? extends Executable> matches = executables.stream()
172169
.filter((executable) -> match(executable, parameterTypes)).collect(Collectors.toList());
@@ -200,14 +197,20 @@ private Class<?> getFactoryBeanClass(BeanDefinition beanDefinition) {
200197
return null;
201198
}
202199

203-
private Class<?> getBeanClass(BeanDefinition beanDefinition) {
200+
private ResolvableType getBeanType(BeanDefinition beanDefinition) {
204201
ResolvableType resolvableType = beanDefinition.getResolvableType();
205202
if (resolvableType != ResolvableType.NONE) {
206-
return resolvableType.toClass();
203+
return resolvableType;
204+
}
205+
if (beanDefinition instanceof RootBeanDefinition) {
206+
RootBeanDefinition rbd = (RootBeanDefinition) beanDefinition;
207+
if (rbd.hasBeanClass()) {
208+
return ResolvableType.forClass(rbd.getBeanClass());
209+
}
207210
}
208211
String beanClassName = beanDefinition.getBeanClassName();
209212
if (beanClassName != null) {
210-
return loadClass(beanClassName);
213+
return ResolvableType.forClass(loadClass(beanClassName));
211214
}
212215
throw new IllegalStateException("Failed to determine bean class of " + beanDefinition);
213216
}

‎spring-aot/src/test/java/org/springframework/context/bootstrap/generator/bean/descriptor/BeanInstanceExecutableSupplierTests.java

+89-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,27 @@
1+
/*
2+
* Copyright 2019-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
117
package org.springframework.context.bootstrap.generator.bean.descriptor;
218

319
import java.lang.reflect.Executable;
20+
import java.util.Locale;
421

522
import org.junit.jupiter.api.Test;
623

24+
import org.springframework.beans.factory.FactoryBean;
725
import org.springframework.beans.factory.config.BeanDefinition;
826
import org.springframework.beans.factory.support.BeanDefinitionBuilder;
927
import org.springframework.beans.factory.support.DefaultListableBeanFactory;
@@ -56,7 +74,7 @@ void beanDefinitionWithConstructorArgsForMultipleConstructors() throws Exception
5674
.addConstructorArgReference("testNumber")
5775
.addConstructorArgReference("testBean").getBeanDefinition();
5876
Executable executable = detectBeanInstanceExecutable(beanFactory, beanDefinition);
59-
assertThat(executable).isNotNull().isEqualTo(SampleBeanWithConstructors.class.getDeclaredConstructor(String.class, Number.class));
77+
assertThat(executable).isNotNull().isEqualTo(SampleBeanWithConstructors.class.getDeclaredConstructor(Number.class, String.class));
6078
}
6179

6280
@Test
@@ -68,7 +86,51 @@ void genericBeanDefinitionWithConstructorArgsForMultipleConstructors() throws Ex
6886
.addConstructorArgReference("testNumber")
6987
.addConstructorArgReference("testBean").getBeanDefinition();
7088
Executable executable = detectBeanInstanceExecutable(beanFactory, beanDefinition);
71-
assertThat(executable).isNotNull().isEqualTo(SampleBeanWithConstructors.class.getDeclaredConstructor(String.class, Number.class));
89+
assertThat(executable).isNotNull().isEqualTo(SampleBeanWithConstructors.class.getDeclaredConstructor(Number.class, String.class));
90+
}
91+
92+
@Test
93+
void beanDefinitionWithMultiArgConstructorAndMatchingValue() throws NoSuchMethodException {
94+
BeanDefinition beanDefinition = BeanDefinitionBuilder.rootBeanDefinition(MultiConstructorSample.class)
95+
.addConstructorArgValue(42).getBeanDefinition();
96+
Executable executable = detectBeanInstanceExecutable(new DefaultListableBeanFactory(), beanDefinition);
97+
assertThat(executable).isNotNull().isEqualTo(MultiConstructorSample.class.getDeclaredConstructor(Integer.class));
98+
}
99+
100+
@Test
101+
void beanDefinitionWithMultiArgConstructorAndMatchingValueAsInnerBean() throws NoSuchMethodException {
102+
BeanDefinition beanDefinition = BeanDefinitionBuilder.rootBeanDefinition(MultiConstructorSample.class)
103+
.addConstructorArgValue(BeanDefinitionBuilder.rootBeanDefinition(Integer.class, "valueOf")
104+
.addConstructorArgValue("42").getBeanDefinition())
105+
.getBeanDefinition();
106+
Executable executable = detectBeanInstanceExecutable(new DefaultListableBeanFactory(), beanDefinition);
107+
assertThat(executable).isNotNull().isEqualTo(MultiConstructorSample.class.getDeclaredConstructor(Integer.class));
108+
}
109+
110+
@Test
111+
void beanDefinitionWithMultiArgConstructorAndMatchingValueAsInnerBeanFactory() throws NoSuchMethodException {
112+
BeanDefinition beanDefinition = BeanDefinitionBuilder.rootBeanDefinition(MultiConstructorSample.class)
113+
.addConstructorArgValue(BeanDefinitionBuilder.rootBeanDefinition(IntegerFactoryBean.class).getBeanDefinition())
114+
.getBeanDefinition();
115+
Executable executable = detectBeanInstanceExecutable(new DefaultListableBeanFactory(), beanDefinition);
116+
assertThat(executable).isNotNull().isEqualTo(MultiConstructorSample.class.getDeclaredConstructor(Integer.class));
117+
}
118+
119+
@Test
120+
void beanDefinitionWithMultiArgConstructorAndNonMatchingValue() {
121+
BeanDefinition beanDefinition = BeanDefinitionBuilder.rootBeanDefinition(MultiConstructorSample.class)
122+
.addConstructorArgValue(Locale.ENGLISH).getBeanDefinition();
123+
Executable executable = detectBeanInstanceExecutable(new DefaultListableBeanFactory(), beanDefinition);
124+
assertThat(executable).isNull();
125+
}
126+
127+
@Test
128+
void beanDefinitionWithMultiArgConstructorAndNonMatchingValueAsInnerBean() {
129+
BeanDefinition beanDefinition = BeanDefinitionBuilder.rootBeanDefinition(MultiConstructorSample.class)
130+
.addConstructorArgValue(BeanDefinitionBuilder.rootBeanDefinition(Locale.class, "getDefault").getBeanDefinition())
131+
.getBeanDefinition();
132+
Executable executable = detectBeanInstanceExecutable(new DefaultListableBeanFactory(), beanDefinition);
133+
assertThat(executable).isNull();
72134
}
73135

74136
@Test
@@ -105,4 +167,29 @@ private Executable detectBeanInstanceExecutable(DefaultListableBeanFactory beanF
105167
return new BeanInstanceExecutableSupplier(beanFactory).detectBeanInstanceExecutable(beanDefinition);
106168
}
107169

170+
static class IntegerFactoryBean implements FactoryBean<Integer> {
171+
172+
@Override
173+
public Integer getObject() {
174+
return 42;
175+
}
176+
177+
@Override
178+
public Class<?> getObjectType() {
179+
return Integer.class;
180+
}
181+
}
182+
183+
static class MultiConstructorSample {
184+
185+
MultiConstructorSample(String name) {
186+
187+
}
188+
189+
MultiConstructorSample(Integer value) {
190+
191+
}
192+
193+
}
194+
108195
}

‎spring-aot/src/test/java/org/springframework/context/bootstrap/generator/sample/constructor/SampleBeanWithConstructors.java

+3-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2019-2021 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.
@@ -16,18 +16,16 @@
1616

1717
package org.springframework.context.bootstrap.generator.sample.constructor;
1818

19+
@SuppressWarnings("unused")
1920
public class SampleBeanWithConstructors {
2021

2122
public SampleBeanWithConstructors() {
22-
2323
}
2424

2525
public SampleBeanWithConstructors(String name) {
26-
2726
}
2827

29-
public SampleBeanWithConstructors(String name, Number number) {
30-
28+
public SampleBeanWithConstructors(Number number, String name) {
3129
}
3230

3331
}

0 commit comments

Comments
 (0)