summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlex Pott2015-07-20 08:29:09 (GMT)
committerAlex Pott2015-07-20 08:29:09 (GMT)
commitbc62529e8b98e8df48858c0e8b66d9f2c2f3d6ed (patch)
tree34c872a255e3e1e13353af213ed997b352c51828
parent7d5e65d8d4031dc74f580ec6d3a9dddff8896f90 (diff)
Revert "Issue #2228217 by pwolanin, ecrown, neclimdul, Fabianx, dawehner, klausi: Further optimize RouteProvider and add web test for large number of path parts"
This reverts commit 7d5e65d8d4031dc74f580ec6d3a9dddff8896f90.
-rw-r--r--core/lib/Drupal/Core/Routing/RouteCompiler.php4
-rw-r--r--core/lib/Drupal/Core/Routing/RouteProvider.php39
-rw-r--r--core/modules/system/src/Tests/Routing/RouteProviderTest.php41
-rw-r--r--core/modules/system/src/Tests/Routing/RouterTest.php9
-rw-r--r--core/modules/system/system.install10
5 files changed, 16 insertions, 87 deletions
diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php
index 639feff..1d223ff 100644
--- a/core/lib/Drupal/Core/Routing/RouteCompiler.php
+++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php
@@ -43,9 +43,7 @@ class RouteCompiler extends SymfonyRouteCompiler implements RouteCompilerInterfa
$stripped_path = static::getPathWithoutDefaults($route);
$fit = static::getFit($stripped_path);
$pattern_outline = static::getPatternOutline($stripped_path);
- // We count the number of parts including any optional trailing parts. This
- // allows the RouteProvider to filter candidate routes more efficiently.
- $num_parts = count(explode('/', trim($route->getPath(), '/')));
+ $num_parts = count(explode('/', trim($pattern_outline, '/')));
return new CompiledRoute(
$fit,
diff --git a/core/lib/Drupal/Core/Routing/RouteProvider.php b/core/lib/Drupal/Core/Routing/RouteProvider.php
index 20e9022..278b486 100644
--- a/core/lib/Drupal/Core/Routing/RouteProvider.php
+++ b/core/lib/Drupal/Core/Routing/RouteProvider.php
@@ -298,9 +298,11 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
* Returns a route collection of matching routes.
*/
protected function getRoutesByPath($path) {
- // Split the path up on the slashes, ignoring multiple slashes in a row
- // or leading or trailing slashes.
- $parts = preg_split('@/+@', $path, NULL, PREG_SPLIT_NO_EMPTY);
+ // 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 !== '';
+ }));
$collection = new RouteCollection();
@@ -309,37 +311,22 @@ class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProv
return $collection;
}
- // 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),
+ $routes = $this->connection->query("SELECT name, route FROM {" . $this->connection->escapeTable($this->tableName) . "} WHERE pattern_outline IN ( :patterns[] ) ORDER BY fit DESC, name ASC", array(
+ ':patterns[]' => $ancestors,
))
- ->fetchAll(\PDO::FETCH_ASSOC);
+ ->fetchAllKeyed();
- // We sort by fit and name in PHP to avoid a SQL filesort.
- usort($routes, array($this, 'routeProviderRouteCompare'));
-
- foreach ($routes as $row) {
- $collection->add($row['name'], unserialize($row['route']));
+ foreach ($routes as $name => $route) {
+ $route = unserialize($route);
+ if (preg_match($route->compile()->getRegex(), $path, $matches)) {
+ $collection->add($name, $route);
+ }
}
return $collection;
}
/**
- * Comparison function for usort on routes.
- */
- public function routeProviderRouteCompare(array $a, array $b) {
- if ($a['fit'] == $b['fit']) {
- return strcmp($a['name'], $b['name']);
- }
- // Reverse sort from highest to lowest fit. PHP should cast to int, but
- // the explicit cast makes this sort more robust against unexpected input.
- return (int) $a['fit'] < (int) $b['fit'] ? 1 : -1;
- }
-
- /**
* {@inheritdoc}
*/
public function getAllRoutes() {
diff --git a/core/modules/system/src/Tests/Routing/RouteProviderTest.php b/core/modules/system/src/Tests/Routing/RouteProviderTest.php
index 8be53ff..74cbaa3 100644
--- a/core/modules/system/src/Tests/Routing/RouteProviderTest.php
+++ b/core/modules/system/src/Tests/Routing/RouteProviderTest.php
@@ -316,46 +316,7 @@ class RouteProviderTest extends KernelTestBase {
$this->assertEqual(array('narf', 'poink'), array_keys($routes_array), 'Ensure the fitness was taken into account.');
$this->assertNotNull($routes->get('narf'), 'The first matching route was found.');
$this->assertNotNull($routes->get('poink'), 'The second matching route was found.');
- $this->assertNull($routes->get('eep'), 'Non-matching route was not found.');
- }
- catch (ResourceNotFoundException $e) {
- $this->fail('No matching route found with default argument value.');
- }
- }
-
- /**
- * Confirms that we can find multiple routes that match the request equally.
- */
- function testOutlinePathMatchDefaultsCollision3() {
- $connection = Database::getConnection();
- $provider = new RouteProvider($connection, $this->state, $this->currentPath, $this->cache, $this->pathProcessor, 'test_routes');
-
- $this->fixtures->createTables($connection);
-
- $collection = new RouteCollection();
- $collection->add('poink', new Route('/some/{value}/path'));
- // Add a second route matching the same path pattern.
- $collection->add('poink2', new Route('/some/{object}/path'));
- $collection->add('narf', new Route('/some/here/path'));
- $collection->add('eep', new Route('/something/completely/different'));
-
- $dumper = new MatcherDumper($connection, $this->state, 'test_routes');
- $dumper->addRoutes($collection);
- $dumper->dump();
-
- $path = '/some/over-there/path';
-
- $request = Request::create($path, 'GET');
-
- try {
- $routes = $provider->getRouteCollectionForRequest($request);
- $routes_array = $routes->all();
-
- $this->assertEqual(count($routes), 2, 'The correct number of routes was found.');
- $this->assertEqual(array('poink', 'poink2'), array_keys($routes_array), 'Ensure the fitness and name were taken into account in the sort.');
- $this->assertNotNull($routes->get('poink'), 'The first matching route was found.');
- $this->assertNotNull($routes->get('poink2'), 'The second matching route was found.');
- $this->assertNull($routes->get('eep'), 'Non-matching route was not found.');
+ $this->assertNull($routes->get('eep'), 'Noin-matching route was not found.');
}
catch (ResourceNotFoundException $e) {
$this->fail('No matching route found with default argument value.');
diff --git a/core/modules/system/src/Tests/Routing/RouterTest.php b/core/modules/system/src/Tests/Routing/RouterTest.php
index 16f4234..b945676 100644
--- a/core/modules/system/src/Tests/Routing/RouterTest.php
+++ b/core/modules/system/src/Tests/Routing/RouterTest.php
@@ -197,15 +197,6 @@ class RouterTest extends WebTestBase {
$this->drupalGet('router_test/test14/2');
$this->assertResponse(200);
$this->assertText('Route not matched.');
-
- // Check that very long paths don't cause an error.
- $path = 'router_test/test1';
- $suffix = '/d/r/u/p/a/l';
- for ($i = 0; $i < 10; $i++) {
- $path .= $suffix;
- $this->drupalGet($path);
- $this->assertResponse(404);
- }
}
/**
diff --git a/core/modules/system/system.install b/core/modules/system/system.install
index d2c1b40..a0849b1 100644
--- a/core/modules/system/system.install
+++ b/core/modules/system/system.install
@@ -946,7 +946,7 @@ function system_schema() {
),
),
'indexes' => array(
- 'pattern_outline_parts' => array('pattern_outline', 'number_parts'),
+ 'pattern_outline_fit' => array('pattern_outline', 'fit'),
),
'primary key' => array('name'),
);
@@ -1085,11 +1085,3 @@ function system_schema() {
return $schema;
}
-
-/**
- * Change the index on the {router} table.
- */
-function system_update_8001() {
- db_drop_index('router', 'pattern_outline_fit');
- db_add_index('router', 'pattern_outline_parts', array('pattern_outline', 'number_parts'));
-}