From 13679bb9065ae047a6f312856713d81cbd7f647d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Mon, 11 Mar 2024 16:39:39 +0100 Subject: [PATCH] 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 --- .../annotation/ConditionEvaluator.java | 35 +++++--- .../annotation/ConfigurationClassParser.java | 31 +++++++ .../context/annotation/Gh23206Tests.java | 89 +++++++++++++++++++ 3 files changed, 144 insertions(+), 11 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/context/annotation/Gh23206Tests.java diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConditionEvaluator.java b/spring-context/src/main/java/org/springframework/context/annotation/ConditionEvaluator.java index c97d975e16..bccce9326f 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConditionEvaluator.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConditionEvaluator.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2023 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -90,16 +90,7 @@ class ConditionEvaluator { return shouldSkip(metadata, ConfigurationPhase.REGISTER_BEAN); } - List conditions = new ArrayList<>(); - for (String[] conditionClasses : getConditionClasses(metadata)) { - for (String conditionClass : conditionClasses) { - Condition condition = getCondition(conditionClass, this.context.getClassLoader()); - conditions.add(condition); - } - } - - AnnotationAwareOrderComparator.sort(conditions); - + List conditions = collectConditions(metadata); for (Condition condition : conditions) { ConfigurationPhase requiredPhase = null; if (condition instanceof ConfigurationCondition configurationCondition) { @@ -113,6 +104,28 @@ class ConditionEvaluator { return false; } + /** + * Return the {@linkplain Condition conditions} that should be applied when + * considering the given annotated type. + * @param metadata the metadata of the annotated type + * @return the ordered list of conditions for that type + */ + List collectConditions(@Nullable AnnotatedTypeMetadata metadata) { + if (metadata == null || !metadata.isAnnotated(Conditional.class.getName())) { + return Collections.emptyList(); + } + + List conditions = new ArrayList<>(); + for (String[] conditionClasses : getConditionClasses(metadata)) { + for (String conditionClass : conditionClasses) { + Condition condition = getCondition(conditionClass, this.context.getClassLoader()); + conditions.add(condition); + } + } + AnnotationAwareOrderComparator.sort(conditions); + return conditions; + } + @SuppressWarnings("unchecked") private List getConditionClasses(AnnotatedTypeMetadata metadata) { MultiValueMap attributes = metadata.getAllAnnotationAttributes(Conditional.class.getName(), true); diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 30ce91b1d8..e42aa2532a 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -49,6 +49,7 @@ import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.BeanDefinitionReader; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanNameGenerator; +import org.springframework.context.ApplicationContextException; import org.springframework.context.annotation.ConfigurationCondition.ConfigurationPhase; import org.springframework.context.annotation.DeferredImportSelector.Group; import org.springframework.core.OrderComparator; @@ -103,6 +104,10 @@ class ConfigurationClassParser { private static final Predicate DEFAULT_EXCLUSION_FILTER = className -> (className.startsWith("java.lang.annotation.") || className.startsWith("org.springframework.stereotype.")); + private static final Predicate REGISTER_BEAN_CONDITION_FILTER = condition -> + (condition instanceof ConfigurationCondition configurationCondition + && ConfigurationPhase.REGISTER_BEAN.equals(configurationCondition.getConfigurationPhase())); + private static final Comparator DEFERRED_IMPORT_COMPARATOR = (o1, o2) -> AnnotationAwareOrderComparator.INSTANCE.compare(o1.getImportSelector(), o2.getImportSelector()); @@ -315,6 +320,11 @@ class ConfigurationClassParser { } if (!componentScans.isEmpty()) { + List registerBeanConditions = collectRegisterBeanConditions(configClass); + if (!registerBeanConditions.isEmpty()) { + throw new ApplicationContextException( + "Component scan could not be used with conditions in REGISTER_BEAN phase: " + registerBeanConditions); + } for (AnnotationAttributes componentScan : componentScans) { // The config class is annotated with @ComponentScan -> perform the scan immediately Set scannedBeanDefinitions = @@ -680,6 +690,27 @@ class ConfigurationClassParser { return new SourceClass(this.metadataReaderFactory.getMetadataReader(className)); } + private List collectRegisterBeanConditions(ConfigurationClass configurationClass) { + AnnotationMetadata metadata = configurationClass.getMetadata(); + List allConditions = new ArrayList<>(this.conditionEvaluator.collectConditions(metadata)); + ConfigurationClass enclosingConfigurationClass = getEnclosingConfigurationClass(configurationClass); + if (enclosingConfigurationClass != null) { + allConditions.addAll(this.conditionEvaluator.collectConditions(enclosingConfigurationClass.getMetadata())); + } + return allConditions.stream().filter(REGISTER_BEAN_CONDITION_FILTER).toList(); + } + + @Nullable + private ConfigurationClass getEnclosingConfigurationClass(ConfigurationClass configurationClass) { + String enclosingClassName = configurationClass.getMetadata().getEnclosingClassName(); + if (enclosingClassName != null) { + return configurationClass.getImportedBy().stream() + .filter(candidate -> enclosingClassName.equals(candidate.getMetadata().getClassName())) + .findFirst().orElse(null); + } + return null; + } + @SuppressWarnings("serial") private class ImportStack extends ArrayDeque implements ImportRegistry { diff --git a/spring-context/src/test/java/org/springframework/context/annotation/Gh23206Tests.java b/spring-context/src/test/java/org/springframework/context/annotation/Gh23206Tests.java new file mode 100644 index 0000000000..08f4bbbf76 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/Gh23206Tests.java @@ -0,0 +1,89 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.context.annotation; + +import org.junit.jupiter.api.Test; + +import org.springframework.beans.factory.BeanDefinitionStoreException; +import org.springframework.context.ApplicationContextException; +import org.springframework.context.annotation.componentscan.simple.SimpleComponent; +import org.springframework.core.type.AnnotatedTypeMetadata; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + +/** + * Tests for gh-23206. + * + * @author Stephane Nicoll + */ +public class Gh23206Tests { + + @Test + void componentScanShouldFailWithRegisterBeanCondition() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.register(ConditionalComponentScanConfiguration.class); + assertThatExceptionOfType(BeanDefinitionStoreException.class).isThrownBy(context::refresh) + .withMessageContaining(ConditionalComponentScanConfiguration.class.getName()) + .havingCause().isInstanceOf(ApplicationContextException.class) + .withMessageContaining("Component scan could not be used with conditions in REGISTER_BEAN phase"); + } + + @Test + void componentScanShouldFailWithRegisterBeanConditionOnClasThatImportedIt() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.register(ConditionalConfiguration.class); + assertThatExceptionOfType(BeanDefinitionStoreException.class).isThrownBy(context::refresh) + .withMessageContaining(ConditionalConfiguration.class.getName()) + .havingCause().isInstanceOf(ApplicationContextException.class) + .withMessageContaining("Component scan could not be used with conditions in REGISTER_BEAN phase"); + } + + + @Configuration(proxyBeanMethods = false) + @Conditional(NeverRegisterBeanCondition.class) + @ComponentScan(basePackageClasses = SimpleComponent.class) + static class ConditionalComponentScanConfiguration { + + } + + + @Configuration(proxyBeanMethods = false) + @Conditional(NeverRegisterBeanCondition.class) + static class ConditionalConfiguration { + + @Configuration(proxyBeanMethods = false) + @ComponentScan(basePackageClasses = SimpleComponent.class) + static class NestedConfiguration { + } + + } + + + static class NeverRegisterBeanCondition implements ConfigurationCondition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return false; + } + + @Override + public ConfigurationPhase getConfigurationPhase() { + return ConfigurationPhase.REGISTER_BEAN; + } + + } +}