From 2a4f2007f5ae36747a2001194a7f700e5fdbd914 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Fri, 3 Nov 2023 17:33:02 +0300 Subject: [PATCH 1/5] Make `Route`, `Group` and `MatchingResult` dispatcher independent (#222) Co-authored-by: Rustam Mamadaminov --- .github/workflows/rector.yml | 4 +- .phpstorm.meta.php/Group.php | 9 +- .phpstorm.meta.php/Route.php | 11 +- CHANGELOG.md | 1 + UPGRADE.md | 34 +++++ src/Debug/DebugRoutesCommand.php | 3 + src/Debug/RouterCollector.php | 3 + src/Debug/UrlMatcherInterfaceProxy.php | 3 + src/Group.php | 107 +++++++------- src/Internal/MiddlewareFilter.php | 33 +++++ src/MatchingResult.php | 42 +----- src/Middleware/Router.php | 6 +- src/Route.php | 136 ++++++++--------- src/RouteCollection.php | 4 +- tests/GroupTest.php | 196 ++++++++++++------------- tests/MatchingResultTest.php | 62 -------- tests/RouteCollectionTest.php | 62 ++++---- tests/RouteTest.php | 169 ++++++++++++--------- 18 files changed, 444 insertions(+), 441 deletions(-) create mode 100644 UPGRADE.md create mode 100644 src/Internal/MiddlewareFilter.php diff --git a/.github/workflows/rector.yml b/.github/workflows/rector.yml index adacd73..bd79331 100644 --- a/.github/workflows/rector.yml +++ b/.github/workflows/rector.yml @@ -14,8 +14,10 @@ name: rector jobs: rector: uses: yiisoft/actions/.github/workflows/rector.yml@master + secrets: + token: ${{ secrets.YIISOFT_GITHUB_TOKEN }} with: os: >- ['ubuntu-latest'] php: >- - ['8.0'] + ['8.2'] diff --git a/.phpstorm.meta.php/Group.php b/.phpstorm.meta.php/Group.php index 70eb3db..c2f0bb0 100644 --- a/.phpstorm.meta.php/Group.php +++ b/.phpstorm.meta.php/Group.php @@ -11,9 +11,8 @@ 'host', 'hosts', 'corsMiddleware', - 'items', - 'middlewareDefinitions', - 'hasDispatcher', - 'hasCorsMiddleware' + 'routes', + 'hasCorsMiddleware', + 'enabledMiddlewares', ); -} \ No newline at end of file +} diff --git a/.phpstorm.meta.php/Route.php b/.phpstorm.meta.php/Route.php index e30885c..4a7df73 100644 --- a/.phpstorm.meta.php/Route.php +++ b/.phpstorm.meta.php/Route.php @@ -7,14 +7,13 @@ registerArgumentsSet( 'routeDataKeys', 'name', + 'pattern', 'host', 'hosts', - 'pattern', 'methods', - 'override', 'defaults', - 'dispatcherWithMiddlewares', - 'hasDispatcher', - 'hasMiddlewares' + 'override', + 'hasMiddlewares', + 'enabledMiddlewares', ); -} \ No newline at end of file +} diff --git a/CHANGELOG.md b/CHANGELOG.md index fda7b26..9eafa70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - New #195: Add debug collector for `yiisoft/yii-debug` (@xepozz) - Chg #207: Replace two `RouteCollectorInterface` methods `addRoute()` and `addGroup()` to single `addRoute()` (@vjik) - Enh #202: Add support for `psr/http-message` version `^2.0` (@vjik) +- Chg #222: Make `Route`, `Group` and `MatchingResult` dispatcher-independent (@rustamwin, @vjik) ## 3.0.0 February 17, 2023 diff --git a/UPGRADE.md b/UPGRADE.md new file mode 100644 index 0000000..4647ada --- /dev/null +++ b/UPGRADE.md @@ -0,0 +1,34 @@ +# Upgrading Instructions for Yii Router + +This file contains the upgrade notes for the Yii Router. +These notes highlight changes that could break your application when you upgrade it from one major version to another. + +## 4.0.0 + +In this release classes `Route`, `Group` and `MatchingResult` are made dispatcher-independent. Now you don't can inject +own middleware dispatcher to group or to route. + +The following backward incompatible changes have been made. + +### `Route` + +- Removed parameter `$dispatcher` from `Route` creating methods: `get()`, `post()`, `put()`, `delete()`, `patch()`, + `head()`, `options()`, `methods()`. +- Removed methods `Route::injectDispatcher()` and `Route::withDispatcher()`. +- `Route::getData()` changes: + - removed elements `dispatcherWithMiddlewares` and `hasDispatcher`; + - added element `enabledMiddlewares`. + +### `Group` + +- Removed parameter `$dispatcher` from `Group::create()` method. +- Removed method `Group::withDispatcher()`. +- `Group::getData()` changes: + - removed element `hasDispatcher`; + - key `items` renamed to `routes`; + - key `middlewareDefinitions` renamed to `enabledMiddlewares`. + +### `MatchingResult` + +- Removed `MatchingResult` implementation from `MiddlewareInterface`, so it is no longer middleware. +- Removed method `MatchingResult::process()`. diff --git a/src/Debug/DebugRoutesCommand.php b/src/Debug/DebugRoutesCommand.php index fc02a0a..4a6fe74 100644 --- a/src/Debug/DebugRoutesCommand.php +++ b/src/Debug/DebugRoutesCommand.php @@ -14,6 +14,9 @@ use Yiisoft\VarDumper\VarDumper; use Yiisoft\Yii\Debug\Debugger; +/** + * @codeCoverageIgnore + */ final class DebugRoutesCommand extends Command { public const COMMAND_NAME = 'debug:routes'; diff --git a/src/Debug/RouterCollector.php b/src/Debug/RouterCollector.php index 99e7110..e9dfc59 100644 --- a/src/Debug/RouterCollector.php +++ b/src/Debug/RouterCollector.php @@ -12,6 +12,9 @@ use Yiisoft\Yii\Debug\Collector\CollectorTrait; use Yiisoft\Yii\Debug\Collector\SummaryCollectorInterface; +/** + * @codeCoverageIgnore + */ final class RouterCollector implements SummaryCollectorInterface { use CollectorTrait; diff --git a/src/Debug/UrlMatcherInterfaceProxy.php b/src/Debug/UrlMatcherInterfaceProxy.php index db6ab19..bbe625c 100644 --- a/src/Debug/UrlMatcherInterfaceProxy.php +++ b/src/Debug/UrlMatcherInterfaceProxy.php @@ -8,6 +8,9 @@ use Yiisoft\Router\MatchingResult; use Yiisoft\Router\UrlMatcherInterface; +/** + * @codeCoverageIgnore + */ final class UrlMatcherInterfaceProxy implements UrlMatcherInterface { public function __construct(private UrlMatcherInterface $urlMatcher, private RouterCollector $routerCollector) diff --git a/src/Group.php b/src/Group.php index ad9e44d..0099cb1 100644 --- a/src/Group.php +++ b/src/Group.php @@ -6,7 +6,7 @@ use InvalidArgumentException; use RuntimeException; -use Yiisoft\Middleware\Dispatcher\MiddlewareDispatcher; +use Yiisoft\Router\Internal\MiddlewareFilter; use function in_array; @@ -15,12 +15,13 @@ final class Group /** * @var Group[]|Route[] */ - private array $items = []; + private array $routes = []; /** * @var array[]|callable[]|string[] + * @psalm-var list */ - private array $middlewareDefinitions = []; + private array $middlewares = []; /** * @var string[] @@ -29,28 +30,31 @@ final class Group private ?string $namePrefix = null; private bool $routesAdded = false; private bool $middlewareAdded = false; - private array $disabledMiddlewareDefinitions = []; + private array $disabledMiddlewares = []; + + /** + * @psalm-var list|null + */ + private ?array $enabledMiddlewaresCache = null; /** * @var array|callable|string|null Middleware definition for CORS requests. */ private $corsMiddleware = null; - private function __construct(private ?string $prefix = null, private ?MiddlewareDispatcher $dispatcher = null) - { + private function __construct( + private ?string $prefix = null + ) { } /** * Create a new group instance. * * @param string|null $prefix URL prefix to prepend to all routes of the group. - * @param MiddlewareDispatcher|null $dispatcher Middleware dispatcher to use for the group. */ - public static function create( - ?string $prefix = null, - MiddlewareDispatcher $dispatcher = null - ): self { - return new self($prefix, $dispatcher); + public static function create(?string $prefix = null): self + { + return new self($prefix); } public function routes(self|Route ...$routes): self @@ -58,33 +62,14 @@ public function routes(self|Route ...$routes): self if ($this->middlewareAdded) { throw new RuntimeException('routes() can not be used after prependMiddleware().'); } - $new = clone $this; - foreach ($routes as $route) { - if ($new->dispatcher !== null && !$route->getData('hasDispatcher')) { - $route = $route->withDispatcher($new->dispatcher); - } - $new->items[] = $route; - } + $new = clone $this; + $new->routes = $routes; $new->routesAdded = true; return $new; } - public function withDispatcher(MiddlewareDispatcher $dispatcher): self - { - $group = clone $this; - $group->dispatcher = $dispatcher; - foreach ($group->items as $index => $item) { - if (!$item->getData('hasDispatcher')) { - $item = $item->withDispatcher($dispatcher); - $group->items[$index] = $item; - } - } - - return $group; - } - /** * Adds a middleware definition that handles CORS requests. * If set, routes for {@see Method::OPTIONS} request will be added automatically. @@ -103,16 +88,20 @@ public function withCors(array|callable|string|null $middlewareDefinition): self * Appends a handler middleware definition that should be invoked for a matched route. * First added handler will be executed first. */ - public function middleware(array|callable|string ...$middlewareDefinition): self + public function middleware(array|callable|string ...$definition): self { if ($this->routesAdded) { throw new RuntimeException('middleware() can not be used after routes().'); } + $new = clone $this; array_push( - $new->middlewareDefinitions, - ...array_values($middlewareDefinition) + $new->middlewares, + ...array_values($definition) ); + + $new->enabledMiddlewaresCache = null; + return $new; } @@ -120,14 +109,17 @@ public function middleware(array|callable|string ...$middlewareDefinition): self * Prepends a handler middleware definition that should be invoked for a matched route. * First added handler will be executed last. */ - public function prependMiddleware(array|callable|string ...$middlewareDefinition): self + public function prependMiddleware(array|callable|string ...$definition): self { $new = clone $this; array_unshift( - $new->middlewareDefinitions, - ...array_values($middlewareDefinition) + $new->middlewares, + ...array_values($definition) ); + $new->middlewareAdded = true; + $new->enabledMiddlewaresCache = null; + return $new; } @@ -163,13 +155,16 @@ public function hosts(string ...$hosts): self * It is useful to avoid invoking one of the parent group middleware for * a certain route. */ - public function disableMiddleware(mixed ...$middlewareDefinition): self + public function disableMiddleware(mixed ...$definition): self { $new = clone $this; array_push( - $new->disabledMiddlewareDefinitions, - ...array_values($middlewareDefinition), + $new->disabledMiddlewares, + ...array_values($definition), ); + + $new->enabledMiddlewaresCache = null; + return $new; } @@ -180,10 +175,10 @@ public function disableMiddleware(mixed ...$middlewareDefinition): self * * @psalm-return ( * T is ('prefix'|'namePrefix'|'host') ? string|null : - * (T is 'items' ? Group[]|Route[] : + * (T is 'routes' ? Group[]|Route[] : * (T is 'hosts' ? array : - * (T is ('hasCorsMiddleware'|'hasDispatcher') ? bool : - * (T is 'middlewareDefinitions' ? list : + * (T is ('hasCorsMiddleware') ? bool : + * (T is 'enabledMiddlewares' ? list : * (T is 'corsMiddleware' ? array|callable|string|null : mixed) * ) * ) @@ -199,23 +194,25 @@ public function getData(string $key): mixed 'host' => $this->hosts[0] ?? null, 'hosts' => $this->hosts, 'corsMiddleware' => $this->corsMiddleware, - 'items' => $this->items, + 'routes' => $this->routes, 'hasCorsMiddleware' => $this->corsMiddleware !== null, - 'hasDispatcher' => $this->dispatcher !== null, - 'middlewareDefinitions' => $this->getMiddlewareDefinitions(), + 'enabledMiddlewares' => $this->getEnabledMiddlewares(), default => throw new InvalidArgumentException('Unknown data key: ' . $key), }; } - private function getMiddlewareDefinitions(): array + /** + * @return array[]|callable[]|string[] + * @psalm-return list + */ + private function getEnabledMiddlewares(): array { - /** @var mixed $definition */ - foreach ($this->middlewareDefinitions as $index => $definition) { - if (in_array($definition, $this->disabledMiddlewareDefinitions, true)) { - unset($this->middlewareDefinitions[$index]); - } + if ($this->enabledMiddlewaresCache !== null) { + return $this->enabledMiddlewaresCache; } - return array_values($this->middlewareDefinitions); + $this->enabledMiddlewaresCache = MiddlewareFilter::filter($this->middlewares, $this->disabledMiddlewares); + + return $this->enabledMiddlewaresCache; } } diff --git a/src/Internal/MiddlewareFilter.php b/src/Internal/MiddlewareFilter.php new file mode 100644 index 0000000..e70f6f2 --- /dev/null +++ b/src/Internal/MiddlewareFilter.php @@ -0,0 +1,33 @@ + $middlewares + * @psalm-return list + */ + public static function filter(array $middlewares, array $disabledMiddlewares): array + { + $result = []; + + foreach ($middlewares as $middleware) { + if (in_array($middleware, $disabledMiddlewares, true)) { + continue; + } + + $result[] = $middleware; + } + + return $result; + } +} diff --git a/src/MatchingResult.php b/src/MatchingResult.php index 30c5918..997d8dd 100644 --- a/src/MatchingResult.php +++ b/src/MatchingResult.php @@ -4,18 +4,14 @@ namespace Yiisoft\Router; -use Psr\Http\Message\ResponseInterface; -use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Server\MiddlewareInterface; -use Psr\Http\Server\RequestHandlerInterface; use RuntimeException; use Yiisoft\Http\Method; -use Yiisoft\Middleware\Dispatcher\MiddlewareDispatcher; -final class MatchingResult implements MiddlewareInterface +final class MatchingResult { /** - * @var array + * @var string[] + * @psalm-var array */ private array $arguments = []; @@ -24,21 +20,13 @@ final class MatchingResult implements MiddlewareInterface */ private array $methods = []; - private ?MiddlewareDispatcher $dispatcher = null; - private function __construct(private ?Route $route) { } - public function withDispatcher(MiddlewareDispatcher $dispatcher): self - { - $new = clone $this; - $new->dispatcher = $dispatcher; - return $new; - } - /** - * @param array $arguments + * @param string[] $arguments + * @psalm-param array $arguments */ public static function fromSuccess(Route $route, array $arguments): self { @@ -71,7 +59,8 @@ public function isMethodFailure(): bool } /** - * @return array + * @return string[] + * @psalm-return array */ public function arguments(): array { @@ -94,21 +83,4 @@ public function route(): Route return $this->route; } - - public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface - { - if (!$this->isSuccess()) { - return $handler->handle($request); - } - - // Inject dispatcher only if we have not previously injected. - // This improves performance in event-loop applications. - if ($this->dispatcher !== null && !$this->route->getData('hasDispatcher')) { - $this->route->injectDispatcher($this->dispatcher); - } - - return $this->route - ->getData('dispatcherWithMiddlewares') - ->dispatch($request, $handler); - } } diff --git a/src/Middleware/Router.php b/src/Middleware/Router.php index 631823f..644bc8c 100644 --- a/src/Middleware/Router.php +++ b/src/Middleware/Router.php @@ -54,8 +54,8 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface $this->currentRoute->setRouteWithArguments($result->route(), $result->arguments()); - return $result - ->withDispatcher($this->dispatcher) - ->process($request, $handler); + return $this->dispatcher + ->withMiddlewares($result->route()->getData('enabledMiddlewares')) + ->dispatch($request, $handler); } } diff --git a/src/Route.php b/src/Route.php index bed2bc7..d594070 100644 --- a/src/Route.php +++ b/src/Route.php @@ -8,7 +8,7 @@ use RuntimeException; use Stringable; use Yiisoft\Http\Method; -use Yiisoft\Middleware\Dispatcher\MiddlewareDispatcher; +use Yiisoft\Router\Internal\MiddlewareFilter; use function in_array; @@ -28,10 +28,16 @@ final class Route implements Stringable /** * @var array[]|callable[]|string[] + * @psalm-var list */ - private array $middlewareDefinitions = []; + private array $middlewares = []; - private array $disabledMiddlewareDefinitions = []; + private array $disabledMiddlewares = []; + + /** + * @psalm-var list|null + */ + private ?array $enabledMiddlewaresCache = null; /** * @var array @@ -44,69 +50,50 @@ final class Route implements Stringable private function __construct( private array $methods, private string $pattern, - private ?MiddlewareDispatcher $dispatcher = null ) { } - /** - * @psalm-assert MiddlewareDispatcher $this->dispatcher - */ - public function injectDispatcher(MiddlewareDispatcher $dispatcher): void - { - $this->dispatcher = $dispatcher; - } - - public function withDispatcher(MiddlewareDispatcher $dispatcher): self + public static function get(string $pattern): self { - $route = clone $this; - $route->dispatcher = $dispatcher; - return $route; + return self::methods([Method::GET], $pattern); } - public static function get(string $pattern, ?MiddlewareDispatcher $dispatcher = null): self + public static function post(string $pattern): self { - return self::methods([Method::GET], $pattern, $dispatcher); + return self::methods([Method::POST], $pattern); } - public static function post(string $pattern, ?MiddlewareDispatcher $dispatcher = null): self + public static function put(string $pattern): self { - return self::methods([Method::POST], $pattern, $dispatcher); + return self::methods([Method::PUT], $pattern); } - public static function put(string $pattern, ?MiddlewareDispatcher $dispatcher = null): self + public static function delete(string $pattern): self { - return self::methods([Method::PUT], $pattern, $dispatcher); + return self::methods([Method::DELETE], $pattern); } - public static function delete(string $pattern, ?MiddlewareDispatcher $dispatcher = null): self + public static function patch(string $pattern): self { - return self::methods([Method::DELETE], $pattern, $dispatcher); + return self::methods([Method::PATCH], $pattern); } - public static function patch(string $pattern, ?MiddlewareDispatcher $dispatcher = null): self + public static function head(string $pattern): self { - return self::methods([Method::PATCH], $pattern, $dispatcher); + return self::methods([Method::HEAD], $pattern); } - public static function head(string $pattern, ?MiddlewareDispatcher $dispatcher = null): self + public static function options(string $pattern): self { - return self::methods([Method::HEAD], $pattern, $dispatcher); - } - - public static function options(string $pattern, ?MiddlewareDispatcher $dispatcher = null): self - { - return self::methods([Method::OPTIONS], $pattern, $dispatcher); + return self::methods([Method::OPTIONS], $pattern); } /** * @param string[] $methods */ - public static function methods( - array $methods, - string $pattern, - ?MiddlewareDispatcher $dispatcher = null - ): self { - return new self($methods, $pattern, $dispatcher); + public static function methods(array $methods, string $pattern): self + { + return new self($methods, $pattern); } public function name(string $name): self @@ -170,16 +157,20 @@ public function defaults(array $defaults): self * Appends a handler middleware definition that should be invoked for a matched route. * First added handler will be executed first. */ - public function middleware(array|callable|string ...$middlewareDefinition): self + public function middleware(array|callable|string ...$definition): self { if ($this->actionAdded) { throw new RuntimeException('middleware() can not be used after action().'); } + $route = clone $this; array_push( - $route->middlewareDefinitions, - ...array_values($middlewareDefinition) + $route->middlewares, + ...array_values($definition) ); + + $route->enabledMiddlewaresCache = null; + return $route; } @@ -187,16 +178,20 @@ public function middleware(array|callable|string ...$middlewareDefinition): self * Prepends a handler middleware definition that should be invoked for a matched route. * Last added handler will be executed first. */ - public function prependMiddleware(array|callable|string ...$middlewareDefinition): self + public function prependMiddleware(array|callable|string ...$definition): self { if (!$this->actionAdded) { throw new RuntimeException('prependMiddleware() can not be used before action().'); } + $route = clone $this; array_unshift( - $route->middlewareDefinitions, - ...array_values($middlewareDefinition) + $route->middlewares, + ...array_values($definition) ); + + $route->enabledMiddlewaresCache = null; + return $route; } @@ -206,7 +201,7 @@ public function prependMiddleware(array|callable|string ...$middlewareDefinition public function action(array|callable|string $middlewareDefinition): self { $route = clone $this; - $route->middlewareDefinitions[] = $middlewareDefinition; + $route->middlewares[] = $middlewareDefinition; $route->actionAdded = true; return $route; } @@ -216,13 +211,16 @@ public function action(array|callable|string $middlewareDefinition): self * It is useful to avoid invoking one of the parent group middleware for * a certain route. */ - public function disableMiddleware(mixed ...$middlewareDefinition): self + public function disableMiddleware(mixed ...$definition): self { $route = clone $this; array_push( - $route->disabledMiddlewareDefinitions, - ...array_values($middlewareDefinition) + $route->disabledMiddlewares, + ...array_values($definition) ); + + $route->enabledMiddlewaresCache = null; + return $route; } @@ -237,8 +235,8 @@ public function disableMiddleware(mixed ...$middlewareDefinition): self * (T is 'hosts' ? array : * (T is 'methods' ? array : * (T is 'defaults' ? array : - * (T is ('override'|'hasMiddlewares'|'hasDispatcher') ? bool : - * (T is 'dispatcherWithMiddlewares' ? MiddlewareDispatcher : mixed) + * (T is ('override'|'hasMiddlewares') ? bool : + * (T is 'enabledMiddlewares' ? array : mixed) * ) * ) * ) @@ -257,9 +255,8 @@ public function getData(string $key): mixed 'methods' => $this->methods, 'defaults' => $this->defaults, 'override' => $this->override, - 'dispatcherWithMiddlewares' => $this->getDispatcherWithMiddlewares(), - 'hasMiddlewares' => $this->middlewareDefinitions !== [], - 'hasDispatcher' => $this->dispatcher !== null, + 'hasMiddlewares' => $this->middlewares !== [], + 'enabledMiddlewares' => $this->getEnabledMiddlewares(), default => throw new InvalidArgumentException('Unknown data key: ' . $key), }; } @@ -297,31 +294,24 @@ public function __debugInfo() 'defaults' => $this->defaults, 'override' => $this->override, 'actionAdded' => $this->actionAdded, - 'middlewareDefinitions' => $this->middlewareDefinitions, - 'disabledMiddlewareDefinitions' => $this->disabledMiddlewareDefinitions, - 'middlewareDispatcher' => $this->dispatcher, + 'middlewares' => $this->middlewares, + 'disabledMiddlewares' => $this->disabledMiddlewares, + 'enabledMiddlewares' => $this->getEnabledMiddlewares(), ]; } - private function getDispatcherWithMiddlewares(): MiddlewareDispatcher + /** + * @return array[]|callable[]|string[] + * @psalm-return list + */ + private function getEnabledMiddlewares(): array { - if ($this->dispatcher === null) { - throw new RuntimeException(sprintf('There is no dispatcher in the route %s.', $this->getData('name'))); + if ($this->enabledMiddlewaresCache !== null) { + return $this->enabledMiddlewaresCache; } - // Don't add middlewares to dispatcher if we did it earlier. - // This improves performance in event-loop applications. - if ($this->dispatcher->hasMiddlewares()) { - return $this->dispatcher; - } - - /** @var mixed $definition */ - foreach ($this->middlewareDefinitions as $index => $definition) { - if (in_array($definition, $this->disabledMiddlewareDefinitions, true)) { - unset($this->middlewareDefinitions[$index]); - } - } + $this->enabledMiddlewaresCache = MiddlewareFilter::filter($this->middlewares, $this->disabledMiddlewares); - return $this->dispatcher = $this->dispatcher->withMiddlewares($this->middlewareDefinitions); + return $this->enabledMiddlewaresCache; } } diff --git a/src/RouteCollection.php b/src/RouteCollection.php index 629ed54..01be086 100644 --- a/src/RouteCollection.php +++ b/src/RouteCollection.php @@ -104,12 +104,12 @@ private function injectGroup(Group $group, array &$tree, string $prefix = '', st { $prefix .= (string) $group->getData('prefix'); $namePrefix .= (string) $group->getData('namePrefix'); - $items = $group->getData('items'); + $items = $group->getData('routes'); $pattern = null; $hosts = []; foreach ($items as $item) { if (!$this->isStaticRoute($item)) { - $item = $item->prependMiddleware(...$group->getData('middlewareDefinitions')); + $item = $item->prependMiddleware(...$group->getData('enabledMiddlewares')); } if (!empty($group->getData('hosts')) && empty($item->getData('hosts'))) { diff --git a/tests/GroupTest.php b/tests/GroupTest.php index 9e8999c..c74eaf9 100644 --- a/tests/GroupTest.php +++ b/tests/GroupTest.php @@ -23,7 +23,6 @@ use Yiisoft\Router\Tests\Support\TestMiddleware1; use Yiisoft\Router\Tests\Support\TestMiddleware2; use Yiisoft\Router\Tests\Support\TestMiddleware3; -use Yiisoft\Test\Support\Container\SimpleContainer; final class GroupTest extends TestCase { @@ -37,9 +36,9 @@ public function testAddMiddleware(): void $group = $group ->middleware($middleware1) ->middleware($middleware2); - $this->assertCount(2, $group->getData('middlewareDefinitions')); - $this->assertSame($middleware1, $group->getData('middlewareDefinitions')[0]); - $this->assertSame($middleware2, $group->getData('middlewareDefinitions')[1]); + $this->assertCount(2, $group->getData('enabledMiddlewares')); + $this->assertSame($middleware1, $group->getData('enabledMiddlewares')[0]); + $this->assertSame($middleware2, $group->getData('enabledMiddlewares')[1]); } public function testDisabledMiddlewareDefinitions(): void @@ -49,19 +48,78 @@ public function testDisabledMiddlewareDefinitions(): void ->prependMiddleware(TestMiddleware1::class, TestMiddleware2::class) ->disableMiddleware(TestMiddleware1::class, TestMiddleware3::class); - $this->assertCount(1, $group->getData('middlewareDefinitions')); - $this->assertSame(TestMiddleware2::class, $group->getData('middlewareDefinitions')[0]); + $this->assertCount(1, $group->getData('enabledMiddlewares')); + $this->assertSame(TestMiddleware2::class, $group->getData('enabledMiddlewares')[0]); + } + + public function testPrependMiddlewaresAfterGetEnabledMiddlewares(): void + { + $group = Group::create() + ->middleware(TestMiddleware3::class) + ->disableMiddleware(TestMiddleware1::class); + + $group->getData('enabledMiddlewares'); + + $group = $group->prependMiddleware(TestMiddleware1::class, TestMiddleware2::class); + + $this->assertSame( + [TestMiddleware2::class, TestMiddleware3::class], + $group->getData('enabledMiddlewares') + ); + } + + public function testAddMiddlewareAfterGetEnabledMiddlewares(): void + { + $group = Group::create() + ->middleware(TestMiddleware3::class); + + $group->getData('enabledMiddlewares'); + + $group = $group->middleware(TestMiddleware1::class, TestMiddleware2::class); + + $this->assertSame( + [TestMiddleware3::class, TestMiddleware1::class, TestMiddleware2::class], + $group->getData('enabledMiddlewares') + ); + } + + public function testDisableMiddlewareAfterGetEnabledMiddlewares(): void + { + $group = Group::create() + ->middleware(TestMiddleware1::class, TestMiddleware2::class, TestMiddleware3::class); + + $group->getData('enabledMiddlewares'); + + $group = $group->disableMiddleware(TestMiddleware1::class, TestMiddleware2::class); + + $this->assertSame( + [TestMiddleware3::class], + $group->getData('enabledMiddlewares') + ); + } + + public function testMiddlewaresWithKeys(): void + { + $group = Group::create() + ->middleware(m3: TestMiddleware3::class) + ->prependMiddleware(m1: TestMiddleware1::class, m2: TestMiddleware2::class) + ->disableMiddleware(m1: TestMiddleware1::class); + + $this->assertSame( + [TestMiddleware2::class, TestMiddleware3::class], + $group->getData('enabledMiddlewares') + ); } public function testNamedArgumentsInMiddlewareMethods(): void { $group = Group::create() - ->middleware(middleware: TestMiddleware3::class) - ->prependMiddleware(middleware1: TestMiddleware1::class, middleware2: TestMiddleware2::class) - ->disableMiddleware(middleware1: TestMiddleware1::class, middleware2: TestMiddleware3::class); + ->middleware(TestMiddleware3::class) + ->prependMiddleware(TestMiddleware1::class, TestMiddleware2::class) + ->disableMiddleware(TestMiddleware1::class, TestMiddleware3::class); - $this->assertCount(1, $group->getData('middlewareDefinitions')); - $this->assertSame(TestMiddleware2::class, $group->getData('middlewareDefinitions')[0]); + $this->assertCount(1, $group->getData('enabledMiddlewares')); + $this->assertSame(TestMiddleware2::class, $group->getData('enabledMiddlewares')[0]); } public function testRoutesAfterMiddleware(): void @@ -82,7 +140,7 @@ public function testAddNestedMiddleware(): void { $request = new ServerRequest('GET', '/outergroup/innergroup/test1'); - $action = static fn (ServerRequestInterface $request) => new Response(200, [], null, '1.1', implode($request->getAttributes())); + $action = static fn (ServerRequestInterface $request) => new Response(200, [], null, '1.1', implode('', $request->getAttributes())); $middleware1 = static function (ServerRequestInterface $request, RequestHandlerInterface $handler) { $request = $request->withAttribute('middleware', 'middleware1'); @@ -94,7 +152,7 @@ public function testAddNestedMiddleware(): void return $handler->handle($request); }; - $group = Group::create('/outergroup', $this->getDispatcher()) + $group = Group::create('/outergroup') ->middleware($middleware1) ->routes( Group::create('/innergroup') @@ -111,8 +169,8 @@ public function testAddNestedMiddleware(): void $routeCollection = new RouteCollection($collector); $route = $routeCollection->getRoute('request1'); - $response = $route - ->getData('dispatcherWithMiddlewares') + $response = $this->getDispatcher() + ->withMiddlewares($route->getData('enabledMiddlewares')) ->dispatch($request, $this->getRequestHandler()); $this->assertSame(200, $response->getStatusCode()); $this->assertSame('middleware2', $response->getReasonPhrase()); @@ -122,7 +180,7 @@ public function testGroupMiddlewareFullStackCalled(): void { $request = new ServerRequest('GET', '/group/test1'); - $action = static fn (ServerRequestInterface $request) => new Response(200, [], null, '1.1', implode($request->getAttributes())); + $action = static fn (ServerRequestInterface $request) => new Response(200, [], null, '1.1', implode('', $request->getAttributes())); $middleware1 = function (ServerRequestInterface $request, RequestHandlerInterface $handler) { $request = $request->withAttribute('middleware', 'middleware1'); return $handler->handle($request); @@ -132,7 +190,7 @@ public function testGroupMiddlewareFullStackCalled(): void return $handler->handle($request); }; - $group = Group::create('/group', $this->getDispatcher()) + $group = Group::create('/group') ->middleware($middleware1) ->middleware($middleware2) ->routes( @@ -146,9 +204,11 @@ public function testGroupMiddlewareFullStackCalled(): void $routeCollection = new RouteCollection($collector); $route = $routeCollection->getRoute('request1'); - $response = $route - ->getData('dispatcherWithMiddlewares') + + $response = $this->getDispatcher() + ->withMiddlewares($route->getData('enabledMiddlewares')) ->dispatch($request, $this->getRequestHandler()); + $this->assertSame(200, $response->getStatusCode()); $this->assertSame('middleware2', $response->getReasonPhrase()); } @@ -161,7 +221,7 @@ public function testGroupMiddlewareStackInterrupted(): void $middleware1 = fn () => new Response(403); $middleware2 = fn () => new Response(405); - $group = Group::create('/group', $this->getDispatcher()) + $group = Group::create('/group') ->middleware($middleware1) ->middleware($middleware2) ->routes( @@ -175,9 +235,11 @@ public function testGroupMiddlewareStackInterrupted(): void $routeCollection = new RouteCollection($collector); $route = $routeCollection->getRoute('request1'); - $response = $route - ->getData('dispatcherWithMiddlewares') + + $response = $this->getDispatcher() + ->withMiddlewares($route->getData('enabledMiddlewares')) ->dispatch($request, $this->getRequestHandler()); + $this->assertSame(403, $response->getStatusCode()); } @@ -205,27 +267,27 @@ public function testAddGroup(): void ), ); - $this->assertCount(1, $root->getData('items')); + $this->assertCount(1, $root->getData('routes')); /** @var Group $api */ - $api = $root->getData('items')[0]; + $api = $root->getData('routes')[0]; $this->assertSame('/api', $api->getData('prefix')); - $this->assertCount(2, $api->getData('items')); - $this->assertSame($logoutRoute, $api->getData('items')[0]); + $this->assertCount(2, $api->getData('routes')); + $this->assertSame($logoutRoute, $api->getData('routes')[0]); /** @var Group $postGroup */ - $postGroup = $api->getData('items')[1]; + $postGroup = $api->getData('routes')[1]; $this->assertInstanceOf(Group::class, $postGroup); - $this->assertCount(2, $api->getData('middlewareDefinitions')); - $this->assertSame($middleware1, $api->getData('middlewareDefinitions')[0]); - $this->assertSame($middleware2, $api->getData('middlewareDefinitions')[1]); + $this->assertCount(2, $api->getData('enabledMiddlewares')); + $this->assertSame($middleware1, $api->getData('enabledMiddlewares')[0]); + $this->assertSame($middleware2, $api->getData('enabledMiddlewares')[1]); $this->assertSame('/post', $postGroup->getData('prefix')); - $this->assertCount(2, $postGroup->getData('items')); - $this->assertSame($listRoute, $postGroup->getData('items')[0]); - $this->assertSame($viewRoute, $postGroup->getData('items')[1]); - $this->assertEmpty($postGroup->getData('middlewareDefinitions')); + $this->assertCount(2, $postGroup->getData('routes')); + $this->assertSame($listRoute, $postGroup->getData('routes')[0]); + $this->assertSame($viewRoute, $postGroup->getData('routes')[1]); + $this->assertEmpty($postGroup->getData('enabledMiddlewares')); } public function testHost(): void @@ -259,54 +321,6 @@ public function testGetDataWithWrongKey(): void $group->getData('wrong'); } - public function testDispatcherInjected(): void - { - $dispatcher = $this->getDispatcher(); - - $apiGroup = Group::create('/api', $dispatcher) - ->routes( - Route::get('/info')->name('api-info'), - Group::create('/v1') - ->routes( - Route::get('/user')->name('api-v1-user/index'), - Route::get('/user/{id}')->name('api-v1-user/view'), - Group::create('/news') - ->routes( - Route::get('/post')->name('api-v1-news-post/index'), - Route::get('/post/{id}')->name('api-v1-news-post/view'), - ), - Group::create('/blog') - ->routes( - Route::get('/post')->name('api-v1-blog-post/index'), - Route::get('/post/{id}')->name('api-v1-blog-post/view'), - ), - Route::get('/note')->name('api-v1-note/index'), - Route::get('/note/{id}')->name('api-v1-note/view'), - ), - Group::create('/v2') - ->routes( - Route::get('/user')->name('api-v2-user/index'), - Route::get('/user/{id}')->name('api-v2-user/view'), - Group::create('/news') - ->routes( - Route::get('/post')->name('api-v2-news-post/index'), - Route::get('/post/{id}')->name('api-v2-news-post/view'), - Group::create('/blog') - ->routes( - Route::get('/post')->name('api-v2-blog-post/index'), - Route::get('/post/{id}')->name('api-v2-blog-post/view'), - Route::get('/note')->name('api-v2-note/index'), - Route::get('/note/{id}')->name('api-v2-note/view') - ) - ) - ) - ); - - $items = $apiGroup->getData('items'); - - $this->assertAllRoutesAndGroupsHaveDispatcher($items); - } - public function testWithCors(): void { $group = Group::create() @@ -440,15 +454,9 @@ public function testDuplicateHosts(): void public function testImmutability(): void { - $container = new SimpleContainer(); - $middlewareDispatcher = new MiddlewareDispatcher( - new MiddlewareFactory($container), - ); - $group = Group::create(); $this->assertNotSame($group, $group->routes()); - $this->assertNotSame($group, $group->withDispatcher($middlewareDispatcher)); $this->assertNotSame($group, $group->withCors(null)); $this->assertNotSame($group, $group->middleware()); $this->assertNotSame($group, $group->prependMiddleware()); @@ -475,16 +483,4 @@ private function getDispatcher(): MiddlewareDispatcher $this->createMock(EventDispatcherInterface::class) ); } - - private function assertAllRoutesAndGroupsHaveDispatcher(array $items): void - { - $func = function ($item) use (&$func) { - $this->assertTrue($item->getData('hasDispatcher')); - if ($item instanceof Group) { - $items = $item->getData('items'); - array_walk($items, $func); - } - }; - array_walk($items, $func); - } } diff --git a/tests/MatchingResultTest.php b/tests/MatchingResultTest.php index b75ec6b..10d67b8 100644 --- a/tests/MatchingResultTest.php +++ b/tests/MatchingResultTest.php @@ -4,21 +4,11 @@ namespace Yiisoft\Router\Tests; -use Nyholm\Psr7\Response; -use Nyholm\Psr7\ServerRequest; use PHPUnit\Framework\TestCase; -use Psr\Container\ContainerInterface; -use Psr\EventDispatcher\EventDispatcherInterface; -use Psr\Http\Message\ResponseInterface; -use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Server\RequestHandlerInterface; use RuntimeException; use Yiisoft\Http\Method; -use Yiisoft\Middleware\Dispatcher\MiddlewareDispatcher; -use Yiisoft\Middleware\Dispatcher\MiddlewareFactory; use Yiisoft\Router\MatchingResult; use Yiisoft\Router\Route; -use Yiisoft\Test\Support\Container\SimpleContainer; final class MatchingResultTest extends TestCase { @@ -48,31 +38,6 @@ public function testFromFailureOnNotFoundFailure(): void $this->assertFalse($result->isMethodFailure()); } - public function testProcessSuccess(): void - { - $container = $this->createMock(ContainerInterface::class); - $dispatcher = new MiddlewareDispatcher( - new MiddlewareFactory($container), - $this->createMock(EventDispatcherInterface::class) - ); - $route = Route::post('/', $dispatcher)->middleware($this->getMiddleware()); - $result = MatchingResult::fromSuccess($route, []); - $request = new ServerRequest('POST', '/'); - - $response = $result->process($request, $this->getRequestHandler()); - $this->assertSame(201, $response->getStatusCode()); - } - - public function testProcessFailure(): void - { - $request = new ServerRequest('POST', '/'); - - $response = MatchingResult::fromFailure([Method::GET, Method::HEAD]) - ->process($request, $this->getRequestHandler()); - - $this->assertSame(404, $response->getStatusCode()); - } - public function testRouteOnFailure(): void { $result = MatchingResult::fromFailure([Method::GET, Method::HEAD]); @@ -81,31 +46,4 @@ public function testRouteOnFailure(): void $this->expectExceptionMessage('There is no route in the matching result.'); $result->route(); } - - public function testImmutability(): void - { - $container = new SimpleContainer(); - $middlewareDispatcher = new MiddlewareDispatcher( - new MiddlewareFactory($container), - ); - - $result = MatchingResult::fromFailure([Method::GET]); - - $this->assertNotSame($result, $result->withDispatcher($middlewareDispatcher)); - } - - private function getMiddleware(): callable - { - return static fn () => new Response(201); - } - - private function getRequestHandler(): RequestHandlerInterface - { - return new class () implements RequestHandlerInterface { - public function handle(ServerRequestInterface $request): ResponseInterface - { - return new Response(404); - } - }; - } } diff --git a/tests/RouteCollectionTest.php b/tests/RouteCollectionTest.php index 23967fe..72d71b9 100644 --- a/tests/RouteCollectionTest.php +++ b/tests/RouteCollectionTest.php @@ -97,7 +97,7 @@ public function testRouteWithoutAction(): void $group = Group::create() ->middleware(fn () => 1) ->routes( - Route::get('/test', $this->getDispatcher()) + Route::get('/test') ->action(fn () => 2) ->name('test'), Route::get('/images/{sile}')->name('image') @@ -115,19 +115,19 @@ public function testGetRouterTree(): void { $group1 = Group::create('/api') ->routes( - Route::get('/test', $this->getDispatcher()) + Route::get('/test') ->action(fn () => 2) ->name('/test'), Route::get('/images/{sile}')->name('/image'), Group::create('/v1') ->routes( - Route::get('/posts', $this->getDispatcher())->name('/posts'), + Route::get('/posts')->name('/posts'), Route::get('/post/{sile}')->name('/post/view') ) ->namePrefix('/v1'), Group::create('/v1') ->routes( - Route::get('/tags', $this->getDispatcher())->name('/tags'), + Route::get('/tags')->name('/tags'), Route::get('/tag/{slug}')->name('/tag/view'), ) ->namePrefix('/v1'), @@ -135,7 +135,7 @@ public function testGetRouterTree(): void $group2 = Group::create('/api') ->routes( - Route::get('/posts', $this->getDispatcher())->name('/posts'), + Route::get('/posts')->name('/posts'), Route::get('/post/{sile}')->name('/post/view'), ) ->namePrefix('/api'); @@ -168,7 +168,7 @@ public function testGetRoutes(): void $group = Group::create() ->middleware(fn () => 1) ->routes( - Route::get('/test', $this->getDispatcher()) + Route::get('/test') ->action(fn () => 2) ->name('test'), Route::get('/images/{sile}')->name('image') @@ -252,16 +252,16 @@ public function testCollectorMiddlewareFullstackCalled(): void [], null, '1.1', - implode($request->getAttributes()) + implode('', $request->getAttributes()) ); $listRoute = Route::get('/') ->action($action) ->name('list'); - $viewRoute = Route::get('/{id}', $this->getDispatcher()) + $viewRoute = Route::get('/{id}') ->action($action) ->name('view'); - $group = Group::create(null, $this->getDispatcher())->routes($listRoute); + $group = Group::create(null)->routes($listRoute); $middleware = function (ServerRequestInterface $request, RequestHandlerInterface $handler) { $request = $request->withAttribute('middleware', 'middleware1'); @@ -276,11 +276,11 @@ public function testCollectorMiddlewareFullstackCalled(): void $route1 = $routeCollection->getRoute('list'); $route2 = $routeCollection->getRoute('view'); $request = new ServerRequest('GET', '/'); - $response1 = $route1 - ->getData('dispatcherWithMiddlewares') + $response1 = $this->getDispatcher() + ->withMiddlewares($route1->getData('enabledMiddlewares')) ->dispatch($request, $this->getRequestHandler()); - $response2 = $route2 - ->getData('dispatcherWithMiddlewares') + $response2 = $this->getDispatcher() + ->withMiddlewares($route2->getData('enabledMiddlewares')) ->dispatch($request, $this->getRequestHandler()); $this->assertEquals('middleware1', $response1->getReasonPhrase()); @@ -302,15 +302,6 @@ public function testMiddlewaresOrder(bool $groupWrapped): void { $request = new ServerRequest('GET', '/'); - $injectDispatcher = $this->getDispatcher( - new SimpleContainer([ - TestMiddleware1::class => new TestMiddleware1(), - TestMiddleware2::class => new TestMiddleware2(), - TestMiddleware3::class => new TestMiddleware3(), - TestController::class => new TestController(), - ]) - ); - $collector = new RouteCollector(); $collector @@ -327,9 +318,17 @@ public function testMiddlewaresOrder(bool $groupWrapped): void ); $route = (new RouteCollection($collector))->getRoute('main'); - $route->injectDispatcher($injectDispatcher); - $dispatcher = $route->getData('dispatcherWithMiddlewares'); + $dispatcher = $this + ->getDispatcher( + new SimpleContainer([ + TestMiddleware1::class => new TestMiddleware1(), + TestMiddleware2::class => new TestMiddleware2(), + TestMiddleware3::class => new TestMiddleware3(), + TestController::class => new TestController(), + ]) + ) + ->withMiddlewares($route->getData('enabledMiddlewares')); $response = $dispatcher->dispatch($request, $this->getRequestHandler()); $this->assertSame(200, $response->getStatusCode()); @@ -340,12 +339,6 @@ public function testStaticRouteWithCollectorMiddlewares(): void { $request = new ServerRequest('GET', '/'); - $injectDispatcher = $this->getDispatcher( - new SimpleContainer([ - TestMiddleware1::class => new TestMiddleware1(), - ]) - ); - $collector = new RouteCollector(); $collector->middleware(TestMiddleware1::class); @@ -354,9 +347,14 @@ public function testStaticRouteWithCollectorMiddlewares(): void ); $route = (new RouteCollection($collector))->getRoute('image'); - $route->injectDispatcher($injectDispatcher); - $dispatcher = $route->getData('dispatcherWithMiddlewares'); + $dispatcher = $this + ->getDispatcher( + new SimpleContainer([ + TestMiddleware1::class => new TestMiddleware1(), + ]) + ) + ->withMiddlewares($route->getData('enabledMiddlewares')); $this->expectException(RuntimeException::class); $this->expectExceptionMessage('Stack is empty.'); diff --git a/tests/RouteTest.php b/tests/RouteTest.php index d2cc255..92b158b 100644 --- a/tests/RouteTest.php +++ b/tests/RouteTest.php @@ -23,7 +23,6 @@ use Yiisoft\Router\Tests\Support\TestMiddleware2; use Yiisoft\Router\Tests\Support\TestController; use Yiisoft\Router\Tests\Support\TestMiddleware3; -use Yiisoft\Test\Support\Container\SimpleContainer; final class RouteTest extends TestCase { @@ -220,12 +219,14 @@ public function testDispatcherInjecting(): void TestController::class => new TestController(), ] ); - $dispatcher = $this->getDispatcher($container); + $route = Route::get('/')->action([TestController::class, 'index']); - $route->injectDispatcher($dispatcher); - $response = $route - ->getData('dispatcherWithMiddlewares') + + $response = $this + ->getDispatcher($container) + ->withMiddlewares($route->getData('enabledMiddlewares')) ->dispatch($request, $this->getRequestHandler()); + $this->assertSame(200, $response->getStatusCode()); } @@ -251,22 +252,21 @@ public function testDisabledMiddlewareDefinitions(): void { $request = new ServerRequest('GET', '/'); - $injectDispatcher = $this->getDispatcher( - $this->getContainer([ - TestMiddleware1::class => new TestMiddleware1(), - TestMiddleware2::class => new TestMiddleware2(), - TestMiddleware3::class => new TestMiddleware3(), - TestController::class => new TestController(), - ]) - ); - $route = Route::get('/') ->middleware(TestMiddleware1::class, TestMiddleware2::class, TestMiddleware3::class) ->action([TestController::class, 'index']) ->disableMiddleware(TestMiddleware1::class, TestMiddleware3::class); - $route->injectDispatcher($injectDispatcher); - $dispatcher = $route->getData('dispatcherWithMiddlewares'); + $dispatcher = $this + ->getDispatcher( + $this->getContainer([ + TestMiddleware1::class => new TestMiddleware1(), + TestMiddleware2::class => new TestMiddleware2(), + TestMiddleware3::class => new TestMiddleware3(), + TestController::class => new TestController(), + ]) + ) + ->withMiddlewares($route->getData('enabledMiddlewares')); $response = $dispatcher->dispatch($request, $this->getRequestHandler()); $this->assertSame(200, $response->getStatusCode()); @@ -277,63 +277,98 @@ public function testPrependMiddlewareDefinitions(): void { $request = new ServerRequest('GET', '/'); - $injectDispatcher = $this->getDispatcher( - $this->getContainer([ - TestMiddleware1::class => new TestMiddleware1(), - TestMiddleware2::class => new TestMiddleware2(), - TestMiddleware3::class => new TestMiddleware3(), - TestController::class => new TestController(), - ]) - ); - $route = Route::get('/') ->middleware(TestMiddleware3::class) ->action([TestController::class, 'index']) ->prependMiddleware(TestMiddleware1::class, TestMiddleware2::class); - $route->injectDispatcher($injectDispatcher); - $dispatcher = $route->getData('dispatcherWithMiddlewares'); + $response = $this + ->getDispatcher( + $this->getContainer([ + TestMiddleware1::class => new TestMiddleware1(), + TestMiddleware2::class => new TestMiddleware2(), + TestMiddleware3::class => new TestMiddleware3(), + TestController::class => new TestController(), + ]) + ) + ->withMiddlewares($route->getData('enabledMiddlewares')) + ->dispatch($request, $this->getRequestHandler()); - $response = $dispatcher->dispatch($request, $this->getRequestHandler()); $this->assertSame(200, $response->getStatusCode()); $this->assertSame('123', (string) $response->getBody()); } - public function testGetDispatcherWithoutDispatcher(): void + public function testPrependMiddlewaresAfterGetEnabledMiddlewares(): void { - $route = Route::get('/')->name('test'); + $route = Route::get('/') + ->middleware(TestMiddleware3::class) + ->disableMiddleware(TestMiddleware1::class) + ->action([TestController::class, 'index']); - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('There is no dispatcher in the route test.'); - $route->getData('dispatcherWithMiddlewares'); + $route->getData('enabledMiddlewares'); + + $route = $route->prependMiddleware(TestMiddleware1::class, TestMiddleware2::class); + + $this->assertSame( + [TestMiddleware2::class, TestMiddleware3::class, [TestController::class, 'index']], + $route->getData('enabledMiddlewares') + ); } - public function testGetDispatcherWithMiddlewares(): void + public function testAddMiddlewareAfterGetEnabledMiddlewares(): void { - $request = new ServerRequest('GET', '/'); + $route = Route::get('/') + ->middleware(TestMiddleware3::class); - $injectDispatcher = $this - ->getDispatcher( - $this->getContainer([ - TestMiddleware1::class => new TestMiddleware1(), - TestMiddleware2::class => new TestMiddleware2(), - TestController::class => new TestController(), - ]) - ) - ->withMiddlewares([ - TestMiddleware1::class, - TestMiddleware2::class, - [TestController::class, 'index'], - ]); + $route->getData('enabledMiddlewares'); - $route = Route::get('/'); - $route->injectDispatcher($injectDispatcher); + $route = $route->middleware(TestMiddleware1::class, TestMiddleware2::class); - $dispatcher = $route->getData('dispatcherWithMiddlewares'); + $this->assertSame( + [TestMiddleware3::class, TestMiddleware1::class, TestMiddleware2::class], + $route->getData('enabledMiddlewares') + ); + } - $response = $dispatcher->dispatch($request, $this->getRequestHandler()); - $this->assertSame(200, $response->getStatusCode()); - $this->assertSame('12', (string) $response->getBody()); + public function testDisableMiddlewareAfterGetEnabledMiddlewares(): void + { + $route = Route::get('/') + ->middleware(TestMiddleware1::class, TestMiddleware2::class, TestMiddleware3::class); + + $route->getData('enabledMiddlewares'); + + $route = $route->disableMiddleware(TestMiddleware1::class, TestMiddleware2::class); + + $this->assertSame( + [TestMiddleware3::class], + $route->getData('enabledMiddlewares') + ); + } + + public function testGetEnabledMiddlewaresTwice(): void + { + $route = Route::get('/') + ->middleware(TestMiddleware1::class, TestMiddleware2::class); + + $result1 = $route->getData('enabledMiddlewares'); + $result2 = $route->getData('enabledMiddlewares'); + + $this->assertSame([TestMiddleware1::class, TestMiddleware2::class], $result1); + $this->assertSame($result1, $result2); + } + + public function testMiddlewaresWithKeys(): void + { + $route = Route::get('/') + ->middleware(m3: TestMiddleware3::class) + ->action([TestController::class, 'index']) + ->prependMiddleware(m1: TestMiddleware1::class, m2: TestMiddleware2::class) + ->disableMiddleware(m1: TestMiddleware1::class); + + $this->assertSame( + [TestMiddleware2::class, TestMiddleware3::class, [TestController::class, 'index']], + $route->getData('enabledMiddlewares') + ); } public function testDebugInfo(): void @@ -343,10 +378,10 @@ public function testDebugInfo(): void ->host('example.com') ->defaults(['age' => 42]) ->override() - ->middleware(middleware: TestMiddleware1::class) - ->disableMiddleware(middleware: TestMiddleware2::class) + ->middleware(TestMiddleware1::class, TestMiddleware2::class) + ->disableMiddleware(TestMiddleware2::class) ->action('go') - ->prependMiddleware(middleware: TestMiddleware3::class); + ->prependMiddleware(TestMiddleware3::class); $expected = << 1 [actionAdded] => 1 - [middlewareDefinitions] => Array + [middlewares] => Array ( [0] => Yiisoft\Router\Tests\Support\TestMiddleware3 [1] => Yiisoft\Router\Tests\Support\TestMiddleware1 - [2] => go + [2] => Yiisoft\Router\Tests\Support\TestMiddleware2 + [3] => go ) - [disabledMiddlewareDefinitions] => Array + [disabledMiddlewares] => Array ( [0] => Yiisoft\Router\Tests\Support\TestMiddleware2 ) - [middlewareDispatcher] => + [enabledMiddlewares] => Array + ( + [0] => Yiisoft\Router\Tests\Support\TestMiddleware3 + [1] => Yiisoft\Router\Tests\Support\TestMiddleware1 + [2] => go + ) ) EOL; @@ -399,15 +440,9 @@ public function testDuplicateHosts(): void public function testImmutability(): void { - $container = new SimpleContainer(); - $middlewareDispatcher = new MiddlewareDispatcher( - new MiddlewareFactory($container), - ); - $route = Route::get('/'); $routeWithAction = $route->action(''); - $this->assertNotSame($route, $route->withDispatcher($middlewareDispatcher)); $this->assertNotSame($route, $route->name('')); $this->assertNotSame($route, $route->pattern('')); $this->assertNotSame($route, $route->host('')); From 0892cbebfdfbe73ae9d2a64a133c96daf1552c7a Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Thu, 9 Nov 2023 14:02:58 +0300 Subject: [PATCH 2/5] Fix debug classes and add more tests (#224) --- src/Debug/DebugRoutesCommand.php | 11 +-- src/Debug/RouterCollector.php | 16 +--- src/Debug/UrlMatcherInterfaceProxy.php | 9 +-- tests/Debug/DebugRoutesCommandTest.php | 80 +++++++++++++++++--- tests/Debug/RouterCollectorTest.php | 39 ++++++++++ tests/Debug/UrlMatcherInterfaceProxyTest.php | 44 +++++++++++ tests/Support/UrlMatcherStub.php | 22 ++++++ 7 files changed, 186 insertions(+), 35 deletions(-) create mode 100644 tests/Debug/UrlMatcherInterfaceProxyTest.php create mode 100644 tests/Support/UrlMatcherStub.php diff --git a/src/Debug/DebugRoutesCommand.php b/src/Debug/DebugRoutesCommand.php index 4a6fe74..efbde07 100644 --- a/src/Debug/DebugRoutesCommand.php +++ b/src/Debug/DebugRoutesCommand.php @@ -14,9 +14,6 @@ use Yiisoft\VarDumper\VarDumper; use Yiisoft\Yii\Debug\Debugger; -/** - * @codeCoverageIgnore - */ final class DebugRoutesCommand extends Command { public const COMMAND_NAME = 'debug:routes'; @@ -55,8 +52,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $data = $route->__debugInfo(); $action = ''; $middlewares = []; - if (!empty($data['middlewareDefinitions'])) { - $middlewareDefinitions = $data['middlewareDefinitions']; + if (!empty($data['enabledMiddlewares'])) { + $middlewareDefinitions = $data['enabledMiddlewares']; $action = array_pop($middlewareDefinitions); $middlewares = $middlewareDefinitions; } @@ -94,8 +91,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int foreach ($this->routeCollection->getRoutes() as $route) { $data = $route->__debugInfo(); $action = ''; - if (!empty($data['middlewareDefinitions'])) { - $middlewareDefinitions = $data['middlewareDefinitions']; + if (!empty($data['enabledMiddlewares'])) { + $middlewareDefinitions = $data['enabledMiddlewares']; $action = array_pop($middlewareDefinitions); } $rows[] = [ diff --git a/src/Debug/RouterCollector.php b/src/Debug/RouterCollector.php index e9dfc59..c1bf9ea 100644 --- a/src/Debug/RouterCollector.php +++ b/src/Debug/RouterCollector.php @@ -12,9 +12,6 @@ use Yiisoft\Yii\Debug\Collector\CollectorTrait; use Yiisoft\Yii\Debug\Collector\SummaryCollectorInterface; -/** - * @codeCoverageIgnore - */ final class RouterCollector implements SummaryCollectorInterface { use CollectorTrait; @@ -134,15 +131,10 @@ private function getMiddlewaresAndAction(?Route $route): array if ($route === null) { return [[], null]; } - $reflection = new ReflectionObject($route); - $reflectionProperty = $reflection->getProperty('middlewareDefinitions'); - $reflectionProperty->setAccessible(true); - /** - * @var array[]|callable[]|string[] $middlewareDefinitions - */ - $middlewareDefinitions = $reflectionProperty->getValue($route); - $action = array_pop($middlewareDefinitions); - return [$middlewareDefinitions, $action]; + $middlewares = $route->getData('enabledMiddlewares'); + $action = array_pop($middlewares); + + return [$middlewares, $action]; } } diff --git a/src/Debug/UrlMatcherInterfaceProxy.php b/src/Debug/UrlMatcherInterfaceProxy.php index bbe625c..53057b7 100644 --- a/src/Debug/UrlMatcherInterfaceProxy.php +++ b/src/Debug/UrlMatcherInterfaceProxy.php @@ -8,13 +8,12 @@ use Yiisoft\Router\MatchingResult; use Yiisoft\Router\UrlMatcherInterface; -/** - * @codeCoverageIgnore - */ final class UrlMatcherInterfaceProxy implements UrlMatcherInterface { - public function __construct(private UrlMatcherInterface $urlMatcher, private RouterCollector $routerCollector) - { + public function __construct( + private UrlMatcherInterface $urlMatcher, + private RouterCollector $routerCollector + ) { } public function match(ServerRequestInterface $request): MatchingResult diff --git a/tests/Debug/DebugRoutesCommandTest.php b/tests/Debug/DebugRoutesCommandTest.php index 30fb9e0..845d165 100644 --- a/tests/Debug/DebugRoutesCommandTest.php +++ b/tests/Debug/DebugRoutesCommandTest.php @@ -7,26 +7,84 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\Console\Tester\CommandTester; use Yiisoft\Router\Debug\DebugRoutesCommand; -use Yiisoft\Router\RouteCollectionInterface; +use Yiisoft\Router\Route; +use Yiisoft\Router\RouteCollection; +use Yiisoft\Router\RouteCollector; +use Yiisoft\Router\Tests\Support\TestController; +use Yiisoft\Router\Tests\Support\TestMiddleware1; use Yiisoft\Yii\Debug\Debugger; use Yiisoft\Yii\Debug\DebuggerIdGenerator; -use Yiisoft\Yii\Debug\Storage\StorageInterface; +use Yiisoft\Yii\Debug\Storage\MemoryStorage; final class DebugRoutesCommandTest extends TestCase { - public function testCommand() + public function testBase(): void { - $routeCollection = $this->createMock(RouteCollectionInterface::class); - $routeCollection->method('getRoutes')->willReturn([]); - $idGenerator = new DebuggerIdGenerator(); - $storage = $this->createMock(StorageInterface::class); - $storage->expects($this->never())->method('clear'); - $debugger = new Debugger($idGenerator, $storage, []); + $debuggerIdGenerator = new DebuggerIdGenerator(); - $command = new DebugRoutesCommand($routeCollection, $debugger); + $command = new DebugRoutesCommand( + new RouteCollection( + (new RouteCollector())->addRoute( + Route::get('/') + ->host('example.com') + ->defaults(['SpecialArg' => 1]) + ->action(fn () => 'Hello, XXXXXX!') + ->name('site/index'), + Route::get('/about') + ->action([TestController::class, 'index']) + ->name('site/about'), + ), + ), + new Debugger( + $debuggerIdGenerator, + new MemoryStorage($debuggerIdGenerator), + [], + ), + ); $commandTester = new CommandTester($command); - $commandTester->execute([]); + $output = $commandTester->getDisplay(); + + $this->assertStringContainsString('site/index', $output); + $this->assertStringContainsString('SpecialArg', $output); + $this->assertStringContainsString('example.com', $output); + $this->assertStringContainsString('XXXXXX', $output); + $this->assertStringContainsString('site/about', $output); + $this->assertStringContainsString(TestController::class . '::index', $output); + } + + public function testSpecificRoute(): void + { + $debuggerIdGenerator = new DebuggerIdGenerator(); + + $command = new DebugRoutesCommand( + new RouteCollection( + (new RouteCollector())->addRoute( + Route::get('/') + ->host('example.com') + ->defaults(['SpecialArg' => 1]) + ->name('site/index') + ->middleware(TestMiddleware1::class) + ->action(fn () => 'Hello world!'), + Route::get('/about')->name('site/about'), + ), + ), + new Debugger( + $debuggerIdGenerator, + new MemoryStorage($debuggerIdGenerator), + [], + ), + ); + + $commandTester = new CommandTester($command); + $commandTester->execute(['route' => ['site/index']]); + $output = $commandTester->getDisplay(); + + $this->assertStringContainsString('site/index', $output); + $this->assertStringContainsString('TestMiddleware1', $output); + $this->assertStringContainsString('SpecialArg', $output); + $this->assertStringContainsString('example.com', $output); + $this->assertStringNotContainsString('site/about', $output); } } diff --git a/tests/Debug/RouterCollectorTest.php b/tests/Debug/RouterCollectorTest.php index e3e5731..651263d 100644 --- a/tests/Debug/RouterCollectorTest.php +++ b/tests/Debug/RouterCollectorTest.php @@ -7,14 +7,17 @@ use PHPUnit\Framework\MockObject\MockObject; use Yiisoft\Di\Container; use Yiisoft\Di\ContainerConfig; +use Yiisoft\Router\CurrentRoute; use Yiisoft\Router\Debug\RouterCollector; use Yiisoft\Router\Group; +use Yiisoft\Router\MatchingResult; use Yiisoft\Router\Route; use Yiisoft\Router\RouteCollection; use Yiisoft\Router\RouteCollectionInterface; use Yiisoft\Router\RouteCollector; use Yiisoft\Router\RouteCollectorInterface; use Yiisoft\Router\UrlMatcherInterface; +use Yiisoft\Test\Support\Container\SimpleContainer; use Yiisoft\Yii\Debug\Collector\CollectorInterface; use Yiisoft\Yii\Debug\Tests\Shared\AbstractCollectorTestCase; @@ -24,6 +27,42 @@ final class RouterCollectorTest extends AbstractCollectorTestCase private ?Container $container = null; + public function testWithoutCurrentRoute(): void + { + $collector = new RouterCollector( + new SimpleContainer() + ); + $collector->startup(); + + $summary = $collector->getSummary(); + + $this->assertNull($summary['router']); + } + + public function testWithoutRouteCollection(): void + { + $route = Route::get('/'); + $arguments = ['a' => 19]; + $result = MatchingResult::fromSuccess($route, $arguments); + + $currentRoute = new CurrentRoute(); + $currentRoute->setRouteWithArguments($result->route(), $result->arguments()); + $collector = new RouterCollector( + new SimpleContainer([ + CurrentRoute::class => $currentRoute, + ]) + ); + $collector->startup(); + + $collected = $collector->getCollected(); + + $this->assertSame(['currentRoute'], array_keys($collected)); + $this->assertSame( + ['matchTime', 'name', 'pattern', 'arguments', 'host', 'uri', 'action', 'middlewares'], + array_keys($collected['currentRoute']) + ); + } + /** * @param CollectorInterface|RouterCollector $collector */ diff --git a/tests/Debug/UrlMatcherInterfaceProxyTest.php b/tests/Debug/UrlMatcherInterfaceProxyTest.php new file mode 100644 index 0000000..c6756d2 --- /dev/null +++ b/tests/Debug/UrlMatcherInterfaceProxyTest.php @@ -0,0 +1,44 @@ + 19]; + $result = MatchingResult::fromSuccess($route, $arguments); + + $currentRoute = new CurrentRoute(); + $currentRoute->setRouteWithArguments($result->route(), $result->arguments()); + + $collector = new RouterCollector( + new SimpleContainer([ + CurrentRoute::class => $currentRoute, + ]) + ); + $collector->startup(); + + $proxy = new UrlMatcherInterfaceProxy(new UrlMatcherStub($result), $collector); + + $proxyResult = $proxy->match($request); + $summary = $collector->getSummary(); + + $this->assertSame($result, $proxyResult); + $this->assertGreaterThan(0, $summary['router']['matchTime']); + } +} diff --git a/tests/Support/UrlMatcherStub.php b/tests/Support/UrlMatcherStub.php new file mode 100644 index 0000000..44db87b --- /dev/null +++ b/tests/Support/UrlMatcherStub.php @@ -0,0 +1,22 @@ +result; + } +} From 29213adb4ffc72cd9b87c68c4549aaf91b1d7a98 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 10 Jan 2024 11:53:37 +0300 Subject: [PATCH 3/5] Update rector/rector requirement from ^0.18.3 to ^0.19.0 (#226) --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 0bf73fd..a00fb4e 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ "nyholm/psr7": "^1.5", "phpunit/phpunit": "^9.5", "psr/container": "^1.1|^2.0", - "rector/rector": "^0.18.3", + "rector/rector": "^0.19.0", "roave/infection-static-analysis-plugin": "^1.18", "spatie/phpunit-watcher": "^1.23", "vimeo/psalm": "^4.30|^5.6", From cd98ba61c00805209db315796e753c32dfa5e272 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Wed, 24 Jan 2024 16:09:17 +0300 Subject: [PATCH 4/5] Fix psalm (#228) --- .github/workflows/static.yml | 11 ++++++++++- composer.json | 4 ++-- psalm.xml | 6 +++++- psalm80.xml | 19 +++++++++++++++++++ 4 files changed, 36 insertions(+), 4 deletions(-) create mode 100644 psalm80.xml diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index 96b2679..e34190e 100644 --- a/.github/workflows/static.yml +++ b/.github/workflows/static.yml @@ -10,6 +10,7 @@ on: - 'phpunit.xml.dist' push: + branches: ['master'] paths-ignore: - 'docs/**' - 'README.md' @@ -28,4 +29,12 @@ jobs: os: >- ['ubuntu-latest'] php: >- - ['8.0', '8.1'] + ['8.1', '8.2', '8.3'] + psalm80: + uses: yiisoft/actions/.github/workflows/psalm.yml@master + with: + psalm-config: psalm80.xml + os: >- + ['ubuntu-latest'] + php: >- + ['8.0'] diff --git a/composer.json b/composer.json index a00fb4e..359976e 100644 --- a/composer.json +++ b/composer.json @@ -36,11 +36,11 @@ "rector/rector": "^0.19.0", "roave/infection-static-analysis-plugin": "^1.18", "spatie/phpunit-watcher": "^1.23", - "vimeo/psalm": "^4.30|^5.6", + "vimeo/psalm": "^4.30|^5.20", "yiisoft/di": "^1.0", "yiisoft/dummy-provider": "^1.0.0", "yiisoft/test-support": "^3.0", - "yiisoft/yii-debug": "dev-master" + "yiisoft/yii-debug": "dev-master|dev-php80" }, "autoload": { "psr-4": { diff --git a/psalm.xml b/psalm.xml index 73ed715..b48c894 100644 --- a/psalm.xml +++ b/psalm.xml @@ -10,7 +10,11 @@ - + + + + + diff --git a/psalm80.xml b/psalm80.xml new file mode 100644 index 0000000..d091d59 --- /dev/null +++ b/psalm80.xml @@ -0,0 +1,19 @@ + + + + + + + + + + + + From 5bc9a72e363626ea1b27acdf566e90e3590bddb5 Mon Sep 17 00:00:00 2001 From: Sergei Predvoditelev Date: Wed, 24 Jan 2024 18:45:36 +0300 Subject: [PATCH 5/5] Add URL arguments' psalm type in `UrlGeneratorInterface` (#229) * Add URL arguments' psalm type in `UrlGeneratorInterface` * improve --- CHANGELOG.md | 1 + src/UrlGeneratorInterface.php | 15 +++++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9eafa70..1af9672 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Chg #207: Replace two `RouteCollectorInterface` methods `addRoute()` and `addGroup()` to single `addRoute()` (@vjik) - Enh #202: Add support for `psr/http-message` version `^2.0` (@vjik) - Chg #222: Make `Route`, `Group` and `MatchingResult` dispatcher-independent (@rustamwin, @vjik) +- Enh #229: Add URL arguments' psalm type in `UrlGeneratorInterface` (@vjik) ## 3.0.0 February 17, 2023 diff --git a/src/UrlGeneratorInterface.php b/src/UrlGeneratorInterface.php index 214e891..94c3045 100644 --- a/src/UrlGeneratorInterface.php +++ b/src/UrlGeneratorInterface.php @@ -8,6 +8,8 @@ /** * `UrlGeneratorInterface` allows generating URL given route name, arguments, and query parameters. + * + * @psalm-type UrlArgumentsType = array */ interface UrlGeneratorInterface { @@ -15,12 +17,14 @@ interface UrlGeneratorInterface * Generates URL from named route, arguments, and query parameters. * * @param string $name Name of the route. - * @param array $arguments Argument-value set. + * @param array $arguments Argument-value set. * @param array $queryParameters Parameter-value set. * * @throws RouteNotFoundException In case there is no route with the name specified. * * @return string URL generated. + * + * @psalm-param UrlArgumentsType $arguments */ public function generate(string $name, array $arguments = [], array $queryParameters = []): string; @@ -28,7 +32,7 @@ public function generate(string $name, array $arguments = [], array $queryParame * Generates absolute URL from named route, arguments, and query parameters. * * @param string $name Name of the route. - * @param array $arguments Argument-value set. + * @param array $arguments Argument-value set. * @param array $queryParameters Parameter-value set. * @param string|null $scheme Host scheme. * @param string|null $host Host for manual setup. @@ -36,6 +40,8 @@ public function generate(string $name, array $arguments = [], array $queryParame * @throws RouteNotFoundException In case there is no route with the name specified. * * @return string URL generated. + * + * @psalm-param UrlArgumentsType $arguments */ public function generateAbsolute( string $name, @@ -48,11 +54,12 @@ public function generateAbsolute( /** * Generate URL from the current route replacing some of its arguments with values specified. * - * @param array $replacedArguments New argument values indexed by replaced argument - * names. + * @param array $replacedArguments New argument values indexed by replaced argument names. * @param array $queryParameters Parameter-value set. * @param string|null $fallbackRouteName Name of a route that should be used if current route. * can not be determined. + * + * @psalm-param UrlArgumentsType $replacedArguments */ public function generateFromCurrent( array $replacedArguments,