From 9124e9d9030dd9181378687629d3bdf80942f530 Mon Sep 17 00:00:00 2001 From: brandonkelly Date: Sat, 11 Jan 2025 09:40:46 -0800 Subject: [PATCH] Cleanup + add unit tests --- phpstan.neon | 1 - ...edirectRuleEvent.php => RedirectEvent.php} | 7 +- src/helpers/StringHelper.php | 28 ---- src/web/ErrorHandler.php | 36 +++-- src/web/RedirectRule.php | 45 +++--- src/web/UrlRule.php | 27 +++- tests/_craft/config/redirects.php | 31 ---- tests/_support/ApiTester.php | 28 ---- tests/acceptance.suite.yml | 5 +- tests/api.suite.yml | 25 --- tests/api/RedirectCest.php | 65 -------- tests/unit/web/RedirectRuleTest.php | 144 ++++++++++++++++++ 12 files changed, 223 insertions(+), 219 deletions(-) rename src/events/{RedirectRuleEvent.php => RedirectEvent.php} (73%) delete mode 100644 tests/_craft/config/redirects.php delete mode 100644 tests/_support/ApiTester.php delete mode 100644 tests/api.suite.yml delete mode 100644 tests/api/RedirectCest.php create mode 100644 tests/unit/web/RedirectRuleTest.php diff --git a/phpstan.neon b/phpstan.neon index 0e60a85862e..3e36217f0c2 100644 --- 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 b/src/events/RedirectEvent.php similarity index 73% rename from src/events/RedirectRuleEvent.php rename to src/events/RedirectEvent.php index 3704daf4748..3e9912fe495 100644 --- a/src/events/RedirectRuleEvent.php +++ b/src/events/RedirectEvent.php @@ -11,11 +11,12 @@ use craft\web\RedirectRule; /** - * RedirectRuleEvent class. + * RedirectEvent class. * * @author Pixel & Tonic, Inc. + * @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 index 8630530aa54..8c88be210b9 100644 --- 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 index fb4f64d3f37..a8bb386d218 100644 --- 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 index 9ba5052263d..efc359a0793 100644 --- 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. + * @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 index e68e6cc213f..648a91a3659 100644 --- 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 deleted file mode 100644 index e88225433f9..00000000000 --- a/tests/_craft/config/redirects.php +++ /dev/null @@ -1,31 +0,0 @@ - 'redirect/to', - - // Path match with Yii URL Rule named parameters - // https://www.yiiframework.com/doc/guide/2.0/en/runtime-routing#named-parameters - 'redirect/from/foo/' => 'redirect/to/', - - 'redirect/from/$special.chars' => 'redirect/to', - - // Path match (case-sensitive) - [ - 'from' => 'redirect/FROM//', - 'to' => 'https://redirect.to//', - 'caseSensitive' => true, - ], - - // Custom match callback - [ - 'match' => function(\Psr\Http\Message\UriInterface $url): ?string { - parse_str($url->getQuery(), $params); - - return isset($params['bar']) - ? sprintf('redirect/to/%s', $params['bar']) - : null; - }, - 'statusCode' => 301, - ], -]; diff --git a/tests/_support/ApiTester.php b/tests/_support/ApiTester.php deleted file mode 100644 index 438cc5e3b2d..00000000000 --- a/tests/_support/ApiTester.php +++ /dev/null @@ -1,28 +0,0 @@ -stopFollowingRedirects(); - $I->sendGet($example['fromPath'], $example['fromParams'] ?? []); - $I->seeResponseCodeIs($example['statusCode']); - if (isset($example['to'])) { - $I->haveHttpHeader('Location', $example['to']); - } - } - - /** - * @phpstan-ignore-next-line - */ - private function redirectDataProvider(): array - { - return [ - [ - 'fromPath' => '/redirect/from', - 'to' => 'https://craft-5-project.ddev.site/redirect/to', - 'statusCode' => 302, - ], - [ - 'fromPath' => '/redirect/from/$special.chars', - 'to' => 'https://craft-5-project.ddev.site/redirect/to', - 'statusCode' => 302, - ], - [ - 'fromPath' => '/redirect/from/1234/56', - 'statusCode' => 404, - ], - [ - 'fromPath' => '/redirect/FROM/1234/56', - 'to' => 'https://redirect.to/1234/56', - 'statusCode' => 302, - ], - [ - 'fromPath' => '/foo', - 'fromParams' => ['bar' => 'baz'], - 'to' => 'https://craft-5-project.ddev.site/redirect/to/baz', - 'statusCode' => 301, - ], - ]; - } -} diff --git a/tests/unit/web/RedirectRuleTest.php b/tests/unit/web/RedirectRuleTest.php new file mode 100644 index 00000000000..6b220417bb5 --- /dev/null +++ b/tests/unit/web/RedirectRuleTest.php @@ -0,0 +1,144 @@ + + * @since 5.6.0 + */ +class RedirectRuleTest extends TestCase +{ + /** + * @var UnitTester + */ + protected UnitTester $tester; + + /** + * @dataProvider getMatchDataProvider + * @param string|null $expected + * @param array $config + */ + public function testGetMatch(?string $expected, string $path, ?string $url, array $config): void + { + $this->tester->mockCraftMethods('request', [ + 'getFullPath' => $path, + 'getAbsoluteUrl' => $url, + ]); + $rule = new RedirectRule($config); + $this->assertSame($expected, $rule->getMatch()); + } + + public static function getMatchDataProvider(): array + { + return [ + // Path match (case-insensitive by default) + [ + null, + 'nope', + UrlHelper::url('nope'), + [ + 'from' => 'redirect/from', + 'to' => 'redirect/to', + ], + ], + [ + 'redirect/to', + 'redirect/from', + UrlHelper::url('redirect/from'), + [ + 'from' => 'redirect/from', + 'to' => 'redirect/to', + ], + ], + [ + 'redirect/to', + 'redirect/from/$special.chars', + UrlHelper::url('redirect/from/$special.chars'), + [ + 'from' => 'redirect/from/$special.chars', + 'to' => 'redirect/to', + ], + ], + + // Path match with Yii URL Rule named parameters + // https://www.yiiframework.com/doc/guide/2.0/en/runtime-routing#named-parameters + [ + 'redirect/to/abc123', + 'redirect/from/foo/abc123', + UrlHelper::url('redirect/from/foo/abc123'), + [ + 'from' => 'redirect/from/foo/', + 'to' => 'redirect/to/', + ], + ], + [ + 'redirect/to/abc-123', + 'redirect/from/foo/abc-123', + UrlHelper::url('redirect/from/foo/abc-123'), + [ + 'from' => 'redirect/from/foo/', + 'to' => 'redirect/to/', + ], + ], + [ + 'redirect/to/55a89943-19a6-4f5e-8db7-8950f7f66e98', + 'redirect/from/foo/55a89943-19a6-4f5e-8db7-8950f7f66e98', + UrlHelper::url('redirect/from/foo/55a89943-19a6-4f5e-8db7-8950f7f66e98'), + [ + 'from' => 'redirect/from/foo/', + 'to' => 'redirect/to/', + ], + ], + + // Path match (case-sensitive) + [ + 'https://redirect.to/2025/01', + 'redirect/FROM/2025/01', + UrlHelper::url('redirect/FROM/2025/01'), + [ + 'from' => 'redirect/FROM//', + 'to' => 'https://redirect.to//', + 'caseSensitive' => true, + ], + ], + [ + null, + 'redirect/from/2025/01', + UrlHelper::url('redirect/from/2025/01'), + [ + 'from' => 'redirect/FROM//', + 'to' => 'https://redirect.to//', + 'caseSensitive' => true, + ], + ], + + // Custom match callback + [ + 'redirect/to/abc123', + 'redirect/from', + UrlHelper::url('redirect/from', ['bar' => 'abc123']), + [ + 'match' => function(UriInterface $url): ?string { + parse_str($url->getQuery(), $params); + return isset($params['bar']) + ? sprintf('redirect/to/%s', $params['bar']) + : null; + }, + ], + ], + ]; + } +}