[jira] [Commented] (COLLECTIONS-889) `IndexedCollection.remove()` clears the whole key bucket in non-unique indexes

["Gary D. Gregory (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/COLLECTIONS-889?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18088574#comment-18088574 ] 

Gary D. Gregory commented on COLLECTIONS-889:
---------------------------------------------

Hello [~rickydong] 

Issues with PRs get a higher priority in my queue ;)

> `IndexedCollection.remove()` clears the whole key bucket in non-unique indexes
> ------------------------------------------------------------------------------
>
>                 Key: COLLECTIONS-889
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-889
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Collection
>            Reporter: Ruiqi Dong
>            Priority: Major
>
> *Summary*
> For non-unique indexes, multiple collection elements may share the same transformed key. The current `remove(...)` implementation removes the object from the decorated collection correctly, but then removes the entire key from the index. That drops the remaining same-key elements from the index even though they are still present in the collection.
>  
> *Affected code*
> File: `src/main/java/org/apache/commons/collections4/collection/IndexedCollection.java`
> {code:java}
> @SuppressWarnings("unchecked")
> @Override
> public boolean remove(final Object object) {
>     final boolean removed = super.remove(object);
>     if (removed) {
>         removeFromIndex((C) object);
>     }
>     return removed;
> }
> private void removeFromIndex(final C object) {
>     index.remove(keyTransformer.apply(object));
> } {code}
> *Reproducer* 
> Add the following test to `src/test/java/org/apache/commons/collections4/collection/IndexedCollectionTest.java`
> {code:java}
> @Test
> void testRemovePreservesRemainingValuesWithSameTransformedKey() {
>     final IndexedCollection<Integer, String> indexed =
>             (IndexedCollection<Integer, String>) decorateCollection(new ArrayList<>());
>     indexed.add("01");
>     indexed.add("1");
>     indexed.remove("01");
>     assertEquals("1", indexed.get(1));
>     assertNotNull(indexed.values(1));
>     assertEquals(asList("1"), new ArrayList<>(indexed.values(1)));
> } {code}
> Run:
> {code:java}
> mvn -q -Dtest=org.apache.commons.collections4.collection.IndexedCollectionTest#testRemovePreservesRemainingValuesWithSameTransformedKey test {code}
> Observed behavior:
> `get(1)` returns `null` even though `"1"` is still present in the decorated collection.
> {code:java}
> expected: <1> but was: <null> {code}
> Expected behavior:
> Removing `"01"` should leave the other same-key value `"1"` indexed and retrievable.
>  
> In the non-unique configuration, the index is supposed to track all values that share a key. Removing one value should only remove that single value from the index entry, not delete the whole key bucket. The current implementation leaves the collection and the index out of sync after a normal successful `remove(...)`.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)