fix(http-routing): dont include non-global middleware in cache

Middleware groups that are not in the "run_always"
configuration should not be in the generated
middleware cache.

If they are included in the middleware cache
every request without a route match
has the middleware applied even though
it should only be run for route matches.

Fix: #135

src/Snicco/Component/http-routing/src/Middleware/MiddlewareResolver.php

@@ -219,6 +219,11 @@ public function createMiddlewareCache(Routes $routes, ContainerInterface $contai
         ];
 
         foreach (array_keys($request_map) as $type) {
+
+            if(!$this->always_run_if_no_route_matches[$type]){
+                continue;
+            }
+
             foreach ($this->resolve([$type]) as $blueprint) {
                 $request_map[$type][] = $blueprint->asArray();
             }

src/Snicco/Component/http-routing/tests/Middleware/CachedMiddlewareResolverTest.php

@@ -24,102 +24,103 @@
  */
 final class CachedMiddlewareResolverTest extends HttpRunnerTestCase
 {
+
     /**
      * @test
      */
-    public function the_middleware_resolver_can_be_constructed_from_cache(): void
+    public function the_middleware_resolver_can_be_constructed_from_cache() :void
     {
         $blueprint1 = MiddlewareBlueprint::from(FooMiddleware::class, ['bar', 'baz']);
         $blueprint2 = MiddlewareBlueprint::from(BarMiddleware::class, ['biz']);
-
+        
         $route_cache = [
             'r1' => [$blueprint1->asArray(), $blueprint2->asArray()],
         ];
-
+        
         $resolver = MiddlewareResolver::fromCache($route_cache, [
             RoutingConfigurator::GLOBAL_MIDDLEWARE => [$blueprint1->asArray()],
             RoutingConfigurator::FRONTEND_MIDDLEWARE => [$blueprint2->asArray()],
         ]);
-
+        
         $route = Route::create('/', Route::DELEGATE, 'r1');
         $route->middleware(BazMiddleware::class);
-
+        
         $controller_action = new ControllerAction(Route::DELEGATE, $this->psr_container);
-
+        
         // BazMiddleware is not present since It's loaded from cache.
         $this->assertEquals([$blueprint1, $blueprint2], $resolver->resolveForRoute($route, $controller_action));
-
+        
         $this->assertEquals(
             [$blueprint1, $blueprint2],
             $resolver->resolveForRequestWithoutRoute($this->frontendRequest())
         );
-
+        
         $this->assertEquals([$blueprint1], $resolver->resolveForRequestWithoutRoute($this->adminRequest('/foo')));
-
+        
         $this->assertEquals([$blueprint1], $resolver->resolveForRequestWithoutRoute($this->adminRequest('/foo')));
     }
-
+    
     /**
      * @test
      */
-    public function middleware_for_api_requests_can_be_loaded_from_cache(): void
+    public function middleware_for_api_requests_can_be_loaded_from_cache() :void
     {
         $blueprint1 = MiddlewareBlueprint::from(FooMiddleware::class, ['bar', 'baz']);
         $blueprint2 = MiddlewareBlueprint::from(BarMiddleware::class, ['biz']);
-
+        
         $resolver = MiddlewareResolver::fromCache([], [
             RoutingConfigurator::GLOBAL_MIDDLEWARE => [$blueprint1->asArray()],
             RoutingConfigurator::API_MIDDLEWARE => [$blueprint2->asArray()],
         ]);
-
+        
         $this->assertEquals(
             [$blueprint1, $blueprint2],
             $resolver->resolveForRequestWithoutRoute($this->apiRequest())
         );
-
+        
         $this->assertEquals([$blueprint1], $resolver->resolveForRequestWithoutRoute($this->adminRequest('/foo')));
-
+        
         $this->assertEquals(
             [$blueprint1],
             $resolver->resolveForRequestWithoutRoute($this->frontendRequest('/foo'))
         );
     }
-
+    
     /**
      * @test
      */
-    public function test_exception_if_a_route_is_not_in_cache(): void
+    public function test_exception_if_a_route_is_not_in_cache() :void
     {
         $blueprint1 = MiddlewareBlueprint::from(FooMiddleware::class, ['bar', 'baz']);
         $blueprint2 = MiddlewareBlueprint::from(BarMiddleware::class, ['biz']);
-
+        
         $route_cache = [
             'r2' => [$blueprint1->asArray(), $blueprint2->asArray()],
         ];
-
+        
         $resolver = MiddlewareResolver::fromCache($route_cache, [
             RoutingConfigurator::GLOBAL_MIDDLEWARE => [$blueprint1->asArray()],
             RoutingConfigurator::FRONTEND_MIDDLEWARE => [$blueprint2->asArray()],
         ]);
-
+        
         $route = Route::create('/', Route::DELEGATE, 'r1');
-
+        
         $this->expectException(LogicException::class);
         $this->expectExceptionMessage('The middleware resolver is cached but has no entry for route [r1].');
         $resolver->resolveForRoute($route, new ControllerAction(Route::DELEGATE, $this->psr_container));
     }
-
+    
     /**
      * @test
      */
-    public function test_create_middleware_cache(): void
+    public function test_create_middleware_cache() :void
     {
         $route1 = Route::create('/foo', Route::DELEGATE, 'r1')->middleware('group1');
         $route2 = Route::create('/bar', Route::DELEGATE, 'r2')->middleware('foo:FOO');
         $route3 = Route::create('/baz', ControllerWithBarMiddleware::class, 'r3')->middleware(['foo:FOO']);
-
+        
         $routes = new RouteCollection([$route1, $route2, $route3]);
-
+        
         $resolver = new MiddlewareResolver(
             [RoutingConfigurator::FRONTEND_MIDDLEWARE],
             [
@@ -133,24 +134,24 @@ public function test_create_middleware_cache(): void
             ],
             [BarMiddleware::class]
         );
-
+        
         $pimple = new Container();
         $psr = new \Pimple\Psr11\Container($pimple);
-
+        
         $cache = $resolver->createMiddlewareCache($routes, $psr);
-
+        
         $this->assertTrue(isset($cache['route_map']));
         $this->assertTrue(isset($cache['route_map']));
         $this->assertTrue(isset($cache['route_map']['r1']));
         $this->assertTrue(isset($cache['route_map']['r2']));
         $this->assertTrue(isset($cache['route_map']['r3']));
-
+        
         $this->assertTrue(isset($cache['request_map']));
         $this->assertTrue(isset($cache['request_map']['frontend']));
         $this->assertTrue(isset($cache['request_map']['api']));
         $this->assertTrue(isset($cache['request_map']['admin']));
         $this->assertTrue(isset($cache['request_map']['global']));
-
+        
         $this->assertSame([
             [
                 'class' => BarMiddleware::class,
@@ -161,14 +162,14 @@ public function test_create_middleware_cache(): void
                 'args' => [],
             ],
         ], $cache['route_map']['r1']);
-
+        
         $this->assertSame([
             [
                 'class' => FooMiddleware::class,
                 'args' => ['FOO'],
             ],
         ], $cache['route_map']['r2']);
-
+        
         $this->assertSame([
             [
                 'class' => BarMiddleware::class,
@@ -179,11 +180,11 @@ public function test_create_middleware_cache(): void
                 'args' => ['FOO'],
             ],
         ], $cache['route_map']['r3']);
-
+        
         $this->assertSame([], $cache['request_map']['global']);
         $this->assertSame([], $cache['request_map']['api']);
         $this->assertSame([], $cache['request_map']['admin']);
-
+        
         // Bar has higher priority
         $this->assertSame([
             [
@@ -196,15 +197,69 @@ public function test_create_middleware_cache(): void
             ],
         ], $cache['request_map']['frontend']);
     }
-
+    
     /**
      * @test
      */
-    public function middleware_caching_detects_recursion(): void
+    public function creating_the_middleware_cache_takes_settings_for_always_run_into_account() :void
+    {
+        $resolver = new MiddlewareResolver(
+            [RoutingConfigurator::FRONTEND_MIDDLEWARE],
+            [
+                'foo' => FooMiddleware::class,
+                'bar' => BarMiddleware::class,
+                'baz' => BazMiddleware::class,
+            ],
+            [
+                'frontend' => ['foo'],
+                'admin' => ['bar'],
+                'api' => ['baz'],
+                'global' => ['foo', 'bar'],
+            ],
+            [BarMiddleware::class]
+        );
+
+        $pimple = new Container();
+        $psr = new \Pimple\Psr11\Container($pimple);
+
+        $cache = $resolver->createMiddlewareCache(new RouteCollection(), $psr);
+
+        $request_map = $cache['request_map'];
+
+        $this->assertTrue(isset($request_map['frontend']), 'Frontend middleware should be in request_map cache');
+        $this->assertTrue(isset($request_map['api']), 'API middleware should have been in request_map cache');
+        $this->assertTrue(isset($request_map['admin']), 'Admin middleware should have been in request_map cache');
+        $this->assertTrue(isset($request_map['global']), 'Global middleware should have been in request_map cache');
+
+        $this->assertSame([
+            [
+                'class' => FooMiddleware::class,
+                'args' => []
+            ],
+        ], $request_map['frontend']);
+
+        $this->assertSame([], $request_map['api']);
+        $this->assertSame([], $request_map['admin']);
+        $this->assertSame([], $request_map['global']);
+
+        $resolver = MiddlewareResolver::fromCache($cache['route_map'], $cache['request_map']);
+
+        $this->assertEquals([
+            new MiddlewareBlueprint(FooMiddleware::class, [])
+        ], $resolver->resolveForRequestWithoutRoute($this->frontendRequest()));
+
+        $this->assertEquals([], $resolver->resolveForRequestWithoutRoute($this->adminRequest('/wp-admin')));
+        $this->assertEquals([], $resolver->resolveForRequestWithoutRoute($this->apiRequest()));
+    }
+
+    /**
+     * @test
+     */
+    public function middleware_caching_detects_recursion() :void
     {
         $this->expectException(MiddlewareRecursion::class);
         $this->expectExceptionMessage('Detected middleware recursion: global->group2->group3->group2');
-
+        
         new MiddlewareResolver(
             [],
             [],
@@ -218,4 +273,5 @@ public function middleware_caching_detects_recursion(): void
             []
         );
     }
+
 }