Consistent meta-annotation attributes lookup through ASM

Issue: SPR-14257

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

spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java

@@ -1749,7 +1749,7 @@ public abstract class AnnotationUtils {
 	}
 
 	/**
-	 * <p>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.
 	 * <p>Otherwise, this method does nothing.

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<String> metaAnnotationTypeNames = new LinkedHashSet<String>();
+		Set<Annotation> visited = new LinkedHashSet<Annotation>();
 		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<String> metaAnnotationTypeNames = new LinkedHashSet<String>(visited.size());
+			for (Annotation ann : visited) {
+				metaAnnotationTypeNames.add(ann.annotationType().getName());
+			}
 			this.metaAnnotationMap.put(annotationClass.getName(), metaAnnotationTypeNames);
 		}
 	}
 
-	private void recursivelyCollectMetaAnnotations(Set<String> 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
+	private void recursivelyCollectMetaAnnotations(Set<Annotation> 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);
+				}
+			}
 		}
 	}
 

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);
 						}
 					}

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())) {