diff --git a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java index 0c0aa542ef..e041ed6ca4 100644 --- a/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java +++ b/src/main/java/org/apache/commons/configuration2/convert/AbstractListDelimiterHandler.java @@ -20,6 +20,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Iterator; import java.util.LinkedList; import java.util.Set; @@ -42,24 +43,25 @@ public abstract class AbstractListDelimiterHandler implements ListDelimiterHandl static Collection flatten(final ListDelimiterHandler handler, final Object value, final int limit, final Set dejaVu) { if (value instanceof String) { return handler.split((String) value, true); + } else if (!isRecursiveContainer(value)) { + return value != null ? Collections.singletonList(value) : Collections.emptyList(); + } + if (!dejaVu.add(value)) { + return Collections.emptyList(); } - dejaVu.add(value); final Collection result = new LinkedList<>(); - if (value instanceof Path) { - // Don't handle as an Iterable. - result.add(value); - } else if (value instanceof Iterable) { - flattenIterator(handler, result, ((Iterable) value).iterator(), limit, dejaVu); - } else if (value instanceof Iterator) { - flattenIterator(handler, result, (Iterator) value, limit, dejaVu); - } else if (value != null) { - if (value.getClass().isArray()) { + try { + if (value instanceof Iterable) { + flattenIterator(handler, result, ((Iterable) value).iterator(), limit, dejaVu); + } else if (value instanceof Iterator) { + flattenIterator(handler, result, (Iterator) value, limit, dejaVu); + } else if (value.getClass().isArray()) { for (int len = Array.getLength(value), idx = 0, size = 0; idx < len && size < limit; idx++, size = result.size()) { - result.addAll(handler.flatten(Array.get(value, idx), limit - size)); + result.addAll(flatten(handler, Array.get(value, idx), limit - size, dejaVu)); } - } else { - result.add(value); } + } finally { + dejaVu.remove(value); } return result; } @@ -77,12 +79,19 @@ static void flattenIterator(final ListDelimiterHandler handler, final Collection final Set dejaVue) { int size = target.size(); while (size < limit && iterator.hasNext()) { - final Object next = iterator.next(); - if (!dejaVue.contains(next)) { - target.addAll(flatten(handler, next, limit - size, dejaVue)); - size = target.size(); - } + target.addAll(flatten(handler, iterator.next(), limit - size, dejaVue)); + size = target.size(); + } + } + + private static boolean isRecursiveContainer(final Object value) { + if (value instanceof Path) { + // Don't handle as an Iterable. + return false; } + return value instanceof Iterator + || value instanceof Iterable + || value != null && value.getClass().isArray(); } /** diff --git a/src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java b/src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java index 604661e481..91223128c4 100644 --- a/src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java +++ b/src/test/java/org/apache/commons/configuration2/TestAbstractConfigurationBasicFeatures.java @@ -26,6 +26,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -676,6 +678,32 @@ void testGetList() { assertEquals(expected, result); } + /** + * Tests typed list conversion for delimited values with duplicates. + */ + @Test + void testGetListTypedWithDuplicatesAndDelimiterHandling() { + final BaseConfiguration config = new BaseConfiguration(); + config.setListDelimiterHandler(new DefaultListDelimiterHandler(',')); + + config.addProperty("list.strings", Arrays.asList("a", "b", "a")); + config.addProperty("list.strings2", Arrays.asList("", "", "a")); + config.addProperty("list.ints", Arrays.asList(1, 2, 1)); + config.addProperty("list.booleans", Arrays.asList(true, false, true)); + config.addProperty("list.doubles", Arrays.asList(1.5, 2.5, 1.5)); + config.addProperty("list.paths", Arrays.asList(Paths.get("path1"), Paths.get("path2"), Paths.get("path1"))); + + assertEquals(Arrays.asList("a", "b", "a"), config.getList(String.class, "list.strings")); + assertEquals(Arrays.asList("", "", "a"), config.getList(String.class, "list.strings2")); + assertEquals(Arrays.asList(1, 2, 1), config.getList(Integer.class, "list.ints")); + assertEquals(Arrays.asList(Boolean.TRUE, Boolean.FALSE, Boolean.TRUE), config.getList(Boolean.class, "list.booleans")); + assertEquals(Arrays.asList(1.5d, 2.5d, 1.5d), config.getList(Double.class, "list.doubles")); + assertEquals( + Arrays.asList(Paths.get("path1"), Paths.get("path2"), Paths.get("path1")), + config.getList(Path.class, "list.paths") + ); + } + /** * Tests getList() for single non-string values. */ diff --git a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java index ced8e3968b..6ea513bc37 100644 --- a/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java +++ b/src/test/java/org/apache/commons/configuration2/TestPropertiesConfiguration.java @@ -511,13 +511,16 @@ void testCompress840ArrayList(final int size) { void testCompress840ArrayListCycle(final int size) { final ArrayList object = new ArrayList<>(); for (int i = 0; i < size; i++) { - object.add(i); + object.add(String.valueOf(i)); object.add(object); object.add(new ArrayList<>(object)); } final Collection result = testCompress840(object); assertNotNull(result); - assertEquals(size, result.size()); + // At each iteration, the previous flattened values and the new scalar appear twice: + // once in the original list and once in its copy. Therefore, f(n) = 2 * (f(n - 1) + 1), + // with f(0) = 0, which gives f(n) = 2^(n + 1) - 2. + assertEquals((1 << (size + 1)) - 2, result.size()); object.add(object); testCompress840(object); } diff --git a/src/test/java/org/apache/commons/configuration2/convert/TestDefaultListDelimiterHandler.java b/src/test/java/org/apache/commons/configuration2/convert/TestDefaultListDelimiterHandler.java index 5a97c98eda..5d40362a20 100644 --- a/src/test/java/org/apache/commons/configuration2/convert/TestDefaultListDelimiterHandler.java +++ b/src/test/java/org/apache/commons/configuration2/convert/TestDefaultListDelimiterHandler.java @@ -23,6 +23,7 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.List; @@ -175,4 +176,29 @@ void testSplitSingleElement() { void testSplitUnexpectedEscape() { checkSplit("\\x, \\,y, \\", true, "\\x", ",y", "\\"); } + + /** + * Tests whether flatten() skips a recursive array reference while keeping the reachable leaf values. + */ + @Test + void testFlattenArrayCycle() { + final Object[] array = new Object[2]; + array[0] = "value1,value2"; + array[1] = array; + + assertIterableEquals(Arrays.asList("value1", "value2"), handler.flatten(array, Integer.MAX_VALUE)); + } + + /** + * Tests whether flatten() skips a recursive list-array cycle while keeping the reachable leaf values. + */ + @Test + void testFlattenMixedListAndArrayCycle() { + final List list = new ArrayList<>(); + final Object[] array = {list}; + list.add("value1,value2"); + list.add(array); + + assertIterableEquals(Arrays.asList("value1", "value2"), handler.flatten(list, Integer.MAX_VALUE)); + } }