From 8b83046e18088a1768955df8617c8501cae1d7a1 Mon Sep 17 00:00:00 2001 From: ayudovin Date: Thu, 11 Apr 2019 16:41:37 +0300 Subject: [PATCH] fixing connection timeout configuration for Netty --- .../NettyWebServerFactoryCustomizer.java | 13 +++++--- .../NettyWebServerFactoryCustomizerTests.java | 32 +++++++++++++++++++ .../context/properties/PropertyMapper.java | 11 +++++-- .../properties/PropertyMapperTests.java | 13 ++++++++ 4 files changed, 63 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java index a93ff7974207..f8f1f92e6fbc 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizer.java @@ -59,13 +59,18 @@ public int getOrder() { @Override public void customize(NettyReactiveWebServerFactory factory) { factory.setUseForwardHeaders(getOrDeduceUseForwardHeaders()); - PropertyMapper propertyMapper = PropertyMapper.get(); - propertyMapper.from(this.serverProperties::getMaxHttpHeaderSize).whenNonNull() + PropertyMapper propertyMapper = PropertyMapper.get().alwaysApplyingWhenNonNull(); + propertyMapper.from(this.serverProperties::getMaxHttpHeaderSize) .asInt(DataSize::toBytes) .to((maxHttpRequestHeaderSize) -> customizeMaxHttpHeaderSize(factory, maxHttpRequestHeaderSize)); - propertyMapper.from(this.serverProperties::getConnectionTimeout).whenNonNull() - .asInt(Duration::toMillis).to((duration) -> factory + + 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))); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java index 4b113b44dc72..5a997e0cccfa 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/embedded/NettyWebServerFactoryCustomizerTests.java @@ -16,21 +16,27 @@ package org.springframework.boot.autoconfigure.web.embedded; +import java.time.Duration; + import org.junit.Before; import org.junit.Test; 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 static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; /** * Tests for {@link NettyWebServerFactoryCustomizer}. * * @author Brian Clozel + * @author Artsiom Yudovin */ public class NettyWebServerFactoryCustomizerTests { @@ -49,6 +55,12 @@ public void setup() { 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", "-"); @@ -72,4 +84,24 @@ public void setUseForwardHeaders() { verify(factory).setUseForwardHeaders(true); } + @Test + public void setConnectionTimeoutAsZero() { + clear(); + this.serverProperties.setConnectionTimeout(Duration.ZERO); + + NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class); + this.customizer.customize(factory); + verify(factory, times(0)).addServerCustomizers(any(NettyServerCustomizer.class)); + } + + @Test + public void setConnectionTimeoutAsMinusOne() { + clear(); + this.serverProperties.setConnectionTimeout(Duration.ofNanos(-1)); + + NettyReactiveWebServerFactory factory = mock(NettyReactiveWebServerFactory.class); + this.customizer.customize(factory); + verify(factory, times(1)).addServerCustomizers(any(NettyServerCustomizer.class)); + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/PropertyMapper.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/PropertyMapper.java index 080ca7a55c39..18ead1742e78 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/PropertyMapper.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/PropertyMapper.java @@ -50,6 +50,7 @@ * {@link Source#toInstance(Function) new instance}. * * @author Phillip Webb + * @author Artsiom Yudovin * @since 2.0.0 */ public final class PropertyMapper { @@ -289,7 +290,7 @@ public Source whenInstanceOf(Class target) { */ public Source whenNot(Predicate predicate) { Assert.notNull(predicate, "Predicate must not be null"); - return new Source<>(this.supplier, predicate.negate()); + return when(predicate.negate()); } /** @@ -300,7 +301,13 @@ public Source whenNot(Predicate predicate) { */ public Source when(Predicate predicate) { Assert.notNull(predicate, "Predicate must not be null"); - return new Source<>(this.supplier, predicate); + + if (Objects.nonNull(this.predicate)) { + return new Source<>(this.supplier, this.predicate.and(predicate)); + } + else { + return new Source<>(this.supplier, predicate); + } } /** diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/PropertyMapperTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/PropertyMapperTests.java index f6e00ec453ec..d1735870b473 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/PropertyMapperTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/properties/PropertyMapperTests.java @@ -28,6 +28,7 @@ * Tests for {@link PropertyMapper}. * * @author Phillip Webb + * @author Artsiom Yudovin */ public class PropertyMapperTests { @@ -201,6 +202,18 @@ public void alwaysApplyingWhenNonNullShouldAlwaysApplyNonNullToSource() { this.map.alwaysApplyingWhenNonNull().from(() -> null).toCall(Assert::fail); } + @Test + public void whenWhenValueNotMatchesShouldCheckTwoWhen() { + this.map.from("123").when("456"::equals).when("123"::equals).toCall(Assert::fail); + } + + @Test + public void whenWhenValueMatchesShouldCheckTwoWhen() { + String result = this.map.from("123").when((s) -> s.contains("2")) + .when("123"::equals).toInstance(String::new); + assertThat(result).isEqualTo("123"); + } + private static class Count implements Supplier { private final Supplier source;