From d72f1446146bed998de2c2df9e6f2da3228883c8 Mon Sep 17 00:00:00 2001
From: TavoNiievez <ganieves@outlook.com>
Date: Fri, 28 Feb 2025 12:48:19 -0500
Subject: [PATCH] fix: improve exception handling and code consistency

     - Standardized `RuntimeException` usage by removing redundant namespace references.
     - Refactored `findAndLoginUser` for clarity using ternary assignment.
     - Used `readonly` property for `ignoreCollidingDSN` in `TransactionForcer`.
     - Optimized server params handling by introducing `getServerParams()`.
     - Improved configuration validation messages for clean and mail methods.
---
 src/Codeception/Lib/Connector/Yii2.php        | 26 ++++------
 src/Codeception/Lib/Connector/Yii2/Logger.php |  9 ++--
 .../Lib/Connector/Yii2/TransactionForcer.php  | 17 +++----
 src/Codeception/Module/Yii2.php               | 49 ++++++++-----------
 4 files changed, 43 insertions(+), 58 deletions(-)

diff --git a/src/Codeception/Lib/Connector/Yii2.php b/src/Codeception/Lib/Connector/Yii2.php
index 8be068c..8662836 100644
--- a/src/Codeception/Lib/Connector/Yii2.php
+++ b/src/Codeception/Lib/Connector/Yii2.php
@@ -10,6 +10,7 @@
 use Codeception\Lib\Connector\Yii2\TestMailer;
 use Codeception\Util\Debug;
 use InvalidArgumentException;
+use RuntimeException;
 use Symfony\Component\BrowserKit\AbstractBrowser as Client;
 use Symfony\Component\BrowserKit\Cookie;
 use Symfony\Component\BrowserKit\CookieJar;
@@ -142,14 +143,14 @@ protected function getApplication(): \yii\base\Application
         if (! isset(Yii::$app)) {
             $this->startApp();
         }
-        return Yii::$app ?? throw new \RuntimeException('Failed to create Yii2 application');
+        return Yii::$app ?? throw new RuntimeException('Failed to create Yii2 application');
     }
 
     private function getWebRequest(): YiiRequest
     {
         $request = $this->getApplication()->request;
         if (! $request instanceof YiiRequest) {
-            throw new \RuntimeException('Request component is not of type ' . YiiRequest::class);
+            throw new RuntimeException('Request component is not of type ' . YiiRequest::class);
         }
         return $request;
     }
@@ -172,8 +173,7 @@ public function resetApplication(bool $closeSession = true): void
      * Finds and logs in a user
      *
      * @internal
-     * @throws   ConfigurationException
-     * @throws   \RuntimeException
+     * @throws   ConfigurationException|RuntimeException
      */
     public function findAndLoginUser(int|string|IdentityInterface $user): void
     {
@@ -183,15 +183,9 @@ public function findAndLoginUser(int|string|IdentityInterface $user): void
             throw new ConfigurationException('The user component is not configured');
         }
 
-        if ($user instanceof IdentityInterface) {
-            $identity = $user;
-        } else {
-            // class name implementing IdentityInterface
-            $identityClass = $userComponent->identityClass;
-            $identity = $identityClass::findIdentity($user);
-            if ($identity === null) {
-                throw new \RuntimeException('User not found');
-            }
+        $identity = $user instanceof IdentityInterface ? $user : ($userComponent->identityClass)::findIdentity($user);
+        if ($identity === null) {
+            throw new RuntimeException('User not found');
         }
         $userComponent->login($identity);
     }
@@ -273,7 +267,7 @@ function ($matches) use (&$parameters): string {
             );
         }
         if ($template === null) {
-            throw new \RuntimeException("Failed to parse domain regex");
+            throw new RuntimeException("Failed to parse domain regex");
         }
         $template = preg_quote($template);
         $template = strtr($template, $parameters);
@@ -411,9 +405,9 @@ public function doRequest(object $request): Response
 
         $content = ob_get_clean();
         if (empty($content) && ! empty($yiiResponse->content) && ! isset($yiiResponse->stream)) {
-            throw new \RuntimeException('No content was sent from Yii application');
+            throw new RuntimeException('No content was sent from Yii application');
         } elseif ($content === false) {
-            throw new \RuntimeException('Failed to get output buffer');
+            throw new RuntimeException('Failed to get output buffer');
         }
 
         return new Response($content, $yiiResponse->statusCode, $yiiResponse->getHeaders()->toArray());
diff --git a/src/Codeception/Lib/Connector/Yii2/Logger.php b/src/Codeception/Lib/Connector/Yii2/Logger.php
index 69ebd4e..2443c39 100644
--- a/src/Codeception/Lib/Connector/Yii2/Logger.php
+++ b/src/Codeception/Lib/Connector/Yii2/Logger.php
@@ -5,6 +5,7 @@
 namespace Codeception\Lib\Connector\Yii2;
 
 use Codeception\Util\Debug;
+use SplQueue;
 use yii\base\Exception as YiiException;
 use yii\helpers\VarDumper;
 use yii\log\Logger as YiiLogger;
@@ -12,9 +13,9 @@
 final class Logger extends YiiLogger
 {
     /**
-     * @var \SplQueue<string>
+     * @var SplQueue<string>
      */
-    private \SplQueue $logQueue;
+    private SplQueue $logQueue;
 
     /**
      * @param array<string, mixed> $config
@@ -24,7 +25,7 @@ public function __construct(
         array $config = []
     ) {
         parent::__construct($config);
-        $this->logQueue = new \SplQueue();
+        $this->logQueue = new SplQueue();
     }
 
     public function init(): void
@@ -69,7 +70,7 @@ public function log($message, $level, $category = 'application'): void
     public function getAndClearLog(): string
     {
         $logs = iterator_to_array($this->logQueue);
-        $this->logQueue = new \SplQueue();
+        $this->logQueue = new SplQueue();
         return implode(PHP_EOL, $logs) . PHP_EOL;
     }
 }
diff --git a/src/Codeception/Lib/Connector/Yii2/TransactionForcer.php b/src/Codeception/Lib/Connector/Yii2/TransactionForcer.php
index e93ac44..1322f86 100644
--- a/src/Codeception/Lib/Connector/Yii2/TransactionForcer.php
+++ b/src/Codeception/Lib/Connector/Yii2/TransactionForcer.php
@@ -29,7 +29,7 @@ final class TransactionForcer extends ConnectionWatcher
     private array $transactions = [];
 
     public function __construct(
-        private bool $ignoreCollidingDSN
+        private readonly bool $ignoreCollidingDSN
     ) {
         parent::__construct();
     }
@@ -45,12 +45,12 @@ protected function connectionOpened(Connection $connection): void
         $key = md5(
             json_encode(
                 [
-                'dsn' => $connection->dsn,
-                'user' => $connection->username,
-                'pass' => $connection->password,
-                'attributes' => $connection->attributes,
-                'emulatePrepare' => $connection->emulatePrepare,
-                'charset' => $connection->charset,
+                    'dsn' => $connection->dsn,
+                    'user' => $connection->username,
+                    'pass' => $connection->password,
+                    'attributes' => $connection->attributes,
+                    'emulatePrepare' => $connection->emulatePrepare,
+                    'charset' => $connection->charset,
                 ],
                 JSON_THROW_ON_ERROR
             )
@@ -87,9 +87,6 @@ protected function connectionOpened(Connection $connection): void
 
     public function rollbackAll(): void
     {
-        /**
-         * @var Transaction $transaction
-         */
         foreach ($this->transactions as $transaction) {
             if ($transaction->db->isActive) {
                 $transaction->rollBack();
diff --git a/src/Codeception/Module/Yii2.php b/src/Codeception/Module/Yii2.php
index 4abfb65..54c289d 100644
--- a/src/Codeception/Module/Yii2.php
+++ b/src/Codeception/Module/Yii2.php
@@ -15,6 +15,7 @@
 use Codeception\Lib\Interfaces\ActiveRecord;
 use Codeception\Lib\Interfaces\PartedModule;
 use Codeception\TestInterface;
+use Exception;
 use PHPUnit\Framework\Assert;
 use ReflectionClass;
 use RuntimeException;
@@ -276,7 +277,9 @@ public function _initialize(): void
 
         $this->defineConstants();
         $this->server = $_SERVER;
-        $this->initServerGlobal();
+        // Adds the required server params. Note this is done separately from the request cycle since someone might call
+        // `Url::to` before doing a request, which would instantiate the request component with incorrect server params.
+        $_SERVER = [...$_SERVER, $this->getServerParams()];
     }
 
     /**
@@ -294,26 +297,27 @@ protected function onReconfigure(): void
     }
 
     /**
-     * Adds the required server params.
-     * Note this is done separately from the request cycle since someone might call
-     * `Url::to` before doing a request, which would instantiate the request component with incorrect server params.
+     * @return array{
+     *     SCRIPT_FILENAME: string,
+     *     SCRIPT_NAME: string,
+     *     SERVER_NAME: string,
+     *     SERVER_PORT: string|int,
+     *     HTTPS: bool
+     * }
      */
-    private function initServerGlobal(): void
+    private function getServerParams(): array
     {
         $entryUrl = $this->config['entryUrl'];
         $parsedUrl = parse_url($entryUrl);
         $entryFile = $this->config['entryScript'] ?: basename($entryUrl);
         $entryScript = $this->config['entryScript'] ?: ($parsedUrl['path'] ?? '');
-        $_SERVER = array_merge(
-            $_SERVER,
-            [
+        return [
             'SCRIPT_FILENAME' => $entryFile,
             'SCRIPT_NAME' => $entryScript,
             'SERVER_NAME' => $parsedUrl['host'] ?? '',
             'SERVER_PORT' => $parsedUrl['port'] ?? '80',
             'HTTPS' => isset($parsedUrl['scheme']) && $parsedUrl['scheme'] === 'https',
-            ]
-        );
+        ];
     }
 
     /**
@@ -335,23 +339,24 @@ protected function validateConfig(): void
                 "The application config file does not exist: " . $pathToConfig,
             );
         }
-        $validMethods = implode(", ", Yii2Connector::CLEAN_METHODS);
+        $validCleanMethods = implode(", ", Yii2Connector::CLEAN_METHODS);
         if (! in_array($this->config['responseCleanMethod'], Yii2Connector::CLEAN_METHODS, true)) {
             throw new ModuleConfigException(
                 self::class,
-                "The response clean method must be one of: " . $validMethods,
+                "The response clean method must be one of: " . $validCleanMethods,
             );
         }
+        $validMailMethods = implode(", ", Yii2Connector::MAIL_METHODS);
         if (! in_array($this->config['mailMethod'], Yii2Connector::MAIL_METHODS, true)) {
             throw new ModuleConfigException(
                 self::class,
-                "The mail method must be one of: " . $validMethods
+                "The mail method must be one of: " . $validMailMethods
             );
         }
         if (! in_array($this->config['requestCleanMethod'], Yii2Connector::CLEAN_METHODS, true)) {
             throw new ModuleConfigException(
                 self::class,
-                "The request clean method must be one of: " . $validMethods,
+                "The request clean method must be one of: " . $validCleanMethods,
             );
         }
     }
@@ -377,19 +382,7 @@ private function configureClient(array $settings): void
      */
     protected function recreateClient(): void
     {
-        $entryUrl = $this->config['entryUrl'];
-        $parsedUrl = parse_url($entryUrl);
-        $entryFile = $this->config['entryScript'] ?: basename($entryUrl);
-        $entryScript = $this->config['entryScript'] ?: ($parsedUrl['path'] ?? '');
-        $this->client = new Yii2Connector(
-            [
-            'SCRIPT_FILENAME' => $entryFile,
-            'SCRIPT_NAME' => $entryScript,
-            'SERVER_NAME' => $parsedUrl['host'] ?? '',
-            'SERVER_PORT' => $parsedUrl['port'] ?? '80',
-            'HTTPS' => isset($parsedUrl['scheme']) && $parsedUrl['scheme'] === 'https',
-            ]
-        );
+        $this->client = new Yii2Connector($this->getServerParams());
         $this->validateConfig();
         $this->configureClient($this->config);
     }
@@ -463,7 +456,7 @@ public function _after(TestInterface $test): void
     }
 
     /**
-     * @param \Exception $fail
+     * @param Exception $fail
      */
     public function _failed(TestInterface $test, $fail): void
     {