Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for parsing Collections and Maps from the Spring YML format #739

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from
Open

Add support for parsing Collections and Maps from the Spring YML format #739

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions archaius2-api/src/main/java/com/netflix/archaius/api/ArchaiusType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
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;
Expand Down Expand Up @@ -81,6 +82,14 @@ public Type getRawType() {
return rawType;
}

public boolean isMap() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move these two to be private static helper methods in AbstractConfig? Then things would work with any ParametrizedType, not just with ArchaiusType.

return Map.class.isAssignableFrom(rawType);
}

public boolean isCollection() {
return Collection.class.isAssignableFrom(rawType);
}

@Override
public Type getOwnerType() {
return null;
Expand Down
52 changes: 52 additions & 0 deletions archaius2-core/src/main/java/com/netflix/archaius/config/AbstractConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.lang.reflect.Type;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
Expand Down Expand Up @@ -298,6 +299,29 @@ protected <T> T getValue(Type type, String key) {
@SuppressWarnings("unchecked")
protected <T> T getValueWithDefault(Type type, String key, T defaultValue) {
Object rawProp = getRawProperty(key);
if (rawProp == null && type instanceof ArchaiusType) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't ... hate this, but let's think about it a bit more. I'd really love it if there was some way we could call the existing decoders here instead of marshaling the entire map into a single string just to unmarshal it back a nanosecond later. The same goes for the List support.

ArchaiusType archaiusType = (ArchaiusType) type;
if (archaiusType.isMap()) {
List<String> vals = new ArrayList<>();
String keyAndDelimiter = key + ".";
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)));
}
}
}
rawProp = vals.isEmpty() ? null : String.join(",", vals);
} else if (archaiusType.isCollection()) {
rawProp = createListStringForKey(key);
}
}

// Not found. Return the default.
if (rawProp == null) {
Expand Down Expand Up @@ -365,6 +389,28 @@ protected <T> T getValueWithDefault(Type type, String key, T defaultValue) {
new IllegalArgumentException("Property " + rawProp + " is not convertible to " + type.getTypeName()));
}

private String createListStringForKey(String key) {
List<String> 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));
}
} else {
break;
}
}
return vals.isEmpty() ? null : String.join(",", vals);
}

@Override
public String resolve(String value) {
return interpolator.create(getLookup()).resolve(value);
Expand Down Expand Up @@ -468,6 +514,9 @@ public Byte getByte(String key, Byte defaultValue) {
@Override
public <T> List<T> getList(String key, Class<T> type) {
Object value = getRawProperty(key);
if (value == null) {
value = createListStringForKey(key);
}
if (value == null) {
return notFound(key);
}
Expand All @@ -490,6 +539,9 @@ public List<?> getList(String key) {
@SuppressWarnings("rawtypes") // Required by legacy API
public List getList(String key, List defaultValue) {
Object value = getRawProperty(key);
if (value == null) {
value = createListStringForKey(key);
}
if (value == null) {
return notFound(key, defaultValue);
}
Expand Down
66 changes: 66 additions & 0 deletions archaius2-core/src/test/java/com/netflix/archaius/config/AbstractConfigTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.function.BiConsumer;

import com.netflix.archaius.api.ArchaiusType;
import com.netflix.archaius.api.Config;
import com.netflix.archaius.api.ConfigListener;
import com.netflix.archaius.exceptions.ParseException;
Expand All @@ -35,6 +38,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;

Expand All @@ -54,6 +58,18 @@ public class AbstractConfigTest {
entries.put("stringList", "a,b,c");
entries.put("uriList", "http://example.com,http://example.org");
entries.put("underlyingList", Arrays.asList("a", "b", "c"));
entries.put("springYmlList[0]", "1");
entries.put("springYmlList[1]", "2");
entries.put("springYmlList[2]", "3");
entries.put("springYmlMap.key1", "1");
entries.put("springYmlMap.key2", "2");
entries.put("springYmlMap.key3", "3");
entries.put("springYmlWithSomeInvalidList[0]", "abc,def");
entries.put("springYmlWithSomeInvalidList[1]", "abc");
entries.put("springYmlWithSomeInvalidList[2]", "a=b");
entries.put("springYmlWithSomeInvalidMap.key1", "a=b");
entries.put("springYmlWithSomeInvalidMap.key2", "c");
entries.put("springYmlWithSomeInvalidMap.key3", "d,e");
}

@Override
Expand Down Expand Up @@ -213,4 +229,54 @@ public void testListeners() {
verify(listener).onError(mockError, mockChildConfig);
}
}

@Test
public void testSpringYml() {
// Working cases for set, list, and map
Set<Integer> set =
config.get(ArchaiusType.forSetOf(Integer.class), "springYmlList", Collections.singleton(1));
assertEquals(set.size(), 3);
assertTrue(set.contains(1));
assertTrue(set.contains(2));
assertTrue(set.contains(3));

List<Integer> list =
config.get(ArchaiusType.forListOf(Integer.class), "springYmlList", Arrays.asList(1));
assertEquals(Arrays.asList(1, 2, 3), list);

Map<String, Integer> map =
config.get(ArchaiusType.forMapOf(String.class, Integer.class),
"springYmlMap", Collections.emptyMap());
assertEquals(map.size(), 3);
assertEquals(1, map.get("key1"));
assertEquals(2, map.get("key2"));
assertEquals(3, map.get("key3"));

// Not a proper list, so we have the default value returned
List<Integer> invalidList =
config.get(ArchaiusType.forListOf(Integer.class), "springYmlMap", Arrays.asList(1));
assertEquals(invalidList, Arrays.asList(1));

// Not a proper map, so we have the default value returned
Map<String, String> invalidMap =
config.get(
ArchaiusType.forMapOf(String.class, String.class),
"springYmlList",
Collections.singletonMap("default", "default"));
assertEquals(1, invalidMap.size());
assertEquals("default", invalidMap.get("default"));

// Some illegal values, so we return with those filtered
List<String> listWithSomeInvalid =
config.get(ArchaiusType.forListOf(String.class), "springYmlWithSomeInvalidList", Arrays.asList("bad"));
assertEquals(listWithSomeInvalid, Arrays.asList("abc", "a=b"));

Map<String, String> mapWithSomeInvalid =
config.get(
ArchaiusType.forMapOf(String.class, String.class),
"springYmlWithSomeInvalidMap",
Collections.emptyMap());
assertEquals(1, mapWithSomeInvalid.size());
assertEquals("c", mapWithSomeInvalid.get("key2"));
}
}
Loading