Merge pull request #402 from elandau/bugfix/load_balancer_status

Fix LBStats default config loading

gradle.properties

@@ -2,7 +2,7 @@ rx_java_version=1.0.9
 rx_netty_version=0.4.9
 servo_version=0.10.1
 hystrix_version=1.4.3
-guava_version=14.0.1
+guava_version=19.0
 archaius_version=0.7.6
 eureka_version=1.7.2
 jersey_version=1.19.1

ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java

@@ -32,6 +32,7 @@ import org.slf4j.LoggerFactory;
 
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Properties;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
@@ -917,7 +918,7 @@ public class DefaultClientConfigImpl implements IClientConfig {
     }
 
     private <T> Property<T> toProperty(IClientConfigKey<T> key, PropertyWrapper<T> propertyWrapper) {
-        return new Property() {
+        return new Property<T>() {
             @Override
             public void onChange(Consumer consumer) {
                 Runnable callback = new Runnable() {
@@ -955,9 +956,14 @@ public class DefaultClientConfigImpl implements IClientConfig {
             }
 
             @Override
-            public Object get() {
+            public T get() {
                 return propertyWrapper.getValue();
             }
+
+            @Override
+            public Optional<T> getOptional() {
+                return Optional.ofNullable(propertyWrapper.getDynamicProperty().getCachedValue(key.type()).orNull());
+            }
         };
     }
 

ribbon-core/src/main/java/com/netflix/client/config/FallbackProperty.java

+package com.netflix.client.config;
+
+import java.util.function.Consumer;
+
+public final class FallbackProperty<T> implements Property<T> {
+    private final Property<T> primary;
+    private final Property<T> fallback;
+
+    public FallbackProperty(Property<T> primary, Property<T> fallback) {
+        this.primary = primary;
+        this.fallback = fallback;
+    }
+
+    @Override
+    public void onChange(Consumer<T> consumer) {
+        primary.onChange(ignore -> consumer.accept(get()));
+        fallback.onChange(ignore -> consumer.accept(get()));
+    }
+
+    @Override
+    public T get() {
+        return primary.getOptional().orElseGet(fallback::get);
+    }
+}

ribbon-core/src/main/java/com/netflix/client/config/Property.java

 package com.netflix.client.config;
 
+import java.util.Optional;
 import java.util.function.Consumer;
 
 /**
@@ -18,6 +19,12 @@ public interface Property<T> {
      */
     T get();
 
+    default Optional<T> getOptional() { return Optional.ofNullable(get()); }
+
+    default Property<T> fallbackWith(Property<T> fallback) {
+        return new FallbackProperty<>(this, fallback);
+    }
+
     static <T> Property<T> of(T value) {
         return new Property<T>() {
             @Override

ribbon-core/src/test/java/com/netflix/client/config/ClientConfigTest.java

@@ -19,13 +19,17 @@ package com.netflix.client.config;
 
 import static org.junit.Assert.*;
 
-import com.netflix.client.config.DefaultClientConfigImpl;
 import com.netflix.config.ConfigurationManager;
 
 import org.junit.AfterClass;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.BeforeClass;
+import org.junit.FixMethodOrder;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TestName;
+import org.junit.runners.MethodSorters;
 
 import java.util.Properties;
 
@@ -35,9 +39,21 @@ import java.util.Properties;
  * @author stonse
  * 
  */
+@FixMethodOrder(MethodSorters.NAME_ASCENDING)
 public class ClientConfigTest {
-    @BeforeClass
-    public static void setUp() throws Exception {
+    @Rule
+    public TestName testName = new TestName();
+
+    IClientConfigKey<Integer> INTEGER_PROPERTY;
+
+    IClientConfigKey<Integer> DEFAULT_INTEGER_PROPERTY;
+
+    @Before
+    public void setUp() throws Exception {
+        INTEGER_PROPERTY = new CommonClientConfigKey<Integer>(
+                "niws.loadbalancer.%s." + testName.getMethodName(), 10) {};
+        DEFAULT_INTEGER_PROPERTY = new CommonClientConfigKey<Integer>(
+                "niws.loadbalancer.default." + testName.getMethodName(), 30) {};
     }
 
     @AfterClass
@@ -113,5 +129,36 @@ public class ClientConfigTest {
         ConfigurationManager.getConfigInstance().clearProperty("testRestClient2.ribbon.EnableZoneAffinity");
         assertNull(clientConfig.getProperty(CommonClientConfigKey.EnableZoneAffinity));
     }
+
+    @Test
+    public void testFallback_noneSet() {
+        DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
+        Property<Integer> prop = clientConfig.getGlobalProperty(INTEGER_PROPERTY.format(testName.getMethodName()))
+                .fallbackWith(clientConfig.getGlobalProperty(DEFAULT_INTEGER_PROPERTY));
+
+        Assert.assertEquals(30, prop.get().intValue());
+    }
+
+    @Test
+    public void testFallback_fallbackSet() {
+        ConfigurationManager.getConfigInstance().setProperty(DEFAULT_INTEGER_PROPERTY.key(), "100");
+        DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
+        Property<Integer> prop = clientConfig.getGlobalProperty(INTEGER_PROPERTY.format(testName.getMethodName()))
+                .fallbackWith(clientConfig.getGlobalProperty(DEFAULT_INTEGER_PROPERTY));
+
+        Assert.assertEquals(100, prop.get().intValue());
+    }
+
+    @Test
+    public void testFallback_primarySet() {
+
+        ConfigurationManager.getConfigInstance().setProperty(INTEGER_PROPERTY.format(testName.getMethodName()).key(), "200");
+
+        DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
+        Property<Integer> prop = clientConfig.getGlobalProperty(INTEGER_PROPERTY.format(testName.getMethodName()))
+                .fallbackWith(clientConfig.getGlobalProperty(DEFAULT_INTEGER_PROPERTY));
+
+        Assert.assertEquals(200, prop.get().intValue());
+    }
 }
 

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

@@ -43,7 +43,7 @@ import static org.powermock.api.easymock.PowerMock.replay;
 @RunWith(PowerMockRunner.class)
 @PrepareForTest( {DiscoveryManager.class, DiscoveryClient.class} )
 @PowerMockIgnore({"javax.management.*", "com.sun.jersey.*", "com.sun.*", "org.apache.*", "weblogic.*", "com.netflix.config.*", "com.sun.jndi.dns.*",
-    "javax.naming.*", "com.netflix.logging.*", "javax.ws.*"})
+    "javax.naming.*", "com.netflix.logging.*", "javax.ws.*", "com.google.*"})
 public class LBBuilderTest {
     
     static Server expected = new Server("www.example.com", 8001);

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

@@ -70,6 +70,15 @@ public class LoadBalancerStats implements IClientConfigAware {
     public static final IClientConfigKey<Integer> CIRCUIT_TRIP_MAX_TIMEOUT_SECONDS = new CommonClientConfigKey<Integer>(
             "niws.loadbalancer.%s.circuitTripMaxTimeoutSeconds", 30) {};
 
+    public static final IClientConfigKey<Integer> DEFAULT_CONNECTION_FAILURE_COUNT_THRESHOLD = new CommonClientConfigKey<Integer>(
+            "niws.loadbalancer.default.connectionFailureCountThreshold", 3) {};
+
+    public static final IClientConfigKey<Integer> DEFAULT_CIRCUIT_TRIP_TIMEOUT_FACTOR_SECONDS = new CommonClientConfigKey<Integer>(
+            "niws.loadbalancer.default.circuitTripTimeoutFactorSeconds", 10) {};
+
+    public static final IClientConfigKey<Integer> DEFAULT_CIRCUIT_TRIP_MAX_TIMEOUT_SECONDS = new CommonClientConfigKey<Integer>(
+            "niws.loadbalancer.default.circuitTripMaxTimeoutSeconds", 30) {};
+
     private String name;
     
     volatile Map<String, ZoneStats> zoneStatsMap = new ConcurrentHashMap<>();
@@ -115,10 +124,20 @@ public class LoadBalancerStats implements IClientConfigAware {
     public void initWithNiwsConfig(IClientConfig clientConfig) {
         this.name = clientConfig.getClientName();
         Preconditions.checkArgument(name != null, "IClientConfig#getCLientName() must not be null");
-        this.connectionFailureThreshold = new UnboxedIntProperty(clientConfig.getGlobalProperty(CONNECTION_FAILURE_COUNT_THRESHOLD.format(name)));
-        this.circuitTrippedTimeoutFactor = new UnboxedIntProperty(clientConfig.getGlobalProperty(CIRCUIT_TRIP_TIMEOUT_FACTOR_SECONDS.format(name)));
-        this.maxCircuitTrippedTimeout = new UnboxedIntProperty(clientConfig.getGlobalProperty(CIRCUIT_TRIP_MAX_TIMEOUT_SECONDS.format(name)));
-        this.activeRequestsCountTimeout = new UnboxedIntProperty(clientConfig.getGlobalProperty(ACTIVE_REQUESTS_COUNT_TIMEOUT));
+        this.connectionFailureThreshold = new UnboxedIntProperty(
+                clientConfig.getGlobalProperty(CONNECTION_FAILURE_COUNT_THRESHOLD.format(name))
+                    .fallbackWith(clientConfig.getGlobalProperty(DEFAULT_CONNECTION_FAILURE_COUNT_THRESHOLD))
+        );
+        this.circuitTrippedTimeoutFactor = new UnboxedIntProperty(
+                clientConfig.getGlobalProperty(CIRCUIT_TRIP_TIMEOUT_FACTOR_SECONDS.format(name))
+                        .fallbackWith(clientConfig.getGlobalProperty(DEFAULT_CIRCUIT_TRIP_TIMEOUT_FACTOR_SECONDS))
+        );
+        this.maxCircuitTrippedTimeout = new UnboxedIntProperty(
+                clientConfig.getGlobalProperty(CIRCUIT_TRIP_MAX_TIMEOUT_SECONDS.format(name))
+                        .fallbackWith(clientConfig.getGlobalProperty(DEFAULT_CIRCUIT_TRIP_MAX_TIMEOUT_SECONDS))
+        );
+        this.activeRequestsCountTimeout = new UnboxedIntProperty(
+                clientConfig.getGlobalProperty(ACTIVE_REQUESTS_COUNT_TIMEOUT));
     }
 
 

ribbon/src/test/java/com/netflix/ribbon/DiscoveryEnabledServerListTest.java

@@ -28,6 +28,7 @@ import io.netty.buffer.ByteBuf;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
 
 import java.io.IOException;
 import java.nio.charset.Charset;
@@ -39,6 +40,7 @@ import static org.junit.Assert.assertEquals;
 /**
  * Created by awang on 7/15/14.
  */
+@PowerMockIgnore("com.google.*")
 public class DiscoveryEnabledServerListTest extends MockedDiscoveryServerListTest {
 
     static MockWebServer server;