diff --git a/archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java b/archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java index 7ed434ea..8a8a87ca 100644 --- a/archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java +++ b/archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java @@ -3,7 +3,6 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -81,15 +80,6 @@ public Type[] getActualTypeArguments() { public Type getRawType() { return rawType; } - - public boolean isMap() { - return Map.class.isAssignableFrom(rawType); - } - - public boolean isCollection() { - return Collection.class.isAssignableFrom(rawType); - } - @Override public Type getOwnerType() { return null; diff --git a/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java b/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java index 534f39f6..b29f03af 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java @@ -28,13 +28,20 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.math.BigDecimal; import java.math.BigInteger; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.NoSuchElementException; +import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.BiConsumer; @@ -296,30 +303,47 @@ protected T getValue(Type type, String key) { } } + private boolean isMap(ParameterizedType type) { + if (type.getRawType() instanceof Class) { + return Map.class.isAssignableFrom((Class) type.getRawType()); + } + return false; + } + + private boolean isCollection(ParameterizedType type) { + if (type.getRawType() instanceof Class) { + return Collection.class.isAssignableFrom((Class) type.getRawType()); + } + return false; + + } + @SuppressWarnings("unchecked") protected T getValueWithDefault(Type type, String key, T defaultValue) { Object rawProp = getRawProperty(key); - if (rawProp == null && type instanceof ArchaiusType) { - ArchaiusType archaiusType = (ArchaiusType) type; - if (archaiusType.isMap()) { - List vals = new ArrayList<>(); + if (rawProp == null && type instanceof ParameterizedType) { + ParameterizedType parameterizedType = (ParameterizedType) type; + if (isMap(parameterizedType)) { + Map ret = new LinkedHashMap<>(); String keyAndDelimiter = key + "."; + Type keyType = parameterizedType.getActualTypeArguments()[0]; + Type valueType = parameterizedType.getActualTypeArguments()[1]; + for (String k : keys()) { if (k.startsWith(keyAndDelimiter)) { - String val = getString(k); - if (val.contains("=") || val.contains(",")) { - log.warn( - "For map resolution of key {}, skipping subkey {} because value {}" - + " contains an invalid character (=/,)", - key, k, val); - } else { - vals.add(String.format("%s=%s", k.substring(keyAndDelimiter.length()), getString(k))); - } + ret.put(getDecoder().decode(keyType, k.substring(keyAndDelimiter.length())), get(valueType, k)); } } - rawProp = vals.isEmpty() ? null : String.join(",", vals); - } else if (archaiusType.isCollection()) { - rawProp = createListStringForKey(key); + return ret.isEmpty() ? defaultValue : (T) Collections.unmodifiableMap(ret); + } else if (isCollection(parameterizedType)) { + Type valueType = parameterizedType.getActualTypeArguments()[0]; + List ret = createListForKey(key, valueType); + if (Set.class.isAssignableFrom((Class) parameterizedType.getRawType())) { + Set retSet = new LinkedHashSet<>(ret); + return ret.isEmpty() ? defaultValue : (T) Collections.unmodifiableSet(retSet); + } else { + return ret.isEmpty() ? defaultValue : (T) Collections.unmodifiableList(ret); + } } } @@ -389,26 +413,18 @@ protected T getValueWithDefault(Type type, String key, T defaultValue) { new IllegalArgumentException("Property " + rawProp + " is not convertible to " + type.getTypeName())); } - private String createListStringForKey(String key) { - List vals = new ArrayList<>(); + private List createListForKey(String key, Type type) { + List vals = new ArrayList<>(); int counter = 0; while (true) { String checkKey = String.format("%s[%s]", key, counter++); if (containsKey(checkKey)) { - String val = getString(checkKey); - if (val.contains(",")) { - log.warn( - "For collection resolution of key {}, skipping subkey {} because value {}" - + " contains an invalid character (,)", - key, checkKey, val); - } else { - vals.add(getString(checkKey)); - } + vals.add(get(type, checkKey)); } else { break; } } - return vals.isEmpty() ? null : String.join(",", vals); + return vals; } @Override @@ -515,7 +531,10 @@ public Byte getByte(String key, Byte defaultValue) { public List getList(String key, Class type) { Object value = getRawProperty(key); if (value == null) { - value = createListStringForKey(key); + List alternativeListCreation = createListForKey(key, type); + if (!alternativeListCreation.isEmpty()) { + return (List) alternativeListCreation; + } } if (value == null) { return notFound(key); @@ -540,7 +559,10 @@ public List getList(String key) { public List getList(String key, List defaultValue) { Object value = getRawProperty(key); if (value == null) { - value = createListStringForKey(key); + List alternativeListCreation = createListForKey(key, String.class); + if (!alternativeListCreation.isEmpty()) { + return alternativeListCreation; + } } if (value == null) { return notFound(key, defaultValue); diff --git a/archaius2-core/src/test/java/com/netflix/archaius/config/AbstractConfigTest.java b/archaius2-core/src/test/java/com/netflix/archaius/config/AbstractConfigTest.java index 9e8ec1a2..32afe4ab 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/config/AbstractConfigTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/config/AbstractConfigTest.java @@ -61,6 +61,8 @@ public class AbstractConfigTest { entries.put("springYmlList[0]", "1"); entries.put("springYmlList[1]", "2"); entries.put("springYmlList[2]", "3"); + // Repeated entry to distinguish set and list + entries.put("springYmlList[3]", "3"); entries.put("springYmlMap.key1", "1"); entries.put("springYmlMap.key2", "2"); entries.put("springYmlMap.key3", "3"); @@ -242,7 +244,7 @@ public void testSpringYml() { List list = config.get(ArchaiusType.forListOf(Integer.class), "springYmlList", Arrays.asList(1)); - assertEquals(Arrays.asList(1, 2, 3), list); + assertEquals(Arrays.asList(1, 2, 3, 3), list); Map map = config.get(ArchaiusType.forMapOf(String.class, Integer.class), @@ -257,6 +259,11 @@ public void testSpringYml() { config.get(ArchaiusType.forListOf(Integer.class), "springYmlMap", Arrays.asList(1)); assertEquals(invalidList, Arrays.asList(1)); + // Not a proper set, so we have the default value returned + Set invalidSet = + config.get(ArchaiusType.forSetOf(Integer.class), "springYmlMap", Collections.singleton(1)); + assertEquals(invalidSet, Collections.singleton(1)); + // Not a proper map, so we have the default value returned Map invalidMap = config.get( @@ -265,18 +272,5 @@ public void testSpringYml() { Collections.singletonMap("default", "default")); assertEquals(1, invalidMap.size()); assertEquals("default", invalidMap.get("default")); - - // Some illegal values, so we return with those filtered - List listWithSomeInvalid = - config.get(ArchaiusType.forListOf(String.class), "springYmlWithSomeInvalidList", Arrays.asList("bad")); - assertEquals(listWithSomeInvalid, Arrays.asList("abc", "a=b")); - - Map mapWithSomeInvalid = - config.get( - ArchaiusType.forMapOf(String.class, String.class), - "springYmlWithSomeInvalidMap", - Collections.emptyMap()); - assertEquals(1, mapWithSomeInvalid.size()); - assertEquals("c", mapWithSomeInvalid.get("key2")); } }