root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Consistent processing of overridden bean methods 

Closes gh-28286
This commit is contained in:
Juergen Hoeller 2024-02-21 18:36:03 +01:00
parent 2a1b30dbed
commit f5397d6426
5 changed files with 103 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
spring-context/src/main/java/org/springframework/context/annotation/BeanMethod.java
View File

@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2024 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.
@ -16,6 +16,8 @@
package org.springframework.context.annotation;
import java.util.Map;
import org.springframework.beans.factory.parsing.Problem;
import org.springframework.beans.factory.parsing.ProblemReporter;
import org.springframework.core.type.MethodMetadata;
@ -52,22 +54,24 @@ final class BeanMethod extends ConfigurationMethod {
return;
}
if (this.configurationClass.getMetadata().isAnnotated(Configuration.class.getName())) {
if (!getMetadata().isOverridable()) {
// instance @Bean methods within @Configuration classes must be overridable to accommodate CGLIB
problemReporter.error(new NonOverridableMethodError());
}
Map<String, Object> attributes =
getConfigurationClass().getMetadata().getAnnotationAttributes(Configuration.class.getName());
if (attributes != null && (Boolean) attributes.get("proxyBeanMethods") && !getMetadata().isOverridable()) {
// instance @Bean methods within @Configuration classes must be overridable to accommodate CGLIB
problemReporter.error(new NonOverridableMethodError());
}
}
@Override
public boolean equals(@Nullable Object other) {
return (this == other || (other instanceof BeanMethod that && this.metadata.equals(that.metadata)));
return (this == other || (other instanceof BeanMethod that &&
this.configurationClass.equals(that.configurationClass) &&
getLocalMethodIdentifier(this.metadata).equals(getLocalMethodIdentifier(that.metadata))));
}
@Override
public int hashCode() {
return this.metadata.hashCode();
return this.configurationClass.hashCode() * 31 + getLocalMethodIdentifier(this.metadata).hashCode();
}
@Override
@ -76,6 +80,14 @@ final class BeanMethod extends ConfigurationMethod {
}
private static String getLocalMethodIdentifier(MethodMetadata metadata) {
String metadataString = metadata.toString();
int index = metadataString.indexOf(metadata.getDeclaringClassName());
return (index >= 0 ? metadataString.substring(index + metadata.getDeclaringClassName().length()) :
metadataString);
}
private class VoidDeclaredMethodError extends Problem {
VoidDeclaredMethodError() {

13
spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClass.java
View File

@ -223,13 +223,12 @@ final class ConfigurationClass {
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());
}
for (BeanMethod beanMethod : this.beanMethods) {
beanMethod.validate(problemReporter);
}
if (attributes != null && (Boolean) attributes.get("proxyBeanMethods") && this.metadata.isFinal()) {
problemReporter.error(new FinalConfigurationProblem());
}
for (BeanMethod beanMethod : this.beanMethods) {
beanMethod.validate(problemReporter);
}
// A configuration class may not contain overloaded bean methods unless it declares enforceUniqueMethods=false

9
spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java
View File

@ -53,6 +53,7 @@ import org.springframework.core.type.StandardAnnotationMetadata;
import org.springframework.core.type.StandardMethodMetadata;
import org.springframework.lang.NonNull;
import org.springframework.util.Assert;
import org.springframework.util.ClassUtils;
import org.springframework.util.StringUtils;
/**
@ -229,8 +230,12 @@ class ConfigurationClassBeanDefinitionReader {
beanDef.setUniqueFactoryMethodName(methodName);
}
if (metadata instanceof StandardMethodMetadata sam) {
beanDef.setResolvedFactoryMethod(sam.getIntrospectedMethod());
if (metadata instanceof StandardMethodMetadata smm &&
configClass.getMetadata() instanceof StandardAnnotationMetadata sam) {
Method method = ClassUtils.getMostSpecificMethod(smm.getIntrospectedMethod(), sam.getIntrospectedClass());
if (method == smm.getIntrospectedMethod()) {
beanDef.setResolvedFactoryMethod(method);
}
}
beanDef.setAutowireMode(AbstractBeanDefinition.AUTOWIRE_CONSTRUCTOR);

58
spring-context/src/test/java/org/springframework/context/annotation/BeanMethodPolymorphismTests.java
View File

@ -29,6 +29,7 @@ import static org.assertj.core.api.Assertions.assertThat;
/**
* Tests regarding overloading and overriding of bean methods.
*
* <p>Related to SPR-6618.
*
* @author Chris Beams
@ -41,6 +42,7 @@ public class BeanMethodPolymorphismTests {
@Test
void beanMethodDetectedOnSuperClass() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(Config.class);
assertThat(ctx.getBean("testBean", BaseTestBean.class)).isNotNull();
}
@ -50,6 +52,7 @@ public class BeanMethodPolymorphismTests {
ctx.register(OverridingConfig.class);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue();
@ -61,17 +64,45 @@ public class BeanMethodPolymorphismTests {
ctx.registerBeanDefinition("config", new RootBeanDefinition(OverridingConfig.class.getName()));
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue();
}
@Test
void beanMethodOverridingWithDifferentBeanName() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.register(OverridingConfigWithDifferentBeanName.class);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("myTestBean")).isFalse();
assertThat(ctx.getBean("myTestBean", BaseTestBean.class).toString()).isEqualTo("overridden");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("myTestBean")).isTrue();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
}
@Test
void beanMethodOverridingWithDifferentBeanNameOnASM() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.registerBeanDefinition("config", new RootBeanDefinition(OverridingConfigWithDifferentBeanName.class.getName()));
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("myTestBean")).isFalse();
assertThat(ctx.getBean("myTestBean", BaseTestBean.class).toString()).isEqualTo("overridden");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("myTestBean")).isTrue();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
}
@Test
void beanMethodOverridingWithNarrowedReturnType() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.register(NarrowedOverridingConfig.class);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue();
@ -83,6 +114,7 @@ public class BeanMethodPolymorphismTests {
ctx.registerBeanDefinition("config", new RootBeanDefinition(NarrowedOverridingConfig.class.getName()));
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isFalse();
assertThat(ctx.getBean("testBean", BaseTestBean.class).toString()).isEqualTo("overridden");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("testBean")).isTrue();
@ -94,6 +126,7 @@ public class BeanMethodPolymorphismTests {
ctx.register(ConfigWithOverloading.class);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getBean(String.class)).isEqualTo("regular");
}
@ -104,6 +137,7 @@ public class BeanMethodPolymorphismTests {
ctx.getDefaultListableBeanFactory().registerSingleton("anInt", 5);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getBean(String.class)).isEqualTo("overloaded5");
}
@ -113,6 +147,7 @@ public class BeanMethodPolymorphismTests {
ctx.register(ConfigWithOverloadingAndAdditionalMetadata.class);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("aString")).isFalse();
assertThat(ctx.getBean(String.class)).isEqualTo("regular");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("aString")).isTrue();
@ -125,6 +160,7 @@ public class BeanMethodPolymorphismTests {
ctx.getDefaultListableBeanFactory().registerSingleton("anInt", 5);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("aString")).isFalse();
assertThat(ctx.getBean(String.class)).isEqualTo("overloaded5");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("aString")).isTrue();
@ -136,18 +172,19 @@ public class BeanMethodPolymorphismTests {
ctx.register(SubConfig.class);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("aString")).isFalse();
assertThat(ctx.getBean(String.class)).isEqualTo("overloaded5");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("aString")).isTrue();
}
// SPR-11025
@Test
@Test // SPR-11025
void beanMethodOverloadingWithInheritanceAndList() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.register(SubConfigWithList.class);
ctx.setAllowBeanDefinitionOverriding(false);
ctx.refresh();
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("aString")).isFalse();
assertThat(ctx.getBean(String.class)).isEqualTo("overloaded5");
assertThat(ctx.getDefaultListableBeanFactory().containsSingleton("aString")).isTrue();
@ -161,6 +198,7 @@ public class BeanMethodPolymorphismTests {
@Test
void beanMethodShadowing() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(ShadowConfig.class);
assertThat(ctx.getBean(String.class)).isEqualTo("shadow");
}
@ -214,6 +252,22 @@ public class BeanMethodPolymorphismTests {
}
@Configuration
static class OverridingConfigWithDifferentBeanName extends BaseConfig {
@Bean("myTestBean") @Lazy
@Override
public BaseTestBean testBean() {
return new BaseTestBean() {
@Override
public String toString() {
return "overridden";
}
};
}
}
@Configuration
static class NarrowedOverridingConfig extends BaseConfig {

14
spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationClassProcessingTests.java
View File

@ -143,6 +143,11 @@ class ConfigurationClassProcessingTests {
initBeanFactory(ConfigWithFinalBean.class));
}
@Test
void finalBeanMethodWithoutProxy() {
initBeanFactory(ConfigWithFinalBeanWithoutProxy.class);
}
@Test // gh-31007
void voidBeanMethod() {
assertThatExceptionOfType(BeanDefinitionParsingException.class).isThrownBy(() ->
@ -438,6 +443,15 @@ class ConfigurationClassProcessingTests {
}
@Configuration(proxyBeanMethods = false)
static class ConfigWithFinalBeanWithoutProxy {
@Bean public final TestBean testBean() {
return new TestBean();
}
}
@Configuration
static class ConfigWithVoidBean {