This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-collections.git
The following commit(s) were added to refs/heads/master by this push:
new a1496edc7 Keep OrderedProperties keySet and values in sync with the
map (#719)
a1496edc7 is described below
commit a1496edc75dcfb0b7987d1c3dee433ffa1888b1d
Author: Naveed Khan <[email protected]>
AuthorDate: Thu Jul 30 10:17:10 2026 +0000
Keep OrderedProperties keySet and values in sync with the map (#719)
* keep OrderedProperties keySet and values in sync with the map
keySet() returned the private orderedKeys set, so mutations through the
view edited the order tracker without touching the backing Hashtable. values()
was not overridden, so the inherited view did the reverse and left a stale key.
Both now route removals through the map.
* hold the OrderedProperties monitor in the ordered key iterator remove
iterator.remove() edited orderedKeys outside the lock every other
mutation holds. Wrap the combined orderedKeys and map removal in
synchronized (OrderedProperties.this).
---
.../collections4/properties/OrderedProperties.java | 118 ++++++++++++++++++++-
.../properties/OrderedPropertiesTest.java | 54 ++++++++++
2 files changed, 171 insertions(+), 1 deletion(-)
diff --git
a/src/main/java/org/apache/commons/collections4/properties/OrderedProperties.java
b/src/main/java/org/apache/commons/collections4/properties/OrderedProperties.java
index c6bd53d37..3e1cec342 100644
---
a/src/main/java/org/apache/commons/collections4/properties/OrderedProperties.java
+++
b/src/main/java/org/apache/commons/collections4/properties/OrderedProperties.java
@@ -16,7 +16,10 @@
*/
package org.apache.commons.collections4.properties;
+import java.util.AbstractCollection;
import java.util.AbstractMap.SimpleEntry;
+import java.util.AbstractSet;
+import java.util.Collection;
import java.util.Collections;
import java.util.Enumeration;
import java.util.Iterator;
@@ -41,6 +44,80 @@ import java.util.stream.Collectors;
*/
public class OrderedProperties extends Properties {
+ /**
+ * A key set view in insertion order.
+ */
+ private final class KeySet extends AbstractSet<Object> {
+
+ @Override
+ public void clear() {
+ OrderedProperties.this.clear();
+ }
+
+ @Override
+ public boolean contains(final Object key) {
+ return containsKey(key);
+ }
+
+ @Override
+ public Iterator<Object> iterator() {
+ return orderedKeysIterator();
+ }
+
+ @Override
+ public boolean remove(final Object key) {
+ return OrderedProperties.this.remove(key) != null;
+ }
+
+ @Override
+ public int size() {
+ return OrderedProperties.this.size();
+ }
+ }
+
+ /**
+ * A values view in key insertion order.
+ */
+ private final class Values extends AbstractCollection<Object> {
+
+ @Override
+ public void clear() {
+ OrderedProperties.this.clear();
+ }
+
+ @Override
+ public boolean contains(final Object value) {
+ return containsValue(value);
+ }
+
+ @Override
+ public Iterator<Object> iterator() {
+ final Iterator<Object> keys = orderedKeysIterator();
+ return new Iterator<Object>() {
+
+ @Override
+ public boolean hasNext() {
+ return keys.hasNext();
+ }
+
+ @Override
+ public Object next() {
+ return get(keys.next());
+ }
+
+ @Override
+ public void remove() {
+ keys.remove();
+ }
+ };
+ }
+
+ @Override
+ public int size() {
+ return OrderedProperties.this.size();
+ }
+ }
+
private static final long serialVersionUID = 1L;
/**
@@ -119,7 +196,7 @@ public class OrderedProperties extends Properties {
@Override
public Set<Object> keySet() {
- return orderedKeys;
+ return new KeySet();
}
@Override
@@ -134,6 +211,40 @@ public class OrderedProperties extends Properties {
return merge;
}
+ /**
+ * Creates an iterator over the keys in insertion order whose {@link
Iterator#remove()} also removes the mapping.
+ *
+ * @return A new iterator.
+ */
+ private Iterator<Object> orderedKeysIterator() {
+ final Iterator<Object> iterator = orderedKeys.iterator();
+ return new Iterator<Object>() {
+
+ private Object last;
+
+ @Override
+ public boolean hasNext() {
+ return iterator.hasNext();
+ }
+
+ @Override
+ public Object next() {
+ last = iterator.next();
+ return last;
+ }
+
+ @Override
+ public void remove() {
+ // All orderedKeys writes happen under the OrderedProperties
monitor.
+ synchronized (OrderedProperties.this) {
+ // Not remove(Object), which would edit orderedKeys while
this iterator walks it.
+ iterator.remove();
+ OrderedProperties.super.remove(last);
+ }
+ }
+ };
+ }
+
@Override
public Enumeration<?> propertyNames() {
return Collections.enumeration(stringPropertyNames());
@@ -209,4 +320,9 @@ public class OrderedProperties extends Properties {
sb.append(", ");
}
}
+
+ @Override
+ public Collection<Object> values() {
+ return new Values();
+ }
}
diff --git
a/src/test/java/org/apache/commons/collections4/properties/OrderedPropertiesTest.java
b/src/test/java/org/apache/commons/collections4/properties/OrderedPropertiesTest.java
index 20b936a02..40476f111 100644
---
a/src/test/java/org/apache/commons/collections4/properties/OrderedPropertiesTest.java
+++
b/src/test/java/org/apache/commons/collections4/properties/OrderedPropertiesTest.java
@@ -18,7 +18,9 @@ package org.apache.commons.collections4.properties;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
import java.io.FileNotFoundException;
import java.io.FileReader;
@@ -30,14 +32,37 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Consumer;
+import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
/**
* Tests {@link OrderedProperties}.
*/
class OrderedPropertiesTest {
+ /**
+ * Every way of dropping the middle mapping through the key and value
views.
+ */
+ static Stream<Arguments> getViewRemovals() {
+ return Stream.of(
+ arguments("keySet().remove", (Consumer<OrderedProperties>)
props -> props.keySet().remove("key2")),
+ arguments("keySet().removeAll", (Consumer<OrderedProperties>)
props -> props.keySet().removeAll(Collections.singleton("key2"))),
+ arguments("keySet().iterator().remove",
(Consumer<OrderedProperties>) props -> removeSecond(props.keySet().iterator())),
+ arguments("values().remove", (Consumer<OrderedProperties>)
props -> props.values().remove("value2")),
+ arguments("values().iterator().remove",
(Consumer<OrderedProperties>) props ->
removeSecond(props.values().iterator())));
+ }
+
+ private static void removeSecond(final Iterator<Object> iterator) {
+ iterator.next();
+ iterator.next();
+ iterator.remove();
+ }
+
private void assertAscendingOrder(final OrderedProperties
orderedProperties) {
final int first = 1;
final int last = 11;
@@ -99,6 +124,14 @@ class OrderedPropertiesTest {
return assertDescendingOrder(orderedProperties);
}
+ private OrderedProperties newThreeKeyProperties() {
+ final OrderedProperties orderedProperties = new OrderedProperties();
+ orderedProperties.put("key1", "value1");
+ orderedProperties.put("key2", "value2");
+ orderedProperties.put("key3", "value3");
+ return orderedProperties;
+ }
+
@Test
void testCompute() {
final OrderedProperties orderedProperties = new OrderedProperties();
@@ -185,6 +218,15 @@ class OrderedPropertiesTest {
});
}
+ @Test
+ void testKeySetRejectsAdd() {
+ final OrderedProperties orderedProperties = newThreeKeyProperties();
+ final Set<Object> keySet = orderedProperties.keySet();
+ assertThrows(UnsupportedOperationException.class, () ->
keySet.add("key4"));
+ assertEquals("[key1, key2, key3]",
orderedProperties.keySet().toString());
+ assertEquals("{key1=value1, key2=value2, key3=value3}",
orderedProperties.toString());
+ }
+
@Test
void testKeys() {
final OrderedProperties orderedProperties = new OrderedProperties();
@@ -340,4 +382,16 @@ class OrderedPropertiesTest {
"{Z=ValueZ, Y=ValueY, X=ValueX, W=ValueW, V=ValueV, U=ValueU,
T=ValueT, S=ValueS, R=ValueR, Q=ValueQ, P=ValueP, O=ValueO, N=ValueN, M=ValueM,
L=ValueL, K=ValueK, J=ValueJ, I=ValueI, H=ValueH, G=ValueG, F=ValueF, E=ValueE,
D=ValueD, C=ValueC, B=ValueB, A=ValueA}",
orderedProperties.toString());
}
+
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("getViewRemovals")
+ void testViewRemovalRemovesMapping(final String description, final
Consumer<OrderedProperties> removal) {
+ final OrderedProperties orderedProperties = newThreeKeyProperties();
+ removal.accept(orderedProperties);
+ assertFalse(orderedProperties.containsKey("key2"));
+ assertEquals(2, orderedProperties.size());
+ assertEquals("[key1, key3]", orderedProperties.keySet().toString());
+ assertEquals("[value1, value3]",
orderedProperties.values().toString());
+ assertEquals("{key1=value1, key3=value3}",
orderedProperties.toString());
+ }
}
