diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index 6ae58e1..32fdcd7 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -43,7 +43,9 @@ public static function compile(Route $route) { $stripped_path = static::getPathWithoutDefaults($route); $fit = static::getFit($stripped_path); $pattern_outline = static::getPatternOutline($stripped_path); - $num_parts = count(explode('/', trim($pattern_outline, '/'))); + // We count the number of parts including any optional trailing parts. This + // allows the RouteProvider to filter candidates routes more efficiently. + $num_parts = count(explode('/', trim($route->getPath(), '/'))); return new CompiledRoute( $route, diff --git a/core/lib/Drupal/Core/Routing/RouteProvider.php b/core/lib/Drupal/Core/Routing/RouteProvider.php index 9835848..79fb7f9 100644 --- a/core/lib/Drupal/Core/Routing/RouteProvider.php +++ b/core/lib/Drupal/Core/Routing/RouteProvider.php @@ -279,10 +279,14 @@ public function getRoutesByPattern($pattern) { */ protected function getRoutesByPath($path) { // Filter out each empty value, though allow '0' and 0, which would be - // filtered out by empty(). - $parts = array_values(array_filter(explode('/', $path), function($value) { - return $value !== NULL && $value !== ''; - })); + // filtered out by empty(). Values may be empty if the incoming path + // has multiple slashes in a row, or a leading or trailing slashes. + $parts = array(); + foreach (explode('/', $path) as $value) { + if ($value !== NULL && $value !== '') { + $parts[] = $value; + } + } $collection = new RouteCollection(); @@ -291,19 +295,19 @@ protected function getRoutesByPath($path) { return $collection; } - $routes = $this->connection->query("SELECT name, route, fit FROM {" . $this->connection->escapeTable($this->tableName) . "} WHERE pattern_outline IN (:patterns)", array( - ':patterns' => $ancestors, + // The >= check on number_parts allows us to match routes with optional + // trailing wildcard parts as long as the pattern matches, since we + // dump the route pattern without those optional parts. + $routes = $this->connection->query("SELECT name, route, fit FROM {" . $this->connection->escapeTable($this->tableName) . "} WHERE pattern_outline IN (:patterns) AND number_parts >= :count_parts", array( + ':patterns' => $ancestors, ':count_parts' => count($parts), )) ->fetchAll(\PDO::FETCH_ASSOC); - // We sort but fit and name in PHP to avoid a SQL filesort. + // We sort by fit and name in PHP to avoid a SQL filesort. usort($routes, array($this, 'routeProviderRouteCompare')); foreach ($routes as $row) { - $route = unserialize($row['route']); - if (preg_match($route->compile()->getRegex(), $path, $matches)) { - $collection->add($row['name'], $route); - } + $collection->add($row['name'], unserialize($row['route'])); } return $collection; diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php index f4dadae..3cc6e82 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php @@ -36,12 +36,9 @@ public static function getInfo() { public function testCanRoute() { $this->drupalGet('router_test/test1'); $this->assertRaw('test1', 'The correct string was returned because the route was successful.'); - } - /** - * Confirms that our default controller logic works properly. - */ - public function testDefaultController() { + // Confirms that our default controller logic works properly. + $this->drupalGet('router_test/test2'); $this->assertRaw('test2', 'The correct string was returned because the route was successful.'); @@ -52,6 +49,12 @@ public function testDefaultController() { // In some instances, the subrequest handling may get confused and render // a page inception style. This test verifies that is not happening. $this->assertNoPattern('#.*#s', 'There was no double-page effect from a misrendered subrequest.'); + + // Checks the generate method on the url generator using the front router. + global $base_path; + + $this->assertEqual($this->container->get('url_generator')->generate(''), $base_path); + $this->assertEqual($this->container->get('url_generator')->generateFromPath(''), $base_path); } /** @@ -93,7 +96,7 @@ public function testControllerPlaceholdersDefaultValues() { } /** - * Confirms that default placeholders in paths work correctly. + * Test default placeholders and dynamically defined and altered routes. */ public function testControllerPlaceholdersDefaultValuesProvided() { $this->drupalGet('router_test/test4/barf'); @@ -107,18 +110,21 @@ public function testControllerPlaceholdersDefaultValuesProvided() { // In some instances, the subrequest handling may get confused and render // a page inception style. This test verifies that is not happening. $this->assertNoPattern('#.*#s', 'There was no double-page effect from a misrendered subrequest.'); - } - /** - * Checks that dynamically defined and altered routes work correctly. - * - * @see \Drupal\router_test\RouteSubscriber - */ - public function testDynamicRoutes() { // Test the altered route. $this->drupalGet('router_test/test6'); $this->assertResponse(200); $this->assertRaw('test5', 'The correct string was returned because the route was successful.'); + + // Tests that a page trying to match a path will succeed. + $this->drupalGet('router_test/test14/1'); + $this->assertResponse(200); + $this->assertText('User route "user.view" was matched.'); + + // Try to match a route for a non-existent user. + $this->drupalGet('router_test/test14/2'); + $this->assertResponse(200); + $this->assertText('Route not matched.'); } /** @@ -139,30 +145,6 @@ public function testControllerResolutionPage() { } /** - * Checks the generate method on the url generator using the front router. - */ - public function testUrlGeneratorFront() { - global $base_path; - - $this->assertEqual($this->container->get('url_generator')->generate(''), $base_path); - $this->assertEqual($this->container->get('url_generator')->generateFromPath(''), $base_path); - } - - /** - * Tests that a page trying to match a path will succeed. - */ - public function testRouterMatching() { - $this->drupalGet('router_test/test14/1'); - $this->assertResponse(200); - $this->assertText('User route "user.view" was matched.'); - - // Try to match a route for a non-existent user. - $this->drupalGet('router_test/test14/2'); - $this->assertResponse(200); - $this->assertText('Route not matched.'); - } - - /** * Tests the user account on the DIC. */ public function testUserAccount() { diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 098bca0..c97ea90 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -824,7 +824,7 @@ function system_schema() { ), ), 'indexes' => array( - 'pattern_outline' => array('pattern_outline'), + 'pattern_outline_parts' => array('pattern_outline', 'number_parts'), 'provider' => array('provider'), ), 'primary key' => array('name'),