From 24f5f368b015f58b6f5a85a7c982d9f476681712 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 30 May 2016 22:39:27 +0200 Subject: [PATCH] Consistent meta-annotation attributes lookup through ASM Issue: SPR-14257 --- .../ConfigurationClassWithConditionTests.java | 64 ++++++++++++++----- .../annotation/AnnotatedElementUtils.java | 2 +- .../core/annotation/AnnotationUtils.java | 2 +- .../AnnotationAttributesReadingVisitor.java | 33 ++++++---- .../AnnotationReadingVisitorUtils.java | 7 +- .../RecursiveAnnotationAttributesVisitor.java | 4 +- 6 files changed, 78 insertions(+), 34 deletions(-) diff --git a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassWithConditionTests.java b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassWithConditionTests.java index 312baf1cdee..8d55a9f03f3 100644 --- a/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassWithConditionTests.java +++ b/spring-context/src/test/java/org/springframework/context/annotation/ConfigurationClassWithConditionTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -22,12 +22,10 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; import java.util.Map; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.support.BeanDefinitionRegistry; +import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.core.annotation.AnnotationAttributes; import org.springframework.core.type.AnnotatedTypeMetadata; import org.springframework.core.type.AnnotationMetadata; @@ -45,9 +43,6 @@ import static org.junit.Assert.*; @SuppressWarnings("resource") public class ConfigurationClassWithConditionTests { - @Rule - public ExpectedException thrown = ExpectedException.none(); - @Test public void conditionalOnMissingBeanMatch() throws Exception { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); @@ -94,13 +89,28 @@ public class ConfigurationClassWithConditionTests { assertTrue(ctx.containsBean("bean")); } + @Test + public void metaConditionalWithAsm() throws Exception { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.registerBeanDefinition("config", new RootBeanDefinition(ConfigurationWithMetaCondition.class.getName())); + ctx.refresh(); + assertTrue(ctx.containsBean("bean")); + } + @Test public void nonConfigurationClass() throws Exception { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(NonConfigurationClass.class); ctx.refresh(); - thrown.expect(NoSuchBeanDefinitionException.class); - assertNull(ctx.getBean(NonConfigurationClass.class)); + assertFalse(ctx.containsBean("bean1")); + } + + @Test + public void nonConfigurationClassWithAsm() throws Exception { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.registerBeanDefinition("config", new RootBeanDefinition(NonConfigurationClass.class.getName())); + ctx.refresh(); + assertFalse(ctx.containsBean("bean1")); } @Test @@ -108,8 +118,15 @@ public class ConfigurationClassWithConditionTests { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); ctx.register(ConditionOnMethodConfiguration.class); ctx.refresh(); - thrown.expect(NoSuchBeanDefinitionException.class); - assertNull(ctx.getBean(ExampleBean.class)); + assertFalse(ctx.containsBean("bean1")); + } + + @Test + public void methodConditionalWithAsm() throws Exception { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.registerBeanDefinition("config", new RootBeanDefinition(ConditionOnMethodConfiguration.class.getName())); + ctx.refresh(); + assertFalse(ctx.containsBean("bean1")); } @Test @@ -144,6 +161,7 @@ public class ConfigurationClassWithConditionTests { @Configuration static class BeanOneConfiguration { + @Bean public ExampleBean bean1() { return new ExampleBean(); @@ -153,6 +171,7 @@ public class ConfigurationClassWithConditionTests { @Configuration @Conditional(NoBeanOneCondition.class) static class BeanTwoConfiguration { + @Bean public ExampleBean bean2() { return new ExampleBean(); @@ -162,6 +181,7 @@ public class ConfigurationClassWithConditionTests { @Configuration @Conditional(HasBeanOneCondition.class) static class BeanThreeConfiguration { + @Bean public ExampleBean bean3() { return new ExampleBean(); @@ -171,6 +191,7 @@ public class ConfigurationClassWithConditionTests { @Configuration @MetaConditional("test") static class ConfigurationWithMetaCondition { + @Bean public ExampleBean bean() { return new ExampleBean(); @@ -180,14 +201,22 @@ public class ConfigurationClassWithConditionTests { @Conditional(MetaConditionalFilter.class) @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) - public static @interface MetaConditional { + public @interface MetaConditional { + String value(); } @Conditional(NeverCondition.class) @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE, ElementType.METHOD}) - public static @interface Never { + public @interface Never { + } + + @Conditional(AlwaysCondition.class) + @Never + @Retention(RetentionPolicy.RUNTIME) + @Target({ElementType.TYPE, ElementType.METHOD}) + public @interface MetaNever { } static class NoBeanOneCondition implements Condition { @@ -238,8 +267,13 @@ public class ConfigurationClassWithConditionTests { } @Component - @Never + @MetaNever static class NonConfigurationClass { + + @Bean + public ExampleBean bean1() { + return new ExampleBean(); + } } @Configuration @@ -254,7 +288,7 @@ public class ConfigurationClassWithConditionTests { @Configuration @Never - @Import({ ConfigurationNotCreated.class, RegistrarNotCreated.class, ImportSelectorNotCreated.class }) + @Import({ConfigurationNotCreated.class, RegistrarNotCreated.class, ImportSelectorNotCreated.class}) static class ImportsNotCreated { static { if (true) throw new RuntimeException(); diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java index 79fbe118e86..b2fc6e27ea4 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotatedElementUtils.java @@ -900,7 +900,7 @@ public class AnnotatedElementUtils { try { return searchWithGetSemantics(element, annotationType, annotationName, containerType, processor, - new HashSet(), 0); + new HashSet(), 0); } catch (Throwable ex) { AnnotationUtils.rethrowAnnotationConfigurationException(ex); diff --git a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java index 4bd6f63bf2c..3ea51230328 100644 --- a/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java +++ b/spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java @@ -1749,7 +1749,7 @@ public abstract class AnnotationUtils { } /** - *

If the supplied throwable is an {@link AnnotationConfigurationException}, + * If the supplied throwable is an {@link AnnotationConfigurationException}, * it will be cast to an {@code AnnotationConfigurationException} and thrown, * allowing it to propagate to the caller. *

Otherwise, this method does nothing. diff --git a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationAttributesReadingVisitor.java b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationAttributesReadingVisitor.java index 6937e811e48..ab001b50ecb 100644 --- a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationAttributesReadingVisitor.java +++ b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationAttributesReadingVisitor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 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. @@ -72,32 +72,43 @@ final class AnnotationAttributesReadingVisitor extends RecursiveAnnotationAttrib else { attributes.add(0, this.attributes); } - Set metaAnnotationTypeNames = new LinkedHashSet(); + Set visited = new LinkedHashSet(); Annotation[] metaAnnotations = AnnotationUtils.getAnnotations(annotationClass); if (!ObjectUtils.isEmpty(metaAnnotations)) { for (Annotation metaAnnotation : metaAnnotations) { if (!AnnotationUtils.isInJavaLangAnnotationPackage(metaAnnotation)) { - recursivelyCollectMetaAnnotations(metaAnnotationTypeNames, metaAnnotation); + recursivelyCollectMetaAnnotations(visited, metaAnnotation); } } } if (this.metaAnnotationMap != null) { + Set metaAnnotationTypeNames = new LinkedHashSet(visited.size()); + for (Annotation ann : visited) { + metaAnnotationTypeNames.add(ann.annotationType().getName()); + } this.metaAnnotationMap.put(annotationClass.getName(), metaAnnotationTypeNames); } } - private void recursivelyCollectMetaAnnotations(Set visited, Annotation annotation) { - String annotationName = annotation.annotationType().getName(); - if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation) && visited.add(annotationName)) { - // Only do further scanning for public annotations; we'd run into - // IllegalAccessExceptions otherwise, and we don't want to mess with - // accessibility in a SecurityManager environment. - if (Modifier.isPublic(annotation.annotationType().getModifiers())) { - this.attributesMap.add(annotationName, AnnotationUtils.getAnnotationAttributes(annotation, false, true)); + private void recursivelyCollectMetaAnnotations(Set visited, Annotation annotation) { + if (!AnnotationUtils.isInJavaLangAnnotationPackage(annotation) && visited.add(annotation)) { + try { + // Only do attribute scanning for public annotations; we'd run into + // IllegalAccessExceptions otherwise, and we don't want to mess with + // accessibility in a SecurityManager environment. + if (Modifier.isPublic(annotation.annotationType().getModifiers())) { + String annotationName = annotation.annotationType().getName(); + this.attributesMap.add(annotationName, AnnotationUtils.getAnnotationAttributes(annotation, false, true)); + } for (Annotation metaMetaAnnotation : annotation.annotationType().getAnnotations()) { recursivelyCollectMetaAnnotations(visited, metaMetaAnnotation); } } + catch (Exception ex) { + if (logger.isDebugEnabled()) { + logger.debug("Failed to introspect meta-annotations on [" + annotation + "]: " + ex); + } + } } } diff --git a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java index d63c72ffa3c..0d7ba35f259 100644 --- a/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java +++ b/spring-core/src/main/java/org/springframework/core/type/classreading/AnnotationReadingVisitorUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -150,9 +150,8 @@ abstract class AnnotationReadingVisitorUtils { for (String overridableAttributeName : overridableAttributeNames) { Object value = currentAttributes.get(overridableAttributeName); if (value != null) { - // Store the value, potentially overriding a value from an - // attribute of the same name found higher in the annotation - // hierarchy. + // Store the value, potentially overriding a value from an attribute + // of the same name found higher in the annotation hierarchy. results.put(overridableAttributeName, value); } } diff --git a/spring-core/src/main/java/org/springframework/core/type/classreading/RecursiveAnnotationAttributesVisitor.java b/spring-core/src/main/java/org/springframework/core/type/classreading/RecursiveAnnotationAttributesVisitor.java index 2f1be81db2c..0d2176f015e 100644 --- a/spring-core/src/main/java/org/springframework/core/type/classreading/RecursiveAnnotationAttributesVisitor.java +++ b/spring-core/src/main/java/org/springframework/core/type/classreading/RecursiveAnnotationAttributesVisitor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 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. @@ -58,7 +58,7 @@ class RecursiveAnnotationAttributesVisitor extends AbstractRecursiveAnnotationVi } private void registerDefaultValues(Class annotationClass) { - // Only do further scanning for public annotations; we'd run into + // Only do defaults scanning for public annotations; we'd run into // IllegalAccessExceptions otherwise, and we don't want to mess with // accessibility in a SecurityManager environment. if (Modifier.isPublic(annotationClass.getModifiers())) {