From 5aff53f379e9a1cedb4fb84a13ce6dbc60ae23d0 Mon Sep 17 00:00:00 2001 From: Andreas Braun Date: Mon, 17 Jul 2023 17:05:14 +0200 Subject: [PATCH] PHPLIB-1192: Remove useCursor option in aggregate (#1130) * PHPLIB-1192: Remove useCursor option in aggregate * Remove unnecessary property for isExplain --- ...oDBCollection-method-aggregate-option.yaml | 16 ----- .../method/MongoDBCollection-aggregate.txt | 9 +-- src/Collection.php | 8 +-- src/Operation/Aggregate.php | 58 +++---------------- src/Operation/CountDocuments.php | 3 - src/Operation/Watch.php | 6 +- tests/Operation/AggregateTest.php | 14 ----- tests/UnifiedSpecTests/Util.php | 4 +- 8 files changed, 13 insertions(+), 105 deletions(-) diff --git a/docs/includes/apiargs-MongoDBCollection-method-aggregate-option.yaml b/docs/includes/apiargs-MongoDBCollection-method-aggregate-option.yaml index b3d19c3e5..9a3d6a4a5 100644 --- a/docs/includes/apiargs-MongoDBCollection-method-aggregate-option.yaml +++ b/docs/includes/apiargs-MongoDBCollection-method-aggregate-option.yaml @@ -59,22 +59,6 @@ source: file: apiargs-MongoDBCollection-common-option.yaml ref: typeMap --- -arg_name: option -name: useCursor -type: boolean -description: | - Indicates whether the command will request that the server provide results - using a cursor. The default is ``true``. - - .. note:: - - MongoDB 3.6+ no longer supports returning results without a cursor (excluding - when the explain option is used) and specifying false for this option will - result in a server error. -interface: phpmethod -operation: ~ -optional: true ---- source: file: apiargs-MongoDBCollection-common-option.yaml ref: writeConcern diff --git a/docs/reference/method/MongoDBCollection-aggregate.txt b/docs/reference/method/MongoDBCollection-aggregate.txt index d26d8c8e9..6975098db 100644 --- a/docs/reference/method/MongoDBCollection-aggregate.txt +++ b/docs/reference/method/MongoDBCollection-aggregate.txt @@ -50,13 +50,8 @@ Errors/Exceptions Behavior -------- -:phpmethod:`MongoDB\\Collection::aggregate()`'s return value depends on the -MongoDB server version and whether the ``useCursor`` option is specified. If -``useCursor`` is ``true``, a :php:`MongoDB\\Driver\\Cursor -` object is returned. If ``useCursor`` is -``false``, an :php:`ArrayIterator ` is returned that wraps the -``result`` array from the command response document. In both cases, the return -value will be :php:`Traversable `. +:phpmethod:`MongoDB\\Collection::aggregate()`'s returns a +:php:`MongoDB\\Driver\\Cursor ` object. Examples -------- diff --git a/src/Collection.php b/src/Collection.php index 41833a24e..801511951 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -59,7 +59,6 @@ use MongoDB\Operation\UpdateMany; use MongoDB\Operation\UpdateOne; use MongoDB\Operation\Watch; -use Traversable; use function array_diff_key; use function array_intersect_key; @@ -186,15 +185,10 @@ public function __toString() /** * Executes an aggregation framework pipeline on the collection. * - * Note: this method's return value depends on the MongoDB server version - * and the "useCursor" option. If "useCursor" is true, a Cursor will be - * returned; otherwise, an ArrayIterator is returned, which wraps the - * "result" array from the command response document. - * * @see Aggregate::__construct() for supported options * @param array $pipeline Aggregation pipeline * @param array $options Command options - * @return Traversable + * @return Cursor * @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/Operation/Aggregate.php b/src/Operation/Aggregate.php index be6cc60a9..2678b7c73 100644 --- a/src/Operation/Aggregate.php +++ b/src/Operation/Aggregate.php @@ -17,7 +17,6 @@ namespace MongoDB\Operation; -use ArrayIterator; use MongoDB\Driver\Command; use MongoDB\Driver\Cursor; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; @@ -31,13 +30,11 @@ use MongoDB\Exception\UnsupportedException; use stdClass; -use function current; use function is_array; use function is_bool; use function is_integer; use function is_object; use function is_string; -use function MongoDB\create_field_path_type_map; use function MongoDB\is_document; use function MongoDB\is_last_pipeline_operator_write; use function MongoDB\is_pipeline; @@ -58,8 +55,6 @@ class Aggregate implements Executable, Explainable private array $options; - private bool $isExplain; - private bool $isWrite; /** @@ -112,12 +107,6 @@ class Aggregate implements Executable, Explainable * * typeMap (array): Type map for BSON deserialization. This will be * applied to the returned Cursor (it is not sent to the server). * - * * useCursor (boolean): Indicates whether the command will request that - * the server provide results using a cursor. The default is true. - * - * This option allows users to turn off cursors if necessary to aid in - * mongod/mongos upgrades. - * * * writeConcern (MongoDB\Driver\WriteConcern): Write concern. This only * applies when an $out or $merge stage is specified. * @@ -136,8 +125,6 @@ public function __construct(string $databaseName, ?string $collectionName, array throw new InvalidArgumentException('$pipeline is not a valid aggregation pipeline'); } - $options += ['useCursor' => true]; - if (isset($options['allowDiskUse']) && ! is_bool($options['allowDiskUse'])) { throw InvalidArgumentException::invalidType('"allowDiskUse" option', $options['allowDiskUse'], 'boolean'); } @@ -190,18 +177,10 @@ public function __construct(string $databaseName, ?string $collectionName, array throw InvalidArgumentException::invalidType('"typeMap" option', $options['typeMap'], 'array'); } - if (! is_bool($options['useCursor'])) { - throw InvalidArgumentException::invalidType('"useCursor" option', $options['useCursor'], 'boolean'); - } - if (isset($options['writeConcern']) && ! $options['writeConcern'] instanceof WriteConcern) { throw InvalidArgumentException::invalidType('"writeConcern" option', $options['writeConcern'], WriteConcern::class); } - if (isset($options['batchSize']) && ! $options['useCursor']) { - throw new InvalidArgumentException('"batchSize" option should not be used if "useCursor" is false'); - } - if (isset($options['bypassDocumentValidation']) && ! $options['bypassDocumentValidation']) { unset($options['bypassDocumentValidation']); } @@ -214,14 +193,7 @@ public function __construct(string $databaseName, ?string $collectionName, array unset($options['writeConcern']); } - $this->isExplain = ! empty($options['explain']); - $this->isWrite = is_last_pipeline_operator_write($pipeline) && ! $this->isExplain; - - // Explain does not use a cursor - if ($this->isExplain) { - $options['useCursor'] = false; - unset($options['batchSize']); - } + $this->isWrite = is_last_pipeline_operator_write($pipeline) && ! ($options['explain'] ?? false); if ($this->isWrite) { /* Ignore batchSize for writes, since no documents are returned and @@ -241,7 +213,7 @@ public function __construct(string $databaseName, ?string $collectionName, array * Execute the operation. * * @see Executable::execute() - * @return ArrayIterator|Cursor + * @return Cursor * @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) @@ -266,25 +238,11 @@ public function execute(Server $server) $cursor = $this->executeCommand($server, $command); - if ($this->options['useCursor'] || $this->isExplain) { - if (isset($this->options['typeMap'])) { - $cursor->setTypeMap($this->options['typeMap']); - } - - return $cursor; - } - if (isset($this->options['typeMap'])) { - $cursor->setTypeMap(create_field_path_type_map($this->options['typeMap'], 'result.$')); + $cursor->setTypeMap($this->options['typeMap']); } - $result = current($cursor->toArray()); - - if (! is_object($result) || ! isset($result->result) || ! is_array($result->result)) { - throw new UnexpectedValueException('aggregate command did not return a "result" array'); - } - - return new ArrayIterator($result->result); + return $cursor; } /** @@ -331,11 +289,9 @@ private function createCommandDocument(): array $cmd['hint'] = is_array($this->options['hint']) ? (object) $this->options['hint'] : $this->options['hint']; } - if ($this->options['useCursor']) { - $cmd['cursor'] = isset($this->options["batchSize"]) - ? ['batchSize' => $this->options["batchSize"]] - : new stdClass(); - } + $cmd['cursor'] = isset($this->options['batchSize']) + ? ['batchSize' => $this->options['batchSize']] + : new stdClass(); return $cmd; } diff --git a/src/Operation/CountDocuments.php b/src/Operation/CountDocuments.php index e1652a394..fad2e94db 100644 --- a/src/Operation/CountDocuments.php +++ b/src/Operation/CountDocuments.php @@ -17,7 +17,6 @@ namespace MongoDB\Operation; -use MongoDB\Driver\Cursor; use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException; use MongoDB\Driver\Server; use MongoDB\Exception\InvalidArgumentException; @@ -25,7 +24,6 @@ use MongoDB\Exception\UnsupportedException; use function array_intersect_key; -use function assert; use function count; use function current; use function is_float; @@ -125,7 +123,6 @@ public function __construct(string $databaseName, string $collectionName, $filte public function execute(Server $server) { $cursor = $this->aggregate->execute($server); - assert($cursor instanceof Cursor); $allResults = $cursor->toArray(); diff --git a/src/Operation/Watch.php b/src/Operation/Watch.php index 0a7d6f07f..ff1052dfb 100644 --- a/src/Operation/Watch.php +++ b/src/Operation/Watch.php @@ -36,7 +36,6 @@ use function array_intersect_key; use function array_key_exists; use function array_unshift; -use function assert; use function count; use function is_array; use function is_bool; @@ -355,10 +354,7 @@ private function executeAggregate(Server $server): Cursor addSubscriber($this); try { - $cursor = $this->aggregate->execute($server); - assert($cursor instanceof Cursor); - - return $cursor; + return $this->aggregate->execute($server); } finally { removeSubscriber($this); } diff --git a/tests/Operation/AggregateTest.php b/tests/Operation/AggregateTest.php index 7ea4ee9fa..433278ebb 100644 --- a/tests/Operation/AggregateTest.php +++ b/tests/Operation/AggregateTest.php @@ -40,23 +40,10 @@ public function provideInvalidConstructorOptions() 'readPreference' => $this->getInvalidReadPreferenceValues(), 'session' => $this->getInvalidSessionValues(), 'typeMap' => $this->getInvalidArrayValues(), - 'useCursor' => $this->getInvalidBooleanValues(true), 'writeConcern' => $this->getInvalidWriteConcernValues(), ]); } - public function testConstructorBatchSizeOptionRequiresUseCursor(): void - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('"batchSize" option should not be used if "useCursor" is false'); - new Aggregate( - $this->getDatabaseName(), - $this->getCollectionName(), - [['$match' => ['x' => 1]]], - ['batchSize' => 100, 'useCursor' => false], - ); - } - public function testExplainableCommandDocument(): void { $options = [ @@ -69,7 +56,6 @@ public function testExplainableCommandDocument(): void 'let' => ['a' => 1], 'maxTimeMS' => 100, 'readConcern' => new ReadConcern(ReadConcern::LOCAL), - 'useCursor' => true, // Intentionally omitted options // The "explain" option is illegal 'readPreference' => new ReadPreference(ReadPreference::SECONDARY_PREFERRED), diff --git a/tests/UnifiedSpecTests/Util.php b/tests/UnifiedSpecTests/Util.php index 84aa15129..3ddb8fcee 100644 --- a/tests/UnifiedSpecTests/Util.php +++ b/tests/UnifiedSpecTests/Util.php @@ -72,7 +72,7 @@ final class Util 'rewrapManyDataKey' => ['filter', 'opts'], ], Database::class => [ - 'aggregate' => ['pipeline', 'session', 'useCursor', 'allowDiskUse', 'batchSize', 'bypassDocumentValidation', 'collation', 'comment', 'explain', 'hint', 'let', 'maxAwaitTimeMS', 'maxTimeMS'], + 'aggregate' => ['pipeline', 'session', 'allowDiskUse', 'batchSize', 'bypassDocumentValidation', 'collation', 'comment', 'explain', 'hint', 'let', 'maxAwaitTimeMS', 'maxTimeMS'], 'createChangeStream' => ['pipeline', 'session', 'fullDocument', 'resumeAfter', 'startAfter', 'startAtOperationTime', 'batchSize', 'collation', 'maxAwaitTimeMS', 'showExpandedEvents'], 'createCollection' => ['collection', 'session', 'autoIndexId', 'capped', 'changeStreamPreAndPostImages', 'clusteredIndex', 'collation', 'expireAfterSeconds', 'flags', 'indexOptionDefaults', 'max', 'maxTimeMS', 'pipeline', 'size', 'storageEngine', 'timeseries', 'validationAction', 'validationLevel', 'validator', 'viewOn'], 'dropCollection' => ['collection', 'session'], @@ -83,7 +83,7 @@ final class Util 'runCommand' => ['command', 'commandName', 'readPreference', 'session'], ], Collection::class => [ - 'aggregate' => ['pipeline', 'session', 'useCursor', 'allowDiskUse', 'batchSize', 'bypassDocumentValidation', 'collation', 'comment', 'explain', 'hint', 'let', 'maxAwaitTimeMS', 'maxTimeMS'], + 'aggregate' => ['pipeline', 'session', 'allowDiskUse', 'batchSize', 'bypassDocumentValidation', 'collation', 'comment', 'explain', 'hint', 'let', 'maxAwaitTimeMS', 'maxTimeMS'], 'bulkWrite' => ['let', 'requests', 'session', 'ordered', 'bypassDocumentValidation', 'comment'], 'createChangeStream' => ['pipeline', 'session', 'fullDocument', 'fullDocumentBeforeChange', 'resumeAfter', 'startAfter', 'startAtOperationTime', 'batchSize', 'collation', 'maxAwaitTimeMS', 'comment', 'showExpandedEvents'], 'createFindCursor' => ['filter', 'session', 'allowDiskUse', 'allowPartialResults', 'batchSize', 'collation', 'comment', 'cursorType', 'hint', 'limit', 'max', 'maxAwaitTimeMS', 'maxScan', 'maxTimeMS', 'min', 'modifiers', 'noCursorTimeout', 'oplogReplay', 'projection', 'returnKey', 'showRecordId', 'skip', 'snapshot', 'sort'],