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 5d0895de5 Do not register a value collection until an add succeeds
(#717)
5d0895de5 is described below
commit 5d0895de5bc00c5a025a17dbd51faa9398a176e2
Author: Naveed Khan <[email protected]>
AuthorDate: Wed Jul 22 14:04:02 2026 +0000
Do not register a value collection until an add succeeds (#717)
The wrapped collections returned by get(key) put a freshly created backing
collection into the map before the add that fills it. addAll of an empty
collection then leaves an empty collection mapped to the key, and
WrappedList.add(int, value) leaves an empty list when the index is out of range
and the insert throws. Store the new collection only after the add succeeds,
matching put(K, V).
---
.../collections4/multimap/AbstractListValuedMap.java | 8 +++++---
.../collections4/multimap/AbstractMultiValuedMap.java | 10 +++++++---
.../collections4/multimap/AbstractMultiValuedMapTest.java | 13 +++++++++++++
.../collections4/multimap/ArrayListValuedHashMapTest.java | 12 ++++++++++++
4 files changed, 37 insertions(+), 6 deletions(-)
diff --git
a/src/main/java/org/apache/commons/collections4/multimap/AbstractListValuedMap.java
b/src/main/java/org/apache/commons/collections4/multimap/AbstractListValuedMap.java
index a073e4355..e2a4e865e 100644
---
a/src/main/java/org/apache/commons/collections4/multimap/AbstractListValuedMap.java
+++
b/src/main/java/org/apache/commons/collections4/multimap/AbstractListValuedMap.java
@@ -126,10 +126,12 @@ public abstract class AbstractListValuedMap<K, V> extends
AbstractMultiValuedMap
@Override
public void add(final int index, final V value) {
- List<V> list = getMapping();
+ final List<V> list = getMapping();
if (list == null) {
- list = createCollection();
- getMap().put(key, list);
+ final List<V> newList = createCollection();
+ newList.add(index, value);
+ getMap().put(key, newList);
+ return;
}
list.add(index, value);
}
diff --git
a/src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java
b/src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java
index 4f7323c0b..7973e378a 100644
---
a/src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java
+++
b/src/main/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMap.java
@@ -441,10 +441,14 @@ public abstract class AbstractMultiValuedMap<K, V>
implements MultiValuedMap<K,
@Override
public boolean addAll(final Collection<? extends V> other) {
- Collection<V> coll = getMapping();
+ final Collection<V> coll = getMapping();
if (coll == null) {
- coll = createCollection();
- AbstractMultiValuedMap.this.map.put(key, coll);
+ final Collection<V> newColl = createCollection();
+ if (newColl.addAll(other)) {
+ AbstractMultiValuedMap.this.map.put(key, newColl);
+ return true;
+ }
+ return false;
}
return coll.addAll(other);
}
diff --git
a/src/test/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMapTest.java
b/src/test/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMapTest.java
index aaade2bcf..890bffdef 100644
---
a/src/test/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMapTest.java
+++
b/src/test/java/org/apache/commons/collections4/multimap/AbstractMultiValuedMapTest.java
@@ -705,6 +705,19 @@ public abstract class AbstractMultiValuedMapTest<K, V>
extends AbstractObjectTes
assertTrue(col2.contains("v1_1"));
}
+ @Test
+ @SuppressWarnings("unchecked")
+ void testAddAllThroughGetEmptyLeavesKeyAbsent() {
+ assumeTrue(isAddSupported());
+ resetEmpty();
+ final MultiValuedMap<K, V> map = getMap();
+ final Collection<V> col = map.get((K) "k0");
+ assertFalse(col.addAll(new ArrayList<>()));
+ assertFalse(map.containsKey("k0"));
+ assertFalse(map.keySet().contains("k0"));
+ assertEquals(0, map.size());
+ }
+
/*void testRemoveViaGetCollectionRemove() {
if (!isRemoveSupported()) {
return;
diff --git
a/src/test/java/org/apache/commons/collections4/multimap/ArrayListValuedHashMapTest.java
b/src/test/java/org/apache/commons/collections4/multimap/ArrayListValuedHashMapTest.java
index f65ee297c..18efa9361 100644
---
a/src/test/java/org/apache/commons/collections4/multimap/ArrayListValuedHashMapTest.java
+++
b/src/test/java/org/apache/commons/collections4/multimap/ArrayListValuedHashMapTest.java
@@ -19,6 +19,7 @@ package org.apache.commons.collections4.multimap;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.ArrayList;
@@ -117,6 +118,17 @@ public class ArrayListValuedHashMapTest<K, V> extends
AbstractMultiValuedMapTest
assertTrue(listMap.containsKey("A"));
}
+ @Test
+ @SuppressWarnings("unchecked")
+ void testListValuedMapAddByIndexInvalidIndexLeavesKeyAbsent() {
+ final ListValuedMap<K, V> listMap = makeObject();
+ final List<V> list = listMap.get((K) "A");
+ assertThrows(IndexOutOfBoundsException.class, () -> list.add(1, (V)
"a1"));
+ assertFalse(listMap.containsKey("A"));
+ assertTrue(listMap.get((K) "A").isEmpty());
+ assertEquals(0, listMap.size());
+ }
+
@Test
@SuppressWarnings("unchecked")
void testListValuedMapAddViaListIterator() {
