From abc343f4076d45172587197480c41f79c23f3aac Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 2 Sep 2014 20:33:46 -0700 Subject: [PATCH] Prevent incorrect ImportAware metdata injection Update ImportRegistry to track all import registrations that occur against an importing class (rather than just keeping the last). In addition, prune imported classes from the registry when a configuration class is removed during the REGISTER_BEAN ConfigurationPhase. This update prevents incorrect metadata from being injected into an ImportAware class which is imported twice by different configurations classes (when one of the configuration classes will be ultimately skipped due to a @Condition). Issue: SPR-12128 --- ...onfigurationClassBeanDefinitionReader.java | 8 +++++- .../annotation/ConfigurationClassParser.java | 28 ++++++++++++++----- .../ConfigurationClassPostProcessor.java | 2 +- .../context/annotation/ImportAwareTests.java | 7 +++-- 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java index 903431ad80..466b2fdf5c 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java @@ -44,6 +44,7 @@ import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; +import org.springframework.context.annotation.ConfigurationClassParser.ImportRegistry; import org.springframework.context.annotation.ConfigurationCondition.ConfigurationPhase; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.env.Environment; @@ -86,6 +87,8 @@ class ConfigurationClassBeanDefinitionReader { private final BeanNameGenerator importBeanNameGenerator; + private final ImportRegistry importRegistry; + private final ConditionEvaluator conditionEvaluator; @@ -95,7 +98,8 @@ class ConfigurationClassBeanDefinitionReader { */ public ConfigurationClassBeanDefinitionReader(BeanDefinitionRegistry registry, SourceExtractor sourceExtractor, ProblemReporter problemReporter, MetadataReaderFactory metadataReaderFactory, - ResourceLoader resourceLoader, Environment environment, BeanNameGenerator importBeanNameGenerator) { + ResourceLoader resourceLoader, Environment environment, BeanNameGenerator importBeanNameGenerator, + ImportRegistry importRegistry) { this.registry = registry; this.sourceExtractor = sourceExtractor; @@ -104,6 +108,7 @@ class ConfigurationClassBeanDefinitionReader { this.resourceLoader = resourceLoader; this.environment = environment; this.importBeanNameGenerator = importBeanNameGenerator; + this.importRegistry = importRegistry; this.conditionEvaluator = new ConditionEvaluator(registry, environment, resourceLoader); } @@ -128,6 +133,7 @@ class ConfigurationClassBeanDefinitionReader { if (trackedConditionEvaluator.shouldSkip(configClass)) { removeBeanDefinition(configClass); + importRegistry.removeImportingClassFor(configClass.getMetadata().getClassName()); return; } 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 fe9836f831..1bdff1bc91 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 @@ -72,6 +72,8 @@ import org.springframework.core.type.classreading.MetadataReader; import org.springframework.core.type.classreading.MetadataReaderFactory; import org.springframework.core.type.filter.AssignableTypeFilter; import org.springframework.util.Assert; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; /** @@ -470,10 +472,8 @@ class ConfigurationClassParser { } else { // Candidate class not an ImportSelector or ImportBeanDefinitionRegistrar -> process it as a @Configuration class - if (!this.conditionEvaluator.shouldSkip(configClass.getMetadata(), ConfigurationPhase.REGISTER_BEAN)) { - this.importStack.registerImport(currentSourceClass.getMetadata(), candidate.getMetadata().getClassName()); - processConfigurationClass(candidate.asConfigClass(configClass)); - } + this.importStack.registerImport(currentSourceClass.getMetadata(), candidate.getMetadata().getClassName()); + processConfigurationClass(candidate.asConfigClass(configClass)); } } } @@ -591,21 +591,35 @@ class ConfigurationClassParser { interface ImportRegistry { AnnotationMetadata getImportingClassFor(String importedClass); + + void removeImportingClassFor(String importedClass); } @SuppressWarnings("serial") private static class ImportStack extends Stack implements ImportRegistry { - private final Map imports = new HashMap(); + private final MultiValueMap imports = new LinkedMultiValueMap(); public void registerImport(AnnotationMetadata importingClass, String importedClass) { - this.imports.put(importedClass, importingClass); + this.imports.add(importedClass, importingClass); + } + + @Override + public void removeImportingClassFor(String importedClass) { + for (List list : this.imports.values()) { + for (Iterator iterator = list.iterator(); iterator.hasNext();) { + if (iterator.next().getClassName().equals(importedClass)) { + iterator.remove(); + } + } + } } @Override public AnnotationMetadata getImportingClassFor(String importedClass) { - return this.imports.get(importedClass); + List list = this.imports.get(importedClass); + return (list == null || list.isEmpty() ? null : list.get(list.size() - 1)); } /** diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java index 4be5707f65..6743f70c09 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java @@ -314,7 +314,7 @@ public class ConfigurationClassPostProcessor implements BeanDefinitionRegistryPo if (this.reader == null) { this.reader = new ConfigurationClassBeanDefinitionReader(registry, this.sourceExtractor, this.problemReporter, this.metadataReaderFactory, this.resourceLoader, this.environment, - this.importBeanNameGenerator); + this.importBeanNameGenerator, parser.getImportRegistry()); } this.reader.loadBeanDefinitions(configClasses); alreadyParsed.addAll(configClasses); diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java index 7212903777..cdea59f98a 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java @@ -234,7 +234,7 @@ public class ImportAwareTests { } - @Conditional(NeverMatchingCondition.class) + @Conditional(OnMissingBeanCondition.class) @EnableSomeConfiguration("foo") @Configuration public static class ConfigurationTwo { @@ -277,11 +277,12 @@ public class ImportAwareTests { } - private static final class NeverMatchingCondition implements ConfigurationCondition { + private static final class OnMissingBeanCondition implements ConfigurationCondition { @Override public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { - return false; + return context.getBeanFactory().getBeanNamesForType(MetadataHolder.class, + true, false).length == 0; } @Override