Ruiqi Dong created COLLECTIONS-892:
--------------------------------------
Summary: IndexedCollection.add() mutates the decorated collection
before rejecting a duplicate unique index key
Key: COLLECTIONS-892
URL: https://issues.apache.org/jira/browse/COLLECTIONS-892
Project: Commons Collections
Issue Type: Bug
Components: Collection
Reporter: Ruiqi Dong
# [BUG] `IndexedCollection.add()` mutates the decorated collection before
rejecting a duplicate unique index key
## Summary
For `uniqueIndexedCollection(...)`, adding an object whose transformed key
already exists is supposed to fail with `IllegalArgumentException`.
The current implementation appends the object to the decorated collection
first, and only afterwards checks the uniqueness constraint while updating the
index. If the key collides, the exception is thrown too late: the decorated
collection has already been mutated.
## Affected code
File:
`src/main/java/org/apache/commons/collections4/collection/IndexedCollection.java`
```java
@Override
public boolean add(final C object) {
finalbooleanadded = super.add(object);
if (added) {
addToIndex(object);
}
return added;
}
private void addToIndex(final C object) {
finalKkey = keyTransformer.apply(object);
if (uniqueIndex && index.containsKey(key)) {
thrownewIllegalArgumentException("Duplicate key in uniquely indexed
collection.");
}
index.put(key, object);
}
```
## Reproducer
Add the following test to
`src/test/java/org/apache/commons/collections4/collection/IndexedCollectionTest.java`:
```java
@Test
void testFailedUniqueAddDoesNotPolluteDecoratedCollection() {
finalIndexedCollection<Integer, String> indexed =
decorateUniqueCollection(newArrayList<>());
indexed.add("01");
assertThrows(IllegalArgumentException.class, () ->indexed.add("1"));
assertEquals(1, indexed.size());
assertEquals(asList("01"), newArrayList<>(indexed));
}
```
Both `"01"` and `"1"` map to the same integer key `1`.
Run:
```bash
mvn -q
-Dtest=org.apache.commons.collections4.collection.IndexedCollectionTest#testFailedUniqueAddDoesNotPolluteDecoratedCollection
test
```
## Observed behavior
```text
expected: <1> but was: <2>
```
## Expected behavior
After the duplicate-key `add(...)` fails, the collection should still contain
only the original element `"01"`.
## Why this is a bug
This is not merely about exception atomicity in the abstract.
`IndexedCollection` promises a unique index, but after a failed add:
- the decorated collection contains both values
- the index contains only the original key mapping
So the collection and its index immediately diverge, and the uniqueness
guarantee is no longer reflected by the actual collection state.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
