[
https://issues.apache.org/jira/browse/COLLECTIONS-892?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gary D. Gregory resolved COLLECTIONS-892.
-----------------------------------------
Fix Version/s: 4.6.0
Resolution: Fixed
> 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
> Priority: Minor
> Fix For: 4.6.0
>
>
> *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`
> {code:java}
> @Override
> public boolean add(final C object) {
> final boolean added = super.add(object);
> if (added) {
> addToIndex(object);
> }
> return added;
> }
> private void addToIndex(final C object) {
> final K key = keyTransformer.apply(object);
> if (uniqueIndex && index.containsKey(key)) {
> throw new IllegalArgumentException("Duplicate key in uniquely indexed
> collection.");
> }
> index.put(key, object);
> } {code}
> *Reproducer*
> Add the following test to
> `src/test/java/org/apache/commons/collections4/collection/IndexedCollectionTest.java`:
> {code:java}
> @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));
> } {code}
> Both `"01"` and `"1"` map to the same integer key `1`.
> Run:
> {code:java}
> mvn -q
> -Dtest=org.apache.commons.collections4.collection.IndexedCollectionTest#testFailedUniqueAddDoesNotPolluteDecoratedCollection
> test {code}
> Observed behavior
> {code:java}
> expected: <1> but was: <2> {code}
> *Expected behavior*
> After the duplicate-key `add(...)` fails, the collection should still contain
> only the original element `"01"`.
> 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)
