Skip to content

Commit

Permalink
Add 'hint' parameter to overloaded methods in RibbonLoadBalancerClient (
Browse files Browse the repository at this point in the history
#3012)

Fixes gh-3009
  • Loading branch information
andersenleo authored and spencergibb committed Sep 5, 2018
1 parent 0f7c643 commit 3ef01f7
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 5 deletions.
Expand Up @@ -73,7 +73,14 @@ public URI reconstructURI(ServiceInstance instance, URI original) {

@Override
public ServiceInstance choose(String serviceId) {
Server server = getServer(serviceId);
return choose(serviceId, null);
}

/**
* New: Select a server using a 'key'.
*/
public ServiceInstance choose(String serviceId, Object hint) {
Server server = getServer(getLoadBalancer(serviceId), hint);
if (server == null) {
return null;
}
Expand All @@ -83,8 +90,17 @@ public ServiceInstance choose(String serviceId) {

@Override
public <T> T execute(String serviceId, LoadBalancerRequest<T> request) throws IOException {
return execute(serviceId, request, null);
}

/**
* New: Execute a request by selecting server using a 'key'.
* The hint will have to be the last parameter to not mess with the `execute(serviceId, ServiceInstance, request)`
* method. This somewhat breaks the fluent coding style when using a lambda to define the LoadBalancerRequest.
*/
public <T> T execute(String serviceId, LoadBalancerRequest<T> request, Object hint) throws IOException {
ILoadBalancer loadBalancer = getLoadBalancer(serviceId);
Server server = getServer(loadBalancer);
Server server = getServer(loadBalancer, hint);
if (server == null) {
throw new IllegalStateException("No instances available for " + serviceId);
}
Expand Down Expand Up @@ -140,15 +156,23 @@ private boolean isSecure(Server server, String serviceId) {
return RibbonUtils.isSecure(config, serverIntrospector, server);
}

/**
* Note: This method could be removed?
*/
protected Server getServer(String serviceId) {
return getServer(getLoadBalancer(serviceId));
return getServer(getLoadBalancer(serviceId), null);
}

protected Server getServer(ILoadBalancer loadBalancer) {
return getServer(loadBalancer, null);
}

protected Server getServer(ILoadBalancer loadBalancer, Object hint) {
if (loadBalancer == null) {
return null;
}
return loadBalancer.chooseServer("default"); // TODO: better handling of key
// Use 'default' on a null hint, or just pass it on?
return loadBalancer.chooseServer(hint != null ? hint : "default");
}

protected ILoadBalancer getLoadBalancer(String serviceId) {
Expand All @@ -162,7 +186,7 @@ public static class RibbonServer implements ServiceInstance {
private Map<String, String> metadata;

public RibbonServer(String serviceId, Server server) {
this(serviceId, server, false, Collections.<String, String> emptyMap());
this(serviceId, server, false, Collections.emptyMap());
}

public RibbonServer(String serviceId, Server server, boolean secure,
Expand Down
Expand Up @@ -43,6 +43,7 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.same;
import static org.mockito.BDDMockito.given;
import static org.mockito.Matchers.anyDouble;
import static org.mockito.Matchers.anyObject;
Expand Down Expand Up @@ -205,6 +206,17 @@ public void testChoose() {
RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server);
ServiceInstance serviceInstance = client.choose(server.getServiceId());
assertServiceInstance(server, serviceInstance);
verify(this.loadBalancer).chooseServer(eq("default"));
}

@Test
public void testChooseWithHint() {
Object hint = new Object();
RibbonServer server = getRibbonServer();
RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server);
ServiceInstance serviceInstance = client.choose(server.getServiceId(), hint);
assertServiceInstance(server, serviceInstance);
verify(this.loadBalancer).chooseServer(same(hint));
}

@Test
Expand All @@ -228,6 +240,23 @@ public void testExecute() throws IOException {
return returnVal;
});
verifyServerStats();
verify(this.loadBalancer).chooseServer(eq("default"));
assertEquals("retVal was wrong", returnVal, actualReturn);
}

@Test
public void testExecuteWithHint() throws IOException {
Object hint = new Object();
final RibbonServer server = getRibbonServer();
RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server);
final String returnVal = "myval";
Object actualReturn = client.execute(server.getServiceId(),
(LoadBalancerRequest<Object>) instance -> {
assertServiceInstance(server, instance);
return returnVal;
}, hint);
verifyServerStats();
verify(this.loadBalancer).chooseServer(same(hint));
assertEquals("retVal was wrong", returnVal, actualReturn);
}

Expand Down

0 comments on commit 3ef01f7

Please sign in to comment.