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 ad9b4f98..8563e83a 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 @@ -29,29 +29,27 @@ import com.netflix.client.AbstractLoadBalancerAwareClient; import com.netflix.client.RequestSpecificRetryHandler; 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.ILoadBalancer; -import lombok.Setter; - /** * @author Christian Lohmann */ public class RibbonLoadBalancingHttpClient - extends AbstractLoadBalancerAwareClient { + extends + AbstractLoadBalancerAwareClient { private final HttpClient delegate = HttpClientBuilder.create().build(); - @Setter private int connectTimeout; - @Setter private int readTimeout; - @Setter private boolean secure; - @Setter - private IClientConfig clientConfig; + private boolean followRedirects; + + private boolean okToRetryOnAllOperations; public RibbonLoadBalancingHttpClient() { super(null); @@ -63,10 +61,29 @@ public class RibbonLoadBalancingHttpClient this.setRetryHandler(RetryHandler.DEFAULT); } + @Override + public void initWithNiwsConfig(IClientConfig clientConfig) { + super.initWithNiwsConfig(clientConfig); + this.connectTimeout = clientConfig.getPropertyAsInteger( + CommonClientConfigKey.ConnectTimeout, + DefaultClientConfigImpl.DEFAULT_CONNECT_TIMEOUT); + this.readTimeout = clientConfig.getPropertyAsInteger( + CommonClientConfigKey.ReadTimeout, + DefaultClientConfigImpl.DEFAULT_READ_TIMEOUT); + this.secure = clientConfig.getPropertyAsBoolean(CommonClientConfigKey.IsSecure, + false); + this.followRedirects = clientConfig.getPropertyAsBoolean( + CommonClientConfigKey.FollowRedirects, + DefaultClientConfigImpl.DEFAULT_FOLLOW_REDIRECTS); + this.okToRetryOnAllOperations = clientConfig.getPropertyAsBoolean( + CommonClientConfigKey.OkToRetryOnAllOperations, + DefaultClientConfigImpl.DEFAULT_OK_TO_RETRY_ON_ALL_OPERATIONS); + } + @Override public RequestSpecificRetryHandler getRequestSpecificRetryHandler( final RibbonApacheHttpRequest request, final IClientConfig requestConfig) { - if (this.clientConfig.get(CommonClientConfigKey.OkToRetryOnAllOperations, false)) { + if (this.okToRetryOnAllOperations) { return new RequestSpecificRetryHandler(true, true, this.getRetryHandler(), requestConfig); } @@ -90,10 +107,13 @@ public class RibbonLoadBalancingHttpClient CommonClientConfigKey.ConnectTimeout, this.connectTimeout)); builder.setConnectionRequestTimeout(configOverride.get( CommonClientConfigKey.ReadTimeout, this.readTimeout)); + builder.setRedirectsEnabled(configOverride.get( + CommonClientConfigKey.FollowRedirects, this.followRedirects)); } else { builder.setConnectTimeout(this.connectTimeout); builder.setConnectionRequestTimeout(this.readTimeout); + builder.setRedirectsEnabled(this.followRedirects); } final RequestConfig requestConfig = builder.build(); @@ -110,6 +130,7 @@ public class RibbonLoadBalancingHttpClient } private boolean isSecure(final IClientConfig config) { - return (config != null) ? config.get(CommonClientConfigKey.IsSecure) : secure; + return (config != null) ? config.get(CommonClientConfigKey.IsSecure) + : this.secure; } } diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java index 59db82c9..dcef3e82 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/apache/HttpClientRibbonCommandFactory.java @@ -16,13 +16,13 @@ package org.springframework.cloud.netflix.zuul.filters.route.apache; +import lombok.RequiredArgsConstructor; + import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.cloud.netflix.ribbon.apache.RibbonLoadBalancingHttpClient; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandContext; import org.springframework.cloud.netflix.zuul.filters.route.RibbonCommandFactory; -import lombok.RequiredArgsConstructor; - /** * @author Christian Lohmann */ @@ -35,10 +35,9 @@ public class HttpClientRibbonCommandFactory implements @Override public HttpClientRibbonCommand create(final RibbonCommandContext context) { final String serviceId = context.getServiceId(); - final RibbonLoadBalancingHttpClient client = clientFactory.getClient(serviceId, - RibbonLoadBalancingHttpClient.class); + final RibbonLoadBalancingHttpClient client = this.clientFactory.getClient( + serviceId, RibbonLoadBalancingHttpClient.class); client.setLoadBalancer(this.clientFactory.getLoadBalancer(serviceId)); - client.setClientConfig(this.clientFactory.getClientConfig(serviceId)); final HttpClientRibbonCommand httpClientRibbonCommand = new HttpClientRibbonCommand( serviceId, client, context.getVerb(), context.getUri(), 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 new file mode 100644 index 00000000..b3c4d247 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java @@ -0,0 +1,144 @@ +/* + * Copyright 2013-2015 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.ribbon.apache; + +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.junit.Test; +import org.mockito.ArgumentCaptor; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.util.ReflectionTestUtils; + +import com.netflix.client.config.CommonClientConfigKey; +import com.netflix.client.config.DefaultClientConfigImpl; +import com.netflix.client.config.IClientConfig; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +/** + * @author Sébastien Nussbaumer + */ +public class RibbonLoadBalancingHttpClientTests { + + @Test + public void testRequestConfigUseDefaultsNoOverride() throws Exception { + RequestConfig result = getBuiltRequestConfig(UseDefaults.class, null); + + assertThat(result.isRedirectsEnabled(), is(false)); + } + + @Test + public void testRequestConfigDoNotFollowRedirectsNoOverride() throws Exception { + RequestConfig result = getBuiltRequestConfig(DoNotFollowRedirects.class, null); + + assertThat(result.isRedirectsEnabled(), is(false)); + } + + @Test + public void testRequestConfigFollowRedirectsNoOverride() throws Exception { + RequestConfig result = getBuiltRequestConfig(FollowRedirects.class, null); + + assertThat(result.isRedirectsEnabled(), is(true)); + } + + @Test + public void testRequestConfigDoNotFollowRedirectsOverrideWithFollowRedirects() + throws Exception { + + DefaultClientConfigImpl override = new DefaultClientConfigImpl(); + override.set(CommonClientConfigKey.FollowRedirects, true); + override.set(CommonClientConfigKey.IsSecure, false); + + RequestConfig result = getBuiltRequestConfig(DoNotFollowRedirects.class, override); + + assertThat(result.isRedirectsEnabled(), is(true)); + } + + @Test + public void testRequestConfigFollowRedirectsOverrideWithDoNotFollowRedirects() + throws Exception { + + DefaultClientConfigImpl override = new DefaultClientConfigImpl(); + override.set(CommonClientConfigKey.FollowRedirects, false); + override.set(CommonClientConfigKey.IsSecure, false); + + RequestConfig result = getBuiltRequestConfig(FollowRedirects.class, override); + + assertThat(result.isRedirectsEnabled(), is(false)); + } + + @Configuration + protected static class UseDefaults { + + } + + @Configuration + protected static class FollowRedirects { + @Bean + public IClientConfig clientConfig() { + DefaultClientConfigImpl config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.FollowRedirects, true); + return config; + } + } + + @Configuration + protected static class DoNotFollowRedirects { + @Bean + public IClientConfig clientConfig() { + DefaultClientConfigImpl config = new DefaultClientConfigImpl(); + config.set(CommonClientConfigKey.FollowRedirects, false); + return config; + } + } + + private RequestConfig getBuiltRequestConfig(Class defaultConfigurationClass, + IClientConfig configOverride) throws Exception { + + SpringClientFactory factory = new SpringClientFactory(); + factory.setApplicationContext(new AnnotationConfigApplicationContext( + defaultConfigurationClass)); + HttpClient delegate = mock(HttpClient.class); + RibbonLoadBalancingHttpClient client = factory.getClient("service", + RibbonLoadBalancingHttpClient.class); + + ReflectionTestUtils.setField(client, "delegate", delegate); + given(delegate.execute(any(HttpUriRequest.class))).willReturn( + mock(HttpResponse.class)); + RibbonApacheHttpRequest request = mock(RibbonApacheHttpRequest.class); + given(request.toRequest(any(RequestConfig.class))).willReturn( + mock(HttpUriRequest.class)); + + client.execute(request, configOverride); + + ArgumentCaptor requestConfigCaptor = ArgumentCaptor + .forClass(RequestConfig.class); + verify(request).toRequest(requestConfigCaptor.capture()); + return requestConfigCaptor.getValue(); + } + +}