upgrade eureka client, copy instance info when doing port override, test changes

build.gradle

@@ -64,7 +64,7 @@ project(':ribbon-httpclient') {
 project(':ribbon-eureka') {   
     dependencies {
         compile project(':ribbon-core')
-        compile 'com.netflix.eureka:eureka-client:1.1.107'
+        compile 'com.netflix.eureka:eureka-client:1.1.126'
     }    
 }
 

ribbon-eureka/src/main/java/com/netflix/niws/loadbalancer/DiscoveryEnabledNIWSServerList.java

@@ -127,10 +127,15 @@ public class DiscoveryEnabledNIWSServerList extends AbstractServerList<Discovery
                                 logger.debug("Overriding port on client name: " + clientName + " to " + overridePort);
                             }
 
+                            // copy is necessary since the InstanceInfo builder just uses the original reference,
+                            // and we don't want to corrupt the global eureka copy of the object which may be
+                            // used by other clients in our system
+                            InstanceInfo copy = new InstanceInfo(ii);
+
                             if(isSecure){
-                                ii = new InstanceInfo.Builder(ii).setSecurePort(overridePort).build();
+                                ii = new InstanceInfo.Builder(copy).setSecurePort(overridePort).build();
                             }else{
-                                ii = new InstanceInfo.Builder(ii).setPort(overridePort).build();
+                                ii = new InstanceInfo.Builder(copy).setPort(overridePort).build();
                             }
                         }
 

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

@@ -68,6 +68,10 @@ public class DiscoveryEnabledLoadBalancerSupportsPortOverrideTest {
     @Before
     public void setupMock(){
 
+        List<InstanceInfo> dummyII = getDummyInstanceInfo("dummy", "http://www.host.com", 8001);
+        List<InstanceInfo> secureDummyII = getDummyInstanceInfo("secureDummy", "http://www.host.com", 8002);
+
+
         PowerMock.mockStatic(DiscoveryManager.class);
         PowerMock.mockStatic(DiscoveryClient.class);
 
@@ -79,8 +83,8 @@ public class DiscoveryEnabledLoadBalancerSupportsPortOverrideTest {
         expect(mockedDiscoveryManager.getDiscoveryClient()).andReturn(mockedDiscoveryClient).anyTimes();
 
 
-        expect(mockedDiscoveryClient.getInstancesByVipAddress("dummy", false, "region")).andReturn(getDummyInstanceInfo("dummy", "http://www.host.com", 8001)).anyTimes();
-        expect(mockedDiscoveryClient.getInstancesByVipAddress("secureDummy", true, "region")).andReturn(getDummyInstanceInfo("secureDummy", "http://www.host.com", 8002)).anyTimes();
+        expect(mockedDiscoveryClient.getInstancesByVipAddress("dummy", false, "region")).andReturn(dummyII).anyTimes();
+        expect(mockedDiscoveryClient.getInstancesByVipAddress("secureDummy", true, "region")).andReturn(secureDummyII).anyTimes();
 
         replay(DiscoveryManager.class);
         replay(DiscoveryClient.class);
@@ -183,8 +187,6 @@ public class DiscoveryEnabledLoadBalancerSupportsPortOverrideTest {
 
         ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testSecureVipPortCanBeOverriden.ribbon.ForceClientPortConfiguration", "true");
 
-
-
         DiscoveryEnabledNIWSServerList deList = new DiscoveryEnabledNIWSServerList();
 
         DefaultClientConfigImpl clientConfig = DefaultClientConfigImpl.class.newInstance();
@@ -200,19 +202,85 @@ public class DiscoveryEnabledLoadBalancerSupportsPortOverrideTest {
     }
 
 
-        protected static List<InstanceInfo> getDummyInstanceInfo(String appName, String host, int port){
+    /**
+     * Tests case where two different clients want to use the same instance, one with overriden ports and one without
+     *
+     * @throws Exception for anything unexpected
+     */
+    @Test
+    public void testTwoInstancesDontStepOnEachOther() throws Exception{
+
+        // setup override client
+
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther1.ribbon.DeploymentContextBasedVipAddresses", "dummy");
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther1.ribbon.IsSecure", "false");
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther1.ribbon.Port", "6001");  // override from 8001
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther1.ribbon.TargetRegion", "region");
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther1.ribbon.NIWSServerListClassName", DiscoveryEnabledNIWSServerList.class.getName());
+
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther1.ribbon.ForceClientPortConfiguration", "true");
+
+        // setup non override client
+
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther2.ribbon.DeploymentContextBasedVipAddresses", "dummy");
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther2.ribbon.IsSecure", "false");
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther2.ribbon.Port", "6001");
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther2.ribbon.TargetRegion", "region");
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther2.ribbon.NIWSServerListClassName", DiscoveryEnabledNIWSServerList.class.getName());
+
+        ConfigurationManager.getConfigInstance().setProperty("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther2.ribbon.ForceClientPortConfiguration", "false");
+
+        // check override client
+
+        DiscoveryEnabledNIWSServerList deList1 = new DiscoveryEnabledNIWSServerList();
+
+        DefaultClientConfigImpl clientConfig1 = DefaultClientConfigImpl.class.newInstance();
+        clientConfig1.loadProperties("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther1");
+        deList1.initWithNiwsConfig(clientConfig1);
+
+        List<DiscoveryEnabledServer> serverList1 = deList1.getInitialListOfServers();
+
+        Assert.assertEquals(1, serverList1.size());
+        Assert.assertEquals(6001, serverList1.get(0).getPort());                           // client property overridden
+        Assert.assertEquals(6001, serverList1.get(0).getInstanceInfo().getPort());         // client property overridden
+        Assert.assertEquals(7002, serverList1.get(0).getInstanceInfo().getSecurePort());   // 7002 is the secure default
 
-            List<InstanceInfo> list = new ArrayList<InstanceInfo>();
+        // check non-override client
 
-            InstanceInfo info = InstanceInfo.Builder.newBuilder().setAppName(appName)
-                    .setHostName(host)
-                    .setPort(port)
-                    .build();
+        DiscoveryEnabledNIWSServerList deList2 = new DiscoveryEnabledNIWSServerList();
 
-            list.add(info);
+        DefaultClientConfigImpl clientConfig2 = DefaultClientConfigImpl.class.newInstance();
+        clientConfig2.loadProperties("DiscoveryEnabled.testTwoInstancesDontStepOnEachOther2");
+        deList2.initWithNiwsConfig(clientConfig2);
 
-            return list;
+        List<DiscoveryEnabledServer> serverList2 = deList2.getInitialListOfServers();
 
-        }
+        Assert.assertEquals(1, serverList2.size());
+
+        Assert.assertEquals(8001, serverList2.get(0).getPort());                           // client property indicated in ii
+        Assert.assertEquals(8001, serverList2.get(0).getInstanceInfo().getPort());         // client property indicated in ii
+        Assert.assertEquals(7002, serverList2.get(0).getInstanceInfo().getSecurePort());   // 7002 is the secure default
+
+
+
+
+    }
+
+
+
+    protected static List<InstanceInfo> getDummyInstanceInfo(String appName, String host, int port){
+
+        List<InstanceInfo> list = new ArrayList<InstanceInfo>();
+
+        InstanceInfo info = InstanceInfo.Builder.newBuilder().setAppName(appName)
+                .setHostName(host)
+                .setPort(port)
+                .build();
+
+        list.add(info);
+
+        return list;
+
+    }
 
 }