From 2c60828cf26144eb8b13253aa45fcf95bb7269f6 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 10 Oct 2013 09:43:07 -0400 Subject: [PATCH] Ensure ServerProperties default values does not override Since ServerProperties had primitive properties for port (in particular) it was not possible to check when applying those properties if the user had actually changed the value. This in turn meant that a custom EmbeddedServletContainerFactory could not set the default values. Fixed by making int properties of ServerProperties into Integer and checking for null before setting on the container factory. Fixes gh-84 --- ...erverPropertiesAutoConfigurationTests.java | 74 ++++++++++++++++++- .../embedded/properties/ServerProperties.java | 28 ++++--- .../properties/ServerPropertiesTests.java | 3 +- 3 files changed, 89 insertions(+), 16 deletions(-) diff --git a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesAutoConfigurationTests.java b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesAutoConfigurationTests.java index 84347fc0092..9d3fb42c0a2 100644 --- a/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesAutoConfigurationTests.java +++ b/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/ServerPropertiesAutoConfigurationTests.java @@ -27,11 +27,12 @@ import org.mockito.Mockito; import org.springframework.beans.factory.BeanCreationException; import org.springframework.boot.TestUtils; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; -import org.springframework.boot.autoconfigure.web.ServerPropertiesAutoConfiguration; import org.springframework.boot.context.embedded.AnnotationConfigEmbeddedWebApplicationContext; import org.springframework.boot.context.embedded.ConfigurableEmbeddedServletContainerFactory; +import org.springframework.boot.context.embedded.EmbeddedServletContainerCustomizer; import org.springframework.boot.context.embedded.EmbeddedServletContainerCustomizerBeanPostProcessor; import org.springframework.boot.context.embedded.EmbeddedServletContainerFactory; +import org.springframework.boot.context.embedded.jetty.JettyEmbeddedServletContainerFactory; import org.springframework.boot.context.embedded.properties.ServerProperties; import org.springframework.boot.context.embedded.tomcat.TomcatEmbeddedServletContainerFactory; import org.springframework.context.annotation.Bean; @@ -76,7 +77,7 @@ public class ServerPropertiesAutoConfigurationTests { this.context.refresh(); ServerProperties server = this.context.getBean(ServerProperties.class); assertNotNull(server); - assertEquals(9000, server.getPort()); + assertEquals(9000, server.getPort().intValue()); Mockito.verify(containerFactory).setPort(9000); } @@ -86,12 +87,44 @@ public class ServerPropertiesAutoConfigurationTests { this.context = new AnnotationConfigEmbeddedWebApplicationContext(); this.context.register(Config.class, ServerPropertiesAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class); - TestUtils.addEnviroment(this.context, "server.tomcat.basedir:target/foo"); + TestUtils.addEnviroment(this.context, "server.tomcat.basedir:target/foo", + "server.port:9000"); this.context.refresh(); ServerProperties server = this.context.getBean(ServerProperties.class); assertNotNull(server); assertEquals(new File("target/foo"), server.getTomcat().getBasedir()); - Mockito.verify(containerFactory).setPort(8080); + Mockito.verify(containerFactory).setPort(9000); + } + + @Test + public void customizeWithContainerFactory() throws Exception { + this.context = new AnnotationConfigEmbeddedWebApplicationContext(); + this.context.register(CustomContainerConfig.class, + ServerPropertiesAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class); + this.context.refresh(); + containerFactory = this.context + .getBean(ConfigurableEmbeddedServletContainerFactory.class); + ServerProperties server = this.context.getBean(ServerProperties.class); + assertNotNull(server); + // The server.port environment property was not explicitly set so the container + // factory should take precedence... + assertEquals(3000, containerFactory.getPort()); + } + + @Test + public void customizeTomcatWithCustomizer() throws Exception { + containerFactory = Mockito.mock(TomcatEmbeddedServletContainerFactory.class); + this.context = new AnnotationConfigEmbeddedWebApplicationContext(); + this.context.register(Config.class, CustomizeConfig.class, + ServerPropertiesAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class); + this.context.refresh(); + ServerProperties server = this.context.getBean(ServerProperties.class); + assertNotNull(server); + // The server.port environment property was not explicitly set so the container + // customizer should take precedence... + Mockito.verify(containerFactory).setPort(3000); } @Test @@ -120,6 +153,39 @@ public class ServerPropertiesAutoConfigurationTests { } + @Configuration + protected static class CustomContainerConfig { + + @Bean + public EmbeddedServletContainerFactory containerFactory() { + JettyEmbeddedServletContainerFactory factory = new JettyEmbeddedServletContainerFactory(); + factory.setPort(3000); + return factory; + } + + @Bean + public EmbeddedServletContainerCustomizerBeanPostProcessor embeddedServletContainerCustomizerBeanPostProcessor() { + return new EmbeddedServletContainerCustomizerBeanPostProcessor(); + } + + } + + @Configuration + protected static class CustomizeConfig { + + @Bean + public EmbeddedServletContainerCustomizer containerCustomizer() { + return new EmbeddedServletContainerCustomizer() { + + @Override + public void customize(ConfigurableEmbeddedServletContainerFactory factory) { + factory.setPort(3000); + } + }; + } + + } + @Configuration protected static class MutiServerPropertiesBeanConfig { diff --git a/spring-boot/src/main/java/org/springframework/boot/context/embedded/properties/ServerProperties.java b/spring-boot/src/main/java/org/springframework/boot/context/embedded/properties/ServerProperties.java index b6d3996b7a7..a91e8b7c23a 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/embedded/properties/ServerProperties.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/embedded/properties/ServerProperties.java @@ -47,11 +47,11 @@ import org.springframework.util.StringUtils; @ConfigurationProperties(name = "server", ignoreUnknownFields = false) public class ServerProperties implements EmbeddedServletContainerCustomizer { - private int port = 8080; + private Integer port; private InetAddress address; - private int sessionTimeout = 30; + private Integer sessionTimeout; @NotNull private String contextPath = ""; @@ -70,11 +70,11 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer { this.contextPath = contextPath; } - public int getPort() { + public Integer getPort() { return this.port; } - public void setPort(int port) { + public void setPort(Integer port) { this.port = port; } @@ -86,11 +86,11 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer { this.address = address; } - public int getSessionTimeout() { + public Integer getSessionTimeout() { return this.sessionTimeout; } - public void setSessionTimeout(int sessionTimeout) { + public void setSessionTimeout(Integer sessionTimeout) { this.sessionTimeout = sessionTimeout; } @@ -100,10 +100,18 @@ public class ServerProperties implements EmbeddedServletContainerCustomizer { @Override public void customize(ConfigurableEmbeddedServletContainerFactory factory) { - factory.setPort(getPort()); - factory.setAddress(getAddress()); - factory.setContextPath(getContextPath()); - factory.setSessionTimeout(getSessionTimeout()); + if (getPort() != null) { + factory.setPort(getPort()); + } + if (getAddress() != null) { + factory.setAddress(getAddress()); + } + if (getContextPath() != null) { + factory.setContextPath(getContextPath()); + } + if (getSessionTimeout() != null) { + factory.setSessionTimeout(getSessionTimeout()); + } if (factory instanceof TomcatEmbeddedServletContainerFactory) { getTomcat().customizeTomcat((TomcatEmbeddedServletContainerFactory) factory); } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/embedded/properties/ServerPropertiesTests.java b/spring-boot/src/test/java/org/springframework/boot/context/embedded/properties/ServerPropertiesTests.java index 6a796e67ef5..bda19a6b82f 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/embedded/properties/ServerPropertiesTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/embedded/properties/ServerPropertiesTests.java @@ -24,7 +24,6 @@ import java.util.Map; import org.junit.Test; import org.springframework.beans.MutablePropertyValues; import org.springframework.boot.bind.RelaxedDataBinder; -import org.springframework.boot.context.embedded.properties.ServerProperties; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -51,7 +50,7 @@ public class ServerPropertiesTests { public void testPortBinding() throws Exception { new RelaxedDataBinder(this.properties, "server").bind(new MutablePropertyValues( Collections.singletonMap("server.port", "9000"))); - assertEquals(9000, this.properties.getPort()); + assertEquals(9000, this.properties.getPort().intValue()); } @Test