[jira] [Work logged] (COLLECTIONS-802) ReferenceMap iterator remove violates contract

[ASF GitHub Bot (Jira)]

     [ 
https://issues.apache.org/jira/browse/COLLECTIONS-802?focusedWorklogId=761531&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-761531
 ]

ASF GitHub Bot logged work on COLLECTIONS-802:
----------------------------------------------

                Author: ASF GitHub Bot
            Created on: 24/Apr/22 23:05
            Start Date: 24/Apr/22 23:05
    Worklog Time Spent: 10m 
      Work Description: ben-manes commented on PR #300:
URL: 
https://github.com/apache/commons-collections/pull/300#issuecomment-1107935459

   Checking and I accidentally attached the wrong version of `ApacheMapTest`. 😦 
   
   The flag `allowNulls` should have been false for `ReferenceMap`, and was 
there just because every collection author does this differently so I had to 
experiment to find Apache's setting. The corrected test case is below.
   
   ```java
   import java.util.Map;
   import java.util.function.Supplier;
   
   import org.apache.commons.collections4.map.HashedMap;
   import org.apache.commons.collections4.map.LRUMap;
   import org.apache.commons.collections4.map.LinkedMap;
   import org.apache.commons.collections4.map.ReferenceMap;
   
   import com.google.common.collect.testing.MapTestSuiteBuilder;
   import com.google.common.collect.testing.TestStringMapGenerator;
   import com.google.common.collect.testing.features.CollectionFeature;
   import com.google.common.collect.testing.features.CollectionSize;
   import com.google.common.collect.testing.features.MapFeature;
   
   import junit.framework.Test;
   import junit.framework.TestCase;
   import junit.framework.TestSuite;
   
   public final class ApacheMapTest extends TestCase {
   
     public static Test suite() {
       var test = new TestSuite();
       test.addTest(suite("HashedMap", HashedMap::new));
       test.addTest(suite("LinkedMap", LinkedMap::new));
       test.addTest(suite("LRUMap", LRUMap::new));
       test.addTest(suite("ReferenceMap", ReferenceMap::new));
       return test;
     }
   
     public static Test suite(String name, Supplier<Map<String, String>> 
factory) {
       return MapTestSuiteBuilder
           .using(new TestStringMapGenerator() {
             @Override protected Map<String, String> create(Map.Entry<String, 
String>[] entries) {
               var map = factory.get();
               for (var entry : entries) {
                 map.put(entry.getKey(), entry.getValue());
               }
               return map;
             }
           })
           .named(name)
           .withFeatures(
               CollectionSize.ANY,
               MapFeature.GENERAL_PURPOSE,
               MapFeature.ALLOWS_ANY_NULL_QUERIES,
               CollectionFeature.SUPPORTS_ITERATOR_REMOVE)
           .createTestSuite();
     }
   }
   ```
   <img width="1792" alt="Screen Shot 2022-04-24 at 4 04 37 PM" 
src="https://user-images.githubusercontent.com/378614/165000396-a85c13ea-9b0e-4206-88f7-8c115cf66985.png";>
   




Issue Time Tracking
-------------------

    Worklog Id:     (was: 761531)
    Time Spent: 1h 10m  (was: 1h)

> ReferenceMap iterator remove violates contract
> ----------------------------------------------
>
>                 Key: COLLECTIONS-802
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-802
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Map
>    Affects Versions: 4.4
>            Reporter: Ben Manes
>            Priority: Minor
>         Attachments: ApacheMapTest.java
>
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> Out of curiosity I ran Guava's testlib Map tests against the Apache types. 
> This uncovered a contract bug where {{Iterator.remove()}} is invalidated by 
> {{{}hasNext(){}}}, causing its call to no-op due to {{currentKey}} becoming 
> {{{}null{}}}. The isolates case is,
> {code:java}
> @Test
> public void iterator_remove() {
>   var map = new ReferenceMap<>();
>   map.put(1, 2);
>   var iter = map.entrySet().iterator();
>   assertTrue(iter.hasNext());
>   assertTrue(iter.hasNext());
>   assertEquals(iter.next(), 1);
>   assertFalse(iter.hasNext());
>   iter.remove();
>   assertEquals(map, Map.of());
> }{code}
> Guava's [testlib|https://github.com/google/guava/tree/master/guava-testlib] 
> has good coverage for the Collections Framework and might be worth 
> integrating. The simple test case that I wrote is attached.



--
This message was sent by Atlassian Jira
(v8.20.7#820007)