diff --git a/archaius2-core/src/main/java/com/netflix/archaius/converters/DefaultCollectionsTypeConverterFactory.java b/archaius2-core/src/main/java/com/netflix/archaius/converters/DefaultCollectionsTypeConverterFactory.java index 7194d826..2532f35b 100644 --- a/archaius2-core/src/main/java/com/netflix/archaius/converters/DefaultCollectionsTypeConverterFactory.java +++ b/archaius2-core/src/main/java/com/netflix/archaius/converters/DefaultCollectionsTypeConverterFactory.java @@ -18,6 +18,7 @@ import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; +import java.util.function.Function; import java.util.function.Supplier; public final class DefaultCollectionsTypeConverterFactory implements TypeConverter.Factory { @@ -27,7 +28,7 @@ private DefaultCollectionsTypeConverterFactory() {} @Override public Optional> get(Type type, TypeConverter.Registry registry) { if (type instanceof ParameterizedType) { - ParameterizedType parameterizedType = (ParameterizedType)type; + ParameterizedType parameterizedType = (ParameterizedType) type; if (parameterizedType.getRawType().equals(Map.class)) { return Optional.of(createMapTypeConverter( registry.get(parameterizedType.getActualTypeArguments()[0]).orElseThrow(() -> new ConverterNotFoundException("No converter found")), @@ -37,50 +38,70 @@ public Optional> get(Type type, TypeConverter.Registry registry return Optional.of(createCollectionTypeConverter( parameterizedType.getActualTypeArguments()[0], registry, - LinkedHashSet::new)); + LinkedHashSet::new, + Collections::emptySet, + Collections::unmodifiableSet)); } else if (parameterizedType.getRawType().equals(SortedSet.class)) { return Optional.of(createCollectionTypeConverter( parameterizedType.getActualTypeArguments()[0], registry, - TreeSet::new)); + TreeSet::new, + Collections::emptySortedSet, + Collections::unmodifiableSortedSet)); } else if (parameterizedType.getRawType().equals(List.class)) { return Optional.of(createCollectionTypeConverter( parameterizedType.getActualTypeArguments()[0], registry, - ArrayList::new)); + ArrayList::new, + Collections::emptyList, + Collections::unmodifiableList)); } else if (parameterizedType.getRawType().equals(LinkedList.class)) { return Optional.of(createCollectionTypeConverter( parameterizedType.getActualTypeArguments()[0], registry, - LinkedList::new)); + LinkedList::new, + LinkedList::new, + Function.identity())); } } return Optional.empty(); } - @SuppressWarnings({ "rawtypes", "unchecked" }) - private TypeConverter createCollectionTypeConverter(final Type type, TypeConverter.Registry registry, final Supplier> collectionFactory) { - TypeConverter elementConverter = registry.get(type).orElseThrow(() -> new ConverterNotFoundException("No converter found")); + private static > TypeConverter createCollectionTypeConverter(final Type elementType, + final TypeConverter.Registry registry, + final Supplier collectionFactory, + final Supplier emptyCollectionFactory, + final Function finisher) { + @SuppressWarnings("unchecked") + TypeConverter elementConverter = (TypeConverter) registry.get(elementType) + .orElseThrow(() -> new ConverterNotFoundException("No converter found")); - boolean ignoreEmpty = !String.class.equals(type); + boolean ignoreEmpty = !String.class.equals(elementType); return value -> { - final Collection collection = collectionFactory.get(); - if (!value.isEmpty()) { - Arrays.asList(value.split("\\s*,\\s*")).forEach(v -> { - if (!v.isEmpty() || !ignoreEmpty) { - collection.add(elementConverter.convert(v)); - } - }); + if (value.isEmpty()) { + return emptyCollectionFactory.get(); } - return collection; + final T collection = collectionFactory.get(); + for (String item : value.split("\\s*,\\s*")) { + if (!item.isEmpty() || !ignoreEmpty) { + collection.add(elementConverter.convert(item)); + } + } + return finisher.apply(collection); }; } - private TypeConverter createMapTypeConverter(final TypeConverter keyConverter, final TypeConverter valueConverter, final Supplier mapFactory) { + private static TypeConverter> createMapTypeConverter(final TypeConverter keyConverter, + final TypeConverter valueConverter, + final Supplier> mapFactory) { return s -> { - Map result = mapFactory.get(); + if (s.isEmpty()) { + return Collections.emptyMap(); + } + Map result = mapFactory.get(); + Arrays .stream(s.split("\\s*,\\s*")) .filter(pair -> !pair.isEmpty()) @@ -91,4 +112,4 @@ private TypeConverter createMapTypeConverter(final TypeConverter keyConver return Collections.unmodifiableMap(result); }; } -} +} \ No newline at end of file diff --git a/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java b/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java index 4309d72f..1c331ffc 100644 --- a/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java +++ b/archaius2-core/src/test/java/com/netflix/archaius/property/PropertyTest.java @@ -19,6 +19,7 @@ import java.math.BigInteger; import java.time.Duration; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -167,6 +168,43 @@ public void testCollectionTypes() { // Test proper handling of unset properties Property> emptyProperty = factory.getMap("fubar", CustomType.class, CustomType.class); Assert.assertNull(emptyProperty.get()); + + config.setProperty("fubar", ""); + Property> emptyListProperty = factory.getList("fubar", String.class); + Assert.assertEquals(Collections.emptyList(), emptyListProperty.get()); + + Property> emptySetProperty = factory.getSet("fubar", String.class); + Assert.assertEquals(Collections.emptySet(), emptySetProperty.get()); + + Property> emptyMapProperty = factory.getMap("fubar", String.class, String.class); + Assert.assertEquals(Collections.emptyMap(), emptyMapProperty.get()); + } + + @Test + public void testCollectionTypesImmutability() { + SettableConfig config = new DefaultSettableConfig(); + DefaultPropertyFactory factory = DefaultPropertyFactory.from(config); + config.setProperty("foo", "10,13,13,20"); + config.setProperty("bar", ""); + config.setProperty("baz", "a=1,b=2"); + + List list = factory.getList("foo", Integer.class).get(); + Assert.assertThrows(UnsupportedOperationException.class, () -> list.add(100)); + + Set set = factory.getSet("foo", Integer.class).get(); + Assert.assertThrows(UnsupportedOperationException.class, () -> set.add(100)); + + Map map = factory.getMap("baz", String.class, Integer.class).get(); + Assert.assertThrows(UnsupportedOperationException.class, () -> map.put("c", 3)); + + List emptyList = factory.getList("bar", Integer.class).get(); + Assert.assertThrows(UnsupportedOperationException.class, () -> emptyList.add(100)); + + Set emptySet = factory.getSet("bar", Integer.class).get(); + Assert.assertThrows(UnsupportedOperationException.class, () -> emptySet.add(100)); + + Map emptyMap = factory.getMap("bar", String.class, Integer.class).get(); + Assert.assertThrows(UnsupportedOperationException.class, () -> emptyMap.put("c", 3)); } @Test