Refactor GridFS chunk inserts and check for buffer fwrite() errors

Author: jmikola

src/GridFS/StreamWrapper.php

@@ -150,7 +150,7 @@ public function stream_write($data)
         }
 
         try {
-            return $this->stream->insertChunks($data);
+            return $this->stream->writeBytes($data);
         } catch (Exception $e) {
             trigger_error(sprintf('%s: %s', get_class($e), $e->getMessage()), \E_USER_WARNING);
             return false;

src/GridFS/WritableStream.php

@@ -6,6 +6,7 @@
 use MongoDB\BSON\ObjectId;
 use MongoDB\BSON\UTCDateTime;
 use MongoDB\Exception\InvalidArgumentException;
+use MongoDB\Exception\RuntimeException;
 
 /**
  * WritableStream abstracts the process of writing a GridFS file.
@@ -113,14 +114,10 @@ public function close()
             return;
         }
 
-        rewind($this->buffer);
-        $cached = stream_get_contents($this->buffer);
-
-        if (strlen($cached) > 0) {
-            $this->insertChunk($cached);
+        if ($this->bufferLength > 0) {
+            $this->insertChunkFromBuffer();
         }
 
-        fclose($this->buffer);
         $this->fileCollectionInsert();
         $this->isClosed = true;
     }
@@ -151,36 +148,36 @@ public function getSize()
      * Inserts binary data into GridFS via chunks.
      *
      * Data will be buffered internally until chunkSizeBytes are accumulated, at
-     * which point a chunk's worth of data will be inserted and the buffer
-     * reset.
+     * which point a chunk document will be inserted and the buffer reset.
      *
-     * @param string $toWrite Binary data to write
+     * @param string $data Binary data to write
      * @return integer
      */
-    public function insertChunks($toWrite)
+    public function writeBytes($data)
     {
         if ($this->isClosed) {
             // TODO: Should this be an error condition? e.g. BadMethodCallException
             return;
         }
 
-        $readBytes = 0;
+        $bytesRead = 0;
+
+        while ($bytesRead != strlen($data)) {
+            $bytesWritten = fwrite($this->buffer, substr($data, $bytesRead, $this->chunkSize - $this->bufferLength));
 
-        while ($readBytes != strlen($toWrite)) {
-            $addToBuffer = substr($toWrite, $readBytes, $this->chunkSize - $this->bufferLength);
-            fwrite($this->buffer, $addToBuffer);
-            $readBytes += strlen($addToBuffer);
-            $this->bufferLength += strlen($addToBuffer);
+            if ($bytesWritten === false) {
+                throw new RuntimeException('fwrite() failed');
+            }
+
+            $bytesRead += $bytesWritten;
+            $this->bufferLength += $bytesWritten;
 
             if ($this->bufferLength == $this->chunkSize) {
-                rewind($this->buffer);
-                $this->insertChunk(stream_get_contents($this->buffer));
-                ftruncate($this->buffer, 0);
-                $this->bufferLength = 0;
+                $this->insertChunkFromBuffer();
             }
         }
 
-        return $readBytes;
+        return $bytesRead;
     }
 
     private function abort()
@@ -206,22 +203,31 @@ private function fileCollectionInsert()
         return $this->file['_id'];
     }
 
-    private function insertChunk($data)
+    private function insertChunkFromBuffer()
     {
         if ($this->isClosed) {
             // TODO: Should this be an error condition? e.g. BadMethodCallException
             return;
         }
 
-        $toUpload = [
+        $data = stream_get_contents($this->buffer, -1, 0);
+
+        if (strlen($data) == 0) {
+            return;
+        }
+
+        ftruncate($this->buffer, 0);
+        $this->bufferLength = 0;
+
+        $chunk = [
             'files_id' => $this->file['_id'],
             'n' => $this->chunkOffset,
             'data' => new Binary($data, Binary::TYPE_GENERIC),
         ];
 
         hash_update($this->ctx, $data);
 
-        $this->collectionWrapper->insertChunk($toUpload);
+        $this->collectionWrapper->insertChunk($chunk);
         $this->length += strlen($data);
         $this->chunkOffset++;
     }

tests/GridFS/WritableStreamFunctionalTest.php

@@ -55,10 +55,10 @@ public function provideInvalidConstructorOptions()
     /**
      * @dataProvider provideInputDataAndExpectedMD5
      */
-    public function testInsertChunksCalculatesMD5($input, $expectedMD5)
+    public function testWriteBytesCalculatesMD5($input, $expectedMD5)
     {
         $stream = new WritableStream($this->collectionWrapper, 'filename');
-        $stream->insertChunks($input);
+        $stream->writeBytes($input);
         $stream->close();
 
         $fileDocument = $this->filesCollection->findOne(