Fix #141, fix #148: Add variadic arguments support, don't add middlew…

…are from collector to static routes (#147)
yiisoft · Dec 26, 2021 · ab74b9e · ab74b9e
1 parent f0dc0cb
commit ab74b9e
Show file tree

Hide file tree

Showing 10 changed files with 230 additions and 67 deletions.
diff --git a/src/Group.php b/src/Group.php
@@ -122,32 +122,38 @@ public function withCors($middlewareDefinition): self
      * Appends a handler middleware definition that should be invoked for a matched route.
      * First added handler will be executed first.
      *
-     * @param array|callable|string $middlewareDefinition
+     * @param array|callable|string ...$middlewareDefinition
      *
      * @return self
      */
-    public function middleware($middlewareDefinition): self
+    public function middleware(...$middlewareDefinition): self
     {
         if ($this->routesAdded) {
             throw new RuntimeException('middleware() can not be used after routes().');
         }
         $new = clone $this;
-        array_unshift($new->middlewareDefinitions, $middlewareDefinition);
+        array_unshift(
+            $new->middlewareDefinitions,
+            ...array_values($middlewareDefinition)
+        );
         return $new;
     }
 
     /**
      * Prepends a handler middleware definition that should be invoked for a matched route.
      * First added handler will be executed last.
      *
-     * @param array|callable|string $middlewareDefinition
+     * @param array|callable|string ...$middlewareDefinition
      *
      * @return self
      */
-    public function prependMiddleware($middlewareDefinition): self
+    public function prependMiddleware(...$middlewareDefinition): self
     {
         $new = clone $this;
-        $new->middlewareDefinitions[] = $middlewareDefinition;
+        array_push(
+            $new->middlewareDefinitions,
+            ...array_values($middlewareDefinition)
+        );
         $new->middlewareAdded = true;
         return $new;
     }
@@ -171,14 +177,17 @@ public function host(string $host): self
      * It is useful to avoid invoking one of the parent group middleware for
      * a certain route.
      *
-     * @param mixed $middlewareDefinition
+     * @param mixed ...$middlewareDefinition
      *
      * @return self
      */
-    public function disableMiddleware($middlewareDefinition): self
+    public function disableMiddleware(...$middlewareDefinition): self
     {
         $new = clone $this;
-        $new->disabledMiddlewareDefinitions[] = $middlewareDefinition;
+        array_push(
+            $new->disabledMiddlewareDefinitions,
+            ...array_values($middlewareDefinition),
+        );
         return $new;
     }
 
@@ -195,7 +204,7 @@ public function disableMiddleware($middlewareDefinition): self
      *   T is ('prefix'|'namePrefix'|'host') ? string|null :
      *   (T is 'items' ? Group[]|Route[] :
      *     (T is ('hasCorsMiddleware'|'hasDispatcher') ? bool :
-     *       (T is 'middlewareDefinitions' ? array<array-key,array|callable|string> :
+     *       (T is 'middlewareDefinitions' ? list<array|callable|string> :
      *         (T is 'corsMiddleware' ? array|callable|string|null : mixed)
      *       )
      *     )
@@ -235,6 +244,6 @@ private function getMiddlewareDefinitions(): array
             }
         }
 
-        return $this->middlewareDefinitions;
+        return array_values($this->middlewareDefinitions);
     }
 }
diff --git a/src/Route.php b/src/Route.php
@@ -217,35 +217,41 @@ 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.
      *
-     * @param array|callable|string $middlewareDefinition
+     * @param array|callable|string ...$middlewareDefinition
      *
      * @return self
      */
-    public function middleware($middlewareDefinition): self
+    public function middleware(...$middlewareDefinition): self
     {
         if ($this->actionAdded) {
             throw new RuntimeException('middleware() can not be used after action().');
         }
         $route = clone $this;
-        $route->middlewareDefinitions[] = $middlewareDefinition;
+        array_push(
+            $route->middlewareDefinitions,
+            ...array_values($middlewareDefinition)
+        );
         return $route;
     }
 
     /**
      * Prepends a handler middleware definition that should be invoked for a matched route.
      * Last added handler will be executed first.
      *
-     * @param array|callable|string $middlewareDefinition
+     * @param array|callable|string ...$middlewareDefinition
      *
      * @return self
      */
-    public function prependMiddleware($middlewareDefinition): self
+    public function prependMiddleware(...$middlewareDefinition): self
     {
         if (!$this->actionAdded) {
             throw new RuntimeException('prependMiddleware() can not be used before action().');
         }
         $route = clone $this;
-        array_unshift($route->middlewareDefinitions, $middlewareDefinition);
+        array_unshift(
+            $route->middlewareDefinitions,
+            ...array_values($middlewareDefinition)
+        );
         return $route;
     }
 
@@ -269,14 +275,17 @@ public function action($middlewareDefinition): self
      * It is useful to avoid invoking one of the parent group middleware for
      * a certain route.
      *
-     * @param mixed $middlewareDefinition
+     * @param mixed ...$middlewareDefinition
      *
      * @return self
      */
-    public function disableMiddleware($middlewareDefinition): self
+    public function disableMiddleware(...$middlewareDefinition): self
     {
         $route = clone $this;
-        $route->disabledMiddlewareDefinitions[] = $middlewareDefinition;
+        array_push(
+            $route->disabledMiddlewareDefinitions,
+            ...array_values($middlewareDefinition)
+        );
         return $route;
     }
 

diff --git a/src/RouteCollection.php b/src/RouteCollection.php
@@ -73,8 +73,8 @@ private function ensureItemsInjected(): void
     private function injectItems(array $items): void
     {
         foreach ($items as $item) {
-            foreach ($this->collector->getMiddlewareDefinitions() as $middlewareDefinition) {
-                $item = $item->prependMiddleware($middlewareDefinition);
+            if (!$this->isStaticRoute($item)) {
+                $item = $item->prependMiddleware(...$this->collector->getMiddlewareDefinitions());
             }
             $this->injectItem($item);
         }
@@ -113,7 +113,7 @@ private function injectGroup(Group $group, array &$tree, string $prefix = '', st
         $pattern = null;
         $host = null;
         foreach ($items as $item) {
-            if ($item instanceof Group || $item->getData('hasMiddlewares')) {
+            if (!$this->isStaticRoute($item)) {
                 foreach ($group->getData('middlewareDefinitions') as $middleware) {
                     $item = $item->prependMiddleware($middleware);
                 }
@@ -215,9 +215,17 @@ private function buildTree(array $items, bool $routeAsString): array
                 /** @psalm-var Items $item */
                 $tree[$key] = $this->buildTree($item, $routeAsString);
             } else {
-                $tree[] = $routeAsString ? (string)$this->getRoute($item) : $this->getRoute($item);
+                $tree[] = $routeAsString ? (string) $this->getRoute($item) : $this->getRoute($item);
             }
         }
         return $tree;
     }
+
+    /**
+     * @param Group|Route $item
+     */
+    private function isStaticRoute($item): bool
+    {
+        return $item instanceof Route && !$item->getData('hasMiddlewares');
+    }
 }
diff --git a/src/RouteCollector.php b/src/RouteCollector.php
@@ -16,27 +16,39 @@ final class RouteCollector implements RouteCollectorInterface
      */
     private array $middlewareDefinitions = [];
 
-    public function addRoute(Route $route): RouteCollectorInterface
+    public function addRoute(Route ...$route): RouteCollectorInterface
     {
-        $this->items[] = $route;
+        array_push(
+            $this->items,
+            ...array_values($route)
+        );
         return $this;
     }
 
-    public function addGroup(Group $group): RouteCollectorInterface
+    public function addGroup(Group ...$group): RouteCollectorInterface
     {
-        $this->items[] = $group;
+        array_push(
+            $this->items,
+            ...array_values($group),
+        );
         return $this;
     }
 
-    public function middleware($middlewareDefinition): RouteCollectorInterface
+    public function middleware(...$middlewareDefinition): RouteCollectorInterface
     {
-        array_unshift($this->middlewareDefinitions, $middlewareDefinition);
+        array_push(
+            $this->middlewareDefinitions,
+            ...array_values($middlewareDefinition)
+        );
         return $this;
     }
 
-    public function prependMiddleware($middlewareDefinition): RouteCollectorInterface
+    public function prependMiddleware(...$middlewareDefinition): RouteCollectorInterface
     {
-        $this->middlewareDefinitions[] = $middlewareDefinition;
+        array_unshift(
+            $this->middlewareDefinitions,
+            ...array_values($middlewareDefinition)
+        );
         return $this;
     }
 

diff --git a/src/RouteCollectorInterface.php b/src/RouteCollectorInterface.php
@@ -9,11 +9,11 @@ interface RouteCollectorInterface
     /**
      * Add a route
      *
-     * @param Route $route
+     * @param Route ...$route
      *
      * @return self
      */
-    public function addRoute(Route $route): self;
+    public function addRoute(Route ...$route): self;
 
     /**
      * Add a group of routes
@@ -28,31 +28,31 @@ public function addRoute(Route $route): self;
      * $routeCollector->addGroup($group);
      * ```
      *
-     * @param Group $group a group to add
+     * @param Group ...$group a group to add
      *
      * @return self
      */
-    public function addGroup(Group $group): self;
+    public function addGroup(Group ...$group): self;
 
     /**
      * Appends a handler middleware definition that should be invoked for a matched route.
      * First added handler will be executed first.
      *
-     * @param array|callable|string $middlewareDefinition
+     * @param array|callable|string ...$middlewareDefinition
      *
      * @return self
      */
-    public function middleware($middlewareDefinition): self;
+    public function middleware(...$middlewareDefinition): self;
 
     /**
      * Prepends a handler middleware definition that should be invoked for a matched route.
      * First added handler will be executed last.
      *
-     * @param array|callable|string $middlewareDefinition
+     * @param array|callable|string ...$middlewareDefinition
      *
      * @return self
      */
-    public function prependMiddleware($middlewareDefinition): self;
+    public function prependMiddleware(...$middlewareDefinition): self;
 
     /**
      * @return Group[]|Route[]

diff --git a/tests/GroupTest.php b/tests/GroupTest.php
@@ -23,6 +23,7 @@
 use Yiisoft\Router\Tests\Support\Container;
 use Yiisoft\Router\Tests\Support\TestMiddleware1;
 use Yiisoft\Router\Tests\Support\TestMiddleware2;
+use Yiisoft\Router\Tests\Support\TestMiddleware3;
 
 final class GroupTest extends TestCase
 {
@@ -48,9 +49,9 @@ public function testAddMiddleware(): void
     public function testDisabledMiddlewareDefinitions(): void
     {
         $group = Group::create()
-            ->middleware(TestMiddleware1::class)
-            ->middleware(TestMiddleware2::class)
-            ->disableMiddleware(TestMiddleware1::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]);