From 03dad33b0c121d465962ca93f65022575e42b1f4 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Fri, 19 Feb 2016 15:28:10 -0800 Subject: [PATCH] Polish diagnostics support Rework a few parts of the diagnostics support: - Move code from SpringApplication to FailureAnalyzers - Allow AbstractFailureAnalyzer to take generic cause type - Move own analyzers into a new package and make package private See gh-4907 --- .../boot/SpringApplication.java | 31 +-------- .../context/embedded/PortInUseException.java | 2 - .../TomcatEmbeddedServletContainer.java | 2 - .../diagnostics/AbstractFailureAnalyzer.java | 50 +++++++++++--- .../boot/diagnostics/FailureAnalysis.java | 4 -- .../diagnostics/FailureAnalysisReporter.java | 5 +- .../boot/diagnostics/FailureAnalyzer.java | 1 - .../boot/diagnostics/FailureAnalyzers.java | 67 +++++++++++++++++++ ...eanCurrentlyInCreationFailureAnalyzer.java | 57 ++++++++-------- .../analyzer}/PortInUseFailureAnalyzer.java | 27 +++----- .../diagnostics/analyzer/package-info.java | 20 ++++++ .../boot/diagnostics/package-info.java | 23 +++++++ .../main/resources/META-INF/spring.factories | 4 +- ...rrentlyInCreationFailureAnalyzerTests.java | 4 +- 14 files changed, 198 insertions(+), 99 deletions(-) create mode 100644 spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java rename spring-boot/src/main/java/org/springframework/boot/diagnostics/{ => analyzer}/BeanCurrentlyInCreationFailureAnalyzer.java (55%) rename spring-boot/src/main/java/org/springframework/boot/{context/embedded => diagnostics/analyzer}/PortInUseFailureAnalyzer.java (56%) create mode 100644 spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/package-info.java create mode 100644 spring-boot/src/main/java/org/springframework/boot/diagnostics/package-info.java rename spring-boot/src/test/java/org/springframework/boot/diagnostics/{ => analyzer}/BeanCurrentlyInCreationFailureAnalyzerTests.java (95%) diff --git a/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java b/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java index 7ba27ad545..5cc03c0431 100644 --- a/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java +++ b/spring-boot/src/main/java/org/springframework/boot/SpringApplication.java @@ -41,9 +41,7 @@ import org.springframework.beans.factory.groovy.GroovyBeanDefinitionReader; import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; -import org.springframework.boot.diagnostics.FailureAnalysis; -import org.springframework.boot.diagnostics.FailureAnalysisReporter; -import org.springframework.boot.diagnostics.FailureAnalyzer; +import org.springframework.boot.diagnostics.FailureAnalyzers; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ApplicationListener; @@ -835,8 +833,7 @@ public class SpringApplication { private void reportFailure(Throwable failure) { try { - FailureAnalysis failureAnalysis = analyzeFailure(failure); - if (failureAnalysis != null && reportFailureAnalysis(failureAnalysis)) { + if (FailureAnalyzers.analyzeAndReport(failure, getClass().getClassLoader())) { registerLoggedException(failure); return; } @@ -850,30 +847,6 @@ public class SpringApplication { } } - private FailureAnalysis analyzeFailure(Throwable failure) { - List analyzers = SpringFactoriesLoader - .loadFactories(FailureAnalyzer.class, getClass().getClassLoader()); - for (FailureAnalyzer analyzer : analyzers) { - FailureAnalysis analysis = analyzer.analyze(failure); - if (analysis != null) { - return analysis; - } - } - return null; - } - - private boolean reportFailureAnalysis(FailureAnalysis failureAnalysis) { - List reporters = SpringFactoriesLoader.loadFactories( - FailureAnalysisReporter.class, getClass().getClassLoader()); - if (!reporters.isEmpty()) { - for (FailureAnalysisReporter reporter : reporters) { - reporter.report(failureAnalysis); - } - return true; - } - return false; - } - /** * Register that the given exception has been logged. By default, if the running in * the main thread, this method will suppress additional printing of the stacktrace. diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/PortInUseException.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/PortInUseException.java index 3b37ff9b9a..7f83c47359 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/PortInUseException.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/PortInUseException.java @@ -29,7 +29,6 @@ public class PortInUseException extends EmbeddedServletContainerException { /** * Creates a new port in use exception for the given {@code port}. - * * @param port the port that was in use */ public PortInUseException(int port) { @@ -39,7 +38,6 @@ public class PortInUseException extends EmbeddedServletContainerException { /** * Returns the port that was in use. - * * @return the port */ public int getPort() { diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java index 2359a74ae9..82b673399a 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/tomcat/TomcatEmbeddedServletContainer.java @@ -167,9 +167,7 @@ public class TomcatEmbeddedServletContainer implements EmbeddedServletContainer if (connector != null && this.autoStart) { startConnector(connector); } - checkThatConnectorsHaveStarted(); - TomcatEmbeddedServletContainer.logger .info("Tomcat started on port(s): " + getPortsDescription(true)); } diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/AbstractFailureAnalyzer.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/AbstractFailureAnalyzer.java index d131b77f31..b508602a8d 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/AbstractFailureAnalyzer.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/AbstractFailureAnalyzer.java @@ -16,25 +16,57 @@ package org.springframework.boot.diagnostics; +import org.springframework.core.ResolvableType; + /** * Abstract base class for most {@code FailureAnalyzer} implementations. * + * @param The type of exception to analyze * @author Andy Wilkinson + * @author Phillip Webb * @since 1.4.0 */ -public abstract class AbstractFailureAnalyzer implements FailureAnalyzer { +public abstract class AbstractFailureAnalyzer + implements FailureAnalyzer { + + @Override + public FailureAnalysis analyze(Throwable failure) { + T cause = findCause(failure, getCauseType()); + if (cause != null) { + return analyze(failure, cause); + } + return null; + } + + /** + * Returns an analysis of the given {@code failure}, or {@code null} if no analysis + * was possible. + * @param rootFailure the root failure passed to the analyzer + * @param cause the actual found cause + * @return the analysis or {@code null} + */ + protected abstract FailureAnalysis analyze(Throwable rootFailure, T cause); + + /** + * Return the cause type being handled by the analyzer. By default the class generic + * is used. + * @return the cause type + */ + @SuppressWarnings("unchecked") + protected Class getCauseType() { + return (Class) ResolvableType + .forClass(AbstractFailureAnalyzer.class, getClass()).resolveGeneric(); + } @SuppressWarnings("unchecked") - protected T findFailure(Throwable failure, Class type) { - Throwable candidate = failure; - T mostSpecificMatch = null; - while (candidate != null) { - if (type.isInstance(candidate)) { - mostSpecificMatch = (T) candidate; + protected final T findCause(Throwable failure, Class type) { + while (failure != null) { + if (type.isInstance(failure)) { + return (T) failure; } - candidate = candidate.getCause(); + failure = failure.getCause(); } - return mostSpecificMatch; + return null; } } diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalysis.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalysis.java index 004a0e9716..0601024751 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalysis.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalysis.java @@ -34,7 +34,6 @@ public class FailureAnalysis { * Creates a new {@code FailureAnalysis} with the given {@code description} and * {@code action}, if any, that the user should take to address the problem. The * failure had the given underlying {@code cause}. - * * @param description the description * @param action the action * @param cause the cause @@ -47,7 +46,6 @@ public class FailureAnalysis { /** * Returns a description of the failure. - * * @return the description */ public String getDescription() { @@ -56,7 +54,6 @@ public class FailureAnalysis { /** * Returns the action, if any, to be taken to address the failure. - * * @return the action or {@code null} */ public String getAction() { @@ -65,7 +62,6 @@ public class FailureAnalysis { /** * Returns the cause of the failure. - * * @return the cause */ public Throwable getCause() { diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalysisReporter.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalysisReporter.java index a4ae4f4366..31dca6fdcb 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalysisReporter.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalysisReporter.java @@ -26,9 +26,8 @@ public interface FailureAnalysisReporter { /** * Reports the given {@code failureAnalysis} to the user. - * - * @param failureAnalysis the analysis + * @param analysis the analysis */ - void report(FailureAnalysis failureAnalysis); + void report(FailureAnalysis analysis); } diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzer.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzer.java index f210da47aa..a3f3192047 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzer.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzer.java @@ -28,7 +28,6 @@ public interface FailureAnalyzer { /** * Returns an analysis of the given {@code failure}, or {@code null} if no analysis * was possible. - * * @param failure the failure * @return the analysis or {@code null} */ diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java new file mode 100644 index 0000000000..dcdf49ca21 --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java @@ -0,0 +1,67 @@ +/* + * Copyright 2012-2016 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.diagnostics; + +import java.util.List; + +import org.springframework.core.io.support.SpringFactoriesLoader; + +/** + * Utility to trigger {@link FailureAnalyzer} and {@link FailureAnalysisReporter} + * instances loaded from {@code spring.factories}. + * + * @author Andy Wilkinson + * @author Phillip Webb + * @since 1.4.0 + */ +public final class FailureAnalyzers { + + private FailureAnalyzers() { + } + + public static boolean analyzeAndReport(Throwable failure, ClassLoader classLoader) { + List analyzers = SpringFactoriesLoader + .loadFactories(FailureAnalyzer.class, classLoader); + List reporters = SpringFactoriesLoader + .loadFactories(FailureAnalysisReporter.class, classLoader); + FailureAnalysis analysis = analyze(failure, analyzers); + return report(analysis, reporters); + } + + private static FailureAnalysis analyze(Throwable failure, + List analyzers) { + for (FailureAnalyzer analyzer : analyzers) { + FailureAnalysis analysis = analyzer.analyze(failure); + if (analysis != null) { + return analysis; + } + } + return null; + } + + private static boolean report(FailureAnalysis analysis, + List reporters) { + if (analysis == null || reporters.isEmpty()) { + return false; + } + for (FailureAnalysisReporter reporter : reporters) { + reporter.report(analysis); + } + return true; + } + +} diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/BeanCurrentlyInCreationFailureAnalyzer.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzer.java similarity index 55% rename from spring-boot/src/main/java/org/springframework/boot/diagnostics/BeanCurrentlyInCreationFailureAnalyzer.java rename to spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzer.java index c750148b8b..99fa2e5587 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/BeanCurrentlyInCreationFailureAnalyzer.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzer.java @@ -14,62 +14,59 @@ * limitations under the License. */ -package org.springframework.boot.diagnostics; +package org.springframework.boot.diagnostics.analyzer; import java.util.ArrayList; import java.util.List; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.BeanCurrentlyInCreationException; +import org.springframework.boot.diagnostics.AbstractFailureAnalyzer; +import org.springframework.boot.diagnostics.FailureAnalysis; import org.springframework.util.StringUtils; /** - * A {@link FailureAnalyzer} the performs analysis of failures caused by a + * A {@link AbstractFailureAnalyzer} the performs analysis of failures caused by a * {@link BeanCurrentlyInCreationException}. * * @author Andy Wilkinson - * @since 1.4.0 */ -public class BeanCurrentlyInCreationFailureAnalyzer extends AbstractFailureAnalyzer { +class BeanCurrentlyInCreationFailureAnalyzer + extends AbstractFailureAnalyzer { private static final String FIELD_AUTOWIRING_FAILURE_MESSAGE_PREFIX = "Could not autowire field: "; @Override - public FailureAnalysis analyze(Throwable failure) { - BeanCurrentlyInCreationException inCreationEx = findFailure(failure, - BeanCurrentlyInCreationException.class); - if (inCreationEx != null) { - List beansInCycle = new ArrayList(); - Throwable candidate = failure; - while (candidate != null) { - if (candidate instanceof BeanCreationException) { - BeanCreationException creationEx = (BeanCreationException) candidate; - if (StringUtils.hasText(creationEx.getBeanName())) { - beansInCycle.add( - creationEx.getBeanName() + getDescription(creationEx)); - } + protected FailureAnalysis analyze(Throwable rootFailure, + BeanCurrentlyInCreationException cause) { + List beansInCycle = new ArrayList(); + Throwable candidate = rootFailure; + while (candidate != null) { + if (candidate instanceof BeanCreationException) { + BeanCreationException creationEx = (BeanCreationException) candidate; + if (StringUtils.hasText(creationEx.getBeanName())) { + beansInCycle + .add(creationEx.getBeanName() + getDescription(creationEx)); } - 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)); - } - - return new FailureAnalysis(message.toString(), null, failure); + 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)); } - return null; + 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()); } - else if (causedByFieldAutowiringFailure(ex)) { + if (causedByFieldAutowiringFailure(ex)) { return String.format(" (field %s)", ex.getCause().getMessage().substring( FIELD_AUTOWIRING_FAILURE_MESSAGE_PREFIX.length(), diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/PortInUseFailureAnalyzer.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/PortInUseFailureAnalyzer.java similarity index 56% rename from spring-boot/src/main/java/org/springframework/boot/context/embedded/PortInUseFailureAnalyzer.java rename to spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/PortInUseFailureAnalyzer.java index e77bddf791..15ac429c2a 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/PortInUseFailureAnalyzer.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/PortInUseFailureAnalyzer.java @@ -14,8 +14,9 @@ * limitations under the License. */ -package org.springframework.boot.context.embedded; +package org.springframework.boot.diagnostics.analyzer; +import org.springframework.boot.context.embedded.PortInUseException; import org.springframework.boot.diagnostics.AbstractFailureAnalyzer; import org.springframework.boot.diagnostics.FailureAnalysis; @@ -24,24 +25,18 @@ import org.springframework.boot.diagnostics.FailureAnalysis; * {@code PortInUseException}. * * @author Andy Wilkinson - * @since 1.4.0 */ -public class PortInUseFailureAnalyzer extends AbstractFailureAnalyzer { +class PortInUseFailureAnalyzer extends AbstractFailureAnalyzer { @Override - public FailureAnalysis analyze(Throwable failure) { - PortInUseException portInUseException = findFailure(failure, - PortInUseException.class); - if (portInUseException != null) { - return new FailureAnalysis( - "Embedded servlet container failed to start. Port " - + portInUseException.getPort() + " was already in use.", - "Identify and stop the process that's listening on port " - + portInUseException.getPort() + " or configure this " - + "application to listen on another port.", - portInUseException); - } - return null; + protected FailureAnalysis analyze(Throwable rootFailure, PortInUseException cause) { + return new FailureAnalysis( + "Embedded servlet container failed to start. Port " + cause.getPort() + + " was already in use.", + "Identify and stop the process that's listening on port " + + cause.getPort() + " or configure this " + + "application to listen on another port.", + cause); } } diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/package-info.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/package-info.java new file mode 100644 index 0000000000..6ae093ca9b --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/analyzer/package-info.java @@ -0,0 +1,20 @@ +/* + * Copyright 2012-2014 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Internal {@link org.springframework.boot.diagnostics.FailureAnalyzer} implementations. + */ +package org.springframework.boot.diagnostics.analyzer; diff --git a/spring-boot/src/main/java/org/springframework/boot/diagnostics/package-info.java b/spring-boot/src/main/java/org/springframework/boot/diagnostics/package-info.java new file mode 100644 index 0000000000..0d669f2d1b --- /dev/null +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/package-info.java @@ -0,0 +1,23 @@ +/* + * Copyright 2012-2014 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Support for failure analysis and reporting. + * + * @see org.springframework.boot.diagnostics.FailureAnalyzer + * @see org.springframework.boot.diagnostics.FailureAnalysisReporter + */ +package org.springframework.boot.diagnostics; diff --git a/spring-boot/src/main/resources/META-INF/spring.factories b/spring-boot/src/main/resources/META-INF/spring.factories index 5f2f151964..0eb778ab8b 100644 --- a/spring-boot/src/main/resources/META-INF/spring.factories +++ b/spring-boot/src/main/resources/META-INF/spring.factories @@ -33,8 +33,8 @@ org.springframework.boot.env.SpringApplicationJsonEnvironmentPostProcessor # Failure Analyzers org.springframework.boot.diagnostics.FailureAnalyzer=\ -org.springframework.boot.context.embedded.PortInUseFailureAnalyzer,\ -org.springframework.boot.diagnostics.BeanCurrentlyInCreationFailureAnalyzer +org.springframework.boot.diagnostics.analyzer.PortInUseFailureAnalyzer,\ +org.springframework.boot.diagnostics.analyzer.BeanCurrentlyInCreationFailureAnalyzer # FailureAnalysisReporters org.springframework.boot.diagnostics.FailureAnalysisReporter=\ diff --git a/spring-boot/src/test/java/org/springframework/boot/diagnostics/BeanCurrentlyInCreationFailureAnalyzerTests.java b/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzerTests.java similarity index 95% rename from spring-boot/src/test/java/org/springframework/boot/diagnostics/BeanCurrentlyInCreationFailureAnalyzerTests.java rename to spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzerTests.java index 8732f09504..f68ff6440e 100644 --- a/spring-boot/src/test/java/org/springframework/boot/diagnostics/BeanCurrentlyInCreationFailureAnalyzerTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/diagnostics/analyzer/BeanCurrentlyInCreationFailureAnalyzerTests.java @@ -14,11 +14,13 @@ * limitations under the License. */ -package org.springframework.boot.diagnostics; +package org.springframework.boot.diagnostics.analyzer; 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.context.ConfigurableApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean;