Review

mongodb · Aug 30, 2023 · 4edad3c · 4edad3c
1 parent 0c31932
commit 4edad3c
Show file tree

Hide file tree

Showing 10 changed files with 78 additions and 45 deletions.
diff --git a/psalm-baseline.xml b/psalm-baseline.xml
@@ -324,10 +324,10 @@
   </file>
   <file src="src/Operation/CreateSearchIndexes.php">
     <MixedArgumentTypeCoercion>
-      <code>(array) $index</code>
+      <code>$index</code>
     </MixedArgumentTypeCoercion>
     <MixedAssignment>
-      <code>$cmd[$option]</code>
+      <code><![CDATA[$cmd['comment']]]></code>
     </MixedAssignment>
   </file>
   <file src="src/Operation/DatabaseCommand.php">
@@ -412,7 +412,7 @@
   </file>
   <file src="src/Operation/DropSearchIndex.php">
     <MixedAssignment>
-      <code>$cmd[$option]</code>
+      <code><![CDATA[$cmd['comment']]]></code>
     </MixedAssignment>
   </file>
   <file src="src/Operation/Explain.php">
@@ -596,7 +596,7 @@
   </file>
   <file src="src/Operation/UpdateSearchIndex.php">
     <MixedAssignment>
-      <code>$cmd[$option]</code>
+      <code><![CDATA[$cmd['comment']]]></code>
     </MixedAssignment>
   </file>
   <file src="src/Operation/Watch.php">

diff --git a/src/Collection.php b/src/Collection.php
@@ -22,7 +22,6 @@
 use MongoDB\BSON\JavascriptInterface;
 use MongoDB\Codec\DocumentCodec;
 use MongoDB\Driver\CursorInterface;
-use MongoDB\Driver\Exception\CommandException;
 use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException;
 use MongoDB\Driver\Manager;
 use MongoDB\Driver\ReadConcern;
@@ -580,14 +579,7 @@ public function dropSearchIndex(string $name, array $options = []): void
         $operation = new DropSearchIndex($this->databaseName, $this->collectionName, $name);
         $server = select_server($this->manager, $options);
 
-        try {
-            $operation->execute($server);
-        } catch (CommandException $e) {
-            // Suppress namespace not found errors for idempotency
-            if ($e->getCode() !== 26) {
-                throw $e;
-            }
-        }
+        $operation->execute($server);
     }
 
     /**

diff --git a/src/Operation/CreateSearchIndexes.php b/src/Operation/CreateSearchIndexes.php
@@ -1,6 +1,6 @@
 <?php
 /*
- * Copyright 2015-present MongoDB, Inc.
+ * Copyright 2023-present MongoDB, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -27,7 +27,7 @@
 use function array_column;
 use function array_is_list;
 use function current;
-use function MongoDB\is_document;
+use function is_array;
 use function sprintf;
 
 /**
@@ -49,7 +49,7 @@ class CreateSearchIndexes implements Executable
      *
      * @param string                 $databaseName   Database name
      * @param string                 $collectionName Collection name
-     * @param list<array|object>     $indexes        List of search index specifications
+     * @param array[]                $indexes        List of search index specifications
      * @param array{comment?: mixed} $options        Command options
      * @throws InvalidArgumentException for parameter parsing errors
      */
@@ -60,11 +60,11 @@ public function __construct(string $databaseName, string $collectionName, array
         }
 
         foreach ($indexes as $i => $index) {
-            if (! is_document($index)) {
-                throw InvalidArgumentException::expectedDocumentType(sprintf('$indexes[%d]', $i), $index);
+            if (! is_array($index)) {
+                throw InvalidArgumentException::invalidType(sprintf('$indexes[%d]', $i), $index, 'array');
             }
 
-            $this->indexes[] = new SearchIndexInput((array) $index);
+            $this->indexes[] = new SearchIndexInput($index);
         }
 
         $this->databaseName = $databaseName;
@@ -87,10 +87,8 @@ public function execute(Server $server): array
             'indexes' => $this->indexes,
         ];
 
-        foreach (['comment'] as $option) {
-            if (isset($this->options[$option])) {
-                $cmd[$option] = $this->options[$option];
-            }
+        if (isset($this->options['comment'])) {
+            $cmd['comment'] = $this->options['comment'];
         }
 
         $cursor = $server->executeCommand($this->databaseName, new Command($cmd));

diff --git a/src/Operation/DropSearchIndex.php b/src/Operation/DropSearchIndex.php
@@ -1,6 +1,6 @@
 <?php
 /*
- * Copyright 2015-present MongoDB, Inc.
+ * Copyright 2023-present MongoDB, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@
 namespace MongoDB\Operation;
 
 use MongoDB\Driver\Command;
+use MongoDB\Driver\Exception\CommandException;
 use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException;
 use MongoDB\Driver\Server;
 use MongoDB\Exception\InvalidArgumentException;
@@ -31,6 +32,8 @@
  */
 class DropSearchIndex implements Executable
 {
+    private const ERROR_CODE_NAMESPACE_NOT_FOUND = 26;
+
     private string $databaseName;
     private string $collectionName;
     private string $name;
@@ -71,12 +74,17 @@ public function execute(Server $server): void
             'name' => $this->name,
         ];
 
-        foreach (['comment'] as $option) {
-            if (isset($this->options[$option])) {
-                $cmd[$option] = $this->options[$option];
-            }
+        if (isset($this->options['comment'])) {
+            $cmd['comment'] = $this->options['comment'];
         }
 
-        $server->executeCommand($this->databaseName, new Command($cmd));
+        try {
+            $server->executeCommand($this->databaseName, new Command($cmd));
+        } catch (CommandException $e) {
+            // Drop operations is idempotent. The server may return an error if the collection does not exist.
+            if ($e->getCode() !== self::ERROR_CODE_NAMESPACE_NOT_FOUND) {
+                throw $e;
+            }
+        }
     }
 }
diff --git a/src/Operation/ListSearchIndexes.php b/src/Operation/ListSearchIndexes.php
@@ -1,6 +1,6 @@
 <?php
 /*
- * Copyright 2015-present MongoDB, Inc.
+ * Copyright 2023-present MongoDB, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

diff --git a/src/Operation/UpdateSearchIndex.php b/src/Operation/UpdateSearchIndex.php
@@ -1,6 +1,6 @@
 <?php
 /*
- * Copyright 2015-present MongoDB, Inc.
+ * Copyright 2023-present MongoDB, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -81,10 +81,8 @@ public function execute(Server $server): void
             'definition' => $this->definition,
         ];
 
-        foreach (['comment'] as $option) {
-            if (isset($this->options[$option])) {
-                $cmd[$option] = $this->options[$option];
-            }
+        if (isset($this->options['comment'])) {
+            $cmd['comment'] = $this->options['comment'];
         }
 
         $server->executeCommand($this->databaseName, new Command($cmd));

diff --git a/tests/Operation/CreateSearchIndexesTest.php b/tests/Operation/CreateSearchIndexesTest.php
@@ -18,7 +18,7 @@ public function testConstructorIndexesArgumentMustBeAList(): void
     public function testConstructorIndexDefinitionMustBeADocument($index): void
     {
         $this->expectException(InvalidArgumentException::class);
-        $this->expectExceptionMessage('Expected $indexes[0] to have type "document"');
+        $this->expectExceptionMessage('Expected $indexes[0] to have type "array"');
         new CreateSearchIndexes($this->getDatabaseName(), $this->getCollectionName(), [$index], []);
     }
 

diff --git a/tests/SpecTests/SearchIndexSpecTest.php b/tests/SpecTests/SearchIndexSpecTest.php
@@ -21,22 +21,29 @@
 /**
  * Functional tests for the Atlas Search index management.
  *
- * @see https://github.com/mongodb/specifications/blob/master/source/index-management/index-management.rst
+ * @see https://github.com/mongodb/specifications/blob/master/source/index-management/tests/README.rst#search-index-management-helpers
  * @group atlas
  */
 class SearchIndexSpecTest extends FunctionalTestCase
 {
-    private const WAIT_TIMEOUT = 300; // 5 minutes
+    private const WAIT_TIMEOUT_SEC = 300;
 
     public function setUp(): void
     {
         if (! self::isAtlas()) {
-            self::markTestSkipped('Search Indexes are only supported on MongoDB Atlas');
+            self::markTestSkipped('Search Indexes are only supported on MongoDB Atlas 7.0+');
         }
 
         parent::setUp();
+
+        $this->skipIfServerVersion('<', '7.0', 'Search Indexes are only supported on MongoDB Atlas 7.0+');
     }
 
+    /**
+     * Case 1: Driver can successfully create and list search indexes
+     *
+     * @see https://github.com/mongodb/specifications/blob/master/source/index-management/tests/README.rst#case-1-driver-can-successfully-create-and-list-search-indexes
+     */
     public function testCreateAndListSearchIndexes(): void
     {
         $collection = $this->createCollection($this->getDatabaseName(), $this->getCollectionName());
@@ -60,6 +67,11 @@ public function testCreateAndListSearchIndexes(): void
         );
     }
 
+    /**
+     * Case 2: Driver can successfully create multiple indexes in batch
+     *
+     * @see https://github.com/mongodb/specifications/blob/master/source/index-management/tests/README.rst#case-2-driver-can-successfully-create-multiple-indexes-in-batch
+     */
     public function testCreateMultipleIndexesInBatch(): void
     {
         $collection = $this->createCollection($this->getDatabaseName(), $this->getCollectionName());
@@ -86,6 +98,11 @@ public function testCreateMultipleIndexesInBatch(): void
         }
     }
 
+    /**
+     * Case 3: Driver can successfully drop search indexes
+     *
+     * @see https://github.com/mongodb/specifications/blob/master/source/index-management/tests/README.rst#case-3-driver-can-successfully-drop-search-indexes
+     */
     public function testDropSearchIndexes(): void
     {
         $collection = $this->createCollection($this->getDatabaseName(), $this->getCollectionName());
@@ -98,15 +115,20 @@ public function testDropSearchIndexes(): void
         );
         $this->assertSame($name, $createdName);
 
-        $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
+        $indexes = $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
+        $this->assertCount(1, $indexes);
 
         $collection->dropSearchIndex($name);
 
         $indexes = $this->waitForIndexes($collection, fn (array $indexes): bool => count($indexes) === 0);
-
         $this->assertCount(0, $indexes);
     }
 
+    /**
+     * Case 4: Driver can update a search index
+     *
+     * @see https://github.com/mongodb/specifications/blob/master/source/index-management/tests/README.rst#case-4-driver-can-update-a-search-index
+     */
     public function testUpdateSearchIndex(): void
     {
         $collection = $this->createCollection($this->getDatabaseName(), $this->getCollectionName());
@@ -119,7 +141,8 @@ public function testUpdateSearchIndex(): void
         );
         $this->assertSame($name, $createdName);
 
-        $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
+        $indexes = $this->waitForIndexes($collection, fn ($indexes) => $this->allIndexesAreQueryable($indexes));
+        $this->assertCount(1, $indexes);
 
         $mapping = ['mappings' => ['dynamic' => true]];
         $collection->updateSearchIndex($name, $mapping);
@@ -135,6 +158,11 @@ public function testUpdateSearchIndex(): void
         );
     }
 
+    /**
+     * Case 5: dropSearchIndex suppresses namespace not found errors
+     *
+     * @see https://github.com/mongodb/specifications/blob/master/source/index-management/tests/README.rst#case-5-dropsearchindex-suppresses-namespace-not-found-errors
+     */
     public function testDropSearchIndexSuppressNamespaceNotFoundError(): void
     {
         $collection = $this->dropCollection($this->getDatabaseName(), $this->getCollectionName());
@@ -155,7 +183,7 @@ protected function getCollectionName(): string
 
     private function waitForIndexes(Collection $collection, Closure $callback): array
     {
-        $timeout = hrtime()[0] + self::WAIT_TIMEOUT;
+        $timeout = hrtime()[0] + self::WAIT_TIMEOUT_SEC;
         while (hrtime()[0] < $timeout) {
             sleep(5);
             $result = $collection->listSearchIndexes();

diff --git a/tests/UnifiedSpecTests/Operation.php b/tests/UnifiedSpecTests/Operation.php
@@ -548,10 +548,19 @@ private function executeForCollection(Collection $collection)
                     $options['name'] = $args['model']->name;
                 }
 
+                assertInstanceOf(stdClass::class, $args['model']->definition);
+
                 return $collection->createSearchIndex($args['model']->definition, $options);
 
             case 'createSearchIndexes':
-                return $collection->createSearchIndexes($args['models']);
+                $indexes = array_map(function ($index) {
+                    $index = (array) $index;
+                    assertInstanceOf(stdClass::class, $index['definition']);
+
+                    return $index;
+                }, $args['models']);
+
+                return $collection->createSearchIndexes($indexes);
 
             case 'dropSearchIndex':
                 assertArrayHasKey('name', $args);

diff --git a/tests/UnifiedSpecTests/UnifiedSpecTest.php b/tests/UnifiedSpecTests/UnifiedSpecTest.php
@@ -277,7 +277,7 @@ public function provideFailingTests()
     public function testIndexManagement(UnifiedTestCase $test): void
     {
         if (self::isAtlas()) {
-            self::markTestSkipped('Search Indexes tests must run on a non-atlas cluster');
+            self::markTestSkipped('Search Indexes tests must run on a non-Atlas cluster');
         }
 
         if (! self::isEnterprise()) {