From fa6a13859871b418d1e01fe3f8579b1bf8c5825e Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 26 Jan 2017 20:42:51 -0800 Subject: [PATCH] Refine ImportsContextCustomizer cache logic Update `ImportsContextCustomizer` so that whenever possible a more specific cache key is used. Prior to this commit the customizer would generate a key based on *all* annotations on the test class. This has repeatedly caused issues where test classes that should have the same cache key did not due to unrelated annotations. A new `DeterminableImports` interface has been added that can be implemented by `ImportSelector` and `ImportBeanDefinitionRegistrar` implementations that are able to determine their imports early. The existing `ImportAutoConfigurationImportSelector` and `AutoConfigurationPackages` classes have been retrofitted with this interface. Fixes gh-7953 --- .../AutoConfigurationPackages.java | 48 ++++- ...ImportAutoConfigurationImportSelector.java | 19 +- ...tAutoConfigurationImportSelectorTests.java | 187 +++++++++++++++--- ...izerFactoryWithAutoConfigurationTests.java | 158 +++++++++++++++ .../context/ImportsContextCustomizer.java | 87 +++++++- .../ImportsContextCustomizerTests.java | 96 ++++++++- .../annotation/DeterminableImports.java | 60 ++++++ .../boot/context/annotation/package-info.java | 21 ++ 8 files changed, 638 insertions(+), 38 deletions(-) create mode 100644 spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/cache/ImportsContextCustomizerFactoryWithAutoConfigurationTests.java create mode 100644 spring-boot/src/main/java/org/springframework/boot/context/annotation/DeterminableImports.java create mode 100644 spring-boot/src/main/java/org/springframework/boot/context/annotation/package-info.java diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationPackages.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationPackages.java index 3aeb097d8b..a2557aa823 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationPackages.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationPackages.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -18,6 +18,7 @@ package org.springframework.boot.autoconfigure; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -31,6 +32,7 @@ import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.ConstructorArgumentValues; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.GenericBeanDefinition; +import org.springframework.boot.context.annotation.DeterminableImports; import org.springframework.context.annotation.ImportBeanDefinitionRegistrar; import org.springframework.core.Ordered; import org.springframework.core.annotation.Order; @@ -122,12 +124,52 @@ public abstract class AutoConfigurationPackages { * configuration. */ @Order(Ordered.HIGHEST_PRECEDENCE) - static class Registrar implements ImportBeanDefinitionRegistrar { + static class Registrar implements ImportBeanDefinitionRegistrar, DeterminableImports { @Override public void registerBeanDefinitions(AnnotationMetadata metadata, BeanDefinitionRegistry registry) { - register(registry, ClassUtils.getPackageName(metadata.getClassName())); + register(registry, new PackageImport(metadata).getPackageName()); + } + + @Override + public Set determineImports(AnnotationMetadata metadata) { + return Collections.singleton(new PackageImport(metadata)); + } + + } + + /** + * Wrapper for a package import. + */ + private final static class PackageImport { + + private final String packageName; + + PackageImport(AnnotationMetadata metadata) { + this.packageName = ClassUtils.getPackageName(metadata.getClassName()); + } + + @Override + public int hashCode() { + return this.packageName.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (obj == null || getClass() != obj.getClass()) { + return false; + } + return this.packageName.equals(((PackageImport) obj).packageName); + } + + public String getPackageName() { + return this.packageName; + } + + @Override + public String toString() { + return "Package Import " + this.packageName; } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelector.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelector.java index 834e392cb1..5b9fcac212 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelector.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelector.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.springframework.boot.context.annotation.DeterminableImports; import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.annotation.AnnotationUtils; @@ -44,7 +45,8 @@ import org.springframework.util.ObjectUtils; * @author Phillip Webb * @author Andy Wilkinson */ -class ImportAutoConfigurationImportSelector extends AutoConfigurationImportSelector { +class ImportAutoConfigurationImportSelector extends AutoConfigurationImportSelector + implements DeterminableImports { private static final Set ANNOTATION_NAMES; @@ -55,6 +57,14 @@ class ImportAutoConfigurationImportSelector extends AutoConfigurationImportSelec ANNOTATION_NAMES = Collections.unmodifiableSet(names); } + @Override + public Set determineImports(AnnotationMetadata metadata) { + Set result = new LinkedHashSet(); + result.addAll(getCandidateConfigurations(metadata, null)); + result.removeAll(getExclusions(metadata, null)); + return Collections.unmodifiableSet(result); + } + @Override protected AnnotationAttributes getAttributes(AnnotationMetadata metadata) { return null; @@ -85,6 +95,10 @@ class ImportAutoConfigurationImportSelector extends AutoConfigurationImportSelec if (classes.length > 0) { return Arrays.asList(classes); } + return loadFactoryNames(source); + } + + protected Collection loadFactoryNames(Class source) { return SpringFactoriesLoader.loadFactoryNames(source, getClass().getClassLoader()); } @@ -117,7 +131,8 @@ class ImportAutoConfigurationImportSelector extends AutoConfigurationImportSelec return exclusions; } - private Map, List> getAnnotations(AnnotationMetadata metadata) { + protected final Map, List> getAnnotations( + AnnotationMetadata metadata) { MultiValueMap, Annotation> annotations = new LinkedMultiValueMap, Annotation>(); Class source = ClassUtils.resolveClassName(metadata.getClassName(), null); collectAnnotations(source, annotations, new HashSet>()); diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelectorTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelectorTests.java index bb0587b01c..cbd254b1a5 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelectorTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/ImportAutoConfigurationImportSelectorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -19,6 +19,9 @@ package org.springframework.boot.autoconfigure; import java.io.IOException; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.Arrays; +import java.util.Collection; +import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -46,7 +49,7 @@ import static org.mockito.Mockito.verifyZeroInteractions; */ public class ImportAutoConfigurationImportSelectorTests { - private final ImportAutoConfigurationImportSelector importSelector = new ImportAutoConfigurationImportSelector(); + private final ImportAutoConfigurationImportSelector importSelector = new TestImportAutoConfigurationImportSelector(); private final ConfigurableListableBeanFactory beanFactory = new DefaultListableBeanFactory(); @@ -63,36 +66,32 @@ public class ImportAutoConfigurationImportSelectorTests { @Test public void importsAreSelected() throws Exception { - AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory() - .getMetadataReader(ImportFreeMarker.class.getName()) - .getAnnotationMetadata(); + AnnotationMetadata annotationMetadata = getAnnotationMetadata( + ImportFreeMarker.class); String[] imports = this.importSelector.selectImports(annotationMetadata); assertThat(imports).containsExactly(FreeMarkerAutoConfiguration.class.getName()); } @Test public void importsAreSelectedUsingClassesAttribute() throws Exception { - AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory() - .getMetadataReader(ImportFreeMarkerUsingClassesAttribute.class.getName()) - .getAnnotationMetadata(); + AnnotationMetadata annotationMetadata = getAnnotationMetadata( + ImportFreeMarkerUsingClassesAttribute.class); String[] imports = this.importSelector.selectImports(annotationMetadata); assertThat(imports).containsExactly(FreeMarkerAutoConfiguration.class.getName()); } @Test public void propertyExclusionsAreNotApplied() throws Exception { - AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory() - .getMetadataReader(ImportFreeMarker.class.getName()) - .getAnnotationMetadata(); + AnnotationMetadata annotationMetadata = getAnnotationMetadata( + ImportFreeMarker.class); this.importSelector.selectImports(annotationMetadata); verifyZeroInteractions(this.environment); } @Test public void multipleImportsAreFound() throws Exception { - AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory() - .getMetadataReader(MultipleImports.class.getName()) - .getAnnotationMetadata(); + AnnotationMetadata annotationMetadata = getAnnotationMetadata( + MultipleImports.class); String[] imports = this.importSelector.selectImports(annotationMetadata); assertThat(imports).containsOnly(FreeMarkerAutoConfiguration.class.getName(), ThymeleafAutoConfiguration.class.getName()); @@ -100,41 +99,94 @@ public class ImportAutoConfigurationImportSelectorTests { @Test public void selfAnnotatingAnnotationDoesNotCauseStackOverflow() throws IOException { - AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory() - .getMetadataReader(ImportWithSelfAnnotatingAnnotation.class.getName()) - .getAnnotationMetadata(); + AnnotationMetadata annotationMetadata = getAnnotationMetadata( + ImportWithSelfAnnotatingAnnotation.class); String[] imports = this.importSelector.selectImports(annotationMetadata); assertThat(imports).containsOnly(ThymeleafAutoConfiguration.class.getName()); } @Test public void exclusionsAreApplied() throws Exception { - AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory() - .getMetadataReader(MultipleImportsWithExclusion.class.getName()) - .getAnnotationMetadata(); + AnnotationMetadata annotationMetadata = getAnnotationMetadata( + MultipleImportsWithExclusion.class); String[] imports = this.importSelector.selectImports(annotationMetadata); assertThat(imports).containsOnly(FreeMarkerAutoConfiguration.class.getName()); } @Test public void exclusionsWithoutImport() throws Exception { - AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory() - .getMetadataReader(ExclusionWithoutImport.class.getName()) - .getAnnotationMetadata(); + AnnotationMetadata annotationMetadata = getAnnotationMetadata( + ExclusionWithoutImport.class); String[] imports = this.importSelector.selectImports(annotationMetadata); assertThat(imports).containsOnly(FreeMarkerAutoConfiguration.class.getName()); } @Test public void exclusionsAliasesAreApplied() throws Exception { - AnnotationMetadata annotationMetadata = new SimpleMetadataReaderFactory() - .getMetadataReader( - ImportWithSelfAnnotatingAnnotationExclude.class.getName()) - .getAnnotationMetadata(); + AnnotationMetadata annotationMetadata = getAnnotationMetadata( + ImportWithSelfAnnotatingAnnotationExclude.class); + String[] imports = this.importSelector.selectImports(annotationMetadata); assertThat(imports).isEmpty(); } + @Test + public void determineImportsWhenUsingMetaWithoutClassesShouldBeEqual() + throws Exception { + Set set1 = this.importSelector.determineImports( + getAnnotationMetadata(ImportMetaAutoConfigurationWithUnrelatedOne.class)); + Set set2 = this.importSelector.determineImports( + getAnnotationMetadata(ImportMetaAutoConfigurationWithUnrelatedTwo.class)); + assertThat(set1).isEqualTo(set2); + assertThat(set1.hashCode()).isEqualTo(set2.hashCode()); + } + + @Test + public void determineImportsWhenUsingNonMetaWithoutClassesShouldBeSame() + throws Exception { + Set set1 = this.importSelector.determineImports( + getAnnotationMetadata(ImportAutoConfigurationWithUnrelatedOne.class)); + Set set2 = this.importSelector.determineImports( + getAnnotationMetadata(ImportAutoConfigurationWithUnrelatedTwo.class)); + assertThat(set1).isEqualTo(set2); + } + + @Test + public void determineImportsWhenUsingNonMetaWithClassesShouldBeSame() + throws Exception { + Set set1 = this.importSelector.determineImports( + getAnnotationMetadata(ImportAutoConfigurationWithItemsOne.class)); + Set set2 = this.importSelector.determineImports( + getAnnotationMetadata(ImportAutoConfigurationWithItemsOne.class)); + assertThat(set1).isEqualTo(set2); + } + + @Test + public void determineImportsWhenUsingMetaExcludeWithoutClassesShouldBeEqual() + throws Exception { + Set set1 = this.importSelector.determineImports(getAnnotationMetadata( + ImportMetaAutoConfigurationExcludeWithUnrelatedOne.class)); + Set set2 = this.importSelector.determineImports(getAnnotationMetadata( + ImportMetaAutoConfigurationExcludeWithUnrelatedTwo.class)); + assertThat(set1).isEqualTo(set2); + assertThat(set1.hashCode()).isEqualTo(set2.hashCode()); + } + + @Test + public void determineImportsWhenUsingMetaDifferentExcludeWithoutClassesShouldBeDifferent() + throws Exception { + Set set1 = this.importSelector.determineImports(getAnnotationMetadata( + ImportMetaAutoConfigurationExcludeWithUnrelatedOne.class)); + Set set2 = this.importSelector.determineImports( + getAnnotationMetadata(ImportMetaAutoConfigurationWithUnrelatedTwo.class)); + assertThat(set1).isNotEqualTo(set2); + } + + private AnnotationMetadata getAnnotationMetadata(Class source) throws IOException { + return new SimpleMetadataReaderFactory().getMetadataReader(source.getName()) + .getAnnotationMetadata(); + } + @ImportAutoConfiguration(FreeMarkerAutoConfiguration.class) static class ImportFreeMarker { @@ -186,6 +238,73 @@ public class ImportAutoConfigurationImportSelectorTests { } + @MetaImportAutoConfiguration + @UnrelatedOne + static class ImportMetaAutoConfigurationWithUnrelatedOne { + + } + + @MetaImportAutoConfiguration + @UnrelatedTwo + static class ImportMetaAutoConfigurationWithUnrelatedTwo { + + } + + @ImportAutoConfiguration + @UnrelatedOne + static class ImportAutoConfigurationWithUnrelatedOne { + + } + + @ImportAutoConfiguration + @UnrelatedTwo + static class ImportAutoConfigurationWithUnrelatedTwo { + + } + + @ImportAutoConfiguration(classes = ThymeleafAutoConfiguration.class) + @UnrelatedOne + static class ImportAutoConfigurationWithItemsOne { + + } + + @ImportAutoConfiguration(classes = ThymeleafAutoConfiguration.class) + @UnrelatedOne + static class ImportAutoConfigurationWithItemsTwo { + + } + + @MetaImportAutoConfiguration(exclude = ThymeleafAutoConfiguration.class) + @UnrelatedOne + static class ImportMetaAutoConfigurationExcludeWithUnrelatedOne { + + } + + @MetaImportAutoConfiguration(exclude = ThymeleafAutoConfiguration.class) + @UnrelatedTwo + static class ImportMetaAutoConfigurationExcludeWithUnrelatedTwo { + + } + + @ImportAutoConfiguration + @Retention(RetentionPolicy.RUNTIME) + static @interface MetaImportAutoConfiguration { + + @AliasFor(annotation = ImportAutoConfiguration.class, attribute = "exclude") + Class[] exclude() default {}; + + } + + @Retention(RetentionPolicy.RUNTIME) + static @interface UnrelatedOne { + + } + + @Retention(RetentionPolicy.RUNTIME) + static @interface UnrelatedTwo { + + } + @Retention(RetentionPolicy.RUNTIME) @ImportAutoConfiguration(ThymeleafAutoConfiguration.class) @SelfAnnotating @@ -196,4 +315,18 @@ public class ImportAutoConfigurationImportSelectorTests { } + private static class TestImportAutoConfigurationImportSelector + extends ImportAutoConfigurationImportSelector { + + @Override + protected Collection loadFactoryNames(Class source) { + if (source == MetaImportAutoConfiguration.class) { + return Arrays.asList(ThymeleafAutoConfiguration.class.getName(), + FreeMarkerAutoConfiguration.class.getName()); + } + return super.loadFactoryNames(source); + } + + } + } diff --git a/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/cache/ImportsContextCustomizerFactoryWithAutoConfigurationTests.java b/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/cache/ImportsContextCustomizerFactoryWithAutoConfigurationTests.java new file mode 100644 index 0000000000..45d5dbbfdc --- /dev/null +++ b/spring-boot-test-autoconfigure/src/test/java/org/springframework/boot/test/autoconfigure/cache/ImportsContextCustomizerFactoryWithAutoConfigurationTests.java @@ -0,0 +1,158 @@ +/* + * Copyright 2012-2017 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 + * + * http://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.boot.test.autoconfigure.cache; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +import org.junit.Test; +import org.junit.runner.notification.RunNotifier; +import org.junit.runners.model.InitializationError; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.ImportAutoConfiguration; +import org.springframework.boot.autoconfigure.domain.EntityScan; +import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; +import org.springframework.boot.test.autoconfigure.orm.jpa.ExampleEntity; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for {@code ImportsContextCustomizerFactory} when used with + * {@link ImportAutoConfiguration}. + * + * @author Phillip Webb + * @author Andy Wilkinson + */ +public class ImportsContextCustomizerFactoryWithAutoConfigurationTests { + + static ApplicationContext contextFromTest; + + @Test + public void testClassesThatHaveSameAnnotationsShareAContext() + throws InitializationError { + RunNotifier notifier = new RunNotifier(); + new SpringJUnit4ClassRunner(DataJpaTest1.class).run(notifier); + ApplicationContext test1Context = contextFromTest; + new SpringJUnit4ClassRunner(DataJpaTest3.class).run(notifier); + ApplicationContext test2Context = contextFromTest; + assertThat(test1Context).isSameAs(test2Context); + } + + @Test + public void testClassesThatOnlyHaveDifferingUnrelatedAnnotationsShareAContext() + throws InitializationError { + RunNotifier notifier = new RunNotifier(); + new SpringJUnit4ClassRunner(DataJpaTest1.class).run(notifier); + ApplicationContext test1Context = contextFromTest; + new SpringJUnit4ClassRunner(DataJpaTest2.class).run(notifier); + ApplicationContext test2Context = contextFromTest; + assertThat(test1Context).isSameAs(test2Context); + } + + @Test + public void testClassesThatOnlyHaveDifferingPropertyMappedAnnotationAttributesDoNotShareAContext() + throws InitializationError { + RunNotifier notifier = new RunNotifier(); + new SpringJUnit4ClassRunner(DataJpaTest1.class).run(notifier); + ApplicationContext test1Context = contextFromTest; + new SpringJUnit4ClassRunner(DataJpaTest4.class).run(notifier); + ApplicationContext test2Context = contextFromTest; + assertThat(test1Context).isNotSameAs(test2Context); + } + + @DataJpaTest + @ContextConfiguration(classes = EmptyConfig.class) + @Unrelated1 + public static class DataJpaTest1 { + + @Autowired + private ApplicationContext context; + + @Test + public void test() { + contextFromTest = this.context; + } + + } + + @ContextConfiguration(classes = EmptyConfig.class) + @DataJpaTest + @Unrelated2 + public static class DataJpaTest2 { + + @Autowired + private ApplicationContext context; + + @Test + public void test() { + contextFromTest = this.context; + } + + } + + @ContextConfiguration(classes = EmptyConfig.class) + @DataJpaTest + @Unrelated1 + public static class DataJpaTest3 { + + @Autowired + private ApplicationContext context; + + @Test + public void test() { + contextFromTest = this.context; + } + + } + + @ContextConfiguration(classes = EmptyConfig.class) + @DataJpaTest(showSql = false) + @Unrelated1 + public static class DataJpaTest4 { + + @Autowired + private ApplicationContext context; + + @Test + public void test() { + contextFromTest = this.context; + } + + } + + @Retention(RetentionPolicy.RUNTIME) + static @interface Unrelated1 { + + } + + @Retention(RetentionPolicy.RUNTIME) + static @interface Unrelated2 { + + } + + @Configuration + @EntityScan(basePackageClasses = ExampleEntity.class) + static class EmptyConfig { + + } + +} diff --git a/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java b/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java index 17eea896f9..2b32b96f05 100644 --- a/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java +++ b/spring-boot-test/src/main/java/org/springframework/boot/test/context/ImportsContextCustomizer.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -18,8 +18,10 @@ package org.springframework.boot.test.context; import java.lang.annotation.Annotation; import java.lang.reflect.AnnotatedElement; +import java.lang.reflect.Constructor; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Set; import org.springframework.beans.BeansException; @@ -30,19 +32,24 @@ import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanDefinitionRegistryPostProcessor; +import org.springframework.boot.context.annotation.DeterminableImports; import org.springframework.context.ApplicationContext; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotatedBeanDefinitionReader; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.ImportBeanDefinitionRegistrar; import org.springframework.context.annotation.ImportSelector; import org.springframework.context.support.AbstractApplicationContext; import org.springframework.core.Ordered; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.core.annotation.Order; +import org.springframework.core.style.ToStringCreator; import org.springframework.core.type.AnnotationMetadata; +import org.springframework.core.type.StandardAnnotationMetadata; import org.springframework.test.context.ContextCustomizer; import org.springframework.test.context.MergedContextConfiguration; +import org.springframework.util.ReflectionUtils; /** * {@link ContextCustomizer} to allow {@code @Import} annotations to be used directly on @@ -127,6 +134,11 @@ class ImportsContextCustomizer implements ContextCustomizer { return this.key.equals(other.key); } + @Override + public String toString() { + return new ToStringCreator(this).append("key", this.key).toString(); + } + /** * {@link Configuration} registered to trigger the {@link ImportsSelector}. */ @@ -214,6 +226,8 @@ class ImportsContextCustomizer implements ContextCustomizer { */ static class ContextCustomizerKey { + private static final Class[] NO_IMPORTS = {}; + private static final Set ANNOTATION_FILTERS; static { @@ -224,13 +238,15 @@ class ImportsContextCustomizer implements ContextCustomizer { ANNOTATION_FILTERS = Collections.unmodifiableSet(filters); } - private final Set annotations; + private final Set key; ContextCustomizerKey(Class testClass) { Set annotations = new HashSet(); Set> seen = new HashSet>(); collectClassAnnotations(testClass, annotations, seen); - this.annotations = Collections.unmodifiableSet(annotations); + Set determinedImports = determineImports(annotations, testClass); + this.key = Collections.unmodifiableSet( + determinedImports != null ? determinedImports : annotations); } private void collectClassAnnotations(Class classType, @@ -266,17 +282,78 @@ class ImportsContextCustomizer implements ContextCustomizer { return false; } + private Set determineImports(Set annotations, + Class testClass) { + Set determinedImports = new LinkedHashSet(); + AnnotationMetadata testClassMetadata = new StandardAnnotationMetadata( + testClass); + for (Annotation annotation : annotations) { + for (Class source : getImports(annotation)) { + Set determinedSourceImports = determineImports(source, + testClassMetadata); + if (determinedSourceImports == null) { + return null; + } + determinedImports.addAll(determinedSourceImports); + } + } + return determinedImports; + } + + private Class[] getImports(Annotation annotation) { + if (annotation instanceof Import) { + return ((Import) annotation).value(); + } + return NO_IMPORTS; + } + + private Set determineImports(Class source, + AnnotationMetadata metadata) { + if (DeterminableImports.class.isAssignableFrom(source)) { + // We can determine the imports + return ((DeterminableImports) instantiate(source)) + .determineImports(metadata); + } + if (ImportSelector.class.isAssignableFrom(source) + || ImportBeanDefinitionRegistrar.class.isAssignableFrom(source)) { + // Standard ImportSelector and ImportBeanDefinitionRegistrar could + // use anything to determine the imports so we can't be sure + return null; + } + // The source itself is the import + return Collections.singleton(source.getName()); + } + + @SuppressWarnings("unchecked") + private T instantiate(Class source) { + try { + Constructor constructor = source.getDeclaredConstructor(); + ReflectionUtils.makeAccessible(constructor); + return (T) constructor.newInstance(); + } + catch (Throwable ex) { + throw new IllegalStateException( + "Unable to instantiate DeterminableImportSelector " + + source.getName(), + ex); + } + } + @Override public int hashCode() { - return this.annotations.hashCode(); + return this.key.hashCode(); } @Override public boolean equals(Object obj) { return (obj != null && getClass().equals(obj.getClass()) - && this.annotations.equals(((ContextCustomizerKey) obj).annotations)); + && this.key.equals(((ContextCustomizerKey) obj).key)); } + @Override + public String toString() { + return this.key.toString(); + } } /** diff --git a/spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerTests.java b/spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerTests.java index aeec7702f6..337eeb6ff2 100644 --- a/spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerTests.java +++ b/spring-boot-test/src/test/java/org/springframework/boot/test/context/ImportsContextCustomizerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2016 the original author or authors. + * Copyright 2012-2017 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. @@ -16,10 +16,21 @@ package org.springframework.boot.test.context; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.util.Collections; +import java.util.Set; + import kotlin.Metadata; import org.junit.Test; import org.spockframework.runtime.model.SpecMetadata; +import org.springframework.boot.context.annotation.DeterminableImports; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.ImportSelector; +import org.springframework.core.type.AnnotationMetadata; + import static org.assertj.core.api.Assertions.assertThat; /** @@ -29,6 +40,26 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class ImportsContextCustomizerTests { + @Test + public void importSelectorsCouldUseAnyAnnotations() throws Exception { + assertThat(new ImportsContextCustomizer(FirstImportSelectorAnnotatedClass.class)) + .isNotEqualTo(new ImportsContextCustomizer( + SecondImportSelectorAnnotatedClass.class)); + } + + @Test + public void determinableImportSelector() throws Exception { + assertThat(new ImportsContextCustomizer( + FirstDeterminableImportSelectorAnnotatedClass.class)) + .isEqualTo(new ImportsContextCustomizer( + SecondDeterminableImportSelectorAnnotatedClass.class)); + } + + @Test + public void importAutoConfigurationCanIgnoreAdditionalAnnotations() throws Exception { + + } + @Test public void customizersForTestClassesWithDifferentKotlinMetadataAreEqual() { assertThat(new ImportsContextCustomizer(FirstKotlinAnnotatedTestClass.class)) @@ -43,6 +74,30 @@ public class ImportsContextCustomizerTests { SecondSpockAnnotatedTestClass.class)); } + @Import(TestImportSelector.class) + @Indicator1 + static class FirstImportSelectorAnnotatedClass { + + } + + @Import(TestImportSelector.class) + @Indicator2 + static class SecondImportSelectorAnnotatedClass { + + } + + @Import(TestDeterminableImportSelector.class) + @Indicator1 + static class FirstDeterminableImportSelectorAnnotatedClass { + + } + + @Import(TestDeterminableImportSelector.class) + @Indicator2 + static class SecondDeterminableImportSelectorAnnotatedClass { + + } + @Metadata(d2 = "foo") static class FirstKotlinAnnotatedTestClass { @@ -63,4 +118,43 @@ public class ImportsContextCustomizerTests { } + @Retention(RetentionPolicy.RUNTIME) + @interface Indicator1 { + + } + + @Retention(RetentionPolicy.RUNTIME) + @interface Indicator2 { + + } + + static class TestImportSelector implements ImportSelector { + + @Override + public String[] selectImports(AnnotationMetadata arg0) { + return new String[] {}; + } + + } + + static class TestDeterminableImportSelector + implements ImportSelector, DeterminableImports { + + @Override + public String[] selectImports(AnnotationMetadata arg0) { + return new String[] { TestConfig.class.getName() }; + } + + @Override + public Set determineImports(AnnotationMetadata metadata) { + return Collections.singleton(TestConfig.class.getName()); + } + + } + + @Configuration + static class TestConfig { + + } + } diff --git a/spring-boot/src/main/java/org/springframework/boot/context/annotation/DeterminableImports.java b/spring-boot/src/main/java/org/springframework/boot/context/annotation/DeterminableImports.java new file mode 100644 index 0000000000..0eb1c3ede8 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/context/annotation/DeterminableImports.java @@ -0,0 +1,60 @@ +/* + * Copyright 2012-2017 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 + * + * http://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.boot.context.annotation; + +import java.util.Set; + +import org.springframework.beans.factory.Aware; +import org.springframework.context.ApplicationContext; +import org.springframework.context.annotation.ImportBeanDefinitionRegistrar; +import org.springframework.context.annotation.ImportSelector; +import org.springframework.core.type.AnnotationMetadata; + +/** + * Interface that can be implemented by {@link ImportSelector} and + * {@link ImportBeanDefinitionRegistrar} implementations when they can determine imports + * early. The {@link ImportSelector} and {@link ImportBeanDefinitionRegistrar} interfaces + * are quite flexible which can make it hard to tell exactly what bean definitions they + * will add. This interface should be used when an implementation consistently result in + * the same imports, given the same source. + *

+ * Using {@link DeterminableImports} is particularly useful when working with Spring's + * testing support. It allows for better generation of {@link ApplicationContext} cache + * keys. + * + * @author Phillip Webb + * @author Andy Wilkinson + * @since 1.5.0 + */ +public interface DeterminableImports { + + /** + * Return a set of objects that represent the imports. Objects within the returned + * {@code Set} must implement a valid {@link Object#hashCode() hashCode} and + * {@link Object#equals(Object) equals}. + *

+ * Imports from multiple {@link DeterminableImports} instances may be combined by the + * caller to create a complete set. + *

+ * Unlike {@link ImportSelector} and {@link ImportBeanDefinitionRegistrar} any + * {@link Aware} callbacks will not be invoked before this method is called. + * @param metadata the source meta-data + * @return a key representing the annotations that actually drive the import + */ + Set determineImports(AnnotationMetadata metadata); + +} diff --git a/spring-boot/src/main/java/org/springframework/boot/context/annotation/package-info.java b/spring-boot/src/main/java/org/springframework/boot/context/annotation/package-info.java new file mode 100644 index 0000000000..614959bf31 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/context/annotation/package-info.java @@ -0,0 +1,21 @@ +/* + * Copyright 2012-2017 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 + * + * http://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. + */ + +/** + * Classes related to Spring's {@link org.springframework.context.ApplicationContext} + * annotations. + */ +package org.springframework.boot.context.annotation;