Fix buffer bug

Author: kvwalker

src/GridFS/ReadableStream.php

@@ -186,11 +186,21 @@ public function seek($offset)
         $this->chunkOffset = (integer) floor($offset / $this->chunkSize);
         $this->bufferOffset = $offset % $this->chunkSize;
 
+        if ($lastChunkOffset === $this->chunkOffset) {
+            return;
+        }
+
+        if ($this->chunksIterator === null) {
+            return;
+        }
+
+        // Clear the buffer since the current chunk will be changed
+        $this->buffer = null;
+
         /* If we are seeking to a previous chunk, we need to reinitialize the
          * chunk iterator.
          */
         if ($lastChunkOffset > $this->chunkOffset) {
-            $this->buffer = null;
             $this->chunksIterator = null;
             return;
         }
@@ -200,12 +210,8 @@ public function seek($offset)
          * to $this->chunkOffset.
          */
         $numChunks = $this->chunkOffset - $lastChunkOffset;
-        $i = 0;
-        if ($this->chunksIterator !== null) {
-            while ($i < $numChunks) {
-                $this->chunksIterator->next();
-                $i++;
-            }
+        for ($i = 0; $i < $numChunks; $i++) {
+            $this->chunksIterator->next();
         }
     }
 

tests/GridFS/ReadableStreamFunctionalTest.php

@@ -5,6 +5,8 @@
 use MongoDB\BSON\Binary;
 use MongoDB\GridFS\CollectionWrapper;
 use MongoDB\GridFS\ReadableStream;
+use MongoDB\Tests\CommandObserver;
+use stdClass;
 
 /**
  * Functional tests for the internal ReadableStream class.
@@ -200,31 +202,108 @@ public function testSeekOutOfRange()
         $stream->seek(11);
     }
 
-    public function testSeekPreviousChunk()
+    /**
+     * @dataProvider providePreviousChunkSeekOffsetAndBytes
+     */
+    public function testSeekPreviousChunk($offset, $length, $expectedBytes)
     {
         $fileDocument = $this->collectionWrapper->findFileById('length-10');
         $stream = new ReadableStream($this->collectionWrapper, $fileDocument);
 
-        $stream->readBytes(1);
-        $stream->seek(5);
-        $stream->seek(2);
-        $stream->readBytes(1);
+        // Read to initialize and advance the chunk iterator to the last chunk
+        $this->assertSame('abcdefghij', $stream->readBytes(10));
+
+        $commands = [];
+
+        (new CommandObserver)->observe(
+            function() use ($stream, $offset, $length, $expectedBytes) {
+                $stream->seek($offset);
+                $this->assertSame($expectedBytes, $stream->readBytes($length));
+            },
+            function(stdClass $command) use (&$commands) {
+                $commands[] = key((array) $command);
+            }
+        );
+
+        $this->assertSame(['find'], $commands);
     }
 
-    public function testSeekSubsequentChunk()
+    public function providePreviousChunkSeekOffsetAndBytes()
+    {
+        return [
+            [0, 4, 'abcd'],
+            [2, 4, 'cdef'],
+            [4, 4, 'efgh'],
+            [6, 4, 'ghij'],
+        ];
+    }
+
+    /**
+     * @dataProvider provideSameChunkSeekOffsetAndBytes
+     */
+    public function testSeekSameChunk($offset, $length, $expectedBytes)
     {
         $fileDocument = $this->collectionWrapper->findFileById('length-10');
+        $stream = new ReadableStream($this->collectionWrapper, $fileDocument);
+
+        // Read to initialize and advance the chunk iterator to the middle chunk
+        $this->assertSame('abcdef', $stream->readBytes(6));
 
-        $observer = $this->getMockBuilder(ReadableStream::class)
-                         ->setConstructorArgs(array($this->collectionWrapper, $fileDocument))
-                         ->getMock();
+        $commands = [];
 
-        $observer->expects($this->never())
-                 ->method('initChunksIterator');
+        (new CommandObserver)->observe(
+            function() use ($stream, $offset, $length, $expectedBytes) {
+                $stream->seek($offset);
+                $this->assertSame($expectedBytes, $stream->readBytes($length));
+            },
+            function(stdClass $command) use (&$commands) {
+                $commands[] = key((array) $command);
+            }
+        );
 
-        $observer->readBytes(1);
-        $observer->seek(5);
-        $observer->seek(2);
-        $observer->readBytes(1);
+        $this->assertSame([], $commands);
+    }
+
+    public function provideSameChunkSeekOffsetAndBytes()
+    {
+        return [
+            [4, 4, 'efgh'],
+            [6, 4, 'ghij'],
+        ];
+    }
+
+    /**
+     * @dataProvider provideSubsequentChunkSeekOffsetAndBytes
+     */
+    public function testSeekSubsequentChunk($offset, $length, $expectedBytes)
+    {
+        $fileDocument = $this->collectionWrapper->findFileById('length-10');
+        $stream = new ReadableStream($this->collectionWrapper, $fileDocument);
+
+        // Read to initialize the chunk iterator to the first chunk
+        $this->assertSame('a', $stream->readBytes(1));
+
+        $commands = [];
+
+        (new CommandObserver)->observe(
+            function() use ($stream, $offset, $length, $expectedBytes) {
+                $stream->seek($offset);
+                $this->assertSame($expectedBytes, $stream->readBytes($length));
+            },
+            function(stdClass $command) use (&$commands) {
+                $commands[] = key((array) $command);
+            }
+        );
+
+        $this->assertSame([], $commands);
+    }
+
+    public function provideSubsequentChunkSeekOffsetAndBytes()
+    {
+        return [
+            [4, 4, 'efgh'],
+            [6, 4, 'ghij'],
+            [8, 2, 'ij'],
+        ];
     }
 }