Resolves #127: Prevent duplicate report outcomes

The collection of outcomes is a list. Sometimes a race condition causes to instances
of the same outcome to get added to the list shown in the report. By replacing this
with a set and propery equals/hashCode, duplicates are prevented from appearing
in the report.

I added test cases to prove that that POJO is properly managed inside a Set and also
to show that duplicates don't appear in the final report.
This commit is contained in:
Greg Turnquist 2013-12-13 09:09:32 -06:00 committed by Dave Syer
parent c71322a0b2
commit 0610378d2f
4 changed files with 136 additions and 28 deletions

View File

@ -16,13 +16,7 @@
package org.springframework.boot.autoconfigure;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
import java.util.*;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
@ -40,9 +34,7 @@ import org.springframework.context.annotation.Condition;
public class AutoConfigurationReport {
private static final String BEAN_NAME = "autoConfigurationReport";
private final SortedMap<String, ConditionAndOutcomes> outcomes = new TreeMap<String, ConditionAndOutcomes>();
private AutoConfigurationReport parent;
/**
@ -75,7 +67,7 @@ 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() {
@ -115,7 +107,7 @@ public class AutoConfigurationReport {
*/
public static class ConditionAndOutcomes implements Iterable<ConditionAndOutcome> {
private List<ConditionAndOutcome> outcomes = new ArrayList<ConditionAndOutcome>();
private Set<ConditionAndOutcome> outcomes = new HashSet<ConditionAndOutcome>();
public void add(Condition condition, ConditionOutcome outcome) {
this.outcomes.add(new ConditionAndOutcome(condition, outcome));
@ -135,7 +127,7 @@ public class AutoConfigurationReport {
@Override
public Iterator<ConditionAndOutcome> iterator() {
return Collections.unmodifiableList(this.outcomes).iterator();
return Collections.unmodifiableSet(this.outcomes).iterator();
}
}
@ -146,7 +138,6 @@ public class AutoConfigurationReport {
public static class ConditionAndOutcome {
private final Condition condition;
private final ConditionOutcome outcome;
public ConditionAndOutcome(Condition condition, ConditionOutcome outcome) {
@ -161,6 +152,31 @@ public class AutoConfigurationReport {
public ConditionOutcome getOutcome() {
return this.outcome;
}
}
@Override
public boolean equals(Object o) {
if (this == o)
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) {
return false;
}
return this.getOutcome().getMessage().equals(that.getOutcome().getMessage());
}
@Override
public int hashCode() {
return outcome != null && outcome.getMessage() != null ? outcome.getMessage().hashCode() : 0;
}
}
}

View File

@ -16,8 +16,10 @@
package org.springframework.boot.autoconfigure;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import org.junit.Before;
import org.junit.Test;
@ -29,17 +31,19 @@ import org.springframework.beans.factory.support.DefaultListableBeanFactory;
import org.springframework.boot.autoconfigure.AutoConfigurationReport.ConditionAndOutcome;
import org.springframework.boot.autoconfigure.AutoConfigurationReport.ConditionAndOutcomes;
import org.springframework.boot.autoconfigure.condition.ConditionOutcome;
import org.springframework.boot.autoconfigure.web.MultipartAutoConfiguration;
import org.springframework.boot.autoconfigure.web.WebMvcAutoConfiguration;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Condition;
import org.springframework.context.annotation.Import;
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.hamcrest.Matchers.*;
import static org.junit.Assert.assertEquals;
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}.
@ -88,8 +92,7 @@ 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()));
@ -98,12 +101,15 @@ 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.report.recordConditionEvaluation("a", this.condition1, this.outcome1);
this.report.recordConditionEvaluation("a", this.condition2, this.outcome2);
this.report.recordConditionEvaluation("b", this.condition3, this.outcome3);
Map<String, ConditionAndOutcomes> map = this.report
.getConditionAndOutcomesBySource();
Map<String, ConditionAndOutcomes> map = this.report.getConditionAndOutcomesBySource();
assertThat(map.size(), equalTo(2));
Iterator<ConditionAndOutcome> iterator = map.get("a").iterator();
ConditionAndOutcome conditionAndOutcome = iterator.next();
@ -146,16 +152,102 @@ 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(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<ConditionAndOutcome> set = new HashSet<ConditionAndOutcome>();
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);
int i = 0;
Iterator<ConditionAndOutcome> iterator = outcomes.iterator();
while (iterator.hasNext()) {
i++;
iterator.next();
}
assertThat(i, equalTo(2));
}
@Test
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));
ConditionAndOutcomes conditionAndOutcomes = report.getConditionAndOutcomesBySource().get(
autoconfigKey);
Iterator<ConditionAndOutcome> 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(i, equalTo(2));
assertTrue(foundConditionalOnClass);
assertTrue(foundConditionalOnBean);
}
@Configurable
@Import(WebMvcAutoConfiguration.class)
static class Config {
}
@Configurable
@Import(MultipartAutoConfiguration.class)
static class DuplicateConfig {
}
}

View File

@ -216,4 +216,5 @@ public abstract class AbstractJpaAutoConfigurationTests {
static class CustomJpaTransactionManager extends JpaTransactionManager {
}
}

View File

@ -18,7 +18,6 @@ package org.springframework.boot.context.embedded;
import java.io.FileWriter;
import java.io.IOException;
import java.net.SocketException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.Charset;
@ -99,7 +98,7 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
this.container = factory
.getEmbeddedServletContainer(exampleServletRegistration());
this.container.start();
this.thrown.expect(SocketException.class);
this.thrown.expect(IOException.class);
getResponse("http://localhost:8080/hello");
}
@ -110,7 +109,7 @@ public abstract class AbstractEmbeddedServletContainerFactoryTests {
.getEmbeddedServletContainer(exampleServletRegistration());
this.container.start();
this.container.stop();
this.thrown.expect(SocketException.class);
this.thrown.expect(IOException.class);
getResponse("http://localhost:8080/hello");
}