Skip to content
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.

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

PHPLIB-1492: Support sort option for updateOne and replaceOne #1605

Merged
merged 2 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
20 changes: 20 additions & 0 deletions src/Operation/BulkWrite.php
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,14 @@ class BulkWrite implements Executable
* * upsert (boolean): When true, a new document is created if no document
* matches the query. The default is false.
*
* Supported options for replaceOne and updateOne operations:
*
* * sort (document): Determines which document the operation modifies if
* the query selects multiple documents.
*
* This is not supported for server versions < 8.0 and will result in an
* exception at execution time if used.
*
* Supported options for updateMany and updateOne operations:
*
* * arrayFilters (document array): A set of filters specifying to which
Expand Down Expand Up @@ -372,6 +380,10 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En
throw InvalidArgumentException::expectedDocumentType(sprintf('$operations[%d]["%s"][2]["collation"]', $i, $type), $args[2]['collation']);
}

if (isset($args[2]['sort']) && ! is_document($args[2]['sort'])) {
throw InvalidArgumentException::expectedDocumentType(sprintf('$operations[%d]["%s"][2]["sort"]', $i, $type), $args[2]['sort']);
}

if (! is_bool($args[2]['upsert'])) {
throw InvalidArgumentException::invalidType(sprintf('$operations[%d]["%s"][2]["upsert"]', $i, $type), $args[2]['upsert'], 'boolean');
}
Expand Down Expand Up @@ -413,6 +425,14 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En
throw InvalidArgumentException::expectedDocumentType(sprintf('$operations[%d]["%s"][2]["collation"]', $i, $type), $args[2]['collation']);
}

if (isset($args[2]['sort']) && ! is_document($args[2]['sort'])) {
throw InvalidArgumentException::expectedDocumentType(sprintf('$operations[%d]["%s"][2]["sort"]', $i, $type), $args[2]['sort']);
}

if (isset($args[2]['sort']) && $args[2]['multi']) {
throw new InvalidArgumentException(sprintf('"sort" option cannot be used with $operations[%d]["%s"]', $i, $type));
}
Comment on lines +432 to +434
Copy link
Member

Choose a reason for hiding this comment

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

Noted that the multi option cannot true for replaceOne, that why the validation is only for updateMany and updateOne.


if (! is_bool($args[2]['upsert'])) {
throw InvalidArgumentException::invalidType(sprintf('$operations[%d]["%s"][2]["upsert"]', $i, $type), $args[2]['upsert'], 'boolean');
}
Expand Down
6 changes: 6 additions & 0 deletions src/Operation/ReplaceOne.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ class ReplaceOne implements Executable
* Parameters can then be accessed as variables in an aggregate
* expression context (e.g. "$$var").
*
* * sort (document): Determines which document the operation modifies if
* the query selects multiple documents.
*
* This is not supported for server versions < 8.0 and will result in an
* exception at execution time if used.
*
* * writeConcern (MongoDB\Driver\WriteConcern): Write concern.
*
* @param string $databaseName Database name
Expand Down
14 changes: 12 additions & 2 deletions src/Operation/Update.php
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ public function __construct(private string $databaseName, private string $collec
throw InvalidArgumentException::expectedDocumentType('"let" option', $options['let']);
}

if (isset($options['sort']) && ! is_document($options['sort'])) {
throw InvalidArgumentException::expectedDocumentType('"sort" option', $options['sort']);
}

if (isset($options['sort']) && $options['multi']) {
throw new InvalidArgumentException('"sort" option cannot be used with multi-document updates');
}

if (isset($options['bypassDocumentValidation']) && ! $options['bypassDocumentValidation']) {
unset($options['bypassDocumentValidation']);
}
Expand Down Expand Up @@ -270,8 +278,10 @@ private function createUpdateOptions(): array
}
}

if (isset($this->options['collation'])) {
$updateOptions['collation'] = (object) $this->options['collation'];
foreach (['collation', 'sort'] as $option) {
if (isset($this->options[$option])) {
$updateOptions[$option] = (object) $this->options[$option];
}
}

return $updateOptions;
Expand Down
6 changes: 6 additions & 0 deletions src/Operation/UpdateOne.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,12 @@ class UpdateOne implements Executable, Explainable
* Parameters can then be accessed as variables in an aggregate
* expression context (e.g. "$$var").
*
* * sort (document): Determines which document the operation modifies if
* the query selects multiple documents.
*
* This is not supported for server versions < 8.0 and will result in an
* exception at execution time if used.
*
* * writeConcern (MongoDB\Driver\WriteConcern): Write concern.
*
* @param string $databaseName Database name
Expand Down
29 changes: 29 additions & 0 deletions tests/Operation/BulkWriteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,16 @@ public function testReplaceOneCollationOptionTypeCheck($collation): void
]);
}

#[DataProvider('provideInvalidDocumentValues')]
public function testReplaceOneSortOptionTypeCheck($sort): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessageMatches('/Expected \$operations\[0\]\["replaceOne"\]\[2\]\["sort"\] to have type "document" \(array or object\) but found ".+"/');
new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [
[BulkWrite::REPLACE_ONE => [['x' => 1], ['y' => 1], ['sort' => $sort]]],
]);
}

#[DataProvider('provideInvalidBooleanValues')]
public function testReplaceOneUpsertOptionTypeCheck($upsert): void
{
Expand Down Expand Up @@ -322,6 +332,15 @@ public function testUpdateManyCollationOptionTypeCheck($collation): void
]);
}

public function testUpdateManyProhibitsSortOption(): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('"sort" option cannot be used with $operations[0]["updateMany"]');
new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [
[BulkWrite::UPDATE_MANY => [['x' => 1], ['$set' => ['y' => 1]], ['sort' => ['z' => 1]]]],
]);
}

#[DataProvider('provideInvalidBooleanValues')]
public function testUpdateManyUpsertOptionTypeCheck($upsert): void
{
Expand Down Expand Up @@ -410,6 +429,16 @@ public function testUpdateOneCollationOptionTypeCheck($collation): void
]);
}

#[DataProvider('provideInvalidDocumentValues')]
public function testUpdateOneSortOptionTypeCheck($sort): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessageMatches('/Expected \$operations\[0\]\["updateOne"\]\[2\]\["sort"\] to have type "document" \(array or object\) but found ".+"/');
new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [
[BulkWrite::UPDATE_ONE => [['x' => 1], ['$set' => ['y' => 1]], ['sort' => $sort]]],
]);
}

#[DataProvider('provideInvalidBooleanValues')]
public function testUpdateOneUpsertOptionTypeCheck($upsert): void
{
Expand Down
8 changes: 8 additions & 0 deletions tests/Operation/UpdateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public static function provideInvalidConstructorOptions()
'collation' => self::getInvalidDocumentValues(),
'hint' => self::getInvalidHintValues(),
'multi' => self::getInvalidBooleanValues(),
'sort' => self::getInvalidDocumentValues(),
'session' => self::getInvalidSessionValues(),
'upsert' => self::getInvalidBooleanValues(),
'writeConcern' => self::getInvalidWriteConcernValues(),
Expand All @@ -55,6 +56,13 @@ public function testConstructorMultiOptionProhibitsReplacementDocumentOrEmptyPip
new Update($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $update, ['multi' => true]);
}

public function testConstructorMultiOptionProhibitsSortOption(): void
{
$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessage('"sort" option cannot be used with multi-document updates');
new Update($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], ['$set' => ['x' => 2]], ['multi' => true, 'sort' => ['x' => 1]]);
}

public function testExplainableCommandDocument(): void
{
$options = [
Expand Down
5 changes: 0 additions & 5 deletions tests/UnifiedSpecTests/UnifiedSpecTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ class UnifiedSpecTest extends FunctionalTestCase
// mongoc_cluster_stream_for_server does not retry handshakes (CDRIVER-4532, PHPLIB-1033, PHPLIB-1042)
'retryable-reads/retryable reads handshake failures' => 'Handshakes are not retried (CDRIVER-4532)',
'retryable-writes/retryable writes handshake failures' => 'Handshakes are not retried (CDRIVER-4532)',
// sort option for update operations is not supported (PHPLIB-1492)
'crud/BulkWrite replaceOne-sort' => 'Sort for replace operations is not supported (PHPLIB-1492)',
'crud/BulkWrite updateOne-sort' => 'Sort for update operations is not supported (PHPLIB-1492)',
'crud/replaceOne-sort' => 'Sort for replace operations is not supported (PHPLIB-1492)',
'crud/updateOne-sort' => 'Sort for update operations is not supported (PHPLIB-1492)',
'crud/bypassDocumentValidation' => 'bypassDocumentValidation is handled by libmongoc (PHPLIB-1576)',
];

Expand Down
4 changes: 2 additions & 2 deletions tests/UnifiedSpecTests/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ final class Util
'findOne' => ['let', 'filter', 'session', 'allowDiskUse', 'allowPartialResults', 'batchSize', 'collation', 'comment', 'cursorType', 'hint', 'max', 'maxAwaitTimeMS', 'maxScan', 'maxTimeMS', 'min', 'modifiers', 'noCursorTimeout', 'oplogReplay', 'projection', 'returnKey', 'showRecordId', 'skip', 'snapshot', 'sort'],
'findOneAndReplace' => ['let', 'returnDocument', 'filter', 'replacement', 'session', 'projection', 'returnDocument', 'upsert', 'arrayFilters', 'bypassDocumentValidation', 'collation', 'hint', 'maxTimeMS', 'new', 'remove', 'sort', 'comment'],
'rename' => ['to', 'comment', 'dropTarget'],
'replaceOne' => ['let', 'filter', 'replacement', 'session', 'upsert', 'arrayFilters', 'bypassDocumentValidation', 'collation', 'hint', 'comment'],
'replaceOne' => ['let', 'filter', 'replacement', 'session', 'upsert', 'arrayFilters', 'bypassDocumentValidation', 'collation', 'hint', 'comment', 'sort'],
'findOneAndUpdate' => ['let', 'returnDocument', 'filter', 'update', 'session', 'upsert', 'projection', 'remove', 'arrayFilters', 'bypassDocumentValidation', 'collation', 'hint', 'maxTimeMS', 'sort', 'comment'],
'updateMany' => ['let', 'filter', 'update', 'session', 'upsert', 'arrayFilters', 'bypassDocumentValidation', 'collation', 'hint', 'comment'],
'updateOne' => ['let', 'filter', 'update', 'session', 'upsert', 'arrayFilters', 'bypassDocumentValidation', 'collation', 'hint', 'comment'],
'updateOne' => ['let', 'filter', 'update', 'session', 'upsert', 'arrayFilters', 'bypassDocumentValidation', 'collation', 'hint', 'comment', 'sort'],
'updateSearchIndex' => ['name', 'definition'],
'insertMany' => ['documents', 'session', 'ordered', 'bypassDocumentValidation', 'comment'],
'insertOne' => ['document', 'session', 'bypassDocumentValidation', 'comment'],
Expand Down
Loading