Polish "Fix connection timeout configuration for Netty"

See gh-16535

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java

@@ -24,7 +24,6 @@ import org.springframework.boot.autoconfigure.web.ServerProperties;
 import org.springframework.boot.cloud.CloudPlatform;
 import org.springframework.boot.context.properties.PropertyMapper;
 import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory;
-import org.springframework.boot.web.embedded.netty.NettyServerCustomizer;
 import org.springframework.boot.web.server.WebServerFactoryCustomizer;
 import org.springframework.core.Ordered;
 import org.springframework.core.env.Environment;
@@ -59,12 +58,10 @@ public class NettyWebServerFactoryCustomizer
 	public void customize(NettyReactiveWebServerFactory factory) {
 		factory.setUseForwardHeaders(getOrDeduceUseForwardHeaders(this.serverProperties, this.environment));
 		PropertyMapper propertyMapper = PropertyMapper.get().alwaysApplyingWhenNonNull();
-		propertyMapper.from(this.serverProperties::getMaxHttpHeaderSize).asInt(DataSize::toBytes)
+		propertyMapper.from(this.serverProperties::getMaxHttpHeaderSize)
 				.to((maxHttpRequestHeaderSize) -> customizeMaxHttpHeaderSize(factory, maxHttpRequestHeaderSize));
-		propertyMapper.from(this.serverProperties::getConnectionTimeout).asInt(Duration::toMillis)
-				.whenNot((connectionTimout) -> connectionTimout.equals(0))
-				.as((connectionTimeout) -> connectionTimeout.equals(-1) ? 0 : connectionTimeout)
-				.to((duration) -> factory.addServerCustomizers(getConnectionTimeOutCustomizer(duration)));
+		propertyMapper.from(this.serverProperties::getConnectionTimeout)
+				.to((connectionTimeout) -> customizeConnectionTimeout(factory, connectionTimeout));
 	}
 
 	private boolean getOrDeduceUseForwardHeaders(ServerProperties serverProperties, Environment environment) {
@@ -75,14 +72,17 @@ public class NettyWebServerFactoryCustomizer
 		return platform != null && platform.isUsingForwardHeaders();
 	}
 
-	private void customizeMaxHttpHeaderSize(NettyReactiveWebServerFactory factory, Integer maxHttpHeaderSize) {
-		factory.addServerCustomizers((NettyServerCustomizer) (httpServer) -> httpServer.httpRequestDecoder(
-				(httpRequestDecoderSpec) -> httpRequestDecoderSpec.maxHeaderSize(maxHttpHeaderSize)));
+	private void customizeMaxHttpHeaderSize(NettyReactiveWebServerFactory factory, DataSize maxHttpHeaderSize) {
+		factory.addServerCustomizers((httpServer) -> httpServer.httpRequestDecoder(
+				(httpRequestDecoderSpec) -> httpRequestDecoderSpec.maxHeaderSize((int) maxHttpHeaderSize.toBytes())));
 	}
 
-	private NettyServerCustomizer getConnectionTimeOutCustomizer(int duration) {
-		return (httpServer) -> httpServer.tcpConfiguration(
-				(tcpServer) -> tcpServer.selectorOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, duration));
+	private void customizeConnectionTimeout(NettyReactiveWebServerFactory factory, Duration connectionTimeout) {
+		if (!connectionTimeout.isZero()) {
+			long timeoutMillis = connectionTimeout.isNegative() ? 0 : connectionTimeout.toMillis();
+			factory.addServerCustomizers((httpServer) -> httpServer.tcpConfiguration((tcpServer) -> tcpServer
+					.selectorOption(ChannelOption.CONNECT_TIMEOUT_MILLIS, (int) timeoutMillis)));
+		}
 	}
 
 }

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java

@@ -17,18 +17,29 @@
 package org.springframework.boot.autoconfigure.web.embedded;
 
 import java.time.Duration;
+import java.util.Map;
 
+import io.netty.bootstrap.ServerBootstrap;
+import io.netty.channel.ChannelOption;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.MockitoAnnotations;
+import reactor.netty.http.server.HttpServer;
+import reactor.netty.tcp.TcpServer;
 
 import org.springframework.boot.autoconfigure.web.ServerProperties;
 import org.springframework.boot.context.properties.source.ConfigurationPropertySources;
 import org.springframework.boot.web.embedded.netty.NettyReactiveWebServerFactory;
 import org.springframework.boot.web.embedded.netty.NettyServerCustomizer;
 import org.springframework.mock.env.MockEnvironment;
+import org.springframework.test.util.ReflectionTestUtils;
 
-import static org.mockito.Mockito.any;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
@@ -46,20 +57,18 @@ public class NettyWebServerFactoryCustomizerTests {
 
 	private NettyWebServerFactoryCustomizer customizer;
 
+	@Captor
+	private ArgumentCaptor<NettyServerCustomizer> customizerCaptor;
+
 	@Before
 	public void setup() {
+		MockitoAnnotations.initMocks(this);
 		this.environment = new MockEnvironment();
 		this.serverProperties = new ServerProperties();
 		ConfigurationPropertySources.attach(this.environment);
 		this.customizer = new NettyWebServerFactoryCustomizer(this.environment, this.serverProperties);
 	}
 
-	private void clear() {
-		this.serverProperties.setUseForwardHeaders(null);
-		this.serverProperties.setMaxHttpHeaderSize(null);
-		this.serverProperties.setConnectionTimeout(null);
-	}
-
 	@Test
 	public void deduceUseForwardHeaders() {
 		this.environment.setProperty("DYNO", "-");
@@ -85,22 +94,47 @@ public class NettyWebServerFactoryCustomizerTests {
 
 	@Test
 	public void setConnectionTimeoutAsZero() {
-		clear();
-		this.serverProperties.setConnectionTimeout(Duration.ZERO);
-
+		setupConnectionTimeout(Duration.ZERO);
 		NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class);
 		this.customizer.customize(factory);
-		verify(factory, times(0)).addServerCustomizers(any(NettyServerCustomizer.class));
+		verifyConnectionTimeout(factory, null);
 	}
 
 	@Test
 	public void setConnectionTimeoutAsMinusOne() {
-		clear();
-		this.serverProperties.setConnectionTimeout(Duration.ofNanos(-1));
+		setupConnectionTimeout(Duration.ofNanos(-1));
+		NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class);
+		this.customizer.customize(factory);
+		verifyConnectionTimeout(factory, 0);
+	}
 
+	@Test
+	public void setConnectionTimeout() {
+		setupConnectionTimeout(Duration.ofSeconds(1));
 		NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class);
 		this.customizer.customize(factory);
-		verify(factory, times(1)).addServerCustomizers(any(NettyServerCustomizer.class));
+		verifyConnectionTimeout(factory, 1000);
+	}
+
+	@SuppressWarnings("unchecked")
+	private void verifyConnectionTimeout(NettyReactiveWebServerFactory factory, Integer expected) {
+		if (expected == null) {
+			verify(factory, never()).addServerCustomizers(any(NettyServerCustomizer.class));
+			return;
+		}
+		verify(factory, times(1)).addServerCustomizers(this.customizerCaptor.capture());
+		NettyServerCustomizer serverCustomizer = this.customizerCaptor.getValue();
+		HttpServer httpServer = serverCustomizer.apply(HttpServer.create());
+		TcpServer tcpConfiguration = ReflectionTestUtils.invokeMethod(httpServer, "tcpConfiguration");
+		ServerBootstrap bootstrap = tcpConfiguration.configure();
+		Map<Object, Object> options = (Map<Object, Object>) ReflectionTestUtils.getField(bootstrap, "options");
+		assertThat(options).containsEntry(ChannelOption.CONNECT_TIMEOUT_MILLIS, expected);
+	}
+
+	private void setupConnectionTimeout(Duration connectionTimeout) {
+		this.serverProperties.setUseForwardHeaders(null);
+		this.serverProperties.setMaxHttpHeaderSize(null);
+		this.serverProperties.setConnectionTimeout(connectionTimeout);
 	}
 
 }