Make Route, Group and MatchingResult dispatcher independent (#222)

Co-authored-by: Rustam Mamadaminov <rmamdaminov@gmail.com>

Author: vjik

.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']

.phpstorm.meta.php/Group.php

@@ -11,9 +11,8 @@
         'host',
         'hosts',
         'corsMiddleware',
-        'items',
-        'middlewareDefinitions',
-        'hasDispatcher',
-        'hasCorsMiddleware'
+        'routes',
+        'hasCorsMiddleware',
+        'enabledMiddlewares',
     );
-}
+}

.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',
     );
-}
+}

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
 

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()`.

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';

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;

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)

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<array|callable|string>
      */
-    private array $middlewareDefinitions = [];
+    private array $middlewares = [];
 
     /**
      * @var string[]
@@ -29,62 +30,46 @@ final class Group
     private ?string $namePrefix = null;
     private bool $routesAdded = false;
     private bool $middlewareAdded = false;
-    private array $disabledMiddlewareDefinitions = [];
+    private array $disabledMiddlewares = [];
+
+    /**
+     * @psalm-var list<array|callable|string>|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
     {
         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,31 +88,38 @@ 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;
     }
 
     /**
      * 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<array-key, string> :
-     *       (T is ('hasCorsMiddleware'|'hasDispatcher') ? bool :
-     *         (T is 'middlewareDefinitions' ? list<array|callable|string> :
+     *       (T is ('hasCorsMiddleware') ? bool :
+     *         (T is 'enabledMiddlewares' ? list<array|callable|string> :
      *           (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<array|callable|string>
+     */
+    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;
     }
 }

src/Internal/MiddlewareFilter.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Yiisoft\Router\Internal;
+
+/**
+ * @internal
+ */
+final class MiddlewareFilter
+{
+    /**
+     * @param array[]|callable[]|string[] $middlewares
+     * @return array[]|callable[]|string[]
+     *
+     * @psalm-param list<array|callable|string> $middlewares
+     * @psalm-return list<array|callable|string>
+     */
+    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;
+    }
+}