Skip to content

Commit 6e4317e

Browse files
jhoellerjhoeller
committed
Properly handle knownSuperclasses in case of an overridden ConfigurationClass
Issue: SPR-10546
1 parent 325d883 commit 6e4317e

File tree

3 files changed

+66
-116
lines changed

3 files changed

+66
-116
lines changed

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

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@
3636

3737
/**
3838
* Represents a user-defined {@link Configuration @Configuration} class.
39-
* Includes a set of {@link Bean} methods, including all such methods defined in the
40-
* ancestry of the class, in a 'flattened-out' manner.
39+
* Includes a set of {@link Bean} methods, including all such methods
40+
* defined in the ancestry of the class, in a 'flattened-out' manner.
4141
*
4242
* @author Chris Beams
4343
* @author Juergen Hoeller
@@ -51,15 +51,15 @@ final class ConfigurationClass {
5151

5252
private final Resource resource;
5353

54-
private final Map<String, Class<? extends BeanDefinitionReader>> importedResources =
55-
new LinkedHashMap<String, Class<? extends BeanDefinitionReader>>();
56-
57-
private final Set<BeanMethod> beanMethods = new LinkedHashSet<BeanMethod>();
58-
5954
private String beanName;
6055

6156
private final boolean imported;
6257

58+
private final Set<BeanMethod> beanMethods = new LinkedHashSet<BeanMethod>();
59+
60+
private final Map<String, Class<? extends BeanDefinitionReader>> importedResources =
61+
new LinkedHashMap<String, Class<? extends BeanDefinitionReader>>();
62+
6363

6464
/**
6565
* Create a new {@link ConfigurationClass} with the given name.
@@ -98,7 +98,7 @@ public ConfigurationClass(MetadataReader metadataReader, boolean imported) {
9898
* @see ConfigurationClass#ConfigurationClass(Class, boolean)
9999
*/
100100
public ConfigurationClass(Class<?> clazz, String beanName) {
101-
Assert.hasText(beanName, "bean name must not be null");
101+
Assert.hasText(beanName, "Bean name must not be null");
102102
this.metadata = new StandardAnnotationMetadata(clazz, true);
103103
this.resource = new DescriptiveResource(clazz.toString());
104104
this.beanName = beanName;
@@ -132,6 +132,14 @@ public String getSimpleName() {
132132
return ClassUtils.getShortName(getMetadata().getClassName());
133133
}
134134

135+
public void setBeanName(String beanName) {
136+
this.beanName = beanName;
137+
}
138+
139+
public String getBeanName() {
140+
return this.beanName;
141+
}
142+
135143
/**
136144
* Return whether this configuration class was registered via @{@link Import} or
137145
* automatically registered due to being nested within another configuration class.
@@ -141,14 +149,6 @@ public boolean isImported() {
141149
return this.imported;
142150
}
143151

144-
public void setBeanName(String beanName) {
145-
this.beanName = beanName;
146-
}
147-
148-
public String getBeanName() {
149-
return this.beanName;
150-
}
151-
152152
public void addBeanMethod(BeanMethod method) {
153153
this.beanMethods.add(method);
154154
}
@@ -157,44 +157,40 @@ public Set<BeanMethod> getBeanMethods() {
157157
return this.beanMethods;
158158
}
159159

160-
public void addImportedResource(
161-
String importedResource, Class<? extends BeanDefinitionReader> readerClass) {
160+
public void addImportedResource(String importedResource, Class<? extends BeanDefinitionReader> readerClass) {
162161
this.importedResources.put(importedResource, readerClass);
163162
}
164163

165164
public Map<String, Class<? extends BeanDefinitionReader>> getImportedResources() {
166165
return this.importedResources;
167166
}
168167

168+
169169
public void validate(ProblemReporter problemReporter) {
170+
// A configuration class may not be final (CGLIB limitation)
171+
if (getMetadata().isAnnotated(Configuration.class.getName())) {
172+
if (getMetadata().isFinal()) {
173+
problemReporter.error(new FinalConfigurationProblem());
174+
}
175+
}
176+
170177
// An @Bean method may only be overloaded through inheritance. No single
171178
// @Configuration class may declare two @Bean methods with the same name.
172-
final char hashDelim = '#';
173179
Map<String, Integer> methodNameCounts = new HashMap<String, Integer>();
174-
for (BeanMethod beanMethod : beanMethods) {
175-
String dClassName = beanMethod.getMetadata().getDeclaringClassName();
176-
String methodName = beanMethod.getMetadata().getMethodName();
177-
String fqMethodName = dClassName + hashDelim + methodName;
180+
for (BeanMethod beanMethod : this.beanMethods) {
181+
String fqMethodName = beanMethod.getFullyQualifiedMethodName();
178182
Integer currentCount = methodNameCounts.get(fqMethodName);
179183
int newCount = currentCount != null ? currentCount + 1 : 1;
180184
methodNameCounts.put(fqMethodName, newCount);
181185
}
182-
183-
for (String methodName : methodNameCounts.keySet()) {
184-
int count = methodNameCounts.get(methodName);
186+
for (String fqMethodName : methodNameCounts.keySet()) {
187+
int count = methodNameCounts.get(fqMethodName);
185188
if (count > 1) {
186-
String shortMethodName = methodName.substring(methodName.indexOf(hashDelim)+1);
189+
String shortMethodName = ConfigurationMethod.getShortMethodName(fqMethodName);
187190
problemReporter.error(new BeanMethodOverloadingProblem(shortMethodName, count));
188191
}
189192
}
190193

191-
// A configuration class may not be final (CGLIB limitation)
192-
if (getMetadata().isAnnotated(Configuration.class.getName())) {
193-
if (getMetadata().isFinal()) {
194-
problemReporter.error(new FinalConfigurationProblem());
195-
}
196-
}
197-
198194
for (BeanMethod beanMethod : this.beanMethods) {
199195
beanMethod.validate(problemReporter);
200196
}

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

Lines changed: 24 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,9 @@
2323
import java.util.Collections;
2424
import java.util.Comparator;
2525
import java.util.HashMap;
26-
import java.util.HashSet;
2726
import java.util.Iterator;
28-
import java.util.LinkedHashMap;
2927
import java.util.LinkedHashSet;
3028
import java.util.Map;
31-
import java.util.Map.Entry;
3229
import java.util.Set;
3330
import java.util.Stack;
3431

@@ -79,7 +76,6 @@
7976
*
8077
* @author Chris Beams
8178
* @author Juergen Hoeller
82-
* @author Rob Winch
8379
* @since 3.0
8480
* @see ConfigurationClassBeanDefinitionReader
8581
*/
@@ -91,14 +87,6 @@ class ConfigurationClassParser {
9187

9288
private final ImportStack importStack = new ImportStack();
9389

94-
private final Set<String> knownSuperclasses = new LinkedHashSet<String>();
95-
96-
private final Map<ConfigurationClass,ConfigurationClass> configurationClasses =
97-
new LinkedHashMap<ConfigurationClass,ConfigurationClass>();
98-
99-
private final Stack<PropertySource<?>> propertySources =
100-
new Stack<PropertySource<?>>();
101-
10290
private final Environment environment;
10391

10492
private final ResourceLoader resourceLoader;
@@ -107,6 +95,12 @@ class ConfigurationClassParser {
10795

10896
private final ComponentScanAnnotationParser componentScanParser;
10997

98+
private final Set<ConfigurationClass> configurationClasses = new LinkedHashSet<ConfigurationClass>();
99+
100+
private final Map<String, ConfigurationClass> knownSuperclasses = new HashMap<String, ConfigurationClass>();
101+
102+
private final Stack<PropertySource<?>> propertySources = new Stack<PropertySource<?>>();
103+
110104

111105
/**
112106
* Create a new {@link ConfigurationClassParser} instance that will be used
@@ -155,70 +149,28 @@ protected void processConfigurationClass(ConfigurationClass configClass) throws
155149
}
156150
}
157151

158-
// recursively process the configuration class and its superclass hierarchy
159-
do {
160-
metadata = doProcessConfigurationClass(configClass, metadata);
161-
}
162-
while (metadata != null);
163-
164-
if (getConfigurationClasses().contains(configClass) && configClass.getBeanName() != null) {
152+
if (this.configurationClasses.contains(configClass) && configClass.getBeanName() != null) {
165153
// Explicit bean definition found, probably replacing an import.
166154
// Let's remove the old one and go with the new one.
167-
ConfigurationClass originalConfigClass = removeConfigurationClass(configClass);
168-
169-
mergeFromOriginalConfig(originalConfigClass,configClass);
170-
}
171-
172-
addConfigurationClass(configClass);
173-
}
174-
175-
176-
/**
177-
* Merges from the original {@link ConfigurationClass} to the new
178-
* {@link ConfigurationClass}. This is necessary if parent classes have already been
179-
* processed.
180-
*
181-
* @param originalConfigClass the original {@link ConfigurationClass} that may have
182-
* additional metadata
183-
* @param configClass the new {@link ConfigurationClass} that will have metadata added
184-
* to it if necessary
185-
*/
186-
private void mergeFromOriginalConfig(ConfigurationClass originalConfigClass,
187-
ConfigurationClass configClass) {
188-
189-
Set<String> beanMethodNames = new HashSet<String>();
190-
for(BeanMethod beanMethod : configClass.getBeanMethods()) {
191-
beanMethodNames.add(createBeanMethodName(beanMethod));
192-
}
193-
194-
for(BeanMethod originalBeanMethod : originalConfigClass.getBeanMethods()) {
195-
String originalBeanMethodName = createBeanMethodName(originalBeanMethod);
196-
if(!beanMethodNames.contains(originalBeanMethodName)) {
197-
configClass.addBeanMethod(new BeanMethod(originalBeanMethod.getMetadata(), configClass));
155+
this.configurationClasses.remove(configClass);
156+
for (Iterator<ConfigurationClass> it = this.knownSuperclasses.values().iterator(); it.hasNext();) {
157+
if (configClass.equals(it.next())) {
158+
it.remove();
159+
}
198160
}
199161
}
200-
for(Entry<String, Class<? extends BeanDefinitionReader>> originalImportedEntry : originalConfigClass.getImportedResources().entrySet()) {
201-
if(!configClass.getImportedResources().containsKey(originalImportedEntry.getKey())) {
202-
configClass.addImportedResource(originalImportedEntry.getKey(), originalImportedEntry.getValue());
203-
}
162+
163+
// Recursively process the configuration class and its superclass hierarchy.
164+
do {
165+
metadata = doProcessConfigurationClass(configClass, metadata);
204166
}
205-
}
167+
while (metadata != null);
206168

207-
/**
208-
* Converts a {@link BeanMethod} into the fully qualified name of the Method
209-
*
210-
* @param beanMethod
211-
* @return fully qualified name of the {@link BeanMethod}
212-
*/
213-
private String createBeanMethodName(BeanMethod beanMethod) {
214-
String hashDelim = "#";
215-
String dClassName = beanMethod.getMetadata().getDeclaringClassName();
216-
String methodName = beanMethod.getMetadata().getMethodName();
217-
return dClassName + hashDelim + methodName;
169+
this.configurationClasses.add(configClass);
218170
}
219171

220172
/**
221-
* @return annotation metadata of superclass, null if none found or previously processed
173+
* @return annotation metadata of superclass, {@code null} if none found or previously processed
222174
*/
223175
protected AnnotationMetadata doProcessConfigurationClass(
224176
ConfigurationClass configClass, AnnotationMetadata metadata) throws IOException {
@@ -232,7 +184,7 @@ protected AnnotationMetadata doProcessConfigurationClass(
232184
processPropertySource(propertySource);
233185
}
234186

235-
// process any @ComponentScan annotions
187+
// process any @ComponentScan annotations
236188
AnnotationAttributes componentScan = attributesFor(metadata, ComponentScan.class);
237189
if (componentScan != null) {
238190
// the config class is annotated with @ComponentScan -> perform the scan immediately
@@ -248,7 +200,6 @@ protected AnnotationMetadata doProcessConfigurationClass(
248200
}
249201

250202
// process any @Import annotations
251-
252203
Set<Object> imports = new LinkedHashSet<Object>();
253204
Set<Object> visited = new LinkedHashSet<Object>();
254205
collectImports(metadata, imports, visited);
@@ -275,7 +226,8 @@ protected AnnotationMetadata doProcessConfigurationClass(
275226
// process superclass, if any
276227
if (metadata.hasSuperClass()) {
277228
String superclass = metadata.getSuperClassName();
278-
if (this.knownSuperclasses.add(superclass)) {
229+
if (!this.knownSuperclasses.containsKey(superclass)) {
230+
this.knownSuperclasses.put(superclass, configClass);
279231
// superclass found, return its annotation metadata and recurse
280232
if (metadata instanceof StandardAnnotationMetadata) {
281233
Class<?> clazz = ((StandardAnnotationMetadata) metadata).getIntrospectedClass();
@@ -302,14 +254,6 @@ else if (superclass.startsWith("java")) {
302254
return null;
303255
}
304256

305-
private void addConfigurationClass(ConfigurationClass configClass) {
306-
this.configurationClasses.put(configClass,configClass);
307-
}
308-
309-
private ConfigurationClass removeConfigurationClass(ConfigurationClass configClass) {
310-
return this.configurationClasses.remove(configClass);
311-
}
312-
313257
/**
314258
* Register member (nested) classes that happen to be configuration classes themselves.
315259
* @param metadata the metadata representation of the containing class
@@ -500,13 +444,13 @@ private void invokeAwareMethods(ImportBeanDefinitionRegistrar registrar) {
500444
* @see ConfigurationClass#validate
501445
*/
502446
public void validate() {
503-
for (ConfigurationClass configClass : getConfigurationClasses()) {
447+
for (ConfigurationClass configClass : this.configurationClasses) {
504448
configClass.validate(this.problemReporter);
505449
}
506450
}
507451

508452
public Set<ConfigurationClass> getConfigurationClasses() {
509-
return this.configurationClasses.keySet();
453+
return this.configurationClasses;
510454
}
511455

512456
public Stack<PropertySource<?>> getPropertySources() {

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2011 the original author or authors.
2+
* Copyright 2002-2013 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.
@@ -36,6 +36,7 @@ public ConfigurationMethod(MethodMetadata metadata, ConfigurationClass configura
3636
this.configurationClass = configurationClass;
3737
}
3838

39+
3940
public MethodMetadata getMetadata() {
4041
return this.metadata;
4142
}
@@ -48,13 +49,22 @@ public Location getResourceLocation() {
4849
return new Location(this.configurationClass.getResource(), this.metadata);
4950
}
5051

52+
String getFullyQualifiedMethodName() {
53+
return this.metadata.getDeclaringClassName() + "#" + this.metadata.getMethodName();
54+
}
55+
56+
static String getShortMethodName(String fullyQualifiedMethodName) {
57+
return fullyQualifiedMethodName.substring(fullyQualifiedMethodName.indexOf('#') + 1);
58+
}
59+
5160
public void validate(ProblemReporter problemReporter) {
5261
}
5362

63+
5464
@Override
5565
public String toString() {
5666
return String.format("[%s:name=%s,declaringClass=%s]",
57-
this.getClass().getSimpleName(), this.getMetadata().getMethodName(), this.getMetadata().getDeclaringClassName());
67+
getClass().getSimpleName(), getMetadata().getMethodName(), getMetadata().getDeclaringClassName());
5868
}
5969

6070
}

0 commit comments

Comments
 (0)