diff --git a/archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java b/archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java index c0870b30a..dea990049 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/ConfigProxyFactory.java @@ -19,16 +19,17 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; -import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -318,10 +319,8 @@ else if ("toString".equals(method.getName())) { propertyNames.put(m, propName); - // For sub-interfaces create a proxy instance where the same proxy instance is returned but its - // methods can still return dynamic values if (returnType.equals(Map.class)) { - invoker = createMapProperty(propName, (ParameterizedType)m.getGenericReturnType(), immutable, defaultSupplier); + invoker = createMapProperty(propName, (ParameterizedType)m.getGenericReturnType(), LinkedHashMap::new, defaultSupplier); } else if (returnType.equals(Set.class)) { invoker = createCollectionProperty(propName, (ParameterizedType)m.getGenericReturnType(), LinkedHashSet::new, defaultSupplier); } else if (returnType.equals(SortedSet.class)) { @@ -356,76 +355,55 @@ else if ("toString".equals(method.getName())) { } private static Supplier memoize(T value) { - return () -> value; + return () -> value; } @SuppressWarnings({ "rawtypes", "unchecked" }) - private MethodInvoker createCollectionProperty(String propName, ParameterizedType type, Supplier collectionFactory, Supplier next) { - final Class valueType = (Class)type.getActualTypeArguments()[0]; + private MethodInvoker createCollectionProperty(String propName, ParameterizedType type, Supplier> collectionFactory, Supplier next) { + final Class valueType = (Class) type.getActualTypeArguments()[0]; final Property prop = propertyRepository .get(propName, String.class) - .map(s -> { - if (s != null) { - Collection list = collectionFactory.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 (T)list; - } else { - return null; - } - }); + .map(s -> { + if (s != null) { + Collection list = collectionFactory.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 (T) list; + } else { + return next.get(); + } + }); - return new MethodInvoker() { - @Override - public T invoke(Object[] args) { - T value = prop.get(); - if (value == null) { - value = next.get(); - } - if (value == null) { - value = (T) collectionFactory.get(); - } - return value; - } - }; + return args -> Optional.ofNullable(prop.get()).orElseGet((Supplier) collectionFactory); } - @SuppressWarnings("unchecked") - private MethodInvoker createMapProperty(final String propName, final ParameterizedType type, final boolean immutable, Supplier next) { + private MethodInvoker createMapProperty(final String propName, final ParameterizedType type, Supplier mapFactory, Supplier next) { + final Class keyType = (Class)type.getActualTypeArguments()[0]; final Class valueType = (Class)type.getActualTypeArguments()[1]; - Map map; - // This is a map for String -> Interface so create a proxy for any value - if (valueType.isInterface()) { - map = new ReadOnlyMap() { - Map lookup = new ConcurrentHashMap(); - @Override - public Object get(final Object key) { - return lookup.computeIfAbsent((String) key, new Function() { - @Override - public Object apply(String key) { - return newProxy(valueType, propName + "." + key, immutable); - } - }); - } - }; - } else { - // This is a map of String -> DecodableType (i.e. String, Long, etc...) - map = new ReadOnlyMap() { - @Override - public Object get(final Object key) { - return config.get(valueType, propName + "." + key); - } - }; - } - return (MethodInvoker) createInterfaceProperty(propName, map, (Supplier>)next); - } + final Property prop = propertyRepository + .get(propName, String.class) + .map(s -> { + if (s == null) + return null; + T result = mapFactory.get(); + Arrays + .stream(s.split("\\s*,\\s*")) + .filter(pair -> !pair.isEmpty()) + .map(pair -> pair.split("\\s*=\\s*")) + .forEach(kv -> result.put(decoder.decode(keyType, kv[0]), decoder.decode(valueType, kv[1]))); + return (T)Collections.unmodifiableMap(result); + } + ).orElse((T) Collections.unmodifiableMap(next.get())); + return args -> Optional.ofNullable(prop.get()).orElseGet(mapFactory); + } + protected Supplier defaultValueFromString(Class type, String defaultValue) { return () -> decoder.decode(type, defaultValue); } @@ -445,16 +423,7 @@ protected MethodInvoker createInterfaceProperty(String propName, final T protected MethodInvoker createScalarProperty(final Class type, final String propName, Supplier next) { final Property prop = propertyRepository.get(propName, type); - return new MethodInvoker() { - @Override - public T invoke(Object[] args) { - T result = prop.get(); - if (result == null) { - result = next.get(); - } - return result; - } - }; + return args -> Optional.ofNullable(prop.get()).orElseGet(next); } protected MethodInvoker createParameterizedProperty(final Class returnType, final String propName, final String nameAnnot, Supplier next) { diff --git a/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java b/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java index cf98927c6..3ce71203d 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/ProxyFactoryTest.java @@ -4,15 +4,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; -import com.netflix.archaius.api.Config; -import com.netflix.archaius.api.PropertyFactory; -import com.netflix.archaius.api.annotations.Configuration; -import com.netflix.archaius.api.annotations.DefaultValue; -import com.netflix.archaius.api.annotations.PropertyName; -import com.netflix.archaius.api.config.SettableConfig; -import com.netflix.archaius.config.DefaultSettableConfig; -import com.netflix.archaius.config.EmptyConfig; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -28,6 +19,15 @@ import org.junit.Assert; import org.junit.Test; +import com.netflix.archaius.api.Config; +import com.netflix.archaius.api.PropertyFactory; +import com.netflix.archaius.api.annotations.Configuration; +import com.netflix.archaius.api.annotations.DefaultValue; +import com.netflix.archaius.api.annotations.PropertyName; +import com.netflix.archaius.api.config.SettableConfig; +import com.netflix.archaius.config.DefaultSettableConfig; +import com.netflix.archaius.config.EmptyConfig; + public class ProxyFactoryTest { public static enum TestEnum { NONE, @@ -211,53 +211,43 @@ public void testWithArguments() { Assert.assertEquals("default", withArgs.getProperty("a", 2)); } - public static interface ConfigWithMap { - Map getChildren(); + public static interface ConfigWithLongMap { + default Map getMap() { return Collections.singletonMap("default", 0L); } } @Test - public void testWithMap() { + public void testWithLongMap() { SettableConfig config = new DefaultSettableConfig(); - config.setProperty("children.1.str", "value1"); - config.setProperty("children.2.str", "value2"); + config.setProperty("map", "1=123,2=456"); PropertyFactory factory = DefaultPropertyFactory.from(config); ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); - ConfigWithMap withArgs = proxy.newProxy(ConfigWithMap.class); + ConfigWithLongMap withArgs = proxy.newProxy(ConfigWithLongMap.class); - SubConfig sub1 = withArgs.getChildren().get("1"); - SubConfig sub2 = withArgs.getChildren().get("2"); + long sub1 = withArgs.getMap().get("1"); + long sub2 = withArgs.getMap().get("2"); - Assert.assertEquals("value1", sub1.str()); - Assert.assertEquals("value2", sub2.str()); + Assert.assertEquals(123, sub1); + Assert.assertEquals(456, sub2); - config.setProperty("children.2.str", "value3"); - Assert.assertEquals("value3", sub2.str()); - } - - public static interface ConfigWithLongMap { - Map getChildren(); + config.setProperty("map", "1=123,2=789"); + sub2 = withArgs.getMap().get("2"); + Assert.assertEquals(789, sub2); } @Test - public void testWithLongMap() { + public void testWithLongMapDefaultValue() { SettableConfig config = new DefaultSettableConfig(); - config.setProperty("children.1", "123"); - config.setProperty("children.2", "456"); PropertyFactory factory = DefaultPropertyFactory.from(config); ConfigProxyFactory proxy = new ConfigProxyFactory(config, config.getDecoder(), factory); ConfigWithLongMap withArgs = proxy.newProxy(ConfigWithLongMap.class); - long sub1 = withArgs.getChildren().get("1"); - long sub2 = withArgs.getChildren().get("2"); - - Assert.assertEquals(123, sub1); - Assert.assertEquals(456, sub2); + Assert.assertEquals(Collections.singletonMap("default", 0L), withArgs.getMap()); - config.setProperty("children.2", "789"); - sub2 = withArgs.getChildren().get("2"); - Assert.assertEquals(789, sub2); + config.setProperty("map", "foo=123"); + + Assert.assertEquals(Collections.singletonMap("foo", 123L), withArgs.getMap()); } public static interface ConfigWithCollections {