From d92dd0bc8520671e6c4ac17c695dcb1f94e1e586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 14 Jun 2023 15:04:40 +0200 Subject: [PATCH 1/6] PHPLIB-1144: Add unit tests on Explainable::getCommandDocument --- tests/Operation/CountTest.php | 28 +++++++++++++ tests/Operation/DeleteTest.php | 26 ++++++++++++ tests/Operation/DistinctTest.php | 26 ++++++++++++ tests/Operation/FindAndModifyTest.php | 42 +++++++++++++++++++ tests/Operation/FindTest.php | 59 +++++++++++++++++++++++++++ tests/Operation/UpdateTest.php | 34 +++++++++++++++ 6 files changed, 215 insertions(+) diff --git a/tests/Operation/CountTest.php b/tests/Operation/CountTest.php index 23969595c..5259a9d44 100644 --- a/tests/Operation/CountTest.php +++ b/tests/Operation/CountTest.php @@ -2,6 +2,7 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\ReadConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Count; @@ -64,4 +65,31 @@ private function getInvalidHintValues() { return [123, 3.14, true]; } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'hint' => '_id_', + 'limit' => 10, + 'skip' => 20, + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'collation' => ['locale' => 'fr'], + 'comment' => 'explain me', + 'maxTimeMS' => 100, + ]; + $operation = new Count($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $options); + + $expected = [ + 'count' => $this->getCollectionName(), + 'query' => (object) ['x' => 1], + 'collation' => (object) ['locale' => 'fr'], + 'hint' => '_id_', + 'comment' => 'explain me', + 'limit' => 10, + 'skip' => 20, + 'maxTimeMS' => 100, + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } diff --git a/tests/Operation/DeleteTest.php b/tests/Operation/DeleteTest.php index acdbefc31..0e479b646 100644 --- a/tests/Operation/DeleteTest.php +++ b/tests/Operation/DeleteTest.php @@ -65,4 +65,30 @@ public function provideInvalidConstructorOptions() return $options; } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'collation' => ['locale' => 'fr'], + 'hint' => '_id_', + // Ignored options + 'let' => ['a' => 1], + 'ordered' => true, + 'comment' => 'explain me', + ]; + $operation = new Delete($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], 0, $options); + + $expected = [ + 'delete' => $this->getCollectionName(), + 'deletes' => [ + [ + 'q' => ['x' => 1], + 'limit' => 0, + 'collation' => (object) ['locale' => 'fr'], + 'hint' => '_id_', + ], + ], + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } diff --git a/tests/Operation/DistinctTest.php b/tests/Operation/DistinctTest.php index 8473e833d..21d3d2781 100644 --- a/tests/Operation/DistinctTest.php +++ b/tests/Operation/DistinctTest.php @@ -2,6 +2,8 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\ReadConcern; +use MongoDB\Driver\ReadPreference; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Distinct; @@ -51,4 +53,28 @@ public function provideInvalidConstructorOptions() return $options; } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'collation' => ['locale' => 'fr'], + 'maxTimeMS' => 100, + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'readPreference' => new ReadPreference(ReadPreference::SECONDARY_PREFERRED), + 'typeMap' => ['root' => 'array'], + 'comment' => 'explain me', + ]; + $operation = new Distinct($this->getDatabaseName(), $this->getCollectionName(), 'f', ['x' => 1], $options); + + $expected = [ + 'distinct' => $this->getCollectionName(), + 'key' => 'f', + 'query' => (object) ['x' => 1], + 'collation' => (object) ['locale' => 'fr'], + 'comment' => 'explain me', + 'maxTimeMS' => 100, + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } diff --git a/tests/Operation/FindAndModifyTest.php b/tests/Operation/FindAndModifyTest.php index 3904385e9..391b82eb5 100644 --- a/tests/Operation/FindAndModifyTest.php +++ b/tests/Operation/FindAndModifyTest.php @@ -2,6 +2,7 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\FindAndModify; @@ -83,4 +84,45 @@ public function testConstructorUpdateAndRemoveOptionsAreMutuallyExclusive(): voi $this->expectExceptionMessage('The "remove" option must be true or an "update" document must be specified, but not both'); new FindAndModify($this->getDatabaseName(), $this->getCollectionName(), ['remove' => true, 'update' => []]); } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'arrayFilters' => [['x' => 1]], + 'bypassDocumentValidation' => true, + 'collation' => ['locale' => 'fr'], + 'comment' => 'explain me', + 'fields' => ['_id' => 0], + 'hint' => '_id_', + 'maxTimeMS' => 100, + 'new' => true, + 'query' => ['y' => 2], + 'remove' => false, + 'sort' => ['x' => 1], + 'typeMap' => ['root' => 'array'], + 'update' => ['$set' => ['x' => 2]], + 'upsert' => true, + 'let' => ['a' => 3], + 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), + ]; + $operation = new FindAndModify($this->getDatabaseName(), $this->getCollectionName(), $options); + + $expected = [ + 'findAndModify' => $this->getCollectionName(), + 'new' => true, + 'upsert' => true, + 'collation' => (object) ['locale' => 'fr'], + 'fields' => (object) ['_id' => 0], + 'let' => (object) ['a' => 3], + 'query' => (object) ['y' => 2], + 'sort' => (object) ['x' => 1], + 'update' => (object) ['$set' => ['x' => 2]], + 'arrayFilters' => [['x' => 1]], + 'bypassDocumentValidation' => true, + 'comment' => 'explain me', + 'hint' => '_id_', + 'maxTimeMS' => 100, + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } diff --git a/tests/Operation/FindTest.php b/tests/Operation/FindTest.php index f5889f260..60a0644b6 100644 --- a/tests/Operation/FindTest.php +++ b/tests/Operation/FindTest.php @@ -2,6 +2,8 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\ReadConcern; +use MongoDB\Driver\ReadPreference; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Find; @@ -154,4 +156,61 @@ public function provideInvalidConstructorCursorTypeOptions() { return $this->wrapValuesForDataProvider([-1, 0, 4]); } + + public function testExplainableCommandDocument(): void + { + // all options except deprecated "snapshot" and "maxScan" + $options = [ + 'allowDiskUse' => true, + 'allowPartialResults' => true, + 'batchSize' => 123, + 'collation' => ['locale' => 'fr'], + 'comment' => 'explain me', + 'cursorType' => Find::NON_TAILABLE, + 'hint' => '_id_', + 'limit' => 15, + 'max' => ['x' => 100], + 'maxAwaitTimeMS' => 500, + 'maxTimeMS' => 100, + 'min' => ['x' => 10], + 'modifiers' => ['foo' => 'bar'], + 'noCursorTimeout' => true, + 'oplogReplay' => true, + 'projection' => ['_id' => 0], + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'readPreference' => new ReadPreference(ReadPreference::SECONDARY_PREFERRED), + 'returnKey' => true, + 'showRecordId' => true, + 'skip' => 5, + 'sort' => ['x' => 1], + 'let' => ['y' => 2], + 'typeMap' => ['root' => 'array'], + ]; + $operation = new Find($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $options); + + $expected = [ + 'find' => $this->getCollectionName(), + 'filter' => (object) ['x' => 1], + 'allowDiskUse' => true, + 'allowPartialResults' => true, + 'batchSize' => 123, + 'comment' => 'explain me', + 'hint' => '_id_', + 'limit' => 15, + 'maxTimeMS' => 100, + 'noCursorTimeout' => true, + 'oplogReplay' => true, + 'projection' => ['_id' => 0], + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'returnKey' => true, + 'showRecordId' => true, + 'skip' => 5, + 'sort' => ['x' => 1], + 'collation' => (object) ['locale' => 'fr'], + 'let' => (object) ['y' => 2], + 'max' => (object) ['x' => 100], + 'min' => (object) ['x' => 10], + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } diff --git a/tests/Operation/UpdateTest.php b/tests/Operation/UpdateTest.php index bcc445544..de9fcbadd 100644 --- a/tests/Operation/UpdateTest.php +++ b/tests/Operation/UpdateTest.php @@ -2,6 +2,7 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Update; @@ -75,4 +76,37 @@ public function testConstructorMultiOptionProhibitsReplacementDocumentOrEmptyPip $this->expectExceptionMessage('"multi" option cannot be true unless $update has update operator(s) or non-empty pipeline'); new Update($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $update, ['multi' => true]); } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'arrayFilters' => [['x' => 1]], + 'bypassDocumentValidation' => true, + 'collation' => ['locale' => 'fr'], + 'comment' => 'explain me', + 'hint' => '_id_', + 'multi' => true, + 'upsert' => true, + 'let' => ['a' => 3], + 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), + ]; + $operation = new Update($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], ['$set' => ['x' => 2]], $options); + + $expected = [ + 'update' => $this->getCollectionName(), + 'bypassDocumentValidation' => true, + 'updates' => [ + [ + 'q' => ['x' => 1], + 'u' => ['$set' => ['x' => 2]], + 'multi' => true, + 'upsert' => true, + 'arrayFilters' => [['x' => 1]], + 'hint' => '_id_', + 'collation' => (object) ['locale' => 'fr'], + ], + ], + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } From 668becb3e66a9b5c59d5dd5378390c3d75957d10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 15 Jun 2023 12:16:04 +0200 Subject: [PATCH 2/6] Add tests on FindOneAnd*::getCommandDocument --- tests/Operation/FindOneAndDeleteTest.php | 31 ++++++++++++++++++ tests/Operation/FindOneAndReplaceTest.php | 36 +++++++++++++++++++++ tests/Operation/FindOneAndUpdateTest.php | 39 +++++++++++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/tests/Operation/FindOneAndDeleteTest.php b/tests/Operation/FindOneAndDeleteTest.php index 42055ba27..3e4caa1de 100644 --- a/tests/Operation/FindOneAndDeleteTest.php +++ b/tests/Operation/FindOneAndDeleteTest.php @@ -2,6 +2,7 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\FindOneAndDelete; @@ -31,4 +32,34 @@ public function provideInvalidConstructorOptions() return $options; } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'collation' => ['locale' => 'fr'], + 'comment' => 'explain me', + 'hint' => '_id_', + 'maxTimeMS' => 100, + 'projection' => ['_id' => 0], + 'sort' => ['x' => 1], + 'let' => ['a' => 3], + 'typeMap' => ['root' => 'array'], + 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), + ]; + $operation = new FindOneAndDelete($this->getDatabaseName(), $this->getCollectionName(), ['y' => 2], $options); + + $expected = [ + 'findAndModify' => $this->getCollectionName(), + 'collation' => (object) ['locale' => 'fr'], + 'fields' => (object) ['_id' => 0], + 'let' => (object) ['a' => 3], + 'query' => (object) ['y' => 2], + 'sort' => (object) ['x' => 1], + 'comment' => 'explain me', + 'hint' => '_id_', + 'maxTimeMS' => 100, + 'remove' => true, + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } diff --git a/tests/Operation/FindOneAndReplaceTest.php b/tests/Operation/FindOneAndReplaceTest.php index b084036f4..ec77d747a 100644 --- a/tests/Operation/FindOneAndReplaceTest.php +++ b/tests/Operation/FindOneAndReplaceTest.php @@ -2,6 +2,7 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\FindOneAndReplace; @@ -82,4 +83,39 @@ public function provideInvalidConstructorReturnDocumentOptions() { return $this->wrapValuesForDataProvider([-1, 0, 3]); } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'bypassDocumentValidation' => true, + 'collation' => ['locale' => 'fr'], + 'comment' => 'explain me', + 'fields' => ['_id' => 0], + 'hint' => '_id_', + 'maxTimeMS' => 100, + 'projection' => ['_id' => 0], + 'returnDocument' => FindOneAndReplace::RETURN_DOCUMENT_AFTER, + 'sort' => ['x' => 1], + 'typeMap' => ['root' => 'array'], + 'let' => ['a' => 3], + 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), + ]; + $operation = new FindOneAndReplace($this->getDatabaseName(), $this->getCollectionName(), ['y' => 2], ['y' => 3], $options); + + $expected = [ + 'findAndModify' => $this->getCollectionName(), + 'new' => true, + 'collation' => (object) ['locale' => 'fr'], + 'fields' => (object) ['_id' => 0], + 'let' => (object) ['a' => 3], + 'query' => (object) ['y' => 2], + 'sort' => (object) ['x' => 1], + 'update' => (object) ['y' => 3], + 'bypassDocumentValidation' => true, + 'comment' => 'explain me', + 'hint' => '_id_', + 'maxTimeMS' => 100, + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } diff --git a/tests/Operation/FindOneAndUpdateTest.php b/tests/Operation/FindOneAndUpdateTest.php index 8514cea86..0b1c81b2d 100644 --- a/tests/Operation/FindOneAndUpdateTest.php +++ b/tests/Operation/FindOneAndUpdateTest.php @@ -2,6 +2,7 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\FindOneAndUpdate; @@ -65,4 +66,42 @@ public function provideInvalidConstructorReturnDocumentOptions() { return $this->wrapValuesForDataProvider([-1, 0, 3]); } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'arrayFilters' => [['x' => 1]], + 'bypassDocumentValidation' => true, + 'collation' => ['locale' => 'fr'], + 'comment' => 'explain me', + 'hint' => '_id_', + 'maxTimeMS' => 100, + 'projection' => ['_id' => 0], + 'returnDocument' => FindOneAndUpdate::RETURN_DOCUMENT_AFTER, + 'sort' => ['x' => 1], + 'typeMap' => ['root' => 'array'], + 'upsert' => true, + 'let' => ['a' => 3], + 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), + ]; + $operation = new FindOneAndUpdate($this->getDatabaseName(), $this->getCollectionName(), ['y' => 2], ['$set' => ['x' => 2]], $options); + + $expected = [ + 'findAndModify' => $this->getCollectionName(), + 'new' => true, + 'upsert' => true, + 'collation' => (object) ['locale' => 'fr'], + 'fields' => (object) ['_id' => 0], + 'let' => (object) ['a' => 3], + 'query' => (object) ['y' => 2], + 'sort' => (object) ['x' => 1], + 'update' => (object) ['$set' => ['x' => 2]], + 'arrayFilters' => [['x' => 1]], + 'bypassDocumentValidation' => true, + 'comment' => 'explain me', + 'hint' => '_id_', + 'maxTimeMS' => 100, + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } From cf740db6c93431c5ef4547cadbc424b287c58bf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 16 Jun 2023 09:30:29 +0200 Subject: [PATCH 3/6] Refactor Find::getCommandDocument Remove private function createCommandDocument introduced by c7b2b033b327b29a619ee0ed2bcd9d055b4ab9d1 and used only for the explain command. --- src/Operation/Find.php | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/src/Operation/Find.php b/src/Operation/Find.php index 01929c60e..e88df3146 100644 --- a/src/Operation/Find.php +++ b/src/Operation/Find.php @@ -329,21 +329,6 @@ public function execute(Server $server) * @return array */ public function getCommandDocument() - { - $cmd = $this->createCommandDocument(); - - // Read concern can change the query plan - if (isset($this->options['readConcern'])) { - $cmd['readConcern'] = $this->options['readConcern']; - } - - return $cmd; - } - - /** - * Construct a command document for Find - */ - private function createCommandDocument(): array { $cmd = ['find' => $this->collectionName, 'filter' => (object) $this->filter]; @@ -353,6 +338,11 @@ private function createCommandDocument(): array return $cmd; } + // Read concern can change the query plan + if (isset($this->options['readConcern'])) { + $cmd['readConcern'] = $this->options['readConcern']; + } + // maxAwaitTimeMS is a Query level option so should not be considered here unset($options['maxAwaitTimeMS']); From aedff6c36a78f49a7d2e17acf4af0a7100c1cca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 16 Jun 2023 11:00:32 +0200 Subject: [PATCH 4/6] Add let and comment to explain delete Separate Intentionally omitted option --- src/Operation/Delete.php | 12 +++++++++++- tests/Operation/DeleteTest.php | 7 +++++-- tests/Operation/DistinctTest.php | 4 +++- tests/Operation/FindAndModifyTest.php | 7 ++++--- tests/Operation/FindOneAndDeleteTest.php | 3 ++- tests/Operation/FindOneAndReplaceTest.php | 5 +++-- tests/Operation/FindOneAndUpdateTest.php | 7 ++++--- tests/Operation/FindTest.php | 9 +++++---- 8 files changed, 37 insertions(+), 17 deletions(-) diff --git a/src/Operation/Delete.php b/src/Operation/Delete.php index 48fe99470..5f3318012 100644 --- a/src/Operation/Delete.php +++ b/src/Operation/Delete.php @@ -177,7 +177,17 @@ public function execute(Server $server) */ public function getCommandDocument() { - return ['delete' => $this->collectionName, 'deletes' => [['q' => $this->filter] + $this->createDeleteOptions()]]; + $cmd = ['delete' => $this->collectionName, 'deletes' => [['q' => $this->filter] + $this->createDeleteOptions()]]; + + if (isset($this->options['comment'])) { + $cmd['comment'] = $this->options['comment']; + } + + if (isset($this->options['let'])) { + $cmd['let'] = (object) $this->options['let']; + } + + return $cmd; } /** diff --git a/tests/Operation/DeleteTest.php b/tests/Operation/DeleteTest.php index 0e479b646..90b9cae52 100644 --- a/tests/Operation/DeleteTest.php +++ b/tests/Operation/DeleteTest.php @@ -7,6 +7,7 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Delete; use TypeError; @@ -71,10 +72,10 @@ public function testExplainableCommandDocument(): void $options = [ 'collation' => ['locale' => 'fr'], 'hint' => '_id_', - // Ignored options 'let' => ['a' => 1], - 'ordered' => true, 'comment' => 'explain me', + // Intentionally omitted options + 'writeConcern' => new WriteConcern(0), ]; $operation = new Delete($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], 0, $options); @@ -88,6 +89,8 @@ public function testExplainableCommandDocument(): void 'hint' => '_id_', ], ], + 'comment' => 'explain me', + 'let' => (object) ['a' => 1], ]; $this->assertEquals($expected, $operation->getCommandDocument()); } diff --git a/tests/Operation/DistinctTest.php b/tests/Operation/DistinctTest.php index 21d3d2781..c27a302bf 100644 --- a/tests/Operation/DistinctTest.php +++ b/tests/Operation/DistinctTest.php @@ -4,6 +4,7 @@ use MongoDB\Driver\ReadConcern; use MongoDB\Driver\ReadPreference; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Distinct; @@ -60,9 +61,10 @@ public function testExplainableCommandDocument(): void 'collation' => ['locale' => 'fr'], 'maxTimeMS' => 100, 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'comment' => 'explain me', + // Intentionally omitted options 'readPreference' => new ReadPreference(ReadPreference::SECONDARY_PREFERRED), 'typeMap' => ['root' => 'array'], - 'comment' => 'explain me', ]; $operation = new Distinct($this->getDatabaseName(), $this->getCollectionName(), 'f', ['x' => 1], $options); diff --git a/tests/Operation/FindAndModifyTest.php b/tests/Operation/FindAndModifyTest.php index 391b82eb5..a0738bb12 100644 --- a/tests/Operation/FindAndModifyTest.php +++ b/tests/Operation/FindAndModifyTest.php @@ -97,13 +97,14 @@ public function testExplainableCommandDocument(): void 'maxTimeMS' => 100, 'new' => true, 'query' => ['y' => 2], - 'remove' => false, 'sort' => ['x' => 1], - 'typeMap' => ['root' => 'array'], 'update' => ['$set' => ['x' => 2]], 'upsert' => true, 'let' => ['a' => 3], - 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), + // Intentionally omitted options + 'remove' => false, // When "update" is set + 'typeMap' => ['root' => 'array'], + 'writeConcern' => new WriteConcern(0), ]; $operation = new FindAndModify($this->getDatabaseName(), $this->getCollectionName(), $options); diff --git a/tests/Operation/FindOneAndDeleteTest.php b/tests/Operation/FindOneAndDeleteTest.php index 3e4caa1de..e63062ace 100644 --- a/tests/Operation/FindOneAndDeleteTest.php +++ b/tests/Operation/FindOneAndDeleteTest.php @@ -40,9 +40,10 @@ public function testExplainableCommandDocument(): void 'comment' => 'explain me', 'hint' => '_id_', 'maxTimeMS' => 100, - 'projection' => ['_id' => 0], 'sort' => ['x' => 1], 'let' => ['a' => 3], + // Intentionally omitted options + 'projection' => ['_id' => 0], 'typeMap' => ['root' => 'array'], 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), ]; diff --git a/tests/Operation/FindOneAndReplaceTest.php b/tests/Operation/FindOneAndReplaceTest.php index ec77d747a..743098e4f 100644 --- a/tests/Operation/FindOneAndReplaceTest.php +++ b/tests/Operation/FindOneAndReplaceTest.php @@ -94,10 +94,11 @@ public function testExplainableCommandDocument(): void 'hint' => '_id_', 'maxTimeMS' => 100, 'projection' => ['_id' => 0], - 'returnDocument' => FindOneAndReplace::RETURN_DOCUMENT_AFTER, 'sort' => ['x' => 1], - 'typeMap' => ['root' => 'array'], 'let' => ['a' => 3], + // Intentionally omitted options + 'returnDocument' => FindOneAndReplace::RETURN_DOCUMENT_AFTER, + 'typeMap' => ['root' => 'array'], 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), ]; $operation = new FindOneAndReplace($this->getDatabaseName(), $this->getCollectionName(), ['y' => 2], ['y' => 3], $options); diff --git a/tests/Operation/FindOneAndUpdateTest.php b/tests/Operation/FindOneAndUpdateTest.php index 0b1c81b2d..d174b73f3 100644 --- a/tests/Operation/FindOneAndUpdateTest.php +++ b/tests/Operation/FindOneAndUpdateTest.php @@ -76,12 +76,13 @@ public function testExplainableCommandDocument(): void 'comment' => 'explain me', 'hint' => '_id_', 'maxTimeMS' => 100, - 'projection' => ['_id' => 0], - 'returnDocument' => FindOneAndUpdate::RETURN_DOCUMENT_AFTER, 'sort' => ['x' => 1], - 'typeMap' => ['root' => 'array'], 'upsert' => true, 'let' => ['a' => 3], + // Intentionally omitted options + 'projection' => ['_id' => 0], + 'returnDocument' => FindOneAndUpdate::RETURN_DOCUMENT_AFTER, + 'typeMap' => ['root' => 'array'], 'writeConcern' => new WriteConcern(WriteConcern::MAJORITY), ]; $operation = new FindOneAndUpdate($this->getDatabaseName(), $this->getCollectionName(), ['y' => 2], ['$set' => ['x' => 2]], $options); diff --git a/tests/Operation/FindTest.php b/tests/Operation/FindTest.php index 60a0644b6..1da7d23fa 100644 --- a/tests/Operation/FindTest.php +++ b/tests/Operation/FindTest.php @@ -166,24 +166,25 @@ public function testExplainableCommandDocument(): void 'batchSize' => 123, 'collation' => ['locale' => 'fr'], 'comment' => 'explain me', - 'cursorType' => Find::NON_TAILABLE, 'hint' => '_id_', 'limit' => 15, 'max' => ['x' => 100], - 'maxAwaitTimeMS' => 500, 'maxTimeMS' => 100, 'min' => ['x' => 10], - 'modifiers' => ['foo' => 'bar'], 'noCursorTimeout' => true, 'oplogReplay' => true, 'projection' => ['_id' => 0], 'readConcern' => new ReadConcern(ReadConcern::LOCAL), - 'readPreference' => new ReadPreference(ReadPreference::SECONDARY_PREFERRED), 'returnKey' => true, 'showRecordId' => true, 'skip' => 5, 'sort' => ['x' => 1], 'let' => ['y' => 2], + // Intentionally omitted options + 'cursorType' => Find::NON_TAILABLE, + 'maxAwaitTimeMS' => 500, + 'modifiers' => ['foo' => 'bar'], + 'readPreference' => new ReadPreference(ReadPreference::SECONDARY_PREFERRED), 'typeMap' => ['root' => 'array'], ]; $operation = new Find($this->getDatabaseName(), $this->getCollectionName(), ['x' => 1], $options); From fac470b82a93847a088ad704db7d20433c208b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Fri, 16 Jun 2023 15:09:46 +0200 Subject: [PATCH 5/6] Aggregate option explain is incompatible with the explain command The 'explain' option is illegal when a explain verbosity is also provided --- src/Operation/Aggregate.php | 3 +++ tests/Operation/AggregateTest.php | 39 +++++++++++++++++++++++++++++++ tests/Operation/DistinctTest.php | 1 - 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/Operation/Aggregate.php b/src/Operation/Aggregate.php index 4d6c5867c..357b38ec5 100644 --- a/src/Operation/Aggregate.php +++ b/src/Operation/Aggregate.php @@ -300,6 +300,9 @@ public function getCommandDocument() { $cmd = $this->createCommandDocument(); + // The 'explain' option is incompatible with the explain command + unset($cmd['explain']); + // Read concern can change the query plan if (isset($this->options['readConcern'])) { $cmd['readConcern'] = $this->options['readConcern']; diff --git a/tests/Operation/AggregateTest.php b/tests/Operation/AggregateTest.php index b046e8583..1c8af61fe 100644 --- a/tests/Operation/AggregateTest.php +++ b/tests/Operation/AggregateTest.php @@ -2,6 +2,9 @@ namespace MongoDB\Tests\Operation; +use MongoDB\Driver\ReadConcern; +use MongoDB\Driver\ReadPreference; +use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Aggregate; @@ -104,4 +107,40 @@ private function getInvalidHintValues() { return [123, 3.14, true]; } + + public function testExplainableCommandDocument(): void + { + $options = [ + 'allowDiskUse' => true, + 'batchSize' => 100, + 'bypassDocumentValidation' => true, + 'collation' => ['locale' => 'fr'], + 'comment' => 'explain me', + 'hint' => '_id_', + 'let' => ['a' => 1], + 'maxTimeMS' => 100, + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'useCursor' => true, + // Intentionally omitted options + 'explain' => true, + 'readPreference' => new ReadPreference(ReadPreference::SECONDARY_PREFERRED), + 'typeMap' => ['root' => 'array', 'document' => 'array'], + 'writeConcern' => new WriteConcern(0), + ]; + $operation = new Aggregate($this->getDatabaseName(), $this->getCollectionName(), [['$project' => ['_id' => 0]]], $options); + + $expected = [ + 'aggregate' => $this->getCollectionName(), + 'pipeline' => [['$project' => ['_id' => 0]]], + 'allowDiskUse' => true, + 'bypassDocumentValidation' => true, + 'collation' => (object) ['locale' => 'fr'], + 'comment' => 'explain me', + 'hint' => '_id_', + 'maxTimeMS' => 100, + 'readConcern' => new ReadConcern(ReadConcern::LOCAL), + 'let' => (object) ['a' => 1], + ]; + $this->assertEquals($expected, $operation->getCommandDocument()); + } } diff --git a/tests/Operation/DistinctTest.php b/tests/Operation/DistinctTest.php index c27a302bf..9ece08fa5 100644 --- a/tests/Operation/DistinctTest.php +++ b/tests/Operation/DistinctTest.php @@ -4,7 +4,6 @@ use MongoDB\Driver\ReadConcern; use MongoDB\Driver\ReadPreference; -use MongoDB\Driver\WriteConcern; use MongoDB\Exception\InvalidArgumentException; use MongoDB\Operation\Distinct; From 8513129b7c846f5a94ceaaac84007a7c22b7350e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 19 Jun 2023 09:37:01 +0200 Subject: [PATCH 6/6] Let the server throw errors --- psalm-baseline.xml | 2 +- src/Operation/Aggregate.php | 3 --- src/Operation/Find.php | 5 ----- tests/Operation/AggregateTest.php | 3 ++- 4 files changed, 3 insertions(+), 10 deletions(-) diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 7a6d55420..b9f153751 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -471,7 +471,7 @@ $this->options['writeConcern'] - $cmd['writeConcern'] + $cmd['comment'] $deleteOptions['hint'] $options['comment'] $options['session'] diff --git a/src/Operation/Aggregate.php b/src/Operation/Aggregate.php index 357b38ec5..4d6c5867c 100644 --- a/src/Operation/Aggregate.php +++ b/src/Operation/Aggregate.php @@ -300,9 +300,6 @@ public function getCommandDocument() { $cmd = $this->createCommandDocument(); - // The 'explain' option is incompatible with the explain command - unset($cmd['explain']); - // Read concern can change the query plan if (isset($this->options['readConcern'])) { $cmd['readConcern'] = $this->options['readConcern']; diff --git a/src/Operation/Find.php b/src/Operation/Find.php index e88df3146..8725d37e8 100644 --- a/src/Operation/Find.php +++ b/src/Operation/Find.php @@ -338,11 +338,6 @@ public function getCommandDocument() return $cmd; } - // Read concern can change the query plan - if (isset($this->options['readConcern'])) { - $cmd['readConcern'] = $this->options['readConcern']; - } - // maxAwaitTimeMS is a Query level option so should not be considered here unset($options['maxAwaitTimeMS']); diff --git a/tests/Operation/AggregateTest.php b/tests/Operation/AggregateTest.php index 1c8af61fe..f72fc5a22 100644 --- a/tests/Operation/AggregateTest.php +++ b/tests/Operation/AggregateTest.php @@ -122,7 +122,7 @@ public function testExplainableCommandDocument(): void 'readConcern' => new ReadConcern(ReadConcern::LOCAL), 'useCursor' => true, // Intentionally omitted options - 'explain' => true, + // The "explain" option is illegal 'readPreference' => new ReadPreference(ReadPreference::SECONDARY_PREFERRED), 'typeMap' => ['root' => 'array', 'document' => 'array'], 'writeConcern' => new WriteConcern(0), @@ -140,6 +140,7 @@ public function testExplainableCommandDocument(): void 'maxTimeMS' => 100, 'readConcern' => new ReadConcern(ReadConcern::LOCAL), 'let' => (object) ['a' => 1], + 'cursor' => ['batchSize' => 100], ]; $this->assertEquals($expected, $operation->getCommandDocument()); }