Skip to content

Commit ced5ea2

Browse files
author
Phillip Webb
committed
Revert "Ensure @conditions consider super classes"
This reverts commit 620c16f.
1 parent 402a3b8 commit ced5ea2

File tree

7 files changed

+30
-128
lines changed

7 files changed

+30
-128
lines changed

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

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ public void registerBean(Class<?> annotatedClass, Class<? extends Annotation>...
134134
}
135135

136136
public void registerBean(Class<?> annotatedClass, String name, Class<? extends Annotation>... qualifiers) {
137-
if (shouldSkip(annotatedClass)) {
137+
AnnotatedGenericBeanDefinition abd = new AnnotatedGenericBeanDefinition(annotatedClass);
138+
if (conditionEvaluator.shouldSkip(abd.getMetadata())) {
138139
return;
139140
}
140-
AnnotatedGenericBeanDefinition abd = new AnnotatedGenericBeanDefinition(annotatedClass);
141141
ScopeMetadata scopeMetadata = this.scopeMetadataResolver.resolveScopeMetadata(abd);
142142
abd.setScope(scopeMetadata.getScopeName());
143143
String beanName = (name != null ? name : this.beanNameGenerator.generateBeanName(abd, this.registry));
@@ -161,17 +161,6 @@ else if (Lazy.class.equals(qualifier)) {
161161
}
162162

163163

164-
private boolean shouldSkip(Class<?> annotatedClass) {
165-
while(annotatedClass != null) {
166-
AnnotatedGenericBeanDefinition abd = new AnnotatedGenericBeanDefinition(annotatedClass);
167-
if(conditionEvaluator.shouldSkip(abd.getMetadata())) {
168-
return true;
169-
}
170-
annotatedClass = annotatedClass.getSuperclass();
171-
}
172-
return false;
173-
}
174-
175164
/**
176165
* Get the Environment from the given registry if possible, otherwise return a new
177166
* StandardEnvironment.

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

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import org.springframework.core.io.support.PathMatchingResourcePatternResolver;
3939
import org.springframework.core.io.support.ResourcePatternResolver;
4040
import org.springframework.core.io.support.ResourcePatternUtils;
41-
import org.springframework.core.type.AnnotationMetadata;
4241
import org.springframework.core.type.classreading.CachingMetadataReaderFactory;
4342
import org.springframework.core.type.classreading.MetadataReader;
4443
import org.springframework.core.type.classreading.MetadataReaderFactory;
@@ -342,29 +341,24 @@ protected boolean isCandidateComponent(MetadataReader metadataReader) throws IOE
342341
}
343342
for (TypeFilter tf : this.includeFilters) {
344343
if (tf.match(metadataReader, this.metadataReaderFactory)) {
345-
return !shouldSkip(metadataReader);
344+
return isConditionMatch(metadataReader);
346345
}
347346
}
348347
return false;
349348
}
350349

351-
private boolean shouldSkip(MetadataReader metadataReader) throws IOException {
350+
/**
351+
* Determine whether the given class is a candidate component based on any
352+
* {@code @Conditional} annotations.
353+
* @param metadataReader the ASM ClassReader for the class
354+
* @return whether the class qualifies as a candidate component
355+
*/
356+
private boolean isConditionMatch(MetadataReader metadataReader) {
352357
if (this.conditionEvaluator == null) {
353358
this.conditionEvaluator = new ConditionEvaluator(getRegistry(),
354359
getEnvironment(), null, null, getResourceLoader());
355360
}
356-
357-
while(metadataReader != null) {
358-
AnnotationMetadata metadata = metadataReader.getAnnotationMetadata();
359-
if(this.conditionEvaluator.shouldSkip(metadata)) {
360-
return true;
361-
}
362-
metadataReader = (metadata.hasSuperClass() ?
363-
this.metadataReaderFactory.getMetadataReader(metadata.getSuperClassName())
364-
: null);
365-
}
366-
367-
return false;
361+
return !conditionEvaluator.shouldSkip(metadataReader.getAnnotationMetadata());
368362
}
369363

370364
/**

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class ConditionEvaluator {
5050
*/
5151
public ConditionEvaluator(BeanDefinitionRegistry registry, Environment environment,
5252
ApplicationContext applicationContext, ClassLoader classLoader, ResourceLoader resourceLoader) {
53+
5354
this.context = new ConditionContextImpl(registry, environment, applicationContext, classLoader, resourceLoader);
5455
}
5556

@@ -72,6 +73,10 @@ public boolean shouldSkip(AnnotatedTypeMetadata metadata) {
7273
* @return if the item should be skipped
7374
*/
7475
public boolean shouldSkip(AnnotatedTypeMetadata metadata, ConfigurationPhase phase) {
76+
if (metadata == null || !metadata.isAnnotated(Conditional.class.getName())) {
77+
return false;
78+
}
79+
7580
if (phase == null) {
7681
if (metadata instanceof AnnotationMetadata &&
7782
ConfigurationClassUtils.isConfigurationCandidate((AnnotationMetadata) metadata)) {
@@ -94,7 +99,6 @@ public boolean shouldSkip(AnnotatedTypeMetadata metadata, ConfigurationPhase pha
9499
}
95100
}
96101
}
97-
98102
return false;
99103
}
100104

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,10 @@
1616

1717
package org.springframework.context.annotation;
1818

19-
import java.util.ArrayList;
2019
import java.util.Collections;
2120
import java.util.HashMap;
2221
import java.util.LinkedHashMap;
2322
import java.util.LinkedHashSet;
24-
import java.util.List;
2523
import java.util.Map;
2624
import java.util.Set;
2725

@@ -53,8 +51,6 @@ final class ConfigurationClass {
5351

5452
private final AnnotationMetadata metadata;
5553

56-
private final List<AnnotationMetadata> metadataHierarchy = new ArrayList<AnnotationMetadata>();
57-
5854
private final Resource resource;
5955

6056
private String beanName;
@@ -133,14 +129,6 @@ public AnnotationMetadata getMetadata() {
133129
return this.metadata;
134130
}
135131

136-
public List<AnnotationMetadata> getMetadataHierarchy() {
137-
return Collections.unmodifiableList(metadataHierarchy);
138-
}
139-
140-
public void addMetadataHierarchy(AnnotationMetadata metadata) {
141-
this.metadataHierarchy.add(metadata);
142-
}
143-
144132
public Resource getResource() {
145133
return this.resource;
146134
}

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

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -380,22 +380,13 @@ public boolean shouldSkip(ConfigurationClass configClass) {
380380
}
381381
}
382382
if (skip == null) {
383-
skip = shouldSkipConsideringHierarchy(configClass);
383+
skip = conditionEvaluator.shouldSkip(configClass.getMetadata(),
384+
ConfigurationPhase.REGISTER_BEAN);
384385
}
385386
this.skipped.put(configClass, skip);
386387
}
387388
return skip;
388389
}
389-
390-
private boolean shouldSkipConsideringHierarchy(ConfigurationClass configClass) {
391-
for (AnnotationMetadata metadata : configClass.getMetadataHierarchy()) {
392-
if (conditionEvaluator.shouldSkip(metadata,
393-
ConfigurationPhase.REGISTER_BEAN)) {
394-
return true;
395-
}
396-
}
397-
return false;
398-
}
399390
}
400391

401392
}

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

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ protected final void parse(Class<?> clazz, String beanName) throws IOException {
183183

184184

185185
protected void processConfigurationClass(ConfigurationClass configClass) throws IOException {
186-
if (shouldSkip(asSourceClass(configClass), ConfigurationPhase.PARSE_CONFIGURATION)) {
186+
if (this.conditionEvaluator.shouldSkip(configClass.getMetadata(), ConfigurationPhase.PARSE_CONFIGURATION)) {
187187
return;
188188
}
189189

@@ -201,7 +201,6 @@ protected void processConfigurationClass(ConfigurationClass configClass) throws
201201
// Recursively process the configuration class and its superclass hierarchy.
202202
SourceClass sourceClass = asSourceClass(configClass);
203203
do {
204-
configClass.addMetadataHierarchy(sourceClass.getMetadata());
205204
sourceClass = doProcessConfigurationClass(configClass, sourceClass);
206205
}
207206
while (sourceClass != null);
@@ -231,7 +230,7 @@ protected final SourceClass doProcessConfigurationClass(ConfigurationClass confi
231230
AnnotationAttributes componentScan = AnnotationConfigUtils.attributesFor(sourceClass.getMetadata(), ComponentScan.class);
232231
if (componentScan != null) {
233232
// the config class is annotated with @ComponentScan -> perform the scan immediately
234-
if (!shouldSkip(sourceClass, ConfigurationPhase.REGISTER_BEAN)) {
233+
if (!conditionEvaluator.shouldSkip(sourceClass.getMetadata(), ConfigurationPhase.REGISTER_BEAN)) {
235234
Set<BeanDefinitionHolder> scannedBeanDefinitions =
236235
this.componentScanParser.parse(componentScan, sourceClass.getMetadata().getClassName());
237236

@@ -270,25 +269,19 @@ protected final SourceClass doProcessConfigurationClass(ConfigurationClass confi
270269
if (!this.knownSuperclasses.containsKey(superclass)) {
271270
this.knownSuperclasses.put(superclass, configClass);
272271
// superclass found, return its annotation metadata and recurse
273-
return sourceClass.getSuperClass();
272+
try {
273+
return sourceClass.getSuperClass();
274+
}
275+
catch (ClassNotFoundException ex) {
276+
throw new IllegalStateException(ex);
277+
}
274278
}
275279
}
276280

277281
// no superclass, processing is complete
278282
return null;
279283
}
280284

281-
private boolean shouldSkip(SourceClass sourceClass, ConfigurationPhase phase)
282-
throws IOException {
283-
while (sourceClass != null) {
284-
if (conditionEvaluator.shouldSkip(sourceClass.getMetadata(), phase)) {
285-
return true;
286-
}
287-
sourceClass = sourceClass.getSuperClass();
288-
}
289-
return false;
290-
}
291-
292285
/**
293286
* Register member (nested) classes that happen to be configuration classes themselves.
294287
* @param sourceClass the source class to process
@@ -699,19 +692,11 @@ public Collection<SourceClass> getMemberClasses() throws IOException {
699692
return members;
700693
}
701694

702-
public SourceClass getSuperClass() throws IOException {
703-
if (!getMetadata().hasSuperClass()) {
704-
return null;
705-
}
706-
try {
707-
if (this.source instanceof Class<?>) {
708-
return asSourceClass(((Class<?>) this.source).getSuperclass());
709-
}
710-
return asSourceClass(((MetadataReader) this.source).getClassMetadata().getSuperClassName());
711-
}
712-
catch (ClassNotFoundException ex) {
713-
throw new IllegalStateException(ex);
695+
public SourceClass getSuperClass() throws IOException, ClassNotFoundException {
696+
if (this.source instanceof Class<?>) {
697+
return asSourceClass(((Class<?>) this.source).getSuperclass());
714698
}
699+
return asSourceClass(((MetadataReader) this.source).getClassMetadata().getSuperClassName());
715700
}
716701

717702
public Set<SourceClass> getAnnotations() throws IOException, ClassNotFoundException {

‎spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassWithConditionTests.java

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -54,21 +54,6 @@ public void closeContext() {
5454
ctx.close();
5555
}
5656

57-
@Test
58-
public void conditionalOnConfiguration() throws Exception {
59-
ctx.register(ConditionOnConfiguration.class);
60-
ctx.refresh();
61-
assertThat(ctx.getBeansOfType(ConditionOnConfiguration.class).size(), equalTo(0));
62-
}
63-
64-
@Test
65-
public void inheritedConditionalOnConfiguration() throws Exception {
66-
ctx.register(InheritedConditionOnConfiguration.class);
67-
ctx.refresh();
68-
assertThat(ctx.getBeansOfType(ConditionOnConfiguration.class).size(), equalTo(0));
69-
assertThat(ctx.getBeansOfType(InheritedConditionOnConfiguration.class).size(), equalTo(0));
70-
}
71-
7257
@Test
7358
public void conditionalOnMissingBeanMatch() throws Exception {
7459
ctx.register(BeanOneConfiguration.class, BeanTwoConfiguration.class);
@@ -118,14 +103,6 @@ public void nonConfigurationClass() throws Exception {
118103
assertNull(ctx.getBean(NonConfigurationClass.class));
119104
}
120105

121-
@Test
122-
public void inheritedNonConfigurationClass() throws Exception {
123-
ctx.register(InheritedNonConfigurationClass.class);
124-
ctx.refresh();
125-
thrown.expect(NoSuchBeanDefinitionException.class);
126-
assertNull(ctx.getBean(InheritedNonConfigurationClass.class));
127-
}
128-
129106
@Test
130107
public void methodConditional() throws Exception {
131108
ctx.register(ConditionOnMethodConfiguration.class);
@@ -141,13 +118,6 @@ public void importsNotCreated() throws Exception {
141118
ctx.refresh();
142119
}
143120

144-
@Test
145-
public void inheritedImportsNotCreated() throws Exception {
146-
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
147-
ctx.register(InheritedImportsNotCreated.class);
148-
ctx.refresh();
149-
}
150-
151121
@Test
152122
public void importsNotLoaded() throws Exception {
153123
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
@@ -258,10 +228,6 @@ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata)
258228
static class NonConfigurationClass {
259229
}
260230

261-
@Component
262-
static class InheritedNonConfigurationClass extends NonConfigurationClass {
263-
}
264-
265231
@Configuration
266232
static class ConditionOnMethodConfiguration {
267233

@@ -281,21 +247,6 @@ static class ImportsNotCreated {
281247
}
282248
}
283249

284-
@Configuration
285-
@Never
286-
static class ConditionOnConfiguration {
287-
}
288-
289-
@Configuration
290-
static class InheritedConditionOnConfiguration extends ConditionOnConfiguration {
291-
}
292-
293-
294-
@Import({ ConfigurationNotCreated.class, RegistrarNotCreated.class, ImportSelectorNotCreated.class })
295-
static class InheritedImportsNotCreated extends ConditionOnConfiguration {
296-
297-
}
298-
299250
@Configuration
300251
static class ConfigurationNotCreated {
301252
static {

0 commit comments

Comments
 (0)