Skip to content

Commit 7a74e45

Browse files
snicollsnicoll
committed
Make use of bean definition overriding more visible
This commit makes the use of bean definition overriding more visible and prepare for a deprecation of the feature in the next major release. As of this commit, use of bean definition overriding logs at INFO level. The previous log level can be restored by setting the allowBeanDefinitionOverriding flag explicitly on the BeanFactory (or via the related ApplicationContext). A number of tests that are using bean overriding on purpose have been updated to set this flag, which will make them easier to find once we actually deprecate the feature. Closes gh-31288
1 parent 8af3eb1 commit 7a74e45

File tree

17 files changed

+186
-71
lines changed

17 files changed

+186
-71
lines changed

‎framework-docs/modules/ROOT/pages/core/beans/definition.adoc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,23 @@ runtime (concurrently with live access to the factory) is not officially support
7373
lead to concurrent access exceptions, inconsistent state in the bean container, or both.
7474
====
7575

76+
[[beans-definition-overriding]]
77+
== Overriding Beans
7678

79+
Bean overriding is happening when a bean is registered using an identifier that is
80+
already allocated. While bean overriding is possible, it makes the configuration harder
81+
to read and this feature will be deprecated in a future release.
82+
83+
To disable bean overriding altogether, you can set the `allowBeanDefinitionOverriding`
84+
to `false` on the `ApplicationContext` before it is refreshed. In such setup, an
85+
exception is thrown if bean overriding is used.
86+
87+
By default, the container logs every bean overriding at `INFO` level so that you can
88+
adapt your configuration accordingly. While not recommended, you can silence those logs
89+
by setting the `allowBeanDefinitionOverriding` flag to `true`.
90+
91+
NOTE: We acknowledge that overriding beans in a test is convenient, and there is
92+
explicit support for this. For more details please refer to xref:testing/annotations/integration-spring/annotation-beanoverriding.adoc[this section].
7793

7894
[[beans-beanname]]
7995
== Naming Beans

‎spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,8 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
152152
private String serializationId;
153153

154154
/** Whether to allow re-registration of a different definition with the same name. */
155-
private boolean allowBeanDefinitionOverriding = true;
155+
@Nullable
156+
private Boolean allowBeanDefinitionOverriding;
156157

157158
/** Whether to allow eager class loading even for lazy-init beans. */
158159
private boolean allowEagerClassLoading = true;
@@ -259,7 +260,7 @@ public void setAllowBeanDefinitionOverriding(boolean allowBeanDefinitionOverridi
259260
* @since 4.1.2
260261
*/
261262
public boolean isAllowBeanDefinitionOverriding() {
262-
return this.allowBeanDefinitionOverriding;
263+
return !Boolean.FALSE.equals(this.allowBeanDefinitionOverriding);
263264
}
264265

265266
/**
@@ -1142,27 +1143,8 @@ public void registerBeanDefinition(String beanName, BeanDefinition beanDefinitio
11421143
if (!isBeanDefinitionOverridable(beanName)) {
11431144
throw new BeanDefinitionOverrideException(beanName, beanDefinition, existingDefinition);
11441145
}
1145-
else if (existingDefinition.getRole() < beanDefinition.getRole()) {
1146-
// e.g. was ROLE_APPLICATION, now overriding with ROLE_SUPPORT or ROLE_INFRASTRUCTURE
1147-
if (logger.isInfoEnabled()) {
1148-
logger.info("Overriding user-defined bean definition for bean '" + beanName +
1149-
"' with a framework-generated bean definition: replacing [" +
1150-
existingDefinition + "] with [" + beanDefinition + "]");
1151-
}
1152-
}
1153-
else if (!beanDefinition.equals(existingDefinition)) {
1154-
if (logger.isDebugEnabled()) {
1155-
logger.debug("Overriding bean definition for bean '" + beanName +
1156-
"' with a different definition: replacing [" + existingDefinition +
1157-
"] with [" + beanDefinition + "]");
1158-
}
1159-
}
11601146
else {
1161-
if (logger.isTraceEnabled()) {
1162-
logger.trace("Overriding bean definition for bean '" + beanName +
1163-
"' with an equivalent definition: replacing [" + existingDefinition +
1164-
"] with [" + beanDefinition + "]");
1165-
}
1147+
logBeanDefinitionOverriding(beanName, beanDefinition, existingDefinition);
11661148
}
11671149
this.beanDefinitionMap.put(beanName, beanDefinition);
11681150
}
@@ -1217,6 +1199,44 @@ else if (isConfigurationFrozen()) {
12171199
}
12181200
}
12191201

1202+
private void logBeanDefinitionOverriding(String beanName, BeanDefinition beanDefinition,
1203+
BeanDefinition existingDefinition) {
1204+
1205+
boolean explicitBeanOverride = (this.allowBeanDefinitionOverriding != null);
1206+
if (existingDefinition.getRole() < beanDefinition.getRole()) {
1207+
// e.g. was ROLE_APPLICATION, now overriding with ROLE_SUPPORT or ROLE_INFRASTRUCTURE
1208+
if (logger.isInfoEnabled()) {
1209+
logger.info("Overriding user-defined bean definition for bean '" + beanName +
1210+
"' with a framework-generated bean definition: replacing [" +
1211+
existingDefinition + "] with [" + beanDefinition + "]");
1212+
}
1213+
}
1214+
else if (!beanDefinition.equals(existingDefinition)) {
1215+
if (explicitBeanOverride && logger.isInfoEnabled()) {
1216+
logger.info("Overriding bean definition for bean '" + beanName +
1217+
"' with a different definition: replacing [" + existingDefinition +
1218+
"] with [" + beanDefinition + "]");
1219+
}
1220+
if (logger.isDebugEnabled()) {
1221+
logger.debug("Overriding bean definition for bean '" + beanName +
1222+
"' with a different definition: replacing [" + existingDefinition +
1223+
"] with [" + beanDefinition + "]");
1224+
}
1225+
}
1226+
else {
1227+
if (explicitBeanOverride && logger.isInfoEnabled()) {
1228+
logger.info("Overriding bean definition for bean '" + beanName +
1229+
"' with an equivalent definition: replacing [" + existingDefinition +
1230+
"] with [" + beanDefinition + "]");
1231+
}
1232+
if (logger.isTraceEnabled()) {
1233+
logger.trace("Overriding bean definition for bean '" + beanName +
1234+
"' with an equivalent definition: replacing [" + existingDefinition +
1235+
"] with [" + beanDefinition + "]");
1236+
}
1237+
}
1238+
}
1239+
12201240
@Override
12211241
public void removeBeanDefinition(String beanName) throws NoSuchBeanDefinitionException {
12221242
Assert.hasText(beanName, "'beanName' must not be empty");

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,7 @@ void mergedBeanDefinitionChangesRetainedAfterFreezeConfiguration() {
840840

841841
@Test
842842
void aliasCircle() {
843+
lbf.setAllowBeanDefinitionOverriding(true);
843844
lbf.registerAlias("test", "test2");
844845
lbf.registerAlias("test2", "test3");
845846

@@ -867,6 +868,7 @@ void aliasChaining() {
867868

868869
@Test
869870
void beanDefinitionOverriding() {
871+
lbf.setAllowBeanDefinitionOverriding(true);
870872
lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class));
871873
lbf.registerBeanDefinition("test", new RootBeanDefinition(NestedTestBean.class));
872874
lbf.registerAlias("otherTest", "test2");
@@ -906,6 +908,7 @@ void beanDefinitionOverridingNotAllowed() {
906908

907909
@Test
908910
void beanDefinitionOverridingWithAlias() {
911+
lbf.setAllowBeanDefinitionOverriding(true);
909912
lbf.registerBeanDefinition("test", new RootBeanDefinition(TestBean.class));
910913
lbf.registerAlias("test", "testAlias");
911914
lbf.registerBeanDefinition("test", new RootBeanDefinition(NestedTestBean.class));
@@ -917,6 +920,7 @@ void beanDefinitionOverridingWithAlias() {
917920

918921
@Test
919922
void beanDefinitionOverridingWithConstructorArgumentMismatch() {
923+
lbf.setAllowBeanDefinitionOverriding(true);
920924
RootBeanDefinition bd1 = new RootBeanDefinition(NestedTestBean.class);
921925
bd1.getConstructorArgumentValues().addIndexedArgumentValue(1, "value1");
922926
lbf.registerBeanDefinition("test", bd1);
@@ -1196,6 +1200,7 @@ void registerExistingSingletonWithAlreadyBound() {
11961200

11971201
@Test
11981202
void reregisterBeanDefinition() {
1203+
lbf.setAllowBeanDefinitionOverriding(true);
11991204
RootBeanDefinition bd1 = new RootBeanDefinition(TestBean.class);
12001205
bd1.setScope(BeanDefinition.SCOPE_PROTOTYPE);
12011206
lbf.registerBeanDefinition("testBean", bd1);
@@ -1306,6 +1311,7 @@ void expressionInStringArray() {
13061311

13071312
@Test
13081313
void withOverloadedSetters() {
1314+
lbf.setAllowBeanDefinitionOverriding(true);
13091315
RootBeanDefinition rbd = new RootBeanDefinition(SetterOverload.class);
13101316
rbd.getPropertyValues().add("object", "a String");
13111317
lbf.registerBeanDefinition("overloaded", rbd);

‎spring-beans/src/test/java/org/springframework/beans/factory/annotation/LookupAnnotationTests.java

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.beans.factory.annotation;
1818

19-
import org.junit.jupiter.api.BeforeEach;
2019
import org.junit.jupiter.api.Test;
2120

2221
import org.springframework.beans.factory.config.BeanDefinition;
@@ -33,25 +32,9 @@
3332
*/
3433
class LookupAnnotationTests {
3534

36-
private DefaultListableBeanFactory beanFactory;
37-
38-
39-
@BeforeEach
40-
void setup() {
41-
beanFactory = new DefaultListableBeanFactory();
42-
AutowiredAnnotationBeanPostProcessor aabpp = new AutowiredAnnotationBeanPostProcessor();
43-
aabpp.setBeanFactory(beanFactory);
44-
beanFactory.addBeanPostProcessor(aabpp);
45-
beanFactory.registerBeanDefinition("abstractBean", new RootBeanDefinition(AbstractBean.class));
46-
beanFactory.registerBeanDefinition("beanConsumer", new RootBeanDefinition(BeanConsumer.class));
47-
RootBeanDefinition tbd = new RootBeanDefinition(TestBean.class);
48-
tbd.setScope(BeanDefinition.SCOPE_PROTOTYPE);
49-
beanFactory.registerBeanDefinition("testBean", tbd);
50-
}
51-
52-
5335
@Test
5436
void testWithoutConstructorArg() {
37+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
5538
AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean");
5639
Object expected = bean.get();
5740
assertThat(expected.getClass()).isEqualTo(TestBean.class);
@@ -60,6 +43,7 @@ void testWithoutConstructorArg() {
6043

6144
@Test
6245
void testWithOverloadedArg() {
46+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
6347
AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean");
6448
TestBean expected = bean.get("haha");
6549
assertThat(expected.getClass()).isEqualTo(TestBean.class);
@@ -69,6 +53,7 @@ void testWithOverloadedArg() {
6953

7054
@Test
7155
void testWithOneConstructorArg() {
56+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
7257
AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean");
7358
TestBean expected = bean.getOneArgument("haha");
7459
assertThat(expected.getClass()).isEqualTo(TestBean.class);
@@ -78,6 +63,7 @@ void testWithOneConstructorArg() {
7863

7964
@Test
8065
void testWithTwoConstructorArg() {
66+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
8167
AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean");
8268
TestBean expected = bean.getTwoArguments("haha", 72);
8369
assertThat(expected.getClass()).isEqualTo(TestBean.class);
@@ -88,6 +74,7 @@ void testWithTwoConstructorArg() {
8874

8975
@Test
9076
void testWithThreeArgsShouldFail() {
77+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
9178
AbstractBean bean = (AbstractBean) beanFactory.getBean("abstractBean");
9279
assertThatExceptionOfType(AbstractMethodError.class).as("TestBean has no three arg constructor").isThrownBy(() ->
9380
bean.getThreeArguments("name", 1, 2));
@@ -96,6 +83,7 @@ void testWithThreeArgsShouldFail() {
9683

9784
@Test
9885
void testWithEarlyInjection() {
86+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
9987
AbstractBean bean = beanFactory.getBean("beanConsumer", BeanConsumer.class).abstractBean;
10088
Object expected = bean.get();
10189
assertThat(expected.getClass()).isEqualTo(TestBean.class);
@@ -106,7 +94,7 @@ void testWithEarlyInjection() {
10694
public void testWithNullBean() {
10795
RootBeanDefinition tbd = new RootBeanDefinition(TestBean.class, () -> null);
10896
tbd.setScope(BeanDefinition.SCOPE_PROTOTYPE);
109-
beanFactory.registerBeanDefinition("testBean", tbd);
97+
DefaultListableBeanFactory beanFactory = configureBeanFactory(tbd);
11098

11199
AbstractBean bean = beanFactory.getBean("beanConsumer", BeanConsumer.class).abstractBean;
112100
Object expected = bean.get();
@@ -116,6 +104,7 @@ public void testWithNullBean() {
116104

117105
@Test
118106
void testWithGenericBean() {
107+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
119108
beanFactory.registerBeanDefinition("numberBean", new RootBeanDefinition(NumberBean.class));
120109
beanFactory.registerBeanDefinition("doubleStore", new RootBeanDefinition(DoubleStore.class));
121110
beanFactory.registerBeanDefinition("floatStore", new RootBeanDefinition(FloatStore.class));
@@ -127,6 +116,7 @@ void testWithGenericBean() {
127116

128117
@Test
129118
void testSingletonWithoutMetadataCaching() {
119+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
130120
beanFactory.setCacheBeanMetadata(false);
131121

132122
beanFactory.registerBeanDefinition("numberBean", new RootBeanDefinition(NumberBean.class));
@@ -140,6 +130,7 @@ void testSingletonWithoutMetadataCaching() {
140130

141131
@Test
142132
void testPrototypeWithoutMetadataCaching() {
133+
DefaultListableBeanFactory beanFactory = configureBeanFactory();
143134
beanFactory.setCacheBeanMetadata(false);
144135

145136
beanFactory.registerBeanDefinition("numberBean", new RootBeanDefinition(NumberBean.class, BeanDefinition.SCOPE_PROTOTYPE, null));
@@ -155,6 +146,23 @@ void testPrototypeWithoutMetadataCaching() {
155146
assertThat(beanFactory.getBean(FloatStore.class)).isSameAs(bean.getFloatStore());
156147
}
157148

149+
private DefaultListableBeanFactory configureBeanFactory(RootBeanDefinition tbd) {
150+
DefaultListableBeanFactory beanFactory = new DefaultListableBeanFactory();
151+
AutowiredAnnotationBeanPostProcessor aabpp = new AutowiredAnnotationBeanPostProcessor();
152+
aabpp.setBeanFactory(beanFactory);
153+
beanFactory.addBeanPostProcessor(aabpp);
154+
beanFactory.registerBeanDefinition("abstractBean", new RootBeanDefinition(AbstractBean.class));
155+
beanFactory.registerBeanDefinition("beanConsumer", new RootBeanDefinition(BeanConsumer.class));
156+
beanFactory.registerBeanDefinition("testBean", tbd);
157+
return beanFactory;
158+
}
159+
160+
private DefaultListableBeanFactory configureBeanFactory() {
161+
RootBeanDefinition tbd = new RootBeanDefinition(TestBean.class);
162+
tbd.setScope(BeanDefinition.SCOPE_PROTOTYPE);
163+
return configureBeanFactory(tbd);
164+
}
165+
158166

159167
public abstract static class AbstractBean {
160168

‎spring-beans/src/test/java/org/springframework/beans/factory/xml/DuplicateBeanIdTests.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2023 the original author or authors.
2+
* Copyright 2002-2024 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.
@@ -52,6 +52,7 @@ void duplicateBeanIdsWithinSameNestingLevelRaisesError() {
5252
@Test
5353
void duplicateBeanIdsAcrossNestingLevels() {
5454
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
55+
bf.setAllowBeanDefinitionOverriding(true);
5556
XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(bf);
5657
reader.loadBeanDefinitions(new ClassPathResource("DuplicateBeanIdTests-multiLevel-context.xml", this.getClass()));
5758
TestBean testBean = bf.getBean(TestBean.class); // there should be only one

‎spring-beans/src/test/java/org/springframework/beans/factory/xml/NestedBeansElementTests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ void getBean_withActiveProfile() {
5151
env.setActiveProfiles("dev");
5252

5353
DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
54+
bf.setAllowBeanDefinitionOverriding(true);
5455
XmlBeanDefinitionReader reader = new XmlBeanDefinitionReader(bf);
5556
reader.setEnvironment(env);
5657
reader.loadBeanDefinitions(XML);

0 commit comments

Comments
 (0)