PHPLIB-1192: Remove useCursor option in aggregate (#1130)

* PHPLIB-1192: Remove useCursor option in aggregate * Remove unnecessary property for isExplain
mongodb · Jul 17, 2023 · 5aff53f · 5aff53f
1 parent f81b4a6
commit 5aff53f
Show file tree

Hide file tree

Showing 8 changed files with 13 additions and 105 deletions.
diff --git 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
@@ -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
-<class.mongodb-driver-cursor>` object is returned. If ``useCursor`` is
-``false``, an :php:`ArrayIterator <arrayiterator>` is returned that wraps the
-``result`` array from the command response document. In both cases, the return
-value will be :php:`Traversable <traversable>`.
+:phpmethod:`MongoDB\\Collection::aggregate()`'s returns a
+:php:`MongoDB\\Driver\\Cursor <class.mongodb-driver-cursor>` object.
 
 Examples
 --------

diff --git 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
@@ -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
@@ -17,15 +17,13 @@
 
 namespace MongoDB\Operation;
 
-use MongoDB\Driver\Cursor;
 use MongoDB\Driver\Exception\RuntimeException as DriverRuntimeException;
 use MongoDB\Driver\Server;
 use MongoDB\Exception\InvalidArgumentException;
 use MongoDB\Exception\UnexpectedValueException;
 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
@@ -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
@@ -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
@@ -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'],