PHPLIB-1238: Order internal methods after non-internal methods (#1162)

mongodb · Sep 15, 2023 · 21781c9 · 21781c9
1 parent fdd7793
commit 21781c9
Show file tree

Hide file tree

Showing 6 changed files with 144 additions and 150 deletions.
diff --git a/src/ChangeStream.php b/src/ChangeStream.php
@@ -87,22 +87,6 @@ class ChangeStream implements Iterator
 
     private ?DocumentCodec $codec;
 
-    /**
-     * @internal
-     *
-     * @param ResumeCallable $resumeCallable
-     */
-    public function __construct(ChangeStreamIterator $iterator, callable $resumeCallable, ?DocumentCodec $codec = null)
-    {
-        $this->iterator = $iterator;
-        $this->resumeCallable = $resumeCallable;
-        $this->codec = $codec;
-
-        if ($codec) {
-            $this->iterator->getInnerIterator()->setTypeMap(['root' => 'bson']);
-        }
-    }
-
     /**
      * @see https://php.net/iterator.current
      * @return array|object|null
@@ -200,6 +184,22 @@ public function valid()
         return $this->iterator->valid();
     }
 
+    /**
+     * @internal
+     *
+     * @param ResumeCallable $resumeCallable
+     */
+    public function __construct(ChangeStreamIterator $iterator, callable $resumeCallable, ?DocumentCodec $codec = null)
+    {
+        $this->iterator = $iterator;
+        $this->resumeCallable = $resumeCallable;
+        $this->codec = $codec;
+
+        if ($codec) {
+            $this->iterator->getInnerIterator()->setTypeMap(['root' => 'bson']);
+        }
+    }
+
     /**
      * Determines if an exception is a resumable error.
      *

diff --git a/src/MapReduceResult.php b/src/MapReduceResult.php
@@ -50,20 +50,6 @@ class MapReduceResult implements IteratorAggregate
 
     private array $timing;
 
-    /**
-     * @internal
-     * @param callable $getIterator Callback that returns a Traversable for mapReduce results
-     * @param stdClass $result      Result document from the mapReduce command
-     * @psalm-param MapReduceCallable $getIterator
-     */
-    public function __construct(callable $getIterator, stdClass $result)
-    {
-        $this->getIterator = $getIterator;
-        $this->executionTimeMS = isset($result->timeMillis) ? (integer) $result->timeMillis : 0;
-        $this->counts = isset($result->counts) ? (array) $result->counts : [];
-        $this->timing = isset($result->timing) ? (array) $result->timing : [];
-    }
-
     /**
      * Returns various count statistics from the mapReduce command.
      *
@@ -108,4 +94,18 @@ public function getTiming()
     {
         return $this->timing;
     }
+
+    /**
+     * @internal
+     * @param callable $getIterator Callback that returns a Traversable for mapReduce results
+     * @param stdClass $result      Result document from the mapReduce command
+     * @psalm-param MapReduceCallable $getIterator
+     */
+    public function __construct(callable $getIterator, stdClass $result)
+    {
+        $this->getIterator = $getIterator;
+        $this->executionTimeMS = isset($result->timeMillis) ? (integer) $result->timeMillis : 0;
+        $this->counts = isset($result->counts) ? (array) $result->counts : [];
+        $this->timing = isset($result->timing) ? (array) $result->timing : [];
+    }
 }
diff --git a/src/Model/BSONIterator.php b/src/Model/BSONIterator.php
@@ -51,34 +51,6 @@ class BSONIterator implements Iterator
 
     private array $options;
 
-    /**
-     * Constructs a BSON Iterator.
-     *
-     * Supported options:
-     *
-     *  * typeMap (array): Type map for BSON deserialization.
-     *
-     * @internal
-     * @see https://php.net/manual/en/function.mongodb.bson-tophp.php
-     * @param string $data    Concatenated, valid, BSON-encoded documents
-     * @param array  $options Iterator options
-     * @throws InvalidArgumentException for parameter/option parsing errors
-     */
-    public function __construct(string $data, array $options = [])
-    {
-        if (isset($options['typeMap']) && ! is_array($options['typeMap'])) {
-            throw InvalidArgumentException::invalidType('"typeMap" option', $options['typeMap'], 'array');
-        }
-
-        if (! isset($options['typeMap'])) {
-            $options['typeMap'] = [];
-        }
-
-        $this->buffer = $data;
-        $this->bufferLength = strlen($data);
-        $this->options = $options;
-    }
-
     /**
      * @see https://php.net/iterator.current
      * @return mixed
@@ -131,6 +103,34 @@ public function valid(): bool
         return $this->current !== null;
     }
 
+    /**
+     * Constructs a BSON Iterator.
+     *
+     * Supported options:
+     *
+     *  * typeMap (array): Type map for BSON deserialization.
+     *
+     * @internal
+     * @see https://php.net/manual/en/function.mongodb.bson-tophp.php
+     * @param string $data    Concatenated, valid, BSON-encoded documents
+     * @param array  $options Iterator options
+     * @throws InvalidArgumentException for parameter/option parsing errors
+     */
+    public function __construct(string $data, array $options = [])
+    {
+        if (isset($options['typeMap']) && ! is_array($options['typeMap'])) {
+            throw InvalidArgumentException::invalidType('"typeMap" option', $options['typeMap'], 'array');
+        }
+
+        if (! isset($options['typeMap'])) {
+            $options['typeMap'] = [];
+        }
+
+        $this->buffer = $data;
+        $this->bufferLength = strlen($data);
+        $this->options = $options;
+    }
+
     private function advance(): void
     {
         if ($this->position === $this->bufferLength) {

diff --git a/src/Model/ChangeStreamIterator.php b/src/Model/ChangeStreamIterator.php
@@ -68,71 +68,6 @@ class ChangeStreamIterator extends IteratorIterator implements CommandSubscriber
 
     private Server $server;
 
-    /**
-     * @internal
-     * @param array|object|null $initialResumeToken
-     * @psalm-param CursorInterface<int, TValue>&Iterator<int, TValue> $cursor
-     */
-    public function __construct(CursorInterface $cursor, int $firstBatchSize, $initialResumeToken, ?object $postBatchResumeToken)
-    {
-        if (! $cursor instanceof Iterator) {
-            throw InvalidArgumentException::invalidType(
-                '$cursor',
-                $cursor,
-                CursorInterface::class . '&' . Iterator::class,
-            );
-        }
-
-        if (isset($initialResumeToken) && ! is_document($initialResumeToken)) {
-            throw InvalidArgumentException::expectedDocumentType('$initialResumeToken', $initialResumeToken);
-        }
-
-        parent::__construct($cursor);
-
-        $this->batchSize = $firstBatchSize;
-        $this->isRewindNop = ($firstBatchSize === 0);
-        $this->postBatchResumeToken = $postBatchResumeToken;
-        $this->resumeToken = $initialResumeToken;
-        $this->server = $cursor->getServer();
-    }
-
-    /** @internal */
-    final public function commandFailed(CommandFailedEvent $event): void
-    {
-    }
-
-    /** @internal */
-    final public function commandStarted(CommandStartedEvent $event): void
-    {
-        if ($event->getCommandName() !== 'getMore') {
-            return;
-        }
-
-        $this->batchPosition = 0;
-        $this->batchSize = 0;
-        $this->postBatchResumeToken = null;
-    }
-
-    /** @internal */
-    final public function commandSucceeded(CommandSucceededEvent $event): void
-    {
-        if ($event->getCommandName() !== 'getMore') {
-            return;
-        }
-
-        $reply = $event->getReply();
-
-        if (! isset($reply->cursor->nextBatch) || ! is_array($reply->cursor->nextBatch)) {
-            throw new UnexpectedValueException('getMore command did not return a "cursor.nextBatch" array');
-        }
-
-        $this->batchSize = count($reply->cursor->nextBatch);
-
-        if (isset($reply->cursor->postBatchResumeToken) && is_object($reply->cursor->postBatchResumeToken)) {
-            $this->postBatchResumeToken = $reply->cursor->postBatchResumeToken;
-        }
-    }
-
     /**
      * @see https://php.net/iteratoriterator.current
      * @return array|object|null
@@ -239,6 +174,71 @@ public function valid(): bool
         return $this->isValid;
     }
 
+    /**
+     * @internal
+     * @param array|object|null $initialResumeToken
+     * @psalm-param CursorInterface<int, TValue>&Iterator<int, TValue> $cursor
+     */
+    public function __construct(CursorInterface $cursor, int $firstBatchSize, $initialResumeToken, ?object $postBatchResumeToken)
+    {
+        if (! $cursor instanceof Iterator) {
+            throw InvalidArgumentException::invalidType(
+                '$cursor',
+                $cursor,
+                CursorInterface::class . '&' . Iterator::class,
+            );
+        }
+
+        if (isset($initialResumeToken) && ! is_document($initialResumeToken)) {
+            throw InvalidArgumentException::expectedDocumentType('$initialResumeToken', $initialResumeToken);
+        }
+
+        parent::__construct($cursor);
+
+        $this->batchSize = $firstBatchSize;
+        $this->isRewindNop = ($firstBatchSize === 0);
+        $this->postBatchResumeToken = $postBatchResumeToken;
+        $this->resumeToken = $initialResumeToken;
+        $this->server = $cursor->getServer();
+    }
+
+    /** @internal */
+    final public function commandFailed(CommandFailedEvent $event): void
+    {
+    }
+
+    /** @internal */
+    final public function commandStarted(CommandStartedEvent $event): void
+    {
+        if ($event->getCommandName() !== 'getMore') {
+            return;
+        }
+
+        $this->batchPosition = 0;
+        $this->batchSize = 0;
+        $this->postBatchResumeToken = null;
+    }
+
+    /** @internal */
+    final public function commandSucceeded(CommandSucceededEvent $event): void
+    {
+        if ($event->getCommandName() !== 'getMore') {
+            return;
+        }
+
+        $reply = $event->getReply();
+
+        if (! isset($reply->cursor->nextBatch) || ! is_array($reply->cursor->nextBatch)) {
+            throw new UnexpectedValueException('getMore command did not return a "cursor.nextBatch" array');
+        }
+
+        $this->batchSize = count($reply->cursor->nextBatch);
+
+        if (isset($reply->cursor->postBatchResumeToken) && is_object($reply->cursor->postBatchResumeToken)) {
+            $this->postBatchResumeToken = $reply->cursor->postBatchResumeToken;
+        }
+    }
+
     /**
      * Extracts the resume token (i.e. "_id" field) from a change document.
      *

diff --git a/src/Operation/Watch.php b/src/Operation/Watch.php
@@ -274,6 +274,23 @@ public function __construct(Manager $manager, ?string $databaseName, ?string $co
         $this->aggregate = $this->createAggregate();
     }
 
+    /**
+     * Execute the operation.
+     *
+     * @see Executable::execute()
+     * @return ChangeStream
+     * @throws UnsupportedException if collation or read concern is used and unsupported
+     * @throws RuntimeException for other driver errors (e.g. connection errors)
+     */
+    public function execute(Server $server)
+    {
+        return new ChangeStream(
+            $this->createChangeStreamIterator($server),
+            fn ($resumeToken, $hasAdvanced): ChangeStreamIterator => $this->resume($resumeToken, $hasAdvanced),
+            $this->codec,
+        );
+    }
+
     /** @internal */
     final public function commandFailed(CommandFailedEvent $event): void
     {
@@ -317,23 +334,6 @@ final public function commandSucceeded(CommandSucceededEvent $event): void
         }
     }
 
-    /**
-     * Execute the operation.
-     *
-     * @see Executable::execute()
-     * @return ChangeStream
-     * @throws UnsupportedException if collation or read concern is used and unsupported
-     * @throws RuntimeException for other driver errors (e.g. connection errors)
-     */
-    public function execute(Server $server)
-    {
-        return new ChangeStream(
-            $this->createChangeStreamIterator($server),
-            fn ($resumeToken, $hasAdvanced): ChangeStreamIterator => $this->resume($resumeToken, $hasAdvanced),
-            $this->codec,
-        );
-    }
-
     /**
      * Create the aggregate command for a change stream.
      *

diff --git a/tests/PedantryTest.php b/tests/PedantryTest.php
@@ -11,6 +11,7 @@
 use function array_filter;
 use function array_map;
 use function realpath;
+use function str_contains;
 use function str_replace;
 use function strcasecmp;
 use function strlen;
@@ -37,17 +38,10 @@ public function testMethodsAreOrderedAlphabeticallyByVisibility($className): voi
         );
 
         $getSortValue = function (ReflectionMethod $method) {
-            if ($method->getModifiers() & ReflectionMethod::IS_PRIVATE) {
-                return '2' . $method->getName();
-            }
-
-            if ($method->getModifiers() & ReflectionMethod::IS_PROTECTED) {
-                return '1' . $method->getName();
-            }
+            $prefix = $method->isPrivate() ? '2' : ($method->isProtected() ? '1' : '0');
+            $prefix .= str_contains($method->getDocComment(), '@internal') ? '1' : '0';
 
-            if ($method->getModifiers() & ReflectionMethod::IS_PUBLIC) {
-                return '0' . $method->getName();
-            }
+            return $prefix . $method->getName();
         };
 
         $sortedMethods = $methods;