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

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<FailureAnalyzer> analyzers = SpringFactoriesLoader
-				.loadFactories(FailureAnalyzer.class, classLoader);
+		List<FailureAnalyzer> analyzers = loadFailureAnalyzers(classLoader);
 		List<FailureAnalysisReporter> reporters = SpringFactoriesLoader
 				.loadFactories(FailureAnalysisReporter.class, classLoader);
 		FailureAnalysis analysis = analyze(failure, analyzers, context);
 		return report(analysis, reporters);
 	}
 
+	private static List<FailureAnalyzer> loadFailureAnalyzers(ClassLoader classLoader) {
+		List<String> analyzerNames = SpringFactoriesLoader
+				.loadFactoryNames(FailureAnalyzer.class, classLoader);
+		List<FailureAnalyzer> analyzers = new ArrayList<FailureAnalyzer>();
+		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<FailureAnalyzer> analyzers, ConfigurableApplicationContext context) {
 		for (FailureAnalyzer analyzer : analyzers) {

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);
+		}
+
+	}
+
+}

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 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 analysisIsPerformed() {
-		try {
-			new SpringApplicationBuilder(TestConfiguration.class).web(false).run();
-			fail("Application started successfully");
+	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<URL> getResources(String name) throws IOException {
+						if ("META-INF/spring.factories".equals(name)) {
+							return super.getResources(
+									"failure-analyzers-tests/" + factoriesName);
+						}
+						else {
+							return super.getResources(name);
+						}
 					}
-		catch (Exception ex) {
-			assertThat(this.outputCapture.toString())
-					.contains("APPLICATION FAILED TO START");
+
+				}, context);
+	}
+
+	static class BasicFailureAnalyzer implements FailureAnalyzer {
+
+		@Override
+		public FailureAnalysis analyze(Throwable failure) {
+			return failureAnalyzer.analyze(failure);
+		}
+
+	}
+
+	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 {
+
+	}
+
+	static class StandardBeanFactoryAwareFailureAnalyzer extends BasicFailureAnalyzer
+			implements BeanFactoryAwareFailureAnalyzer {
 
-		@PostConstruct
-		public void fail() {
-			throw new PortInUseException(8080);
+		@Override
+		public void setBeanFactory(BeanFactory beanFactory) throws BeansException {
+			failureAnalyzer.setBeanFactory(beanFactory);
 		}
 
 	}

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

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