Fix the problem that LoadBalancerContext unnecessarily create new Server object. Code clean up.

build.gradle

@@ -81,12 +81,12 @@ project(':ribbon-transport') {
 }
 
 
-project(':ribbon-eureka') {   
+project(':ribbon-eureka') {
     dependencies {
         compile project(':ribbon-core')
         compile project(':ribbon-loadbalancer')
         compile 'com.netflix.eureka:eureka-client:1.1.136'
-    }    
+    }
 }
 
 project(':ribbon-examples') {
@@ -114,20 +114,6 @@ project(':ribbon-test') {
     }
 }
 
-project(':ribbon-guice') {
-    dependencies {
-        compile project(':ribbon')
-        compile project(':ribbon-core')
-        compile project(':ribbon-httpclient')
-        compile project(':ribbon-loadbalancer')
-
-        compile 'com.google.inject:guice:3.0'
-        compile 'com.google.inject.extensions:guice-multibindings:3.0'
-
-        testCompile project(':ribbon-examples')
-    }
-}
-
 project(':ribbon') {
     configurations {
         // add new configuration

ribbon-eureka/src/test/java/com/netflix/niws/loadbalancer/LBBuilderTest.java

 package com.netflix.niws.loadbalancer;
 
-import static org.easymock.EasyMock.expect;
-import static org.junit.Assert.*;
-import static org.powermock.api.easymock.PowerMock.createMock;
-import static org.powermock.api.easymock.PowerMock.replay;
-
-import java.util.ArrayList;
-import java.util.List;
-
-import org.apache.commons.configuration.Configuration;
-import org.easymock.EasyMock;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.powermock.api.easymock.PowerMock;
-import org.powermock.core.classloader.annotations.PowerMockIgnore;
-import org.powermock.core.classloader.annotations.PrepareForTest;
-import org.powermock.modules.junit4.PowerMockRunner;
-
 import com.google.common.collect.Lists;
 import com.netflix.appinfo.InstanceInfo;
 import com.netflix.client.config.DefaultClientConfigImpl;
@@ -39,6 +21,23 @@ import com.netflix.loadbalancer.ServerList;
 import com.netflix.loadbalancer.ServerListFilter;
 import com.netflix.loadbalancer.ZoneAffinityServerListFilter;
 import com.netflix.loadbalancer.ZoneAwareLoadBalancer;
+import org.apache.commons.configuration.Configuration;
+import org.easymock.EasyMock;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.powermock.api.easymock.PowerMock;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.easymock.EasyMock.expect;
+import static org.junit.Assert.*;
+import static org.powermock.api.easymock.PowerMock.createMock;
+import static org.powermock.api.easymock.PowerMock.replay;
 
 @RunWith(PowerMockRunner.class)
 @PrepareForTest( {DiscoveryManager.class, DiscoveryClient.class} )
@@ -105,6 +104,9 @@ public class LBBuilderTest {
         assertEquals(Lists.newArrayList(expected), lb.getServerList(false));
         assertSame(filter, lb.getFilter());
         assertSame(list, lb.getServerListImpl());
+        Server server = lb.chooseServer();
+        // make sure load balancer does not recreate the server instance
+        assertTrue(server instanceof DiscoveryEnabledServer);
     }
 
     @Test

ribbon-loadbalancer/src/main/java/com/netflix/loadbalancer/LoadBalancerContext.java

@@ -470,15 +470,12 @@ public class LoadBalancerContext implements IClientConfigAware {
                                     + clientName);
                 }
                 host = svc.getHost();
-                port = svc.getPort();
-
                 if (host == null){
                     throw new ClientException(ClientException.ErrorType.GENERAL,
                             "Invalid Server for :" + svc);
                 }
-                if (logger.isDebugEnabled()){
-                    logger.debug(clientName + " using LB returned Server:" + svc + "for request");
-                }
+                logger.debug("{} using LB returned Server: {} for request {}", clientName, svc, original);
+                return svc;
             } else {
                 // No Full URL - and we dont have a LoadBalancer registered to
                 // obtain a server
@@ -488,12 +485,11 @@ public class LoadBalancerContext implements IClientConfigAware {
                 if (vipAddresses != null && vipAddresses.contains(",")) {
                     throw new ClientException(
                             ClientException.ErrorType.GENERAL,
-                            this.clientName
-                            + "Partial URI of ("
+                            "Method is invoked for client " + clientName + " with partial URI of ("
                             + original
-                            + ") has been sent in to RestClient (with no LB) to be executed."
-                            + " Also, there are multiple vipAddresses and hence RestClient cant pick"
-                            + "one vipAddress to complete this partial uri");
+                            + ") with no load balancer configured."
+                            + " Also, there are multiple vipAddresses and hence no vip address can be chosen"
+                            + " to complete this partial uri");
                 } else if (vipAddresses != null) {
                     try {
                         Pair<String,Integer> hostAndPort = deriveHostAndPortFromVipAddress(vipAddresses);
@@ -502,13 +498,12 @@ public class LoadBalancerContext implements IClientConfigAware {
                     } catch (URISyntaxException e) {
                         throw new ClientException(
                                 ClientException.ErrorType.GENERAL,
-                                this.clientName
-                                + "Partial URI of ("
+                                "Method is invoked for client " + clientName + " with partial URI of ("
                                 + original
-                                + ") has been sent in to RestClient (with no LB) to be executed."
+                                + ") with no load balancer configured. "
                                 + " Also, the configured/registered vipAddress is unparseable (to determine host and port)");
                     }
-                }else{
+                } else {
                     throw new ClientException(
                             ClientException.ErrorType.GENERAL,
                             this.clientName
@@ -534,26 +529,20 @@ public class LoadBalancerContext implements IClientConfigAware {
                 Server svc = lb.chooseServer(loadBalancerKey);
                 if (svc != null){
                     host = svc.getHost();
-                    port = svc.getPort();
                     if (host == null){
                         throw new ClientException(ClientException.ErrorType.GENERAL,
                                 "Invalid Server for :" + svc);
                     }
-                    if (logger.isDebugEnabled()){
-                        logger.debug("using LB returned Server:" + svc + "for request:" + original);
-                    }
-                }else{
+                    logger.debug("using LB returned Server: {} for request: {}", svc, original);
+                    return svc;
+                } else {
                     // just fall back as real DNS
-                    if (logger.isDebugEnabled()){
-                        logger.debug(host + ":" + port + " assumed to be a valid VIP address or exists in the DNS");
-                    }
+                    logger.debug("{}:{} assumed to be a valid VIP address or exists in the DNS", host, port);
                 }
             } else {
                 // consult LB to obtain vipAddress backed instance given full URL
                 //Full URL execute request - where url!=vipAddress
-                if (logger.isDebugEnabled()){
-                    logger.debug("Using full URL passed in by caller (not using LB/Discovery):" + original);
-                }
+                logger.debug("Using full URL passed in by caller (not using load balancer): {}", original);
             }
         }
         // end of creating final URL