From 7b10f77e0b22a7f076bff9cc63aec10ec0478408 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 17 Jul 2023 10:16:03 +0200 Subject: [PATCH 01/37] Add Codec support to find operations --- psalm-baseline.xml | 13 ++ src/Collection.php | 4 +- src/GridFS/Bucket.php | 5 +- src/GridFS/CollectionWrapper.php | 8 +- src/GridFS/ReadableStream.php | 6 +- src/Model/CodecCursor.php | 126 +++++++++++++++++++ src/Operation/Find.php | 18 ++- src/Operation/FindOne.php | 3 + tests/Fixtures/Codec/TestDocumentCodec.php | 71 +++++++++++ tests/Fixtures/Document/TestNestedObject.php | 23 ++++ tests/Fixtures/Document/TestObject.php | 39 ++++++ tests/Operation/FindFunctionalTest.php | 21 ++++ tests/Operation/FindOneFunctionalTest.php | 14 +++ tests/Operation/FindTest.php | 1 + tests/TestCase.php | 6 + 15 files changed, 348 insertions(+), 10 deletions(-) create mode 100644 src/Model/CodecCursor.php create mode 100644 tests/Fixtures/Codec/TestDocumentCodec.php create mode 100644 tests/Fixtures/Document/TestNestedObject.php create mode 100644 tests/Fixtures/Document/TestObject.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index e6b0a3436..032b55298 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -401,6 +401,7 @@ + options['codec']]]> options['typeMap']]]> @@ -439,6 +440,18 @@ value ?? null) : null]]> + + + $document + + + array|object|null + + + $document === false ? null : $document + $document === false ? null : $document + + diff --git a/src/Collection.php b/src/Collection.php index 801511951..89fec9504 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -17,8 +17,10 @@ namespace MongoDB; +use Iterator; use MongoDB\BSON\JavascriptInterface; use MongoDB\Driver\Cursor; +use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Manager; use MongoDB\Driver\ReadConcern; @@ -618,7 +620,7 @@ public function explain(Explainable $explainable, array $options = []) * @see https://mongodb.com/docs/manual/crud/#read-operations * @param array|object $filter Query by which to filter documents * @param array $options Additional options - * @return Cursor + * @return CursorInterface&Iterator * @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/GridFS/Bucket.php b/src/GridFS/Bucket.php index f571586ed..aeba6708f 100644 --- a/src/GridFS/Bucket.php +++ b/src/GridFS/Bucket.php @@ -17,8 +17,9 @@ namespace MongoDB\GridFS; +use Iterator; use MongoDB\Collection; -use MongoDB\Driver\Cursor; +use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Manager; use MongoDB\Driver\ReadConcern; @@ -301,7 +302,7 @@ public function drop() * @see Find::__construct() for supported options * @param array|object $filter Query by which to filter documents * @param array $options Additional options - * @return Cursor + * @return CursorInterface&Iterator * @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/GridFS/CollectionWrapper.php b/src/GridFS/CollectionWrapper.php index 11ad145a6..16ef8343a 100644 --- a/src/GridFS/CollectionWrapper.php +++ b/src/GridFS/CollectionWrapper.php @@ -18,8 +18,9 @@ namespace MongoDB\GridFS; use ArrayIterator; +use Iterator; use MongoDB\Collection; -use MongoDB\Driver\Cursor; +use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Manager; use MongoDB\Driver\ReadPreference; use MongoDB\Exception\InvalidArgumentException; @@ -104,8 +105,9 @@ public function dropCollections(): void * * @param mixed $id File ID * @param integer $fromChunk Starting chunk (inclusive) + * @return CursorInterface&Iterator */ - public function findChunksByFileId($id, int $fromChunk = 0): Cursor + public function findChunksByFileId($id, int $fromChunk = 0) { return $this->chunksCollection->find( [ @@ -182,7 +184,7 @@ public function findFileById($id): ?object * @see Find::__construct() for supported options * @param array|object $filter Query by which to filter documents * @param array $options Additional options - * @return Cursor + * @return CursorInterface&Iterator */ public function findFiles($filter, array $options = []) { diff --git a/src/GridFS/ReadableStream.php b/src/GridFS/ReadableStream.php index 577384f2b..d975ec43b 100644 --- a/src/GridFS/ReadableStream.php +++ b/src/GridFS/ReadableStream.php @@ -17,8 +17,9 @@ namespace MongoDB\GridFS; +use Iterator; use MongoDB\BSON\Binary; -use MongoDB\Driver\Cursor; +use MongoDB\Driver\CursorInterface; use MongoDB\Exception\InvalidArgumentException; use MongoDB\GridFS\Exception\CorruptFileException; @@ -47,7 +48,8 @@ class ReadableStream private int $chunkOffset = 0; - private ?Cursor $chunksIterator = null; + /** @var (CursorInterface&Iterator)|null */ + private $chunksIterator = null; private CollectionWrapper $collectionWrapper; diff --git a/src/Model/CodecCursor.php b/src/Model/CodecCursor.php new file mode 100644 index 000000000..c4c9e9b97 --- /dev/null +++ b/src/Model/CodecCursor.php @@ -0,0 +1,126 @@ + + * @template-implements Iterator + */ +class CodecCursor implements CursorInterface, Iterator +{ + private const TYPEMAP = ['root' => 'bson']; + + private Cursor $cursor; + + /** @var DocumentCodec */ + private DocumentCodec $codec; + + /** @var TValue|null */ + private ?object $current = null; + + /** @return TValue */ + public function current(): ?object + { + if (! $this->current && $this->valid()) { + $value = $this->cursor->current(); + assert($value instanceof Document); + $this->current = $this->codec->decode($value); + } + + return $this->current; + } + + /** + * @template NativeClass of Object + * @param DocumentCodec $codec + * @return self + */ + public static function fromCursor(Cursor $cursor, DocumentCodec $codec): self + { + $cursor->setTypeMap(self::TYPEMAP); + + return new self($cursor, $codec); + } + + public function getId(): CursorId + { + return $this->cursor->getId(); + } + + public function getServer(): Server + { + return $this->cursor->getServer(); + } + + public function isDead(): bool + { + return $this->cursor->isDead(); + } + + public function key(): int + { + return $this->cursor->key(); + } + + public function next(): void + { + $this->current = null; + $this->cursor->next(); + } + + public function rewind(): void + { + $this->current = null; + $this->cursor->rewind(); + } + + public function setTypeMap(array $typemap): void + { + // Not supported + } + + /** @return array */ + public function toArray(): array + { + return iterator_to_array($this); + } + + public function valid(): bool + { + return $this->cursor->valid(); + } + + /** @param DocumentCodec $codec */ + private function __construct(Cursor $cursor, DocumentCodec $codec) + { + $this->cursor = $cursor; + $this->codec = $codec; + } +} diff --git a/src/Operation/Find.php b/src/Operation/Find.php index 622b5a540..e461f5ad2 100644 --- a/src/Operation/Find.php +++ b/src/Operation/Find.php @@ -17,7 +17,9 @@ namespace MongoDB\Operation; -use MongoDB\Driver\Cursor; +use Iterator; +use MongoDB\Codec\DocumentCodec; +use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Query; use MongoDB\Driver\ReadConcern; @@ -26,6 +28,7 @@ use MongoDB\Driver\Session; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; +use MongoDB\Model\CodecCursor; use function is_array; use function is_bool; @@ -74,6 +77,9 @@ class Find implements Executable, Explainable * * * batchSize (integer): The number of documents to return per batch. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * collation (document): Collation specification. * * * comment (mixed): BSON value to attach as a comment to this command. @@ -176,6 +182,10 @@ public function __construct(string $databaseName, string $collectionName, $filte throw InvalidArgumentException::invalidType('"batchSize" option', $options['batchSize'], 'integer'); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (isset($options['collation']) && ! is_document($options['collation'])) { throw InvalidArgumentException::expectedDocumentType('"collation" option', $options['collation']); } @@ -300,7 +310,7 @@ public function __construct(string $databaseName, string $collectionName, $filte * Execute the operation. * * @see Executable::execute() - * @return Cursor + * @return CursorInterface&Iterator * @throws UnsupportedException if read concern is used and unsupported * @throws DriverRuntimeException for other driver errors (e.g. connection errors) */ @@ -313,6 +323,10 @@ public function execute(Server $server) $cursor = $server->executeQuery($this->databaseName . '.' . $this->collectionName, new Query($this->filter, $this->createQueryOptions()), $this->createExecuteOptions()); + if (isset($this->options['codec'])) { + return CodecCursor::fromCursor($cursor, $this->options['codec']); + } + if (isset($this->options['typeMap'])) { $cursor->setTypeMap($this->options['typeMap']); } diff --git a/src/Operation/FindOne.php b/src/Operation/FindOne.php index 8d6430058..48f931fab 100644 --- a/src/Operation/FindOne.php +++ b/src/Operation/FindOne.php @@ -40,6 +40,9 @@ class FindOne implements Executable, Explainable * * Supported options: * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * collation (document): Collation specification. * * * comment (mixed): BSON value to attach as a comment to this command. diff --git a/tests/Fixtures/Codec/TestDocumentCodec.php b/tests/Fixtures/Codec/TestDocumentCodec.php new file mode 100644 index 000000000..3df41e304 --- /dev/null +++ b/tests/Fixtures/Codec/TestDocumentCodec.php @@ -0,0 +1,71 @@ +id = $value->get('_id'); + $object->decoded = true; + + $object->x = new TestNestedObject(); + $object->x->foo = $value->get('x')->get('foo'); + + return $object; + } + + public function canEncode($value): bool + { + return $value instanceof TestObject; + } + + public function encode($value): Document + { + if (! $value instanceof TestObject) { + throw UnsupportedValueException::invalidEncodableValue($value); + } + + return Document::fromPHP([ + '_id' => $value->id, + 'x' => Document::fromPHP(['foo' => $value->x->foo]), + 'encoded' => true, + ]); + } +} diff --git a/tests/Fixtures/Document/TestNestedObject.php b/tests/Fixtures/Document/TestNestedObject.php new file mode 100644 index 000000000..3c3354ecf --- /dev/null +++ b/tests/Fixtures/Document/TestNestedObject.php @@ -0,0 +1,23 @@ +id = $i; + $instance->decoded = $decoded; + + $instance->x = new TestNestedObject(); + $instance->x->foo = 'bar'; + + return $instance; + } +} diff --git a/tests/Operation/FindFunctionalTest.php b/tests/Operation/FindFunctionalTest.php index 53b50f4da..c3283c66f 100644 --- a/tests/Operation/FindFunctionalTest.php +++ b/tests/Operation/FindFunctionalTest.php @@ -9,6 +9,8 @@ use MongoDB\Operation\CreateIndexes; use MongoDB\Operation\Find; use MongoDB\Tests\CommandObserver; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; +use MongoDB\Tests\Fixtures\Document\TestObject; use stdClass; use function microtime; @@ -196,6 +198,25 @@ public function provideTypeMapOptionsAndExpectedDocuments() ]; } + public function testCodecOption(): void + { + $this->createFixtures(3); + + $codec = new TestDocumentCodec(); + + $operation = new Find($this->getDatabaseName(), $this->getCollectionName(), [], ['codec' => $codec]); + $cursor = $operation->execute($this->getPrimaryServer()); + + $this->assertEquals( + [ + TestObject::createForFixture(1, true), + TestObject::createForFixture(2, true), + TestObject::createForFixture(3, true), + ], + $cursor->toArray(), + ); + } + public function testMaxAwaitTimeMS(): void { $maxAwaitTimeMS = 100; diff --git a/tests/Operation/FindOneFunctionalTest.php b/tests/Operation/FindOneFunctionalTest.php index d18d056e3..afbc9a6f2 100644 --- a/tests/Operation/FindOneFunctionalTest.php +++ b/tests/Operation/FindOneFunctionalTest.php @@ -4,6 +4,8 @@ use MongoDB\Driver\BulkWrite; use MongoDB\Operation\FindOne; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; +use MongoDB\Tests\Fixtures\Document\TestObject; class FindOneFunctionalTest extends FunctionalTestCase { @@ -36,6 +38,18 @@ public function provideTypeMapOptionsAndExpectedDocument() ]; } + public function testCodecOption(): void + { + $this->createFixtures(1); + + $codec = new TestDocumentCodec(); + + $operation = new FindOne($this->getDatabaseName(), $this->getCollectionName(), [], ['codec' => $codec]); + $document = $operation->execute($this->getPrimaryServer()); + + $this->assertEquals(TestObject::createForFixture(1, true), $document); + } + /** * Create data fixtures. */ diff --git a/tests/Operation/FindTest.php b/tests/Operation/FindTest.php index 3a24ed3ee..68341d2ac 100644 --- a/tests/Operation/FindTest.php +++ b/tests/Operation/FindTest.php @@ -28,6 +28,7 @@ public function provideInvalidConstructorOptions() return $this->createOptionDataProvider([ 'allowPartialResults' => $this->getInvalidBooleanValues(), 'batchSize' => $this->getInvalidIntegerValues(), + 'codec' => $this->getInvalidDocumentCodecValues(), 'collation' => $this->getInvalidDocumentValues(), 'cursorType' => $this->getInvalidIntegerValues(), 'hint' => $this->getInvalidHintValues(), diff --git a/tests/TestCase.php b/tests/TestCase.php index a5d20b837..9b2f69982 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -4,6 +4,7 @@ use InvalidArgumentException; use MongoDB\BSON\PackedArray; +use MongoDB\Codec\Codec; use MongoDB\Driver\ReadConcern; use MongoDB\Driver\ReadPreference; use MongoDB\Driver\WriteConcern; @@ -248,6 +249,11 @@ protected function getInvalidDocumentValues(bool $includeNull = false): array return array_merge([123, 3.14, 'foo', true, PackedArray::fromPHP([])], $includeNull ? [null] : []); } + protected function getInvalidDocumentCodecValues(): array + { + return [123, 3.14, 'foo', true, [], new stdClass(), $this->createMock(Codec::class)]; + } + /** * Return a list of invalid hint values. */ From 2ceed92d2240e665cddf5298771afd84aa4671e3 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 17 Jul 2023 10:16:35 +0200 Subject: [PATCH 02/37] Add codec support to insert operations --- psalm-baseline.xml | 2 + src/Operation/InsertMany.php | 20 ++++++++- src/Operation/InsertOne.php | 21 ++++++++- tests/Operation/InsertManyFunctionalTest.php | 46 ++++++++++++++++++++ tests/Operation/InsertManyTest.php | 1 + tests/Operation/InsertOneFunctionalTest.php | 27 ++++++++++++ tests/Operation/InsertOneTest.php | 1 + 7 files changed, 116 insertions(+), 2 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 032b55298..c9ec7b6fd 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -475,6 +475,7 @@ + encodeIfSupported isInTransaction @@ -486,6 +487,7 @@ + encodeIfSupported isInTransaction diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index 1d5cc981b..5c6020417 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -17,6 +17,7 @@ namespace MongoDB\Operation; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\BulkWrite as Bulk; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Server; @@ -27,7 +28,10 @@ use MongoDB\InsertManyResult; use function array_is_list; +use function assert; +use function is_array; use function is_bool; +use function is_object; use function MongoDB\is_document; use function sprintf; @@ -56,6 +60,9 @@ class InsertMany implements Executable * * bypassDocumentValidation (boolean): If true, allows the write to * circumvent document level validation. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to encode PHP objects + * into BSON. + * * * comment (mixed): BSON value to attach as a comment to the command(s) * associated with this insert. * @@ -97,6 +104,10 @@ public function __construct(string $databaseName, string $collectionName, array throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean'); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (! is_bool($options['ordered'])) { throw InvalidArgumentException::invalidType('"ordered" option', $options['ordered'], 'boolean'); } @@ -142,7 +153,14 @@ public function execute(Server $server) $insertedIds = []; foreach ($this->documents as $i => $document) { - $insertedIds[$i] = $bulk->insert($document); + $insertedDocument = isset($this->options['codec']) + ? $this->options['codec']->encodeIfSupported($document) + : $document; + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + assert(is_array($insertedDocument) || is_object($insertedDocument)); + + $insertedIds[$i] = $bulk->insert($insertedDocument); } $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); diff --git a/src/Operation/InsertOne.php b/src/Operation/InsertOne.php index 1b93b4fc5..8e59efc6d 100644 --- a/src/Operation/InsertOne.php +++ b/src/Operation/InsertOne.php @@ -17,6 +17,7 @@ namespace MongoDB\Operation; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\BulkWrite as Bulk; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Server; @@ -26,7 +27,10 @@ use MongoDB\Exception\UnsupportedException; use MongoDB\InsertOneResult; +use function assert; +use function is_array; use function is_bool; +use function is_object; use function MongoDB\is_document; /** @@ -54,6 +58,9 @@ class InsertOne implements Executable * * bypassDocumentValidation (boolean): If true, allows the write to * circumvent document level validation. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to encode PHP objects + * into BSON. + * * * comment (mixed): BSON value to attach as a comment to this command. * * This is not supported for servers versions < 4.4. @@ -78,6 +85,10 @@ public function __construct(string $databaseName, string $collectionName, $docum throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean'); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (isset($options['session']) && ! $options['session'] instanceof Session) { throw InvalidArgumentException::invalidType('"session" option', $options['session'], Session::class); } @@ -116,7 +127,15 @@ public function execute(Server $server) } $bulk = new Bulk($this->createBulkWriteOptions()); - $insertedId = $bulk->insert($this->document); + + $insertedDocument = isset($this->options['codec']) + ? $this->options['codec']->encodeIfSupported($this->document) + : $this->document; + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + assert(is_array($insertedDocument) || is_object($insertedDocument)); + + $insertedId = $bulk->insert($insertedDocument); $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); diff --git a/tests/Operation/InsertManyFunctionalTest.php b/tests/Operation/InsertManyFunctionalTest.php index b8ca886be..ae17f7342 100644 --- a/tests/Operation/InsertManyFunctionalTest.php +++ b/tests/Operation/InsertManyFunctionalTest.php @@ -11,6 +11,8 @@ use MongoDB\Model\BSONDocument; use MongoDB\Operation\InsertMany; use MongoDB\Tests\CommandObserver; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; +use MongoDB\Tests\Fixtures\Document\TestObject; class InsertManyFunctionalTest extends FunctionalTestCase { @@ -200,4 +202,48 @@ public function testUnacknowledgedWriteConcernAccessesInsertedId(InsertManyResul { $this->assertInstanceOf(ObjectId::class, $result->getInsertedIds()[0]); } + + public function testInsertingWithCodec(): void + { + $documents = [ + TestObject::createForFixture(1), + TestObject::createForFixture(2), + TestObject::createForFixture(3), + ]; + + $expectedDocuments = [ + (object) [ + '_id' => 1, + 'x' => (object) ['foo' => 'bar'], + 'encoded' => true, + ], + (object) [ + '_id' => 2, + 'x' => (object) ['foo' => 'bar'], + 'encoded' => true, + ], + (object) [ + '_id' => 3, + 'x' => (object) ['foo' => 'bar'], + 'encoded' => true, + ], + ]; + + (new CommandObserver())->observe( + function () use ($documents): void { + $operation = new InsertMany( + $this->getDatabaseName(), + $this->getCollectionName(), + $documents, + ['codec' => new TestDocumentCodec()], + ); + + $result = $operation->execute($this->getPrimaryServer()); + $this->assertEquals([1, 2, 3], $result->getInsertedIds()); + }, + function (array $event) use ($expectedDocuments): void { + $this->assertEquals($expectedDocuments, $event['started']->getCommand()->documents ?? null); + }, + ); + } } diff --git a/tests/Operation/InsertManyTest.php b/tests/Operation/InsertManyTest.php index c3edec535..a16710b75 100644 --- a/tests/Operation/InsertManyTest.php +++ b/tests/Operation/InsertManyTest.php @@ -40,6 +40,7 @@ public function provideInvalidConstructorOptions() { return $this->createOptionDataProvider([ 'bypassDocumentValidation' => $this->getInvalidBooleanValues(), + 'codec' => $this->getInvalidDocumentCodecValues(), 'ordered' => $this->getInvalidBooleanValues(true), 'session' => $this->getInvalidSessionValues(), 'writeConcern' => $this->getInvalidWriteConcernValues(), diff --git a/tests/Operation/InsertOneFunctionalTest.php b/tests/Operation/InsertOneFunctionalTest.php index 0a158ccb5..2075f0fc0 100644 --- a/tests/Operation/InsertOneFunctionalTest.php +++ b/tests/Operation/InsertOneFunctionalTest.php @@ -11,6 +11,8 @@ use MongoDB\Model\BSONDocument; use MongoDB\Operation\InsertOne; use MongoDB\Tests\CommandObserver; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; +use MongoDB\Tests\Fixtures\Document\TestObject; use stdClass; class InsertOneFunctionalTest extends FunctionalTestCase @@ -191,4 +193,29 @@ public function testUnacknowledgedWriteConcernAccessesInsertedId(InsertOneResult { $this->assertInstanceOf(ObjectId::class, $result->getInsertedId()); } + + public function testInsertingWithCodec(): void + { + (new CommandObserver())->observe( + function (): void { + $document = TestObject::createForFixture(1); + $options = ['codec' => new TestDocumentCodec()]; + + $operation = new InsertOne($this->getDatabaseName(), $this->getCollectionName(), $document, $options); + $result = $operation->execute($this->getPrimaryServer()); + + $this->assertSame(1, $result->getInsertedId()); + }, + function (array $event): void { + $this->assertEquals( + (object) [ + '_id' => 1, + 'x' => (object) ['foo' => 'bar'], + 'encoded' => true, + ], + $event['started']->getCommand()->documents[0] ?? null, + ); + }, + ); + } } diff --git a/tests/Operation/InsertOneTest.php b/tests/Operation/InsertOneTest.php index 17fde0343..868c0fe47 100644 --- a/tests/Operation/InsertOneTest.php +++ b/tests/Operation/InsertOneTest.php @@ -25,6 +25,7 @@ public function provideInvalidConstructorOptions() { return $this->createOptionDataProvider([ 'bypassDocumentValidation' => $this->getInvalidBooleanValues(), + 'codec' => $this->getInvalidDocumentCodecValues(), 'session' => $this->getInvalidSessionValues(), 'writeConcern' => $this->getInvalidWriteConcernValues(), ]); From db998c32868e6628f92dc2694cff165ebd1ea614 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 17 Jul 2023 10:16:54 +0200 Subject: [PATCH 03/37] Add codec support to replace operation --- psalm-baseline.xml | 5 +++ src/Operation/ReplaceOne.php | 20 ++++++++++- tests/Operation/ReplaceOneFunctionalTest.php | 38 ++++++++++++++++++++ tests/Operation/ReplaceOneTest.php | 14 ++++++++ 4 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 tests/Operation/ReplaceOneFunctionalTest.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index c9ec7b6fd..b619a7c2b 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -537,6 +537,11 @@ isInTransaction + + + is_object($replacementDocument) + + options['writeConcern']]]> diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index c56d9c94d..3f1528b24 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -17,12 +17,16 @@ namespace MongoDB\Operation; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Server; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; use MongoDB\UpdateResult; +use function assert; +use function is_array; +use function is_object; use function MongoDB\is_document; use function MongoDB\is_first_key_operator; use function MongoDB\is_pipeline; @@ -45,6 +49,9 @@ class ReplaceOne implements Executable * * bypassDocumentValidation (boolean): If true, allows the write to * circumvent document level validation. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to encode PHP objects + * into BSON. + * * * collation (document): Collation specification. * * * comment (mixed): BSON value to attach as a comment to this command. @@ -96,11 +103,22 @@ public function __construct(string $databaseName, string $collectionName, $filte throw new InvalidArgumentException('$replacement is an update pipeline'); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + + $replacementDocument = isset($options['codec']) + ? $options['codec']->encodeIfSupported($replacement) + : $replacement; + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + assert(is_array($replacementDocument) || is_object($replacementDocument)); + $this->update = new Update( $databaseName, $collectionName, $filter, - $replacement, + $replacementDocument, ['multi' => false] + $options, ); } diff --git a/tests/Operation/ReplaceOneFunctionalTest.php b/tests/Operation/ReplaceOneFunctionalTest.php new file mode 100644 index 000000000..7a15efbf4 --- /dev/null +++ b/tests/Operation/ReplaceOneFunctionalTest.php @@ -0,0 +1,38 @@ +observe( + function (): void { + $operation = new ReplaceOne( + $this->getDatabaseName(), + $this->getCollectionName(), + ['x' => 1], + TestObject::createForFixture(1), + ['codec' => new TestDocumentCodec()], + ); + + $operation->execute($this->getPrimaryServer()); + }, + function (array $event): void { + $this->assertEquals( + (object) [ + '_id' => 1, + 'x' => (object) ['foo' => 'bar'], + 'encoded' => true, + ], + $event['started']->getCommand()->updates[0]->u ?? null, + ); + }, + ); + } +} diff --git a/tests/Operation/ReplaceOneTest.php b/tests/Operation/ReplaceOneTest.php index 1290f90df..0de93f271 100644 --- a/tests/Operation/ReplaceOneTest.php +++ b/tests/Operation/ReplaceOneTest.php @@ -48,4 +48,18 @@ public function testConstructorReplacementArgumentProhibitsUpdatePipeline($repla $this->expectExceptionMessageMatches('#(\$replacement is an update pipeline)|(Expected \$replacement to have type "document" \(array or object\))#'); new ReplaceOne($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $replacement); } + + /** @dataProvider provideInvalidConstructorOptions */ + public function testConstructorOptionsTypeCheck($options): void + { + $this->expectException(InvalidArgumentException::class); + new ReplaceOne($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], ['y' => 1], $options); + } + + public function provideInvalidConstructorOptions() + { + return $this->createOptionDataProvider([ + 'codec' => $this->getInvalidDocumentCodecValues(), + ]); + } } From d30c8b8a88a02758e6f92381d773f35aa12cead9 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 17 Jul 2023 10:17:15 +0200 Subject: [PATCH 04/37] Add codec support to aggregate operation --- psalm-baseline.xml | 3 ++- src/Collection.php | 3 +-- src/Model/ChangeStreamIterator.php | 13 ++++++----- src/Operation/Aggregate.php | 17 +++++++++++++- src/Operation/Watch.php | 7 ++++-- tests/Operation/AggregateFunctionalTest.php | 26 +++++++++++++++++++++ tests/Operation/AggregateTest.php | 1 + 7 files changed, 58 insertions(+), 12 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index b619a7c2b..ef2ad0902 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -170,6 +170,7 @@ + options['codec']]]> options['typeMap']]]> @@ -538,7 +539,7 @@ - + is_object($replacementDocument) diff --git a/src/Collection.php b/src/Collection.php index 89fec9504..3d36e95b1 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -19,7 +19,6 @@ use Iterator; use MongoDB\BSON\JavascriptInterface; -use MongoDB\Driver\Cursor; use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Manager; @@ -190,7 +189,7 @@ public function __toString() * @see Aggregate::__construct() for supported options * @param array $pipeline Aggregation pipeline * @param array $options Command options - * @return Cursor + * @return CursorInterface&Iterator * @throws UnexpectedValueException if the command response was malformed * @throws UnsupportedException if options are not supported by the selected server * @throws InvalidArgumentException for parameter/option parsing errors diff --git a/src/Model/ChangeStreamIterator.php b/src/Model/ChangeStreamIterator.php index bf972a163..119ba426c 100644 --- a/src/Model/ChangeStreamIterator.php +++ b/src/Model/ChangeStreamIterator.php @@ -17,9 +17,10 @@ namespace MongoDB\Model; +use Iterator; use IteratorIterator; use MongoDB\BSON\Serializable; -use MongoDB\Driver\Cursor; +use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Monitoring\CommandFailedEvent; use MongoDB\Driver\Monitoring\CommandStartedEvent; use MongoDB\Driver\Monitoring\CommandSubscriber; @@ -47,7 +48,7 @@ * * @internal * @template TValue of array|object - * @template-extends IteratorIterator> + * @template-extends IteratorIterator> */ class ChangeStreamIterator extends IteratorIterator implements CommandSubscriber { @@ -69,9 +70,9 @@ class ChangeStreamIterator extends IteratorIterator implements CommandSubscriber /** * @internal * @param array|object|null $initialResumeToken - * @psalm-param Cursor $cursor + * @psalm-param CursorInterface&Iterator $cursor */ - public function __construct(Cursor $cursor, int $firstBatchSize, $initialResumeToken, ?object $postBatchResumeToken) + public function __construct(CursorInterface $cursor, int $firstBatchSize, $initialResumeToken, ?object $postBatchResumeToken) { if (isset($initialResumeToken) && ! is_document($initialResumeToken)) { throw InvalidArgumentException::expectedDocumentType('$initialResumeToken', $initialResumeToken); @@ -140,10 +141,10 @@ public function current() * but it's very much an invalid use-case. This method can be dropped in 2.0 * once the class is final. */ - final public function getInnerIterator(): Cursor + final public function getInnerIterator(): CursorInterface { $cursor = parent::getInnerIterator(); - assert($cursor instanceof Cursor); + assert($cursor instanceof CursorInterface); return $cursor; } diff --git a/src/Operation/Aggregate.php b/src/Operation/Aggregate.php index 2678b7c73..a63c45ac9 100644 --- a/src/Operation/Aggregate.php +++ b/src/Operation/Aggregate.php @@ -17,8 +17,11 @@ namespace MongoDB\Operation; +use Iterator; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\Command; use MongoDB\Driver\Cursor; +use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\ReadConcern; use MongoDB\Driver\ReadPreference; @@ -28,6 +31,7 @@ use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnexpectedValueException; use MongoDB\Exception\UnsupportedException; +use MongoDB\Model\CodecCursor; use stdClass; use function is_array; @@ -72,6 +76,9 @@ class Aggregate implements Executable, Explainable * circumvent document level validation. This only applies when an $out * or $merge stage is specified. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * collation (document): Collation specification. * * * comment (mixed): BSON value to attach as a comment to this command. @@ -137,6 +144,10 @@ public function __construct(string $databaseName, ?string $collectionName, array throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean'); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (isset($options['collation']) && ! is_document($options['collation'])) { throw InvalidArgumentException::expectedDocumentType('"collation" option', $options['collation']); } @@ -213,7 +224,7 @@ public function __construct(string $databaseName, ?string $collectionName, array * Execute the operation. * * @see Executable::execute() - * @return Cursor + * @return CursorInterface&Iterator * @throws UnexpectedValueException if the command response was malformed * @throws UnsupportedException if read concern or write concern is used and unsupported * @throws DriverRuntimeException for other driver errors (e.g. connection errors) @@ -238,6 +249,10 @@ public function execute(Server $server) $cursor = $this->executeCommand($server, $command); + if (isset($this->options['codec'])) { + return CodecCursor::fromCursor($cursor, $this->options['codec']); + } + if (isset($this->options['typeMap'])) { $cursor->setTypeMap($this->options['typeMap']); } diff --git a/src/Operation/Watch.php b/src/Operation/Watch.php index ee1fa0b2a..3fa6296de 100644 --- a/src/Operation/Watch.php +++ b/src/Operation/Watch.php @@ -17,9 +17,10 @@ namespace MongoDB\Operation; +use Iterator; use MongoDB\BSON\TimestampInterface; use MongoDB\ChangeStream; -use MongoDB\Driver\Cursor; +use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Exception\RuntimeException; use MongoDB\Driver\Manager; use MongoDB\Driver\Monitoring\CommandFailedEvent; @@ -348,8 +349,10 @@ private function createChangeStreamIterator(Server $server): ChangeStreamIterato * * The command will be executed using APM so that we can capture data from * its response (e.g. firstBatch size, postBatchResumeToken). + * + * @return CursorInterface&Iterator */ - private function executeAggregate(Server $server): Cursor + private function executeAggregate(Server $server) { addSubscriber($this); diff --git a/tests/Operation/AggregateFunctionalTest.php b/tests/Operation/AggregateFunctionalTest.php index 4aad0a667..115404b29 100644 --- a/tests/Operation/AggregateFunctionalTest.php +++ b/tests/Operation/AggregateFunctionalTest.php @@ -9,6 +9,8 @@ use MongoDB\Driver\WriteConcern; use MongoDB\Operation\Aggregate; use MongoDB\Tests\CommandObserver; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; +use MongoDB\Tests\Fixtures\Document\TestObject; use stdClass; use function current; @@ -335,6 +337,30 @@ public function testReadPreferenceWithinTransaction(): void } } + public function testCodecOption(): void + { + $this->createFixtures(3); + + $codec = new TestDocumentCodec(); + + $operation = new Aggregate( + $this->getDatabaseName(), + $this->getCollectionName(), + [['$match' => ['_id' => ['$gt' => 1]]]], + ['codec' => $codec], + ); + + $cursor = $operation->execute($this->getPrimaryServer()); + + $this->assertEquals( + [ + TestObject::createForFixture(2, true), + TestObject::createForFixture(3, true), + ], + $cursor->toArray(), + ); + } + /** * Create data fixtures. */ diff --git a/tests/Operation/AggregateTest.php b/tests/Operation/AggregateTest.php index 433278ebb..81128dca4 100644 --- a/tests/Operation/AggregateTest.php +++ b/tests/Operation/AggregateTest.php @@ -30,6 +30,7 @@ public function provideInvalidConstructorOptions() 'allowDiskUse' => $this->getInvalidBooleanValues(), 'batchSize' => $this->getInvalidIntegerValues(), 'bypassDocumentValidation' => $this->getInvalidBooleanValues(), + 'codec' => $this->getInvalidDocumentCodecValues(), 'collation' => $this->getInvalidDocumentValues(), 'hint' => $this->getInvalidHintValues(), 'let' => $this->getInvalidDocumentValues(), From 53202173865cebb661ceea74b0c8702dda35f003 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 17 Jul 2023 10:17:29 +0200 Subject: [PATCH 05/37] Add codec support to BulkWrite operation --- psalm-baseline.xml | 8 ++++ src/Operation/BulkWrite.php | 31 ++++++++++++++- tests/Operation/BulkWriteFunctionalTest.php | 43 +++++++++++++++++++++ tests/Operation/BulkWriteTest.php | 1 + 4 files changed, 82 insertions(+), 1 deletion(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index ef2ad0902..c10dccc9d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -199,6 +199,7 @@ $args[1] $args[1] $args[2] + $args[2] $args[0] @@ -206,6 +207,9 @@ $args[0] $args[0] $args[0] + $args[0] + $args[0] + $args[1] $args[1] $args[1] $args[1] @@ -220,6 +224,8 @@ $args[1] $args[1] $args[1] + $args[1] + $args[2] $args[2] $args[2] $args[2] @@ -263,6 +269,8 @@ + encodeIfSupported + encodeIfSupported isInTransaction diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index 495bdb819..c4239b2d0 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -18,6 +18,7 @@ namespace MongoDB\Operation; use MongoDB\BulkWriteResult; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\BulkWrite as Bulk; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Server; @@ -28,10 +29,12 @@ use function array_is_list; use function array_key_exists; +use function assert; use function count; 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; @@ -100,6 +103,10 @@ class BulkWrite implements Executable * * bypassDocumentValidation (boolean): If true, allows the write to * circumvent document level validation. The default is false. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. This option is also used to encode PHP + * objects into BSON for insertOne and replaceOne operations. + * * * comment (mixed): BSON value to attach as a comment to this command(s) * associated with this bulk write. * @@ -271,6 +278,10 @@ public function __construct(string $databaseName, string $collectionName, array throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean'); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (! is_bool($options['ordered'])) { throw InvalidArgumentException::invalidType('"ordered" option', $options['ordered'], 'boolean'); } @@ -330,13 +341,31 @@ public function execute(Server $server) break; case self::INSERT_ONE: - $insertedIds[$i] = $bulk->insert($args[0]); + $insertedDocument = isset($this->options['codec']) + ? $this->options['codec']->encodeIfSupported($args[0]) + : $args[0]; + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + assert(is_array($insertedDocument) || is_object($insertedDocument)); + + $insertedIds[$i] = $bulk->insert($insertedDocument); break; case self::REPLACE_ONE: + $replacementDocument = isset($this->options['codec']) + ? $this->options['codec']->encodeIfSupported($args[1]) + : $args[1]; + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + assert(is_array($replacementDocument) || is_object($replacementDocument)); + + $bulk->update($args[0], $replacementDocument, $args[2]); + break; + case self::UPDATE_MANY: case self::UPDATE_ONE: $bulk->update($args[0], $args[1], $args[2]); + break; } } diff --git a/tests/Operation/BulkWriteFunctionalTest.php b/tests/Operation/BulkWriteFunctionalTest.php index c1089150d..e5c3f8c74 100644 --- a/tests/Operation/BulkWriteFunctionalTest.php +++ b/tests/Operation/BulkWriteFunctionalTest.php @@ -12,6 +12,8 @@ use MongoDB\Model\BSONDocument; use MongoDB\Operation\BulkWrite; use MongoDB\Tests\CommandObserver; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; +use MongoDB\Tests\Fixtures\Document\TestObject; use stdClass; use function is_array; @@ -449,6 +451,47 @@ public function testBulkWriteWithPipelineUpdates(): void $this->assertSameDocuments($expected, $this->collection->find()); } + public function testCodecOption(): void + { + $this->createFixtures(3); + + $codec = new TestDocumentCodec(); + + $replaceObject = TestObject::createForFixture(3); + $replaceObject->x->foo = 'baz'; + + $operations = [ + ['insertOne' => [TestObject::createForFixture(4)]], + ['replaceOne' => [['_id' => 3], $replaceObject]], + ]; + + $operation = new BulkWrite( + $this->getDatabaseName(), + $this->getCollectionName(), + $operations, + ['codec' => $codec], + ); + + $result = $operation->execute($this->getPrimaryServer()); + + $this->assertInstanceOf(BulkWriteResult::class, $result); + $this->assertSame(1, $result->getInsertedCount()); + $this->assertSame(1, $result->getMatchedCount()); + $this->assertSame(1, $result->getModifiedCount()); + + $replacedObject = TestObject::createForFixture(3, true); + $replacedObject->x->foo = 'baz'; + + // Only read the last two documents as the other two don't fit our codec + $this->assertEquals( + [ + $replacedObject, + TestObject::createForFixture(4, true), + ], + $this->collection->find(['_id' => ['$gte' => 3]], ['codec' => $codec])->toArray(), + ); + } + /** * Create data fixtures. */ diff --git a/tests/Operation/BulkWriteTest.php b/tests/Operation/BulkWriteTest.php index 5da144343..8d35d2d74 100644 --- a/tests/Operation/BulkWriteTest.php +++ b/tests/Operation/BulkWriteTest.php @@ -425,6 +425,7 @@ public function provideInvalidConstructorOptions() { return $this->createOptionDataProvider([ 'bypassDocumentValidation' => $this->getInvalidBooleanValues(), + 'codec' => $this->getInvalidDocumentCodecValues(), 'ordered' => $this->getInvalidBooleanValues(true), 'session' => $this->getInvalidSessionValues(), 'writeConcern' => $this->getInvalidWriteConcernValues(), From dead64ba17a11d712d3795e8713322fba57c367b Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 17 Jul 2023 10:52:28 +0200 Subject: [PATCH 06/37] Add codec support to findAndModify operations --- psalm-baseline.xml | 4 ++ src/Operation/FindAndModify.php | 21 ++++++ src/Operation/FindOneAndDelete.php | 3 + src/Operation/FindOneAndReplace.php | 20 +++++- src/Operation/FindOneAndUpdate.php | 3 + .../Operation/FindAndModifyFunctionalTest.php | 47 +++++++++++++ tests/Operation/FindAndModifyTest.php | 1 + .../FindOneAndReplaceFunctionalTest.php | 66 +++++++++++++++++++ tests/Operation/FindOneAndReplaceTest.php | 1 + 9 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 tests/Operation/FindOneAndReplaceFunctionalTest.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index c10dccc9d..0d0877c7c 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -442,6 +442,7 @@ array|object|null + decodeIfSupported isInTransaction @@ -470,6 +471,9 @@ + + is_object($replacementDocument) + diff --git a/src/Operation/FindAndModify.php b/src/Operation/FindAndModify.php index 8f23d1237..c3c679ead 100644 --- a/src/Operation/FindAndModify.php +++ b/src/Operation/FindAndModify.php @@ -17,6 +17,8 @@ namespace MongoDB\Operation; +use MongoDB\BSON\Document; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\Command; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Server; @@ -27,6 +29,7 @@ use MongoDB\Exception\UnsupportedException; use function array_key_exists; +use function assert; use function current; use function is_array; use function is_bool; @@ -68,6 +71,9 @@ class FindAndModify implements Executable, Explainable * * arrayFilters (document array): A set of filters specifying to which * array elements an update should apply. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * collation (document): Collation specification. * * * comment (mixed): BSON value to attach as a comment to this command. @@ -137,6 +143,10 @@ public function __construct(string $databaseName, string $collectionName, array throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean'); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (isset($options['collation']) && ! is_document($options['collation'])) { throw InvalidArgumentException::expectedDocumentType('"collation" option', $options['collation']); } @@ -243,6 +253,17 @@ public function execute(Server $server) $cursor = $server->executeWriteCommand($this->databaseName, new Command($this->createCommandDocument()), $this->createOptions()); + if (isset($this->options['codec'])) { + $cursor->setTypeMap(['root' => 'bson']); + $result = current($cursor->toArray()); + assert($result instanceof Document); + + $decoded = $this->options['codec']->decodeIfSupported($result->get('value') ?? null); + assert($decoded === null || is_object($decoded)); + + return $decoded; + } + if (isset($this->options['typeMap'])) { $cursor->setTypeMap(create_field_path_type_map($this->options['typeMap'], 'value')); } diff --git a/src/Operation/FindOneAndDelete.php b/src/Operation/FindOneAndDelete.php index 39381b1fb..cae2a1788 100644 --- a/src/Operation/FindOneAndDelete.php +++ b/src/Operation/FindOneAndDelete.php @@ -39,6 +39,9 @@ class FindOneAndDelete implements Executable, Explainable * * Supported options: * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * collation (document): Collation specification. * * * comment (mixed): BSON value to attach as a comment to this command. diff --git a/src/Operation/FindOneAndReplace.php b/src/Operation/FindOneAndReplace.php index 34fe6a2d8..5c7a1852f 100644 --- a/src/Operation/FindOneAndReplace.php +++ b/src/Operation/FindOneAndReplace.php @@ -17,13 +17,17 @@ namespace MongoDB\Operation; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Server; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Exception\UnsupportedException; use function array_key_exists; +use function assert; +use function is_array; 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; @@ -49,6 +53,9 @@ class FindOneAndReplace implements Executable, Explainable * * bypassDocumentValidation (boolean): If true, allows the write to * circumvent document level validation. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * collation (document): Collation specification. * * * comment (mixed): BSON value to attach as a comment to this command. @@ -121,6 +128,10 @@ public function __construct(string $databaseName, string $collectionName, $filte throw new InvalidArgumentException('$replacement is an update pipeline'); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (isset($options['projection']) && ! is_document($options['projection'])) { throw InvalidArgumentException::expectedDocumentType('"projection" option', $options['projection']); } @@ -147,10 +158,17 @@ public function __construct(string $databaseName, string $collectionName, $filte unset($options['projection'], $options['returnDocument']); + $replacementDocument = isset($options['codec']) + ? $options['codec']->encodeIfSupported($replacement) + : $replacement; + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + assert(is_array($replacementDocument) || is_object($replacementDocument)); + $this->findAndModify = new FindAndModify( $databaseName, $collectionName, - ['query' => $filter, 'update' => $replacement] + $options, + ['query' => $filter, 'update' => $replacementDocument] + $options, ); } diff --git a/src/Operation/FindOneAndUpdate.php b/src/Operation/FindOneAndUpdate.php index ae8374ba4..783888b1f 100644 --- a/src/Operation/FindOneAndUpdate.php +++ b/src/Operation/FindOneAndUpdate.php @@ -54,6 +54,9 @@ class FindOneAndUpdate implements Executable, Explainable * * bypassDocumentValidation (boolean): If true, allows the write to * circumvent document level validation. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * collation (document): Collation specification. * * * comment (mixed): BSON value to attach as a comment to this command. diff --git a/tests/Operation/FindAndModifyFunctionalTest.php b/tests/Operation/FindAndModifyFunctionalTest.php index 51723a93c..5c8e30474 100644 --- a/tests/Operation/FindAndModifyFunctionalTest.php +++ b/tests/Operation/FindAndModifyFunctionalTest.php @@ -10,6 +10,8 @@ use MongoDB\Model\BSONDocument; use MongoDB\Operation\FindAndModify; use MongoDB\Tests\CommandObserver; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; +use MongoDB\Tests\Fixtures\Document\TestObject; use stdClass; class FindAndModifyFunctionalTest extends FunctionalTestCase @@ -279,6 +281,51 @@ public function provideTypeMapOptionsAndExpectedDocument() ]; } + public function testFindOneAndDeleteWithCodec(): void + { + $this->createFixtures(1); + + $operation = new FindAndModify( + $this->getDatabaseName(), + $this->getCollectionName(), + ['remove' => true, 'codec' => new TestDocumentCodec()], + ); + + $result = $operation->execute($this->getPrimaryServer()); + + self::assertEquals(TestObject::createForFixture(1, true), $result); + } + + public function testFindOneAndUpdateWithCodec(): void + { + $this->createFixtures(1); + + $operation = new FindAndModify( + $this->getDatabaseName(), + $this->getCollectionName(), + ['update' => ['$set' => ['x.foo' => 'baz']], 'codec' => new TestDocumentCodec()], + ); + + $result = $operation->execute($this->getPrimaryServer()); + + self::assertEquals(TestObject::createForFixture(1, true), $result); + } + + public function testFindOneAndReplaceWithCodec(): void + { + $this->createFixtures(1); + + $operation = new FindAndModify( + $this->getDatabaseName(), + $this->getCollectionName(), + ['update' => ['_id' => 1], 'codec' => new TestDocumentCodec()], + ); + + $result = $operation->execute($this->getPrimaryServer()); + + self::assertEquals(TestObject::createForFixture(1, true), $result); + } + /** * Create data fixtures. */ diff --git a/tests/Operation/FindAndModifyTest.php b/tests/Operation/FindAndModifyTest.php index f2de657c4..65676d289 100644 --- a/tests/Operation/FindAndModifyTest.php +++ b/tests/Operation/FindAndModifyTest.php @@ -20,6 +20,7 @@ public function provideInvalidConstructorOptions() return $this->createOptionDataProvider([ 'arrayFilters' => $this->getInvalidArrayValues(), 'bypassDocumentValidation' => $this->getInvalidBooleanValues(), + 'codec' => $this->getInvalidDocumentCodecValues(), 'collation' => $this->getInvalidDocumentValues(), 'fields' => $this->getInvalidDocumentValues(), 'maxTimeMS' => $this->getInvalidIntegerValues(), diff --git a/tests/Operation/FindOneAndReplaceFunctionalTest.php b/tests/Operation/FindOneAndReplaceFunctionalTest.php new file mode 100644 index 000000000..782d46c0a --- /dev/null +++ b/tests/Operation/FindOneAndReplaceFunctionalTest.php @@ -0,0 +1,66 @@ +createFixtures(1); + + (new CommandObserver())->observe( + function (): void { + $replaceObject = TestObject::createForFixture(1); + $replaceObject->x->foo = 'baz'; + + $operation = new FindOneAndReplace( + $this->getDatabaseName(), + $this->getCollectionName(), + ['_id' => 1], + $replaceObject, + ['codec' => new TestDocumentCodec()], + ); + + $this->assertEquals( + TestObject::createForFixture(1, true), + $operation->execute($this->getPrimaryServer()), + ); + }, + function (array $event): void { + $this->assertEquals( + (object) [ + '_id' => 1, + 'x' => (object) ['foo' => 'baz'], + 'encoded' => true, + ], + $event['started']->getCommand()->update ?? null, + ); + }, + ); + } + + /** + * Create data fixtures. + */ + private function createFixtures(int $n): void + { + $bulkWrite = new BulkWrite(['ordered' => true]); + + for ($i = 1; $i <= $n; $i++) { + $bulkWrite->insert([ + '_id' => $i, + 'x' => (object) ['foo' => 'bar'], + ]); + } + + $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite); + + $this->assertEquals($n, $result->getInsertedCount()); + } +} diff --git a/tests/Operation/FindOneAndReplaceTest.php b/tests/Operation/FindOneAndReplaceTest.php index 73951596d..94075b89d 100644 --- a/tests/Operation/FindOneAndReplaceTest.php +++ b/tests/Operation/FindOneAndReplaceTest.php @@ -60,6 +60,7 @@ public function testConstructorOptionTypeChecks(array $options): void public function provideInvalidConstructorOptions() { return $this->createOptionDataProvider([ + 'codec' => $this->getInvalidDocumentCodecValues(), 'projection' => $this->getInvalidDocumentValues(), 'returnDocument' => $this->getInvalidIntegerValues(true), ]); From 274d50a728e6ddd3f90fe25b0284673c0672c6e9 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 17 Jul 2023 13:20:34 +0200 Subject: [PATCH 07/37] Add codec support to Watch operation --- psalm-baseline.xml | 1 + src/ChangeStream.php | 26 ++- src/Model/ChangeStreamIterator.php | 17 +- src/Operation/Watch.php | 12 ++ tests/Operation/WatchFunctionalTest.php | 261 ++++++++++++++++-------- tests/Operation/WatchTest.php | 1 + 6 files changed, 228 insertions(+), 90 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 0d0877c7c..cfd570908 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -101,6 +101,7 @@ cursor->nextBatch]]> + $resumeToken postBatchResumeToken]]> diff --git a/src/ChangeStream.php b/src/ChangeStream.php index 6fc3bbaa1..11ec3528e 100644 --- a/src/ChangeStream.php +++ b/src/ChangeStream.php @@ -18,6 +18,8 @@ namespace MongoDB; use Iterator; +use MongoDB\BSON\Document; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\CursorId; use MongoDB\Driver\Exception\ConnectionException; use MongoDB\Driver\Exception\RuntimeException; @@ -27,6 +29,7 @@ use MongoDB\Model\ChangeStreamIterator; use ReturnTypeWillChange; +use function assert; use function call_user_func; use function in_array; @@ -82,15 +85,22 @@ class ChangeStream implements Iterator */ private bool $hasAdvanced = false; + private ?DocumentCodec $codec; + /** * @internal * * @param ResumeCallable $resumeCallable */ - public function __construct(ChangeStreamIterator $iterator, callable $resumeCallable) + public function __construct(ChangeStreamIterator $iterator, callable $resumeCallable, ?DocumentCodec $codec = null) { $this->iterator = $iterator; $this->resumeCallable = $resumeCallable; + $this->codec = $codec; + + if ($codec) { + $this->iterator->getInnerIterator()->setTypeMap(['root' => 'bson']); + } } /** @@ -100,7 +110,15 @@ public function __construct(ChangeStreamIterator $iterator, callable $resumeCall #[ReturnTypeWillChange] public function current() { - return $this->iterator->current(); + $value = $this->iterator->current(); + + if (! $this->codec) { + return $value; + } + + assert($value === null || $value instanceof Document); + + return $this->codec->decodeIfSupported($value); } /** @return CursorId */ @@ -252,6 +270,10 @@ private function resume(): void $this->iterator->rewind(); + if ($this->codec) { + $this->iterator->getInnerIterator()->setTypeMap(['root' => 'bson']); + } + $this->onIteration($this->hasAdvanced); } diff --git a/src/Model/ChangeStreamIterator.php b/src/Model/ChangeStreamIterator.php index 119ba426c..6a21609a1 100644 --- a/src/Model/ChangeStreamIterator.php +++ b/src/Model/ChangeStreamIterator.php @@ -19,6 +19,7 @@ use Iterator; use IteratorIterator; +use MongoDB\BSON\Document; use MongoDB\BSON\Serializable; use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Monitoring\CommandFailedEvent; @@ -245,9 +246,19 @@ private function extractResumeToken($document) return $this->extractResumeToken($document->bsonSerialize()); } - $resumeToken = is_array($document) - ? ($document['_id'] ?? null) - : ($document->_id ?? null); + if ($document instanceof Document) { + $resumeToken = $document->has('_id') + ? $document->get('_id') + : null; + + if ($resumeToken instanceof Document) { + $resumeToken = $resumeToken->toPHP(); + } + } else { + $resumeToken = is_array($document) + ? ($document['_id'] ?? null) + : ($document->_id ?? null); + } if (! isset($resumeToken)) { $this->isValid = false; diff --git a/src/Operation/Watch.php b/src/Operation/Watch.php index 3fa6296de..2caab579c 100644 --- a/src/Operation/Watch.php +++ b/src/Operation/Watch.php @@ -20,6 +20,7 @@ use Iterator; use MongoDB\BSON\TimestampInterface; use MongoDB\ChangeStream; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Exception\RuntimeException; use MongoDB\Driver\Manager; @@ -92,6 +93,8 @@ class Watch implements Executable, /* @internal */ CommandSubscriber private ?object $postBatchResumeToken = null; + private ?DocumentCodec $codec; + /** * Constructs an aggregate command for creating a change stream. * @@ -99,6 +102,9 @@ class Watch implements Executable, /* @internal */ CommandSubscriber * * * batchSize (integer): The number of documents to return per batch. * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * collation (document): Specifies a collation. * * * comment (mixed): BSON value to attach as a comment to this command. @@ -200,6 +206,10 @@ public function __construct(Manager $manager, ?string $databaseName, ?string $co 'readPreference' => new ReadPreference(ReadPreference::PRIMARY), ]; + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (array_key_exists('fullDocument', $options) && ! is_string($options['fullDocument'])) { throw InvalidArgumentException::invalidType('"fullDocument" option', $options['fullDocument'], 'string'); } @@ -255,6 +265,7 @@ public function __construct(Manager $manager, ?string $databaseName, ?string $co $this->databaseName = $databaseName; $this->collectionName = $collectionName; $this->pipeline = $pipeline; + $this->codec = $options['codec'] ?? null; $this->aggregate = $this->createAggregate(); } @@ -315,6 +326,7 @@ public function execute(Server $server) return new ChangeStream( $this->createChangeStreamIterator($server), fn ($resumeToken, $hasAdvanced): ChangeStreamIterator => $this->resume($resumeToken, $hasAdvanced), + $this->codec, ); } diff --git a/tests/Operation/WatchFunctionalTest.php b/tests/Operation/WatchFunctionalTest.php index 447d8cc4a..fb38435cd 100644 --- a/tests/Operation/WatchFunctionalTest.php +++ b/tests/Operation/WatchFunctionalTest.php @@ -3,9 +3,14 @@ namespace MongoDB\Tests\Operation; use Closure; +use Generator; use Iterator; +use MongoDB\BSON\Document; use MongoDB\BSON\TimestampInterface; use MongoDB\ChangeStream; +use MongoDB\Codec\DecodeIfSupported; +use MongoDB\Codec\DocumentCodec; +use MongoDB\Codec\EncodeIfSupported; use MongoDB\Driver\Cursor; use MongoDB\Driver\Exception\CommandException; use MongoDB\Driver\Exception\ConnectionTimeoutException; @@ -15,6 +20,7 @@ use MongoDB\Driver\ReadPreference; use MongoDB\Driver\WriteConcern; use MongoDB\Exception\ResumeTokenException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\Operation\InsertOne; use MongoDB\Operation\Watch; use MongoDB\Tests\CommandObserver; @@ -52,15 +58,72 @@ public function setUp(): void $this->createCollection($this->getDatabaseName(), $this->getCollectionName()); } + public static function provideCodecOptions(): Generator + { + yield 'No codec' => [ + 'options' => [], + 'getIdentifier' => static fn (object $document): object => $document->_id, + ]; + + $codec = new class implements DocumentCodec { + use DecodeIfSupported; + use EncodeIfSupported; + + public function canDecode($value): bool + { + return $value instanceof Document; + } + + public function canEncode($value): bool + { + return false; + } + + public function decode($value): stdClass + { + if (! $value instanceof Document) { + throw UnsupportedValueException::invalidDecodableValue($value); + } + + return (object) [ + 'id' => $value->get('_id')->toPHP(), + 'fullDocument' => $value->get('fullDocument')->toPHP(), + ]; + } + + public function encode($value): Document + { + return Document::fromPHP([]); + } + }; + + yield 'Codec' => [ + 'options' => ['codec' => $codec], + 'getIdentifier' => static function (object $document): object { + self::assertObjectHasAttribute('id', $document); + + return $document->id; + }, + ]; + } + /** * Prose test 1: "ChangeStream must continuously track the last seen * resumeToken" + * + * @dataProvider provideCodecOptions */ - public function testGetResumeToken(): void + public function testGetResumeToken(array $options, Closure $getIdentifier): void { $this->skipIfServerVersion('>=', '4.0.7', 'postBatchResumeToken is supported'); - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $this->defaultOptions); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $changeStream->rewind(); @@ -71,16 +134,16 @@ public function testGetResumeToken(): void $this->insertDocument(['x' => 2]); $this->advanceCursorUntilValid($changeStream); - $this->assertSameDocument($changeStream->current()->_id, $changeStream->getResumeToken()); + $this->assertSameDocument($getIdentifier($changeStream->current()), $changeStream->getResumeToken()); $changeStream->next(); $this->assertTrue($changeStream->valid()); - $this->assertSameDocument($changeStream->current()->_id, $changeStream->getResumeToken()); + $this->assertSameDocument($getIdentifier($changeStream->current()), $changeStream->getResumeToken()); $this->insertDocument(['x' => 3]); $this->advanceCursorUntilValid($changeStream); - $this->assertSameDocument($changeStream->current()->_id, $changeStream->getResumeToken()); + $this->assertSameDocument($getIdentifier($changeStream->current()), $changeStream->getResumeToken()); } /** @@ -100,12 +163,20 @@ public function testGetResumeToken(): void * - The batch has been iterated up to but not including the last element. * Expected result: getResumeToken must return the _id of the previous * document returned. + * + * @dataProvider provideCodecOptions */ - public function testGetResumeTokenWithPostBatchResumeToken(): void + public function testGetResumeTokenWithPostBatchResumeToken(array $options, Closure $getIdentifier): void { $this->skipIfServerVersion('<', '4.0.7', 'postBatchResumeToken is not supported'); - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $this->defaultOptions); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + $options + $this->defaultOptions, + ); $events = []; @@ -144,7 +215,7 @@ function (array $event) use (&$lastEvent): void { $this->assertSame('getMore', $lastEvent['started']->getCommandName()); $postBatchResumeToken = $this->getPostBatchResumeTokenFromReply($lastEvent['succeeded']->getReply()); - $this->assertSameDocument($changeStream->current()->_id, $changeStream->getResumeToken()); + $this->assertSameDocument($getIdentifier($changeStream->current()), $changeStream->getResumeToken()); $changeStream->next(); $this->assertSameDocument($postBatchResumeToken, $changeStream->getResumeToken()); @@ -423,9 +494,16 @@ public function testRewindMultipleTimesWithNoResults(): void $this->assertNull($changeStream->current()); } - public function testNoChangeAfterResumeBeforeInsert(): void + /** @dataProvider provideCodecOptions */ + public function testNoChangeAfterResumeBeforeInsert(array $options): void { - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $this->defaultOptions); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $this->assertNoCommandExecuted(function () use ($changeStream): void { @@ -437,15 +515,7 @@ public function testNoChangeAfterResumeBeforeInsert(): void $this->advanceCursorUntilValid($changeStream); - $expectedResult = [ - '_id' => $changeStream->current()->_id, - 'operationType' => 'insert', - 'fullDocument' => ['_id' => 1, 'x' => 'foo'], - 'ns' => ['db' => $this->getDatabaseName(), 'coll' => $this->getCollectionName()], - 'documentKey' => ['_id' => 1], - ]; - - $this->assertMatchesDocument($expectedResult, $changeStream->current()); + $this->assertMatchesDocument(['_id' => 1, 'x' => 'foo'], $changeStream->current()->fullDocument); $this->forceChangeStreamResume(); @@ -456,15 +526,7 @@ public function testNoChangeAfterResumeBeforeInsert(): void $this->advanceCursorUntilValid($changeStream); - $expectedResult = [ - '_id' => $changeStream->current()->_id, - 'operationType' => 'insert', - 'fullDocument' => ['_id' => 2, 'x' => 'bar'], - 'ns' => ['db' => $this->getDatabaseName(), 'coll' => $this->getCollectionName()], - 'documentKey' => ['_id' => 2], - ]; - - $this->assertMatchesDocument($expectedResult, $changeStream->current()); + $this->assertMatchesDocument(['_id' => 2, 'x' => 'bar'], $changeStream->current()->fullDocument); } public function testResumeMultipleTimesInSuccession(): void @@ -841,9 +903,16 @@ public function testMaxAwaitTimeMS(): void } } - public function testRewindExtractsResumeTokenAndNextResumes(): void + /** @dataProvider provideCodecOptions */ + public function testRewindExtractsResumeTokenAndNextResumes(array $options, Closure $getIdentifier): void { - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $this->defaultOptions); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $this->insertDocument(['_id' => 1, 'x' => 'foo']); @@ -859,9 +928,14 @@ public function testRewindExtractsResumeTokenAndNextResumes(): void $this->advanceCursorUntilValid($changeStream); - $resumeToken = $changeStream->current()->_id; - $options = ['resumeAfter' => $resumeToken] + $this->defaultOptions; - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $options); + $resumeToken = $getIdentifier($changeStream->current()); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + ['resumeAfter' => $resumeToken] + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $this->assertSameDocument($resumeToken, $changeStream->getResumeToken()); @@ -877,33 +951,26 @@ public function testRewindExtractsResumeTokenAndNextResumes(): void } $this->assertSame(0, $changeStream->key()); - $expectedResult = [ - '_id' => $changeStream->current()->_id, - 'operationType' => 'insert', - 'fullDocument' => ['_id' => 2, 'x' => 'bar'], - 'ns' => ['db' => $this->getDatabaseName(), 'coll' => $this->getCollectionName()], - 'documentKey' => ['_id' => 2], - ]; - $this->assertMatchesDocument($expectedResult, $changeStream->current()); + $this->assertMatchesDocument(['_id' => 2, 'x' => 'bar'], $changeStream->current()->fullDocument); $this->forceChangeStreamResume(); $this->advanceCursorUntilValid($changeStream); $this->assertSame(1, $changeStream->key()); - $expectedResult = [ - '_id' => $changeStream->current()->_id, - 'operationType' => 'insert', - 'fullDocument' => ['_id' => 3, 'x' => 'baz'], - 'ns' => ['db' => $this->getDatabaseName(), 'coll' => $this->getCollectionName()], - 'documentKey' => ['_id' => 3], - ]; - $this->assertMatchesDocument($expectedResult, $changeStream->current()); + $this->assertMatchesDocument(['_id' => 3, 'x' => 'baz'], $changeStream->current()->fullDocument); } - public function testResumeAfterOption(): void + /** @dataProvider provideCodecOptions */ + public function testResumeAfterOption(array $options, Closure $getIdentifier): void { - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $this->defaultOptions); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $changeStream->rewind(); @@ -914,10 +981,15 @@ public function testResumeAfterOption(): void $this->advanceCursorUntilValid($changeStream); - $resumeToken = $changeStream->current()->_id; + $resumeToken = $getIdentifier($changeStream->current()); - $options = $this->defaultOptions + ['resumeAfter' => $resumeToken]; - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $options); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + ['resumeAfter' => $resumeToken] + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $this->assertSameDocument($resumeToken, $changeStream->getResumeToken()); @@ -932,22 +1004,21 @@ public function testResumeAfterOption(): void $this->assertTrue($changeStream->valid()); } - $expectedResult = [ - '_id' => $changeStream->current()->_id, - 'operationType' => 'insert', - 'fullDocument' => ['_id' => 2, 'x' => 'bar'], - 'ns' => ['db' => $this->getDatabaseName(), 'coll' => $this->getCollectionName()], - 'documentKey' => ['_id' => 2], - ]; - - $this->assertMatchesDocument($expectedResult, $changeStream->current()); + $this->assertMatchesDocument(['_id' => 2, 'x' => 'bar'], $changeStream->current()->fullDocument); } - public function testStartAfterOption(): void + /** @dataProvider provideCodecOptions */ + public function testStartAfterOption(array $options, Closure $getIdentifier): void { $this->skipIfServerVersion('<', '4.1.1', 'startAfter is not supported'); - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $this->defaultOptions); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $changeStream->rewind(); @@ -958,10 +1029,15 @@ public function testStartAfterOption(): void $this->advanceCursorUntilValid($changeStream); - $resumeToken = $changeStream->current()->_id; + $resumeToken = $getIdentifier($changeStream->current()); - $options = $this->defaultOptions + ['startAfter' => $resumeToken]; - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $options); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + ['startAfter' => $resumeToken] + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $this->assertSameDocument($resumeToken, $changeStream->getResumeToken()); @@ -976,15 +1052,7 @@ public function testStartAfterOption(): void $this->assertTrue($changeStream->valid()); } - $expectedResult = [ - '_id' => $changeStream->current()->_id, - 'operationType' => 'insert', - 'fullDocument' => ['_id' => 2, 'x' => 'bar'], - 'ns' => ['db' => $this->getDatabaseName(), 'coll' => $this->getCollectionName()], - 'documentKey' => ['_id' => 2], - ]; - - $this->assertMatchesDocument($expectedResult, $changeStream->current()); + $this->assertMatchesDocument(['_id' => 2, 'x' => 'bar'], $changeStream->current()->fullDocument); } /** @dataProvider provideTypeMapOptionsAndExpectedChangeDocument */ @@ -1368,13 +1436,21 @@ public function testGetResumeTokenReturnsOriginalResumeTokenOnEmptyBatch(): void * - getResumeToken must return startAfter from the initial aggregate if the option was specified. * - getResumeToken must return resumeAfter from the initial aggregate if the option was specified. * - If neither the startAfter nor resumeAfter options were specified, the getResumeToken result must be empty. + * + * @dataProvider provideCodecOptions */ - public function testResumeTokenBehaviour(): void + public function testResumeTokenBehaviour(array $options): void { $this->skipIfServerVersion('<', '4.1.1', 'Testing resumeAfter and startAfter can only be tested on servers >= 4.1.1'); $this->skipIfIsShardedCluster('Resume token behaviour can\'t be reliably tested on sharded clusters.'); - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $this->defaultOptions); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + $options + $this->defaultOptions, + ); $lastOpTime = null; @@ -1398,22 +1474,37 @@ public function testResumeTokenBehaviour(): void $this->insertDocument(['x' => 2]); // Test startAfter option - $options = ['startAfter' => $resumeToken] + $this->defaultOptions; - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $options); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + ['startAfter' => $resumeToken] + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $this->assertEquals($resumeToken, $changeStream->getResumeToken()); // Test resumeAfter option - $options = ['resumeAfter' => $resumeToken] + $this->defaultOptions; - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $options); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + ['resumeAfter' => $resumeToken] + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $this->assertEquals($resumeToken, $changeStream->getResumeToken()); // Test without option - $options = ['startAtOperationTime' => $lastOpTime] + $this->defaultOptions; - $operation = new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $options); + $operation = new Watch( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + [], + ['startAtOperationTime' => $lastOpTime] + $options + $this->defaultOptions, + ); $changeStream = $operation->execute($this->getPrimaryServer()); $this->assertNull($changeStream->getResumeToken()); diff --git a/tests/Operation/WatchTest.php b/tests/Operation/WatchTest.php index 1a9e5fa0e..8b4f3e0d6 100644 --- a/tests/Operation/WatchTest.php +++ b/tests/Operation/WatchTest.php @@ -42,6 +42,7 @@ public function provideInvalidConstructorOptions() { return $this->createOptionDataProvider([ 'batchSize' => $this->getInvalidIntegerValues(), + 'codec' => $this->getInvalidDocumentCodecValues(), 'collation' => $this->getInvalidDocumentValues(), 'fullDocument' => $this->getInvalidStringValues(true), 'fullDocumentBeforeChange' => $this->getInvalidStringValues(), From c67ff641abedcdcba791596cc221a6f42191fa04 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 21 Jul 2023 10:19:19 +0200 Subject: [PATCH 08/37] Support passing default codec to collection class --- src/Collection.php | 59 +++ .../CodecCollectionFunctionalTest.php | 370 ++++++++++++++++++ tests/Collection/CollectionFunctionalTest.php | 1 + 3 files changed, 430 insertions(+) create mode 100644 tests/Collection/CodecCollectionFunctionalTest.php diff --git a/src/Collection.php b/src/Collection.php index 3d36e95b1..237d73e14 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -19,6 +19,7 @@ use Iterator; use MongoDB\BSON\JavascriptInterface; +use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\CursorInterface; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Manager; @@ -63,6 +64,7 @@ use function array_diff_key; use function array_intersect_key; +use function array_key_exists; use function current; use function is_array; use function strlen; @@ -77,6 +79,8 @@ class Collection private const WIRE_VERSION_FOR_READ_CONCERN_WITH_WRITE_STAGE = 8; + private ?DocumentCodec $codec = null; + private string $collectionName; private string $databaseName; @@ -99,6 +103,9 @@ class Collection * * Supported options: * + * * codec (MongoDB\Codec\DocumentCodec): Codec used to decode documents + * from BSON to PHP objects. + * * * readConcern (MongoDB\Driver\ReadConcern): The default read concern to * use for collection operations. Defaults to the Manager's read concern. * @@ -128,6 +135,10 @@ public function __construct(Manager $manager, string $databaseName, string $coll throw new InvalidArgumentException('$collectionName is invalid: ' . $collectionName); } + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + if (isset($options['readConcern']) && ! $options['readConcern'] instanceof ReadConcern) { throw InvalidArgumentException::invalidType('"readConcern" option', $options['readConcern'], ReadConcern::class); } @@ -147,6 +158,8 @@ public function __construct(Manager $manager, string $databaseName, string $coll $this->manager = $manager; $this->databaseName = $databaseName; $this->collectionName = $collectionName; + + $this->codec = $options['codec'] ?? null; $this->readConcern = $options['readConcern'] ?? $this->manager->getReadConcern(); $this->readPreference = $options['readPreference'] ?? $this->manager->getReadPreference(); $this->typeMap = $options['typeMap'] ?? self::DEFAULT_TYPE_MAP; @@ -162,6 +175,7 @@ public function __construct(Manager $manager, string $databaseName, string $coll public function __debugInfo() { return [ + 'codec' => $this->codec, 'collectionName' => $this->collectionName, 'databaseName' => $this->databaseName, 'manager' => $this->manager, @@ -220,6 +234,10 @@ public function aggregate(array $pipeline, array $options = []) $options['readConcern'] = $this->readConcern; } + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + if (! isset($options['typeMap'])) { $options['typeMap'] = $this->typeMap; } @@ -246,6 +264,10 @@ public function aggregate(array $pipeline, array $options = []) */ public function bulkWrite(array $operations, array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { $options['writeConcern'] = $this->writeConcern; } @@ -626,6 +648,10 @@ public function explain(Explainable $explainable, array $options = []) */ public function find($filter = [], array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + if (! isset($options['readPreference']) && ! is_in_transaction($options)) { $options['readPreference'] = $this->readPreference; } @@ -659,6 +685,10 @@ public function find($filter = [], array $options = []) */ public function findOne($filter = [], array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + if (! isset($options['readPreference']) && ! is_in_transaction($options)) { $options['readPreference'] = $this->readPreference; } @@ -695,6 +725,10 @@ public function findOne($filter = [], array $options = []) */ public function findOneAndDelete($filter, array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + $server = select_server($this->manager, $options); if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { @@ -732,6 +766,10 @@ public function findOneAndDelete($filter, array $options = []) */ public function findOneAndReplace($filter, $replacement, array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + $server = select_server($this->manager, $options); if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { @@ -769,6 +807,10 @@ public function findOneAndReplace($filter, $replacement, array $options = []) */ public function findOneAndUpdate($filter, $update, array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + $server = select_server($this->manager, $options); if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { @@ -880,6 +922,10 @@ public function getWriteConcern() */ public function insertMany(array $documents, array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { $options['writeConcern'] = $this->writeConcern; } @@ -903,6 +949,10 @@ public function insertMany(array $documents, array $options = []) */ public function insertOne($document, array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { $options['writeConcern'] = $this->writeConcern; } @@ -1029,6 +1079,10 @@ public function rename(string $toCollectionName, ?string $toDatabaseName = null, */ public function replaceOne($filter, $replacement, array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { $options['writeConcern'] = $this->writeConcern; } @@ -1100,6 +1154,10 @@ public function updateOne($filter, $update, array $options = []) */ public function watch(array $pipeline = [], array $options = []) { + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + if (! isset($options['readPreference']) && ! is_in_transaction($options)) { $options['readPreference'] = $this->readPreference; } @@ -1137,6 +1195,7 @@ public function watch(array $pipeline = [], array $options = []) public function withOptions(array $options = []) { $options += [ + 'codec' => $this->codec, 'readConcern' => $this->readConcern, 'readPreference' => $this->readPreference, 'typeMap' => $this->typeMap, diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php new file mode 100644 index 000000000..101cefcc3 --- /dev/null +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -0,0 +1,370 @@ +collection = new Collection( + $this->manager, + $this->getDatabaseName(), + $this->getCollectionName(), + ['codec' => new TestDocumentCodec()], + ); + } + + public static function provideAggregateOptions(): Generator + { + yield 'Default codec' => [ + 'expected' => [ + TestObject::createForFixture(2, true), + TestObject::createForFixture(3, true), + ], + 'options' => [], + ]; + + yield 'No codec' => [ + 'expected' => [ + self::createFixtureResult(2), + self::createFixtureResult(3), + ], + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideAggregateOptions */ + public function testAggregate($expected, $options): void + { + $this->createFixtures(3); + + $cursor = $this->collection->aggregate([['$match' => ['_id' => ['$gt' => 1]]]], $options); + + $this->assertEquals($expected, $cursor->toArray()); + } + + public static function provideBulkWriteOptions(): Generator + { + $replacedObject = TestObject::createForFixture(3, true); + $replacedObject->x->foo = 'baz'; + + yield 'Default codec' => [ + 'expected' => [ + TestObject::createForFixture(1, true), + TestObject::createForFixture(2, true), + $replacedObject, + TestObject::createForFixture(4, true), + ], + 'options' => [], + ]; + + $replacedObject = new BSONDocument(['_id' => 3, 'id' => 3, 'x' => new BSONDocument(['foo' => 'baz']), 'decoded' => false]); + $replacedObject->x->foo = 'baz'; + + yield 'No codec' => [ + 'expected' => [ + self::createFixtureResult(1), + self::createFixtureResult(2), + $replacedObject, + self::createObjectFixtureResult(4, true), + ], + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideBulkWriteOptions */ + public function testBulkWrite($expected, $options): void + { + $this->createFixtures(3); + + $replaceObject = TestObject::createForFixture(3); + $replaceObject->x->foo = 'baz'; + + $operations = [ + ['insertOne' => [TestObject::createForFixture(4)]], + ['replaceOne' => [['_id' => 3], $replaceObject]], + ]; + + $result = $this->collection->bulkWrite($operations, $options); + + $this->assertInstanceOf(BulkWriteResult::class, $result); + $this->assertSame(1, $result->getInsertedCount()); + $this->assertSame(1, $result->getMatchedCount()); + $this->assertSame(1, $result->getModifiedCount()); + + // Extract inserted ID when not using codec as it's an automatically generated ObjectId + if ($expected[3] instanceof BSONDocument && $expected[3]->_id === null) { + $expected[3]->_id = $result->getInsertedIds()[0]; + } + + $this->assertEquals( + $expected, + $this->collection->find([], $options)->toArray(), + ); + } + + public function provideFindOneAndModifyOptions(): Generator + { + yield 'Default codec' => [ + 'expected' => TestObject::createForFixture(1, true), + 'options' => [], + ]; + + yield 'No codec' => [ + 'expected' => self::createFixtureResult(1), + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideFindOneAndModifyOptions */ + public function testFindOneAndDelete($expected, $options): void + { + $this->createFixtures(1); + + $result = $this->collection->findOneAndDelete(['_id' => 1], $options); + + self::assertEquals($expected, $result); + } + + /** @dataProvider provideFindOneAndModifyOptions */ + public function testFindOneAndUpdate($expected, $options): void + { + $this->createFixtures(1); + + $result = $this->collection->findOneAndUpdate(['_id' => 1], ['$set' => ['x.foo' => 'baz']], $options); + + self::assertEquals($expected, $result); + } + + public static function provideFindOneAndReplaceOptions(): Generator + { + $replacedObject = TestObject::createForFixture(1, true); + $replacedObject->x->foo = 'baz'; + + yield 'Default codec' => [ + 'expected' => $replacedObject, + 'options' => [], + ]; + + $replacedObject = self::createObjectFixtureResult(1); + $replacedObject->x->foo = 'baz'; + + yield 'No codec' => [ + 'expected' => $replacedObject, + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideFindOneAndReplaceOptions */ + public function testFindOneAndReplace($expected, $options): void + { + $this->createFixtures(1); + + $replaceObject = TestObject::createForFixture(1); + $replaceObject->x->foo = 'baz'; + + $result = $this->collection->findOneAndReplace( + ['_id' => 1], + $replaceObject, + $options + ['returnDocument' => FindOneAndReplace::RETURN_DOCUMENT_AFTER], + ); + + self::assertEquals($expected, $result); + } + + public static function provideFindOptions(): Generator + { + yield 'Default codec' => [ + 'expected' => [ + TestObject::createForFixture(1, true), + TestObject::createForFixture(2, true), + TestObject::createForFixture(3, true), + ], + 'options' => [], + ]; + + yield 'No codec' => [ + 'expected' => [ + self::createFixtureResult(1), + self::createFixtureResult(2), + self::createFixtureResult(3), + ], + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideFindOptions */ + public function testFind($expected, $options): void + { + $this->createFixtures(3); + + $cursor = $this->collection->find([], $options); + + $this->assertEquals($expected, $cursor->toArray()); + } + + public static function provideFindOneOptions(): Generator + { + yield 'Default codec' => [ + 'expected' => TestObject::createForFixture(1, true), + 'options' => [], + ]; + + yield 'No codec' => [ + 'expected' => self::createFixtureResult(1), + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideFindOneOptions */ + public function testFindOne($expected, $options): void + { + $this->createFixtures(1); + + $document = $this->collection->findOne([], $options); + + $this->assertEquals($expected, $document); + } + + public static function provideInsertManyOptions(): Generator + { + yield 'Default codec' => [ + 'expected' => [ + TestObject::createForFixture(1, true), + TestObject::createForFixture(2, true), + TestObject::createForFixture(3, true), + ], + 'options' => [], + ]; + + yield 'No codec' => [ + 'expected' => [ + self::createObjectFixtureResult(1, true), + self::createObjectFixtureResult(2, true), + self::createObjectFixtureResult(3, true), + ], + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideInsertManyOptions */ + public function testInsertMany($expected, $options): void + { + $documents = [ + TestObject::createForFixture(1), + TestObject::createForFixture(2), + TestObject::createForFixture(3), + ]; + + $result = $this->collection->insertMany($documents, $options); + $this->assertSame(3, $result->getInsertedCount()); + + foreach ($expected as $index => $expectedDocument) { + if ($expectedDocument instanceof BSONDocument && $expectedDocument->_id === null) { + $expectedDocument->_id = $result->getInsertedIds()[$index]; + } + } + + $this->assertEquals($expected, $this->collection->find([], $options)->toArray()); + } + + public static function provideInsertOneOptions(): Generator + { + yield 'Default codec' => [ + 'expected' => TestObject::createForFixture(1, true), + 'options' => [], + ]; + + yield 'No codec' => [ + 'expected' => self::createObjectFixtureResult(1, true), + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideInsertOneOptions */ + public function testInsertOne($expected, $options): void + { + $result = $this->collection->insertOne(TestObject::createForFixture(1), $options); + $this->assertSame(1, $result->getInsertedCount()); + + if ($expected instanceof BSONDocument && $expected->_id === null) { + $expected->_id = $result->getInsertedId(); + } + + $this->assertEquals($expected, $this->collection->findOne([], $options)); + } + + public static function provideReplaceOneOptions(): Generator + { + $replacedObject = TestObject::createForFixture(1, true); + $replacedObject->x->foo = 'baz'; + + yield 'Default codec' => [ + 'expected' => $replacedObject, + 'options' => [], + ]; + + $replacedObject = self::createObjectFixtureResult(1); + $replacedObject->x->foo = 'baz'; + + yield 'No codec' => [ + 'expected' => $replacedObject, + 'options' => ['codec' => null], + ]; + } + + /** @dataProvider provideReplaceOneOptions */ + public function testReplaceOne($expected, $options): void + { + $this->createFixtures(1); + + $replaceObject = TestObject::createForFixture(1); + $replaceObject->x->foo = 'baz'; + + $result = $this->collection->replaceOne(['_id' => 1], $replaceObject, $options); + $this->assertSame(1, $result->getMatchedCount()); + $this->assertSame(1, $result->getModifiedCount()); + + $this->assertEquals($expected, $this->collection->findOne([], $options)); + } + + /** + * Create data fixtures. + */ + private function createFixtures(int $n, array $executeBulkWriteOptions = []): void + { + $bulkWrite = new BulkWrite(['ordered' => true]); + + for ($i = 1; $i <= $n; $i++) { + $bulkWrite->insert([ + '_id' => $i, + 'x' => (object) ['foo' => 'bar'], + ]); + } + + $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite, $executeBulkWriteOptions); + + $this->assertEquals($n, $result->getInsertedCount()); + } + + private static function createFixtureResult(int $id): BSONDocument + { + return new BSONDocument(['_id' => $id, 'x' => new BSONDocument(['foo' => 'bar'])]); + } + + private static function createObjectFixtureResult(int $id, bool $isInserted = false): BSONDocument + { + return new BSONDocument(['_id' => $isInserted ? null : $id, 'id' => $id, 'x' => new BSONDocument(['foo' => 'bar']), 'decoded' => false]); + } +} diff --git a/tests/Collection/CollectionFunctionalTest.php b/tests/Collection/CollectionFunctionalTest.php index ee38671df..709ec6c5a 100644 --- a/tests/Collection/CollectionFunctionalTest.php +++ b/tests/Collection/CollectionFunctionalTest.php @@ -66,6 +66,7 @@ public function testConstructorOptionTypeChecks(array $options): void public function provideInvalidConstructorOptions(): array { return $this->createOptionDataProvider([ + 'codec' => $this->getInvalidDocumentCodecValues(), 'readConcern' => $this->getInvalidReadConcernValues(), 'readPreference' => $this->getInvalidReadPreferenceValues(), 'typeMap' => $this->getInvalidArrayValues(), From 97c6035af01006d76763e13bd78fdbf36fe5b70d Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 25 Jul 2023 08:58:43 +0200 Subject: [PATCH 09/37] Remove temporary variable usage in operations --- psalm-baseline.xml | 10 +++++++--- src/Operation/FindOneAndReplace.php | 11 ++++++----- src/Operation/InsertMany.php | 11 ++++++----- src/Operation/InsertOne.php | 17 +++++++++-------- src/Operation/ReplaceOne.php | 11 ++++++----- 5 files changed, 34 insertions(+), 26 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index cfd570908..64f157f47 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -473,7 +473,7 @@ - is_object($replacementDocument) + is_object($replacement) @@ -483,6 +483,7 @@ + $document $insertedIds[$i] $options[$option] @@ -501,9 +502,12 @@ - encodeIfSupported isInTransaction + + assert(is_array($document) || is_object($document)) + is_array($document) + @@ -553,7 +557,7 @@ - is_object($replacementDocument) + is_object($replacement) diff --git a/src/Operation/FindOneAndReplace.php b/src/Operation/FindOneAndReplace.php index 5c7a1852f..5f4d0e305 100644 --- a/src/Operation/FindOneAndReplace.php +++ b/src/Operation/FindOneAndReplace.php @@ -158,17 +158,18 @@ public function __construct(string $databaseName, string $collectionName, $filte unset($options['projection'], $options['returnDocument']); - $replacementDocument = isset($options['codec']) - ? $options['codec']->encodeIfSupported($replacement) - : $replacement; + if (isset($options['codec'])) { + $replacement = $options['codec']->encodeIfSupported($replacement); + } + // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document - assert(is_array($replacementDocument) || is_object($replacementDocument)); + assert(is_array($replacement) || is_object($replacement)); $this->findAndModify = new FindAndModify( $databaseName, $collectionName, - ['query' => $filter, 'update' => $replacementDocument] + $options, + ['query' => $filter, 'update' => $replacement] + $options, ); } diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index 5c6020417..cc51493ec 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -153,14 +153,15 @@ public function execute(Server $server) $insertedIds = []; foreach ($this->documents as $i => $document) { - $insertedDocument = isset($this->options['codec']) - ? $this->options['codec']->encodeIfSupported($document) - : $document; + if (isset($this->options['codec'])) { + $document = $this->options['codec']->encodeIfSupported($document); + } + // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document - assert(is_array($insertedDocument) || is_object($insertedDocument)); + assert(is_array($document) || is_object($document)); - $insertedIds[$i] = $bulk->insert($insertedDocument); + $insertedIds[$i] = $bulk->insert($document); } $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); diff --git a/src/Operation/InsertOne.php b/src/Operation/InsertOne.php index 8e59efc6d..5947874da 100644 --- a/src/Operation/InsertOne.php +++ b/src/Operation/InsertOne.php @@ -105,6 +105,14 @@ public function __construct(string $databaseName, string $collectionName, $docum unset($options['writeConcern']); } + if (isset($options['codec'])) { + $document = $options['codec']->encodeIfSupported($document); + + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + assert(is_array($document) || is_object($document)); + } + $this->databaseName = $databaseName; $this->collectionName = $collectionName; $this->document = $document; @@ -128,14 +136,7 @@ public function execute(Server $server) $bulk = new Bulk($this->createBulkWriteOptions()); - $insertedDocument = isset($this->options['codec']) - ? $this->options['codec']->encodeIfSupported($this->document) - : $this->document; - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - assert(is_array($insertedDocument) || is_object($insertedDocument)); - - $insertedId = $bulk->insert($insertedDocument); + $insertedId = $bulk->insert($this->document); $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index 3f1528b24..57a3fcf65 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -107,18 +107,19 @@ public function __construct(string $databaseName, string $collectionName, $filte throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); } - $replacementDocument = isset($options['codec']) - ? $options['codec']->encodeIfSupported($replacement) - : $replacement; + if (isset($options['codec'])) { + $replacement = $options['codec']->encodeIfSupported($replacement); + } + // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document - assert(is_array($replacementDocument) || is_object($replacementDocument)); + assert(is_array($replacement) || is_object($replacement)); $this->update = new Update( $databaseName, $collectionName, $filter, - $replacementDocument, + $replacement, ['multi' => false] + $options, ); } From a3f463a7d4cb4cb500b4bce0e5dcbfe7eb5500c3 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 25 Jul 2023 09:12:07 +0200 Subject: [PATCH 10/37] Enforce iterator type when creating ChangeStreamIterator --- src/Model/ChangeStreamIterator.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Model/ChangeStreamIterator.php b/src/Model/ChangeStreamIterator.php index 6a21609a1..898d5bfbe 100644 --- a/src/Model/ChangeStreamIterator.php +++ b/src/Model/ChangeStreamIterator.php @@ -75,6 +75,14 @@ class ChangeStreamIterator extends IteratorIterator implements CommandSubscriber */ public function __construct(CursorInterface $cursor, int $firstBatchSize, $initialResumeToken, ?object $postBatchResumeToken) { + if (! $cursor instanceof Iterator) { + throw InvalidArgumentException::invalidType( + '$cursor', + $cursor, + CursorInterface::class . '&' . Iterator::class, + ); + } + if (isset($initialResumeToken) && ! is_document($initialResumeToken)) { throw InvalidArgumentException::expectedDocumentType('$initialResumeToken', $initialResumeToken); } From 4d3278535004027f4ed45e87e4c05dc269b5bddb Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 25 Jul 2023 09:14:35 +0200 Subject: [PATCH 11/37] Remove unnecessary null-coalesce --- src/Operation/FindAndModify.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Operation/FindAndModify.php b/src/Operation/FindAndModify.php index c3c679ead..aa162c0ac 100644 --- a/src/Operation/FindAndModify.php +++ b/src/Operation/FindAndModify.php @@ -258,7 +258,7 @@ public function execute(Server $server) $result = current($cursor->toArray()); assert($result instanceof Document); - $decoded = $this->options['codec']->decodeIfSupported($result->get('value') ?? null); + $decoded = $this->options['codec']->decodeIfSupported($result->get('value')); assert($decoded === null || is_object($decoded)); return $decoded; From 4a0d94456e2be6377b134da084c30aac643eabe1 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 25 Jul 2023 09:28:53 +0200 Subject: [PATCH 12/37] Ensure operation-level type map gets precedence over collection-level codec --- src/Collection.php | 22 ++--- .../CodecCollectionFunctionalTest.php | 82 ++++++++++++++++++- 2 files changed, 92 insertions(+), 12 deletions(-) diff --git a/src/Collection.php b/src/Collection.php index 237d73e14..92127404d 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -234,7 +234,7 @@ public function aggregate(array $pipeline, array $options = []) $options['readConcern'] = $this->readConcern; } - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -264,7 +264,7 @@ public function aggregate(array $pipeline, array $options = []) */ public function bulkWrite(array $operations, array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -648,7 +648,7 @@ public function explain(Explainable $explainable, array $options = []) */ public function find($filter = [], array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -685,7 +685,7 @@ public function find($filter = [], array $options = []) */ public function findOne($filter = [], array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -725,7 +725,7 @@ public function findOne($filter = [], array $options = []) */ public function findOneAndDelete($filter, array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -766,7 +766,7 @@ public function findOneAndDelete($filter, array $options = []) */ public function findOneAndReplace($filter, $replacement, array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -807,7 +807,7 @@ public function findOneAndReplace($filter, $replacement, array $options = []) */ public function findOneAndUpdate($filter, $update, array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -922,7 +922,7 @@ public function getWriteConcern() */ public function insertMany(array $documents, array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -949,7 +949,7 @@ public function insertMany(array $documents, array $options = []) */ public function insertOne($document, array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -1079,7 +1079,7 @@ public function rename(string $toCollectionName, ?string $toDatabaseName = null, */ public function replaceOne($filter, $replacement, array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } @@ -1154,7 +1154,7 @@ public function updateOne($filter, $update, array $options = []) */ public function watch(array $pipeline = [], array $options = []) { - if (! array_key_exists('codec', $options)) { + if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { $options['codec'] = $this->codec; } diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index 101cefcc3..415efae65 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -3,6 +3,7 @@ namespace MongoDB\Tests\Collection; use Generator; +use MongoDB\BSON\Document; use MongoDB\BulkWriteResult; use MongoDB\Collection; use MongoDB\Driver\BulkWrite; @@ -42,6 +43,14 @@ public static function provideAggregateOptions(): Generator ], 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => [ + Document::fromPHP(self::createFixtureResult(2)), + Document::fromPHP(self::createFixtureResult(3)), + ], + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideAggregateOptions */ @@ -81,6 +90,16 @@ public static function provideBulkWriteOptions(): Generator ], 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => [ + Document::fromPHP(self::createFixtureResult(1)), + Document::fromPHP(self::createFixtureResult(2)), + Document::fromPHP($replacedObject), + Document::fromPHP(self::createObjectFixtureResult(4, true)), + ], + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideBulkWriteOptions */ @@ -108,6 +127,12 @@ public function testBulkWrite($expected, $options): void $expected[3]->_id = $result->getInsertedIds()[0]; } + if ($expected[3] instanceof Document && $expected[3]->get('_id') === null) { + $inserted = $expected[3]->toPHP(); + $inserted->_id = $result->getInsertedIds()[0]; + $expected[3] = Document::fromPHP($inserted); + } + $this->assertEquals( $expected, $this->collection->find([], $options)->toArray(), @@ -125,6 +150,11 @@ public function provideFindOneAndModifyOptions(): Generator 'expected' => self::createFixtureResult(1), 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => Document::fromPHP(self::createFixtureResult(1)), + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideFindOneAndModifyOptions */ @@ -164,6 +194,11 @@ public static function provideFindOneAndReplaceOptions(): Generator 'expected' => $replacedObject, 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => Document::FromPHP($replacedObject), + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideFindOneAndReplaceOptions */ @@ -202,6 +237,15 @@ public static function provideFindOptions(): Generator ], 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => [ + Document::fromPHP(self::createFixtureResult(1)), + Document::fromPHP(self::createFixtureResult(2)), + Document::fromPHP(self::createFixtureResult(3)), + ], + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideFindOptions */ @@ -225,6 +269,11 @@ public static function provideFindOneOptions(): Generator 'expected' => self::createFixtureResult(1), 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => Document::fromPHP(self::createFixtureResult(1)), + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideFindOneOptions */ @@ -256,6 +305,15 @@ public static function provideInsertManyOptions(): Generator ], 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => [ + Document::fromPHP(self::createObjectFixtureResult(1, true)), + Document::fromPHP(self::createObjectFixtureResult(2, true)), + Document::fromPHP(self::createObjectFixtureResult(3, true)), + ], + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideInsertManyOptions */ @@ -270,10 +328,16 @@ public function testInsertMany($expected, $options): void $result = $this->collection->insertMany($documents, $options); $this->assertSame(3, $result->getInsertedCount()); - foreach ($expected as $index => $expectedDocument) { + foreach ($expected as $index => &$expectedDocument) { if ($expectedDocument instanceof BSONDocument && $expectedDocument->_id === null) { $expectedDocument->_id = $result->getInsertedIds()[$index]; } + + if ($expectedDocument instanceof Document && $expectedDocument->get('_id') === null) { + $inserted = $expectedDocument->toPHP(); + $inserted->_id = $result->getInsertedIds()[$index]; + $expectedDocument = Document::fromPHP($inserted); + } } $this->assertEquals($expected, $this->collection->find([], $options)->toArray()); @@ -290,6 +354,11 @@ public static function provideInsertOneOptions(): Generator 'expected' => self::createObjectFixtureResult(1, true), 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => Document::fromPHP(self::createObjectFixtureResult(1, true)), + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideInsertOneOptions */ @@ -302,6 +371,12 @@ public function testInsertOne($expected, $options): void $expected->_id = $result->getInsertedId(); } + if ($expected instanceof Document && $expected->get('_id') === null) { + $inserted = $expected->toPHP(); + $inserted->_id = $result->getInsertedId(); + $expected = Document::fromPHP($inserted); + } + $this->assertEquals($expected, $this->collection->findOne([], $options)); } @@ -322,6 +397,11 @@ public static function provideReplaceOneOptions(): Generator 'expected' => $replacedObject, 'options' => ['codec' => null], ]; + + yield 'BSON type map' => [ + 'expected' => Document::fromPHP($replacedObject), + 'options' => ['typeMap' => ['root' => 'bson']], + ]; } /** @dataProvider provideReplaceOneOptions */ From 8ce722159a1f415830a13013c47e7fd994177a51 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 27 Jul 2023 09:02:37 +0200 Subject: [PATCH 13/37] Reference psalm issue regarding unions and assert-if type annotations --- src/Operation/BulkWrite.php | 2 ++ src/Operation/FindOneAndReplace.php | 1 + src/Operation/InsertMany.php | 1 + src/Operation/InsertOne.php | 1 + src/Operation/ReplaceOne.php | 1 + 5 files changed, 6 insertions(+) diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index c4239b2d0..3c1e92123 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -346,6 +346,7 @@ public function execute(Server $server) : $args[0]; // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 assert(is_array($insertedDocument) || is_object($insertedDocument)); $insertedIds[$i] = $bulk->insert($insertedDocument); @@ -357,6 +358,7 @@ public function execute(Server $server) : $args[1]; // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 assert(is_array($replacementDocument) || is_object($replacementDocument)); $bulk->update($args[0], $replacementDocument, $args[2]); diff --git a/src/Operation/FindOneAndReplace.php b/src/Operation/FindOneAndReplace.php index 5f4d0e305..f19fd8154 100644 --- a/src/Operation/FindOneAndReplace.php +++ b/src/Operation/FindOneAndReplace.php @@ -164,6 +164,7 @@ public function __construct(string $databaseName, string $collectionName, $filte // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 assert(is_array($replacement) || is_object($replacement)); $this->findAndModify = new FindAndModify( diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index cc51493ec..ef0beb730 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -159,6 +159,7 @@ public function execute(Server $server) // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 assert(is_array($document) || is_object($document)); $insertedIds[$i] = $bulk->insert($document); diff --git a/src/Operation/InsertOne.php b/src/Operation/InsertOne.php index 5947874da..72b1c855d 100644 --- a/src/Operation/InsertOne.php +++ b/src/Operation/InsertOne.php @@ -110,6 +110,7 @@ public function __construct(string $databaseName, string $collectionName, $docum // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 assert(is_array($document) || is_object($document)); } diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index 57a3fcf65..dbde0961e 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -113,6 +113,7 @@ public function __construct(string $databaseName, string $collectionName, $filte // Psalm's assert-if-true annotation does not work with unions, so // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 assert(is_array($replacement) || is_object($replacement)); $this->update = new Update( From 45e5a77e87d1645e3470bde4f3f5e3ded3b685b6 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 27 Jul 2023 09:02:47 +0200 Subject: [PATCH 14/37] Add iterator type in ReadableStream --- src/GridFS/ReadableStream.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GridFS/ReadableStream.php b/src/GridFS/ReadableStream.php index d975ec43b..a0664e821 100644 --- a/src/GridFS/ReadableStream.php +++ b/src/GridFS/ReadableStream.php @@ -49,7 +49,7 @@ class ReadableStream private int $chunkOffset = 0; /** @var (CursorInterface&Iterator)|null */ - private $chunksIterator = null; + private ?Iterator $chunksIterator = null; private CollectionWrapper $collectionWrapper; From 6ee86c430a679b44607b36fedacab3f19a7deb98 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 27 Jul 2023 09:06:31 +0200 Subject: [PATCH 15/37] Assume _id property will be present in change stream result document. --- src/Model/ChangeStreamIterator.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Model/ChangeStreamIterator.php b/src/Model/ChangeStreamIterator.php index 898d5bfbe..06260035d 100644 --- a/src/Model/ChangeStreamIterator.php +++ b/src/Model/ChangeStreamIterator.php @@ -255,9 +255,7 @@ private function extractResumeToken($document) } if ($document instanceof Document) { - $resumeToken = $document->has('_id') - ? $document->get('_id') - : null; + $resumeToken = $document->get('_id'); if ($resumeToken instanceof Document) { $resumeToken = $resumeToken->toPHP(); From 21edb06ca9335beb8d9dfeaf15fb40482cab2de3 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 27 Jul 2023 09:46:04 +0200 Subject: [PATCH 16/37] Prohibit specifying typeMap and codec options for operations This commit also refactors the logic of inheriting collection-level options to operations to reduce code duplication. --- src/Collection.php | 339 +++++++----------- src/Exception/InvalidArgumentException.php | 5 + src/Operation/Aggregate.php | 4 + src/Operation/Find.php | 4 + src/Operation/FindAndModify.php | 4 + src/Operation/ReplaceOne.php | 4 + .../CodecCollectionFunctionalTest.php | 78 ++++ 7 files changed, 231 insertions(+), 207 deletions(-) diff --git a/src/Collection.php b/src/Collection.php index 92127404d..509a231d1 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -234,16 +234,10 @@ public function aggregate(array $pipeline, array $options = []) $options['readConcern'] = $this->readConcern; } - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } + $options = $this->inheritCodecOrTypeMap($options); - if ($hasWriteStage && ! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; + if ($hasWriteStage) { + $options = $this->inheritWriteOptions($options); } $operation = new Aggregate($this->databaseName, $this->collectionName, $pipeline, $options); @@ -264,13 +258,9 @@ public function aggregate(array $pipeline, array $options = []) */ public function bulkWrite(array $operations, array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritCodec($options), + ); $operation = new BulkWrite($this->databaseName, $this->collectionName, $operations, $options); $server = select_server($this->manager, $options); @@ -294,16 +284,10 @@ public function bulkWrite(array $operations, array $options = []) */ public function count($filter = [], array $options = []) { - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } + $options = $this->inheritReadOptions($options); $server = select_server($this->manager, $options); - if (! isset($options['readConcern']) && ! is_in_transaction($options)) { - $options['readConcern'] = $this->readConcern; - } - $operation = new Count($this->databaseName, $this->collectionName, $filter, $options); return $operation->execute($server); @@ -323,16 +307,10 @@ public function count($filter = [], array $options = []) */ public function countDocuments($filter = [], array $options = []) { - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } + $options = $this->inheritReadOptions($options); $server = select_server($this->manager, $options); - if (! isset($options['readConcern']) && ! is_in_transaction($options)) { - $options['readConcern'] = $this->readConcern; - } - $operation = new CountDocuments($this->databaseName, $this->collectionName, $filter, $options); return $operation->execute($server); @@ -391,9 +369,7 @@ public function createIndexes(array $indexes, array $options = []) { $server = select_server($this->manager, $options); - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions($options); $operation = new CreateIndexes($this->databaseName, $this->collectionName, $indexes, $options); @@ -414,9 +390,7 @@ public function createIndexes(array $indexes, array $options = []) */ public function deleteMany($filter, array $options = []) { - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions($options); $operation = new DeleteMany($this->databaseName, $this->collectionName, $filter, $options); $server = select_server($this->manager, $options); @@ -438,9 +412,7 @@ public function deleteMany($filter, array $options = []) */ public function deleteOne($filter, array $options = []) { - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions($options); $operation = new DeleteOne($this->databaseName, $this->collectionName, $filter, $options); $server = select_server($this->manager, $options); @@ -463,20 +435,12 @@ public function deleteOne($filter, array $options = []) */ public function distinct(string $fieldName, $filter = [], array $options = []) { - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } - - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } + $options = $this->inheritReadOptions( + $this->inheritTypeMap($options), + ); $server = select_server($this->manager, $options); - if (! isset($options['readConcern']) && ! is_in_transaction($options)) { - $options['readConcern'] = $this->readConcern; - } - $operation = new Distinct($this->databaseName, $this->collectionName, $fieldName, $filter, $options); return $operation->execute($server); @@ -494,15 +458,11 @@ public function distinct(string $fieldName, $filter = [], array $options = []) */ public function drop(array $options = []) { - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } - $server = select_server($this->manager, $options); - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritTypeMap($options), + ); if (! isset($options['encryptedFields'])) { $options['encryptedFields'] = get_encrypted_fields_from_driver($this->databaseName, $this->collectionName, $this->manager) @@ -535,15 +495,11 @@ public function dropIndex($indexName, array $options = []) throw new InvalidArgumentException('dropIndexes() must be used to drop multiple indexes'); } - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } - $server = select_server($this->manager, $options); - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritTypeMap($options), + ); $operation = new DropIndexes($this->databaseName, $this->collectionName, $indexName, $options); @@ -562,15 +518,11 @@ public function dropIndex($indexName, array $options = []) */ public function dropIndexes(array $options = []) { - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } - $server = select_server($this->manager, $options); - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritTypeMap($options), + ); $operation = new DropIndexes($this->databaseName, $this->collectionName, '*', $options); @@ -590,16 +542,10 @@ public function dropIndexes(array $options = []) */ public function estimatedDocumentCount(array $options = []) { - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } + $options = $this->inheritReadOptions($options); $server = select_server($this->manager, $options); - if (! isset($options['readConcern']) && ! is_in_transaction($options)) { - $options['readConcern'] = $this->readConcern; - } - $operation = new EstimatedDocumentCount($this->databaseName, $this->collectionName, $options); return $operation->execute($server); @@ -623,9 +569,7 @@ public function explain(Explainable $explainable, array $options = []) $options['readPreference'] = $this->readPreference; } - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } + $options = $this->inheritTypeMap($options); $server = select_server($this->manager, $options); @@ -648,24 +592,12 @@ public function explain(Explainable $explainable, array $options = []) */ public function find($filter = [], array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } + $options = $this->inheritReadOptions( + $this->inheritCodecOrTypeMap($options), + ); $server = select_server($this->manager, $options); - if (! isset($options['readConcern']) && ! is_in_transaction($options)) { - $options['readConcern'] = $this->readConcern; - } - - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } - $operation = new Find($this->databaseName, $this->collectionName, $filter, $options); return $operation->execute($server); @@ -685,24 +617,12 @@ public function find($filter = [], array $options = []) */ public function findOne($filter = [], array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } + $options = $this->inheritReadOptions( + $this->inheritCodecOrTypeMap($options), + ); $server = select_server($this->manager, $options); - if (! isset($options['readConcern']) && ! is_in_transaction($options)) { - $options['readConcern'] = $this->readConcern; - } - - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } - $operation = new FindOne($this->databaseName, $this->collectionName, $filter, $options); return $operation->execute($server); @@ -725,19 +645,11 @@ public function findOne($filter = [], array $options = []) */ public function findOneAndDelete($filter, array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - $server = select_server($this->manager, $options); - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } - - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } + $options = $this->inheritWriteOptions( + $this->inheritCodecOrTypeMap($options), + ); $operation = new FindOneAndDelete($this->databaseName, $this->collectionName, $filter, $options); @@ -766,19 +678,11 @@ public function findOneAndDelete($filter, array $options = []) */ public function findOneAndReplace($filter, $replacement, array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - $server = select_server($this->manager, $options); - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } - - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } + $options = $this->inheritWriteOptions( + $this->inheritCodecOrTypeMap($options), + ); $operation = new FindOneAndReplace($this->databaseName, $this->collectionName, $filter, $replacement, $options); @@ -807,19 +711,11 @@ public function findOneAndReplace($filter, $replacement, array $options = []) */ public function findOneAndUpdate($filter, $update, array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - $server = select_server($this->manager, $options); - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } - - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } + $options = $this->inheritWriteOptions( + $this->inheritCodecOrTypeMap($options), + ); $operation = new FindOneAndUpdate($this->databaseName, $this->collectionName, $filter, $update, $options); @@ -922,13 +818,9 @@ public function getWriteConcern() */ public function insertMany(array $documents, array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritCodec($options), + ); $operation = new InsertMany($this->databaseName, $this->collectionName, $documents, $options); $server = select_server($this->manager, $options); @@ -949,13 +841,9 @@ public function insertMany(array $documents, array $options = []) */ public function insertOne($document, array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritCodec($options), + ); $operation = new InsertOne($this->databaseName, $this->collectionName, $document, $options); $server = select_server($this->manager, $options); @@ -1018,13 +906,9 @@ public function mapReduce(JavascriptInterface $map, JavascriptInterface $reduce, $options['readConcern'] = $this->readConcern; } - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } - - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritTypeMap($options), + ); $operation = new MapReduce($this->databaseName, $this->collectionName, $map, $reduce, $out, $options); @@ -1049,15 +933,11 @@ public function rename(string $toCollectionName, ?string $toDatabaseName = null, $toDatabaseName = $this->databaseName; } - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } - $server = select_server($this->manager, $options); - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritTypeMap($options), + ); $operation = new RenameCollection($this->databaseName, $this->collectionName, $toDatabaseName, $toCollectionName, $options); @@ -1079,13 +959,9 @@ public function rename(string $toCollectionName, ?string $toDatabaseName = null, */ public function replaceOne($filter, $replacement, array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions( + $this->inheritCodec($options), + ); $operation = new ReplaceOne($this->databaseName, $this->collectionName, $filter, $replacement, $options); $server = select_server($this->manager, $options); @@ -1108,9 +984,7 @@ public function replaceOne($filter, $replacement, array $options = []) */ public function updateMany($filter, $update, array $options = []) { - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions($options); $operation = new UpdateMany($this->databaseName, $this->collectionName, $filter, $update, $options); $server = select_server($this->manager, $options); @@ -1133,9 +1007,7 @@ public function updateMany($filter, $update, array $options = []) */ public function updateOne($filter, $update, array $options = []) { - if (! isset($options['writeConcern']) && ! is_in_transaction($options)) { - $options['writeConcern'] = $this->writeConcern; - } + $options = $this->inheritWriteOptions($options); $operation = new UpdateOne($this->databaseName, $this->collectionName, $filter, $update, $options); $server = select_server($this->manager, $options); @@ -1154,31 +1026,12 @@ public function updateOne($filter, $update, array $options = []) */ public function watch(array $pipeline = [], array $options = []) { - if (! array_key_exists('codec', $options) && ! isset($options['typeMap'])) { - $options['codec'] = $this->codec; - } - - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } + $options = $this->inheritReadOptions( + $this->inheritCodecOrTypeMap($options), + ); $server = select_server($this->manager, $options); - /* Although change streams require a newer version of the server than - * read concerns, perform the usual wire version check before inheriting - * the collection's read concern. In the event that the server is too - * old, this makes it more likely that users will encounter an error - * related to change streams being unsupported instead of an - * UnsupportedException regarding use of the "readConcern" option from - * the Aggregate operation class. */ - if (! isset($options['readConcern']) && ! is_in_transaction($options)) { - $options['readConcern'] = $this->readConcern; - } - - if (! isset($options['typeMap'])) { - $options['typeMap'] = $this->typeMap; - } - $operation = new Watch($this->manager, $this->databaseName, $this->collectionName, $pipeline, $options); return $operation->execute($server); @@ -1204,4 +1057,76 @@ public function withOptions(array $options = []) return new Collection($this->manager, $this->databaseName, $this->collectionName, $options); } + + private function inheritCodec(array $options): array + { + // If the options contain a type map, don't inherit anything + if (isset($options['typeMap'])) { + return $options; + } + + if (! array_key_exists('codec', $options)) { + $options['codec'] = $this->codec; + } + + return $options; + } + + private function inheritCodecOrTypeMap(array $options): array + { + // If the options contain a type map, don't inherit anything + if (isset($options['typeMap'])) { + return $options; + } + + // If this collection does not use a codec, or if a codec was explicitly + // defined in the options, only inherit the type map (if possible) + if (! $this->codec || array_key_exists('codec', $options)) { + return $this->inheritTypeMap($options); + } + + // At this point, we know that we use a codec and the options array did + // not explicitly contain a codec, so we can inherit ours + $options['codec'] = $this->codec; + + return $options; + } + + private function inheritReadOptions(array $options): array + { + // ReadConcern and ReadPreference may not change within a transaction + if (! is_in_transaction($options)) { + if (! isset($options['readConcern'])) { + $options['readConcern'] = $this->readConcern; + } + + if (! isset($options['readPreference'])) { + $options['readPreference'] = $this->readPreference; + } + } + + return $options; + } + + private function inheritTypeMap(array $options): array + { + // Only inherit the type map if no codec is used + if (! isset($options['typeMap']) && ! isset($options['codec'])) { + $options['typeMap'] = $this->typeMap; + } + + return $options; + } + + private function inheritWriteOptions(array $options): array + { + // WriteConcern may not change within a transaction + if (! is_in_transaction($options)) { + if (! isset($options['writeConcern'])) { + $options['writeConcern'] = $this->writeConcern; + } + } + + return $options; + } } diff --git a/src/Exception/InvalidArgumentException.php b/src/Exception/InvalidArgumentException.php index fd043651c..7091e82af 100644 --- a/src/Exception/InvalidArgumentException.php +++ b/src/Exception/InvalidArgumentException.php @@ -29,6 +29,11 @@ class InvalidArgumentException extends DriverInvalidArgumentException implements Exception { + public static function cannotCombineCodecAndTypeMap(): self + { + return new self('Cannot provide both "codec" and "typeMap" options'); + } + /** * Thrown when an argument or option is expected to be a document. * diff --git a/src/Operation/Aggregate.php b/src/Operation/Aggregate.php index a63c45ac9..0b49748de 100644 --- a/src/Operation/Aggregate.php +++ b/src/Operation/Aggregate.php @@ -204,6 +204,10 @@ public function __construct(string $databaseName, ?string $collectionName, array unset($options['writeConcern']); } + if (isset($options['codec']) && isset($options['typeMap'])) { + throw InvalidArgumentException::cannotCombineCodecAndTypeMap(); + } + $this->isWrite = is_last_pipeline_operator_write($pipeline) && ! ($options['explain'] ?? false); if ($this->isWrite) { diff --git a/src/Operation/Find.php b/src/Operation/Find.php index e461f5ad2..23b33d482 100644 --- a/src/Operation/Find.php +++ b/src/Operation/Find.php @@ -300,6 +300,10 @@ public function __construct(string $databaseName, string $collectionName, $filte trigger_error('The "maxScan" option is deprecated and will be removed in a future release', E_USER_DEPRECATED); } + if (isset($options['codec']) && isset($options['typeMap'])) { + throw InvalidArgumentException::cannotCombineCodecAndTypeMap(); + } + $this->databaseName = $databaseName; $this->collectionName = $collectionName; $this->filter = $filter; diff --git a/src/Operation/FindAndModify.php b/src/Operation/FindAndModify.php index aa162c0ac..c1711cdc4 100644 --- a/src/Operation/FindAndModify.php +++ b/src/Operation/FindAndModify.php @@ -215,6 +215,10 @@ public function __construct(string $databaseName, string $collectionName, array unset($options['writeConcern']); } + if (isset($options['codec']) && isset($options['typeMap'])) { + throw InvalidArgumentException::cannotCombineCodecAndTypeMap(); + } + $this->databaseName = $databaseName; $this->collectionName = $collectionName; $this->options = $options; diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index dbde0961e..75619fb80 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -108,6 +108,10 @@ public function __construct(string $databaseName, string $collectionName, $filte } if (isset($options['codec'])) { + if (isset($options['typeMap'])) { + throw InvalidArgumentException::cannotCombineCodecAndTypeMap(); + } + $replacement = $options['codec']->encodeIfSupported($replacement); } diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index 415efae65..6ad6c2cdb 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -7,6 +7,7 @@ use MongoDB\BulkWriteResult; use MongoDB\Collection; use MongoDB\Driver\BulkWrite; +use MongoDB\Exception\InvalidArgumentException; use MongoDB\Model\BSONDocument; use MongoDB\Operation\FindOneAndReplace; use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; @@ -63,6 +64,17 @@ public function testAggregate($expected, $options): void $this->assertEquals($expected, $cursor->toArray()); } + public function testAggregateWithCodecAndTypemap(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + $this->collection->aggregate([['$match' => ['_id' => ['$gt' => 1]]]], $options); + } + public static function provideBulkWriteOptions(): Generator { $replacedObject = TestObject::createForFixture(3, true); @@ -167,6 +179,17 @@ public function testFindOneAndDelete($expected, $options): void self::assertEquals($expected, $result); } + public function testFindOneAndDeleteWithCodecAndTypemap(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + $this->collection->findOneAndDelete(['_id' => 1], $options); + } + /** @dataProvider provideFindOneAndModifyOptions */ public function testFindOneAndUpdate($expected, $options): void { @@ -177,6 +200,17 @@ public function testFindOneAndUpdate($expected, $options): void self::assertEquals($expected, $result); } + public function testFindOneAndUpdateWithCodecAndTypemap(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + $this->collection->findOneAndUpdate(['_id' => 1], ['$set' => ['x.foo' => 'baz']], $options); + } + public static function provideFindOneAndReplaceOptions(): Generator { $replacedObject = TestObject::createForFixture(1, true); @@ -218,6 +252,17 @@ public function testFindOneAndReplace($expected, $options): void self::assertEquals($expected, $result); } + public function testFindOneAndReplaceWithCodecAndTypemap(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + $this->collection->findOneAndReplace(['_id' => 1], ['foo' => 'bar'], $options); + } + public static function provideFindOptions(): Generator { yield 'Default codec' => [ @@ -258,6 +303,17 @@ public function testFind($expected, $options): void $this->assertEquals($expected, $cursor->toArray()); } + public function testFindWithCodecAndTypemap(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + $this->collection->find([], $options); + } + public static function provideFindOneOptions(): Generator { yield 'Default codec' => [ @@ -286,6 +342,17 @@ public function testFindOne($expected, $options): void $this->assertEquals($expected, $document); } + public function testFindOneWithCodecAndTypemap(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + $this->collection->findOne([], $options); + } + public static function provideInsertManyOptions(): Generator { yield 'Default codec' => [ @@ -419,6 +486,17 @@ public function testReplaceOne($expected, $options): void $this->assertEquals($expected, $this->collection->findOne([], $options)); } + public function testReplaceOneWithCodecAndTypemap(): void + { + $options = [ + 'codec' => new TestDocumentCodec(), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + ]; + + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + $this->collection->replaceOne(['_id' => 1], ['foo' => 'bar'], $options); + } + /** * Create data fixtures. */ From 53f9fbf25fe0651c690d26850ac9a0aa2709fecb Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 27 Jul 2023 10:03:17 +0200 Subject: [PATCH 17/37] Defer server selection until executing operation --- src/Collection.php | 91 +++++++++++++--------------------------------- 1 file changed, 25 insertions(+), 66 deletions(-) diff --git a/src/Collection.php b/src/Collection.php index 509a231d1..c3d20eec7 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -263,9 +263,8 @@ public function bulkWrite(array $operations, array $options = []) ); $operation = new BulkWrite($this->databaseName, $this->collectionName, $operations, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -286,11 +285,9 @@ public function count($filter = [], array $options = []) { $options = $this->inheritReadOptions($options); - $server = select_server($this->manager, $options); - $operation = new Count($this->databaseName, $this->collectionName, $filter, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -309,11 +306,9 @@ public function countDocuments($filter = [], array $options = []) { $options = $this->inheritReadOptions($options); - $server = select_server($this->manager, $options); - $operation = new CountDocuments($this->databaseName, $this->collectionName, $filter, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -367,13 +362,11 @@ public function createIndex($key, array $options = []) */ public function createIndexes(array $indexes, array $options = []) { - $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions($options); $operation = new CreateIndexes($this->databaseName, $this->collectionName, $indexes, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -393,9 +386,8 @@ public function deleteMany($filter, array $options = []) $options = $this->inheritWriteOptions($options); $operation = new DeleteMany($this->databaseName, $this->collectionName, $filter, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -415,9 +407,8 @@ public function deleteOne($filter, array $options = []) $options = $this->inheritWriteOptions($options); $operation = new DeleteOne($this->databaseName, $this->collectionName, $filter, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -439,11 +430,9 @@ public function distinct(string $fieldName, $filter = [], array $options = []) $this->inheritTypeMap($options), ); - $server = select_server($this->manager, $options); - $operation = new Distinct($this->databaseName, $this->collectionName, $fieldName, $filter, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -495,15 +484,13 @@ public function dropIndex($indexName, array $options = []) throw new InvalidArgumentException('dropIndexes() must be used to drop multiple indexes'); } - $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions( $this->inheritTypeMap($options), ); $operation = new DropIndexes($this->databaseName, $this->collectionName, $indexName, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -518,15 +505,13 @@ public function dropIndex($indexName, array $options = []) */ public function dropIndexes(array $options = []) { - $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions( $this->inheritTypeMap($options), ); $operation = new DropIndexes($this->databaseName, $this->collectionName, '*', $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -544,11 +529,9 @@ public function estimatedDocumentCount(array $options = []) { $options = $this->inheritReadOptions($options); - $server = select_server($this->manager, $options); - $operation = new EstimatedDocumentCount($this->databaseName, $this->collectionName, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -571,11 +554,9 @@ public function explain(Explainable $explainable, array $options = []) $options = $this->inheritTypeMap($options); - $server = select_server($this->manager, $options); - $operation = new Explain($this->databaseName, $explainable, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -596,11 +577,9 @@ public function find($filter = [], array $options = []) $this->inheritCodecOrTypeMap($options), ); - $server = select_server($this->manager, $options); - $operation = new Find($this->databaseName, $this->collectionName, $filter, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -621,11 +600,9 @@ public function findOne($filter = [], array $options = []) $this->inheritCodecOrTypeMap($options), ); - $server = select_server($this->manager, $options); - $operation = new FindOne($this->databaseName, $this->collectionName, $filter, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -645,15 +622,13 @@ public function findOne($filter = [], array $options = []) */ public function findOneAndDelete($filter, array $options = []) { - $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions( $this->inheritCodecOrTypeMap($options), ); $operation = new FindOneAndDelete($this->databaseName, $this->collectionName, $filter, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -678,15 +653,13 @@ public function findOneAndDelete($filter, array $options = []) */ public function findOneAndReplace($filter, $replacement, array $options = []) { - $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions( $this->inheritCodecOrTypeMap($options), ); $operation = new FindOneAndReplace($this->databaseName, $this->collectionName, $filter, $replacement, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -711,15 +684,13 @@ public function findOneAndReplace($filter, $replacement, array $options = []) */ public function findOneAndUpdate($filter, $update, array $options = []) { - $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions( $this->inheritCodecOrTypeMap($options), ); $operation = new FindOneAndUpdate($this->databaseName, $this->collectionName, $filter, $update, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -823,9 +794,8 @@ public function insertMany(array $documents, array $options = []) ); $operation = new InsertMany($this->databaseName, $this->collectionName, $documents, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -846,9 +816,8 @@ public function insertOne($document, array $options = []) ); $operation = new InsertOne($this->databaseName, $this->collectionName, $document, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -862,9 +831,8 @@ public function insertOne($document, array $options = []) public function listIndexes(array $options = []) { $operation = new ListIndexes($this->databaseName, $this->collectionName, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -895,8 +863,6 @@ public function mapReduce(JavascriptInterface $map, JavascriptInterface $reduce, $options['readPreference'] = new ReadPreference(ReadPreference::PRIMARY); } - $server = select_server($this->manager, $options); - /* A "majority" read concern is not compatible with inline output, so * avoid providing the Collection's read concern if it would conflict. * @@ -912,7 +878,7 @@ public function mapReduce(JavascriptInterface $map, JavascriptInterface $reduce, $operation = new MapReduce($this->databaseName, $this->collectionName, $map, $reduce, $out, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -933,15 +899,13 @@ public function rename(string $toCollectionName, ?string $toDatabaseName = null, $toDatabaseName = $this->databaseName; } - $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions( $this->inheritTypeMap($options), ); $operation = new RenameCollection($this->databaseName, $this->collectionName, $toDatabaseName, $toCollectionName, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -964,9 +928,8 @@ public function replaceOne($filter, $replacement, array $options = []) ); $operation = new ReplaceOne($this->databaseName, $this->collectionName, $filter, $replacement, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -987,9 +950,8 @@ public function updateMany($filter, $update, array $options = []) $options = $this->inheritWriteOptions($options); $operation = new UpdateMany($this->databaseName, $this->collectionName, $filter, $update, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -1010,9 +972,8 @@ public function updateOne($filter, $update, array $options = []) $options = $this->inheritWriteOptions($options); $operation = new UpdateOne($this->databaseName, $this->collectionName, $filter, $update, $options); - $server = select_server($this->manager, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** @@ -1030,11 +991,9 @@ public function watch(array $pipeline = [], array $options = []) $this->inheritCodecOrTypeMap($options), ); - $server = select_server($this->manager, $options); - $operation = new Watch($this->manager, $this->databaseName, $this->collectionName, $pipeline, $options); - return $operation->execute($server); + return $operation->execute(select_server($this->manager, $options)); } /** From f484bd7ff2d6db2efef1c03cf966c743b5e3742c Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Thu, 27 Jul 2023 10:06:15 +0200 Subject: [PATCH 18/37] Remove useless assertion --- src/ChangeStream.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ChangeStream.php b/src/ChangeStream.php index 11ec3528e..f54e012d2 100644 --- a/src/ChangeStream.php +++ b/src/ChangeStream.php @@ -18,7 +18,6 @@ namespace MongoDB; use Iterator; -use MongoDB\BSON\Document; use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\CursorId; use MongoDB\Driver\Exception\ConnectionException; @@ -29,7 +28,6 @@ use MongoDB\Model\ChangeStreamIterator; use ReturnTypeWillChange; -use function assert; use function call_user_func; use function in_array; @@ -116,8 +114,6 @@ public function current() return $value; } - assert($value === null || $value instanceof Document); - return $this->codec->decodeIfSupported($value); } From f06b40db454612708c7a39f89834eb5df30bf192 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 31 Jul 2023 14:00:48 +0200 Subject: [PATCH 19/37] Assert iterator type in ChangeStreamIterator::getInnerIterator --- src/Model/ChangeStreamIterator.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Model/ChangeStreamIterator.php b/src/Model/ChangeStreamIterator.php index 06260035d..d01ba7815 100644 --- a/src/Model/ChangeStreamIterator.php +++ b/src/Model/ChangeStreamIterator.php @@ -149,11 +149,14 @@ public function current() * iterator. This could be side-stepped due to the class not being final, * but it's very much an invalid use-case. This method can be dropped in 2.0 * once the class is final. + * + * @return CursorInterface&Iterator */ - final public function getInnerIterator(): CursorInterface + final public function getInnerIterator(): Iterator { $cursor = parent::getInnerIterator(); assert($cursor instanceof CursorInterface); + assert($cursor instanceof Iterator); return $cursor; } From 1f23d3466a5376b0bbd176bf63165d887613d389 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 31 Jul 2023 14:14:01 +0200 Subject: [PATCH 20/37] Make assertions on encoded result conditional --- psalm-baseline.xml | 14 +++++----- src/Operation/BulkWrite.php | 40 +++++++++++++++-------------- src/Operation/FindOneAndReplace.php | 10 ++++---- src/Operation/InsertMany.php | 10 ++++---- src/Operation/ReplaceOne.php | 10 ++++---- 5 files changed, 44 insertions(+), 40 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 64f157f47..ef694ae8a 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -200,7 +200,6 @@ $args[1] $args[1] $args[2] - $args[2] $args[0] @@ -209,7 +208,6 @@ $args[0] $args[0] $args[0] - $args[0] $args[1] $args[1] $args[1] @@ -225,8 +223,6 @@ $args[1] $args[1] $args[1] - $args[1] - $args[2] $args[2] $args[2] $args[2] @@ -240,6 +236,8 @@ + $args[0] + $args[1] $args[1] $args[1] $args[1] @@ -259,6 +257,8 @@ $args $args + $args[0] + $args[1] $args[2] $args[2] $insertedIds[$i] @@ -473,7 +473,8 @@ - is_object($replacement) + assert(is_array($replacement) || is_object($replacement)) + is_array($replacement) @@ -557,7 +558,8 @@ - is_object($replacement) + assert(is_array($replacement) || is_object($replacement)) + is_array($replacement) diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index 3c1e92123..efb77b931 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -341,28 +341,30 @@ public function execute(Server $server) break; case self::INSERT_ONE: - $insertedDocument = isset($this->options['codec']) - ? $this->options['codec']->encodeIfSupported($args[0]) - : $args[0]; - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($insertedDocument) || is_object($insertedDocument)); - - $insertedIds[$i] = $bulk->insert($insertedDocument); + if (isset($this->options['codec'])) { + $args[0] = $this->options['codec']->encodeIfSupported($args[0]); + + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 + assert(is_array($args[0]) || is_object($args[0])); + } + + $insertedIds[$i] = $bulk->insert($args[0]); break; case self::REPLACE_ONE: - $replacementDocument = isset($this->options['codec']) - ? $this->options['codec']->encodeIfSupported($args[1]) - : $args[1]; - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($replacementDocument) || is_object($replacementDocument)); - - $bulk->update($args[0], $replacementDocument, $args[2]); - break; + if (isset($this->options['codec'])) { + $args[1] = $this->options['codec']->encodeIfSupported($args[1]); + + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 + assert(is_array($args[1]) || is_object($args[1])); + } + + // break intentionally missing, as replace is handled + // through update as well case self::UPDATE_MANY: case self::UPDATE_ONE: diff --git a/src/Operation/FindOneAndReplace.php b/src/Operation/FindOneAndReplace.php index f19fd8154..e2d3713f8 100644 --- a/src/Operation/FindOneAndReplace.php +++ b/src/Operation/FindOneAndReplace.php @@ -160,12 +160,12 @@ public function __construct(string $databaseName, string $collectionName, $filte if (isset($options['codec'])) { $replacement = $options['codec']->encodeIfSupported($replacement); - } - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($replacement) || is_object($replacement)); + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 + assert(is_array($replacement) || is_object($replacement)); + } $this->findAndModify = new FindAndModify( $databaseName, diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index ef0beb730..4ab3a02fd 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -155,12 +155,12 @@ public function execute(Server $server) foreach ($this->documents as $i => $document) { if (isset($this->options['codec'])) { $document = $this->options['codec']->encodeIfSupported($document); - } - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($document) || is_object($document)); + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 + assert(is_array($document) || is_object($document)); + } $insertedIds[$i] = $bulk->insert($document); } diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index 75619fb80..c3143b982 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -113,12 +113,12 @@ public function __construct(string $databaseName, string $collectionName, $filte } $replacement = $options['codec']->encodeIfSupported($replacement); - } - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($replacement) || is_object($replacement)); + // Psalm's assert-if-true annotation does not work with unions, so + // assert the type manually instead of using is_document + // See https://github.com/vimeo/psalm/issues/6831 + assert(is_array($replacement) || is_object($replacement)); + } $this->update = new Update( $databaseName, From 18a0ed1b51850425aa162022c094d8ebbe38842e Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 31 Jul 2023 14:14:15 +0200 Subject: [PATCH 21/37] Extract factory to created decoded fixtures --- .../CodecCollectionFunctionalTest.php | 34 +++++++++---------- tests/Fixtures/Document/TestObject.php | 15 +++++--- tests/Operation/AggregateFunctionalTest.php | 4 +-- tests/Operation/BulkWriteFunctionalTest.php | 4 +-- .../Operation/FindAndModifyFunctionalTest.php | 6 ++-- tests/Operation/FindFunctionalTest.php | 6 ++-- .../FindOneAndReplaceFunctionalTest.php | 2 +- tests/Operation/FindOneFunctionalTest.php | 2 +- 8 files changed, 40 insertions(+), 33 deletions(-) diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index 6ad6c2cdb..13e932511 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -31,8 +31,8 @@ public static function provideAggregateOptions(): Generator { yield 'Default codec' => [ 'expected' => [ - TestObject::createForFixture(2, true), - TestObject::createForFixture(3, true), + TestObject::createDecodedForFixture(2), + TestObject::createDecodedForFixture(3), ], 'options' => [], ]; @@ -77,15 +77,15 @@ public function testAggregateWithCodecAndTypemap(): void public static function provideBulkWriteOptions(): Generator { - $replacedObject = TestObject::createForFixture(3, true); + $replacedObject = TestObject::createDecodedForFixture(3); $replacedObject->x->foo = 'baz'; yield 'Default codec' => [ 'expected' => [ - TestObject::createForFixture(1, true), - TestObject::createForFixture(2, true), + TestObject::createDecodedForFixture(1), + TestObject::createDecodedForFixture(2), $replacedObject, - TestObject::createForFixture(4, true), + TestObject::createDecodedForFixture(4), ], 'options' => [], ]; @@ -154,7 +154,7 @@ public function testBulkWrite($expected, $options): void public function provideFindOneAndModifyOptions(): Generator { yield 'Default codec' => [ - 'expected' => TestObject::createForFixture(1, true), + 'expected' => TestObject::createDecodedForFixture(1), 'options' => [], ]; @@ -213,7 +213,7 @@ public function testFindOneAndUpdateWithCodecAndTypemap(): void public static function provideFindOneAndReplaceOptions(): Generator { - $replacedObject = TestObject::createForFixture(1, true); + $replacedObject = TestObject::createDecodedForFixture(1); $replacedObject->x->foo = 'baz'; yield 'Default codec' => [ @@ -267,9 +267,9 @@ public static function provideFindOptions(): Generator { yield 'Default codec' => [ 'expected' => [ - TestObject::createForFixture(1, true), - TestObject::createForFixture(2, true), - TestObject::createForFixture(3, true), + TestObject::createDecodedForFixture(1), + TestObject::createDecodedForFixture(2), + TestObject::createDecodedForFixture(3), ], 'options' => [], ]; @@ -317,7 +317,7 @@ public function testFindWithCodecAndTypemap(): void public static function provideFindOneOptions(): Generator { yield 'Default codec' => [ - 'expected' => TestObject::createForFixture(1, true), + 'expected' => TestObject::createDecodedForFixture(1), 'options' => [], ]; @@ -357,9 +357,9 @@ public static function provideInsertManyOptions(): Generator { yield 'Default codec' => [ 'expected' => [ - TestObject::createForFixture(1, true), - TestObject::createForFixture(2, true), - TestObject::createForFixture(3, true), + TestObject::createDecodedForFixture(1), + TestObject::createDecodedForFixture(2), + TestObject::createDecodedForFixture(3), ], 'options' => [], ]; @@ -413,7 +413,7 @@ public function testInsertMany($expected, $options): void public static function provideInsertOneOptions(): Generator { yield 'Default codec' => [ - 'expected' => TestObject::createForFixture(1, true), + 'expected' => TestObject::createDecodedForFixture(1), 'options' => [], ]; @@ -449,7 +449,7 @@ public function testInsertOne($expected, $options): void public static function provideReplaceOneOptions(): Generator { - $replacedObject = TestObject::createForFixture(1, true); + $replacedObject = TestObject::createDecodedForFixture(1); $replacedObject->x->foo = 'baz'; yield 'Default codec' => [ diff --git a/tests/Fixtures/Document/TestObject.php b/tests/Fixtures/Document/TestObject.php index 20d79b9f4..8cfc59d9d 100644 --- a/tests/Fixtures/Document/TestObject.php +++ b/tests/Fixtures/Document/TestObject.php @@ -23,17 +23,24 @@ final class TestObject public TestNestedObject $x; - public bool $decoded; + public bool $decoded = false; - public static function createForFixture(int $i, bool $decoded = false): self + public static function createForFixture(int $id): self { $instance = new self(); - $instance->id = $i; - $instance->decoded = $decoded; + $instance->id = $id; $instance->x = new TestNestedObject(); $instance->x->foo = 'bar'; return $instance; } + + public static function createDecodedForFixture(int $id): self + { + $instance = self::createForFixture($id); + $instance->decoded = true; + + return $instance; + } } diff --git a/tests/Operation/AggregateFunctionalTest.php b/tests/Operation/AggregateFunctionalTest.php index 115404b29..c4244ec23 100644 --- a/tests/Operation/AggregateFunctionalTest.php +++ b/tests/Operation/AggregateFunctionalTest.php @@ -354,8 +354,8 @@ public function testCodecOption(): void $this->assertEquals( [ - TestObject::createForFixture(2, true), - TestObject::createForFixture(3, true), + TestObject::createDecodedForFixture(2), + TestObject::createDecodedForFixture(3), ], $cursor->toArray(), ); diff --git a/tests/Operation/BulkWriteFunctionalTest.php b/tests/Operation/BulkWriteFunctionalTest.php index e5c3f8c74..46958e324 100644 --- a/tests/Operation/BulkWriteFunctionalTest.php +++ b/tests/Operation/BulkWriteFunctionalTest.php @@ -479,14 +479,14 @@ public function testCodecOption(): void $this->assertSame(1, $result->getMatchedCount()); $this->assertSame(1, $result->getModifiedCount()); - $replacedObject = TestObject::createForFixture(3, true); + $replacedObject = TestObject::createDecodedForFixture(3); $replacedObject->x->foo = 'baz'; // Only read the last two documents as the other two don't fit our codec $this->assertEquals( [ $replacedObject, - TestObject::createForFixture(4, true), + TestObject::createDecodedForFixture(4), ], $this->collection->find(['_id' => ['$gte' => 3]], ['codec' => $codec])->toArray(), ); diff --git a/tests/Operation/FindAndModifyFunctionalTest.php b/tests/Operation/FindAndModifyFunctionalTest.php index 5c8e30474..c6b386c4a 100644 --- a/tests/Operation/FindAndModifyFunctionalTest.php +++ b/tests/Operation/FindAndModifyFunctionalTest.php @@ -293,7 +293,7 @@ public function testFindOneAndDeleteWithCodec(): void $result = $operation->execute($this->getPrimaryServer()); - self::assertEquals(TestObject::createForFixture(1, true), $result); + self::assertEquals(TestObject::createDecodedForFixture(1), $result); } public function testFindOneAndUpdateWithCodec(): void @@ -308,7 +308,7 @@ public function testFindOneAndUpdateWithCodec(): void $result = $operation->execute($this->getPrimaryServer()); - self::assertEquals(TestObject::createForFixture(1, true), $result); + self::assertEquals(TestObject::createDecodedForFixture(1), $result); } public function testFindOneAndReplaceWithCodec(): void @@ -323,7 +323,7 @@ public function testFindOneAndReplaceWithCodec(): void $result = $operation->execute($this->getPrimaryServer()); - self::assertEquals(TestObject::createForFixture(1, true), $result); + self::assertEquals(TestObject::createDecodedForFixture(1), $result); } /** diff --git a/tests/Operation/FindFunctionalTest.php b/tests/Operation/FindFunctionalTest.php index c3283c66f..b9108c8c4 100644 --- a/tests/Operation/FindFunctionalTest.php +++ b/tests/Operation/FindFunctionalTest.php @@ -209,9 +209,9 @@ public function testCodecOption(): void $this->assertEquals( [ - TestObject::createForFixture(1, true), - TestObject::createForFixture(2, true), - TestObject::createForFixture(3, true), + TestObject::createDecodedForFixture(1), + TestObject::createDecodedForFixture(2), + TestObject::createDecodedForFixture(3), ], $cursor->toArray(), ); diff --git a/tests/Operation/FindOneAndReplaceFunctionalTest.php b/tests/Operation/FindOneAndReplaceFunctionalTest.php index 782d46c0a..547c86f4e 100644 --- a/tests/Operation/FindOneAndReplaceFunctionalTest.php +++ b/tests/Operation/FindOneAndReplaceFunctionalTest.php @@ -28,7 +28,7 @@ function (): void { ); $this->assertEquals( - TestObject::createForFixture(1, true), + TestObject::createDecodedForFixture(1), $operation->execute($this->getPrimaryServer()), ); }, diff --git a/tests/Operation/FindOneFunctionalTest.php b/tests/Operation/FindOneFunctionalTest.php index afbc9a6f2..9b4b79487 100644 --- a/tests/Operation/FindOneFunctionalTest.php +++ b/tests/Operation/FindOneFunctionalTest.php @@ -47,7 +47,7 @@ public function testCodecOption(): void $operation = new FindOne($this->getDatabaseName(), $this->getCollectionName(), [], ['codec' => $codec]); $document = $operation->execute($this->getPrimaryServer()); - $this->assertEquals(TestObject::createForFixture(1, true), $document); + $this->assertEquals(TestObject::createDecodedForFixture(1), $document); } /** From 38aaf71d039e5c1100fcef1d0ad3768b3fc6393e Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 31 Jul 2023 14:15:41 +0200 Subject: [PATCH 22/37] Split document creation for legibility --- tests/Collection/CodecCollectionFunctionalTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index 13e932511..ea6e8a772 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -523,6 +523,11 @@ private static function createFixtureResult(int $id): BSONDocument private static function createObjectFixtureResult(int $id, bool $isInserted = false): BSONDocument { - return new BSONDocument(['_id' => $isInserted ? null : $id, 'id' => $id, 'x' => new BSONDocument(['foo' => 'bar']), 'decoded' => false]); + return new BSONDocument([ + '_id' => $isInserted ? null : $id, + 'id' => $id, + 'x' => new BSONDocument(['foo' => 'bar']), + 'decoded' => false, + ]); } } From 1d9699fd84d960e09f6da6ec63052a3245e0a3ff Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 31 Jul 2023 14:21:08 +0200 Subject: [PATCH 23/37] Insert fixture through factory in test object --- tests/Collection/CodecCollectionFunctionalTest.php | 5 +---- tests/Fixtures/Document/TestObject.php | 10 ++++++++++ tests/Operation/AggregateFunctionalTest.php | 5 +---- tests/Operation/FindAndModifyFunctionalTest.php | 5 +---- tests/Operation/FindFunctionalTest.php | 5 +---- tests/Operation/FindOneAndReplaceFunctionalTest.php | 5 +---- tests/Operation/FindOneFunctionalTest.php | 5 +---- 7 files changed, 16 insertions(+), 24 deletions(-) diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index ea6e8a772..9b4be93e1 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -505,10 +505,7 @@ private function createFixtures(int $n, array $executeBulkWriteOptions = []): vo $bulkWrite = new BulkWrite(['ordered' => true]); for ($i = 1; $i <= $n; $i++) { - $bulkWrite->insert([ - '_id' => $i, - 'x' => (object) ['foo' => 'bar'], - ]); + $bulkWrite->insert(TestObject::createDocument($i)); } $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite, $executeBulkWriteOptions); diff --git a/tests/Fixtures/Document/TestObject.php b/tests/Fixtures/Document/TestObject.php index 8cfc59d9d..37a0c0da5 100644 --- a/tests/Fixtures/Document/TestObject.php +++ b/tests/Fixtures/Document/TestObject.php @@ -17,6 +17,8 @@ namespace MongoDB\Tests\Fixtures\Document; +use MongoDB\BSON\Document; + final class TestObject { public int $id; @@ -25,6 +27,14 @@ final class TestObject public bool $decoded = false; + public static function createDocument(int $id): Document + { + return Document::fromPHP([ + '_id' => $id, + 'x' => ['foo' => 'bar'], + ]); + } + public static function createForFixture(int $id): self { $instance = new self(); diff --git a/tests/Operation/AggregateFunctionalTest.php b/tests/Operation/AggregateFunctionalTest.php index c4244ec23..3d1057ac9 100644 --- a/tests/Operation/AggregateFunctionalTest.php +++ b/tests/Operation/AggregateFunctionalTest.php @@ -369,10 +369,7 @@ private function createFixtures(int $n, array $executeBulkWriteOptions = []): vo $bulkWrite = new BulkWrite(['ordered' => true]); for ($i = 1; $i <= $n; $i++) { - $bulkWrite->insert([ - '_id' => $i, - 'x' => (object) ['foo' => 'bar'], - ]); + $bulkWrite->insert(TestObject::createDocument($i)); } $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite, $executeBulkWriteOptions); diff --git a/tests/Operation/FindAndModifyFunctionalTest.php b/tests/Operation/FindAndModifyFunctionalTest.php index c6b386c4a..5683c8990 100644 --- a/tests/Operation/FindAndModifyFunctionalTest.php +++ b/tests/Operation/FindAndModifyFunctionalTest.php @@ -334,10 +334,7 @@ private function createFixtures(int $n): void $bulkWrite = new BulkWrite(['ordered' => true]); for ($i = 1; $i <= $n; $i++) { - $bulkWrite->insert([ - '_id' => $i, - 'x' => (object) ['foo' => 'bar'], - ]); + $bulkWrite->insert(TestObject::createDocument($i)); } $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite); diff --git a/tests/Operation/FindFunctionalTest.php b/tests/Operation/FindFunctionalTest.php index b9108c8c4..2b70a421b 100644 --- a/tests/Operation/FindFunctionalTest.php +++ b/tests/Operation/FindFunctionalTest.php @@ -323,10 +323,7 @@ private function createFixtures(int $n, array $executeBulkWriteOptions = []): vo $bulkWrite = new BulkWrite(['ordered' => true]); for ($i = 1; $i <= $n; $i++) { - $bulkWrite->insert([ - '_id' => $i, - 'x' => (object) ['foo' => 'bar'], - ]); + $bulkWrite->insert(TestObject::createDocument($i)); } $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite, $executeBulkWriteOptions); diff --git a/tests/Operation/FindOneAndReplaceFunctionalTest.php b/tests/Operation/FindOneAndReplaceFunctionalTest.php index 547c86f4e..9bbd8047c 100644 --- a/tests/Operation/FindOneAndReplaceFunctionalTest.php +++ b/tests/Operation/FindOneAndReplaceFunctionalTest.php @@ -53,10 +53,7 @@ private function createFixtures(int $n): void $bulkWrite = new BulkWrite(['ordered' => true]); for ($i = 1; $i <= $n; $i++) { - $bulkWrite->insert([ - '_id' => $i, - 'x' => (object) ['foo' => 'bar'], - ]); + $bulkWrite->insert(TestObject::createDocument($i)); } $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite); diff --git a/tests/Operation/FindOneFunctionalTest.php b/tests/Operation/FindOneFunctionalTest.php index 9b4b79487..57e01f442 100644 --- a/tests/Operation/FindOneFunctionalTest.php +++ b/tests/Operation/FindOneFunctionalTest.php @@ -58,10 +58,7 @@ private function createFixtures(int $n): void $bulkWrite = new BulkWrite(['ordered' => true]); for ($i = 1; $i <= $n; $i++) { - $bulkWrite->insert([ - '_id' => $i, - 'x' => (object) ['foo' => 'bar'], - ]); + $bulkWrite->insert(TestObject::createDocument($i)); } $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite); From ec53fe7a11df4592739d571f91494fb94bc6db7a Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 31 Jul 2023 14:35:15 +0200 Subject: [PATCH 24/37] Trigger warning when calling CodecCursor::setTypeMap --- src/Model/CodecCursor.php | 5 ++++ tests/Model/CodecCursorFunctionalTest.php | 30 +++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 tests/Model/CodecCursorFunctionalTest.php diff --git a/src/Model/CodecCursor.php b/src/Model/CodecCursor.php index c4c9e9b97..b3b5061ae 100644 --- a/src/Model/CodecCursor.php +++ b/src/Model/CodecCursor.php @@ -27,6 +27,10 @@ use function assert; use function iterator_to_array; +use function sprintf; +use function trigger_error; + +use const E_USER_WARNING; /** * @template TValue of object @@ -104,6 +108,7 @@ public function rewind(): void public function setTypeMap(array $typemap): void { // Not supported + trigger_error(sprintf('Discarding type map for %s', __METHOD__), E_USER_WARNING); } /** @return array */ diff --git a/tests/Model/CodecCursorFunctionalTest.php b/tests/Model/CodecCursorFunctionalTest.php new file mode 100644 index 000000000..563cbbbdf --- /dev/null +++ b/tests/Model/CodecCursorFunctionalTest.php @@ -0,0 +1,30 @@ +dropCollection($this->getDatabaseName(), $this->getCollectionName()); + } + + public function testSetTypeMap(): void + { + $collection = self::createTestClient()->selectCollection($this->getDatabaseName(), $this->getCollectionName()); + $cursor = $collection->find(); + + $codecCursor = CodecCursor::fromCursor($cursor, $this->createMock(DocumentCodec::class)); + + $this->expectWarning(); + $this->expectWarningMessage('Discarding type map for MongoDB\Model\CodecCursor::setTypeMap'); + + $codecCursor->setTypeMap(['root' => 'array']); + } +} From c4d46b84a04466de4d338b2918fe9e7045bd4edb Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 2 Aug 2023 09:34:08 +0200 Subject: [PATCH 25/37] Encode documents before type checks This commit also changes the behaviour to use encode() instead of encodeIfSupported(), requiring documents to be encodable by the given codec in order to be inserted/updated. --- psalm-baseline.xml | 36 +-- src/Collection.php | 4 +- src/Operation/BulkWrite.php | 290 +++++++++--------- src/Operation/FindOneAndReplace.php | 59 ++-- src/Operation/InsertMany.php | 63 ++-- src/Operation/InsertOne.php | 35 ++- src/Operation/ReplaceOne.php | 59 ++-- .../CodecCollectionFunctionalTest.php | 2 +- tests/Operation/BulkWriteTest.php | 26 ++ tests/Operation/InsertManyTest.php | 9 + tests/Operation/InsertOneTest.php | 9 + tests/Operation/ReplaceOneTest.php | 9 + 12 files changed, 326 insertions(+), 275 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index ef694ae8a..56eb17f4d 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -193,6 +193,8 @@ $args[0] $args[0] $args[0] + $args[0] + $args[1] $args[1] $args[1] $args[1] @@ -236,8 +238,6 @@ - $args[0] - $args[1] $args[1] $args[1] $args[1] @@ -250,6 +250,8 @@ $args[2] + $operations[$i][$type][0] + $operations[$i][$type][1] $operations[$i][$type][1] $operations[$i][$type][2] $operations[$i][$type][2] @@ -257,8 +259,6 @@ $args $args - $args[0] - $args[1] $args[2] $args[2] $insertedIds[$i] @@ -270,8 +270,6 @@ - encodeIfSupported - encodeIfSupported isInTransaction @@ -472,10 +470,9 @@ - - assert(is_array($replacement) || is_object($replacement)) - is_array($replacement) - + + $replacement + @@ -484,16 +481,17 @@ - $document $insertedIds[$i] $options[$option] - encodeIfSupported isInTransaction + + $document + @@ -505,10 +503,9 @@ isInTransaction - - assert(is_array($document) || is_object($document)) - is_array($document) - + + $document + @@ -557,10 +554,9 @@ - - assert(is_array($replacement) || is_object($replacement)) - is_array($replacement) - + + $replacement + diff --git a/src/Collection.php b/src/Collection.php index c3d20eec7..427308d0d 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -781,8 +781,8 @@ public function getWriteConcern() * * @see InsertMany::__construct() for supported options * @see https://mongodb.com/docs/manual/reference/command/insert/ - * @param array[]|object[] $documents The documents to insert - * @param array $options Command options + * @param list $documents The documents to insert + * @param array $options Command options * @return InsertManyResult * @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 efb77b931..e42f2fdc2 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -29,12 +29,10 @@ use function array_is_list; use function array_key_exists; -use function assert; use function count; 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; @@ -141,6 +139,139 @@ public function __construct(string $databaseName, string $collectionName, array throw new InvalidArgumentException('$operations is not a list'); } + $options += ['ordered' => true]; + + if (isset($options['bypassDocumentValidation']) && ! is_bool($options['bypassDocumentValidation'])) { + throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean'); + } + + if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { + throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); + } + + if (! is_bool($options['ordered'])) { + throw InvalidArgumentException::invalidType('"ordered" option', $options['ordered'], 'boolean'); + } + + if (isset($options['session']) && ! $options['session'] instanceof Session) { + throw InvalidArgumentException::invalidType('"session" option', $options['session'], Session::class); + } + + if (isset($options['writeConcern']) && ! $options['writeConcern'] instanceof WriteConcern) { + throw InvalidArgumentException::invalidType('"writeConcern" option', $options['writeConcern'], WriteConcern::class); + } + + if (isset($options['let']) && ! is_document($options['let'])) { + throw InvalidArgumentException::expectedDocumentType('"let" option', $options['let']); + } + + if (isset($options['bypassDocumentValidation']) && ! $options['bypassDocumentValidation']) { + unset($options['bypassDocumentValidation']); + } + + if (isset($options['writeConcern']) && $options['writeConcern']->isDefault()) { + unset($options['writeConcern']); + } + + $this->databaseName = $databaseName; + $this->collectionName = $collectionName; + $this->operations = $this->validateOperations($operations, $options['codec'] ?? null); + $this->options = $options; + } + + /** + * Execute the operation. + * + * @see Executable::execute() + * @return BulkWriteResult + * @throws UnsupportedException if write concern is used and unsupported + * @throws DriverRuntimeException for other driver errors (e.g. connection errors) + */ + public function execute(Server $server) + { + $inTransaction = isset($this->options['session']) && $this->options['session']->isInTransaction(); + if ($inTransaction && isset($this->options['writeConcern'])) { + throw UnsupportedException::writeConcernNotSupportedInTransaction(); + } + + $bulk = new Bulk($this->createBulkWriteOptions()); + $insertedIds = []; + + foreach ($this->operations as $i => $operation) { + $type = key($operation); + $args = current($operation); + + switch ($type) { + case self::DELETE_MANY: + case self::DELETE_ONE: + $bulk->delete($args[0], $args[1]); + break; + + case self::INSERT_ONE: + $insertedIds[$i] = $bulk->insert($args[0]); + break; + + case self::UPDATE_MANY: + case self::UPDATE_ONE: + case self::REPLACE_ONE: + $bulk->update($args[0], $args[1], $args[2]); + break; + } + } + + $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); + + return new BulkWriteResult($writeResult, $insertedIds); + } + + /** + * Create options for constructing the bulk write. + * + * @see https://php.net/manual/en/mongodb-driver-bulkwrite.construct.php + */ + private function createBulkWriteOptions(): array + { + $options = ['ordered' => $this->options['ordered']]; + + foreach (['bypassDocumentValidation', 'comment'] as $option) { + if (isset($this->options[$option])) { + $options[$option] = $this->options[$option]; + } + } + + if (isset($this->options['let'])) { + $options['let'] = (object) $this->options['let']; + } + + return $options; + } + + /** + * Create options for executing the bulk write. + * + * @see https://php.net/manual/en/mongodb-driver-server.executebulkwrite.php + */ + private function createExecuteOptions(): array + { + $options = []; + + if (isset($this->options['session'])) { + $options['session'] = $this->options['session']; + } + + if (isset($this->options['writeConcern'])) { + $options['writeConcern'] = $this->options['writeConcern']; + } + + return $options; + } + + /** + * @param array[] $operations + * @return array[] + */ + private function validateOperations(array $operations, ?DocumentCodec $codec): array + { foreach ($operations as $i => $operation) { if (! is_array($operation)) { throw InvalidArgumentException::invalidType(sprintf('$operations[%d]', $i), $operation, 'array'); @@ -163,6 +294,10 @@ public function __construct(string $databaseName, string $collectionName, array switch ($type) { case self::INSERT_ONE: + if ($codec) { + $operations[$i][$type][0] = $codec->encode($args[0]); + } + break; case self::DELETE_MANY: @@ -190,6 +325,10 @@ public function __construct(string $databaseName, string $collectionName, array throw new InvalidArgumentException(sprintf('Missing second argument for $operations[%d]["%s"]', $i, $type)); } + if ($codec) { + $operations[$i][$type][1] = $codec->encode($args[1]); + } + if (! is_document($args[1])) { throw InvalidArgumentException::expectedDocumentType(sprintf('$operations[%d]["%s"][1]', $i, $type), $args[1]); } @@ -272,151 +411,6 @@ public function __construct(string $databaseName, string $collectionName, array } } - $options += ['ordered' => true]; - - if (isset($options['bypassDocumentValidation']) && ! is_bool($options['bypassDocumentValidation'])) { - throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean'); - } - - if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { - throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); - } - - if (! is_bool($options['ordered'])) { - throw InvalidArgumentException::invalidType('"ordered" option', $options['ordered'], 'boolean'); - } - - if (isset($options['session']) && ! $options['session'] instanceof Session) { - throw InvalidArgumentException::invalidType('"session" option', $options['session'], Session::class); - } - - if (isset($options['writeConcern']) && ! $options['writeConcern'] instanceof WriteConcern) { - throw InvalidArgumentException::invalidType('"writeConcern" option', $options['writeConcern'], WriteConcern::class); - } - - if (isset($options['let']) && ! is_document($options['let'])) { - throw InvalidArgumentException::expectedDocumentType('"let" option', $options['let']); - } - - if (isset($options['bypassDocumentValidation']) && ! $options['bypassDocumentValidation']) { - unset($options['bypassDocumentValidation']); - } - - if (isset($options['writeConcern']) && $options['writeConcern']->isDefault()) { - unset($options['writeConcern']); - } - - $this->databaseName = $databaseName; - $this->collectionName = $collectionName; - $this->operations = $operations; - $this->options = $options; - } - - /** - * Execute the operation. - * - * @see Executable::execute() - * @return BulkWriteResult - * @throws UnsupportedException if write concern is used and unsupported - * @throws DriverRuntimeException for other driver errors (e.g. connection errors) - */ - public function execute(Server $server) - { - $inTransaction = isset($this->options['session']) && $this->options['session']->isInTransaction(); - if ($inTransaction && isset($this->options['writeConcern'])) { - throw UnsupportedException::writeConcernNotSupportedInTransaction(); - } - - $bulk = new Bulk($this->createBulkWriteOptions()); - $insertedIds = []; - - foreach ($this->operations as $i => $operation) { - $type = key($operation); - $args = current($operation); - - switch ($type) { - case self::DELETE_MANY: - case self::DELETE_ONE: - $bulk->delete($args[0], $args[1]); - break; - - case self::INSERT_ONE: - if (isset($this->options['codec'])) { - $args[0] = $this->options['codec']->encodeIfSupported($args[0]); - - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($args[0]) || is_object($args[0])); - } - - $insertedIds[$i] = $bulk->insert($args[0]); - break; - - case self::REPLACE_ONE: - if (isset($this->options['codec'])) { - $args[1] = $this->options['codec']->encodeIfSupported($args[1]); - - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($args[1]) || is_object($args[1])); - } - - // break intentionally missing, as replace is handled - // through update as well - - case self::UPDATE_MANY: - case self::UPDATE_ONE: - $bulk->update($args[0], $args[1], $args[2]); - break; - } - } - - $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); - - return new BulkWriteResult($writeResult, $insertedIds); - } - - /** - * Create options for constructing the bulk write. - * - * @see https://php.net/manual/en/mongodb-driver-bulkwrite.construct.php - */ - private function createBulkWriteOptions(): array - { - $options = ['ordered' => $this->options['ordered']]; - - foreach (['bypassDocumentValidation', 'comment'] as $option) { - if (isset($this->options[$option])) { - $options[$option] = $this->options[$option]; - } - } - - if (isset($this->options['let'])) { - $options['let'] = (object) $this->options['let']; - } - - return $options; - } - - /** - * Create options for executing the bulk write. - * - * @see https://php.net/manual/en/mongodb-driver-server.executebulkwrite.php - */ - private function createExecuteOptions(): array - { - $options = []; - - if (isset($this->options['session'])) { - $options['session'] = $this->options['session']; - } - - if (isset($this->options['writeConcern'])) { - $options['writeConcern'] = $this->options['writeConcern']; - } - - return $options; + return $operations; } } diff --git a/src/Operation/FindOneAndReplace.php b/src/Operation/FindOneAndReplace.php index e2d3713f8..20412d645 100644 --- a/src/Operation/FindOneAndReplace.php +++ b/src/Operation/FindOneAndReplace.php @@ -24,10 +24,7 @@ use MongoDB\Exception\UnsupportedException; use function array_key_exists; -use function assert; -use function is_array; 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; @@ -111,23 +108,6 @@ public function __construct(string $databaseName, string $collectionName, $filte throw InvalidArgumentException::expectedDocumentType('$filter', $filter); } - if (! is_document($replacement)) { - throw InvalidArgumentException::expectedDocumentType('$replacement', $replacement); - } - - // Treat empty arrays as replacement documents for BC - if ($replacement === []) { - $replacement = (object) $replacement; - } - - if (is_first_key_operator($replacement)) { - throw new InvalidArgumentException('First key in $replacement is an update operator'); - } - - if (is_pipeline($replacement, true /* allowEmpty */)) { - throw new InvalidArgumentException('$replacement is an update pipeline'); - } - if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); } @@ -158,14 +138,7 @@ public function __construct(string $databaseName, string $collectionName, $filte unset($options['projection'], $options['returnDocument']); - if (isset($options['codec'])) { - $replacement = $options['codec']->encodeIfSupported($replacement); - - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($replacement) || is_object($replacement)); - } + $replacement = $this->validateReplacement($replacement, $options['codec'] ?? null); $this->findAndModify = new FindAndModify( $databaseName, @@ -197,4 +170,34 @@ public function getCommandDocument() { return $this->findAndModify->getCommandDocument(); } + + /** + * @param array|object $replacement + * @return array|object + */ + private function validateReplacement($replacement, ?DocumentCodec $codec) + { + if (isset($codec)) { + $replacement = $codec->encode($replacement); + } + + if (! is_document($replacement)) { + throw InvalidArgumentException::expectedDocumentType('$replacement', $replacement); + } + + // Treat empty arrays as replacement documents for BC + if ($replacement === []) { + $replacement = (object) $replacement; + } + + if (is_first_key_operator($replacement)) { + throw new InvalidArgumentException('First key in $replacement is an update operator'); + } + + if (is_pipeline($replacement, true /* allowEmpty */)) { + throw new InvalidArgumentException('$replacement is an update pipeline'); + } + + return $replacement; + } } diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index 4ab3a02fd..42b3a344f 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -28,10 +28,7 @@ use MongoDB\InsertManyResult; use function array_is_list; -use function assert; -use function is_array; use function is_bool; -use function is_object; use function MongoDB\is_document; use function sprintf; @@ -76,28 +73,14 @@ class InsertMany implements Executable * * * writeConcern (MongoDB\Driver\WriteConcern): Write concern. * - * @param string $databaseName Database name - * @param string $collectionName Collection name - * @param array[]|object[] $documents List of documents to insert - * @param array $options Command options + * @param string $databaseName Database name + * @param string $collectionName Collection name + * @param list $documents List of documents to insert + * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors */ public function __construct(string $databaseName, string $collectionName, array $documents, array $options = []) { - if (empty($documents)) { - throw new InvalidArgumentException('$documents is empty'); - } - - if (! array_is_list($documents)) { - throw new InvalidArgumentException('$documents is not a list'); - } - - foreach ($documents as $i => $document) { - if (! is_document($document)) { - throw InvalidArgumentException::expectedDocumentType(sprintf('$documents[%d]', $i), $document); - } - } - $options += ['ordered' => true]; if (isset($options['bypassDocumentValidation']) && ! is_bool($options['bypassDocumentValidation'])) { @@ -130,7 +113,7 @@ public function __construct(string $databaseName, string $collectionName, array $this->databaseName = $databaseName; $this->collectionName = $collectionName; - $this->documents = $documents; + $this->documents = $this->validateDocuments($documents, $options['codec'] ?? null); $this->options = $options; } @@ -153,15 +136,6 @@ public function execute(Server $server) $insertedIds = []; foreach ($this->documents as $i => $document) { - if (isset($this->options['codec'])) { - $document = $this->options['codec']->encodeIfSupported($document); - - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($document) || is_object($document)); - } - $insertedIds[$i] = $bulk->insert($document); } @@ -207,4 +181,31 @@ private function createExecuteOptions(): array return $options; } + + /** + * @param list $documents + * @return list + */ + private function validateDocuments(array $documents, ?DocumentCodec $codec): array + { + if (empty($documents)) { + throw new InvalidArgumentException('$documents is empty'); + } + + if (! array_is_list($documents)) { + throw new InvalidArgumentException('$documents is not a list'); + } + + foreach ($documents as $i => $document) { + if ($codec) { + $document = $documents[$i] = $codec->encode($document); + } + + if (! is_document($document)) { + throw InvalidArgumentException::expectedDocumentType(sprintf('$documents[%d]', $i), $document); + } + } + + return $documents; + } } diff --git a/src/Operation/InsertOne.php b/src/Operation/InsertOne.php index 72b1c855d..f745664d7 100644 --- a/src/Operation/InsertOne.php +++ b/src/Operation/InsertOne.php @@ -27,10 +27,7 @@ use MongoDB\Exception\UnsupportedException; use MongoDB\InsertOneResult; -use function assert; -use function is_array; use function is_bool; -use function is_object; use function MongoDB\is_document; /** @@ -77,10 +74,6 @@ class InsertOne implements Executable */ public function __construct(string $databaseName, string $collectionName, $document, array $options = []) { - if (! is_document($document)) { - throw InvalidArgumentException::expectedDocumentType('$document', $document); - } - if (isset($options['bypassDocumentValidation']) && ! is_bool($options['bypassDocumentValidation'])) { throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean'); } @@ -105,18 +98,9 @@ public function __construct(string $databaseName, string $collectionName, $docum unset($options['writeConcern']); } - if (isset($options['codec'])) { - $document = $options['codec']->encodeIfSupported($document); - - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($document) || is_object($document)); - } - $this->databaseName = $databaseName; $this->collectionName = $collectionName; - $this->document = $document; + $this->document = $this->validateDocument($document, $options['codec'] ?? null); $this->options = $options; } @@ -181,4 +165,21 @@ private function createExecuteOptions(): array return $options; } + + /** + * @param array|object $document + * @return array|object + */ + private function validateDocument($document, ?DocumentCodec $codec) + { + if ($codec) { + $document = $codec->encode($document); + } + + if (! is_document($document)) { + throw InvalidArgumentException::expectedDocumentType('$document', $document); + } + + return $document; + } } diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index c3143b982..92f4d9efd 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -24,9 +24,6 @@ use MongoDB\Exception\UnsupportedException; use MongoDB\UpdateResult; -use function assert; -use function is_array; -use function is_object; use function MongoDB\is_document; use function MongoDB\is_first_key_operator; use function MongoDB\is_pipeline; @@ -86,23 +83,6 @@ class ReplaceOne implements Executable */ public function __construct(string $databaseName, string $collectionName, $filter, $replacement, array $options = []) { - if (! is_document($replacement)) { - throw InvalidArgumentException::expectedDocumentType('$replacement', $replacement); - } - - // Treat empty arrays as replacement documents for BC - if ($replacement === []) { - $replacement = (object) $replacement; - } - - if (is_first_key_operator($replacement)) { - throw new InvalidArgumentException('First key in $replacement is an update operator'); - } - - if (is_pipeline($replacement, true /* allowEmpty */)) { - throw new InvalidArgumentException('$replacement is an update pipeline'); - } - if (isset($options['codec']) && ! $options['codec'] instanceof DocumentCodec) { throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); } @@ -111,20 +91,13 @@ public function __construct(string $databaseName, string $collectionName, $filte if (isset($options['typeMap'])) { throw InvalidArgumentException::cannotCombineCodecAndTypeMap(); } - - $replacement = $options['codec']->encodeIfSupported($replacement); - - // Psalm's assert-if-true annotation does not work with unions, so - // assert the type manually instead of using is_document - // See https://github.com/vimeo/psalm/issues/6831 - assert(is_array($replacement) || is_object($replacement)); } $this->update = new Update( $databaseName, $collectionName, $filter, - $replacement, + $this->validateReplacement($replacement, $options['codec'] ?? null), ['multi' => false] + $options, ); } @@ -141,4 +114,34 @@ public function execute(Server $server) { return $this->update->execute($server); } + + /** + * @param array|object $replacement + * @return array|object + */ + private function validateReplacement($replacement, ?DocumentCodec $codec) + { + if ($codec) { + $replacement = $codec->encode($replacement); + } + + if (! is_document($replacement)) { + throw InvalidArgumentException::expectedDocumentType('$replacement', $replacement); + } + + // Treat empty arrays as replacement documents for BC + if ($replacement === []) { + $replacement = (object) $replacement; + } + + if (is_first_key_operator($replacement)) { + throw new InvalidArgumentException('First key in $replacement is an update operator'); + } + + if (is_pipeline($replacement, true /* allowEmpty */)) { + throw new InvalidArgumentException('$replacement is an update pipeline'); + } + + return $replacement; + } } diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index 9b4be93e1..47d6d80d4 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -260,7 +260,7 @@ public function testFindOneAndReplaceWithCodecAndTypemap(): void ]; $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); - $this->collection->findOneAndReplace(['_id' => 1], ['foo' => 'bar'], $options); + $this->collection->findOneAndReplace(['_id' => 1], TestObject::createForFixture(1), $options); } public static function provideFindOptions(): Generator diff --git a/tests/Operation/BulkWriteTest.php b/tests/Operation/BulkWriteTest.php index 8d35d2d74..a307f5b01 100644 --- a/tests/Operation/BulkWriteTest.php +++ b/tests/Operation/BulkWriteTest.php @@ -3,7 +3,9 @@ namespace MongoDB\Tests\Operation; use MongoDB\Exception\InvalidArgumentException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\Operation\BulkWrite; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; class BulkWriteTest extends TestCase { @@ -63,6 +65,18 @@ public function testInsertOneDocumentArgumentTypeCheck($document): void ]); } + public function testInsertOneWithCodecRejectsInvalidDocuments(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + + new BulkWrite( + $this->getDatabaseName(), + $this->getCollectionName(), + [[BulkWrite::INSERT_ONE => [['x' => 1]]]], + ['codec' => new TestDocumentCodec()], + ); + } + public function testDeleteManyFilterArgumentMissing(): void { $this->expectException(InvalidArgumentException::class); @@ -223,6 +237,18 @@ public function provideInvalidBooleanValues() return $this->wrapValuesForDataProvider($this->getInvalidBooleanValues()); } + public function testReplaceOneWithCodecRejectsInvalidDocuments(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + + new BulkWrite( + $this->getDatabaseName(), + $this->getCollectionName(), + [[BulkWrite::REPLACE_ONE => [['x' => 1], ['y' => 1]]]], + ['codec' => new TestDocumentCodec()], + ); + } + public function testUpdateManyFilterArgumentMissing(): void { $this->expectException(InvalidArgumentException::class); diff --git a/tests/Operation/InsertManyTest.php b/tests/Operation/InsertManyTest.php index a16710b75..26e62a7eb 100644 --- a/tests/Operation/InsertManyTest.php +++ b/tests/Operation/InsertManyTest.php @@ -3,7 +3,9 @@ namespace MongoDB\Tests\Operation; use MongoDB\Exception\InvalidArgumentException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\Operation\InsertMany; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; class InsertManyTest extends TestCase { @@ -46,4 +48,11 @@ public function provideInvalidConstructorOptions() 'writeConcern' => $this->getInvalidWriteConcernValues(), ]); } + + public function testCodecRejectsInvalidDocuments(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + + new InsertMany($this->getDatabaseName(), $this->getCollectionName(), [[]], ['codec' => new TestDocumentCodec()]); + } } diff --git a/tests/Operation/InsertOneTest.php b/tests/Operation/InsertOneTest.php index 868c0fe47..af0946d17 100644 --- a/tests/Operation/InsertOneTest.php +++ b/tests/Operation/InsertOneTest.php @@ -3,7 +3,9 @@ namespace MongoDB\Tests\Operation; use MongoDB\Exception\InvalidArgumentException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\Operation\InsertOne; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; class InsertOneTest extends TestCase { @@ -30,4 +32,11 @@ public function provideInvalidConstructorOptions() 'writeConcern' => $this->getInvalidWriteConcernValues(), ]); } + + public function testCodecRejectsInvalidDocuments(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + + new InsertOne($this->getDatabaseName(), $this->getCollectionName(), [], ['codec' => new TestDocumentCodec()]); + } } diff --git a/tests/Operation/ReplaceOneTest.php b/tests/Operation/ReplaceOneTest.php index 0de93f271..404cdae78 100644 --- a/tests/Operation/ReplaceOneTest.php +++ b/tests/Operation/ReplaceOneTest.php @@ -3,7 +3,9 @@ namespace MongoDB\Tests\Operation; use MongoDB\Exception\InvalidArgumentException; +use MongoDB\Exception\UnsupportedValueException; use MongoDB\Operation\ReplaceOne; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; class ReplaceOneTest extends TestCase { @@ -62,4 +64,11 @@ public function provideInvalidConstructorOptions() 'codec' => $this->getInvalidDocumentCodecValues(), ]); } + + public function testCodecRejectsInvalidDocuments(): void + { + $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); + + new ReplaceOne($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], ['y' => 1], ['codec' => new TestDocumentCodec()]); + } } From 4eceaf6601108b643e8de219825510a636e6ef60 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 4 Aug 2023 09:14:18 +0200 Subject: [PATCH 26/37] Use more descriptive value in failing tests --- tests/Operation/InsertManyTest.php | 2 +- tests/Operation/InsertOneTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Operation/InsertManyTest.php b/tests/Operation/InsertManyTest.php index 26e62a7eb..ece556cae 100644 --- a/tests/Operation/InsertManyTest.php +++ b/tests/Operation/InsertManyTest.php @@ -53,6 +53,6 @@ public function testCodecRejectsInvalidDocuments(): void { $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); - new InsertMany($this->getDatabaseName(), $this->getCollectionName(), [[]], ['codec' => new TestDocumentCodec()]); + new InsertMany($this->getDatabaseName(), $this->getCollectionName(), [['x' => 1]], ['codec' => new TestDocumentCodec()]); } } diff --git a/tests/Operation/InsertOneTest.php b/tests/Operation/InsertOneTest.php index af0946d17..ad7212c7c 100644 --- a/tests/Operation/InsertOneTest.php +++ b/tests/Operation/InsertOneTest.php @@ -37,6 +37,6 @@ public function testCodecRejectsInvalidDocuments(): void { $this->expectExceptionObject(UnsupportedValueException::invalidEncodableValue([])); - new InsertOne($this->getDatabaseName(), $this->getCollectionName(), [], ['codec' => new TestDocumentCodec()]); + new InsertOne($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], ['codec' => new TestDocumentCodec()]); } } From 1cb170c038f6b3dacedbed3867169a0a768a2c3c Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 4 Aug 2023 09:14:37 +0200 Subject: [PATCH 27/37] Add clarifying comment on skipping validation --- src/Operation/BulkWrite.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index e42f2fdc2..4457d9930 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -294,6 +294,8 @@ private function validateOperations(array $operations, ?DocumentCodec $codec): a switch ($type) { 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) { $operations[$i][$type][0] = $codec->encode($args[0]); } From 56fc1efeee1182b2c3272c662a8a22ae8fa7cab4 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 4 Aug 2023 09:14:43 +0200 Subject: [PATCH 28/37] Simplify double isset check --- src/Operation/ReplaceOne.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Operation/ReplaceOne.php b/src/Operation/ReplaceOne.php index 92f4d9efd..943206761 100644 --- a/src/Operation/ReplaceOne.php +++ b/src/Operation/ReplaceOne.php @@ -87,10 +87,8 @@ public function __construct(string $databaseName, string $collectionName, $filte throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); } - if (isset($options['codec'])) { - if (isset($options['typeMap'])) { - throw InvalidArgumentException::cannotCombineCodecAndTypeMap(); - } + if (isset($options['codec'], $options['typeMap'])) { + throw InvalidArgumentException::cannotCombineCodecAndTypeMap(); } $this->update = new Update( From 003f73b9735077241aa46a117d508bd6465c59ab Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 4 Aug 2023 09:18:06 +0200 Subject: [PATCH 29/37] Split chained calls when inheriting options --- src/Collection.php | 80 +++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 48 deletions(-) diff --git a/src/Collection.php b/src/Collection.php index 427308d0d..f8b696dec 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -258,9 +258,8 @@ public function aggregate(array $pipeline, array $options = []) */ public function bulkWrite(array $operations, array $options = []) { - $options = $this->inheritWriteOptions( - $this->inheritCodec($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritCodec($options); $operation = new BulkWrite($this->databaseName, $this->collectionName, $operations, $options); @@ -426,9 +425,8 @@ public function deleteOne($filter, array $options = []) */ public function distinct(string $fieldName, $filter = [], array $options = []) { - $options = $this->inheritReadOptions( - $this->inheritTypeMap($options), - ); + $options = $this->inheritReadOptions($options); + $options = $this->inheritTypeMap($options); $operation = new Distinct($this->databaseName, $this->collectionName, $fieldName, $filter, $options); @@ -449,9 +447,8 @@ public function drop(array $options = []) { $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions( - $this->inheritTypeMap($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritTypeMap($options); if (! isset($options['encryptedFields'])) { $options['encryptedFields'] = get_encrypted_fields_from_driver($this->databaseName, $this->collectionName, $this->manager) @@ -484,9 +481,8 @@ public function dropIndex($indexName, array $options = []) throw new InvalidArgumentException('dropIndexes() must be used to drop multiple indexes'); } - $options = $this->inheritWriteOptions( - $this->inheritTypeMap($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritTypeMap($options); $operation = new DropIndexes($this->databaseName, $this->collectionName, $indexName, $options); @@ -505,9 +501,8 @@ public function dropIndex($indexName, array $options = []) */ public function dropIndexes(array $options = []) { - $options = $this->inheritWriteOptions( - $this->inheritTypeMap($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritTypeMap($options); $operation = new DropIndexes($this->databaseName, $this->collectionName, '*', $options); @@ -573,9 +568,8 @@ public function explain(Explainable $explainable, array $options = []) */ public function find($filter = [], array $options = []) { - $options = $this->inheritReadOptions( - $this->inheritCodecOrTypeMap($options), - ); + $options = $this->inheritReadOptions($options); + $options = $this->inheritCodecOrTypeMap($options); $operation = new Find($this->databaseName, $this->collectionName, $filter, $options); @@ -596,9 +590,8 @@ public function find($filter = [], array $options = []) */ public function findOne($filter = [], array $options = []) { - $options = $this->inheritReadOptions( - $this->inheritCodecOrTypeMap($options), - ); + $options = $this->inheritReadOptions($options); + $options = $this->inheritCodecOrTypeMap($options); $operation = new FindOne($this->databaseName, $this->collectionName, $filter, $options); @@ -622,9 +615,8 @@ public function findOne($filter = [], array $options = []) */ public function findOneAndDelete($filter, array $options = []) { - $options = $this->inheritWriteOptions( - $this->inheritCodecOrTypeMap($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritCodecOrTypeMap($options); $operation = new FindOneAndDelete($this->databaseName, $this->collectionName, $filter, $options); @@ -653,9 +645,8 @@ public function findOneAndDelete($filter, array $options = []) */ public function findOneAndReplace($filter, $replacement, array $options = []) { - $options = $this->inheritWriteOptions( - $this->inheritCodecOrTypeMap($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritCodecOrTypeMap($options); $operation = new FindOneAndReplace($this->databaseName, $this->collectionName, $filter, $replacement, $options); @@ -684,9 +675,8 @@ public function findOneAndReplace($filter, $replacement, array $options = []) */ public function findOneAndUpdate($filter, $update, array $options = []) { - $options = $this->inheritWriteOptions( - $this->inheritCodecOrTypeMap($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritCodecOrTypeMap($options); $operation = new FindOneAndUpdate($this->databaseName, $this->collectionName, $filter, $update, $options); @@ -789,9 +779,8 @@ public function getWriteConcern() */ public function insertMany(array $documents, array $options = []) { - $options = $this->inheritWriteOptions( - $this->inheritCodec($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritCodec($options); $operation = new InsertMany($this->databaseName, $this->collectionName, $documents, $options); @@ -811,9 +800,8 @@ public function insertMany(array $documents, array $options = []) */ public function insertOne($document, array $options = []) { - $options = $this->inheritWriteOptions( - $this->inheritCodec($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritCodec($options); $operation = new InsertOne($this->databaseName, $this->collectionName, $document, $options); @@ -872,9 +860,8 @@ public function mapReduce(JavascriptInterface $map, JavascriptInterface $reduce, $options['readConcern'] = $this->readConcern; } - $options = $this->inheritWriteOptions( - $this->inheritTypeMap($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritTypeMap($options); $operation = new MapReduce($this->databaseName, $this->collectionName, $map, $reduce, $out, $options); @@ -899,9 +886,8 @@ public function rename(string $toCollectionName, ?string $toDatabaseName = null, $toDatabaseName = $this->databaseName; } - $options = $this->inheritWriteOptions( - $this->inheritTypeMap($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritTypeMap($options); $operation = new RenameCollection($this->databaseName, $this->collectionName, $toDatabaseName, $toCollectionName, $options); @@ -923,9 +909,8 @@ public function rename(string $toCollectionName, ?string $toDatabaseName = null, */ public function replaceOne($filter, $replacement, array $options = []) { - $options = $this->inheritWriteOptions( - $this->inheritCodec($options), - ); + $options = $this->inheritWriteOptions($options); + $options = $this->inheritCodec($options); $operation = new ReplaceOne($this->databaseName, $this->collectionName, $filter, $replacement, $options); @@ -987,9 +972,8 @@ public function updateOne($filter, $update, array $options = []) */ public function watch(array $pipeline = [], array $options = []) { - $options = $this->inheritReadOptions( - $this->inheritCodecOrTypeMap($options), - ); + $options = $this->inheritReadOptions($options); + $options = $this->inheritCodecOrTypeMap($options); $operation = new Watch($this->manager, $this->databaseName, $this->collectionName, $pipeline, $options); From 66b9308951446879b3befe5c4cfe0f8d68c2fe3e Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 4 Aug 2023 09:18:47 +0200 Subject: [PATCH 30/37] Defer server selection in Collection::drop() --- src/Collection.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Collection.php b/src/Collection.php index f8b696dec..b746ea235 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -445,11 +445,11 @@ public function distinct(string $fieldName, $filter = [], array $options = []) */ public function drop(array $options = []) { - $server = select_server($this->manager, $options); - $options = $this->inheritWriteOptions($options); $options = $this->inheritTypeMap($options); + $server = select_server($this->manager, $options); + if (! isset($options['encryptedFields'])) { $options['encryptedFields'] = get_encrypted_fields_from_driver($this->databaseName, $this->collectionName, $this->manager) ?? get_encrypted_fields_from_server($this->databaseName, $this->collectionName, $this->manager, $server); From 69c32c846ec00eab4fa5681a9535a41dcc14b3c1 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 4 Aug 2023 09:25:30 +0200 Subject: [PATCH 31/37] Split inheritReadOptions method --- src/Collection.php | 58 +++++++++++++++++++++------------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/src/Collection.php b/src/Collection.php index b746ea235..b5a672308 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -213,9 +213,7 @@ public function aggregate(array $pipeline, array $options = []) { $hasWriteStage = is_last_pipeline_operator_write($pipeline); - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } + $options = $this->inheritReadPreference($options); $server = $hasWriteStage ? select_server_for_aggregate_write_stage($this->manager, $options) @@ -223,15 +221,9 @@ public function aggregate(array $pipeline, array $options = []) /* MongoDB 4.2 and later supports a read concern when an $out stage is * being used, but earlier versions do not. - * - * A read concern is also not compatible with transactions. */ - if ( - ! isset($options['readConcern']) && - ! is_in_transaction($options) && - ( ! $hasWriteStage || server_supports_feature($server, self::WIRE_VERSION_FOR_READ_CONCERN_WITH_WRITE_STAGE)) - ) { - $options['readConcern'] = $this->readConcern; + if (! $hasWriteStage || server_supports_feature($server, self::WIRE_VERSION_FOR_READ_CONCERN_WITH_WRITE_STAGE)) { + $options = $this->inheritReadConcern($options); } $options = $this->inheritCodecOrTypeMap($options); @@ -543,10 +535,7 @@ public function estimatedDocumentCount(array $options = []) */ public function explain(Explainable $explainable, array $options = []) { - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } - + $options = $this->inheritReadPreference($options); $options = $this->inheritTypeMap($options); $operation = new Explain($this->databaseName, $explainable, $options); @@ -842,22 +831,18 @@ public function mapReduce(JavascriptInterface $map, JavascriptInterface $reduce, { $hasOutputCollection = ! is_mapreduce_output_inline($out); - if (! isset($options['readPreference']) && ! is_in_transaction($options)) { - $options['readPreference'] = $this->readPreference; - } - // Check if the out option is inline because we will want to coerce a primary read preference if not if ($hasOutputCollection) { $options['readPreference'] = new ReadPreference(ReadPreference::PRIMARY); + } else { + $options = $this->inheritReadPreference($options); } /* A "majority" read concern is not compatible with inline output, so * avoid providing the Collection's read concern if it would conflict. - * - * A read concern is also not compatible with transactions. */ - if (! isset($options['readConcern']) && ! ($hasOutputCollection && $this->readConcern->getLevel() === ReadConcern::MAJORITY) && ! is_in_transaction($options)) { - $options['readConcern'] = $this->readConcern; + if (! $hasOutputCollection || $this->readConcern->getLevel() !== ReadConcern::MAJORITY) { + $options = $this->inheritReadConcern($options); } $options = $this->inheritWriteOptions($options); @@ -1035,17 +1020,28 @@ private function inheritCodecOrTypeMap(array $options): array return $options; } - private function inheritReadOptions(array $options): array + private function inheritReadConcern(array $options): array { // ReadConcern and ReadPreference may not change within a transaction - if (! is_in_transaction($options)) { - if (! isset($options['readConcern'])) { - $options['readConcern'] = $this->readConcern; - } + if (! isset($options['readConcern']) && ! is_in_transaction($options)) { + $options['readConcern'] = $this->readConcern; + } - if (! isset($options['readPreference'])) { - $options['readPreference'] = $this->readPreference; - } + return $options; + } + + private function inheritReadOptions(array $options): array + { + $options = $this->inheritReadConcern($options); + + return $this->inheritReadPreference($options); + } + + private function inheritReadPreference(array $options): array + { + // ReadConcern and ReadPreference may not change within a transaction + if (! isset($options['readPreference']) && ! is_in_transaction($options)) { + $options['readPreference'] = $this->readPreference; } return $options; From 6f8cdeae22e2ca3f7c7b8ddb3cb1d0396062b16e Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Fri, 4 Aug 2023 17:48:50 +0200 Subject: [PATCH 32/37] Use decode() instead of decodeIfSupported() --- psalm-baseline.xml | 3 ++- src/ChangeStream.php | 6 +++++- src/Operation/FindAndModify.php | 5 +---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 56eb17f4d..a86adc001 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -441,10 +441,11 @@ array|object|null - decodeIfSupported + decode isInTransaction + options['codec']->decode($result->get('value'))]]> value ?? null) : null]]> value ?? null) : null]]> diff --git a/src/ChangeStream.php b/src/ChangeStream.php index f54e012d2..8193a35eb 100644 --- a/src/ChangeStream.php +++ b/src/ChangeStream.php @@ -18,6 +18,7 @@ namespace MongoDB; use Iterator; +use MongoDB\BSON\Document; use MongoDB\Codec\DocumentCodec; use MongoDB\Driver\CursorId; use MongoDB\Driver\Exception\ConnectionException; @@ -28,6 +29,7 @@ use MongoDB\Model\ChangeStreamIterator; use ReturnTypeWillChange; +use function assert; use function call_user_func; use function in_array; @@ -114,7 +116,9 @@ public function current() return $value; } - return $this->codec->decodeIfSupported($value); + assert($value instanceof Document); + + return $this->codec->decode($value); } /** @return CursorId */ diff --git a/src/Operation/FindAndModify.php b/src/Operation/FindAndModify.php index c1711cdc4..7d857b48b 100644 --- a/src/Operation/FindAndModify.php +++ b/src/Operation/FindAndModify.php @@ -262,10 +262,7 @@ public function execute(Server $server) $result = current($cursor->toArray()); assert($result instanceof Document); - $decoded = $this->options['codec']->decodeIfSupported($result->get('value')); - assert($decoded === null || is_object($decoded)); - - return $decoded; + return $this->options['codec']->decode($result->get('value')); } if (isset($this->options['typeMap'])) { From ed56c09fb0f54b2de8df4afae0b188d17f7ae4a6 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 16 Aug 2023 13:08:12 +0200 Subject: [PATCH 33/37] Add explanatory comment for ID filling behaviour --- tests/Collection/CodecCollectionFunctionalTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index 47d6d80d4..f8b01addd 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -395,6 +395,8 @@ public function testInsertMany($expected, $options): void $result = $this->collection->insertMany($documents, $options); $this->assertSame(3, $result->getInsertedCount()); + // Add missing identifiers. This is relevant for the "No codec" data set, as the encoded document will not have + // an "_id" field and the driver will automatically generate one. foreach ($expected as $index => &$expectedDocument) { if ($expectedDocument instanceof BSONDocument && $expectedDocument->_id === null) { $expectedDocument->_id = $result->getInsertedIds()[$index]; @@ -434,6 +436,8 @@ public function testInsertOne($expected, $options): void $result = $this->collection->insertOne(TestObject::createForFixture(1), $options); $this->assertSame(1, $result->getInsertedCount()); + // Add missing identifiers. This is relevant for the "No codec" data set, as the encoded document will not have + // an "_id" field and the driver will automatically generate one. if ($expected instanceof BSONDocument && $expected->_id === null) { $expected->_id = $result->getInsertedId(); } From e3d68a79ea8ad17a433e4e6f2d43eb432c9cda71 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 22 Aug 2023 14:47:42 +0200 Subject: [PATCH 34/37] Handle null values in findAndModify responses --- psalm-baseline.xml | 4 +- src/Operation/FindAndModify.php | 4 +- .../Operation/FindAndModifyFunctionalTest.php | 42 +++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index a86adc001..17571a178 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -436,6 +436,7 @@ + $value array|object|null @@ -445,7 +446,8 @@ isInTransaction - options['codec']->decode($result->get('value'))]]> + options['codec']->decode($value)]]> + options['codec']->decode($value)]]> value ?? null) : null]]> value ?? null) : null]]> diff --git a/src/Operation/FindAndModify.php b/src/Operation/FindAndModify.php index 7d857b48b..300f4e97a 100644 --- a/src/Operation/FindAndModify.php +++ b/src/Operation/FindAndModify.php @@ -262,7 +262,9 @@ public function execute(Server $server) $result = current($cursor->toArray()); assert($result instanceof Document); - return $this->options['codec']->decode($result->get('value')); + $value = $result->get('value'); + + return $value === null ? $value : $this->options['codec']->decode($value); } if (isset($this->options['typeMap'])) { diff --git a/tests/Operation/FindAndModifyFunctionalTest.php b/tests/Operation/FindAndModifyFunctionalTest.php index 5683c8990..05e819132 100644 --- a/tests/Operation/FindAndModifyFunctionalTest.php +++ b/tests/Operation/FindAndModifyFunctionalTest.php @@ -296,6 +296,20 @@ public function testFindOneAndDeleteWithCodec(): void self::assertEquals(TestObject::createDecodedForFixture(1), $result); } + public function testFindOneAndDeleteNothingWithCodec(): void + { + // When the query does not match any documents, the operation returns null + $operation = new FindAndModify( + $this->getDatabaseName(), + $this->getCollectionName(), + ['remove' => true, 'codec' => new TestDocumentCodec()], + ); + + $result = $operation->execute($this->getPrimaryServer()); + + self::assertNull($result); + } + public function testFindOneAndUpdateWithCodec(): void { $this->createFixtures(1); @@ -311,6 +325,20 @@ public function testFindOneAndUpdateWithCodec(): void self::assertEquals(TestObject::createDecodedForFixture(1), $result); } + public function testFindOneAndUpdateNothingWithCodec(): void + { + // When the query does not match any documents, the operation returns null + $operation = new FindAndModify( + $this->getDatabaseName(), + $this->getCollectionName(), + ['update' => ['$set' => ['x.foo' => 'baz']], 'codec' => new TestDocumentCodec()], + ); + + $result = $operation->execute($this->getPrimaryServer()); + + self::assertNull($result); + } + public function testFindOneAndReplaceWithCodec(): void { $this->createFixtures(1); @@ -326,6 +354,20 @@ public function testFindOneAndReplaceWithCodec(): void self::assertEquals(TestObject::createDecodedForFixture(1), $result); } + public function testFindOneAndReplaceNothingWithCodec(): void + { + // When the query does not match any documents, the operation returns null + $operation = new FindAndModify( + $this->getDatabaseName(), + $this->getCollectionName(), + ['update' => ['_id' => 1], 'codec' => new TestDocumentCodec()], + ); + + $result = $operation->execute($this->getPrimaryServer()); + + self::assertNull($result); + } + /** * Create data fixtures. */ From 0ed7f67932b42c8d1e7d92bd2af47e0ad89b9ff8 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Tue, 22 Aug 2023 14:49:26 +0200 Subject: [PATCH 35/37] Update comment regarding missing identifiers in tests --- tests/Collection/CodecCollectionFunctionalTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Collection/CodecCollectionFunctionalTest.php b/tests/Collection/CodecCollectionFunctionalTest.php index f8b01addd..2eb0c09dd 100644 --- a/tests/Collection/CodecCollectionFunctionalTest.php +++ b/tests/Collection/CodecCollectionFunctionalTest.php @@ -436,8 +436,8 @@ public function testInsertOne($expected, $options): void $result = $this->collection->insertOne(TestObject::createForFixture(1), $options); $this->assertSame(1, $result->getInsertedCount()); - // Add missing identifiers. This is relevant for the "No codec" data set, as the encoded document will not have - // an "_id" field and the driver will automatically generate one. + // Add missing identifiers. This is relevant for the "No codec" data set, as the encoded document will have an + // automatically generated identifier, which needs to be used in the expected document. if ($expected instanceof BSONDocument && $expected->_id === null) { $expected->_id = $result->getInsertedId(); } From 9cdb66af5758fd661b6e6a845aded3400c11fae6 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 23 Aug 2023 09:10:34 +0200 Subject: [PATCH 36/37] Use intersection type for change streams --- src/Model/ChangeStreamIterator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Model/ChangeStreamIterator.php b/src/Model/ChangeStreamIterator.php index d01ba7815..b41787502 100644 --- a/src/Model/ChangeStreamIterator.php +++ b/src/Model/ChangeStreamIterator.php @@ -49,7 +49,7 @@ * * @internal * @template TValue of array|object - * @template-extends IteratorIterator> + * @template-extends IteratorIterator&Iterator> */ class ChangeStreamIterator extends IteratorIterator implements CommandSubscriber { From f53211981bb14ced6a351acbf6b5d9b787393770 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Wed, 23 Aug 2023 09:25:57 +0200 Subject: [PATCH 37/37] Prohibit specifying codec and typeMap options to Watch --- src/Operation/Watch.php | 4 ++++ tests/Operation/WatchTest.php | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/src/Operation/Watch.php b/src/Operation/Watch.php index 2caab579c..075c86d70 100644 --- a/src/Operation/Watch.php +++ b/src/Operation/Watch.php @@ -210,6 +210,10 @@ public function __construct(Manager $manager, ?string $databaseName, ?string $co throw InvalidArgumentException::invalidType('"codec" option', $options['codec'], DocumentCodec::class); } + if (isset($options['codec']) && isset($options['typeMap'])) { + throw InvalidArgumentException::cannotCombineCodecAndTypeMap(); + } + if (array_key_exists('fullDocument', $options) && ! is_string($options['fullDocument'])) { throw InvalidArgumentException::invalidType('"fullDocument" option', $options['fullDocument'], 'string'); } diff --git a/tests/Operation/WatchTest.php b/tests/Operation/WatchTest.php index 8b4f3e0d6..fd97a1c5e 100644 --- a/tests/Operation/WatchTest.php +++ b/tests/Operation/WatchTest.php @@ -4,6 +4,7 @@ use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Watch; +use MongoDB\Tests\Fixtures\Codec\TestDocumentCodec; use stdClass; /** @@ -57,6 +58,14 @@ public function provideInvalidConstructorOptions() ]); } + public function testConstructorRejectsCodecAndTypemap(): void + { + $this->expectExceptionObject(InvalidArgumentException::cannotCombineCodecAndTypeMap()); + + $options = ['codec' => new TestDocumentCodec(), 'typeMap' => ['root' => 'array']]; + new Watch($this->manager, $this->getDatabaseName(), $this->getCollectionName(), [], $options); + } + private function getInvalidTimestampValues() { return [123, 3.14, 'foo', true, [], new stdClass()];