Skip to content

Commit 3f7007f

Browse files
jhoellerjhoeller
committed
Properly handle knownSuperclasses in case of an overridden ConfigurationClass
Issue: SPR-10546 (cherry picked from commit 6e4317e)
1 parent d1859c8 commit 3f7007f

File tree

3 files changed

+104
-95
lines changed

3 files changed

+104
-95
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
@@ -52,15 +52,15 @@ final class ConfigurationClass {
5252

5353
private final Resource resource;
5454

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

6257
private final ConfigurationClass importedBy;
6358

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

6565
/**
6666
* Create a new {@link ConfigurationClass} with the given name.
@@ -99,7 +99,7 @@ public ConfigurationClass(MetadataReader metadataReader, ConfigurationClass impo
9999
* @see ConfigurationClass#ConfigurationClass(Class, ConfigurationClass)
100100
*/
101101
public ConfigurationClass(Class<?> clazz, String beanName) {
102-
Assert.hasText(beanName, "bean name must not be null");
102+
Assert.hasText(beanName, "Bean name must not be null");
103103
this.metadata = new StandardAnnotationMetadata(clazz, true);
104104
this.resource = new DescriptiveResource(clazz.toString());
105105
this.beanName = beanName;
@@ -133,6 +133,14 @@ public String getSimpleName() {
133133
return ClassUtils.getShortName(getMetadata().getClassName());
134134
}
135135

136+
public void setBeanName(String beanName) {
137+
this.beanName = beanName;
138+
}
139+
140+
public String getBeanName() {
141+
return this.beanName;
142+
}
143+
136144
/**
137145
* Return whether this configuration class was registered via @{@link Import} or
138146
* automatically registered due to being nested within another configuration class.
@@ -153,14 +161,6 @@ public ConfigurationClass getImportedBy() {
153161
return importedBy;
154162
}
155163

156-
public void setBeanName(String beanName) {
157-
this.beanName = beanName;
158-
}
159-
160-
public String getBeanName() {
161-
return this.beanName;
162-
}
163-
164164
public void addBeanMethod(BeanMethod method) {
165165
this.beanMethods.add(method);
166166
}
@@ -169,44 +169,40 @@ public Set<BeanMethod> getBeanMethods() {
169169
return this.beanMethods;
170170
}
171171

172-
public void addImportedResource(
173-
String importedResource, Class<? extends BeanDefinitionReader> readerClass) {
172+
public void addImportedResource(String importedResource, Class<? extends BeanDefinitionReader> readerClass) {
174173
this.importedResources.put(importedResource, readerClass);
175174
}
176175

177176
public Map<String, Class<? extends BeanDefinitionReader>> getImportedResources() {
178177
return this.importedResources;
179178
}
180179

180+
181181
public void validate(ProblemReporter problemReporter) {
182+
// A configuration class may not be final (CGLIB limitation)
183+
if (getMetadata().isAnnotated(Configuration.class.getName())) {
184+
if (getMetadata().isFinal()) {
185+
problemReporter.error(new FinalConfigurationProblem());
186+
}
187+
}
188+
182189
// An @Bean method may only be overloaded through inheritance. No single
183190
// @Configuration class may declare two @Bean methods with the same name.
184-
final char hashDelim = '#';
185191
Map<String, Integer> methodNameCounts = new HashMap<String, Integer>();
186-
for (BeanMethod beanMethod : beanMethods) {
187-
String dClassName = beanMethod.getMetadata().getDeclaringClassName();
188-
String methodName = beanMethod.getMetadata().getMethodName();
189-
String fqMethodName = dClassName + hashDelim + methodName;
192+
for (BeanMethod beanMethod : this.beanMethods) {
193+
String fqMethodName = beanMethod.getFullyQualifiedMethodName();
190194
Integer currentCount = methodNameCounts.get(fqMethodName);
191195
int newCount = currentCount != null ? currentCount + 1 : 1;
192196
methodNameCounts.put(fqMethodName, newCount);
193197
}
194-
195-
for (String methodName : methodNameCounts.keySet()) {
196-
int count = methodNameCounts.get(methodName);
198+
for (String fqMethodName : methodNameCounts.keySet()) {
199+
int count = methodNameCounts.get(fqMethodName);
197200
if (count > 1) {
198-
String shortMethodName = methodName.substring(methodName.indexOf(hashDelim)+1);
201+
String shortMethodName = ConfigurationMethod.getShortMethodName(fqMethodName);
199202
problemReporter.error(new BeanMethodOverloadingProblem(shortMethodName, count));
200203
}
201204
}
202205

203-
// A configuration class may not be final (CGLIB limitation)
204-
if (getMetadata().isAnnotated(Configuration.class.getName())) {
205-
if (getMetadata().isFinal()) {
206-
problemReporter.error(new FinalConfigurationProblem());
207-
}
208-
}
209-
210206
for (BeanMethod beanMethod : this.beanMethods) {
211207
beanMethod.validate(problemReporter);
212208
}

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

Lines changed: 62 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -90,40 +90,37 @@ class ConfigurationClassParser {
9090

9191
private static final Comparator<DeferredImportSelectorHolder> DEFERRED_IMPORT_COMPARATOR =
9292
new Comparator<ConfigurationClassParser.DeferredImportSelectorHolder>() {
93-
@Override
94-
public int compare(DeferredImportSelectorHolder o1,
95-
DeferredImportSelectorHolder o2) {
96-
return AnnotationAwareOrderComparator.INSTANCE.compare(
97-
o1.getImportSelector(), o2.getImportSelector());
98-
}
99-
};
93+
@Override
94+
public int compare(DeferredImportSelectorHolder o1, DeferredImportSelectorHolder o2) {
95+
return AnnotationAwareOrderComparator.INSTANCE.compare(o1.getImportSelector(), o2.getImportSelector());
96+
}
97+
};
98+
10099

101100
private final MetadataReaderFactory metadataReaderFactory;
102101

103102
private final ProblemReporter problemReporter;
104103

105-
private final ImportStack importStack = new ImportStack();
106-
107-
private final Set<String> knownSuperclasses = new LinkedHashSet<String>();
108-
109-
private final Set<ConfigurationClass> configurationClasses =
110-
new LinkedHashSet<ConfigurationClass>();
111-
112-
private final Stack<PropertySource<?>> propertySources =
113-
new Stack<PropertySource<?>>();
114-
115104
private final Environment environment;
116105

117106
private final ResourceLoader resourceLoader;
118107

119108
private final BeanDefinitionRegistry registry;
120109

110+
private final BeanNameGenerator beanNameGenerator;
111+
121112
private final ComponentScanAnnotationParser componentScanParser;
122113

123-
private final BeanNameGenerator beanNameGenerator;
114+
private final Set<ConfigurationClass> configurationClasses = new LinkedHashSet<ConfigurationClass>();
115+
116+
private final Map<String, ConfigurationClass> knownSuperclasses = new HashMap<String, ConfigurationClass>();
117+
118+
private final Stack<PropertySource<?>> propertySources = new Stack<PropertySource<?>>();
119+
120+
private final ImportStack importStack = new ImportStack();
121+
122+
private final List<DeferredImportSelectorHolder> deferredImportSelectors = new LinkedList<DeferredImportSelectorHolder>();
124123

125-
private final List<DeferredImportSelectorHolder> deferredImportSelectors =
126-
new LinkedList<DeferredImportSelectorHolder>();
127124

128125
/**
129126
* Create a new {@link ConfigurationClassParser} instance that will be used
@@ -143,6 +140,7 @@ public ConfigurationClassParser(MetadataReaderFactory metadataReaderFactory,
143140
resourceLoader, environment, componentScanBeanNameGenerator, registry);
144141
}
145142

143+
146144
public void parse(Set<BeanDefinitionHolder> configCandidates) {
147145
for (BeanDefinitionHolder holder : configCandidates) {
148146
BeanDefinition bd = holder.getBeanDefinition();
@@ -177,35 +175,39 @@ public void parse(String className, String beanName) throws IOException {
177175
* @param clazz the Class to parse
178176
* @param beanName must not be null (as of Spring 3.1.1)
179177
*/
180-
protected void parse(Class<?> clazz, String beanName) throws IOException {
178+
public void parse(Class<?> clazz, String beanName) throws IOException {
181179
processConfigurationClass(new ConfigurationClass(clazz, beanName));
182180
}
183181

184-
protected void processConfigurationClass(ConfigurationClass configClass) throws IOException {
185-
AnnotationMetadata metadata = configClass.getMetadata();
186182

187-
if (ConditionalAnnotationHelper.shouldSkip(configClass, this.registry,
188-
this.environment, this.beanNameGenerator)) {
183+
protected void processConfigurationClass(ConfigurationClass configClass) throws IOException {
184+
if (ConditionalAnnotationHelper.shouldSkip(configClass, this.registry, this.environment, this.beanNameGenerator)) {
189185
return;
190186
}
191187

192-
// recursively process the configuration class and its superclass hierarchy
193-
do {
194-
metadata = doProcessConfigurationClass(configClass, metadata);
195-
}
196-
while (metadata != null);
197-
198188
if (this.configurationClasses.contains(configClass) && configClass.getBeanName() != null) {
199189
// Explicit bean definition found, probably replacing an import.
200190
// Let's remove the old one and go with the new one.
201191
this.configurationClasses.remove(configClass);
192+
for (Iterator<ConfigurationClass> it = this.knownSuperclasses.values().iterator(); it.hasNext();) {
193+
if (configClass.equals(it.next())) {
194+
it.remove();
195+
}
196+
}
202197
}
203198

199+
// Recursively process the configuration class and its superclass hierarchy.
200+
AnnotationMetadata metadata = configClass.getMetadata();
201+
do {
202+
metadata = doProcessConfigurationClass(configClass, metadata);
203+
}
204+
while (metadata != null);
205+
204206
this.configurationClasses.add(configClass);
205207
}
206208

207209
/**
208-
* @return annotation metadata of superclass, null if none found or previously processed
210+
* @return annotation metadata of superclass, {@code null} if none found or previously processed
209211
*/
210212
protected AnnotationMetadata doProcessConfigurationClass(
211213
ConfigurationClass configClass, AnnotationMetadata metadata) throws IOException {
@@ -219,7 +221,7 @@ protected AnnotationMetadata doProcessConfigurationClass(
219221
processPropertySource(propertySource);
220222
}
221223

222-
// process any @ComponentScan annotions
224+
// process any @ComponentScan annotations
223225
AnnotationAttributes componentScan = attributesFor(metadata, ComponentScan.class);
224226
if (componentScan != null) {
225227
// the config class is annotated with @ComponentScan -> perform the scan immediately
@@ -235,7 +237,6 @@ protected AnnotationMetadata doProcessConfigurationClass(
235237
}
236238

237239
// process any @Import annotations
238-
239240
Set<Object> imports = new LinkedHashSet<Object>();
240241
Set<Object> visited = new LinkedHashSet<Object>();
241242
collectImports(metadata, imports, visited);
@@ -262,7 +263,8 @@ protected AnnotationMetadata doProcessConfigurationClass(
262263
// process superclass, if any
263264
if (metadata.hasSuperClass()) {
264265
String superclass = metadata.getSuperClassName();
265-
if (this.knownSuperclasses.add(superclass)) {
266+
if (!this.knownSuperclasses.containsKey(superclass)) {
267+
this.knownSuperclasses.put(superclass, configClass);
266268
// superclass found, return its annotation metadata and recurse
267269
if (metadata instanceof StandardAnnotationMetadata) {
268270
Class<?> clazz = ((StandardAnnotationMetadata) metadata).getIntrospectedClass();
@@ -416,7 +418,7 @@ private void processDeferredImportSelectors() {
416418
throw new BeanDefinitionStoreException("Failed to load bean class: ", ex);
417419
}
418420
}
419-
deferredImportSelectors.clear();
421+
this.deferredImportSelectors.clear();
420422
}
421423

422424
private void processImport(ConfigurationClass configClass, Collection<?> classesToImport, boolean checkForCircularImports) throws IOException {
@@ -435,9 +437,9 @@ private void processImport(ConfigurationClass configClass, Collection<?> classes
435437
ImportSelector selector = BeanUtils.instantiateClass(candidateClass, ImportSelector.class);
436438
invokeAwareMethods(selector);
437439
if(selector instanceof DeferredImportSelector) {
438-
this.deferredImportSelectors.add(new DeferredImportSelectorHolder(
439-
configClass, (DeferredImportSelector) selector));
440-
} else {
440+
this.deferredImportSelectors.add(new DeferredImportSelectorHolder(configClass, (DeferredImportSelector) selector));
441+
}
442+
else {
441443
processImport(configClass, Arrays.asList(selector.selectImports(importingClassMetadata)), false);
442444
}
443445
}
@@ -516,7 +518,7 @@ public Stack<PropertySource<?>> getPropertySources() {
516518
return this.propertySources;
517519
}
518520

519-
public ImportRegistry getImportRegistry() {
521+
ImportRegistry getImportRegistry() {
520522
return this.importStack;
521523
}
522524

@@ -582,38 +584,39 @@ public String toString() {
582584
}
583585

584586

585-
/**
586-
* {@link Problem} registered upon detection of a circular {@link Import}.
587-
*/
588-
private static class CircularImportProblem extends Problem {
589-
590-
public CircularImportProblem(ConfigurationClass attemptedImport, Stack<ConfigurationClass> importStack, AnnotationMetadata metadata) {
591-
super(String.format("A circular @Import has been detected: " +
592-
"Illegal attempt by @Configuration class '%s' to import class '%s' as '%s' is " +
593-
"already present in the current import stack [%s]", importStack.peek().getSimpleName(),
594-
attemptedImport.getSimpleName(), attemptedImport.getSimpleName(), importStack),
595-
new Location(importStack.peek().getResource(), metadata));
596-
}
597-
}
598-
599-
600587
private static class DeferredImportSelectorHolder {
601588

602-
private ConfigurationClass configurationClass;
589+
private final ConfigurationClass configurationClass;
603590

604-
private DeferredImportSelector importSelector;
591+
private final DeferredImportSelector importSelector;
605592

606593
public DeferredImportSelectorHolder(ConfigurationClass configurationClass, DeferredImportSelector importSelector) {
607594
this.configurationClass = configurationClass;
608595
this.importSelector = importSelector;
609596
}
610597

611598
public ConfigurationClass getConfigurationClass() {
612-
return configurationClass;
599+
return this.configurationClass;
613600
}
614601

615602
public DeferredImportSelector getImportSelector() {
616-
return importSelector;
603+
return this.importSelector;
617604
}
618605
}
606+
607+
608+
/**
609+
* {@link Problem} registered upon detection of a circular {@link Import}.
610+
*/
611+
private static class CircularImportProblem extends Problem {
612+
613+
public CircularImportProblem(ConfigurationClass attemptedImport, Stack<ConfigurationClass> importStack, AnnotationMetadata metadata) {
614+
super(String.format("A circular @Import has been detected: " +
615+
"Illegal attempt by @Configuration class '%s' to import class '%s' as '%s' is " +
616+
"already present in the current import stack [%s]", importStack.peek().getSimpleName(),
617+
attemptedImport.getSimpleName(), attemptedImport.getSimpleName(), importStack),
618+
new Location(importStack.peek().getResource(), metadata));
619+
}
620+
}
621+
619622
}

0 commit comments

Comments
 (0)