From bbd5fc6f3569a088b619ba55f149e2d21f585207 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 30 Oct 2012 22:13:04 -0700 Subject: [PATCH 1/4] Backport "Polish Javadoc for @Import" Issue: SPR-9925 Backport-Commit: e6c4840356a9e92661e84178db3de554600ca6c8 Conflicts: spring-context/src/main/java/org/springframework/context/annotation/Import.java --- .../java/org/springframework/context/annotation/Import.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/Import.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/Import.java index 4a2fe947ca8..db173a1892f 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/Import.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/Import.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2012 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. @@ -53,7 +53,8 @@ import java.lang.annotation.Target; public @interface Import { /** - * The @{@link Configuration} and/or {@link ImportSelector} classes to import. + * The @{@link Configuration}, {@link ImportSelector} and/or + * {@link ImportBeanDefinitionRegistrar} classes to import. */ Class[] value(); } From e9616b7204e5fb6ef4920fd3a9815f09862fdbae Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 30 Oct 2012 22:13:15 -0700 Subject: [PATCH 2/4] Backport "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 Backport-Commit: 6d8b37d8bbce8c6e6cb4890291469c80742132f7 Conflicts: spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java --- .../annotation/ConfigurationClassParser.java | 74 ++++++++----------- .../context/annotation/ImportAwareTests.java | 53 +++++++++++++ 2 files changed, 84 insertions(+), 43 deletions(-) diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index bb0bb8b1a78..4db33d8a835 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/org.springframework.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; @@ -221,10 +220,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 @@ -274,45 +272,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 { @@ -451,5 +440,4 @@ class ConfigurationClassParser { new Location(importStack.peek().getResource(), metadata)); } } - } diff --git a/org.springframework.context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java b/org.springframework.context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java index 822923d9c1f..964348e48f5 100644 --- a/org.springframework.context/src/test/java/org/springframework/context/annotation/ImportAwareTests.java +++ b/org.springframework.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; + } + } } From 1f52f09089a3c622545726a067bb00cd79469251 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 31 Oct 2012 13:38:34 +0100 Subject: [PATCH 3/4] Backport "Ensure @Imports are processed in correct order" Issue: SPR-9925 Backport-Commit: 3416e058a01d80d22c52c8c6fb720454be4c4290 Conflicts: spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java --- .../annotation/ConfigurationClassParser.java | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 4db33d8a835..217c73a6be9 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -24,6 +24,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.Map; import java.util.Set; import java.util.Stack; @@ -72,6 +73,8 @@ import static org.springframework.context.annotation.MetadataUtils.*; */ class ConfigurationClassParser { + private static final String[] EMPTY_IMPORTS = {}; + private final MetadataReaderFactory metadataReaderFactory; private final ProblemReporter problemReporter; @@ -220,10 +223,7 @@ class ConfigurationClassParser { } // process any @Import annotations - Set imports = getImports(metadata.getClassName(), null, new HashSet()); - if (imports != null && !imports.isEmpty()) { - processImport(configClass, imports.toArray(new String[imports.size()]), true); - } + processImport(configClass, getImports(metadata.getClassName()), true); // process any @ImportResource annotations if (metadata.isAnnotated(ImportResource.class.getName())) { @@ -285,23 +285,31 @@ class ConfigurationClassParser { * @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 Set getImports(String className, Set imports, + private String[] getImports(String className) throws IOException { + LinkedList imports = new LinkedList(); + collectImports(className, imports, new HashSet()); + if(imports == null || imports.isEmpty()) { + return EMPTY_IMPORTS; + } + LinkedHashSet uniqueImports = new LinkedHashSet(imports); + return uniqueImports.toArray(new String[uniqueImports.size()]); + } + + private void collectImports(String className, LinkedList imports, Set visited) throws IOException { if (visited.add(className)) { AnnotationMetadata metadata = metadataReaderFactory.getMetadataReader(className).getAnnotationMetadata(); + for (String annotationType : metadata.getAnnotationTypes()) { + collectImports(annotationType, imports, visited); + } 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)); } } - for (String annotationType : metadata.getAnnotationTypes()) { - getImports(annotationType, imports, visited); - } } - return imports; } private void processImport(ConfigurationClass configClass, String[] classesToImport, boolean checkForCircularImports) throws IOException { From a9290d80c4fcac4c6f2ed7b2fec74cb3f692f201 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 31 Oct 2012 09:30:17 -0700 Subject: [PATCH 4/4] Backport "Polish @Imports search code" Issue: SPR-9925 Backport-Commit: 4cdf46f83c775c5101bc664c819fd5c0bb0682f7 Conflicts: spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java --- .../annotation/ConfigurationClassParser.java | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index 217c73a6be9..9995feb27a5 100644 --- a/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -16,6 +16,8 @@ package org.springframework.context.annotation; +import static org.springframework.context.annotation.MetadataUtils.attributesFor; + import java.io.IOException; import java.util.Arrays; import java.util.Collections; @@ -24,7 +26,6 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; -import java.util.LinkedList; import java.util.Map; import java.util.Set; import java.util.Stack; @@ -51,8 +52,6 @@ import org.springframework.core.type.classreading.MetadataReaderFactory; import org.springframework.core.type.filter.AssignableTypeFilter; import org.springframework.util.StringUtils; -import static org.springframework.context.annotation.MetadataUtils.*; - /** * Parses a {@link Configuration} class definition, populating a collection of * {@link ConfigurationClass} objects (parsing a single Configuration class may result in @@ -73,8 +72,6 @@ import static org.springframework.context.annotation.MetadataUtils.*; */ class ConfigurationClassParser { - private static final String[] EMPTY_IMPORTS = {}; - private final MetadataReaderFactory metadataReaderFactory; private final ProblemReporter problemReporter; @@ -223,7 +220,10 @@ class ConfigurationClassParser { } // process any @Import annotations - processImport(configClass, getImports(metadata.getClassName()), 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 if (metadata.isAnnotated(ImportResource.class.getName())) { @@ -285,31 +285,23 @@ class ConfigurationClassParser { * @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 String[] getImports(String className) throws IOException { - LinkedList imports = new LinkedList(); - collectImports(className, imports, new HashSet()); - if(imports == null || imports.isEmpty()) { - return EMPTY_IMPORTS; - } - LinkedHashSet uniqueImports = new LinkedHashSet(imports); - return uniqueImports.toArray(new String[uniqueImports.size()]); - } - - private void collectImports(String className, LinkedList imports, + private Set getImports(String className, Set imports, Set visited) throws IOException { if (visited.add(className)) { AnnotationMetadata metadata = metadataReaderFactory.getMetadataReader(className).getAnnotationMetadata(); for (String annotationType : metadata.getAnnotationTypes()) { - collectImports(annotationType, imports, visited); + imports = getImports(annotationType, imports, visited); } 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)); } } } + return imports; } private void processImport(ConfigurationClass configClass, String[] classesToImport, boolean checkForCircularImports) throws IOException {