From 52405c5203db7d634bb973d1fd616b7fcfd57d5f Mon Sep 17 00:00:00 2001 From: Toshiaki Maki Date: Mon, 28 Oct 2024 21:44:45 +0900 Subject: [PATCH 1/2] Support `ServerBuilderCustomizer` and `spring.grpc.server.max-inbound-message-size` property for `GrpcServlet` Signed-off-by: Toshiaki Maki --- .../GrpcServerFactoryAutoConfiguration.java | 18 ++++- .../GrpcServletAutoConfigurationTests.java | 77 +++++++++++-------- 2 files changed, 63 insertions(+), 32 deletions(-) diff --git a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/GrpcServerFactoryAutoConfiguration.java b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/GrpcServerFactoryAutoConfiguration.java index 8182cf04..4aae02ae 100644 --- a/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/GrpcServerFactoryAutoConfiguration.java +++ b/spring-grpc-spring-boot-autoconfigure/src/main/java/org/springframework/grpc/autoconfigure/server/GrpcServerFactoryAutoConfiguration.java @@ -25,14 +25,17 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnNotWebApplication; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; +import org.springframework.boot.context.properties.PropertyMapper; import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.core.Ordered; +import org.springframework.util.unit.DataSize; import io.grpc.BindableService; import io.grpc.servlet.jakarta.GrpcServlet; +import io.grpc.servlet.jakarta.ServletServerBuilder; /** * {@link EnableAutoConfiguration Auto-configuration} for gRPC server-side components. @@ -42,6 +45,7 @@ * * @author David Syer * @author Chris Bono + * @author Toshiaki Maki */ @AutoConfiguration @AutoConfigureOrder(Ordered.HIGHEST_PRECEDENCE) @@ -62,11 +66,21 @@ static class GrpcServerFactoryConfiguration { static class GrpcServletConfiguration { @Bean - public ServletRegistrationBean grpcServlet(List services) { + public ServletRegistrationBean grpcServlet(GrpcServerProperties properties, + List services, ServerBuilderCustomizers serverBuilderCustomizers) { List paths = services.stream() .map(service -> "/" + service.bindService().getServiceDescriptor().getName() + "/*") .collect(Collectors.toList()); - ServletRegistrationBean servlet = new ServletRegistrationBean<>(new GrpcServlet(services)); + ServletServerBuilder servletServerBuilder = new ServletServerBuilder(); + services.forEach(servletServerBuilder::addService); + PropertyMapper mapper = PropertyMapper.get().alwaysApplyingWhenNonNull(); + // Only maxInboundMessageSize is customizable + mapper.from(properties.getMaxInboundMessageSize()) + .asInt(DataSize::toBytes) + .to(servletServerBuilder::maxInboundMessageSize); + serverBuilderCustomizers.customize(servletServerBuilder); + ServletRegistrationBean servlet = new ServletRegistrationBean<>( + servletServerBuilder.buildServlet()); servlet.setUrlMappings(paths); return servlet; } diff --git a/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/GrpcServletAutoConfigurationTests.java b/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/GrpcServletAutoConfigurationTests.java index 5e83f9bf..4c22bc1c 100644 --- a/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/GrpcServletAutoConfigurationTests.java +++ b/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/GrpcServletAutoConfigurationTests.java @@ -16,49 +16,33 @@ package org.springframework.grpc.autoconfigure.server; -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import java.util.concurrent.atomic.AtomicBoolean; -import java.time.Duration; -import java.util.List; -import java.util.concurrent.TimeUnit; - -import org.assertj.core.api.InstanceOfAssertFactories; +import io.grpc.BindableService; +import io.grpc.ServerServiceDefinition; +import io.grpc.internal.GrpcUtil; +import io.grpc.servlet.jakarta.GrpcServlet; +import io.grpc.servlet.jakarta.ServletAdapter; +import io.grpc.servlet.jakarta.ServletServerBuilder; import org.junit.jupiter.api.Test; -import org.mockito.InOrder; -import org.mockito.MockedStatic; -import org.mockito.Mockito; -import org.mockito.stubbing.Answer; + import org.springframework.boot.autoconfigure.AutoConfigurations; -import org.springframework.boot.autoconfigure.ssl.SslAutoConfiguration; import org.springframework.boot.test.context.FilteredClassLoader; import org.springframework.boot.test.context.runner.ApplicationContextRunner; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.boot.web.servlet.ServletRegistrationBean; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.core.annotation.Order; -import org.springframework.grpc.server.GrpcServerFactory; -import org.springframework.grpc.server.NettyGrpcServerFactory; import org.springframework.grpc.server.ServerBuilderCustomizer; -import org.springframework.grpc.server.ShadedNettyGrpcServerFactory; -import org.springframework.grpc.server.lifecycle.GrpcServerLifecycle; +import org.springframework.util.unit.DataSize; -import io.grpc.BindableService; -import io.grpc.Grpc; -import io.grpc.ServerBuilder; -import io.grpc.ServerServiceDefinition; -import io.grpc.ServiceDescriptor; -import io.grpc.netty.NettyServerBuilder; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * Tests for {@link GrpcServerAutoConfiguration}. * * @author Chris Bono + * @author Toshiaki Maki */ class GrpcServletAutoConfigurationTests { @@ -90,7 +74,40 @@ void whenNoBindableServicesRegisteredAutoConfigurationIsSkipped() { @Test void whenWebApplicationServletIsAutoConfigured() { - this.contextRunner().run((context) -> assertThat(context).getBean(ServletRegistrationBean.class).isNotNull()); + this.contextRunner().run((context) -> { + assertThat(context).getBean(ServletRegistrationBean.class) + .isNotNull() + .extracting("servlet") + .isInstanceOf(GrpcServlet.class) + .extracting("servletAdapter") + .isInstanceOf(ServletAdapter.class) + .extracting("maxInboundMessageSize") + .isEqualTo(GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE); + }); + } + + @Test + void whenCustomizerIsRegistered() { + AtomicBoolean invoked = new AtomicBoolean(false); + ServerBuilderCustomizer customizer = serverBuilder -> invoked.set(true); + this.contextRunner().withBean(ServerBuilderCustomizer.class, () -> customizer).run(context -> { + assertThat(context).getBean(ServletRegistrationBean.class).isNotNull(); + assertThat(invoked.get()).isTrue(); + }); + } + + @Test + void whenMaxInboundMessageSizeIsConfigured() { + this.contextRunner().withPropertyValues("spring.grpc.server.max-inbound-message-size=10KB").run(context -> { + assertThat(context).getBean(ServletRegistrationBean.class) + .isNotNull() + .extracting("servlet") + .isInstanceOf(GrpcServlet.class) + .extracting("servletAdapter") + .isInstanceOf(ServletAdapter.class) + .extracting("maxInboundMessageSize") + .isEqualTo((int) DataSize.ofKilobytes(10).toBytes()); + }); } } From 2f50e5c27732722dd6027127135d1673f1503a59 Mon Sep 17 00:00:00 2001 From: Toshiaki Maki Date: Tue, 29 Oct 2024 01:31:16 +0900 Subject: [PATCH 2/2] Address review comments Signed-off-by: Toshiaki Maki --- .../GrpcServletAutoConfigurationTests.java | 52 ++++++++----------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/GrpcServletAutoConfigurationTests.java b/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/GrpcServletAutoConfigurationTests.java index 4c22bc1c..14a5dc3a 100644 --- a/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/GrpcServletAutoConfigurationTests.java +++ b/spring-grpc-spring-boot-autoconfigure/src/test/java/org/springframework/grpc/autoconfigure/server/GrpcServletAutoConfigurationTests.java @@ -16,13 +16,9 @@ package org.springframework.grpc.autoconfigure.server; -import java.util.concurrent.atomic.AtomicBoolean; - import io.grpc.BindableService; import io.grpc.ServerServiceDefinition; import io.grpc.internal.GrpcUtil; -import io.grpc.servlet.jakarta.GrpcServlet; -import io.grpc.servlet.jakarta.ServletAdapter; import io.grpc.servlet.jakarta.ServletServerBuilder; import org.junit.jupiter.api.Test; @@ -35,7 +31,9 @@ import org.springframework.util.unit.DataSize; 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.verify; import static org.mockito.Mockito.when; /** @@ -74,40 +72,32 @@ void whenNoBindableServicesRegisteredAutoConfigurationIsSkipped() { @Test void whenWebApplicationServletIsAutoConfigured() { - this.contextRunner().run((context) -> { - assertThat(context).getBean(ServletRegistrationBean.class) - .isNotNull() - .extracting("servlet") - .isInstanceOf(GrpcServlet.class) - .extracting("servletAdapter") - .isInstanceOf(ServletAdapter.class) - .extracting("maxInboundMessageSize") - .isEqualTo(GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE); - }); + this.contextRunner().run((context) -> assertThat(context).getBean(ServletRegistrationBean.class).isNotNull()); } @Test void whenCustomizerIsRegistered() { - AtomicBoolean invoked = new AtomicBoolean(false); - ServerBuilderCustomizer customizer = serverBuilder -> invoked.set(true); - this.contextRunner().withBean(ServerBuilderCustomizer.class, () -> customizer).run(context -> { - assertThat(context).getBean(ServletRegistrationBean.class).isNotNull(); - assertThat(invoked.get()).isTrue(); - }); + ServerBuilderCustomizer customizer = mock(); + this.contextRunner() + .withBean(ServerBuilderCustomizer.class, () -> customizer) + .run(context -> verify(customizer).customize(any(ServletServerBuilder.class))); } @Test - void whenMaxInboundMessageSizeIsConfigured() { - this.contextRunner().withPropertyValues("spring.grpc.server.max-inbound-message-size=10KB").run(context -> { - assertThat(context).getBean(ServletRegistrationBean.class) - .isNotNull() - .extracting("servlet") - .isInstanceOf(GrpcServlet.class) - .extracting("servletAdapter") - .isInstanceOf(ServletAdapter.class) - .extracting("maxInboundMessageSize") - .isEqualTo((int) DataSize.ofKilobytes(10).toBytes()); - }); + void whenMaxInboundMessageSizeIsSetThenItIsUsed() { + this.contextRunner() + .withPropertyValues("spring.grpc.server.max-inbound-message-size=10KB") + .run(context -> assertThat(context).getBean(ServletRegistrationBean.class) + .hasFieldOrPropertyWithValue("servlet.servletAdapter.maxInboundMessageSize", + Math.toIntExact(DataSize.ofKilobytes(10).toBytes()))); + } + + @Test + void whenMaxInboundMessageSizeIsNotSetThenDefaultIsUsed() { + this.contextRunner() + .run((context) -> assertThat(context).getBean(ServletRegistrationBean.class) + .hasFieldOrPropertyWithValue("servlet.servletAdapter.maxInboundMessageSize", + GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE)); } }