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 7d5e65d8d4.
8.0.x
Alex Pott
parent
7d5e65d8d4
commit
bc62529e8b
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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,36 +311,21 @@ 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}
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -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.');
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -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'));
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue