From 6d8b37d8bbce8c6e6cb4890291469c80742132f7 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 30 Oct 2012 22:13:15 -0700 Subject: [PATCH] Prevent duplicate @Import processing Refactor ConfigurationClassParser to recursively find values from all @Import annotations, combining them into a single unique set. This change prevents ImportBeanDefinitionRegistrars from being invoked twice. Issue: SPR-9925 --- .../annotation/ConfigurationClassParser.java | 74 ++++++++----------- .../context/annotation/ImportAwareTests.java | 53 +++++++++++++ 2 files changed, 84 insertions(+), 43 deletions(-) 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 cf6e12f296..2690c5b03e 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 @@ -17,14 +17,13 @@ package org.springframework.context.annotation; import java.io.IOException; -import java.lang.annotation.Annotation; -import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.Stack; @@ -222,10 +221,9 @@ class ConfigurationClassParser { } // process any @Import annotations - List imports = - findAllAnnotationAttributes(Import.class, metadata.getClassName(), true); - for (AnnotationAttributes importAnno : imports) { - processImport(configClass, importAnno.getStringArray("value"), true); + Set imports = getImports(metadata.getClassName(), null, new HashSet()); + if (imports != null && !imports.isEmpty()) { + processImport(configClass, imports.toArray(new String[imports.size()]), true); } // process any @ImportResource annotations @@ -265,45 +263,36 @@ class ConfigurationClassParser { } /** - * Return a list of attribute maps for all declarations of the given annotation - * on the given annotated class using the given MetadataReaderFactory to introspect - * annotation metadata. Meta-annotations are ordered first in the list, and if the - * target annotation is declared directly on the class, its map of attributes will be - * ordered last in the list. - * @param targetAnnotation the annotation to search for, both locally and as a meta-annotation - * @param annotatedClassName the class to inspect - * @param classValuesAsString whether class attributes should be returned as strings + * Recursively collect all declared {@code @Import} values. Unlike most + * meta-annotations it is valid to have several {@code @Import}s declared with + * different values, the usual process or returning values from the first + * meta-annotation on a class is not sufficient. + *

For example, it is common for a {@code @Configuration} class to declare direct + * {@code @Import}s in addition to meta-imports originating from an {@code @Enable} + * annotation. + * @param className the class name to search + * @param imports the imports collected so far or {@code null} + * @param visited used to track visited classes to prevent infinite recursion (must not be null) + * @return a set of all {@link Import#value() import values} or {@code null} + * @throws IOException if there is any problem reading metadata from the named class */ - private List findAllAnnotationAttributes( - Class targetAnnotation, String annotatedClassName, - boolean classValuesAsString) throws IOException { - - List allAttribs = new ArrayList(); - - MetadataReader reader = this.metadataReaderFactory.getMetadataReader(annotatedClassName); - AnnotationMetadata metadata = reader.getAnnotationMetadata(); - String targetAnnotationType = targetAnnotation.getName(); - - for (String annotationType : metadata.getAnnotationTypes()) { - if (annotationType.equals(targetAnnotationType)) { - continue; + private Set getImports(String className, Set imports, + Set visited) throws IOException { + if (visited.add(className)) { + AnnotationMetadata metadata = metadataReaderFactory.getMetadataReader(className).getAnnotationMetadata(); + Map attributes = metadata.getAnnotationAttributes(Import.class.getName(), true); + if (attributes != null) { + String[] value = (String[]) attributes.get("value"); + if (value != null && value.length > 0) { + imports = (imports == null ? new LinkedHashSet() : imports); + imports.addAll(Arrays.asList(value)); + } } - AnnotationMetadata metaAnnotations = - this.metadataReaderFactory.getMetadataReader(annotationType).getAnnotationMetadata(); - AnnotationAttributes targetAttribs = - AnnotationAttributes.fromMap(metaAnnotations.getAnnotationAttributes(targetAnnotationType, classValuesAsString)); - if (targetAttribs != null) { - allAttribs.add(targetAttribs); + for (String annotationType : metadata.getAnnotationTypes()) { + getImports(annotationType, imports, visited); } } - - AnnotationAttributes localAttribs = - AnnotationAttributes.fromMap(metadata.getAnnotationAttributes(targetAnnotationType, classValuesAsString)); - if (localAttribs != null) { - allAttribs.add(localAttribs); - } - - return allAttribs; + return imports; } private void processImport(ConfigurationClass configClass, String[] classesToImport, boolean checkForCircularImports) throws IOException { @@ -440,5 +429,4 @@ class ConfigurationClassParser { new Location(importStack.peek().getResource(), metadata)); } } - } 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 822923d9c1..964348e48f 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 @@ -25,10 +25,14 @@ import org.junit.Test; import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanPostProcessor; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.GenericBeanDefinition; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.type.AnnotationMetadata; import org.springframework.scheduling.annotation.AsyncAnnotationBeanPostProcessor; +import org.springframework.util.Assert; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; @@ -77,6 +81,24 @@ public class ImportAwareTests { assertThat(foo, is("xyz")); } + @Test + public void importRegistrar() throws Exception { + ImportedRegistrar.called = false; + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(ImportingRegistrarConfig.class); + ctx.refresh(); + assertNotNull(ctx.getBean("registrarImportedBean")); + } + + @Test + public void importRegistrarWithImport() throws Exception { + ImportedRegistrar.called = false; + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.register(ImportingRegistrarConfigWithImport.class); + ctx.refresh(); + assertNotNull(ctx.getBean("registrarImportedBean")); + assertNotNull(ctx.getBean(ImportedConfig.class)); + } @Configuration @Import(ImportedConfig.class) @@ -131,4 +153,35 @@ public class ImportAwareTests { public void setBeanFactory(BeanFactory beanFactory) throws BeansException { } } + + @Configuration + @EnableImportRegistrar + static class ImportingRegistrarConfig { + } + + @Configuration + @EnableImportRegistrar + @Import(ImportedConfig.class) + static class ImportingRegistrarConfigWithImport { + } + + @Target(ElementType.TYPE) + @Retention(RetentionPolicy.RUNTIME) + @Import(ImportedRegistrar.class) + public @interface EnableImportRegistrar { + } + + static class ImportedRegistrar implements ImportBeanDefinitionRegistrar { + + static boolean called; + + public void registerBeanDefinitions(AnnotationMetadata importingClassMetadata, + BeanDefinitionRegistry registry) { + BeanDefinition beanDefinition = new GenericBeanDefinition(); + beanDefinition.setBeanClassName(String.class.getName()); + registry.registerBeanDefinition("registrarImportedBean", beanDefinition ); + Assert.state(called == false, "ImportedRegistrar called twice"); + called = true; + } + } }