Response to PR comments to mongodb-php-library pull 57

src/Exception/GridFSFileNotFoundException.php

@@ -4,7 +4,7 @@ namespace MongoDB\Exception;
 
 class GridFSFileNotFoundException extends \MongoDB\Driver\Exception\RuntimeException implements Exception
 {
-	public function __construct($fname, $bucketName, $databaseName){
-        parent::__construct(sprintf('Unable to find file by: %s in %s.%s', $fname,$databaseName, $bucketName));
+	public function __construct($fname, $nameSpace){
+        parent::__construct(sprintf('Unable to find file by: %s in %s', $fname,$nameSpace));
 	}
 }

src/GridFS/GridFSCollectionsWrapper.php

+<?php
+namespace MongoDB\GridFS;
+
+use MongoDB\Collection;
+use MongoDB\Database;
+use MongoDB\BSON\ObjectId;
+use MongoDB\Driver\ReadPreference;
+use MongoDB\Driver\WriteConcern;
+use MongoDB\Driver\Manager;
+use MongoDB\Exception\InvalidArgumentException;
+use MongoDB\Exception\InvalidArgumentTypeException;
+use MongoDB\Exception\RuntimeException;
+use MongoDB\Exception\UnexpectedValueException;
+/**
+ * Bucket abstracts the GridFS files and chunks collections.
+ *
+ * @api
+ */
+class GridFSCollectionsWrapper
+{
+    private $filesCollection;
+    private $chunksCollection;
+    private $ensuredIndexes = false;
+    /**
+     * Constructs a GridFS bucket.
+     *
+     * Supported options:
+     *
+     *  * bucketName (string): The bucket name, which will be used as a prefix
+     *    for the files and chunks collections. Defaults to "fs".
+     *
+     *  * chunkSizeBytes (integer): The chunk size in bytes. Defaults to
+     *    261120 (i.e. 255 KiB).
+     *
+     *  * readPreference (MongoDB\Driver\ReadPreference): Read preference.
+     *
+     *  * writeConcern (MongoDB\Driver\WriteConcern): Write concern.
+     *
+     * @param Manager  $manager      Manager instance from the driver
+     * @param string   $databaseName Database name
+     * @param array    $options      Bucket options
+     * @throws InvalidArgumentException
+     */
+    public function __construct(Manager $manager, $databaseName, $options)
+    {
+        $collectionOptions = [];
+        $options += [
+            'bucketName' => 'fs',
+        ];
+        if (isset($options['readPreference'])) {
+            if (! $options['readPreference'] instanceof ReadPreference) {
+                throw new InvalidArgumentTypeException('"readPreference" option', $options['readPreference'], 'MongoDB\Driver\ReadPreference');
+            } else {
+                $collectionOptions['readPreference'] = $options['readPreference'];
+            }
+        }
+        if (isset($options['writeConcern'])) {
+            if (! $options['writeConcern'] instanceof WriteConcern) {
+                throw new InvalidArgumentTypeException('"writeConcern" option', $options['writeConcern'], 'MongoDB\Driver\WriteConcern');
+            } else {
+                $collectionOptions['writeConcern'] = $options['writeConcern'];
+            }
+        }
+        $this->filesCollection = new Collection(
+            $manager,
+            sprintf('%s.%s.files', $databaseName, $options['bucketName']),
+            $collectionOptions
+        );
+        $this->chunksCollection = new Collection(
+            $manager,
+            sprintf('%s.%s.chunks', $databaseName, $options['bucketName']),
+            $collectionOptions
+        );
+    }
+
+    public function chunkInsert($toUpload)
+    {
+        $this->ensureIndexes();
+        $this->chunksCollection->insertOne($toUpload);
+    }
+    public function fileInsert($toUpload)
+    {
+        $this->ensureIndexes();
+        $this->filesCollection->insertOne($toUpload);
+    }
+    public function getChunksCollection()
+    {
+        return $this->chunksCollection;
+    }
+    public function getFilesCollection()
+    {
+        return $this->filesCollection;
+    }
+
+
+    private function ensureIndexes()
+    {
+        if ($this->ensuredIndexes) {
+            return;
+        }
+        if ( ! $this->isFilesCollectionEmpty()) {
+            return;
+        }
+        $this->ensureFilesIndex();
+        $this->ensureChunksIndex();
+        $this->ensuredIndexes = true;
+    }
+    private function ensureChunksIndex()
+    {
+        foreach ($this->chunksCollection->listIndexes() as $index) {
+            if ($index->isUnique() && $index->getKey() === ['files_id' => 1, 'n' => 1]) {
+                return;
+            }
+        }
+        $this->chunksCollection->createIndex(['files_id' => 1, 'n' => 1], ['unique' => true]);
+    }
+    private function ensureFilesIndex()
+    {
+        foreach ($this->filesCollection->listIndexes() as $index) {
+            if ($index->getKey() === ['filename' => 1, 'uploadDate' => 1]) {
+                return;
+            }
+        }
+        $this->filesCollection->createIndex(['filename' => 1, 'uploadDate' => 1]);
+    }
+    private function isFilesCollectionEmpty()
+    {
+        return null === $this->filesCollection->findOne([], [
+            'readPreference' => new ReadPreference(ReadPreference::RP_PRIMARY),
+            'projection' => ['_id' => 1],
+        ]);
+    }
+}

src/GridFS/GridFsDownload.php

@@ -9,7 +9,7 @@ use MongoDB\BSON\ObjectId;
  *
  * @api
  */
-class GridFsDownload extends GridFsStream
+class GridFsDownload
 {
     private $chunksIterator;
     private $bytesSeen=0;
@@ -18,46 +18,41 @@ class GridFsDownload extends GridFsStream
     private $firstCheck=true;
     private $bufferFresh=true;
     private $bufferEmpty=true;
+    private $collectionsWrapper;
+    private $chunkOffset = 0;
+    private $buffer;
+    private $file;
     /**
      * Constructs a GridFS download stream
      *
-     * Supported options:
-     *
-     *  * contentType (string): DEPRECATED content type to be stored with the file.
-     *    This information should now be added to the metadata
-     *
-     *  * aliases (array of strings): DEPRECATED An array of aliases.
-     *    Applications wishing to store aliases should add an aliases field to the
-     *    metadata document instead.
-     *
-     *  * metadata (array or object): User data for the 'metadata' field of the files
-     *    collection document.
-     *
-     *  * writeConcern (MongoDB\Driver\WriteConcern): Write concern.
      *
+     * @param GridFSCollectionsWrapper $collectionsWrapper   File options
+     * @param \MongoDB\BSON\ObjectId   $options              File options
      * @param array                    $options              File options
      * @throws FileNotFoundException
      */
     public function __construct(
-        Bucket $bucket,
+        GridFSCollectionsWrapper $collectionsWrapper,
         $objectId,
         $file = null
         )
     {
+        $this->collectionsWrapper = $collectionsWrapper;
+
         if(!is_null($file)) {
             $this->file = $file;
         } else {
-            $this->file = $bucket->getFilesCollection()->findOne(['_id' => $objectId]);
+            $this->file = $collectionsWrapper->getFilesCollection()->findOne(['_id' => $objectId]);
             if (is_null($this->file)) {
-              throw new \MongoDB\Exception\GridFSFileNotFoundException($objectId, $bucket->getBucketName(), $bucket->getDatabaseName());
+              throw new \MongoDB\Exception\GridFSFileNotFoundException($objectId, $this->collectionsWrapper->getFilesCollection()->getNameSpace());
             }
         }
-        if ($this->file->length > 0) {
-            $cursor = $bucket->getChunksCollection()->find(['files_id' => $this->file->_id], ['sort' => ['n' => 1]]);
+        if ($this->file->length >= 0) {
+            $cursor = $this->collectionsWrapper->getChunksCollection()->find(['files_id' => $this->file->_id], ['sort' => ['n' => 1]]);
             $this->chunksIterator = new \IteratorIterator($cursor);
             $this->numChunks = ceil($this->file->length / $this->file->chunkSize);
         }
-        parent::__construct($bucket);
+        $this->buffer = fopen('php://temp', 'w+');
     }
     /**
     * Reads data from a stream into GridFS
@@ -104,7 +99,7 @@ class GridFsDownload extends GridFsStream
 
     private function advanceChunks()
     {
-        if($this->n >= $this->numChunks) {
+        if($this->chunkOffset >= $this->numChunks) {
             $this->iteratorEmpty=true;
             return false;
         }
@@ -117,22 +112,22 @@ class GridFsDownload extends GridFsStream
         if (!$this->chunksIterator->valid()) {
             throw new \MongoDB\Exception\GridFSCorruptFileException();
         }
-        if ($this->chunksIterator->current()->n != $this->n) {
+        if ($this->chunksIterator->current()->n != $this->chunkOffset) {
             throw new \MongoDB\Exception\GridFSCorruptFileException();
         }
         $chunkSizeIs = strlen($this->chunksIterator->current()->data->getData());
-        if ($this->n == $this->numChunks - 1) {
+        if ($this->chunkOffset == $this->numChunks - 1) {
             $chunkSizeShouldBe = $this->file->length - $this->bytesSeen;
             if($chunkSizeShouldBe != $chunkSizeIs) {
                 throw new \MongoDB\Exception\GridFSCorruptFileException();
             }
-        } else if ($this->n < $this->numChunks - 1) {
+        } else if ($this->chunkOffset < $this->numChunks - 1) {
             if($chunkSizeIs != $this->file->chunkSize) {
                 throw new \MongoDB\Exception\GridFSCorruptFileException();
             }
         }
         $this->bytesSeen+= $chunkSizeIs;
-        $this->n++;
+        $this->chunkOffset++;
         return true;
     }
     public function close()

src/GridFS/GridFsUpload.php

@@ -11,12 +11,17 @@ use MongoDB\BSON;
  *
  * @api
  */
-class GridFsUpload extends GridFsStream
+class GridFsUpload
 {
     private $ctx;
-    private $bufferLength;
+    private $bufferLength = 0;
     private $indexChecker;
-    private $length=0;
+    private $length = 0;
+    private $collectionsWrapper;
+    private $chunkOffset = 0;
+    private $chunkSize;
+    private $buffer;
+    private $file;
     /**
      * Constructs a GridFS upload stream
      *
@@ -34,26 +39,28 @@ class GridFsUpload extends GridFsStream
      *
      *  * writeConcern (MongoDB\Driver\WriteConcern): Write concern.
      *
-     * @param \MongoDB\Collection    $filesCollection  Files Collection
-     * @param \MongoDB\Collection    $chunksCollection Chunks Collection
-     * @param int32                  $chunkSizeBytes   Size of chunk
+     *  * chunkSizeBytes: size of each chunk
+     *
+     * @param GridFSCollectionsWrapper $collectionsWrapper  Files Collection
      * @param string                   $filename            Filename to insert
      * @param array                    $options             File options
      * @throws InvalidArgumentException
      */
     public function __construct(
-        Bucket $bucket,
+        GridFsCollectionsWrapper $collectionsWrapper,
         $filename,
         array $options=[]
         )
     {
-        $this->bufferLength = 0;
         $this->ctx = hash_init('md5');
+        $this->collectionsWrapper = $collectionsWrapper;
+        $this->buffer = fopen('php://temp', 'w+');
+        $this->chunkSize = $options['chunkSizeBytes'];
 
         $uploadDate = time();
         $objectId = new \MongoDB\BSON\ObjectId();
         $main_file = [
-            "chunkSize" => $bucket->getChunkSizeBytes(),
+            "chunkSize" => $this->chunkSize,
             "filename" => $filename,
             "uploadDate" => $uploadDate,
             "_id" => $objectId
@@ -83,7 +90,7 @@ class GridFsUpload extends GridFsStream
             }
         }
         $this->file = array_merge($main_file, $fileOptions);
-        parent::__construct($bucket);
+
     }
     /**
     * Reads data from a stream into GridFS
@@ -93,8 +100,6 @@ class GridFsUpload extends GridFsStream
     */
     public function uploadFromStream($source)
     {
-        $this->bucket->ensureIndexes();
-
         if (!is_resource($source) || get_resource_type($source) != "stream") {
             throw new UnexpectedTypeException('stream', $source);
         } else{
@@ -103,7 +108,7 @@ class GridFsUpload extends GridFsStream
      //       throw new InvalidArgumentException("stream not readable");
             //issue being that php's is_readable reports native streams as not readable like php://temp
         }
-        while ($data = fread($source, $this->bucket->getChunkSizeBytes())) {
+        while ($data = fread($source, $this->chunkSize)) {
             $this->insertChunk($data);
         }
         return $this->fileCollectionInsert();
@@ -116,17 +121,15 @@ class GridFsUpload extends GridFsStream
     */
     public function insertChunks($toWrite)
     {
-        $this->bucket->ensureIndexes();
-
         $readBytes = 0;
         while($readBytes != strlen($toWrite)) {
-            $addToBuffer = substr($toWrite, $readBytes, $this->bucket->getChunkSizeBytes() - $this->bufferLength);
+            $addToBuffer = substr($toWrite, $readBytes, $this->chunkSize - $this->bufferLength);
             fwrite($this->buffer, $addToBuffer);
             $readBytes += strlen($addToBuffer);
             $this->bufferLength += strlen($addToBuffer);
-            if($this->bufferLength == $this->bucket->getChunkSizeBytes()) {
+            if($this->bufferLength == $this->chunkSize) {
                 rewind($this->buffer);
-                $this->insertChunk(fread($this->buffer, $this->bucket->getChunkSizeBytes()));
+                $this->insertChunk(stream_get_contents($this->buffer));
                 ftruncate($this->buffer,0);
                 $this->bufferLength = 0;
             }
@@ -140,30 +143,28 @@ class GridFsUpload extends GridFsStream
     public function close()
     {
         rewind($this->buffer);
-        $cached = fread($this->buffer, $this->bucket->getChunkSizeBytes());
+        $cached = stream_get_contents($this->buffer);
 
         if(strlen($cached) > 0) {
-            insertChunk($cached);
+            $this->insertChunk($cached);
         }
-
         fclose($this->buffer);
-
         $this->fileCollectionInsert();
     }
     private function insertChunk($data)
     {
-        $toUpload = ["files_id" => $this->file['_id'], "n" => $this->n, "data" => new \MongoDB\BSON\Binary($data, \MongoDB\BSON\Binary::TYPE_GENERIC)];
+        $toUpload = ["files_id" => $this->file['_id'], "n" => $this->chunkOffset, "data" => new \MongoDB\BSON\Binary($data, \MongoDB\BSON\Binary::TYPE_GENERIC)];
         hash_update($this->ctx, $data);
-        $this->bucket->chunkInsert($toUpload);
+        $this->collectionsWrapper->chunkInsert($toUpload);
         $this->length += strlen($data);
-        $this->n++;
+        $this->chunkOffset++;
     }
 
     private function fileCollectionInsert()
     {
         $md5 = hash_final($this->ctx);
         $this->file = array_merge($this->file, ['length' => $this->length, 'md5' => $md5]);
-        $this->bucket->fileInsert($this->file);
+        $this->collectionsWrapper->fileInsert($this->file);
         return $this->file['_id'];
     }
 }

src/GridFS/StreamWrapper.php

@@ -21,7 +21,7 @@ class StreamWrapper
     private $protocol = 'gridfs';
     private $mode;
     private $gridFsStream;
-    private $bucket;
+    private $collectionsWrapper;
 
     /**
      * Register the GridFS stream wrapper.
@@ -58,7 +58,7 @@ class StreamWrapper
     {
         $this->initProtocol($path);
         $context = stream_context_get_options($this->context);
-        $this->bucket =$context['gridfs']['bucket'];
+        $this->collectionsWrapper =$context['gridfs']['collectionsWrapper'];
         $this->mode = $mode;
         switch ($this->mode) {
             case 'w' : return $this ->openWriteStream();
@@ -69,17 +69,17 @@ class StreamWrapper
     public function openWriteStream() {
         $context = stream_context_get_options($this->context);
         $options =$context['gridfs']['uploadOptions'];
-        $this->gridFsStream = new GridFsUpload($this->bucket, $this->identifier, $options);
+        $this->gridFsStream = new GridFsUpload($this->collectionsWrapper, $this->identifier, $options);
         return true;
     }
 
     public function openReadStream() {
         $context = stream_context_get_options($this->context);
         if(isset($context['gridfs']['file'])){
-            $this->gridFsStream = new GridFsDownload($this->bucket, null, $context['gridfs']['file']);
+            $this->gridFsStream = new GridFsDownload($this->collectionsWrapper, null, $context['gridfs']['file']);
         } else {
             $objectId = new \MongoDB\BSON\ObjectId($this->identifier);
-            $this->gridFsStream = new GridFsDownload($this->bucket, $objectId);
+            $this->gridFsStream = new GridFsDownload($this->collectionsWrapper, $objectId);
         }
         return true;
     }

tests/GridFS/SpecificationTests.php

@@ -45,7 +45,6 @@ class SpecificationTests extends FunctionalTestCase
                 $options =[];
             }
             $this->bucket = new \MongoDB\GridFS\Bucket($this->manager, $this->getDatabaseName(), $this->fixTypes($options,false));
-            $this->bucketReadWriter = new \MongoDB\GridFS\BucketReadWriter($this->bucket);
             $func = $test['act']['operation'] . "Command";
             $error = null;
             try {
@@ -53,37 +52,35 @@ class SpecificationTests extends FunctionalTestCase
             } catch(\MongoDB\Exception\Exception $e) {
                 $error = $e;
             }
-
             $errors = ['FileNotFound' =>  '\MongoDB\Exception\GridFSFileNotFoundException',
                         'ChunkIsMissing' => '\MongoDB\Exception\GridFSCorruptFileException',
                         'ExtraChunk' => '\MongoDB\Exception\GridFSCorruptFileException',
                         'ChunkIsWrongSize' => '\MongoDB\Exception\GridFSCorruptFileException',
                         'RevisionNotFound' => '\MongoDB\Exception\GridFSFileNotFoundException'
                 ];
-
             if (!isset($test['assert']['error'])) {
                 $this->assertNull($error);
-
             } else {
                 $shouldError = $test['assert']['error'];
                 $this->assertTrue($error instanceof $errors[$shouldError]);
             }
-            $fixedAssert = $this->fixTypes($test['assert'], false);
             if (isset($test['assert']['result'])) {
                     $testResult = $test['assert']['result'];
-                if ($testResult == "&result") {
+                if ($testResult == '&result') {
                     $test['assert']['result'] = $result;
                 }
                 if ($testResult == "void") {
-                    $fixedAssert['result'] = null;
+                    $test['assert']['result'] = null;
                 }
-                $this->assertEquals($result, $fixedAssert['result']);
+                $fixedAssertFalse = $this->fixTypes($test['assert'], false);
+                $this->assertEquals($result, $fixedAssertFalse['result']);
             }
+            $fixedAssertTrue = $this->fixTypes($test['assert'], true);
             if (isset($test['assert']['data'])) {
-                $this->runCommands($fixedAssert['data'], $result);
-                $this->collectionsEqual($this->collections['expected.files'],$this->bucket->getFilesCollection());
+                $this->runCommands($fixedAssertTrue['data'], $result);
+                $this->collectionsEqual($this->collections['expected.files'],$this->bucket->getCollectionsWrapper()->getFilesCollection());
                 if(isset($this->collections['expected.chunks'])) {
-                    $this->collectionsEqual($this->collections['expected.chunks'],$this->bucket->getChunksCollection());
+                    $this->collectionsEqual($this->collections['expected.chunks'],$this->bucket->getCollectionsWrapper()->getChunksCollection());
                 }
             }
         }
@@ -91,8 +88,7 @@ class SpecificationTests extends FunctionalTestCase
 
     public function provideSpecificationTests()
     {
-        $testPath=getcwd().'/tests/GridFS/Specification/tests/download_by_name.json';
-
+        $testPath= __DIR__.'/Specification/tests/*.json';
         $testArgs = [];
         foreach(glob($testPath) as $filename) {
             $fileContents = file_get_contents($filename);
@@ -214,7 +210,7 @@ class SpecificationTests extends FunctionalTestCase
         $stream = fopen('php://temp', 'w+');
         fwrite($stream, $args['source']);
         rewind($stream);
-        $result = $this->bucketReadWriter->uploadFromStream($args['filename'], $stream, $args['options']);
+        $result = $this->bucket->uploadFromStream($args['filename'], $stream, $args['options']);
         fclose($stream);
         return $result;
     }
@@ -224,7 +220,7 @@ class SpecificationTests extends FunctionalTestCase
         $streamWrapper = new \MongoDB\GridFS\StreamWrapper();
         $streamWrapper->register($this->manager);
         $stream = fopen('php://temp', 'w+');
-        $this->bucketReadWriter->downloadToStream($args['id'], $stream);
+        $this->bucket->downloadToStream($args['id'], $stream);
         rewind($stream);
         $result = stream_get_contents($stream);
         fclose($stream);
@@ -233,7 +229,7 @@ class SpecificationTests extends FunctionalTestCase
     function deleteCommand($args)
     {
         $args = $this->fixTypes($args, false);
-        $this->bucketReadWriter->delete($args['id']);
+        $this->bucket->delete($args['id']);
     }
     function download_by_nameCommand($args)
     {
@@ -242,9 +238,9 @@ class SpecificationTests extends FunctionalTestCase
         $streamWrapper->register($this->manager);
         $stream = fopen('php://temp', 'w+');
         if(isset($args['options']['revision'])) {
-            $this->bucketReadWriter->downloadToStreamByName($args['filename'], $stream, $args['options']['revision']);
+            $this->bucket->downloadToStreamByName($args['filename'], $stream, $args['options']['revision']);
         } else {
-            $this->bucketReadWriter->downloadToStreamByName($args['filename'], $stream);
+            $this->bucket->downloadToStreamByName($args['filename'], $stream);
         }
         rewind($stream);
         $result = stream_get_contents($stream);