[
https://issues.apache.org/jira/browse/COLLECTIONS-802?focusedWorklogId=761525&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-761525
]
ASF GitHub Bot logged work on COLLECTIONS-802:
----------------------------------------------
Author: ASF GitHub Bot
Created on: 24/Apr/22 22:22
Start Date: 24/Apr/22 22:22
Worklog Time Spent: 10m
Work Description: kinow commented on code in PR #300:
URL:
https://github.com/apache/commons-collections/pull/300#discussion_r857185327
##########
src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java:
##########
@@ -315,6 +316,24 @@ public void testDataSizeAfterSerialization() throws
IOException, ClassNotFoundEx
}
+ /**
+ * Test whether remove is not removing last entry after calling hasNext.
+ * <p>
+ * See <a
href="https://issues.apache.org/jira/browse/COLLECTIONS-802";>COLLECTIONS-802:
ReferenceMap iterator remove violates contract</a>
+ */
+ @Test
+ public void testIteratorLastEntryCanBeRemovedAfterHasNext() {
+ ReferenceMap<Integer, Integer> map = new ReferenceMap<>();
+ map.put(1, 2);
+ Iterator<Map.Entry<Integer, Integer>> iter = map.entrySet().iterator();
+ assertTrue(iter.hasNext());
+ iter.next();
+ // below line should not affect remove
+ assertFalse(iter.hasNext());
+ iter.remove();
+ assertTrue("Expect empty but have entry: " + map, map.isEmpty());
+ }
Review Comment:
Fix looks OK, and the test is failing on `master`, passing on this branch.
We could also modify the test to be more similar to the one reported in the
issue.
```diff
diff --git
a/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
b/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java
index 509ac514..c6625909 100644
-
Issue Time Tracking
-------------------
Worklog Id: (was: 761525)
Time Spent: 0.5h (was: 20m)
> 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: 0.5h
> 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)
