mirror of
https://github.com/spring-projects/spring-boot.git
synced 2024-09-03 04:26:12 +08:00
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:
parent
c71322a0b2
commit
0610378d2f
@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -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 {
|
||||
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -216,4 +216,5 @@ public abstract class AbstractJpaAutoConfigurationTests {
|
||||
|
||||
static class CustomJpaTransactionManager extends JpaTransactionManager {
|
||||
}
|
||||
|
||||
}
|
||||
|
@ -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");
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user