Skip to content

Commit 13679bb

Browse files
snicollsnicoll
committed
Reject use of component scan with REGISTER_BEAN condition
This commit introduce a change of behaviour when component scan is used with conditions. Previously, any condition in the REGISTER_BEAN phase were ignored and the scan was applied regardless of the outcome of those conditions. This is because REGISTER_BEAN condition evaluation happens later in the bean factory preparation. Rather than ignoring those conditions, this commit fails fast when it detects such use case. Code will have to be adapted accordingly. Closes gh-23206
1 parent 6fc1f72 commit 13679bb

File tree

3 files changed

+144
-11
lines changed

3 files changed

+144
-11
lines changed

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

+24-11
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.
@@ -90,16 +90,7 @@ public boolean shouldSkip(@Nullable AnnotatedTypeMetadata metadata, @Nullable Co
9090
return shouldSkip(metadata, ConfigurationPhase.REGISTER_BEAN);
9191
}
9292

93-
List<Condition> conditions = new ArrayList<>();
94-
for (String[] conditionClasses : getConditionClasses(metadata)) {
95-
for (String conditionClass : conditionClasses) {
96-
Condition condition = getCondition(conditionClass, this.context.getClassLoader());
97-
conditions.add(condition);
98-
}
99-
}
100-
101-
AnnotationAwareOrderComparator.sort(conditions);
102-
93+
List<Condition> conditions = collectConditions(metadata);
10394
for (Condition condition : conditions) {
10495
ConfigurationPhase requiredPhase = null;
10596
if (condition instanceof ConfigurationCondition configurationCondition) {
@@ -113,6 +104,28 @@ public boolean shouldSkip(@Nullable AnnotatedTypeMetadata metadata, @Nullable Co
113104
return false;
114105
}
115106

107+
/**
108+
* Return the {@linkplain Condition conditions} that should be applied when
109+
* considering the given annotated type.
110+
* @param metadata the metadata of the annotated type
111+
* @return the ordered list of conditions for that type
112+
*/
113+
List<Condition> collectConditions(@Nullable AnnotatedTypeMetadata metadata) {
114+
if (metadata == null || !metadata.isAnnotated(Conditional.class.getName())) {
115+
return Collections.emptyList();
116+
}
117+
118+
List<Condition> conditions = new ArrayList<>();
119+
for (String[] conditionClasses : getConditionClasses(metadata)) {
120+
for (String conditionClass : conditionClasses) {
121+
Condition condition = getCondition(conditionClass, this.context.getClassLoader());
122+
conditions.add(condition);
123+
}
124+
}
125+
AnnotationAwareOrderComparator.sort(conditions);
126+
return conditions;
127+
}
128+
116129
@SuppressWarnings("unchecked")
117130
private List<String[]> getConditionClasses(AnnotatedTypeMetadata metadata) {
118131
MultiValueMap<String, Object> attributes = metadata.getAllAnnotationAttributes(Conditional.class.getName(), true);

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

+31
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.springframework.beans.factory.support.BeanDefinitionReader;
5050
import org.springframework.beans.factory.support.BeanDefinitionRegistry;
5151
import org.springframework.beans.factory.support.BeanNameGenerator;
52+
import org.springframework.context.ApplicationContextException;
5253
import org.springframework.context.annotation.ConfigurationCondition.ConfigurationPhase;
5354
import org.springframework.context.annotation.DeferredImportSelector.Group;
5455
import org.springframework.core.OrderComparator;
@@ -103,6 +104,10 @@ class ConfigurationClassParser {
103104
private static final Predicate<String> DEFAULT_EXCLUSION_FILTER = className ->
104105
(className.startsWith("java.lang.annotation.") || className.startsWith("org.springframework.stereotype."));
105106

107+
private static final Predicate<Condition> REGISTER_BEAN_CONDITION_FILTER = condition ->
108+
(condition instanceof ConfigurationCondition configurationCondition
109+
&& ConfigurationPhase.REGISTER_BEAN.equals(configurationCondition.getConfigurationPhase()));
110+
106111
private static final Comparator<DeferredImportSelectorHolder> DEFERRED_IMPORT_COMPARATOR =
107112
(o1, o2) -> AnnotationAwareOrderComparator.INSTANCE.compare(o1.getImportSelector(), o2.getImportSelector());
108113

@@ -315,6 +320,11 @@ protected final SourceClass doProcessConfigurationClass(
315320
}
316321

317322
if (!componentScans.isEmpty()) {
323+
List<Condition> registerBeanConditions = collectRegisterBeanConditions(configClass);
324+
if (!registerBeanConditions.isEmpty()) {
325+
throw new ApplicationContextException(
326+
"Component scan could not be used with conditions in REGISTER_BEAN phase: " + registerBeanConditions);
327+
}
318328
for (AnnotationAttributes componentScan : componentScans) {
319329
// The config class is annotated with @ComponentScan -> perform the scan immediately
320330
Set<BeanDefinitionHolder> scannedBeanDefinitions =
@@ -680,6 +690,27 @@ SourceClass asSourceClass(@Nullable String className, Predicate<String> filter)
680690
return new SourceClass(this.metadataReaderFactory.getMetadataReader(className));
681691
}
682692

693+
private List<Condition> collectRegisterBeanConditions(ConfigurationClass configurationClass) {
694+
AnnotationMetadata metadata = configurationClass.getMetadata();
695+
List<Condition> allConditions = new ArrayList<>(this.conditionEvaluator.collectConditions(metadata));
696+
ConfigurationClass enclosingConfigurationClass = getEnclosingConfigurationClass(configurationClass);
697+
if (enclosingConfigurationClass != null) {
698+
allConditions.addAll(this.conditionEvaluator.collectConditions(enclosingConfigurationClass.getMetadata()));
699+
}
700+
return allConditions.stream().filter(REGISTER_BEAN_CONDITION_FILTER).toList();
701+
}
702+
703+
@Nullable
704+
private ConfigurationClass getEnclosingConfigurationClass(ConfigurationClass configurationClass) {
705+
String enclosingClassName = configurationClass.getMetadata().getEnclosingClassName();
706+
if (enclosingClassName != null) {
707+
return configurationClass.getImportedBy().stream()
708+
.filter(candidate -> enclosingClassName.equals(candidate.getMetadata().getClassName()))
709+
.findFirst().orElse(null);
710+
}
711+
return null;
712+
}
713+
683714

684715
@SuppressWarnings("serial")
685716
private class ImportStack extends ArrayDeque<ConfigurationClass> implements ImportRegistry {

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

+89
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.context.annotation;
18+
19+
import org.junit.jupiter.api.Test;
20+
21+
import org.springframework.beans.factory.BeanDefinitionStoreException;
22+
import org.springframework.context.ApplicationContextException;
23+
import org.springframework.context.annotation.componentscan.simple.SimpleComponent;
24+
import org.springframework.core.type.AnnotatedTypeMetadata;
25+
26+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
27+
28+
/**
29+
* Tests for gh-23206.
30+
*
31+
* @author Stephane Nicoll
32+
*/
33+
public class Gh23206Tests {
34+
35+
@Test
36+
void componentScanShouldFailWithRegisterBeanCondition() {
37+
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
38+
context.register(ConditionalComponentScanConfiguration.class);
39+
assertThatExceptionOfType(BeanDefinitionStoreException.class).isThrownBy(context::refresh)
40+
.withMessageContaining(ConditionalComponentScanConfiguration.class.getName())
41+
.havingCause().isInstanceOf(ApplicationContextException.class)
42+
.withMessageContaining("Component scan could not be used with conditions in REGISTER_BEAN phase");
43+
}
44+
45+
@Test
46+
void componentScanShouldFailWithRegisterBeanConditionOnClasThatImportedIt() {
47+
AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
48+
context.register(ConditionalConfiguration.class);
49+
assertThatExceptionOfType(BeanDefinitionStoreException.class).isThrownBy(context::refresh)
50+
.withMessageContaining(ConditionalConfiguration.class.getName())
51+
.havingCause().isInstanceOf(ApplicationContextException.class)
52+
.withMessageContaining("Component scan could not be used with conditions in REGISTER_BEAN phase");
53+
}
54+
55+
56+
@Configuration(proxyBeanMethods = false)
57+
@Conditional(NeverRegisterBeanCondition.class)
58+
@ComponentScan(basePackageClasses = SimpleComponent.class)
59+
static class ConditionalComponentScanConfiguration {
60+
61+
}
62+
63+
64+
@Configuration(proxyBeanMethods = false)
65+
@Conditional(NeverRegisterBeanCondition.class)
66+
static class ConditionalConfiguration {
67+
68+
@Configuration(proxyBeanMethods = false)
69+
@ComponentScan(basePackageClasses = SimpleComponent.class)
70+
static class NestedConfiguration {
71+
}
72+
73+
}
74+
75+
76+
static class NeverRegisterBeanCondition implements ConfigurationCondition {
77+
78+
@Override
79+
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) {
80+
return false;
81+
}
82+
83+
@Override
84+
public ConfigurationPhase getConfigurationPhase() {
85+
return ConfigurationPhase.REGISTER_BEAN;
86+
}
87+
88+
}
89+
}

0 commit comments

Comments
 (0)