Skip to content

Commit 620c16f

Browse files
author
Phillip Webb
committed
Ensure @conditions consider super classes
Fix @condition evaluation to also consider super classes for both @configuration classes and regular @components. This change allows @conditional annotations to be inherited and restores the previous behavior of @Profile. Issue: SPR-10840
1 parent 8f90eac commit 620c16f

File tree

7 files changed

+128
-30
lines changed

7 files changed

+128
-30
lines changed

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

Lines changed: 13 additions & 2 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-
AnnotatedGenericBeanDefinition abd = new AnnotatedGenericBeanDefinition(annotatedClass);
138-
if (conditionEvaluator.shouldSkip(abd.getMetadata())) {
137+
if (shouldSkip(annotatedClass)) {
139138
return;
140139
}
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,6 +161,17 @@ 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+
164175
/**
165176
* Get the Environment from the given registry if possible, otherwise return a new
166177
* StandardEnvironment.

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
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;
4142
import org.springframework.core.type.classreading.CachingMetadataReaderFactory;
4243
import org.springframework.core.type.classreading.MetadataReader;
4344
import org.springframework.core.type.classreading.MetadataReaderFactory;
@@ -341,24 +342,29 @@ protected boolean isCandidateComponent(MetadataReader metadataReader) throws IOE
341342
}
342343
for (TypeFilter tf : this.includeFilters) {
343344
if (tf.match(metadataReader, this.metadataReaderFactory)) {
344-
return isConditionMatch(metadataReader);
345+
return !shouldSkip(metadataReader);
345346
}
346347
}
347348
return false;
348349
}
349350

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) {
351+
private boolean shouldSkip(MetadataReader metadataReader) throws IOException {
357352
if (this.conditionEvaluator == null) {
358353
this.conditionEvaluator = new ConditionEvaluator(getRegistry(),
359354
getEnvironment(), null, null, getResourceLoader());
360355
}
361-
return !conditionEvaluator.shouldSkip(metadataReader.getAnnotationMetadata());
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;
362368
}
363369

364370
/**

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

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

@@ -73,10 +72,6 @@ public boolean shouldSkip(AnnotatedTypeMetadata metadata) {
7372
* @return if the item should be skipped
7473
*/
7574
public boolean shouldSkip(AnnotatedTypeMetadata metadata, ConfigurationPhase phase) {
76-
if (metadata == null || !metadata.isAnnotated(Conditional.class.getName())) {
77-
return false;
78-
}
79-
8075
if (phase == null) {
8176
if (metadata instanceof AnnotationMetadata &&
8277
ConfigurationClassUtils.isConfigurationCandidate((AnnotationMetadata) metadata)) {
@@ -99,6 +94,7 @@ public boolean shouldSkip(AnnotatedTypeMetadata metadata, ConfigurationPhase pha
9994
}
10095
}
10196
}
97+
10298
return false;
10399
}
104100

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

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

1717
package org.springframework.context.annotation;
1818

19+
import java.util.ArrayList;
1920
import java.util.Collections;
2021
import java.util.HashMap;
2122
import java.util.LinkedHashMap;
2223
import java.util.LinkedHashSet;
24+
import java.util.List;
2325
import java.util.Map;
2426
import java.util.Set;
2527

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

5254
private final AnnotationMetadata metadata;
5355

56+
private final List<AnnotationMetadata> metadataHierarchy = new ArrayList<AnnotationMetadata>();
57+
5458
private final Resource resource;
5559

5660
private String beanName;
@@ -129,6 +133,14 @@ public AnnotationMetadata getMetadata() {
129133
return this.metadata;
130134
}
131135

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+
132144
public Resource getResource() {
133145
return this.resource;
134146
}

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,13 +380,22 @@ public boolean shouldSkip(ConfigurationClass configClass) {
380380
}
381381
}
382382
if (skip == null) {
383-
skip = conditionEvaluator.shouldSkip(configClass.getMetadata(),
384-
ConfigurationPhase.REGISTER_BEAN);
383+
skip = shouldSkipConsideringHierarchy(configClass);
385384
}
386385
this.skipped.put(configClass, skip);
387386
}
388387
return skip;
389388
}
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+
}
390399
}
391400

392401
}

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

Lines changed: 27 additions & 12 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 (this.conditionEvaluator.shouldSkip(configClass.getMetadata(), ConfigurationPhase.PARSE_CONFIGURATION)) {
186+
if (shouldSkip(asSourceClass(configClass), ConfigurationPhase.PARSE_CONFIGURATION)) {
187187
return;
188188
}
189189

@@ -201,6 +201,7 @@ 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());
204205
sourceClass = doProcessConfigurationClass(configClass, sourceClass);
205206
}
206207
while (sourceClass != null);
@@ -230,7 +231,7 @@ protected final SourceClass doProcessConfigurationClass(ConfigurationClass confi
230231
AnnotationAttributes componentScan = AnnotationConfigUtils.attributesFor(sourceClass.getMetadata(), ComponentScan.class);
231232
if (componentScan != null) {
232233
// the config class is annotated with @ComponentScan -> perform the scan immediately
233-
if (!conditionEvaluator.shouldSkip(sourceClass.getMetadata(), ConfigurationPhase.REGISTER_BEAN)) {
234+
if (!shouldSkip(sourceClass, ConfigurationPhase.REGISTER_BEAN)) {
234235
Set<BeanDefinitionHolder> scannedBeanDefinitions =
235236
this.componentScanParser.parse(componentScan, sourceClass.getMetadata().getClassName());
236237

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

281277
// no superclass, processing is complete
282278
return null;
283279
}
284280

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+
285292
/**
286293
* Register member (nested) classes that happen to be configuration classes themselves.
287294
* @param sourceClass the source class to process
@@ -692,11 +699,19 @@ public Collection<SourceClass> getMemberClasses() throws IOException {
692699
return members;
693700
}
694701

695-
public SourceClass getSuperClass() throws IOException, ClassNotFoundException {
696-
if (this.source instanceof Class<?>) {
697-
return asSourceClass(((Class<?>) this.source).getSuperclass());
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);
698714
}
699-
return asSourceClass(((MetadataReader) this.source).getClassMetadata().getSuperClassName());
700715
}
701716

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

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,21 @@ 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+
5772
@Test
5873
public void conditionalOnMissingBeanMatch() throws Exception {
5974
ctx.register(BeanOneConfiguration.class, BeanTwoConfiguration.class);
@@ -103,6 +118,14 @@ public void nonConfigurationClass() throws Exception {
103118
assertNull(ctx.getBean(NonConfigurationClass.class));
104119
}
105120

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+
106129
@Test
107130
public void methodConditional() throws Exception {
108131
ctx.register(ConditionOnMethodConfiguration.class);
@@ -118,6 +141,13 @@ public void importsNotCreated() throws Exception {
118141
ctx.refresh();
119142
}
120143

144+
@Test
145+
public void inheritedImportsNotCreated() throws Exception {
146+
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
147+
ctx.register(InheritedImportsNotCreated.class);
148+
ctx.refresh();
149+
}
150+
121151
@Test
122152
public void importsNotLoaded() throws Exception {
123153
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
@@ -228,6 +258,10 @@ public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata)
228258
static class NonConfigurationClass {
229259
}
230260

261+
@Component
262+
static class InheritedNonConfigurationClass extends NonConfigurationClass {
263+
}
264+
231265
@Configuration
232266
static class ConditionOnMethodConfiguration {
233267

@@ -247,6 +281,21 @@ static class ImportsNotCreated {
247281
}
248282
}
249283

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+
250299
@Configuration
251300
static class ConfigurationNotCreated {
252301
static {

0 commit comments

Comments
 (0)