From 7929dfec0f1db34cc91785e9d8f4690f2cd143d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Tue, 2 Aug 2016 13:39:04 +0200 Subject: [PATCH 1/9] Add a zuul property to customize Hystrix ExecutionIsolationStrategy --- .../netflix/zuul/ZuulProxyConfiguration.java | 18 +++++++-- .../netflix/zuul/filters/ZuulProperties.java | 19 ++++++++++ .../route/RestClientRibbonCommand.java | 15 ++++---- .../route/RestClientRibbonCommandFactory.java | 16 +++++--- .../route/apache/HttpClientRibbonCommand.java | 7 +++- .../HttpClientRibbonCommandFactory.java | 5 ++- .../route/okhttp/OkHttpRibbonCommand.java | 7 +++- .../okhttp/OkHttpRibbonCommandFactory.java | 5 ++- .../route/support/AbstractRibbonCommand.java | 38 +++++++++++-------- .../route/RestClientRibbonCommandTests.java | 13 ++++++- ...tpClientRibbonCommandIntegrationTests.java | 3 +- .../OkHttpRibbonCommandIntegrationTests.java | 3 +- ...stClientRibbonCommandIntegrationTests.java | 4 +- 13 files changed, 110 insertions(+), 43 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 4ac23b6d..8b8b8c90 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -86,20 +86,28 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Configuration @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) protected static class HttpClientRibbonConfiguration { + + @Autowired + protected ZuulProperties zuulProperties; + @Bean @ConditionalOnMissingBean public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { - return new HttpClientRibbonCommandFactory(clientFactory); + return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties); } } @Configuration @ConditionalOnProperty("zuul.ribbon.restclient.enabled") protected static class RestClientRibbonConfiguration { + + @Autowired + protected ZuulProperties zuulProperties; + @Bean @ConditionalOnMissingBean public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { - return new RestClientRibbonCommandFactory(clientFactory); + return new RestClientRibbonCommandFactory(clientFactory, zuulProperties); } } @@ -107,10 +115,14 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") @ConditionalOnClass(name = "okhttp3.OkHttpClient") protected static class OkHttpRibbonConfiguration { + + @Autowired + protected ZuulProperties zuulProperties; + @Bean @ConditionalOnMissingBean public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { - return new OkHttpRibbonCommandFactory(clientFactory); + return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java index debf0d10..d6f68fb3 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/ZuulProperties.java @@ -31,10 +31,14 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.util.ClassUtils; import org.springframework.util.StringUtils; +import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; + import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; +import static com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE; + /** * @author Spencer Gibb * @author Dave Syer @@ -131,6 +135,10 @@ public class ZuulProperties { */ private boolean sslHostnameValidationEnabled =true; + private ExecutionIsolationStrategy ribbonIsolationStrategy = SEMAPHORE; + + private HystrixSemaphore semaphore = new HystrixSemaphore(); + public Set getIgnoredHeaders() { Set ignoredHeaders = new LinkedHashSet<>(this.ignoredHeaders); if (ClassUtils.isPresent( @@ -298,6 +306,17 @@ public class ZuulProperties { */ private int maxPerRouteConnections = 20; } + + @Data + @AllArgsConstructor + @NoArgsConstructor + public static class HystrixSemaphore { + /** + * The maximum number of total semaphores for Hystrix. + */ + private int maxSemaphores = 100; + + } public String getServletPattern() { String path = this.servletPath; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java index 2631a3b2..5c1c7ff6 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java @@ -23,6 +23,7 @@ import java.io.InputStream; import java.net.URI; import java.util.List; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; import org.springframework.util.MultiValueMap; @@ -33,9 +34,7 @@ import com.netflix.niws.client.http.RestClient; /** * Hystrix wrapper around Eureka Ribbon command * - * see original - * https://github.com/Netflix/zuul/blob/master/zuul-netflix/src/main/java/com/ - * netflix/zuul/dependency/ribbon/hystrix/RibbonCommand.java + * see original */ @SuppressWarnings("deprecation") public class RestClientRibbonCommand extends AbstractRibbonCommand { @@ -44,12 +43,14 @@ public class RestClientRibbonCommand extends AbstractRibbonCommand headers, - MultiValueMap params, InputStream requestEntity) { - this(commandKey, restClient, new RibbonCommandContext(commandKey, verb.verb(), uri, retryable, headers, params, requestEntity)); + MultiValueMap params, InputStream requestEntity, + ZuulProperties zuulProperties) { + this(commandKey, restClient, + new RibbonCommandContext(commandKey, verb.verb(), uri, retryable, headers, params, requestEntity), zuulProperties); } - public RestClientRibbonCommand(String commandKey, RestClient client, RibbonCommandContext context) { - super(commandKey, client, context); + public RestClientRibbonCommand(String commandKey, RestClient client, RibbonCommandContext context, ZuulProperties zuulProperties) { + super(commandKey, client, context, zuulProperties); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java index f5ab494e..9f8718f2 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java @@ -17,28 +17,32 @@ package org.springframework.cloud.netflix.zuul.filters.route; -import com.netflix.client.http.HttpRequest; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; +import com.netflix.client.http.HttpRequest; import com.netflix.niws.client.http.RestClient; +import lombok.RequiredArgsConstructor; + /** * @author Spencer Gibb */ -public class RestClientRibbonCommandFactory implements RibbonCommandFactory { +@RequiredArgsConstructor +public class RestClientRibbonCommandFactory + implements RibbonCommandFactory { private final SpringClientFactory clientFactory; - public RestClientRibbonCommandFactory(SpringClientFactory clientFactory) { - this.clientFactory = clientFactory; - } + private final ZuulProperties zuulProperties; @Override @SuppressWarnings("deprecation") public RestClientRibbonCommand create(RibbonCommandContext context) { RestClient restClient = this.clientFactory.getClient(context.getServiceId(), RestClient.class); - return new RestClientRibbonCommand(context.getServiceId(), restClient, context); + return new RestClientRibbonCommand(context.getServiceId(), restClient, context, + this.zuulProperties); } public SpringClientFactory getClientFactory() { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java index e9d17d9c..5061fb1f 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommand.java @@ -20,6 +20,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.apache; import org.springframework.cloud.netflix.ribbon.apache.RibbonApacheHttpRequest; import org.springframework.cloud.netflix.ribbon.apache.RibbonApacheHttpResponse; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; @@ -29,8 +30,10 @@ import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibb public class HttpClientRibbonCommand extends AbstractRibbonCommand { public HttpClientRibbonCommand(final String commandKey, - final RibbonLoadBalancingHttpClient client, RibbonCommandContext context) { - super(commandKey, client, context); + final RibbonLoadBalancingHttpClient client, + final RibbonCommandContext context, + final ZuulProperties zuulProperties) { + super(commandKey, client, context, zuulProperties); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java index 1f3063a6..ec9be369 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java @@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.apache; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; @@ -31,6 +32,8 @@ public class HttpClientRibbonCommandFactory implements RibbonCommandFactory { private final SpringClientFactory clientFactory; + + private final ZuulProperties zuulProperties; @Override public HttpClientRibbonCommand create(final RibbonCommandContext context) { @@ -39,7 +42,7 @@ public class HttpClientRibbonCommandFactory implements serviceId, RibbonLoadBalancingHttpClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - return new HttpClientRibbonCommand(serviceId, client, context); + return new HttpClientRibbonCommand(serviceId, client, context, zuulProperties); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java index 9709ae16..f66f36b7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommand.java @@ -20,6 +20,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpRibbonRequest; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpRibbonResponse; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibbonCommand; @@ -29,8 +30,10 @@ import org.springframework.cloud.netflix.zuul.filters.route.support.AbstractRibb public class OkHttpRibbonCommand extends AbstractRibbonCommand { public OkHttpRibbonCommand(final String commandKey, - final OkHttpLoadBalancingClient client, RibbonCommandContext context) { - super(commandKey, client, context); + final OkHttpLoadBalancingClient client, + final RibbonCommandContext context, + final ZuulProperties zuulProperties) { + super(commandKey, client, context, zuulProperties); } @Override diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java index fc65e683..61001443 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandFactory.java @@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.okhttp; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; @@ -31,6 +32,8 @@ public class OkHttpRibbonCommandFactory implements RibbonCommandFactory { private final SpringClientFactory clientFactory; + + private final ZuulProperties zuulProperties; @Override public OkHttpRibbonCommand create(final RibbonCommandContext context) { @@ -39,7 +42,7 @@ public class OkHttpRibbonCommandFactory implements serviceId, OkHttpLoadBalancingClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - return new OkHttpRibbonCommand(serviceId, client, context); + return new OkHttpRibbonCommand(serviceId, client, context, zuulProperties); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java index 04261c79..25fc0dfa 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java @@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.zuul.filters.route.support; import org.springframework.cloud.netflix.ribbon.RibbonHttpResponse; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.http.client.ClientHttpResponse; @@ -32,39 +33,46 @@ import com.netflix.hystrix.HystrixCommand; import com.netflix.hystrix.HystrixCommandGroupKey; import com.netflix.hystrix.HystrixCommandKey; import com.netflix.hystrix.HystrixCommandProperties; +import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy; import com.netflix.zuul.constants.ZuulConstants; import com.netflix.zuul.context.RequestContext; /** * @author Spencer Gibb */ -public abstract class AbstractRibbonCommand, RQ extends ClientRequest, RS extends HttpResponse> extends HystrixCommand implements - RibbonCommand { +public abstract class AbstractRibbonCommand, RQ extends ClientRequest, RS extends HttpResponse> + extends HystrixCommand implements RibbonCommand { protected final LBC client; protected RibbonCommandContext context; - public AbstractRibbonCommand(LBC client, RibbonCommandContext context) { - this("default", client, context); + public AbstractRibbonCommand(LBC client, RibbonCommandContext context, ZuulProperties zuulProperties) { + this("default", client, context, zuulProperties); } + - public AbstractRibbonCommand(String commandKey, LBC client, RibbonCommandContext context) { - super(getSetter(commandKey)); + public AbstractRibbonCommand(String commandKey, LBC client, RibbonCommandContext context, ZuulProperties zuulProperties) { + super(getSetter(commandKey, zuulProperties)); this.client = client; this.context = context; } - protected static Setter getSetter(final String commandKey) { + protected static Setter getSetter(final String commandKey, ZuulProperties zuulProperties) { - // we want to default to semaphore-isolation since this wraps - // 2 others commands that are already thread isolated // @formatter:off - final String name = ZuulConstants.ZUUL_EUREKA + commandKey + ".semaphore.maxSemaphores"; - final DynamicIntProperty value = DynamicPropertyFactory.getInstance() - .getIntProperty(name, 100); - final HystrixCommandProperties.Setter setter = HystrixCommandProperties .Setter() - .withExecutionIsolationStrategy(HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE) - .withExecutionIsolationSemaphoreMaxConcurrentRequests(value.get()); + final HystrixCommandProperties.Setter setter = HystrixCommandProperties.Setter() + .withExecutionIsolationStrategy(zuulProperties.getRibbonIsolationStrategy()); + if (zuulProperties.getRibbonIsolationStrategy() == ExecutionIsolationStrategy.SEMAPHORE){ + final String name = ZuulConstants.ZUUL_EUREKA + commandKey + ".semaphore.maxSemaphores"; + // we want to default to semaphore-isolation since this wraps + // 2 others commands that are already thread isolated + final DynamicIntProperty value = DynamicPropertyFactory.getInstance() + .getIntProperty(name, 100); + setter.withExecutionIsolationSemaphoreMaxConcurrentRequests(value.get()); + } else { + // FIXME Find out which parameters can be set here + } + return Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("RibbonCommand")) .andCommandKey(HystrixCommandKey.Factory.asKey(commandKey + "RibbonCommand")) .andCommandPropertiesDefaults(setter); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java index f8d43345..244c5d5b 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java @@ -29,8 +29,10 @@ import java.net.URI; import java.nio.charset.Charset; import java.util.Collections; +import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.StreamUtils; @@ -41,6 +43,13 @@ import com.netflix.client.http.HttpRequest; */ public class RestClientRibbonCommandTests { + private ZuulProperties zuulProperties; + + @Before + public void setUp() { + zuulProperties = new ZuulProperties(); + } + @Test public void testNullEntity() throws Exception { String uri = "http://example.com"; @@ -49,7 +58,7 @@ public class RestClientRibbonCommandTests { LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, new RibbonCommandContext("example", "GET", uri, false, - headers, params, null)); + headers, params, null), zuulProperties); HttpRequest request = command.createRequest(); @@ -96,7 +105,7 @@ public class RestClientRibbonCommandTests { uri.toString(), false, headers, new LinkedMultiValueMap(), requestEntity, Collections.singletonList(requestCustomizer)); context.setContentLength(length); - RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, context); + RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, context, zuulProperties); HttpRequest request = command.createRequest(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java index 483294c1..3f8adc31 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandIntegrationTests.java @@ -37,6 +37,7 @@ import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.StaticServerList; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; @@ -178,7 +179,7 @@ public class HttpClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { @Bean public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { - return new HttpClientRibbonCommandFactory(clientFactory); + return new HttpClientRibbonCommandFactory(clientFactory, new ZuulProperties()); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java index 725f7327..8ca9aff0 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java @@ -32,6 +32,7 @@ import org.springframework.cloud.netflix.ribbon.RibbonClient; import org.springframework.cloud.netflix.ribbon.RibbonClients; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; import org.springframework.cloud.netflix.zuul.filters.route.support.ZuulProxyTestBase; import org.springframework.context.annotation.Bean; @@ -109,7 +110,7 @@ public class OkHttpRibbonCommandIntegrationTests extends ZuulProxyTestBase { @Bean public RibbonCommandFactory ribbonCommandFactory( final SpringClientFactory clientFactory) { - return new OkHttpRibbonCommandFactory(clientFactory); + return new OkHttpRibbonCommandFactory(clientFactory, new ZuulProperties()); } @Bean diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java index db862e19..fcef65b9 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/restclient/RestClientRibbonCommandIntegrationTests.java @@ -307,7 +307,7 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { private SpringClientFactory clientFactory; public MyRibbonCommandFactory(SpringClientFactory clientFactory) { - super(clientFactory); + super(clientFactory, new ZuulProperties()); this.clientFactory = clientFactory; } @@ -333,7 +333,7 @@ public class RestClientRibbonCommandIntegrationTests extends ZuulProxyTestBase { public MyCommand(int errorCode, String commandKey, RestClient restClient, RibbonCommandContext context) { - super(commandKey, restClient, context); + super(commandKey, restClient, context, new ZuulProperties()); this.errorCode = errorCode; } From b1ec12af43204622cec6385a764b6e5c6ab6648a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Wed, 3 Aug 2016 15:32:08 +0200 Subject: [PATCH 2/9] Add a check to current context as contentLength is already in the context --- .../route/support/AbstractRibbonCommand.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java index 25fc0dfa..0b12a2e5 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java @@ -46,18 +46,20 @@ public abstract class AbstractRibbonCommand Date: Fri, 5 Aug 2016 18:57:20 +0200 Subject: [PATCH 3/9] Get content length from Headers when RibbonCommandContext is built + add tests --- .../cloud/netflix/ribbon/RibbonHttpRequest.java | 2 +- .../zuul/filters/route/RibbonCommandContext.java | 16 +++++++++++----- .../route/support/AbstractRibbonCommand.java | 9 +-------- .../apache/RibbonApacheHttpRequestTests.java | 4 ++++ .../ribbon/okhttp/OkHttpRibbonRequestTests.java | 3 +++ 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java index 2d68b12d..ec610c2c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonHttpRequest.java @@ -108,6 +108,6 @@ public class RibbonHttpRequest extends AbstractClientHttpRequest { } private boolean isDynamic(String name) { - return name.equalsIgnoreCase("Content-Length") || name.equalsIgnoreCase("Transfer-Encoding"); + return "Content-Length".equalsIgnoreCase(name) || "Transfer-Encoding".equalsIgnoreCase(name); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java index 173cd09d..eaed3cbb 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java @@ -25,6 +25,7 @@ import java.util.List; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.util.MultiValueMap; import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; import lombok.AllArgsConstructor; import lombok.Data; @@ -54,17 +55,22 @@ public class RibbonCommandContext { private final List requestCustomizers; private Long contentLength; - public RibbonCommandContext(String serviceId, String method, String uri, Boolean retryable, - MultiValueMap headers, MultiValueMap params, - InputStream requestEntity) { + public RibbonCommandContext(String serviceId, String method, String uri, + Boolean retryable, MultiValueMap headers, + MultiValueMap params, InputStream requestEntity) { this(serviceId, method, uri, retryable, headers, params, requestEntity, - new ArrayList(), null); + new ArrayList(), + headers.containsKey("Content-Length") && StringUtils + .hasText(headers.getFirst("Content-Length").toString()) + ? new Long(headers.getFirst("Content-Length").toString()) + : null); } public URI uri() { try { return new URI(this.uri); - } catch (URISyntaxException e) { + } + catch (URISyntaxException e) { ReflectionUtils.rethrowRuntimeException(e); } return null; diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java index 0b12a2e5..36beb774 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/support/AbstractRibbonCommand.java @@ -22,7 +22,6 @@ import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommand; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.http.client.ClientHttpResponse; -import org.springframework.util.StringUtils; import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.ClientRequest; @@ -72,7 +71,7 @@ public abstract class AbstractRibbonCommand headers = new LinkedMultiValueMap<>(); headers.add("my-header", "my-value"); + headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); RibbonApacheHttpRequest httpRequest = new RibbonApacheHttpRequest(new RibbonCommandContext("example", "GET", uri, false, @@ -63,7 +65,9 @@ public class RibbonApacheHttpRequestTests { assertThat("uri is wrong", request.getURI().toString(), startsWith(uri)); assertThat("my-header is missing", request.getFirstHeader("my-header"), is(notNullValue())); assertThat("my-header is wrong", request.getFirstHeader("my-header").getValue(), is(equalTo("my-value"))); + assertThat("Content-Length is wrong", request.getFirstHeader(HttpEncoding.CONTENT_LENGTH).getValue(), is(equalTo("5192"))); assertThat("myparam is missing", request.getURI().getQuery(), is(equalTo("myparam=myparamval"))); + } @Test diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java index 4c6c84fd..765ad2e3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java @@ -29,6 +29,7 @@ import java.io.IOException; import java.util.Collections; import org.junit.Test; +import org.springframework.cloud.netflix.feign.encoding.HttpEncoding; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.util.LinkedMultiValueMap; @@ -47,6 +48,7 @@ public class OkHttpRibbonRequestTests { String uri = "http://example.com"; LinkedMultiValueMap headers = new LinkedMultiValueMap<>(); headers.add("my-header", "my-value"); + headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); RibbonCommandContext context = new RibbonCommandContext("example", "GET", uri, false, headers, params, null); @@ -57,6 +59,7 @@ public class OkHttpRibbonRequestTests { assertThat("body is not null", request.body(), is(nullValue())); assertThat("uri is wrong", request.url().toString(), startsWith(uri)); assertThat("my-header is wrong", request.header("my-header"), is(equalTo("my-value"))); + assertThat("Content-Length is wrong", request.header(HttpEncoding.CONTENT_LENGTH), is(equalTo("5192"))); assertThat("myparam is missing", request.url().queryParameter("myparam"), is(equalTo("myparamval"))); } From 072f7760a4d817371e38ebd4efbf09e375ce0152 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Mon, 8 Aug 2016 17:07:35 +0200 Subject: [PATCH 4/9] Use request content length RibbonRoutingFilter --- .../zuul/filters/route/RibbonRoutingFilter.java | 11 ++++++----- .../okhttp/OkHttpRibbonCommandIntegrationTests.java | 2 +- .../zuul/filters/route/support/ZuulProxyTestBase.java | 3 +-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java index 8e355424..2df7c7bd 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java @@ -47,8 +47,8 @@ public class RibbonRoutingFilter extends ZuulFilter { protected List requestCustomizers; public RibbonRoutingFilter(ProxyRequestHelper helper, - RibbonCommandFactory ribbonCommandFactory, - List requestCustomizers) { + RibbonCommandFactory ribbonCommandFactory, + List requestCustomizers) { this.helper = helper; this.ribbonCommandFactory = ribbonCommandFactory; this.requestCustomizers = requestCustomizers; @@ -120,12 +120,13 @@ public class RibbonRoutingFilter extends ZuulFilter { uri = uri.replace("//", "/"); return new RibbonCommandContext(serviceId, verb, uri, retryable, headers, params, - requestEntity, this.requestCustomizers); + requestEntity, this.requestCustomizers, request.getContentLengthLong()); } protected ClientHttpResponse forward(RibbonCommandContext context) throws Exception { - Map info = this.helper.debug(context.getMethod(), context.getUri(), - context.getHeaders(), context.getParams(), context.getRequestEntity()); + Map info = this.helper.debug(context.getMethod(), + context.getUri(), context.getHeaders(), context.getParams(), + context.getRequestEntity()); RibbonCommand command = this.ribbonCommandFactory.create(context); try { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java index 8ca9aff0..bc72299e 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonCommandIntegrationTests.java @@ -26,7 +26,7 @@ import org.junit.runner.RunWith; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.web.ErrorAttributes; import org.springframework.boot.test.SpringApplicationConfiguration; -import org.springframework.boot.test.TestRestTemplate; +import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.netflix.ribbon.RibbonClient; import org.springframework.cloud.netflix.ribbon.RibbonClients; diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java index ffdb3771..b97b9280 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/support/ZuulProxyTestBase.java @@ -25,7 +25,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.http.HttpServletRequest; -import org.junit.Assume; import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -33,7 +32,7 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.web.BasicErrorController; import org.springframework.boot.autoconfigure.web.ErrorAttributes; import org.springframework.boot.autoconfigure.web.ErrorProperties; -import org.springframework.boot.test.TestRestTemplate; +import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.cloud.netflix.ribbon.StaticServerList; import org.springframework.cloud.netflix.zuul.RoutesEndpoint; import org.springframework.cloud.netflix.zuul.filters.Route; From 67838807e8248ccae003e7aad2bb168e71460218 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Tue, 9 Aug 2016 09:36:30 +0200 Subject: [PATCH 5/9] Add zuulProperties as a method parameter --- .../netflix/zuul/ZuulProxyConfiguration.java | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java index 8b8b8c90..b21d4d40 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfiguration.java @@ -86,13 +86,11 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Configuration @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) protected static class HttpClientRibbonConfiguration { - - @Autowired - protected ZuulProperties zuulProperties; - + @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { return new HttpClientRibbonCommandFactory(clientFactory, zuulProperties); } } @@ -100,13 +98,11 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Configuration @ConditionalOnProperty("zuul.ribbon.restclient.enabled") protected static class RestClientRibbonConfiguration { - - @Autowired - protected ZuulProperties zuulProperties; - + @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { return new RestClientRibbonCommandFactory(clientFactory, zuulProperties); } } @@ -115,13 +111,11 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") @ConditionalOnClass(name = "okhttp3.OkHttpClient") protected static class OkHttpRibbonConfiguration { - - @Autowired - protected ZuulProperties zuulProperties; - + @Bean @ConditionalOnMissingBean - public RibbonCommandFactory ribbonCommandFactory(SpringClientFactory clientFactory) { + public RibbonCommandFactory ribbonCommandFactory( + SpringClientFactory clientFactory, ZuulProperties zuulProperties) { return new OkHttpRibbonCommandFactory(clientFactory, zuulProperties); } } @@ -130,18 +124,16 @@ public class ZuulProxyConfiguration extends ZuulConfiguration { @Bean public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, ProxyRequestHelper proxyRequestHelper) { - return new PreDecorationFilter(routeLocator, - this.server.getServletPrefix(), - this.zuulProperties, - proxyRequestHelper); + return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), + this.zuulProperties, proxyRequestHelper); } // route filters @Bean public RibbonRoutingFilter ribbonRoutingFilter(ProxyRequestHelper helper, RibbonCommandFactory ribbonCommandFactory) { - RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, - ribbonCommandFactory, this.requestCustomizers); + RibbonRoutingFilter filter = new RibbonRoutingFilter(helper, ribbonCommandFactory, + this.requestCustomizers); return filter; } From 58559aff16707d920c4306f1872cd71e335d49c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Tue, 9 Aug 2016 18:32:36 +0200 Subject: [PATCH 6/9] Add documentation for new zuul.ribbonIsolationStrategy property --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 31b6d208..77d25bd5 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1050,6 +1050,8 @@ Zuul's rule engine allows rules and filters to be written in essentially any JVM NOTE: The configuration property `zuul.max.host.connections` has been replaced by two new properties, `zuul.host.maxTotalConnections` and `zuul.host.maxPerRouteConnections` which default to 200 and 20 respectively. +NOTE: Default Hystrix isolation pattern (ExecutionIsolationStrategy) for all routes is SEMAPHORE. `zuul.ribbonIsolationStrategy` can be changed to THREAD if this isolation pattern is preferred. + [[netflix-zuul-reverse-proxy]] === Embedded Zuul Reverse Proxy From 03caa1b8cee8eeee22a2e516d07c962ade75419f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Thu, 11 Aug 2016 09:41:15 +0200 Subject: [PATCH 7/9] Remove deprecated and unused constructors for RibbonCommandContext and RestClientRibbonCommand --- .../route/RestClientRibbonCommand.java | 10 ------ .../filters/route/RibbonCommandContext.java | 13 ------- .../apache/RibbonApacheHttpRequestTests.java | 6 ++-- .../okhttp/OkHttpRibbonRequestTests.java | 34 ++++++++++++------- .../route/RestClientRibbonCommandTests.java | 7 ++-- 5 files changed, 30 insertions(+), 40 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java index 5c1c7ff6..39c850a4 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java @@ -39,16 +39,6 @@ import com.netflix.niws.client.http.RestClient; @SuppressWarnings("deprecation") public class RestClientRibbonCommand extends AbstractRibbonCommand { - @SuppressWarnings("unused") - @Deprecated - public RestClientRibbonCommand(String commandKey, RestClient restClient, HttpRequest.Verb verb, String uri, - Boolean retryable, MultiValueMap headers, - MultiValueMap params, InputStream requestEntity, - ZuulProperties zuulProperties) { - this(commandKey, restClient, - new RibbonCommandContext(commandKey, verb.verb(), uri, retryable, headers, params, requestEntity), zuulProperties); - } - public RestClientRibbonCommand(String commandKey, RestClient client, RibbonCommandContext context, ZuulProperties zuulProperties) { super(commandKey, client, context, zuulProperties); } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java index eaed3cbb..a558c5fd 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java @@ -19,13 +19,11 @@ package org.springframework.cloud.netflix.zuul.filters.route; import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; -import java.util.ArrayList; import java.util.List; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.util.MultiValueMap; import org.springframework.util.ReflectionUtils; -import org.springframework.util.StringUtils; import lombok.AllArgsConstructor; import lombok.Data; @@ -55,17 +53,6 @@ public class RibbonCommandContext { private final List requestCustomizers; private Long contentLength; - public RibbonCommandContext(String serviceId, String method, String uri, - Boolean retryable, MultiValueMap headers, - MultiValueMap params, InputStream requestEntity) { - this(serviceId, method, uri, retryable, headers, params, requestEntity, - new ArrayList(), - headers.containsKey("Content-Length") && StringUtils - .hasText(headers.getFirst("Content-Length").toString()) - ? new Long(headers.getFirst("Content-Length").toString()) - : null); - } - public URI uri() { try { return new URI(this.uri); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonApacheHttpRequestTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonApacheHttpRequestTests.java index c6972539..6a5f23fd 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonApacheHttpRequestTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonApacheHttpRequestTests.java @@ -29,6 +29,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.net.URI; import java.nio.charset.Charset; +import java.util.ArrayList; import java.util.Collections; import org.apache.http.HttpEntity; @@ -56,8 +57,9 @@ public class RibbonApacheHttpRequestTests { headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RibbonApacheHttpRequest httpRequest = new RibbonApacheHttpRequest(new RibbonCommandContext("example", "GET", uri, false, - headers, params, null)); + RibbonApacheHttpRequest httpRequest = + new RibbonApacheHttpRequest( + new RibbonCommandContext("example", "GET", uri, false, headers, params, null, new ArrayList())); HttpUriRequest request = httpRequest.toRequest(RequestConfig.custom().build()); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java index 765ad2e3..d35ea62d 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpRibbonRequestTests.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertThat; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import org.junit.Test; @@ -48,35 +49,41 @@ public class OkHttpRibbonRequestTests { String uri = "http://example.com"; LinkedMultiValueMap headers = new LinkedMultiValueMap<>(); headers.add("my-header", "my-value"); - headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); + // headers.add(HttpEncoding.CONTENT_LENGTH, "5192"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RibbonCommandContext context = new RibbonCommandContext("example", "GET", uri, false, headers, params, null); + RibbonCommandContext context = new RibbonCommandContext("example", "GET", uri, + false, headers, params, null, new ArrayList()); OkHttpRibbonRequest httpRequest = new OkHttpRibbonRequest(context); Request request = httpRequest.toRequest(); assertThat("body is not null", request.body(), is(nullValue())); assertThat("uri is wrong", request.url().toString(), startsWith(uri)); - assertThat("my-header is wrong", request.header("my-header"), is(equalTo("my-value"))); - assertThat("Content-Length is wrong", request.header(HttpEncoding.CONTENT_LENGTH), is(equalTo("5192"))); - assertThat("myparam is missing", request.url().queryParameter("myparam"), is(equalTo("myparamval"))); + assertThat("my-header is wrong", request.header("my-header"), + is(equalTo("my-value"))); + assertThat("myparam is missing", request.url().queryParameter("myparam"), + is(equalTo("myparamval"))); } @Test - // this situation happens, see https://github.com/spring-cloud/spring-cloud-netflix/issues/1042#issuecomment-227723877 + // this situation happens, see + // https://github.com/spring-cloud/spring-cloud-netflix/issues/1042#issuecomment-227723877 public void testEmptyEntityGet() throws Exception { String entityValue = ""; - testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), false, "GET"); + testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), false, + "GET"); } @Test public void testNonEmptyEntityPost() throws Exception { String entityValue = "abcd"; - testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), true, "POST"); + testEntity(entityValue, new ByteArrayInputStream(entityValue.getBytes()), true, + "POST"); } - void testEntity(String entityValue, ByteArrayInputStream requestEntity, boolean addContentLengthHeader, String method) throws IOException { + void testEntity(String entityValue, ByteArrayInputStream requestEntity, + boolean addContentLengthHeader, String method) throws IOException { String lengthString = String.valueOf(entityValue.length()); Long length = null; String uri = "http://example.com"; @@ -97,8 +104,9 @@ public class OkHttpRibbonRequestTests { builder.addHeader("from-customizer", "foo"); } }; - RibbonCommandContext context = new RibbonCommandContext("example", method, uri, false, - headers, new LinkedMultiValueMap(), requestEntity, Collections.singletonList(requestCustomizer)); + RibbonCommandContext context = new RibbonCommandContext("example", method, uri, + false, headers, new LinkedMultiValueMap(), requestEntity, + Collections.singletonList(requestCustomizer)); context.setContentLength(length); OkHttpRibbonRequest httpRequest = new OkHttpRibbonRequest(context); @@ -115,7 +123,8 @@ public class OkHttpRibbonRequestTests { if (!method.equalsIgnoreCase("get")) { assertThat("body is null", request.body(), is(notNullValue())); RequestBody body = request.body(); - assertThat("contentLength is wrong", body.contentLength(), is(equalTo((long) entityValue.length()))); + assertThat("contentLength is wrong", body.contentLength(), + is(equalTo((long) entityValue.length()))); Buffer content = new Buffer(); body.writeTo(content); String string = content.readByteString().utf8(); @@ -123,4 +132,3 @@ public class OkHttpRibbonRequestTests { } } } - diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java index 244c5d5b..5b4671d4 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java @@ -27,6 +27,7 @@ import java.io.ByteArrayInputStream; import java.io.InputStream; import java.net.URI; import java.nio.charset.Charset; +import java.util.ArrayList; import java.util.Collections; import org.junit.Before; @@ -57,8 +58,10 @@ public class RestClientRibbonCommandTests { headers.add("my-header", "my-value"); LinkedMultiValueMap params = new LinkedMultiValueMap<>(); params.add("myparam", "myparamval"); - RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, new RibbonCommandContext("example", "GET", uri, false, - headers, params, null), zuulProperties); + RestClientRibbonCommand command = + new RestClientRibbonCommand("cmd", null, + new RibbonCommandContext("example", "GET", uri, false, headers, params, null,new ArrayList()), + zuulProperties); HttpRequest request = command.createRequest(); From f5bb5ff9c148bdd71bf268cf0c80de0804e387bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Thu, 18 Aug 2016 11:24:15 +0200 Subject: [PATCH 8/9] Keep old constructors to maintain backwards compatibility with spring cloud sleuth --- .../route/RestClientRibbonCommand.java | 17 ++++++++++---- .../route/RestClientRibbonCommandFactory.java | 23 ++++++++++++++----- .../filters/route/RibbonCommandContext.java | 8 +++++++ 3 files changed, 38 insertions(+), 10 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java index 39c850a4..b5423ce7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommand.java @@ -39,18 +39,27 @@ import com.netflix.niws.client.http.RestClient; @SuppressWarnings("deprecation") public class RestClientRibbonCommand extends AbstractRibbonCommand { - public RestClientRibbonCommand(String commandKey, RestClient client, RibbonCommandContext context, ZuulProperties zuulProperties) { + public RestClientRibbonCommand(String commandKey, RestClient client, + RibbonCommandContext context, ZuulProperties zuulProperties) { super(commandKey, client, context, zuulProperties); } + @Deprecated + public RestClientRibbonCommand(String commandKey, RestClient restClient, + HttpRequest.Verb verb, String uri, Boolean retryable, + MultiValueMap headers, MultiValueMap params, + InputStream requestEntity) { + this(commandKey, restClient, new RibbonCommandContext(commandKey, verb.verb(), + uri, retryable, headers, params, requestEntity), new ZuulProperties()); + } + @Override protected HttpRequest createRequest() throws Exception { HttpRequest.Builder builder = HttpRequest.newBuilder() - .verb(getVerb(this.context.getMethod())) - .uri(this.context.uri()) + .verb(getVerb(this.context.getMethod())).uri(this.context.uri()) .entity(this.context.getRequestEntity()); - if(this.context.getRetryable() != null) { + if (this.context.getRetryable() != null) { builder.setRetriable(this.context.getRetryable()); } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java index 9f8718f2..b184e370 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandFactory.java @@ -23,18 +23,24 @@ import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import com.netflix.client.http.HttpRequest; import com.netflix.niws.client.http.RestClient; -import lombok.RequiredArgsConstructor; - /** * @author Spencer Gibb */ -@RequiredArgsConstructor -public class RestClientRibbonCommandFactory - implements RibbonCommandFactory { +public class RestClientRibbonCommandFactory implements RibbonCommandFactory { private final SpringClientFactory clientFactory; - private final ZuulProperties zuulProperties; + private ZuulProperties zuulProperties; + + public RestClientRibbonCommandFactory(SpringClientFactory clientFactory) { + this(clientFactory, new ZuulProperties()); + } + + public RestClientRibbonCommandFactory(SpringClientFactory clientFactory, + ZuulProperties zuulProperties) { + this.clientFactory = clientFactory; + this.zuulProperties = zuulProperties; + } @Override @SuppressWarnings("deprecation") @@ -49,7 +55,12 @@ public class RestClientRibbonCommandFactory return clientFactory; } + public void setZuulProperties(ZuulProperties zuulProperties) { + this.zuulProperties = zuulProperties; + } + protected static HttpRequest.Verb getVerb(String method) { return RestClientRibbonCommand.getVerb(method); } + } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java index a558c5fd..288f4e52 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.zuul.filters.route; import java.io.InputStream; import java.net.URI; import java.net.URISyntaxException; +import java.util.ArrayList; import java.util.List; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; @@ -53,6 +54,13 @@ public class RibbonCommandContext { private final List requestCustomizers; private Long contentLength; + public RibbonCommandContext(String serviceId, String method, String uri, + Boolean retryable, MultiValueMap headers, + MultiValueMap params, InputStream requestEntity) { + this(serviceId, method, uri, retryable, headers, params, requestEntity, + new ArrayList(), null); + } + public URI uri() { try { return new URI(this.uri); From b98c9ebc1d197a38f9b5d2abbf0c5616e60e3a8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lagraulet?= Date: Thu, 18 Aug 2016 14:27:11 +0200 Subject: [PATCH 9/9] Add tests to verify old constructors --- .../filters/route/RibbonCommandContext.java | 4 +++ .../route/RestClientRibbonCommandTests.java | 35 ++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java index 288f4e52..79c2e72c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonCommandContext.java @@ -54,6 +54,10 @@ public class RibbonCommandContext { private final List requestCustomizers; private Long contentLength; + /** + * Kept for backwards compatibility with Spring Cloud Sleuth 1.x versions + */ + @Deprecated public RibbonCommandContext(String serviceId, String method, String uri, Boolean retryable, MultiValueMap headers, MultiValueMap params, InputStream requestEntity) { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java index 5b4671d4..7b0b4614 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RestClientRibbonCommandTests.java @@ -38,6 +38,7 @@ import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.StreamUtils; import com.netflix.client.http.HttpRequest; +import com.netflix.client.http.HttpRequest.Verb; /** * @author Spencer Gibb @@ -51,6 +52,38 @@ public class RestClientRibbonCommandTests { zuulProperties = new ZuulProperties(); } + /** + * Tests old constructors kept for backwards compatibility with Spring Cloud Sleuth 1.x versions + */ + @Test + @Deprecated + public void testNullEntityWithOldConstruct() throws Exception { + String uri = "http://example.com"; + LinkedMultiValueMap headers = new LinkedMultiValueMap<>(); + headers.add("my-header", "my-value"); + LinkedMultiValueMap params = new LinkedMultiValueMap<>(); + params.add("myparam", "myparamval"); + RestClientRibbonCommand command = + new RestClientRibbonCommand("cmd", null,Verb.GET ,uri, false, headers, params, null); + + HttpRequest request = command.createRequest(); + + assertThat("uri is wrong", request.getUri().toString(), startsWith(uri)); + assertThat("my-header is wrong", request.getHttpHeaders().getFirstValue("my-header"), is(equalTo("my-value"))); + assertThat("myparam is missing", request.getQueryParams().get("myparam").iterator().next(), is(equalTo("myparamval"))); + + command = + new RestClientRibbonCommand("cmd", null, + new RibbonCommandContext("example", "GET", uri, false, headers, params, null), + zuulProperties); + + request = command.createRequest(); + + assertThat("uri is wrong", request.getUri().toString(), startsWith(uri)); + assertThat("my-header is wrong", request.getHttpHeaders().getFirstValue("my-header"), is(equalTo("my-value"))); + assertThat("myparam is missing", request.getQueryParams().get("myparam").iterator().next(), is(equalTo("myparamval"))); + } + @Test public void testNullEntity() throws Exception { String uri = "http://example.com"; @@ -60,7 +93,7 @@ public class RestClientRibbonCommandTests { params.add("myparam", "myparamval"); RestClientRibbonCommand command = new RestClientRibbonCommand("cmd", null, - new RibbonCommandContext("example", "GET", uri, false, headers, params, null,new ArrayList()), + new RibbonCommandContext("example", "GET", uri, false, headers, params, null, new ArrayList()), zuulProperties); HttpRequest request = command.createRequest();