Skip to content

Improve type of Collection Bulk Write operations #1694

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

Open
wants to merge 2 commits into
base: v2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
106 changes: 20 additions & 86 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -450,106 +450,40 @@
</file>
<file src="src/Operation/BulkWrite.php">
<MixedArgument>
<code><![CDATA[$args]]></code>
<code><![CDATA[$args]]></code>
<code><![CDATA[$args]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[2]]]></code>
</MixedArgument>
<MixedArrayAccess>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]['upsert']]]></code>
<code><![CDATA[$args[2]['upsert']]]></code>
<code><![CDATA[$args[2]['upsert']]]></code>
<code><![CDATA[$args[2]['upsert']]]></code>
</MixedArrayAccess>
<MixedArrayAssignment>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]['limit']]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]['multi']]]></code>
<code><![CDATA[$args[2]['multi']]]></code>
<code><![CDATA[$operations[$i][$type][0]]]></code>
<code><![CDATA[$operations[$i][$type][0]]]></code>
<code><![CDATA[$operations[$i][$type][0]]]></code>
<code><![CDATA[$operations[$i][$type][0]]]></code>
<code><![CDATA[$operations[$i][$type][1]]]></code>
<code><![CDATA[$operations[$i][$type][1]]]></code>
<code><![CDATA[$operations[$i][$type][1]]]></code>
<code><![CDATA[$operations[$i][$type][2]]]></code>
<code><![CDATA[$operations[$i][$type][2]]]></code>
</MixedArrayAssignment>
<MixedAssignment>
<code><![CDATA[$args]]></code>
<code><![CDATA[$args]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$args[2]]]></code>
<code><![CDATA[$insertedIds[$i]]]></code>
<code><![CDATA[$operations[$i][$type][0]]]></code>
<code><![CDATA[$operations[$i][$type][0]]]></code>
<code><![CDATA[$operations[$i][$type][0]]]></code>
<code><![CDATA[$operations[$i][$type][1]]]></code>
<code><![CDATA[$operations[$i][$type][1]]]></code>
<code><![CDATA[$operations[$i][$type][2]]]></code>
<code><![CDATA[$operations[$i][$type][2]]]></code>
<code><![CDATA[$operation[$type][0]]]></code>
<code><![CDATA[$operation[$type][0]]]></code>
<code><![CDATA[$operation[$type][0]]]></code>
<code><![CDATA[$operation[$type][1]]]></code>
<code><![CDATA[$options[$option]]]></code>
<code><![CDATA[$options['session']]]></code>
<code><![CDATA[$options['writeConcern']]]></code>
</MixedAssignment>
<MixedMethodCall>
<code><![CDATA[isInTransaction]]></code>
</MixedMethodCall>
<MixedOperand>
<code><![CDATA[$args[2]]]></code>
<NullArgument>
<code><![CDATA[$type]]></code>
</NullArgument>
<PossiblyInvalidArgument>
<code><![CDATA[$args[0]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
</PossiblyInvalidArgument>
<PossiblyNullArgument>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
</PossiblyNullArgument>
<PossiblyUndefinedArrayOffset>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[1]]]></code>
<code><![CDATA[$args[2]]]></code>
</MixedOperand>
</PossiblyUndefinedArrayOffset>
</file>
<file src="src/Operation/ClientBulkWriteCommand.php">
<MixedMethodCall>
Expand Down
5 changes: 3 additions & 2 deletions src/Collection.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
use function is_array;
use function strlen;

/** @psalm-import-type OperationType from BulkWrite */
class Collection
{
private const DEFAULT_TYPE_MAP = [
Expand Down Expand Up @@ -254,8 +255,8 @@ 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 array $options Command options
* @psalm-param list<OperationType> $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)
Expand Down
38 changes: 21 additions & 17 deletions src/Operation/BulkWrite.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -55,7 +58,7 @@
public const UPDATE_MANY = 'updateMany';
public const UPDATE_ONE = 'updateOne';

/** @var array[] */
/** @psalm-var list<OperationType> */
private array $operations;

private array $options;
Expand Down Expand Up @@ -132,10 +135,11 @@
*
* * 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<OperationType> $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 = [])
Expand Down Expand Up @@ -276,12 +280,12 @@
}

/**
* @param array[] $operations
* @return array[]
* @psalm-param list<OperationType> $operations
* @psalm-return list<OperationType>
*/
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');
}
Expand All @@ -306,14 +310,14 @@
// $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]);

Check notice

Code scanning / Psalm

PossiblyInvalidArgument Note

Argument 1 of MongoDB\Codec\DocumentCodec::encode expects object, but possibly different type array<array-key, mixed>|null|object provided
}

break;

case self::DELETE_MANY:
case self::DELETE_ONE:
$operations[$i][$type][0] = $builderEncoder->encodeIfSupported($args[0]);
$operation[$type][0] = $builderEncoder->encodeIfSupported($args[0]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

if (! isset($args[1])) {
$args[1] = [];
Expand All @@ -329,19 +333,19 @@
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]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

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]);

Check notice

Code scanning / Psalm

PossiblyInvalidArgument Note

Argument 1 of MongoDB\Codec\DocumentCodec::encode expects object, but possibly different type array<array-key, mixed>|null|object provided
}

if (! is_document($args[1])) {
Expand Down Expand Up @@ -384,19 +388,19 @@
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]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

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]);

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

Check notice

Code scanning / Psalm

MixedAssignment Note

Unable to determine the type of this assignment

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));
Expand Down Expand Up @@ -433,7 +437,7 @@
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;

Expand Down
9 changes: 9 additions & 0 deletions tests/Operation/BulkWriteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down