AbstractMapBag.BagIterator.remove breaks class invariants when removing last entry

bug 35747, from Steve Clark

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@219131 13f79535-47bb-0310-9956-ffa450edef68

RELEASE-NOTES.html

@@ -83,6 +83,7 @@ If this causes major headaches to anyone please contact commons-dev at jakarta.a
 <li>MultiHashMap.putAll(multimap) - Was adding the collection as a single item rather than individually [35631]</li>
 <li>Flat3Map.equals() - Fix to make flat mode comparison actually work [34917]</li>
 <li>TransformedMap.putAll - Now allows putAll of an empty map [34686]</li>
+<li>AbstractMapBag.BagIterator.remove - Removing the last entry used to break the class invariants [35747]</li>
 <li>BoundedFifoBuffer/CircularFifoBuffer - Fix serialization to work in case where buffer serialized when full [31433]</li>
 <li>BoundedFifoBuffer - Fix iterator remove bug causing ArrayIndexOutOfBounds error [33071]</li>
 <li>IteratorChain.remove() - Fix to avoid IllegalStateException when one of the underlying iterators is a FilterIterator [34267]</li>

project.xml

@@ -171,6 +171,9 @@
     <contributor>
       <name>Ram Chidambaram</name>
     </contributor>
+    <contributor>
+      <name>Steve Clark</name>
+    </contributor>
     <contributor>
       <name>Eric Crampton</name>
     </contributor>

src/java/org/apache/commons/collections/bag/AbstractMapBag.java

@@ -43,6 +43,7 @@ import org.apache.commons.collections.set.UnmodifiableSet;
  * @author Michael A. Smith
  * @author Stephen Colebourne
  * @author Janek Bogucki
+ * @author Steve Clark
  */
 public abstract class AbstractMapBag implements Bag {
     
@@ -221,12 +222,12 @@ public abstract class AbstractMapBag implements Bag {
                 throw new IllegalStateException();
             }
             MutableInteger mut = (MutableInteger) current.getValue();
-            if (mut.value > 0) {
+            if (mut.value > 1) {
                 mut.value--;
-                parent.size--;
             } else {
                 entryIterator.remove();
             }
+            parent.size--;
             canRemove = false;
         }
     }

src/test/org/apache/commons/collections/bag/AbstractTestBag.java

@@ -319,6 +319,27 @@ public abstract class AbstractTestBag extends AbstractTestObject {
         assertEquals(1, bag.size());
     }
     
+    public void testIteratorRemoveProtectsInvariants() {
+        Bag bag = makeBag();
+        bag.add("A");
+        bag.add("A");
+        assertEquals(2, bag.size());
+        Iterator it = bag.iterator();
+        assertEquals("A", it.next());
+        assertEquals(true, it.hasNext());
+        it.remove();
+        assertEquals(1, bag.size());
+        assertEquals(true, it.hasNext());
+        assertEquals("A", it.next());
+        assertEquals(false, it.hasNext());
+        it.remove();
+        assertEquals(0, bag.size());
+        assertEquals(false, it.hasNext());
+        
+        Iterator it2 = bag.iterator();
+        assertEquals(false, it2.hasNext());
+    }
+    
     public void testToArray() {
         Bag bag = makeBag();
         bag.add("A");