diff --git a/flight/Engine.php b/flight/Engine.php index e0b4db2..47e84f7 100644 --- a/flight/Engine.php +++ b/flight/Engine.php @@ -382,64 +382,82 @@ class Engine * 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 @@ class Engine } // Route the request - $failed_middleware_check = false; + $failedMiddlewareCheck = false; while ($route = $router->route($request)) { $params = array_values($route->params); @@ -506,18 +524,18 @@ class Engine // 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 @@ class Engine $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 @@ class Engine $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 index 1d40f95..20e27b8 100644 --- a/flight/core/Dispatcher.php +++ b/flight/core/Dispatcher.php @@ -356,10 +356,7 @@ class Dispatcher [$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 @@ class Dispatcher * * @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 @@ class Dispatcher 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 index 4d005b9..4e6e83c 100644 --- 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 + * @var array */ public array $middleware = []; @@ -226,7 +226,7 @@ class Route /** * Sets the route middleware * - * @param array|callable $middleware + * @param array|callable|string $middleware */ public function addMiddleware($middleware): self { diff --git a/phpunit.xml b/phpunit.xml index 18134c0..cb460e5 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -11,10 +11,13 @@ stopOnFailure="true" verbose="true" colors="true"> - + flight/ + + flight/autoload.php + diff --git a/tests/EngineTest.php b/tests/EngineTest.php index 6c1837e..ee6ad76 100644 --- a/tests/EngineTest.php +++ b/tests/EngineTest.php @@ -572,6 +572,49 @@ class EngineTest extends TestCase $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 index a71ff1f..1f91aec 100644 --- a/tests/classes/ContainerDefault.php +++ b/tests/classes/ContainerDefault.php @@ -15,6 +15,11 @@ class ContainerDefault $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');