From 471947b4007e612fe2f31e4bf32435cc03f527b6 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 16 Dec 2015 17:43:48 +0000 Subject: [PATCH] Be more defensive when instantiating custom ServerProperties If the user provides their own ServerProperties bean we want to peek at it to see if they set the port (and only that) when we are deciding if the actuator context needs to be created. This happens very early (in a @Condition) so we need to be very defensive. There are already quite a few checks in place to prevent a ServerProperties bean from being instantiated unless we really need it, and yet, when it is we can do more. This change creates the bean (and the ManagementProperties) in a throwaway BeanFactory using the same BeanDefinition as the main context. This ensures that when the main context bean is created it will be in the "natural" order and binding to the Environment can take place as normal. Fixes gh-4631 --- .../EndpointWebMvcAutoConfiguration.java | 24 ++++++++++++++++--- .../EndpointWebMvcAutoConfigurationTests.java | 20 ++++++++++++++++ ...urationPropertiesBindingPostProcessor.java | 1 + 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java index c6c792f638..b0e8106177 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfiguration.java @@ -34,6 +34,7 @@ import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.annotation.AnnotatedBeanDefinition; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.endpoint.mvc.ManagementServletContext; import org.springframework.boot.autoconfigure.AutoConfigureAfter; @@ -314,15 +315,15 @@ public class EndpointWebMvcAutoConfiguration Integer serverPort = getPortProperty(environment, "server."); if (serverPort == null && hasCustomBeanDefinition(beanFactory, ServerProperties.class, ServerPropertiesAutoConfiguration.class)) { - ServerProperties bean = beanFactory.getBean(ServerProperties.class); + ServerProperties bean = getBean(beanFactory, ServerProperties.class); serverPort = bean.getPort(); } Integer managementPort = getPortProperty(environment, "management."); if (managementPort == null && hasCustomBeanDefinition(beanFactory, ManagementServerProperties.class, ManagementServerPropertiesAutoConfiguration.class)) { - ManagementServerProperties bean = beanFactory - .getBean(ManagementServerProperties.class); + ManagementServerProperties bean = getBean(beanFactory, + ManagementServerProperties.class); managementPort = bean.getPort(); } if (managementPort != null && managementPort < 0) { @@ -334,6 +335,23 @@ public class EndpointWebMvcAutoConfiguration : DIFFERENT); } + private static T getBean(BeanFactory beanFactory, Class type) { + if (!(beanFactory instanceof ConfigurableListableBeanFactory)) { + return null; + } + ConfigurableListableBeanFactory listable = (ConfigurableListableBeanFactory) beanFactory; + String[] names = listable.getBeanNamesForType(type, true, false); + if (names == null || names.length != 1) { + return null; + } + // Use a temporary child bean factory to avoid instantiating the bean in the + // parent (it won't be bound to the environment yet) + BeanDefinition definition = listable.getBeanDefinition(names[0]); + DefaultListableBeanFactory temp = new DefaultListableBeanFactory(listable); + temp.registerBeanDefinition(type.getName(), definition); + return temp.getBean(type); + } + private static Integer getPortProperty(Environment environment, String prefix) { RelaxedPropertyResolver resolver = new RelaxedPropertyResolver(environment, prefix); diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfigurationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfigurationTests.java index ee165039dc..b99db580a9 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfigurationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/autoconfigure/EndpointWebMvcAutoConfigurationTests.java @@ -320,6 +320,26 @@ public class EndpointWebMvcAutoConfigurationTests { assertAllClosed(); } + @Test + public void overrideServerProperties() throws Exception { + EnvironmentTestUtils.addEnvironment(this.applicationContext, + "server.displayName:foo"); + this.applicationContext.register(RootConfig.class, EndpointConfig.class, + ServerPortConfig.class, PropertyPlaceholderAutoConfiguration.class, + ManagementServerPropertiesAutoConfiguration.class, + ServerPropertiesAutoConfiguration.class, JacksonAutoConfiguration.class, + EmbeddedServletContainerAutoConfiguration.class, + HttpMessageConvertersAutoConfiguration.class, + DispatcherServletAutoConfiguration.class, WebMvcAutoConfiguration.class, + EndpointWebMvcAutoConfiguration.class); + this.applicationContext.refresh(); + assertContent("/controller", ports.get().server, "controlleroutput"); + assertEquals("foo", + this.applicationContext.getBean(ServerProperties.class).getDisplayName()); + this.applicationContext.close(); + assertAllClosed(); + } + @Test public void portPropertiesOnSamePort() throws Exception { this.applicationContext.register(RootConfig.class, BaseConfiguration.class, diff --git a/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java b/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java index c68acd7618..8b8f2bacec 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/properties/ConfigurationPropertiesBindingPostProcessor.java @@ -232,6 +232,7 @@ public class ConfigurationPropertiesBindingPostProcessor PropertySourcesPlaceholderConfigurer configurer = getSinglePropertySourcesPlaceholderConfigurer(); if (configurer != null) { // Flatten the sources into a single list so they can be iterated + // TODO: maybe we don't really need this (and it has lifecycle implications) return new FlatPropertySources(configurer.getAppliedPropertySources()); } if (this.environment instanceof ConfigurableEnvironment) {