From 3a292554594be09e3b81bb047c3a017e2fb9c424 Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Mon, 18 Mar 2019 21:28:19 -0600 Subject: [PATCH 1/9] refactor RouteGroupInterface and RouteGroup implementation --- Slim/Interfaces/RouteGroupInterface.php | 27 +++++------ Slim/RouteGroup.php | 61 ++++++++++++++----------- Slim/Router.php | 2 +- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/Slim/Interfaces/RouteGroupInterface.php b/Slim/Interfaces/RouteGroupInterface.php index f5a01c7e6..a7c69eb3e 100644 --- a/Slim/Interfaces/RouteGroupInterface.php +++ b/Slim/Interfaces/RouteGroupInterface.php @@ -11,22 +11,10 @@ use Psr\Http\Server\MiddlewareInterface; use Slim\App; +use Slim\MiddlewareDispatcher; -/** - * RouteGroup Interface - * - * @package Slim - * @since 3.0.0 - */ interface RouteGroupInterface { - /** - * Get route pattern - * - * @return string - */ - public function getPattern(): string; - /** * @param MiddlewareInterface|string|callable $middleware * @return RouteGroupInterface @@ -39,6 +27,19 @@ public function add($middleware): RouteGroupInterface; */ public function addMiddleware(MiddlewareInterface $middleware): RouteGroupInterface; + /** + * @param MiddlewareDispatcher $dispatcher + * @return RouteGroupInterface + */ + public function appendMiddlewareToDispatcher(MiddlewareDispatcher $dispatcher): RouteGroupInterface; + + /** + * Get route pattern + * + * @return string + */ + public function getPattern(): string; + /** * Execute route group callable in the context of the Slim App * diff --git a/Slim/RouteGroup.php b/Slim/RouteGroup.php index bac6ad968..1bd4e8c68 100644 --- a/Slim/RouteGroup.php +++ b/Slim/RouteGroup.php @@ -9,48 +9,51 @@ namespace Slim; -use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Server\MiddlewareInterface; use Slim\Interfaces\CallableResolverInterface; use Slim\Interfaces\RouteGroupInterface; -/** - * A collector for Routable objects with a common middleware stack - * - * @package Slim - */ -class RouteGroup extends Routable implements RouteGroupInterface +class RouteGroup implements RouteGroupInterface { /** - * middleware - * - * @var array + * @var callable|string + */ + protected $callable; + + /** + * @var CallableResolverInterface + */ + protected $callableResolver; + + /** + * @var MiddlewareInterface[]|string[]|callable[] */ protected $middleware = []; + /** + * @var string + */ + protected $pattern; + /** * Create a new RouteGroup * * @param string $pattern The pattern prefix for the group * @param callable $callable The group callable - * @param ResponseFactoryInterface $responseFactory * @param CallableResolverInterface $callableResolver */ public function __construct( string $pattern, $callable, - ResponseFactoryInterface $responseFactory, CallableResolverInterface $callableResolver ) { $this->pattern = $pattern; $this->callable = $callable; - $this->responseFactory = $responseFactory; $this->callableResolver = $callableResolver; } /** - * @param MiddlewareInterface|string|callable $middleware - * @return RouteGroupInterface + * {@inheritdoc} */ public function add($middleware): RouteGroupInterface { @@ -59,8 +62,7 @@ public function add($middleware): RouteGroupInterface } /** - * @param MiddlewareInterface $middleware - * @return RouteGroupInterface + * {@inheritdoc} */ public function addMiddleware(MiddlewareInterface $middleware): RouteGroupInterface { @@ -69,20 +71,27 @@ public function addMiddleware(MiddlewareInterface $middleware): RouteGroupInterf } /** - * Get the middleware registered for the group - * - * @return mixed[] + * {@inheritdoc} */ - public function getMiddleware(): array + public function appendMiddlewareToDispatcher(MiddlewareDispatcher $dispatcher): RouteGroupInterface { - return $this->middleware; + foreach ($this->middleware as $middleware) { + $dispatcher->add($middleware); + } + + return $this; } /** - * Invoke the group to register any Routable objects within it. - * - * @param App $app The App instance to bind/pass to the group callable - * @return self + * {@inheritdoc} + */ + public function getPattern(): string + { + return $this->pattern; + } + + /** + * {@inheritdoc} */ public function __invoke(App $app = null): RouteGroupInterface { diff --git a/Slim/Router.php b/Slim/Router.php index ee30a5770..9dae154ac 100644 --- a/Slim/Router.php +++ b/Slim/Router.php @@ -365,7 +365,7 @@ protected function processGroups(): string */ public function pushGroup(string $pattern, $callable): RouteGroupInterface { - $group = new RouteGroup($pattern, $callable, $this->responseFactory, $this->callableResolver); + $group = new RouteGroup($pattern, $callable, $this->callableResolver); $this->routeGroups[] = $group; return $group; } From f95ba87a1defd14fc30d9bc32b7e0873080bbfb2 Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Mon, 18 Mar 2019 21:28:45 -0600 Subject: [PATCH 2/9] refactor RouteInterface and Route implementation --- Slim/Interfaces/RouteInterface.php | 6 - Slim/Route.php | 177 ++++++++++++----------------- 2 files changed, 73 insertions(+), 110 deletions(-) diff --git a/Slim/Interfaces/RouteInterface.php b/Slim/Interfaces/RouteInterface.php index 5efa478b7..56b2500b9 100644 --- a/Slim/Interfaces/RouteInterface.php +++ b/Slim/Interfaces/RouteInterface.php @@ -13,12 +13,6 @@ use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\MiddlewareInterface; -/** - * Route Interface - * - * @package Slim - * @since 3.0.0 - */ interface RouteInterface { /** diff --git a/Slim/Route.php b/Slim/Route.php index fc6624ab3..8868852bd 100644 --- a/Slim/Route.php +++ b/Slim/Route.php @@ -19,12 +19,10 @@ use Slim\Handlers\Strategies\RequestResponse; use Slim\Interfaces\CallableResolverInterface; use Slim\Interfaces\InvocationStrategyInterface; +use Slim\Interfaces\RouteGroupInterface; use Slim\Interfaces\RouteInterface; -/** - * Route - */ -class Route extends Routable implements RouteInterface, RequestHandlerInterface +class Route implements RouteInterface, RequestHandlerInterface { /** * HTTP methods supported by this route @@ -50,15 +48,10 @@ class Route extends Routable implements RouteInterface, RequestHandlerInterface /** * Parent route groups * - * @var RouteGroup[] + * @var RouteGroupInterface[] */ protected $groups; - /** - * @var bool - */ - private $finalized = false; - /** * @var InvocationStrategyInterface */ @@ -90,6 +83,30 @@ class Route extends Routable implements RouteInterface, RequestHandlerInterface */ protected $middlewareDispatcher; + /** + * Route callable + * + * @var callable|string + */ + protected $callable; + + /** + * @var CallableResolverInterface + */ + protected $callableResolver; + + /** + * @var ResponseFactoryInterface + */ + protected $responseFactory; + + /** + * Route pattern + * + * @var string + */ + protected $pattern; + /** * Create new route * @@ -127,9 +144,7 @@ public function __construct( } /** - * Get route invocation strategy - * - * @return InvocationStrategyInterface + * {@inheritdoc} */ public function getInvocationStrategy(): InvocationStrategyInterface { @@ -137,9 +152,7 @@ public function getInvocationStrategy(): InvocationStrategyInterface } /** - * Get route callable - * - * @return callable|string + * {@inheritdoc} */ public function getCallable() { @@ -147,21 +160,16 @@ public function getCallable() } /** - * This method enables you to override the Route's callable - * - * @param callable|string $callable - * @return self + * {@inheritdoc} */ - public function setCallable($callable): RouteInterface + public function setCallable($callable): self { $this->callable = $callable; return $this; } /** - * Get route methods - * - * @return string[] + * {@inheritdoc} */ public function getMethods(): array { @@ -169,9 +177,7 @@ public function getMethods(): array } /** - * Get parent route groups - * - * @return RouteGroup[] + * {@inheritdoc} */ public function getGroups(): array { @@ -179,9 +185,7 @@ public function getGroups(): array } /** - * Get route name - * - * @return string|null + * {@inheritdoc} */ public function getName(): ?string { @@ -189,9 +193,7 @@ public function getName(): ?string } /** - * Get route identifier - * - * @return string + * {@inheritdoc} */ public function getIdentifier(): string { @@ -199,11 +201,7 @@ public function getIdentifier(): string } /** - * Set route name - * - * @param string $name - * - * @return self + * {@inheritdoc} */ public function setName(string $name): RouteInterface { @@ -212,11 +210,31 @@ public function setName(string $name): RouteInterface } /** - * Retrieve a specific route argument - * - * @param string $name - * @param string|null $default - * @return mixed + * {@inheritdoc} + */ + public function getCallableResolver(): CallableResolverInterface + { + return $this->callableResolver; + } + + /** + * {@inheritdoc} + */ + public function getPattern(): string + { + return $this->pattern; + } + + /** + * {@inheritdoc} + */ + public function setPattern(string $newPattern) + { + $this->pattern = $newPattern; + } + + /** + * {@inheritdoc} */ public function getArgument(string $name, $default = null) { @@ -227,12 +245,7 @@ public function getArgument(string $name, $default = null) } /** - * Set a route argument - * - * @param string $name - * @param string $value - * @param bool $includeInSavedArguments - * @return self + * {@inheritdoc} */ public function setArgument(string $name, string $value, bool $includeInSavedArguments = true): RouteInterface { @@ -245,9 +258,7 @@ public function setArgument(string $name, string $value, bool $includeInSavedArg } /** - * Retrieve route arguments - * - * @return array + * {@inheritdoc} */ public function getArguments(): array { @@ -255,11 +266,7 @@ public function getArguments(): array } /** - * Replace route arguments - * - * @param array $arguments - * @param bool $includeInSavedArguments - * @return self + * {@inheritdoc} */ public function setArguments(array $arguments, bool $includeInSavedArguments = true): RouteInterface { @@ -272,8 +279,7 @@ public function setArguments(array $arguments, bool $includeInSavedArguments = t } /** - * @param MiddlewareInterface|string|callable $middleware - * @return RouteInterface + * {@inheritdoc} */ public function add($middleware): RouteInterface { @@ -282,8 +288,7 @@ public function add($middleware): RouteInterface } /** - * @param MiddlewareInterface $middleware - * @return RouteInterface + * {@inheritdoc} */ public function addMiddleware(MiddlewareInterface $middleware): RouteInterface { @@ -296,11 +301,7 @@ public function addMiddleware(MiddlewareInterface $middleware): RouteInterface *******************************************************************************/ /** - * Prepare the route for use - * - * @param ServerRequestInterface $request - * @param array $arguments - * @return self + * {@inheritdoc} */ public function prepare(ServerRequestInterface $request, array $arguments): RouteInterface { @@ -312,61 +313,29 @@ public function prepare(ServerRequestInterface $request, array $arguments): Rout $this->setArgument($k, $v, false); } - return $this; - } - - /** - * Finalize the route in preparation for dispatching - * @return void - */ - public function finalize(): void - { - if ($this->finalized) { - return; - } - - // Chain a new (outer) middleware dispatcher for route-group-middleware - // to the existing route-middleware dispatcher + // Add Middleware From Groups $inner = $this->middlewareDispatcher; $this->middlewareDispatcher = new MiddlewareDispatcher($inner, $this->container); + /** @var RouteGroupInterface $group */ foreach (array_reverse($this->groups) as $group) { - foreach ($group->getMiddleware() as $middleware) { - $this->middlewareDispatcher->add($middleware); - } + $group->appendMiddlewareToDispatcher($this->middlewareDispatcher); } - $this->finalized = true; + return $this; } /** - * Run route - * - * This method traverses the middleware stack, including the route's callable - * and captures the resultant HTTP response object. It then sends the response - * back to the Application. - * - * @param ServerRequestInterface $request - * @return ResponseInterface + * {@inheritdoc} */ public function run(ServerRequestInterface $request): ResponseInterface { - // Finalise route now that we are about to run it - $this->finalize(); - // Traverse middleware stack and fetch updated response return $this->middlewareDispatcher->handle($request); } /** - * Dispatch route callable against current Request and Response objects - * - * This method invokes the route object's callable. If middleware is - * registered for the route, each callable middleware is invoked in - * the order specified. - * - * @param ServerRequestInterface $request The current Request object - * @return ResponseInterface + * {@inheritdoc} */ public function handle(ServerRequestInterface $request): ResponseInterface { From d65421d14aed397d00cceeada74c68191ad61968 Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Mon, 18 Mar 2019 21:28:53 -0600 Subject: [PATCH 3/9] remove Routable --- Slim/Routable.php | 76 ----------------------------------------------- 1 file changed, 76 deletions(-) delete mode 100644 Slim/Routable.php diff --git a/Slim/Routable.php b/Slim/Routable.php deleted file mode 100644 index 124f15327..000000000 --- a/Slim/Routable.php +++ /dev/null @@ -1,76 +0,0 @@ -callableResolver; - } - - /** - * Get the route pattern - * - * @return string - */ - public function getPattern(): string - { - return $this->pattern; - } - - /** - * Set the route pattern - * - * @param string $newPattern - * @return self - */ - public function setPattern(string $newPattern): self - { - $this->pattern = $newPattern; - return $this; - } -} From 98b1ecf3534d9d3e14d6a8971bc535a2491f38f6 Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Tue, 19 Mar 2019 15:04:58 -0600 Subject: [PATCH 4/9] add missing methods to RouteInterface --- Slim/Interfaces/RouteInterface.php | 15 +++++++++++++++ Slim/Route.php | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Slim/Interfaces/RouteInterface.php b/Slim/Interfaces/RouteInterface.php index 56b2500b9..c4e437a6b 100644 --- a/Slim/Interfaces/RouteInterface.php +++ b/Slim/Interfaces/RouteInterface.php @@ -32,6 +32,21 @@ public function getArgument(string $name, $default = null); */ public function getArguments(): array; + /** + * Get route callable + * + * @return callable|string + */ + public function getCallable(); + + /** + * Set route callable + * + * @param $callable + * @return RouteInterface + */ + public function setCallable($callable): RouteInterface; + /** * Get route name * diff --git a/Slim/Route.php b/Slim/Route.php index 8868852bd..47da63464 100644 --- a/Slim/Route.php +++ b/Slim/Route.php @@ -162,7 +162,7 @@ public function getCallable() /** * {@inheritdoc} */ - public function setCallable($callable): self + public function setCallable($callable): RouteInterface { $this->callable = $callable; return $this; From 442a40b89506c02056b53f3dc55fbd506a3469dc Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Tue, 19 Mar 2019 15:05:31 -0600 Subject: [PATCH 5/9] fix comment letter capitalization in Route --- Slim/Route.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Slim/Route.php b/Slim/Route.php index 47da63464..bc96c65aa 100644 --- a/Slim/Route.php +++ b/Slim/Route.php @@ -313,7 +313,7 @@ public function prepare(ServerRequestInterface $request, array $arguments): Rout $this->setArgument($k, $v, false); } - // Add Middleware From Groups + // Add middleware from Groups $inner = $this->middlewareDispatcher; $this->middlewareDispatcher = new MiddlewareDispatcher($inner, $this->container); From b9d1a710a71a4d90360cae9ad557406ebd75948c Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Wed, 20 Mar 2019 22:10:19 -0600 Subject: [PATCH 6/9] refactor group middleware appending logic in Route --- Slim/Interfaces/RouteInterface.php | 3 +-- Slim/Middleware/RoutingMiddleware.php | 2 +- Slim/Route.php | 23 +++++++++++++++++++---- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Slim/Interfaces/RouteInterface.php b/Slim/Interfaces/RouteInterface.php index c4e437a6b..1b97a1d62 100644 --- a/Slim/Interfaces/RouteInterface.php +++ b/Slim/Interfaces/RouteInterface.php @@ -104,11 +104,10 @@ public function addMiddleware(MiddlewareInterface $middleware): RouteInterface; /** * Prepare the route for use * - * @param ServerRequestInterface $request * @param array $arguments * @return RouteInterface */ - public function prepare(ServerRequestInterface $request, array $arguments): RouteInterface; + public function prepare(array $arguments): RouteInterface; /** * Run route diff --git a/Slim/Middleware/RoutingMiddleware.php b/Slim/Middleware/RoutingMiddleware.php index 59f5b0a44..5ac00d8c1 100644 --- a/Slim/Middleware/RoutingMiddleware.php +++ b/Slim/Middleware/RoutingMiddleware.php @@ -74,7 +74,7 @@ public function performRouting(ServerRequestInterface $request): ServerRequestIn $routeArguments = $routingResults->getRouteArguments(); $routeIdentifier = $routingResults->getRouteIdentifier() ?? ''; $route = $this->router->lookupRoute($routeIdentifier); - $route->prepare($request, $routeArguments); + $route->prepare($routeArguments); return $request ->withAttribute('route', $route) ->withAttribute('routingResults', $routingResults); diff --git a/Slim/Route.php b/Slim/Route.php index bc96c65aa..d8a3f388c 100644 --- a/Slim/Route.php +++ b/Slim/Route.php @@ -107,6 +107,11 @@ class Route implements RouteInterface, RequestHandlerInterface */ protected $pattern; + /** + * @var bool + */ + protected $groupMiddlewareAppended = false; + /** * Create new route * @@ -303,7 +308,7 @@ public function addMiddleware(MiddlewareInterface $middleware): RouteInterface /** * {@inheritdoc} */ - public function prepare(ServerRequestInterface $request, array $arguments): RouteInterface + public function prepare(array $arguments): RouteInterface { // Remove temp arguments $this->setArguments($this->savedArguments); @@ -313,7 +318,14 @@ public function prepare(ServerRequestInterface $request, array $arguments): Rout $this->setArgument($k, $v, false); } - // Add middleware from Groups + return $this; + } + + /** + * @return void + */ + protected function appendGroupMiddlewareToRoute(): void + { $inner = $this->middlewareDispatcher; $this->middlewareDispatcher = new MiddlewareDispatcher($inner, $this->container); @@ -322,7 +334,7 @@ public function prepare(ServerRequestInterface $request, array $arguments): Rout $group->appendMiddlewareToDispatcher($this->middlewareDispatcher); } - return $this; + $this->groupMiddlewareAppended = true; } /** @@ -330,7 +342,10 @@ public function prepare(ServerRequestInterface $request, array $arguments): Rout */ public function run(ServerRequestInterface $request): ResponseInterface { - // Traverse middleware stack and fetch updated response + if (!$this->groupMiddlewareAppended) { + $this->appendGroupMiddlewareToRoute(); + } + return $this->middlewareDispatcher->handle($request); } From 0067840822af04e09ce45c444e54efa8588d5361 Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Wed, 20 Mar 2019 22:11:02 -0600 Subject: [PATCH 7/9] remove superfluous test for removed Route::finalize() method --- tests/RouteTest.php | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/tests/RouteTest.php b/tests/RouteTest.php index 523f86565..74cda748c 100644 --- a/tests/RouteTest.php +++ b/tests/RouteTest.php @@ -130,7 +130,7 @@ public function testGetGroups() $callableResolver = new CallableResolver(); $strategy = new RequestResponse(); - $routeGroup = new RouteGroup('/group', $callable, $responseFactory, $callableResolver); + $routeGroup = new RouteGroup('/group', $callable, $callableResolver); $groups = [$routeGroup]; $route = new Route(['GET'], '/', $callable, $responseFactory, $callableResolver, null, $strategy, $groups); @@ -188,7 +188,7 @@ public function testAddMiddlewareOnGroup() $called++; return $handler->handle($request); }; - $routeGroup = new RouteGroup('/group', $callable, $responseFactory, $callableResolver); + $routeGroup = new RouteGroup('/group', $callable, $callableResolver); $routeGroup->add($mw); $groups = [$routeGroup]; @@ -243,25 +243,6 @@ public function testAddMiddlewareAsStringNotImplementingInterfaceThrowsException $route->add(new MockMiddlewareWithoutInterface()); } - public function testRefinalizing() - { - $route = $this->createRoute(); - $called = 0; - - $route->add(function ($request, $handler) use (&$called) { - $called++; - return $handler->handle($request); - }); - - $route->finalize(); - $route->finalize(); - - $request = $this->createServerRequest('/'); - $route->run($request); - - $this->assertSame($called, 1); - } - public function testIdentifier() { $route = $this->createRoute(); From 98753b6f61a4236909bb17aab64445a50424b805 Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Wed, 20 Mar 2019 22:18:32 -0600 Subject: [PATCH 8/9] add missing type for $callable parameter in RouteInterface::setCallable() docblock --- Slim/Interfaces/RouteInterface.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Slim/Interfaces/RouteInterface.php b/Slim/Interfaces/RouteInterface.php index 1b97a1d62..a3aa92690 100644 --- a/Slim/Interfaces/RouteInterface.php +++ b/Slim/Interfaces/RouteInterface.php @@ -42,7 +42,7 @@ public function getCallable(); /** * Set route callable * - * @param $callable + * @param callable|string $callable * @return RouteInterface */ public function setCallable($callable): RouteInterface; From 5a1d90fe853bb4126c0aea0ec4a868238cb01663 Mon Sep 17 00:00:00 2001 From: l0gicgate Date: Tue, 2 Apr 2019 12:00:20 -0600 Subject: [PATCH 9/9] add entry to CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ede67b2e..7162ff016 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ ### Removed +- [#2612](https://github.com/slimphp/Slim/pull/2612) Remove Routable, refactored RouteGroup and Route interface - [#2589](https://github.com/slimphp/Slim/pull/2589) Remove App::$settings altogether - [#2587](https://github.com/slimphp/Slim/pull/2587) Remove Pimple as a dev-dependency - [#2398](https://github.com/slimphp/Slim/pull/2398) Slim no longer has error handling built into App. Add ErrorMiddleware() as the outermost middleware.