diff --git a/spring-context/src/main/java/org/springframework/context/annotation/Configuration.java b/spring-context/src/main/java/org/springframework/context/annotation/Configuration.java index c228da0ddb1..54b089b53dc 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/Configuration.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/Configuration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -460,4 +460,16 @@ public @interface Configuration { */ boolean proxyBeanMethods() default true; + /** + * Specify whether {@code @Bean} methods need to have unique method names, + * raising an exception otherwise in order to prevent accidental overloading. + *

The default is {@code true}, preventing accidental method overloads which + * get interpreted as overloaded factory methods for the same bean definition + * (as opposed to separate bean definitions with individual conditions etc). + * Switch this flag to {@code false} in order to allow for method overloading + * according to those semantics, accepting the risk for accidental overlaps. + * @since 6.0 + */ + boolean enforceUniqueMethods() default true; + } diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java index b6155f6d7bf..58293278b46 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -29,6 +29,7 @@ import org.springframework.beans.factory.support.BeanDefinitionReader; import org.springframework.core.io.DescriptiveResource; import org.springframework.core.io.Resource; import org.springframework.core.type.AnnotationMetadata; +import org.springframework.core.type.MethodMetadata; import org.springframework.core.type.classreading.MetadataReader; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -210,8 +211,9 @@ final class ConfigurationClass { } void validate(ProblemReporter problemReporter) { - // A configuration class may not be final (CGLIB limitation) unless it declares proxyBeanMethods=false Map attributes = this.metadata.getAnnotationAttributes(Configuration.class.getName()); + + // A configuration class may not be final (CGLIB limitation) unless it declares proxyBeanMethods=false if (attributes != null && (Boolean) attributes.get("proxyBeanMethods")) { if (this.metadata.isFinal()) { problemReporter.error(new FinalConfigurationProblem()); @@ -220,6 +222,18 @@ final class ConfigurationClass { beanMethod.validate(problemReporter); } } + + // A configuration class may not contain overloaded bean methods unless it declares enforceUniqueMethods=false + if (attributes != null && (Boolean) attributes.get("enforceUniqueMethods")) { + Map beanMethodsByName = new LinkedHashMap<>(); + for (BeanMethod beanMethod : this.beanMethods) { + MethodMetadata current = beanMethod.getMetadata(); + MethodMetadata existing = beanMethodsByName.put(current.getMethodName(), current); + if (existing != null && existing.getDeclaringClassName().equals(current.getDeclaringClassName())) { + problemReporter.error(new BeanMethodOverloadingProblem(existing.getMethodName())); + } + } + } } @Override @@ -250,4 +264,19 @@ final class ConfigurationClass { } } + + /** + * Configuration classes are not allowed to contain overloaded bean methods + * by default (as of 6.0). + */ + private class BeanMethodOverloadingProblem extends Problem { + + BeanMethodOverloadingProblem(String methodName) { + super(String.format("@Configuration class '%s' contains overloaded @Bean methods with name '%s'. Use " + + "unique method names for separate bean definitions (with individual conditions etc) " + + "or switch '@Configuration.enforceUniqueMethods' to 'false'.", + getSimpleName(), methodName), new Location(getResource(), getMetadata())); + } + } + } diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 423078625f9..204b54ec23a 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -967,8 +967,7 @@ class ConfigurationClassParser { public Collection getMemberClasses() throws IOException { Object sourceToProcess = this.source; - if (sourceToProcess instanceof Class) { - Class sourceClass = (Class) sourceToProcess; + if (sourceToProcess instanceof Class sourceClass) { try { Class[] declaredClasses = sourceClass.getDeclaredClasses(); List members = new ArrayList<>(declaredClasses.length); @@ -1013,8 +1012,7 @@ class ConfigurationClassParser { public Set getInterfaces() throws IOException { Set result = new LinkedHashSet<>(); - if (this.source instanceof Class) { - Class sourceClass = (Class) this.source; + if (this.source instanceof Class sourceClass) { for (Class ifcClass : sourceClass.getInterfaces()) { result.add(asSourceClass(ifcClass, DEFAULT_EXCLUSION_FILTER)); } @@ -1029,8 +1027,7 @@ class ConfigurationClassParser { public Set getAnnotations() { Set result = new LinkedHashSet<>(); - if (this.source instanceof Class) { - Class sourceClass = (Class) this.source; + if (this.source instanceof Class sourceClass) { for (Annotation ann : sourceClass.getDeclaredAnnotations()) { Class annType = ann.annotationType(); if (!annType.getName().startsWith("java")) { diff --git a/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java b/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java index 170a02cc961..4e62e4212c5 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -41,7 +41,7 @@ public class BeanMethodPolymorphismTests { @Test public void beanMethodDetectedOnSuperClass() { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Config.class); - assertThat(ctx.getBean("testBean", TestBean.class)).isNotNull(); + assertThat(ctx.getBean("testBean", BaseTestBean.class)).isNotNull(); } @Test @@ -51,7 +51,7 @@ public class BeanMethodPolymorphismTests { ctx.setAllowBeanDefinitionOverriding(false); ctx.refresh(); assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse(); - assertThat(ctx.getBean("testBean", TestBean.class).toString()).isEqualTo("overridden"); + assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden"); assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue(); } @@ -62,7 +62,7 @@ public class BeanMethodPolymorphismTests { ctx.setAllowBeanDefinitionOverriding(false); ctx.refresh(); assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse(); - assertThat(ctx.getBean("testBean", TestBean.class).toString()).isEqualTo("overridden"); + assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden"); assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue(); } @@ -73,7 +73,7 @@ public class BeanMethodPolymorphismTests { ctx.setAllowBeanDefinitionOverriding(false); ctx.refresh(); assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse(); - assertThat(ctx.getBean("testBean", TestBean.class).toString()).isEqualTo("overridden"); + assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden"); assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue(); } @@ -84,7 +84,7 @@ public class BeanMethodPolymorphismTests { ctx.setAllowBeanDefinitionOverriding(false); ctx.refresh(); assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse(); - assertThat(ctx.getBean("testBean", TestBean.class).toString()).isEqualTo("overridden"); + assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden"); assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue(); } @@ -171,7 +171,15 @@ public class BeanMethodPolymorphismTests { ctx.register(AnnotationAwareAspectJAutoProxyCreator.class); ctx.register(TestAdvisor.class); ctx.refresh(); - ctx.getBean("testBean", TestBean.class); + ctx.getBean("testBean", BaseTestBean.class); + } + + + static class BaseTestBean { + } + + + static class ExtendedTestBean extends BaseTestBean { } @@ -179,8 +187,8 @@ public class BeanMethodPolymorphismTests { static class BaseConfig { @Bean - public TestBean testBean() { - return new TestBean(); + public BaseTestBean testBean() { + return new BaseTestBean(); } } @@ -195,8 +203,8 @@ public class BeanMethodPolymorphismTests { @Bean @Lazy @Override - public TestBean testBean() { - return new TestBean() { + public BaseTestBean testBean() { + return new BaseTestBean() { @Override public String toString() { return "overridden"; @@ -206,10 +214,6 @@ public class BeanMethodPolymorphismTests { } - static class ExtendedTestBean extends TestBean { - } - - @Configuration static class NarrowedOverridingConfig extends BaseConfig { @@ -226,7 +230,7 @@ public class BeanMethodPolymorphismTests { } - @Configuration + @Configuration(enforceUniqueMethods = false) static class ConfigWithOverloading { @Bean @@ -241,7 +245,7 @@ public class BeanMethodPolymorphismTests { } - @Configuration + @Configuration(enforceUniqueMethods = false) static class ConfigWithOverloadingAndAdditionalMetadata { @Bean @Lazy diff --git a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java index 491617d0bfe..d4ceff45574 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 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. @@ -626,7 +626,7 @@ public class ConfigurationClassProcessingTests { } - @Configuration + @Configuration(enforceUniqueMethods = false) public static class OverloadedBeanMismatch { @Bean(name = "other")