From b2420be8867e854c4abf8876335a4aa911661890 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 10 Aug 2016 17:31:13 +0100 Subject: [PATCH] Load FailureAnalyzers defensively Previously, if a single FailureAnalyzer failed to load, no failure analysis would be performed. This commit updates FailureAnalyzers to load the analysers defensively so that all but the problematic analyzer are used for analysis. Closes gh-6606 --- .../boot/diagnostics/FailureAnalyzers.java | 32 ++++- .../FailureAnalyzersIntegrationTests.java | 64 ++++++++++ .../diagnostics/FailureAnalyzersTests.java | 109 ++++++++++++++---- .../failure-analyzers-tests/basic.factories | 4 + .../failure-analyzers-tests/broken.factories | 4 + 5 files changed, 188 insertions(+), 25 deletions(-) create mode 100644 spring-boot/src/test/java/org/springframework/boot/diagnostics/FailureAnalyzersIntegrationTests.java create mode 100644 spring-boot/src/test/resources/failure-analyzers-tests/basic.factories create mode 100644 spring-boot/src/test/resources/failure-analyzers-tests/broken.factories 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 index e4a2e04a149..1effcbdd44d 100644 --- a/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java +++ b/spring-boot/src/main/java/org/springframework/boot/diagnostics/FailureAnalyzers.java @@ -16,12 +16,20 @@ package org.springframework.boot.diagnostics; +import java.lang.reflect.Constructor; +import java.util.ArrayList; import java.util.List; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.core.annotation.AnnotationAwareOrderComparator; import org.springframework.core.io.support.SpringFactoriesLoader; +import org.springframework.util.ClassUtils; +import org.springframework.util.ReflectionUtils; /** * Utility to trigger {@link FailureAnalyzer} and {@link FailureAnalysisReporter} @@ -38,19 +46,39 @@ import org.springframework.core.io.support.SpringFactoriesLoader; */ public final class FailureAnalyzers { + private static final Log log = LogFactory.getLog(FailureAnalyzers.class); + private FailureAnalyzers() { } public static boolean analyzeAndReport(Throwable failure, ClassLoader classLoader, ConfigurableApplicationContext context) { - List analyzers = SpringFactoriesLoader - .loadFactories(FailureAnalyzer.class, classLoader); + List analyzers = loadFailureAnalyzers(classLoader); List reporters = SpringFactoriesLoader .loadFactories(FailureAnalysisReporter.class, classLoader); FailureAnalysis analysis = analyze(failure, analyzers, context); return report(analysis, reporters); } + private static List loadFailureAnalyzers(ClassLoader classLoader) { + List analyzerNames = SpringFactoriesLoader + .loadFactoryNames(FailureAnalyzer.class, classLoader); + List analyzers = new ArrayList(); + for (String analyzerName : analyzerNames) { + try { + Constructor constructor = ClassUtils.forName(analyzerName, classLoader) + .getDeclaredConstructor(); + ReflectionUtils.makeAccessible(constructor); + analyzers.add((FailureAnalyzer) constructor.newInstance()); + } + catch (Throwable ex) { + log.trace("Failed to load " + analyzerName, ex); + } + } + AnnotationAwareOrderComparator.sort(analyzers); + return analyzers; + } + private static FailureAnalysis analyze(Throwable failure, List analyzers, ConfigurableApplicationContext context) { for (FailureAnalyzer analyzer : analyzers) { diff --git a/spring-boot/src/test/java/org/springframework/boot/diagnostics/FailureAnalyzersIntegrationTests.java b/spring-boot/src/test/java/org/springframework/boot/diagnostics/FailureAnalyzersIntegrationTests.java new file mode 100644 index 00000000000..b82f2d933f7 --- /dev/null +++ b/spring-boot/src/test/java/org/springframework/boot/diagnostics/FailureAnalyzersIntegrationTests.java @@ -0,0 +1,64 @@ +/* + * 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 javax.annotation.PostConstruct; + +import org.junit.Rule; +import org.junit.Test; + +import org.springframework.boot.builder.SpringApplicationBuilder; +import org.springframework.boot.context.embedded.PortInUseException; +import org.springframework.boot.testutil.InternalOutputCapture; +import org.springframework.context.annotation.Configuration; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.fail; + +/** + * Integration tests for {@link FailureAnalyzers}. + * + * @author Andy Wilkinson + */ +public class FailureAnalyzersIntegrationTests { + + @Rule + public InternalOutputCapture outputCapture = new InternalOutputCapture(); + + @Test + public void analysisIsPerformed() { + try { + new SpringApplicationBuilder(TestConfiguration.class).web(false).run(); + fail("Application started successfully"); + } + catch (Exception ex) { + assertThat(this.outputCapture.toString()) + .contains("APPLICATION FAILED TO START"); + } + } + + @Configuration + static class TestConfiguration { + + @PostConstruct + public void fail() { + throw new PortInUseException(8080); + } + + } + +} diff --git a/spring-boot/src/test/java/org/springframework/boot/diagnostics/FailureAnalyzersTests.java b/spring-boot/src/test/java/org/springframework/boot/diagnostics/FailureAnalyzersTests.java index 9940f2a8640..9a8137b0a3d 100644 --- a/spring-boot/src/test/java/org/springframework/boot/diagnostics/FailureAnalyzersTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/diagnostics/FailureAnalyzersTests.java @@ -16,47 +16,110 @@ package org.springframework.boot.diagnostics; -import javax.annotation.PostConstruct; +import java.io.IOException; +import java.net.URL; +import java.util.Enumeration; -import org.junit.Rule; +import org.junit.Before; import org.junit.Test; -import org.springframework.boot.builder.SpringApplicationBuilder; -import org.springframework.boot.context.embedded.PortInUseException; -import org.springframework.boot.testutil.InternalOutputCapture; -import org.springframework.context.annotation.Configuration; +import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.BeanFactoryAware; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; -import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; /** - * Tests for {@link FailureAnalyzers} + * Tests for {@link FailureAnalyzers}. * * @author Andy Wilkinson */ public class FailureAnalyzersTests { - @Rule - public InternalOutputCapture outputCapture = new InternalOutputCapture(); + private static BeanFactoryAwareFailureAnalyzer failureAnalyzer; + + @Before + public void configureMock() { + failureAnalyzer = mock(BeanFactoryAwareFailureAnalyzer.class); + } @Test - public void analysisIsPerformed() { - try { - new SpringApplicationBuilder(TestConfiguration.class).web(false).run(); - fail("Application started successfully"); + public void analyzersAreLoadedAndCalled() { + RuntimeException failure = new RuntimeException(); + analyzeAndReport("basic.factories", failure); + verify(failureAnalyzer, times(2)).analyze(failure); + } + + @Test + public void beanFactoryIsInjectedIntoBeanFactoryAwareFailureAnalyzers() { + RuntimeException failure = new RuntimeException(); + analyzeAndReport("basic.factories", failure); + verify(failureAnalyzer).setBeanFactory(any(BeanFactory.class)); + } + + @Test + public void brokenAnalyzerDoesNotPreventOtherAnalyzersFromBeingCalled() { + RuntimeException failure = new RuntimeException(); + analyzeAndReport("broken.factories", failure); + verify(failureAnalyzer, times(1)).analyze(failure); + } + + private void analyzeAndReport(final String factoriesName, Throwable failure) { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + + FailureAnalyzers.analyzeAndReport(failure, + new ClassLoader(getClass().getClassLoader()) { + + @Override + public Enumeration getResources(String name) throws IOException { + if ("META-INF/spring.factories".equals(name)) { + return super.getResources( + "failure-analyzers-tests/" + factoriesName); + } + else { + return super.getResources(name); + } + } + + }, context); + } + + static class BasicFailureAnalyzer implements FailureAnalyzer { + + @Override + public FailureAnalysis analyze(Throwable failure) { + return failureAnalyzer.analyze(failure); } - catch (Exception ex) { - assertThat(this.outputCapture.toString()) - .contains("APPLICATION FAILED TO START"); + + } + + static class BrokenFailureAnalyzer implements FailureAnalyzer { + + static { + Object foo = null; + foo.toString(); + } + + @Override + public FailureAnalysis analyze(Throwable failure) { + return null; } } - @Configuration - static class TestConfiguration { + interface BeanFactoryAwareFailureAnalyzer extends BeanFactoryAware, FailureAnalyzer { - @PostConstruct - public void fail() { - throw new PortInUseException(8080); + } + + static class StandardBeanFactoryAwareFailureAnalyzer extends BasicFailureAnalyzer + implements BeanFactoryAwareFailureAnalyzer { + + @Override + public void setBeanFactory(BeanFactory beanFactory) throws BeansException { + failureAnalyzer.setBeanFactory(beanFactory); } } diff --git a/spring-boot/src/test/resources/failure-analyzers-tests/basic.factories b/spring-boot/src/test/resources/failure-analyzers-tests/basic.factories new file mode 100644 index 00000000000..d67b9cb731b --- /dev/null +++ b/spring-boot/src/test/resources/failure-analyzers-tests/basic.factories @@ -0,0 +1,4 @@ +# Failure Analyzers +org.springframework.boot.diagnostics.FailureAnalyzer=\ +org.springframework.boot.diagnostics.FailureAnalyzersTests$BasicFailureAnalyzer,\ +org.springframework.boot.diagnostics.FailureAnalyzersTests$StandardBeanFactoryAwareFailureAnalyzer diff --git a/spring-boot/src/test/resources/failure-analyzers-tests/broken.factories b/spring-boot/src/test/resources/failure-analyzers-tests/broken.factories new file mode 100644 index 00000000000..8f9e5e4d91b --- /dev/null +++ b/spring-boot/src/test/resources/failure-analyzers-tests/broken.factories @@ -0,0 +1,4 @@ +# Failure Analyzers +org.springframework.boot.diagnostics.FailureAnalyzer=\ +org.springframework.boot.diagnostics.FailureAnalyzersTests$BrokenFailureAnalyzer,\ +org.springframework.boot.diagnostics.FailureAnalyzersTests$BasicFailureAnalyzer