diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingLBClientFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingLBClientFactory.java deleted file mode 100644 index 604434ad..00000000 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingLBClientFactory.java +++ /dev/null @@ -1,47 +0,0 @@ -/* - * 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.feign.ribbon; - -import feign.ribbon.LBClient; -import feign.ribbon.LBClientFactory; -import org.springframework.util.ConcurrentReferenceHashMap; - -import java.util.Map; - -/** - * LBClientFactory that caches entries created. - * @author Spencer Gibb - */ -public class CachingLBClientFactory implements LBClientFactory { - - private volatile Map cache = new ConcurrentReferenceHashMap<>(); - private final LBClientFactory delegate; - - public CachingLBClientFactory(LBClientFactory delegate) { - this.delegate = delegate; - } - - @Override - public LBClient create(String clientName) { - if (cache.containsKey(clientName)) { - return cache.get(clientName); - } - LBClient client = delegate.create(clientName); - cache.put(clientName, client); - return client; - } -} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java new file mode 100644 index 00000000..295f507a --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactory.java @@ -0,0 +1,56 @@ +/* + * 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.feign.ribbon; + +import java.util.Map; + +import org.springframework.cloud.netflix.ribbon.ServerIntrospector; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; +import org.springframework.util.ConcurrentReferenceHashMap; + +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.ILoadBalancer; + +/** + * Factory for SpringLoadBalancer instances that caches the entries created. + * + * @author Spencer Gibb + * @author Dave Syer + */ +public class CachingSpringLoadBalancerFactory { + + private final SpringClientFactory factory; + + private volatile Map cache = new ConcurrentReferenceHashMap<>(); + + public CachingSpringLoadBalancerFactory(SpringClientFactory factory) { + this.factory = factory; + } + + public FeignLoadBalancer create(String clientName) { + if (this.cache.containsKey(clientName)) { + return this.cache.get(clientName); + } + IClientConfig config = this.factory.getClientConfig(clientName); + ILoadBalancer lb = this.factory.getLoadBalancer(clientName); + ServerIntrospector serverIntrospector = this.factory.getInstance(clientName, ServerIntrospector.class); + FeignLoadBalancer client = new FeignLoadBalancer(lb, config, serverIntrospector); + this.cache.put(clientName, client); + return client; + } + +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancer.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java similarity index 74% rename from spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancer.java rename to spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java index 2e73a2b4..24b7fb23 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancer.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignLoadBalancer.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 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. @@ -21,6 +21,7 @@ import java.net.URI; import java.util.Collection; import java.util.Map; +import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.web.util.UriComponentsBuilder; import com.netflix.client.AbstractLoadBalancerAwareClient; @@ -32,35 +33,29 @@ 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 feign.Client; import feign.Request; import feign.RequestTemplate; import feign.Response; -public class RibbonLoadBalancer - extends - AbstractLoadBalancerAwareClient { - - private final Client delegate; +public class FeignLoadBalancer extends + AbstractLoadBalancerAwareClient { private final int connectTimeout; - private final int readTimeout; - private final IClientConfig clientConfig; + private final ServerIntrospector serverIntrospector; - private final boolean secure; - - public RibbonLoadBalancer(Client delegate, ILoadBalancer lb, - IClientConfig clientConfig) { + public FeignLoadBalancer(ILoadBalancer lb, IClientConfig clientConfig, + ServerIntrospector serverIntrospector) { super(lb, clientConfig); this.setRetryHandler(RetryHandler.DEFAULT); this.clientConfig = clientConfig; - this.secure = clientConfig.get(CommonClientConfigKey.IsSecure); - this.delegate = delegate; this.connectTimeout = clientConfig.get(CommonClientConfigKey.ConnectTimeout); this.readTimeout = clientConfig.get(CommonClientConfigKey.ReadTimeout); + this.serverIntrospector = serverIntrospector; } @Override @@ -68,31 +63,24 @@ public class RibbonLoadBalancer throws IOException { Request.Options options; if (configOverride != null) { - options = new Request.Options(configOverride.get( - CommonClientConfigKey.ConnectTimeout, this.connectTimeout), + options = new Request.Options( + configOverride.get(CommonClientConfigKey.ConnectTimeout, + this.connectTimeout), (configOverride.get(CommonClientConfigKey.ReadTimeout, this.readTimeout))); } else { options = new Request.Options(this.connectTimeout, this.readTimeout); } - if (isSecure(configOverride)) { - URI secureUri = UriComponentsBuilder.fromUri(request.getUri()) - .scheme("https").build().toUri(); - request = new RibbonRequest(request.toRequest(), secureUri); - } - Response response = this.delegate.execute(request.toRequest(), options); + Response response = request.client().execute(request.toRequest(), options); return new RibbonResponse(request.getUri(), response); } - private boolean isSecure(IClientConfig config) { - return (config != null) ? config.get(CommonClientConfigKey.IsSecure) : secure; - } - @Override public RequestSpecificRetryHandler getRequestSpecificRetryHandler( RibbonRequest request, IClientConfig requestConfig) { - if (this.clientConfig.get(CommonClientConfigKey.OkToRetryOnAllOperations, false)) { + if (this.clientConfig.get(CommonClientConfigKey.OkToRetryOnAllOperations, + false)) { return new RequestSpecificRetryHandler(true, true, this.getRetryHandler(), requestConfig); } @@ -106,11 +94,24 @@ public class RibbonLoadBalancer } } + @Override + public URI reconstructURIWithServer(Server server, URI original) { + String scheme = original.getScheme(); + if (!"https".equals(scheme) && (this.serverIntrospector.isSecure(server) + || this.clientConfig.get(CommonClientConfigKey.IsSecure, false))) { + original = UriComponentsBuilder.fromUri(original).scheme("https").build() + .toUri(); + } + return super.reconstructURIWithServer(server, original); + } + static class RibbonRequest extends ClientRequest implements Cloneable { private final Request request; + private final Client client; - RibbonRequest(Request request, URI uri) { + RibbonRequest(Client client, Request request, URI uri) { + this.client = client; this.request = request; setUri(uri); } @@ -121,9 +122,13 @@ public class RibbonLoadBalancer .body(this.request.body(), this.request.charset()).request(); } + Client client() { + return this.client; + } + @Override public Object clone() { - return new RibbonRequest(this.request, getUri()); + return new RibbonRequest(this.client, this.request, getUri()); } } @@ -175,4 +180,4 @@ public class RibbonLoadBalancer } -} +} \ No newline at end of file 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 252b1205..5f90b45f 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 @@ -16,9 +16,6 @@ package org.springframework.cloud.netflix.feign.ribbon; -import feign.httpclient.ApacheHttpClient; -import feign.ribbon.LBClientFactory; -import feign.ribbon.RibbonClient; import org.apache.http.client.HttpClient; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureBefore; @@ -29,12 +26,13 @@ import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Primary; import com.netflix.loadbalancer.ILoadBalancer; import feign.Client; import feign.Feign; -import org.springframework.context.annotation.Primary; +import feign.httpclient.ApacheHttpClient; /** * Autoconfiguration to be activated if Feign is in use and needs to be use Ribbon as a @@ -47,27 +45,19 @@ import org.springframework.context.annotation.Primary; @AutoConfigureBefore(FeignAutoConfiguration.class) public class FeignRibbonClientAutoConfiguration { - @Autowired - private SpringClientFactory factory; - - @Bean - public SpringLBClientFactory springLBClientFactory() { - return new SpringLBClientFactory(factory); - } - @Bean @Primary - public CachingLBClientFactory cachingLBClientFactory() { - return new CachingLBClientFactory(springLBClientFactory()); + public CachingSpringLoadBalancerFactory cachingLBClientFactory( + SpringClientFactory factory) { + return new CachingSpringLoadBalancerFactory(factory); } @Bean @ConditionalOnMissingBean - public Client feignClient() { - return RibbonClient.builder().lbClientFactory(cachingLBClientFactory()).build(); + public Client feignClient(CachingSpringLoadBalancerFactory cachingFactory) { + return new LoadBalancerFeignClient(new Client.Default(null, null), cachingFactory); } - @Configuration @ConditionalOnClass(ApacheHttpClient.class) @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) @@ -76,24 +66,19 @@ public class FeignRibbonClientAutoConfiguration { @Autowired(required = false) private HttpClient httpClient; - @Autowired(required = false) - private LBClientFactory lbClientFactory; + @Autowired + CachingSpringLoadBalancerFactory cachingFactory; @Bean public Client feignClient() { - RibbonClient.Builder builder = RibbonClient.builder(); - - if (httpClient != null) { - builder.delegate(new ApacheHttpClient(httpClient)); - } else { - builder.delegate(new ApacheHttpClient()); + ApacheHttpClient delegate; + if (this.httpClient != null) { + delegate = new ApacheHttpClient(this.httpClient); } - - if (lbClientFactory != null) { - builder.lbClientFactory(lbClientFactory); + else { + delegate = new ApacheHttpClient(); } - - return builder.build(); + return new LoadBalancerFeignClient(delegate, this.cachingFactory); } } } \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java new file mode 100644 index 00000000..7d233bff --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java @@ -0,0 +1,94 @@ +/* + * Copyright 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.feign.ribbon; + +import java.io.IOException; +import java.net.URI; + +import com.netflix.client.ClientException; +import com.netflix.client.config.CommonClientConfigKey; +import com.netflix.client.config.DefaultClientConfigImpl; + +import feign.Client; +import feign.Request; +import feign.Response; + +/** + * @author Dave Syer + * + */ +public class LoadBalancerFeignClient implements Client { + + private final Client delegate; + private CachingSpringLoadBalancerFactory lbClientFactory; + + public LoadBalancerFeignClient(Client delegate, CachingSpringLoadBalancerFactory lbClientFactory) { + this.delegate = delegate; + this.lbClientFactory = lbClientFactory; + } + + @Override + public Response execute(Request request, Request.Options options) throws IOException { + try { + URI asUri = URI.create(request.url()); + String clientName = asUri.getHost(); + URI uriWithoutHost = cleanUrl(request.url(), clientName); + FeignLoadBalancer.RibbonRequest ribbonRequest = new FeignLoadBalancer.RibbonRequest(this.delegate, + request, uriWithoutHost); + return lbClient(clientName).executeWithLoadBalancer(ribbonRequest, + new FeignOptionsClientConfig(options)).toResponse(); + } + catch (ClientException e) { + if (e.getCause() instanceof IOException) { + throw IOException.class.cast(e.getCause()); + } + throw new RuntimeException(e); + } + } + + public Client getDelegate() { + return this.delegate; + } + + static URI cleanUrl(String originalUrl, String host) { + return URI.create(originalUrl.replaceFirst(host, "")); + } + + private FeignLoadBalancer lbClient(String clientName) { + return this.lbClientFactory.create(clientName); + } + + static class FeignOptionsClientConfig extends DefaultClientConfigImpl { + + public FeignOptionsClientConfig(Request.Options options) { + setProperty(CommonClientConfigKey.ConnectTimeout, + options.connectTimeoutMillis()); + setProperty(CommonClientConfigKey.ReadTimeout, options.readTimeoutMillis()); + } + + @Override + public void loadProperties(String clientName) { + + } + + @Override + public void loadDefaultValues() { + + } + + } +} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/SpringLBClientFactory.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/SpringLBClientFactory.java deleted file mode 100644 index 27f3f75d..00000000 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/SpringLBClientFactory.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * 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.feign.ribbon; - -import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.ILoadBalancer; -import feign.ribbon.LBClient; -import feign.ribbon.LBClientFactory; -import org.springframework.cloud.netflix.ribbon.SpringClientFactory; - -/** - * @author Spencer Gibb - */ -public class SpringLBClientFactory implements LBClientFactory { - - private final SpringClientFactory factory; - - public SpringLBClientFactory(SpringClientFactory factory) { - this.factory = factory; - } - - @Override - public LBClient create(String clientName) { - IClientConfig config = factory.getClientConfig(clientName); - ILoadBalancer lb = factory.getLoadBalancer(clientName); - return LBClient.create(lb, config); - } -} diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java index 2b7f810e..29e8bcb7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonAutoConfiguration.java @@ -19,7 +19,6 @@ package org.springframework.cloud.netflix.ribbon; import java.util.ArrayList; import java.util.List; -import com.netflix.ribbon.Ribbon; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.AutoConfigureBefore; @@ -36,6 +35,7 @@ import org.springframework.web.client.RestTemplate; import com.netflix.client.IClient; import com.netflix.client.http.HttpRequest; +import com.netflix.ribbon.Ribbon; /** * Auto configuration for Ribbon (client side load balancing). @@ -93,8 +93,8 @@ public class RibbonAutoConfiguration { @Bean public RibbonClientHttpRequestFactory ribbonClientHttpRequestFactory() { - return new RibbonClientHttpRequestFactory(springClientFactory, - loadBalancerClient); + return new RibbonClientHttpRequestFactory(this.springClientFactory, + this.loadBalancerClient); } } 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 600c648b..b574cd9e 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 @@ -158,7 +158,7 @@ public class RibbonClientConfiguration { @Override public URI reconstructURIWithServer(Server server, URI original) { String scheme = original.getScheme(); - if (!"https".equals(scheme) && serverIntrospector.isSecure(server)) { + if (!"https".equals(scheme) && this.serverIntrospector.isSecure(server)) { original = UriComponentsBuilder.fromUri(original).scheme("https").build() .toUri(); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingLBClientFactoryTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java similarity index 62% rename from spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingLBClientFactoryTests.java rename to spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java index 7a2e7045..5594f0b4 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingLBClientFactoryTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/CachingSpringLoadBalancerFactoryTests.java @@ -16,28 +16,30 @@ package org.springframework.cloud.netflix.feign.ribbon; -import static org.junit.Assert.*; -import static org.mockito.Mockito.*; +import static org.junit.Assert.assertNotNull; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; -import com.netflix.client.config.CommonClientConfigKey; -import com.netflix.client.config.DefaultClientConfigImpl; -import com.netflix.client.config.IClientConfig; -import feign.ribbon.LBClient; -import feign.ribbon.LBClientFactory; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; + +import com.netflix.client.config.CommonClientConfigKey; +import com.netflix.client.config.DefaultClientConfigImpl; +import com.netflix.client.config.IClientConfig; /** * @author Spencer Gibb */ -public class CachingLBClientFactoryTests { +public class CachingSpringLoadBalancerFactoryTests { @Mock - private LBClientFactory delegate; + private SpringClientFactory delegate; - private CachingLBClientFactory factory; + private CachingSpringLoadBalancerFactory factory; @Before public void init() { @@ -47,32 +49,29 @@ public class CachingLBClientFactoryTests { config.set(CommonClientConfigKey.ConnectTimeout, 1000); config.set(CommonClientConfigKey.ReadTimeout, 500); - LBClient client1 = LBClient.create(null, config); - LBClient client2 = LBClient.create(null, config); + when(this.delegate.getClientConfig("client1")).thenReturn(config); + when(this.delegate.getClientConfig("client2")).thenReturn(config); - when(delegate.create("client1")).thenReturn(client1); - when(delegate.create("client2")).thenReturn(client2); - - factory = new CachingLBClientFactory(delegate); + this.factory = new CachingSpringLoadBalancerFactory(this.delegate); } @Test public void delegateCreatesWhenMissing() { - LBClient client = factory.create("client1"); + FeignLoadBalancer client = this.factory.create("client1"); assertNotNull("client was null", client); - verify(delegate, times(1)).create("client1"); + verify(this.delegate, times(1)).getClientConfig("client1"); } @Test public void cacheWorks() { - LBClient client = factory.create("client2"); + FeignLoadBalancer client = this.factory.create("client2"); assertNotNull("client was null", client); - client = factory.create("client2"); + client = this.factory.create("client2"); assertNotNull("client was null", client); - verify(delegate, times(1)).create("client2"); + verify(this.delegate, times(1)).getClientConfig("client2"); } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java index cac76a3f..714991b0 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java @@ -16,19 +16,27 @@ package org.springframework.cloud.netflix.feign.ribbon; -import static org.mockito.Matchers.*; -import static org.mockito.Mockito.*; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.argThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; -import com.netflix.loadbalancer.*; -import feign.ribbon.RibbonClient; import org.hamcrest.CustomMatcher; import org.junit.Before; import org.junit.Test; +import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; +import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.AbstractLoadBalancer; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.LoadBalancerStats; +import com.netflix.loadbalancer.Server; +import com.netflix.loadbalancer.ServerStats; import feign.Client; import feign.Request; @@ -53,6 +61,16 @@ public class FeignRibbonClientTests { return config; } + @Override + public C getInstance(String name, Class type) { + if (type.isAssignableFrom(ServerIntrospector.class)) { + @SuppressWarnings("unchecked") + C instance = (C) new DefaultServerIntrospector(); + return instance; + } + return null; + } + @Override public ILoadBalancer getLoadBalancer(String name) { return FeignRibbonClientTests.this.loadBalancer; @@ -61,10 +79,7 @@ public class FeignRibbonClientTests { // Even though we don't maintain FeignRibbonClient, keep these tests // around to make sure the expected behaviour doesn't break - private Client client = RibbonClient.builder() - .lbClientFactory(new SpringLBClientFactory(factory)) - .delegate(delegate) - .build(); + private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory)); @Before public void init() { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancerTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancerTests.java index d4b1dc28..734bc651 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancerTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/RibbonLoadBalancerTests.java @@ -13,30 +13,32 @@ import static org.junit.Assert.assertThat; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.net.URI; import java.util.Collection; import java.util.Collections; -import lombok.SneakyThrows; - import org.junit.Before; import org.junit.Test; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import org.springframework.cloud.netflix.feign.ribbon.RibbonLoadBalancer.RibbonRequest; -import org.springframework.cloud.netflix.feign.ribbon.RibbonLoadBalancer.RibbonResponse; +import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer.RibbonRequest; +import org.springframework.cloud.netflix.feign.ribbon.FeignLoadBalancer.RibbonResponse; +import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; +import org.springframework.cloud.netflix.ribbon.ServerIntrospector; import com.netflix.client.config.IClientConfig; import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.Server; import feign.Client; import feign.Request; import feign.Request.Options; import feign.RequestTemplate; import feign.Response; +import lombok.SneakyThrows; public class RibbonLoadBalancerTests { @@ -47,7 +49,9 @@ public class RibbonLoadBalancerTests { @Mock private IClientConfig config; - private RibbonLoadBalancer ribbonLoadBalancer; + private FeignLoadBalancer feignLoadBalancer; + + private ServerIntrospector inspector = new DefaultServerIntrospector(); private Integer defaultConnectTimeout = 10000; private Integer defaultReadTimeout = 10000; @@ -55,29 +59,32 @@ public class RibbonLoadBalancerTests { @Before public void setup() { MockitoAnnotations.initMocks(this); - when(config.get(MaxAutoRetries, DEFAULT_MAX_AUTO_RETRIES)).thenReturn(1); - when(config.get(MaxAutoRetriesNextServer, DEFAULT_MAX_AUTO_RETRIES_NEXT_SERVER)) - .thenReturn(1); - when(config.get(OkToRetryOnAllOperations, eq(anyBoolean()))).thenReturn(true); - when(config.get(ConnectTimeout)).thenReturn(defaultConnectTimeout); - when(config.get(ReadTimeout)).thenReturn(defaultReadTimeout); + when(this.config.get(MaxAutoRetries, DEFAULT_MAX_AUTO_RETRIES)).thenReturn(1); + when(this.config.get(MaxAutoRetriesNextServer, + DEFAULT_MAX_AUTO_RETRIES_NEXT_SERVER)).thenReturn(1); + when(this.config.get(OkToRetryOnAllOperations, eq(anyBoolean()))) + .thenReturn(true); + when(this.config.get(ConnectTimeout)).thenReturn(this.defaultConnectTimeout); + when(this.config.get(ReadTimeout)).thenReturn(this.defaultReadTimeout); } @Test @SneakyThrows public void testUriInsecure() { - when(config.get(IsSecure)).thenReturn(false); - ribbonLoadBalancer = new RibbonLoadBalancer(delegate, lb, config); + when(this.config.get(IsSecure)).thenReturn(false); + this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, + this.inspector); Request request = new RequestTemplate().method("GET").append("http://foo/") .request(); - RibbonRequest ribbonRequest = new RibbonRequest(request, new URI(request.url())); + RibbonRequest ribbonRequest = new RibbonRequest(this.delegate, request, + new URI(request.url())); Response response = Response.create(200, "Test", Collections.> emptyMap(), new byte[0]); - when(delegate.execute(any(Request.class), any(Options.class))).thenReturn( - response); + when(this.delegate.execute(any(Request.class), any(Options.class))) + .thenReturn(response); - RibbonResponse resp = ribbonLoadBalancer.execute(ribbonRequest, null); + RibbonResponse resp = this.feignLoadBalancer.execute(ribbonRequest, null); assertThat(resp.getRequestedURI(), is(new URI("http://foo/"))); } @@ -85,46 +92,25 @@ public class RibbonLoadBalancerTests { @Test @SneakyThrows public void testSecureUriFromClientConfig() { - when(config.get(IsSecure)).thenReturn(true); - ribbonLoadBalancer = new RibbonLoadBalancer(delegate, lb, config); - Request request = new RequestTemplate().method("GET").append("http://foo/") - .request(); - RibbonRequest ribbonRequest = new RibbonRequest(request, new URI(request.url())); - - Response response = Response.create(200, "Test", - Collections.> emptyMap(), new byte[0]); - when(delegate.execute(any(Request.class), any(Options.class))).thenReturn( - response); - - RibbonResponse resp = ribbonLoadBalancer.execute(ribbonRequest, null); - - assertThat(resp.getRequestedURI(), is(new URI("https://foo/"))); + when(this.config.get(IsSecure)).thenReturn(true); + this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, + this.inspector); + Server server = new Server("foo", 7777); + URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, + new URI("http://foo/")); + assertThat(uri, is(new URI("https://foo:7777/"))); } @Test @SneakyThrows public void testSecureUriFromClientConfigOverride() { - when(config.get(IsSecure)).thenReturn(true); - ribbonLoadBalancer = new RibbonLoadBalancer(delegate, lb, config); - Request request = new RequestTemplate().method("GET").append("http://foo/") - .request(); - RibbonRequest ribbonRequest = new RibbonRequest(request, new URI(request.url())); - - Response response = Response.create(200, "Test", - Collections.> emptyMap(), new byte[0]); - when(delegate.execute(any(Request.class), any(Options.class))).thenReturn( - response); - - IClientConfig override = mock(IClientConfig.class); - when(override.get(ConnectTimeout, defaultConnectTimeout)).thenReturn(5000); - when(override.get(ReadTimeout, defaultConnectTimeout)).thenReturn(5000); - /* - * Override secure value. - */ - when(override.get(IsSecure)).thenReturn(false); - - RibbonResponse resp = ribbonLoadBalancer.execute(ribbonRequest, override); - - assertThat(resp.getRequestedURI(), is(new URI("http://foo/"))); + this.feignLoadBalancer = new FeignLoadBalancer(this.lb, this.config, + this.inspector); + Server server = Mockito.mock(Server.class); + when(server.getPort()).thenReturn(443); + when(server.getHost()).thenReturn("foo"); + URI uri = this.feignLoadBalancer.reconstructURIWithServer(server, + new URI("http://bar/")); + assertThat(uri, is(new URI("https://foo:443/"))); } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java index 660bcc78..7e54e13c 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignClientTests.java @@ -23,17 +23,12 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import java.lang.reflect.Field; import java.lang.reflect.InvocationHandler; import java.lang.reflect.Proxy; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -44,13 +39,13 @@ import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.WebIntegrationTest; 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.RibbonClient; import org.springframework.cloud.netflix.ribbon.StaticServerList; 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.ReflectionUtils; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; @@ -63,6 +58,9 @@ import com.netflix.loadbalancer.ServerList; import feign.Client; import feign.RequestInterceptor; import feign.RequestTemplate; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; /** * @author Spencer Gibb @@ -221,17 +219,16 @@ public class FeignClientTests { @Test public void testFeignClientType() throws IllegalAccessException { - assertThat(this.feignClient, is(instanceOf(feign.ribbon.RibbonClient.class))); - Field field = ReflectionUtils.findField(feign.ribbon.RibbonClient.class, "delegate", Client.class); - ReflectionUtils.makeAccessible(field); - Client delegate = (Client) field.get(this.feignClient); + assertThat(this.feignClient, is(instanceOf(LoadBalancerFeignClient.class))); + LoadBalancerFeignClient client = (LoadBalancerFeignClient) this.feignClient; + Client delegate = client.getDelegate(); assertThat(delegate, is(instanceOf(feign.Client.Default.class))); } @Test public void testServiceId() { - assertNotNull("testClientServiceId was null", testClientServiceId); - final Hello hello = testClientServiceId.getHello(); + assertNotNull("testClientServiceId was null", this.testClientServiceId); + final Hello hello = this.testClientServiceId.getHello(); assertNotNull("The hello response was null", hello); assertEquals("first hello didn't match", new Hello("hello world 1"), hello); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java index 3fd2df76..1ee1f1b0 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/valid/FeignHttpClientTests.java @@ -24,12 +24,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; -import java.lang.reflect.Field; - -import lombok.AllArgsConstructor; -import lombok.Data; -import lombok.NoArgsConstructor; - import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; @@ -40,6 +34,7 @@ import org.springframework.boot.test.IntegrationTest; import org.springframework.boot.test.SpringApplicationConfiguration; 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.RibbonClient; import org.springframework.cloud.netflix.ribbon.StaticServerList; import org.springframework.context.annotation.Bean; @@ -48,7 +43,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.test.context.web.WebAppConfiguration; -import org.springframework.util.ReflectionUtils; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.RequestMethod; @@ -58,6 +52,9 @@ import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerList; import feign.Client; +import lombok.AllArgsConstructor; +import lombok.Data; +import lombok.NoArgsConstructor; /** * @author Spencer Gibb @@ -146,18 +143,16 @@ public class FeignHttpClientTests { @Test public void testFeignClientType() throws IllegalAccessException { - assertThat(this.feignClient, is(instanceOf(feign.ribbon.RibbonClient.class))); - Field field = ReflectionUtils.findField(feign.ribbon.RibbonClient.class, - "delegate", Client.class); - ReflectionUtils.makeAccessible(field); - Client delegate = (Client) field.get(this.feignClient); + assertThat(this.feignClient, is(instanceOf(LoadBalancerFeignClient.class))); + LoadBalancerFeignClient client = (LoadBalancerFeignClient) this.feignClient; + Client delegate = client.getDelegate(); assertThat(delegate, is(instanceOf(feign.httpclient.ApacheHttpClient.class))); } @Test public void testFeignInheritanceSupport() { - assertNotNull("UserClient was null", userClient); - final User user = userClient.getUser(1); + assertNotNull("UserClient was null", this.userClient); + final User user = this.userClient.getUser(1); assertNotNull("Returned user was null", user); assertEquals("Users were different", user, new User("John Smith")); }