Cleanup + add unit tests

craftcms · Jan 11, 2025 · 9124e9d · 9124e9d
1 parent 8735a83
commit 9124e9d
Show file tree

Hide file tree

Showing 12 changed files with 223 additions and 219 deletions.
diff --git a/phpstan.neon b/phpstan.neon
@@ -17,7 +17,6 @@ parameters:
     scanFiles:
         - lib/craft/behaviors/CustomFieldBehavior.php
         - tests/_support/_generated/AcceptanceTesterActions.php
-        - tests/_support/_generated/ApiTesterActions.php
         - tests/_support/_generated/FunctionalTesterActions.php
         - tests/_support/_generated/GqlTesterActions.php
         - tests/_support/_generated/UnitTesterActions.php

diff --git a/src/events/RedirectRuleEvent.php → src/events/RedirectEvent.php b/src/events/RedirectRuleEvent.php → src/events/RedirectEvent.php
@@ -11,11 +11,12 @@
 use craft\web\RedirectRule;
 
 /**
- * RedirectRuleEvent class.
+ * RedirectEvent class.
  *
  * @author Pixel & Tonic, Inc. <[email protected]>
+ * @since 5.6.0
  */
-class RedirectRuleEvent extends Event
+class RedirectEvent extends Event
 {
-    public RedirectRule $redirectRule;
+    public RedirectRule $rule;
 }
diff --git a/src/helpers/StringHelper.php b/src/helpers/StringHelper.php
@@ -33,7 +33,6 @@ class StringHelper extends \yii\helpers\StringHelper
      * @since 3.0.37
      */
     public const UUID_PATTERN = '[A-Za-z0-9]{8}-[A-Za-z0-9]{4}-4[A-Za-z0-9]{3}-[89abAB][A-Za-z0-9]{3}-[A-Za-z0-9]{12}';
-    public const HANDLE_PATTERN = '(?:[a-zA-Z][a-zA-Z0-9_]*)';
 
     /**
      * @var array Character mappings
@@ -1360,33 +1359,6 @@ public static function slugify(string $str, string $replacement = '-', ?string $
         return (string)BaseStringy::create($str)->slugify($replacement, $language);
     }
 
-    /**
-     * @return string The regex pattern for a slug.
-     */
-    public static function slugPattern(): string
-    {
-        $slugChars = ['.', '_', '-'];
-        $slugWordSeparator = Craft::$app->getConfig()->getGeneral()->slugWordSeparator;
-
-        if ($slugWordSeparator !== '/' && !in_array($slugWordSeparator, $slugChars, true)) {
-            $slugChars[] = $slugWordSeparator;
-        }
-
-        return '(?:[\p{L}\p{N}\p{M}' . preg_quote(implode($slugChars), '/') . ']+)';
-    }
-
-    /**
-     * @return array An array of regex tokens as keys and patterns as values.
-     */
-    public static function regexTokens(): array
-    {
-        return [
-            '{handle}' => self::HANDLE_PATTERN,
-            '{slug}' => self::slugPattern(),
-            '{uid}' => self::UUID_PATTERN,
-        ];
-    }
-
     /**
      * Splits a string into chunks on a given delimiter.
      *

diff --git a/src/web/ErrorHandler.php b/src/web/ErrorHandler.php
@@ -9,7 +9,7 @@
 
 use Craft;
 use craft\events\ExceptionEvent;
-use craft\events\RedirectRuleEvent;
+use craft\events\RedirectEvent;
 use craft\helpers\App;
 use craft\helpers\Json;
 use craft\helpers\Template;
@@ -41,6 +41,7 @@ class ErrorHandler extends \yii\web\ErrorHandler
 
     /**
      * @event RedirectEvent The event that is triggered before a 404 redirect.
+     * @since 5.6.0
      */
     public const EVENT_BEFORE_REDIRECT = 'beforeRedirect';
 
@@ -66,25 +67,30 @@ public function handleException($exception): void
             $redirectRules = Craft::$app->getConfig()->getConfigFromFile('redirects');
             if ($redirectRules) {
                 foreach ($redirectRules as $from => $rule) {
-                    $callback = function(RedirectRule $redirectRule) {
-                        $this->trigger(
-                            self::EVENT_BEFORE_REDIRECT,
-                            new RedirectRuleEvent(['redirectRule' => $redirectRule])
-                        );
-                    };
-
-                    if ($rule instanceof RedirectRule) {
-                        $rule($callback);
-                        continue;
+                    if (!$rule instanceof RedirectRule) {
+                        $config = is_string($rule) ? ['to' => $rule] : $rule;
+                        $rule = Craft::createObject([
+                            'class' => RedirectRule::class,
+                            'from' => $from,
+                            ...$config,
+                        ]);
                     }
 
-                    $config = is_string($rule) ? ['to' => $rule] : $rule;
+                    $url = $rule->getMatch();
+
+                    if ($url === null) {
+                        continue;
+                    }
 
-                    if (!isset($config['from']) && is_string($from)) {
-                        $config['from'] = $from;
+                    if ($this->hasEventHandlers(self::EVENT_BEFORE_REDIRECT)) {
+                        $this->trigger(self::EVENT_BEFORE_REDIRECT, new RedirectEvent([
+                            'rule' => $rule,
+                        ]));
+                        ;
                     }
 
-                    Craft::createObject(RedirectRule::class, [$config])($callback);
+                    Craft::$app->getResponse()->redirect($url, $rule->statusCode);
+                    Craft::$app->end();
                 }
             }
 

diff --git a/src/web/RedirectRule.php b/src/web/RedirectRule.php
@@ -7,44 +7,48 @@
 
 namespace craft\web;
 
+use Closure;
 use Craft;
-use craft\helpers\StringHelper;
 use Illuminate\Support\Collection;
 use League\Uri\Http;
+use yii\base\Component;
 
-class RedirectRule extends \yii\base\BaseObject
+/**
+ * Class RedirectRule
+ *
+ * @author Pixel & Tonic, Inc. <[email protected]>
+ * @since 5.6.0
+ */
+class RedirectRule extends Component
 {
+    /**
+     * @event \yii\base\Event The event that is triggered before redirecting the request.
+     */
+    public const EVENT_BEFORE_REDIRECT = 'beforeRedirect';
+
     public string $to;
     public string $from;
     public int $statusCode = 302;
     public bool $caseSensitive = false;
-    private \Closure $_match;
+    private Closure $_match;
     private array $regexTokens = [];
 
-    public function __invoke(?callable $callback = null): void
+    public function __invoke(): void
     {
-        $to = $this->getMatch();
+        $url = $this->getMatch();
 
-        if ($to === null) {
+        if ($url === null) {
             return;
         }
 
-        if ($callback) {
-            $callback($this);
+        if ($this->hasEventHandlers(self::EVENT_BEFORE_REDIRECT)) {
+            $this->trigger(self::EVENT_BEFORE_REDIRECT);
         }
 
-        Craft::$app->getResponse()->redirect(
-            $to,
-            $this->statusCode,
-        );
+        Craft::$app->getResponse()->redirect($url, $this->statusCode);
         Craft::$app->end();
     }
 
-    public function setMatch(\Closure $match): void
-    {
-        $this->_match = $match;
-    }
-
     public function getMatch(): ?string
     {
         if (isset($this->_match)) {
@@ -72,6 +76,11 @@ public function getMatch(): ?string
         return strcasecmp($this->from, $subject) === 0 ? $this->to : null;
     }
 
+    public function setMatch(callable $match): void
+    {
+        $this->_match = $match;
+    }
+
     private function replaceParams(string $value, array $params): string
     {
         $params = Collection::make($params)
@@ -85,7 +94,7 @@ private function toRegexPattern(string $from): string
         // Tokenize the patterns first, so we only escape regex chars outside of patterns
         $tokenizedPattern = preg_replace_callback('/<([\w._-]+):?([^>]+)?>/', function($match) {
             $name = $match[1];
-            $pattern = strtr($match[2] ?? '[^\/]+', StringHelper::regexTokens());
+            $pattern = strtr($match[2] ?? '[^\/]+', UrlRule::regexTokens());
             $token = "<$name>";
             $this->regexTokens[$token] = "(?P<$name>$pattern)";
 

diff --git a/src/web/UrlRule.php b/src/web/UrlRule.php
@@ -7,8 +7,10 @@
 
 namespace craft\web;
 
+use Craft;
 use craft\helpers\ArrayHelper;
 use craft\helpers\StringHelper;
+use craft\validators\HandleValidator;
 
 /**
  * @inheritdoc
@@ -17,6 +19,28 @@
  */
 class UrlRule extends \yii\web\UrlRule
 {
+    /**
+     * Returns an array of regex tokens supported by URL rules.
+     *
+     * @return array
+     * @since 5.6.0
+     */
+    public static function regexTokens(): array
+    {
+        $slugChars = ['.', '_', '-'];
+        $slugWordSeparator = Craft::$app->getConfig()->getGeneral()->slugWordSeparator;
+        if ($slugWordSeparator !== '/' && !in_array($slugWordSeparator, $slugChars, true)) {
+            $slugChars[] = $slugWordSeparator;
+        }
+
+        return [
+            '{handle}' => sprintf('(?:%s)', HandleValidator::$handlePattern),
+            // Reference: http://www.regular-expressions.info/unicode.html
+            '{slug}' => sprintf('(?:[\p{L}\p{N}\p{M}%s]+)', preg_quote(implode($slugChars), '/')),
+            '{uid}' => sprintf('(?:%s)', StringHelper::UUID_PATTERN),
+        ];
+    }
+
     /**
      * @var array Pattern tokens that will be swapped out at runtime.
      */
@@ -49,8 +73,7 @@ public function __construct(array $config = [])
         if (isset($config['pattern'])) {
             // Swap out any regex tokens in the pattern
             if (!isset(self::$_regexTokens)) {
-                // Reference: http://www.regular-expressions.info/unicode.html
-                self::$_regexTokens = StringHelper::regexTokens();
+                self::$_regexTokens = static::regexTokens();
             }
 
             $config['pattern'] = strtr($config['pattern'], self::$_regexTokens);

diff --git a/tests/_craft/config/redirects.php b/tests/_craft/config/redirects.php
diff --git a/tests/_support/ApiTester.php b/tests/_support/ApiTester.php
diff --git a/tests/acceptance.suite.yml b/tests/acceptance.suite.yml
@@ -7,7 +7,6 @@
 actor: AcceptanceTester
 modules:
   enabled:
+    - PhpBrowser:
+        url: http://localhost/myapp
     - \Helper\Acceptance
-    - REST:
-        url: 'https://craft-5-project.ddev.site/'
-        depends: PhpBrowser
diff --git a/tests/api.suite.yml b/tests/api.suite.yml