From 4bd7c3bdcf95186b9cdb49f13cbc5369a98d6062 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Thu, 5 Mar 2015 11:42:09 -0700 Subject: [PATCH] upgrade feign to 7.3.0 fixes gh-243 --- pom.xml | 2 +- .../feign/ribbon/FeignRibbonClient.java | 115 ------------------ .../FeignRibbonClientAutoConfiguration.java | 18 ++- .../feign/ribbon/SpringLBClientFactory.java | 42 +++++++ .../feign/ribbon/FeignRibbonClientTests.java | 37 +++--- .../ribbon/RibbonLoadBalancerClientTests.java | 15 ++- 6 files changed, 95 insertions(+), 134 deletions(-) delete mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClient.java create mode 100644 spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/SpringLBClientFactory.java diff --git a/pom.xml b/pom.xml index 347559a0..43115a51 100644 --- a/pom.xml +++ b/pom.xml @@ -23,7 +23,7 @@ ${basedir} 0.6.5 1.1.147 - 7.1.0 + 7.3.0 1.4.0 2.0-RC13 1.0.28 diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClient.java deleted file mode 100644 index bc1a348b..00000000 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClient.java +++ /dev/null @@ -1,115 +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 java.io.IOException; -import java.net.URI; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.SSLSocketFactory; - -import lombok.Data; - -import org.springframework.cloud.netflix.feign.ribbon.RibbonLoadBalancer.RibbonRequest; -import org.springframework.cloud.netflix.ribbon.SpringClientFactory; -import org.springframework.util.ReflectionUtils; - -import com.netflix.client.ClientException; -import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.ILoadBalancer; - -import dagger.Lazy; -import feign.Client; -import feign.Request; -import feign.Response; - -/** - * @author Julien Roy - * @author Spencer Gibb - */ -public class FeignRibbonClient implements Client { - - private Client defaultClient = new Default(new LazySSLSocketFactory(), - new LazyHostnameVerifier()); - - private SpringClientFactory factory; - - public FeignRibbonClient(SpringClientFactory factory) { - this.factory = factory; - } - - @Override - public Response execute(Request request, Request.Options options) throws IOException { - try { - URI asUri = URI.create(request.url()); - String clientName = asUri.getHost(); - URI uriWithoutSchemeAndPort = URI.create(request.url().replace( - asUri.getScheme() + "://" + asUri.getHost(), - asUri.getScheme() + "://")); - RibbonLoadBalancer.RibbonRequest ribbonRequest = new RibbonRequest(request, - uriWithoutSchemeAndPort); - LBClient client = getClient(clientName); - return client.getLoadBalancer() - .executeWithLoadBalancer(ribbonRequest, client.config).toResponse(); - } - catch (ClientException ex) { - if (ex.getCause() instanceof IOException) { - throw IOException.class.cast(ex.getCause()); - } - ReflectionUtils.rethrowRuntimeException(ex); - return null; - } - } - - @Data - private class LBClient { - private final IClientConfig config; - private final RibbonLoadBalancer loadBalancer; - } - - private LBClient getClient(String clientName) { - IClientConfig config = this.factory.getClientConfig(clientName); - ILoadBalancer lb = this.factory.getLoadBalancer(clientName); - RibbonLoadBalancer loadBalancer = new RibbonLoadBalancer(this.defaultClient, lb, - config); - return new LBClient(config, loadBalancer); - } - - public void setDefaultClient(Client defaultClient) { - this.defaultClient = defaultClient; - } - - private static class LazySSLSocketFactory implements Lazy { - - @Override - public SSLSocketFactory get() { - return (SSLSocketFactory) SSLSocketFactory.getDefault(); - } - - } - - private static class LazyHostnameVerifier implements Lazy { - - @Override - public HostnameVerifier get() { - return HttpsURLConnection.getDefaultHostnameVerifier(); - } - - } - -} 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 455d43a4..e70623ee 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,8 +16,12 @@ package org.springframework.cloud.netflix.feign.ribbon; +import feign.ribbon.LBClientFactory; +import feign.ribbon.RibbonClient; +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.cloud.netflix.feign.FeignAutoConfiguration; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import org.springframework.context.annotation.Bean; @@ -38,8 +42,18 @@ import feign.Feign; @Configuration @AutoConfigureBefore(FeignAutoConfiguration.class) public class FeignRibbonClientAutoConfiguration { + + @Autowired + private SpringClientFactory factory; + @Bean - public Client feignRibbonClient(SpringClientFactory factory) { - return new FeignRibbonClient(factory); + public LBClientFactory lbClientFactory() { + return new SpringLBClientFactory(factory); + } + + @Bean + @ConditionalOnMissingBean + public Client feignRibbonClient() { + return RibbonClient.builder().lbClientFactory(lbClientFactory()).build(); } } \ No newline at end of file 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 new file mode 100644 index 00000000..27f3f75d --- /dev/null +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/SpringLBClientFactory.java @@ -0,0 +1,42 @@ +/* + * 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/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 87a50177..cac76a3f 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,19 @@ package org.springframework.cloud.netflix.feign.ribbon; +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; + +import com.netflix.loadbalancer.*; +import feign.ribbon.RibbonClient; import org.hamcrest.CustomMatcher; import org.junit.Before; import org.junit.Test; -import org.mockito.Matchers; -import org.mockito.Mockito; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; -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 com.netflix.loadbalancer.ILoadBalancer; -import com.netflix.loadbalancer.Server; import feign.Client; import feign.Request; @@ -37,11 +37,12 @@ import feign.RequestTemplate; /** * @author Dave Syer + * @author Spencer Gibb */ public class FeignRibbonClientTests { - private ILoadBalancer loadBalancer = Mockito.mock(ILoadBalancer.class); - private Client delegate = Mockito.mock(Client.class); + private AbstractLoadBalancer loadBalancer = mock(AbstractLoadBalancer.class); + private Client delegate = mock(Client.class); private SpringClientFactory factory = new SpringClientFactory() { @Override @@ -58,13 +59,21 @@ public class FeignRibbonClientTests { } }; - private FeignRibbonClient client = new FeignRibbonClient(this.factory); + // 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(); @Before public void init() { - ReflectionTestUtils.setField(this.client, "defaultClient", this.delegate); - Mockito.when(this.loadBalancer.chooseServer(Matchers.any())).thenReturn( + when(this.loadBalancer.chooseServer(any())).thenReturn( new Server("foo.com", 8000)); + //to fix NPE + LoadBalancerStats stats = mock(LoadBalancerStats.class); + when(this.loadBalancer.getLoadBalancerStats()).thenReturn(stats); + when(stats.getSingleServerStat(any(Server.class))).thenReturn(mock(ServerStats.class)); } @Test @@ -73,8 +82,8 @@ public class FeignRibbonClientTests { .request(); this.client.execute(request, new Options()); RequestMatcher matcher = new RequestMatcher("http://foo.com:8000/"); - Mockito.verify(this.delegate).execute(Matchers.argThat(matcher), - Matchers.any(Options.class)); + verify(this.delegate).execute(argThat(matcher), + any(Options.class)); } @Test @@ -83,8 +92,8 @@ public class FeignRibbonClientTests { .request(); this.client.execute(request, new Options()); RequestMatcher matcher = new RequestMatcher("https://foo.com:8000/"); - Mockito.verify(this.delegate).execute(Matchers.argThat(matcher), - Matchers.any(Options.class)); + verify(this.delegate).execute(argThat(matcher), + any(Options.class)); } private final static class RequestMatcher extends CustomMatcher { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java index 96304641..aff17a40 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.ribbon; import java.net.URI; import java.net.URL; +import lombok.SneakyThrows; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -66,11 +67,21 @@ public class RibbonLoadBalancerClientTests { } @Test - public void reconstructURI() throws Exception { + public void reconstructURI() { + testReconstructURI("http"); + } + + @Test + public void reconstructSecureURI() throws Exception { + testReconstructURI("https"); + } + + @SneakyThrows + private void testReconstructURI(String scheme) { RibbonServer server = getRibbonServer(); RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server); ServiceInstance serviceInstance = client.choose(server.getServiceId()); - URI uri = client.reconstructURI(serviceInstance, new URL("http://" + URI uri = client.reconstructURI(serviceInstance, new URL(scheme +"://" + server.getServiceId()).toURI()); assertEquals(server.getHost(), uri.getHost()); assertEquals(server.getPort(), uri.getPort());