From 457d0414ea419dbe9044343d9aa69009743a0548 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 6 Feb 2017 12:54:11 +0000 Subject: [PATCH] Make ConditionalOnBean a logical AND rather than an OR Previously ConditionalOnBean was the inverse of ConditionalOnMissingBean. This meant that the former was a logical OR while the latter was a logical AND of the requiremnts declared via the annotation's attributes. This commit changes the logic for ConditionalOnBean so that it is now a logical AND. A side-effect of this change is that more information about what has and has not matched must be tracked during the evaluation. This extra information is now used to provide more informative messages in the condition evaluation report that indicate exactly why @ConditionalOnBean or @ConditionalOnMissingBean did not match. Closes gh-5279 --- .../condition/ConditionalOnBean.java | 16 +- .../condition/OnBeanCondition.java | 210 +++++++++++++++--- .../condition/ConditionalOnBeanTests.java | 4 +- 3 files changed, 188 insertions(+), 42 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBean.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBean.java index 9b33f74e55..7a98c89e41 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBean.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBean.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. @@ -45,30 +45,30 @@ import org.springframework.context.annotation.Conditional; public @interface ConditionalOnBean { /** - * The class type of bean that should be checked. The condition matches when any of - * the classes specified is contained in the {@link ApplicationContext}. + * The class type of bean that should be checked. The condition matches when all of + * the classes specified are contained in the {@link ApplicationContext}. * @return the class types of beans to check */ Class[] value() default {}; /** - * The class type names of bean that should be checked. The condition matches when any - * of the classes specified is contained in the {@link ApplicationContext}. + * The class type names of bean that should be checked. The condition matches when all + * of the classes specified are contained in the {@link ApplicationContext}. * @return the class type names of beans to check */ String[] type() default {}; /** * The annotation type decorating a bean that should be checked. The condition matches - * when any of the annotations specified is defined on a bean in the + * when all of the annotations specified are defined on beans in the * {@link ApplicationContext}. * @return the class-level annotation types to check */ Class[] annotation() default {}; /** - * The names of beans to check. The condition matches when any of the bean names - * specified is contained in the {@link ApplicationContext}. + * The names of beans to check. The condition matches when all of the bean names + * specified are contained in the {@link ApplicationContext}. * @return the name of beans to check */ String[] name() default {}; diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java index 06a75016b2..a6dad49ffc 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/OnBeanCondition.java @@ -21,8 +21,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; import org.springframework.beans.factory.BeanFactory; @@ -76,43 +79,47 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit if (metadata.isAnnotated(ConditionalOnBean.class.getName())) { BeanSearchSpec spec = new BeanSearchSpec(context, metadata, ConditionalOnBean.class); - List matching = getMatchingBeans(context, spec); - if (matching.isEmpty()) { - return ConditionOutcome.noMatch( - ConditionMessage.forCondition(ConditionalOnBean.class, spec) - .didNotFind("any beans").atAll()); + MatchResult matchResult = getMatchingBeans(context, spec); + if (!matchResult.isAllMatched()) { + String reason = createOnBeanNoMatchReason(matchResult); + return ConditionOutcome.noMatch(ConditionMessage + .forCondition(ConditionalOnBean.class, spec).because(reason)); } matchMessage = matchMessage.andCondition(ConditionalOnBean.class, spec) - .found("bean", "beans").items(Style.QUOTE, matching); + .found("bean", "beans") + .items(Style.QUOTE, matchResult.getNamesOfAllMatches()); } if (metadata.isAnnotated(ConditionalOnSingleCandidate.class.getName())) { BeanSearchSpec spec = new SingleCandidateBeanSearchSpec(context, metadata, ConditionalOnSingleCandidate.class); - List matching = getMatchingBeans(context, spec); - if (matching.isEmpty()) { + MatchResult matchResult = getMatchingBeans(context, spec); + if (!matchResult.isAllMatched()) { return ConditionOutcome.noMatch(ConditionMessage .forCondition(ConditionalOnSingleCandidate.class, spec) .didNotFind("any beans").atAll()); } - else if (!hasSingleAutowireCandidate(context.getBeanFactory(), matching, + else if (!hasSingleAutowireCandidate(context.getBeanFactory(), + matchResult.getNamesOfAllMatches(), spec.getStrategy() == SearchStrategy.ALL)) { return ConditionOutcome.noMatch(ConditionMessage .forCondition(ConditionalOnSingleCandidate.class, spec) .didNotFind("a primary bean from beans") - .items(Style.QUOTE, matching)); + .items(Style.QUOTE, matchResult.getNamesOfAllMatches())); } matchMessage = matchMessage .andCondition(ConditionalOnSingleCandidate.class, spec) - .found("a primary bean from beans").items(Style.QUOTE, matching); + .found("a primary bean from beans") + .items(Style.QUOTE, matchResult.namesOfAllMatches); } if (metadata.isAnnotated(ConditionalOnMissingBean.class.getName())) { BeanSearchSpec spec = new BeanSearchSpec(context, metadata, ConditionalOnMissingBean.class); - List matching = getMatchingBeans(context, spec); - if (!matching.isEmpty()) { + MatchResult matchResult = getMatchingBeans(context, spec); + if (matchResult.isAnyMatched()) { + String reason = createOnMissingBeanNoMatchReason(matchResult); return ConditionOutcome.noMatch(ConditionMessage .forCondition(ConditionalOnMissingBean.class, spec) - .found("bean", "beans").items(Style.QUOTE, matching)); + .because(reason)); } matchMessage = matchMessage.andCondition(ConditionalOnMissingBean.class, spec) .didNotFind("any beans").atAll(); @@ -120,8 +127,62 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit return ConditionOutcome.match(matchMessage); } - private List getMatchingBeans(ConditionContext context, - BeanSearchSpec beans) { + private String createOnBeanNoMatchReason(MatchResult matchResult) { + StringBuilder reason = new StringBuilder(); + appendMessageForNoMatches(reason, matchResult.unmatchedAnnotations, + "annotated with"); + appendMessageForNoMatches(reason, matchResult.unmatchedTypes, "of type"); + appendMessageForNoMatches(reason, matchResult.unmatchedNames, "named"); + return reason.toString(); + } + + private void appendMessageForNoMatches(StringBuilder reason, + Collection unmatched, String description) { + if (!unmatched.isEmpty()) { + if (reason.length() > 0) { + reason.append(" and "); + } + reason.append("did not find any beans "); + reason.append(description); + reason.append(" "); + reason.append(StringUtils.collectionToDelimitedString(unmatched, ", ")); + } + } + + private String createOnMissingBeanNoMatchReason(MatchResult matchResult) { + StringBuilder reason = new StringBuilder(); + appendMessageForMatches(reason, matchResult.matchedAnnotations, "annotated with"); + appendMessageForMatches(reason, matchResult.matchedTypes, "of type"); + if (!matchResult.matchedNames.isEmpty()) { + if (reason.length() > 0) { + reason.append(" and "); + } + reason.append("found beans named "); + reason.append(StringUtils + .collectionToDelimitedString(matchResult.matchedNames, ", ")); + } + return reason.toString(); + } + + private void appendMessageForMatches(StringBuilder reason, + Map> matches, String description) { + if (!matches.isEmpty()) { + for (Map.Entry> match : matches.entrySet()) { + if (reason.length() > 0) { + reason.append(" and "); + } + reason.append("found beans "); + reason.append(description); + reason.append("'"); + reason.append(match.getKey()); + reason.append("'"); + reason.append( + StringUtils.collectionToDelimitedString(match.getValue(), ", ")); + } + } + } + + private MatchResult getMatchingBeans(ConditionContext context, BeanSearchSpec beans) { ConfigurableListableBeanFactory beanFactory = context.getBeanFactory(); if (beans.getStrategy() == SearchStrategy.ANCESTORS) { BeanFactory parent = beanFactory.getParentBeanFactory(); @@ -129,27 +190,52 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit "Unable to use SearchStrategy.PARENTS"); beanFactory = (ConfigurableListableBeanFactory) parent; } - if (beanFactory == null) { - return Collections.emptyList(); - } - List beanNames = new ArrayList(); + MatchResult matchResult = new MatchResult(); boolean considerHierarchy = beans.getStrategy() != SearchStrategy.CURRENT; + List beansIgnoredByType = getNamesOfBeansIgnoredByType( + beans.getIgnoredTypes(), beanFactory, context, considerHierarchy); for (String type : beans.getTypes()) { - beanNames.addAll(getBeanNamesForType(beanFactory, type, - context.getClassLoader(), considerHierarchy)); - } - for (String ignoredType : beans.getIgnoredTypes()) { - beanNames.removeAll(getBeanNamesForType(beanFactory, ignoredType, - context.getClassLoader(), considerHierarchy)); + Collection typeMatches = getBeanNamesForType(beanFactory, type, + context.getClassLoader(), considerHierarchy); + typeMatches.removeAll(beansIgnoredByType); + if (typeMatches.isEmpty()) { + matchResult.recordUnmatchedType(type); + } + else { + matchResult.recordMatchedType(type, typeMatches); + } } for (String annotation : beans.getAnnotations()) { - beanNames.addAll(Arrays.asList(getBeanNamesForAnnotation(beanFactory, - annotation, context.getClassLoader(), considerHierarchy))); + List annotationMatches = Arrays + .asList(getBeanNamesForAnnotation(beanFactory, annotation, + context.getClassLoader(), considerHierarchy)); + annotationMatches.removeAll(beansIgnoredByType); + if (annotationMatches.isEmpty()) { + matchResult.recordUnmatchedAnnotation(annotation); + } + else { + matchResult.recordMatchedAnnotation(annotation, annotationMatches); + } } for (String beanName : beans.getNames()) { - if (containsBean(beanFactory, beanName, considerHierarchy)) { - beanNames.add(beanName); + if (!beansIgnoredByType.contains(beanName) + && containsBean(beanFactory, beanName, considerHierarchy)) { + matchResult.recordMatchedName(beanName); } + else { + matchResult.recordUnmatchedName(beanName); + } + } + return matchResult; + } + + private List getNamesOfBeansIgnoredByType(List ignoredTypes, + ListableBeanFactory beanFactory, ConditionContext context, + boolean considerHierarchy) { + List beanNames = new ArrayList(); + for (String ignoredType : ignoredTypes) { + beanNames.addAll(getBeanNamesForType(beanFactory, ignoredType, + context.getClassLoader(), considerHierarchy)); } return beanNames; } @@ -227,7 +313,7 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit } private boolean hasSingleAutowireCandidate( - ConfigurableListableBeanFactory beanFactory, List beanNames, + ConfigurableListableBeanFactory beanFactory, Set beanNames, boolean considerHierarchy) { return (beanNames.size() == 1 || getPrimaryBeans(beanFactory, beanNames, considerHierarchy) @@ -235,7 +321,7 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit } private List getPrimaryBeans(ConfigurableListableBeanFactory beanFactory, - List beanNames, boolean considerHierarchy) { + Set beanNames, boolean considerHierarchy) { List primaryBeans = new ArrayList(); for (String beanName : beanNames) { BeanDefinition beanDefinition = findBeanDefinition(beanFactory, beanName, @@ -439,4 +525,64 @@ class OnBeanCondition extends SpringBootCondition implements ConfigurationCondit } + static final class MatchResult { + + private final Map> matchedAnnotations = new HashMap>(); + + private final List matchedNames = new ArrayList(); + + private final Map> matchedTypes = new HashMap>(); + + private final List unmatchedAnnotations = new ArrayList(); + + private final List unmatchedNames = new ArrayList(); + + private final List unmatchedTypes = new ArrayList(); + + private final Set namesOfAllMatches = new HashSet(); + + private void recordMatchedName(String name) { + this.matchedNames.add(name); + this.namesOfAllMatches.add(name); + } + + private void recordUnmatchedName(String name) { + this.unmatchedNames.add(name); + } + + private void recordMatchedAnnotation(String annotation, + Collection matchingNames) { + this.matchedAnnotations.put(annotation, matchingNames); + this.namesOfAllMatches.addAll(matchingNames); + } + + private void recordUnmatchedAnnotation(String annotation) { + this.unmatchedAnnotations.add(annotation); + } + + private void recordMatchedType(String type, Collection matchingNames) { + this.matchedTypes.put(type, matchingNames); + this.namesOfAllMatches.addAll(matchingNames); + } + + private void recordUnmatchedType(String type) { + this.unmatchedTypes.add(type); + } + + private boolean isAllMatched() { + return this.unmatchedAnnotations.isEmpty() && this.unmatchedNames.isEmpty() + && this.unmatchedTypes.isEmpty(); + } + + private boolean isAnyMatched() { + return (!this.matchedAnnotations.isEmpty()) || (!this.matchedNames.isEmpty()) + || (!this.matchedTypes.isEmpty()); + } + + private Set getNamesOfAllMatches() { + return this.namesOfAllMatches; + } + + } + } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java index 7a5aa7f43c..5c798ac948 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/condition/ConditionalOnBeanTests.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. @@ -57,7 +57,7 @@ public class ConditionalOnBeanTests { this.context.register(FooConfiguration.class, OnBeanNameAndTypeConfiguration.class); this.context.refresh(); - assertThat(this.context.containsBean("bar")).isTrue(); + assertThat(this.context.containsBean("bar")).isFalse(); } @Test