From af23cb24ccc89090a348a712b83aea3ae0b63350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 9 May 2025 15:56:14 +0200 Subject: [PATCH 1/3] PHPLIB-1679 Call DocumentCodec::encode() only on objects --- src/Operation/BulkWrite.php | 5 +++-- src/Operation/FindOneAndReplace.php | 7 +++---- src/Operation/InsertMany.php | 9 ++++----- src/Operation/InsertOne.php | 7 +++---- src/Operation/ReplaceOne.php | 7 +++---- 5 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index dc6dbb4c7..300747552 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -35,6 +35,7 @@ use function current; use function is_array; use function is_bool; +use function is_object; use function key; use function MongoDB\is_document; use function MongoDB\is_first_key_operator; @@ -305,7 +306,7 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En case self::INSERT_ONE: // $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) { + if ($codec && is_object($args[0])) { $operations[$i][$type][0] = $codec->encode($args[0]); } @@ -340,7 +341,7 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En throw new InvalidArgumentException(sprintf('Missing second argument for $operations[%d]["%s"]', $i, $type)); } - if ($codec) { + if ($codec && is_object($args[1])) { $operations[$i][$type][1] = $codec->encode($args[1]); } diff --git a/src/Operation/FindOneAndReplace.php b/src/Operation/FindOneAndReplace.php index 5ea0c5a34..cfcdfdf99 100644 --- a/src/Operation/FindOneAndReplace.php +++ b/src/Operation/FindOneAndReplace.php @@ -25,6 +25,7 @@ use function array_key_exists; use function is_integer; +use function is_object; use function MongoDB\is_document; use function MongoDB\is_first_key_operator; use function MongoDB\is_pipeline; @@ -170,11 +171,9 @@ public function getCommandDocument(): array private function validateReplacement(array|object $replacement, ?DocumentCodec $codec): array|object { - if (isset($codec)) { + if ($codec && is_object($replacement)) { $replacement = $codec->encode($replacement); - } - - if (! is_document($replacement)) { + } elseif (! is_document($replacement)) { throw InvalidArgumentException::expectedDocumentType('$replacement', $replacement); } diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index 70e149076..c1b547e02 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -29,6 +29,7 @@ use function array_is_list; use function is_bool; +use function is_object; use function MongoDB\is_document; use function sprintf; @@ -189,11 +190,9 @@ private function validateDocuments(array $documents, ?DocumentCodec $codec): arr } foreach ($documents as $i => $document) { - if ($codec) { - $document = $documents[$i] = $codec->encode($document); - } - - if (! is_document($document)) { + if ($codec && is_object($document)) { + $documents[$i] = $codec->encode($document); + } elseif (! is_document($document)) { throw InvalidArgumentException::expectedDocumentType(sprintf('$documents[%d]', $i), $document); } } diff --git a/src/Operation/InsertOne.php b/src/Operation/InsertOne.php index dff9f79f6..3c4930900 100644 --- a/src/Operation/InsertOne.php +++ b/src/Operation/InsertOne.php @@ -28,6 +28,7 @@ use MongoDB\InsertOneResult; use function is_bool; +use function is_object; use function MongoDB\is_document; /** @@ -156,11 +157,9 @@ private function createExecuteOptions(): array private function validateDocument(array|object $document, ?DocumentCodec $codec): array|object { - if ($codec) { + if ($codec && is_object($document)) { $document = $codec->encode($document); - } - - if (! is_document($document)) { + } elseif (! is_document($document)) { throw InvalidArgumentException::expectedDocumentType('$document', $document); } diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index 70bfd8f77..faf6a67e5 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -24,6 +24,7 @@ use MongoDB\Exception\UnsupportedException; use MongoDB\UpdateResult; +use function is_object; use function MongoDB\is_document; use function MongoDB\is_first_key_operator; use function MongoDB\is_pipeline; @@ -119,11 +120,9 @@ public function execute(Server $server): UpdateResult private function validateReplacement(array|object $replacement, ?DocumentCodec $codec): array|object { - if ($codec) { + if ($codec && is_object($replacement)) { $replacement = $codec->encode($replacement); - } - - if (! is_document($replacement)) { + } elseif (! is_document($replacement)) { throw InvalidArgumentException::expectedDocumentType('$replacement', $replacement); } From c2695fd09c330cf98237a90bc39575bd7e88fc2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 9 May 2025 17:11:47 +0200 Subject: [PATCH 2/3] Throw an UnsupportedValueException instead of skipping the codec for array input --- src/Operation/BulkWrite.php | 13 +++++++++++-- src/Operation/FindOneAndReplace.php | 5 +++++ src/Operation/InsertMany.php | 7 ++++++- src/Operation/InsertOne.php | 7 ++++++- src/Operation/ReplaceOne.php | 7 ++++++- 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index 300747552..bac22e3b3 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -28,6 +28,7 @@ use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; +use MongoDB\Exception\UnsupportedValueException; use function array_is_list; use function array_key_exists; @@ -306,7 +307,11 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En case self::INSERT_ONE: // $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 && is_object($args[0])) { + if ($codec) { + if (! is_object($args[0])) { + throw UnsupportedValueException::invalidEncodableValue($args[0]); + } + $operations[$i][$type][0] = $codec->encode($args[0]); } @@ -341,7 +346,11 @@ private function validateOperations(array $operations, ?DocumentCodec $codec, En throw new InvalidArgumentException(sprintf('Missing second argument for $operations[%d]["%s"]', $i, $type)); } - if ($codec && is_object($args[1])) { + if ($codec) { + if (! is_object($args[1])) { + throw UnsupportedValueException::invalidEncodableValue($args[1]); + } + $operations[$i][$type][1] = $codec->encode($args[1]); } diff --git a/src/Operation/FindOneAndReplace.php b/src/Operation/FindOneAndReplace.php index cfcdfdf99..578b78649 100644 --- a/src/Operation/FindOneAndReplace.php +++ b/src/Operation/FindOneAndReplace.php @@ -22,6 +22,7 @@ use MongoDB\Driver\Server; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; +use MongoDB\Exception\UnsupportedValueException; use function array_key_exists; use function is_integer; @@ -172,6 +173,10 @@ public function getCommandDocument(): array private function validateReplacement(array|object $replacement, ?DocumentCodec $codec): array|object { if ($codec && is_object($replacement)) { + if (! is_object($replacement)) { + throw UnsupportedValueException::invalidEncodableValue($replacement); + } + $replacement = $codec->encode($replacement); } elseif (! is_document($replacement)) { throw InvalidArgumentException::expectedDocumentType('$replacement', $replacement); diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index c1b547e02..0f7dd1c2a 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -25,6 +25,7 @@ use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\InsertManyResult; use function array_is_list; @@ -190,7 +191,11 @@ private function validateDocuments(array $documents, ?DocumentCodec $codec): arr } foreach ($documents as $i => $document) { - if ($codec && is_object($document)) { + if ($codec) { + if (! is_object($document)) { + throw UnsupportedValueException::invalidEncodableValue($document); + } + $documents[$i] = $codec->encode($document); } elseif (! is_document($document)) { throw InvalidArgumentException::expectedDocumentType(sprintf('$documents[%d]', $i), $document); diff --git a/src/Operation/InsertOne.php b/src/Operation/InsertOne.php index 3c4930900..0e41db74d 100644 --- a/src/Operation/InsertOne.php +++ b/src/Operation/InsertOne.php @@ -25,6 +25,7 @@ use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\InsertOneResult; use function is_bool; @@ -157,7 +158,11 @@ private function createExecuteOptions(): array private function validateDocument(array|object $document, ?DocumentCodec $codec): array|object { - if ($codec && is_object($document)) { + if ($codec) { + if (! is_object($document)) { + throw UnsupportedValueException::invalidEncodableValue($document); + } + $document = $codec->encode($document); } elseif (! is_document($document)) { throw InvalidArgumentException::expectedDocumentType('$document', $document); diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index faf6a67e5..de359b477 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -22,6 +22,7 @@ use MongoDB\Driver\Server; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\UpdateResult; use function is_object; @@ -120,7 +121,11 @@ public function execute(Server $server): UpdateResult private function validateReplacement(array|object $replacement, ?DocumentCodec $codec): array|object { - if ($codec && is_object($replacement)) { + if ($codec) { + if (! is_object($replacement)) { + throw UnsupportedValueException::invalidEncodableValue($replacement); + } + $replacement = $codec->encode($replacement); } elseif (! is_document($replacement)) { throw InvalidArgumentException::expectedDocumentType('$replacement', $replacement); From 28bac3115a264ba686bdf1615198eac6995e0dc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 9 May 2025 17:42:59 +0200 Subject: [PATCH 3/3] Update tests and psalm baseline --- psalm-baseline.xml | 18 +++---------- src/Operation/FindOneAndReplace.php | 2 +- .../CodecCollectionFunctionalTest.php | 27 ++++++++++++++++++- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 005534e0a..2a1f58f76 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -470,6 +470,10 @@ + + + + @@ -726,9 +730,6 @@ - - - @@ -745,9 +746,6 @@ - - - @@ -759,9 +757,6 @@ - - - @@ -789,11 +784,6 @@ - - - - - options['writeConcern']]]> diff --git a/src/Operation/FindOneAndReplace.php b/src/Operation/FindOneAndReplace.php index 578b78649..26d1cf751 100644 --- a/src/Operation/FindOneAndReplace.php +++ b/src/Operation/FindOneAndReplace.php @@ -172,7 +172,7 @@ public function getCommandDocument(): array private function validateReplacement(array|object $replacement, ?DocumentCodec $codec): array|object { - if ($codec && is_object($replacement)) { + if ($codec) { if (! is_object($replacement)) { throw UnsupportedValueException::invalidEncodableValue($replacement); } diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index 2447243d0..a145899bf 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -8,6 +8,7 @@ use MongoDB\Collection; use MongoDB\Driver\BulkWrite; use MongoDB\Exception\InvalidArgumentException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\Model\BSONDocument; use MongoDB\Operation\FindOneAndReplace; use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; @@ -264,6 +265,12 @@ public function testFindOneAndReplaceWithCodecAndTypemap(): void $this->collection->findOneAndReplace(['_id' => 1], TestObject::createForFixture(1), $options); } + public function testFindOneAndReplaceWithArray(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + $this->collection->findOneAndReplace(['_id' => 1], ['foo' => 'bar']); + } + public static function provideFindOptions(): Generator { yield 'Default codec' => [ @@ -413,6 +420,12 @@ public function testInsertMany($expected, $options): void $this->assertEquals($expected, $this->collection->find([], $options)->toArray()); } + public function testInsertManyWithArray(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + $this->collection->insertMany([['foo' => 'bar']]); + } + public static function provideInsertOneOptions(): Generator { yield 'Default codec' => [ @@ -452,6 +465,12 @@ public function testInsertOne($expected, $options): void $this->assertEquals($expected, $this->collection->findOne([], $options)); } + public function testInsertOneWithArray(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + $this->collection->insertOne(['foo' => 'bar']); + } + public static function provideReplaceOneOptions(): Generator { $replacedObject = TestObject::createDecodedForFixture(1); @@ -499,7 +518,13 @@ public function testReplaceOneWithCodecAndTypemap(): void ]; $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); - $this->collection->replaceOne(['_id' => 1], ['foo' => 'bar'], $options); + $this->collection->replaceOne(['_id' => 1], (object) ['foo' => 'bar'], $options); + } + + public function testReplaceOneWithArray(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + $this->collection->replaceOne(['_id' => 1], ['foo' => 'bar']); } /**