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 c666f2c6899..6d687b342fe 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 @@ -74,6 +74,7 @@ import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.util.StringUtils; /** * Parses a {@link Configuration} class definition, populating a collection of @@ -81,10 +82,9 @@ import org.springframework.util.MultiValueMap; * any number of ConfigurationClass objects because one Configuration class may import * another using the {@link Import} annotation). * - *

This class helps separate the concern of parsing the structure of a Configuration - * class from the concern of registering BeanDefinition objects based on the content of - * that model (with the exception of {@code @ComponentScan} annotations which need to be - * registered immediately). + *

This class helps separate the concern of parsing the structure of a Configuration class + * from the concern of registering BeanDefinition objects based on the content of that model + * (except {@code @ComponentScan} annotations which need to be registered immediately). * *

This ASM-based implementation avoids reflection and eager class loading in order to * interoperate effectively with lazy class loading in a Spring ApplicationContext. @@ -127,7 +127,7 @@ class ConfigurationClassParser { private final Map configurationClasses = new LinkedHashMap<>(); - private final Map knownSuperclasses = new HashMap<>(); + private final MultiValueMap knownSuperclasses = new LinkedMultiValueMap<>(); private final ImportStack importStack = new ImportStack(); @@ -239,7 +239,7 @@ class ConfigurationClassParser { } else if (configClass.isScanned()) { String beanName = configClass.getBeanName(); - if (beanName != null) { + if (StringUtils.hasLength(beanName) && this.registry.containsBeanDefinition(beanName)) { this.registry.removeBeanDefinition(beanName); } // An implicitly scanned bean definition should not override an explicit import. @@ -249,7 +249,7 @@ class ConfigurationClassParser { // Explicit bean definition found, probably replacing an import. // Let's remove the old one and go with the new one. this.configurationClasses.remove(configClass); - this.knownSuperclasses.values().removeIf(configClass::equals); + removeKnownSuperclass(configClass.getMetadata().getClassName(), false); } } @@ -358,11 +358,13 @@ class ConfigurationClassParser { // Process superclass, if any if (sourceClass.getMetadata().hasSuperClass()) { String superclass = sourceClass.getMetadata().getSuperClassName(); - if (superclass != null && !superclass.startsWith("java") && - !this.knownSuperclasses.containsKey(superclass)) { - this.knownSuperclasses.put(superclass, configClass); - // Superclass found, return its annotation metadata and recurse - return sourceClass.getSuperClass(); + if (superclass != null && !superclass.startsWith("java")) { + boolean superclassKnown = this.knownSuperclasses.containsKey(superclass); + this.knownSuperclasses.add(superclass, configClass); + if (!superclassKnown) { + // Superclass found, return its annotation metadata and recurse + return sourceClass.getSuperClass(); + } } } @@ -460,6 +462,36 @@ class ConfigurationClassParser { return beanMethods; } + /** + * Remove known superclasses for the given removed class, potentially replacing + * the superclass exposure on a different config class with the same superclass. + */ + private void removeKnownSuperclass(String removedClass, boolean replace) { + Iterator>> it = this.knownSuperclasses.entrySet().iterator(); + while (it.hasNext()) { + Map.Entry> entry = it.next(); + if (entry.getValue().removeIf(configClass -> configClass.getMetadata().getClassName().equals(removedClass))) { + if (entry.getValue().isEmpty()) { + it.remove(); + } + else if (replace) { + try { + ConfigurationClass otherClass = entry.getValue().get(0); + SourceClass sourceClass = asSourceClass(otherClass, DEFAULT_EXCLUSION_FILTER).getSuperClass(); + while (!sourceClass.getMetadata().getClassName().equals(entry.getKey()) && + sourceClass.getMetadata().getSuperClassName() != null) { + sourceClass = sourceClass.getSuperClass(); + } + doProcessConfigurationClass(otherClass, sourceClass, DEFAULT_EXCLUSION_FILTER); + } + catch (IOException ex) { + throw new BeanDefinitionStoreException( + "I/O failure while removing configuration class [" + removedClass + "]", ex); + } + } + } + } + } /** * Returns {@code @Import} class, considering all meta-annotations. @@ -499,8 +531,7 @@ class ConfigurationClassParser { } private void processImports(ConfigurationClass configClass, SourceClass currentSourceClass, - Collection importCandidates, Predicate exclusionFilter, - boolean checkForCircularImports) { + Collection importCandidates, Predicate filter, boolean checkForCircularImports) { if (importCandidates.isEmpty()) { return; @@ -520,15 +551,15 @@ class ConfigurationClassParser { this.environment, this.resourceLoader, this.registry); Predicate selectorFilter = selector.getExclusionFilter(); if (selectorFilter != null) { - exclusionFilter = exclusionFilter.or(selectorFilter); + filter = filter.or(selectorFilter); } if (selector instanceof DeferredImportSelector deferredImportSelector) { this.deferredImportSelectorHandler.handle(configClass, deferredImportSelector); } else { String[] importClassNames = selector.selectImports(currentSourceClass.getMetadata()); - Collection importSourceClasses = asSourceClasses(importClassNames, exclusionFilter); - processImports(configClass, currentSourceClass, importSourceClasses, exclusionFilter, false); + Collection importSourceClasses = asSourceClasses(importClassNames, filter); + processImports(configClass, currentSourceClass, importSourceClasses, filter, false); } } else if (candidate.isAssignable(ImportBeanDefinitionRegistrar.class)) { @@ -545,7 +576,7 @@ class ConfigurationClassParser { // process it as an @Configuration class this.importStack.registerImport( currentSourceClass.getMetadata(), candidate.getMetadata().getClassName()); - processConfigurationClass(candidate.asConfigClass(configClass), exclusionFilter); + processConfigurationClass(candidate.asConfigClass(configClass), filter); } } } @@ -641,7 +672,7 @@ class ConfigurationClassParser { @SuppressWarnings("serial") - private static class ImportStack extends ArrayDeque implements ImportRegistry { + private class ImportStack extends ArrayDeque implements ImportRegistry { private final MultiValueMap imports = new LinkedMultiValueMap<>(); @@ -665,6 +696,7 @@ class ConfigurationClassParser { } } } + removeKnownSuperclass(importingClass, true); } /** @@ -748,13 +780,13 @@ class ConfigurationClassParser { void processGroupImports() { for (DeferredImportSelectorGrouping grouping : this.groupings.values()) { - Predicate exclusionFilter = grouping.getCandidateFilter(); + Predicate filter = grouping.getCandidateFilter(); grouping.getImports().forEach(entry -> { ConfigurationClass configurationClass = this.configurationClasses.get(entry.getMetadata()); try { - processImports(configurationClass, asSourceClass(configurationClass, exclusionFilter), - Collections.singleton(asSourceClass(entry.getImportClassName(), exclusionFilter)), - exclusionFilter, false); + processImports(configurationClass, asSourceClass(configurationClass, filter), + Collections.singleton(asSourceClass(entry.getImportClassName(), filter)), + filter, false); } catch (BeanDefinitionStoreException ex) { throw ex; diff --git a/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationPhasesKnownSuperclassesTests.java b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationPhasesKnownSuperclassesTests.java new file mode 100644 index 00000000000..0c33738f530 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/configuration/ConfigurationPhasesKnownSuperclassesTests.java @@ -0,0 +1,129 @@ +/* + * 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.configuration; + +import org.junit.jupiter.api.Test; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.ConditionContext; +import org.springframework.context.annotation.Conditional; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.ConfigurationCondition; +import org.springframework.context.annotation.Import; +import org.springframework.core.type.AnnotatedTypeMetadata; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Andy Wilkinson + * @since 6.2 + */ +class ConfigurationPhasesKnownSuperclassesTests { + + @Test + void superclassSkippedInParseConfigurationPhaseShouldNotPreventSubsequentProcessingOfSameSuperclass() { + try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(ParseConfigurationPhase.class)) { + assertThat(context.getBean("subclassBean")).isEqualTo("bravo"); + assertThat(context.getBean("superclassBean")).isEqualTo("superclass"); + } + } + + @Test + void superclassSkippedInRegisterBeanPhaseShouldNotPreventSubsequentProcessingOfSameSuperclass() { + try (AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(RegisterBeanPhase.class)) { + assertThat(context.getBean("subclassBean")).isEqualTo("bravo"); + assertThat(context.getBean("superclassBean")).isEqualTo("superclass"); + } + } + + + @Configuration(proxyBeanMethods = false) + static class Example { + + @Bean + String superclassBean() { + return "superclass"; + } + } + + @Configuration(proxyBeanMethods = false) + @Import({RegisterBeanPhaseExample.class, BravoExample.class}) + static class RegisterBeanPhase { + } + + @Conditional(NonMatchingRegisterBeanPhaseCondition.class) + @Configuration(proxyBeanMethods = false) + static class RegisterBeanPhaseExample extends Example { + + @Bean + String subclassBean() { + return "alpha"; + } + } + + @Configuration(proxyBeanMethods = false) + @Import({ParseConfigurationPhaseExample.class, BravoExample.class}) + static class ParseConfigurationPhase { + } + + @Conditional(NonMatchingParseConfigurationPhaseCondition.class) + @Configuration(proxyBeanMethods = false) + static class ParseConfigurationPhaseExample extends Example { + + @Bean + String subclassBean() { + return "alpha"; + } + } + + @Configuration(proxyBeanMethods = false) + static class BravoExample extends Example { + + @Bean + String subclassBean() { + return "bravo"; + } + } + + static class NonMatchingRegisterBeanPhaseCondition implements ConfigurationCondition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return false; + } + + @Override + public ConfigurationPhase getConfigurationPhase() { + return ConfigurationPhase.REGISTER_BEAN; + } + } + + static class NonMatchingParseConfigurationPhaseCondition implements ConfigurationCondition { + + @Override + public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { + return false; + } + + @Override + public ConfigurationPhase getConfigurationPhase() { + return ConfigurationPhase.PARSE_CONFIGURATION; + } + } + +}