Skip to content

PHPLIB-1394: Support "type" option for createSearchIndex(es) #1375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 6, 2024
Merged

PHPLIB-1394: Support "type" option for createSearchIndex(es) #1375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9cf05f7
PHPLIB-1394: Support "type" option for createSearchIndex(es)
jmikola Sep 5, 2024
921f449
Test type validation for search index "type" option
jmikola Sep 5, 2024
728bd60
Use correct field name in createSearchIndex assertion
jmikola Sep 6, 2024
5aa0423
Cast createSearchIndex model arg before array_diff
jmikola Sep 6, 2024
e2278c0
Update psalm baseline
jmikola Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@
<code><![CDATA[$mergedDriver['platform']]]></code>
</MixedAssignment>
</file>
<file src="src/Collection.php">
<MixedArgumentTypeCoercion>
<code><![CDATA[$options]]></code>
</MixedArgumentTypeCoercion>
</file>
<file src="src/Command/ListCollections.php">
<MixedAssignment>
<code><![CDATA[$cmd[$option]]]></code>
Expand Down
22 changes: 11 additions & 11 deletions src/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,22 +371,22 @@ 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
* @throws DriverRuntimeException for other driver errors (e.g. connection errors)
*/
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);
}
Expand All @@ -400,16 +400,16 @@ 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]]],
* ];
*
* @see https://www.mongodb.com/docs/manual/reference/command/createSearchIndexes/
* @see https://mongodb.com/docs/manual/reference/method/db.collection.createSearchIndex/
* @param list<array{name?: string, definition: array|object}> $indexes List of search index specifications
* @param array{comment?: string} $options Command options
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was originally typed as a string in f6ce211, despite createSearchIndex() correctly using mixed for its option.

* @param list<array{definition: array|object, name?: string, type?: string}> $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
Expand Down
6 changes: 5 additions & 1 deletion src/Model/SearchIndexInput.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
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)
Expand All @@ -57,6 +57,10 @@
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;
}

Expand Down
8 changes: 8 additions & 0 deletions tests/Model/SearchIndexInputTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GromNaN: Added a unit test here and in CreateSearchIndexesTest below. The test in the other file seems redundant, but we already had a test for name so I thought it best to preserve the consistency.


public function testBsonSerialization(): void
{
$expected = (object) [
Expand Down
8 changes: 8 additions & 0 deletions tests/Operation/CreateSearchIndexesTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 13 additions & 7 deletions tests/UnifiedSpecTests/Operation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
84 changes: 79 additions & 5 deletions tests/UnifiedSpecTests/index-management/createSearchIndex.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -50,7 +60,8 @@
"mappings": {
"dynamic": true
}
}
},
"type": "search"
}
},
"expectError": {
Expand All @@ -73,7 +84,8 @@
"mappings": {
"dynamic": true
}
}
},
"type": "search"
}
],
"$db": "database0"
Expand All @@ -97,7 +109,8 @@
"dynamic": true
}
},
"name": "test index"
"name": "test index",
"type": "search"
}
},
"expectError": {
Expand All @@ -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"
Expand Down
86 changes: 81 additions & 5 deletions tests/UnifiedSpecTests/index-management/createSearchIndexes.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -83,7 +93,8 @@
"mappings": {
"dynamic": true
}
}
},
"type": "search"
}
]
},
Expand All @@ -107,7 +118,8 @@
"mappings": {
"dynamic": true
}
}
},
"type": "search"
}
],
"$db": "database0"
Expand All @@ -132,7 +144,8 @@
"dynamic": true
}
},
"name": "test index"
"name": "test index",
"type": "search"
}
]
},
Expand All @@ -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"
Expand Down
Loading