From 8e37d537c61b6a2ed400e47f64146a0ea1b490c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 20 May 2025 13:24:12 +0200 Subject: [PATCH 1/2] Improve type of Collection Bulk Write operations --- psalm-baseline.xml | 104 +++++++----------------------- src/Collection.php | 3 +- src/Operation/BulkWrite.php | 18 ++++-- tests/Operation/BulkWriteTest.php | 9 +++ 4 files changed, 44 insertions(+), 90 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index a6dd2d5bf..37851a79d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -449,96 +449,20 @@ + + + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -546,10 +470,26 @@ - - + + ]]> + + + + + + + + + + + + + + + + - + diff --git a/src/Collection.php b/src/Collection.php index 04a61981c..c0f73525b 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -77,6 +77,7 @@ use function is_array; use function strlen; +/** @psalm-import-type OperationType from BulkWrite */ class Collection { private const DEFAULT_TYPE_MAP = [ @@ -254,7 +255,7 @@ public function aggregate(array|Pipeline $pipeline, array $options = []): Cursor * Executes multiple write operations. * * @see BulkWrite::__construct() for supported options - * @param array[] $operations List of write operations + * @param list $operations List of write operations * @param array $options Command options * @throws UnsupportedException if options are not supported by the selected server * @throws InvalidArgumentException for parameter/option parsing errors diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index dc6dbb4c7..6229e4daf 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -45,6 +45,9 @@ * Operation for executing multiple write operations. * * @see \MongoDB\Collection::bulkWrite() + * + * @psalm-type Document = object|array + * @psalm-type OperationType = array{deleteMany: array{0: Document, 1?: array}}|array{deleteOne: array{0: Document, 1?: array}}|array{insertOne: array{0: Document}}|array{replaceOne: array{0: Document, 1: Document, 2?: array}}|array{updateMany: array{0: Document, 1: Document, 2?: array}}|array{updateOne: array{0: Document, 1: Document, 2?: array}} */ final class BulkWrite { @@ -55,7 +58,7 @@ final class BulkWrite public const UPDATE_MANY = 'updateMany'; public const UPDATE_ONE = 'updateOne'; - /** @var array[] */ + /** @psalm-var list */ private array $operations; private array $options; @@ -132,10 +135,11 @@ final class BulkWrite * * * writeConcern (MongoDB\Driver\WriteConcern): Write concern. * - * @param string $databaseName Database name - * @param string $collectionName Collection name - * @param array[] $operations List of write operations - * @param array $options Command options + * @param string $databaseName Database name + * @param string $collectionName Collection name + * @param array $operations List of write operations + * @psalm-param list $operations + * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors */ public function __construct(private string $databaseName, private string $collectionName, array $operations, array $options = []) @@ -276,8 +280,8 @@ private function createExecuteOptions(): array } /** - * @param array[] $operations - * @return array[] + * @psalm-param list $operations + * @psalm-return list */ private function validateOperations(array $operations, ?DocumentCodec $codec, Encoder $builderEncoder): array { diff --git a/tests/Operation/BulkWriteTest.php b/tests/Operation/BulkWriteTest.php index 30d5c6374..58df1493f 100644 --- a/tests/Operation/BulkWriteTest.php +++ b/tests/Operation/BulkWriteTest.php @@ -41,6 +41,15 @@ public function testMultipleOperationsInOneElement(): void ]); } + public function testEmptyOperation(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Expected one element in $operation[0], actually: 0'); + new BulkWrite($this->getDatabaseName(), $this->getCollectionName(), [ + [], + ]); + } + public function testUnknownOperation(): void { $this->expectException(InvalidArgumentException::class); From f4a3b44d0a2a8cec83608e9fab185c7a41d7f472 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 22 May 2025 16:06:52 +0200 Subject: [PATCH 2/2] Update $operation by reference --- psalm-baseline.xml | 14 ++++---------- src/Collection.php | 4 ++-- src/Operation/BulkWrite.php | 22 +++++++++++----------- 3 files changed, 17 insertions(+), 23 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 37851a79d..5d728d764 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -449,9 +449,6 @@ - - - @@ -459,10 +456,10 @@ - - - - + + + + @@ -470,9 +467,6 @@ - - ]]> - diff --git a/src/Collection.php b/src/Collection.php index c0f73525b..1f1e55b79 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -255,8 +255,8 @@ public function aggregate(array|Pipeline $pipeline, array $options = []): Cursor * Executes multiple write operations. * * @see BulkWrite::__construct() for supported options - * @param list $operations List of write operations - * @param array $options Command options + * @psalm-param list $operations List of write operations + * @param array $options Command options * @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) diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index 6229e4daf..a4bfde99a 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -137,7 +137,7 @@ final class BulkWrite * * @param string $databaseName Database name * @param string $collectionName Collection name - * @param array $operations List of write operations + * @param array $operations List of write operations * @psalm-param list $operations * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors @@ -285,7 +285,7 @@ private function createExecuteOptions(): array */ private function validateOperations(array $operations, ?DocumentCodec $codec, Encoder $builderEncoder): array { - foreach ($operations as $i => $operation) { + foreach ($operations as $i => &$operation) { if (! is_array($operation)) { throw InvalidArgumentException::invalidType(sprintf('$operations[%d]', $i), $operation, 'array'); } @@ -310,14 +310,14 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En // $args[0] was already validated above. Since DocumentCodec::encode will always return a Document // instance, there is no need to re-validate the returned value here. if ($codec) { - $operations[$i][$type][0] = $codec->encode($args[0]); + $operation[$type][0] = $codec->encode($args[0]); } break; case self::DELETE_MANY: case self::DELETE_ONE: - $operations[$i][$type][0] = $builderEncoder->encodeIfSupported($args[0]); + $operation[$type][0] = $builderEncoder->encodeIfSupported($args[0]); if (! isset($args[1])) { $args[1] = []; @@ -333,19 +333,19 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En throw InvalidArgumentException::expectedDocumentType(sprintf('$operations[%d]["%s"][1]["collation"]', $i, $type), $args[1]['collation']); } - $operations[$i][$type][1] = $args[1]; + $operation[$type][1] = $args[1]; break; case self::REPLACE_ONE: - $operations[$i][$type][0] = $builderEncoder->encodeIfSupported($args[0]); + $operation[$type][0] = $builderEncoder->encodeIfSupported($args[0]); if (! isset($args[1]) && ! array_key_exists(1, $args)) { throw new InvalidArgumentException(sprintf('Missing second argument for $operations[%d]["%s"]', $i, $type)); } if ($codec) { - $operations[$i][$type][1] = $codec->encode($args[1]); + $operation[$type][1] = $codec->encode($args[1]); } if (! is_document($args[1])) { @@ -388,19 +388,19 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En throw InvalidArgumentException::invalidType(sprintf('$operations[%d]["%s"][2]["upsert"]', $i, $type), $args[2]['upsert'], 'boolean'); } - $operations[$i][$type][2] = $args[2]; + $operation[$type][2] = $args[2]; break; case self::UPDATE_MANY: case self::UPDATE_ONE: - $operations[$i][$type][0] = $builderEncoder->encodeIfSupported($args[0]); + $operation[$type][0] = $builderEncoder->encodeIfSupported($args[0]); if (! isset($args[1]) && ! array_key_exists(1, $args)) { throw new InvalidArgumentException(sprintf('Missing second argument for $operations[%d]["%s"]', $i, $type)); } - $operations[$i][$type][1] = $args[1] = $builderEncoder->encodeIfSupported($args[1]); + $operation[$type][1] = $args[1] = $builderEncoder->encodeIfSupported($args[1]); if ((! is_document($args[1]) || ! is_first_key_operator($args[1])) && ! is_pipeline($args[1])) { throw new InvalidArgumentException(sprintf('Expected update operator(s) or non-empty pipeline for $operations[%d]["%s"][1]', $i, $type)); @@ -437,7 +437,7 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En throw InvalidArgumentException::invalidType(sprintf('$operations[%d]["%s"][2]["upsert"]', $i, $type), $args[2]['upsert'], 'boolean'); } - $operations[$i][$type][2] = $args[2]; + $operation[$type][2] = $args[2]; break;