From 99f3b77233d24b7c5156ba5251d724aea813ad10 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 6 Sep 2024 14:41:21 -0400 Subject: [PATCH] PHPLIB-1394: Support "type" option for createSearchIndex(es) (#1375) Synced with mongodb/specifications@8be1189abcc9b879a081f1f606159f3d2578cb27 * Update psalm baseline --- psalm-baseline.xml | 5 -- src/Collection.php | 22 ++--- src/Model/SearchIndexInput.php | 6 +- tests/Model/SearchIndexInputTest.php | 8 ++ tests/Operation/CreateSearchIndexesTest.php | 8 ++ tests/UnifiedSpecTests/Operation.php | 20 +++-- .../index-management/createSearchIndex.json | 84 ++++++++++++++++-- .../index-management/createSearchIndexes.json | 86 +++++++++++++++++-- .../searchIndexIgnoresReadWriteConcern.json | 12 ++- 9 files changed, 216 insertions(+), 35 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 8ba6e2760..8bd15f1cd 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -57,11 +57,6 @@ - - - - - diff --git a/src/Collection.php b/src/Collection.php index 708d43d68..d09156400 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -371,8 +371,8 @@ public function createIndexes(array $indexes, array $options = []) * * @see https://www.mongodb.com/docs/manual/reference/command/createSearchIndexes/ * @see https://mongodb.com/docs/manual/reference/method/db.collection.createSearchIndex/ - * @param array|object $definition Atlas Search index mapping definition - * @param array{name?: string, comment?: mixed} $options Command options + * @param array|object $definition Atlas Search index mapping definition + * @param array{comment?: mixed, name?: string, type?: string} $options Index and command options * @return string The name of the created search index * @throws UnsupportedException if options are not supported by the selected server * @throws InvalidArgumentException for parameter/option parsing errors @@ -380,13 +380,13 @@ public function createIndexes(array $indexes, array $options = []) */ public function createSearchIndex($definition, array $options = []): string { - $index = ['definition' => $definition]; - if (isset($options['name'])) { - $index['name'] = $options['name']; - unset($options['name']); - } + $indexOptionKeys = ['name' => 1, 'type' => 1]; + /** @psalm-var array{name?: string, type?: string} */ + $indexOptions = array_intersect_key($options, $indexOptionKeys); + /** @psalm-var array{comment?: mixed} */ + $operationOptions = array_diff_key($options, $indexOptionKeys); - $names = $this->createSearchIndexes([$index], $options); + $names = $this->createSearchIndexes([['definition' => $definition] + $indexOptions], $operationOptions); return current($names); } @@ -400,7 +400,7 @@ public function createSearchIndex($definition, array $options = []): string * For example: * * $indexes = [ - * // Create a search index with the default name, on + * // Create a search index with the default name on a single field * ['definition' => ['mappings' => ['dynamic' => false, 'fields' => ['title' => ['type' => 'string']]]]], * // Create a named search index on all fields * ['name' => 'search_all', 'definition' => ['mappings' => ['dynamic' => true]]], @@ -408,8 +408,8 @@ public function createSearchIndex($definition, array $options = []): string * * @see https://www.mongodb.com/docs/manual/reference/command/createSearchIndexes/ * @see https://mongodb.com/docs/manual/reference/method/db.collection.createSearchIndex/ - * @param list $indexes List of search index specifications - * @param array{comment?: string} $options Command options + * @param list $indexes List of search index specifications + * @param array{comment?: mixed} $options Command options * @return string[] The names of the created search indexes * @throws UnsupportedException if options are not supported by the selected server * @throws InvalidArgumentException for parameter/option parsing errors diff --git a/src/Model/SearchIndexInput.php b/src/Model/SearchIndexInput.php index e28aea9b0..5562b7015 100644 --- a/src/Model/SearchIndexInput.php +++ b/src/Model/SearchIndexInput.php @@ -39,7 +39,7 @@ class SearchIndexInput implements Serializable private array $index; /** - * @param array{name?: string, definition: array|object} $index Search index specification + * @param array{definition: array|object, name?: string, type?: string} $index Search index specification * @throws InvalidArgumentException */ public function __construct(array $index) @@ -57,6 +57,10 @@ public function __construct(array $index) throw InvalidArgumentException::invalidType('"name" option', $index['name'], 'string'); } + if (isset($index['type']) && ! is_string($index['type'])) { + throw InvalidArgumentException::invalidType('"type" option', $index['type'], 'string'); + } + $this->index = $index; } diff --git a/tests/Model/SearchIndexInputTest.php b/tests/Model/SearchIndexInputTest.php index 0126aaec6..aef36c8c2 100644 --- a/tests/Model/SearchIndexInputTest.php +++ b/tests/Model/SearchIndexInputTest.php @@ -32,6 +32,14 @@ public function testConstructorShouldRequireNameToBeString($name): void new SearchIndexInput(['definition' => ['mapping' => ['dynamic' => true]], 'name' => $name]); } + /** @dataProvider provideInvalidStringValues */ + public function testConstructorShouldRequireTypeToBeString($type): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected "type" option to have type "string"'); + new SearchIndexInput(['definition' => ['mapping' => ['dynamic' => true]], 'type' => $type]); + } + public function testBsonSerialization(): void { $expected = (object) [ diff --git a/tests/Operation/CreateSearchIndexesTest.php b/tests/Operation/CreateSearchIndexesTest.php index e2dcb1e65..27246873b 100644 --- a/tests/Operation/CreateSearchIndexesTest.php +++ b/tests/Operation/CreateSearchIndexesTest.php @@ -30,6 +30,14 @@ public function testConstructorIndexNameMustBeAString($name): void new CreateSearchIndexes($this->getDatabaseName(), $this->getCollectionName(), [['name' => $name, 'definition' => ['mappings' => ['dynamic' => true]]]], []); } + /** @dataProvider provideInvalidStringValues */ + public function testConstructorIndexTypeMustBeAString($type): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected "type" option to have type "string"'); + new CreateSearchIndexes($this->getDatabaseName(), $this->getCollectionName(), [['type' => $type, 'definition' => ['mappings' => ['dynamic' => true]]]], []); + } + public function testConstructorIndexDefinitionMustBeDefined(): void { $this->expectException(InvalidArgumentException::class); diff --git a/tests/UnifiedSpecTests/Operation.php b/tests/UnifiedSpecTests/Operation.php index 83c6a4924..3fdf5ea70 100644 --- a/tests/UnifiedSpecTests/Operation.php +++ b/tests/UnifiedSpecTests/Operation.php @@ -537,19 +537,25 @@ private function executeForCollection(Collection $collection) ); case 'createSearchIndex': - $options = []; - if (isset($args['model']->name)) { - assertIsString($args['model']->name); - $options['name'] = $args['model']->name; - } - + assertArrayHasKey('model', $args); + assertIsObject($args['model']); + assertObjectHasAttribute('definition', $args['model']); assertInstanceOf(stdClass::class, $args['model']->definition); - return $collection->createSearchIndex($args['model']->definition, $options); + /* Note: tests specify options within "model". A top-level + * "options" key (CreateSearchIndexOptions) is not used. */ + $definition = $args['model']->definition; + $options = array_diff_key((array) $args['model'], ['definition' => 1]); + + return $collection->createSearchIndex($definition, $options); case 'createSearchIndexes': + assertArrayHasKey('models', $args); + assertIsArray($args['models']); + $indexes = array_map(function ($index) { $index = (array) $index; + assertArrayHasKey('definition', $index); assertInstanceOf(stdClass::class, $index['definition']); return $index; diff --git a/tests/UnifiedSpecTests/index-management/createSearchIndex.json b/tests/UnifiedSpecTests/index-management/createSearchIndex.json index f9c4e44d3..f4f2a6c66 100644 --- a/tests/UnifiedSpecTests/index-management/createSearchIndex.json +++ b/tests/UnifiedSpecTests/index-management/createSearchIndex.json @@ -28,7 +28,17 @@ ], "runOnRequirements": [ { - "minServerVersion": "7.0.0", + "minServerVersion": "7.0.5", + "maxServerVersion": "7.0.99", + "topologies": [ + "replicaset", + "load-balanced", + "sharded" + ], + "serverless": "forbid" + }, + { + "minServerVersion": "7.2.0", "topologies": [ "replicaset", "load-balanced", @@ -50,7 +60,8 @@ "mappings": { "dynamic": true } - } + }, + "type": "search" } }, "expectError": { @@ -73,7 +84,8 @@ "mappings": { "dynamic": true } - } + }, + "type": "search" } ], "$db": "database0" @@ -97,7 +109,8 @@ "dynamic": true } }, - "name": "test index" + "name": "test index", + "type": "search" } }, "expectError": { @@ -121,7 +134,68 @@ "dynamic": true } }, - "name": "test index" + "name": "test index", + "type": "search" + } + ], + "$db": "database0" + } + } + } + ] + } + ] + }, + { + "description": "create a vector search index", + "operations": [ + { + "name": "createSearchIndex", + "object": "collection0", + "arguments": { + "model": { + "definition": { + "fields": [ + { + "type": "vector", + "path": "plot_embedding", + "numDimensions": 1536, + "similarity": "euclidean" + } + ] + }, + "name": "test index", + "type": "vectorSearch" + } + }, + "expectError": { + "isError": true, + "errorContains": "Atlas" + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "createSearchIndexes": "collection0", + "indexes": [ + { + "definition": { + "fields": [ + { + "type": "vector", + "path": "plot_embedding", + "numDimensions": 1536, + "similarity": "euclidean" + } + ] + }, + "name": "test index", + "type": "vectorSearch" } ], "$db": "database0" diff --git a/tests/UnifiedSpecTests/index-management/createSearchIndexes.json b/tests/UnifiedSpecTests/index-management/createSearchIndexes.json index 3cf56ce12..01300b1b7 100644 --- a/tests/UnifiedSpecTests/index-management/createSearchIndexes.json +++ b/tests/UnifiedSpecTests/index-management/createSearchIndexes.json @@ -28,7 +28,17 @@ ], "runOnRequirements": [ { - "minServerVersion": "7.0.0", + "minServerVersion": "7.0.5", + "maxServerVersion": "7.0.99", + "topologies": [ + "replicaset", + "load-balanced", + "sharded" + ], + "serverless": "forbid" + }, + { + "minServerVersion": "7.2.0", "topologies": [ "replicaset", "load-balanced", @@ -83,7 +93,8 @@ "mappings": { "dynamic": true } - } + }, + "type": "search" } ] }, @@ -107,7 +118,8 @@ "mappings": { "dynamic": true } - } + }, + "type": "search" } ], "$db": "database0" @@ -132,7 +144,8 @@ "dynamic": true } }, - "name": "test index" + "name": "test index", + "type": "search" } ] }, @@ -157,7 +170,70 @@ "dynamic": true } }, - "name": "test index" + "name": "test index", + "type": "search" + } + ], + "$db": "database0" + } + } + } + ] + } + ] + }, + { + "description": "create a vector search index", + "operations": [ + { + "name": "createSearchIndexes", + "object": "collection0", + "arguments": { + "models": [ + { + "definition": { + "fields": [ + { + "type": "vector", + "path": "plot_embedding", + "numDimensions": 1536, + "similarity": "euclidean" + } + ] + }, + "name": "test index", + "type": "vectorSearch" + } + ] + }, + "expectError": { + "isError": true, + "errorContains": "Atlas" + } + } + ], + "expectEvents": [ + { + "client": "client0", + "events": [ + { + "commandStartedEvent": { + "command": { + "createSearchIndexes": "collection0", + "indexes": [ + { + "definition": { + "fields": [ + { + "type": "vector", + "path": "plot_embedding", + "numDimensions": 1536, + "similarity": "euclidean" + } + ] + }, + "name": "test index", + "type": "vectorSearch" } ], "$db": "database0" diff --git a/tests/UnifiedSpecTests/index-management/searchIndexIgnoresReadWriteConcern.json b/tests/UnifiedSpecTests/index-management/searchIndexIgnoresReadWriteConcern.json index edf71b7b7..5b57cd22c 100644 --- a/tests/UnifiedSpecTests/index-management/searchIndexIgnoresReadWriteConcern.json +++ b/tests/UnifiedSpecTests/index-management/searchIndexIgnoresReadWriteConcern.json @@ -32,7 +32,17 @@ ], "runOnRequirements": [ { - "minServerVersion": "7.0.0", + "minServerVersion": "7.0.5", + "maxServerVersion": "7.0.99", + "topologies": [ + "replicaset", + "load-balanced", + "sharded" + ], + "serverless": "forbid" + }, + { + "minServerVersion": "7.2.0", "topologies": [ "replicaset", "load-balanced",