Merge pull request #410 from elandau/feature/value_of_properties

config: custom types with valueOf
Netflix · May 9, 2019 · 39b4a41 · 39b4a41
2 parents 1cbcf28 + 7274de5
commit 39b4a41
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 10 deletions.
diff --git a/ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java b/ribbon-archaius/src/main/java/com/netflix/client/config/DefaultClientConfigImpl.java
@@ -21,6 +21,8 @@
 import com.netflix.config.ConfigurationManager;
 import org.apache.commons.configuration.event.ConfigurationEvent;
 import org.apache.commons.configuration.event.ConfigurationListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.Arrays;
 import java.util.Optional;
@@ -73,6 +75,7 @@ values are specified in this class as constants.
  *
  */
 public class DefaultClientConfigImpl extends AbstractReloadableClientConfig {
+    private static final Logger LOG = LoggerFactory.getLogger(DefaultClientConfigImpl.class);
 
     @Deprecated
     public static final Boolean DEFAULT_PRIORITIZE_VIP_ADDRESS_BASED_SERVERS = Boolean.TRUE;
@@ -616,11 +619,9 @@ public String getVersion(){
 
     @Override
     protected <T> Optional<T> loadProperty(String key, Class<T> type) {
-        if (String.class.equals(type)) {
-            return Optional.ofNullable(ConfigurationManager.getConfigInstance().getStringArray(key))
-                    .filter(ar -> ar.length > 0)
-                    .map(ar -> (T)Arrays.stream(ar).collect(Collectors.joining(",")));
-        } else if (Integer.class.equals(type)) {
+        LOG.debug("Loading property {}", key);
+
+        if (Integer.class.equals(type)) {
             return Optional.ofNullable((T) ConfigurationManager.getConfigInstance().getInteger(key, null));
         } else if (Boolean.class.equals(type)) {
             return Optional.ofNullable((T) ConfigurationManager.getConfigInstance().getBoolean(key, null));
@@ -632,9 +633,19 @@ protected <T> Optional<T> loadProperty(String key, Class<T> type) {
             return Optional.ofNullable((T) ConfigurationManager.getConfigInstance().getDouble(key, null));
         } else if (TimeUnit.class.equals(type)) {
             return Optional.ofNullable((T) TimeUnit.valueOf(ConfigurationManager.getConfigInstance().getString(key, null)));
+        } else {
+            return Optional.ofNullable(ConfigurationManager.getConfigInstance().getStringArray(key))
+                    .filter(ar -> ar.length > 0)
+                    .map(ar -> Arrays.stream(ar).collect(Collectors.joining(",")))
+                    .map(value -> {
+                        if (type.equals(String.class)) {
+                            return (T)value;
+                        } else {
+                            return resolveWithValueOf(type, value)
+                                .orElseThrow(() -> new IllegalArgumentException("Unable to convert value to desired type " + type));
+                        }
+                    });
         }
-
-        throw new IllegalArgumentException("Unable to convert value to desired type " + type);
     }
 
     public DefaultClientConfigImpl withProperty(IClientConfigKey key, Object value) {

diff --git a/ribbon-core/src/main/java/com/netflix/client/config/AbstractReloadableClientConfig.java b/ribbon-core/src/main/java/com/netflix/client/config/AbstractReloadableClientConfig.java
@@ -6,6 +6,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -250,15 +251,38 @@ private <T> Optional<T> resolveFinalProperty(IClientConfigKey<T> key) {
     /**
      * Returns the internal property to the desiredn type
      */
+    private static Map<Class<?>, Optional<Method>> valueOfMethods = new ConcurrentHashMap<>();
+
+    public static <T> Optional<T> resolveWithValueOf(Class<T> type, String value) {
+        return valueOfMethods.computeIfAbsent(type, ignore -> {
+            try {
+                return Optional.of(type.getDeclaredMethod("valueOf", String.class));
+            } catch (NoSuchMethodException e) {
+                return Optional.empty();
+            } catch (Exception e) {
+                LOG.warn("Unable to determine if type " + type + " has a valueOf() static method", e);
+                return Optional.empty();
+            }
+        }).map(method -> {
+            try {
+                return (T)method.invoke(null, value);
+            } catch (Exception e) {
+                throw new RuntimeException(e);
+            }
+        });
+    }
+
     protected <T> Optional<T> resolveDefaultProperty(IClientConfigKey<T> key) {
         return Optional.ofNullable(defaultProperties.get(key.key()))
                 .map(value -> {
-                    final Class type = key.type();
+                    final Class<T> type = key.type();
                     // Unfortunately there's some legacy code setting string values for typed keys.  Here are do our best to parse
                     // and store the typed value
                     if (!value.getClass().equals(type)) {
                         try {
-                            if (value.getClass().equals(String.class)) {
+                            if (type.equals(String.class)) {
+                                return (T) value.toString();
+                            } else if (value.getClass().equals(String.class)) {
                                 final String strValue = (String) value;
                                 if (Integer.class.equals(type)) {
                                     return (T) Integer.valueOf(strValue);
@@ -273,7 +297,8 @@ protected <T> Optional<T> resolveDefaultProperty(IClientConfigKey<T> key) {
                                 } else if (TimeUnit.class.equals(type)) {
                                     return (T) TimeUnit.valueOf(strValue);
                                 } else {
-                                    throw new IllegalArgumentException("Unsupported value type `" + type + "'");
+                                    return resolveWithValueOf(type, strValue)
+                                        .orElseThrow(() -> new IllegalArgumentException("Unsupported value type `" + type + "'"));
                                 }
                             } else {
                                 throw new IllegalArgumentException("Incompatible value type `" + value.getClass() + "` while expecting '" + type + "`");

diff --git a/ribbon-core/src/test/java/com/netflix/client/config/ClientConfigTest.java b/ribbon-core/src/test/java/com/netflix/client/config/ClientConfigTest.java
@@ -154,5 +154,42 @@ public void testFallback_primarySet() {
 
         Assert.assertEquals(200, prop.get().intValue());
     }
+
+    static class CustomValueOf {
+        private final String value;
+
+        public static CustomValueOf valueOf(String value) {
+            return new CustomValueOf(value);
+        }
+
+        public CustomValueOf(String value) {
+            this.value = value;
+        }
+
+        public String getValue() {
+            return value;
+        }
+    }
+
+    public static IClientConfigKey<CustomValueOf> CUSTOM_KEY = new CommonClientConfigKey<CustomValueOf>("CustomValueOf", new CustomValueOf("default")) {};
+
+    @Test
+    public void testValueOfWithDefault() {
+        DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
+
+        CustomValueOf prop = clientConfig.getOrDefault(CUSTOM_KEY);
+        Assert.assertEquals("default", prop.getValue());
+    }
+
+    @Test
+    public void testValueOf() {
+        ConfigurationManager.getConfigInstance().setProperty("testValueOf.ribbon.CustomValueOf", "value");
+
+        DefaultClientConfigImpl clientConfig = new DefaultClientConfigImpl();
+        clientConfig.setClientName("testValueOf");
+
+        Property<CustomValueOf> prop = clientConfig.getDynamicProperty(CUSTOM_KEY);
+        Assert.assertEquals("value", prop.get().getValue());
+    }
 }