Skip to content

Commit fef3cf8

Browse files
snicollsnicoll
committed
Review AOT-generated code for beanClass and targetType
This commit reviews when an AOT-generated bean definition defines a beanClass or targetType. Previously, a beanClass was not consistently set which could lead to issues. Closes gh-31242
1 parent ce0923b commit fef3cf8

File tree

2 files changed

+108
-21
lines changed

2 files changed

+108
-21
lines changed

‎spring-beans/src/main/java/org/springframework/beans/factory/aot/DefaultBeanRegistrationCodeFragments.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ public CodeBlock generateNewBeanDefinitionCode(GenerationContext generationConte
117117
Class<?> beanClass = (mergedBeanDefinition.hasBeanClass()
118118
? ClassUtils.getUserClass(mergedBeanDefinition.getBeanClass()) : null);
119119
CodeBlock beanClassCode = generateBeanClassCode(
120-
beanRegistrationCode.getClassName().packageName(), beanClass);
120+
beanRegistrationCode.getClassName().packageName(),
121+
(beanClass != null ? beanClass : beanType.toClass()));
121122
code.addStatement("$T $L = new $T($L)", RootBeanDefinition.class,
122123
BEAN_DEFINITION_VARIABLE, RootBeanDefinition.class, beanClassCode);
123124
if (targetTypeNecessary(beanType, beanClass)) {
@@ -127,16 +128,13 @@ public CodeBlock generateNewBeanDefinitionCode(GenerationContext generationConte
127128
return code.build();
128129
}
129130

130-
private CodeBlock generateBeanClassCode(String targetPackage, @Nullable Class<?> beanClass) {
131-
if (beanClass != null) {
132-
if (Modifier.isPublic(beanClass.getModifiers()) || targetPackage.equals(beanClass.getPackageName())) {
133-
return CodeBlock.of("$T.class", beanClass);
134-
}
135-
else {
136-
return CodeBlock.of("$S", beanClass.getName());
137-
}
131+
private CodeBlock generateBeanClassCode(String targetPackage, Class<?> beanClass) {
132+
if (Modifier.isPublic(beanClass.getModifiers()) || targetPackage.equals(beanClass.getPackageName())) {
133+
return CodeBlock.of("$T.class", beanClass);
134+
}
135+
else {
136+
return CodeBlock.of("$S", beanClass.getName());
138137
}
139-
return CodeBlock.of("");
140138
}
141139

142140
private CodeBlock generateBeanTypeCode(ResolvableType beanType) {
@@ -147,11 +145,14 @@ private CodeBlock generateBeanTypeCode(ResolvableType beanType) {
147145
}
148146

149147
private boolean targetTypeNecessary(ResolvableType beanType, @Nullable Class<?> beanClass) {
150-
if (beanType.hasGenerics() || beanClass == null) {
148+
if (beanType.hasGenerics()) {
149+
return true;
150+
}
151+
if (beanClass != null
152+
&& this.registeredBean.getMergedBeanDefinition().getFactoryMethodName() != null) {
151153
return true;
152154
}
153-
return (!beanType.toClass().equals(beanClass)
154-
|| this.registeredBean.getMergedBeanDefinition().getFactoryMethodName() != null);
155+
return (beanClass != null && !beanType.toClass().equals(beanClass));
155156
}
156157

157158
@Override

‎spring-beans/src/test/java/org/springframework/beans/factory/aot/BeanDefinitionMethodGeneratorTests.java

Lines changed: 94 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.springframework.beans.testfixture.beans.factory.aot.InnerBeanConfiguration;
4848
import org.springframework.beans.testfixture.beans.factory.aot.MockBeanRegistrationsCode;
4949
import org.springframework.beans.testfixture.beans.factory.aot.SimpleBean;
50+
import org.springframework.beans.testfixture.beans.factory.aot.SimpleBeanConfiguration;
5051
import org.springframework.beans.testfixture.beans.factory.aot.TestHierarchy;
5152
import org.springframework.beans.testfixture.beans.factory.aot.TestHierarchy.Implementation;
5253
import org.springframework.beans.testfixture.beans.factory.aot.TestHierarchy.One;
@@ -92,9 +93,27 @@ class BeanDefinitionMethodGeneratorTests {
9293
this.beanRegistrationsCode = new MockBeanRegistrationsCode(this.generationContext);
9394
}
9495

96+
@Test
97+
void generateWithBeanClassSetsOnlyBeanClass() {
98+
RootBeanDefinition beanDefinition = new RootBeanDefinition(TestBean.class);
99+
RegisteredBean registeredBean = registerBean(beanDefinition);
100+
BeanDefinitionMethodGenerator generator = new BeanDefinitionMethodGenerator(
101+
this.methodGeneratorFactory, registeredBean, null,
102+
Collections.emptyList());
103+
MethodReference method = generator.generateBeanDefinitionMethod(
104+
this.generationContext, this.beanRegistrationsCode);
105+
compile(method, (actual, compiled) -> {
106+
SourceFile sourceFile = compiled.getSourceFile(".*BeanDefinitions");
107+
assertThat(sourceFile).contains("Get the bean definition for 'testBean'");
108+
assertThat(sourceFile).contains("new RootBeanDefinition(TestBean.class)");
109+
assertThat(sourceFile).doesNotContain("setTargetType(");
110+
assertThat(sourceFile).contains("setInstanceSupplier(TestBean::new)");
111+
assertThat(actual).isInstanceOf(RootBeanDefinition.class);
112+
});
113+
}
95114

96115
@Test
97-
void generateBeanDefinitionMethodWithOnlyTargetTypeDoesNotSetBeanClass() {
116+
void generateWithTargetTypeWithNoGenericSetsOnlyBeanClass() {
98117
RootBeanDefinition beanDefinition = new RootBeanDefinition();
99118
beanDefinition.setTargetType(TestBean.class);
100119
RegisteredBean registeredBean = registerBean(beanDefinition);
@@ -106,34 +125,79 @@ void generateBeanDefinitionMethodWithOnlyTargetTypeDoesNotSetBeanClass() {
106125
compile(method, (actual, compiled) -> {
107126
SourceFile sourceFile = compiled.getSourceFile(".*BeanDefinitions");
108127
assertThat(sourceFile).contains("Get the bean definition for 'testBean'");
109-
assertThat(sourceFile).contains("new RootBeanDefinition()");
110-
assertThat(sourceFile).contains("setTargetType(TestBean.class)");
128+
assertThat(sourceFile).contains("new RootBeanDefinition(TestBean.class)");
111129
assertThat(sourceFile).contains("setInstanceSupplier(TestBean::new)");
112130
assertThat(actual).isInstanceOf(RootBeanDefinition.class);
113131
});
114132
}
115133

116134
@Test
117-
void generateBeanDefinitionMethodSpecifiesBeanClassIfSet() {
118-
RootBeanDefinition beanDefinition = new RootBeanDefinition(TestBean.class);
135+
void generateWithTargetTypeUsingGenericsSetsBothBeanClassAndTargetType() {
136+
RootBeanDefinition beanDefinition = new RootBeanDefinition();
137+
beanDefinition.setTargetType(ResolvableType.forClassWithGenerics(GenericBean.class, Integer.class));
119138
RegisteredBean registeredBean = registerBean(beanDefinition);
120139
BeanDefinitionMethodGenerator generator = new BeanDefinitionMethodGenerator(
121140
this.methodGeneratorFactory, registeredBean, null,
122141
Collections.emptyList());
123142
MethodReference method = generator.generateBeanDefinitionMethod(
124143
this.generationContext, this.beanRegistrationsCode);
125144
compile(method, (actual, compiled) -> {
145+
assertThat(actual.getResolvableType().resolve()).isEqualTo(GenericBean.class);
126146
SourceFile sourceFile = compiled.getSourceFile(".*BeanDefinitions");
127147
assertThat(sourceFile).contains("Get the bean definition for 'testBean'");
128-
assertThat(sourceFile).contains("new RootBeanDefinition(TestBean.class)");
148+
assertThat(sourceFile).contains("new RootBeanDefinition(GenericBean.class)");
149+
assertThat(sourceFile).contains(
150+
"setTargetType(ResolvableType.forClassWithGenerics(GenericBean.class, Integer.class))");
151+
assertThat(sourceFile).contains("setInstanceSupplier(GenericBean::new)");
152+
assertThat(actual).isInstanceOf(RootBeanDefinition.class);
153+
});
154+
}
155+
156+
@Test
157+
void generateWithBeanClassAndFactoryMethodNameSetsTargetTypeAndBeanClass() {
158+
this.beanFactory.registerSingleton("factory", new SimpleBeanConfiguration());
159+
RootBeanDefinition beanDefinition = new RootBeanDefinition(SimpleBean.class);
160+
beanDefinition.setFactoryBeanName("factory");
161+
beanDefinition.setFactoryMethodName("simpleBean");
162+
RegisteredBean registeredBean = registerBean(beanDefinition);
163+
BeanDefinitionMethodGenerator generator = new BeanDefinitionMethodGenerator(
164+
this.methodGeneratorFactory, registeredBean, null,
165+
Collections.emptyList());
166+
MethodReference method = generator.generateBeanDefinitionMethod(
167+
this.generationContext, this.beanRegistrationsCode);
168+
compile(method, (actual, compiled) -> {
169+
SourceFile sourceFile = compiled.getSourceFile(".*BeanDefinitions");
170+
assertThat(sourceFile).contains("Get the bean definition for 'testBean'");
171+
assertThat(sourceFile).contains("new RootBeanDefinition(SimpleBean.class)");
172+
assertThat(sourceFile).contains("setTargetType(SimpleBean.class)");
173+
assertThat(actual).isInstanceOf(RootBeanDefinition.class);
174+
});
175+
}
176+
177+
@Test
178+
void generateWithTargetTypeAndFactoryMethodNameSetsOnlyBeanClass() {
179+
this.beanFactory.registerSingleton("factory", new SimpleBeanConfiguration());
180+
RootBeanDefinition beanDefinition = new RootBeanDefinition();
181+
beanDefinition.setTargetType(SimpleBean.class);
182+
beanDefinition.setFactoryBeanName("factory");
183+
beanDefinition.setFactoryMethodName("simpleBean");
184+
RegisteredBean registeredBean = registerBean(beanDefinition);
185+
BeanDefinitionMethodGenerator generator = new BeanDefinitionMethodGenerator(
186+
this.methodGeneratorFactory, registeredBean, null,
187+
Collections.emptyList());
188+
MethodReference method = generator.generateBeanDefinitionMethod(
189+
this.generationContext, this.beanRegistrationsCode);
190+
compile(method, (actual, compiled) -> {
191+
SourceFile sourceFile = compiled.getSourceFile(".*BeanDefinitions");
192+
assertThat(sourceFile).contains("Get the bean definition for 'testBean'");
193+
assertThat(sourceFile).contains("new RootBeanDefinition(SimpleBean.class)");
129194
assertThat(sourceFile).doesNotContain("setTargetType(");
130-
assertThat(sourceFile).contains("setInstanceSupplier(TestBean::new)");
131195
assertThat(actual).isInstanceOf(RootBeanDefinition.class);
132196
});
133197
}
134198

135199
@Test
136-
void generateBeanDefinitionMethodSpecifiesBeanClassAndTargetTypIfDifferent() {
200+
void generateWithBeanClassAndTargetTypeDifferentSetsBoth() {
137201
RootBeanDefinition beanDefinition = new RootBeanDefinition(One.class);
138202
beanDefinition.setTargetType(Implementation.class);
139203
beanDefinition.setResolvedFactoryMethod(ReflectionUtils.findMethod(TestHierarchy.class, "oneBean"));
@@ -152,6 +216,28 @@ void generateBeanDefinitionMethodSpecifiesBeanClassAndTargetTypIfDifferent() {
152216
});
153217
}
154218

219+
@Test
220+
void generateWithBeanClassAndTargetTypWithGenericSetsBoth() {
221+
RootBeanDefinition beanDefinition = new RootBeanDefinition(Integer.class);
222+
beanDefinition.setTargetType(ResolvableType.forClassWithGenerics(GenericBean.class, Integer.class));
223+
RegisteredBean registeredBean = registerBean(beanDefinition);
224+
BeanDefinitionMethodGenerator generator = new BeanDefinitionMethodGenerator(
225+
this.methodGeneratorFactory, registeredBean, null,
226+
Collections.emptyList());
227+
MethodReference method = generator.generateBeanDefinitionMethod(
228+
this.generationContext, this.beanRegistrationsCode);
229+
compile(method, (actual, compiled) -> {
230+
assertThat(actual.getResolvableType().resolve()).isEqualTo(GenericBean.class);
231+
SourceFile sourceFile = compiled.getSourceFile(".*BeanDefinitions");
232+
assertThat(sourceFile).contains("Get the bean definition for 'testBean'");
233+
assertThat(sourceFile).contains("new RootBeanDefinition(Integer.class)");
234+
assertThat(sourceFile).contains(
235+
"setTargetType(ResolvableType.forClassWithGenerics(GenericBean.class, Integer.class))");
236+
assertThat(sourceFile).contains("setInstanceSupplier(GenericBean::new)");
237+
assertThat(actual).isInstanceOf(RootBeanDefinition.class);
238+
});
239+
}
240+
155241
@Test
156242
void generateBeanDefinitionMethodUSeBeanClassNameIfNotReachable() {
157243
RootBeanDefinition beanDefinition = new RootBeanDefinition(PackagePrivateTestBean.class);

0 commit comments

Comments
 (0)