Skip to content

Commit 976df00

Browse files
committed
Merge branch 'v1.7'
* v1.7: PHPLIB-558: Don't inherit read preference in Database::command PHPLIB-561: Retry replSetStepDown in connection failover test PHPLIB-559: Make aggregate command explainable
2 parents 81a57c0 + 3f1558a commit 976df00

File tree

9 files changed

+121
-26
lines changed

9 files changed

+121
-26
lines changed

docs/reference/method/MongoDBCollection-explain.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ Explainable Commands
5050

5151
Explainable commands include, but are not limited to:
5252

53+
- :phpclass:`MongoDB\\Operation\\Aggregate`
5354
- :phpclass:`MongoDB\\Operation\\Count`
5455
- :phpclass:`MongoDB\\Operation\\DeleteMany`
5556
- :phpclass:`MongoDB\\Operation\\DeleteOne`

src/Database.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,6 @@ public function aggregate(array $pipeline, array $options = [])
249249
*/
250250
public function command($command, array $options = [])
251251
{
252-
if (! isset($options['readPreference']) && ! is_in_transaction($options)) {
253-
$options['readPreference'] = $this->readPreference;
254-
}
255-
256252
if (! isset($options['typeMap'])) {
257253
$options['typeMap'] = $this->typeMap;
258254
}

src/Operation/Aggregate.php

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
* @see \MongoDB\Collection::aggregate()
4949
* @see http://docs.mongodb.org/manual/reference/command/aggregate/
5050
*/
51-
class Aggregate implements Executable
51+
class Aggregate implements Executable, Explainable
5252
{
5353
/** @var integer */
5454
private static $wireVersionForCollation = 5;
@@ -285,9 +285,12 @@ public function execute(Server $server)
285285
}
286286

287287
$hasExplain = ! empty($this->options['explain']);
288-
$hasWriteStage = is_last_pipeline_operator_write($this->pipeline);
288+
$hasWriteStage = $this->hasWriteStage();
289289

290-
$command = $this->createCommand($server, $hasWriteStage);
290+
$command = new Command(
291+
$this->createCommandDocument($server, $hasWriteStage),
292+
$this->createCommandOptions()
293+
);
291294
$options = $this->createOptions($hasWriteStage, $hasExplain);
292295

293296
$cursor = $hasWriteStage && ! $hasExplain
@@ -315,20 +318,17 @@ public function execute(Server $server)
315318
return new ArrayIterator($result->result);
316319
}
317320

318-
/**
319-
* Create the aggregate command.
320-
*
321-
* @param Server $server
322-
* @param boolean $hasWriteStage
323-
* @return Command
324-
*/
325-
private function createCommand(Server $server, $hasWriteStage)
321+
public function getCommandDocument(Server $server)
322+
{
323+
return $this->createCommandDocument($server, $this->hasWriteStage());
324+
}
325+
326+
private function createCommandDocument(Server $server, bool $hasWriteStage) : array
326327
{
327328
$cmd = [
328329
'aggregate' => $this->collectionName ?? 1,
329330
'pipeline' => $this->pipeline,
330331
];
331-
$cmdOptions = [];
332332

333333
$cmd['allowDiskUse'] = $this->options['allowDiskUse'];
334334

@@ -352,10 +352,6 @@ private function createCommand(Server $server, $hasWriteStage)
352352
$cmd['hint'] = is_array($this->options['hint']) ? (object) $this->options['hint'] : $this->options['hint'];
353353
}
354354

355-
if (isset($this->options['maxAwaitTimeMS'])) {
356-
$cmdOptions['maxAwaitTimeMS'] = $this->options['maxAwaitTimeMS'];
357-
}
358-
359355
if ($this->options['useCursor']) {
360356
/* Ignore batchSize if pipeline includes an $out or $merge stage, as
361357
* no documents will be returned and sending a batchSize of zero
@@ -365,7 +361,18 @@ private function createCommand(Server $server, $hasWriteStage)
365361
: new stdClass();
366362
}
367363

368-
return new Command($cmd, $cmdOptions);
364+
return $cmd;
365+
}
366+
367+
private function createCommandOptions() : array
368+
{
369+
$cmdOptions = [];
370+
371+
if (isset($this->options['maxAwaitTimeMS'])) {
372+
$cmdOptions['maxAwaitTimeMS'] = $this->options['maxAwaitTimeMS'];
373+
}
374+
375+
return $cmdOptions;
369376
}
370377

371378
/**
@@ -399,4 +406,9 @@ private function createOptions($hasWriteStage, $hasExplain)
399406

400407
return $options;
401408
}
409+
410+
private function hasWriteStage() : bool
411+
{
412+
return is_last_pipeline_operator_write($this->pipeline);
413+
}
402414
}

src/Operation/Explain.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ class Explain implements Executable
4141
const VERBOSITY_EXEC_STATS = 'executionStats';
4242
const VERBOSITY_QUERY = 'queryPlanner';
4343

44+
/** @var integer */
45+
private static $wireVersionForAggregate = 7;
46+
4447
/** @var integer */
4548
private static $wireVersionForDistinct = 4;
4649

@@ -108,6 +111,10 @@ public function execute(Server $server)
108111
throw UnsupportedException::explainNotSupported();
109112
}
110113

114+
if ($this->explainable instanceof Aggregate && ! server_supports_feature($server, self::$wireVersionForAggregate)) {
115+
throw UnsupportedException::explainNotSupported();
116+
}
117+
111118
$cmd = ['explain' => $this->explainable->getCommandDocument($server)];
112119

113120
if (isset($this->options['verbosity'])) {

src/Operation/Explainable.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
use MongoDB\Driver\Server;
2121

2222
/**
23-
* Explainable interface for explainable operations (count, distinct, find,
24-
* findAndModify, delete, and update).
23+
* Explainable interface for explainable operations (aggregate, count, distinct,
24+
* find, findAndModify, delete, and update).
2525
*
2626
* @internal
2727
*/

tests/Database/DatabaseFunctionalTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use MongoDB\Driver\Cursor;
88
use MongoDB\Driver\ReadConcern;
99
use MongoDB\Driver\ReadPreference;
10+
use MongoDB\Driver\Server;
1011
use MongoDB\Driver\WriteConcern;
1112
use MongoDB\Exception\InvalidArgumentException;
1213
use MongoDB\Operation\CreateIndexes;
@@ -100,6 +101,22 @@ public function testCommand()
100101
$this->assertTrue($commandResult->ismaster);
101102
}
102103

104+
public function testCommandDoesNotInheritReadPreference()
105+
{
106+
if ($this->isReplicaSet()) {
107+
$this->markTestSkipped('Test only applies to replica sets');
108+
}
109+
110+
$this->database = new Database($this->manager, $this->getDatabaseName(), ['readPreference' => new ReadPreference(ReadPreference::RP_SECONDARY)]);
111+
112+
$command = ['ping' => 1];
113+
114+
$cursor = $this->database->command($command);
115+
116+
$this->assertInstanceOf(Cursor::class, $cursor);
117+
$this->assertTrue($cursor->getServer()->isPrimary());
118+
}
119+
103120
public function testCommandAppliesTypeMapToCursor()
104121
{
105122
$command = ['isMaster' => 1];

tests/FunctionalTestCase.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,11 @@ protected function getServerStorageEngine(ReadPreference $readPreference = null)
301301
throw new UnexpectedValueException('Could not determine server storage engine');
302302
}
303303

304+
protected function isReplicaSet()
305+
{
306+
return $this->getPrimaryServer()->getType() !== Server::TYPE_RS_PRIMARY;
307+
}
308+
304309
protected function isShardedCluster()
305310
{
306311
if ($this->getPrimaryServer()->getType() == Server::TYPE_MONGOS) {

tests/Operation/ExplainFunctionalTest.php

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace MongoDB\Tests\Operation;
44

55
use MongoDB\Driver\BulkWrite;
6+
use MongoDB\Operation\Aggregate;
67
use MongoDB\Operation\Count;
78
use MongoDB\Operation\CreateCollection;
89
use MongoDB\Operation\Delete;
@@ -387,6 +388,43 @@ public function testUpdateOne($verbosity, $executionStatsExpected, $allPlansExec
387388
$this->assertExplainResult($result, $executionStatsExpected, $allPlansExecutionExpected);
388389
}
389390

391+
public function testAggregate()
392+
{
393+
if (version_compare($this->getServerVersion(), '4.0.0', '<')) {
394+
$this->markTestSkipped('Explaining aggregate command requires server version >= 4.0');
395+
}
396+
397+
$this->createFixtures(3);
398+
399+
$pipeline = [['$group' => ['_id' => null]]];
400+
$operation = new Aggregate($this->getDatabaseName(), $this->getCollectionName(), $pipeline);
401+
402+
$explainOperation = new Explain($this->getDatabaseName(), $operation, ['verbosity' => Explain::VERBOSITY_QUERY, 'typeMap' => ['root' => 'array', 'document' => 'array']]);
403+
$result = $explainOperation->execute($this->getPrimaryServer());
404+
405+
$this->assertExplainResult($result, false, false, true);
406+
}
407+
408+
/**
409+
* @dataProvider provideVerbosityInformation
410+
*/
411+
public function testAggregateOptimizedToQuery($verbosity, $executionStatsExpected, $allPlansExecutionExpected)
412+
{
413+
if (version_compare($this->getServerVersion(), '4.2.0', '<')) {
414+
$this->markTestSkipped('MongoDB < 4.2 does not optimize simple aggregation pipelines');
415+
}
416+
417+
$this->createFixtures(3);
418+
419+
$pipeline = [['$match' => ['_id' => ['$ne' => 2]]]];
420+
$operation = new Aggregate($this->getDatabaseName(), $this->getCollectionName(), $pipeline);
421+
422+
$explainOperation = new Explain($this->getDatabaseName(), $operation, ['verbosity' => $verbosity, 'typeMap' => ['root' => 'array', 'document' => 'array']]);
423+
$result = $explainOperation->execute($this->getPrimaryServer());
424+
425+
$this->assertExplainResult($result, $executionStatsExpected, $allPlansExecutionExpected);
426+
}
427+
390428
public function provideVerbosityInformation()
391429
{
392430
return [
@@ -396,9 +434,14 @@ public function provideVerbosityInformation()
396434
];
397435
}
398436

399-
private function assertExplainResult($result, $executionStatsExpected, $allPlansExecutionExpected)
437+
private function assertExplainResult($result, $executionStatsExpected, $allPlansExecutionExpected, $stagesExpected = false)
400438
{
401-
$this->assertArrayHasKey('queryPlanner', $result);
439+
if ($stagesExpected) {
440+
$this->assertArrayHasKey('stages', $result);
441+
} else {
442+
$this->assertArrayHasKey('queryPlanner', $result);
443+
}
444+
402445
if ($executionStatsExpected) {
403446
$this->assertArrayHasKey('executionStats', $result);
404447
if ($allPlansExecutionExpected) {

tests/SpecTests/PrimaryStepDownSpecTest.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Symfony\Bridge\PhpUnit\SetUpTearDownTrait;
1717
use UnexpectedValueException;
1818
use function current;
19+
use function sprintf;
1920

2021
/**
2122
* @see https://github.com/mongodb/specifications/tree/master/source/connections-survive-step-down/tests
@@ -215,7 +216,20 @@ public function testGetMoreIteration()
215216

216217
// Send a {replSetStepDown: 5, force: true} command to the current primary and verify that the command succeeded
217218
$primary = $this->client->getManager()->selectServer(new ReadPreference(ReadPreference::RP_PRIMARY));
218-
$primary->executeCommand('admin', new Command(['replSetStepDown' => 5, 'force' => true]));
219+
220+
$success = false;
221+
$attempts = 0;
222+
do {
223+
try {
224+
$attempts++;
225+
$primary->executeCommand('admin', new Command(['replSetStepDown' => 5, 'force' => true]));
226+
$success = true;
227+
} catch (DriverException $e) {
228+
if ($attempts == 10) {
229+
$this->fail(sprintf('Could not successfully execute replSetStepDown within %d attempts', $attempts));
230+
}
231+
}
232+
} while (! $success);
219233

220234
// Retrieve the next batch of results from the cursor obtained in the find operation, and verify that this operation succeeded.
221235
$events = [];

0 commit comments

Comments
 (0)