PHPLIB-522: Loosen index equality checks for GridFS

mongodb · Apr 7, 2020 · 79c2993 · 79c2993
1 parent 051f345
commit 79c2993
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 4 deletions.
diff --git a/src/GridFS/CollectionWrapper.php b/src/GridFS/CollectionWrapper.php
@@ -17,14 +17,18 @@
 
 namespace MongoDB\GridFS;
 
+use ArrayIterator;
 use MongoDB\Collection;
 use MongoDB\Driver\Cursor;
 use MongoDB\Driver\Manager;
 use MongoDB\Driver\ReadPreference;
 use MongoDB\Exception\InvalidArgumentException;
 use MongoDB\UpdateResult;
+use MultipleIterator;
 use stdClass;
 use function abs;
+use function count;
+use function is_numeric;
 use function sprintf;
 
 /**
@@ -289,27 +293,31 @@ public function updateFilenameForId($id, $filename)
      */
     private function ensureChunksIndex()
     {
+        $expectedIndex = ['files_id' => 1, 'n' => 1];
+
         foreach ($this->chunksCollection->listIndexes() as $index) {
-            if ($index->isUnique() && $index->getKey() === ['files_id' => 1, 'n' => 1]) {
+            if ($index->isUnique() && $this->indexKeysMatch($expectedIndex, $index->getKey())) {
                 return;
             }
         }
 
-        $this->chunksCollection->createIndex(['files_id' => 1, 'n' => 1], ['unique' => true]);
+        $this->chunksCollection->createIndex($expectedIndex, ['unique' => true]);
     }
 
     /**
      * Create an index on the files collection if it does not already exist.
      */
     private function ensureFilesIndex()
     {
+        $expectedIndex = ['filename' => 1, 'uploadDate' => 1];
+
         foreach ($this->filesCollection->listIndexes() as $index) {
-            if ($index->getKey() === ['filename' => 1, 'uploadDate' => 1]) {
+            if ($this->indexKeysMatch($expectedIndex, $index->getKey())) {
                 return;
             }
         }
 
-        $this->filesCollection->createIndex(['filename' => 1, 'uploadDate' => 1]);
+        $this->filesCollection->createIndex($expectedIndex);
     }
 
     /**
@@ -334,6 +342,36 @@ private function ensureIndexes()
         $this->ensureChunksIndex();
     }
 
+    private function indexKeysMatch(array $expectedKeys, array $actualKeys) : bool
+    {
+        if (count($expectedKeys) !== count($actualKeys)) {
+            return false;
+        }
+
+        $iterator = new MultipleIterator(MultipleIterator::MIT_NEED_ANY);
+        $iterator->attachIterator(new ArrayIterator($expectedKeys));
+        $iterator->attachIterator(new ArrayIterator($actualKeys));
+
+        foreach ($iterator as $key => $value) {
+            list($expectedKey, $actualKey)     = $key;
+            list($expectedValue, $actualValue) = $value;
+
+            if ($expectedKey !== $actualKey) {
+                return false;
+            }
+
+            /* Since we don't expect special indexes (e.g. text), we mark any
+             * index with a non-numeric definition as unequal. All others are
+             * compared against their int value to avoid differences due to
+             * some drivers using float values in the key specification. */
+            if (! is_numeric($actualValue) || (int) $expectedValue !== (int) $actualValue) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     /**
      * Returns whether the files collection is empty.
      *

diff --git a/tests/GridFS/BucketFunctionalTest.php b/tests/GridFS/BucketFunctionalTest.php
@@ -697,6 +697,17 @@ public function testUploadingFirstFileCreatesIndexes()
         });
     }
 
+    public function testExistingIndexIsReused()
+    {
+        $this->filesCollection->createIndex(['filename' => 1.0, 'uploadDate' => 1], ['name' => 'test']);
+        $this->chunksCollection->createIndex(['files_id' => 1.0, 'n' => 1], ['name' => 'test', 'unique' => true]);
+
+        $this->bucket->uploadFromStream('filename', $this->createStream('foo'));
+
+        $this->assertIndexNotExists($this->filesCollection->getCollectionName(), 'filename_1_uploadDate_1');
+        $this->assertIndexNotExists($this->chunksCollection->getCollectionName(), 'files_id_1_n_1');
+    }
+
     /**
      * Asserts that a collection with the given name does not exist on the
      * server.
@@ -757,6 +768,29 @@ private function assertIndexExists($collectionName, $indexName, $callback = null
         }
     }
 
+    /**
+     * Asserts that an index with the given name does not exist for the collection.
+     *
+     * @param string $collectionName
+     * @param string $indexName
+     */
+    private function assertIndexNotExists($collectionName, $indexName)
+    {
+        $operation = new ListIndexes($this->getDatabaseName(), $collectionName);
+        $indexes = $operation->execute($this->getPrimaryServer());
+
+        $foundIndex = false;
+
+        foreach ($indexes as $index) {
+            if ($index->getName() === $indexName) {
+                $foundIndex = true;
+                break;
+            }
+        }
+
+        $this->assertFalse($foundIndex, sprintf('Index %s exists', $indexName));
+    }
+
     /**
      * Return a list of invalid stream values.
      *