Fixed bug in iterator remove. Shift operation was not incrementing indexes

properly.  Also improved documentation.
BZ #33071
Reported by: Amir Tahvildaran


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

RELEASE-NOTES.html

@@ -52,6 +52,7 @@ There are no new deprecations.
 <ul>
 <li>FastArrayList - Fix iterators and views to work better in multithreaded environments</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>MultiHashMap.remove(key, item) - Was returning the item even when nothing was removed [32366]</li>
 </ul>
 
@@ -61,4 +62,4 @@ There are no new deprecations.
 <li>ListOrderedSet.decorate(List) - Better comment [32073]</li>
 </ul>
 </body>
-</html>
+</html>

src/java/org/apache/commons/collections/buffer/BoundedFifoBuffer.java

@@ -53,7 +53,7 @@ import org.apache.commons.collections.BufferUnderflowException;
  * This class is Serializable from Commons Collections 3.1.
  *
  * @since Commons Collections 3.0 (previously in main package v2.1)
- * @version $Revision: 1.9 $ $Date: 2004/10/16 22:23:40 $
+ * @version $Revision: 1.10 $ $Date: 2005/01/15 22:47:40 $
  * 
  * @author Avalon
  * @author Berin Loritsch
@@ -67,10 +67,24 @@ public class BoundedFifoBuffer extends AbstractCollection
     /** Serialization version */
     private static final long serialVersionUID = 5603722811189451017L;
 
+    /** Underlying storage array */
     private transient Object[] elements;
+    
+    /** Array index of first (oldest) buffer element */
     private transient int start = 0;
+    
+    /** 
+     * Index mod maxElements of the array position following the last buffer
+     * element.  Buffer elements start at elements[start] and "wrap around"
+     * elements[maxElements-1], ending at elements[decrement(end)].  
+     * For example, elements = {c,a,b}, start=1, end=1 corresponds to 
+     * the buffer [a,b,c].
+     */
     private transient int end = 0;
+    
     private transient boolean full = false;
+    
+    /** Capacity of the buffer */
     private final int maxElements;
 
     /**
@@ -348,8 +362,8 @@ public class BoundedFifoBuffer extends AbstractCollection
                         elements[i - 1] = elements[0];
                         i = 0;
                     } else {
-                        elements[i - 1] = elements[i];
-                        i++;
+                        elements[decrement(i)] = elements[i];
+                        i = increment(i);
                     }
                 }
 

src/test/org/apache/commons/collections/buffer/TestBoundedFifoBuffer.java

@@ -28,7 +28,7 @@ import org.apache.commons.collections.collection.AbstractTestCollection;
 /**
  * Test cases for BoundedFifoBuffer.
  * 
- * @version $Revision: 1.4 $ $Date: 2004/06/02 23:12:45 $
+ * @version $Revision: 1.5 $ $Date: 2005/01/15 22:47:40 $
  * 
  * @author Paul Jack
  */
@@ -168,6 +168,17 @@ public class TestBoundedFifoBuffer extends AbstractTestCollection {
     public String getCompatibilityVersion() {
         return "3.1";
     }
+    
+    // BZ 33071 -- gets start=end=1 before removal of interior element
+    public void testShift() {
+        BoundedFifoBuffer fifo = new BoundedFifoBuffer(3);
+        fifo.add("a");
+        fifo.add("b");
+        fifo.add("c");
+        fifo.remove();
+        fifo.add("e");
+        fifo.remove("c");
+    }
 
 //    public void testCreate() throws Exception {
 //        resetEmpty();