From a5b4c8e6db176b5455130a7ab024076870d6cdcd Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 7 Oct 2016 16:47:56 +0100 Subject: [PATCH] Make cycle clearer in bean currently in creation failure analysis Note: the fully-qualified references to @Configuration in some of the test configuration classes are required to work around a bug in javac. 1.8.0_102 (and earlier). Without them, compilation fails as it cannot resolve the symbol despite the import statement and the unqualified references working elsewhere in the same source file. Closes gh-7056 --- ...eanCurrentlyInCreationFailureAnalyzer.java | 93 +++++++--- ...rrentlyInCreationFailureAnalyzerTests.java | 171 ++++++++++++++++-- 2 files changed, 229 insertions(+), 35 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzer.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzer.java index 4e44ba960e..cbec191884 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzer.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzer.java @@ -39,45 +39,96 @@ class BeanCurrentlyInCreationFailureAnalyzer @Override protected FailureAnalysis analyze(Throwable rootFailure, BeanCurrentlyInCreationException cause) { - List beansInCycle = new ArrayList(); + List beansInCycle = new ArrayList(); Throwable candidate = rootFailure; + int cycleStart = -1; while (candidate != null) { if (candidate instanceof BeanCreationException) { BeanCreationException creationEx = (BeanCreationException) candidate; if (StringUtils.hasText(creationEx.getBeanName())) { - beansInCycle - .add(creationEx.getBeanName() + getDescription(creationEx)); + BeanInCycle beanInCycle = new BeanInCycle(creationEx); + int index = beansInCycle.indexOf(beanInCycle); + if (index == -1) { + beansInCycle.add(beanInCycle); + } + else { + cycleStart = index; + } } } candidate = candidate.getCause(); } StringBuilder message = new StringBuilder(); - int uniqueBeans = beansInCycle.size() - 1; - message.append( - String.format("There is a circular dependency between %s beans in the " - + "application context:%n", uniqueBeans)); - for (String bean : beansInCycle) { - message.append(String.format("\t- %s%n", bean)); + message.append(String.format("The dependencies of some of the beans in the " + + "application context form a cycle:%n%n")); + for (int i = 0; i < beansInCycle.size(); i++) { + if (i == cycleStart) { + message.append(String.format("┌─────┐%n")); + } + else if (i > 0) { + message.append(String.format("%s ↓%n", i < cycleStart ? " " : "↑")); + } + message.append(String.format("%s %s%n", i < cycleStart ? " " : "|", + beansInCycle.get(i))); } + message.append(String.format("└─────┘%n")); return new FailureAnalysis(message.toString(), null, cause); } - private String getDescription(BeanCreationException ex) { - if (StringUtils.hasText(ex.getResourceDescription())) { - return String.format(" defined in %s", ex.getResourceDescription()); + private static final class BeanInCycle { + + private final String name; + + private final String description; + + private BeanInCycle(BeanCreationException ex) { + this.name = ex.getBeanName(); + this.description = determineDescription(ex); } - InjectionPoint failedInjectionPoint = findFailedInjectionPoint(ex); - if (failedInjectionPoint != null && failedInjectionPoint.getField() != null) { - return String.format(" (field %s)", failedInjectionPoint.getField()); + + private String determineDescription(BeanCreationException ex) { + if (StringUtils.hasText(ex.getResourceDescription())) { + return String.format(" defined in %s", ex.getResourceDescription()); + } + InjectionPoint failedInjectionPoint = findFailedInjectionPoint(ex); + if (failedInjectionPoint != null && failedInjectionPoint.getField() != null) { + return String.format(" (field %s)", failedInjectionPoint.getField()); + } + return ""; + } + + private InjectionPoint findFailedInjectionPoint(BeanCreationException ex) { + if (!(ex instanceof UnsatisfiedDependencyException)) { + return null; + } + return ((UnsatisfiedDependencyException) ex).getInjectionPoint(); + } + + @Override + public int hashCode() { + return this.name.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + BeanInCycle other = (BeanInCycle) obj; + return this.name.equals(other.name); } - return ""; - } - private InjectionPoint findFailedInjectionPoint(BeanCreationException ex) { - if (!(ex instanceof UnsatisfiedDependencyException)) { - return null; + @Override + public String toString() { + return this.name + this.description; } - return ((UnsatisfiedDependencyException) ex).getInjectionPoint(); + } } diff --git a/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzerTests.java b/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzerTests.java index aaf0eb826a..e82d376a90 100644 --- a/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzerTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzerTests.java @@ -16,11 +16,21 @@ package org.springframework.boot.diagnostics.analyzer; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.StringReader; +import java.util.ArrayList; +import java.util.List; + import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.diagnostics.FailureAnalysis; import org.springframework.boot.diagnostics.FailureAnalyzer; +import org.springframework.boot.diagnostics.analyzer.BeanCurrentlyInCreationFailureAnalyzerTests.CycleWithAutowiredFields.BeanThreeConfiguration; +import org.springframework.boot.diagnostics.analyzer.BeanCurrentlyInCreationFailureAnalyzerTests.CycleWithAutowiredFields.BeanTwoConfiguration; +import org.springframework.boot.diagnostics.analyzer.BeanCurrentlyInCreationFailureAnalyzerTests.CyclicBeanMethodsConfiguration.InnerConfiguration; +import org.springframework.boot.diagnostics.analyzer.BeanCurrentlyInCreationFailureAnalyzerTests.CyclicBeanMethodsConfiguration.InnerConfiguration.InnerInnerConfiguration; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -39,10 +49,25 @@ public class BeanCurrentlyInCreationFailureAnalyzerTests { private final FailureAnalyzer analyzer = new BeanCurrentlyInCreationFailureAnalyzer(); @Test - public void cyclicBeanMethods() { + public void cyclicBeanMethods() throws IOException { FailureAnalysis analysis = performAnalysis(CyclicBeanMethodsConfiguration.class); + List lines = readDescriptionLines(analysis); + assertThat(lines).hasSize(9); + assertThat(lines.get(0)).isEqualTo( + "The dependencies of some of the beans in the application context form a cycle:"); + assertThat(lines.get(1)).isEqualTo(""); + assertThat(lines.get(2)).isEqualTo("┌─────┐"); + assertThat(lines.get(3)).startsWith( + "| one defined in " + InnerInnerConfiguration.class.getName()); + assertThat(lines.get(4)).isEqualTo("↑ ↓"); + assertThat(lines.get(5)) + .startsWith("| two defined in " + InnerConfiguration.class.getName()); + assertThat(lines.get(6)).isEqualTo("↑ ↓"); + assertThat(lines.get(7)).startsWith( + "| three defined in " + CyclicBeanMethodsConfiguration.class.getName()); + assertThat(lines.get(8)).isEqualTo("└─────┘"); assertThat(analysis.getDescription()).startsWith( - "There is a circular dependency between 3 beans in the application context:"); + "The dependencies of some of the beans in the application context form a cycle:"); assertThat(analysis.getDescription()).contains( "one defined in " + CyclicBeanMethodsConfiguration.class.getName()); assertThat(analysis.getDescription()).contains( @@ -52,22 +77,74 @@ public class BeanCurrentlyInCreationFailureAnalyzerTests { } @Test - public void cycleWithAutowiredFields() { + public void cycleWithAutowiredFields() throws IOException { FailureAnalysis analysis = performAnalysis(CycleWithAutowiredFields.class); assertThat(analysis.getDescription()).startsWith( - "There is a circular dependency between 3 beans in the application context:"); - assertThat(analysis.getDescription()).contains("three defined in " - + CycleWithAutowiredFields.BeanThreeConfiguration.class.getName()); - assertThat(analysis.getDescription()) - .contains("one defined in " + CycleWithAutowiredFields.class.getName()); - assertThat(analysis.getDescription()) - .contains(CycleWithAutowiredFields.BeanTwoConfiguration.class.getName() - + " (field private " + BeanThree.class.getName()); + "The dependencies of some of the beans in the application context form a cycle:"); + List lines = readDescriptionLines(analysis); + assertThat(lines).hasSize(9); + assertThat(lines.get(0)).isEqualTo( + "The dependencies of some of the beans in the application context form a cycle:"); + assertThat(lines.get(1)).isEqualTo(""); + assertThat(lines.get(2)).isEqualTo("┌─────┐"); + assertThat(lines.get(3)).startsWith( + "| three defined in " + BeanThreeConfiguration.class.getName()); + assertThat(lines.get(4)).isEqualTo("↑ ↓"); + assertThat(lines.get(5)).startsWith( + "| one defined in " + CycleWithAutowiredFields.class.getName()); + assertThat(lines.get(6)).isEqualTo("↑ ↓"); + assertThat(lines.get(7)).startsWith("| " + BeanTwoConfiguration.class.getName() + + " (field private " + BeanThree.class.getName()); + assertThat(lines.get(8)).isEqualTo("└─────┘"); + } + + @Test + public void cycleReferencedViaOtherBeans() throws IOException { + FailureAnalysis analysis = performAnalysis( + CycleReferencedViaOtherBeansConfiguration.class); + List lines = readDescriptionLines(analysis); + assertThat(lines).hasSize(12); + assertThat(lines.get(0)).isEqualTo( + "The dependencies of some of the beans in the application context form a cycle:"); + assertThat(lines.get(1)).isEqualTo(""); + assertThat(lines.get(2)) + .contains("refererOne " + "(field " + RefererTwo.class.getName()); + assertThat(lines.get(3)).isEqualTo(" ↓"); + assertThat(lines.get(4)) + .contains("refererTwo " + "(field " + BeanOne.class.getName()); + assertThat(lines.get(5)).isEqualTo("┌─────┐"); + assertThat(lines.get(6)).startsWith("| one defined in " + + CycleReferencedViaOtherBeansConfiguration.class.getName()); + assertThat(lines.get(7)).isEqualTo("↑ ↓"); + assertThat(lines.get(8)).startsWith("| two defined in " + + CycleReferencedViaOtherBeansConfiguration.class.getName()); + assertThat(lines.get(9)).isEqualTo("↑ ↓"); + assertThat(lines.get(10)).startsWith("| three defined in " + + CycleReferencedViaOtherBeansConfiguration.class.getName()); + assertThat(lines.get(11)).isEqualTo("└─────┘"); + } + + private List readDescriptionLines(FailureAnalysis analysis) + throws IOException { + BufferedReader lineReader = new BufferedReader( + new StringReader(analysis.getDescription())); + try { + List lines = new ArrayList(); + String line; + while ((line = lineReader.readLine()) != null) { + lines.add(line); + } + return lines; + } + finally { + lineReader.close(); + } } private FailureAnalysis performAnalysis(Class configuration) { FailureAnalysis analysis = this.analyzer.analyze(createFailure(configuration)); assertThat(analysis).isNotNull(); + System.out.println(analysis.getDescription()); return analysis; } @@ -89,9 +166,39 @@ public class BeanCurrentlyInCreationFailureAnalyzerTests { return null; } - @Configuration + @org.springframework.context.annotation.Configuration static class CyclicBeanMethodsConfiguration { + @Bean + public BeanThree three(BeanOne one) { + return new BeanThree(); + } + + @org.springframework.context.annotation.Configuration + static class InnerConfiguration { + + @Bean + public BeanTwo two(BeanThree three) { + return new BeanTwo(); + } + + @Configuration + static class InnerInnerConfiguration { + + @Bean + public BeanOne one(BeanTwo two) { + return new BeanOne(); + } + + } + + } + + } + + @Configuration + static class CycleReferencedViaOtherBeansConfiguration { + @Bean public BeanOne one(BeanTwo two) { return new BeanOne(); @@ -103,13 +210,33 @@ public class BeanCurrentlyInCreationFailureAnalyzerTests { } @Bean - public BeanThree three(BeanOne one) { + public BeanThree three(BeanOne beanOne) { return new BeanThree(); } + @Configuration + static class InnerConfiguration { + + @Bean + public RefererTwo refererTwo() { + return new RefererTwo(); + } + + @Configuration + static class InnerInnerConfiguration { + + @Bean + public RefererOne refererOne() { + return new RefererOne(); + } + + } + + } + } - @Configuration + @org.springframework.context.annotation.Configuration public static class CycleWithAutowiredFields { @Bean @@ -127,6 +254,7 @@ public class BeanCurrentlyInCreationFailureAnalyzerTests { public BeanTwo two() { return new BeanTwo(); } + } public static class BeanThreeConfiguration { @@ -135,10 +263,25 @@ public class BeanCurrentlyInCreationFailureAnalyzerTests { public BeanThree three(BeanOne one) { return new BeanThree(); } + } } + static class RefererOne { + + @Autowired + RefererTwo refererTwo; + + } + + static class RefererTwo { + + @Autowired + BeanOne beanOne; + + } + static class BeanOne { }