Make Mock/Spy qualifiers part of context cache key

Refine @MockBean/@SpyBean qualifier support so that qualifiers form part
of the context cache key. Prior to this commit is was possible that two
different tests could accidentally share the same context if they
defined the same @Mock but with different @Qualifiers.

See gh-6753

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/Definition.java

@@ -16,8 +16,6 @@
 
 package org.springframework.boot.test.mock.mockito;
 
-import java.lang.reflect.AnnotatedElement;
-
 import org.springframework.util.ObjectUtils;
 
 /**
@@ -30,28 +28,20 @@ abstract class Definition {
 
 	private static final int MULTIPLIER = 31;
 
-	private final AnnotatedElement element;
-
 	private final String name;
 
 	private final MockReset reset;
 
 	private final boolean proxyTargetAware;
 
-	Definition(AnnotatedElement element, String name, MockReset reset,
-			boolean proxyTargetAware) {
-		this.element = element;
+	private final QualifierDefinition qualifier;
+
+	Definition(String name, MockReset reset, boolean proxyTargetAware,
+			QualifierDefinition qualifier) {
 		this.name = name;
 		this.reset = (reset != null ? reset : MockReset.AFTER);
 		this.proxyTargetAware = proxyTargetAware;
-	}
-
-	/**
-	 * Return the {@link AnnotatedElement} that holds this definition.
-	 * @return the element that defines this definition or {@code null}
-	 */
-	public AnnotatedElement getElement() {
-		return this.element;
+		this.qualifier = qualifier;
 	}
 
 	/**
@@ -78,6 +68,14 @@ abstract class Definition {
 		return this.proxyTargetAware;
 	}
 
+	/**
+	 * Return the qualifier or {@code null}.
+	 * @return the qualifier
+	 */
+	public QualifierDefinition getQualifier() {
+		return this.qualifier;
+	}
+
 	@Override
 	public int hashCode() {
 		int result = 1;
@@ -85,6 +83,7 @@ abstract class Definition {
 		result = MULTIPLIER * result + ObjectUtils.nullSafeHashCode(this.reset);
 		result = MULTIPLIER * result
 				+ ObjectUtils.nullSafeHashCode(this.proxyTargetAware);
+		result = MULTIPLIER * result + ObjectUtils.nullSafeHashCode(this.qualifier);
 		return result;
 	}
 
@@ -102,6 +101,7 @@ abstract class Definition {
 		result &= ObjectUtils.nullSafeEquals(this.reset, other.reset);
 		result &= ObjectUtils.nullSafeEquals(this.proxyTargetAware,
 				other.proxyTargetAware);
+		result &= ObjectUtils.nullSafeEquals(this.qualifier, other.qualifier);
 		return result;
 	}
 

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/DefinitionsParser.java

@@ -91,10 +91,11 @@ class DefinitionsParser {
 					"The name attribute can only be used when mocking a single class");
 		}
 		for (ResolvableType typeToMock : typesToMock) {
-			MockDefinition definition = new MockDefinition(element, annotation.name(),
-					typeToMock, annotation.extraInterfaces(), annotation.answer(),
-					annotation.serializable(), annotation.reset());
-			addDefinition(definition, "mock");
+			MockDefinition definition = new MockDefinition(annotation.name(), typeToMock,
+					annotation.extraInterfaces(), annotation.answer(),
+					annotation.serializable(), annotation.reset(),
+					QualifierDefinition.forElement(element));
+			addDefinition(element, definition, "mock");
 		}
 	}
 
@@ -107,16 +108,17 @@ class DefinitionsParser {
 					"The name attribute can only be used when spying a single class");
 		}
 		for (ResolvableType typeToSpy : typesToSpy) {
-			SpyDefinition definition = new SpyDefinition(element, annotation.name(),
-					typeToSpy, annotation.reset(), annotation.proxyTargetAware());
-			addDefinition(definition, "spy");
+			SpyDefinition definition = new SpyDefinition(annotation.name(), typeToSpy,
+					annotation.reset(), annotation.proxyTargetAware(),
+					QualifierDefinition.forElement(element));
+			addDefinition(element, definition, "spy");
 		}
 	}
 
-	private void addDefinition(Definition definition, String type) {
+	private void addDefinition(AnnotatedElement element, Definition definition,
+			String type) {
 		boolean isNewDefinition = this.definitions.add(definition);
 		Assert.state(isNewDefinition, "Duplicate " + type + " definition " + definition);
-		AnnotatedElement element = definition.getElement();
 		if (element instanceof Field) {
 			Field field = (Field) element;
 			this.definitionFields.put(definition, field);

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockDefinition.java

@@ -16,7 +16,6 @@
 
 package org.springframework.boot.test.mock.mockito;
 
-import java.lang.reflect.AnnotatedElement;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.LinkedHashSet;
@@ -50,10 +49,10 @@ class MockDefinition extends Definition {
 
 	private final boolean serializable;
 
-	MockDefinition(AnnotatedElement element, String name, ResolvableType typeToMock,
-			Class<?>[] extraInterfaces, Answers answer, boolean serializable,
-			MockReset reset) {
-		super(element, name, reset, false);
+	MockDefinition(String name, ResolvableType typeToMock, Class<?>[] extraInterfaces,
+			Answers answer, boolean serializable, MockReset reset,
+			QualifierDefinition qualifier) {
+		super(name, reset, false, qualifier);
 		Assert.notNull(typeToMock, "TypeToMock must not be null");
 		this.typeToMock = typeToMock;
 		this.extraInterfaces = asClassSet(extraInterfaces);

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/MockitoPostProcessor.java

@@ -17,7 +17,6 @@
 package org.springframework.boot.test.mock.mockito;
 
 import java.beans.PropertyDescriptor;
-import java.lang.reflect.AnnotatedElement;
 import java.lang.reflect.Field;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -44,7 +43,6 @@ import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
 import org.springframework.beans.factory.config.ConstructorArgumentValues;
 import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder;
-import org.springframework.beans.factory.config.DependencyDescriptor;
 import org.springframework.beans.factory.config.InstantiationAwareBeanPostProcessorAdapter;
 import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
@@ -209,9 +207,8 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda
 		definition.setFactoryMethodName("createMock");
 		definition.getConstructorArgumentValues().addIndexedArgumentValue(0,
 				mockDefinition);
-		AnnotatedElement element = mockDefinition.getElement();
-		if (element instanceof Field) {
-			definition.setQualifiedElement(element);
+		if (mockDefinition.getQualifier() != null) {
+			mockDefinition.getQualifier().applyTo(definition);
 		}
 		return definition;
 	}
@@ -232,17 +229,17 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda
 		if (StringUtils.hasLength(mockDefinition.getName())) {
 			return mockDefinition.getName();
 		}
-		String[] existingBeans = findCandidateBeans(beanFactory, mockDefinition);
-		if (ObjectUtils.isEmpty(existingBeans)) {
+		Set<String> existingBeans = findCandidateBeans(beanFactory, mockDefinition);
+		if (existingBeans.isEmpty()) {
 			return this.beanNameGenerator.generateBeanName(beanDefinition, registry);
 		}
-		if (existingBeans.length == 1) {
-			return existingBeans[0];
+		if (existingBeans.size() == 1) {
+			return existingBeans.iterator().next();
 		}
 		throw new IllegalStateException(
 				"Unable to register mock bean " + mockDefinition.getTypeToMock()
 						+ " expected a single matching bean to replace but found "
-						+ new TreeSet<String>(Arrays.asList(existingBeans)));
+						+ existingBeans);
 	}
 
 	private void registerSpy(ConfigurableListableBeanFactory beanFactory,
@@ -256,22 +253,17 @@ public class MockitoPostProcessor extends InstantiationAwareBeanPostProcessorAda
 		}
 	}
 
-	private String[] findCandidateBeans(ConfigurableListableBeanFactory beanFactory,
+	private Set<String> findCandidateBeans(ConfigurableListableBeanFactory beanFactory,
 			MockDefinition mockDefinition) {
-		String[] beans = getExistingBeans(beanFactory, mockDefinition.getTypeToMock());
-		// Attempt to filter using qualifiers
-		if (beans.length > 1 && mockDefinition.getElement() instanceof Field) {
-			DependencyDescriptor descriptor = new DependencyDescriptor(
-					(Field) mockDefinition.getElement(), true);
-			Set<String> candidates = new LinkedHashSet<String>();
-			for (String bean : beans) {
-				if (beanFactory.isAutowireCandidate(bean, descriptor)) {
-					candidates.add(bean);
-				}
+		QualifierDefinition qualifier = mockDefinition.getQualifier();
+		Set<String> candidates = new TreeSet<String>();
+		for (String candidate : getExistingBeans(beanFactory,
+				mockDefinition.getTypeToMock())) {
+			if (qualifier == null || qualifier.matches(beanFactory, candidate)) {
+				candidates.add(candidate);
 			}
-			return candidates.toArray(new String[candidates.size()]);
 		}
-		return beans;
+		return candidates;
 	}
 
 	private String[] getExistingBeans(ConfigurableListableBeanFactory beanFactory,

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/QualifierDefinition.java

@@ -0,0 +1,110 @@
+/*
+ * Copyright 2012-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.
+ * 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.mock.mockito;
+
+import java.lang.annotation.Annotation;
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Field;
+import java.util.HashSet;
+import java.util.Set;
+
+import org.springframework.beans.factory.annotation.Qualifier;
+import org.springframework.beans.factory.config.ConfigurableListableBeanFactory;
+import org.springframework.beans.factory.config.DependencyDescriptor;
+import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.core.annotation.AnnotationUtils;
+
+/**
+ * Definition of a Spring {@link Qualifier @Qualifier}.
+ *
+ * @author Phillip Webb
+ * @author Stephane Nicoll
+ * @see Definition
+ */
+class QualifierDefinition {
+
+	private final Field field;
+
+	private final DependencyDescriptor descriptor;
+
+	private final Set<Annotation> annotations;
+
+	QualifierDefinition(Field field, Set<Annotation> annotations) {
+		// We can't use the field or descriptor as part of the context key
+		// but we can assume that if two fields have the same qualifiers then
+		// it's safe for Spring to use either for qualifier logic
+		this.field = field;
+		this.descriptor = new DependencyDescriptor(field, true);
+		this.annotations = annotations;
+	}
+
+	public boolean matches(ConfigurableListableBeanFactory beanFactory, String beanName) {
+		return beanFactory.isAutowireCandidate(beanName, this.descriptor);
+	}
+
+	public void applyTo(RootBeanDefinition definition) {
+		definition.setQualifiedElement(this.field);
+	}
+
+	@Override
+	public int hashCode() {
+		return this.annotations.hashCode();
+	}
+
+	@Override
+	public boolean equals(Object obj) {
+		if (obj == this) {
+			return true;
+		}
+		if (obj == null || !getClass().isAssignableFrom(obj.getClass())) {
+			return false;
+		}
+		QualifierDefinition other = (QualifierDefinition) obj;
+		return this.annotations.equals(other.annotations);
+	}
+
+	public static QualifierDefinition forElement(AnnotatedElement element) {
+		if (element != null && element instanceof Field) {
+			Field field = (Field) element;
+			Set<Annotation> annotations = getQualifierAnnotations(field);
+			if (!annotations.isEmpty()) {
+				return new QualifierDefinition(field, annotations);
+			}
+		}
+		return null;
+	}
+
+	private static Set<Annotation> getQualifierAnnotations(Field field) {
+		// Assume that any annotations other than @MockBean/@SpyBean are qualifiers
+		Annotation[] candidates = field.getDeclaredAnnotations();
+		Set<Annotation> annotations = new HashSet<Annotation>(candidates.length);
+		for (Annotation candidate : candidates) {
+			if (!isMockOrSpyAnnotation(candidate)) {
+				annotations.add(candidate);
+			}
+		}
+		return annotations;
+	}
+
+	private static boolean isMockOrSpyAnnotation(Annotation candidate) {
+		Class<? extends Annotation> type = candidate.annotationType();
+		return (type.equals(MockBean.class) || type.equals(SpyBean.class)
+				|| AnnotationUtils.isAnnotationMetaPresent(type, MockBean.class)
+				|| AnnotationUtils.isAnnotationMetaPresent(type, SpyBean.class));
+	}
+
+}

spring-boot-test/src/main/java/org/springframework/boot/test/mock/mockito/SpyDefinition.java

@@ -16,8 +16,6 @@
 
 package org.springframework.boot.test.mock.mockito;
 
-import java.lang.reflect.AnnotatedElement;
-
 import org.mockito.MockSettings;
 import org.mockito.Mockito;
 import org.mockito.internal.util.MockUtil;
@@ -41,9 +39,9 @@ class SpyDefinition extends Definition {
 
 	private final ResolvableType typeToSpy;
 
-	SpyDefinition(AnnotatedElement element, String name, ResolvableType typeToSpy,
-			MockReset reset, boolean proxyTargetAware) {
-		super(element, name, reset, proxyTargetAware);
+	SpyDefinition(String name, ResolvableType typeToSpy, MockReset reset,
+			boolean proxyTargetAware, QualifierDefinition qualifier) {
+		super(name, reset, proxyTargetAware, qualifier);
 		Assert.notNull(typeToSpy, "TypeToSpy must not be null");
 		this.typeToSpy = typeToSpy;