Fix middlewares order in group (#150)

yiisoft · Dec 26, 2021 · e0a9c08 · e0a9c08
1 parent ab74b9e
commit e0a9c08
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 27 deletions.
diff --git a/src/Group.php b/src/Group.php
@@ -132,7 +132,7 @@ public function middleware(...$middlewareDefinition): self
             throw new RuntimeException('middleware() can not be used after routes().');
         }
         $new = clone $this;
-        array_unshift(
+        array_push(
             $new->middlewareDefinitions,
             ...array_values($middlewareDefinition)
         );
@@ -150,7 +150,7 @@ public function middleware(...$middlewareDefinition): self
     public function prependMiddleware(...$middlewareDefinition): self
     {
         $new = clone $this;
-        array_push(
+        array_unshift(
             $new->middlewareDefinitions,
             ...array_values($middlewareDefinition)
         );

diff --git a/src/RouteCollection.php b/src/RouteCollection.php
@@ -114,9 +114,7 @@ private function injectGroup(Group $group, array &$tree, string $prefix = '', st
         $host = null;
         foreach ($items as $item) {
             if (!$this->isStaticRoute($item)) {
-                foreach ($group->getData('middlewareDefinitions') as $middleware) {
-                    $item = $item->prependMiddleware($middleware);
-                }
+                $item = $item->prependMiddleware(...$group->getData('middlewareDefinitions'));
             }
 
             if ($group->getData('host') !== null && $item->getData('host') === null) {

diff --git a/tests/GroupTest.php b/tests/GroupTest.php
@@ -31,16 +31,12 @@ public function testAddMiddleware(): void
     {
         $group = Group::create();
 
-        $middleware1 = static function () {
-            return new Response();
-        };
-        $middleware2 = static function () {
-            return new Response();
-        };
+        $middleware1 = static fn () => new Response();
+        $middleware2 = static fn () => new Response();
 
         $group = $group
-            ->middleware($middleware2)
-            ->middleware($middleware1);
+            ->middleware($middleware1)
+            ->middleware($middleware2);
         $this->assertCount(2, $group->getData('middlewareDefinitions'));
         $this->assertSame($middleware1, $group->getData('middlewareDefinitions')[0]);
         $this->assertSame($middleware2, $group->getData('middlewareDefinitions')[1]);
@@ -192,18 +188,14 @@ public function testAddGroup(): void
         $listRoute = Route::get('/');
         $viewRoute = Route::get('/{id}');
 
-        $middleware1 = static function () {
-            return new Response();
-        };
-        $middleware2 = static function () {
-            return new Response();
-        };
+        $middleware1 = static fn () => new Response();
+        $middleware2 = static fn () => new Response();
 
         $root = Group::create()
             ->routes(
                 Group::create('/api')
-                    ->middleware($middleware2)
                     ->middleware($middleware1)
+                    ->middleware($middleware2)
                     ->routes(
                         $logoutRoute,
                         Group::create('/post')
@@ -215,6 +207,7 @@ public function testAddGroup(): void
             );
 
         $this->assertCount(1, $root->getData('items'));
+
         /** @var Group $api */
         $api = $root->getData('items')[0];
 

diff --git a/tests/RouteCollectionTest.php b/tests/RouteCollectionTest.php
@@ -259,7 +259,18 @@ public function testCollectorMiddlewareFullstackCalled(): void
         $this->assertEquals('middleware1', $response2->getReasonPhrase());
     }
 
-    public function testMiddlewaresOrder(): void
+    public function dataMiddlewaresOrder(): array
+    {
+        return [
+            [true],
+            [false],
+        ];
+    }
+
+    /**
+     * @dataProvider dataMiddlewaresOrder
+     */
+    public function testMiddlewaresOrder(bool $groupWrapped): void
     {
         $request = new ServerRequest('GET', '/');
 
@@ -278,12 +289,18 @@ public function testMiddlewaresOrder(): void
             ->middleware(TestMiddleware2::class)
             ->prependMiddleware(TestMiddleware1::class);
 
-        $collector->addRoute(
-            Route::get('/')
-                ->middleware(TestMiddleware3::class)
-                ->action([TestController::class, 'index'])
-                ->name('main')
-        );
+        $rawRoute = Route::get('/')
+            ->middleware(TestMiddleware3::class)
+            ->action([TestController::class, 'index'])
+            ->name('main');
+
+        if ($groupWrapped) {
+            $collector->addGroup(
+                Group::create()->routes($rawRoute)
+            );
+        } else {
+            $collector->addRoute($rawRoute);
+        }
 
         $route = (new RouteCollection($collector))->getRoute('main');
         $route->injectDispatcher($injectDispatcher);