From 2aa93abc0f301a2e99e193c2f23eef2fdd306ad7 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 28 Jun 2017 11:43:52 -0400 Subject: [PATCH 1/7] Centralizing HTTP client creation and configuration --- .../netflix/feign/FeignAutoConfiguration.java | 61 +++++- .../FeignRibbonClientAutoConfiguration.java | 63 +++++- .../support/FeignHttpClientProperties.java | 110 ++++++++++ .../ribbon/RibbonClientConfiguration.java | 88 +++++++- ...etryableRibbonLoadBalancingHttpClient.java | 10 +- .../apache/RibbonLoadBalancingHttpClient.java | 19 +- .../zuul/ZuulProxyAutoConfiguration.java | 12 +- .../route/SimpleHostRoutingFilter.java | 109 ++-------- .../netflix/HttpClientConfigurationTests.java | 203 ++++++++++++++++++ .../netflix/feign/FeignCompressionTests.java | 3 +- .../FeignHttpClientPropertiesTests.java | 98 +++++++++ .../valid/FeignClientValidationTests.java | 7 +- ...onClientsPreprocessorIntegrationTests.java | 3 +- .../ribbon/SpringClientFactoryTests.java | 3 +- .../ribbon/SpringRetryEnabledTests.java | 3 +- .../RibbonLoadBalancingHttpClientTests.java | 23 +- ...ClientDefaultConfigurationTestsConfig.java | 3 +- .../filters/CustomHostRoutingFilterTests.java | 29 ++- .../route/SimpleHostRoutingFilterTests.java | 20 +- 19 files changed, 705 insertions(+), 162 deletions(-) create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FeignHttpClientProperties.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/HttpClientConfigurationTests.java create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/FeignHttpClientPropertiesTests.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java index c643df8a..4393ea3c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java @@ -18,14 +18,24 @@ package org.springframework.cloud.netflix.feign; import java.util.ArrayList; import java.util.List; +import java.util.Timer; +import java.util.TimerTask; import org.apache.http.client.HttpClient; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.actuator.HasFeatures; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; +import org.springframework.cloud.netflix.feign.support.FeignHttpClientProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -34,12 +44,17 @@ import feign.Feign; import feign.httpclient.ApacheHttpClient; import feign.okhttp.OkHttpClient; +import javax.annotation.PreDestroy; +import com.netflix.client.config.CommonClientConfigKey; +import com.netflix.client.config.DefaultClientConfigImpl; + /** * @author Spencer Gibb * @author Julien Roy */ @Configuration @ConditionalOnClass(Feign.class) +@EnableConfigurationProperties({FeignHttpClientProperties.class}) public class FeignAutoConfiguration { @Autowired(required = false) @@ -86,17 +101,51 @@ public class FeignAutoConfiguration { @ConditionalOnMissingClass("com.netflix.loadbalancer.ILoadBalancer") @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) protected static class HttpClientFeignConfiguration { + private final Timer connectionManagerTimer = new Timer( + "FeignApacheHttpClientConfiguration.connectionManagerTimer", true); @Autowired(required = false) - private HttpClient httpClient; + private RegistryBuilder registryBuilder; + + private CloseableHttpClient httpClient; + + @Bean + public HttpClientConnectionManager connectionManager(ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + FeignHttpClientProperties httpClientProperties) { + final HttpClientConnectionManager connectionManager = connectionManagerFactory.newConnectionManager(false, httpClientProperties.getMaxConnections(), + httpClientProperties.getMaxConnectionsPerRoute(), httpClientProperties.getTimeToLive(), + httpClientProperties.getTimeToLiveUnit(), + registryBuilder); + this.connectionManagerTimer.schedule(new TimerTask() { + @Override + public void run() { + connectionManager.closeExpiredConnections(); + } + }, 30000, httpClientProperties.getConnectionTimerRepeat()); + return connectionManager; + } + + @Bean + public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, HttpClientConnectionManager httpClientConnectionManager, + FeignHttpClientProperties httpClientProperties) { + RequestConfig defaultRequestConfig = RequestConfig.custom(). + setConnectTimeout(httpClientProperties.getConnectionTimeout()). + setRedirectsEnabled(httpClientProperties.isFollowRedirects()). + build(); + this.httpClient = httpClientFactory.createClient(defaultRequestConfig, httpClientConnectionManager); + return this.httpClient; + } @Bean @ConditionalOnMissingBean(Client.class) - public Client feignClient() { - if (this.httpClient != null) { - return new ApacheHttpClient(this.httpClient); - } - return new ApacheHttpClient(); + public Client feignClient(HttpClient httpClient) { + return new ApacheHttpClient(httpClient); + } + + @PreDestroy + public void destroy() throws Exception { + connectionManagerTimer.cancel(); + httpClient.close(); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index 4210fa6c..137e6440 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java @@ -17,14 +17,22 @@ package org.springframework.cloud.netflix.feign.ribbon; import org.apache.http.client.HttpClient; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; +import org.springframework.cloud.netflix.feign.support.FeignHttpClientProperties; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -38,6 +46,10 @@ import feign.Request; import feign.httpclient.ApacheHttpClient; import feign.okhttp.OkHttpClient; +import java.util.Timer; +import java.util.TimerTask; +import javax.annotation.PreDestroy; + /** * Autoconfiguration to be activated if Feign is in use and needs to be use Ribbon as a * load balancer. @@ -47,6 +59,7 @@ import feign.okhttp.OkHttpClient; @ConditionalOnClass({ ILoadBalancer.class, Feign.class }) @Configuration @AutoConfigureBefore(FeignAutoConfiguration.class) +@EnableConfigurationProperties({FeignHttpClientProperties.class}) public class FeignRibbonClientAutoConfiguration { @Bean @@ -84,22 +97,54 @@ public class FeignRibbonClientAutoConfiguration { @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) protected static class HttpClientFeignLoadBalancedConfiguration { + private final Timer connectionManagerTimer = new Timer( + "FeignApacheHttpClientConfiguration.connectionManagerTimer", true); + + private CloseableHttpClient httpClient; + @Autowired(required = false) - private HttpClient httpClient; + private RegistryBuilder registryBuilder; + + @Bean + public HttpClientConnectionManager connectionManager(ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + FeignHttpClientProperties httpClientProperties) { + final HttpClientConnectionManager connectionManager = connectionManagerFactory.newConnectionManager(false, httpClientProperties.getMaxConnections(), + httpClientProperties.getMaxConnectionsPerRoute(), httpClientProperties.getTimeToLive(), + httpClientProperties.getTimeToLiveUnit(), + registryBuilder); + this.connectionManagerTimer.schedule(new TimerTask() { + @Override + public void run() { + connectionManager.closeExpiredConnections(); + } + }, 30000, httpClientProperties.getConnectionTimerRepeat()); + return connectionManager; + } + + @Bean + public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, HttpClientConnectionManager httpClientConnectionManager, + FeignHttpClientProperties httpClientProperties) { + RequestConfig defaultRequestConfig = RequestConfig.custom(). + setConnectTimeout(httpClientProperties.getConnectionTimeout()). + setRedirectsEnabled(httpClientProperties.isFollowRedirects()). + build(); + this.httpClient = httpClientFactory.createClient(defaultRequestConfig, httpClientConnectionManager); + return this.httpClient; + } @Bean @ConditionalOnMissingBean(Client.class) public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory, - SpringClientFactory clientFactory) { - ApacheHttpClient delegate; - if (this.httpClient != null) { - delegate = new ApacheHttpClient(this.httpClient); - } - else { - delegate = new ApacheHttpClient(); - } + SpringClientFactory clientFactory, HttpClient httpClient) { + ApacheHttpClient delegate = new ApacheHttpClient(httpClient); return new LoadBalancerFeignClient(delegate, cachingFactory, clientFactory); } + + @PreDestroy + public void destroy() throws Exception { + connectionManagerTimer.cancel(); + httpClient.close(); + } } @Configuration diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FeignHttpClientProperties.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FeignHttpClientProperties.java new file mode 100644 index 00000000..501e3e7d --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/support/FeignHttpClientProperties.java @@ -0,0 +1,110 @@ +/* + * + * * Copyright 2013-2016 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package org.springframework.cloud.netflix.feign.support; + +import java.util.concurrent.TimeUnit; +import org.springframework.boot.context.properties.ConfigurationProperties; + +/** + * @author Ryan Baxter + */ +@ConfigurationProperties(prefix = "feign.httpclient") +public class FeignHttpClientProperties { + public static final boolean DEFAULT_DISABLE_SSL_VALIDATION = false; + public static final int DEFAULT_MAX_CONNECTIONS = 200; + public static final int DEFAULT_MAX_CONNECTIONS_PER_ROUTE = 50; + public static final long DEFAULT_TIME_TO_LIVE = 900L; + public static final TimeUnit DEFAULT_TIME_TO_LIVE_UNIT = TimeUnit.SECONDS; + public static final boolean DEFAULT_FOLLOW_REDIRECTS = true; + public static final int DEFAULT_CONNECTION_TIMEOUT = 2000; + public static final int DEFAULT_CONNECTION_TIMER_REPEAT = 3000; + + private boolean disableSslValidation = DEFAULT_DISABLE_SSL_VALIDATION; + private int maxConnections = DEFAULT_MAX_CONNECTIONS; + private int maxConnectionsPerRoute = DEFAULT_MAX_CONNECTIONS_PER_ROUTE; + private long timeToLive = DEFAULT_TIME_TO_LIVE; + private TimeUnit timeToLiveUnit = DEFAULT_TIME_TO_LIVE_UNIT; + private boolean followRedirects = DEFAULT_FOLLOW_REDIRECTS; + private int connectionTimeout = DEFAULT_CONNECTION_TIMEOUT; + private int connectionTimerRepeat = DEFAULT_CONNECTION_TIMER_REPEAT; + + public int getConnectionTimerRepeat() { + return connectionTimerRepeat; + } + + public void setConnectionTimerRepeat(int connectionTimerRepeat) { + this.connectionTimerRepeat = connectionTimerRepeat; + } + + public boolean isDisableSslValidation() { + return disableSslValidation; + } + + public void setDisableSslValidation(boolean disableSslValidation) { + this.disableSslValidation = disableSslValidation; + } + + public int getMaxConnections() { + return maxConnections; + } + + public void setMaxConnections(int maxConnections) { + this.maxConnections = maxConnections; + } + + public int getMaxConnectionsPerRoute() { + return maxConnectionsPerRoute; + } + + public void setMaxConnectionsPerRoute(int maxConnectionsPerRoute) { + this.maxConnectionsPerRoute = maxConnectionsPerRoute; + } + + public long getTimeToLive() { + return timeToLive; + } + + public void setTimeToLive(long timeToLive) { + this.timeToLive = timeToLive; + } + + public TimeUnit getTimeToLiveUnit() { + return timeToLiveUnit; + } + + public void setTimeToLiveUnit(TimeUnit timeToLiveUnit) { + this.timeToLiveUnit = timeToLiveUnit; + } + + public boolean isFollowRedirects() { + return followRedirects; + } + + public void setFollowRedirects(boolean followRedirects) { + this.followRedirects = followRedirects; + } + + public int getConnectionTimeout() { + return connectionTimeout; + } + + public void setConnectionTimeout(int connectionTimeout) { + this.connectionTimeout = connectionTimeout; + } +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java index d33c214e..cb1bcf38 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java @@ -17,11 +17,19 @@ package org.springframework.cloud.netflix.ribbon; import java.net.URI; +import java.util.Timer; +import java.util.TimerTask; +import java.util.concurrent.TimeUnit; import javax.annotation.PostConstruct; +import javax.annotation.PreDestroy; +import org.apache.http.client.config.RequestConfig; import org.apache.http.client.params.ClientPNames; import org.apache.http.client.params.CookiePolicy; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; @@ -30,17 +38,22 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClas import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.ribbon.apache.RetryableRibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; import org.springframework.cloud.netflix.ribbon.okhttp.RetryableOkHttpLoadBalancingClient; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Lazy; import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.DefaultLoadBalancerRetryHandler; import com.netflix.client.RetryHandler; +import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ConfigurationBasedServerList; @@ -70,6 +83,7 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps @SuppressWarnings("deprecation") @Configuration @EnableConfigurationProperties +@Import(HttpClientConfiguration.class) public class RibbonClientConfiguration { @Value("${ribbon.client.name}") @@ -121,6 +135,71 @@ public class RibbonClientConfiguration { return serverList; } + @Configuration + @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) + protected static class ApacheHttpClientConfiguration { + private final Timer connectionManagerTimer = new Timer( + "RibbonApacheHttpClientConfiguration.connectionManagerTimer", true); + private CloseableHttpClient httpClient; + + @Autowired(required = false) + private RegistryBuilder registryBuilder; + + @Bean + public HttpClientConnectionManager httpClientConnectionManager(IClientConfig config, + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory httpClientFactory) { + Integer maxTotalConnections = config.getPropertyAsInteger(CommonClientConfigKey.MaxTotalConnections, + DefaultClientConfigImpl.DEFAULT_MAX_TOTAL_CONNECTIONS); + Integer maxConnectionsPerHost = config.getPropertyAsInteger(CommonClientConfigKey.MaxConnectionsPerHost, + DefaultClientConfigImpl.DEFAULT_MAX_CONNECTIONS_PER_HOST); + Integer timerRepeat = config.getPropertyAsInteger(CommonClientConfigKey.ConnectionCleanerRepeatInterval, + DefaultClientConfigImpl.DEFAULT_CONNECTION_IDLE_TIMERTASK_REPEAT_IN_MSECS); + Object timeToLiveObj = config.getProperty(CommonClientConfigKey.PoolKeepAliveTime); + Long timeToLive = DefaultClientConfigImpl.DEFAULT_POOL_KEEP_ALIVE_TIME; + Object ttlUnitObj = config.getProperty(CommonClientConfigKey.PoolKeepAliveTimeUnits); + TimeUnit ttlUnit = DefaultClientConfigImpl.DEFAULT_POOL_KEEP_ALIVE_TIME_UNITS; + if(timeToLiveObj instanceof Long) { + timeToLive = (Long)timeToLiveObj; + } + if(ttlUnitObj instanceof TimeUnit) { + ttlUnit = (TimeUnit)ttlUnitObj; + } + final HttpClientConnectionManager connectionManager = connectionManagerFactory.newConnectionManager(false, maxTotalConnections, + maxConnectionsPerHost, timeToLive, ttlUnit, registryBuilder); + this.connectionManagerTimer.schedule(new TimerTask() { + @Override + public void run() { + connectionManager.closeExpiredConnections(); + } + }, 30000, timerRepeat); + return connectionManager; + } + + @Bean + public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, + HttpClientConnectionManager connectionManager, IClientConfig config) { + Boolean followRedirects = config.getPropertyAsBoolean( + CommonClientConfigKey.FollowRedirects, + DefaultClientConfigImpl.DEFAULT_FOLLOW_REDIRECTS); + Integer connectTimeout = config.getPropertyAsInteger( + CommonClientConfigKey.ConnectTimeout, + DefaultClientConfigImpl.DEFAULT_CONNECT_TIMEOUT); + RequestConfig defaultRequestConfig = RequestConfig.custom(). + setConnectTimeout(connectTimeout). + setRedirectsEnabled(followRedirects). + build(); + this.httpClient = httpClientFactory.createClient(defaultRequestConfig, connectionManager); + return httpClient; + } + + @PreDestroy + public void destroy() throws Exception { + connectionManagerTimer.cancel(); + httpClient.close(); + } + } + @Configuration @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) protected static class HttpClientRibbonConfiguration { @@ -132,8 +211,8 @@ public class RibbonClientConfiguration { @ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate") public RibbonLoadBalancingHttpClient ribbonLoadBalancingHttpClient( IClientConfig config, ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, RetryHandler retryHandler) { - RibbonLoadBalancingHttpClient client = new RibbonLoadBalancingHttpClient( + ILoadBalancer loadBalancer, RetryHandler retryHandler, CloseableHttpClient httpClient) { + RibbonLoadBalancingHttpClient client = new RibbonLoadBalancingHttpClient(httpClient, config, serverIntrospector); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); @@ -147,8 +226,9 @@ public class RibbonClientConfiguration { public RetryableRibbonLoadBalancingHttpClient retryableRibbonLoadBalancingHttpClient( IClientConfig config, ServerIntrospector serverIntrospector, ILoadBalancer loadBalancer, RetryHandler retryHandler, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { - RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient( + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + CloseableHttpClient httpClient) { + RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient(httpClient, config, serverIntrospector, loadBalancedRetryPolicyFactory); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java index a46eb011..c17855fc 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java @@ -22,6 +22,7 @@ import org.apache.http.HttpResponse; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; @@ -50,10 +51,17 @@ import com.netflix.loadbalancer.Server; public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient implements ServiceInstanceChooser { private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new LoadBalancedRetryPolicyFactory.NeverRetryFactory(); - public RetryableRibbonLoadBalancingHttpClient(IClientConfig config, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + + public RetryableRibbonLoadBalancingHttpClient(IClientConfig config, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { super(config, serverIntrospector); this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } + public RetryableRibbonLoadBalancingHttpClient(CloseableHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + super(delegate, config, serverIntrospector); + this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; + } @Override public RibbonApacheHttpResponse execute(final RibbonApacheHttpRequest request, final IClientConfig configOverride) throws Exception { diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java index 18db9b68..008d09ec 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java @@ -20,12 +20,11 @@ import com.netflix.client.RequestSpecificRetryHandler; import com.netflix.client.RetryHandler; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; import org.apache.http.HttpResponse; -import org.apache.http.client.HttpClient; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.support.AbstractLoadBalancingClient; @@ -41,27 +40,17 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps */ //TODO: rename (ie new class that extends this in Dalston) to ApacheHttpLoadBalancingClient public class RibbonLoadBalancingHttpClient extends - AbstractLoadBalancingClient { - - @Deprecated - public RibbonLoadBalancingHttpClient() { - super(); - } - - @Deprecated - public RibbonLoadBalancingHttpClient(final ILoadBalancer lb) { - super(lb); - } + AbstractLoadBalancingClient { public RibbonLoadBalancingHttpClient(IClientConfig config, ServerIntrospector serverIntrospector) { super(config, serverIntrospector); } - public RibbonLoadBalancingHttpClient(HttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector) { + public RibbonLoadBalancingHttpClient(CloseableHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector) { super(delegate, config, serverIntrospector); } - protected HttpClient createDelegate(IClientConfig config) { + protected CloseableHttpClient createDelegate(IClientConfig config) { return HttpClientBuilder.create() // already defaults to 0 in builder, so resetting to 0 won't hurt .setMaxConnTotal(config.getPropertyAsInteger(CommonClientConfigKey.MaxTotalConnections, 0)) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java index 1f11d571..82f47439 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java @@ -32,6 +32,9 @@ import org.springframework.cloud.client.discovery.event.HeartbeatEvent; import org.springframework.cloud.client.discovery.event.HeartbeatMonitor; import org.springframework.cloud.client.discovery.event.InstanceRegisteredEvent; import org.springframework.cloud.client.discovery.event.ParentHeartbeatEvent; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; @@ -59,7 +62,8 @@ import org.springframework.context.annotation.Import; @Configuration @Import({ RibbonCommandFactoryConfiguration.RestClientRibbonConfiguration.class, RibbonCommandFactoryConfiguration.OkHttpRibbonConfiguration.class, - RibbonCommandFactoryConfiguration.HttpClientRibbonConfiguration.class }) + RibbonCommandFactoryConfiguration.HttpClientRibbonConfiguration.class, + HttpClientConfiguration.class}) @ConditionalOnBean(ZuulProxyMarkerConfiguration.Marker.class) public class ZuulProxyAutoConfiguration extends ZuulServerAutoConfiguration { @@ -102,8 +106,10 @@ public class ZuulProxyAutoConfiguration extends ZuulServerAutoConfiguration { @Bean @ConditionalOnMissingBean(SimpleHostRoutingFilter.class) - public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties zuulProperties) { - return new SimpleHostRoutingFilter(helper, zuulProperties); + public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties zuulProperties, + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory httpClientFactory) { + return new SimpleHostRoutingFilter(helper, zuulProperties, connectionManagerFactory, httpClientFactory); } @Bean diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index f6b9909c..f1173205 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -19,9 +19,6 @@ package org.springframework.cloud.netflix.zuul.filters.route; import java.io.IOException; import java.io.InputStream; import java.net.URL; -import java.security.SecureRandom; -import java.security.cert.CertificateException; -import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -30,9 +27,6 @@ import java.util.TimerTask; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; -import javax.net.ssl.SSLContext; -import javax.net.ssl.TrustManager; -import javax.net.ssl.X509TrustManager; import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.Log; @@ -41,32 +35,21 @@ import org.apache.http.Header; import org.apache.http.HttpHost; import org.apache.http.HttpRequest; import org.apache.http.HttpResponse; -import org.apache.http.ProtocolException; -import org.apache.http.client.RedirectStrategy; import org.apache.http.client.config.CookieSpecs; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpPatch; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; -import org.apache.http.client.methods.HttpUriRequest; -import org.apache.http.config.Registry; -import org.apache.http.config.RegistryBuilder; -import org.apache.http.conn.socket.ConnectionSocketFactory; -import org.apache.http.conn.socket.PlainConnectionSocketFactory; -import org.apache.http.conn.ssl.NoopHostnameVerifier; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.entity.ContentType; import org.apache.http.entity.InputStreamEntity; import org.apache.http.impl.client.CloseableHttpClient; -import org.apache.http.impl.client.DefaultHttpRequestRetryHandler; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.impl.client.HttpClients; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.message.BasicHeader; import org.apache.http.message.BasicHttpEntityEnclosingRequest; import org.apache.http.message.BasicHttpRequest; -import org.apache.http.protocol.HttpContext; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; import org.springframework.cloud.context.environment.EnvironmentChangeEvent; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; @@ -80,8 +63,6 @@ import org.springframework.util.StringUtils; import com.netflix.zuul.ZuulFilter; import com.netflix.zuul.context.RequestContext; -import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.HTTPS_SCHEME; -import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.HTTP_SCHEME; import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.ROUTE_TYPE; import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.SIMPLE_HOST_ROUTING_FILTER_ORDER; @@ -105,7 +86,9 @@ public class SimpleHostRoutingFilter extends ZuulFilter { private ProxyRequestHelper helper; private Host hostProperties; - private PoolingHttpClientConnectionManager connectionManager; + private ApacheHttpClientConnectionManagerFactory connectionManagerFactory; + private ApacheHttpClientFactory httpClientFactory; + private HttpClientConnectionManager connectionManager; private CloseableHttpClient httpClient; @EventListener @@ -130,16 +113,23 @@ public class SimpleHostRoutingFilter extends ZuulFilter { } } - public SimpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties properties) { + public SimpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties properties, + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory httpClientFactory) { this.helper = helper; this.hostProperties = properties.getHost(); this.sslHostnameValidationEnabled = properties.isSslHostnameValidationEnabled(); this.forceOriginalQueryStringEncoding = properties .isForceOriginalQueryStringEncoding(); + this.connectionManagerFactory = connectionManagerFactory; + this.httpClientFactory = httpClientFactory; } @PostConstruct private void initialize() { + this.connectionManager = connectionManagerFactory.newConnectionManager(this.sslHostnameValidationEnabled, + this.hostProperties.getMaxTotalConnections(), this.hostProperties.getMaxPerRouteConnections(), + this.hostProperties.getTimeToLive(), this.hostProperties.getTimeUnit(), null); this.httpClient = newClient(); this.connectionManagerTimer.schedule(new TimerTask() { @Override @@ -201,50 +191,8 @@ public class SimpleHostRoutingFilter extends ZuulFilter { return null; } - protected PoolingHttpClientConnectionManager newConnectionManager() { - try { - final SSLContext sslContext = SSLContext.getInstance("SSL"); - sslContext.init(null, new TrustManager[] { new X509TrustManager() { - @Override - public void checkClientTrusted(X509Certificate[] x509Certificates, - String s) throws CertificateException { - } - - @Override - public void checkServerTrusted(X509Certificate[] x509Certificates, - String s) throws CertificateException { - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return null; - } - } }, new SecureRandom()); - - RegistryBuilder registryBuilder = RegistryBuilder - . create() - .register(HTTP_SCHEME, PlainConnectionSocketFactory.INSTANCE); - if (this.sslHostnameValidationEnabled) { - registryBuilder.register(HTTPS_SCHEME, - new SSLConnectionSocketFactory(sslContext)); - } - else { - registryBuilder.register(HTTPS_SCHEME, new SSLConnectionSocketFactory( - sslContext, NoopHostnameVerifier.INSTANCE)); - } - final Registry registry = registryBuilder.build(); - - this.connectionManager = new PoolingHttpClientConnectionManager(registry, null, null, null, - hostProperties.getTimeToLive(), hostProperties.getTimeUnit()); - this.connectionManager - .setMaxTotal(this.hostProperties.getMaxTotalConnections()); - this.connectionManager.setDefaultMaxPerRoute( - this.hostProperties.getMaxPerRouteConnections()); - return this.connectionManager; - } - catch (Exception ex) { - throw new RuntimeException(ex); - } + protected HttpClientConnectionManager getConnectionManager() { + return connectionManager; } protected CloseableHttpClient newClient() { @@ -252,30 +200,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter { .setSocketTimeout(this.hostProperties.getSocketTimeoutMillis()) .setConnectTimeout(this.hostProperties.getConnectTimeoutMillis()) .setCookieSpec(CookieSpecs.IGNORE_COOKIES).build(); - - HttpClientBuilder httpClientBuilder = HttpClients.custom(); - if (!this.sslHostnameValidationEnabled) { - httpClientBuilder.setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE); - } - return httpClientBuilder.setConnectionManager(newConnectionManager()) - .disableContentCompression() - .useSystemProperties().setDefaultRequestConfig(requestConfig) - .setRetryHandler(new DefaultHttpRequestRetryHandler(0, false)) - .setRedirectStrategy(new RedirectStrategy() { - @Override - public boolean isRedirected(HttpRequest request, - HttpResponse response, HttpContext context) - throws ProtocolException { - return false; - } - - @Override - public HttpUriRequest getRedirect(HttpRequest request, - HttpResponse response, HttpContext context) - throws ProtocolException { - return null; - } - }).build(); + return httpClientFactory.createClient(requestConfig, this.connectionManager); } private CloseableHttpResponse forward(CloseableHttpClient httpclient, String verb, diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/HttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/HttpClientConfigurationTests.java new file mode 100644 index 00000000..7468f109 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/HttpClientConfigurationTests.java @@ -0,0 +1,203 @@ +/* + * + * * Copyright 2013-2016 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package org.springframework.cloud.netflix; + +import feign.Client; +import feign.httpclient.ApacheHttpClient; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.concurrent.TimeUnit; +import org.apache.http.Header; +import org.apache.http.StatusLine; +import org.apache.http.client.HttpClient; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.apache.http.message.BasicHeader; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.MockingDetails; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; +import org.springframework.cloud.commons.httpclient.DefaultApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.DefaultApacheHttpClientFactory; +import org.springframework.cloud.netflix.feign.EnableFeignClients; +import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient; +import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; +import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; +import org.springframework.cloud.netflix.zuul.EnableZuulProxy; +import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; +import org.springframework.cloud.netflix.zuul.filters.route.SimpleHostRoutingFilter; +import org.springframework.cloud.netflix.zuul.filters.route.apache.HttpClientRibbonCommand; +import org.springframework.cloud.netflix.zuul.filters.route.apache.HttpClientRibbonCommandFactory; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.ReflectionUtils; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockingDetails; + +/** + * @author Ryan Baxter + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = HttpClientConfigurationTestApp.class, value = {"feign.okhttp.enabled: false", + "ribbon.eureka.enabled = false"}) +@DirtiesContext +public class HttpClientConfigurationTests { + + @Autowired + ApacheHttpClientConnectionManagerFactory connectionManagerFactory; + + @Autowired + ApacheHttpClientFactory httpClientFactory; + + @Autowired + SimpleHostRoutingFilter simpleHostRoutingFilter; + + @Autowired + LoadBalancerFeignClient feignClient; + + @Autowired + HttpClientRibbonCommandFactory httpClientRibbonCommandFactory; + + @Test + public void testFactories() { + assertTrue(ApacheHttpClientConnectionManagerFactory.class.isInstance(connectionManagerFactory)); + assertTrue(HttpClientConfigurationTestApp.MyApacheHttpClientConnectionManagerFactory.class.isInstance(connectionManagerFactory)); + assertTrue(ApacheHttpClientFactory.class.isInstance(httpClientFactory)); + assertTrue(HttpClientConfigurationTestApp.MyApacheHttpClientFactory.class.isInstance(httpClientFactory)); + } + + @Test + public void testHttpClientSimpleHostRoutingFilter() { + PoolingHttpClientConnectionManager connectionManager = getField(simpleHostRoutingFilter, "connectionManager"); + MockingDetails connectionManagerDetails = mockingDetails(connectionManager); + assertTrue(connectionManagerDetails.isMock()); + CloseableHttpClient httpClient = getField(simpleHostRoutingFilter, "httpClient"); + MockingDetails httpClientDetails = mockingDetails(httpClient); + assertTrue(httpClientDetails.isMock()); + } + + @Test + public void testHttpClientWithFeign() { + Client delegate = feignClient.getDelegate(); + assertTrue(ApacheHttpClient.class.isInstance(delegate)); + ApacheHttpClient apacheHttpClient = (ApacheHttpClient)delegate; + HttpClient httpClient = getField(apacheHttpClient, "client"); + MockingDetails httpClientDetails = mockingDetails(httpClient); + assertTrue(httpClientDetails.isMock()); + } + + @Test + public void testRibbonLoadBalancingHttpClient() { + RibbonCommandContext context = new RibbonCommandContext("foo"," GET", "http://localhost", + false, new LinkedMultiValueMap(), new LinkedMultiValueMap(), + null, new ArrayList(), 0l); + HttpClientRibbonCommand command = httpClientRibbonCommandFactory.create(context); + RibbonLoadBalancingHttpClient ribbonClient = command.getClient(); + CloseableHttpClient httpClient = getField(ribbonClient, "delegate"); + MockingDetails httpClientDetails = mockingDetails(httpClient); + assertTrue(httpClientDetails.isMock()); + } + + protected T getField(Object target, String name) { + Field field = ReflectionUtils.findField(target.getClass(), name); + ReflectionUtils.makeAccessible(field); + Object value = ReflectionUtils.getField(field, target); + return (T)value; + } +} + +@Configuration +@EnableAutoConfiguration +@RestController +@EnableFeignClients(clients = {HttpClientConfigurationTestApp.FooClient.class}) +@EnableZuulProxy +class HttpClientConfigurationTestApp { + + @RequestMapping + public String index() { + return "hello"; + } + + static class MyApacheHttpClientConnectionManagerFactory extends DefaultApacheHttpClientConnectionManagerFactory { + @Override + public HttpClientConnectionManager newConnectionManager(boolean disableSslValidation, int maxTotalConnections, int maxConnectionsPerRoute, long timeToLive, TimeUnit timeUnit, RegistryBuilder registry) { + return mock(PoolingHttpClientConnectionManager.class); + } + } + + static class MyApacheHttpClientFactory extends DefaultApacheHttpClientFactory { + @Override + public CloseableHttpClient createClient(RequestConfig requestConfig, HttpClientConnectionManager connectionManager) { + CloseableHttpClient client = mock(CloseableHttpClient.class); + CloseableHttpResponse response = mock(CloseableHttpResponse.class); + StatusLine statusLine = mock(StatusLine.class); + doReturn(200).when(statusLine).getStatusCode(); + doReturn(statusLine).when(response).getStatusLine(); + Header[] headers = new BasicHeader[0]; + doReturn(headers).when(response).getAllHeaders(); + try { + doReturn(response).when(client).execute(any(HttpUriRequest.class)); + } catch (IOException e) { + e.printStackTrace(); + } + return client; + } + } + + @Configuration + static class MyConfig { + + @Bean + public ApacheHttpClientFactory apacheHttpClientFactory() { + return new MyApacheHttpClientFactory(); + } + + @Bean + public ApacheHttpClientConnectionManagerFactory connectionManagerFactory() { + return new MyApacheHttpClientConnectionManagerFactory(); + } + + } + + @FeignClient(name="foo", serviceId = "foo") + static interface FooClient {} +} + + diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignCompressionTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignCompressionTests.java index 17ba30d1..515d0401 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignCompressionTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignCompressionTests.java @@ -32,6 +32,7 @@ import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfigurati import org.springframework.boot.builder.SpringApplicationBuilder; import org.springframework.cloud.ClassPathExclusions; import org.springframework.cloud.FilteredClassPathRunner; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.archaius.ArchaiusAutoConfiguration; import org.springframework.cloud.netflix.feign.encoding.FeignAcceptGzipEncodingAutoConfiguration; import org.springframework.cloud.netflix.feign.encoding.FeignAcceptGzipEncodingInterceptor; @@ -58,7 +59,7 @@ public class FeignCompressionTests { context = new SpringApplicationBuilder().properties("feign.compression.response.enabled=true", "feign.compression.request.enabled=true", "feign.okhttp.enabled=false").sources(PropertyPlaceholderAutoConfiguration.class, ArchaiusAutoConfiguration.class, FeignAutoConfiguration.class, PlainConfig.class, FeignContentGzipEncodingAutoConfiguration.class, - FeignAcceptGzipEncodingAutoConfiguration.class).web(false).run(); + FeignAcceptGzipEncodingAutoConfiguration.class, HttpClientConfiguration.class).web(false).run(); } @After diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/FeignHttpClientPropertiesTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/FeignHttpClientPropertiesTests.java new file mode 100644 index 00000000..6431c6a4 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/support/FeignHttpClientPropertiesTests.java @@ -0,0 +1,98 @@ +/* + * + * * Copyright 2013-2016 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package org.springframework.cloud.netflix.feign.support; + +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoConfiguration; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringRunner; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.springframework.boot.test.util.EnvironmentTestUtils.addEnvironment; + +/** + * @author Ryan Baxter + */ +@RunWith(SpringRunner.class) +@DirtiesContext +public class FeignHttpClientPropertiesTests { + + private AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + + @After + public void clear() { + if (this.context != null) { + this.context.close(); + } + } + + @Test + public void testDefaults() { + setupContext(); + assertEquals(FeignHttpClientProperties.DEFAULT_CONNECTION_TIMEOUT, getProperties().getConnectionTimeout()); + assertEquals(FeignHttpClientProperties.DEFAULT_MAX_CONNECTIONS, getProperties().getMaxConnections()); + assertEquals(FeignHttpClientProperties.DEFAULT_MAX_CONNECTIONS_PER_ROUTE, getProperties().getMaxConnectionsPerRoute()); + assertEquals(FeignHttpClientProperties.DEFAULT_TIME_TO_LIVE, getProperties().getTimeToLive()); + assertEquals(FeignHttpClientProperties.DEFAULT_DISABLE_SSL_VALIDATION, getProperties().isDisableSslValidation()); + assertEquals(FeignHttpClientProperties.DEFAULT_FOLLOW_REDIRECTS, getProperties().isFollowRedirects()); + } + + @Test + public void testCustomization() { + addEnvironment(this.context, "feign.httpclient.maxConnections=2", + "feign.httpclient.connectionTimeout=2", + "feign.httpclient.maxConnectionsPerRoute=2", + "feign.httpclient.timeToLive=2", + "feign.httpclient.disableSslValidation=true", + "feign.httpclient.followRedirects=false"); + setupContext(); + assertEquals(2, getProperties().getMaxConnections()); + assertEquals(2, getProperties().getConnectionTimeout()); + assertEquals(2, getProperties().getMaxConnectionsPerRoute()); + assertEquals(2L, getProperties().getTimeToLive()); + assertTrue(getProperties().isDisableSslValidation()); + assertFalse(getProperties().isFollowRedirects()); + } + + private void setupContext() { + this.context.register(PropertyPlaceholderAutoConfiguration.class, TestConfiguration.class); + this.context.refresh(); + } + + private FeignHttpClientProperties getProperties() { + return this.context.getBean(FeignHttpClientProperties.class); + } + + @Configuration + @EnableConfigurationProperties + protected static class TestConfiguration { + @Bean + FeignHttpClientProperties zuulProperties() { + return new FeignHttpClientProperties() ; + } + } +} \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java index 26be6f78..a3e7a50f 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientValidationTests.java @@ -18,6 +18,7 @@ package org.springframework.cloud.netflix.feign.valid; import org.junit.Test; import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.feign.FeignClient; @@ -45,7 +46,7 @@ public class FeignClientValidationTests { } @Configuration - @Import(FeignAutoConfiguration.class) + @Import({FeignAutoConfiguration.class, HttpClientConfiguration.class}) @EnableFeignClients(clients = GoodUrlConfiguration.Client.class) protected static class GoodUrlConfiguration { @@ -67,7 +68,7 @@ public class FeignClientValidationTests { } @Configuration - @Import(FeignAutoConfiguration.class) + @Import({FeignAutoConfiguration.class, HttpClientConfiguration.class}) @EnableFeignClients(clients = PlaceholderUrlConfiguration.Client.class) protected static class PlaceholderUrlConfiguration { @@ -92,7 +93,7 @@ public class FeignClientValidationTests { } @Configuration - @Import(FeignAutoConfiguration.class) + @Import({FeignAutoConfiguration.class, HttpClientConfiguration.class}) @EnableFeignClients(clients = GoodServiceIdConfiguration.Client.class) protected static class GoodServiceIdConfiguration { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientsPreprocessorIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientsPreprocessorIntegrationTests.java index 302a6f90..18c25b3a 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientsPreprocessorIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientsPreprocessorIntegrationTests.java @@ -23,6 +23,7 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.commons.util.UtilAutoConfiguration; import org.springframework.cloud.netflix.archaius.ArchaiusAutoConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonClientsPreprocessorIntegrationTests.TestConfiguration; @@ -66,7 +67,7 @@ public class RibbonClientsPreprocessorIntegrationTests { @Configuration @RibbonClients(@RibbonClient(name = "foo", configuration = FooConfiguration.class)) @Import({ UtilAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, - ArchaiusAutoConfiguration.class, RibbonAutoConfiguration.class }) + ArchaiusAutoConfiguration.class, RibbonAutoConfiguration.class, HttpClientConfiguration.class}) protected static class TestConfiguration { } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringClientFactoryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringClientFactoryTests.java index 2dd65e9c..b927b5f8 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringClientFactoryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringClientFactoryTests.java @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.ribbon; import org.apache.http.client.params.ClientPNames; import org.apache.http.client.params.CookiePolicy; import org.junit.Test; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.archaius.ArchaiusAutoConfiguration; import org.springframework.context.annotation.AnnotationConfigApplicationContext; @@ -69,7 +70,7 @@ public class SpringClientFactoryTests { public void testConfigureRetry() { SpringClientFactory factory = new SpringClientFactory(); AnnotationConfigApplicationContext parent = new AnnotationConfigApplicationContext( - RibbonAutoConfiguration.class, ArchaiusAutoConfiguration.class); + RibbonAutoConfiguration.class, ArchaiusAutoConfiguration.class, HttpClientConfiguration.class); addEnvironment(parent, "foo.ribbon.MaxAutoRetries:2"); factory.setApplicationContext(parent); DefaultLoadBalancerRetryHandler retryHandler = (DefaultLoadBalancerRetryHandler) factory diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryEnabledTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryEnabledTests.java index ea93d0e3..b15cb38b 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryEnabledTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/SpringRetryEnabledTests.java @@ -25,6 +25,7 @@ import org.junit.runner.RunWith; import org.springframework.beans.BeansException; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.feign.ribbon.CachingSpringLoadBalancerFactory; import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer; import org.springframework.cloud.netflix.feign.ribbon.FeignRibbonClientAutoConfiguration; @@ -45,7 +46,7 @@ import static org.hamcrest.collection.IsCollectionWithSize.hasSize; */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(classes = {RibbonAutoConfiguration.class, RibbonClientConfiguration.class, LoadBalancerAutoConfiguration.class, - FeignRibbonClientAutoConfiguration.class}) + FeignRibbonClientAutoConfiguration.class, HttpClientConfiguration.class}) public class SpringRetryEnabledTests implements ApplicationContextAware { private ApplicationContext context; diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java index b77318f0..a263a2f4 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java @@ -24,6 +24,7 @@ import org.apache.http.client.HttpClient; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.junit.After; import org.junit.Before; @@ -169,7 +170,7 @@ public class RibbonLoadBalancingHttpClientTests { @Test public void testNeverRetry() throws Exception { ServerIntrospector introspector = mock(ServerIntrospector.class); - HttpClient delegate = mock(HttpClient.class); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); HttpResponse response = mock(HttpResponse.class); doThrow(new IOException("boom")).when(delegate).execute(any(HttpUriRequest.class)); DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl(); @@ -220,8 +221,8 @@ public class RibbonLoadBalancingHttpClientTests { int port = 80; HttpMethod method = HttpMethod.GET; URI uri = new URI("http://" + host + ":" + port); - HttpClient delegate = mock(HttpClient.class); - final HttpResponse response = mock(HttpResponse.class); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); StatusLine statusLine = mock(StatusLine.class); doReturn(200).when(statusLine).getStatusCode(); doReturn(statusLine).when(response).getStatusLine(); @@ -252,8 +253,8 @@ public class RibbonLoadBalancingHttpClientTests { int port = 80; HttpMethod method = HttpMethod.GET; URI uri = new URI("http://" + host + ":" + port); - HttpClient delegate = mock(HttpClient.class); - final HttpResponse response = mock(HttpResponse.class); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); StatusLine statusLine = mock(StatusLine.class); doReturn(200).when(statusLine).getStatusCode(); doReturn(statusLine).when(response).getStatusLine(); @@ -285,7 +286,7 @@ public class RibbonLoadBalancingHttpClientTests { int port = 80; HttpMethod method = HttpMethod.POST; URI uri = new URI("http://" + host + ":" + port); - HttpClient delegate = mock(HttpClient.class); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); final CloseableHttpResponse response = mock(CloseableHttpResponse.class); StatusLine statusLine = mock(StatusLine.class); doReturn(200).when(statusLine).getStatusCode(); @@ -318,7 +319,7 @@ public class RibbonLoadBalancingHttpClientTests { int port = 80; HttpMethod method = HttpMethod.POST; URI uri = new URI("http://" + host + ":" + port); - HttpClient delegate = mock(HttpClient.class); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); final CloseableHttpResponse response = mock(CloseableHttpResponse.class); doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). when(delegate).execute(any(HttpUriRequest.class)); @@ -353,8 +354,8 @@ public class RibbonLoadBalancingHttpClientTests { int port = 80; HttpMethod method = HttpMethod.GET; URI uri = new URI("http://" + host + ":" + port); - HttpClient delegate = mock(HttpClient.class); - final HttpResponse response = mock(HttpResponse.class); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); + final CloseableHttpResponse response = mock(CloseableHttpResponse.class); StatusLine statusLine = mock(StatusLine.class); doReturn(200).when(statusLine).getStatusCode(); doReturn(statusLine).when(response).getStatusLine(); @@ -442,13 +443,13 @@ public class RibbonLoadBalancingHttpClientTests { String host = serviceName; int port = 80; URI uri = new URI("http://" + host + ":" + port); - HttpClient delegate = mock(HttpClient.class); + CloseableHttpClient delegate = mock(CloseableHttpClient.class); RibbonLoadBalancingHttpClient client = factory.getClient("service", RibbonLoadBalancingHttpClient.class); ReflectionTestUtils.setField(client, "delegate", delegate); ReflectionTestUtils.setField(client, "lb", loadBalancer); - HttpResponse httpResponse = mock(HttpResponse.class); + CloseableHttpResponse httpResponse = mock(CloseableHttpResponse.class); StatusLine statusLine = mock(StatusLine.class); doReturn(200).when(statusLine).getStatusCode(); doReturn(statusLine).when(httpResponse).getStatusLine(); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/test/RibbonClientDefaultConfigurationTestsConfig.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/test/RibbonClientDefaultConfigurationTestsConfig.java index 10d86df1..17ac91fc 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/test/RibbonClientDefaultConfigurationTestsConfig.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/test/RibbonClientDefaultConfigurationTestsConfig.java @@ -17,6 +17,7 @@ package org.springframework.cloud.netflix.ribbon.test; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.commons.util.UtilAutoConfiguration; import org.springframework.cloud.netflix.archaius.ArchaiusAutoConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; @@ -40,7 +41,7 @@ import com.netflix.loadbalancer.ServerListSubsetFilter; */ @Configuration @Import({ PropertyPlaceholderAutoConfiguration.class, ArchaiusAutoConfiguration.class, - UtilAutoConfiguration.class, RibbonAutoConfiguration.class }) + UtilAutoConfiguration.class, RibbonAutoConfiguration.class, HttpClientConfiguration.class}) @RibbonClients(defaultConfiguration = DefaultRibbonConfig.class) public class RibbonClientDefaultConfigurationTestsConfig { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java index 2b1015fd..98ead810 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java @@ -23,6 +23,7 @@ import com.netflix.zuul.context.RequestContext; import org.apache.http.client.config.CookieSpecs; import org.apache.http.client.config.RequestConfig; +import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.client.BasicCookieStore; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClients; @@ -38,6 +39,9 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; +import org.springframework.cloud.commons.httpclient.DefaultApacheHttpClientFactory; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; import org.springframework.cloud.netflix.zuul.RoutesMvcEndpoint; import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientRouteLocator; @@ -210,16 +214,23 @@ class SampleCustomZuulProxyApplication { @EnableZuulProxy protected static class CustomZuulProxyConfig { + @Bean + public ApacheHttpClientFactory customHttpClientFactory() { + return new CustomApacheHttpClientFactory(); + } + @Bean public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, - ZuulProperties zuulProperties) { - return new CustomHostRoutingFilter(helper, zuulProperties); + ZuulProperties zuulProperties, ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory httpClientFactory) { + return new CustomHostRoutingFilter(helper, zuulProperties, connectionManagerFactory, httpClientFactory); } private class CustomHostRoutingFilter extends SimpleHostRoutingFilter { public CustomHostRoutingFilter(ProxyRequestHelper helper, - ZuulProperties zuulProperties) { - super(helper, zuulProperties); + ZuulProperties zuulProperties, ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory httpClientFactory) { + super(helper, zuulProperties, connectionManagerFactory, httpClientFactory); } @Override @@ -227,13 +238,13 @@ class SampleCustomZuulProxyApplication { super.addIgnoredHeaders("X-Ignored"); return super.run(); } + } + + private class CustomApacheHttpClientFactory extends DefaultApacheHttpClientFactory { @Override - protected CloseableHttpClient newClient() { - // Custom client with cookie support. - // In practice, we would want a custom cookie store using a multimap with - // a user key. - return HttpClients.custom().setConnectionManager(newConnectionManager()) + public CloseableHttpClient createClient(RequestConfig requestConfig, HttpClientConnectionManager connectionManager) { + return HttpClients.custom().setConnectionManager(connectionManager) .setDefaultCookieStore(new BasicCookieStore()) .setDefaultRequestConfig(RequestConfig.custom() .setCookieSpec(CookieSpecs.DEFAULT).build()) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java index f7cd9c53..43212d12 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java @@ -47,6 +47,10 @@ import org.springframework.boot.autoconfigure.context.PropertyPlaceholderAutoCon import org.springframework.boot.context.embedded.LocalServerPort; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; +import org.springframework.cloud.commons.httpclient.DefaultApacheHttpClientConnectionManagerFactory; +import org.springframework.cloud.commons.httpclient.DefaultApacheHttpClientFactory; import org.springframework.cloud.context.environment.EnvironmentChangeEvent; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; @@ -113,7 +117,7 @@ public class SimpleHostRoutingFilterTests { "zuul.host.maxPerRouteConnections=10", "zuul.host.timeToLive=5", "zuul.host.timeUnit=SECONDS"); setupContext(); - PoolingHttpClientConnectionManager connMgr = getFilter().newConnectionManager(); + PoolingHttpClientConnectionManager connMgr = (PoolingHttpClientConnectionManager)getFilter().getConnectionManager(); assertEquals(100, connMgr.getMaxTotal()); assertEquals(10, connMgr.getDefaultMaxPerRoute()); Object pool = getField(connMgr, "pool"); @@ -148,7 +152,7 @@ public class SimpleHostRoutingFilterTests { @Test public void defaultPropertiesAreApplied() { setupContext(); - PoolingHttpClientConnectionManager connMgr = getFilter().newConnectionManager(); + PoolingHttpClientConnectionManager connMgr = (PoolingHttpClientConnectionManager)getFilter().getConnectionManager(); assertEquals(200, connMgr.getMaxTotal()); assertEquals(20, connMgr.getDefaultMaxPerRoute()); @@ -222,8 +226,16 @@ public class SimpleHostRoutingFilterTests { } @Bean - SimpleHostRoutingFilter simpleHostRoutingFilter(ZuulProperties zuulProperties) { - return new SimpleHostRoutingFilter(new ProxyRequestHelper(), zuulProperties); + ApacheHttpClientFactory clientFactory() {return new DefaultApacheHttpClientFactory(); } + + @Bean + ApacheHttpClientConnectionManagerFactory connectionManagerFactory() { return new DefaultApacheHttpClientConnectionManagerFactory(); } + + @Bean + SimpleHostRoutingFilter simpleHostRoutingFilter(ZuulProperties zuulProperties, + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory clientFactory) { + return new SimpleHostRoutingFilter(new ProxyRequestHelper(), zuulProperties, connectionManagerFactory, clientFactory); } } } From 572305d7c8e76b2b89cefc305db91d704b908e09 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 28 Jun 2017 12:00:11 -0400 Subject: [PATCH 2/7] Fixing code formatting --- .../netflix/feign/FeignAutoConfiguration.java | 34 ++--- .../FeignRibbonClientAutoConfiguration.java | 39 +++--- .../ribbon/RibbonClientConfiguration.java | 116 +++++++++--------- ...etryableRibbonLoadBalancingHttpClient.java | 102 ++++++++------- .../apache/RibbonLoadBalancingHttpClient.java | 39 +++--- .../zuul/ZuulProxyAutoConfiguration.java | 35 +++--- .../route/SimpleHostRoutingFilter.java | 71 ++++++----- 7 files changed, 243 insertions(+), 193 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java index 4393ea3c..44f1ec7b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java @@ -45,8 +45,6 @@ import feign.httpclient.ApacheHttpClient; import feign.okhttp.OkHttpClient; import javax.annotation.PreDestroy; -import com.netflix.client.config.CommonClientConfigKey; -import com.netflix.client.config.DefaultClientConfigImpl; /** * @author Spencer Gibb @@ -54,7 +52,7 @@ import com.netflix.client.config.DefaultClientConfigImpl; */ @Configuration @ConditionalOnClass(Feign.class) -@EnableConfigurationProperties({FeignHttpClientProperties.class}) +@EnableConfigurationProperties({ FeignHttpClientProperties.class }) public class FeignAutoConfiguration { @Autowired(required = false) @@ -110,12 +108,14 @@ public class FeignAutoConfiguration { private CloseableHttpClient httpClient; @Bean - public HttpClientConnectionManager connectionManager(ApacheHttpClientConnectionManagerFactory connectionManagerFactory, - FeignHttpClientProperties httpClientProperties) { - final HttpClientConnectionManager connectionManager = connectionManagerFactory.newConnectionManager(false, httpClientProperties.getMaxConnections(), - httpClientProperties.getMaxConnectionsPerRoute(), httpClientProperties.getTimeToLive(), - httpClientProperties.getTimeToLiveUnit(), - registryBuilder); + public HttpClientConnectionManager connectionManager( + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + FeignHttpClientProperties httpClientProperties) { + final HttpClientConnectionManager connectionManager = connectionManagerFactory + .newConnectionManager(false, httpClientProperties.getMaxConnections(), + httpClientProperties.getMaxConnectionsPerRoute(), + httpClientProperties.getTimeToLive(), + httpClientProperties.getTimeToLiveUnit(), registryBuilder); this.connectionManagerTimer.schedule(new TimerTask() { @Override public void run() { @@ -126,13 +126,15 @@ public class FeignAutoConfiguration { } @Bean - public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, HttpClientConnectionManager httpClientConnectionManager, - FeignHttpClientProperties httpClientProperties) { - RequestConfig defaultRequestConfig = RequestConfig.custom(). - setConnectTimeout(httpClientProperties.getConnectionTimeout()). - setRedirectsEnabled(httpClientProperties.isFollowRedirects()). - build(); - this.httpClient = httpClientFactory.createClient(defaultRequestConfig, httpClientConnectionManager); + public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, + HttpClientConnectionManager httpClientConnectionManager, + FeignHttpClientProperties httpClientProperties) { + RequestConfig defaultRequestConfig = RequestConfig.custom() + .setConnectTimeout(httpClientProperties.getConnectionTimeout()) + .setRedirectsEnabled(httpClientProperties.isFollowRedirects()) + .build(); + this.httpClient = httpClientFactory.createClient(defaultRequestConfig, + httpClientConnectionManager); return this.httpClient; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index 137e6440..da4baaf4 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java @@ -59,7 +59,7 @@ import javax.annotation.PreDestroy; @ConditionalOnClass({ ILoadBalancer.class, Feign.class }) @Configuration @AutoConfigureBefore(FeignAutoConfiguration.class) -@EnableConfigurationProperties({FeignHttpClientProperties.class}) +@EnableConfigurationProperties({ FeignHttpClientProperties.class }) public class FeignRibbonClientAutoConfiguration { @Bean @@ -74,7 +74,8 @@ public class FeignRibbonClientAutoConfiguration { @Primary @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") public CachingSpringLoadBalancerFactory retryabeCachingLBClientFactory( - SpringClientFactory factory, LoadBalancedRetryPolicyFactory retryPolicyFactory) { + SpringClientFactory factory, + LoadBalancedRetryPolicyFactory retryPolicyFactory) { return new CachingSpringLoadBalancerFactory(factory, retryPolicyFactory, true); } @@ -82,8 +83,8 @@ public class FeignRibbonClientAutoConfiguration { @ConditionalOnMissingBean public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory, SpringClientFactory clientFactory) { - return new LoadBalancerFeignClient(new Client.Default(null, null), - cachingFactory, clientFactory); + return new LoadBalancerFeignClient(new Client.Default(null, null), cachingFactory, + clientFactory); } @Bean @@ -106,12 +107,14 @@ public class FeignRibbonClientAutoConfiguration { private RegistryBuilder registryBuilder; @Bean - public HttpClientConnectionManager connectionManager(ApacheHttpClientConnectionManagerFactory connectionManagerFactory, - FeignHttpClientProperties httpClientProperties) { - final HttpClientConnectionManager connectionManager = connectionManagerFactory.newConnectionManager(false, httpClientProperties.getMaxConnections(), - httpClientProperties.getMaxConnectionsPerRoute(), httpClientProperties.getTimeToLive(), - httpClientProperties.getTimeToLiveUnit(), - registryBuilder); + public HttpClientConnectionManager connectionManager( + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + FeignHttpClientProperties httpClientProperties) { + final HttpClientConnectionManager connectionManager = connectionManagerFactory + .newConnectionManager(false, httpClientProperties.getMaxConnections(), + httpClientProperties.getMaxConnectionsPerRoute(), + httpClientProperties.getTimeToLive(), + httpClientProperties.getTimeToLiveUnit(), registryBuilder); this.connectionManagerTimer.schedule(new TimerTask() { @Override public void run() { @@ -122,13 +125,15 @@ public class FeignRibbonClientAutoConfiguration { } @Bean - public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, HttpClientConnectionManager httpClientConnectionManager, - FeignHttpClientProperties httpClientProperties) { - RequestConfig defaultRequestConfig = RequestConfig.custom(). - setConnectTimeout(httpClientProperties.getConnectionTimeout()). - setRedirectsEnabled(httpClientProperties.isFollowRedirects()). - build(); - this.httpClient = httpClientFactory.createClient(defaultRequestConfig, httpClientConnectionManager); + public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, + HttpClientConnectionManager httpClientConnectionManager, + FeignHttpClientProperties httpClientProperties) { + RequestConfig defaultRequestConfig = RequestConfig.custom() + .setConnectTimeout(httpClientProperties.getConnectionTimeout()) + .setRedirectsEnabled(httpClientProperties.isFollowRedirects()) + .build(); + this.httpClient = httpClientFactory.createClient(defaultRequestConfig, + httpClientConnectionManager); return this.httpClient; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java index cb1bcf38..8e8b4ba1 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java @@ -146,26 +146,33 @@ public class RibbonClientConfiguration { private RegistryBuilder registryBuilder; @Bean - public HttpClientConnectionManager httpClientConnectionManager(IClientConfig config, - ApacheHttpClientConnectionManagerFactory connectionManagerFactory, - ApacheHttpClientFactory httpClientFactory) { - Integer maxTotalConnections = config.getPropertyAsInteger(CommonClientConfigKey.MaxTotalConnections, + public HttpClientConnectionManager httpClientConnectionManager( + IClientConfig config, + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory httpClientFactory) { + Integer maxTotalConnections = config.getPropertyAsInteger( + CommonClientConfigKey.MaxTotalConnections, DefaultClientConfigImpl.DEFAULT_MAX_TOTAL_CONNECTIONS); - Integer maxConnectionsPerHost = config.getPropertyAsInteger(CommonClientConfigKey.MaxConnectionsPerHost, + Integer maxConnectionsPerHost = config.getPropertyAsInteger( + CommonClientConfigKey.MaxConnectionsPerHost, DefaultClientConfigImpl.DEFAULT_MAX_CONNECTIONS_PER_HOST); - Integer timerRepeat = config.getPropertyAsInteger(CommonClientConfigKey.ConnectionCleanerRepeatInterval, + Integer timerRepeat = config.getPropertyAsInteger( + CommonClientConfigKey.ConnectionCleanerRepeatInterval, DefaultClientConfigImpl.DEFAULT_CONNECTION_IDLE_TIMERTASK_REPEAT_IN_MSECS); - Object timeToLiveObj = config.getProperty(CommonClientConfigKey.PoolKeepAliveTime); + Object timeToLiveObj = config + .getProperty(CommonClientConfigKey.PoolKeepAliveTime); Long timeToLive = DefaultClientConfigImpl.DEFAULT_POOL_KEEP_ALIVE_TIME; - Object ttlUnitObj = config.getProperty(CommonClientConfigKey.PoolKeepAliveTimeUnits); + Object ttlUnitObj = config + .getProperty(CommonClientConfigKey.PoolKeepAliveTimeUnits); TimeUnit ttlUnit = DefaultClientConfigImpl.DEFAULT_POOL_KEEP_ALIVE_TIME_UNITS; - if(timeToLiveObj instanceof Long) { - timeToLive = (Long)timeToLiveObj; + if (timeToLiveObj instanceof Long) { + timeToLive = (Long) timeToLiveObj; } - if(ttlUnitObj instanceof TimeUnit) { - ttlUnit = (TimeUnit)ttlUnitObj; + if (ttlUnitObj instanceof TimeUnit) { + ttlUnit = (TimeUnit) ttlUnitObj; } - final HttpClientConnectionManager connectionManager = connectionManagerFactory.newConnectionManager(false, maxTotalConnections, + final HttpClientConnectionManager connectionManager = connectionManagerFactory + .newConnectionManager(false, maxTotalConnections, maxConnectionsPerHost, timeToLive, ttlUnit, registryBuilder); this.connectionManagerTimer.schedule(new TimerTask() { @Override @@ -178,18 +185,18 @@ public class RibbonClientConfiguration { @Bean public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, - HttpClientConnectionManager connectionManager, IClientConfig config) { + HttpClientConnectionManager connectionManager, IClientConfig config) { Boolean followRedirects = config.getPropertyAsBoolean( CommonClientConfigKey.FollowRedirects, DefaultClientConfigImpl.DEFAULT_FOLLOW_REDIRECTS); Integer connectTimeout = config.getPropertyAsInteger( CommonClientConfigKey.ConnectTimeout, DefaultClientConfigImpl.DEFAULT_CONNECT_TIMEOUT); - RequestConfig defaultRequestConfig = RequestConfig.custom(). - setConnectTimeout(connectTimeout). - setRedirectsEnabled(followRedirects). - build(); - this.httpClient = httpClientFactory.createClient(defaultRequestConfig, connectionManager); + RequestConfig defaultRequestConfig = RequestConfig.custom() + .setConnectTimeout(connectTimeout) + .setRedirectsEnabled(followRedirects).build(); + this.httpClient = httpClientFactory.createClient(defaultRequestConfig, + connectionManager); return httpClient; } @@ -211,9 +218,10 @@ public class RibbonClientConfiguration { @ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate") public RibbonLoadBalancingHttpClient ribbonLoadBalancingHttpClient( IClientConfig config, ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, RetryHandler retryHandler, CloseableHttpClient httpClient) { - RibbonLoadBalancingHttpClient client = new RibbonLoadBalancingHttpClient(httpClient, - config, serverIntrospector); + ILoadBalancer loadBalancer, RetryHandler retryHandler, + CloseableHttpClient httpClient) { + RibbonLoadBalancingHttpClient client = new RibbonLoadBalancingHttpClient( + httpClient, config, serverIntrospector); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); @@ -228,8 +236,9 @@ public class RibbonClientConfiguration { ILoadBalancer loadBalancer, RetryHandler retryHandler, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, CloseableHttpClient httpClient) { - RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient(httpClient, - config, serverIntrospector, loadBalancedRetryPolicyFactory); + RetryableRibbonLoadBalancingHttpClient client = new RetryableRibbonLoadBalancingHttpClient( + httpClient, config, serverIntrospector, + loadBalancedRetryPolicyFactory); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); @@ -244,18 +253,15 @@ public class RibbonClientConfiguration { @Value("${ribbon.client.name}") private String name = "client"; - - @Bean @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnClass(name = "org.springframework.retry.support.RetryTemplate") - public RetryableOkHttpLoadBalancingClient okHttpLoadBalancingClient(IClientConfig config, - ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, - RetryHandler retryHandler, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { - RetryableOkHttpLoadBalancingClient client = new RetryableOkHttpLoadBalancingClient(config, - serverIntrospector, loadBalancedRetryPolicyFactory); + public RetryableOkHttpLoadBalancingClient okHttpLoadBalancingClient( + IClientConfig config, ServerIntrospector serverIntrospector, + ILoadBalancer loadBalancer, RetryHandler retryHandler, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + RetryableOkHttpLoadBalancingClient client = new RetryableOkHttpLoadBalancingClient( + config, serverIntrospector, loadBalancedRetryPolicyFactory); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); @@ -265,9 +271,9 @@ public class RibbonClientConfiguration { @Bean @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) @ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate") - public OkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient(IClientConfig config, - ServerIntrospector serverIntrospector, ILoadBalancer loadBalancer, - RetryHandler retryHandler) { + public OkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient( + IClientConfig config, ServerIntrospector serverIntrospector, + ILoadBalancer loadBalancer, RetryHandler retryHandler) { OkHttpLoadBalancingClient client = new OkHttpLoadBalancingClient(config, serverIntrospector); client.setLoadBalancer(loadBalancer); @@ -284,21 +290,23 @@ public class RibbonClientConfiguration { private String name = "client"; /** - * Create a Netflix {@link RestClient} integrated with Ribbon if none already exists - * in the application context. It is not required for Ribbon to work properly and is - * therefore created lazily if ever another component requires it. + * Create a Netflix {@link RestClient} integrated with Ribbon if none already + * exists in the application context. It is not required for Ribbon to work + * properly and is therefore created lazily if ever another component requires it. * - * @param config the configuration to use by the underlying Ribbon instance - * @param loadBalancer the load balancer to use by the underlying Ribbon instance - * @param serverIntrospector server introspector to use by the underlying Ribbon instance - * @param retryHandler retry handler to use by the underlying Ribbon instance + * @param config the configuration to use by the underlying Ribbon instance + * @param loadBalancer the load balancer to use by the underlying Ribbon instance + * @param serverIntrospector server introspector to use by the underlying Ribbon + * instance + * @param retryHandler retry handler to use by the underlying Ribbon instance * @return a {@link RestClient} instances backed by Ribbon */ @Bean @Lazy @ConditionalOnMissingBean(AbstractLoadBalancerAwareClient.class) - public RestClient ribbonRestClient(IClientConfig config, ILoadBalancer loadBalancer, - ServerIntrospector serverIntrospector, RetryHandler retryHandler) { + public RestClient ribbonRestClient(IClientConfig config, + ILoadBalancer loadBalancer, ServerIntrospector serverIntrospector, + RetryHandler retryHandler) { RestClient client = new OverrideRestClient(config, serverIntrospector); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); @@ -339,8 +347,8 @@ public class RibbonClientConfiguration { @Bean @ConditionalOnMissingBean - public RibbonLoadBalancerContext ribbonLoadBalancerContext( - ILoadBalancer loadBalancer, IClientConfig config, RetryHandler retryHandler) { + public RibbonLoadBalancerContext ribbonLoadBalancerContext(ILoadBalancer loadBalancer, + IClientConfig config, RetryHandler retryHandler) { return new RibbonLoadBalancerContext(loadBalancer, config, retryHandler); } @@ -349,7 +357,7 @@ public class RibbonClientConfiguration { public RetryHandler retryHandler(IClientConfig config) { return new DefaultLoadBalancerRetryHandler(config); } - + @Bean @ConditionalOnMissingBean public ServerIntrospector serverIntrospector() { @@ -376,18 +384,16 @@ public class RibbonClientConfiguration { @Override public URI reconstructURIWithServer(Server server, URI original) { - URI uri = updateToHttpsIfNeeded(original, this.config, this.serverIntrospector, server); + URI uri = updateToHttpsIfNeeded(original, this.config, + this.serverIntrospector, server); return super.reconstructURIWithServer(server, uri); } @Override protected Client apacheHttpClientSpecificInitialization() { - ApacheHttpClient4 apache = (ApacheHttpClient4) super - .apacheHttpClientSpecificInitialization(); - apache.getClientHandler() - .getHttpClient() - .getParams() - .setParameter(ClientPNames.COOKIE_POLICY, CookiePolicy.IGNORE_COOKIES); + ApacheHttpClient4 apache = (ApacheHttpClient4) super.apacheHttpClientSpecificInitialization(); + apache.getClientHandler().getHttpClient().getParams().setParameter( + ClientPNames.COOKIE_POLICY, CookiePolicy.IGNORE_COOKIES); return apache; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java index c17855fc..769c75bc 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java @@ -48,75 +48,94 @@ import com.netflix.loadbalancer.Server; * An Apache HTTP client which leverages Spring Retry to retry failed requests. * @author Ryan Baxter */ -public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient implements ServiceInstanceChooser { - private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = - new LoadBalancedRetryPolicyFactory.NeverRetryFactory(); +public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingHttpClient + implements ServiceInstanceChooser { + private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory = new LoadBalancedRetryPolicyFactory.NeverRetryFactory(); - public RetryableRibbonLoadBalancingHttpClient(IClientConfig config, ServerIntrospector serverIntrospector, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + public RetryableRibbonLoadBalancingHttpClient(IClientConfig config, + ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { super(config, serverIntrospector); this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } - public RetryableRibbonLoadBalancingHttpClient(CloseableHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + + public RetryableRibbonLoadBalancingHttpClient(CloseableHttpClient delegate, + IClientConfig config, ServerIntrospector serverIntrospector, + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { super(delegate, config, serverIntrospector); this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } @Override - public RibbonApacheHttpResponse execute(final RibbonApacheHttpRequest request, final IClientConfig configOverride) throws Exception { + public RibbonApacheHttpResponse execute(final RibbonApacheHttpRequest request, + final IClientConfig configOverride) throws Exception { final RequestConfig.Builder builder = RequestConfig.custom(); IClientConfig config = configOverride != null ? configOverride : this.config; - builder.setConnectTimeout(config.get( - CommonClientConfigKey.ConnectTimeout, this.connectTimeout)); - builder.setSocketTimeout(config.get( - CommonClientConfigKey.ReadTimeout, this.readTimeout)); - builder.setRedirectsEnabled(config.get( - CommonClientConfigKey.FollowRedirects, this.followRedirects)); + builder.setConnectTimeout( + config.get(CommonClientConfigKey.ConnectTimeout, this.connectTimeout)); + builder.setSocketTimeout( + config.get(CommonClientConfigKey.ReadTimeout, this.readTimeout)); + builder.setRedirectsEnabled( + config.get(CommonClientConfigKey.FollowRedirects, this.followRedirects)); final RequestConfig requestConfig = builder.build(); - final LoadBalancedRetryPolicy retryPolicy = loadBalancedRetryPolicyFactory.create(this.getClientName(), this); + final LoadBalancedRetryPolicy retryPolicy = loadBalancedRetryPolicyFactory + .create(this.getClientName(), this); RetryCallback retryCallback = new RetryCallback() { @Override - public RibbonApacheHttpResponse doWithRetry(RetryContext context) throws Exception { - //on retries the policy will choose the server and set it in the context - //extract the server and update the request being made + public RibbonApacheHttpResponse doWithRetry(RetryContext context) + throws Exception { + // on retries the policy will choose the server and set it in the context + // extract the server and update the request being made RibbonApacheHttpRequest newRequest = request; - if(context instanceof LoadBalancedRetryContext) { - ServiceInstance service = ((LoadBalancedRetryContext)context).getServiceInstance(); - if(service != null) { - //Reconstruct the request URI using the host and port set in the retry context - newRequest = newRequest.withNewUri(new URI(service.getUri().getScheme(), - newRequest.getURI().getUserInfo(), service.getHost(), service.getPort(), - newRequest.getURI().getPath(), newRequest.getURI().getQuery(), + if (context instanceof LoadBalancedRetryContext) { + ServiceInstance service = ((LoadBalancedRetryContext) context) + .getServiceInstance(); + if (service != null) { + // Reconstruct the request URI using the host and port set in the + // retry context + newRequest = newRequest.withNewUri(new URI( + service.getUri().getScheme(), + newRequest.getURI().getUserInfo(), service.getHost(), + service.getPort(), newRequest.getURI().getPath(), + newRequest.getURI().getQuery(), newRequest.getURI().getFragment())); } } if (isSecure(configOverride)) { - final URI secureUri = UriComponentsBuilder.fromUri(newRequest.getUri()) - .scheme("https").build().toUri(); + final URI secureUri = UriComponentsBuilder + .fromUri(newRequest.getUri()).scheme("https").build().toUri(); newRequest = newRequest.withNewUri(secureUri); } HttpUriRequest httpUriRequest = newRequest.toRequest(requestConfig); - final HttpResponse httpResponse = RetryableRibbonLoadBalancingHttpClient.this.delegate.execute(httpUriRequest); - if(retryPolicy.retryableStatusCode(httpResponse.getStatusLine().getStatusCode())) { - if(CloseableHttpResponse.class.isInstance(httpResponse)) { - ((CloseableHttpResponse)httpResponse).close(); + final HttpResponse httpResponse = RetryableRibbonLoadBalancingHttpClient.this.delegate + .execute(httpUriRequest); + if (retryPolicy.retryableStatusCode( + httpResponse.getStatusLine().getStatusCode())) { + if (CloseableHttpResponse.class.isInstance(httpResponse)) { + ((CloseableHttpResponse) httpResponse).close(); } - throw new RetryableStatusCodeException(RetryableRibbonLoadBalancingHttpClient.this.clientName, + throw new RetryableStatusCodeException( + RetryableRibbonLoadBalancingHttpClient.this.clientName, httpResponse.getStatusLine().getStatusCode()); } - return new RibbonApacheHttpResponse(httpResponse, httpUriRequest.getURI()); + return new RibbonApacheHttpResponse(httpResponse, + httpUriRequest.getURI()); } }; return this.executeWithRetry(request, retryPolicy, retryCallback); } - private RibbonApacheHttpResponse executeWithRetry(RibbonApacheHttpRequest request, LoadBalancedRetryPolicy retryPolicy, RetryCallback callback) throws Exception { + private RibbonApacheHttpResponse executeWithRetry(RibbonApacheHttpRequest request, + LoadBalancedRetryPolicy retryPolicy, + RetryCallback callback) + throws Exception { RetryTemplate retryTemplate = new RetryTemplate(); - boolean retryable = request.getContext() == null ? true : - BooleanUtils.toBooleanDefaultIfNull(request.getContext().getRetryable(), true); - retryTemplate.setRetryPolicy(retryPolicy == null || !retryable ? new NeverRetryPolicy() + boolean retryable = request.getContext() == null ? true + : BooleanUtils.toBooleanDefaultIfNull(request.getContext().getRetryable(), + true); + retryTemplate.setRetryPolicy(retryPolicy == null || !retryable + ? new NeverRetryPolicy() : new RetryPolicy(request, retryPolicy, this, this.getClientName())); return retryTemplate.execute(callback); } @@ -124,17 +143,18 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH @Override public ServiceInstance choose(String serviceId) { Server server = this.getLoadBalancer().chooseServer(serviceId); - return new RibbonLoadBalancerClient.RibbonServer(serviceId, - server); + return new RibbonLoadBalancerClient.RibbonServer(serviceId, server); } @Override - public RequestSpecificRetryHandler getRequestSpecificRetryHandler(RibbonApacheHttpRequest request, IClientConfig requestConfig) { + public RequestSpecificRetryHandler getRequestSpecificRetryHandler( + RibbonApacheHttpRequest request, IClientConfig requestConfig) { return new RequestSpecificRetryHandler(false, false, RetryHandler.DEFAULT, null); } static class RetryPolicy extends FeignRetryPolicy { - public RetryPolicy(HttpRequest request, LoadBalancedRetryPolicy policy, ServiceInstanceChooser serviceInstanceChooser, String serviceName) { + public RetryPolicy(HttpRequest request, LoadBalancedRetryPolicy policy, + ServiceInstanceChooser serviceInstanceChooser, String serviceName) { super(request, policy, serviceInstanceChooser, serviceName); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java index 008d09ec..17eaa9af 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java @@ -38,26 +38,29 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps * @author Christian Lohmann * @author Ryan Baxter */ -//TODO: rename (ie new class that extends this in Dalston) to ApacheHttpLoadBalancingClient +// TODO: rename (ie new class that extends this in Dalston) to ApacheHttpLoadBalancingClient public class RibbonLoadBalancingHttpClient extends AbstractLoadBalancingClient { - public RibbonLoadBalancingHttpClient(IClientConfig config, ServerIntrospector serverIntrospector) { + public RibbonLoadBalancingHttpClient(IClientConfig config, + ServerIntrospector serverIntrospector) { super(config, serverIntrospector); } - public RibbonLoadBalancingHttpClient(CloseableHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector) { + public RibbonLoadBalancingHttpClient(CloseableHttpClient delegate, + IClientConfig config, ServerIntrospector serverIntrospector) { super(delegate, config, serverIntrospector); } protected CloseableHttpClient createDelegate(IClientConfig config) { return HttpClientBuilder.create() // already defaults to 0 in builder, so resetting to 0 won't hurt - .setMaxConnTotal(config.getPropertyAsInteger(CommonClientConfigKey.MaxTotalConnections, 0)) + .setMaxConnTotal(config.getPropertyAsInteger( + CommonClientConfigKey.MaxTotalConnections, 0)) // already defaults to 0 in builder, so resetting to 0 won't hurt - .setMaxConnPerRoute(config.getPropertyAsInteger(CommonClientConfigKey.MaxConnectionsPerHost, 0)) - .disableCookieManagement() - .useSystemProperties() // for proxy + .setMaxConnPerRoute(config.getPropertyAsInteger( + CommonClientConfigKey.MaxConnectionsPerHost, 0)) + .disableCookieManagement().useSystemProperties() // for proxy .build(); } @@ -66,12 +69,12 @@ public class RibbonLoadBalancingHttpClient extends final IClientConfig configOverride) throws Exception { final RequestConfig.Builder builder = RequestConfig.custom(); IClientConfig config = configOverride != null ? configOverride : this.config; - builder.setConnectTimeout(config.get( - CommonClientConfigKey.ConnectTimeout, this.connectTimeout)); - builder.setSocketTimeout(config.get( - CommonClientConfigKey.ReadTimeout, this.readTimeout)); - builder.setRedirectsEnabled(config.get( - CommonClientConfigKey.FollowRedirects, this.followRedirects)); + builder.setConnectTimeout( + config.get(CommonClientConfigKey.ConnectTimeout, this.connectTimeout)); + builder.setSocketTimeout( + config.get(CommonClientConfigKey.ReadTimeout, this.readTimeout)); + builder.setRedirectsEnabled( + config.get(CommonClientConfigKey.FollowRedirects, this.followRedirects)); final RequestConfig requestConfig = builder.build(); if (isSecure(configOverride)) { @@ -86,13 +89,15 @@ public class RibbonLoadBalancingHttpClient extends @Override public URI reconstructURIWithServer(Server server, URI original) { - URI uri = updateToHttpsIfNeeded(original, this.config, this.serverIntrospector, server); + URI uri = updateToHttpsIfNeeded(original, this.config, this.serverIntrospector, + server); return super.reconstructURIWithServer(server, uri); } @Override - public RequestSpecificRetryHandler getRequestSpecificRetryHandler(RibbonApacheHttpRequest request, IClientConfig requestConfig) { - return new RequestSpecificRetryHandler(false, false, - RetryHandler.DEFAULT, requestConfig); + public RequestSpecificRetryHandler getRequestSpecificRetryHandler( + RibbonApacheHttpRequest request, IClientConfig requestConfig) { + return new RequestSpecificRetryHandler(false, false, RetryHandler.DEFAULT, + requestConfig); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java index 82f47439..42e1853b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java @@ -63,7 +63,7 @@ import org.springframework.context.annotation.Import; @Import({ RibbonCommandFactoryConfiguration.RestClientRibbonConfiguration.class, RibbonCommandFactoryConfiguration.OkHttpRibbonConfiguration.class, RibbonCommandFactoryConfiguration.HttpClientRibbonConfiguration.class, - HttpClientConfiguration.class}) + HttpClientConfiguration.class }) @ConditionalOnBean(ZuulProxyMarkerConfiguration.Marker.class) public class ZuulProxyAutoConfiguration extends ZuulServerAutoConfiguration { @@ -79,37 +79,42 @@ public class ZuulProxyAutoConfiguration extends ZuulServerAutoConfiguration { @Override public HasFeatures zuulFeature() { - return HasFeatures.namedFeature("Zuul (Discovery)", ZuulProxyAutoConfiguration.class); + return HasFeatures.namedFeature("Zuul (Discovery)", + ZuulProxyAutoConfiguration.class); } @Bean @ConditionalOnMissingBean(DiscoveryClientRouteLocator.class) public DiscoveryClientRouteLocator discoveryRouteLocator() { - return new DiscoveryClientRouteLocator(this.server.getServletPrefix(), this.discovery, this.zuulProperties, - this.serviceRouteMapper); + return new DiscoveryClientRouteLocator(this.server.getServletPrefix(), + this.discovery, this.zuulProperties, this.serviceRouteMapper); } // pre filters @Bean - public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, ProxyRequestHelper proxyRequestHelper) { - return new PreDecorationFilter(routeLocator, this.server.getServletPrefix(), this.zuulProperties, - proxyRequestHelper); + public PreDecorationFilter preDecorationFilter(RouteLocator routeLocator, + ProxyRequestHelper 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; } @Bean @ConditionalOnMissingBean(SimpleHostRoutingFilter.class) - public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties zuulProperties, - ApacheHttpClientConnectionManagerFactory connectionManagerFactory, - ApacheHttpClientFactory httpClientFactory) { - return new SimpleHostRoutingFilter(helper, zuulProperties, connectionManagerFactory, httpClientFactory); + public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, + ZuulProperties zuulProperties, + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory httpClientFactory) { + return new SimpleHostRoutingFilter(helper, zuulProperties, + connectionManagerFactory, httpClientFactory); } @Bean @@ -150,7 +155,8 @@ public class ZuulProxyAutoConfiguration extends ZuulServerAutoConfiguration { } @Bean - public RoutesMvcEndpoint zuulMvcEndpoint(RouteLocator routeLocator, RoutesEndpoint endpoint) { + public RoutesMvcEndpoint zuulMvcEndpoint(RouteLocator routeLocator, + RoutesEndpoint endpoint) { return new RoutesMvcEndpoint(endpoint, routeLocator); } @@ -166,7 +172,8 @@ public class ZuulProxyAutoConfiguration extends ZuulServerAutoConfiguration { } } - private static class ZuulDiscoveryRefreshListener implements ApplicationListener { + private static class ZuulDiscoveryRefreshListener + implements ApplicationListener { private HeartbeatMonitor monitor = new HeartbeatMonitor(); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index f1173205..baee16dd 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -67,8 +67,8 @@ import static org.springframework.cloud.netflix.zuul.filters.support.FilterConst import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.SIMPLE_HOST_ROUTING_FILTER_ORDER; /** - * Route {@link ZuulFilter} that sends requests to predetermined URLs via apache {@link HttpClient}. - * URLs are found in {@link RequestContext#getRouteHost()}. + * Route {@link ZuulFilter} that sends requests to predetermined URLs via apache + * {@link HttpClient}. URLs are found in {@link RequestContext#getRouteHost()}. * * @author Spencer Gibb * @author Dave Syer @@ -114,8 +114,8 @@ public class SimpleHostRoutingFilter extends ZuulFilter { } public SimpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties properties, - ApacheHttpClientConnectionManagerFactory connectionManagerFactory, - ApacheHttpClientFactory httpClientFactory) { + ApacheHttpClientConnectionManagerFactory connectionManagerFactory, + ApacheHttpClientFactory httpClientFactory) { this.helper = helper; this.hostProperties = properties.getHost(); this.sslHostnameValidationEnabled = properties.isSslHostnameValidationEnabled(); @@ -127,9 +127,12 @@ public class SimpleHostRoutingFilter extends ZuulFilter { @PostConstruct private void initialize() { - this.connectionManager = connectionManagerFactory.newConnectionManager(this.sslHostnameValidationEnabled, - this.hostProperties.getMaxTotalConnections(), this.hostProperties.getMaxPerRouteConnections(), - this.hostProperties.getTimeToLive(), this.hostProperties.getTimeUnit(), null); + this.connectionManager = connectionManagerFactory.newConnectionManager( + this.sslHostnameValidationEnabled, + this.hostProperties.getMaxTotalConnections(), + this.hostProperties.getMaxPerRouteConnections(), + this.hostProperties.getTimeToLive(), this.hostProperties.getTimeUnit(), + null); this.httpClient = newClient(); this.connectionManagerTimer.schedule(new TimerTask() { @Override @@ -220,9 +223,11 @@ public class SimpleHostRoutingFilter extends ZuulFilter { contentType = ContentType.parse(request.getContentType()); } - InputStreamEntity entity = new InputStreamEntity(requestEntity, contentLength, contentType); + InputStreamEntity entity = new InputStreamEntity(requestEntity, contentLength, + contentType); - HttpRequest httpRequest = buildHttpRequest(verb, uri, entity, headers, params, request); + HttpRequest httpRequest = buildHttpRequest(verb, uri, entity, headers, params, + request); try { log.debug(httpHost.getHostName() + " " + httpHost.getPort() + " " + httpHost.getSchemeName()); @@ -248,30 +253,30 @@ public class SimpleHostRoutingFilter extends ZuulFilter { ? getEncodedQueryString(request) : this.helper.getQueryString(params)); switch (verb.toUpperCase()) { - case "POST": - HttpPost httpPost = new HttpPost(uriWithQueryString); - httpRequest = httpPost; - httpPost.setEntity(entity); - break; - case "PUT": - HttpPut httpPut = new HttpPut(uriWithQueryString); - httpRequest = httpPut; - httpPut.setEntity(entity); - break; - case "PATCH": - HttpPatch httpPatch = new HttpPatch(uriWithQueryString); - httpRequest = httpPatch; - httpPatch.setEntity(entity); - break; - case "DELETE": - BasicHttpEntityEnclosingRequest entityRequest = new BasicHttpEntityEnclosingRequest( - verb, uriWithQueryString); - httpRequest = entityRequest; - entityRequest.setEntity(entity); - break; - default: - httpRequest = new BasicHttpRequest(verb, uriWithQueryString); - log.debug(uriWithQueryString); + case "POST": + HttpPost httpPost = new HttpPost(uriWithQueryString); + httpRequest = httpPost; + httpPost.setEntity(entity); + break; + case "PUT": + HttpPut httpPut = new HttpPut(uriWithQueryString); + httpRequest = httpPut; + httpPut.setEntity(entity); + break; + case "PATCH": + HttpPatch httpPatch = new HttpPatch(uriWithQueryString); + httpRequest = httpPatch; + httpPatch.setEntity(entity); + break; + case "DELETE": + BasicHttpEntityEnclosingRequest entityRequest = new BasicHttpEntityEnclosingRequest( + verb, uriWithQueryString); + httpRequest = entityRequest; + entityRequest.setEntity(entity); + break; + default: + httpRequest = new BasicHttpRequest(verb, uriWithQueryString); + log.debug(uriWithQueryString); } httpRequest.setHeaders(convertHeaders(headers)); From b2f608b884d3be6077ec6ec9bb2cf8af1ba0ccee Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Fri, 7 Jul 2017 10:10:53 -0400 Subject: [PATCH 3/7] OK HTTP Client changes --- .../main/asciidoc/spring-cloud-netflix.adoc | 2 +- .../netflix/feign/FeignAutoConfiguration.java | 49 +++++- .../FeignRibbonClientAutoConfiguration.java | 52 ++++-- .../ribbon/RibbonClientConfiguration.java | 81 ++++++++- .../okhttp/OkHttpLoadBalancingClient.java | 10 -- .../RetryableOkHttpLoadBalancingClient.java | 4 +- .../RibbonCommandFactoryConfiguration.java | 16 +- .../java/OkHttpClientConfigurationTests.java | 160 ++++++++++++++++++ ...> ApacheHttpClientConfigurationTests.java} | 12 +- .../netflix/feign/valid/FeignOkHttpTests.java | 2 +- .../RibbonClientConfigurationTests.java | 15 +- .../OkHttpLoadBalancingClientTests.java | 17 +- .../SpringRetryDisableOkHttpClientTests.java | 27 ++- .../SpringRetryEnabledOkHttpClientTests.java | 19 ++- .../zuul/ZuulProxyConfigurationTests.java | 7 +- .../OkHttpRibbonCommandIntegrationTests.java | 20 +++ .../OkHttpRibbonRetryIntegrationTests.java | 4 +- 17 files changed, 409 insertions(+), 88 deletions(-) create mode 100644 spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java rename spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/{HttpClientConfigurationTests.java => ApacheHttpClientConfigurationTests.java} (93%) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 3b0dd846..eb42bb06 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1514,7 +1514,7 @@ path rendering the `users` path unreachable. The default HTTP client used by zuul is now backed by the Apache HTTP Client instead of the deprecated Ribbon `RestClient`. To use `RestClient` or to use the `okhttp3.OkHttpClient` set -`ribbon.restclient.enabled=true` or `ribbon.okhttp.enabled=true` respectively. +`ribbon.restclient.enabled=true` or `spring.cloud.httpclient.ok.enabled=true` respectively. === Cookies and Sensitive Headers diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java index 44f1ec7b..5294a603 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Timer; import java.util.TimerTask; +import java.util.concurrent.TimeUnit; import org.apache.http.client.HttpClient; import org.apache.http.client.config.RequestConfig; @@ -35,6 +36,8 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.client.actuator.HasFeatures; import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; +import org.springframework.cloud.commons.httpclient.OkHttpClientConnectionPoolFactory; +import org.springframework.cloud.commons.httpclient.OkHttpClientFactory; import org.springframework.cloud.netflix.feign.support.FeignHttpClientProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -43,6 +46,7 @@ import feign.Client; import feign.Feign; import feign.httpclient.ApacheHttpClient; import feign.okhttp.OkHttpClient; +import okhttp3.ConnectionPool; import javax.annotation.PreDestroy; @@ -108,6 +112,7 @@ public class FeignAutoConfiguration { private CloseableHttpClient httpClient; @Bean + @ConditionalOnMissingBean(HttpClientConnectionManager.class) public HttpClientConnectionManager connectionManager( ApacheHttpClientConnectionManagerFactory connectionManagerFactory, FeignHttpClientProperties httpClientProperties) { @@ -126,6 +131,7 @@ public class FeignAutoConfiguration { } @Bean + @ConditionalOnMissingBean(CloseableHttpClient.class) public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, HttpClientConnectionManager httpClientConnectionManager, FeignHttpClientProperties httpClientProperties) { @@ -147,26 +153,55 @@ public class FeignAutoConfiguration { @PreDestroy public void destroy() throws Exception { connectionManagerTimer.cancel(); - httpClient.close(); + if(httpClient != null) { + httpClient.close(); + } } } @Configuration @ConditionalOnClass(OkHttpClient.class) @ConditionalOnMissingClass("com.netflix.loadbalancer.ILoadBalancer") - @ConditionalOnProperty(value = "feign.okhttp.enabled", matchIfMissing = true) + @ConditionalOnProperty(value = "feign.okhttp.enabled") protected static class OkHttpFeignConfiguration { - @Autowired(required = false) private okhttp3.OkHttpClient okHttpClient; + @Bean + @ConditionalOnMissingBean(ConnectionPool.class) + public ConnectionPool httpClientConnectionPool(FeignHttpClientProperties httpClientProperties, + OkHttpClientConnectionPoolFactory connectionPoolFactory) { + Integer maxTotalConnections = httpClientProperties.getMaxConnections(); + Long timeToLive = httpClientProperties.getTimeToLive(); + TimeUnit ttlUnit = httpClientProperties.getTimeToLiveUnit(); + return connectionPoolFactory.create(maxTotalConnections, timeToLive, ttlUnit); + } + + @Bean + @ConditionalOnMissingBean(okhttp3.OkHttpClient.class) + public okhttp3.OkHttpClient client(OkHttpClientFactory httpClientFactory, + ConnectionPool connectionPool, FeignHttpClientProperties httpClientProperties) { + Boolean followRedirects = httpClientProperties.isFollowRedirects(); + Integer connectTimeout = httpClientProperties.getConnectionTimeout(); + //TODO Remove read timeout constant after changing to builder pattern + this.okHttpClient = httpClientFactory.create(false, connectTimeout, TimeUnit.MILLISECONDS, + followRedirects, 2000, TimeUnit.MILLISECONDS, connectionPool, null, + null); + return this.okHttpClient; + } + + @PreDestroy + public void destroy() { + if(okHttpClient != null) { + okHttpClient.dispatcher().executorService().shutdown(); + okHttpClient.connectionPool().evictAll(); + } + } + @Bean @ConditionalOnMissingBean(Client.class) public Client feignClient() { - if (this.okHttpClient != null) { - return new OkHttpClient(this.okHttpClient); - } - return new OkHttpClient(); + return new OkHttpClient(this.okHttpClient); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index da4baaf4..c946a0c5 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java @@ -31,6 +31,8 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; +import org.springframework.cloud.commons.httpclient.OkHttpClientConnectionPoolFactory; +import org.springframework.cloud.commons.httpclient.OkHttpClientFactory; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.feign.support.FeignHttpClientProperties; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; @@ -45,9 +47,11 @@ import feign.Feign; import feign.Request; import feign.httpclient.ApacheHttpClient; import feign.okhttp.OkHttpClient; +import okhttp3.ConnectionPool; import java.util.Timer; import java.util.TimerTask; +import java.util.concurrent.TimeUnit; import javax.annotation.PreDestroy; /** @@ -107,6 +111,7 @@ public class FeignRibbonClientAutoConfiguration { private RegistryBuilder registryBuilder; @Bean + @ConditionalOnMissingBean(HttpClientConnectionManager.class) public HttpClientConnectionManager connectionManager( ApacheHttpClientConnectionManagerFactory connectionManagerFactory, FeignHttpClientProperties httpClientProperties) { @@ -125,6 +130,7 @@ public class FeignRibbonClientAutoConfiguration { } @Bean + @ConditionalOnMissingBean(CloseableHttpClient.class) public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, HttpClientConnectionManager httpClientConnectionManager, FeignHttpClientProperties httpClientProperties) { @@ -148,29 +154,55 @@ public class FeignRibbonClientAutoConfiguration { @PreDestroy public void destroy() throws Exception { connectionManagerTimer.cancel(); - httpClient.close(); + if(httpClient != null) { + httpClient.close(); + } } } @Configuration @ConditionalOnClass(OkHttpClient.class) - @ConditionalOnProperty(value = "feign.okhttp.enabled", matchIfMissing = true) + @ConditionalOnProperty(value = "feign.okhttp.enabled") protected static class OkHttpFeignLoadBalancedConfiguration { - @Autowired(required = false) private okhttp3.OkHttpClient okHttpClient; + @Bean + @ConditionalOnMissingBean(ConnectionPool.class) + public ConnectionPool httpClientConnectionPool(FeignHttpClientProperties httpClientProperties, + OkHttpClientConnectionPoolFactory connectionPoolFactory) { + Integer maxTotalConnections = httpClientProperties.getMaxConnections(); + Long timeToLive = httpClientProperties.getTimeToLive(); + TimeUnit ttlUnit = httpClientProperties.getTimeToLiveUnit(); + return connectionPoolFactory.create(maxTotalConnections, timeToLive, ttlUnit); + } + + @Bean + @ConditionalOnMissingBean(okhttp3.OkHttpClient.class) + public okhttp3.OkHttpClient client(OkHttpClientFactory httpClientFactory, + ConnectionPool connectionPool, FeignHttpClientProperties httpClientProperties) { + Boolean followRedirects = httpClientProperties.isFollowRedirects(); + Integer connectTimeout = httpClientProperties.getConnectionTimeout(); + this.okHttpClient = httpClientFactory.create(false, connectTimeout, TimeUnit.MILLISECONDS, + //TODO Remove read timeout constant after changing to builder pattern + followRedirects, 2000, TimeUnit.MILLISECONDS, connectionPool, null, + null); + return this.okHttpClient; + } + + @PreDestroy + public void destroy() { + if(okHttpClient != null) { + okHttpClient.dispatcher().executorService().shutdown(); + okHttpClient.connectionPool().evictAll(); + } + } + @Bean @ConditionalOnMissingBean(Client.class) public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory, SpringClientFactory clientFactory) { - OkHttpClient delegate; - if (this.okHttpClient != null) { - delegate = new OkHttpClient(this.okHttpClient); - } - else { - delegate = new OkHttpClient(); - } + OkHttpClient delegate = new OkHttpClient(this.okHttpClient); return new LoadBalancerFeignClient(delegate, cachingFactory, clientFactory); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java index 8e8b4ba1..86188dae 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java @@ -16,6 +16,9 @@ package org.springframework.cloud.netflix.ribbon; +import okhttp3.ConnectionPool; +import okhttp3.OkHttpClient; + import java.net.URI; import java.util.Timer; import java.util.TimerTask; @@ -41,6 +44,8 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFact import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; +import org.springframework.cloud.commons.httpclient.OkHttpClientConnectionPoolFactory; +import org.springframework.cloud.commons.httpclient.OkHttpClientFactory; import org.springframework.cloud.netflix.ribbon.apache.RetryableRibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; @@ -136,7 +141,7 @@ public class RibbonClientConfiguration { } @Configuration - @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) + @ConditionalOnProperty(name = "spring.cloud.httpclient.apache.enable", matchIfMissing = true) protected static class ApacheHttpClientConfiguration { private final Timer connectionManagerTimer = new Timer( "RibbonApacheHttpClientConfiguration.connectionManagerTimer", true); @@ -146,6 +151,7 @@ public class RibbonClientConfiguration { private RegistryBuilder registryBuilder; @Bean + @ConditionalOnMissingBean(HttpClientConnectionManager.class) public HttpClientConnectionManager httpClientConnectionManager( IClientConfig config, ApacheHttpClientConnectionManagerFactory connectionManagerFactory, @@ -184,6 +190,7 @@ public class RibbonClientConfiguration { } @Bean + @ConditionalOnMissingBean(CloseableHttpClient.class) public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, HttpClientConnectionManager connectionManager, IClientConfig config) { Boolean followRedirects = config.getPropertyAsBoolean( @@ -203,12 +210,69 @@ public class RibbonClientConfiguration { @PreDestroy public void destroy() throws Exception { connectionManagerTimer.cancel(); - httpClient.close(); + if(httpClient != null) { + httpClient.close(); + } } } @Configuration - @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) + @ConditionalOnProperty("spring.cloud.httpclient.ok.enabled") + @ConditionalOnClass(name = "okhttp3.OkHttpClient") + protected static class OkHttpClientConfiguration { + private OkHttpClient httpClient; + + @Bean + @ConditionalOnMissingBean(ConnectionPool.class) + public ConnectionPool httpClientConnectionPool(IClientConfig config, + OkHttpClientConnectionPoolFactory connectionPoolFactory) { + Integer maxTotalConnections = config.getPropertyAsInteger( + CommonClientConfigKey.MaxTotalConnections, + DefaultClientConfigImpl.DEFAULT_MAX_TOTAL_CONNECTIONS); + Object timeToLiveObj = config + .getProperty(CommonClientConfigKey.PoolKeepAliveTime); + Long timeToLive = DefaultClientConfigImpl.DEFAULT_POOL_KEEP_ALIVE_TIME; + Object ttlUnitObj = config + .getProperty(CommonClientConfigKey.PoolKeepAliveTimeUnits); + TimeUnit ttlUnit = DefaultClientConfigImpl.DEFAULT_POOL_KEEP_ALIVE_TIME_UNITS; + if (timeToLiveObj instanceof Long) { + timeToLive = (Long) timeToLiveObj; + } + if (ttlUnitObj instanceof TimeUnit) { + ttlUnit = (TimeUnit) ttlUnitObj; + } + return connectionPoolFactory.create(maxTotalConnections, timeToLive, ttlUnit); + } + + @Bean + @ConditionalOnMissingBean(OkHttpClient.class) + public OkHttpClient client(OkHttpClientFactory httpClientFactory, + ConnectionPool connectionPool, IClientConfig config) { + Boolean followRedirects = config.getPropertyAsBoolean( + CommonClientConfigKey.FollowRedirects, + DefaultClientConfigImpl.DEFAULT_FOLLOW_REDIRECTS); + Integer connectTimeout = config.getPropertyAsInteger( + CommonClientConfigKey.ConnectTimeout, + DefaultClientConfigImpl.DEFAULT_CONNECT_TIMEOUT); + Integer readTimeout = config.getPropertyAsInteger(CommonClientConfigKey.ReadTimeout, + DefaultClientConfigImpl.DEFAULT_READ_TIMEOUT); + this.httpClient = httpClientFactory.create(false, connectTimeout, TimeUnit.MILLISECONDS, + followRedirects, readTimeout, TimeUnit.MILLISECONDS, connectionPool, null, + null); + return this.httpClient; + } + + @PreDestroy + public void destroy() { + if(httpClient != null) { + httpClient.dispatcher().executorService().shutdown(); + httpClient.connectionPool().evictAll(); + } + } + } + + @Configuration + @ConditionalOnProperty(name = "spring.cloud.httpclient.apache.enable", matchIfMissing = true) protected static class HttpClientRibbonConfiguration { @Value("${ribbon.client.name}") private String name = "client"; @@ -247,7 +311,7 @@ public class RibbonClientConfiguration { } @Configuration - @ConditionalOnProperty("ribbon.okhttp.enabled") + @ConditionalOnProperty("spring.cloud.httpclient.ok.enabled") @ConditionalOnClass(name = "okhttp3.OkHttpClient") protected static class OkHttpRibbonConfiguration { @Value("${ribbon.client.name}") @@ -259,9 +323,10 @@ public class RibbonClientConfiguration { public RetryableOkHttpLoadBalancingClient okHttpLoadBalancingClient( IClientConfig config, ServerIntrospector serverIntrospector, ILoadBalancer loadBalancer, RetryHandler retryHandler, - LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { + LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory, + OkHttpClient delegate) { RetryableOkHttpLoadBalancingClient client = new RetryableOkHttpLoadBalancingClient( - config, serverIntrospector, loadBalancedRetryPolicyFactory); + delegate, config, serverIntrospector, loadBalancedRetryPolicyFactory); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); Monitors.registerObject("Client_" + this.name, client); @@ -273,8 +338,8 @@ public class RibbonClientConfiguration { @ConditionalOnMissingClass(value = "org.springframework.retry.support.RetryTemplate") public OkHttpLoadBalancingClient retryableOkHttpLoadBalancingClient( IClientConfig config, ServerIntrospector serverIntrospector, - ILoadBalancer loadBalancer, RetryHandler retryHandler) { - OkHttpLoadBalancingClient client = new OkHttpLoadBalancingClient(config, + ILoadBalancer loadBalancer, RetryHandler retryHandler, OkHttpClient delegate) { + OkHttpLoadBalancingClient client = new OkHttpLoadBalancingClient(delegate, config, serverIntrospector); client.setLoadBalancer(loadBalancer); client.setRetryHandler(retryHandler); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java index 70d7eeb3..ae319d2d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClient.java @@ -41,16 +41,6 @@ import static org.springframework.cloud.netflix.ribbon.RibbonUtils.updateToHttps public class OkHttpLoadBalancingClient extends AbstractLoadBalancingClient { - @Deprecated - public OkHttpLoadBalancingClient() { - super(); - } - - @Deprecated - public OkHttpLoadBalancingClient(final ILoadBalancer lb) { - super(lb); - } - public OkHttpLoadBalancingClient(IClientConfig config, ServerIntrospector serverIntrospector) { super(config, serverIntrospector); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java index 170c275a..3fd53528 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java @@ -49,9 +49,9 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien private LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory; - public RetryableOkHttpLoadBalancingClient(IClientConfig config, ServerIntrospector serverIntrospector, + public RetryableOkHttpLoadBalancingClient(OkHttpClient delegate, IClientConfig config, ServerIntrospector serverIntrospector, LoadBalancedRetryPolicyFactory loadBalancedRetryPolicyFactory) { - super(config, serverIntrospector); + super(delegate, config, serverIntrospector); this.loadBalancedRetryPolicyFactory = loadBalancedRetryPolicyFactory; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java index 95741603..21b1a246 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java @@ -105,11 +105,7 @@ public class RibbonCommandFactoryConfiguration { super(ConfigurationPhase.PARSE_CONFIGURATION); } - @Deprecated //remove in Edgware" - @ConditionalOnProperty(name = "zuul.ribbon.httpclient.enabled", matchIfMissing = true) - static class ZuulProperty {} - - @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) + @ConditionalOnProperty(name = "spring.cloud.httpclient.apache.enable", matchIfMissing = true) static class RibbonProperty {} } @@ -124,11 +120,7 @@ public class RibbonCommandFactoryConfiguration { super(ConfigurationPhase.PARSE_CONFIGURATION); } - @Deprecated //remove in Edgware" - @ConditionalOnProperty("zuul.ribbon.okhttp.enabled") - static class ZuulProperty {} - - @ConditionalOnProperty("ribbon.okhttp.enabled") + @ConditionalOnProperty("spring.cloud.httpclient.ok.enabled") static class RibbonProperty {} } @@ -143,10 +135,6 @@ public class RibbonCommandFactoryConfiguration { super(ConfigurationPhase.PARSE_CONFIGURATION); } - @Deprecated //remove in Edgware" - @ConditionalOnProperty("zuul.ribbon.restclient.enabled") - static class ZuulProperty {} - @ConditionalOnProperty("ribbon.restclient.enabled") static class RibbonProperty {} } diff --git a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java new file mode 100644 index 00000000..3e5e2bdf --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java @@ -0,0 +1,160 @@ +/* + * + * * Copyright 2013-2016 the original author or authors. + * * + * * Licensed under the Apache License, Version 2.0 (the "License"); + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +import feign.Client; +import okhttp3.ConnectionPool; +import okhttp3.OkHttpClient; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.concurrent.TimeUnit; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.X509TrustManager; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.MockingDetails; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.commons.httpclient.DefaultOkHttpClientConnectionPoolFactory; +import org.springframework.cloud.commons.httpclient.DefaultOkHttpClientFactory; +import org.springframework.cloud.commons.httpclient.OkHttpClientConnectionPoolFactory; +import org.springframework.cloud.commons.httpclient.OkHttpClientFactory; +import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient; +import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; +import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; +import org.springframework.cloud.netflix.zuul.EnableZuulProxy; +import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; +import org.springframework.cloud.netflix.zuul.filters.route.okhttp.OkHttpRibbonCommand; +import org.springframework.cloud.netflix.zuul.filters.route.okhttp.OkHttpRibbonCommandFactory; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.ReflectionUtils; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockingDetails; + +/** + * @author Ryan Baxter + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringBootTest(classes = OkHttpClientConfigurationTestApp.class, value = {"feign.okhttp.enabled: true", + "spring.cloud.httpclient.ok.enabled: true", "ribbon.eureka.enabled = false"}) +@DirtiesContext +public class OkHttpClientConfigurationTests { + + @Autowired + OkHttpClientFactory okHttpClientFactory; + + @Autowired + OkHttpClientConnectionPoolFactory connectionPoolFactory; + + @Autowired + LoadBalancerFeignClient feignClient; + + @Autowired + OkHttpRibbonCommandFactory okHttpRibbonCommandFactory; + + @Test + public void testFactories() { + assertTrue(OkHttpClientConnectionPoolFactory.class.isInstance(connectionPoolFactory)); + assertTrue(OkHttpClientConfigurationTestApp.MyOkHttpClientConnectionPoolFactory.class.isInstance(connectionPoolFactory)); + assertTrue(OkHttpClientFactory.class.isInstance(okHttpClientFactory)); + assertTrue(OkHttpClientConfigurationTestApp.MyOkHttpClientFactory.class.isInstance(okHttpClientFactory)); + } + + @Test + public void testHttpClientWithFeign() { + Client delegate = feignClient.getDelegate(); + assertTrue(feign.okhttp.OkHttpClient.class.isInstance(delegate)); + feign.okhttp.OkHttpClient okHttpClient = (feign.okhttp.OkHttpClient)delegate; + OkHttpClient httpClient = getField(okHttpClient, "delegate"); + MockingDetails httpClientDetails = mockingDetails(httpClient); + assertTrue(httpClientDetails.isMock()); + } + + @Test + public void testOkHttpLoadBalancingHttpClient() { + RibbonCommandContext context = new RibbonCommandContext("foo"," GET", "http://localhost", + false, new LinkedMultiValueMap(), new LinkedMultiValueMap(), + null, new ArrayList(), 0l); + OkHttpRibbonCommand command = okHttpRibbonCommandFactory.create(context); + OkHttpLoadBalancingClient ribbonClient = command.getClient(); + OkHttpClient httpClient = getField(ribbonClient, "delegate"); + MockingDetails httpClientDetails = mockingDetails(httpClient); + assertTrue(httpClientDetails.isMock()); + } + + protected T getField(Object target, String name) { + Field field = ReflectionUtils.findField(target.getClass(), name); + ReflectionUtils.makeAccessible(field); + Object value = ReflectionUtils.getField(field, target); + return (T)value; + } +} + +@Configuration +@EnableAutoConfiguration +@RestController +//@EnableFeignClients(clients = {}) +@EnableZuulProxy +class OkHttpClientConfigurationTestApp { + + @RequestMapping + public String index() { + return "hello"; + } + + static class MyOkHttpClientConnectionPoolFactory extends DefaultOkHttpClientConnectionPoolFactory { + @Override + public ConnectionPool create(int maxIdleConnections, long keepAliveDuration, TimeUnit timeUnit) { + return new ConnectionPool(); + } + } + + static class MyOkHttpClientFactory extends DefaultOkHttpClientFactory { + @Override + public OkHttpClient create(boolean disableSslValidation, long connectTimeout, TimeUnit connectTimeoutUnit, boolean followRedirects, long readTimeout, TimeUnit readTimeoutUnit, ConnectionPool connectionPool, SSLSocketFactory sslSocketFactory, X509TrustManager x509TrustManager) { + return mock(OkHttpClient.class); + } + } + + @Configuration + static class MyConfig { + @Bean + public OkHttpClientConnectionPoolFactory connectionPoolFactory() { + return new MyOkHttpClientConnectionPoolFactory(); + } + + @Bean + public OkHttpClientFactory clientFactory() { + return new MyOkHttpClientFactory(); + } + + } + + @FeignClient(name="foo", serviceId = "foo") + static interface FooClient {} +} diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/HttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java similarity index 93% rename from spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/HttpClientConfigurationTests.java rename to spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java index 7468f109..aca8c6cf 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/HttpClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java @@ -75,10 +75,10 @@ import static org.mockito.Mockito.mockingDetails; * @author Ryan Baxter */ @RunWith(SpringJUnit4ClassRunner.class) -@SpringBootTest(classes = HttpClientConfigurationTestApp.class, value = {"feign.okhttp.enabled: false", +@SpringBootTest(classes = ApacheHttpClientConfigurationTestApp.class, value = {"feign.okhttp.enabled: false", "ribbon.eureka.enabled = false"}) @DirtiesContext -public class HttpClientConfigurationTests { +public class ApacheHttpClientConfigurationTests { @Autowired ApacheHttpClientConnectionManagerFactory connectionManagerFactory; @@ -98,9 +98,9 @@ public class HttpClientConfigurationTests { @Test public void testFactories() { assertTrue(ApacheHttpClientConnectionManagerFactory.class.isInstance(connectionManagerFactory)); - assertTrue(HttpClientConfigurationTestApp.MyApacheHttpClientConnectionManagerFactory.class.isInstance(connectionManagerFactory)); + assertTrue(ApacheHttpClientConfigurationTestApp.MyApacheHttpClientConnectionManagerFactory.class.isInstance(connectionManagerFactory)); assertTrue(ApacheHttpClientFactory.class.isInstance(httpClientFactory)); - assertTrue(HttpClientConfigurationTestApp.MyApacheHttpClientFactory.class.isInstance(httpClientFactory)); + assertTrue(ApacheHttpClientConfigurationTestApp.MyApacheHttpClientFactory.class.isInstance(httpClientFactory)); } @Test @@ -146,9 +146,9 @@ public class HttpClientConfigurationTests { @Configuration @EnableAutoConfiguration @RestController -@EnableFeignClients(clients = {HttpClientConfigurationTestApp.FooClient.class}) +@EnableFeignClients(clients = {ApacheHttpClientConfigurationTestApp.FooClient.class}) @EnableZuulProxy -class HttpClientConfigurationTestApp { +class ApacheHttpClientConfigurationTestApp { @RequestMapping public String index() { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignOkHttpTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignOkHttpTests.java index 5b2d917f..ffd7c6ca 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignOkHttpTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignOkHttpTests.java @@ -63,7 +63,7 @@ import lombok.NoArgsConstructor; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = FeignOkHttpTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { "spring.application.name=feignclienttest", "feign.hystrix.enabled=false", - "feign.okhttp.enabled=true" }) + "feign.okhttp.enabled=true", "spring.cloud.httpclient.ok.enabled=true" }) @DirtiesContext public class FeignOkHttpTests { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java index efdac8ac..21c333a9 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java @@ -28,6 +28,7 @@ import org.springframework.beans.factory.BeanFactoryUtils; import org.springframework.beans.factory.ListableBeanFactory; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.util.EnvironmentTestUtils; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration.OverrideRestClient; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.ribbon.okhttp.OkHttpLoadBalancingClient; @@ -151,27 +152,27 @@ public class RibbonClientConfigurationTests { @Test public void testDefaultsToApacheHttpClient() { testClient(RibbonLoadBalancingHttpClient.class, null, RestClient.class, OkHttpLoadBalancingClient.class); - testClient(RibbonLoadBalancingHttpClient.class, "ribbon.httpclient.enabled", RestClient.class, OkHttpLoadBalancingClient.class); + testClient(RibbonLoadBalancingHttpClient.class, new String[]{"spring.cloud.httpclient.apache.enable"}, RestClient.class, OkHttpLoadBalancingClient.class); } @Test public void testEnableRestClient() { - testClient(RestClient.class, "ribbon.restclient.enabled", RibbonLoadBalancingHttpClient.class, + testClient(RestClient.class, new String[]{"ribbon.restclient.enabled"}, RibbonLoadBalancingHttpClient.class, OkHttpLoadBalancingClient.class); } @Test public void testEnableOkHttpClient() { - testClient(OkHttpLoadBalancingClient.class, "ribbon.okhttp.enabled", RibbonLoadBalancingHttpClient.class, + testClient(OkHttpLoadBalancingClient.class, new String[]{"spring.cloud.httpclient.ok.enabled"}, RibbonLoadBalancingHttpClient.class, RestClient.class); } - void testClient(Class clientType, String property, Class... excludedTypes) { + void testClient(Class clientType, String[] properties, Class... excludedTypes) { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); - context.register(RibbonAutoConfiguration.class, + context.register(HttpClientConfiguration.class, RibbonAutoConfiguration.class, RibbonClientConfiguration.class); - if (property != null) { - EnvironmentTestUtils.addEnvironment(context, property); + if (properties != null) { + EnvironmentTestUtils.addEnvironment(context, properties); } context.refresh(); context.getBean(clientType); diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClientTests.java index 2411d0bf..eb622957 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/OkHttpLoadBalancingClientTests.java @@ -20,6 +20,8 @@ import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertThat; import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.context.annotation.AnnotationConfigApplicationContext; @@ -124,7 +126,7 @@ public class OkHttpLoadBalancingClientTests { IClientConfig configOverride, SpringClientFactory factory) throws Exception { factory.setApplicationContext(new AnnotationConfigApplicationContext( - RibbonAutoConfiguration.class, defaultConfigurationClass)); + RibbonAutoConfiguration.class, OkHttpClientConfiguration.class, defaultConfigurationClass)); OkHttpLoadBalancingClient client = factory.getClient("service", OkHttpLoadBalancingClient.class); @@ -132,6 +134,19 @@ public class OkHttpLoadBalancingClientTests { return client.getOkHttpClient(configOverride, false); } + @Configuration + protected static class OkHttpClientConfiguration { + @Autowired(required = false) + IClientConfig clientConfig; + @Bean + public OkHttpLoadBalancingClient okHttpLoadBalancingClient() { + if(clientConfig == null) { + clientConfig = new DefaultClientConfigImpl(); + } + return new OkHttpLoadBalancingClient(new OkHttpClient(), clientConfig, new DefaultServerIntrospector()); + } + } + @Configuration protected static class UseDefaults { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryDisableOkHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryDisableOkHttpClientTests.java index ec5a4b9d..bb69994b 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryDisableOkHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryDisableOkHttpClientTests.java @@ -25,6 +25,7 @@ import org.springframework.cloud.ClassPathExclusions; import org.springframework.cloud.FilteredClassPathRunner; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration; import org.springframework.context.ConfigurableApplicationContext; @@ -37,32 +38,42 @@ import static org.hamcrest.Matchers.instanceOf; * @author Ryan Baxter */ @RunWith(FilteredClassPathRunner.class) -@ClassPathExclusions({"spring-retry-*.jar", "spring-boot-starter-aop-*.jar"}) +@ClassPathExclusions({ "spring-retry-*.jar", "spring-boot-starter-aop-*.jar" }) public class SpringRetryDisableOkHttpClientTests { private ConfigurableApplicationContext context; @Before public void setUp() { - context = new SpringApplicationBuilder().web(false).properties("ribbon.okhttp.enabled=true") - .sources(RibbonAutoConfiguration.class,LoadBalancerAutoConfiguration.class, RibbonClientConfiguration.class).run(); + context = new SpringApplicationBuilder().web(false) + .properties("spring.cloud.httpclient.ok.enabled=true") + .sources(RibbonAutoConfiguration.class, + LoadBalancerAutoConfiguration.class, + HttpClientConfiguration.class, + OkHttpLoadBalancingClientTests.OkHttpClientConfiguration.class, + RibbonClientConfiguration.class) + .run(); } @After public void tearDown() { - if(context != null) { + if (context != null) { context.close(); } } @Test public void testLoadBalancedRetryFactoryBean() throws Exception { - Map factories = context.getBeansOfType(LoadBalancedRetryPolicyFactory.class); + Map factories = context + .getBeansOfType(LoadBalancedRetryPolicyFactory.class); assertThat(factories.values(), hasSize(1)); - assertThat(factories.values().toArray()[0], instanceOf(LoadBalancedRetryPolicyFactory.NeverRetryFactory.class)); - Map clients = context.getBeansOfType(OkHttpLoadBalancingClient.class); + assertThat(factories.values().toArray()[0], + instanceOf(LoadBalancedRetryPolicyFactory.NeverRetryFactory.class)); + Map clients = context + .getBeansOfType(OkHttpLoadBalancingClient.class); assertThat(clients.values(), hasSize(1)); - assertThat(clients.values().toArray()[0], instanceOf(OkHttpLoadBalancingClient.class)); + assertThat(clients.values().toArray()[0], + instanceOf(OkHttpLoadBalancingClient.class)); } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java index c8b3dcab..a9006634 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java @@ -22,6 +22,7 @@ import org.springframework.beans.BeansException; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerAutoConfiguration; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; @@ -38,20 +39,26 @@ import static org.hamcrest.Matchers.instanceOf; * @author Ryan Baxter */ @RunWith(SpringJUnit4ClassRunner.class) -@SpringBootTest(value = {"ribbon.okhttp.enabled: true"}) -@ContextConfiguration(classes = {RibbonAutoConfiguration.class, RibbonClientConfiguration.class, LoadBalancerAutoConfiguration.class}) +@SpringBootTest(value = { "spring.cloud.httpclient.ok.enabled: true" }) +@ContextConfiguration(classes = { RibbonAutoConfiguration.class, + HttpClientConfiguration.class, RibbonClientConfiguration.class, + LoadBalancerAutoConfiguration.class }) public class SpringRetryEnabledOkHttpClientTests implements ApplicationContextAware { private ApplicationContext context; @Test public void testLoadBalancedRetryFactoryBean() throws Exception { - Map factories = context.getBeansOfType(LoadBalancedRetryPolicyFactory.class); + Map factories = context + .getBeansOfType(LoadBalancedRetryPolicyFactory.class); assertThat(factories.values(), hasSize(1)); - assertThat(factories.values().toArray()[0], instanceOf(RibbonLoadBalancedRetryPolicyFactory.class)); - Map clients = context.getBeansOfType(OkHttpLoadBalancingClient.class); + assertThat(factories.values().toArray()[0], + instanceOf(RibbonLoadBalancedRetryPolicyFactory.class)); + Map clients = context + .getBeansOfType(OkHttpLoadBalancingClient.class); assertThat(clients.values(), hasSize(1)); - assertThat(clients.values().toArray()[0], instanceOf(RetryableOkHttpLoadBalancingClient.class)); + assertThat(clients.values().toArray()[0], + instanceOf(RetryableOkHttpLoadBalancingClient.class)); } @Override diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfigurationTests.java index 48ca7300..81e92b83 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfigurationTests.java @@ -43,20 +43,17 @@ public class ZuulProxyConfigurationTests { @Test public void testDefaultsToApacheHttpClient() { testClient(HttpClientRibbonCommandFactory.class, null); - testClient(HttpClientRibbonCommandFactory.class, "zuul.ribbon.httpclient.enabled=true"); - testClient(HttpClientRibbonCommandFactory.class, "ribbon.httpclient.enabled=true"); + testClient(HttpClientRibbonCommandFactory.class, "spring.cloud.httpclient.apache.enable=true"); } @Test public void testEnableRestClient() { - testClient(RestClientRibbonCommandFactory.class, "zuul.ribbon.restclient.enabled=true"); testClient(RestClientRibbonCommandFactory.class, "ribbon.restclient.enabled=true"); } @Test public void testEnableOkHttpClient() { - testClient(OkHttpRibbonCommandFactory.class, "zuul.ribbon.okhttp.enabled=true"); - testClient(OkHttpRibbonCommandFactory.class, "ribbon.okhttp.enabled=true"); + testClient(OkHttpRibbonCommandFactory.class, "spring.cloud.httpclient.ok.enabled=true"); } void testClient(Class clientType, String property) { 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 bfd5b7ea..ff221ec3 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,9 +32,12 @@ import org.springframework.boot.autoconfigure.web.ErrorAttributes; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; import org.springframework.boot.test.web.client.TestRestTemplate; +import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; 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.ribbon.okhttp.OkHttpLoadBalancingClient; +import org.springframework.cloud.netflix.ribbon.test.TestLoadBalancer; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; @@ -49,6 +52,9 @@ import org.springframework.http.ResponseEntity; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.web.bind.annotation.RestController; +import com.netflix.client.DefaultLoadBalancerRetryHandler; +import com.netflix.client.config.DefaultClientConfigImpl; +import com.netflix.client.config.IClientConfig; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = OkHttpRibbonCommandIntegrationTests.TestConfig.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { @@ -119,5 +125,19 @@ public class OkHttpRibbonCommandIntegrationTests extends ZuulProxyTestBase { public MyErrorController myErrorController(ErrorAttributes errorAttributes) { return new MyErrorController(errorAttributes); } + + @Bean + public IClientConfig config() { + return new DefaultClientConfigImpl(); + } + + @Bean + public OkHttpLoadBalancingClient okClient(IClientConfig config) { + final OkHttpLoadBalancingClient client = new OkHttpLoadBalancingClient(config, + new DefaultServerIntrospector()); + client.setLoadBalancer(new TestLoadBalancer<>()); + client.setRetryHandler(new DefaultLoadBalancerRetryHandler()); + return client; + } } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java index e8c39e3d..570f1323 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java @@ -30,7 +30,6 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = RibbonRetryIntegrationTestBase.RetryableTestConfig.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, value = { "zuul.retryable: false", /* Disable retry by default, have each route enable it */ - "ribbon.okhttp.enabled: true", "hystrix.command.default.execution.timeout.enabled: false", /* Disable hystrix so its timeout doesnt get in the way */ "ribbon.ReadTimeout: 1000", /* Make sure ribbon will timeout before the thread is done sleeping */ "zuul.routes.retryable: /retryable/**", @@ -49,7 +48,8 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; "disableretry.ribbon.MaxAutoRetriesNextServer: 1", "zuul.routes.globalretrydisabled: /globalretrydisabled/**", "globalretrydisabled.ribbon.MaxAutoRetries: 1", - "globalretrydisabled.ribbon.MaxAutoRetriesNextServer: 1" + "globalretrydisabled.ribbon.MaxAutoRetriesNextServer: 1", + "spring.cloud.httpclient.ok.enabled: true" }) @DirtiesContext public class OkHttpRibbonRetryIntegrationTests extends RibbonRetryIntegrationTestBase { From 69739420fa5676ae494f3d063ebf43bdce376eb9 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Sun, 9 Jul 2017 13:39:04 -0400 Subject: [PATCH 4/7] Takes advantage of new builder pattern. Also reverted elimination of properties for feign and ribbon. --- .../netflix/feign/FeignAutoConfiguration.java | 18 +++-- .../FeignRibbonClientAutoConfiguration.java | 60 +++++++++------ .../ribbon/RibbonClientConfiguration.java | 26 ++++--- .../RibbonCommandFactoryConfiguration.java | 4 +- .../zuul/ZuulProxyAutoConfiguration.java | 12 ++- .../route/SimpleHostRoutingFilter.java | 75 ++++++++++++------- .../java/OkHttpClientConfigurationTests.java | 12 +-- .../ApacheHttpClientConfigurationTests.java | 10 +-- .../feign/FeignHttpClientUrlTests.java | 2 +- .../RibbonClientConfigurationTests.java | 4 +- ...orPropertiesOverridesIntegrationTests.java | 3 +- .../RibbonLoadBalancingHttpClientTests.java | 3 +- .../SpringRetryDisableOkHttpClientTests.java | 2 +- .../SpringRetryEnabledOkHttpClientTests.java | 2 +- .../zuul/ZuulProxyConfigurationTests.java | 4 +- .../filters/CustomHostRoutingFilterTests.java | 45 +++++++---- .../OkHttpRibbonRetryIntegrationTests.java | 4 +- 17 files changed, 175 insertions(+), 111 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java index 5294a603..fe2bf281 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/FeignAutoConfiguration.java @@ -101,6 +101,7 @@ public class FeignAutoConfiguration { @Configuration @ConditionalOnClass(ApacheHttpClient.class) @ConditionalOnMissingClass("com.netflix.loadbalancer.ILoadBalancer") + @ConditionalOnMissingBean(CloseableHttpClient.class) @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) protected static class HttpClientFeignConfiguration { private final Timer connectionManagerTimer = new Timer( @@ -131,7 +132,7 @@ public class FeignAutoConfiguration { } @Bean - @ConditionalOnMissingBean(CloseableHttpClient.class) + public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, HttpClientConnectionManager httpClientConnectionManager, FeignHttpClientProperties httpClientProperties) { @@ -139,8 +140,9 @@ public class FeignAutoConfiguration { .setConnectTimeout(httpClientProperties.getConnectionTimeout()) .setRedirectsEnabled(httpClientProperties.isFollowRedirects()) .build(); - this.httpClient = httpClientFactory.createClient(defaultRequestConfig, - httpClientConnectionManager); + this.httpClient = httpClientFactory.createBuilder(). + setConnectionManager(httpClientConnectionManager). + setDefaultRequestConfig(defaultRequestConfig).build(); return this.httpClient; } @@ -162,6 +164,7 @@ public class FeignAutoConfiguration { @Configuration @ConditionalOnClass(OkHttpClient.class) @ConditionalOnMissingClass("com.netflix.loadbalancer.ILoadBalancer") + @ConditionalOnMissingBean(okhttp3.OkHttpClient.class) @ConditionalOnProperty(value = "feign.okhttp.enabled") protected static class OkHttpFeignConfiguration { @@ -178,15 +181,14 @@ public class FeignAutoConfiguration { } @Bean - @ConditionalOnMissingBean(okhttp3.OkHttpClient.class) public okhttp3.OkHttpClient client(OkHttpClientFactory httpClientFactory, ConnectionPool connectionPool, FeignHttpClientProperties httpClientProperties) { Boolean followRedirects = httpClientProperties.isFollowRedirects(); Integer connectTimeout = httpClientProperties.getConnectionTimeout(); - //TODO Remove read timeout constant after changing to builder pattern - this.okHttpClient = httpClientFactory.create(false, connectTimeout, TimeUnit.MILLISECONDS, - followRedirects, 2000, TimeUnit.MILLISECONDS, connectionPool, null, - null); + this.okHttpClient = httpClientFactory.createBuilder(false). + connectTimeout(connectTimeout, TimeUnit.MILLISECONDS). + followRedirects(followRedirects). + connectionPool(connectionPool).build(); return this.okHttpClient; } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java index c946a0c5..66256e65 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientAutoConfiguration.java @@ -100,8 +100,8 @@ public class FeignRibbonClientAutoConfiguration { @Configuration @ConditionalOnClass(ApacheHttpClient.class) @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) - protected static class HttpClientFeignLoadBalancedConfiguration { - + @ConditionalOnMissingBean(CloseableHttpClient.class) + protected static class HttpClientFeignConfiguration { private final Timer connectionManagerTimer = new Timer( "FeignApacheHttpClientConfiguration.connectionManagerTimer", true); @@ -130,27 +130,19 @@ public class FeignRibbonClientAutoConfiguration { } @Bean - @ConditionalOnMissingBean(CloseableHttpClient.class) public CloseableHttpClient httpClient(ApacheHttpClientFactory httpClientFactory, - HttpClientConnectionManager httpClientConnectionManager, - FeignHttpClientProperties httpClientProperties) { + HttpClientConnectionManager httpClientConnectionManager, + FeignHttpClientProperties httpClientProperties) { RequestConfig defaultRequestConfig = RequestConfig.custom() .setConnectTimeout(httpClientProperties.getConnectionTimeout()) .setRedirectsEnabled(httpClientProperties.isFollowRedirects()) .build(); - this.httpClient = httpClientFactory.createClient(defaultRequestConfig, - httpClientConnectionManager); + this.httpClient = httpClientFactory.createBuilder(). + setDefaultRequestConfig(defaultRequestConfig). + setConnectionManager(httpClientConnectionManager).build(); return this.httpClient; } - @Bean - @ConditionalOnMissingBean(Client.class) - public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory, - SpringClientFactory clientFactory, HttpClient httpClient) { - ApacheHttpClient delegate = new ApacheHttpClient(httpClient); - return new LoadBalancerFeignClient(delegate, cachingFactory, clientFactory); - } - @PreDestroy public void destroy() throws Exception { connectionManagerTimer.cancel(); @@ -160,11 +152,26 @@ public class FeignRibbonClientAutoConfiguration { } } + @Configuration + @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) + @ConditionalOnClass(ApacheHttpClient.class) + protected static class HttpClientFeignLoadBalancedConfiguration { + + @Bean + @ConditionalOnMissingBean(Client.class) + public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory, + SpringClientFactory clientFactory, HttpClient httpClient) { + ApacheHttpClient delegate = new ApacheHttpClient(httpClient); + return new LoadBalancerFeignClient(delegate, cachingFactory, clientFactory); + } + + } + @Configuration + @ConditionalOnMissingBean(okhttp3.OkHttpClient.class) @ConditionalOnClass(OkHttpClient.class) @ConditionalOnProperty(value = "feign.okhttp.enabled") - protected static class OkHttpFeignLoadBalancedConfiguration { - + protected static class OkHttpFeignConfiguration { private okhttp3.OkHttpClient okHttpClient; @Bean @@ -178,15 +185,14 @@ public class FeignRibbonClientAutoConfiguration { } @Bean - @ConditionalOnMissingBean(okhttp3.OkHttpClient.class) public okhttp3.OkHttpClient client(OkHttpClientFactory httpClientFactory, ConnectionPool connectionPool, FeignHttpClientProperties httpClientProperties) { Boolean followRedirects = httpClientProperties.isFollowRedirects(); Integer connectTimeout = httpClientProperties.getConnectionTimeout(); - this.okHttpClient = httpClientFactory.create(false, connectTimeout, TimeUnit.MILLISECONDS, - //TODO Remove read timeout constant after changing to builder pattern - followRedirects, 2000, TimeUnit.MILLISECONDS, connectionPool, null, - null); + this.okHttpClient = httpClientFactory.createBuilder(false). + connectTimeout(connectTimeout, TimeUnit.MILLISECONDS). + followRedirects(followRedirects). + connectionPool(connectionPool).build(); return this.okHttpClient; } @@ -197,12 +203,18 @@ public class FeignRibbonClientAutoConfiguration { okHttpClient.connectionPool().evictAll(); } } + } + + @Configuration + @ConditionalOnClass(OkHttpClient.class) + @ConditionalOnProperty(value = "feign.okhttp.enabled") + protected static class OkHttpFeignLoadBalancedConfiguration { @Bean @ConditionalOnMissingBean(Client.class) public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory, - SpringClientFactory clientFactory) { - OkHttpClient delegate = new OkHttpClient(this.okHttpClient); + SpringClientFactory clientFactory, okhttp3.OkHttpClient okHttpClient) { + OkHttpClient delegate = new OkHttpClient(okHttpClient); return new LoadBalancerFeignClient(delegate, cachingFactory, clientFactory); } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java index 86188dae..9383b6fb 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfiguration.java @@ -35,6 +35,8 @@ import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.AutoConfigureAfter; +import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass; @@ -141,7 +143,7 @@ public class RibbonClientConfiguration { } @Configuration - @ConditionalOnProperty(name = "spring.cloud.httpclient.apache.enable", matchIfMissing = true) + @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) protected static class ApacheHttpClientConfiguration { private final Timer connectionManagerTimer = new Timer( "RibbonApacheHttpClientConfiguration.connectionManagerTimer", true); @@ -154,8 +156,7 @@ public class RibbonClientConfiguration { @ConditionalOnMissingBean(HttpClientConnectionManager.class) public HttpClientConnectionManager httpClientConnectionManager( IClientConfig config, - ApacheHttpClientConnectionManagerFactory connectionManagerFactory, - ApacheHttpClientFactory httpClientFactory) { + ApacheHttpClientConnectionManagerFactory connectionManagerFactory) { Integer maxTotalConnections = config.getPropertyAsInteger( CommonClientConfigKey.MaxTotalConnections, DefaultClientConfigImpl.DEFAULT_MAX_TOTAL_CONNECTIONS); @@ -202,8 +203,9 @@ public class RibbonClientConfiguration { RequestConfig defaultRequestConfig = RequestConfig.custom() .setConnectTimeout(connectTimeout) .setRedirectsEnabled(followRedirects).build(); - this.httpClient = httpClientFactory.createClient(defaultRequestConfig, - connectionManager); + this.httpClient = httpClientFactory.createBuilder(). + setDefaultRequestConfig(defaultRequestConfig). + setConnectionManager(connectionManager).build(); return httpClient; } @@ -217,7 +219,7 @@ public class RibbonClientConfiguration { } @Configuration - @ConditionalOnProperty("spring.cloud.httpclient.ok.enabled") + @ConditionalOnProperty(value = {"ribbon.okhttp.enabled"}) @ConditionalOnClass(name = "okhttp3.OkHttpClient") protected static class OkHttpClientConfiguration { private OkHttpClient httpClient; @@ -256,9 +258,11 @@ public class RibbonClientConfiguration { DefaultClientConfigImpl.DEFAULT_CONNECT_TIMEOUT); Integer readTimeout = config.getPropertyAsInteger(CommonClientConfigKey.ReadTimeout, DefaultClientConfigImpl.DEFAULT_READ_TIMEOUT); - this.httpClient = httpClientFactory.create(false, connectTimeout, TimeUnit.MILLISECONDS, - followRedirects, readTimeout, TimeUnit.MILLISECONDS, connectionPool, null, - null); + this.httpClient = httpClientFactory.createBuilder(false). + connectTimeout(connectTimeout, TimeUnit.MILLISECONDS). + readTimeout(readTimeout, TimeUnit.MILLISECONDS). + followRedirects(followRedirects). + connectionPool(connectionPool).build(); return this.httpClient; } @@ -272,7 +276,7 @@ public class RibbonClientConfiguration { } @Configuration - @ConditionalOnProperty(name = "spring.cloud.httpclient.apache.enable", matchIfMissing = true) + @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) protected static class HttpClientRibbonConfiguration { @Value("${ribbon.client.name}") private String name = "client"; @@ -311,7 +315,7 @@ public class RibbonClientConfiguration { } @Configuration - @ConditionalOnProperty("spring.cloud.httpclient.ok.enabled") + @ConditionalOnProperty(value = {"ribbon.okhttp.enabled"}) @ConditionalOnClass(name = "okhttp3.OkHttpClient") protected static class OkHttpRibbonConfiguration { @Value("${ribbon.client.name}") diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java index 21b1a246..da5fb6ee 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/RibbonCommandFactoryConfiguration.java @@ -105,7 +105,7 @@ public class RibbonCommandFactoryConfiguration { super(ConfigurationPhase.PARSE_CONFIGURATION); } - @ConditionalOnProperty(name = "spring.cloud.httpclient.apache.enable", matchIfMissing = true) + @ConditionalOnProperty(name = "ribbon.httpclient.enabled", matchIfMissing = true) static class RibbonProperty {} } @@ -120,7 +120,7 @@ public class RibbonCommandFactoryConfiguration { super(ConfigurationPhase.PARSE_CONFIGURATION); } - @ConditionalOnProperty("spring.cloud.httpclient.ok.enabled") + @ConditionalOnProperty("ribbon.okhttp.enabled") static class RibbonProperty {} } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java index 42e1853b..95f1350d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.zuul; import java.util.Collections; import java.util.List; +import org.apache.http.impl.client.CloseableHttpClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.actuate.endpoint.Endpoint; import org.springframework.boot.actuate.trace.TraceRepository; @@ -108,7 +109,7 @@ public class ZuulProxyAutoConfiguration extends ZuulServerAutoConfiguration { } @Bean - @ConditionalOnMissingBean(SimpleHostRoutingFilter.class) + @ConditionalOnMissingBean({SimpleHostRoutingFilter.class, CloseableHttpClient.class}) public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties zuulProperties, ApacheHttpClientConnectionManagerFactory connectionManagerFactory, @@ -117,6 +118,15 @@ public class ZuulProxyAutoConfiguration extends ZuulServerAutoConfiguration { connectionManagerFactory, httpClientFactory); } + @Bean + @ConditionalOnMissingBean({SimpleHostRoutingFilter.class}) + public SimpleHostRoutingFilter simpleHostRoutingFilter2(ProxyRequestHelper helper, + ZuulProperties zuulProperties, + CloseableHttpClient httpClient) { + return new SimpleHostRoutingFilter(helper, zuulProperties, + httpClient); + } + @Bean public ApplicationListener zuulDiscoveryRefreshRoutesListener() { return new ZuulDiscoveryRefreshListener(); diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index baee16dd..58a368d7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -90,26 +90,28 @@ public class SimpleHostRoutingFilter extends ZuulFilter { private ApacheHttpClientFactory httpClientFactory; private HttpClientConnectionManager connectionManager; private CloseableHttpClient httpClient; + private boolean customHttpClient = false; @EventListener public void onPropertyChange(EnvironmentChangeEvent event) { - boolean createNewClient = false; + if(!customHttpClient) { + boolean createNewClient = false; - for (String key : event.getKeys()) { - if (key.startsWith("zuul.host.")) { - createNewClient = true; - break; + for (String key : event.getKeys()) { + if (key.startsWith("zuul.host.")) { + createNewClient = true; + break; + } } - } - if (createNewClient) { - try { - SimpleHostRoutingFilter.this.httpClient.close(); + if (createNewClient) { + try { + SimpleHostRoutingFilter.this.httpClient.close(); + } catch (IOException ex) { + log.error("error closing client", ex); + } + SimpleHostRoutingFilter.this.httpClient = newClient(); } - catch (IOException ex) { - log.error("error closing client", ex); - } - SimpleHostRoutingFilter.this.httpClient = newClient(); } } @@ -125,24 +127,37 @@ public class SimpleHostRoutingFilter extends ZuulFilter { this.httpClientFactory = httpClientFactory; } + public SimpleHostRoutingFilter(ProxyRequestHelper helper, ZuulProperties properties, + CloseableHttpClient httpClient) { + this.helper = helper; + this.hostProperties = properties.getHost(); + this.sslHostnameValidationEnabled = properties.isSslHostnameValidationEnabled(); + this.forceOriginalQueryStringEncoding = properties + .isForceOriginalQueryStringEncoding(); + this.httpClient = httpClient; + this.customHttpClient = true; + } + @PostConstruct private void initialize() { - this.connectionManager = connectionManagerFactory.newConnectionManager( - this.sslHostnameValidationEnabled, - this.hostProperties.getMaxTotalConnections(), - this.hostProperties.getMaxPerRouteConnections(), - this.hostProperties.getTimeToLive(), this.hostProperties.getTimeUnit(), - null); - this.httpClient = newClient(); - this.connectionManagerTimer.schedule(new TimerTask() { - @Override - public void run() { - if (SimpleHostRoutingFilter.this.connectionManager == null) { - return; + if(!customHttpClient) { + this.connectionManager = connectionManagerFactory.newConnectionManager( + this.sslHostnameValidationEnabled, + this.hostProperties.getMaxTotalConnections(), + this.hostProperties.getMaxPerRouteConnections(), + this.hostProperties.getTimeToLive(), this.hostProperties.getTimeUnit(), + null); + this.httpClient = newClient(); + this.connectionManagerTimer.schedule(new TimerTask() { + @Override + public void run() { + if (SimpleHostRoutingFilter.this.connectionManager == null) { + return; + } + SimpleHostRoutingFilter.this.connectionManager.closeExpiredConnections(); } - SimpleHostRoutingFilter.this.connectionManager.closeExpiredConnections(); - } - }, 30000, 5000); + }, 30000, 5000); + } } @PreDestroy @@ -203,7 +218,9 @@ public class SimpleHostRoutingFilter extends ZuulFilter { .setSocketTimeout(this.hostProperties.getSocketTimeoutMillis()) .setConnectTimeout(this.hostProperties.getConnectTimeoutMillis()) .setCookieSpec(CookieSpecs.IGNORE_COOKIES).build(); - return httpClientFactory.createClient(requestConfig, this.connectionManager); + return httpClientFactory.createBuilder(). + setDefaultRequestConfig(requestConfig). + setConnectionManager(this.connectionManager).build(); } private CloseableHttpResponse forward(CloseableHttpClient httpclient, String verb, diff --git a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java index 3e5e2bdf..24f9f919 100644 --- a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java @@ -61,7 +61,7 @@ import static org.mockito.Mockito.mockingDetails; */ @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = OkHttpClientConfigurationTestApp.class, value = {"feign.okhttp.enabled: true", - "spring.cloud.httpclient.ok.enabled: true", "ribbon.eureka.enabled = false"}) + "spring.cloud.httpclient.ok.enabled: true", "ribbon.eureka.enabled = false", "ribbon.okhttp.enabled: true"}) @DirtiesContext public class OkHttpClientConfigurationTests { @@ -118,7 +118,6 @@ public class OkHttpClientConfigurationTests { @Configuration @EnableAutoConfiguration @RestController -//@EnableFeignClients(clients = {}) @EnableZuulProxy class OkHttpClientConfigurationTestApp { @@ -135,10 +134,6 @@ class OkHttpClientConfigurationTestApp { } static class MyOkHttpClientFactory extends DefaultOkHttpClientFactory { - @Override - public OkHttpClient create(boolean disableSslValidation, long connectTimeout, TimeUnit connectTimeoutUnit, boolean followRedirects, long readTimeout, TimeUnit readTimeoutUnit, ConnectionPool connectionPool, SSLSocketFactory sslSocketFactory, X509TrustManager x509TrustManager) { - return mock(OkHttpClient.class); - } } @Configuration @@ -153,6 +148,11 @@ class OkHttpClientConfigurationTestApp { return new MyOkHttpClientFactory(); } + @Bean + public OkHttpClient client() { + return mock(OkHttpClient.class); + } + } @FeignClient(name="foo", serviceId = "foo") diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java index aca8c6cf..8f65f4c3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java @@ -34,6 +34,7 @@ import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.config.RegistryBuilder; import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.message.BasicHeader; import org.junit.Test; @@ -105,9 +106,6 @@ public class ApacheHttpClientConfigurationTests { @Test public void testHttpClientSimpleHostRoutingFilter() { - PoolingHttpClientConnectionManager connectionManager = getField(simpleHostRoutingFilter, "connectionManager"); - MockingDetails connectionManagerDetails = mockingDetails(connectionManager); - assertTrue(connectionManagerDetails.isMock()); CloseableHttpClient httpClient = getField(simpleHostRoutingFilter, "httpClient"); MockingDetails httpClientDetails = mockingDetails(httpClient); assertTrue(httpClientDetails.isMock()); @@ -164,7 +162,7 @@ class ApacheHttpClientConfigurationTestApp { static class MyApacheHttpClientFactory extends DefaultApacheHttpClientFactory { @Override - public CloseableHttpClient createClient(RequestConfig requestConfig, HttpClientConnectionManager connectionManager) { + public HttpClientBuilder createBuilder() { CloseableHttpClient client = mock(CloseableHttpClient.class); CloseableHttpResponse response = mock(CloseableHttpResponse.class); StatusLine statusLine = mock(StatusLine.class); @@ -177,7 +175,9 @@ class ApacheHttpClientConfigurationTestApp { } catch (IOException e) { e.printStackTrace(); } - return client; + HttpClientBuilder builder = mock(HttpClientBuilder.class); + doReturn(client).when(builder).build(); + return builder; } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java index eabfd678..08d70848 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java @@ -58,7 +58,7 @@ import lombok.NoArgsConstructor; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = FeignHttpClientUrlTests.TestConfig.class, webEnvironment = WebEnvironment.DEFINED_PORT, value = { "spring.application.name=feignclienturltest", "feign.hystrix.enabled=false", - "feign.okhttp.enabled=false" }) + "spring.cloud.httpclient.ok.enabled=false" }) @DirtiesContext public class FeignHttpClientUrlTests { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java index 21c333a9..5e3523df 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientConfigurationTests.java @@ -152,7 +152,7 @@ public class RibbonClientConfigurationTests { @Test public void testDefaultsToApacheHttpClient() { testClient(RibbonLoadBalancingHttpClient.class, null, RestClient.class, OkHttpLoadBalancingClient.class); - testClient(RibbonLoadBalancingHttpClient.class, new String[]{"spring.cloud.httpclient.apache.enable"}, RestClient.class, OkHttpLoadBalancingClient.class); + testClient(RibbonLoadBalancingHttpClient.class, new String[]{"ribbon.httpclient.enabled"}, RestClient.class, OkHttpLoadBalancingClient.class); } @Test @@ -163,7 +163,7 @@ public class RibbonClientConfigurationTests { @Test public void testEnableOkHttpClient() { - testClient(OkHttpLoadBalancingClient.class, new String[]{"spring.cloud.httpclient.ok.enabled"}, RibbonLoadBalancingHttpClient.class, + testClient(OkHttpLoadBalancingClient.class, new String[]{"ribbon.okhttp.enabled"}, RibbonLoadBalancingHttpClient.class, RestClient.class); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientPreprocessorPropertiesOverridesIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientPreprocessorPropertiesOverridesIntegrationTests.java index c505e791..94009427 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientPreprocessorPropertiesOverridesIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonClientPreprocessorPropertiesOverridesIntegrationTests.java @@ -26,6 +26,7 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.commons.util.UtilAutoConfiguration; import org.springframework.cloud.netflix.archaius.ArchaiusAutoConfiguration; import org.springframework.cloud.netflix.ribbon.test.TestLoadBalancer; @@ -102,7 +103,7 @@ public class RibbonClientPreprocessorPropertiesOverridesIntegrationTests { @Configuration @RibbonClients - @Import({ UtilAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class, + @Import({ UtilAutoConfiguration.class, HttpClientConfiguration.class, PropertyPlaceholderAutoConfiguration.class, ArchaiusAutoConfiguration.class, RibbonAutoConfiguration.class }) protected static class TestConfiguration { } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java index a263a2f4..18401459 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java @@ -31,6 +31,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicyFactory; +import org.springframework.cloud.commons.httpclient.HttpClientConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonAutoConfiguration; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicy; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; @@ -437,7 +438,7 @@ public class RibbonLoadBalancingHttpClientTests { IClientConfig configOverride, SpringClientFactory factory) throws Exception { - factory.setApplicationContext(new AnnotationConfigApplicationContext( + factory.setApplicationContext(new AnnotationConfigApplicationContext(HttpClientConfiguration.class, RibbonAutoConfiguration.class, defaultConfigurationClass)); String serviceName = "foo"; String host = serviceName; diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryDisableOkHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryDisableOkHttpClientTests.java index bb69994b..5aeac8e3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryDisableOkHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryDisableOkHttpClientTests.java @@ -46,7 +46,7 @@ public class SpringRetryDisableOkHttpClientTests { @Before public void setUp() { context = new SpringApplicationBuilder().web(false) - .properties("spring.cloud.httpclient.ok.enabled=true") + .properties("ribbon.okhttp.enabled=true") .sources(RibbonAutoConfiguration.class, LoadBalancerAutoConfiguration.class, HttpClientConfiguration.class, diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java index a9006634..2cf328d3 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/okhttp/SpringRetryEnabledOkHttpClientTests.java @@ -39,7 +39,7 @@ import static org.hamcrest.Matchers.instanceOf; * @author Ryan Baxter */ @RunWith(SpringJUnit4ClassRunner.class) -@SpringBootTest(value = { "spring.cloud.httpclient.ok.enabled: true" }) +@SpringBootTest(value = { "ribbon.okhttp.enabled: true", "ribbon.httpclient.enabled: false" }) @ContextConfiguration(classes = { RibbonAutoConfiguration.class, HttpClientConfiguration.class, RibbonClientConfiguration.class, LoadBalancerAutoConfiguration.class }) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfigurationTests.java index 81e92b83..67b087b4 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/ZuulProxyConfigurationTests.java @@ -43,7 +43,7 @@ public class ZuulProxyConfigurationTests { @Test public void testDefaultsToApacheHttpClient() { testClient(HttpClientRibbonCommandFactory.class, null); - testClient(HttpClientRibbonCommandFactory.class, "spring.cloud.httpclient.apache.enable=true"); + testClient(HttpClientRibbonCommandFactory.class, "ribbon.httpclient.enabled=true"); } @Test @@ -53,7 +53,7 @@ public class ZuulProxyConfigurationTests { @Test public void testEnableOkHttpClient() { - testClient(OkHttpRibbonCommandFactory.class, "spring.cloud.httpclient.ok.enabled=true"); + testClient(OkHttpRibbonCommandFactory.class, "ribbon.okhttp.enabled=true"); } void testClient(Class clientType, String property) { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java index 98ead810..2dad556c 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java @@ -26,7 +26,9 @@ import org.apache.http.client.config.RequestConfig; import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.impl.client.BasicCookieStore; import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClients; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -35,6 +37,7 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.SpringBootTest.WebEnvironment; @@ -42,6 +45,7 @@ import org.springframework.boot.test.web.client.TestRestTemplate; import org.springframework.cloud.commons.httpclient.ApacheHttpClientConnectionManagerFactory; import org.springframework.cloud.commons.httpclient.ApacheHttpClientFactory; import org.springframework.cloud.commons.httpclient.DefaultApacheHttpClientFactory; +import org.springframework.cloud.netflix.feign.ribbon.FeignRibbonClientAutoConfiguration; import org.springframework.cloud.netflix.zuul.EnableZuulProxy; import org.springframework.cloud.netflix.zuul.RoutesMvcEndpoint; import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientRouteLocator; @@ -212,6 +216,7 @@ class SampleCustomZuulProxyApplication { @Configuration @EnableZuulProxy + @AutoConfigureBefore({FeignRibbonClientAutoConfiguration.class}) protected static class CustomZuulProxyConfig { @Bean @@ -219,18 +224,25 @@ class SampleCustomZuulProxyApplication { return new CustomApacheHttpClientFactory(); } + @Bean + public CloseableHttpClient closeableClient() { + return HttpClients.custom() + .setDefaultCookieStore(new BasicCookieStore()) + .setDefaultRequestConfig(RequestConfig.custom() + .setCookieSpec(CookieSpecs.DEFAULT).build()) + .build(); + } + @Bean public SimpleHostRoutingFilter simpleHostRoutingFilter(ProxyRequestHelper helper, - ZuulProperties zuulProperties, ApacheHttpClientConnectionManagerFactory connectionManagerFactory, - ApacheHttpClientFactory httpClientFactory) { - return new CustomHostRoutingFilter(helper, zuulProperties, connectionManagerFactory, httpClientFactory); + ZuulProperties zuulProperties, CloseableHttpClient httpClient) { + return new CustomHostRoutingFilter(helper, zuulProperties, httpClient); } private class CustomHostRoutingFilter extends SimpleHostRoutingFilter { public CustomHostRoutingFilter(ProxyRequestHelper helper, - ZuulProperties zuulProperties, ApacheHttpClientConnectionManagerFactory connectionManagerFactory, - ApacheHttpClientFactory httpClientFactory) { - super(helper, zuulProperties, connectionManagerFactory, httpClientFactory); + ZuulProperties zuulProperties, CloseableHttpClient httpClient) { + super(helper, zuulProperties, httpClient); } @Override @@ -242,14 +254,19 @@ class SampleCustomZuulProxyApplication { private class CustomApacheHttpClientFactory extends DefaultApacheHttpClientFactory { - @Override - public CloseableHttpClient createClient(RequestConfig requestConfig, HttpClientConnectionManager connectionManager) { - return HttpClients.custom().setConnectionManager(connectionManager) - .setDefaultCookieStore(new BasicCookieStore()) - .setDefaultRequestConfig(RequestConfig.custom() - .setCookieSpec(CookieSpecs.DEFAULT).build()) - .build(); - } +//@Override +//public HttpClientBuilder createBuilder() { +// return HttpClients.custom() +// .setDefaultCookieStore(new BasicCookieStore()) +// .setDefaultRequestConfig(RequestConfig.custom() +// .setCookieSpec(CookieSpecs.DEFAULT).build()); +//}public CloseableHttpClient createClient(RequestConfig requestConfig, HttpClientConnectionManager connectionManager) { +// return HttpClients.custom().setConnectionManager(connectionManager) +// .setDefaultCookieStore(new BasicCookieStore()) +// .setDefaultRequestConfig(RequestConfig.custom() +// .setCookieSpec(CookieSpecs.DEFAULT).build()) +// .build(); +// } } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java index 570f1323..f23dc053 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java @@ -30,6 +30,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = RibbonRetryIntegrationTestBase.RetryableTestConfig.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, value = { "zuul.retryable: false", /* Disable retry by default, have each route enable it */ + "ribbon.okhttp.enabled", "hystrix.command.default.execution.timeout.enabled: false", /* Disable hystrix so its timeout doesnt get in the way */ "ribbon.ReadTimeout: 1000", /* Make sure ribbon will timeout before the thread is done sleeping */ "zuul.routes.retryable: /retryable/**", @@ -48,8 +49,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; "disableretry.ribbon.MaxAutoRetriesNextServer: 1", "zuul.routes.globalretrydisabled: /globalretrydisabled/**", "globalretrydisabled.ribbon.MaxAutoRetries: 1", - "globalretrydisabled.ribbon.MaxAutoRetriesNextServer: 1", - "spring.cloud.httpclient.ok.enabled: true" + "globalretrydisabled.ribbon.MaxAutoRetriesNextServer: 1" }) @DirtiesContext public class OkHttpRibbonRetryIntegrationTests extends RibbonRetryIntegrationTestBase { From ff3390570478ba6b9cc3eeb8f1c5d4c4a7679fd2 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Sun, 9 Jul 2017 14:21:10 -0400 Subject: [PATCH 5/7] Updating property names --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 2 +- .../src/test/java/OkHttpClientConfigurationTests.java | 2 +- .../cloud/netflix/feign/FeignHttpClientUrlTests.java | 2 +- .../cloud/netflix/feign/valid/FeignOkHttpTests.java | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 4f7443b3..99a1bc65 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1536,7 +1536,7 @@ path rendering the `users` path unreachable. The default HTTP client used by zuul is now backed by the Apache HTTP Client instead of the deprecated Ribbon `RestClient`. To use `RestClient` or to use the `okhttp3.OkHttpClient` set -`ribbon.restclient.enabled=true` or `spring.cloud.httpclient.ok.enabled=true` respectively. +`ribbon.restclient.enabled=true` or `ribbon.okhttp.enabled=true` respectively. === Cookies and Sensitive Headers diff --git a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java index 24f9f919..503ebf89 100644 --- a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java @@ -61,7 +61,7 @@ import static org.mockito.Mockito.mockingDetails; */ @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = OkHttpClientConfigurationTestApp.class, value = {"feign.okhttp.enabled: true", - "spring.cloud.httpclient.ok.enabled: true", "ribbon.eureka.enabled = false", "ribbon.okhttp.enabled: true"}) + "spring.cloud.httpclientfactories.ok.enabled: true", "ribbon.eureka.enabled = false", "ribbon.okhttp.enabled: true"}) @DirtiesContext public class OkHttpClientConfigurationTests { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java index 08d70848..a685c3a2 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java @@ -58,7 +58,7 @@ import lombok.NoArgsConstructor; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = FeignHttpClientUrlTests.TestConfig.class, webEnvironment = WebEnvironment.DEFINED_PORT, value = { "spring.application.name=feignclienturltest", "feign.hystrix.enabled=false", - "spring.cloud.httpclient.ok.enabled=false" }) + "spring.cloud.httpclientfactories.ok.enabled=false" }) @DirtiesContext public class FeignHttpClientUrlTests { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignOkHttpTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignOkHttpTests.java index ffd7c6ca..04ddb12e 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignOkHttpTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignOkHttpTests.java @@ -63,7 +63,7 @@ import lombok.NoArgsConstructor; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = FeignOkHttpTests.Application.class, webEnvironment = WebEnvironment.RANDOM_PORT, value = { "spring.application.name=feignclienttest", "feign.hystrix.enabled=false", - "feign.okhttp.enabled=true", "spring.cloud.httpclient.ok.enabled=true" }) + "feign.okhttp.enabled=true", "spring.cloud.httpclientfactories.ok.enabled=true" }) @DirtiesContext public class FeignOkHttpTests { From 56ccc46f5f971278bdaa72bb5af8c52753055bfc Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Sun, 9 Jul 2017 15:26:24 -0400 Subject: [PATCH 6/7] Added documentation --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 99a1bc65..67fa5779 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -11,6 +11,10 @@ include::intro.adoc[] +== HTTP Clients + + + == Service Discovery: Eureka Clients Service Discovery is one of the key tenets of a microservice based architecture. Trying to hand configure each client or some form of convention can be very difficult to do and can be very brittle. Eureka is the Netflix Service Discovery Server and Client. The server can be configured and deployed to be highly available, with each server replicating state about the registered services to the others. @@ -1012,6 +1016,7 @@ Spring Cloud Netflix provides the following beans by default for feign (`BeanTyp * `Client` feignClient: if Ribbon is enabled it is a `LoadBalancerFeignClient`, otherwise the default feign client is used. The OkHttpClient and ApacheHttpClient feign clients can be used by setting `feign.okhttp.enabled` or `feign.httpclient.enabled` to `true`, respectively, and having them on the classpath. +You can customize the HTTP client used by providing a bean of either `ClosableHttpClient` when using Apache or `OkHttpClient` whe using OK HTTP. Spring Cloud Netflix _does not_ provide the following beans by default for feign, but still looks up beans of these types from the application context to create the feign client: @@ -1536,7 +1541,9 @@ path rendering the `users` path unreachable. The default HTTP client used by zuul is now backed by the Apache HTTP Client instead of the deprecated Ribbon `RestClient`. To use `RestClient` or to use the `okhttp3.OkHttpClient` set -`ribbon.restclient.enabled=true` or `ribbon.okhttp.enabled=true` respectively. +`ribbon.restclient.enabled=true` or `ribbon.okhttp.enabled=true` respectively. If you would +like to customize the Apache HTTP client or the OK HTTP client provide a bean of type +`ClosableHttpClient` or `OkHttpClient`. === Cookies and Sensitive Headers From 78ba3c5394facd9c6952e48b17536d147cf744da Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 11 Jul 2017 14:49:44 -0400 Subject: [PATCH 7/7] Cleanup and code review fixes. --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 14 +++++++++++--- .../test/java/OkHttpClientConfigurationTests.java | 9 +++++---- .../ApacheHttpClientConfigurationTests.java | 10 +++++----- .../netflix/feign/FeignHttpClientUrlTests.java | 2 +- .../zuul/filters/CustomHostRoutingFilterTests.java | 13 ------------- .../okhttp/OkHttpRibbonRetryIntegrationTests.java | 2 +- 6 files changed, 23 insertions(+), 27 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 67fa5779..6501d291 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -11,9 +11,6 @@ include::intro.adoc[] -== HTTP Clients - - == Service Discovery: Eureka Clients @@ -2511,3 +2508,14 @@ method to determine whether you want to retry a request given the status code. You can turn off Zuul's retry functionality by setting `zuul.retryable` to `false`. You can also disable retry functionality on route by route basis by setting `zuul.routes.routename.retryable` to `false`. + +== HTTP Clients + +Spring Cloud Netflix will automatically create the HTTP client used by Ribbon, Feign, and +Zuul for you. However you can also provide your own HTTP clients customized how you please +yourself. To do this you can either create a bean of type `ClosableHttpClient` if you +are using the Apache Http Cient, or `OkHttpClient` if you are using OK HTTP. + +NOTE: When you create your own HTTP client you are also responsible for implementing +the correct connection management strategies for these clients. Doing this improperly +can result in resource management issues. diff --git a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java index 503ebf89..752bacf5 100644 --- a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509TrustManager; +import org.assertj.core.api.Assertions; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.MockingDetails; @@ -79,10 +80,10 @@ public class OkHttpClientConfigurationTests { @Test public void testFactories() { - assertTrue(OkHttpClientConnectionPoolFactory.class.isInstance(connectionPoolFactory)); - assertTrue(OkHttpClientConfigurationTestApp.MyOkHttpClientConnectionPoolFactory.class.isInstance(connectionPoolFactory)); - assertTrue(OkHttpClientFactory.class.isInstance(okHttpClientFactory)); - assertTrue(OkHttpClientConfigurationTestApp.MyOkHttpClientFactory.class.isInstance(okHttpClientFactory)); + Assertions.assertThat(connectionPoolFactory).isInstanceOf(OkHttpClientConnectionPoolFactory.class); + Assertions.assertThat(connectionPoolFactory).isInstanceOf(OkHttpClientConfigurationTestApp.MyOkHttpClientConnectionPoolFactory.class); + Assertions.assertThat(okHttpClientFactory).isInstanceOf(OkHttpClientFactory.class); + Assertions.assertThat(okHttpClientFactory).isInstanceOf(OkHttpClientConfigurationTestApp.MyOkHttpClientFactory.class); } @Test diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java index 8f65f4c3..dc27ce47 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java @@ -28,7 +28,6 @@ import java.util.concurrent.TimeUnit; import org.apache.http.Header; import org.apache.http.StatusLine; import org.apache.http.client.HttpClient; -import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.config.RegistryBuilder; @@ -37,6 +36,7 @@ import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.message.BasicHeader; +import org.assertj.core.api.Assertions; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.MockingDetails; @@ -98,10 +98,10 @@ public class ApacheHttpClientConfigurationTests { @Test public void testFactories() { - assertTrue(ApacheHttpClientConnectionManagerFactory.class.isInstance(connectionManagerFactory)); - assertTrue(ApacheHttpClientConfigurationTestApp.MyApacheHttpClientConnectionManagerFactory.class.isInstance(connectionManagerFactory)); - assertTrue(ApacheHttpClientFactory.class.isInstance(httpClientFactory)); - assertTrue(ApacheHttpClientConfigurationTestApp.MyApacheHttpClientFactory.class.isInstance(httpClientFactory)); + Assertions.assertThat(connectionManagerFactory).isInstanceOf(ApacheHttpClientConnectionManagerFactory.class); + Assertions.assertThat(connectionManagerFactory).isInstanceOf(ApacheHttpClientConfigurationTestApp.MyApacheHttpClientConnectionManagerFactory.class); + Assertions.assertThat(httpClientFactory).isInstanceOf(ApacheHttpClientFactory.class); + Assertions.assertThat(httpClientFactory).isInstanceOf(ApacheHttpClientConfigurationTestApp.MyApacheHttpClientFactory.class); } @Test diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java index a685c3a2..eabfd678 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/FeignHttpClientUrlTests.java @@ -58,7 +58,7 @@ import lombok.NoArgsConstructor; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = FeignHttpClientUrlTests.TestConfig.class, webEnvironment = WebEnvironment.DEFINED_PORT, value = { "spring.application.name=feignclienturltest", "feign.hystrix.enabled=false", - "spring.cloud.httpclientfactories.ok.enabled=false" }) + "feign.okhttp.enabled=false" }) @DirtiesContext public class FeignHttpClientUrlTests { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java index 2dad556c..413f64c1 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/CustomHostRoutingFilterTests.java @@ -254,19 +254,6 @@ class SampleCustomZuulProxyApplication { private class CustomApacheHttpClientFactory extends DefaultApacheHttpClientFactory { -//@Override -//public HttpClientBuilder createBuilder() { -// return HttpClients.custom() -// .setDefaultCookieStore(new BasicCookieStore()) -// .setDefaultRequestConfig(RequestConfig.custom() -// .setCookieSpec(CookieSpecs.DEFAULT).build()); -//}public CloseableHttpClient createClient(RequestConfig requestConfig, HttpClientConnectionManager connectionManager) { -// return HttpClients.custom().setConnectionManager(connectionManager) -// .setDefaultCookieStore(new BasicCookieStore()) -// .setDefaultRequestConfig(RequestConfig.custom() -// .setCookieSpec(CookieSpecs.DEFAULT).build()) -// .build(); -// } } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java index f23dc053..e8c39e3d 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/okhttp/OkHttpRibbonRetryIntegrationTests.java @@ -30,7 +30,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @RunWith(SpringJUnit4ClassRunner.class) @SpringBootTest(classes = RibbonRetryIntegrationTestBase.RetryableTestConfig.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT, value = { "zuul.retryable: false", /* Disable retry by default, have each route enable it */ - "ribbon.okhttp.enabled", + "ribbon.okhttp.enabled: true", "hystrix.command.default.execution.timeout.enabled: false", /* Disable hystrix so its timeout doesnt get in the way */ "ribbon.ReadTimeout: 1000", /* Make sure ribbon will timeout before the thread is done sleeping */ "zuul.routes.retryable: /retryable/**",