Skip to content

Commit

Permalink
Proper support for empty values in collections
Browse files Browse the repository at this point in the history
  • Loading branch information
@elandau
elandau committed Sep 2, 2016
1 parent 943cf07 commit 3d810bb
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 26 deletions.
32 changes: 15 additions & 17 deletions archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.lang.reflect.Proxy;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashSet;
Expand Down Expand Up @@ -247,13 +248,13 @@ <T> T newProxy(final Class<T> type, final String initialPrefix, boolean immutabl
if (returnType.equals(Map.class)) {
invokers.put(m, createMapProperty(propName, (ParameterizedType)m.getGenericReturnType(), immutable));
} else if (returnType.equals(Set.class)) {
invokers.put(m, createSetProperty(propName, (ParameterizedType)m.getGenericReturnType(), LinkedHashSet::new));
invokers.put(m, createCollectionProperty(propName, (ParameterizedType)m.getGenericReturnType(), LinkedHashSet::new));
} else if (returnType.equals(SortedSet.class)) {
invokers.put(m, createSetProperty(propName, (ParameterizedType)m.getGenericReturnType(), TreeSet::new));
invokers.put(m, createCollectionProperty(propName, (ParameterizedType)m.getGenericReturnType(), TreeSet::new));
} else if (returnType.equals(List.class)) {
invokers.put(m, createListProperty(propName, (ParameterizedType)m.getGenericReturnType(), ArrayList::new));
invokers.put(m, createCollectionProperty(propName, (ParameterizedType)m.getGenericReturnType(), ArrayList::new));
} else if (returnType.equals(LinkedList.class)) {
invokers.put(m, createListProperty(propName, (ParameterizedType)m.getGenericReturnType(), LinkedList::new));
invokers.put(m, createCollectionProperty(propName, (ParameterizedType)m.getGenericReturnType(), LinkedList::new));
} else if (returnType.isInterface()) {
invokers.put(m, createInterfaceProperty(propName, newProxy(returnType, propName, immutable)));
} else if (m.getParameterTypes() != null && m.getParameterTypes().length > 0) {
Expand Down Expand Up @@ -323,7 +324,7 @@ <T> T newProxy(final Class<T> type, final String initialPrefix, boolean immutabl
protected <T> MethodInvoker<T> createCustomProperty(final Function<String, T> converter, final String propName) {
final Property<T> prop = propertyFactory
.getProperty(propName)
.asType(converter, null);
.asType(converter, "");
return new MethodInvoker<T>() {
@Override
public T invoke(Object obj, Object[] args) {
Expand All @@ -337,24 +338,21 @@ public String getKey() {
};
}

private MethodInvoker<?> createListProperty(String propName, ParameterizedType type, Supplier<List> listSupplier) {
private MethodInvoker<?> createCollectionProperty(String propName, ParameterizedType type, Supplier<Collection> listSupplier) {
final Class<?> valueType = (Class<?>)type.getActualTypeArguments()[0];
return createCustomProperty(s -> {
List list = listSupplier.get();
Arrays.asList(s.split("\\s*,\\s*")).forEach(v -> list.add(decoder.decode(valueType, v)));
Collection list = listSupplier.get();
if (!s.isEmpty()) {
Arrays.asList(s.split("\\s*,\\s*")).forEach(v -> {
if (!v.isEmpty() || valueType == String.class) {
list.add(decoder.decode(valueType, v));
}
});
}
return list;
}, propName);
}

private MethodInvoker<?> createSetProperty(String propName, ParameterizedType type, Supplier<Set> setSupplier) {
final Class<?> valueType = (Class<?>)type.getActualTypeArguments()[0];
return createCustomProperty(s -> {
Set set = setSupplier.get();
Arrays.asList(s.split("\\s*,\\s*")).forEach(v -> set.add(decoder.decode(valueType, v)));
return set;
}, propName);
}

@SuppressWarnings("unchecked")
private <T> MethodInvoker<T> createMapProperty(final String propName, final ParameterizedType type, final boolean immutable) {
final Class<?> valueType = (Class<?>)type.getActualTypeArguments()[1];
Expand Down
5 changes: 2 additions & 3 deletions archaius2-core/src/main/java/com/netflix/archaius/property/DefaultPropertyContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,8 @@ public T get() {
T newValue = null;
try {
newValue = resolveCurrent();
}
catch (Exception e) {
LOG.warn("Unable to get current version of property '{}'. Error: {}", key, e.getMessage());
} catch (Exception e) {
LOG.warn("Unable to get current version of property '{}'", key, e);
}

if (cache.compareAndSet(currentValue, newValue, cacheVersion, latestVersion)) {
Expand Down
58 changes: 52 additions & 6 deletions archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public static interface ConfigWithCollections {
public void testCollections() {
SettableConfig config = new DefaultSettableConfig();
config.setProperty("list", "5,4,3,2,1");
config.setProperty("set", "5,4,3,2,1");
config.setProperty("set", "1,2,3,5,4");
config.setProperty("sortedSet", "5,4,3,2,1");
config.setProperty("linkedList", "5,4,3,2,1");

Expand All @@ -262,13 +262,59 @@ public void testCollections() {

Assert.assertEquals(Arrays.asList(5,4,3,2,1), new ArrayList<>(withCollections.getLinkedList()));
Assert.assertEquals(Arrays.asList(5,4,3,2,1), new ArrayList<>(withCollections.getList()));
Assert.assertEquals(Arrays.asList(5,4,3,2,1), new ArrayList<>(withCollections.getSet()));
Assert.assertEquals(Arrays.asList(1,2,3,5,4), new ArrayList<>(withCollections.getSet()));
Assert.assertEquals(Arrays.asList(1,2,3,4,5), new ArrayList<>(withCollections.getSortedSet()));

config.setProperty("list", "6,7,8,9,10");
Assert.assertEquals(Arrays.asList(6,7,8,9,10), new ArrayList<>(withCollections.getList()));
}

@Test
public void emptyNonStringValuesIgnoredInCollections() {
SettableConfig config = new DefaultSettableConfig();
config.setProperty("list", ",4, ,2,1");
config.setProperty("set", ",2, ,5,4");
config.setProperty("sortedSet", ",4, ,2,1");
config.setProperty("linkedList", ",4, ,2,1");

PropertyFactory factory = DefaultPropertyFactory.from(config);
ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory);
ConfigWithCollections withCollections = proxy.newProxy(ConfigWithCollections.class);

Assert.assertEquals(Arrays.asList(4,2,1), new ArrayList<>(withCollections.getLinkedList()));
Assert.assertEquals(Arrays.asList(4,2,1), new ArrayList<>(withCollections.getList()));
Assert.assertEquals(Arrays.asList(2,5,4), new ArrayList<>(withCollections.getSet()));
Assert.assertEquals(Arrays.asList(1,2,4), new ArrayList<>(withCollections.getSortedSet()));
}

public static interface ConfigWithStringCollections {
List<String> getList();

Set<String> getSet();

SortedSet<String> getSortedSet();

LinkedList<String> getLinkedList();
}

@Test
public void emptyStringValuesAreAddedToCollection() {
SettableConfig config = new DefaultSettableConfig();
config.setProperty("list", ",4, ,2,1");
config.setProperty("set", ",2, ,5,4");
config.setProperty("sortedSet", ",4, ,2,1");
config.setProperty("linkedList", ",4, ,2,1");

PropertyFactory factory = DefaultPropertyFactory.from(config);
ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory);
ConfigWithStringCollections withCollections = proxy.newProxy(ConfigWithStringCollections.class);

Assert.assertEquals(Arrays.asList("", "4","", "2","1"), new ArrayList<>(withCollections.getLinkedList()));
Assert.assertEquals(Arrays.asList("", "4","", "2","1"), new ArrayList<>(withCollections.getList()));
Assert.assertEquals(Arrays.asList("" ,"2","5","4"), new ArrayList<>(withCollections.getSet()));
Assert.assertEquals(Arrays.asList("", "1","2","4"), new ArrayList<>(withCollections.getSortedSet()));
}

@Test
public void testCollectionsWithoutValue() {
SettableConfig config = new DefaultSettableConfig();
Expand All @@ -277,10 +323,10 @@ public void testCollectionsWithoutValue() {
ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory);
ConfigWithCollections withCollections = proxy.newProxy(ConfigWithCollections.class);

Assert.assertNull(withCollections.getLinkedList());
Assert.assertNull(withCollections.getList());
Assert.assertNull(withCollections.getSet());
Assert.assertNull(withCollections.getSortedSet());
Assert.assertTrue(withCollections.getLinkedList().isEmpty());
Assert.assertTrue(withCollections.getList().isEmpty());
Assert.assertTrue(withCollections.getSet().isEmpty());
Assert.assertTrue(withCollections.getSortedSet().isEmpty());
}

public static interface ConfigWithCollectionsWithDefaultValueAnnotation {
Expand Down

0 comments on commit 3d810bb

Please sign in to comment.