From 5b73fdb5fe46555b1918c5173050bcc47c016c6a Mon Sep 17 00:00:00 2001 From: spencergibb Date: Mon, 14 Oct 2024 13:12:38 -0400 Subject: [PATCH] Refactored HttpUtils to AbstractProxyExchange This allows the streaming media types and the buffer size to be configured with new properties GatewayMvcProperties.streamingBufferSize and GatewayMvcProperties.streamingMediaTypes. See gh-3486 --- .../GatewayServerMvcAutoConfiguration.java | 5 +-- ...pUtils.java => AbstractProxyExchange.java} | 21 ++++++------ .../mvc/config/GatewayMvcProperties.java | 32 +++++++++++++++++++ ...ClientHttpRequestFactoryProxyExchange.java | 15 +++++++-- .../mvc/handler/RestClientProxyExchange.java | 17 +++++++--- ...s.java => AbstractProxyExchangeTests.java} | 29 +++++++++++------ 6 files changed, 90 insertions(+), 29 deletions(-) rename spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/{HttpUtils.java => AbstractProxyExchange.java} (71%) rename spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/common/{HttpUtilsTests.java => AbstractProxyExchangeTests.java} (75%) diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/GatewayServerMvcAutoConfiguration.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/GatewayServerMvcAutoConfiguration.java index 20e5ff68..9baad4ce 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/GatewayServerMvcAutoConfiguration.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/GatewayServerMvcAutoConfiguration.java @@ -88,8 +88,9 @@ public class GatewayServerMvcAutoConfiguration { @Bean @ConditionalOnMissingBean(ProxyExchange.class) - public RestClientProxyExchange restClientProxyExchange(RestClient.Builder restClientBuilder) { - return new RestClientProxyExchange(restClientBuilder.build()); + public RestClientProxyExchange restClientProxyExchange(RestClient.Builder restClientBuilder, + GatewayMvcProperties properties) { + return new RestClientProxyExchange(restClientBuilder.build(), properties); } @Bean diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/HttpUtils.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/AbstractProxyExchange.java similarity index 71% rename from spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/HttpUtils.java rename to spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/AbstractProxyExchange.java index 9cd20c9f..9cc4606b 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/HttpUtils.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/common/AbstractProxyExchange.java @@ -20,21 +20,21 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import org.springframework.cloud.gateway.server.mvc.config.GatewayMvcProperties; +import org.springframework.cloud.gateway.server.mvc.handler.ProxyExchange; import org.springframework.http.client.ClientHttpResponse; import org.springframework.util.Assert; import org.springframework.util.StreamUtils; -import static org.springframework.http.MediaType.TEXT_EVENT_STREAM; +public abstract class AbstractProxyExchange implements ProxyExchange { -public abstract class HttpUtils { + private final GatewayMvcProperties properties; - private static final int BUFFER_SIZE = 16384; - - private HttpUtils() { - throw new AssertionError("Must not instantiate utility class."); + protected AbstractProxyExchange(GatewayMvcProperties properties) { + this.properties = properties; } - public static int copyResponseBody(ClientHttpResponse clientResponse, InputStream inputStream, + protected int copyResponseBody(ClientHttpResponse clientResponse, InputStream inputStream, OutputStream outputStream) throws IOException { Assert.notNull(clientResponse, "No ClientResponse specified"); Assert.notNull(inputStream, "No InputStream specified"); @@ -42,7 +42,7 @@ public abstract class HttpUtils { int transferredBytes; - if (TEXT_EVENT_STREAM.equals(clientResponse.getHeaders().getContentType())) { + if (properties.getStreamingMediaTypes().contains(clientResponse.getHeaders().getContentType())) { transferredBytes = copyResponseBodyWithFlushing(inputStream, outputStream); } else { @@ -52,11 +52,10 @@ public abstract class HttpUtils { return transferredBytes; } - private static int copyResponseBodyWithFlushing(InputStream inputStream, OutputStream outputStream) - throws IOException { + private int copyResponseBodyWithFlushing(InputStream inputStream, OutputStream outputStream) throws IOException { int readBytes; var totalReadBytes = 0; - var buffer = new byte[BUFFER_SIZE]; + var buffer = new byte[properties.getStreamingBufferSize()]; while ((readBytes = inputStream.read(buffer)) != -1) { outputStream.write(buffer, 0, readBytes); diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/GatewayMvcProperties.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/GatewayMvcProperties.java index 9bea9ddd..ac553b7d 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/GatewayMvcProperties.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/config/GatewayMvcProperties.java @@ -18,6 +18,7 @@ package org.springframework.cloud.gateway.server.mvc.config; import java.time.Duration; import java.util.ArrayList; +import java.util.Arrays; import java.util.LinkedHashMap; import java.util.List; @@ -26,6 +27,7 @@ import jakarta.validation.constraints.NotNull; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.core.style.ToStringCreator; +import org.springframework.http.MediaType; @ConfigurationProperties(GatewayMvcProperties.PREFIX) public class GatewayMvcProperties { @@ -51,6 +53,18 @@ public class GatewayMvcProperties { private HttpClient httpClient = new HttpClient(); + /** + * Mime-types that are streaming. + */ + private List streamingMediaTypes = Arrays.asList(MediaType.TEXT_EVENT_STREAM, + new MediaType("application", "stream+json"), new MediaType("application", "grpc"), + new MediaType("application", "grpc+protobuf"), new MediaType("application", "grpc+json")); + + /** + * Buffer size for streaming media mime-types. + */ + private int streamingBufferSize = 16384; + public List getRoutes() { return routes; } @@ -71,11 +85,29 @@ public class GatewayMvcProperties { return httpClient; } + public List getStreamingMediaTypes() { + return streamingMediaTypes; + } + + public void setStreamingMediaTypes(List streamingMediaTypes) { + this.streamingMediaTypes = streamingMediaTypes; + } + + public int getStreamingBufferSize() { + return streamingBufferSize; + } + + public void setStreamingBufferSize(int streamingBufferSize) { + this.streamingBufferSize = streamingBufferSize; + } + @Override public String toString() { return new ToStringCreator(this).append("httpClient", httpClient) .append("routes", routes) .append("routesMap", routesMap) + .append("streamingMediaTypes", streamingMediaTypes) + .append("streamingBufferSize", streamingBufferSize) .toString(); } diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/ClientHttpRequestFactoryProxyExchange.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/ClientHttpRequestFactoryProxyExchange.java index 056653c1..5c17dd2d 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/ClientHttpRequestFactoryProxyExchange.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/ClientHttpRequestFactoryProxyExchange.java @@ -20,19 +20,28 @@ import java.io.IOException; import java.io.InputStream; import java.io.UncheckedIOException; -import org.springframework.cloud.gateway.server.mvc.common.HttpUtils; +import org.springframework.cloud.gateway.server.mvc.common.AbstractProxyExchange; import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; +import org.springframework.cloud.gateway.server.mvc.config.GatewayMvcProperties; import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.ClientHttpResponse; import org.springframework.util.StreamUtils; import org.springframework.web.servlet.function.ServerResponse; -public class ClientHttpRequestFactoryProxyExchange implements ProxyExchange { +public class ClientHttpRequestFactoryProxyExchange extends AbstractProxyExchange { private final ClientHttpRequestFactory requestFactory; + @Deprecated public ClientHttpRequestFactoryProxyExchange(ClientHttpRequestFactory requestFactory) { + super(new GatewayMvcProperties()); + this.requestFactory = requestFactory; + } + + public ClientHttpRequestFactoryProxyExchange(ClientHttpRequestFactory requestFactory, + GatewayMvcProperties properties) { + super(properties); this.requestFactory = requestFactory; } @@ -55,7 +64,7 @@ public class ClientHttpRequestFactoryProxyExchange implements ProxyExchange { InputStream inputStream = MvcUtils.getAttribute(request.getServerRequest(), MvcUtils.CLIENT_RESPONSE_INPUT_STREAM_ATTR); // copy body from request to clientHttpRequest - HttpUtils.copyResponseBody(clientHttpResponse, inputStream, + ClientHttpRequestFactoryProxyExchange.this.copyResponseBody(clientHttpResponse, inputStream, httpServletResponse.getOutputStream()); } return null; diff --git a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/RestClientProxyExchange.java b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/RestClientProxyExchange.java index 91fce3f7..46e9d032 100644 --- a/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/RestClientProxyExchange.java +++ b/spring-cloud-gateway-server-mvc/src/main/java/org/springframework/cloud/gateway/server/mvc/handler/RestClientProxyExchange.java @@ -21,18 +21,26 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.UncheckedIOException; -import org.springframework.cloud.gateway.server.mvc.common.HttpUtils; +import org.springframework.cloud.gateway.server.mvc.common.AbstractProxyExchange; import org.springframework.cloud.gateway.server.mvc.common.MvcUtils; +import org.springframework.cloud.gateway.server.mvc.config.GatewayMvcProperties; import org.springframework.http.client.ClientHttpResponse; import org.springframework.util.StreamUtils; import org.springframework.web.client.RestClient; import org.springframework.web.servlet.function.ServerResponse; -public class RestClientProxyExchange implements ProxyExchange { +public class RestClientProxyExchange extends AbstractProxyExchange { private final RestClient restClient; + @Deprecated public RestClientProxyExchange(RestClient restClient) { + super(new GatewayMvcProperties()); + this.restClient = restClient; + } + + public RestClientProxyExchange(RestClient restClient, GatewayMvcProperties properties) { + super(properties); this.restClient = restClient; } @@ -60,7 +68,7 @@ public class RestClientProxyExchange implements ProxyExchange { return StreamUtils.copy(request.getServerRequest().servletRequest().getInputStream(), outputStream); } - private static ServerResponse doExchange(Request request, ClientHttpResponse clientResponse) throws IOException { + private ServerResponse doExchange(Request request, ClientHttpResponse clientResponse) throws IOException { InputStream body = clientResponse.getBody(); // put the body input stream in a request attribute so filters can read it. MvcUtils.putAttribute(request.getServerRequest(), MvcUtils.CLIENT_RESPONSE_INPUT_STREAM_ATTR, body); @@ -72,7 +80,8 @@ public class RestClientProxyExchange implements ProxyExchange { InputStream inputStream = MvcUtils.getAttribute(request.getServerRequest(), MvcUtils.CLIENT_RESPONSE_INPUT_STREAM_ATTR); // copy body from request to clientHttpRequest - HttpUtils.copyResponseBody(clientResponse, inputStream, httpServletResponse.getOutputStream()); + RestClientProxyExchange.this.copyResponseBody(clientResponse, inputStream, + httpServletResponse.getOutputStream()); } return null; }); diff --git a/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/common/HttpUtilsTests.java b/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/common/AbstractProxyExchangeTests.java similarity index 75% rename from spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/common/HttpUtilsTests.java rename to spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/common/AbstractProxyExchangeTests.java index 839232ca..a153c3c8 100644 --- a/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/common/HttpUtilsTests.java +++ b/spring-cloud-gateway-server-mvc/src/test/java/org/springframework/cloud/gateway/server/mvc/common/AbstractProxyExchangeTests.java @@ -22,8 +22,10 @@ import java.io.OutputStream; import org.junit.jupiter.api.Test; +import org.springframework.cloud.gateway.server.mvc.config.GatewayMvcProperties; import org.springframework.http.MediaType; import org.springframework.mock.http.client.MockClientHttpResponse; +import org.springframework.web.servlet.function.ServerResponse; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -35,7 +37,7 @@ import static org.mockito.Mockito.when; /** * @author Jens Mallien */ -public class HttpUtilsTests { +public class AbstractProxyExchangeTests { @Test public void copyResponseBodyForJson() throws IOException { @@ -46,7 +48,7 @@ public class HttpUtilsTests { when(inputStream.transferTo(any())).thenReturn(3L); OutputStream outputStream = mock(OutputStream.class); - int result = HttpUtils.copyResponseBody(mockResponse, inputStream, outputStream); + int result = new TestProxyExchange().copyResponseBody(mockResponse, inputStream, outputStream); assertThat(result).isEqualTo(3); verify(outputStream, times(1)).flush(); @@ -58,14 +60,10 @@ public class HttpUtilsTests { mockResponse.getHeaders().setContentType(MediaType.TEXT_EVENT_STREAM); InputStream inputStream = mock(InputStream.class); - when(inputStream.read(any())) - .thenReturn(1) - .thenReturn(1) - .thenReturn(1) - .thenReturn(-1); + when(inputStream.read(any())).thenReturn(1).thenReturn(1).thenReturn(1).thenReturn(-1); OutputStream outputStream = mock(OutputStream.class); - int result = HttpUtils.copyResponseBody(mockResponse, inputStream, outputStream); + int result = new TestProxyExchange().copyResponseBody(mockResponse, inputStream, outputStream); assertThat(result).isEqualTo(3); verify(outputStream, times(4)).flush(); @@ -79,10 +77,23 @@ public class HttpUtilsTests { when(inputStream.transferTo(any())).thenReturn(3L); OutputStream outputStream = mock(OutputStream.class); - int result = HttpUtils.copyResponseBody(mockResponse, inputStream, outputStream); + int result = new TestProxyExchange().copyResponseBody(mockResponse, inputStream, outputStream); assertThat(result).isEqualTo(3); verify(outputStream, times(1)).flush(); } + class TestProxyExchange extends AbstractProxyExchange { + + protected TestProxyExchange() { + super(new GatewayMvcProperties()); + } + + @Override + public ServerResponse exchange(Request request) { + return ServerResponse.ok().build(); + } + + } + }