Handle numeric conversions for non-string raw values.

For implementations of AbstractConfig that return non-string values from getRawProperty(), be smarter about doing numeric conversions. Without this, users are forced to guess if the property source will interpret a given number as an integer or a long and code to that, instead of to the types that make sense for their own use case.

Fixes: #653

Author: rgallardo-netflix

archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java

@@ -254,27 +254,74 @@ protected <T> T getValue(Type type, String key) {
         }
     }
 
+    @SuppressWarnings("unchecked")
     protected <T> T getValueWithDefault(Type type, String key, T defaultValue) {
         Object rawProp = getRawProperty(key);
+
+        // Not found. Return the default.
         if (rawProp == null) {
             return defaultValue;
         }
+
+        // raw prop is a String. Decode it or fail.
         if (rawProp instanceof String) {
             try {
                 String value = resolve(rawProp.toString());
                 return decoder.decode(type, value);
-            } catch (NumberFormatException e) {
+            } catch (RuntimeException e) {
                 return parseError(key, rawProp.toString(), e);
             }
-        } else if (type instanceof Class) {
+        }
+
+        if (type instanceof Class) {
+            // Caller wants a simple class.
             Class<?> cls = (Class<?>) type;
-            if (cls.isInstance(rawProp) || cls.isPrimitive()) {
+
+            // The raw object is already of the right type
+            if (cls.isInstance(rawProp)) {
+                return (T) rawProp;
+            }
+
+            // Caller wants a string
+            if (String.class.isAssignableFrom(cls)) {
+                return (T) rawProp.toString();
+            }
+
+            // Caller wants an unwrapped boolean.
+            if (rawProp instanceof Boolean &&  cls == boolean.class) {
                 return (T) rawProp;
             }
+
+            // Caller wants a number AND we have one. Handle widening and narrowing conversions.
+            // Char is not included here. It's not a Number and the semantics of converting it to/from a number or a
+            // string have rough edges. Just ask users to avoid it.
+            if (rawProp instanceof Number
+                && ( Number.class.isAssignableFrom(cls)
+                   || ( cls.isPrimitive() && cls != char.class ))) { // We handled boolean above, so if cls is a primitive and not char then it's a number type
+                if (cls == int.class || cls == Integer.class) {
+                    return (T) Integer.valueOf(((Number) rawProp).intValue());
+                }
+                if (cls == long.class || cls == Long.class) {
+                    return (T) Long.valueOf(((Number) rawProp).longValue());
+                }
+                if (cls == double.class || cls == Double.class) {
+                    return (T) Double.valueOf(((Number) rawProp).doubleValue());
+                }
+                if (cls == float.class || cls == Float.class) {
+                    return (T) Float.valueOf(((Number) rawProp).floatValue());
+                }
+                if (cls == short.class || cls == Short.class) {
+                    return (T) Short.valueOf(((Number) rawProp).shortValue());
+                }
+                if (cls == byte.class || cls == Byte.class) {
+                    return (T) Byte.valueOf(((Number) rawProp).byteValue());
+                }
+            }
         }
 
+        // Nothing matches (ie, caller wants a ParametrizedType, or the rawProp is not easily cast to the desired type)
         return parseError(key, rawProp.toString(),
-                new NumberFormatException("Property " + rawProp.toString() + " is of wrong format " + type.getTypeName()));
+                new IllegalArgumentException("Property " + rawProp + " is not convertible to " + type.getTypeName()));
     }
 
     @Override
@@ -284,7 +331,7 @@ public String resolve(String value) {
 
     @Override
     public <T> T resolve(String value, Class<T> type) {
-        return getDecoder().decode(type, resolve(value));
+        return getDecoder().decode((Type) type, resolve(value));
     }
 
     @Override
@@ -384,14 +431,15 @@ public <T> List<T> getList(String key, Class<T> type) {
             return notFound(key);
         }
         String[] parts = value.split(getListDelimiter());
-        List<T> result = new ArrayList<T>();
+        List<T> result = new ArrayList<>();
         for (String part : parts) {
-            result.add(decoder.decode(type, part));
+            result.add(decoder.decode((Type) type, part));
         }
         return result;
     }
 
     @Override
+    @SuppressWarnings("rawtypes") // Required by legacy API
     public List getList(String key) {
         String value = getString(key);
         if (value == null) {
@@ -402,6 +450,7 @@ public List getList(String key) {
     }
 
     @Override
+    @SuppressWarnings("rawtypes") // Required by legacy API
     public List getList(String key, List defaultValue) {
         String value = getString(key, null);
         if (value == null) {

archaius2-core/src/test/java/com/netflix/archaius/config/AbstractConfigTest.java

@@ -16,7 +16,9 @@
 package com.netflix.archaius.config;
 
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
+import java.util.Map;
 import java.util.function.BiConsumer;
 
 import org.junit.jupiter.api.Test;
@@ -27,9 +29,20 @@
 public class AbstractConfigTest {
 
     private final AbstractConfig config = new AbstractConfig() {
+        private final Map<String, Object> entries = new HashMap<>();
+
+        {
+            entries.put("foo", "bar");
+            entries.put("byte", (byte) 42);
+            entries.put("int", 42);
+            entries.put("long", 42L);
+            entries.put("float", 42.0f);
+            entries.put("double", 42.0d);
+        }
+
         @Override
         public boolean containsKey(String key) {
-            return "foo".equals(key);
+            return entries.containsKey(key);
         }
 
         @Override
@@ -39,35 +52,90 @@ public boolean isEmpty() {
 
         @Override
         public Iterator<String> getKeys() {
-            return Collections.singletonList("foo").iterator();
+            return Collections.unmodifiableSet(entries.keySet()).iterator();
         }
 
         @Override
         public Object getRawProperty(String key) {
-            if ("foo".equals(key)) {
-                return "bar";
-            }
-            return null;
+            return entries.get(key);
         }
 
         @Override
         public void forEachProperty(BiConsumer<String, Object> consumer) {
-            consumer.accept("foo",  "bar");
+            entries.forEach(consumer);
         }
     };
 
     @Test
-    public void testGet() throws Exception {
+    public void testGet() {
         assertEquals("bar", config.get(String.class, "foo"));
     }
 
     @Test
     public void getExistingProperty() {
+        //noinspection OptionalGetWithoutIsPresent
         assertEquals("bar", config.getProperty("foo").get());
     }
 
     @Test
     public void getNonExistentProperty() {
         assertFalse(config.getProperty("non_existent").isPresent());
     }
+
+    @Test
+    public void testGetRawNumerics() {
+        // First, get each entry as its expected type and the corresponding wrapper.
+        assertEquals(42, config.get(int.class, "int"));
+        assertEquals(42, config.get(Integer.class, "int"));
+        assertEquals(42L, config.get(long.class, "long"));
+        assertEquals(42L, config.get(Long.class, "long"));
+        assertEquals((byte) 42, config.get(byte.class, "byte"));
+        assertEquals((byte) 42, config.get(Byte.class, "byte"));
+        assertEquals(42.0f, config.get(float.class, "float"));
+        assertEquals(42.0f, config.get(Float.class, "float"));
+        assertEquals(42.0d, config.get(double.class, "double"));
+        assertEquals(42.0d, config.get(Double.class, "double"));
+
+        // Then, get each entry as a string
+        assertEquals("42", config.get(String.class, "int"));
+        assertEquals("42", config.get(String.class, "long"));
+        assertEquals("42", config.get(String.class, "byte"));
+        assertEquals("42.0", config.get(String.class, "float"));
+        assertEquals("42.0", config.get(String.class, "double"));
+
+        // Then, narrowed types
+        assertEquals((byte) 42, config.get(byte.class, "int"));
+        assertEquals((byte) 42, config.get(byte.class, "long"));
+        assertEquals((byte) 42, config.get(byte.class, "float"));
+        assertEquals((byte) 42, config.get(byte.class, "double"));
+        assertEquals(42.0f, config.get(double.class, "double"));
+
+        // Then, widened
+        assertEquals(42L, config.get(long.class, "int"));
+        assertEquals(42L, config.get(long.class, "byte"));
+        assertEquals(42L, config.get(long.class, "float"));
+        assertEquals(42L, config.get(long.class, "double"));
+        assertEquals(42.0d, config.get(double.class, "float"));
+
+        // On floating point
+        assertEquals(42.0f, config.get(float.class, "int"));
+        assertEquals(42.0f, config.get(float.class, "byte"));
+        assertEquals(42.0f, config.get(float.class, "long"));
+        assertEquals(42.0f, config.get(float.class, "double"));
+
+        // As doubles
+        assertEquals(42.0d, config.get(double.class, "int"));
+        assertEquals(42.0d, config.get(double.class, "byte"));
+        assertEquals(42.0d, config.get(double.class, "long"));
+        assertEquals(42.0d, config.get(double.class, "float"));
+
+        // Narrowed types in wrapper classes
+        assertEquals((byte) 42, config.get(Byte.class, "int"));
+        assertEquals((byte) 42, config.get(Byte.class, "long"));
+        assertEquals((byte) 42, config.get(Byte.class, "float"));
+
+        // Widened types in wrappers
+        assertEquals(42L, config.get(Long.class, "int"));
+        assertEquals(42L, config.get(Long.class, "byte"));
+    }
 }

archaius2-core/src/test/java/com/netflix/archaius/config/MapConfigTest.java

@@ -183,6 +183,15 @@ public void numericInterpolationShouldWork() {
         assertEquals(123L, (long) config.getLong("value"));
     }
 
+    @Test
+    public void numericInterpolationShouldWork_withNonStringValues() {
+        Config config = MapConfig.builder()
+                .put("default",     123)
+                .put("value",       "${default}")
+                .build();
+        assertEquals(123L, (long) config.getLong("value"));
+    }
+
     @Test
     public void getKeys() {
         Map<String, String> props = new HashMap<>();