Skip to content

Commit

Permalink
Handle numeric conversions for non-string raw values.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@rgallardo-netflix
rgallardo-netflix committed May 23, 2024
1 parent 424287b commit aa5d3f0
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 15 deletions.
63 changes: 56 additions & 7 deletions archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
84 changes: 76 additions & 8 deletions archaius2-core/src/test/java/com/netflix/archaius/config/AbstractConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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"));
}
}
9 changes: 9 additions & 0 deletions archaius2-core/src/test/java/com/netflix/archaius/config/MapConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();
Expand Down

0 comments on commit aa5d3f0

Please sign in to comment.