AspectJ bean pointcut supports qualifier match

Issue: SPR-11217

spring-aop/src/main/java/org/springframework/aop/aspectj/AspectJExpressionPointcut.java

@@ -55,6 +55,7 @@ import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryAware;
 import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.annotation.BeanFactoryAnnotationUtils;
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.util.ClassUtils;
 import org.springframework.util.ObjectUtils;
@@ -215,7 +216,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 		PointcutParser parser = PointcutParser
 				.getPointcutParserSupportingSpecifiedPrimitivesAndUsingSpecifiedClassLoaderForResolution(
 						SUPPORTED_PRIMITIVES, cl);
-		parser.registerPointcutDesignatorHandler(new BeanNamePointcutDesignatorHandler());
+		parser.registerPointcutDesignatorHandler(new BeanPointcutDesignatorHandler());
 		return parser;
 	}
 
@@ -521,7 +522,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 	 * automatically by examining a thread local variable and therefore a matching
 	 * context need not be set on the pointcut.
 	 */
-	private class BeanNamePointcutDesignatorHandler implements PointcutDesignatorHandler {
+	private class BeanPointcutDesignatorHandler implements PointcutDesignatorHandler {
 
 		private static final String BEAN_DESIGNATOR_NAME = "bean";
 
@@ -532,7 +533,7 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 
 		@Override
 		public ContextBasedMatcher parse(String expression) {
-			return new BeanNameContextMatcher(expression);
+			return new BeanContextMatcher(expression);
 		}
 	}
 
@@ -544,11 +545,11 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 	 * For static match tests, this matcher abstains to allow the overall
 	 * pointcut to match even when negation is used with the bean() pointcut.
 	 */
-	private class BeanNameContextMatcher implements ContextBasedMatcher {
+	private class BeanContextMatcher implements ContextBasedMatcher {
 
 		private final NamePattern expressionPattern;
 
-		public BeanNameContextMatcher(String expression) {
+		public BeanContextMatcher(String expression) {
 			this.expressionPattern = new NamePattern(expression);
 		}
 
@@ -593,27 +594,16 @@ public class AspectJExpressionPointcut extends AbstractExpressionPointcut
 			if (targetType != null) {
 				boolean isFactory = FactoryBean.class.isAssignableFrom(targetType);
 				return FuzzyBoolean.fromBoolean(
-						matchesBeanName(isFactory ? BeanFactory.FACTORY_BEAN_PREFIX + advisedBeanName : advisedBeanName));
+						matchesBean(isFactory ? BeanFactory.FACTORY_BEAN_PREFIX + advisedBeanName : advisedBeanName));
 			}
 			else {
-				return FuzzyBoolean.fromBoolean(matchesBeanName(advisedBeanName) ||
-						matchesBeanName(BeanFactory.FACTORY_BEAN_PREFIX + advisedBeanName));
+				return FuzzyBoolean.fromBoolean(matchesBean(advisedBeanName) ||
+						matchesBean(BeanFactory.FACTORY_BEAN_PREFIX + advisedBeanName));
 			}
 		}
 
-		private boolean matchesBeanName(String advisedBeanName) {
-			if (this.expressionPattern.matches(advisedBeanName)) {
-				return true;
-			}
-			if (beanFactory != null) {
-				String[] aliases = beanFactory.getAliases(advisedBeanName);
-				for (String alias : aliases) {
-					if (this.expressionPattern.matches(alias)) {
-						return true;
-					}
-				}
-			}
-			return false;
+		private boolean matchesBean(String advisedBeanName) {
+			return BeanFactoryAnnotationUtils.isQualifierMatch(this.expressionPattern::matches, advisedBeanName, beanFactory);
 		}
 	}
 

spring-beans/src/main/java/org/springframework/beans/factory/annotation/BeanFactoryAnnotationUtils.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.
@@ -17,28 +17,32 @@
 package org.springframework.beans.factory.annotation;
 
 import java.lang.reflect.Method;
+import java.util.function.Predicate;
 
+import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.BeanFactoryUtils;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
+import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
 import org.springframework.beans.factory.config.BeanDefinition;
+import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.support.AbstractBeanDefinition;
 import org.springframework.beans.factory.support.AutowireCandidateQualifier;
 import org.springframework.beans.factory.support.RootBeanDefinition;
 import org.springframework.core.annotation.AnnotationUtils;
-import org.springframework.util.ObjectUtils;
+import org.springframework.util.Assert;
 
 /**
  * Convenience methods performing bean lookups related to annotations, for example
  * Spring's {@link Qualifier @Qualifier} annotation.
  *
- * @author Chris Beams
  * @author Juergen Hoeller
+ * @author Chris Beams
  * @since 3.1.2
  * @see BeanFactoryUtils
  */
-public class BeanFactoryAnnotationUtils {
+public abstract class BeanFactoryAnnotationUtils {
 
 	/**
 	 * Obtain a bean of type {@code T} from the given {@code BeanFactory} declaring a
@@ -48,9 +52,16 @@ public class BeanFactoryAnnotationUtils {
 	 * @param beanType the type of bean to retrieve
 	 * @param qualifier the qualifier for selecting between multiple bean matches
 	 * @return the matching bean of type {@code T} (never {@code null})
+	 * @throws NoUniqueBeanDefinitionException if multiple matching beans of type {@code T} found
 	 * @throws NoSuchBeanDefinitionException if no matching bean of type {@code T} found
+	 * @throws BeansException if the bean could not be created
+	 * @see BeanFactory#getBean(Class)
 	 */
-	public static <T> T qualifiedBeanOfType(BeanFactory beanFactory, Class<T> beanType, String qualifier) {
+	public static <T> T qualifiedBeanOfType(BeanFactory beanFactory, Class<T> beanType, String qualifier)
+			throws BeansException {
+
+		Assert.notNull(beanFactory, "BeanFactory must not be null");
+
 		if (beanFactory instanceof ConfigurableListableBeanFactory) {
 			// Full qualifier matching supported.
 			return qualifiedBeanOfType((ConfigurableListableBeanFactory) beanFactory, beanType, qualifier);
@@ -74,16 +85,14 @@ public class BeanFactoryAnnotationUtils {
 	 * @param beanType the type of bean to retrieve
 	 * @param qualifier the qualifier for selecting between multiple bean matches
 	 * @return the matching bean of type {@code T} (never {@code null})
-	 * @throws NoSuchBeanDefinitionException if no matching bean of type {@code T} found
 	 */
 	private static <T> T qualifiedBeanOfType(ConfigurableListableBeanFactory bf, Class<T> beanType, String qualifier) {
 		String[] candidateBeans = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(bf, beanType);
 		String matchingBean = null;
 		for (String beanName : candidateBeans) {
-			if (isQualifierMatch(qualifier, beanName, bf)) {
+			if (isQualifierMatch(qualifier::equals, beanName, bf)) {
 				if (matchingBean != null) {
-					throw new NoSuchBeanDefinitionException(qualifier, "No unique " + beanType.getSimpleName() +
-							" bean found for qualifier '" + qualifier + "'");
+					throw new NoUniqueBeanDefinitionException(beanType, matchingBean, beanName);
 				}
 				matchingBean = beanName;
 			}
@@ -105,40 +114,54 @@ public class BeanFactoryAnnotationUtils {
 	 * Check whether the named bean declares a qualifier of the given name.
 	 * @param qualifier the qualifier to match
 	 * @param beanName the name of the candidate bean
-	 * @param bf the {@code BeanFactory} from which to retrieve the named bean
+	 * @param beanFactory the {@code BeanFactory} from which to retrieve the named bean
 	 * @return {@code true} if either the bean definition (in the XML case)
 	 * or the bean's factory method (in the {@code @Bean} case) defines a matching
 	 * qualifier value (through {@code <qualifier>} or {@code @Qualifier})
+	 * @since 5.0
 	 */
-	private static boolean isQualifierMatch(String qualifier, String beanName, ConfigurableListableBeanFactory bf) {
-		if (bf.containsBean(beanName)) {
+	public static boolean isQualifierMatch(Predicate<String> qualifier, String beanName, BeanFactory beanFactory) {
+		// Try quick bean name or alias match first...
+		if (qualifier.test(beanName)) {
+			return true;
+		}
+		if (beanFactory != null) {
+			for (String alias : beanFactory.getAliases(beanName)) {
+				if (qualifier.test(alias)) {
+					return true;
+				}
+			}
 			try {
-				BeanDefinition bd = bf.getMergedBeanDefinition(beanName);
-				// Explicit qualifier metadata on bean definition? (typically in XML definition)
-				if (bd instanceof AbstractBeanDefinition) {
-					AbstractBeanDefinition abd = (AbstractBeanDefinition) bd;
-					AutowireCandidateQualifier candidate = abd.getQualifier(Qualifier.class.getName());
-					if ((candidate != null && qualifier.equals(candidate.getAttribute(AutowireCandidateQualifier.VALUE_KEY))) ||
-							qualifier.equals(beanName) || ObjectUtils.containsElement(bf.getAliases(beanName), qualifier)) {
-						return true;
+				if (beanFactory instanceof ConfigurableBeanFactory) {
+					BeanDefinition bd = ((ConfigurableBeanFactory) beanFactory).getMergedBeanDefinition(beanName);
+					// Explicit qualifier metadata on bean definition? (typically in XML definition)
+					if (bd instanceof AbstractBeanDefinition) {
+						AbstractBeanDefinition abd = (AbstractBeanDefinition) bd;
+						AutowireCandidateQualifier candidate = abd.getQualifier(Qualifier.class.getName());
+						if (candidate != null) {
+							Object value = candidate.getAttribute(AutowireCandidateQualifier.VALUE_KEY);
+							if (value != null && qualifier.test(value.toString())) {
+								return true;
+							}
+						}
 					}
-				}
-				// Corresponding qualifier on factory method? (typically in configuration class)
-				if (bd instanceof RootBeanDefinition) {
-					Method factoryMethod = ((RootBeanDefinition) bd).getResolvedFactoryMethod();
-					if (factoryMethod != null) {
-						Qualifier targetAnnotation = AnnotationUtils.getAnnotation(factoryMethod, Qualifier.class);
-						if (targetAnnotation != null) {
-							return qualifier.equals(targetAnnotation.value());
+					// Corresponding qualifier on factory method? (typically in configuration class)
+					if (bd instanceof RootBeanDefinition) {
+						Method factoryMethod = ((RootBeanDefinition) bd).getResolvedFactoryMethod();
+						if (factoryMethod != null) {
+							Qualifier targetAnnotation = AnnotationUtils.getAnnotation(factoryMethod, Qualifier.class);
+							if (targetAnnotation != null) {
+								return qualifier.test(targetAnnotation.value());
+							}
 						}
 					}
 				}
 				// Corresponding qualifier on bean implementation class? (for custom user types)
-				Class<?> beanType = bf.getType(beanName);
+				Class<?> beanType = beanFactory.getType(beanName);
 				if (beanType != null) {
 					Qualifier targetAnnotation = AnnotationUtils.getAnnotation(beanType, Qualifier.class);
 					if (targetAnnotation != null) {
-						return qualifier.equals(targetAnnotation.value());
+						return qualifier.test(targetAnnotation.value());
 					}
 				}
 			}

spring-context/src/test/java/org/springframework/aop/aspectj/BeanNamePointcutTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 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.
@@ -69,6 +69,7 @@ public class BeanNamePointcutTests {
 		counterAspect.reset();
 	}
 
+
 	// We don't need to test all combination of pointcuts due to BeanNamePointcutMatchingTests
 
 	@Test

spring-context/src/test/resources/org/springframework/aop/aspectj/BeanNamePointcutTests.xml

@@ -21,9 +21,11 @@
 		</aop:aspect>
 	</aop:config>
 
-	<bean id="testBean1" name="myBean" class="org.springframework.tests.sample.beans.TestBean"/>
+	<bean id="tb1" name="testBean1" class="org.springframework.tests.sample.beans.TestBean">
+		<qualifier value="myBean"/>
+	</bean>
 
-	<bean id="testBean2" class="org.springframework.tests.sample.beans.TestBean"/>
+	<bean id="tb2" name="testBean2" class="org.springframework.tests.sample.beans.TestBean"/>
 
 	<bean id="testBeanContainingNestedBean" class="org.springframework.tests.sample.beans.TestBean">
 		<property name="doctor">
@@ -50,7 +52,7 @@
 	<bean id="counterAspect" class="org.springframework.aop.aspectj.Counter"/>
 
 	<aop:config>
-     <aop:advisor pointcut="bean(*This) and !bean(dont*)" advice-ref="testInterceptor"/>
+		<aop:advisor pointcut="bean(*This) and !bean(dont*)" advice-ref="testInterceptor"/>
 	</aop:config>
 
 	<bean id="interceptThis" class="org.springframework.tests.sample.beans.TestBean"/>