From 33445b4be56f14217e9c4121be6d6f9aeefbfebe Mon Sep 17 00:00:00 2001 From: Matheus Marques Date: Sat, 28 Nov 2015 20:07:37 -0200 Subject: [PATCH 1/3] Removed \Slim\Router->finalize() Made Route finalize Lazy --- Slim/App.php | 3 +-- Slim/Interfaces/RouterInterface.php | 8 ------ Slim/Route.php | 23 +++++++++++----- Slim/Router.php | 18 ------------- tests/AppTest.php | 41 ----------------------------- 5 files changed, 17 insertions(+), 76 deletions(-) diff --git a/Slim/App.php b/Slim/App.php index 85de30a6d..0220f44c4 100644 --- a/Slim/App.php +++ b/Slim/App.php @@ -293,9 +293,8 @@ public function run($silent = false) $request = $this->container->get('request'); $response = $this->container->get('response'); - // Finalize routes here for middleware stack & ensure basePath is set + // Ensure basePath is set $router = $this->container->get('router'); - $router->finalize(); if (is_callable([$request->getUri(), 'getBasePath']) && is_callable([$router, 'setBasePath'])) { $router->setBasePath($request->getUri()->getBasePath()); } diff --git a/Slim/Interfaces/RouterInterface.php b/Slim/Interfaces/RouterInterface.php index 75eb7de32..8df636867 100644 --- a/Slim/Interfaces/RouterInterface.php +++ b/Slim/Interfaces/RouterInterface.php @@ -31,14 +31,6 @@ interface RouterInterface */ public function map($methods, $pattern, $handler); - - /** - * Finalize registered routes in preparation for dispatching - * - * NOTE: The routes can only be finalized once. - */ - public function finalize(); - /** * Dispatch router for HTTP request * diff --git a/Slim/Route.php b/Slim/Route.php index d77eeb262..8541406fd 100644 --- a/Slim/Route.php +++ b/Slim/Route.php @@ -52,6 +52,8 @@ class Route extends Routable implements RouteInterface */ protected $groups; + private $finalized = false; + /** * Output buffering mode * @@ -110,15 +112,19 @@ public function add($callable) */ public function finalize() { - $groupMiddleware = []; - foreach ($this->getGroups() as $group) { - $groupMiddleware = array_merge($group->getMiddleware(), $groupMiddleware); - } + if (!$this->finalized) { + $groupMiddleware = []; + foreach ($this->getGroups() as $group) { + $groupMiddleware = array_merge($group->getMiddleware(), $groupMiddleware); + } - $this->middleware = array_merge($this->middleware, $groupMiddleware); + $this->middleware = array_merge($this->middleware, $groupMiddleware); - foreach ($this->getMiddleware() as $middleware) { - $this->addMiddleware($middleware); + foreach ($this->getMiddleware() as $middleware) { + $this->addMiddleware($middleware); + } + + $this->finalized = true; } } @@ -302,6 +308,9 @@ public function prepare(ServerRequestInterface $request, array $arguments) */ public function run(ServerRequestInterface $request, ResponseInterface $response) { + // Lazy Route Finalize + $this->finalize(); + // Traverse middleware stack and fetch updated response return $this->callMiddlewareStack($request, $response); } diff --git a/Slim/Router.php b/Slim/Router.php index 326cee8cf..b85f31114 100644 --- a/Slim/Router.php +++ b/Slim/Router.php @@ -71,8 +71,6 @@ class Router implements RouterInterface */ protected $routeGroups = []; - private $finalized = false; - /** * @var \FastRoute\Dispatcher */ @@ -139,21 +137,6 @@ public function map($methods, $pattern, $handler) return $route; } - /** - * Finalize registered routes in preparation for dispatching - * - * NOTE: The routes can only be finalized once. - */ - public function finalize() - { - if (!$this->finalized) { - foreach ($this->getRoutes() as $route) { - $route->finalize(); - } - $this->finalized = true; - } - } - /** * Dispatch router for HTTP request * @@ -165,7 +148,6 @@ public function finalize() */ public function dispatch(ServerRequestInterface $request) { - $this->finalize(); $uri = '/' . ltrim($request->getUri()->getPath(), '/'); return $this->createDispatcher()->dispatch( diff --git a/tests/AppTest.php b/tests/AppTest.php index 76f921c56..106134d87 100644 --- a/tests/AppTest.php +++ b/tests/AppTest.php @@ -160,7 +160,6 @@ public function testSegmentRouteThatDoesNotEndInASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo', 'pattern', $router->lookupRoute('route0')); } @@ -172,7 +171,6 @@ public function testSegmentRouteThatEndsInASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/', 'pattern', $router->lookupRoute('route0')); } @@ -184,7 +182,6 @@ public function testSegmentRouteThatDoesNotStartWithASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('foo', 'pattern', $router->lookupRoute('route0')); } @@ -196,7 +193,6 @@ public function testSingleSlashRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/', 'pattern', $router->lookupRoute('route0')); } @@ -208,7 +204,6 @@ public function testEmptyRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('', 'pattern', $router->lookupRoute('route0')); } @@ -225,7 +220,6 @@ public function testGroupSegmentWithSegmentRouteThatDoesNotEndInASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/bar', 'pattern', $router->lookupRoute('route0')); } @@ -239,7 +233,6 @@ public function testGroupSegmentWithSegmentRouteThatEndsInASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/bar/', 'pattern', $router->lookupRoute('route0')); } @@ -253,7 +246,6 @@ public function testGroupSegmentWithSingleSlashRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/', 'pattern', $router->lookupRoute('route0')); } @@ -267,7 +259,6 @@ public function testGroupSegmentWithEmptyRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo', 'pattern', $router->lookupRoute('route0')); } @@ -283,7 +274,6 @@ public function testTwoGroupSegmentsWithSingleSlashRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/baz/', 'pattern', $router->lookupRoute('route0')); } @@ -299,7 +289,6 @@ public function testTwoGroupSegmentsWithAnEmptyRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/baz', 'pattern', $router->lookupRoute('route0')); } @@ -315,7 +304,6 @@ public function testTwoGroupSegmentsWithSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/baz/bar', 'pattern', $router->lookupRoute('route0')); } @@ -331,7 +319,6 @@ public function testTwoGroupSegmentsWithSegmentRouteThatHasATrailingSlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/baz/bar/', 'pattern', $router->lookupRoute('route0')); } @@ -347,7 +334,6 @@ public function testGroupSegmentWithSingleSlashNestedGroupAndSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo//bar', 'pattern', $router->lookupRoute('route0')); } @@ -363,7 +349,6 @@ public function testGroupSegmentWithSingleSlashGroupAndSegmentRouteWithoutLeadin }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/bar', 'pattern', $router->lookupRoute('route0')); } @@ -379,7 +364,6 @@ public function testGroupSegmentWithEmptyNestedGroupAndSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foo/bar', 'pattern', $router->lookupRoute('route0')); } @@ -395,7 +379,6 @@ public function testGroupSegmentWithEmptyNestedGroupAndSegmentRouteWithoutLeadin }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/foobar', 'pattern', $router->lookupRoute('route0')); } @@ -409,7 +392,6 @@ public function testGroupSingleSlashWithSegmentRouteThatDoesNotEndInASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//bar', 'pattern', $router->lookupRoute('route0')); } @@ -423,7 +405,6 @@ public function testGroupSingleSlashWithSegmentRouteThatEndsInASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//bar/', 'pattern', $router->lookupRoute('route0')); } @@ -437,7 +418,6 @@ public function testGroupSingleSlashWithSingleSlashRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//', 'pattern', $router->lookupRoute('route0')); } @@ -451,7 +431,6 @@ public function testGroupSingleSlashWithEmptyRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/', 'pattern', $router->lookupRoute('route0')); } @@ -467,7 +446,6 @@ public function testGroupSingleSlashWithNestedGroupSegmentWithSingleSlashRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//baz/', 'pattern', $router->lookupRoute('route0')); } @@ -483,7 +461,6 @@ public function testGroupSingleSlashWithNestedGroupSegmentWithAnEmptyRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//baz', 'pattern', $router->lookupRoute('route0')); } @@ -499,7 +476,6 @@ public function testGroupSingleSlashWithNestedGroupSegmentWithSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//baz/bar', 'pattern', $router->lookupRoute('route0')); } @@ -515,7 +491,6 @@ public function testGroupSingleSlashWithNestedGroupSegmentWithSegmentRouteThatHa }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//baz/bar/', 'pattern', $router->lookupRoute('route0')); } @@ -531,7 +506,6 @@ public function testGroupSingleSlashWithSingleSlashNestedGroupAndSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('///bar', 'pattern', $router->lookupRoute('route0')); } @@ -547,7 +521,6 @@ public function testGroupSingleSlashWithSingleSlashGroupAndSegmentRouteWithoutLe }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//bar', 'pattern', $router->lookupRoute('route0')); } @@ -563,7 +536,6 @@ public function testGroupSingleSlashWithEmptyNestedGroupAndSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//bar', 'pattern', $router->lookupRoute('route0')); } @@ -579,7 +551,6 @@ public function testGroupSingleSlashWithEmptyNestedGroupAndSegmentRouteWithoutLe }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/bar', 'pattern', $router->lookupRoute('route0')); } @@ -593,7 +564,6 @@ public function testEmptyGroupWithSegmentRouteThatDoesNotEndInASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/bar', 'pattern', $router->lookupRoute('route0')); } @@ -607,7 +577,6 @@ public function testEmptyGroupWithSegmentRouteThatEndsInASlash() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/bar/', 'pattern', $router->lookupRoute('route0')); } @@ -621,7 +590,6 @@ public function testEmptyGroupWithSingleSlashRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/', 'pattern', $router->lookupRoute('route0')); } @@ -635,7 +603,6 @@ public function testEmptyGroupWithEmptyRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('', 'pattern', $router->lookupRoute('route0')); } @@ -651,7 +618,6 @@ public function testEmptyGroupWithNestedGroupSegmentWithSingleSlashRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/baz/', 'pattern', $router->lookupRoute('route0')); } @@ -667,7 +633,6 @@ public function testEmptyGroupWithNestedGroupSegmentWithAnEmptyRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/baz', 'pattern', $router->lookupRoute('route0')); } @@ -683,7 +648,6 @@ public function testEmptyGroupWithNestedGroupSegmentWithSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/baz/bar', 'pattern', $router->lookupRoute('route0')); } @@ -699,7 +663,6 @@ public function testEmptyGroupWithNestedGroupSegmentWithSegmentRouteThatHasATrai }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/baz/bar/', 'pattern', $router->lookupRoute('route0')); } @@ -715,7 +678,6 @@ public function testEmptyGroupWithSingleSlashNestedGroupAndSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('//bar', 'pattern', $router->lookupRoute('route0')); } @@ -731,7 +693,6 @@ public function testEmptyGroupWithSingleSlashGroupAndSegmentRouteWithoutLeadingS }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/bar', 'pattern', $router->lookupRoute('route0')); } @@ -747,7 +708,6 @@ public function testEmptyGroupWithEmptyNestedGroupAndSegmentRoute() }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('/bar', 'pattern', $router->lookupRoute('route0')); } @@ -763,7 +723,6 @@ public function testEmptyGroupWithEmptyNestedGroupAndSegmentRouteWithoutLeadingS }); /** @var \Slim\Router $router */ $router = $app->getContainer()->get('router'); - $router->finalize(); $this->assertAttributeEquals('bar', 'pattern', $router->lookupRoute('route0')); } From 1b08820fe0dc3be3c7771070c0be8ab6f8500960 Mon Sep 17 00:00:00 2001 From: Matheus Marques Date: Sun, 29 Nov 2015 09:59:53 -0200 Subject: [PATCH 2/3] Changed if (!$this->finalized) --- Slim/Route.php | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/Slim/Route.php b/Slim/Route.php index 8541406fd..3770ff5c8 100644 --- a/Slim/Route.php +++ b/Slim/Route.php @@ -112,20 +112,22 @@ public function add($callable) */ public function finalize() { - if (!$this->finalized) { - $groupMiddleware = []; - foreach ($this->getGroups() as $group) { - $groupMiddleware = array_merge($group->getMiddleware(), $groupMiddleware); - } + if ($this->finalized) { + return; + } - $this->middleware = array_merge($this->middleware, $groupMiddleware); + $groupMiddleware = []; + foreach ($this->getGroups() as $group) { + $groupMiddleware = array_merge($group->getMiddleware(), $groupMiddleware); + } - foreach ($this->getMiddleware() as $middleware) { - $this->addMiddleware($middleware); - } + $this->middleware = array_merge($this->middleware, $groupMiddleware); - $this->finalized = true; + foreach ($this->getMiddleware() as $middleware) { + $this->addMiddleware($middleware); } + + $this->finalized = true; } /** @@ -308,7 +310,7 @@ public function prepare(ServerRequestInterface $request, array $arguments) */ public function run(ServerRequestInterface $request, ResponseInterface $response) { - // Lazy Route Finalize + // Lazy route finalize $this->finalize(); // Traverse middleware stack and fetch updated response From d7085acda3e8255f0b6ae8d933ab3fb1a3b4c390 Mon Sep 17 00:00:00 2001 From: Rob Allen Date: Sun, 29 Nov 2015 14:40:39 +0000 Subject: [PATCH 3/3] Improve comment --- Slim/Route.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Slim/Route.php b/Slim/Route.php index 3770ff5c8..5d7b8d478 100644 --- a/Slim/Route.php +++ b/Slim/Route.php @@ -310,7 +310,7 @@ public function prepare(ServerRequestInterface $request, array $arguments) */ public function run(ServerRequestInterface $request, ResponseInterface $response) { - // Lazy route finalize + // Finalise route now that we are about to run it $this->finalize(); // Traverse middleware stack and fetch updated response