Merge pull request #304

Author: jmikola

src/GridFS/Bucket.php

@@ -80,6 +80,10 @@ public function __construct(Manager $manager, $databaseName, array $options = []
             throw InvalidArgumentException::invalidType('"chunkSizeBytes" option', $options['chunkSizeBytes'], 'integer');
         }
 
+        if (isset($options['chunkSizeBytes']) && $options['chunkSizeBytes'] < 1) {
+            throw new InvalidArgumentException(sprintf('Expected "chunkSizeBytes" option to be >= 1, %d given', $options['chunkSizeBytes']));
+        }
+
         if (isset($options['readConcern']) && ! $options['readConcern'] instanceof ReadConcern) {
             throw InvalidArgumentException::invalidType('"readConcern" option', $options['readConcern'], 'MongoDB\Driver\ReadConcern');
         }

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

@@ -5,7 +5,9 @@
 use MongoDB\BSON\Binary;
 use MongoDB\BSON\ObjectId;
 use MongoDB\BSON\UTCDateTime;
+use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException;
 use MongoDB\Exception\InvalidArgumentException;
+use MongoDB\Exception\RuntimeException;
 
 /**
  * WritableStream abstracts the process of writing a GridFS file.
@@ -16,8 +18,7 @@ class WritableStream
 {
     private static $defaultChunkSizeBytes = 261120;
 
-    private $buffer;
-    private $bufferLength = 0;
+    private $buffer = '';
     private $chunkOffset = 0;
     private $chunkSize;
     private $collectionWrapper;
@@ -66,6 +67,10 @@ public function __construct(CollectionWrapper $collectionWrapper, $filename, arr
             throw InvalidArgumentException::invalidType('"chunkSizeBytes" option', $options['chunkSizeBytes'], 'integer');
         }
 
+        if (isset($options['chunkSizeBytes']) && $options['chunkSizeBytes'] < 1) {
+            throw new InvalidArgumentException(sprintf('Expected "chunkSizeBytes" option to be >= 1, %d given', $options['chunkSizeBytes']));
+        }
+
         if (isset($options['contentType']) && ! is_string($options['contentType'])) {
             throw InvalidArgumentException::invalidType('"contentType" option', $options['contentType'], 'string');
         }
@@ -76,15 +81,13 @@ public function __construct(CollectionWrapper $collectionWrapper, $filename, arr
 
         $this->chunkSize = $options['chunkSizeBytes'];
         $this->collectionWrapper = $collectionWrapper;
-        $this->buffer = fopen('php://memory', 'w+b');
         $this->ctx = hash_init('md5');
 
         $this->file = [
             '_id' => $options['_id'],
             'chunkSize' => $this->chunkSize,
             'filename' => (string) $filename,
-            // TODO: This is necessary until PHPC-536 is implemented
-            'uploadDate' => new UTCDateTime((int) floor(microtime(true) * 1000)),
+            'uploadDate' => new UTCDateTime,
         ] + array_intersect_key($options, ['aliases' => 1, 'contentType' => 1, 'metadata' => 1]);
     }
 
@@ -113,14 +116,10 @@ public function close()
             return;
         }
 
-        rewind($this->buffer);
-        $cached = stream_get_contents($this->buffer);
-
-        if (strlen($cached) > 0) {
-            $this->insertChunk($cached);
+        if (strlen($this->buffer) > 0) {
+            $this->insertChunkFromBuffer();
         }
 
-        fclose($this->buffer);
         $this->fileCollectionInsert();
         $this->isClosed = true;
     }
@@ -151,77 +150,87 @@ 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 ($readBytes != strlen($toWrite)) {
-            $addToBuffer = substr($toWrite, $readBytes, $this->chunkSize - $this->bufferLength);
-            fwrite($this->buffer, $addToBuffer);
-            $readBytes += strlen($addToBuffer);
-            $this->bufferLength += strlen($addToBuffer);
+        while ($bytesRead != strlen($data)) {
+            $initialBufferLength = strlen($this->buffer);
+            $this->buffer .= substr($data, $bytesRead, $this->chunkSize - $initialBufferLength);
+            $bytesRead += strlen($this->buffer) - $initialBufferLength;
 
-            if ($this->bufferLength == $this->chunkSize) {
-                rewind($this->buffer);
-                $this->insertChunk(stream_get_contents($this->buffer));
-                ftruncate($this->buffer, 0);
-                $this->bufferLength = 0;
+            if (strlen($this->buffer) == $this->chunkSize) {
+                $this->insertChunkFromBuffer();
             }
         }
 
-        return $readBytes;
+        return $bytesRead;
     }
 
     private function abort()
     {
-        $this->collectionWrapper->deleteChunksByFilesId($this->file['_id']);
+        try {
+            $this->collectionWrapper->deleteChunksByFilesId($this->file['_id']);
+        } catch (DriverRuntimeException $e) {
+            // We are already handling an error if abort() is called, so suppress this
+        }
+
         $this->isClosed = true;
     }
 
     private function fileCollectionInsert()
     {
-        if ($this->isClosed) {
-            // TODO: Should this be an error condition? e.g. BadMethodCallException
-            return;
-        }
-
         $md5 = hash_final($this->ctx);
 
         $this->file['length'] = $this->length;
         $this->file['md5'] = $md5;
 
-        $this->collectionWrapper->insertFile($this->file);
+        try {
+            $this->collectionWrapper->insertFile($this->file);
+        } catch (DriverRuntimeException $e) {
+            $this->abort();
+
+            throw $e;
+        }
 
         return $this->file['_id'];
     }
 
-    private function insertChunk($data)
+    private function insertChunkFromBuffer()
     {
-        if ($this->isClosed) {
-            // TODO: Should this be an error condition? e.g. BadMethodCallException
+        if (strlen($this->buffer) == 0) {
             return;
         }
 
-        $toUpload = [
+        $data = $this->buffer;
+        $this->buffer = '';
+
+        $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);
+        try {
+            $this->collectionWrapper->insertChunk($chunk);
+        } catch (DriverRuntimeException $e) {
+            $this->abort();
+
+            throw $e;
+        }
+
         $this->length += strlen($data);
         $this->chunkOffset++;
     }

tests/GridFS/BucketFunctionalTest.php

@@ -68,6 +68,15 @@ public function provideInvalidConstructorOptions()
         return $options;
     }
 
+    /**
+     * @expectedException MongoDB\Exception\InvalidArgumentException
+     * @expectedExceptionMessage Expected "chunkSizeBytes" option to be >= 1, 0 given
+     */
+    public function testConstructorShouldRequireChunkSizeBytesOptionToBePositive()
+    {
+        new Bucket($this->manager, $this->getDatabaseName(), ['chunkSizeBytes' => 0]);
+    }
+
     /**
      * @dataProvider provideInputDataAndExpectedChunks
      */

tests/GridFS/SpecFunctionalTest.php

@@ -150,9 +150,7 @@ private function convertTypes(array $data, $createBinary = true)
             }
 
             if (isset($value['$date'])) {
-                // TODO: This is necessary until PHPC-536 is implemented
-                $milliseconds = floor((new DateTime($value['$date']))->format('U.u') * 1000);
-                $value = new UTCDateTime((int) $milliseconds);
+                $value = new UTCDateTime(new DateTime($value['$date']));
                 return;
             }
 

tests/GridFS/WritableStreamFunctionalTest.php

@@ -52,13 +52,22 @@ public function provideInvalidConstructorOptions()
         return $options;
     }
 
+    /**
+     * @expectedException MongoDB\Exception\InvalidArgumentException
+     * @expectedExceptionMessage Expected "chunkSizeBytes" option to be >= 1, 0 given
+     */
+    public function testConstructorShouldRequireChunkSizeBytesOptionToBePositive()
+    {
+        new WritableStream($this->collectionWrapper, 'filename', ['chunkSizeBytes' => 0]);
+    }
+
     /**
      * @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(