PHPLIB-202: Use stream_copy_to_stream() in Bucket upload/download methods

This also improves test coverage for downloadToStreamByName().

src/GridFS/Bucket.php

@@ -104,17 +104,15 @@ class Bucket
      * @param mixed    $id          File ID
      * @param resource $destination Writable Stream
      * @throws FileNotFoundException
+     * @throws InvalidArgumentException if $destination is not a stream
      */
     public function downloadToStream($id, $destination)
     {
-        $file = $this->collectionWrapper->findFileById($id);
-
-        if ($file === null) {
-            throw FileNotFoundException::byId($id, $this->getFilesNamespace());
+        if ( ! is_resource($destination) || get_resource_type($destination) != "stream") {
+            throw InvalidArgumentException::invalidType('$destination', $destination, 'resource');
         }
 
-        $stream = new ReadableStream($this->collectionWrapper, $file);
-        $stream ->downloadToStream($destination);
+        stream_copy_to_stream($this->openDownloadStream($id), $destination);
     }
 
     /**
@@ -140,19 +138,15 @@ class Bucket
      * @param resource $destination Writable Stream
      * @param array    $options     Download options
      * @throws FileNotFoundException
+     * @throws InvalidArgumentException if $destination is not a stream
      */
     public function downloadToStreamByName($filename, $destination, array $options = [])
     {
-        $options += ['revision' => -1];
-
-        $file = $this->collectionWrapper->findFileByFilenameAndRevision($filename, $options['revision']);
-
-        if ($file === null) {
-            throw FileNotFoundException::byFilenameAndRevision($filename, $options['revision'], $this->getFilesNamespace());
+        if ( ! is_resource($destination) || get_resource_type($destination) != "stream") {
+            throw InvalidArgumentException::invalidType('$destination', $destination, 'resource');
         }
 
-        $stream = new ReadableStream($this->collectionWrapper, $file);
-        $stream->downloadToStream($destination);
+        stream_copy_to_stream($this->openDownloadStreamByName($filename, $options), $destination);
     }
 
     /**
@@ -339,15 +333,18 @@ class Bucket
      * @param resource $source   Readable stream
      * @param array    $options  Stream options
      * @return ObjectId ID of the newly created GridFS file
-     * @throws InvalidArgumentException
+     * @throws InvalidArgumentException if $source is not a stream
      */
     public function uploadFromStream($filename, $source, array $options = [])
     {
-        $options += ['chunkSizeBytes' => $this->options['chunkSizeBytes']];
+        if ( ! is_resource($source) || get_resource_type($source) != "stream") {
+            throw InvalidArgumentException::invalidType('$source', $source, 'resource');
+        }
 
-        $stream = new WritableStream($this->collectionWrapper, $filename, $options);
+        $destination = $this->openUploadStream($filename, $options);
+        stream_copy_to_stream($source, $destination);
 
-        return $stream->uploadFromStream($source);
+        return $this->getIdFromStream($destination);
     }
 
     /**

src/GridFS/ReadableStream.php

@@ -129,24 +129,6 @@ class ReadableStream
         return $output;
     }
 
-    /**
-     * Writes the contents of this GridFS file to a writable stream.
-     *
-     * @param resource $destination Writable stream
-     * @throws InvalidArgumentException
-     */
-    public function downloadToStream($destination)
-    {
-        if ( ! is_resource($destination) || get_resource_type($destination) != "stream") {
-            throw InvalidArgumentException::invalidType('$destination', $destination, 'resource');
-        }
-
-        while ($this->advanceChunks()) {
-            // TODO: Should we be checking for fwrite errors here?
-            fwrite($destination, $this->chunksIterator->current()->data->getData());
-        }
-    }
-
     /**
      * Return the stream's ID (i.e. file document identifier).
      *

src/GridFS/WritableStream.php

@@ -189,26 +189,6 @@ class WritableStream
         return $this->isClosed;
     }
 
-    /**
-     * Writes the contents of a readable stream to a GridFS file.
-     *
-     * @param resource $source Readable stream
-     * @return ObjectId
-     * @throws InvalidArgumentException
-     */
-    public function uploadFromStream($source)
-    {
-        if ( ! is_resource($source) || get_resource_type($source) != "stream") {
-            throw InvalidArgumentException::invalidType('$source', $source, 'resource');
-        }
-
-        while ($data = $this->readChunk($source)) {
-            $this->insertChunk($data);
-        }
-
-        return $this->fileCollectionInsert();
-    }
-
     private function abort()
     {
         $this->collectionWrapper->deleteChunksByFilesId($this->file['_id']);

tests/GridFS/BucketFunctionalTest.php

@@ -172,6 +172,20 @@ class BucketFunctionalTest extends FunctionalTestCase
         $this->assertStreamContents($input, $destination);
     }
 
+    /**
+     * @expectedException MongoDB\Exception\InvalidArgumentException
+     * @dataProvider provideInvalidStreamValues
+     */
+    public function testDownloadToStreamShouldRequireDestinationStream($destination)
+    {
+        $this->bucket->downloadToStream('id', $destination);
+    }
+
+    public function provideInvalidStreamValues()
+    {
+        return $this->wrapValuesForDataProvider([null, 123, 'foo', [], hash_init('md5')]);
+    }
+
     /**
      * @expectedException MongoDB\GridFS\Exception\FileNotFoundException
      */
@@ -180,6 +194,73 @@ class BucketFunctionalTest extends FunctionalTestCase
         $this->bucket->downloadToStream('nonexistent-id', $this->createStream());
     }
 
+    public function testDownloadToStreamByName()
+    {
+        $this->bucket->uploadFromStream('filename', $this->createStream('foo'));
+        $this->bucket->uploadFromStream('filename', $this->createStream('bar'));
+        $this->bucket->uploadFromStream('filename', $this->createStream('baz'));
+
+        $destination = $this->createStream();
+        $this->bucket->downloadToStreamByName('filename', $destination);
+        $this->assertStreamContents('baz', $destination);
+
+        $destination = $this->createStream();
+        $this->bucket->downloadToStreamByName('filename', $destination, ['revision' => -3]);
+        $this->assertStreamContents('foo', $destination);
+
+        $destination = $this->createStream();
+        $this->bucket->downloadToStreamByName('filename', $destination, ['revision' => -2]);
+        $this->assertStreamContents('bar', $destination);
+
+        $destination = $this->createStream();
+        $this->bucket->downloadToStreamByName('filename', $destination, ['revision' => -1]);
+        $this->assertStreamContents('baz', $destination);
+
+        $destination = $this->createStream();
+        $this->bucket->downloadToStreamByName('filename', $destination, ['revision' => 0]);
+        $this->assertStreamContents('foo', $destination);
+
+        $destination = $this->createStream();
+        $this->bucket->downloadToStreamByName('filename', $destination, ['revision' => 1]);
+        $this->assertStreamContents('bar', $destination);
+
+        $destination = $this->createStream();
+        $this->bucket->downloadToStreamByName('filename', $destination, ['revision' => 2]);
+        $this->assertStreamContents('baz', $destination);
+    }
+
+    /**
+     * @expectedException MongoDB\Exception\InvalidArgumentException
+     * @dataProvider provideInvalidStreamValues
+     */
+    public function testDownloadToStreamByNameShouldRequireDestinationStream($destination)
+    {
+        $this->bucket->downloadToStreamByName('filename', $destination);
+    }
+
+    /**
+     * @expectedException MongoDB\GridFS\Exception\FileNotFoundException
+     * @dataProvider provideNonexistentFilenameAndRevision
+     */
+    public function testDownloadToStreamByNameShouldRequireFilenameAndRevisionToExist($filename, $revision)
+    {
+        $this->bucket->uploadFromStream('filename', $this->createStream('foo'));
+        $this->bucket->uploadFromStream('filename', $this->createStream('bar'));
+
+        $destination = $this->createStream();
+        $this->bucket->downloadToStreamByName($filename, $destination, ['revision' => $revision]);
+    }
+
+    public function provideNonexistentFilenameAndRevision()
+    {
+        return [
+            ['filename', 2],
+            ['filename', -3],
+            ['nonexistent-filename', 0],
+            ['nonexistent-filename', -1],
+        ];
+    }
+
     public function testDrop()
     {
         $this->bucket->uploadFromStream('filename', $this->createStream('foobar'));
@@ -306,16 +387,6 @@ class BucketFunctionalTest extends FunctionalTestCase
         $this->bucket->openDownloadStream($filename, ['revision' => $revision]);
     }
 
-    public function provideNonexistentFilenameAndRevision()
-    {
-        return [
-            ['filename', 2],
-            ['filename', -3],
-            ['nonexistent-filename', 0],
-            ['nonexistent-filename', -1],
-        ];
-    }
-
     public function testOpenUploadStream()
     {
         $stream = $this->bucket->openUploadStream('filename');
@@ -401,6 +472,15 @@ class BucketFunctionalTest extends FunctionalTestCase
         $this->assertSameDocument(['foo' => 'bar'], $fileDocument['metadata']);
     }
 
+    /**
+     * @expectedException MongoDB\Exception\InvalidArgumentException
+     * @dataProvider provideInvalidStreamValues
+     */
+    public function testUploadFromStreamShouldRequireSourceStream($source)
+    {
+        $this->bucket->uploadFromStream('filename', $source);
+    }
+
     public function testUploadingAnEmptyFile()
     {
         $id = $this->bucket->uploadFromStream('filename', $this->createStream(''));

tests/GridFS/WritableStreamFunctionalTest.php

@@ -80,21 +80,4 @@ class WritableStreamFunctionalTest extends FunctionalTestCase
             [str_repeat('foobar', 87041), '95e78f624f8e745bcfd2d11691fa601e'],
         ];
     }
-
-    /**
-     * @dataProvider provideInputDataAndExpectedMD5
-     */
-    public function testUploadFromStreamCalculatesMD5($input, $expectedMD5)
-    {
-        $stream = new WritableStream($this->collectionWrapper, 'filename');
-        $stream->uploadFromStream($this->createStream($input));
-        //$stream->close();
-
-        $fileDocument = $this->filesCollection->findOne(
-            ['_id' => $stream->getId()],
-            ['projection' => ['md5' => 1, '_id' => 0]]
-        );
-
-        $this->assertSameDocument(['md5' => $expectedMD5], $fileDocument);
-    }
 }