From 751beda2cb978de3250fed90c2db9e82c2d49825 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 6 Jan 2014 10:52:33 -0800 Subject: [PATCH] Additional fixes for auto-configuration report Fix the following issues that were introduced in commit 0610378: - Formatting of code including imports - Improve hashcode/equals implementations by using ObjectUtils - Provide hashcode/equals for ConditionOutcome - Use LinkedHashSet in ConditionAndOutcomes to maintain insert order Fixed gh-127 --- .../AutoConfigurationReport.java | 43 +++--- .../condition/ConditionOutcome.java | 24 +++ .../AutoConfigurationReportTests.java | 144 ++++++++---------- 3 files changed, 111 insertions(+), 100 deletions(-) diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationReport.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationReport.java index 6e0df96d85d..2f83d718200 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationReport.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationReport.java @@ -16,13 +16,21 @@ package org.springframework.boot.autoconfigure; -import java.util.*; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.boot.autoconfigure.condition.ConditionOutcome; import org.springframework.context.annotation.Condition; +import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * Records auto-configuration details for reporting and logging. @@ -34,7 +42,9 @@ import org.springframework.context.annotation.Condition; public class AutoConfigurationReport { private static final String BEAN_NAME = "autoConfigurationReport"; + private final SortedMap outcomes = new TreeMap(); + private AutoConfigurationReport parent; /** @@ -52,6 +62,9 @@ public class AutoConfigurationReport { */ public void recordConditionEvaluation(String source, Condition condition, ConditionOutcome outcome) { + Assert.notNull(source, "Source must not be null"); + Assert.notNull(condition, "Condition must not be null"); + Assert.notNull(outcome, "Outcome must not be null"); if (!this.outcomes.containsKey(source)) { this.outcomes.put(source, new ConditionAndOutcomes()); } @@ -67,7 +80,6 @@ public class AutoConfigurationReport { /** * The parent report (from a parent BeanFactory if there is one). - * * @return the parent report (or null if there isn't one) */ public AutoConfigurationReport getParent() { @@ -107,7 +119,7 @@ public class AutoConfigurationReport { */ public static class ConditionAndOutcomes implements Iterable { - private Set outcomes = new HashSet(); + private Set outcomes = new LinkedHashSet(); public void add(Condition condition, ConditionOutcome outcome) { this.outcomes.add(new ConditionAndOutcome(condition, outcome)); @@ -138,6 +150,7 @@ public class AutoConfigurationReport { public static class ConditionAndOutcome { private final Condition condition; + private final ConditionOutcome outcome; public ConditionAndOutcome(Condition condition, ConditionOutcome outcome) { @@ -154,29 +167,23 @@ public class AutoConfigurationReport { } @Override - public boolean equals(Object o) { - if (this == o) + public boolean equals(Object obj) { + if (this == obj) { return true; - if (o == null || getClass() != o.getClass()) - return false; - - if (this.outcome == null || this.outcome.getMessage() == null) { - return false; } - - ConditionAndOutcome that = (ConditionAndOutcome) o; - - if (that.getOutcome() == null || this.getOutcome().getMessage() == null) { + if (obj == null || getClass() != obj.getClass()) { return false; } - - return this.getOutcome().getMessage().equals(that.getOutcome().getMessage()); + ConditionAndOutcome other = (ConditionAndOutcome) obj; + return (ObjectUtils.nullSafeEquals(this.condition.getClass(), + other.condition.getClass()) && ObjectUtils.nullSafeEquals( + this.outcome, other.outcome)); } @Override public int hashCode() { - return outcome != null && outcome.getMessage() != null ? outcome.getMessage().hashCode() : 0; + return this.condition.getClass().hashCode() * 31 + this.outcome.hashCode(); } - } + } } diff --git a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionOutcome.java b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionOutcome.java index d9329c6e57d..a48ae3374ea 100644 --- a/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionOutcome.java +++ b/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/condition/ConditionOutcome.java @@ -16,6 +16,8 @@ package org.springframework.boot.autoconfigure.condition; +import org.springframework.util.ObjectUtils; + /** * Outcome for a condition match, including log message. * @@ -69,4 +71,26 @@ public class ConditionOutcome { return this.message; } + @Override + public int hashCode() { + return ObjectUtils.hashCode(this.match) * 31 + + ObjectUtils.nullSafeHashCode(this.message); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() == obj.getClass()) { + ConditionOutcome other = (ConditionOutcome) obj; + return (this.match == other.match && ObjectUtils.nullSafeEquals(this.message, + other.message)); + } + return super.equals(obj); + } + } diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationReportTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationReportTests.java index fad46f13ea6..a072ed0c53d 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationReportTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationReportTests.java @@ -16,11 +16,12 @@ package org.springframework.boot.autoconfigure; -import java.util.HashSet; +import java.util.ArrayList; import java.util.Iterator; +import java.util.List; import java.util.Map; -import java.util.Set; +import org.hamcrest.Matcher; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -37,13 +38,13 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext import org.springframework.context.annotation.Condition; import org.springframework.context.annotation.Import; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertEquals; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; -import static org.mockito.BDDMockito.given; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; /** * Tests for {@link AutoConfigurationReport}. @@ -66,13 +67,10 @@ public class AutoConfigurationReportTests { @Mock private Condition condition3; - @Mock private ConditionOutcome outcome1; - @Mock private ConditionOutcome outcome2; - @Mock private ConditionOutcome outcome3; @Before @@ -92,7 +90,8 @@ public class AutoConfigurationReportTests { @Test public void parent() throws Exception { this.beanFactory.setParentBeanFactory(new DefaultListableBeanFactory()); - AutoConfigurationReport.get((ConfigurableListableBeanFactory) this.beanFactory.getParentBeanFactory()); + AutoConfigurationReport.get((ConfigurableListableBeanFactory) this.beanFactory + .getParentBeanFactory()); assertThat(this.report, sameInstance(AutoConfigurationReport.get(this.beanFactory))); assertThat(this.report, not(nullValue())); @@ -101,24 +100,26 @@ public class AutoConfigurationReportTests { @Test public void recordConditionEvaluations() throws Exception { - given(this.outcome1.getMessage()).willReturn("Message 1"); - given(this.outcome2.getMessage()).willReturn("Message 2"); - given(this.outcome3.getMessage()).willReturn("Message 3"); - + this.outcome1 = new ConditionOutcome(false, "m1"); + this.outcome2 = new ConditionOutcome(false, "m2"); + this.outcome3 = new ConditionOutcome(false, "m3"); this.report.recordConditionEvaluation("a", this.condition1, this.outcome1); this.report.recordConditionEvaluation("a", this.condition2, this.outcome2); this.report.recordConditionEvaluation("b", this.condition3, this.outcome3); - - Map map = this.report.getConditionAndOutcomesBySource(); + Map map = this.report + .getConditionAndOutcomesBySource(); assertThat(map.size(), equalTo(2)); Iterator iterator = map.get("a").iterator(); + ConditionAndOutcome conditionAndOutcome = iterator.next(); assertThat(conditionAndOutcome.getCondition(), equalTo(this.condition1)); assertThat(conditionAndOutcome.getOutcome(), equalTo(this.outcome1)); + conditionAndOutcome = iterator.next(); assertThat(conditionAndOutcome.getCondition(), equalTo(this.condition2)); assertThat(conditionAndOutcome.getOutcome(), equalTo(this.outcome2)); assertThat(iterator.hasNext(), equalTo(false)); + iterator = map.get("b").iterator(); conditionAndOutcome = iterator.next(); assertThat(conditionAndOutcome.getCondition(), equalTo(this.condition3)); @@ -141,9 +142,9 @@ public class AutoConfigurationReportTests { } private void prepareMatches(boolean m1, boolean m2, boolean m3) { - given(this.outcome1.isMatch()).willReturn(m1); - given(this.outcome2.isMatch()).willReturn(m2); - given(this.outcome3.isMatch()).willReturn(m3); + this.outcome1 = new ConditionOutcome(m1, "m1"); + this.outcome2 = new ConditionOutcome(m2, "m2"); + this.outcome3 = new ConditionOutcome(m3, "m3"); this.report.recordConditionEvaluation("a", this.condition1, this.outcome1); this.report.recordConditionEvaluation("a", this.condition2, this.outcome2); this.report.recordConditionEvaluation("a", this.condition3, this.outcome3); @@ -152,90 +153,69 @@ public class AutoConfigurationReportTests { @Test @SuppressWarnings("resource") public void springBootConditionPopulatesReport() throws Exception { - AutoConfigurationReport report = AutoConfigurationReport.get(new AnnotationConfigApplicationContext( - Config.class).getBeanFactory()); + AutoConfigurationReport report = AutoConfigurationReport + .get(new AnnotationConfigApplicationContext(Config.class) + .getBeanFactory()); assertThat(report.getConditionAndOutcomesBySource().size(), not(equalTo(0))); } @Test public void testDuplicateConditionAndOutcomes() { - Condition condition1 = mock(Condition.class); - ConditionOutcome conditionOutcome1 = mock(ConditionOutcome.class); - given(conditionOutcome1.getMessage()).willReturn("This is message 1"); - - Condition condition2 = mock(Condition.class); - ConditionOutcome conditionOutcome2 = mock(ConditionOutcome.class); - given(conditionOutcome2.getMessage()).willReturn("This is message 2"); - - Condition condition3 = mock(Condition.class); - ConditionOutcome conditionOutcome3 = mock(ConditionOutcome.class); - given(conditionOutcome3.getMessage()).willReturn("This is message 2"); // identical in value to #2 + ConditionAndOutcome outcome1 = new ConditionAndOutcome(this.condition1, + new ConditionOutcome(true, "Message 1")); + ConditionAndOutcome outcome2 = new ConditionAndOutcome(this.condition2, + new ConditionOutcome(true, "Message 2")); + ConditionAndOutcome outcome3 = new ConditionAndOutcome(this.condition3, + new ConditionOutcome(true, "Message 2")); - ConditionAndOutcome outcome1 = new ConditionAndOutcome(condition1, - conditionOutcome1); assertThat(outcome1, equalTo(outcome1)); - - ConditionAndOutcome outcome2 = new ConditionAndOutcome(condition2, - conditionOutcome2); assertThat(outcome1, not(equalTo(outcome2))); - - ConditionAndOutcome outcome3 = new ConditionAndOutcome(condition3, - conditionOutcome3); assertThat(outcome2, equalTo(outcome3)); - Set set = new HashSet(); - set.add(outcome1); - set.add(outcome2); - set.add(outcome3); - assertThat(set.size(), equalTo(2)); - ConditionAndOutcomes outcomes = new ConditionAndOutcomes(); - outcomes.add(condition1, conditionOutcome1); - outcomes.add(condition2, conditionOutcome2); - outcomes.add(condition3, conditionOutcome3); + outcomes.add(this.condition1, new ConditionOutcome(true, "Message 1")); + outcomes.add(this.condition2, new ConditionOutcome(true, "Message 2")); + outcomes.add(this.condition3, new ConditionOutcome(true, "Message 2")); - int i = 0; - Iterator iterator = outcomes.iterator(); - while (iterator.hasNext()) { - i++; - iterator.next(); - } - assertThat(i, equalTo(2)); + assertThat(getNumberOfOutcomes(outcomes), equalTo(2)); } @Test + @SuppressWarnings("unchecked") public void duplicateOutcomes() { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext( DuplicateConfig.class); - AutoConfigurationReport report = AutoConfigurationReport.get(context.getBeanFactory()); - String autoconfigKey = "org.springframework.boot.autoconfigure.web.MultipartAutoConfiguration"; - - assertThat(report.getConditionAndOutcomesBySource().keySet(), - hasItem(autoconfigKey)); + AutoConfigurationReport report = AutoConfigurationReport.get(context + .getBeanFactory()); + String autoconfigKey = MultipartAutoConfiguration.class.getName(); - ConditionAndOutcomes conditionAndOutcomes = report.getConditionAndOutcomesBySource().get( + ConditionAndOutcomes outcomes = report.getConditionAndOutcomesBySource().get( autoconfigKey); - Iterator iterator = conditionAndOutcomes.iterator(); - int i = 0; - boolean foundConditionalOnClass = false; - boolean foundConditionalOnBean = false; - while (iterator.hasNext()) { - ConditionAndOutcome conditionAndOutcome = iterator.next(); - if (conditionAndOutcome.getOutcome().getMessage().contains( - "@ConditionalOnClass classes found: javax.servlet.Servlet,org.springframework.web.multipart.support.StandardServletMultipartResolver")) { - foundConditionalOnClass = true; - } - if (conditionAndOutcome.getOutcome().getMessage().contains( - "@ConditionalOnBean (types: javax.servlet.MultipartConfigElement; SearchStrategy: all) found no beans")) { - foundConditionalOnBean = true; - } - i++; + assertThat(outcomes, not(nullValue())); + assertThat(getNumberOfOutcomes(outcomes), equalTo(2)); + + List messages = new ArrayList(); + for (ConditionAndOutcome outcome : outcomes) { + messages.add(outcome.getOutcome().getMessage()); + System.out.println(outcome.getOutcome().getMessage()); } - assertThat(i, equalTo(2)); - assertTrue(foundConditionalOnClass); - assertTrue(foundConditionalOnBean); + Matcher onClassMessage = containsString("@ConditionalOnClass " + + "classes found: javax.servlet.Servlet,org.springframework.web.multipart.support.StandardServletMultipartResolver"); + Matcher onBeanMessage = containsString("@ConditionalOnBean " + + "(types: javax.servlet.MultipartConfigElement; SearchStrategy: all) found no beans"); + assertThat(messages, containsInAnyOrder(onClassMessage, onBeanMessage)); + context.close(); + } + private int getNumberOfOutcomes(ConditionAndOutcomes outcomes) { + Iterator iterator = outcomes.iterator(); + int numberOfOutcomesAdded = 0; + while (iterator.hasNext()) { + numberOfOutcomesAdded++; + iterator.next(); + } + return numberOfOutcomesAdded; } @Configurable