PHPLIB-81: CachingIterator fixes for rewind and count

Ensure that a call to CachingIterator::rewind() no longer exhausts results up front, which would impose a BufferedIterator design.

Initializing the cache to an empty array also ensures that we can count an empty result set.

Author: jmikola

src/Model/CachingIterator.php

@@ -33,11 +33,20 @@
  */
 class CachingIterator implements Countable, Iterator
 {
-    private $items;
+    private $items = [];
     private $iterator;
+    private $iteratorAdvanced = false;
     private $iteratorExhausted = false;
 
     /**
+     * Constructor.
+     *
+     * Initialize the iterator and stores the first item in the cache. This
+     * effectively rewinds the Traversable and the wrapping Generator, which
+     * will execute up to its first yield statement. Additionally, this mimics
+     * behavior of the SPL iterators and allows users to omit an explicit call
+     * to rewind() before using the other methods.
+     *
      * @param Traversable $traversable
      */
     public function __construct(Traversable $traversable)
@@ -95,7 +104,13 @@ public function next()
      */
     public function rewind()
     {
-        $this->exhaustIterator();
+        /* If the iterator has advanced, exhaust it now so that future iteration
+         * can rely on the cache.
+         */
+        if ($this->iteratorAdvanced) {
+            $this->exhaustIterator();
+        }
+
         reset($this->items);
     }
 
@@ -143,6 +158,7 @@ private function wrapTraversable(Traversable $traversable)
     {
         foreach ($traversable as $key => $value) {
             yield $key => $value;
+            $this->iteratorAdvanced = true;
         }
 
         $this->iteratorExhausted = true;

tests/Model/CachingIteratorTest.php

@@ -3,6 +3,7 @@
 namespace MongoDB\Tests\Model;
 
 use MongoDB\Model\CachingIterator;
+use Exception;
 
 class CachingIteratorTest extends \PHPUnit_Framework_TestCase
 {
@@ -19,6 +20,15 @@ public function testTraversingGeneratorConsumesIt()
         $this->assertSame([1, 2, 3], iterator_to_array($iterator));
     }
 
+    public function testConstructorRewinds()
+    {
+        $iterator = new CachingIterator($this->getTraversable([1, 2, 3]));
+
+        $this->assertTrue($iterator->valid());
+        $this->assertSame(0, $iterator->key());
+        $this->assertSame(1, $iterator->current());
+    }
+
     public function testIteration()
     {
         $iterator = new CachingIterator($this->getTraversable([1, 2, 3]));
@@ -34,13 +44,44 @@ public function testIteration()
         $this->assertFalse($iterator->valid());
     }
 
+    public function testIterationWithEmptySet()
+    {
+        $iterator = new CachingIterator($this->getTraversable([]));
+
+        $iterator->rewind();
+        $this->assertFalse($iterator->valid());
+    }
+
+    public function testPartialIterationDoesNotExhaust()
+    {
+        $traversable = $this->getTraversableThatThrows([1, 2, new Exception]);
+        $iterator = new CachingIterator($traversable);
+
+        $expectedKey = 0;
+        $expectedItem = 1;
+
+        foreach ($iterator as $key => $item) {
+            $this->assertSame($expectedKey++, $key);
+            $this->assertSame($expectedItem++, $item);
+
+            if ($key === 1) {
+                break;
+            }
+        }
+
+        $this->assertTrue($iterator->valid());
+    }
+
     public function testRewindAfterPartialIteration()
     {
         $iterator = new CachingIterator($this->getTraversable([1, 2, 3]));
 
+        $iterator->rewind();
+        $this->assertTrue($iterator->valid());
+        $this->assertSame(0, $iterator->key());
         $this->assertSame(1, $iterator->current());
-        $iterator->next();
 
+        $iterator->next();
         $this->assertSame([1, 2, 3], iterator_to_array($iterator));
     }
 
@@ -53,14 +94,37 @@ public function testCount()
     public function testCountAfterPartialIteration()
     {
         $iterator = new CachingIterator($this->getTraversable([1, 2, 3]));
+
+        $iterator->rewind();
+        $this->assertTrue($iterator->valid());
+        $this->assertSame(0, $iterator->key());
+        $this->assertSame(1, $iterator->current());
+
         $iterator->next();
         $this->assertCount(3, $iterator);
     }
 
+    public function testCountWithEmptySet()
+    {
+        $iterator = new CachingIterator($this->getTraversable([]));
+        $this->assertCount(0, $iterator);
+    }
+
     private function getTraversable($items)
     {
         foreach ($items as $item) {
             yield $item;
         }
     }
+
+    private function getTraversableThatThrows($items)
+    {
+        foreach ($items as $item) {
+            if ($item instanceof Exception) {
+                throw $item;
+            } else {
+                yield $item;
+            }
+        }
+    }
 }