Skip to content

Commit 4a470e0

Browse files
jhoellerjhoeller
committed
Prevent @bean method overloading by default (with enforceUniqueMethods flag)
Closes gh-22609
1 parent 41ee233 commit 4a470e0

File tree

5 files changed

+71
-29
lines changed

5 files changed

+71
-29
lines changed

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

+13-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 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.
@@ -460,4 +460,16 @@
460460
*/
461461
boolean proxyBeanMethods() default true;
462462

463+
/**
464+
* Specify whether {@code @Bean} methods need to have unique method names,
465+
* raising an exception otherwise in order to prevent accidental overloading.
466+
* <p>The default is {@code true}, preventing accidental method overloads which
467+
* get interpreted as overloaded factory methods for the same bean definition
468+
* (as opposed to separate bean definitions with individual conditions etc).
469+
* Switch this flag to {@code false} in order to allow for method overloading
470+
* according to those semantics, accepting the risk for accidental overlaps.
471+
* @since 6.0
472+
*/
473+
boolean enforceUniqueMethods() default true;
474+
463475
}

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

+31-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 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.
@@ -29,6 +29,7 @@
2929
import org.springframework.core.io.DescriptiveResource;
3030
import org.springframework.core.io.Resource;
3131
import org.springframework.core.type.AnnotationMetadata;
32+
import org.springframework.core.type.MethodMetadata;
3233
import org.springframework.core.type.classreading.MetadataReader;
3334
import org.springframework.lang.Nullable;
3435
import org.springframework.util.Assert;
@@ -210,8 +211,9 @@ Map<String, Class<? extends BeanDefinitionReader>> getImportedResources() {
210211
}
211212

212213
void validate(ProblemReporter problemReporter) {
213-
// A configuration class may not be final (CGLIB limitation) unless it declares proxyBeanMethods=false
214214
Map<String, Object> attributes = this.metadata.getAnnotationAttributes(Configuration.class.getName());
215+
216+
// A configuration class may not be final (CGLIB limitation) unless it declares proxyBeanMethods=false
215217
if (attributes != null && (Boolean) attributes.get("proxyBeanMethods")) {
216218
if (this.metadata.isFinal()) {
217219
problemReporter.error(new FinalConfigurationProblem());
@@ -220,6 +222,18 @@ void validate(ProblemReporter problemReporter) {
220222
beanMethod.validate(problemReporter);
221223
}
222224
}
225+
226+
// A configuration class may not contain overloaded bean methods unless it declares enforceUniqueMethods=false
227+
if (attributes != null && (Boolean) attributes.get("enforceUniqueMethods")) {
228+
Map<String, MethodMetadata> beanMethodsByName = new LinkedHashMap<>();
229+
for (BeanMethod beanMethod : this.beanMethods) {
230+
MethodMetadata current = beanMethod.getMetadata();
231+
MethodMetadata existing = beanMethodsByName.put(current.getMethodName(), current);
232+
if (existing != null && existing.getDeclaringClassName().equals(current.getDeclaringClassName())) {
233+
problemReporter.error(new BeanMethodOverloadingProblem(existing.getMethodName()));
234+
}
235+
}
236+
}
223237
}
224238

225239
@Override
@@ -250,4 +264,19 @@ private class FinalConfigurationProblem extends Problem {
250264
}
251265
}
252266

267+
268+
/**
269+
* Configuration classes are not allowed to contain overloaded bean methods
270+
* by default (as of 6.0).
271+
*/
272+
private class BeanMethodOverloadingProblem extends Problem {
273+
274+
BeanMethodOverloadingProblem(String methodName) {
275+
super(String.format("@Configuration class '%s' contains overloaded @Bean methods with name '%s'. Use " +
276+
"unique method names for separate bean definitions (with individual conditions etc) " +
277+
"or switch '@Configuration.enforceUniqueMethods' to 'false'.",
278+
getSimpleName(), methodName), new Location(getResource(), getMetadata()));
279+
}
280+
}
281+
253282
}

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

+4-7
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2022 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.
@@ -967,8 +967,7 @@ public ConfigurationClass asConfigClass(ConfigurationClass importedBy) {
967967

968968
public Collection<SourceClass> getMemberClasses() throws IOException {
969969
Object sourceToProcess = this.source;
970-
if (sourceToProcess instanceof Class) {
971-
Class<?> sourceClass = (Class<?>) sourceToProcess;
970+
if (sourceToProcess instanceof Class<?> sourceClass) {
972971
try {
973972
Class<?>[] declaredClasses = sourceClass.getDeclaredClasses();
974973
List<SourceClass> members = new ArrayList<>(declaredClasses.length);
@@ -1013,8 +1012,7 @@ public SourceClass getSuperClass() throws IOException {
10131012

10141013
public Set<SourceClass> getInterfaces() throws IOException {
10151014
Set<SourceClass> result = new LinkedHashSet<>();
1016-
if (this.source instanceof Class) {
1017-
Class<?> sourceClass = (Class<?>) this.source;
1015+
if (this.source instanceof Class<?> sourceClass) {
10181016
for (Class<?> ifcClass : sourceClass.getInterfaces()) {
10191017
result.add(asSourceClass(ifcClass, DEFAULT_EXCLUSION_FILTER));
10201018
}
@@ -1029,8 +1027,7 @@ public Set<SourceClass> getInterfaces() throws IOException {
10291027

10301028
public Set<SourceClass> getAnnotations() {
10311029
Set<SourceClass> result = new LinkedHashSet<>();
1032-
if (this.source instanceof Class) {
1033-
Class<?> sourceClass = (Class<?>) this.source;
1030+
if (this.source instanceof Class<?> sourceClass) {
10341031
for (Annotation ann : sourceClass.getDeclaredAnnotations()) {
10351032
Class<?> annType = ann.annotationType();
10361033
if (!annType.getName().startsWith("java")) {

‎spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java

+21-17
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2022 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.
@@ -41,7 +41,7 @@ public class BeanMethodPolymorphismTests {
4141
@Test
4242
public void beanMethodDetectedOnSuperClass() {
4343
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Config.class);
44-
assertThat(ctx.getBean("testBean", TestBean.class)).isNotNull();
44+
assertThat(ctx.getBean("testBean", BaseTestBean.class)).isNotNull();
4545
}
4646

4747
@Test
@@ -51,7 +51,7 @@ public void beanMethodOverriding() {
5151
ctx.setAllowBeanDefinitionOverriding(false);
5252
ctx.refresh();
5353
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
54-
assertThat(ctx.getBean("testBean", TestBean.class).toString()).isEqualTo("overridden");
54+
assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden");
5555
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue();
5656
}
5757

@@ -62,7 +62,7 @@ public void beanMethodOverridingOnASM() {
6262
ctx.setAllowBeanDefinitionOverriding(false);
6363
ctx.refresh();
6464
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
65-
assertThat(ctx.getBean("testBean", TestBean.class).toString()).isEqualTo("overridden");
65+
assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden");
6666
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue();
6767
}
6868

@@ -73,7 +73,7 @@ public void beanMethodOverridingWithNarrowedReturnType() {
7373
ctx.setAllowBeanDefinitionOverriding(false);
7474
ctx.refresh();
7575
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
76-
assertThat(ctx.getBean("testBean", TestBean.class).toString()).isEqualTo("overridden");
76+
assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden");
7777
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue();
7878
}
7979

@@ -84,7 +84,7 @@ public void beanMethodOverridingWithNarrowedReturnTypeOnASM() {
8484
ctx.setAllowBeanDefinitionOverriding(false);
8585
ctx.refresh();
8686
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
87-
assertThat(ctx.getBean("testBean", TestBean.class).toString()).isEqualTo("overridden");
87+
assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden");
8888
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue();
8989
}
9090

@@ -171,16 +171,24 @@ public void beanMethodThroughAopProxy() {
171171
ctx.register(AnnotationAwareAspectJAutoProxyCreator.class);
172172
ctx.register(TestAdvisor.class);
173173
ctx.refresh();
174-
ctx.getBean("testBean", TestBean.class);
174+
ctx.getBean("testBean", BaseTestBean.class);
175+
}
176+
177+
178+
static class BaseTestBean {
179+
}
180+
181+
182+
static class ExtendedTestBean extends BaseTestBean {
175183
}
176184

177185

178186
@Configuration
179187
static class BaseConfig {
180188

181189
@Bean
182-
public TestBean testBean() {
183-
return new TestBean();
190+
public BaseTestBean testBean() {
191+
return new BaseTestBean();
184192
}
185193
}
186194

@@ -195,8 +203,8 @@ static class OverridingConfig extends BaseConfig {
195203

196204
@Bean @Lazy
197205
@Override
198-
public TestBean testBean() {
199-
return new TestBean() {
206+
public BaseTestBean testBean() {
207+
return new BaseTestBean() {
200208
@Override
201209
public String toString() {
202210
return "overridden";
@@ -206,10 +214,6 @@ public String toString() {
206214
}
207215

208216

209-
static class ExtendedTestBean extends TestBean {
210-
}
211-
212-
213217
@Configuration
214218
static class NarrowedOverridingConfig extends BaseConfig {
215219

@@ -226,7 +230,7 @@ public String toString() {
226230
}
227231

228232

229-
@Configuration
233+
@Configuration(enforceUniqueMethods = false)
230234
static class ConfigWithOverloading {
231235

232236
@Bean
@@ -241,7 +245,7 @@ String aString(Integer dependency) {
241245
}
242246

243247

244-
@Configuration
248+
@Configuration(enforceUniqueMethods = false)
245249
static class ConfigWithOverloadingAndAdditionalMetadata {
246250

247251
@Bean @Lazy

‎spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2022 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.
@@ -626,7 +626,7 @@ public ApplicationListener<ContextClosedEvent> listener() {
626626
}
627627

628628

629-
@Configuration
629+
@Configuration(enforceUniqueMethods = false)
630630
public static class OverloadedBeanMismatch {
631631

632632
@Bean(name = "other")

0 commit comments

Comments
 (0)