From 4f6f8d4ecd9fe6ecb9ef0abe9eb96bb02aacaf6e Mon Sep 17 00:00:00 2001 From: Brendan Nolan Date: Thu, 19 Feb 2015 20:54:45 +0000 Subject: [PATCH] Adds LBClientFactory to enable caching of Ribbon LBClients Before, LBClients were created for each request, which led to issues such as #182. Moreover, a user could not avoid using Ribbon's static factories. Adding LBClientFactory allows users to control how Ribbon resources are created. --- CHANGELOG.md | 1 + .../src/main/java/feign/ribbon/LBClient.java | 24 ++++-- .../java/feign/ribbon/LBClientFactory.java | 22 +++++ .../feign/ribbon/LoadBalancingTarget.java | 2 +- .../main/java/feign/ribbon/RibbonClient.java | 81 ++++++++++++++++--- .../main/java/feign/ribbon/RibbonModule.java | 9 ++- .../feign/ribbon/LBClientFactoryTest.java | 18 +++++ .../java/feign/ribbon/RibbonClientTest.java | 8 +- 8 files changed, 138 insertions(+), 27 deletions(-) create mode 100644 ribbon/src/main/java/feign/ribbon/LBClientFactory.java create mode 100644 ribbon/src/test/java/feign/ribbon/LBClientFactoryTest.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 82de32f7..eb062ce6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ### Version 7.3 * Adds Request.Options support to RibbonClient +* Adds LBClientFactory to enable caching of Ribbon LBClients * Updates to Ribbon 2.0-RC13 * Updates to Jackson 2.5.1 * Supports query parameters without values diff --git a/ribbon/src/main/java/feign/ribbon/LBClient.java b/ribbon/src/main/java/feign/ribbon/LBClient.java index c3a4ca0d..0d5a7b98 100644 --- a/ribbon/src/main/java/feign/ribbon/LBClient.java +++ b/ribbon/src/main/java/feign/ribbon/LBClient.java @@ -35,19 +35,21 @@ import feign.Request; import feign.RequestTemplate; import feign.Response; -class LBClient - extends AbstractLoadBalancerAwareClient { +public final class LBClient extends + AbstractLoadBalancerAwareClient { - private final Client delegate; private final int connectTimeout; private final int readTimeout; private final IClientConfig clientConfig; - LBClient(Client delegate, ILoadBalancer lb, IClientConfig clientConfig) { + public static LBClient create(ILoadBalancer lb, IClientConfig clientConfig) { + return new LBClient(lb, clientConfig); + } + + LBClient(ILoadBalancer lb, IClientConfig clientConfig) { super(lb, clientConfig); this.setRetryHandler(RetryHandler.DEFAULT); this.clientConfig = clientConfig; - this.delegate = delegate; connectTimeout = clientConfig.get(CommonClientConfigKey.ConnectTimeout); readTimeout = clientConfig.get(CommonClientConfigKey.ReadTimeout); } @@ -64,7 +66,7 @@ class LBClient } else { options = new Request.Options(connectTimeout, readTimeout); } - Response response = delegate.execute(request.toRequest(), options); + Response response = request.client().execute(request.toRequest(), options); return new RibbonResponse(request.getUri(), response); } @@ -84,8 +86,10 @@ class LBClient 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); } @@ -99,8 +103,12 @@ class LBClient .request(); } + Client client() { + return client; + } + public Object clone() { - return new RibbonRequest(request, getUri()); + return new RibbonRequest(client, request, getUri()); } } diff --git a/ribbon/src/main/java/feign/ribbon/LBClientFactory.java b/ribbon/src/main/java/feign/ribbon/LBClientFactory.java new file mode 100644 index 00000000..30bd8c98 --- /dev/null +++ b/ribbon/src/main/java/feign/ribbon/LBClientFactory.java @@ -0,0 +1,22 @@ +package feign.ribbon; + +import com.netflix.client.ClientFactory; +import com.netflix.client.config.IClientConfig; +import com.netflix.loadbalancer.ILoadBalancer; + +public interface LBClientFactory { + + LBClient create(String clientName); + + /** + * Uses {@link ClientFactory} static factories from ribbon to create an LBClient. + */ + public static final class Default implements LBClientFactory { + @Override + public LBClient create(String clientName) { + IClientConfig config = ClientFactory.getNamedConfig(clientName); + ILoadBalancer lb = ClientFactory.getNamedLoadBalancer(clientName); + return LBClient.create(lb, config); + } + } +} diff --git a/ribbon/src/main/java/feign/ribbon/LoadBalancingTarget.java b/ribbon/src/main/java/feign/ribbon/LoadBalancingTarget.java index 56275475..e217afe4 100644 --- a/ribbon/src/main/java/feign/ribbon/LoadBalancingTarget.java +++ b/ribbon/src/main/java/feign/ribbon/LoadBalancingTarget.java @@ -103,7 +103,7 @@ public class LoadBalancingTarget implements Target { @Override public boolean equals(Object obj) { if (obj instanceof LoadBalancingTarget) { - LoadBalancingTarget other = (LoadBalancingTarget) obj; + LoadBalancingTarget other = (LoadBalancingTarget) obj; return type.equals(other.type) && name.equals(other.name); } diff --git a/ribbon/src/main/java/feign/ribbon/RibbonClient.java b/ribbon/src/main/java/feign/ribbon/RibbonClient.java index 409a40e7..1fc4f43e 100644 --- a/ribbon/src/main/java/feign/ribbon/RibbonClient.java +++ b/ribbon/src/main/java/feign/ribbon/RibbonClient.java @@ -1,11 +1,8 @@ package feign.ribbon; import com.netflix.client.ClientException; -import com.netflix.client.ClientFactory; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.DefaultClientConfigImpl; -import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.ILoadBalancer; import java.io.IOException; import java.net.URI; @@ -20,21 +17,37 @@ import feign.Request; import feign.Response; /** - * RibbonClient can be used in Fiegn builder to activate smart routing and resiliency capabilities + * RibbonClient can be used in Feign builder to activate smart routing and resiliency capabilities * provided by Ribbon. Ex. + * * 
- * MyService api = Feign.builder.client(new RibbonClient()).target(MyService.class,
- * "http://myAppProd");
+ * MyService api = Feign.builder.client(RibbonClient.create()).target(MyService.class,
+ *     "http://myAppProd");
  *
+ * * Where {@code myAppProd} is the ribbon client name and {@code myAppProd.ribbon.listOfServers} * configuration is set. */ public class RibbonClient implements Client { private final Client delegate; + private final LBClientFactory lbClientFactory; + + public static RibbonClient create() { + return builder().build(); + } + + public static Builder builder() { + return new Builder(); + } + + /** + * @deprecated Use the {@link RibbonClient#create()} + */ + @Deprecated public RibbonClient() { - this.delegate = new Client.Default( + this(new Client.Default( new Lazy() { public SSLSocketFactory get() { return (SSLSocketFactory) SSLSocketFactory.getDefault(); @@ -45,11 +58,20 @@ public class RibbonClient implements Client { return HttpsURLConnection.getDefaultHostnameVerifier(); } } - ); + )); } + /** + * @deprecated Use the {@link RibbonClient#create()} + */ + @Deprecated public RibbonClient(Client delegate) { + this(delegate, new LBClientFactory.Default()); + } + + RibbonClient(Client delegate, LBClientFactory lbClientFactory) { this.delegate = delegate; + this.lbClientFactory = lbClientFactory; } @Override @@ -58,7 +80,8 @@ public class RibbonClient implements Client { URI asUri = URI.create(request.url()); String clientName = asUri.getHost(); URI uriWithoutHost = URI.create(request.url().replace(asUri.getHost(), "")); - LBClient.RibbonRequest ribbonRequest = new LBClient.RibbonRequest(request, uriWithoutHost); + LBClient.RibbonRequest ribbonRequest = + new LBClient.RibbonRequest(delegate, request, uriWithoutHost); return lbClient(clientName).executeWithLoadBalancer(ribbonRequest, new FeignOptionsClientConfig(options)).toResponse(); } catch (ClientException e) { @@ -70,9 +93,7 @@ public class RibbonClient implements Client { } private LBClient lbClient(String clientName) { - IClientConfig config = ClientFactory.getNamedConfig(clientName); - ILoadBalancer lb = ClientFactory.getNamedLoadBalancer(clientName); - return new LBClient(delegate, lb, config); + return lbClientFactory.create(clientName); } static class FeignOptionsClientConfig extends DefaultClientConfigImpl { @@ -94,4 +115,40 @@ public class RibbonClient implements Client { } + public static final class Builder { + + Builder() { + } + + private Client delegate; + private LBClientFactory lbClientFactory; + + public Builder delegate(Client delegate) { + this.delegate = delegate; + return this; + } + + public Builder lbClientFactory(LBClientFactory lbClientFactory) { + this.lbClientFactory = lbClientFactory; + return this; + } + + public RibbonClient build() { + return new RibbonClient( + delegate != null ? delegate : new Client.Default( + new Lazy() { + public SSLSocketFactory get() { + return (SSLSocketFactory) SSLSocketFactory.getDefault(); + } + }, + new Lazy() { + public HostnameVerifier get() { + return HttpsURLConnection.getDefaultHostnameVerifier(); + } + } + ), + lbClientFactory != null ? lbClientFactory : new LBClientFactory.Default() + ); + } + } } diff --git a/ribbon/src/main/java/feign/ribbon/RibbonModule.java b/ribbon/src/main/java/feign/ribbon/RibbonModule.java index d95b6c52..7ce02ae4 100644 --- a/ribbon/src/main/java/feign/ribbon/RibbonModule.java +++ b/ribbon/src/main/java/feign/ribbon/RibbonModule.java @@ -36,6 +36,11 @@ import feign.Client; @dagger.Module(overrides = true, library = true, complete = false) public class RibbonModule { + @Provides + LBClientFactory lbClientFactory() { + return new LBClientFactory.Default(); + } + @Provides @Named("delegate") Client delegate(Client.Default delegate) { @@ -44,7 +49,7 @@ public class RibbonModule { @Provides @Singleton - Client httpClient(@Named("delegate") Client client) { - return new RibbonClient(client); + Client httpClient(@Named("delegate") Client client, LBClientFactory lbClientFactory) { + return new RibbonClient(client, lbClientFactory); } } diff --git a/ribbon/src/test/java/feign/ribbon/LBClientFactoryTest.java b/ribbon/src/test/java/feign/ribbon/LBClientFactoryTest.java new file mode 100644 index 00000000..3eccf50a --- /dev/null +++ b/ribbon/src/test/java/feign/ribbon/LBClientFactoryTest.java @@ -0,0 +1,18 @@ +package feign.ribbon; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +import com.netflix.client.ClientFactory; + +public class LBClientFactoryTest { + + @Test + public void testCreateLBClient() { + LBClientFactory.Default lbClientFactory = new LBClientFactory.Default(); + LBClient client = lbClientFactory.create("clientName"); + assertEquals("clientName", client.getClientName()); + assertEquals(ClientFactory.getNamedLoadBalancer("clientName"), client.getLoadBalancer()); + } +} diff --git a/ribbon/src/test/java/feign/ribbon/RibbonClientTest.java b/ribbon/src/test/java/feign/ribbon/RibbonClientTest.java index 09d11756..9825a3de 100644 --- a/ribbon/src/test/java/feign/ribbon/RibbonClientTest.java +++ b/ribbon/src/test/java/feign/ribbon/RibbonClientTest.java @@ -156,7 +156,7 @@ public class RibbonClientTest { getConfigInstance().setProperty(serverListKey(), hostAndPort(server1.getUrl(""))); TestInterface api = - Feign.builder().client(new RibbonClient(trustSSLSockets)) + Feign.builder().client(RibbonClient.builder().delegate(trustSSLSockets).build()) .target(TestInterface.class, "https://" + client()); api.post(); assertEquals(1, server1.getRequestCount()); @@ -170,9 +170,9 @@ public class RibbonClientTest { getConfigInstance().setProperty(serverListKey(), hostAndPort(server1.getUrl(""))); - TestInterface api = Feign.builder(). - client(new RibbonClient()). - target(TestInterface.class, "http://" + client()); + TestInterface api = + Feign.builder().client(RibbonClient.create()) + .target(TestInterface.class, "http://" + client()); api.post();