From 9bb6ae19a886d2ad61e097e13adcfe72c14bb9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 12 Jun 2023 10:45:22 +0200 Subject: [PATCH 1/3] PHPLIB-722: Ignore writeConcern option for read-only aggregations --- src/Operation/Aggregate.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Operation/Aggregate.php b/src/Operation/Aggregate.php index 4d6c5867c..b1306c25d 100644 --- a/src/Operation/Aggregate.php +++ b/src/Operation/Aggregate.php @@ -228,10 +228,12 @@ public function __construct(string $databaseName, ?string $collectionName, array unset($options['batchSize']); } - /* Ignore batchSize for writes, since no documents are returned and a - * batchSize of zero could prevent the pipeline from executing. */ if ($this->isWrite) { + /* Ignore batchSize for writes, since no documents are returned and + * a batchSize of zero could prevent the pipeline from executing. */ unset($options['batchSize']); + } else { + unset($options['writeConcern']); } $this->databaseName = $databaseName; @@ -365,16 +367,12 @@ private function executeCommand(Server $server, Command $command): Cursor { $options = []; - foreach (['readConcern', 'readPreference', 'session'] as $option) { + foreach (['readConcern', 'readPreference', 'session', 'writeConcern'] as $option) { if (isset($this->options[$option])) { $options[$option] = $this->options[$option]; } } - if ($this->isWrite && isset($this->options['writeConcern'])) { - $options['writeConcern'] = $this->options['writeConcern']; - } - if (! $this->isWrite) { return $server->executeReadCommand($this->databaseName, $command, $options); } From 52340dee0b0e3264c7a406d54caa08beaef042da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 21 Jun 2023 13:25:16 +0200 Subject: [PATCH 2/3] Test read-write aggregation pipeline --- tests/Collection/CollectionFunctionalTest.php | 90 +++++++++++-------- 1 file changed, 54 insertions(+), 36 deletions(-) diff --git a/tests/Collection/CollectionFunctionalTest.php b/tests/Collection/CollectionFunctionalTest.php index 24c5903b9..551cdc271 100644 --- a/tests/Collection/CollectionFunctionalTest.php +++ b/tests/Collection/CollectionFunctionalTest.php @@ -7,6 +7,7 @@ use MongoDB\Collection; use MongoDB\Database; use MongoDB\Driver\BulkWrite; +use MongoDB\Driver\Exception\CommandException; use MongoDB\Driver\ReadConcern; use MongoDB\Driver\ReadPreference; use MongoDB\Driver\WriteConcern; @@ -21,7 +22,7 @@ use function call_user_func; use function is_scalar; use function json_encode; -use function strchr; +use function str_contains; use function usort; use function version_compare; @@ -447,16 +448,28 @@ public function testMapReduce(): void public function collectionMethodClosures() { return [ - [ + 'read-only aggregate' => [ function ($collection, $session, $options = []): void { $collection->aggregate( [['$match' => ['_id' => ['$lt' => 3]]]], ['session' => $session] + $options ); + }, 'r', + ], + + 'read-write aggregate' => [ + function ($collection, $session, $options = []): void { + $collection->aggregate( + [ + ['$match' => ['_id' => ['$lt' => 3]]], + ['$merge' => $collection . '_out'], + ], + ['session' => $session] + $options + ); }, 'rw', ], - [ + 'bulkWrite insertOne' => [ function ($collection, $session, $options = []): void { $collection->bulkWrite( [['insertOne' => [['test' => 'foo']]]], @@ -466,7 +479,7 @@ function ($collection, $session, $options = []): void { ], /* Disabled, as count command can't be used in transactions - [ + 'count' => [ function($collection, $session, $options = []) { $collection->count( [], @@ -476,7 +489,7 @@ function($collection, $session, $options = []) { ], */ - [ + 'countDocuments' => [ function ($collection, $session, $options = []): void { $collection->countDocuments( [], @@ -486,7 +499,7 @@ function ($collection, $session, $options = []): void { ], /* Disabled, as it's illegal to use createIndex command in transactions - [ + 'createIndex' => [ function($collection, $session, $options = []) { $collection->createIndex( ['test' => 1], @@ -496,7 +509,7 @@ function($collection, $session, $options = []) { ], */ - [ + 'deleteMany' => [ function ($collection, $session, $options = []): void { $collection->deleteMany( ['test' => 'foo'], @@ -505,7 +518,7 @@ function ($collection, $session, $options = []): void { }, 'w', ], - [ + 'deleteOne' => [ function ($collection, $session, $options = []): void { $collection->deleteOne( ['test' => 'foo'], @@ -514,7 +527,7 @@ function ($collection, $session, $options = []): void { }, 'w', ], - [ + 'distinct' => [ function ($collection, $session, $options = []): void { $collection->distinct( '_id', @@ -525,7 +538,7 @@ function ($collection, $session, $options = []): void { ], /* Disabled, as it's illegal to use drop command in transactions - [ + 'drop' => [ function($collection, $session, $options = []) { $collection->drop( ['session' => $session] + $options @@ -535,7 +548,7 @@ function($collection, $session, $options = []) { */ /* Disabled, as it's illegal to use dropIndexes command in transactions - [ + 'dropIndex' => [ function($collection, $session, $options = []) { $collection->dropIndex( '_id_1', @@ -545,7 +558,7 @@ function($collection, $session, $options = []) { ], */ /* Disabled, as it's illegal to use dropIndexes command in transactions - [ + 'dropIndexes' => [ function($collection, $session, $options = []) { $collection->dropIndexes( ['session' => $session] + $options @@ -555,7 +568,7 @@ function($collection, $session, $options = []) { */ /* Disabled, as count command can't be used in transactions - [ + 'estimatedDocumentCount' => [ function($collection, $session, $options = []) { $collection->estimatedDocumentCount( ['session' => $session] + $options @@ -564,7 +577,7 @@ function($collection, $session, $options = []) { ], */ - [ + 'find' => [ function ($collection, $session, $options = []): void { $collection->find( ['test' => 'foo'], @@ -573,7 +586,7 @@ function ($collection, $session, $options = []): void { }, 'r', ], - [ + 'findOne' => [ function ($collection, $session, $options = []): void { $collection->findOne( ['test' => 'foo'], @@ -582,7 +595,7 @@ function ($collection, $session, $options = []): void { }, 'r', ], - [ + 'findOneAndDelete' => [ function ($collection, $session, $options = []): void { $collection->findOneAndDelete( ['test' => 'foo'], @@ -591,7 +604,7 @@ function ($collection, $session, $options = []): void { }, 'w', ], - [ + 'findOneAndReplace' => [ function ($collection, $session, $options = []): void { $collection->findOneAndReplace( ['test' => 'foo'], @@ -601,7 +614,7 @@ function ($collection, $session, $options = []): void { }, 'w', ], - [ + 'findOneAndUpdate' => [ function ($collection, $session, $options = []): void { $collection->findOneAndUpdate( ['test' => 'foo'], @@ -611,7 +624,7 @@ function ($collection, $session, $options = []): void { }, 'w', ], - [ + 'insertMany' => [ function ($collection, $session, $options = []): void { $collection->insertMany( [ @@ -623,7 +636,7 @@ function ($collection, $session, $options = []): void { }, 'w', ], - [ + 'insertOne' => [ function ($collection, $session, $options = []): void { $collection->insertOne( ['test' => 'foo'], @@ -633,7 +646,7 @@ function ($collection, $session, $options = []): void { ], /* Disabled, as it's illegal to use listIndexes command in transactions - [ + 'listIndexes' => [ function($collection, $session, $options = []) { $collection->listIndexes( ['session' => $session] + $options @@ -643,7 +656,7 @@ function($collection, $session, $options = []) { */ /* Disabled, as it's illegal to use mapReduce command in transactions - [ + 'mapReduce' => [ function($collection, $session, $options = []) { $collection->mapReduce( new \MongoDB\BSON\Javascript('function() { emit(this.state, this.pop); }'), @@ -655,7 +668,7 @@ function($collection, $session, $options = []) { ], */ - [ + 'replaceOne' => [ function ($collection, $session, $options = []): void { $collection->replaceOne( ['test' => 'foo'], @@ -665,7 +678,7 @@ function ($collection, $session, $options = []): void { }, 'w', ], - [ + 'updateMany' => [ function ($collection, $session, $options = []): void { $collection->updateMany( ['test' => 'foo'], @@ -675,7 +688,7 @@ function ($collection, $session, $options = []): void { }, 'w', ], - [ + 'updateOne' => [ function ($collection, $session, $options = []): void { $collection->updateOne( ['test' => 'foo'], @@ -686,7 +699,7 @@ function ($collection, $session, $options = []): void { ], /* Disabled, as it's illegal to use change streams in transactions - [ + 'watch' => [ function($collection, $session, $options = []) { $collection->watch( [], @@ -698,32 +711,28 @@ function($collection, $session, $options = []) { ]; } - public function collectionReadMethodClosures() + public function collectionReadMethodClosures(): array { return array_filter( $this->collectionMethodClosures(), function ($rw) { - if (strchr($rw[1], 'r') !== false) { - return true; - } + return str_contains($rw[1], 'r'); } ); } - public function collectionWriteMethodClosures() + public function collectionWriteMethodClosures(): array { return array_filter( $this->collectionMethodClosures(), function ($rw) { - if (strchr($rw[1], 'w') !== false) { - return true; - } + return str_contains($rw[1], 'w'); } ); } /** @dataProvider collectionMethodClosures */ - public function testMethodDoesNotInheritReadWriteConcernInTranasaction(Closure $method): void + public function testMethodDoesNotInheritReadWriteConcernInTransaction(Closure $method): void { $this->skipIfTransactionsAreNotSupported(); @@ -739,7 +748,16 @@ public function testMethodDoesNotInheritReadWriteConcernInTranasaction(Closure $ (new CommandObserver())->observe( function () use ($method, $collection, $session): void { - call_user_func($method, $collection, $session); + try { + call_user_func($method, $collection, $session); + } catch (CommandException $exception) { + // Write aggregates are not supported in transactions + if ($exception->getResultDocument()->codeName === 'OperationNotSupportedInTransaction') { + $this->markTestSkipped('OperationNotSupportedInTransaction: ' . $exception->getMessage()); + } + + throw $exception; + } }, function (array $event): void { $this->assertObjectNotHasAttribute('writeConcern', $event['started']->getCommand()); From 28cc1f5f01108822364c0dc7ae0534ab0a7e2d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 26 Jun 2023 20:12:19 +0200 Subject: [PATCH 3/3] Remove rw aggregation not supported in a transaction --- tests/Collection/CollectionFunctionalTest.php | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/tests/Collection/CollectionFunctionalTest.php b/tests/Collection/CollectionFunctionalTest.php index 551cdc271..8fc1f68fb 100644 --- a/tests/Collection/CollectionFunctionalTest.php +++ b/tests/Collection/CollectionFunctionalTest.php @@ -7,7 +7,6 @@ use MongoDB\Collection; use MongoDB\Database; use MongoDB\Driver\BulkWrite; -use MongoDB\Driver\Exception\CommandException; use MongoDB\Driver\ReadConcern; use MongoDB\Driver\ReadPreference; use MongoDB\Driver\WriteConcern; @@ -457,6 +456,7 @@ function ($collection, $session, $options = []): void { }, 'r', ], + /* Disabled, as write aggregations are not supported in transactions 'read-write aggregate' => [ function ($collection, $session, $options = []): void { $collection->aggregate( @@ -468,6 +468,7 @@ function ($collection, $session, $options = []): void { ); }, 'rw', ], + */ 'bulkWrite insertOne' => [ function ($collection, $session, $options = []): void { @@ -748,16 +749,7 @@ public function testMethodDoesNotInheritReadWriteConcernInTransaction(Closure $m (new CommandObserver())->observe( function () use ($method, $collection, $session): void { - try { - call_user_func($method, $collection, $session); - } catch (CommandException $exception) { - // Write aggregates are not supported in transactions - if ($exception->getResultDocument()->codeName === 'OperationNotSupportedInTransaction') { - $this->markTestSkipped('OperationNotSupportedInTransaction: ' . $exception->getMessage()); - } - - throw $exception; - } + call_user_func($method, $collection, $session); }, function (array $event): void { $this->assertObjectNotHasAttribute('writeConcern', $event['started']->getCommand());