[COLLECTIONS-877] OrderedProperties.stringPropertyNames() returns an

unordered set

Author: garydgregory

src/changes/changes.xml

@@ -35,6 +35,7 @@
     <action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz, Joerg Budischewski" issue="COLLECTIONS-838">Calling SetUtils.union on multiple instances of SetView causes JVM to hang</action>
     <action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz, Joerg Budischewski" issue="COLLECTIONS-722">Improve IteratorUtils.chainedIterator() performance.</action>
     <action type="fix" dev="ggregory" due-to="Gary Gregory, PMD">Fix UselessOverridingMethod in org.apache.commons.collections4.properties.PropertiesFactory.</action>
+    <action type="fix" dev="ggregory" due-to="Adam Rauch, Gary Gregory" issue="COLLECTIONS-693">OrderedProperties.stringPropertyNames() returns an unordered set.</action>
     <!-- ADD -->
     <action type="add" dev="ggregory" due-to="Gary Gregory">Add generics to UnmodifiableIterator for the wrapped type.</action>
     <!-- UPDATE -->

src/main/java/org/apache/commons/collections4/properties/OrderedProperties.java

@@ -84,6 +84,26 @@ public Set<Map.Entry<Object, Object>> entrySet() {
         return orderedKeys.stream().map(k -> new SimpleEntry<>(k, get(k))).collect(Collectors.toCollection(LinkedHashSet::new));
     }
 
+    /**
+     * Enumerates all key/value pairs in the specified LinkedHashSet and omits the property if the key or value is not a string.
+     *
+     * @param result The result set to populate.
+     * @return The given set.
+     */
+    private synchronized LinkedHashSet<String> enumerateStringProperties(final LinkedHashSet<String> result) {
+        if (defaults != null) {
+            result.addAll(defaults.stringPropertyNames());
+        }
+        for (final Enumeration<?> e = keys(); e.hasMoreElements();) {
+            final Object k = e.nextElement();
+            final Object v = get(k);
+            if (k instanceof String && v instanceof String) {
+                result.add((String) k);
+            }
+        }
+        return result;
+    }
+
     @Override
     public synchronized void forEach(final BiConsumer<? super Object, ? super Object> action) {
         Objects.requireNonNull(action);
@@ -154,6 +174,11 @@ public synchronized boolean remove(final Object key, final Object value) {
         return remove;
     }
 
+    @Override
+    public Set<String> stringPropertyNames() {
+        return enumerateStringProperties(new LinkedHashSet<>());
+    }
+
     @Override
     public synchronized String toString() {
         // Must override for Java 17 to maintain order since the implementation is based on a map

src/test/java/org/apache/commons/collections4/properties/OrderedPropertiesTest.java

@@ -18,6 +18,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.FileNotFoundException;
 import java.io.FileReader;
@@ -27,6 +28,7 @@
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.junit.jupiter.api.Test;
@@ -57,6 +59,9 @@ private void assertAscendingOrder(final OrderedProperties orderedProperties) {
         for (int i = first; i <= last; i++) {
             assertEquals("key" + i, propertyNames.nextElement());
         }
+        final Set<String> propertyNameSet = orderedProperties.stringPropertyNames();
+        final AtomicInteger i = new AtomicInteger(first);
+        propertyNameSet.forEach(e -> assertEquals("key" + i.getAndIncrement(), e));
     }
 
     private OrderedProperties assertDescendingOrder(final OrderedProperties orderedProperties) {
@@ -80,6 +85,9 @@ private OrderedProperties assertDescendingOrder(final OrderedProperties orderedP
         for (int i = first; i <= last; i--) {
             assertEquals("key" + i, propertyNames.nextElement());
         }
+        final Set<String> propertyNameSet = orderedProperties.stringPropertyNames();
+        final AtomicInteger i = new AtomicInteger(first);
+        propertyNameSet.forEach(e -> assertEquals("key" + i.getAndDecrement(), e));
         return orderedProperties;
     }
 
@@ -287,6 +295,12 @@ void testRemoveKeyValue() throws FileNotFoundException, IOException {
         assertFalse(Collections.list(props.propertyNames()).contains(k));
     }
 
+    @Test
+    void testStringPropertyName() {
+        final OrderedProperties orderedProperties = new OrderedProperties();
+        assertTrue(orderedProperties.stringPropertyNames().isEmpty());
+    }
+
     @Test
     void testToString() {
         final OrderedProperties orderedProperties = new OrderedProperties();