(commons-collections) branch master updated: [COLLECTIONS-892] Fix unique IndexedCollection add atomicity (#688)

[ggregory]

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 ca83f8699 [COLLECTIONS-892] Fix unique IndexedCollection add atomicity 
(#688)
ca83f8699 is described below

commit ca83f8699e10403be6987b92fb1a877743df8262
Author: OldTruckDriver <43225823+oldtruckdri...@users.noreply.github.com>
AuthorDate: Sat Jun 20 03:26:26 2026 +1000

    [COLLECTIONS-892] Fix unique IndexedCollection add atomicity (#688)
    
    add(Object) called super.add() before addToIndex() checked the unique-key 
constraint,
    so a duplicate-key add mutated the decorated collection before throwing
    IllegalArgumentException. For a unique index, check the key up front and 
throw before
    adding. Add a test asserting the collection is unchanged after a rejected 
add.
    
    Reviewed-by: OpenAI Codex
    Reviewed-by: Anthropic Claude Code
---
 src/changes/changes.xml                                       |  1 +
 .../commons/collections4/collection/IndexedCollection.java    | 11 +++++++++++
 .../collections4/collection/IndexedCollectionTest.java        | 11 +++++++++++
 3 files changed, 23 insertions(+)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 4c334d631..1db10ee3f 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -52,6 +52,7 @@
     <action type="fix" dev="ggregory" due-to="Dexter.k, Gary 
Gregory">Re-validate entries in PredicatedMap/PredicatedCollection readObject 
(#682).</action>
     <action type="fix" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory" 
issue="COLLECTIONS-889">IndexedCollection.remove(Object) removes all values for 
a non-unique index key.</action>
     <action type="fix" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory" 
issue="COLLECTIONS-890">TransformIterator throws NullPointerException when its 
transformer is null.</action>
+    <action type="fix" dev="ggregory" due-to="Ruiqi Dong, Gary Gregory" 
issue="COLLECTIONS-892">IndexedCollection.add(Object) mutates unique indexed 
collections before rejecting duplicate keys.</action>
     <action type="fix" dev="ggregory" due-to="Maxim Safronov, Gary Gregory" 
issue="COLLECTIONS-878">MapUtils.invertMap() improves HashMap construction 
(#652).</action>
     <action type="fix" dev="ggregory" due-to="Dexter.k, Gary Gregory">Validate 
order and uniqueness invariants in decorator readObject() (#684).</action>
     <action type="fix" dev="ggregory" due-to="Gary Gregory">TransformIterator 
now fail-fast on a null Iterator (#686).</action>
diff --git 
a/src/main/java/org/apache/commons/collections4/collection/IndexedCollection.java
 
b/src/main/java/org/apache/commons/collections4/collection/IndexedCollection.java
index c534703a3..71d4b5ed3 100644
--- 
a/src/main/java/org/apache/commons/collections4/collection/IndexedCollection.java
+++ 
b/src/main/java/org/apache/commons/collections4/collection/IndexedCollection.java
@@ -113,6 +113,17 @@ public class IndexedCollection<K, C> extends 
AbstractCollectionDecorator<C> {
      */
     @Override
     public boolean add(final C object) {
+        if (uniqueIndex) {
+            final K key = keyTransformer.apply(object);
+            if (index.containsKey(key)) {
+                throw new IllegalArgumentException("Duplicate key in uniquely 
indexed collection.");
+            }
+            final boolean added = super.add(object);
+            if (added) {
+                index.put(key, object);
+            }
+            return added;
+        }
         final boolean added = super.add(object);
         if (added) {
             addToIndex(object);
diff --git 
a/src/test/java/org/apache/commons/collections4/collection/IndexedCollectionTest.java
 
b/src/test/java/org/apache/commons/collections4/collection/IndexedCollectionTest.java
index db427309d..c79810f17 100644
--- 
a/src/test/java/org/apache/commons/collections4/collection/IndexedCollectionTest.java
+++ 
b/src/test/java/org/apache/commons/collections4/collection/IndexedCollectionTest.java
@@ -135,6 +135,17 @@ class IndexedCollectionTest extends 
AbstractCollectionTest<String> {
         assertThrows(IllegalArgumentException.class, () -> coll.add("1"));
     }
 
+    @Test
+    void testFailedUniqueAddDoesNotPolluteDecoratedCollection() {
+        final IndexedCollection<Integer, String> indexed = 
decorateUniqueCollection(new ArrayList<>());
+        indexed.add("01");
+
+        assertThrows(IllegalArgumentException.class, () -> indexed.add("1"));
+        assertEquals(1, indexed.size());
+        assertEquals(asList("01"), new ArrayList<>(indexed));
+        assertEquals("01", indexed.get(1));
+    }
+
     @Test
     void testReindexUpdatesIndexWhenDecoratedCollectionIsModifiedSeparately() 
throws Exception {
         final Collection<String> original = new ArrayList<>();