Merge pull request #585 from flightphp/middleware-use-container

Allowed for middlewares to be created with container
flightphp · May 9, 2024 · c3c5c29 · c3c5c29
2 parents e18c8a7 + f8811e1
commit c3c5c29
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 51 deletions.
diff --git a/flight/Engine.php b/flight/Engine.php
@@ -382,64 +382,82 @@ public function path(string $dir): void
      * Processes each routes middleware.
      *
      * @param Route $route The route to process the middleware for.
-     * @param string $event_name If this is the before or after method.
+     * @param string $eventName If this is the before or after method.
      */
-    protected function processMiddleware(Route $route, string $event_name): bool
+    protected function processMiddleware(Route $route, string $eventName): bool
     {
-        $at_least_one_middleware_failed = false;
+        $atLeastOneMiddlewareFailed = false;
 
-        $middlewares = $event_name === Dispatcher::FILTER_BEFORE ? $route->middleware : array_reverse($route->middleware);
+		// Process things normally for before, and then in reverse order for after.
+        $middlewares = $eventName === Dispatcher::FILTER_BEFORE 
+			? $route->middleware 
+			: array_reverse($route->middleware);
         $params = $route->params;
 
         foreach ($middlewares as $middleware) {
-            $middleware_object = false;
-
-            if ($event_name === Dispatcher::FILTER_BEFORE) {
-                // can be a callable or a class
-                $middleware_object = (is_callable($middleware) === true
-                    ? $middleware
-                    : (method_exists($middleware, Dispatcher::FILTER_BEFORE) === true
-                        ? [$middleware, Dispatcher::FILTER_BEFORE]
-                        : false
-                    )
-                );
-            } elseif ($event_name === Dispatcher::FILTER_AFTER) {
-                // must be an object. No functions allowed here
-                if (
-                    is_object($middleware) === true
-                    && !($middleware instanceof Closure)
-                    && method_exists($middleware, Dispatcher::FILTER_AFTER) === true
-                ) {
-                    $middleware_object = [$middleware, Dispatcher::FILTER_AFTER];
+
+			// Assume that nothing is going to be executed for the middleware.
+            $middlewareObject = false;
+
+            // Closure functions can only run on the before event
+            if ($eventName === Dispatcher::FILTER_BEFORE && is_object($middleware) === true && ($middleware instanceof Closure)) {
+                $middlewareObject = $middleware;
+
+            // If the object has already been created, we can just use it if the event name exists.
+            } elseif (is_object($middleware) === true) {
+                $middlewareObject = method_exists($middleware, $eventName) === true ? [ $middleware, $eventName ] : false;
+
+            // If the middleware is a string, we need to create the object and then call the event.
+            } elseif (is_string($middleware) === true && method_exists($middleware, $eventName) === true) {
+                $resolvedClass = null;
+
+                // if there's a container assigned, we should use it to create the object
+                if ($this->dispatcher->mustUseContainer($middleware) === true) {
+                    $resolvedClass = $this->dispatcher->resolveContainerClass($middleware, $params);
+                // otherwise just assume it's a plain jane class, so inject the engine
+                // just like in Dispatcher::invokeCallable()
+                } elseif (class_exists($middleware) === true) {
+                    $resolvedClass = new $middleware($this);
+                }
+
+                // If something was resolved, create an array callable that will be passed in later.
+                if ($resolvedClass !== null) {
+                    $middlewareObject = [ $resolvedClass, $eventName ];
                 }
             }
 
-            if ($middleware_object === false) {
+			// If nothing was resolved, go to the next thing
+            if ($middlewareObject === false) {
                 continue;
             }
 
-            $use_v3_output_buffering =
+			// This is the way that v3 handles output buffering (which captures output correctly)
+            $useV3OutputBuffering =
                 $this->response()->v2_output_buffering === false &&
                 $route->is_streamed === false;
 
-            if ($use_v3_output_buffering === true) {
+            if ($useV3OutputBuffering === true) {
                 ob_start();
             }
 
-            // It's assumed if you don't declare before, that it will be assumed as the before method
-            $middleware_result = $middleware_object($params);
+            // Here is the array callable $middlewareObject that we created earlier.
+			// It looks bizarre but it's really calling [ $class, $method ]($params)
+			// Which loosely translates to $class->$method($params)
+            $middlewareResult = $middlewareObject($params);
 
-            if ($use_v3_output_buffering === true) {
+            if ($useV3OutputBuffering === true) {
                 $this->response()->write(ob_get_clean());
             }
 
-            if ($middleware_result === false) {
-                $at_least_one_middleware_failed = true;
+			// If you return false in your middleware, it will halt the request
+			// and throw a 403 forbidden error by default.
+            if ($middlewareResult === false) {
+                $atLeastOneMiddlewareFailed = true;
                 break;
             }
         }
 
-        return $at_least_one_middleware_failed;
+        return $atLeastOneMiddlewareFailed;
     }
 
     ////////////////////////
@@ -475,7 +493,7 @@ public function _start(): void
         }
 
         // Route the request
-        $failed_middleware_check = false;
+        $failedMiddlewareCheck = false;
 
         while ($route = $router->route($request)) {
             $params = array_values($route->params);
@@ -506,18 +524,18 @@ public function _start(): void
 
             // Run any before middlewares
             if (count($route->middleware) > 0) {
-                $at_least_one_middleware_failed = $this->processMiddleware($route, 'before');
-                if ($at_least_one_middleware_failed === true) {
-                    $failed_middleware_check = true;
+                $atLeastOneMiddlewareFailed = $this->processMiddleware($route, 'before');
+                if ($atLeastOneMiddlewareFailed === true) {
+                    $failedMiddlewareCheck = true;
                     break;
                 }
             }
 
-            $use_v3_output_buffering =
+            $useV3OutputBuffering =
                 $this->response()->v2_output_buffering === false &&
                 $route->is_streamed === false;
 
-            if ($use_v3_output_buffering === true) {
+            if ($useV3OutputBuffering === true) {
                 ob_start();
             }
 
@@ -527,17 +545,17 @@ public function _start(): void
                 $params
             );
 
-            if ($use_v3_output_buffering === true) {
+            if ($useV3OutputBuffering === true) {
                 $response->write(ob_get_clean());
             }
 
             // Run any before middlewares
             if (count($route->middleware) > 0) {
                 // process the middleware in reverse order now
-                $at_least_one_middleware_failed = $this->processMiddleware($route, 'after');
+                $atLeastOneMiddlewareFailed = $this->processMiddleware($route, 'after');
 
-                if ($at_least_one_middleware_failed === true) {
-                    $failed_middleware_check = true;
+                if ($atLeastOneMiddlewareFailed === true) {
+                    $failedMiddlewareCheck = true;
                     break;
                 }
             }
@@ -558,7 +576,7 @@ public function _start(): void
             $response->clearBody();
         }
 
-        if ($failed_middleware_check === true) {
+        if ($failedMiddlewareCheck === true) {
             $this->halt(403, 'Forbidden', empty(getenv('PHPUNIT_TEST')));
         } elseif ($dispatched === false) {
             $this->notFound();

diff --git a/flight/core/Dispatcher.php b/flight/core/Dispatcher.php
@@ -356,10 +356,7 @@ public function invokeCallable($func, array &$params = [])
 
         [$class, $method] = $func;
 
-        $mustUseTheContainer = $this->containerHandler !== null && (
-            (is_object($class) === true && strpos(get_class($class), 'flight\\') === false)
-            || is_string($class)
-        );
+        $mustUseTheContainer = $this->mustUseContainer($class);
 
         if ($mustUseTheContainer === true) {
             $resolvedClass = $this->resolveContainerClass($class, $params);
@@ -437,7 +434,7 @@ protected function verifyValidClassCallable($class, $method, $resolvedClass): vo
      *
      * @return ?object Class object.
      */
-    protected function resolveContainerClass(string $class, array &$params)
+    public function resolveContainerClass(string $class, array &$params)
     {
         // PSR-11
         if (
@@ -468,6 +465,21 @@ protected function resolveContainerClass(string $class, array &$params)
         return null;
     }
 
+    /**
+     * Checks to see if a container should be used or not.
+     *
+     * @param string|object $class the class to verify
+     *
+     * @return boolean
+     */
+    public function mustUseContainer($class): bool
+    {
+        return $this->containerHandler !== null && (
+            (is_object($class) === true && strpos(get_class($class), 'flight\\') === false)
+            || is_string($class)
+        );
+    }
+
     /** Because this could throw an exception in the middle of an output buffer, */
     protected function fixOutputBuffering(): void
     {

diff --git a/flight/net/Route.php b/flight/net/Route.php
@@ -63,7 +63,7 @@ class Route
     /**
      * The middleware to be applied to the route
      *
-     * @var array<int, callable|object>
+     * @var array<int, callable|object|string>
      */
     public array $middleware = [];
 
@@ -226,7 +226,7 @@ public function setAlias(string $alias): self
     /**
      * Sets the route middleware
      *
-     * @param array<int, callable>|callable $middleware
+     * @param array<int, callable|string>|callable|string $middleware
      */
     public function addMiddleware($middleware): self
     {

diff --git a/phpunit.xml b/phpunit.xml
@@ -11,10 +11,13 @@
     stopOnFailure="true"
     verbose="true"
     colors="true">
-    <coverage processUncoveredFiles="true">
+    <coverage processUncoveredFiles="false">
         <include>
             <directory suffix=".php">flight/</directory>
         </include>
+		<exclude>
+			<file>flight/autoload.php</file>
+		</exclude>
     </coverage>
     <testsuites>
         <testsuite name="default">

diff --git a/tests/EngineTest.php b/tests/EngineTest.php
@@ -572,6 +572,49 @@ public function after($params)
         $this->expectOutputString('OK123after123');
     }
 
+	public function testMiddlewareClassStringNoContainer()
+    {
+        $middleware = new class {
+            public function after($params)
+            {
+                echo 'after' . $params['id'];
+            }
+        };
+        $engine = new Engine();
+
+        $engine->route('/path1/@id', function ($id) {
+            echo 'OK' . $id;
+        })
+            ->addMiddleware(get_class($middleware));
+        $engine->request()->url = '/path1/123';
+        $engine->start();
+        $this->expectOutputString('OK123after123');
+    }
+
+	public function testMiddlewareClassStringWithContainer()
+    {
+
+		$engine = new Engine();
+        $dice = new \Dice\Dice();
+        $dice = $dice->addRule('*', [
+            'substitutions' => [
+                Engine::class => $engine
+            ]
+        ]);
+        $engine->registerContainerHandler(function ($class, $params) use ($dice) {
+            return $dice->create($class, $params);
+        });
+
+
+        $engine->route('/path1/@id', function ($id) {
+            echo 'OK' . $id;
+        })
+            ->addMiddleware(ContainerDefault::class);
+        $engine->request()->url = '/path1/123';
+        $engine->start();
+        $this->expectOutputString('I returned before the route was called with the following parameters: {"id":"123"}OK123');
+    }
+
     public function testMiddlewareClassAfterFailedCheck()
     {
         $middleware = new class {

diff --git a/tests/classes/ContainerDefault.php b/tests/classes/ContainerDefault.php
@@ -15,6 +15,11 @@ public function __construct(Engine $engine)
         $this->app = $engine;
     }
 
+    public function before(array $params)
+    {
+        echo 'I returned before the route was called with the following parameters: ' . json_encode($params);
+    }
+
     public function testTheContainer()
     {
         return $this->app->get('test_me_out');