Skip to content

Commit 33e23ae

Browse files
GromNaNGromNaN
authored
PHPLIB-722: Ignore writeConcern option for read-only aggregations (#1100)
Improve test readability
1 parent d67a3d1 commit 33e23ae

File tree

2 files changed

+50
-42
lines changed

2 files changed

+50
-42
lines changed

‎src/Operation/Aggregate.php

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,12 @@ public function __construct(string $databaseName, ?string $collectionName, array
229229
unset($options['batchSize']);
230230
}
231231

232-
/* Ignore batchSize for writes, since no documents are returned and a
233-
* batchSize of zero could prevent the pipeline from executing. */
234232
if ($this->isWrite) {
233+
/* Ignore batchSize for writes, since no documents are returned and
234+
* a batchSize of zero could prevent the pipeline from executing. */
235235
unset($options['batchSize']);
236+
} else {
237+
unset($options['writeConcern']);
236238
}
237239

238240
$this->databaseName = $databaseName;
@@ -366,16 +368,12 @@ private function executeCommand(Server $server, Command $command): Cursor
366368
{
367369
$options = [];
368370

369-
foreach (['readConcern', 'readPreference', 'session'] as $option) {
371+
foreach (['readConcern', 'readPreference', 'session', 'writeConcern'] as $option) {
370372
if (isset($this->options[$option])) {
371373
$options[$option] = $this->options[$option];
372374
}
373375
}
374376

375-
if ($this->isWrite && isset($this->options['writeConcern'])) {
376-
$options['writeConcern'] = $this->options['writeConcern'];
377-
}
378-
379377
if (! $this->isWrite) {
380378
return $server->executeReadCommand($this->databaseName, $command, $options);
381379
}

‎tests/Collection/CollectionFunctionalTest.php

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
use function call_user_func;
2222
use function is_scalar;
2323
use function json_encode;
24-
use function strchr;
24+
use function str_contains;
2525
use function usort;
2626
use function version_compare;
2727

@@ -434,16 +434,30 @@ public function testMapReduce(): void
434434
public function collectionMethodClosures()
435435
{
436436
return [
437-
[
437+
'read-only aggregate' => [
438438
function ($collection, $session, $options = []): void {
439439
$collection->aggregate(
440440
[['$match' => ['_id' => ['$lt' => 3]]]],
441441
['session' => $session] + $options
442442
);
443+
}, 'r',
444+
],
445+
446+
/* Disabled, as write aggregations are not supported in transactions
447+
'read-write aggregate' => [
448+
function ($collection, $session, $options = []): void {
449+
$collection->aggregate(
450+
[
451+
['$match' => ['_id' => ['$lt' => 3]]],
452+
['$merge' => $collection . '_out'],
453+
],
454+
['session' => $session] + $options
455+
);
443456
}, 'rw',
444457
],
458+
*/
445459

446-
[
460+
'bulkWrite insertOne' => [
447461
function ($collection, $session, $options = []): void {
448462
$collection->bulkWrite(
449463
[['insertOne' => [['test' => 'foo']]]],
@@ -453,7 +467,7 @@ function ($collection, $session, $options = []): void {
453467
],
454468

455469
/* Disabled, as count command can't be used in transactions
456-
[
470+
'count' => [
457471
function($collection, $session, $options = []) {
458472
$collection->count(
459473
[],
@@ -463,7 +477,7 @@ function($collection, $session, $options = []) {
463477
],
464478
*/
465479

466-
[
480+
'countDocuments' => [
467481
function ($collection, $session, $options = []): void {
468482
$collection->countDocuments(
469483
[],
@@ -473,7 +487,7 @@ function ($collection, $session, $options = []): void {
473487
],
474488

475489
/* Disabled, as it's illegal to use createIndex command in transactions
476-
[
490+
'createIndex' => [
477491
function($collection, $session, $options = []) {
478492
$collection->createIndex(
479493
['test' => 1],
@@ -483,7 +497,7 @@ function($collection, $session, $options = []) {
483497
],
484498
*/
485499

486-
[
500+
'deleteMany' => [
487501
function ($collection, $session, $options = []): void {
488502
$collection->deleteMany(
489503
['test' => 'foo'],
@@ -492,7 +506,7 @@ function ($collection, $session, $options = []): void {
492506
}, 'w',
493507
],
494508

495-
[
509+
'deleteOne' => [
496510
function ($collection, $session, $options = []): void {
497511
$collection->deleteOne(
498512
['test' => 'foo'],
@@ -501,7 +515,7 @@ function ($collection, $session, $options = []): void {
501515
}, 'w',
502516
],
503517

504-
[
518+
'distinct' => [
505519
function ($collection, $session, $options = []): void {
506520
$collection->distinct(
507521
'_id',
@@ -512,7 +526,7 @@ function ($collection, $session, $options = []): void {
512526
],
513527

514528
/* Disabled, as it's illegal to use drop command in transactions
515-
[
529+
'drop' => [
516530
function($collection, $session, $options = []) {
517531
$collection->drop(
518532
['session' => $session] + $options
@@ -522,7 +536,7 @@ function($collection, $session, $options = []) {
522536
*/
523537

524538
/* Disabled, as it's illegal to use dropIndexes command in transactions
525-
[
539+
'dropIndex' => [
526540
function($collection, $session, $options = []) {
527541
$collection->dropIndex(
528542
'_id_1',
@@ -532,7 +546,7 @@ function($collection, $session, $options = []) {
532546
], */
533547

534548
/* Disabled, as it's illegal to use dropIndexes command in transactions
535-
[
549+
'dropIndexes' => [
536550
function($collection, $session, $options = []) {
537551
$collection->dropIndexes(
538552
['session' => $session] + $options
@@ -542,7 +556,7 @@ function($collection, $session, $options = []) {
542556
*/
543557

544558
/* Disabled, as count command can't be used in transactions
545-
[
559+
'estimatedDocumentCount' => [
546560
function($collection, $session, $options = []) {
547561
$collection->estimatedDocumentCount(
548562
['session' => $session] + $options
@@ -551,7 +565,7 @@ function($collection, $session, $options = []) {
551565
],
552566
*/
553567

554-
[
568+
'find' => [
555569
function ($collection, $session, $options = []): void {
556570
$collection->find(
557571
['test' => 'foo'],
@@ -560,7 +574,7 @@ function ($collection, $session, $options = []): void {
560574
}, 'r',
561575
],
562576

563-
[
577+
'findOne' => [
564578
function ($collection, $session, $options = []): void {
565579
$collection->findOne(
566580
['test' => 'foo'],
@@ -569,7 +583,7 @@ function ($collection, $session, $options = []): void {
569583
}, 'r',
570584
],
571585

572-
[
586+
'findOneAndDelete' => [
573587
function ($collection, $session, $options = []): void {
574588
$collection->findOneAndDelete(
575589
['test' => 'foo'],
@@ -578,7 +592,7 @@ function ($collection, $session, $options = []): void {
578592
}, 'w',
579593
],
580594

581-
[
595+
'findOneAndReplace' => [
582596
function ($collection, $session, $options = []): void {
583597
$collection->findOneAndReplace(
584598
['test' => 'foo'],
@@ -588,7 +602,7 @@ function ($collection, $session, $options = []): void {
588602
}, 'w',
589603
],
590604

591-
[
605+
'findOneAndUpdate' => [
592606
function ($collection, $session, $options = []): void {
593607
$collection->findOneAndUpdate(
594608
['test' => 'foo'],
@@ -598,7 +612,7 @@ function ($collection, $session, $options = []): void {
598612
}, 'w',
599613
],
600614

601-
[
615+
'insertMany' => [
602616
function ($collection, $session, $options = []): void {
603617
$collection->insertMany(
604618
[
@@ -610,7 +624,7 @@ function ($collection, $session, $options = []): void {
610624
}, 'w',
611625
],
612626

613-
[
627+
'insertOne' => [
614628
function ($collection, $session, $options = []): void {
615629
$collection->insertOne(
616630
['test' => 'foo'],
@@ -620,7 +634,7 @@ function ($collection, $session, $options = []): void {
620634
],
621635

622636
/* Disabled, as it's illegal to use listIndexes command in transactions
623-
[
637+
'listIndexes' => [
624638
function($collection, $session, $options = []) {
625639
$collection->listIndexes(
626640
['session' => $session] + $options
@@ -630,7 +644,7 @@ function($collection, $session, $options = []) {
630644
*/
631645

632646
/* Disabled, as it's illegal to use mapReduce command in transactions
633-
[
647+
'mapReduce' => [
634648
function($collection, $session, $options = []) {
635649
$collection->mapReduce(
636650
new \MongoDB\BSON\Javascript('function() { emit(this.state, this.pop); }'),
@@ -642,7 +656,7 @@ function($collection, $session, $options = []) {
642656
],
643657
*/
644658

645-
[
659+
'replaceOne' => [
646660
function ($collection, $session, $options = []): void {
647661
$collection->replaceOne(
648662
['test' => 'foo'],
@@ -652,7 +666,7 @@ function ($collection, $session, $options = []): void {
652666
}, 'w',
653667
],
654668

655-
[
669+
'updateMany' => [
656670
function ($collection, $session, $options = []): void {
657671
$collection->updateMany(
658672
['test' => 'foo'],
@@ -662,7 +676,7 @@ function ($collection, $session, $options = []): void {
662676
}, 'w',
663677
],
664678

665-
[
679+
'updateOne' => [
666680
function ($collection, $session, $options = []): void {
667681
$collection->updateOne(
668682
['test' => 'foo'],
@@ -673,7 +687,7 @@ function ($collection, $session, $options = []): void {
673687
],
674688

675689
/* Disabled, as it's illegal to use change streams in transactions
676-
[
690+
'watch' => [
677691
function($collection, $session, $options = []) {
678692
$collection->watch(
679693
[],
@@ -685,32 +699,28 @@ function($collection, $session, $options = []) {
685699
];
686700
}
687701

688-
public function collectionReadMethodClosures()
702+
public function collectionReadMethodClosures(): array
689703
{
690704
return array_filter(
691705
$this->collectionMethodClosures(),
692706
function ($rw) {
693-
if (strchr($rw[1], 'r') !== false) {
694-
return true;
695-
}
707+
return str_contains($rw[1], 'r');
696708
}
697709
);
698710
}
699711

700-
public function collectionWriteMethodClosures()
712+
public function collectionWriteMethodClosures(): array
701713
{
702714
return array_filter(
703715
$this->collectionMethodClosures(),
704716
function ($rw) {
705-
if (strchr($rw[1], 'w') !== false) {
706-
return true;
707-
}
717+
return str_contains($rw[1], 'w');
708718
}
709719
);
710720
}
711721

712722
/** @dataProvider collectionMethodClosures */
713-
public function testMethodDoesNotInheritReadWriteConcernInTranasaction(Closure $method): void
723+
public function testMethodDoesNotInheritReadWriteConcernInTransaction(Closure $method): void
714724
{
715725
$this->skipIfTransactionsAreNotSupported();
716726

0 commit comments

Comments
 (0)