Prevent @Bean method overloading by default (with enforceUniqueMethods flag)
Closes gh-22609
This commit is contained in:
		
							parent
							
								
									41ee23345d
								
							
						
					
					
						commit
						4a470e0a37
					
				|  | @ -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. | ||||
| 	 * <p>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; | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -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<String, Object> 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<String, MethodMetadata> 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())); | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| } | ||||
|  |  | |||
|  | @ -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<SourceClass> 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<SourceClass> members = new ArrayList<>(declaredClasses.length); | ||||
|  | @ -1013,8 +1012,7 @@ class ConfigurationClassParser { | |||
| 
 | ||||
| 		public Set<SourceClass> getInterfaces() throws IOException { | ||||
| 			Set<SourceClass> 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<SourceClass> getAnnotations() { | ||||
| 			Set<SourceClass> 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")) { | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
|  | @ -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") | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue