Polish "Introduce HealthIndicatorRegistry"

This commit is work in progress and polishes the initial submission by
making sure that the CompositeHealthIndicator is also a registry.

The current infrastructure prevents the composite to be registered as a
bean as it would be an extra HealthIndicator in the context and
therefore may be taken into account as a regular indicator. The registry
on the other hand must be registered in the context so that users can
manipulate its content.

Ultimately, both those objects share common base features that we should
not duplicate. This commit makes sure that the registry has a `health()`
method but does not implement HealthIndicator. The composite now extends
from the registry and implements HealthIndicator, simply delegating to
that parent method.

It's unclear at this point if we want to keep this arrangement as the
duplication of factories is a bit annoying.

See gh-4965

Author: snicoll

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/health/CompositeHealthIndicatorConfiguration.java

@@ -44,7 +44,7 @@ protected HealthIndicator createHealthIndicator(Map<String, S> beans) {
 		}
 		CompositeHealthIndicator composite = new CompositeHealthIndicator(
 				this.healthAggregator);
-		beans.forEach((name, source) -> composite.addHealthIndicator(name,
+		beans.forEach((name, source) -> composite.register(name,
 				createHealthIndicator(source)));
 		return composite;
 	}

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/health/HealthEndpointConfiguration.java

@@ -16,41 +16,28 @@
 
 package org.springframework.boot.actuate.autoconfigure.health;
 
-import org.springframework.beans.factory.ObjectProvider;
 import org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnEnabledEndpoint;
-import org.springframework.boot.actuate.health.HealthAggregator;
 import org.springframework.boot.actuate.health.HealthEndpoint;
 import org.springframework.boot.actuate.health.HealthIndicatorRegistry;
-import org.springframework.boot.actuate.health.OrderedHealthAggregator;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnSingleCandidate;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 
 /**
  * Configuration for {@link HealthEndpoint}.
  *
  * @author Stephane Nicoll
- * @author Vedran Pavic
  */
 @Configuration
+@ConditionalOnSingleCandidate(HealthIndicatorRegistry.class)
 class HealthEndpointConfiguration {
 
-	private final HealthAggregator healthAggregator;
-
-	private final HealthIndicatorRegistry healthIndicatorRegistry;
-
-	HealthEndpointConfiguration(ObjectProvider<HealthAggregator> healthAggregator,
-			ObjectProvider<HealthIndicatorRegistry> healthIndicatorRegistry) {
-		this.healthAggregator = healthAggregator
-				.getIfAvailable(OrderedHealthAggregator::new);
-		this.healthIndicatorRegistry = healthIndicatorRegistry.getObject();
-	}
-
 	@Bean
 	@ConditionalOnMissingBean
 	@ConditionalOnEnabledEndpoint
-	public HealthEndpoint healthEndpoint() {
-		return new HealthEndpoint(this.healthAggregator, this.healthIndicatorRegistry);
+	public HealthEndpoint healthEndpoint(HealthIndicatorRegistry registry) {
+		return new HealthEndpoint(registry);
 	}
 
 }

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/health/HealthIndicatorAutoConfiguration.java

@@ -16,11 +16,7 @@
 
 package org.springframework.boot.actuate.autoconfigure.health;
 
-import java.util.LinkedHashMap;
-import java.util.Map;
-
 import org.springframework.boot.actuate.health.ApplicationHealthIndicator;
-import org.springframework.boot.actuate.health.DefaultHealthIndicatorRegistry;
 import org.springframework.boot.actuate.health.HealthAggregator;
 import org.springframework.boot.actuate.health.HealthIndicator;
 import org.springframework.boot.actuate.health.HealthIndicatorRegistry;
@@ -32,7 +28,6 @@
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.util.ClassUtils;
 
 /**
  * {@link EnableAutoConfiguration Auto-configuration} for {@link HealthIndicator}s.
@@ -73,30 +68,7 @@ public OrderedHealthAggregator healthAggregator() {
 	@ConditionalOnMissingBean(HealthIndicatorRegistry.class)
 	public HealthIndicatorRegistry healthIndicatorRegistry(
 			ApplicationContext applicationContext) {
-		HealthIndicatorRegistry registry = new DefaultHealthIndicatorRegistry();
-		Map<String, HealthIndicator> indicators = new LinkedHashMap<>();
-		indicators.putAll(applicationContext.getBeansOfType(HealthIndicator.class));
-		if (ClassUtils.isPresent("reactor.core.publisher.Flux", null)) {
-			new ReactiveHealthIndicators().get(applicationContext)
-					.forEach(indicators::putIfAbsent);
-		}
-		indicators.forEach(registry::register);
-		return registry;
-	}
-
-	private static class ReactiveHealthIndicators {
-
-		public Map<String, HealthIndicator> get(ApplicationContext applicationContext) {
-			Map<String, HealthIndicator> indicators = new LinkedHashMap<>();
-			applicationContext.getBeansOfType(ReactiveHealthIndicator.class)
-					.forEach((name, indicator) -> indicators.put(name, adapt(indicator)));
-			return indicators;
-		}
-
-		private HealthIndicator adapt(ReactiveHealthIndicator indicator) {
-			return () -> indicator.health().block();
-		}
-
+		return HealthIndicatorRegistryBeans.get(applicationContext);
 	}
 
 }

spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/health/HealthIndicatorRegistryBeans.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright 2012-2018 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.actuate.autoconfigure.health;
+
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import org.springframework.beans.factory.NoSuchBeanDefinitionException;
+import org.springframework.boot.actuate.health.HealthAggregator;
+import org.springframework.boot.actuate.health.HealthIndicator;
+import org.springframework.boot.actuate.health.HealthIndicatorRegistry;
+import org.springframework.boot.actuate.health.HealthIndicatorRegistryFactory;
+import org.springframework.boot.actuate.health.OrderedHealthAggregator;
+import org.springframework.boot.actuate.health.ReactiveHealthIndicator;
+import org.springframework.context.ApplicationContext;
+import org.springframework.util.ClassUtils;
+
+/**
+ * Creates a {@link HealthIndicatorRegistry} from beans in the {@link ApplicationContext}.
+ *
+ * @author Phillip Webb
+ * @author Stephane Nicoll
+ */
+final class HealthIndicatorRegistryBeans {
+
+	private HealthIndicatorRegistryBeans() {
+	}
+
+	public static HealthIndicatorRegistry get(ApplicationContext applicationContext) {
+		HealthAggregator healthAggregator = getHealthAggregator(applicationContext);
+		Map<String, HealthIndicator> indicators = new LinkedHashMap<>();
+		indicators.putAll(applicationContext.getBeansOfType(HealthIndicator.class));
+		if (ClassUtils.isPresent("reactor.core.publisher.Flux", null)) {
+			new ReactiveHealthIndicators().get(applicationContext)
+					.forEach(indicators::putIfAbsent);
+		}
+		HealthIndicatorRegistryFactory factory = new HealthIndicatorRegistryFactory();
+		return factory.createHealthIndicatorRegistry(healthAggregator, indicators);
+	}
+
+	private static HealthAggregator getHealthAggregator(
+			ApplicationContext applicationContext) {
+		try {
+			return applicationContext.getBean(HealthAggregator.class);
+		}
+		catch (NoSuchBeanDefinitionException ex) {
+			return new OrderedHealthAggregator();
+		}
+	}
+
+	private static class ReactiveHealthIndicators {
+
+		public Map<String, HealthIndicator> get(ApplicationContext applicationContext) {
+			Map<String, HealthIndicator> indicators = new LinkedHashMap<>();
+			applicationContext.getBeansOfType(ReactiveHealthIndicator.class)
+					.forEach((name, indicator) -> indicators.put(name, adapt(indicator)));
+			return indicators;
+		}
+
+		private HealthIndicator adapt(ReactiveHealthIndicator indicator) {
+			return () -> indicator.health().block();
+		}
+
+	}
+
+}

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/cloudfoundry/CloudFoundryWebEndpointDiscovererTests.java

@@ -34,7 +34,6 @@
 import org.springframework.boot.actuate.endpoint.web.PathMapper;
 import org.springframework.boot.actuate.endpoint.web.WebOperation;
 import org.springframework.boot.actuate.endpoint.web.annotation.EndpointWebExtension;
-import org.springframework.boot.actuate.health.HealthAggregator;
 import org.springframework.boot.actuate.health.HealthEndpoint;
 import org.springframework.boot.actuate.health.HealthIndicatorRegistry;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
@@ -110,8 +109,7 @@ public TestEndpointWebExtension testEndpointWebExtension() {
 
 		@Bean
 		public HealthEndpoint healthEndpoint() {
-			return new HealthEndpoint(mock(HealthAggregator.class),
-					mock(HealthIndicatorRegistry.class));
+			return new HealthEndpoint(mock(HealthIndicatorRegistry.class));
 		}
 
 		@Bean

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/HealthEndpointDocumentationTests.java

@@ -23,10 +23,9 @@
 
 import org.junit.Test;
 
-import org.springframework.boot.actuate.health.DefaultHealthIndicatorRegistry;
+import org.springframework.boot.actuate.health.CompositeHealthIndicator;
 import org.springframework.boot.actuate.health.HealthEndpoint;
 import org.springframework.boot.actuate.health.HealthIndicator;
-import org.springframework.boot.actuate.health.HealthIndicatorRegistry;
 import org.springframework.boot.actuate.health.OrderedHealthAggregator;
 import org.springframework.boot.actuate.jdbc.DataSourceHealthIndicator;
 import org.springframework.boot.actuate.system.DiskSpaceHealthIndicator;
@@ -73,9 +72,8 @@ static class TestConfiguration {
 
 		@Bean
 		public HealthEndpoint endpoint(Map<String, HealthIndicator> healthIndicators) {
-			HealthIndicatorRegistry registry = new DefaultHealthIndicatorRegistry();
-			healthIndicators.forEach(registry::register);
-			return new HealthEndpoint(new OrderedHealthAggregator(), registry);
+			return new HealthEndpoint(new CompositeHealthIndicator(
+					new OrderedHealthAggregator(), healthIndicators));
 		}
 
 		@Bean

spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/integrationtest/JmxEndpointIntegrationTests.java

@@ -48,7 +48,8 @@ public class JmxEndpointIntegrationTests {
 	private final WebApplicationContextRunner contextRunner = new WebApplicationContextRunner()
 			.withConfiguration(AutoConfigurations.of(JmxAutoConfiguration.class,
 					EndpointAutoConfiguration.class, JmxEndpointAutoConfiguration.class,
-					HttpTraceAutoConfiguration.class, HealthIndicatorAutoConfiguration.class))
+					HealthIndicatorAutoConfiguration.class,
+					HttpTraceAutoConfiguration.class))
 			.withConfiguration(
 					AutoConfigurations.of(EndpointAutoConfigurationClasses.ALL));
 

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/CompositeHealthIndicator.java

@@ -16,11 +16,8 @@
 
 package org.springframework.boot.actuate.health;
 
-import java.util.LinkedHashMap;
 import java.util.Map;
 
-import org.springframework.util.Assert;
-
 /**
  * {@link HealthIndicator} that returns health indications from all registered delegates.
  *
@@ -29,18 +26,15 @@
  * @author Christian Dupuis
  * @since 1.1.0
  */
-public class CompositeHealthIndicator implements HealthIndicator {
-
-	private final Map<String, HealthIndicator> indicators;
-
-	private final HealthAggregator healthAggregator;
+public class CompositeHealthIndicator
+		extends DefaultHealthIndicatorRegistry implements HealthIndicator {
 
 	/**
 	 * Create a new {@link CompositeHealthIndicator}.
 	 * @param healthAggregator the health aggregator
 	 */
 	public CompositeHealthIndicator(HealthAggregator healthAggregator) {
-		this(healthAggregator, new LinkedHashMap<>());
+		super(healthAggregator);
 	}
 
 	/**
@@ -51,23 +45,21 @@ public CompositeHealthIndicator(HealthAggregator healthAggregator) {
 	 */
 	public CompositeHealthIndicator(HealthAggregator healthAggregator,
 			Map<String, HealthIndicator> indicators) {
-		Assert.notNull(healthAggregator, "HealthAggregator must not be null");
-		Assert.notNull(indicators, "Indicators must not be null");
-		this.indicators = new LinkedHashMap<>(indicators);
-		this.healthAggregator = healthAggregator;
+		super(healthAggregator, indicators);
 	}
 
+	/**
+	 * Registers the given {@code healthIndicator}, associating it with the given
+	 * {@code name}.
+	 * @param name the name of the indicator
+	 * @param indicator the indicator
+	 * @throws IllegalStateException if an indicator with the given {@code name} is
+	 * already registered.
+	 * @deprecated as of 2.1.0 in favour of {@link #register(String, HealthIndicator)}
+	 */
+	@Deprecated
 	public void addHealthIndicator(String name, HealthIndicator indicator) {
-		this.indicators.put(name, indicator);
-	}
-
-	@Override
-	public Health health() {
-		Map<String, Health> healths = new LinkedHashMap<>();
-		for (Map.Entry<String, HealthIndicator> entry : this.indicators.entrySet()) {
-			healths.put(entry.getKey(), entry.getValue().health());
-		}
-		return this.healthAggregator.aggregate(healths);
+		register(name, indicator);
 	}
 
 }

spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/health/CompositeHealthIndicatorFactory.java

@@ -27,17 +27,15 @@
  * @author Stephane Nicoll
  * @since 2.0.0
  */
-public class CompositeHealthIndicatorFactory {
-
-	private final Function<String, String> healthIndicatorNameFactory;
+public class CompositeHealthIndicatorFactory extends HealthIndicatorRegistryFactory {
 
 	public CompositeHealthIndicatorFactory(
 			Function<String, String> healthIndicatorNameFactory) {
-		this.healthIndicatorNameFactory = healthIndicatorNameFactory;
+		super(healthIndicatorNameFactory);
 	}
 
 	public CompositeHealthIndicatorFactory() {
-		this(new HealthIndicatorNameFactory());
+		super();
 	}
 
 	/**
@@ -52,13 +50,8 @@ public CompositeHealthIndicator createHealthIndicator(
 			Map<String, HealthIndicator> healthIndicators) {
 		Assert.notNull(healthAggregator, "HealthAggregator must not be null");
 		Assert.notNull(healthIndicators, "HealthIndicators must not be null");
-		CompositeHealthIndicator healthIndicator = new CompositeHealthIndicator(
-				healthAggregator);
-		for (Map.Entry<String, HealthIndicator> entry : healthIndicators.entrySet()) {
-			String name = this.healthIndicatorNameFactory.apply(entry.getKey());
-			healthIndicator.addHealthIndicator(name, entry.getValue());
-		}
-		return healthIndicator;
+		return initialize(new CompositeHealthIndicator(healthAggregator),
+				healthIndicators);
 	}
 
 }