Merge pull request #61 from alcaeus/fix-index-creation

Fix errors with index creation
alcaeus · Feb 13, 2016 · 601aa3a · 601aa3a
2 parents a164a9d + 3731f19
commit 601aa3a
Show file tree

Hide file tree

Showing 2 changed files with 106 additions and 17 deletions.
diff --git a/lib/Mongo/MongoCollection.php b/lib/Mongo/MongoCollection.php
@@ -550,8 +550,6 @@ public function findOne(array $query = [], array $fields = [], array $options =
      * @param array $keys Field or fields to use as index.
      * @param array $options [optional] This parameter is an associative array of the form array("optionname" => <boolean>, ...).
      * @return array Returns the database response.
-     *
-     * @todo This method does not yet return the correct result
      */
     public function createIndex($keys, array $options = [])
     {
@@ -570,37 +568,38 @@ public function createIndex($keys, array $options = [])
             throw new MongoException('index specification has no elements');
         }
 
+        if (! isset($options['name'])) {
+            $options['name'] = \MongoDB\generate_index_name($keys);
+        }
+
         // duplicate
-        $neededOptions = ['unique' => 1, 'sparse' => 1, 'expireAfterSeconds' => 1, 'background' => 1, 'dropDups' => 1];
-        $indexOptions = array_intersect_key($options, $neededOptions);
+        $skippedOptions = ['background' => 1, 'dropDups' => 1, 'name' => 1, 'v' => 1, 'ns' => 1, 'key' => 1, 'writeConcern' => 1];
+        $indexOptions = array_diff_key($options, $skippedOptions);
         $indexes = iterator_to_array($this->collection->listIndexes());
         $indexCount = count($indexes);
+        $collectionExists = true;
 
         // listIndexes returns 0 for non-existing collections while the legacy driver returns 1
         if ($indexCount === 0) {
+            $collectionExists = false;
             $indexCount = 1;
         }
 
         foreach ($indexes as $index) {
-            if (! empty($options['name']) && $index->getName() === $options['name']) {
-                throw new \MongoResultException(sprintf('index with name: %s already exists', $index->getName()));
-            }
-
-            if ($index->getKey() == $keys) {
-                $currentIndexOptions = array_intersect_key($index->__debugInfo(), $neededOptions);
-
-                unset($currentIndexOptions['name']);
-                if ($currentIndexOptions != $indexOptions) {
-                    throw new \MongoResultException('Index with same keys but different options already exists');
+            if ($index->getKey() === $keys) {
+                if ($indexOptions != array_diff_key($index->__debugInfo(), $skippedOptions)) {
+                    throw new \MongoResultException(sprintf('Index with name: %s already exists with different options', $options['name']));
                 }
 
                 return [
-                    'createdCollectionAutomatically' => false,
+                    'createdCollectionAutomatically' => ! $collectionExists,
                     'numIndexesBefore' => $indexCount,
                     'numIndexesAfter' => $indexCount,
                     'note' => 'all indexes already exist',
                     'ok' => 1.0
                 ];
+            } elseif ($index->getName() === $options['name']) {
+                throw new \MongoResultException(sprintf('index with name: %s already exists', $index->getName()));
             }
         }
 
@@ -611,7 +610,7 @@ public function createIndex($keys, array $options = [])
         }
 
         return [
-            'createdCollectionAutomatically' => true,
+            'createdCollectionAutomatically' => ! $collectionExists,
             'numIndexesBefore' => $indexCount,
             'numIndexesAfter' => $indexCount + 1,
             'ok' => 1.0

diff --git a/tests/Alcaeus/MongoDbAdapter/Mongo/MongoCollectionTest.php b/tests/Alcaeus/MongoDbAdapter/Mongo/MongoCollectionTest.php
@@ -830,6 +830,47 @@ public function testCreateIndexTwice()
         $this->assertSame($expected, $this->getCollection()->createIndex(['foo' => 1]));
     }
 
+    public function testCreateIndexTwiceWithSameName()
+    {
+        $this->getCollection()->createIndex(['foo' => 1], ['name' => 'test_index']);
+
+        $expected = [
+            'createdCollectionAutomatically' => false,
+            'numIndexesBefore' => 2,
+            'numIndexesAfter' => 2,
+            'note' => 'all indexes already exist',
+            'ok' => 1.0
+        ];
+        $this->assertSame($expected, $this->getCollection()->createIndex(['foo' => 1], ['name' => 'test_index']));
+    }
+
+    public function testCreateIndexTwiceWithDifferentName()
+    {
+        $this->getCollection()->createIndex(['foo' => 1], ['name' => 'test_index']);
+
+        $expected = [
+            'createdCollectionAutomatically' => false,
+            'numIndexesBefore' => 2,
+            'numIndexesAfter' => 2,
+            'note' => 'all indexes already exist',
+            'ok' => 1.0
+        ];
+        $this->assertSame($expected, $this->getCollection()->createIndex(['foo' => 1], ['name' => 'index_test']));
+    }
+
+    public function testCreateIndexTwiceWithDifferentOrder()
+    {
+        $this->getCollection()->createIndex(['foo' => 1, 'bar' => 1]);
+
+        $expected = [
+            'createdCollectionAutomatically' => false,
+            'numIndexesBefore' => 2,
+            'numIndexesAfter' => 3,
+            'ok' => 1.0
+        ];
+        $this->assertSame($expected, $this->getCollection()->createIndex(['bar' => 1, 'foo' => 1]));
+    }
+
     public function testCreateIndexesWithDifferentOptions()
     {
         $this->setExpectedException('MongoResultException');
@@ -839,7 +880,32 @@ public function testCreateIndexesWithDifferentOptions()
         $this->getCollection()->createIndex(['foo' => 1], ['unique' => true]);
     }
 
-    public function testCreateIndexWithSameName()
+    /**
+     * @dataProvider createIndexIgnoredOptions
+     */
+    public function testCreateIndexesWithIgnoredOptions($option)
+    {
+        $this->getCollection()->createIndex(['foo' => 1]);
+
+        $expected = [
+            'createdCollectionAutomatically' => false,
+            'numIndexesBefore' => 2,
+            'numIndexesAfter' => 2,
+            'note' => 'all indexes already exist',
+            'ok' => 1.0
+        ];
+        $this->assertSame($expected, $this->getCollection()->createIndex(['foo' => 1], [$option => true]));
+    }
+
+    public static function createIndexIgnoredOptions()
+    {
+        return [
+            'background' => ['background'],
+            'dropDups' => ['dropDups'],
+        ];
+    }
+
+    public function testCreateIndexWithSameNameAndDifferentOptions()
     {
         $this->setExpectedException('MongoResultException');
 
@@ -869,6 +935,30 @@ public function testEnsureIndex()
         $this->assertSame('mongo-php-adapter.test', $index->getNamespace());
     }
 
+    public function testEnsureIndexAlreadyExists()
+    {
+        $collection = $this->getCollection();
+        $collection->ensureIndex(['bar' => 1], ['unique' => true]);
+
+        $expected = [
+            'createdCollectionAutomatically' => false,
+            'numIndexesBefore' => 2,
+            'numIndexesAfter' => 2,
+            'ok' => 1.0,
+            'note' => 'all indexes already exist',
+        ];
+        $this->assertEquals($expected, $collection->ensureIndex(['bar' => 1], ['unique' => true]));
+    }
+
+    public function testEnsureIndexAlreadyExistsWithDifferentOptions()
+    {
+        $collection = $this->getCollection();
+        $collection->ensureIndex(['bar' => 1], ['unique' => true]);
+
+        $this->setExpectedException('MongoResultException', 'Index with name: bar_1 already exists with different options');
+        $collection->ensureIndex(['bar' => 1]);
+    }
+
     public function testDeleteIndexUsingIndexName()
     {
         $newCollection = $this->getCheckDatabase()->selectCollection('test');