From eba77ad5c66e8e73de05b100c3fda34c4ea9fb2f Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 23 May 2012 23:07:22 -0500 Subject: [PATCH 01/72] Add a basic dumper object. This is not yet complete, but it can have routes added to it and retrieved from it. --- .../Drupal/Core/Routing/UrlMatcherDumper.php | 74 +++++++++++++ core/modules/system/tests/routing.test | 103 ++++++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 core/lib/Drupal/Core/Routing/UrlMatcherDumper.php create mode 100644 core/modules/system/tests/routing.test diff --git a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php b/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php new file mode 100644 index 00000000000..105cd2e06aa --- /dev/null +++ b/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php @@ -0,0 +1,74 @@ +connection = $connection; + } + + /** + * Adds additional routes to be dumped. + * + * @param RouteCollection $routes + */ + public function addRoutes(RouteCollection $routes) { + if (empty($this->routes)) { + $this->routes = $routes; + } + else { + $this->routes->addCollection($routes); + } + } + + /** + * Dumps a set of routes to a PHP class. + * + * Available options: + * + * * class: The class name + * * base_class: The base class name + * + * @param array $options An array of options + * + * @return string A PHP class representing the matcher class + */ + function dump(array $options = array()) { + + } + + /** + * Gets the routes to match. + * + * @return RouteCollection A RouteCollection instance + */ + function getRoutes() { + return $this->routes; + } +} + diff --git a/core/modules/system/tests/routing.test b/core/modules/system/tests/routing.test new file mode 100644 index 00000000000..8ede46e8b11 --- /dev/null +++ b/core/modules/system/tests/routing.test @@ -0,0 +1,103 @@ + 'Routing', + 'description' => 'Confirm that the matcher dumper is functioning properly.', + 'group' => 'Routng', + ); + } + + function setUp() { + parent::setUp(); + } + + /** + * Confirms that the dumper can be instantiated successfuly. + */ + function testCreate() { + $connection = Database::getConnection(); + $dumper= new UrlMatcherDumper($connection); + + $class_name = 'Drupal\Core\Routing\UrlMatcherDumper'; + $this->assertTrue($dumper instanceof $class_name, t('Dumper created successfully')); + } + + /** + * Confirms that we can add routes to the dumper. + */ + function testAddRoutes() { + $connection = Database::getConnection(); + $dumper= new UrlMatcherDumper($connection); + + $route = new Route('test'); + $collection = new RouteCollection(); + $collection->add('test_route', $route); + + $dumper->addRoutes($collection); + + $dumper_routes = $dumper->getRoutes()->all(); + $collection_routes = $collection->all(); + + foreach ($dumper_routes as $name => $route) { + $this->assertEqual($route->getPattern(), $collection_routes[$name]->getPattern(), t('Routes match')); + } + } + + /** + * Confirms that we can add routes to the dumper when it already has some. + */ + function testAddAdditionalRoutes() { + $connection = Database::getConnection(); + $dumper= new UrlMatcherDumper($connection); + + $route = new Route('test'); + $collection = new RouteCollection(); + $collection->add('test_route', $route); + $dumper->addRoutes($collection); + + $route = new Route('test2'); + $collection2 = new RouteCollection(); + $collection2->add('test_route2', $route); + $dumper->addRoutes($collection2); + + // Merge the two collections together so we can test them. + $collection->addCollection(clone $collection2); + + $dumper_routes = $dumper->getRoutes()->all(); + $collection_routes = $collection->all(); + + $success = TRUE; + foreach ($collection_routes as $name => $route) { + if (empty($dumper_routes[$name])) { + $success = FALSE; + $this->fail(t('Not all routes found in the dumper.')); + } + } + + if ($success) { + $this->pass('All routes found in the dumper.'); + } + } + + +} From 2c79c025d2d2ba659d3b0bb68ef268d4d3aa3349 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Mon, 28 May 2012 16:19:18 -0500 Subject: [PATCH 02/72] Copy in old dumping logic. Still being refactored. --- .../Drupal/Core/Routing/UrlMatcherDumper.php | 220 +++++++++++++++++- 1 file changed, 215 insertions(+), 5 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php b/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php index 105cd2e06aa..022e72b2b0e 100644 --- a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php @@ -3,17 +3,21 @@ namespace Drupal\Core\Routing; use Symfony\Component\Routing\Matcher\Dumper\MatcherDumperInterface; +use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; use Drupal\Core\Database\Connection; /** - * Description of UrlMatcherDumper - * - * @author crell + * Description of UrlMatcherDumper. */ class UrlMatcherDumper implements MatcherDumperInterface { + /** + * The maximum number of path elements for a route pattern; + */ + const MAX_PARTS = 9; + /** * The database connection to which to dump route information. * @@ -58,7 +62,34 @@ class UrlMatcherDumper implements MatcherDumperInterface { * * @return string A PHP class representing the matcher class */ - function dump(array $options = array()) { + public function dump(array $options = array()) { + $options += array( + 'route_set' => '', + ); + + $compiled = $this->compileRoutes($this->routes, $route_set); + + // Convert all of the routes into database records. + $insert = $this->connection->insert('router'); + + + + foreach ($this->routes as $name => $route) { + $insert->values($record); + } + + // Delete any old records in this route set first, then insert the new ones. + // That avoids stale data. The transaction makes it atomic to avoid + // unstable router states due to random failures. + $txn = $this->connection->startTransaction(); + + $this->connection->delete('router') + ->condition('route_set', $options['route_set']) + ->execute(); + + $insert->execute(); + + // Transaction ends here. } @@ -67,8 +98,187 @@ class UrlMatcherDumper implements MatcherDumperInterface { * * @return RouteCollection A RouteCollection instance */ - function getRoutes() { + public function getRoutes() { return $this->routes; } + + protected function compileRoutes(RouteCollection $routes, $route_set) { + + // First pass: separate callbacks from paths, making paths ready for + // matching. Calculate fitness, and fill some default values. + $menu = array(); + $masks = array(); + foreach ($routes as $name => $item) { + $path = $item->getPattern(); + $move = FALSE; + + $parts = explode('/', $path, static::MAX_PARTS); + $number_parts = count($parts); + // We store the highest index of parts here to save some work in the fit + // calculation loop. + $slashes = $number_parts - 1; + + $num_placeholders = count(array_filter($parts, function($value) { + return strpos($value, '{') !== FALSE; + })); + + $fit = $this->getFit($path); + + if ($fit) { + $move = TRUE; + } + else { + // If there is no placeholder, it fits maximally. + $fit = (1 << $number_parts) - 1; + } + + $masks[$fit] = 1; + $item += array( + 'title' => '', + 'weight' => 0, + 'type' => MENU_NORMAL_ITEM, + 'module' => '', + '_number_parts' => $number_parts, + '_parts' => $parts, + '_fit' => $fit, + ); + + if ($move) { + $new_path = implode('/', $item['_parts']); + $menu[$new_path] = $item; + $sort[$new_path] = $number_parts; + } + else { + $menu[$path] = $item; + $sort[$path] = $number_parts; + } + } + + // Sort the route list. + array_multisort($sort, SORT_NUMERIC, $menu); + // Apply inheritance rules. + foreach ($menu as $path => $v) { + $item = &$menu[$path]; + + for ($i = $item['_number_parts'] - 1; $i; $i--) { + $parent_path = implode('/', array_slice($item['_parts'], 0, $i)); + if (isset($menu[$parent_path])) { + + $parent = &$menu[$parent_path]; + + // If an access callback is not found for a default local task we use + // the callback from the parent, since we expect them to be identical. + // In all other cases, the access parameters must be specified. + if (($item['type'] == MENU_DEFAULT_LOCAL_TASK) && !isset($item['access callback']) && isset($parent['access callback'])) { + $item['access callback'] = $parent['access callback']; + if (!isset($item['access arguments']) && isset($parent['access arguments'])) { + $item['access arguments'] = $parent['access arguments']; + } + } + + // Same for theme callbacks. + if (!isset($item['theme callback']) && isset($parent['theme callback'])) { + $item['theme callback'] = $parent['theme callback']; + if (!isset($item['theme arguments']) && isset($parent['theme arguments'])) { + $item['theme arguments'] = $parent['theme arguments']; + } + } + } + } + if (!isset($item['access callback']) && isset($item['access arguments'])) { + // Default callback. + $item['access callback'] = 'user_access'; + } + if (!isset($item['access callback']) || empty($item['page callback'])) { + $item['access callback'] = 0; + } + if (is_bool($item['access callback'])) { + $item['access callback'] = intval($item['access callback']); + } + + $item += array( + 'access arguments' => array(), + 'access callback' => '', + 'page arguments' => array(), + 'page callback' => '', + 'delivery callback' => '', + 'title arguments' => array(), + 'title callback' => 't', + 'theme arguments' => array(), + 'theme callback' => '', + 'description' => '', + 'position' => '', + 'context' => 0, + 'tab_parent' => '', + 'tab_root' => $path, + 'path' => $path, + 'file' => '', + 'file path' => '', + 'include file' => '', + ); + + // Calculate out the file to be included for each callback, if any. + if ($item['file']) { + $file_path = $item['file path'] ? $item['file path'] : drupal_get_path('module', $item['module']); + $item['include file'] = $file_path . '/' . $item['file']; + } + } + + // Sort the masks so they are in order of descending fit. + $masks = array_keys($masks); + rsort($masks); + + return array($menu, $masks); + + + // The old menu_router record structure, copied here for easy referencing. + array( + 'path' => $item['path'], + 'load_functions' => $item['load_functions'], + 'to_arg_functions' => $item['to_arg_functions'], + 'access_callback' => $item['access callback'], + 'access_arguments' => serialize($item['access arguments']), + 'page_callback' => $item['page callback'], + 'page_arguments' => serialize($item['page arguments']), + 'delivery_callback' => $item['delivery callback'], + 'fit' => $item['_fit'], + 'number_parts' => $item['_number_parts'], + 'context' => $item['context'], + 'tab_parent' => $item['tab_parent'], + 'tab_root' => $item['tab_root'], + 'title' => $item['title'], + 'title_callback' => $item['title callback'], + 'title_arguments' => ($item['title arguments'] ? serialize($item['title arguments']) : ''), + 'theme_callback' => $item['theme callback'], + 'theme_arguments' => serialize($item['theme arguments']), + 'type' => $item['type'], + 'description' => $item['description'], + 'position' => $item['position'], + 'weight' => $item['weight'], + 'include_file' => $item['include file'], + ); + } + + /** + * Determines the fitness of the provided path. + * + * @param string $path + * The path whose fitness we want. + * + * @return int + * The fitness of the path, as an integer. + */ + public function getFit($path) { + $fit = 0; + + $parts = explode('/', $path, static::MAX_PARTS); + foreach ($parts as $k => $part) { + if (strpos($part, '{') === FALSE) { + $fit |= 1 << ($slashes - $k); + } + } + + return $fit; + } } From a524a35d1d0d3522a0a4bbb1dc81b4a787081a73 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Mon, 28 May 2012 21:12:03 -0500 Subject: [PATCH 03/72] Add basic route compilation mechanism. --- .../lib/Drupal/Core/Routing/CompiledRoute.php | 45 ++++++++ .../lib/Drupal/Core/Routing/RouteCompiler.php | 64 +++++++++++ core/modules/system/tests/routing.test | 104 ++++++++++++++++++ 3 files changed, 213 insertions(+) create mode 100644 core/lib/Drupal/Core/Routing/CompiledRoute.php create mode 100644 core/lib/Drupal/Core/Routing/RouteCompiler.php diff --git a/core/lib/Drupal/Core/Routing/CompiledRoute.php b/core/lib/Drupal/Core/Routing/CompiledRoute.php new file mode 100644 index 00000000000..52292b31123 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/CompiledRoute.php @@ -0,0 +1,45 @@ +fit = $fit; + } + + public function getFit() { + return $this->fit; + } + +} + diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php new file mode 100644 index 00000000000..99dd915dc9a --- /dev/null +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -0,0 +1,64 @@ +getFit($route->getPattern()); + + return new CompiledRoute($route, $fit); + + } + + + /** + * Determines the fitness of the provided path. + * + * @param string $path + * The path whose fitness we want. + * + * @return int + * The fitness of the path, as an integer. + */ + public function getFit($path) { + + $parts = explode('/', trim($path, '/'), static::MAX_PARTS); + $number_parts = count($parts); + // We store the highest index of parts here to save some work in the fit + // calculation loop. + $slashes = $number_parts - 1; + + $fit = 0; + foreach ($parts as $k => $part) { + if (strpos($part, '{') === FALSE) { + $fit |= 1 << ($slashes - $k); + } + } + + return $fit; + } +} + diff --git a/core/modules/system/tests/routing.test b/core/modules/system/tests/routing.test index 8ede46e8b11..28eff1ea564 100644 --- a/core/modules/system/tests/routing.test +++ b/core/modules/system/tests/routing.test @@ -99,5 +99,109 @@ class UrlMatcherDumperTestCase extends WebTestBase { } } + /** + * Confirms that we can add routes to the dumper when it already has some. + */ + protected function prepareTables() { + $connection = Database::getConnection(); + + $tables['test_routes'] = array( + 'description' => 'Maps paths to various callbacks (access, page and title)', + 'fields' => array( + 'name' => array( + 'description' => 'Primary Key: Machine name of this route', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'pattern' => array( + 'description' => 'The path pattern for this URI', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'pattern_outline' => array( + 'description' => 'The pattern', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'access_callback' => array( + 'description' => 'The callback which determines the access to this router path. Defaults to user_access.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'access_arguments' => array( + 'description' => 'A serialized array of arguments for the access callback.', + 'type' => 'blob', + 'not null' => FALSE, + ), + 'fit' => array( + 'description' => 'A numeric representation of how specific the path is.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + ), + 'number_parts' => array( + 'description' => 'Number of parts in this router path.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + 'size' => 'small', + ), + 'route' => array( + 'description' => 'A serialized Route object', + 'type' => 'text', + ), + ), + 'indexes' => array( + 'fit' => array('fit'), + 'pattern_outline' => array('pattern_outline'), + ), + 'primary key' => array('name'), + ); + + + $schema = $connection->schema(); + $schema->dropTable('test_routes'); + $schema->createTable('test_routes', $tables['test_routes']); + } + +} + +/** + * Basic tests for the Route. + * + * Note: This should be a UnitTestBase, but those are broken right now in + * Drupal HEAD. Convert it to a UnitTest when that gets fixed. + */ +class RouteTestCase extends WebTestBase { + public static function getInfo() { + return array( + 'name' => 'Routes', + 'description' => 'Confirm that route object is functioning properly.', + 'group' => 'Routng', + ); + } + + function setUp() { + parent::setUp(); + } + + public function testCompilation() { + + $route = new Route('test/{something}/more'); + $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); + $compiled = $route->compile(); + + $this->assertEqual($route, $compiled->getRoute(), t('Compiled route has the correct route object.')); + $this->assertEqual($compiled->getFit(), 5 /* That's 101 binary*/, t('The fit was correct.')); + + } } From 2ed208b1e08785fa529a27ae95356ae4ee410cc9 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Mon, 28 May 2012 22:06:11 -0500 Subject: [PATCH 04/72] Add pattern outline generation to the route compilation. --- .../lib/Drupal/Core/Routing/CompiledRoute.php | 19 +++++++++++++++---- .../lib/Drupal/Core/Routing/RouteCompiler.php | 19 ++++++++++++++++++- core/modules/system/tests/routing.test | 4 ++-- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/CompiledRoute.php b/core/lib/Drupal/Core/Routing/CompiledRoute.php index 52292b31123..e2e595ebb42 100644 --- a/core/lib/Drupal/Core/Routing/CompiledRoute.php +++ b/core/lib/Drupal/Core/Routing/CompiledRoute.php @@ -17,6 +17,13 @@ class CompiledRoute extends SymfonyCompiledRoute { */ protected $fit; + /** + * The pattern outline of this route. + * + * @var string + */ + protected $patternOutline; + /** * Constructor. * @@ -24,22 +31,26 @@ class CompiledRoute extends SymfonyCompiledRoute { * A original Route instance. * @param int $fit * The fitness of the route. - * @param string $regex The regular expression to use to match this route - * @param array $tokens An array of tokens to use to generate URL for this route - * @param array $variables An array of variables + * @param string $fit + * The pattern outline for this route. */ - public function __construct(Route $route, $fit) { + public function __construct(Route $route, $fit, $pattern_outline) { // We're ignoring the rest of this stuff; really this should be just using // an interface, but the Symfony component doesn't have one. This is a bug // in Symfony. parent::__construct($route, '', '', array(), array()); $this->fit = $fit; + $this->patternOutline = $pattern_outline; } public function getFit() { return $this->fit; } + public function getPatternOutline() { + return $this->patternOutline; + } + } diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index 99dd915dc9a..8c3b7b9ef93 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -29,10 +29,27 @@ class RouteCompiler implements RouteCompilerInterface { $fit = $this->getFit($route->getPattern()); - return new CompiledRoute($route, $fit); + $pattern_outline = $this->getPatternOutline($route->getPattern()); + + return new CompiledRoute($route, $fit, $pattern_outline); } + /** + * Returns the pattern outline. + * + * The pattern outline is the path pattern but normalized so that all + * placeholders are equal strings. + * + * @param string $path + * The path pattern to normalize to an outline. + * + * @return string + * The path pattern outline. + */ + public function getPatternOutline($path) { + return preg_replace('#\{\w+\}#', '%', $path); + } /** * Determines the fitness of the provided path. diff --git a/core/modules/system/tests/routing.test b/core/modules/system/tests/routing.test index 28eff1ea564..63cb65ec4a3 100644 --- a/core/modules/system/tests/routing.test +++ b/core/modules/system/tests/routing.test @@ -194,13 +194,13 @@ class RouteTestCase extends WebTestBase { } public function testCompilation() { - - $route = new Route('test/{something}/more'); + $route = new Route('/test/{something}/more'); $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); $compiled = $route->compile(); $this->assertEqual($route, $compiled->getRoute(), t('Compiled route has the correct route object.')); $this->assertEqual($compiled->getFit(), 5 /* That's 101 binary*/, t('The fit was correct.')); + $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', t('The pattern outline was correct.')); } From f14521489acd66d06e284f38f39c53cef315fd60 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Mon, 28 May 2012 22:27:57 -0500 Subject: [PATCH 05/72] Add ablity to dump a route collection to the database. --- .../Drupal/Core/Routing/UrlMatcherDumper.php | 35 +++++++++++++--- core/modules/system/tests/routing.test | 41 ++++++++++++++++++- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php b/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php index 022e72b2b0e..9a0bf0626cf 100644 --- a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php @@ -32,8 +32,17 @@ class UrlMatcherDumper implements MatcherDumperInterface { */ protected $routes; - public function __construct(Connection $connection) { + /** + * The name of the SQL table to which to dump the routes. + * + * @var string + */ + protected $tableName; + + public function __construct(Connection $connection, $table = 'router') { $this->connection = $connection; + + $this->tableName = $table; } /** @@ -67,15 +76,31 @@ class UrlMatcherDumper implements MatcherDumperInterface { 'route_set' => '', ); - $compiled = $this->compileRoutes($this->routes, $route_set); + //$compiled = $this->compileRoutes($this->routes, $route_set); // Convert all of the routes into database records. - $insert = $this->connection->insert('router'); + $insert = $this->connection->insert($this->tableName)->fields(array( + 'name', + 'route_set', + 'fit', + 'pattern', + 'pattern_outline', + 'route', + )); foreach ($this->routes as $name => $route) { - $insert->values($record); + $compiled = $route->compile(); + $values = array( + 'name' => $name, + 'route_set' => $options['route_set'], + 'fit' => $compiled->getFit(), + 'pattern' => $compiled->getPattern(), + 'pattern_outline' => $compiled->getPatternOutline(), + 'route' => serialize($route), + ); + $insert->values($values); } // Delete any old records in this route set first, then insert the new ones. @@ -83,7 +108,7 @@ class UrlMatcherDumper implements MatcherDumperInterface { // unstable router states due to random failures. $txn = $this->connection->startTransaction(); - $this->connection->delete('router') + $this->connection->delete($this->tableName) ->condition('route_set', $options['route_set']) ->execute(); diff --git a/core/modules/system/tests/routing.test b/core/modules/system/tests/routing.test index 63cb65ec4a3..c0a9bcbcbfd 100644 --- a/core/modules/system/tests/routing.test +++ b/core/modules/system/tests/routing.test @@ -100,10 +100,39 @@ class UrlMatcherDumperTestCase extends WebTestBase { } /** - * Confirms that we can add routes to the dumper when it already has some. + * Confirm that we can dump a route collection to the database. */ - protected function prepareTables() { + public function testDump() { $connection = Database::getConnection(); + $dumper= new UrlMatcherDumper($connection, 'test_routes'); + + $route = new Route('/test/{my}/path'); + $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); + $collection = new RouteCollection(); + $collection->add('test_route', $route); + + $dumper->addRoutes($collection); + + $this->prepareTables($connection); + + $dumper->dump(array('route_set' => 'test')); + + $record = $connection->query("SELECT * FROM {test_routes} WHERE name= :name", array(':name' => 'test_route'))->fetchObject(); + + $loaded_route = unserialize($record->route); + + $this->assertEqual($record->name, 'test_route', t('Dumped route has correct name.')); + $this->assertEqual($record->pattern, '/test/{my}/path', t('Dumped route has correct pattern.')); + $this->assertEqual($record->pattern_outline, '/test/%/path', t('Dumped route has correct pattern outline.')); + $this->assertEqual($record->fit, 5 /* 101 in binary */, t('Dumped route has correct fit.')); + $this->assertTrue($loaded_route instanceof Route, t('Route object retrieved successfully.')); + + } + + /** + * Creates a test database table just for unit testing purposes. + */ + protected function prepareTables($connection) { $tables['test_routes'] = array( 'description' => 'Maps paths to various callbacks (access, page and title)', @@ -129,6 +158,13 @@ class UrlMatcherDumperTestCase extends WebTestBase { 'not null' => TRUE, 'default' => '', ), + 'route_set' => array( + 'description' => 'The route set grouping to which a route belongs.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), 'access_callback' => array( 'description' => 'The callback which determines the access to this router path. Defaults to user_access.', 'type' => 'varchar', @@ -162,6 +198,7 @@ class UrlMatcherDumperTestCase extends WebTestBase { 'indexes' => array( 'fit' => array('fit'), 'pattern_outline' => array('pattern_outline'), + 'route_set' => array('route_set'), ), 'primary key' => array('name'), ); From 81c3bf6dff61c93ed275d18cd17df6079cd387a1 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 10 Jun 2012 17:10:13 -0500 Subject: [PATCH 06/72] Make CompiledRoute its own class rather than extending Symfony's CompiledRoute. --- .../lib/Drupal/Core/Routing/CompiledRoute.php | 87 +++++++++++++++++-- 1 file changed, 79 insertions(+), 8 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/CompiledRoute.php b/core/lib/Drupal/Core/Routing/CompiledRoute.php index e2e595ebb42..bd58c8a134a 100644 --- a/core/lib/Drupal/Core/Routing/CompiledRoute.php +++ b/core/lib/Drupal/Core/Routing/CompiledRoute.php @@ -2,13 +2,12 @@ namespace Drupal\Core\Routing; -use Symfony\Component\Routing\CompiledRoute as SymfonyCompiledRoute; use Symfony\Component\Routing\Route; /** * Description of CompiledRoute */ -class CompiledRoute extends SymfonyCompiledRoute { +class CompiledRoute { /** * The fitness of this route. @@ -25,7 +24,20 @@ class CompiledRoute extends SymfonyCompiledRoute { protected $patternOutline; /** - * Constructor. + * The Route object of which this object is the compiled version. + * + * @var Symfony\Component\Routing\Route + */ + protected $route; + + protected $variables; + protected $tokens; + protected $staticPrefix; + protected $regex; + + + /** + * Constructs a new CompiledRoute object. * * @param Route $route * A original Route instance. @@ -35,22 +47,81 @@ class CompiledRoute extends SymfonyCompiledRoute { * The pattern outline for this route. */ public function __construct(Route $route, $fit, $pattern_outline) { - // We're ignoring the rest of this stuff; really this should be just using - // an interface, but the Symfony component doesn't have one. This is a bug - // in Symfony. - parent::__construct($route, '', '', array(), array()); - + $this->route = $route; $this->fit = $fit; $this->patternOutline = $pattern_outline; } + /** + * Returns the fit of this route + * + * See RouteCompiler for a definition of how the fit is calculated. + * + * @return int + * The fit of the route. + */ public function getFit() { return $this->fit; } + /** + * Returns the pattern outline of this route. + * + * The pattern outline of a route is the path pattern of the route, but + * normalized such that all placeholders are replaced with %. + * + * @return string + * The normalized path pattern. + */ public function getPatternOutline() { return $this->patternOutline; } + /** + * Returns the Route instance. + * + * @return Route + * A Route instance + */ + public function getRoute() { + return $this->route; + } + + /** + * Returns the pattern. + * + * @return string The pattern + */ + public function getPattern() { + return $this->route->getPattern(); + } + + /** + * Returns the options. + * + * @return array The options + */ + public function getOptions() { + return $this->route->getOptions(); + } + + /** + * Returns the defaults. + * + * @return array The defaults + */ + public function getDefaults() { + return $this->route->getDefaults(); + } + + /** + * Returns the requirements. + * + * @return array The requirements + */ + public function getRequirements() { + return $this->route->getRequirements(); + } + } From 3019c6f74d8b8127e2f1034f5bb8ddeca8d49ace Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 10 Jun 2012 17:21:24 -0500 Subject: [PATCH 07/72] Convert WebTests to UnitTests. --- core/modules/system/tests/routing.test | 12 +++--------- core/vendor/Routing | 1 + 2 files changed, 4 insertions(+), 9 deletions(-) create mode 160000 core/vendor/Routing diff --git a/core/modules/system/tests/routing.test b/core/modules/system/tests/routing.test index c0a9bcbcbfd..5bf499570af 100644 --- a/core/modules/system/tests/routing.test +++ b/core/modules/system/tests/routing.test @@ -8,17 +8,14 @@ use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; -use Drupal\simpletest\WebTestBase; +use Drupal\simpletest\UnitTestBase; use Drupal\Core\Database\Database; use Drupal\Core\Routing\UrlMatcherDumper; /** * Basic tests for the UrlMatcherDumper. - * - * Note: This should be a UnitTestBase, but those are broken right now in - * Drupal HEAD. Convert it to a UnitTest when that gets fixed. */ -class UrlMatcherDumperTestCase extends WebTestBase { +class UrlMatcherDumperTestCase extends UnitTestBase { public static function getInfo() { return array( 'name' => 'Routing', @@ -213,11 +210,8 @@ class UrlMatcherDumperTestCase extends WebTestBase { /** * Basic tests for the Route. - * - * Note: This should be a UnitTestBase, but those are broken right now in - * Drupal HEAD. Convert it to a UnitTest when that gets fixed. */ -class RouteTestCase extends WebTestBase { +class RouteTestCase extends UnitTestBase { public static function getInfo() { return array( 'name' => 'Routes', diff --git a/core/vendor/Routing b/core/vendor/Routing new file mode 160000 index 00000000000..a05bcaaaa43 --- /dev/null +++ b/core/vendor/Routing @@ -0,0 +1 @@ +Subproject commit a05bcaaaa43025037a0667e158aed9b65a147e80 From 52fd27522b026ea2ea80f5a84bb039bc0f4eb4b1 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 10 Jun 2012 17:32:07 -0500 Subject: [PATCH 08/72] Convert tests to namespaced format. --- .../Drupal/system/Tests/Routing/RouteTest.php | 41 +++++++++++++++++++ .../Tests/Routing/UrlMatcherDumperTest.php} | 39 +++--------------- 2 files changed, 47 insertions(+), 33 deletions(-) create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php rename core/modules/system/{tests/routing.test => lib/Drupal/system/Tests/Routing/UrlMatcherDumperTest.php} (86%) diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php new file mode 100644 index 00000000000..af840b2e7c4 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php @@ -0,0 +1,41 @@ + 'Routes', + 'description' => 'Confirm that route object is functioning properly.', + 'group' => 'Routing', + ); + } + + function setUp() { + parent::setUp(); + } + + public function testCompilation() { + $route = new Route('/test/{something}/more'); + $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); + $compiled = $route->compile(); + + $this->assertEqual($route, $compiled->getRoute(), t('Compiled route has the correct route object.')); + $this->assertEqual($compiled->getFit(), 5 /* That's 101 binary*/, t('The fit was correct.')); + $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', t('The pattern outline was correct.')); + } + +} diff --git a/core/modules/system/tests/routing.test b/core/modules/system/lib/Drupal/system/Tests/Routing/UrlMatcherDumperTest.php similarity index 86% rename from core/modules/system/tests/routing.test rename to core/modules/system/lib/Drupal/system/Tests/Routing/UrlMatcherDumperTest.php index 5bf499570af..be1164ca7ef 100644 --- a/core/modules/system/tests/routing.test +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/UrlMatcherDumperTest.php @@ -2,9 +2,11 @@ /** * @file - * Tests for Symfony2-related functionality. + * Definition of Drupal\system\Tests\Routing\UrlMatcherDumperTest. */ +namespace Drupal\system\Tests\Routing; + use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -15,12 +17,12 @@ use Drupal\Core\Routing\UrlMatcherDumper; /** * Basic tests for the UrlMatcherDumper. */ -class UrlMatcherDumperTestCase extends UnitTestBase { +class UrlMatcherDumperTest extends UnitTestBase { public static function getInfo() { return array( - 'name' => 'Routing', + 'name' => 'Dumper tests', 'description' => 'Confirm that the matcher dumper is functioning properly.', - 'group' => 'Routng', + 'group' => 'Routing', ); } @@ -207,32 +209,3 @@ class UrlMatcherDumperTestCase extends UnitTestBase { } } - -/** - * Basic tests for the Route. - */ -class RouteTestCase extends UnitTestBase { - public static function getInfo() { - return array( - 'name' => 'Routes', - 'description' => 'Confirm that route object is functioning properly.', - 'group' => 'Routng', - ); - } - - function setUp() { - parent::setUp(); - } - - public function testCompilation() { - $route = new Route('/test/{something}/more'); - $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); - $compiled = $route->compile(); - - $this->assertEqual($route, $compiled->getRoute(), t('Compiled route has the correct route object.')); - $this->assertEqual($compiled->getFit(), 5 /* That's 101 binary*/, t('The fit was correct.')); - $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', t('The pattern outline was correct.')); - - } - -} From b0f90a1046be2a35cea2e2edd699caadee6d09d9 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 10 Jun 2012 19:30:58 -0500 Subject: [PATCH 09/72] Add a basic framework for stackable partial matching. --- .../Drupal/Core/Routing/HttpMethodMatcher.php | 44 ++++++++ .../Core/Routing/NestedMatcherInterface.php | 36 +++++++ .../Core/Routing/PartialMatcherInterface.php | 22 ++++ core/lib/Drupal/Core/Routing/UrlMatcher.php | 100 ++++++++++++++++++ .../Drupal/Core/Routing/UrlMatcherDumper.php | 15 ++- .../Tests/Routing/PartialMatcherTest.php | 69 ++++++++++++ 6 files changed, 278 insertions(+), 8 deletions(-) create mode 100644 core/lib/Drupal/Core/Routing/HttpMethodMatcher.php create mode 100644 core/lib/Drupal/Core/Routing/NestedMatcherInterface.php create mode 100644 core/lib/Drupal/Core/Routing/PartialMatcherInterface.php create mode 100644 core/lib/Drupal/Core/Routing/UrlMatcher.php create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php diff --git a/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php b/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php new file mode 100644 index 00000000000..54ce82eaf89 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php @@ -0,0 +1,44 @@ +routes = $routes; + } + + /** + * Matches a request against multiple routes. + * + * @param Request $request + * A Request object against which to match. + * + * @return RouteCollection + * A RouteCollection of matched routes. + */ + public function matchByRequest(Request $request) { + + $method = $request->getMethod(); + + $collection = new RouteCollection(); + + foreach ($this->routes->all() as $name => $route) { + $allowed_methods = $route->getRequirement('_method'); + if ($allowed_methods === NULL || in_array($method, explode('|', strtoupper($allowed_methods)))) { + $collection->add($name, $route); + } + } + return $collection; + } + +} + diff --git a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php new file mode 100644 index 00000000000..ec9664ec556 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php @@ -0,0 +1,36 @@ +context = new RequestContext(); + } + + /** + * Sets the request context. + * + * This method is just to satisfy the interface, and is largely vestigial. + * The request context object does not contain the information we need, so + * we will use the original request object. + * + * @param Symfony\Component\Routing\RequestContext $context + * The context. + * + * @api + */ + public function setContext(RequestContext $context) { + $this->context = $context; + } + + /** + * Gets the request context. + * + * This method is just to satisfy the interface, and is largely vestigial. + * The request context object does not contain the information we need, so + * we will use the original request object. + * + * @return Symfony\Component\Routing\RequestContext + * The context. + */ + public function getContext() { + return $this->context; + } + + /** + * Sets the request object to use. + * + * This is used by the RouterListener to make additional request attributes + * available. + * + * @param Symfony\Component\HttpFoundation\Request $request + * The request object. + */ + public function setRequest(Request $request) { + $this->request = $request; + } + + /** + * Gets the request object. + * + * @return Symfony\Component\HttpFoundation\Request $request + * The request object. + */ + public function getRequest() { + return $this->request; + } + + public function match($pathinfo) { + + } + +} \ No newline at end of file diff --git a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php b/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php index 9a0bf0626cf..f7969aa8bf4 100644 --- a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php @@ -64,12 +64,12 @@ class UrlMatcherDumper implements MatcherDumperInterface { * * Available options: * - * * class: The class name + * * route_set: The route grouping that is being dumped. All existing + * routes with this route set will be deleted on dump. * * base_class: The base class name * - * @param array $options An array of options - * - * @return string A PHP class representing the matcher class + * @param $options array + * $options An array of options */ public function dump(array $options = array()) { $options += array( @@ -88,8 +88,6 @@ class UrlMatcherDumper implements MatcherDumperInterface { 'route', )); - - foreach ($this->routes as $name => $route) { $compiled = $route->compile(); $values = array( @@ -115,13 +113,14 @@ class UrlMatcherDumper implements MatcherDumperInterface { $insert->execute(); // Transaction ends here. - } /** * Gets the routes to match. * - * @return RouteCollection A RouteCollection instance + * @return RouteCollection + * A RouteCollection instance representing all routes currently in the + * dumper. */ public function getRoutes() { return $this->routes; diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php new file mode 100644 index 00000000000..9e73678efa9 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php @@ -0,0 +1,69 @@ + 'Partial matcher HTTP Method tests', + 'description' => 'Confirm that the Http Method partial matcher is functioning properly.', + 'group' => 'Routing', + ); + } + + function setUp() { + parent::setUp(); + } + + /** + * Confirms that the HttpMethod matcher matches properly. + */ + function testFilterRoutes() { + $collection = new RouteCollection(); + + $route = new Route('path/one'); + $route->setRequirement('_method', 'GET'); + $collection->add('route_a', $route); + + $route = new Route('path/one'); + $route->setRequirement('_method', 'PUT'); + $collection->add('route_b', $route); + + $route = new Route('path/two'); + $route->setRequirement('_method', 'GET'); + $collection->add('route_c', $route); + + $route = new Route('path/three'); + $collection->add('route_d', $route); + + $route = new Route('path/two'); + $route->setRequirement('_method', 'GET|HEAD'); + $collection->add('route_e', $route); + + $matcher = new HttpMethodMatcher($collection, 'GET'); + + $routes = $matcher->matchByRequest(Request::create('path/one', 'GET')); + + $this->assertEqual(count($routes->all()), 4, t('The correct number of routes was found.')); + $this->assertNotNull($routes->get('route_a'), t('The first matching route was found.')); + $this->assertNull($routes->get('route_b'), t('The non-matching route was not found.')); + $this->assertNotNull($routes->get('route_c'), t('The second matching route was found.')); + $this->assertNotNull($routes->get('route_d'), t('The all-matching route was found.')); + $this->assertNotNull($routes->get('route_e'), t('The multi-matching route was found.')); + } +} + From 806ff4acc8d5f18956322569254e136f4208e4ec Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 23 Jun 2012 16:19:54 -0500 Subject: [PATCH 10/72] Add a mechanism for a NestedRouter. A Nested router is a series of partial routers, each of which whittle down a RouteCollection until it is left with a single matching route. That single route is the final route that matches the request. --- .../Core/Routing/FinalMatcherInterface.php | 25 +++ .../Drupal/Core/Routing/HttpMethodMatcher.php | 4 +- .../Core/Routing/InitialMatcherInterface.php | 22 +++ .../lib/Drupal/Core/Routing/NestedMatcher.php | 149 ++++++++++++++++++ .../Core/Routing/NestedMatcherInterface.php | 22 ++- .../Core/Routing/PartialMatcherInterface.php | 5 +- .../system/Tests/Routing/MockFinalMatcher.php | 57 +++++++ .../system/Tests/Routing/MockPathMatcher.php | 50 ++++++ .../Tests/Routing/PartialMatcherTest.php | 55 +++++-- 9 files changed, 370 insertions(+), 19 deletions(-) create mode 100644 core/lib/Drupal/Core/Routing/FinalMatcherInterface.php create mode 100644 core/lib/Drupal/Core/Routing/InitialMatcherInterface.php create mode 100644 core/lib/Drupal/Core/Routing/NestedMatcher.php create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/MockFinalMatcher.php create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/MockPathMatcher.php diff --git a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php new file mode 100644 index 00000000000..6d43b19831d --- /dev/null +++ b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php @@ -0,0 +1,25 @@ +routes = $routes; } @@ -25,7 +25,7 @@ class HttpMethodMatcher implements PartialMatcherInterface { * @return RouteCollection * A RouteCollection of matched routes. */ - public function matchByRequest(Request $request) { + public function matchRequestPartial(Request $request) { $method = $request->getMethod(); diff --git a/core/lib/Drupal/Core/Routing/InitialMatcherInterface.php b/core/lib/Drupal/Core/Routing/InitialMatcherInterface.php new file mode 100644 index 00000000000..a08cb12b586 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/InitialMatcherInterface.php @@ -0,0 +1,22 @@ +partialMatchers[] = $matcher; + + return $this; + } + + /** + * Sets the final matcher for the matching plan. + * + * @param UrlMatcherInterface $final + * The matcher that will be called last to ensure only a single route is + * found. + * + * @return NestedMatcherInterface + * The current matcher. + */ + public function setFinalMatcher(FinalMatcherInterface $final) { + $this->finalMatcher = $final; + + return $this; + } + + /** + * Sets the first matcher for the matching plan. + * + * Partial matchers will be run in the order in which they are added. + * + * @param InitialMatcherInterface $matcher + * An initial matcher. It is responsible for its own configuration and + * initial route collection + * + * @return NestedMatcherInterface + * The current matcher. + */ + public function setInitialMatcher(InitialMatcherInterface $initial) { + $this->initialMatcher = $initial; + + return $this; + } + + /** + * Tries to match a request with a set of routes. + * + * If the matcher can not find information, it must throw one of the exceptions documented + * below. + * + * @param Request $request The request to match + * + * @return array An array of parameters + * + * @throws ResourceNotFoundException If no matching resource could be found + * @throws MethodNotAllowedException If a matching resource was found but the request method is not allowed + */ + public function matchRequest(Request $request) { + $collection = $this->initialMatcher->matchRequestPartial($request); + + foreach ($this->partialMatchers as $matcher) { + if ($collection) { + $matcher->setCollection($collection); + } + $collection = $matcher->matchRequestPartial($request); + } + + $route = $this->finalMatcher->setCollection($collection)->matchRequest($request); + + return $route; + } + + /** + * Sets the request context. + * + * This method is unused. It is here only to satisfy the interface. + * + * @param RequestContext $context The context + */ + public function setContext(RequestContext $context) { + $this->context = $context; + } + + /** + * Gets the request context. + * + * This method is unused. It is here only to satisfy the interface. + * + * @return RequestContext The context + */ + public function getContext() { + return $this->context; + } + +} + diff --git a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php index ec9664ec556..6ae7428fd6e 100644 --- a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php @@ -2,12 +2,26 @@ namespace Drupal\Core\Routing; -use Symfony\Component\Routing\Matcher\UrlMatcherInterface; +use Symfony\Component\Routing\Matcher\RequestMatcherInterface; /** * A NestedMatcher allows for multiple-stage resolution of a route. */ -interface NestedMatcherInterface extends UrlMatcherInterface { +interface NestedMatcherInterface extends RequestMatcherInterface { + + /** + * Sets the first matcher for the matching plan. + * + * Partial matchers will be run in the order in which they are added. + * + * @param InitialMatcherInterface $matcher + * An initial matcher. It is responsible for its own configuration and + * initial route collection + * + * @return NestedMatcherInterface + * The current matcher. + */ + public function setInitialMatcher(InitialMatcherInterface $initial); /** * Adds a partial matcher to the matching plan. @@ -15,7 +29,7 @@ interface NestedMatcherInterface extends UrlMatcherInterface { * Partial matchers will be run in the order in which they are added. * * @param PartialMatcherInterface $matcher - * A partial + * A partial matcher. * * @return NestedMatcherInterface * The current matcher. @@ -32,5 +46,5 @@ interface NestedMatcherInterface extends UrlMatcherInterface { * @return NestedMatcherInterface * The current matcher. */ - public function setFinalMatcher(UrlMatcherInterface $final); + public function setFinalMatcher(FinalMatcherInterface $final); } diff --git a/core/lib/Drupal/Core/Routing/PartialMatcherInterface.php b/core/lib/Drupal/Core/Routing/PartialMatcherInterface.php index aff44b7dfc0..03ffb5f688a 100644 --- a/core/lib/Drupal/Core/Routing/PartialMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/PartialMatcherInterface.php @@ -3,12 +3,15 @@ namespace Drupal\Core\Routing; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\RouteCollection; /** * A PartialMatcher works like a UrlMatcher, but will return multiple candidate routes. */ interface PartialMatcherInterface { + public function setCollection(RouteCollection $collection); + /** * Matches a request against multiple routes. * @@ -18,5 +21,5 @@ interface PartialMatcherInterface { * @return RouteCollection * A RouteCollection of matched routes. */ - public function matchByRequest(Request $request); + public function matchRequestPartial(Request $request); } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MockFinalMatcher.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MockFinalMatcher.php new file mode 100644 index 00000000000..ca5d68be8bb --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MockFinalMatcher.php @@ -0,0 +1,57 @@ +routes = $collection; + + return $this; + } + + + public function matchRequest(Request $request) { + // For testing purposes, just return whatever the first route is. + foreach ($this->routes as $name => $route) { + return array_merge($this->mergeDefaults(array(), $route->getDefaults()), array('_route' => $name)); + } + } + + /** + * Get merged default parameters. + * + * @param array $params + * The parameters + * @param array $defaults + * The defaults + * + * @return array + * Merged default parameters + */ + protected function mergeDefaults($params, $defaults) { + $parameters = $defaults; + foreach ($params as $key => $value) { + if (!is_int($key)) { + $parameters[$key] = $value; + } + } + + return $parameters; + } + +} + diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MockPathMatcher.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MockPathMatcher.php new file mode 100644 index 00000000000..b545ebe766a --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MockPathMatcher.php @@ -0,0 +1,50 @@ +routes = $routes; + } + + /** + * Matches a request against multiple routes. + * + * @param Request $request + * A Request object against which to match. + * + * @return RouteCollection + * A RouteCollection of matched routes. + */ + public function matchRequestPartial(Request $request) { + // For now for testing we'll just do a straight string match. + + $path = $request->getPathInfo(); + + $return = new RouteCollection(); + + foreach ($this->routes as $name => $route) { + if ($route->getPattern() == $path) { + $return->add($name, $route); + } + } + + return $return; + } + + +} diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php index 9e73678efa9..557dd55d766 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php @@ -1,4 +1,5 @@ setCollection($this->sampleRouteCollection()); + + $routes = $matcher->matchRequestPartial(Request::create('path/one', 'GET')); + + $this->assertEqual(count($routes->all()), 4, t('The correct number of routes was found.')); + $this->assertNotNull($routes->get('route_a'), t('The first matching route was found.')); + $this->assertNull($routes->get('route_b'), t('The non-matching route was not found.')); + $this->assertNotNull($routes->get('route_c'), t('The second matching route was found.')); + $this->assertNotNull($routes->get('route_d'), t('The all-matching route was found.')); + $this->assertNotNull($routes->get('route_e'), t('The multi-matching route was found.')); + } + + /** + * Confirms we can nest multiple partial matchers. + */ + public function testNestedMatcher() { + + $matcher = new NestedMatcher(); + + $matcher->setInitialMatcher(new MockPathMatcher($this->sampleRouteCollection())); + $matcher->addPartialMatcher(new HttpMethodMatcher()); + $matcher->setFinalMatcher(new MockFinalMatcher()); + + $request = Request::create('/path/one', 'GET'); + + $attributes = $matcher->matchRequest($request); + + $this->assertEqual($attributes['_route'], 'route_a', t('The correct matching route was found.')); + } + + /** + * Returns a standard set of routes for testing. + * + * @return \Symfony\Component\Routing\RouteCollection + */ + protected function sampleRouteCollection() { $collection = new RouteCollection(); $route = new Route('path/one'); @@ -54,16 +94,7 @@ class PartialMatcherTest extends UnitTestBase { $route->setRequirement('_method', 'GET|HEAD'); $collection->add('route_e', $route); - $matcher = new HttpMethodMatcher($collection, 'GET'); - - $routes = $matcher->matchByRequest(Request::create('path/one', 'GET')); - - $this->assertEqual(count($routes->all()), 4, t('The correct number of routes was found.')); - $this->assertNotNull($routes->get('route_a'), t('The first matching route was found.')); - $this->assertNull($routes->get('route_b'), t('The non-matching route was not found.')); - $this->assertNotNull($routes->get('route_c'), t('The second matching route was found.')); - $this->assertNotNull($routes->get('route_d'), t('The all-matching route was found.')); - $this->assertNotNull($routes->get('route_e'), t('The multi-matching route was found.')); + return $collection; } } From db11de09c822985da32f5b905038eaca52233303 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 23 Jun 2012 17:49:39 -0500 Subject: [PATCH 11/72] Rename UrlMatcherDumper to MatcherDumper, since we use more than just the Url for matching. --- ...UrlMatcherDumper.php => MatcherDumper.php} | 4 +- core/lib/Drupal/Core/Routing/UrlMatcher.php | 100 ------------------ ...erDumperTest.php => MatcherDumperTest.php} | 14 +-- 3 files changed, 9 insertions(+), 109 deletions(-) rename core/lib/Drupal/Core/Routing/{UrlMatcherDumper.php => MatcherDumper.php} (98%) delete mode 100644 core/lib/Drupal/Core/Routing/UrlMatcher.php rename core/modules/system/lib/Drupal/system/Tests/Routing/{UrlMatcherDumperTest.php => MatcherDumperTest.php} (94%) diff --git a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php similarity index 98% rename from core/lib/Drupal/Core/Routing/UrlMatcherDumper.php rename to core/lib/Drupal/Core/Routing/MatcherDumper.php index f7969aa8bf4..6c425af2800 100644 --- a/core/lib/Drupal/Core/Routing/UrlMatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -9,9 +9,9 @@ use Symfony\Component\Routing\RouteCollection; use Drupal\Core\Database\Connection; /** - * Description of UrlMatcherDumper. + * Dumps Route information to a database table. */ -class UrlMatcherDumper implements MatcherDumperInterface { +class MatcherDumper implements MatcherDumperInterface { /** * The maximum number of path elements for a route pattern; diff --git a/core/lib/Drupal/Core/Routing/UrlMatcher.php b/core/lib/Drupal/Core/Routing/UrlMatcher.php deleted file mode 100644 index d84a45bfc93..00000000000 --- a/core/lib/Drupal/Core/Routing/UrlMatcher.php +++ /dev/null @@ -1,100 +0,0 @@ -context = new RequestContext(); - } - - /** - * Sets the request context. - * - * This method is just to satisfy the interface, and is largely vestigial. - * The request context object does not contain the information we need, so - * we will use the original request object. - * - * @param Symfony\Component\Routing\RequestContext $context - * The context. - * - * @api - */ - public function setContext(RequestContext $context) { - $this->context = $context; - } - - /** - * Gets the request context. - * - * This method is just to satisfy the interface, and is largely vestigial. - * The request context object does not contain the information we need, so - * we will use the original request object. - * - * @return Symfony\Component\Routing\RequestContext - * The context. - */ - public function getContext() { - return $this->context; - } - - /** - * Sets the request object to use. - * - * This is used by the RouterListener to make additional request attributes - * available. - * - * @param Symfony\Component\HttpFoundation\Request $request - * The request object. - */ - public function setRequest(Request $request) { - $this->request = $request; - } - - /** - * Gets the request object. - * - * @return Symfony\Component\HttpFoundation\Request $request - * The request object. - */ - public function getRequest() { - return $this->request; - } - - public function match($pathinfo) { - - } - -} \ No newline at end of file diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/UrlMatcherDumperTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php similarity index 94% rename from core/modules/system/lib/Drupal/system/Tests/Routing/UrlMatcherDumperTest.php rename to core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php index be1164ca7ef..cb99cf0efc5 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/UrlMatcherDumperTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php @@ -12,12 +12,12 @@ use Symfony\Component\Routing\RouteCollection; use Drupal\simpletest\UnitTestBase; use Drupal\Core\Database\Database; -use Drupal\Core\Routing\UrlMatcherDumper; +use Drupal\Core\Routing\MatcherDumper; /** * Basic tests for the UrlMatcherDumper. */ -class UrlMatcherDumperTest extends UnitTestBase { +class MatcherDumperTest extends UnitTestBase { public static function getInfo() { return array( 'name' => 'Dumper tests', @@ -35,9 +35,9 @@ class UrlMatcherDumperTest extends UnitTestBase { */ function testCreate() { $connection = Database::getConnection(); - $dumper= new UrlMatcherDumper($connection); + $dumper= new MatcherDumper($connection); - $class_name = 'Drupal\Core\Routing\UrlMatcherDumper'; + $class_name = 'Drupal\Core\Routing\MatcherDumper'; $this->assertTrue($dumper instanceof $class_name, t('Dumper created successfully')); } @@ -46,7 +46,7 @@ class UrlMatcherDumperTest extends UnitTestBase { */ function testAddRoutes() { $connection = Database::getConnection(); - $dumper= new UrlMatcherDumper($connection); + $dumper= new MatcherDumper($connection); $route = new Route('test'); $collection = new RouteCollection(); @@ -67,7 +67,7 @@ class UrlMatcherDumperTest extends UnitTestBase { */ function testAddAdditionalRoutes() { $connection = Database::getConnection(); - $dumper= new UrlMatcherDumper($connection); + $dumper= new MatcherDumper($connection); $route = new Route('test'); $collection = new RouteCollection(); @@ -103,7 +103,7 @@ class UrlMatcherDumperTest extends UnitTestBase { */ public function testDump() { $connection = Database::getConnection(); - $dumper= new UrlMatcherDumper($connection, 'test_routes'); + $dumper= new MatcherDumper($connection, 'test_routes'); $route = new Route('/test/{my}/path'); $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); From e9a95aa1fb79988d237c32dfcff5cb7b190cac9c Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 23 Jun 2012 19:59:46 -0500 Subject: [PATCH 12/72] Add a skeleton for a Path matcher. The PathMatcher matches against the database table structure generated by the MatcherDumper. As of this commit the lookup is not yet implemented. It's still in testing. --- .../lib/Drupal/Core/Routing/CompiledRoute.php | 25 +++- .../lib/Drupal/Core/Routing/MatcherDumper.php | 2 + core/lib/Drupal/Core/Routing/PathMatcher.php | 118 ++++++++++++++++++ .../lib/Drupal/Core/Routing/RouteCompiler.php | 4 +- .../Tests/Routing/MatcherDumperTest.php | 93 +++----------- .../system/Tests/Routing/PathMatcherTest.php | 69 ++++++++++ .../system/Tests/Routing/RoutingFixtures.php | 116 +++++++++++++++++ 7 files changed, 352 insertions(+), 75 deletions(-) create mode 100644 core/lib/Drupal/Core/Routing/PathMatcher.php create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php diff --git a/core/lib/Drupal/Core/Routing/CompiledRoute.php b/core/lib/Drupal/Core/Routing/CompiledRoute.php index bd58c8a134a..d8e93f89c71 100644 --- a/core/lib/Drupal/Core/Routing/CompiledRoute.php +++ b/core/lib/Drupal/Core/Routing/CompiledRoute.php @@ -23,6 +23,13 @@ class CompiledRoute { */ protected $patternOutline; + /** + * The number of parts in the path of this route. + * + * @var int + */ + protected $numParts; + /** * The Route object of which this object is the compiled version. * @@ -45,11 +52,14 @@ class CompiledRoute { * The fitness of the route. * @param string $fit * The pattern outline for this route. + * @param int $num_parts + * The number of parts in the path. */ - public function __construct(Route $route, $fit, $pattern_outline) { + public function __construct(Route $route, $fit, $pattern_outline, $num_parts) { $this->route = $route; $this->fit = $fit; $this->patternOutline = $pattern_outline; + $this->numParts = $num_parts; } /** @@ -64,6 +74,19 @@ class CompiledRoute { return $this->fit; } + /** + * Returns the number of parts in this route's path. + * + * The string "foo/bar/baz" has 3 parts, regardless of how many of them are + * placeholders. + * + * @return int + * The number of parts in the path. + */ + public function getNumParts() { + return $this->numParts; + } + /** * Returns the pattern outline of this route. * diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index 6c425af2800..1ca60e4df24 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -85,6 +85,7 @@ class MatcherDumper implements MatcherDumperInterface { 'fit', 'pattern', 'pattern_outline', + 'number_parts', 'route', )); @@ -96,6 +97,7 @@ class MatcherDumper implements MatcherDumperInterface { 'fit' => $compiled->getFit(), 'pattern' => $compiled->getPattern(), 'pattern_outline' => $compiled->getPatternOutline(), + 'number_parts' => $compiled->getNumParts(), 'route' => serialize($route), ); $insert->values($values); diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php new file mode 100644 index 00000000000..7f39dcf7390 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -0,0 +1,118 @@ +connection = $connection; + + $this->tableName = $table; + } + + /** + * Matches a request against multiple routes. + * + * @param Request $request + * A Request object against which to match. + * + * @return RouteCollection + * A RouteCollection of matched routes. + */ + public function matchRequestPartial(Request $request) { + + $path = $request->getPathInfo(); + + $parts = array_slide(explode('/', $path), 0, MatcherDumper::MAX_PARTS); + + $number_parts = count($parts); + + $ancestors = $this->getCandidateOutlines($parts); + + // @todo We want to allow matching more than one result because there could + // be more than one result with the same path. But how do we do that and + // limit by fit? + $routes = $this->connection + ->select($this->tableName, 'r') + ->fields('r', array('name', 'route')) + ->condition('pattern_outline', $ancestors, 'IN') + ->condition('number_parts', $number_parts) + ->execute() + ->fetchAllKeyed(); + + $collection = new RouteCollection(); + foreach ($routes as $name => $route) { + $collection->add($name, $route); + } + + return $collection; + } + + /** + * Returns an array of path pattern outlines that could match the path parts. + * + * @param array $parts + * The parts of the path for which we want candidates. + * @return array + * An array of outlines that could match the specified path parts. + */ + public function getCandidateOutlines(array $parts) { + + $number_parts = count($parts); + $length = $number_parts - 1; + $end = (1 << $number_parts) - 1; + $candidates = array(); + + $start = pow($number_parts-1, 2); + + // The highest possible mask is a 1 bit for every part of the path. We will + // check every value down from there to generate a possible outline. + $masks = range($end, $start); + + foreach ($masks as $i) { + $current = ''; + for ($j = $length; $j >= 0; $j--) { + // Check the bit on the $j offset. + if ($i & (1 << $j)) { + // Bit one means the original value. + $current .= $parts[$length - $j]; + } + else { + // Bit zero means means wildcard. + $current .= '%'; + } + // Unless we are at offset 0, add a slash. + if ($j) { + $current .= '/'; + } + } + $candidates[] = $current; + } + + return $candidates; + } +} + diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index 8c3b7b9ef93..c05e72590ec 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -31,7 +31,9 @@ class RouteCompiler implements RouteCompilerInterface { $pattern_outline = $this->getPatternOutline($route->getPattern()); - return new CompiledRoute($route, $fit, $pattern_outline); + $num_parts = count(explode('/', $pattern_outline)); + + return new CompiledRoute($route, $fit, $pattern_outline, $num_parts); } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php index cb99cf0efc5..5da3befaa04 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php @@ -18,6 +18,14 @@ use Drupal\Core\Routing\MatcherDumper; * Basic tests for the UrlMatcherDumper. */ class MatcherDumperTest extends UnitTestBase { + + /** + * A collection of shared fixture data for tests. + * + * @var RoutingFixtures + */ + protected $fixtures; + public static function getInfo() { return array( 'name' => 'Dumper tests', @@ -26,6 +34,12 @@ class MatcherDumperTest extends UnitTestBase { ); } + function __construct($test_id = NULL) { + parent::__construct($test_id); + + $this->fixtures = new RoutingFixtures(); + } + function setUp() { parent::setUp(); } @@ -132,80 +146,13 @@ class MatcherDumperTest extends UnitTestBase { * Creates a test database table just for unit testing purposes. */ protected function prepareTables($connection) { - - $tables['test_routes'] = array( - 'description' => 'Maps paths to various callbacks (access, page and title)', - 'fields' => array( - 'name' => array( - 'description' => 'Primary Key: Machine name of this route', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - 'default' => '', - ), - 'pattern' => array( - 'description' => 'The path pattern for this URI', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - 'default' => '', - ), - 'pattern_outline' => array( - 'description' => 'The pattern', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - 'default' => '', - ), - 'route_set' => array( - 'description' => 'The route set grouping to which a route belongs.', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - 'default' => '', - ), - 'access_callback' => array( - 'description' => 'The callback which determines the access to this router path. Defaults to user_access.', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - 'default' => '', - ), - 'access_arguments' => array( - 'description' => 'A serialized array of arguments for the access callback.', - 'type' => 'blob', - 'not null' => FALSE, - ), - 'fit' => array( - 'description' => 'A numeric representation of how specific the path is.', - 'type' => 'int', - 'not null' => TRUE, - 'default' => 0, - ), - 'number_parts' => array( - 'description' => 'Number of parts in this router path.', - 'type' => 'int', - 'not null' => TRUE, - 'default' => 0, - 'size' => 'small', - ), - 'route' => array( - 'description' => 'A serialized Route object', - 'type' => 'text', - ), - ), - 'indexes' => array( - 'fit' => array('fit'), - 'pattern_outline' => array('pattern_outline'), - 'route_set' => array('route_set'), - ), - 'primary key' => array('name'), - ); - - + $tables = $this->fixtures->routingTableDefinition(); $schema = $connection->schema(); - $schema->dropTable('test_routes'); - $schema->createTable('test_routes', $tables['test_routes']); + + foreach ($tables as $name => $table) { + $schema->dropTable($name); + $schema->createTable($name, $table); + } } } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php new file mode 100644 index 00000000000..2931f62a20a --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -0,0 +1,69 @@ + 'Path matcher tests', + 'description' => 'Confirm that the path matching library is working correctly.', + 'group' => 'Routing', + ); + } + + function __construct($test_id = NULL) { + parent::__construct($test_id); + + $this->fixtures = new RoutingFixtures(); + } + + /** + * Confirms that the correct candidate outlines are generated. + */ + public function testCandidateOutlines() { + + $connection = Database::getConnection(); + $matcher = new PathMatcher($connection); + + $parts = array('node', '5', 'edit'); + + $candidates = $matcher->getCandidateOutlines($parts); + + //debug($candidates); + + $candidates = array_flip($candidates); + + $this->assertTrue(count($candidates) == 4, t('Correct number of candidates found')); + $this->assertTrue(array_key_exists('node/5/edit', $candidates), t('First candidate found.')); + $this->assertTrue(array_key_exists('node/5/%', $candidates), t('Second candidate found.')); + $this->assertTrue(array_key_exists('node/%/edit', $candidates), t('Third candidate found.')); + $this->assertTrue(array_key_exists('node/%/%', $candidates), t('Fourth candidate found.')); + } + + +} + diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php new file mode 100644 index 00000000000..f7889afa424 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php @@ -0,0 +1,116 @@ +setRequirement('_method', 'GET'); + $collection->add('route_a', $route); + + $route = new Route('path/one'); + $route->setRequirement('_method', 'PUT'); + $collection->add('route_b', $route); + + $route = new Route('path/two'); + $route->setRequirement('_method', 'GET'); + $collection->add('route_c', $route); + + $route = new Route('path/three'); + $collection->add('route_d', $route); + + $route = new Route('path/two'); + $route->setRequirement('_method', 'GET|HEAD'); + $collection->add('route_e', $route); + + return $collection; + } + + public function routingTableDefinition() { + + $tables['test_routes'] = array( + 'description' => 'Maps paths to various callbacks (access, page and title)', + 'fields' => array( + 'name' => array( + 'description' => 'Primary Key: Machine name of this route', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'pattern' => array( + 'description' => 'The path pattern for this URI', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'pattern_outline' => array( + 'description' => 'The pattern', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'route_set' => array( + 'description' => 'The route set grouping to which a route belongs.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'access_callback' => array( + 'description' => 'The callback which determines the access to this router path. Defaults to user_access.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'access_arguments' => array( + 'description' => 'A serialized array of arguments for the access callback.', + 'type' => 'blob', + 'not null' => FALSE, + ), + 'fit' => array( + 'description' => 'A numeric representation of how specific the path is.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + ), + 'number_parts' => array( + 'description' => 'Number of parts in this router path.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + 'size' => 'small', + ), + 'route' => array( + 'description' => 'A serialized Route object', + 'type' => 'text', + ), + ), + 'indexes' => array( + 'fit' => array('fit'), + 'pattern_outline' => array('pattern_outline'), + 'route_set' => array('route_set'), + ), + 'primary key' => array('name'), + ); + + return $tables; + } +} From 40e5531952af47731d6dd0cea6ad5bfb5a703ca9 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 30 Jun 2012 15:40:25 -0500 Subject: [PATCH 13/72] Enforce the Drupal RouteCompiler for all routes dumped with our dumper. --- core/lib/Drupal/Core/Routing/MatcherDumper.php | 1 + 1 file changed, 1 insertion(+) diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index 1ca60e4df24..d3e292b5f03 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -90,6 +90,7 @@ class MatcherDumper implements MatcherDumperInterface { )); foreach ($this->routes as $name => $route) { + $route->setOption('compiler_class', '\Drupal\Core\Routing\RouteCompiler'); $compiled = $route->compile(); $values = array( 'name' => $name, From f55ab1aff639fd3e4928a09459068415a6319562 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 30 Jun 2012 15:43:44 -0500 Subject: [PATCH 14/72] Move more fixture logic to the fixture class. --- .../Tests/Routing/MatcherDumperTest.php | 16 +------------- .../system/Tests/Routing/PathMatcherTest.php | 6 +++++ .../system/Tests/Routing/RoutingFixtures.php | 22 +++++++++++++++++++ 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php index 5da3befaa04..d7c9a01f20d 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php @@ -126,7 +126,7 @@ class MatcherDumperTest extends UnitTestBase { $dumper->addRoutes($collection); - $this->prepareTables($connection); + $this->fixtures->createTables($connection); $dumper->dump(array('route_set' => 'test')); @@ -141,18 +141,4 @@ class MatcherDumperTest extends UnitTestBase { $this->assertTrue($loaded_route instanceof Route, t('Route object retrieved successfully.')); } - - /** - * Creates a test database table just for unit testing purposes. - */ - protected function prepareTables($connection) { - $tables = $this->fixtures->routingTableDefinition(); - $schema = $connection->schema(); - - foreach ($tables as $name => $table) { - $schema->dropTable($name); - $schema->createTable($name, $table); - } - } - } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php index 2931f62a20a..9d29bff844c 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -41,6 +41,12 @@ class PathMatcherTest extends UnitTestBase { $this->fixtures = new RoutingFixtures(); } + public function tearDown() { + $this->fixtures->dropTables(Database::getConnection()); + + parent::tearDown(); + } + /** * Confirms that the correct candidate outlines are generated. */ diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php index f7889afa424..cdcb26572f5 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php @@ -5,11 +5,33 @@ namespace Drupal\system\Tests\Routing; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; +use Drupal\Core\Database\Connection; + /** * Utility methods to generate sample data, database configuration, etc. */ class RoutingFixtures { + public function createTables(Connection $connection) { + $tables = $this->routingTableDefinition(); + $schema = $connection->schema(); + + foreach ($tables as $name => $table) { + $schema->dropTable($name); + $schema->createTable($name, $table); + } + } + + public function dropTables(Connection $connection) { + $tables = $this->routingTableDefinition(); + $schema = $connection->schema(); + + foreach ($tables as $name => $table) { + $schema->dropTable($name); + } + } + + /** * Returns a standard set of routes for testing. * From d372d6de9961ad52dc93e682d99608941106c5e8 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 30 Jun 2012 15:44:30 -0500 Subject: [PATCH 15/72] Complete exact-path matching. --- core/lib/Drupal/Core/Routing/PathMatcher.php | 23 ++++------- .../system/Tests/Routing/PathMatcherTest.php | 40 +++++++++++++++++-- 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index 7f39dcf7390..73f1cb8419c 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -29,7 +29,6 @@ class PathMatcher implements InitialMatcherInterface { public function __construct(Connection $connection, $table = 'router') { $this->connection = $connection; - $this->tableName = $table; } @@ -46,26 +45,18 @@ class PathMatcher implements InitialMatcherInterface { $path = $request->getPathInfo(); - $parts = array_slide(explode('/', $path), 0, MatcherDumper::MAX_PARTS); - - $number_parts = count($parts); + $parts = array_slice(array_filter(explode('/', $path)), 0, MatcherDumper::MAX_PARTS); $ancestors = $this->getCandidateOutlines($parts); - // @todo We want to allow matching more than one result because there could - // be more than one result with the same path. But how do we do that and - // limit by fit? - $routes = $this->connection - ->select($this->tableName, 'r') - ->fields('r', array('name', 'route')) - ->condition('pattern_outline', $ancestors, 'IN') - ->condition('number_parts', $number_parts) - ->execute() - ->fetchAllKeyed(); + $routes = $this->connection->query("SELECT name, route FROM {{$this->tableName}} WHERE pattern_outline IN (:patterns) ORDER BY fit", array( + ':patterns' => $ancestors, + )) + ->fetchAllKeyed(); $collection = new RouteCollection(); foreach ($routes as $name => $route) { - $collection->add($name, $route); + $collection->add($name, unserialize($route)); } return $collection; @@ -93,7 +84,7 @@ class PathMatcher implements InitialMatcherInterface { $masks = range($end, $start); foreach ($masks as $i) { - $current = ''; + $current = '/'; for ($j = $length; $j >= 0; $j--) { // Check the bit on the $j offset. if ($i & (1 << $j)) { diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php index 9d29bff844c..9e17cfe693c 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -14,6 +14,7 @@ use Symfony\Component\Routing\RouteCollection; use Drupal\simpletest\UnitTestBase; use Drupal\Core\Routing\PathMatcher; use Drupal\Core\Database\Database; +use Drupal\Core\Routing\MatcherDumper; /** * Basic tests for the UrlMatcherDumper. @@ -64,10 +65,41 @@ class PathMatcherTest extends UnitTestBase { $candidates = array_flip($candidates); $this->assertTrue(count($candidates) == 4, t('Correct number of candidates found')); - $this->assertTrue(array_key_exists('node/5/edit', $candidates), t('First candidate found.')); - $this->assertTrue(array_key_exists('node/5/%', $candidates), t('Second candidate found.')); - $this->assertTrue(array_key_exists('node/%/edit', $candidates), t('Third candidate found.')); - $this->assertTrue(array_key_exists('node/%/%', $candidates), t('Fourth candidate found.')); + $this->assertTrue(array_key_exists('/node/5/edit', $candidates), t('First candidate found.')); + $this->assertTrue(array_key_exists('/node/5/%', $candidates), t('Second candidate found.')); + $this->assertTrue(array_key_exists('/node/%/edit', $candidates), t('Third candidate found.')); + $this->assertTrue(array_key_exists('/node/%/%', $candidates), t('Fourth candidate found.')); + } + + /** + * Confirms that we can find routes with the exact incoming path. + */ + function testExactPathMatch() { + $connection = Database::getConnection(); + $matcher = new PathMatcher($connection, 'test_routes'); + + $this->fixtures->createTables($connection); + + $dumper = new MatcherDumper($connection, 'test_routes'); + $dumper->addRoutes($this->fixtures->sampleRouteCollection()); + $dumper->dump(); + + $path = '/path/one'; + + $request = Request::create($path, 'GET'); + + $routes = $matcher->matchRequestPartial($request); + + foreach ($routes as $route) { + $this->assertEqual($route->getPattern(), $path, t('Found path has correct pattern')); + } + } + + /** + * Confirms that we can find routes whose pattern would match the request. + */ + function testOutlinePathMatch() { + } From a3e002dbe4995c666b9deee7d939791b838a465a Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Tue, 3 Jul 2012 20:20:34 -0500 Subject: [PATCH 16/72] Fix documentation to clarify that a FinalMatcher returns attributes, not a Route or RouteCollection. --- core/lib/Drupal/Core/Routing/FinalMatcherInterface.php | 4 ++-- core/lib/Drupal/Core/Routing/NestedMatcher.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php index 6d43b19831d..56804465eb3 100644 --- a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php @@ -18,8 +18,8 @@ interface FinalMatcherInterface { * @param Request $request * A Request object against which to match. * - * @return RouteCollection - * A RouteCollection of matched routes. + * @return array + * An array of parameters */ public function matchRequest(Request $request); } diff --git a/core/lib/Drupal/Core/Routing/NestedMatcher.php b/core/lib/Drupal/Core/Routing/NestedMatcher.php index ccfc5ecfee0..db8a429af4f 100644 --- a/core/lib/Drupal/Core/Routing/NestedMatcher.php +++ b/core/lib/Drupal/Core/Routing/NestedMatcher.php @@ -118,9 +118,9 @@ class NestedMatcher implements NestedMatcherInterface { $collection = $matcher->matchRequestPartial($request); } - $route = $this->finalMatcher->setCollection($collection)->matchRequest($request); + $attributes = $this->finalMatcher->setCollection($collection)->matchRequest($request); - return $route; + return $attributes; } /** From 329fde3f413d2ea0829645e06f403c93477dd8c6 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Tue, 3 Jul 2012 20:34:20 -0500 Subject: [PATCH 17/72] Document that PartialMatcherInterface::setCollection() is chainable. --- core/lib/Drupal/Core/Routing/HttpMethodMatcher.php | 2 ++ core/lib/Drupal/Core/Routing/PartialMatcherInterface.php | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php b/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php index 8b9e57e7419..458b70c83d7 100644 --- a/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php +++ b/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php @@ -14,6 +14,8 @@ class HttpMethodMatcher implements PartialMatcherInterface { public function setCollection(RouteCollection $routes) { $this->routes = $routes; + + return $this; } /** diff --git a/core/lib/Drupal/Core/Routing/PartialMatcherInterface.php b/core/lib/Drupal/Core/Routing/PartialMatcherInterface.php index 03ffb5f688a..1b234e860c7 100644 --- a/core/lib/Drupal/Core/Routing/PartialMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/PartialMatcherInterface.php @@ -10,6 +10,15 @@ use Symfony\Component\Routing\RouteCollection; */ interface PartialMatcherInterface { + /** + * Sets the route collection this matcher should use. + * + * @param RouteCollection $collection + * The collection against which to match. + * + * @return PartialMatcherInterface + * The current matcher. + */ public function setCollection(RouteCollection $collection); /** From 0e4b90e09bf4727cda870eef01c488be25348c95 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Tue, 3 Jul 2012 20:42:09 -0500 Subject: [PATCH 18/72] Add a base class for partial matchers. --- .../Drupal/Core/Routing/HttpMethodMatcher.php | 11 ++---- .../Drupal/Core/Routing/PartialMatcher.php | 35 +++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 core/lib/Drupal/Core/Routing/PartialMatcher.php diff --git a/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php b/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php index 458b70c83d7..60ab36922eb 100644 --- a/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php +++ b/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php @@ -4,19 +4,12 @@ namespace Drupal\Core\Routing; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\RouteCollection; +use Symfony\Component\Routing\Exception\MethodNotAllowedException; /** * This class filters routes based on their HTTP Method. */ -class HttpMethodMatcher implements PartialMatcherInterface { - - protected $routes; - - public function setCollection(RouteCollection $routes) { - $this->routes = $routes; - - return $this; - } +class HttpMethodMatcher extends PartialMatcher { /** * Matches a request against multiple routes. diff --git a/core/lib/Drupal/Core/Routing/PartialMatcher.php b/core/lib/Drupal/Core/Routing/PartialMatcher.php new file mode 100644 index 00000000000..775bb3a89fb --- /dev/null +++ b/core/lib/Drupal/Core/Routing/PartialMatcher.php @@ -0,0 +1,35 @@ +routes = $collection; + + return $this; + } + +} From 7a8d3df9a60968ececa656244a28bf20642760ad Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Tue, 3 Jul 2012 21:10:20 -0500 Subject: [PATCH 19/72] Make the HttpMatcher throw a MethodNotAllowedException if it filters out all possible routes. --- .../Drupal/Core/Routing/HttpMethodMatcher.php | 18 +++++- ...cherTest.php => HttpMethodMatcherTest.php} | 58 +++++++++++-------- 2 files changed, 49 insertions(+), 27 deletions(-) rename core/modules/system/lib/Drupal/system/Tests/Routing/{PartialMatcherTest.php => HttpMethodMatcherTest.php} (63%) diff --git a/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php b/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php index 60ab36922eb..b0a18789bbd 100644 --- a/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php +++ b/core/lib/Drupal/Core/Routing/HttpMethodMatcher.php @@ -21,17 +21,31 @@ class HttpMethodMatcher extends PartialMatcher { * A RouteCollection of matched routes. */ public function matchRequestPartial(Request $request) { + $possible_methods = array(); $method = $request->getMethod(); $collection = new RouteCollection(); foreach ($this->routes->all() as $name => $route) { - $allowed_methods = $route->getRequirement('_method'); - if ($allowed_methods === NULL || in_array($method, explode('|', strtoupper($allowed_methods)))) { + // _method could be a |-delimited list of allowed methods, or null. If + // null, we accept any method. + $allowed_methods = array_filter(explode('|', strtoupper($route->getRequirement('_method')))); + if (empty($allowed_methods) || in_array($method, $allowed_methods)) { $collection->add($name, $route); } + else { + // Build a list of methods that would have matched. Note that we only + // need to do this if a route doesn't match, because if even one route + // passes then we'll never throw the exception that needs this array. + $possible_methods += $allowed_methods; + } } + + if (!count($collection->all())) { + throw new MethodNotAllowedException(array_unique($possible_methods)); + } + return $collection; } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php similarity index 63% rename from core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php rename to core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php index 557dd55d766..48210d3cd61 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PartialMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php @@ -10,15 +10,26 @@ namespace Drupal\system\Tests\Routing; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; +use Symfony\Component\Routing\Exception\MethodNotAllowedException; use Drupal\simpletest\UnitTestBase; use Drupal\Core\Routing\HttpMethodMatcher; use Drupal\Core\Routing\NestedMatcher; +use Exception; + /** * Basic tests for the UrlMatcherDumper. */ -class PartialMatcherTest extends UnitTestBase { +class HttpMethodMatcherTest extends UnitTestBase { + + /** + * A collection of shared fixture data for tests. + * + * @var RoutingFixtures + */ + protected $fixtures; + public static function getInfo() { return array( 'name' => 'Partial matcher HTTP Method tests', @@ -27,6 +38,11 @@ class PartialMatcherTest extends UnitTestBase { ); } + function __construct($test_id = NULL) { + parent::__construct($test_id); + + $this->fixtures = new RoutingFixtures(); + } public function setUp() { parent::setUp(); } @@ -37,7 +53,7 @@ class PartialMatcherTest extends UnitTestBase { public function testFilterRoutes() { $matcher = new HttpMethodMatcher(); - $matcher->setCollection($this->sampleRouteCollection()); + $matcher->setCollection($this->fixtures->sampleRouteCollection()); $routes = $matcher->matchRequestPartial(Request::create('path/one', 'GET')); @@ -56,7 +72,7 @@ class PartialMatcherTest extends UnitTestBase { $matcher = new NestedMatcher(); - $matcher->setInitialMatcher(new MockPathMatcher($this->sampleRouteCollection())); + $matcher->setInitialMatcher(new MockPathMatcher($this->fixtures->sampleRouteCollection())); $matcher->addPartialMatcher(new HttpMethodMatcher()); $matcher->setFinalMatcher(new MockFinalMatcher()); @@ -68,33 +84,25 @@ class PartialMatcherTest extends UnitTestBase { } /** - * Returns a standard set of routes for testing. - * - * @return \Symfony\Component\Routing\RouteCollection + * Confirms that the HttpMethod matcher throws an exception for no-route. */ - protected function sampleRouteCollection() { - $collection = new RouteCollection(); + public function testNoRouteFound() { + $matcher = new HttpMethodMatcher(); - $route = new Route('path/one'); - $route->setRequirement('_method', 'GET'); - $collection->add('route_a', $route); + // Remove the sample route that would match any method. + $routes = $this->fixtures->sampleRouteCollection(); + $routes->remove('route_d'); - $route = new Route('path/one'); - $route->setRequirement('_method', 'PUT'); - $collection->add('route_b', $route); + $matcher->setCollection($routes); - $route = new Route('path/two'); - $route->setRequirement('_method', 'GET'); - $collection->add('route_c', $route); + try { + $routes = $matcher->matchRequestPartial(Request::create('path/one', 'DELETE')); + $this->fail(t('No exception was thrown.')); + } + catch (Exception $e) { + $this->assertTrue($e instanceof MethodNotAllowedException, t('The correct exception was thrown.')); + } - $route = new Route('path/three'); - $collection->add('route_d', $route); - - $route = new Route('path/two'); - $route->setRequirement('_method', 'GET|HEAD'); - $collection->add('route_e', $route); - - return $collection; } } From f0c3b571e723b34cb44e4d50ab874ac9ae31825f Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 4 Jul 2012 14:52:54 -0500 Subject: [PATCH 20/72] Make the mock FinalMatcher a real matcher, since it's useful on its own. --- .../Core/Routing/FinalMatcherInterface.php | 9 +++++++ .../Core/Routing/FirstEntryFinalMatcher.php} | 26 ++++++++++++++----- .../Tests/Routing/HttpMethodMatcherTest.php | 3 ++- 3 files changed, 30 insertions(+), 8 deletions(-) rename core/{modules/system/lib/Drupal/system/Tests/Routing/MockFinalMatcher.php => lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php} (64%) diff --git a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php index 56804465eb3..2c8c80082c5 100644 --- a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php @@ -10,6 +10,15 @@ use Symfony\Component\Routing\RouteCollection; */ interface FinalMatcherInterface { + /** + * Sets the route collection this matcher should use. + * + * @param RouteCollection $collection + * The collection against which to match. + * + * @return FinalMatcherInterface + * The current matcher. + */ public function setCollection(RouteCollection $collection); /** diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MockFinalMatcher.php b/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php similarity index 64% rename from core/modules/system/lib/Drupal/system/Tests/Routing/MockFinalMatcher.php rename to core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php index ca5d68be8bb..81880cfaf3f 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/MockFinalMatcher.php +++ b/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php @@ -1,22 +1,34 @@ routes = $collection; @@ -25,7 +37,7 @@ class MockFinalMatcher implements FinalMatcherInterface { public function matchRequest(Request $request) { - // For testing purposes, just return whatever the first route is. + // Return whatever the first route in the collection is. foreach ($this->routes as $name => $route) { return array_merge($this->mergeDefaults(array(), $route->getDefaults()), array('_route' => $name)); } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php index 48210d3cd61..4b5a90cfe44 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php @@ -15,6 +15,7 @@ use Symfony\Component\Routing\Exception\MethodNotAllowedException; use Drupal\simpletest\UnitTestBase; use Drupal\Core\Routing\HttpMethodMatcher; use Drupal\Core\Routing\NestedMatcher; +use Drupal\Core\Routing\FirstEntryFinalMatcher; use Exception; @@ -74,7 +75,7 @@ class HttpMethodMatcherTest extends UnitTestBase { $matcher->setInitialMatcher(new MockPathMatcher($this->fixtures->sampleRouteCollection())); $matcher->addPartialMatcher(new HttpMethodMatcher()); - $matcher->setFinalMatcher(new MockFinalMatcher()); + $matcher->setFinalMatcher(new FirstEntryFinalMatcher()); $request = Request::create('/path/one', 'GET'); From 5701e622f6602c77f34b0fb9ec604e7ef86b28b3 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 4 Jul 2012 15:08:58 -0500 Subject: [PATCH 21/72] Split PartialMatcher tests out into their own test class. --- .../Tests/Routing/HttpMethodMatcherTest.php | 4 +- .../Tests/Routing/NestedMatcherTest.php | 69 +++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php index 4b5a90cfe44..687db2cc9c1 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php @@ -2,7 +2,7 @@ /** * @file - * Definition of Drupal\system\Tests\Routing\PartialMatcherTest. + * Definition of Drupal\system\Tests\Routing\HttpMethodMMatcherTest. */ namespace Drupal\system\Tests\Routing; @@ -20,7 +20,7 @@ use Drupal\Core\Routing\FirstEntryFinalMatcher; use Exception; /** - * Basic tests for the UrlMatcherDumper. + * Basic tests for the HttpMethodMatcher class. */ class HttpMethodMatcherTest extends UnitTestBase { diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php new file mode 100644 index 00000000000..5f351ac0d03 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php @@ -0,0 +1,69 @@ + 'NestedMatcher tests', + 'description' => 'Confirm that the NestedMatcher system is working properly.', + 'group' => 'Routing', + ); + } + + function __construct($test_id = NULL) { + parent::__construct($test_id); + + $this->fixtures = new RoutingFixtures(); + } + public function setUp() { + parent::setUp(); + } + + /** + * Confirms we can nest multiple partial matchers. + */ + public function testNestedMatcher() { + + $matcher = new NestedMatcher(); + + $matcher->setInitialMatcher(new MockPathMatcher($this->fixtures->sampleRouteCollection())); + $matcher->addPartialMatcher(new HttpMethodMatcher()); + $matcher->setFinalMatcher(new FirstEntryFinalMatcher()); + + $request = Request::create('/path/one', 'GET'); + + $attributes = $matcher->matchRequest($request); + + $this->assertEqual($attributes['_route'], 'route_a', t('The correct matching route was found.')); + } +} + From 8ae0b323f23cdcd379a0693780fcce1fc5fddeda Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 4 Jul 2012 16:33:48 -0500 Subject: [PATCH 22/72] Don't serialize the compiled route object along with the route. --- core/lib/Drupal/Core/Routing/MatcherDumper.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index d3e292b5f03..28d46e43158 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -99,7 +99,12 @@ class MatcherDumper implements MatcherDumperInterface { 'pattern' => $compiled->getPattern(), 'pattern_outline' => $compiled->getPatternOutline(), 'number_parts' => $compiled->getNumParts(), - 'route' => serialize($route), + // This is only temporary. We need to strip off the compiled route from + // route object in order to serialize it. Cloning strips off the + // compiled route object. Remove this once + // https://github.com/symfony/symfony/pull/4755 is merged and brought + // back downstream. + 'route' => serialize(clone($route)), ); $insert->values($values); } From e31cf8274dfb81a0da877d9cc8a940ecc9fc0263 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 4 Jul 2012 16:35:47 -0500 Subject: [PATCH 23/72] Add tests for the path matcher, both outline-based paths and not-found paths. --- core/lib/Drupal/Core/Routing/PathMatcher.php | 5 ++ .../system/Tests/Routing/PathMatcherTest.php | 54 ++++++++++++++++++- .../system/Tests/Routing/RoutingFixtures.php | 31 ++++++++++- 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index 73f1cb8419c..a5f007c804b 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -4,6 +4,7 @@ namespace Drupal\Core\Routing; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\RouteCollection; +use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Drupal\Core\Database\Connection; @@ -59,6 +60,10 @@ class PathMatcher implements InitialMatcherInterface { $collection->add($name, unserialize($route)); } + if (!count($collection->all())) { + throw new ResourceNotFoundException(); + } + return $collection; } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php index 9e17cfe693c..1e91a34e84e 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -10,12 +10,15 @@ namespace Drupal\system\Tests\Routing; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; +use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Drupal\simpletest\UnitTestBase; use Drupal\Core\Routing\PathMatcher; use Drupal\Core\Database\Database; use Drupal\Core\Routing\MatcherDumper; +use Exception; + /** * Basic tests for the UrlMatcherDumper. */ @@ -99,9 +102,56 @@ class PathMatcherTest extends UnitTestBase { * Confirms that we can find routes whose pattern would match the request. */ function testOutlinePathMatch() { + $connection = Database::getConnection(); + $matcher = new PathMatcher($connection, 'test_routes'); + + $this->fixtures->createTables($connection); + + $dumper = new MatcherDumper($connection, 'test_routes'); + $dumper->addRoutes($this->fixtures->complexRouteCollection()); + $dumper->dump(); + + $path = '/path/1/one'; + + $request = Request::create($path, 'GET'); + + $routes = $matcher->matchRequestPartial($request); + + // All of the matching paths have the correct pattern. + foreach ($routes as $route) { + $this->assertEqual($route->compile()->getPatternOutline(), '/path/%/one', t('Found path has correct pattern')); + } + + $this->assertEqual(count($routes->all()), 2, t('The correct number of routes was found.')); + $this->assertNotNull($routes->get('route_a'), t('The first matching route was found.')); + $this->assertNotNull($routes->get('route_b'), t('The second matching route was not found.')); + } + + /** + * Confirm that an exception is thrown when no matching path is found. + */ + function testOutlinePathNoMatch() { + $connection = Database::getConnection(); + $matcher = new PathMatcher($connection, 'test_routes'); + + $this->fixtures->createTables($connection); + + $dumper = new MatcherDumper($connection, 'test_routes'); + $dumper->addRoutes($this->fixtures->complexRouteCollection()); + $dumper->dump(); + + $path = '/no/such/path'; + + $request = Request::create($path, 'GET'); + + try { + $routes = $matcher->matchRequestPartial($request); + $this->fail(t('No exception was thrown.')); + } + catch (Exception $e) { + $this->assertTrue($e instanceof ResourceNotFoundException, t('The correct exception was thrown.')); + } } - } - diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php index cdcb26572f5..8937c75563a 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php @@ -31,7 +31,6 @@ class RoutingFixtures { } } - /** * Returns a standard set of routes for testing. * @@ -62,6 +61,36 @@ class RoutingFixtures { return $collection; } + /** + * Returns a complex set of routes for testing. + * + * @return \Symfony\Component\Routing\RouteCollection + */ + public function complexRouteCollection() { + $collection = new RouteCollection(); + + $route = new Route('/path/{thing}/one'); + $route->setRequirement('_method', 'GET'); + $collection->add('route_a', $route); + + $route = new Route('/path/{thing}/one'); + $route->setRequirement('_method', 'PUT'); + $collection->add('route_b', $route); + + $route = new Route('/somewhere/{item}/over/the/rainbow'); + $route->setRequirement('_method', 'GET'); + $collection->add('route_c', $route); + + $route = new Route('/another/{thing}/about/{item}'); + $collection->add('route_d', $route); + + $route = new Route('/path/add/one'); + $route->setRequirement('_method', 'GET|HEAD'); + $collection->add('route_e', $route); + + return $collection; + } + public function routingTableDefinition() { $tables['test_routes'] = array( From 5c6979bb1d8806152d870f8f6af86b4e2fb15bcf Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 28 Jul 2012 13:23:34 -0500 Subject: [PATCH 24/72] Add start of a router_test module for hooking in the new routing system. --- .../tests/modules/router_test/router_test.info | 6 ++++++ .../tests/modules/router_test/router_test.module | 13 +++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 core/modules/system/tests/modules/router_test/router_test.info create mode 100644 core/modules/system/tests/modules/router_test/router_test.module diff --git a/core/modules/system/tests/modules/router_test/router_test.info b/core/modules/system/tests/modules/router_test/router_test.info new file mode 100644 index 00000000000..d729865bbaf --- /dev/null +++ b/core/modules/system/tests/modules/router_test/router_test.info @@ -0,0 +1,6 @@ +name = "Router test" +description = "Support module for routing testing." +package = Testing +version = VERSION +core = 8.x +hidden = TRUE diff --git a/core/modules/system/tests/modules/router_test/router_test.module b/core/modules/system/tests/modules/router_test/router_test.module new file mode 100644 index 00000000000..10319946890 --- /dev/null +++ b/core/modules/system/tests/modules/router_test/router_test.module @@ -0,0 +1,13 @@ +add('router_test_1', $route); + + return $collection; +} From 66a2409303ba16d3cb263784703d47f3f361d077 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 14:22:09 -0500 Subject: [PATCH 25/72] Add a basic ChainMatcher, modeled on Symfony CMF's ChainRouter. --- core/lib/Drupal/Core/Routing/ChainMatcher.php | 106 +++++++++++++++++ .../system/Tests/Routing/ChainMatcherTest.php | 111 ++++++++++++++++++ .../system/Tests/Routing/MockMatcher.php | 35 ++++++ 3 files changed, 252 insertions(+) create mode 100644 core/lib/Drupal/Core/Routing/ChainMatcher.php create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/ChainMatcherTest.php create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/MockMatcher.php diff --git a/core/lib/Drupal/Core/Routing/ChainMatcher.php b/core/lib/Drupal/Core/Routing/ChainMatcher.php new file mode 100644 index 00000000000..ecb00cbb5e1 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/ChainMatcher.php @@ -0,0 +1,106 @@ +all() as $matcher) { + try { + return $matcher->matchRequest($request); + } catch (ResourceNotFoundException $e) { + // Needs special care + } catch (MethodNotAllowedException $e) { + $methodNotAllowed = $e; + } + } + + throw $methodNotAllowed ?: new ResourceNotFoundException("None of the matchers in the chain matched this request."); + } + + /** + * Adds a Matcher to the index. + * + * @param MatcherInterface $matcher + * The matcher to add. + * @param int $priority + * The priority of the matcher. Higher number matchers will be checked + * first. + */ + public function add(RequestMatcherInterface $matcher, $priority = 0) { + if (empty($this->matchers[$priority])) { + $this->matchers[$priority] = array(); + } + + $this->matchers[$priority][] = $matcher; + $this->sortedMatchers = array(); + } + + /** + * Sorts the matchers and flattens them. + * + * @return array + * An array of RequestMatcherInterface objects. + */ + public function all() { + if (empty($this->sortedMatchers)) { + $this->sortedMatchers = $this->sortMatchers(); + } + + return $this->sortedMatchers; + } + + /** + * Sort matchers by priority. + * + * The highest priority number is the highest priority (reverse sorting). + * + * @return \Symfony\Component\Routing\RequestMatcherInterface[] + * An array of Matcher objects in the order they should be used. + */ + protected function sortMatchers() { + $sortedMatchers = array(); + krsort($this->matchers); + + foreach ($this->matchers as $matchers) { + $sortedMatchers = array_merge($sortedMatchers, $matchers); + } + + return $sortedMatchers; + } + +} diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/ChainMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/ChainMatcherTest.php new file mode 100644 index 00000000000..cec4d79283a --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/ChainMatcherTest.php @@ -0,0 +1,111 @@ + 'Chain matcher tests', + 'description' => 'Confirm that the chain matcher is working correctly.', + 'group' => 'Routing', + ); + } + + /** + * Confirms that the expected exception is thrown. + */ + public function testMethodNotAllowed() { + + $chain = new ChainMatcher(); + + $method_not_allowed = new MockMatcher(function(Request $request) { + throw new MethodNotAllowedException(array('POST')); + }); + + try { + $chain->add($method_not_allowed); + $chain->matchRequest(Request::create('my/path')); + } + catch (MethodNotAllowedException $e) { + $this->pass('Correct exception thrown.'); + } + catch (Exception $e) { + $this->fail('Incorrect exception thrown: ' . get_class($e)); + } + } + + /** + * Confirms that the expected exception is thrown. + */ + public function testRequestNotFound() { + + $chain = new ChainMatcher(); + + $resource_not_found = new MockMatcher(function(Request $request) { + throw new ResourceNotFoundException(); + }); + + try { + $chain->add($resource_not_found); + $chain->matchRequest(Request::create('my/path')); + } + catch (ResourceNotFoundException $e) { + $this->pass('Correct exception thrown.'); + } + catch (Exception $e) { + $this->fail('Incorrect exception thrown: ' . get_class($e)); + } + } + + /** + * Confirms that the expected exception is thrown. + */ + public function testRequestFound() { + + $chain = new ChainMatcher(); + + $method_not_allowed = new MockMatcher(function(Request $request) { + throw new MethodNotAllowedException(array('POST')); + }); + + $resource_not_found = new MockMatcher(function(Request $request) { + throw new ResourceNotFoundException(); + }); + + $found_data = new MockMatcher(function(Request $request) { + return array('_controller' => 'foo'); + }); + + try { + $chain->add($method_not_allowed); + $chain->add($resource_not_found); + $chain->add($found_data); + $request = Request::create('my/path'); + $attributes = $chain->matchRequest($request); + $this->assertEqual($attributes['_controller'], 'foo', 'Correct attributes returned.'); + } + catch (Exception $e) { + $this->fail('Exception thrown when a match should have been successful: ' . get_class($e)); + } + } + +} diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MockMatcher.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MockMatcher.php new file mode 100644 index 00000000000..6cd58fcb369 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MockMatcher.php @@ -0,0 +1,35 @@ +matcher = $matcher; + } + + public function matchRequest(Request $request) { + $matcher = $this->matcher; + return $matcher($request); + } +} + From 41586439a4f4d7487e3ca221d36a16ab300ce578 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 14:29:25 -0500 Subject: [PATCH 26/72] Remove extraneous t() calls from assertion messages. --- .../Tests/Routing/HttpMethodMatcherTest.php | 16 +++++++------- .../Tests/Routing/MatcherDumperTest.php | 14 ++++++------ .../Tests/Routing/NestedMatcherTest.php | 2 +- .../system/Tests/Routing/PathMatcherTest.php | 22 +++++++++---------- .../Drupal/system/Tests/Routing/RouteTest.php | 6 ++--- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php index 687db2cc9c1..962abfc3d8f 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php @@ -58,12 +58,12 @@ class HttpMethodMatcherTest extends UnitTestBase { $routes = $matcher->matchRequestPartial(Request::create('path/one', 'GET')); - $this->assertEqual(count($routes->all()), 4, t('The correct number of routes was found.')); - $this->assertNotNull($routes->get('route_a'), t('The first matching route was found.')); - $this->assertNull($routes->get('route_b'), t('The non-matching route was not found.')); - $this->assertNotNull($routes->get('route_c'), t('The second matching route was found.')); - $this->assertNotNull($routes->get('route_d'), t('The all-matching route was found.')); - $this->assertNotNull($routes->get('route_e'), t('The multi-matching route was found.')); + $this->assertEqual(count($routes->all()), 4, 'The correct number of routes was found.'); + $this->assertNotNull($routes->get('route_a'), 'The first matching route was found.'); + $this->assertNull($routes->get('route_b'), 'The non-matching route was not found.'); + $this->assertNotNull($routes->get('route_c'), 'The second matching route was found.'); + $this->assertNotNull($routes->get('route_d'), 'The all-matching route was found.'); + $this->assertNotNull($routes->get('route_e'), 'The multi-matching route was found.'); } /** @@ -81,7 +81,7 @@ class HttpMethodMatcherTest extends UnitTestBase { $attributes = $matcher->matchRequest($request); - $this->assertEqual($attributes['_route'], 'route_a', t('The correct matching route was found.')); + $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); } /** @@ -101,7 +101,7 @@ class HttpMethodMatcherTest extends UnitTestBase { $this->fail(t('No exception was thrown.')); } catch (Exception $e) { - $this->assertTrue($e instanceof MethodNotAllowedException, t('The correct exception was thrown.')); + $this->assertTrue($e instanceof MethodNotAllowedException, 'The correct exception was thrown.'); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php index d7c9a01f20d..7f6f312c731 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php @@ -52,7 +52,7 @@ class MatcherDumperTest extends UnitTestBase { $dumper= new MatcherDumper($connection); $class_name = 'Drupal\Core\Routing\MatcherDumper'; - $this->assertTrue($dumper instanceof $class_name, t('Dumper created successfully')); + $this->assertTrue($dumper instanceof $class_name, 'Dumper created successfully'); } /** @@ -72,7 +72,7 @@ class MatcherDumperTest extends UnitTestBase { $collection_routes = $collection->all(); foreach ($dumper_routes as $name => $route) { - $this->assertEqual($route->getPattern(), $collection_routes[$name]->getPattern(), t('Routes match')); + $this->assertEqual($route->getPattern(), $collection_routes[$name]->getPattern(), 'Routes match'); } } @@ -134,11 +134,11 @@ class MatcherDumperTest extends UnitTestBase { $loaded_route = unserialize($record->route); - $this->assertEqual($record->name, 'test_route', t('Dumped route has correct name.')); - $this->assertEqual($record->pattern, '/test/{my}/path', t('Dumped route has correct pattern.')); - $this->assertEqual($record->pattern_outline, '/test/%/path', t('Dumped route has correct pattern outline.')); - $this->assertEqual($record->fit, 5 /* 101 in binary */, t('Dumped route has correct fit.')); - $this->assertTrue($loaded_route instanceof Route, t('Route object retrieved successfully.')); + $this->assertEqual($record->name, 'test_route', 'Dumped route has correct name.'); + $this->assertEqual($record->pattern, '/test/{my}/path', 'Dumped route has correct pattern.'); + $this->assertEqual($record->pattern_outline, '/test/%/path', 'Dumped route has correct pattern outline.'); + $this->assertEqual($record->fit, 5 /* 101 in binary */, 'Dumped route has correct fit.'); + $this->assertTrue($loaded_route instanceof Route, 'Route object retrieved successfully.'); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php index 5f351ac0d03..9c0f5de310f 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php @@ -63,7 +63,7 @@ class NestedMatcherTest extends UnitTestBase { $attributes = $matcher->matchRequest($request); - $this->assertEqual($attributes['_route'], 'route_a', t('The correct matching route was found.')); + $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php index 1e91a34e84e..055a1e76c34 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -67,11 +67,11 @@ class PathMatcherTest extends UnitTestBase { $candidates = array_flip($candidates); - $this->assertTrue(count($candidates) == 4, t('Correct number of candidates found')); - $this->assertTrue(array_key_exists('/node/5/edit', $candidates), t('First candidate found.')); - $this->assertTrue(array_key_exists('/node/5/%', $candidates), t('Second candidate found.')); - $this->assertTrue(array_key_exists('/node/%/edit', $candidates), t('Third candidate found.')); - $this->assertTrue(array_key_exists('/node/%/%', $candidates), t('Fourth candidate found.')); + $this->assertTrue(count($candidates) == 4, 'Correct number of candidates found'); + $this->assertTrue(array_key_exists('/node/5/edit', $candidates), 'First candidate found.'); + $this->assertTrue(array_key_exists('/node/5/%', $candidates), 'Second candidate found.'); + $this->assertTrue(array_key_exists('/node/%/edit', $candidates), 'Third candidate found.'); + $this->assertTrue(array_key_exists('/node/%/%', $candidates), 'Fourth candidate found.'); } /** @@ -94,7 +94,7 @@ class PathMatcherTest extends UnitTestBase { $routes = $matcher->matchRequestPartial($request); foreach ($routes as $route) { - $this->assertEqual($route->getPattern(), $path, t('Found path has correct pattern')); + $this->assertEqual($route->getPattern(), $path, 'Found path has correct pattern'); } } @@ -119,12 +119,12 @@ class PathMatcherTest extends UnitTestBase { // All of the matching paths have the correct pattern. foreach ($routes as $route) { - $this->assertEqual($route->compile()->getPatternOutline(), '/path/%/one', t('Found path has correct pattern')); + $this->assertEqual($route->compile()->getPatternOutline(), '/path/%/one', 'Found path has correct pattern'); } - $this->assertEqual(count($routes->all()), 2, t('The correct number of routes was found.')); - $this->assertNotNull($routes->get('route_a'), t('The first matching route was found.')); - $this->assertNotNull($routes->get('route_b'), t('The second matching route was not found.')); + $this->assertEqual(count($routes->all()), 2, 'The correct number of routes was found.'); + $this->assertNotNull($routes->get('route_a'), 'The first matching route was found.'); + $this->assertNotNull($routes->get('route_b'), 'The second matching route was not found.'); } /** @@ -149,7 +149,7 @@ class PathMatcherTest extends UnitTestBase { $this->fail(t('No exception was thrown.')); } catch (Exception $e) { - $this->assertTrue($e instanceof ResourceNotFoundException, t('The correct exception was thrown.')); + $this->assertTrue($e instanceof ResourceNotFoundException, 'The correct exception was thrown.'); } } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php index af840b2e7c4..7ed212b8ea0 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php @@ -33,9 +33,9 @@ class RouteTest extends UnitTestBase { $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); $compiled = $route->compile(); - $this->assertEqual($route, $compiled->getRoute(), t('Compiled route has the correct route object.')); - $this->assertEqual($compiled->getFit(), 5 /* That's 101 binary*/, t('The fit was correct.')); - $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', t('The pattern outline was correct.')); + $this->assertEqual($route, $compiled->getRoute(), 'Compiled route has the correct route object.'); + $this->assertEqual($compiled->getFit(), 5 /* That's 101 binary*/, 'The fit was correct.'); + $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', 'The pattern outline was correct.'); } } From 1bf98066ba6546039f8d2c888381aa4e9d7d4a5b Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 15:25:48 -0500 Subject: [PATCH 27/72] Convert LegacyUrlMatcher to use RequestMatcherInterface rather than UrlMatcherInterface. --- core/lib/Drupal/Core/CoreBundle.php | 2 +- core/lib/Drupal/Core/LegacyUrlMatcher.php | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index 4dca8043089..925e315cb0f 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -59,7 +59,7 @@ class CoreBundle extends Bundle $dispatcher = $container->get('dispatcher'); $matcher = new \Drupal\Core\LegacyUrlMatcher(); $content_negotation = new \Drupal\Core\ContentNegotiation(); - $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\RouterListener($matcher)); + $dispatcher->addSubscriber(new \Symfony\Component\HttpKernel\EventListener\RouterListener($matcher)); $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\ViewSubscriber($content_negotation)); $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\AccessSubscriber()); $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\MaintenanceModeSubscriber()); diff --git a/core/lib/Drupal/Core/LegacyUrlMatcher.php b/core/lib/Drupal/Core/LegacyUrlMatcher.php index 8828f36d27b..48987a9291f 100644 --- a/core/lib/Drupal/Core/LegacyUrlMatcher.php +++ b/core/lib/Drupal/Core/LegacyUrlMatcher.php @@ -9,13 +9,14 @@ namespace Drupal\Core; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Exception\ResourceNotFoundException; -use Symfony\Component\Routing\Matcher\UrlMatcherInterface; +use Symfony\Component\Routing\Matcher\RequestMatcherInterface; +use Symfony\Component\Routing\RequestContextAwareInterface; use Symfony\Component\Routing\RequestContext; /** * UrlMatcher matches URL based on a set of routes. */ -class LegacyUrlMatcher implements UrlMatcherInterface { +class LegacyUrlMatcher implements RequestMatcherInterface, RequestContextAwareInterface { /** * The request context for this matcher. @@ -98,8 +99,8 @@ class LegacyUrlMatcher implements UrlMatcherInterface { * * @api */ - public function match($pathinfo) { - if ($router_item = $this->matchDrupalItem($pathinfo)) { + public function matchRequest(Request $request) { + if ($router_item = $this->matchDrupalItem($request->attributes->get('system_path'))) { $ret = $this->convertDrupalItem($router_item); // Stash the router item in the attributes while we're transitioning. $ret['drupal_menu_item'] = $router_item; From a6d59f6d18a6282b43c7fa36bf22e2309b06dbd5 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 15:31:20 -0500 Subject: [PATCH 28/72] Run LegacyUrlMatcher through ChainMatcher. That necessitates making ChainMatcher temporarily context-aware. --- core/lib/Drupal/Core/CoreBundle.php | 3 +- core/lib/Drupal/Core/Routing/ChainMatcher.php | 57 ++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index 925e315cb0f..865afe50660 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -57,7 +57,8 @@ class CoreBundle extends Bundle // @todo Replace below lines with the commented out block below it when it's // performant to do so: http://drupal.org/node/1706064. $dispatcher = $container->get('dispatcher'); - $matcher = new \Drupal\Core\LegacyUrlMatcher(); + $matcher = new \Drupal\Core\Routing\ChainMatcher(); + $matcher->add(new \Drupal\Core\LegacyUrlMatcher()); $content_negotation = new \Drupal\Core\ContentNegotiation(); $dispatcher->addSubscriber(new \Symfony\Component\HttpKernel\EventListener\RouterListener($matcher)); $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\ViewSubscriber($content_negotation)); diff --git a/core/lib/Drupal/Core/Routing/ChainMatcher.php b/core/lib/Drupal/Core/Routing/ChainMatcher.php index ecb00cbb5e1..cf8a66d8a4c 100644 --- a/core/lib/Drupal/Core/Routing/ChainMatcher.php +++ b/core/lib/Drupal/Core/Routing/ChainMatcher.php @@ -7,11 +7,16 @@ use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\Exception\RouteNotFoundException; use Symfony\Component\Routing\Exception\MethodNotAllowedException; +use Symfony\Component\Routing\RequestContextAwareInterface; +use Symfony\Component\Routing\RequestContext; /** - * Description of ChainMatcher + * Aggregates multiple matchers together in series. + * + * The RequestContext is entirely unused. It's included only to satisfy the + * interface needed for RouterListener. Hopefully we can remove it later. */ -class ChainMatcher implements RequestMatcherInterface { +class ChainMatcher implements RequestMatcherInterface, RequestContextAwareInterface { /** * Array of RequestMatcherInterface objects to be checked in order. @@ -26,6 +31,54 @@ class ChainMatcher implements RequestMatcherInterface { */ protected $sortedMatchers = array(); + /** + * The request context for this matcher. + * + * This is unused. It's just to satisfy the interface. + * + * @var Symfony\Component\Routing\RequestContext + */ + protected $context; + + /** + * Constructor. + */ + public function __construct() { + // We will not actually use this object, but it's needed to conform to + // the interface. + $this->context = new RequestContext(); + } + + /** + * Sets the request context. + * + * This method is just to satisfy the interface, and is largely vestigial. + * The request context object does not contain the information we need, so + * we will use the original request object. + * + * @param Symfony\Component\Routing\RequestContext $context + * The context. + * + * @api + */ + public function setContext(RequestContext $context) { + $this->context = $context; + } + + /** + * Gets the request context. + * + * This method is just to satisfy the interface, and is largely vestigial. + * The request context object does not contain the information we need, so + * we will use the original request object. + * + * @return Symfony\Component\Routing\RequestContext + * The context. + */ + public function getContext() { + return $this->context; + } + /** * Matches a request against all queued matchers. * From ac10076ccf5be2dc4f216dd29d451adeb9aac358 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 15:53:39 -0500 Subject: [PATCH 29/72] Wire the new PartialMatcher and PathMatcher into the routing configuration. --- core/lib/Drupal/Core/CoreBundle.php | 9 ++ core/modules/system/system.install | 188 ++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+) diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index 865afe50660..bf51079c4bc 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -15,6 +15,8 @@ use Symfony\Component\DependencyInjection\Scope; use Symfony\Component\HttpKernel\Bundle\Bundle; use Symfony\Component\DependencyInjection\Compiler\PassConfig; +use Drupal\Core\Database\Database; + /** * Bundle class for mandatory core services. * @@ -59,6 +61,13 @@ class CoreBundle extends Bundle $dispatcher = $container->get('dispatcher'); $matcher = new \Drupal\Core\Routing\ChainMatcher(); $matcher->add(new \Drupal\Core\LegacyUrlMatcher()); + + $nested = new \Drupal\Core\Routing\NestedMatcher(); + $nested->setInitialMatcher(new \Drupal\Core\Routing\PathMatcher(Database::getConnection())); + $nested->addPartialMatcher(new \Drupal\Core\Routing\HttpMethodMatcher()); + $nested->setFinalMatcher(new \Drupal\Core\Routing\FirstEntryFinalMatcher()); + $matcher->add($nested, 5); + $content_negotation = new \Drupal\Core\ContentNegotiation(); $dispatcher->addSubscriber(new \Symfony\Component\HttpKernel\EventListener\RouterListener($matcher)); $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\ViewSubscriber($content_negotation)); diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 3790f2b407d..5b8fd17dd2d 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -1225,6 +1225,125 @@ function system_schema() { ), ); + $schema['registry'] = array( + 'description' => "Each record is a function, class, or interface name and the file it is in.", + 'fields' => array( + 'name' => array( + 'description' => 'The name of the function, class, or interface.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'type' => array( + 'description' => 'Either function or class or interface.', + 'type' => 'varchar', + 'length' => 9, + 'not null' => TRUE, + 'default' => '', + ), + 'filename' => array( + 'description' => 'Name of the file.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + ), + 'module' => array( + 'description' => 'Name of the module the file belongs to.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '' + ), + 'weight' => array( + 'description' => "The order in which this module's hooks should be invoked relative to other modules. Equal-weighted modules are ordered by name.", + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + ), + ), + 'primary key' => array('name', 'type'), + 'indexes' => array( + 'hook' => array('type', 'weight', 'module'), + ), + ); + + $schema['registry_file'] = array( + 'description' => "Files parsed to build the registry.", + 'fields' => array( + 'filename' => array( + 'description' => 'Path to the file.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + ), + 'hash' => array( + 'description' => "sha-256 hash of the file's contents when last parsed.", + 'type' => 'varchar', + 'length' => 64, + 'not null' => TRUE, + ), + ), + 'primary key' => array('filename'), + ); + + $schema['router'] = array( + 'description' => 'Maps paths to various callbacks (access, page and title)', + 'fields' => array( + 'name' => array( + 'description' => 'Primary Key: Machine name of this route', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'pattern' => array( + 'description' => 'The path pattern for this URI', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'pattern_outline' => array( + 'description' => 'The pattern', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'route_set' => array( + 'description' => 'The route set grouping to which a route belongs.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'fit' => array( + 'description' => 'A numeric representation of how specific the path is.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + ), + 'route' => array( + 'description' => 'A serialized Route object', + 'type' => 'text', + ), + 'number_parts' => array( + 'description' => 'Number of parts in this router path.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + 'size' => 'small', + ), + ), + 'indexes' => array( + 'fit' => array('fit'), + 'pattern_outline' => array('pattern_outline'), + 'route_set' => array('route_set'), + ), + 'primary key' => array('name'), + ); + $schema['semaphore'] = array( 'description' => 'Table for holding semaphores, locks, flags, etc. that cannot be stored as Drupal variables since they must not be cached.', 'fields' => array( @@ -1914,6 +2033,75 @@ function system_update_8019() { db_drop_table('registry_file'); } +/* + * Create the new routing table. + */ +function system_update_8020() { + + $tables['router'] = array( + 'description' => 'Maps paths to various callbacks (access, page and title)', + 'fields' => array( + 'name' => array( + 'description' => 'Primary Key: Machine name of this route', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'pattern' => array( + 'description' => 'The path pattern for this URI', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'pattern_outline' => array( + 'description' => 'The pattern', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'route_set' => array( + 'description' => 'The route set grouping to which a route belongs.', + 'type' => 'varchar', + 'length' => 255, + 'not null' => TRUE, + 'default' => '', + ), + 'fit' => array( + 'description' => 'A numeric representation of how specific the path is.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + ), + 'route' => array( + 'description' => 'A serialized Route object', + 'type' => 'text', + ), + 'number_parts' => array( + 'description' => 'Number of parts in this router path.', + 'type' => 'int', + 'not null' => TRUE, + 'default' => 0, + 'size' => 'small', + ), + ), + 'indexes' => array( + 'fit' => array('fit'), + 'pattern_outline' => array('pattern_outline'), + 'route_set' => array('route_set'), + ), + 'primary key' => array('name'), + ); + + $schema = Database::getConnection()->schema(); + + $schema->dropTable('router'); + + $schema->createTable('router', $tables['router']); +} + /** * Conditionally enable the new Ban module. */ From 2bf5b4d17ea42f09da9e164362337502243b1026 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Fri, 7 Sep 2012 00:33:00 -0500 Subject: [PATCH 30/72] Wire the new PartialMatcher and PathMatcher into the routing configuration. --- core/modules/system/system.install | 62 ------------------------------ 1 file changed, 62 deletions(-) diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 5b8fd17dd2d..1c0d43cdb25 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -1225,68 +1225,6 @@ function system_schema() { ), ); - $schema['registry'] = array( - 'description' => "Each record is a function, class, or interface name and the file it is in.", - 'fields' => array( - 'name' => array( - 'description' => 'The name of the function, class, or interface.', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - 'default' => '', - ), - 'type' => array( - 'description' => 'Either function or class or interface.', - 'type' => 'varchar', - 'length' => 9, - 'not null' => TRUE, - 'default' => '', - ), - 'filename' => array( - 'description' => 'Name of the file.', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - ), - 'module' => array( - 'description' => 'Name of the module the file belongs to.', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - 'default' => '' - ), - 'weight' => array( - 'description' => "The order in which this module's hooks should be invoked relative to other modules. Equal-weighted modules are ordered by name.", - 'type' => 'int', - 'not null' => TRUE, - 'default' => 0, - ), - ), - 'primary key' => array('name', 'type'), - 'indexes' => array( - 'hook' => array('type', 'weight', 'module'), - ), - ); - - $schema['registry_file'] = array( - 'description' => "Files parsed to build the registry.", - 'fields' => array( - 'filename' => array( - 'description' => 'Path to the file.', - 'type' => 'varchar', - 'length' => 255, - 'not null' => TRUE, - ), - 'hash' => array( - 'description' => "sha-256 hash of the file's contents when last parsed.", - 'type' => 'varchar', - 'length' => 64, - 'not null' => TRUE, - ), - ), - 'primary key' => array('filename'), - ); - $schema['router'] = array( 'description' => 'Maps paths to various callbacks (access, page and title)', 'fields' => array( From d4641c4a5b186b6f006d092d9af1704db3a8dbb6 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 17:21:55 -0500 Subject: [PATCH 31/72] Don't count the leading / as a part when counting the elements in the path. --- core/lib/Drupal/Core/Routing/RouteCompiler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index c05e72590ec..a34e5c8d4e3 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -31,7 +31,7 @@ class RouteCompiler implements RouteCompilerInterface { $pattern_outline = $this->getPatternOutline($route->getPattern()); - $num_parts = count(explode('/', $pattern_outline)); + $num_parts = count(explode('/', trim($pattern_outline, '/'))); return new CompiledRoute($route, $fit, $pattern_outline, $num_parts); From b1fd850de2009af30e1654662afcbe355e73eb67 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 17:57:16 -0500 Subject: [PATCH 32/72] Register the MatcherDumper with the DIC. --- core/lib/Drupal/Core/CoreBundle.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index bf51079c4bc..7cd68ce05d9 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -56,6 +56,18 @@ class CoreBundle extends Bundle ->addArgument('slave'); $container->register('typed_data', 'Drupal\Core\TypedData\TypedDataManager'); + $container->register('router.dumper', '\Drupal\Core\Routing\MatcherDumper') + ->addArgument(new Reference('database')); + + $container->register('database', 'Drupal\Core\Database\Connection') + ->setFactoryClass('Drupal\Core\Database\Database') + ->setFactoryMethod('getConnection') + ->addArgument('default'); + $container->register('database.slave', 'Drupal\Core\Database\Connection') + ->setFactoryClass('Drupal\Core\Database\Database') + ->setFactoryMethod('getConnection') + ->addArgument('slave'); + // @todo Replace below lines with the commented out block below it when it's // performant to do so: http://drupal.org/node/1706064. $dispatcher = $container->get('dispatcher'); From c2e1a308de5cf8620c3fe171f62551ab8569a400 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 17:57:47 -0500 Subject: [PATCH 33/72] Flush the dumper after each use, so that we don't end up re-saving old routes. --- core/lib/Drupal/Core/Routing/MatcherDumper.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index 28d46e43158..6f211d976fb 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -120,6 +120,10 @@ class MatcherDumper implements MatcherDumperInterface { $insert->execute(); + // We want to reuse the dumper for multiple route sets, so on dump, flush + // the queued routes. + $this->routes = NULL; + // Transaction ends here. } From 7139f0253c080b20c1558f9fb656bbec4d59bc37 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 17:59:13 -0500 Subject: [PATCH 34/72] Add a new-router rebuild step to the global flush operation. --- core/includes/common.inc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core/includes/common.inc b/core/includes/common.inc index aea8529060a..6300dece781 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -6829,6 +6829,7 @@ function drupal_flush_all_caches() { // Rebuild the menu router based on all rebuilt data. // Important: This rebuild must happen last, so the menu router is guaranteed // to be based on up to date information. + router_rebuild(); menu_router_rebuild(); // Re-initialize the maintenance theme, if the current request attempted to @@ -6840,6 +6841,21 @@ function drupal_flush_all_caches() { } } +function router_rebuild() { + // We need to manually call each module so that we can know which module + // a given item came from. + $callbacks = array(); + + $dumper = drupal_container()->get('router.dumper'); + + foreach (module_implements('route_info') as $module) { + $routes = call_user_func($module . '_route_info'); + drupal_alter('router_info', $routes); + $dumper->addRoutes($routes); + $dumper->dump(array('route_set' => $module)); + } +} + /** * Changes the dummy query string added to all CSS and JavaScript files. * From 684f00dcc13142c4791e55af9b97b985b7a0adff Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 18:00:28 -0500 Subject: [PATCH 35/72] Add a functional test for the new router. --- .../system/Tests/Routing/RouterTest.php | 33 +++++++++++++++++++ .../Drupal/router_test/TestControllers.php | 17 ++++++++++ .../modules/router_test/router_test.module | 6 ++-- 3 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php create mode 100644 core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php new file mode 100644 index 00000000000..a5449afc04c --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php @@ -0,0 +1,33 @@ + 'Integrated Router tests', + 'description' => 'Function Tests for the fully integrated routing system.', + 'group' => 'Routing', + ); + } + + public function testCanRoute() { + $this->drupalGet('router_test/test1'); + $this->assertRaw('test1', 'The correct string was returned because the route was successful.'); + } + +} diff --git a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php new file mode 100644 index 00000000000..1476e46de22 --- /dev/null +++ b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php @@ -0,0 +1,17 @@ + '\Drupal\router_test\TestControllers::test1' + )); $collection->add('router_test_1', $route); return $collection; From dca406aa6444c39f630ec67497890c5d1f2d27af Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 11 Aug 2012 23:45:08 -0500 Subject: [PATCH 36/72] Only try to rebuild the router table if the dumper is actually available. --- core/includes/common.inc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/core/includes/common.inc b/core/includes/common.inc index 6300dece781..b75185e2365 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -4,6 +4,7 @@ use Drupal\Component\Utility\NestedArray; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Drupal\Core\Cache\CacheBackendInterface; +use Symfony\Component\DependencyInjection\Container; use Drupal\Core\Database\Database; use Drupal\Core\Template\Attribute; @@ -6846,13 +6847,15 @@ function router_rebuild() { // a given item came from. $callbacks = array(); - $dumper = drupal_container()->get('router.dumper'); + $dumper = drupal_container()->get('router.dumper', Container::NULL_ON_INVALID_REFERENCE); - foreach (module_implements('route_info') as $module) { - $routes = call_user_func($module . '_route_info'); - drupal_alter('router_info', $routes); - $dumper->addRoutes($routes); - $dumper->dump(array('route_set' => $module)); + if ($dumper) { + foreach (module_implements('route_info') as $module) { + $routes = call_user_func($module . '_route_info'); + drupal_alter('router_info', $routes); + $dumper->addRoutes($routes); + $dumper->dump(array('route_set' => $module)); + } } } From f368409fcc65e42a02e83a6c26ddc51485f9269f Mon Sep 17 00:00:00 2001 From: Katherine Bailey Date: Thu, 30 Aug 2012 21:00:35 -0700 Subject: [PATCH 37/72] The database service has been added to the DIC in core so ended up being duplicated here --- core/lib/Drupal/Core/CoreBundle.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index 7cd68ce05d9..6437d697613 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -59,15 +59,6 @@ class CoreBundle extends Bundle $container->register('router.dumper', '\Drupal\Core\Routing\MatcherDumper') ->addArgument(new Reference('database')); - $container->register('database', 'Drupal\Core\Database\Connection') - ->setFactoryClass('Drupal\Core\Database\Database') - ->setFactoryMethod('getConnection') - ->addArgument('default'); - $container->register('database.slave', 'Drupal\Core\Database\Connection') - ->setFactoryClass('Drupal\Core\Database\Database') - ->setFactoryMethod('getConnection') - ->addArgument('slave'); - // @todo Replace below lines with the commented out block below it when it's // performant to do so: http://drupal.org/node/1706064. $dispatcher = $container->get('dispatcher'); From c17d7dce9d2199aef55d62180999fbb3cb51385a Mon Sep 17 00:00:00 2001 From: Katherine Bailey Date: Thu, 30 Aug 2012 21:46:07 -0700 Subject: [PATCH 38/72] Updating some docblocks --- core/lib/Drupal/Core/Routing/FinalMatcherInterface.php | 2 +- core/lib/Drupal/Core/Routing/MatcherDumper.php | 2 +- core/lib/Drupal/Core/Routing/NestedMatcherInterface.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php index 2c8c80082c5..ae2bba00111 100644 --- a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php @@ -6,7 +6,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\RouteCollection; /** - * A PartialMatcher works like a UrlMatcher, but will return multiple candidate routes. + * A FinalMatcher returns only one route from a collection of candidate routes. */ interface FinalMatcherInterface { diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index 6f211d976fb..2d454f45463 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -60,7 +60,7 @@ class MatcherDumper implements MatcherDumperInterface { } /** - * Dumps a set of routes to a PHP class. + * Dumps a set of routes to the router table in the database. * * Available options: * diff --git a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php index 6ae7428fd6e..cd55d32dd8c 100644 --- a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php @@ -39,7 +39,7 @@ interface NestedMatcherInterface extends RequestMatcherInterface { /** * Sets the final matcher for the matching plan. * - * @param UrlMatcherInterface $final + * @param FinalMatcherInterface $final * The matcher that will be called last to ensure only a single route is * found. * From 89e4c42bbd9124087ddd2d13649fd2aaab736949 Mon Sep 17 00:00:00 2001 From: Katherine Bailey Date: Thu, 30 Aug 2012 22:04:04 -0700 Subject: [PATCH 39/72] Fixing the update number in system.install --- .../system/lib/Drupal/system/FileDownload.php | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 core/modules/system/lib/Drupal/system/FileDownload.php diff --git a/core/modules/system/lib/Drupal/system/FileDownload.php b/core/modules/system/lib/Drupal/system/FileDownload.php new file mode 100644 index 00000000000..b55872c14f1 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/FileDownload.php @@ -0,0 +1,54 @@ + Date: Sat, 1 Sep 2012 20:31:47 -0500 Subject: [PATCH 40/72] Minor fixups in router_rebuild(). --- core/includes/common.inc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/includes/common.inc b/core/includes/common.inc index b75185e2365..e27f4d9f0cd 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -6845,14 +6845,13 @@ function drupal_flush_all_caches() { function router_rebuild() { // We need to manually call each module so that we can know which module // a given item came from. - $callbacks = array(); $dumper = drupal_container()->get('router.dumper', Container::NULL_ON_INVALID_REFERENCE); if ($dumper) { foreach (module_implements('route_info') as $module) { $routes = call_user_func($module . '_route_info'); - drupal_alter('router_info', $routes); + drupal_alter('router_info', $routes, $module); $dumper->addRoutes($routes); $dumper->dump(array('route_set' => $module)); } From b9d568998736b06c0d91933244e9974729091520 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 2 Sep 2012 11:40:09 -0500 Subject: [PATCH 41/72] Introduce a default controller for pages with a _content request attribute. --- core/lib/Drupal/Core/CoreBundle.php | 1 + .../RouteProcessorSubscriber.php | 72 +++++++++++++++++++ core/lib/Drupal/Core/HtmlPageController.php | 57 +++++++++++++++ .../system/Tests/Routing/RouterTest.php | 12 ++++ .../Drupal/router_test/TestControllers.php | 11 ++- .../modules/router_test/router_test.module | 8 +++ 6 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php create mode 100644 core/lib/Drupal/Core/HtmlPageController.php diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index 6437d697613..fceaf76ef8b 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -82,6 +82,7 @@ class CoreBundle extends Bundle $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\FinishResponseSubscriber()); $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\RequestCloseSubscriber()); $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\ConfigGlobalOverrideSubscriber()); + $dispatcher->addSubscriber(new \Drupal\Core\EventSubscriber\RouteProcessorSubscriber()); $container->set('content_negotiation', $content_negotation); $dispatcher->addSubscriber(\Drupal\Core\ExceptionController::getExceptionListener($container)); diff --git a/core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php new file mode 100644 index 00000000000..816366657fa --- /dev/null +++ b/core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php @@ -0,0 +1,72 @@ +getRequest(); + + if (!$request->attributes->has('_controller') && $request->attributes->has('_content')) { + $request->attributes->set('_controller', '\Drupal\Core\HtmlPageController::content'); + } + + } + + /** + * Registers the methods in this class that should be listeners. + * + * @return array + * An array of event listener definitions. + */ + static function getSubscribedEvents() { + // The RouterListener has priority 32, and we need to run after that. + $events[KernelEvents::REQUEST][] = array('onRequestSetController', 30); + + return $events; + } +} diff --git a/core/lib/Drupal/Core/HtmlPageController.php b/core/lib/Drupal/Core/HtmlPageController.php new file mode 100644 index 00000000000..000e9abe483 --- /dev/null +++ b/core/lib/Drupal/Core/HtmlPageController.php @@ -0,0 +1,57 @@ +getContentController($_content); + + $page_callback_result = call_user_func_array($content_controller, array()); + + return new Response(drupal_render_page($page_callback_result)); + } + + protected function getContentController($controller) { + if (is_array($controller) || (is_object($controller) && method_exists($controller, '__invoke'))) { + return $controller; + } + + if (FALSE === strpos($controller, ':')) { + if (method_exists($controller, '__invoke')) { + return new $controller; + } elseif (function_exists($controller)) { + return $controller; + } + } + + list($controller, $method) = $this->createController($controller); + + if (!method_exists($controller, $method)) { + throw new \InvalidArgumentException(sprintf('Method "%s::%s" does not exist.', get_class($controller), $method)); + } + + return array($controller, $method); + } + + protected function createController($controller) { + if (false === strpos($controller, '::')) { + throw new \InvalidArgumentException(sprintf('Unable to find controller "%s".', $controller)); + } + + list($class, $method) = explode('::', $controller, 2); + + if (!class_exists($class)) { + throw new \InvalidArgumentException(sprintf('Class "%s" does not exist.', $class)); + } + + return array(new $class(), $method); + } + + +} 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 a5449afc04c..ac7d5b7239b 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php @@ -25,9 +25,21 @@ class RouterTest extends WebTestBase { ); } + /** + * Confirms that the router can get to a controller. + */ 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() { + $this->drupalGet('router_test/test2'); + $this->assertRaw('test2', 'The correct string was returned because the route was successful.'); + $this->assertRaw('', 'Page markup was found.'); + } + } diff --git a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php index 1476e46de22..e6c5c9eb2ca 100644 --- a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php +++ b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php @@ -1,11 +1,16 @@ add('router_test_1', $route); + $route = new Route('router_test/test2', array( + '_content' => '\Drupal\router_test\TestControllers::test2' + )); + $collection->add('router_test_2', $route); + return $collection; } From 404e74e1872d5e1ce3b2afc5d90bd16e4ce151e7 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 2 Sep 2012 12:46:06 -0500 Subject: [PATCH 42/72] Make the new HtmlPageController Container-aware. --- core/lib/Drupal/Core/HtmlPageController.php | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/HtmlPageController.php b/core/lib/Drupal/Core/HtmlPageController.php index 000e9abe483..bfc6d578b62 100644 --- a/core/lib/Drupal/Core/HtmlPageController.php +++ b/core/lib/Drupal/Core/HtmlPageController.php @@ -4,9 +4,21 @@ namespace Drupal\Core; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\DependencyInjection\ContainerAwareInterface; +use Symfony\Component\DependencyInjection\ContainerInterface; +class HtmlPageController implements ContainerAwareInterface { -class HtmlPageController { + /** + * The injection container for this object. + * + * @var ContainerInterface + */ + protected $container; + + public function setContainer(ContainerInterface $container = NULL) { + $this->container = $container; + } public function content(Request $request, $_content) { From f6bf963097ed4acfd28eee6a97ce9ab62f435960 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 2 Sep 2012 12:46:29 -0500 Subject: [PATCH 43/72] Move router rebuilding into an object so we can break it up more easily. --- core/includes/common.inc | 18 +--------- core/lib/Drupal/Core/CoreBundle.php | 2 ++ core/lib/Drupal/Core/Routing/RouteBuilder.php | 35 +++++++++++++++++++ 3 files changed, 38 insertions(+), 17 deletions(-) create mode 100644 core/lib/Drupal/Core/Routing/RouteBuilder.php diff --git a/core/includes/common.inc b/core/includes/common.inc index e27f4d9f0cd..5427cb32ca5 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -6830,7 +6830,7 @@ function drupal_flush_all_caches() { // Rebuild the menu router based on all rebuilt data. // Important: This rebuild must happen last, so the menu router is guaranteed // to be based on up to date information. - router_rebuild(); + drupal_container()->get('router.builder')->rebuild(); menu_router_rebuild(); // Re-initialize the maintenance theme, if the current request attempted to @@ -6842,22 +6842,6 @@ function drupal_flush_all_caches() { } } -function router_rebuild() { - // We need to manually call each module so that we can know which module - // a given item came from. - - $dumper = drupal_container()->get('router.dumper', Container::NULL_ON_INVALID_REFERENCE); - - if ($dumper) { - foreach (module_implements('route_info') as $module) { - $routes = call_user_func($module . '_route_info'); - drupal_alter('router_info', $routes, $module); - $dumper->addRoutes($routes); - $dumper->dump(array('route_set' => $module)); - } - } -} - /** * Changes the dummy query string added to all CSS and JavaScript files. * diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index fceaf76ef8b..c1abfe55ca6 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -58,6 +58,8 @@ class CoreBundle extends Bundle $container->register('router.dumper', '\Drupal\Core\Routing\MatcherDumper') ->addArgument(new Reference('database')); + $container->register('router.builder', 'Drupal\Core\Routing\RouteBuilder') + ->addArgument(new Reference('router.dumper')); // @todo Replace below lines with the commented out block below it when it's // performant to do so: http://drupal.org/node/1706064. diff --git a/core/lib/Drupal/Core/Routing/RouteBuilder.php b/core/lib/Drupal/Core/Routing/RouteBuilder.php new file mode 100644 index 00000000000..a1d8dafd993 --- /dev/null +++ b/core/lib/Drupal/Core/Routing/RouteBuilder.php @@ -0,0 +1,35 @@ +dumper = $dumper; + } + + public function rebuild() { + // We need to manually call each module so that we can know which module + // a given item came from. + + foreach (module_implements('route_info') as $module) { + $routes = call_user_func($module . '_route_info'); + drupal_alter('router_info', $routes, $module); + $this->dumper->addRoutes($routes); + $this->dumper->dump(array('route_set' => $module)); + } + } + +} From 867e7707f5b5e68a56de654bfc0eb140d45ca0e2 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 2 Sep 2012 13:59:53 -0500 Subject: [PATCH 44/72] Pass the _content callback as a proper controller through HttpKernel::forward(). --- .../Core/EventSubscriber/ViewSubscriber.php | 21 ++++++++++++++----- core/lib/Drupal/Core/HtmlPageController.php | 15 ++++++++++--- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php index 637d48d5f60..cec3c49935e 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php @@ -10,6 +10,7 @@ namespace Drupal\Core\EventSubscriber; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -44,15 +45,25 @@ class ViewSubscriber implements EventSubscriberInterface { */ public function onView(GetResponseForControllerResultEvent $event) { - $request = $event->getRequest(); + // For a master request, we process the result and wrap it as needed. + // For a subrequest, all we want is the string value. We assume that + // is just an HTML string from a controller, so wrap that into a response + // object. The subrequest's response will get dissected and placed into + // the larger page as needed. + if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) { + $request = $event->getRequest(); - $method = 'on' . $this->negotiation->getContentType($request); + $method = 'on' . $this->negotiation->getContentType($request); - if (method_exists($this, $method)) { - $event->setResponse($this->$method($event)); + if (method_exists($this, $method)) { + $event->setResponse($this->$method($event)); + } + else { + $event->setResponse(new Response('Unsupported Media Type', 415)); + } } else { - $event->setResponse(new Response('Unsupported Media Type', 415)); + $event->setResponse(new Response($event->getControllerResult())); } } diff --git a/core/lib/Drupal/Core/HtmlPageController.php b/core/lib/Drupal/Core/HtmlPageController.php index bfc6d578b62..b087b82c67d 100644 --- a/core/lib/Drupal/Core/HtmlPageController.php +++ b/core/lib/Drupal/Core/HtmlPageController.php @@ -22,11 +22,20 @@ class HtmlPageController implements ContainerAwareInterface { public function content(Request $request, $_content) { - $content_controller = $this->getContentController($_content); + // @todo When we have a Generator, we can replace the forward() call with + // a render() call, which would handle ESI and hInclude as well. That will + // require an _internal route. For examples, see: + // https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing/internal.xml + // https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php + $attributes = $request->attributes; + $controller = $attributes->get('_content'); + $attributes->remove('system_path'); + $attributes->remove('_content'); + $response = $this->container->get('http_kernel')->forward($controller, $attributes->all(), $request->query->all()); - $page_callback_result = call_user_func_array($content_controller, array()); + $page_content = $response->getContent(); - return new Response(drupal_render_page($page_callback_result)); + return new Response(drupal_render_page($page_content)); } protected function getContentController($controller) { From 820a633ccead27eb75d91c783cadc6ac17b796b9 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Thu, 13 Sep 2012 01:31:05 -0500 Subject: [PATCH 45/72] Documentation fixes. --- .../RouteProcessorSubscriber.php | 28 +-------- core/lib/Drupal/Core/HtmlPageController.php | 63 +++++++------------ .../system/Tests/Routing/RouterTest.php | 6 +- 3 files changed, 31 insertions(+), 66 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php index 816366657fa..cba2fce69fa 100644 --- a/core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/RouteProcessorSubscriber.php @@ -2,7 +2,7 @@ /** * @file - * Definition of Drupal\Core\EventSubscriber\RouterListener. + * Definition of Drupal\Core\EventSubscriber\RouteProcessorSubscriber. */ namespace Drupal\Core\EventSubscriber; @@ -18,33 +18,10 @@ use Symfony\Component\Routing\Matcher\UrlMatcherInterface; use Symfony\Component\Routing\Exception\ResourceNotFoundException; /** - * Drupal-specific Router listener. - * - * This is the bridge from the kernel to the UrlMatcher. + * Listener to process request controller information. */ class RouteProcessorSubscriber implements EventSubscriberInterface { - /** - * The Matcher object for this listener. - * - * This property is private in the base class, so we have to hack around it. - * - * @var Symfony\Component\Router\Matcher\UrlMatcherInterface - */ - protected $urlMatcher; - - /** - * The Logging object for this listener. - * - * This property is private in the base class, so we have to hack around it. - * - * @var Symfony\Component\HttpKernel\Log\LoggerInterface - */ - protected $logger; - - public function __construct() { - } - /** * Sets a default controller for a route if one was not specified. */ @@ -54,7 +31,6 @@ class RouteProcessorSubscriber implements EventSubscriberInterface { if (!$request->attributes->has('_controller') && $request->attributes->has('_content')) { $request->attributes->set('_controller', '\Drupal\Core\HtmlPageController::content'); } - } /** diff --git a/core/lib/Drupal/Core/HtmlPageController.php b/core/lib/Drupal/Core/HtmlPageController.php index b087b82c67d..863f33f0e48 100644 --- a/core/lib/Drupal/Core/HtmlPageController.php +++ b/core/lib/Drupal/Core/HtmlPageController.php @@ -1,5 +1,10 @@ container = $container; } + /** + * Controller method for generic HTML pages. + * + * @param Request $request + * The request object. + * @param type $_content + * The body content callable that contains the body region of this page. + * @return \Symfony\Component\HttpFoundation\Response + */ public function content(Request $request, $_content) { // @todo When we have a Generator, we can replace the forward() call with @@ -28,7 +51,7 @@ class HtmlPageController implements ContainerAwareInterface { // https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing/internal.xml // https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php $attributes = $request->attributes; - $controller = $attributes->get('_content'); + $controller = $_content; $attributes->remove('system_path'); $attributes->remove('_content'); $response = $this->container->get('http_kernel')->forward($controller, $attributes->all(), $request->query->all()); @@ -37,42 +60,4 @@ class HtmlPageController implements ContainerAwareInterface { return new Response(drupal_render_page($page_content)); } - - protected function getContentController($controller) { - if (is_array($controller) || (is_object($controller) && method_exists($controller, '__invoke'))) { - return $controller; - } - - if (FALSE === strpos($controller, ':')) { - if (method_exists($controller, '__invoke')) { - return new $controller; - } elseif (function_exists($controller)) { - return $controller; - } - } - - list($controller, $method) = $this->createController($controller); - - if (!method_exists($controller, $method)) { - throw new \InvalidArgumentException(sprintf('Method "%s::%s" does not exist.', get_class($controller), $method)); - } - - return array($controller, $method); - } - - protected function createController($controller) { - if (false === strpos($controller, '::')) { - throw new \InvalidArgumentException(sprintf('Unable to find controller "%s".', $controller)); - } - - list($class, $method) = explode('::', $controller, 2); - - if (!class_exists($class)) { - throw new \InvalidArgumentException(sprintf('Class "%s" does not exist.', $class)); - } - - return array(new $class(), $method); - } - - } 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 ac7d5b7239b..58a90508d7c 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php @@ -1,8 +1,12 @@ Date: Sun, 2 Sep 2012 17:43:13 -0500 Subject: [PATCH 46/72] Port the regex path matching from Symfony to our CompiledRoute class so that we can match placeholder-using paths. --- .../lib/Drupal/Core/Routing/CompiledRoute.php | 37 ++++-- .../Core/Routing/FirstEntryFinalMatcher.php | 9 +- .../lib/Drupal/Core/Routing/RouteCompiler.php | 122 +++++++++++++++++- .../system/Tests/Routing/RouterTest.php | 10 ++ .../Drupal/router_test/TestControllers.php | 5 +- .../modules/router_test/router_test.module | 5 + 6 files changed, 172 insertions(+), 16 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/CompiledRoute.php b/core/lib/Drupal/Core/Routing/CompiledRoute.php index d8e93f89c71..1600e766687 100644 --- a/core/lib/Drupal/Core/Routing/CompiledRoute.php +++ b/core/lib/Drupal/Core/Routing/CompiledRoute.php @@ -44,22 +44,25 @@ class CompiledRoute { /** - * Constructs a new CompiledRoute object. - * - * @param Route $route - * A original Route instance. - * @param int $fit - * The fitness of the route. - * @param string $fit - * The pattern outline for this route. - * @param int $num_parts - * The number of parts in the path. - */ - public function __construct(Route $route, $fit, $pattern_outline, $num_parts) { + * Constructs a new CompiledRoute object. + * + * @param Route $route + * A original Route instance. + * @param int $fit + * The fitness of the route. + * @param string $fit + * The pattern outline for this route. + * @param int $num_parts + * The number of parts in the path. + * @param string $regex + * The regular expression to match placeholders out of this path. + */ + public function __construct(Route $route, $fit, $pattern_outline, $num_parts, $regex) { $this->route = $route; $this->fit = $fit; $this->patternOutline = $pattern_outline; $this->numParts = $num_parts; + $this->regex = $regex; } /** @@ -100,6 +103,16 @@ class CompiledRoute { return $this->patternOutline; } + /** + * Returns the placeholder regex. + * + * @return string + * The regex to locate placeholders in this pattern. + */ + public function getRegex() { + return $this->regex; + } + /** * Returns the Route instance. * diff --git a/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php b/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php index 81880cfaf3f..f1262dffaca 100644 --- a/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php +++ b/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php @@ -39,7 +39,14 @@ class FirstEntryFinalMatcher implements FinalMatcherInterface { public function matchRequest(Request $request) { // Return whatever the first route in the collection is. foreach ($this->routes as $name => $route) { - return array_merge($this->mergeDefaults(array(), $route->getDefaults()), array('_route' => $name)); + $path = '/' . $request->attributes->get('system_path'); + + $route->setOption('compiler_class', '\Drupal\Core\Routing\RouteCompiler'); + $compiled = $route->compile(); + + preg_match($compiled->getRegex(), $path, $matches); + + return array_merge($this->mergeDefaults($matches, $route->getDefaults()), array('_route' => $name)); } } diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index a34e5c8d4e3..d3f9b4c573e 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -15,6 +15,8 @@ class RouteCompiler implements RouteCompilerInterface { */ const MAX_PARTS = 9; + const REGEX_DELIMITER = '#'; + /** * Compiles the current route instance. * @@ -26,17 +28,133 @@ class RouteCompiler implements RouteCompilerInterface { */ public function compile(Route $route) { - $fit = $this->getFit($route->getPattern()); $pattern_outline = $this->getPatternOutline($route->getPattern()); $num_parts = count(explode('/', trim($pattern_outline, '/'))); - return new CompiledRoute($route, $fit, $pattern_outline, $num_parts); + $regex = $this->getRegex($route, $route->getPattern()); + return new CompiledRoute($route, $fit, $pattern_outline, $num_parts, $regex); } + /** + * Generates a regular expression that will match this pattern. + * + * This regex can be used in preg_match() to extract values inside {}. + * + * This algorithm was lifted directly from Symfony's RouteCompiler class. + * It is not factored out nicely there, so we cannot simply subclass it. + * @todo Refactor Symfony's RouteCompiler so that it's useful to subclass. + * + * @param Route $route + * The route object. + * @param string $pattern + * The pattern for which we want a matching regex. + * @return type + * @throws \LogicException + */ + public function getRegex(Route $route, $pattern) { + $len = strlen($pattern); + $tokens = array(); + $variables = array(); + $pos = 0; + preg_match_all('#.\{(\w+)\}#', $pattern, $matches, PREG_OFFSET_CAPTURE | PREG_SET_ORDER); + foreach ($matches as $match) { + if ($text = substr($pattern, $pos, $match[0][1] - $pos)) { + $tokens[] = array('text', $text); + } + + $pos = $match[0][1] + strlen($match[0][0]); + $var = $match[1][0]; + + if ($req = $route->getRequirement($var)) { + $regexp = $req; + } + else { + // Use the character preceding the variable as a separator + $separators = array($match[0][0][0]); + + if ($pos !== $len) { + // Use the character following the variable as the separator when available + $separators[] = $pattern[$pos]; + } + $regexp = sprintf('[^%s]+', preg_quote(implode('', array_unique($separators)), self::REGEX_DELIMITER)); + } + + $tokens[] = array('variable', $match[0][0][0], $regexp, $var); + + if (in_array($var, $variables)) { + throw new \LogicException(sprintf('Route pattern "%s" cannot reference variable name "%s" more than once.', $route->getPattern(), $var)); + } + + $variables[] = $var; + } + + if ($pos < $len) { + $tokens[] = array('text', substr($pattern, $pos)); + } + + // find the first optional token + $firstOptional = INF; + for ($i = count($tokens) - 1; $i >= 0; $i--) { + $token = $tokens[$i]; + if ('variable' === $token[0] && $route->hasDefault($token[3])) { + $firstOptional = $i; + } else { + break; + } + } + + // compute the matching regexp + $regexp = ''; + for ($i = 0, $nbToken = count($tokens); $i < $nbToken; $i++) { + $regexp .= $this->computeRegexp($tokens, $i, $firstOptional); + } + + return self::REGEX_DELIMITER.'^'.$regexp.'$'.self::REGEX_DELIMITER.'s'; + } + + /** + * Computes the regexp used to match a specific token. It can be static text or a subpattern. + * + * @param array $tokens The route tokens + * @param integer $index The index of the current token + * @param integer $firstOptional The index of the first optional token + * + * @return string The regexp pattern for a single token + */ + private function computeRegexp(array $tokens, $index, $firstOptional) { + $token = $tokens[$index]; + if ('text' === $token[0]) { + // Text tokens + return preg_quote($token[1], self::REGEX_DELIMITER); + } else { + // Variable tokens + if (0 === $index && 0 === $firstOptional) { + // When the only token is an optional variable token, the separator is required + return sprintf('%s(?<%s>%s)?', preg_quote($token[1], self::REGEX_DELIMITER), $token[3], $token[2]); + } else { + $regexp = sprintf('%s(?<%s>%s)', preg_quote($token[1], self::REGEX_DELIMITER), $token[3], $token[2]); + if ($index >= $firstOptional) { + // Enclose each optional token in a subpattern to make it optional. + // "?:" means it is non-capturing, i.e. the portion of the subject string that + // matched the optional subpattern is not passed back. + $regexp = "(?:$regexp"; + $nbTokens = count($tokens); + if ($nbTokens - 1 == $index) { + // Close the optional subpatterns + $regexp .= str_repeat(")?", $nbTokens - $firstOptional - (0 === $firstOptional ? 1 : 0)); + } + } + + return $regexp; + } + } + } + + /** * Returns the pattern outline. * 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 58a90508d7c..04817088404 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php @@ -46,4 +46,14 @@ class RouterTest extends WebTestBase { $this->assertRaw('', 'Page markup was found.'); } + /** + * Confirms that placeholders in paths work correctly. + */ + public function testDefaultControllerPlaceholders() { + $value = $this->randomName(); + $this->drupalGet('router_test/test3/' . $value); + $this->assertRaw($value, 'The correct string was returned because the route was successful.'); + $this->assertRaw('', 'Page markup was found.'); + } + } diff --git a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php index e6c5c9eb2ca..1b0f23c8584 100644 --- a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php +++ b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php @@ -22,5 +22,8 @@ class TestControllers { return "test2"; } -} + public function test3($value) { + return $value; + } +} diff --git a/core/modules/system/tests/modules/router_test/router_test.module b/core/modules/system/tests/modules/router_test/router_test.module index 21ea2b957be..529a276c32b 100644 --- a/core/modules/system/tests/modules/router_test/router_test.module +++ b/core/modules/system/tests/modules/router_test/router_test.module @@ -19,5 +19,10 @@ function router_test_route_info() { )); $collection->add('router_test_2', $route); + $route = new Route('router_test/test3/{value}', array( + '_content' => '\Drupal\router_test\TestControllers::test3' + )); + $collection->add('router_test_3', $route); + return $collection; } From 1d8036319a82d377468e705399d911787918233b Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Mon, 3 Sep 2012 00:16:48 -0500 Subject: [PATCH 47/72] Add unit tests for FirstEntryFinalMatcher. --- .../Routing/FirstEntryFinalMatcherTest.php | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php new file mode 100644 index 00000000000..0f174770b71 --- /dev/null +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php @@ -0,0 +1,116 @@ + 'FirstEntryFinalMatcher tests', + 'description' => 'Confirm that the FirstEntryFinalMatcher is working properly.', + 'group' => 'Routing', + ); + } + + function __construct($test_id = NULL) { + parent::__construct($test_id); + + $this->fixtures = new RoutingFixtures(); + } + public function setUp() { + parent::setUp(); + } + + /** + * Confirms the final matcher returns correct attributes for static paths. + */ + public function testFinalMatcherStatic() { + + $collection = new RouteCollection(); + $collection->add('route_a', new Route('/path/one', array( + '_controller' => 'foo', + ))); + + $request = Request::create('/path/one', 'GET'); + + $matcher = new FirstEntryFinalMatcher(); + $matcher->setCollection($collection); + $attributes = $matcher->matchRequest($request); + + $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); + $this->assertEqual($attributes['_controller'], 'foo', 'The correct controller was found.'); + } + + /** + * Confirms the final matcher returns correct attributes for pattern paths. + */ + public function testFinalMatcherPattern() { + + $collection = new RouteCollection(); + $collection->add('route_a', new Route('/path/one/{value}', array( + '_controller' => 'foo', + ))); + + $request = Request::create('/path/one/narf', 'GET'); + $request->attributes->set('system_path', 'path/one/narf'); + + $matcher = new FirstEntryFinalMatcher(); + $matcher->setCollection($collection); + $attributes = $matcher->matchRequest($request); + + $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); + $this->assertEqual($attributes['_controller'], 'foo', 'The correct controller was found.'); + $this->assertEqual($attributes['value'], 'narf', 'Required placeholder value found.'); + } + + /** + * Confirms the final matcher returns correct attributes with default values. + */ + public function testFinalMatcherPatternDefalts() { + + $collection = new RouteCollection(); + $collection->add('route_a', new Route('/path/one/{value}', array( + '_controller' => 'foo', + 'value' => 'poink' + ))); + + $request = Request::create('/path/one', 'GET'); + $request->attributes->set('system_path', 'path/one'); + + $matcher = new FirstEntryFinalMatcher(); + $matcher->setCollection($collection); + $attributes = $matcher->matchRequest($request); + + $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); + $this->assertEqual($attributes['_controller'], 'foo', 'The correct controller was found.'); + $this->assertEqual($attributes['value'], 'poink', 'Optional placeholder value used default.'); + } +} + From 80bc8856f59d8d5d34be9a5cd772a4641ccc1efd Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Mon, 3 Sep 2012 00:33:37 -0500 Subject: [PATCH 48/72] Remove long-dead code. --- .../lib/Drupal/Core/Routing/MatcherDumper.php | 157 ------------------ 1 file changed, 157 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index 2d454f45463..5237fb152e6 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -138,163 +138,6 @@ class MatcherDumper implements MatcherDumperInterface { return $this->routes; } - protected function compileRoutes(RouteCollection $routes, $route_set) { - - // First pass: separate callbacks from paths, making paths ready for - // matching. Calculate fitness, and fill some default values. - $menu = array(); - $masks = array(); - foreach ($routes as $name => $item) { - $path = $item->getPattern(); - $move = FALSE; - - $parts = explode('/', $path, static::MAX_PARTS); - $number_parts = count($parts); - // We store the highest index of parts here to save some work in the fit - // calculation loop. - $slashes = $number_parts - 1; - - $num_placeholders = count(array_filter($parts, function($value) { - return strpos($value, '{') !== FALSE; - })); - - $fit = $this->getFit($path); - - if ($fit) { - $move = TRUE; - } - else { - // If there is no placeholder, it fits maximally. - $fit = (1 << $number_parts) - 1; - } - - $masks[$fit] = 1; - $item += array( - 'title' => '', - 'weight' => 0, - 'type' => MENU_NORMAL_ITEM, - 'module' => '', - '_number_parts' => $number_parts, - '_parts' => $parts, - '_fit' => $fit, - ); - - if ($move) { - $new_path = implode('/', $item['_parts']); - $menu[$new_path] = $item; - $sort[$new_path] = $number_parts; - } - else { - $menu[$path] = $item; - $sort[$path] = $number_parts; - } - } - - // Sort the route list. - array_multisort($sort, SORT_NUMERIC, $menu); - // Apply inheritance rules. - foreach ($menu as $path => $v) { - $item = &$menu[$path]; - - for ($i = $item['_number_parts'] - 1; $i; $i--) { - $parent_path = implode('/', array_slice($item['_parts'], 0, $i)); - if (isset($menu[$parent_path])) { - - $parent = &$menu[$parent_path]; - - // If an access callback is not found for a default local task we use - // the callback from the parent, since we expect them to be identical. - // In all other cases, the access parameters must be specified. - if (($item['type'] == MENU_DEFAULT_LOCAL_TASK) && !isset($item['access callback']) && isset($parent['access callback'])) { - $item['access callback'] = $parent['access callback']; - if (!isset($item['access arguments']) && isset($parent['access arguments'])) { - $item['access arguments'] = $parent['access arguments']; - } - } - - // Same for theme callbacks. - if (!isset($item['theme callback']) && isset($parent['theme callback'])) { - $item['theme callback'] = $parent['theme callback']; - if (!isset($item['theme arguments']) && isset($parent['theme arguments'])) { - $item['theme arguments'] = $parent['theme arguments']; - } - } - } - } - if (!isset($item['access callback']) && isset($item['access arguments'])) { - // Default callback. - $item['access callback'] = 'user_access'; - } - if (!isset($item['access callback']) || empty($item['page callback'])) { - $item['access callback'] = 0; - } - if (is_bool($item['access callback'])) { - $item['access callback'] = intval($item['access callback']); - } - - $item += array( - 'access arguments' => array(), - 'access callback' => '', - 'page arguments' => array(), - 'page callback' => '', - 'delivery callback' => '', - 'title arguments' => array(), - 'title callback' => 't', - 'theme arguments' => array(), - 'theme callback' => '', - 'description' => '', - 'position' => '', - 'context' => 0, - 'tab_parent' => '', - 'tab_root' => $path, - 'path' => $path, - 'file' => '', - 'file path' => '', - 'include file' => '', - ); - - // Calculate out the file to be included for each callback, if any. - if ($item['file']) { - $file_path = $item['file path'] ? $item['file path'] : drupal_get_path('module', $item['module']); - $item['include file'] = $file_path . '/' . $item['file']; - } - } - - // Sort the masks so they are in order of descending fit. - $masks = array_keys($masks); - rsort($masks); - - return array($menu, $masks); - - - // The old menu_router record structure, copied here for easy referencing. - array( - 'path' => $item['path'], - 'load_functions' => $item['load_functions'], - 'to_arg_functions' => $item['to_arg_functions'], - 'access_callback' => $item['access callback'], - 'access_arguments' => serialize($item['access arguments']), - 'page_callback' => $item['page callback'], - 'page_arguments' => serialize($item['page arguments']), - 'delivery_callback' => $item['delivery callback'], - 'fit' => $item['_fit'], - 'number_parts' => $item['_number_parts'], - 'context' => $item['context'], - 'tab_parent' => $item['tab_parent'], - 'tab_root' => $item['tab_root'], - 'title' => $item['title'], - 'title_callback' => $item['title callback'], - 'title_arguments' => ($item['title arguments'] ? serialize($item['title arguments']) : ''), - 'theme_callback' => $item['theme callback'], - 'theme_arguments' => serialize($item['theme arguments']), - 'type' => $item['type'], - 'description' => $item['description'], - 'position' => $item['position'], - 'weight' => $item['weight'], - 'include_file' => $item['include file'], - ); - } - /** * Determines the fitness of the provided path. * From a3deb0349d8563f274171f375dbcf80bf6fc5389 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sun, 9 Sep 2012 23:40:27 -0500 Subject: [PATCH 49/72] Properly render legacy subrequests. --- core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php index cec3c49935e..e8bfc8ed729 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php @@ -63,7 +63,7 @@ class ViewSubscriber implements EventSubscriberInterface { } } else { - $event->setResponse(new Response($event->getControllerResult())); + $event->setResponse($this->onHtml($event)); } } From 8306c5a32aa5a5bec80572f75875c10112f50080 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Mon, 3 Sep 2012 22:48:31 -0500 Subject: [PATCH 50/72] Non-working tests to demonstrate that default values don't work yet. --- .../system/Tests/Routing/PathMatcherTest.php | 40 +++++++++++++++++++ .../system/Tests/Routing/RouterTest.php | 11 ++++- .../Drupal/router_test/TestControllers.php | 4 ++ .../modules/router_test/router_test.module | 6 +++ 4 files changed, 60 insertions(+), 1 deletion(-) diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php index 055a1e76c34..302122aa199 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -127,6 +127,46 @@ class PathMatcherTest extends UnitTestBase { $this->assertNotNull($routes->get('route_b'), 'The second matching route was not found.'); } + /** + * Confirms that we can find routes whose pattern would match the request. + */ + function testOutlinePathMatchDefaults() { + $connection = Database::getConnection(); + $matcher = new PathMatcher($connection, 'test_routes'); + + $this->fixtures->createTables($connection); + + $collection = new RouteCollection(); + $collection->add('poink', new Route('/some/path/{value}', array( + 'value' => 'poink', + ))); + + $dumper = new MatcherDumper($connection, 'test_routes'); + $dumper->addRoutes($collection); + $dumper->dump(); + + $path = '/some/path'; + + $request = Request::create($path, 'GET'); + + try { + $routes = $matcher->matchRequestPartial($request); + + // All of the matching paths have the correct pattern. + foreach ($routes as $route) { + $compiled = $route->compile(); + debug($compiled->getPatternOutline()); + $this->assertEqual($route->compile()->getPatternOutline(), '/path/path/%', 'Found path has correct pattern'); + } + + $this->assertEqual(count($routes->all()), 1, 'The correct number of routes was found.'); + $this->assertNotNull($routes->get('poink'), 'The first matching route was found.'); + } + catch (ResourceNotFoundException $e) { + $this->fail('No matching route found with default argument value.'); + } + } + /** * Confirm that an exception is thrown when no matching path is found. */ 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 04817088404..f5b4a1893f4 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php @@ -49,11 +49,20 @@ class RouterTest extends WebTestBase { /** * Confirms that placeholders in paths work correctly. */ - public function testDefaultControllerPlaceholders() { + public function testControllerPlaceholders() { $value = $this->randomName(); $this->drupalGet('router_test/test3/' . $value); $this->assertRaw($value, 'The correct string was returned because the route was successful.'); $this->assertRaw('', 'Page markup was found.'); } + /** + * Confirms that default placeholders in paths work correctly. + */ + public function testControllerPlaceholdersDefaultValues() { + $this->drupalGet('router_test/test4'); + $this->assertRaw('narf', 'The correct string was returned because the route was successful.'); + $this->assertRaw('', 'Page markup was found.'); + } + } diff --git a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php index 1b0f23c8584..fa92fd89b3e 100644 --- a/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php +++ b/core/modules/system/tests/modules/router_test/lib/Drupal/router_test/TestControllers.php @@ -26,4 +26,8 @@ class TestControllers { return $value; } + public function test4($value) { + return $value; + } + } diff --git a/core/modules/system/tests/modules/router_test/router_test.module b/core/modules/system/tests/modules/router_test/router_test.module index 529a276c32b..4da939d7059 100644 --- a/core/modules/system/tests/modules/router_test/router_test.module +++ b/core/modules/system/tests/modules/router_test/router_test.module @@ -24,5 +24,11 @@ function router_test_route_info() { )); $collection->add('router_test_3', $route); + $route = new Route('router_test/test4/{value}', array( + '_content' => '\Drupal\router_test\TestControllers::test4', + 'value' => 'narf', + )); + $collection->add('router_test_4', $route); + return $collection; } From fac9b6ed0e62e609c722100c6d788c119e84c4a6 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 12 Sep 2012 21:35:22 -0500 Subject: [PATCH 51/72] Make use of the compiled regex when filtering routes by path, to account for default values and regex filters on placeholders. --- core/lib/Drupal/Core/Routing/PathMatcher.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index a5f007c804b..a1ef5f71f50 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -57,7 +57,10 @@ class PathMatcher implements InitialMatcherInterface { $collection = new RouteCollection(); foreach ($routes as $name => $route) { - $collection->add($name, unserialize($route)); + $route = unserialize($route); + if (preg_match($route->compile()->getRegex(), $path, $matches)) { + $collection->add($name, $route); + } } if (!count($collection->all())) { From ce54838752a3ebe3c52449a7927b9c2be175dd6f Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 12 Sep 2012 21:41:27 -0500 Subject: [PATCH 52/72] Fix handling of default values for placeholders. In order to make default placeholders work, we had to modify the fit and path outline routines to ignore them. That also necessitated switching back to the original outline/ancestors logic from Drupal 7, which with a very slight tweak to the masks and '/'-prefix on paths still works just as it should. --- core/lib/Drupal/Core/Routing/PathMatcher.php | 23 ++++-- .../lib/Drupal/Core/Routing/RouteCompiler.php | 40 +++++++-- .../system/Tests/Routing/PathMatcherTest.php | 82 ++++++++++++++++++- .../Drupal/system/Tests/Routing/RouteTest.php | 20 +++++ 4 files changed, 149 insertions(+), 16 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index a1ef5f71f50..b888446c598 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -79,20 +79,26 @@ class PathMatcher implements InitialMatcherInterface { * An array of outlines that could match the specified path parts. */ public function getCandidateOutlines(array $parts) { - $number_parts = count($parts); + $ancestors = array(); $length = $number_parts - 1; $end = (1 << $number_parts) - 1; - $candidates = array(); - - $start = pow($number_parts-1, 2); // The highest possible mask is a 1 bit for every part of the path. We will // check every value down from there to generate a possible outline. - $masks = range($end, $start); + $masks = range($end, pow($number_parts - 1, 2)); + // Only examine patterns that actually exist as router items (the masks). foreach ($masks as $i) { - $current = '/'; + if ($i > $end) { + // Only look at masks that are not longer than the path of interest. + continue; + } + elseif ($i < (1 << $length)) { + // We have exhausted the masks of a given length, so decrease the length. + --$length; + } + $current = ''; for ($j = $length; $j >= 0; $j--) { // Check the bit on the $j offset. if ($i & (1 << $j)) { @@ -108,10 +114,9 @@ class PathMatcher implements InitialMatcherInterface { $current .= '/'; } } - $candidates[] = $current; + $ancestors[] = '/' . $current; } - - return $candidates; + return $ancestors; } } diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index d3f9b4c573e..6af19e734e8 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -28,9 +28,11 @@ class RouteCompiler implements RouteCompilerInterface { */ public function compile(Route $route) { - $fit = $this->getFit($route->getPattern()); + $stripped_path = $this->getPathWithoutDefaults($route); - $pattern_outline = $this->getPatternOutline($route->getPattern()); + $fit = $this->getFit($stripped_path); + + $pattern_outline = $this->getPatternOutline($stripped_path); $num_parts = count(explode('/', trim($pattern_outline, '/'))); @@ -159,10 +161,10 @@ class RouteCompiler implements RouteCompilerInterface { * Returns the pattern outline. * * The pattern outline is the path pattern but normalized so that all - * placeholders are equal strings. + * placeholders are equal strings and default values are removed. * * @param string $path - * The path pattern to normalize to an outline. + * The path for which we want the normalized outline. * * @return string * The path pattern outline. @@ -181,7 +183,6 @@ class RouteCompiler implements RouteCompilerInterface { * The fitness of the path, as an integer. */ public function getFit($path) { - $parts = explode('/', trim($path, '/'), static::MAX_PARTS); $number_parts = count($parts); // We store the highest index of parts here to save some work in the fit @@ -197,5 +198,34 @@ class RouteCompiler implements RouteCompilerInterface { return $fit; } + + /** + * Returns the path of the route, without placeholders with a default value. + * + * When computing the path outline and fit, we want to skip default-value + * placeholders. If we didn't, the path would never match. Note that this + * only works for placeholders at the end of the path. Infix placeholders + * with default values don't make sense anyway, so that should not be a + * problem. + * + * @param Route $route + * + * @return string + * The path string, stripped of placeholders that have default values. + */ + protected function getPathWithoutDefaults(Route $route) { + $path = $route->getPattern(); + $defaults = $route->getDefaults(); + + // Remove placeholders with default values from the outline, so that they + // will still match. + $remove = array_map(function($a) { + return '/{' . $a . '}'; + }, array_keys($defaults)); + $path = str_replace($remove, '', $path); + + return $path; + } + } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php index 302122aa199..7cd39751b8b 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -155,8 +155,7 @@ class PathMatcherTest extends UnitTestBase { // All of the matching paths have the correct pattern. foreach ($routes as $route) { $compiled = $route->compile(); - debug($compiled->getPatternOutline()); - $this->assertEqual($route->compile()->getPatternOutline(), '/path/path/%', 'Found path has correct pattern'); + $this->assertEqual($route->compile()->getPatternOutline(), '/some/path', 'Found path has correct pattern'); } $this->assertEqual(count($routes->all()), 1, 'The correct number of routes was found.'); @@ -167,6 +166,85 @@ class PathMatcherTest extends UnitTestBase { } } + /** + * Confirms that we can find routes whose pattern would match the request. + */ + function testOutlinePathMatchDefaultsCollision() { + $connection = Database::getConnection(); + $matcher = new PathMatcher($connection, 'test_routes'); + + $this->fixtures->createTables($connection); + + $collection = new RouteCollection(); + $collection->add('poink', new Route('/some/path/{value}', array( + 'value' => 'poink', + ))); + $collection->add('narf', new Route('/some/path/here')); + + $dumper = new MatcherDumper($connection, 'test_routes'); + $dumper->addRoutes($collection); + $dumper->dump(); + + $path = '/some/path'; + + $request = Request::create($path, 'GET'); + + try { + $routes = $matcher->matchRequestPartial($request); + + // All of the matching paths have the correct pattern. + foreach ($routes as $route) { + $compiled = $route->compile(); + $this->assertEqual($route->compile()->getPatternOutline(), '/some/path', 'Found path has correct pattern'); + } + + $this->assertEqual(count($routes->all()), 1, 'The correct number of routes was found.'); + $this->assertNotNull($routes->get('poink'), 'The first matching route was found.'); + } + catch (ResourceNotFoundException $e) { + $this->fail('No matching route found with default argument value.'); + } + } + + /** + * Confirms that we can find routes whose pattern would match the request. + */ + function testOutlinePathMatchDefaultsCollision2() { + $connection = Database::getConnection(); + $matcher = new PathMatcher($connection, 'test_routes'); + + $this->fixtures->createTables($connection); + + $collection = new RouteCollection(); + $collection->add('poink', new Route('/some/path/{value}', array( + 'value' => 'poink', + ))); + $collection->add('narf', new Route('/some/path/here')); + + $dumper = new MatcherDumper($connection, 'test_routes'); + $dumper->addRoutes($collection); + $dumper->dump(); + + $path = '/some/path/here'; + + $request = Request::create($path, 'GET'); + + try { + $routes = $matcher->matchRequestPartial($request); + + // All of the matching paths have the correct pattern. + foreach ($routes as $route) { + $this->assertEqual($route->compile()->getPatternOutline(), '/some/path/here', 'Found path has correct pattern'); + } + + $this->assertEqual(count($routes->all()), 1, 'The correct number of routes was found.'); + $this->assertNotNull($routes->get('narf'), 'The first matching route was found.'); + } + catch (ResourceNotFoundException $e) { + $this->fail('No matching route found with default argument value.'); + } + } + /** * Confirm that an exception is thrown when no matching path is found. */ diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php index 7ed212b8ea0..9e4ba634733 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php @@ -28,6 +28,9 @@ class RouteTest extends UnitTestBase { parent::setUp(); } + /** + * Confirms that a route compiles properly with the necessary data. + */ public function testCompilation() { $route = new Route('/test/{something}/more'); $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); @@ -38,4 +41,21 @@ class RouteTest extends UnitTestBase { $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', 'The pattern outline was correct.'); } + /** + * Confirms that a compiled route with default values has the correct outline. + */ + public function testCompilationDefaultValue() { + // Because "here" has a default value, it should not factor into the + // outline or the fitness. + $route = new Route('/test/{something}/more/{here}', array( + 'here' => 'there', + )); + $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler'); + $compiled = $route->compile(); + + $this->assertEqual($route, $compiled->getRoute(), 'Compiled route has the correct route object.'); + $this->assertEqual($compiled->getFit(), 5 /* That's 101 binary*/, 'The fit was correct.'); + $this->assertEqual($compiled->getPatternOutline(), '/test/%/more', 'The pattern outline was correct.'); + } + } From fa58bbefade5574a28e30cb6f11f615af0395da1 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 12 Sep 2012 22:21:07 -0500 Subject: [PATCH 53/72] Documentation fixes. --- core/lib/Drupal/Core/Routing/PathMatcher.php | 21 ++++++++++++++----- .../lib/Drupal/Core/Routing/RouteCompiler.php | 16 ++++++++++---- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index b888446c598..d66ed649873 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -1,5 +1,10 @@ connection = $connection; $this->tableName = $table; @@ -36,10 +47,10 @@ class PathMatcher implements InitialMatcherInterface { /** * Matches a request against multiple routes. * - * @param Request $request + * @param \Symfony\Component\HttpFoundation\Request $request * A Request object against which to match. * - * @return RouteCollection + * @return \Symfony\Component\Routing\RouteCollection * A RouteCollection of matched routes. */ public function matchRequestPartial(Request $request) { diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index 6af19e734e8..0564a45541b 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -1,12 +1,17 @@ Date: Wed, 12 Sep 2012 22:44:42 -0500 Subject: [PATCH 54/72] Allow a trailing / to still match as if it weren't there. --- core/lib/Drupal/Core/Routing/PathMatcher.php | 2 +- .../system/Tests/Routing/PathMatcherTest.php | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index d66ed649873..8930932d1e8 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -55,7 +55,7 @@ class PathMatcher implements InitialMatcherInterface { */ public function matchRequestPartial(Request $request) { - $path = $request->getPathInfo(); + $path = rtrim($request->getPathInfo(), '/'); $parts = array_slice(array_filter(explode('/', $path)), 0, MatcherDumper::MAX_PARTS); diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php index 7cd39751b8b..10cfc823ebb 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -127,6 +127,35 @@ class PathMatcherTest extends UnitTestBase { $this->assertNotNull($routes->get('route_b'), 'The second matching route was not found.'); } + /** + * Confirms that a trailing slash on the request doesn't result in a 404. + */ + function testOutlinePathMatchTrailingSlash() { + $connection = Database::getConnection(); + $matcher = new PathMatcher($connection, 'test_routes'); + + $this->fixtures->createTables($connection); + + $dumper = new MatcherDumper($connection, 'test_routes'); + $dumper->addRoutes($this->fixtures->complexRouteCollection()); + $dumper->dump(); + + $path = '/path/1/one/'; + + $request = Request::create($path, 'GET'); + + $routes = $matcher->matchRequestPartial($request); + + // All of the matching paths have the correct pattern. + foreach ($routes as $route) { + $this->assertEqual($route->compile()->getPatternOutline(), '/path/%/one', 'Found path has correct pattern'); + } + + $this->assertEqual(count($routes->all()), 2, 'The correct number of routes was found.'); + $this->assertNotNull($routes->get('route_a'), 'The first matching route was found.'); + $this->assertNotNull($routes->get('route_b'), 'The second matching route was not found.'); + } + /** * Confirms that we can find routes whose pattern would match the request. */ From 67fdbebdaf20d9d70eba85ab783baebb1d3889c8 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 12 Sep 2012 22:49:00 -0500 Subject: [PATCH 55/72] Don't render subrequest as a full page, just pass them through normal drupal_render(). --- core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php index e8bfc8ed729..93590830788 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php @@ -63,7 +63,8 @@ class ViewSubscriber implements EventSubscriberInterface { } } else { - $event->setResponse($this->onHtml($event)); + $page_callback_result = $event->getControllerResult(); + $event->setResponse(new Response(drupal_render($page_callback_result))); } } From e2c30c933bae065797947e885dabba494cc4bafc Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Thu, 13 Sep 2012 01:51:16 -0500 Subject: [PATCH 56/72] Adjust ViewSubscriber's subrequest handling again for render API. --- core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php index 93590830788..d96e5aeada4 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php @@ -63,8 +63,13 @@ class ViewSubscriber implements EventSubscriberInterface { } } else { - $page_callback_result = $event->getControllerResult(); - $event->setResponse(new Response(drupal_render($page_callback_result))); + $page_result = $event->getControllerResult(); + if (!is_array($page_result)) { + $page_result = array( + '#markup' => $page_result, + ); + } + $event->setResponse(new Response(drupal_render($page_result))); } } From 26b46f8d72deba0910d66997e36195648675659d Mon Sep 17 00:00:00 2001 From: Lin Clark Date: Wed, 19 Sep 2012 16:50:10 -0400 Subject: [PATCH 57/72] Switch user_page to redirect to prevent router test failures. --- core/modules/user/user.pages.inc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/modules/user/user.pages.inc b/core/modules/user/user.pages.inc index 9dd2cc99461..34f24ab0299 100644 --- a/core/modules/user/user.pages.inc +++ b/core/modules/user/user.pages.inc @@ -420,9 +420,7 @@ function user_page() { global $user; if ($user->uid) { // @todo: Cleaner sub request handling. - $request = drupal_container()->get('request'); - $subrequest = Request::create('/user/' . $user->uid, 'GET', $request->query->all(), $request->cookies->all(), array(), $request->server->all()); - return drupal_container()->get('http_kernel')->handle($subrequest, HttpKernelInterface::SUB_REQUEST); + drupal_goto('user/' . $user->uid); } else { return drupal_get_form('user_login'); From acd7dd9c16df092e738f2e6be05d6b99b10a193f Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Wed, 19 Sep 2012 22:34:47 -0500 Subject: [PATCH 58/72] Switch to RedirectResponse for the /user page. --- core/modules/user/user.pages.inc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/modules/user/user.pages.inc b/core/modules/user/user.pages.inc index 34f24ab0299..e94a97f9317 100644 --- a/core/modules/user/user.pages.inc +++ b/core/modules/user/user.pages.inc @@ -7,6 +7,7 @@ use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; use Symfony\Component\HttpKernel\HttpKernelInterface; @@ -419,8 +420,8 @@ function user_cancel_confirm($account, $timestamp = 0, $hashed_pass = '') { function user_page() { global $user; if ($user->uid) { - // @todo: Cleaner sub request handling. - drupal_goto('user/' . $user->uid); + $request = drupal_container()->get('request'); + return new RedirectResponse($request->getUriForPath('/user/' . $user->uid)); } else { return drupal_get_form('user_login'); From ea8d2911b7f351ee3054c6af6f12f83ff8acee5c Mon Sep 17 00:00:00 2001 From: Lin Clark Date: Thu, 20 Sep 2012 15:38:24 -0400 Subject: [PATCH 59/72] Fixed RouterTest fail that results from /user redirect. --- core/modules/system/lib/Drupal/system/Tests/Menu/RouterTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/modules/system/lib/Drupal/system/Tests/Menu/RouterTest.php b/core/modules/system/lib/Drupal/system/Tests/Menu/RouterTest.php index 90c0fc6839b..94832611476 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Menu/RouterTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Menu/RouterTest.php @@ -178,7 +178,7 @@ class RouterTest extends WebTestBase { $this->drupalGet('user/login'); // Check that we got to 'user'. - $this->assertTrue($this->url == url('user', array('absolute' => TRUE)), "Logged-in user redirected to user on accessing user/login"); + $this->assertTrue($this->url == url('user/' . $this->loggedInUser->uid, array('absolute' => TRUE)), "Logged-in user redirected to user on accessing user/login"); // user/register should redirect to user/UID/edit. $this->drupalGet('user/register'); From fdcd2d2a73c3777f08cf7c2924ddb8c6b3de2fac Mon Sep 17 00:00:00 2001 From: Lin Clark Date: Thu, 20 Sep 2012 15:39:58 -0400 Subject: [PATCH 60/72] Fixed old style subrequests by running through drupal_render_page. --- core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php index d96e5aeada4..cb3aeeaaa6e 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php @@ -69,7 +69,7 @@ class ViewSubscriber implements EventSubscriberInterface { '#markup' => $page_result, ); } - $event->setResponse(new Response(drupal_render($page_result))); + $event->setResponse(new Response(drupal_render_page($page_result))); } } From 6779c0794eb50386d5ddc5d2049bea9bd3b7a782 Mon Sep 17 00:00:00 2001 From: effulgentsia Date: Fri, 21 Sep 2012 16:57:05 -0400 Subject: [PATCH 61/72] Fix redirect user_page to include current language. --- core/modules/user/user.pages.inc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/modules/user/user.pages.inc b/core/modules/user/user.pages.inc index e94a97f9317..90d804e841a 100644 --- a/core/modules/user/user.pages.inc +++ b/core/modules/user/user.pages.inc @@ -420,8 +420,7 @@ function user_cancel_confirm($account, $timestamp = 0, $hashed_pass = '') { function user_page() { global $user; if ($user->uid) { - $request = drupal_container()->get('request'); - return new RedirectResponse($request->getUriForPath('/user/' . $user->uid)); + return new RedirectResponse(url('user/' . $user->uid, array('absolute' => TRUE))); } else { return drupal_get_form('user_login'); From 7019a0c1ca2fc7f7b8a32d2b0f16a0daebd598f9 Mon Sep 17 00:00:00 2001 From: effulgentsia Date: Fri, 21 Sep 2012 17:01:35 -0400 Subject: [PATCH 62/72] Ensure router.builder and other services are available during upgrade. This is mostly temporary measures to work around bootstrap circular dependencies until those are fixed. --- .../system/Tests/Upgrade/UpgradePathTestBase.php | 10 +++++++++- core/update.php | 12 ++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/core/modules/system/lib/Drupal/system/Tests/Upgrade/UpgradePathTestBase.php b/core/modules/system/lib/Drupal/system/Tests/Upgrade/UpgradePathTestBase.php index 86456e5008d..812bde7752a 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Upgrade/UpgradePathTestBase.php +++ b/core/modules/system/lib/Drupal/system/Tests/Upgrade/UpgradePathTestBase.php @@ -10,6 +10,7 @@ namespace Drupal\system\Tests\Upgrade; use Drupal\Core\Database\Database; use Drupal\simpletest\WebTestBase; use Exception; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; /** * Perform end-to-end tests of the upgrade path. @@ -246,7 +247,14 @@ abstract class UpgradePathTestBase extends WebTestBase { module_load_all(FALSE, TRUE); // Rebuild caches. - drupal_flush_all_caches(); + // @todo Remove the try/catch when UpgradePathTestBase::setup() is fixed to + // boot DrupalKernel (as WebTestBase::setup() does). + drupal_static_reset(); + try { + drupal_flush_all_caches(); + } + catch (InvalidArgumentException $e) { + } // Reload global $conf array and permissions. $this->refreshVariables(); diff --git a/core/update.php b/core/update.php index 52cf3d01a99..2273b408e4b 100644 --- a/core/update.php +++ b/core/update.php @@ -16,6 +16,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\DependencyInjection\Reference; // Change the directory to the Drupal root. chdir('..'); @@ -445,6 +446,17 @@ update_fix_d8_requirements(); drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL); drupal_maintenance_theme(); +// @todo Remove after converting update.php to use DrupalKernel. +$container = drupal_container(); +$container->register('database', 'Drupal\Core\Database\Connection') + ->setFactoryClass('Drupal\Core\Database\Database') + ->setFactoryMethod('getConnection') + ->addArgument('default'); +$container->register('router.dumper', '\Drupal\Core\Routing\MatcherDumper') + ->addArgument(new Reference('database')); +$container->register('router.builder', 'Drupal\Core\Routing\RouteBuilder') + ->addArgument(new Reference('router.dumper')); + // Turn error reporting back on. From now on, only fatal errors (which are // not passed through the error handler) will cause a message to be printed. ini_set('display_errors', TRUE); From 0703718bcd20077ae1a20334ddba4527fffc588c Mon Sep 17 00:00:00 2001 From: larowlan Date: Sat, 22 Sep 2012 12:24:46 -0500 Subject: [PATCH 63/72] Various documentation and whitespace fixes. --- core/includes/common.inc | 4 +- .../RouteProcessorSubscriber.php | 3 + core/lib/Drupal/Core/HtmlPageController.php | 1 + core/lib/Drupal/Core/Routing/ChainMatcher.php | 5 +- .../lib/Drupal/Core/Routing/CompiledRoute.php | 26 +++-- .../Core/Routing/FinalMatcherInterface.php | 2 +- .../Core/Routing/FirstEntryFinalMatcher.php | 11 +- .../lib/Drupal/Core/Routing/MatcherDumper.php | 25 +++-- .../lib/Drupal/Core/Routing/NestedMatcher.php | 17 +-- core/lib/Drupal/Core/Routing/PathMatcher.php | 3 +- core/lib/Drupal/Core/Routing/RouteBuilder.php | 9 ++ .../lib/Drupal/Core/Routing/RouteCompiler.php | 104 ++++++++++-------- .../system/Tests/Routing/ChainMatcherTest.php | 1 + .../Routing/FirstEntryFinalMatcherTest.php | 4 - .../Tests/Routing/HttpMethodMatcherTest.php | 6 +- .../system/Tests/Routing/MockMatcher.php | 4 +- .../system/Tests/Routing/MockPathMatcher.php | 15 ++- .../Tests/Routing/NestedMatcherTest.php | 4 - .../system/Tests/Routing/PathMatcherTest.php | 6 +- .../Drupal/system/Tests/Routing/RouteTest.php | 4 +- .../system/Tests/Routing/RoutingFixtures.php | 18 +++ 21 files changed, 163 insertions(+), 109 deletions(-) diff --git a/core/includes/common.inc b/core/includes/common.inc index 5427cb32ca5..6879def8781 100644 --- a/core/includes/common.inc +++ b/core/includes/common.inc @@ -1,10 +1,10 @@ getRequest(); diff --git a/core/lib/Drupal/Core/HtmlPageController.php b/core/lib/Drupal/Core/HtmlPageController.php index 863f33f0e48..27a9e4e694b 100644 --- a/core/lib/Drupal/Core/HtmlPageController.php +++ b/core/lib/Drupal/Core/HtmlPageController.php @@ -41,6 +41,7 @@ class HtmlPageController implements ContainerAwareInterface { * The request object. * @param type $_content * The body content callable that contains the body region of this page. + * * @return \Symfony\Component\HttpFoundation\Response */ public function content(Request $request, $_content) { diff --git a/core/lib/Drupal/Core/Routing/ChainMatcher.php b/core/lib/Drupal/Core/Routing/ChainMatcher.php index cf8a66d8a4c..2ef53b4dc51 100644 --- a/core/lib/Drupal/Core/Routing/ChainMatcher.php +++ b/core/lib/Drupal/Core/Routing/ChainMatcher.php @@ -27,6 +27,7 @@ class ChainMatcher implements RequestMatcherInterface, RequestContextAwareInterf /** * Array of RequestMatcherInterface objects, sorted. + * * @var type */ protected $sortedMatchers = array(); @@ -111,8 +112,8 @@ class ChainMatcher implements RequestMatcherInterface, RequestContextAwareInterf * @param MatcherInterface $matcher * The matcher to add. * @param int $priority - * The priority of the matcher. Higher number matchers will be checked - * first. + * (optional) The priority of the matcher. Higher number matchers will be checked + * first. Default to 0. */ public function add(RequestMatcherInterface $matcher, $priority = 0) { if (empty($this->matchers[$priority])) { diff --git a/core/lib/Drupal/Core/Routing/CompiledRoute.php b/core/lib/Drupal/Core/Routing/CompiledRoute.php index 1600e766687..cd0998a7a07 100644 --- a/core/lib/Drupal/Core/Routing/CompiledRoute.php +++ b/core/lib/Drupal/Core/Routing/CompiledRoute.php @@ -36,17 +36,22 @@ class CompiledRoute { * @var Symfony\Component\Routing\Route */ protected $route; - protected $variables; protected $tokens; protected $staticPrefix; + + /** + * The regular expression to match placeholders out of this path. + * + * @var string + */ protected $regex; /** * Constructs a new CompiledRoute object. * - * @param Route $route + * @param Route $route * A original Route instance. * @param int $fit * The fitness of the route. @@ -66,7 +71,7 @@ class CompiledRoute { } /** - * Returns the fit of this route + * Returns the fit of this route. * * See RouteCompiler for a definition of how the fit is calculated. * @@ -117,7 +122,7 @@ class CompiledRoute { * Returns the Route instance. * * @return Route - * A Route instance + * A Route instance. */ public function getRoute() { return $this->route; @@ -126,7 +131,8 @@ class CompiledRoute { /** * Returns the pattern. * - * @return string The pattern + * @return string + * The pattern. */ public function getPattern() { return $this->route->getPattern(); @@ -135,7 +141,8 @@ class CompiledRoute { /** * Returns the options. * - * @return array The options + * @return array + * The options. */ public function getOptions() { return $this->route->getOptions(); @@ -144,7 +151,8 @@ class CompiledRoute { /** * Returns the defaults. * - * @return array The defaults + * @return array + * The defaults. */ public function getDefaults() { return $this->route->getDefaults(); @@ -153,11 +161,11 @@ class CompiledRoute { /** * Returns the requirements. * - * @return array The requirements + * @return array + * The requirements. */ public function getRequirements() { return $this->route->getRequirements(); } } - diff --git a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php index ae2bba00111..69683f59fbe 100644 --- a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php @@ -28,7 +28,7 @@ interface FinalMatcherInterface { * A Request object against which to match. * * @return array - * An array of parameters + * An array of parameters. */ public function matchRequest(Request $request); } diff --git a/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php b/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php index f1262dffaca..d642f4dfcf2 100644 --- a/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php +++ b/core/lib/Drupal/Core/Routing/FirstEntryFinalMatcher.php @@ -35,7 +35,9 @@ class FirstEntryFinalMatcher implements FinalMatcherInterface { return $this; } - + /** + * Implements Drupal\Core\Routing\FinalMatcherInterface::matchRequest(). + */ public function matchRequest(Request $request) { // Return whatever the first route in the collection is. foreach ($this->routes as $name => $route) { @@ -54,12 +56,12 @@ class FirstEntryFinalMatcher implements FinalMatcherInterface { * Get merged default parameters. * * @param array $params - * The parameters + * The parameters. * @param array $defaults - * The defaults + * The defaults. * * @return array - * Merged default parameters + * Merged default parameters. */ protected function mergeDefaults($params, $defaults) { $parameters = $defaults; @@ -73,4 +75,3 @@ class FirstEntryFinalMatcher implements FinalMatcherInterface { } } - diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index 5237fb152e6..5659452f82f 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -39,6 +39,15 @@ class MatcherDumper implements MatcherDumperInterface { */ protected $tableName; + /** + * Construct the MatcherDumper. + * + * @param Drupal\Core\Database\Connection $connection + * The database connection which will be used to store the route + * information. + * @param string $table + * (optional) The table to store the route info in. Defaults to 'router'. + */ public function __construct(Connection $connection, $table = 'router') { $this->connection = $connection; @@ -63,21 +72,18 @@ class MatcherDumper implements MatcherDumperInterface { * Dumps a set of routes to the router table in the database. * * Available options: + * - route_set: The route grouping that is being dumped. All existing + * routes with this route set will be deleted on dump. + * - base_class: The base class name * - * * route_set: The route grouping that is being dumped. All existing - * routes with this route set will be deleted on dump. - * * base_class: The base class name - * - * @param $options array - * $options An array of options + * @param array $options + * An array of options */ public function dump(array $options = array()) { $options += array( 'route_set' => '', ); - //$compiled = $this->compileRoutes($this->routes, $route_set); - // Convert all of the routes into database records. $insert = $this->connection->insert($this->tableName)->fields(array( 'name', @@ -104,7 +110,7 @@ class MatcherDumper implements MatcherDumperInterface { // compiled route object. Remove this once // https://github.com/symfony/symfony/pull/4755 is merged and brought // back downstream. - 'route' => serialize(clone($route)), + 'route' => serialize(clone($route)), ); $insert->values($values); } @@ -160,4 +166,3 @@ class MatcherDumper implements MatcherDumperInterface { return $fit; } } - diff --git a/core/lib/Drupal/Core/Routing/NestedMatcher.php b/core/lib/Drupal/Core/Routing/NestedMatcher.php index db8a429af4f..f8aed116154 100644 --- a/core/lib/Drupal/Core/Routing/NestedMatcher.php +++ b/core/lib/Drupal/Core/Routing/NestedMatcher.php @@ -16,9 +16,9 @@ use Symfony\Component\HttpFoundation\Request; class NestedMatcher implements NestedMatcherInterface { /** - * The final matcher + * The final matcher. * - * @var RequestMatcherInterface + * @var Symfony\Component\Routing\Matcher\RequestMatcherInterface */ protected $finalMatcher; @@ -32,14 +32,14 @@ class NestedMatcher implements NestedMatcherInterface { /** * The initial matcher to match against. * - * @var InitialMatcherInterface + * @var Drupal\core\Routing\InitialMatcherInterface */ protected $initialMatcher; /** * The request context. * - * @var RequestContext + * @var Symfony\Component\Routing\RequestContext */ protected $context; @@ -50,7 +50,7 @@ class NestedMatcher implements NestedMatcherInterface { * Partial matchers will be run in the order in which they are added. * * @param PartialMatcherInterface $matcher - * A partial + * A partial. * * @return NestedMatcherInterface * The current matcher. @@ -101,9 +101,11 @@ class NestedMatcher implements NestedMatcherInterface { * If the matcher can not find information, it must throw one of the exceptions documented * below. * - * @param Request $request The request to match + * @param Request $request + * The request to match. * - * @return array An array of parameters + * @return array + * An array of parameters. * * @throws ResourceNotFoundException If no matching resource could be found * @throws MethodNotAllowedException If a matching resource was found but the request method is not allowed @@ -146,4 +148,3 @@ class NestedMatcher implements NestedMatcherInterface { } } - diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index 8930932d1e8..ede6bde1f82 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -17,6 +17,7 @@ use Drupal\Core\Database\Connection; * Initial matcher to match a route against a built database, by path. */ class PathMatcher implements InitialMatcherInterface { + /** * The database connection from which to read route information. * @@ -86,6 +87,7 @@ class PathMatcher implements InitialMatcherInterface { * * @param array $parts * The parts of the path for which we want candidates. + * * @return array * An array of outlines that could match the specified path parts. */ @@ -130,4 +132,3 @@ class PathMatcher implements InitialMatcherInterface { return $ancestors; } } - diff --git a/core/lib/Drupal/Core/Routing/RouteBuilder.php b/core/lib/Drupal/Core/Routing/RouteBuilder.php index a1d8dafd993..a2fc5a247b7 100644 --- a/core/lib/Drupal/Core/Routing/RouteBuilder.php +++ b/core/lib/Drupal/Core/Routing/RouteBuilder.php @@ -16,10 +16,19 @@ class RouteBuilder { protected $dumper; + /** + * Construcs the RouteBuilder using the passed MatcherDumperInterface + * + * @param Symfony\Component\Routing\Matcher\Dumper\MatcherDumperInterface $dumper + * The matcher dumper used to store the route information. + */ public function __construct(MatcherDumperInterface $dumper) { $this->dumper = $dumper; } + /** + * Rebuilds the route info and dumps to dumper. + */ public function rebuild() { // We need to manually call each module so that we can know which module // a given item came from. diff --git a/core/lib/Drupal/Core/Routing/RouteCompiler.php b/core/lib/Drupal/Core/Routing/RouteCompiler.php index 0564a45541b..5585e989da5 100644 --- a/core/lib/Drupal/Core/Routing/RouteCompiler.php +++ b/core/lib/Drupal/Core/Routing/RouteCompiler.php @@ -22,18 +22,18 @@ class RouteCompiler implements RouteCompilerInterface { /** * Utility constant to use for regular expressions against the path. -*/ + */ const REGEX_DELIMITER = '#'; /** - * Compiles the current route instance. - * - * @param \Symfony\Component\Routing\Route $route - * A Route instance - * - * @return CompiledRoute - * A CompiledRoute instance - */ + * Compiles the current route instance. + * + * @param \Symfony\Component\Routing\Route $route + * A Route instance. + * + * @return CompiledRoute + * A CompiledRoute instance. + */ public function compile(Route $route) { $stripped_path = $this->getPathWithoutDefaults($route); @@ -62,7 +62,9 @@ class RouteCompiler implements RouteCompilerInterface { * The route object. * @param string $pattern * The pattern for which we want a matching regex. + * * @return type + * * @throws \LogicException */ public function getRegex(Route $route, $pattern) { @@ -107,11 +109,11 @@ class RouteCompiler implements RouteCompilerInterface { } // find the first optional token - $firstOptional = INF; + $first_optional = INF; for ($i = count($tokens) - 1; $i >= 0; $i--) { $token = $tokens[$i]; if ('variable' === $token[0] && $route->hasDefault($token[3])) { - $firstOptional = $i; + $first_optional = $i; } else { break; } @@ -120,50 +122,56 @@ class RouteCompiler implements RouteCompilerInterface { // compute the matching regexp $regexp = ''; for ($i = 0, $nbToken = count($tokens); $i < $nbToken; $i++) { - $regexp .= $this->computeRegexp($tokens, $i, $firstOptional); + $regexp .= $this->computeRegexp($tokens, $i, $first_optional); } return self::REGEX_DELIMITER.'^'.$regexp.'$'.self::REGEX_DELIMITER.'s'; } /** - * Computes the regexp used to match a specific token. It can be static text or a subpattern. - * - * @param array $tokens The route tokens - * @param integer $index The index of the current token - * @param integer $firstOptional The index of the first optional token - * - * @return string The regexp pattern for a single token - */ - private function computeRegexp(array $tokens, $index, $firstOptional) { - $token = $tokens[$index]; - if ('text' === $token[0]) { - // Text tokens - return preg_quote($token[1], self::REGEX_DELIMITER); - } else { - // Variable tokens - if (0 === $index && 0 === $firstOptional) { - // When the only token is an optional variable token, the separator is required - return sprintf('%s(?<%s>%s)?', preg_quote($token[1], self::REGEX_DELIMITER), $token[3], $token[2]); - } else { - $regexp = sprintf('%s(?<%s>%s)', preg_quote($token[1], self::REGEX_DELIMITER), $token[3], $token[2]); - if ($index >= $firstOptional) { - // Enclose each optional token in a subpattern to make it optional. - // "?:" means it is non-capturing, i.e. the portion of the subject string that - // matched the optional subpattern is not passed back. - $regexp = "(?:$regexp"; - $nbTokens = count($tokens); - if ($nbTokens - 1 == $index) { - // Close the optional subpatterns - $regexp .= str_repeat(")?", $nbTokens - $firstOptional - (0 === $firstOptional ? 1 : 0)); - } - } - - return $regexp; - } + * Computes the regexp used to match a specific token. It can be static text or a subpattern. + * + * @param array $tokens + * The route tokens + * @param integer $index + * The index of the current token + * @param integer $first_optional + * The index of the first optional token + * + * @return string + * The regexp pattern for a single token + */ + private function computeRegexp(array $tokens, $index, $first_optional) { + $token = $tokens[$index]; + if ('text' === $token[0]) { + // Text tokens + return preg_quote($token[1], self::REGEX_DELIMITER); + } + else { + // Variable tokens + if (0 === $index && 0 === $first_optional) { + // When the only token is an optional variable token, the separator is + // required. + return sprintf('%s(?<%s>%s)?', preg_quote($token[1], self::REGEX_DELIMITER), $token[3], $token[2]); } - } + else { + $regexp = sprintf('%s(?<%s>%s)', preg_quote($token[1], self::REGEX_DELIMITER), $token[3], $token[2]); + if ($index >= $first_optional) { + // Enclose each optional token in a subpattern to make it optional. + // "?:" means it is non-capturing, i.e. the portion of the subject + // string that matched the optional subpattern is not passed back. + $regexp = "(?:$regexp"; + $nbTokens = count($tokens); + if ($nbTokens - 1 == $index) { + // Close the optional subpatterns. + $regexp .= str_repeat(")?", $nbTokens - $first_optional - (0 === $first_optional ? 1 : 0)); + } + } + return $regexp; + } + } + } /** * Returns the pattern outline. @@ -217,6 +225,7 @@ class RouteCompiler implements RouteCompilerInterface { * problem. * * @param \Symfony\Component\Routing\Route $route + * The route to have the placeholders removed from. * * @return string * The path string, stripped of placeholders that have default values. @@ -236,4 +245,3 @@ class RouteCompiler implements RouteCompilerInterface { } } - diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/ChainMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/ChainMatcherTest.php index cec4d79283a..c6b28ccb297 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/ChainMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/ChainMatcherTest.php @@ -22,6 +22,7 @@ use Exception; * Basic tests for the ChainMatcher. */ class ChainMatcherTest extends UnitTestBase { + public static function getInfo() { return array( 'name' => 'Chain matcher tests', diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php index 0f174770b71..a288b9eb16e 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/FirstEntryFinalMatcherTest.php @@ -44,9 +44,6 @@ class FirstEntryFinalMatcherTest extends UnitTestBase { $this->fixtures = new RoutingFixtures(); } - public function setUp() { - parent::setUp(); - } /** * Confirms the final matcher returns correct attributes for static paths. @@ -113,4 +110,3 @@ class FirstEntryFinalMatcherTest extends UnitTestBase { $this->assertEqual($attributes['value'], 'poink', 'Optional placeholder value used default.'); } } - diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php index 962abfc3d8f..c98da2eef30 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/HttpMethodMatcherTest.php @@ -44,10 +44,7 @@ class HttpMethodMatcherTest extends UnitTestBase { $this->fixtures = new RoutingFixtures(); } - public function setUp() { - parent::setUp(); - } - + /** * Confirms that the HttpMethod matcher matches properly. */ @@ -106,4 +103,3 @@ class HttpMethodMatcherTest extends UnitTestBase { } } - diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MockMatcher.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MockMatcher.php index 6cd58fcb369..71611d464bd 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/MockMatcher.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MockMatcher.php @@ -21,6 +21,9 @@ use Closure; */ class MockMatcher implements RequestMatcherInterface { + /** + * The matcher being tested. + */ protected $matcher; public function __construct(Closure $matcher) { @@ -32,4 +35,3 @@ class MockMatcher implements RequestMatcherInterface { return $matcher($request); } } - diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/MockPathMatcher.php b/core/modules/system/lib/Drupal/system/Tests/Routing/MockPathMatcher.php index b545ebe766a..1592cbf0791 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/MockPathMatcher.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/MockPathMatcher.php @@ -9,14 +9,23 @@ use Symfony\Component\Routing\RouteCollection; use Drupal\Core\Routing\InitialMatcherInterface; /** - * Description of MockPathMatcher - * - * @author crell + * Provides a mock path matcher. */ class MockPathMatcher implements InitialMatcherInterface { + /** + * Routes to be matched. + * + * @var Symfony\Component\Routing\RouteCollection + */ protected $routes; + /** + * Construct the matcher given the route collection. + * + * @param Symfony\Component\Routing\RouteCollection $routes + * The routes being matched. + */ public function __construct(RouteCollection $routes) { $this->routes = $routes; } diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php index 9c0f5de310f..ad17ea7c1a6 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php @@ -44,9 +44,6 @@ class NestedMatcherTest extends UnitTestBase { $this->fixtures = new RoutingFixtures(); } - public function setUp() { - parent::setUp(); - } /** * Confirms we can nest multiple partial matchers. @@ -66,4 +63,3 @@ class NestedMatcherTest extends UnitTestBase { $this->assertEqual($attributes['_route'], 'route_a', 'The correct matching route was found.'); } } - diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php index 10cfc823ebb..078025085da 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/PathMatcherTest.php @@ -48,7 +48,7 @@ class PathMatcherTest extends UnitTestBase { public function tearDown() { $this->fixtures->dropTables(Database::getConnection()); - parent::tearDown(); + parent::tearDown(); } /** @@ -63,8 +63,6 @@ class PathMatcherTest extends UnitTestBase { $candidates = $matcher->getCandidateOutlines($parts); - //debug($candidates); - $candidates = array_flip($candidates); $this->assertTrue(count($candidates) == 4, 'Correct number of candidates found'); @@ -275,7 +273,7 @@ class PathMatcherTest extends UnitTestBase { } /** - * Confirm that an exception is thrown when no matching path is found. + * Confirms that an exception is thrown when no matching path is found. */ function testOutlinePathNoMatch() { $connection = Database::getConnection(); diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php index 9e4ba634733..4fc7a111b9c 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouteTest.php @@ -45,8 +45,8 @@ class RouteTest extends UnitTestBase { * Confirms that a compiled route with default values has the correct outline. */ public function testCompilationDefaultValue() { - // Because "here" has a default value, it should not factor into the - // outline or the fitness. + // Because "here" has a default value, it should not factor into the outline + // or the fitness. $route = new Route('/test/{something}/more/{here}', array( 'here' => 'there', )); diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php b/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php index 8937c75563a..f243ff134af 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RoutingFixtures.php @@ -12,6 +12,12 @@ use Drupal\Core\Database\Connection; */ class RoutingFixtures { + /** + * Create the tables required for the sample data. + * + * @param Drupal\Core\Database\Connection $connection + * The connection to use to create the tables. + */ public function createTables(Connection $connection) { $tables = $this->routingTableDefinition(); $schema = $connection->schema(); @@ -22,6 +28,12 @@ class RoutingFixtures { } } + /** + * Drop the tables used for the sample data. + * + * @param Drupal\Core\Database\Connection $connection + * The connection to use to drop the tables. + */ public function dropTables(Connection $connection) { $tables = $this->routingTableDefinition(); $schema = $connection->schema(); @@ -91,6 +103,12 @@ class RoutingFixtures { return $collection; } + /** + * Returns the table definition for the routing fixtures. + * + * @return array + * Table definitions. + */ public function routingTableDefinition() { $tables['test_routes'] = array( From 5ae1aca2df2bb8eebdfc324d3d6828683ebe7406 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 22 Sep 2012 13:10:36 -0500 Subject: [PATCH 64/72] Remove unnecessary files. --- .../system/lib/Drupal/system/FileDownload.php | 54 ------------------- core/vendor/Routing | 1 - 2 files changed, 55 deletions(-) delete mode 100644 core/modules/system/lib/Drupal/system/FileDownload.php delete mode 160000 core/vendor/Routing diff --git a/core/modules/system/lib/Drupal/system/FileDownload.php b/core/modules/system/lib/Drupal/system/FileDownload.php deleted file mode 100644 index b55872c14f1..00000000000 --- a/core/modules/system/lib/Drupal/system/FileDownload.php +++ /dev/null @@ -1,54 +0,0 @@ - Date: Sat, 22 Sep 2012 13:21:10 -0500 Subject: [PATCH 65/72] Remove no-longer-needed RouterListener subclass. We can use the Symfony one directly now. --- .../Core/EventSubscriber/RouterListener.php | 96 ------------------- 1 file changed, 96 deletions(-) delete mode 100644 core/lib/Drupal/Core/EventSubscriber/RouterListener.php diff --git a/core/lib/Drupal/Core/EventSubscriber/RouterListener.php b/core/lib/Drupal/Core/EventSubscriber/RouterListener.php deleted file mode 100644 index 7b158fdf5f6..00000000000 --- a/core/lib/Drupal/Core/EventSubscriber/RouterListener.php +++ /dev/null @@ -1,96 +0,0 @@ -urlMatcher = $urlMatcher; - $this->logger = $logger; - } - - /** - * {@inheritdoc} - * - * This method is nearly identical to the parent, except it passes the - * $request->attributes->get('system_path') variable to the matcher. - * That is where Drupal stores its processed, de-aliased, and sanitized - * internal path. We also pass the full request object to the URL Matcher, - * since we want attributes to be available to the matcher and to controllers. - */ - public function onKernelRequest(GetResponseEvent $event) { - $request = $event->getRequest(); - - if (HttpKernelInterface::MASTER_REQUEST === $event->getRequestType()) { - $this->urlMatcher->getContext()->fromRequest($request); - $this->urlMatcher->setRequest($request); - } - - if ($request->attributes->has('_controller')) { - // Routing is already done. - return; - } - - // Add attributes based on the path info (routing). - try { - $parameters = $this->urlMatcher->match($request->attributes->get('system_path')); - - if (null !== $this->logger) { - $this->logger->info(sprintf('Matched route "%s" (parameters: %s)', $parameters['_route'], $this->parametersToString($parameters))); - } - - $request->attributes->add($parameters); - unset($parameters['_route']); - unset($parameters['_controller']); - $request->attributes->set('_route_params', $parameters); - } - catch (ResourceNotFoundException $e) { - $message = sprintf('No route found for "%s %s"', $request->getMethod(), $request->getPathInfo()); - - throw new NotFoundHttpException($message, $e); - } - catch (MethodNotAllowedException $e) { - $message = sprintf('No route found for "%s %s": Method Not Allowed (Allow: %s)', $request->getMethod(), $request->getPathInfo(), strtoupper(implode(', ', $e->getAllowedMethods()))); - - throw new MethodNotAllowedHttpException($e->getAllowedMethods(), $message, $e); - } - } -} From 0ea8230787338b89abaeb6f01f2e8fe717da96dc Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 22 Sep 2012 14:16:28 -0500 Subject: [PATCH 66/72] Split handling of old and new style subrequests to avoid empty or inceptioned pages. --- .../LegacyControllerSubscriber.php | 9 +++++++- .../Core/EventSubscriber/ViewSubscriber.php | 20 +++++++++++++++--- .../system/Tests/Routing/RouterTest.php | 21 +++++++++++++++++++ 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php index f088f7ef2e2..7009eff9154 100644 --- a/core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/LegacyControllerSubscriber.php @@ -36,11 +36,18 @@ class LegacyControllerSubscriber implements EventSubscriberInterface { * The Event to process. */ public function onKernelControllerLegacy(FilterControllerEvent $event) { - $router_item = $event->getRequest()->attributes->get('drupal_menu_item'); + $request = $event->getRequest(); + $router_item = $request->attributes->get('drupal_menu_item'); $controller = $event->getController(); // This BC logic applies only to functions. Otherwise, skip it. if (is_string($controller) && function_exists($controller)) { + // Flag this as a legacy request. We need to use this for subrequest + // handling so that we can treat older page callbacks and new routes + // differently. + // @todo Remove this line as soon as possible. + $request->attributes->set('_legacy', TRUE); + $new_controller = function() use ($router_item) { return call_user_func_array($router_item['page_callback'], $router_item['page_arguments']); }; diff --git a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php index cb3aeeaaa6e..e88525e7ed4 100644 --- a/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/ViewSubscriber.php @@ -45,14 +45,14 @@ class ViewSubscriber implements EventSubscriberInterface { */ public function onView(GetResponseForControllerResultEvent $event) { + $request = $event->getRequest(); + // For a master request, we process the result and wrap it as needed. // For a subrequest, all we want is the string value. We assume that // is just an HTML string from a controller, so wrap that into a response // object. The subrequest's response will get dissected and placed into // the larger page as needed. if ($event->getRequestType() == HttpKernelInterface::MASTER_REQUEST) { - $request = $event->getRequest(); - $method = 'on' . $this->negotiation->getContentType($request); if (method_exists($this, $method)) { @@ -62,7 +62,9 @@ class ViewSubscriber implements EventSubscriberInterface { $event->setResponse(new Response('Unsupported Media Type', 415)); } } - else { + elseif ($request->attributes->get('_legacy')) { + // This is an old hook_menu-based subrequest, which means we assume + // the body is supposed to be the complete page. $page_result = $event->getControllerResult(); if (!is_array($page_result)) { $page_result = array( @@ -71,6 +73,18 @@ class ViewSubscriber implements EventSubscriberInterface { } $event->setResponse(new Response(drupal_render_page($page_result))); } + else { + // This is a new-style Symfony-esque subrequest, which means we assume + // the body is not supposed to be a complete page but just a page + // fragment. + $page_result = $event->getControllerResult(); + if (!is_array($page_result)) { + $page_result = array( + '#markup' => $page_result, + ); + } + $event->setResponse(new Response(drupal_render($page_result))); + } } public function onJson(GetResponseForControllerResultEvent $event) { 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 f5b4a1893f4..413737516ac 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/RouterTest.php @@ -43,7 +43,14 @@ class RouterTest extends WebTestBase { public function testDefaultController() { $this->drupalGet('router_test/test2'); $this->assertRaw('test2', 'The correct string was returned because the route was successful.'); + + // Confirm that the page wrapping is being added, so we're not getting a + // raw body returned. $this->assertRaw('', 'Page markup was found.'); + + // 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.'); } /** @@ -53,7 +60,14 @@ class RouterTest extends WebTestBase { $value = $this->randomName(); $this->drupalGet('router_test/test3/' . $value); $this->assertRaw($value, 'The correct string was returned because the route was successful.'); + + // Confirm that the page wrapping is being added, so we're not getting a + // raw body returned. $this->assertRaw('', 'Page markup was found.'); + + // 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.'); } /** @@ -62,7 +76,14 @@ class RouterTest extends WebTestBase { public function testControllerPlaceholdersDefaultValues() { $this->drupalGet('router_test/test4'); $this->assertRaw('narf', 'The correct string was returned because the route was successful.'); + + // Confirm that the page wrapping is being added, so we're not getting a + // raw body returned. $this->assertRaw('', 'Page markup was found.'); + + // 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.'); } } From 001c75b25e7691fcecf5b252416c22b459efda39 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Sat, 22 Sep 2012 15:05:53 -0500 Subject: [PATCH 67/72] Various and sundry documentation formatting fixes. --- core/lib/Drupal/Core/HtmlPageController.php | 11 +++- core/lib/Drupal/Core/Routing/ChainMatcher.php | 13 ++-- .../lib/Drupal/Core/Routing/CompiledRoute.php | 65 ++++++++++--------- .../Core/Routing/FinalMatcherInterface.php | 11 +++- .../Core/Routing/FirstEntryFinalMatcher.php | 9 ++- .../Drupal/Core/Routing/HttpMethodMatcher.php | 9 ++- .../Core/Routing/InitialMatcherInterface.php | 9 ++- .../lib/Drupal/Core/Routing/MatcherDumper.php | 14 ++-- .../lib/Drupal/Core/Routing/NestedMatcher.php | 49 +++++++------- .../Core/Routing/NestedMatcherInterface.php | 17 +++-- .../Drupal/Core/Routing/PartialMatcher.php | 11 +++- .../Core/Routing/PartialMatcherInterface.php | 13 ++-- core/lib/Drupal/Core/Routing/RouteBuilder.php | 12 +++- .../lib/Drupal/Core/Routing/RouteCompiler.php | 5 +- 14 files changed, 157 insertions(+), 91 deletions(-) diff --git a/core/lib/Drupal/Core/HtmlPageController.php b/core/lib/Drupal/Core/HtmlPageController.php index 27a9e4e694b..7d14ac00097 100644 --- a/core/lib/Drupal/Core/HtmlPageController.php +++ b/core/lib/Drupal/Core/HtmlPageController.php @@ -20,14 +20,14 @@ class HtmlPageController implements ContainerAwareInterface { /** * The injection container for this object. * - * @var ContainerInterface + * @var \Symfony\Component\DependencyInjection\ContainerInterface */ protected $container; /** * Injects the service container used by this object. * - * @param ContainerInterface $container + * @param \Symfony\Component\DependencyInjection\ContainerInterface $container * The service container this object should use. */ public function setContainer(ContainerInterface $container = NULL) { @@ -39,10 +39,11 @@ class HtmlPageController implements ContainerAwareInterface { * * @param Request $request * The request object. - * @param type $_content + * @param callable $_content * The body content callable that contains the body region of this page. * * @return \Symfony\Component\HttpFoundation\Response + * A response object. */ public function content(Request $request, $_content) { @@ -53,8 +54,12 @@ class HtmlPageController implements ContainerAwareInterface { // https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Controller/InternalController.php $attributes = $request->attributes; $controller = $_content; + + // We need to clean off the derived information and such so that the + // subrequest can be processed properly without leaking data through. $attributes->remove('system_path'); $attributes->remove('_content'); + $response = $this->container->get('http_kernel')->forward($controller, $attributes->all(), $request->query->all()); $page_content = $response->getContent(); diff --git a/core/lib/Drupal/Core/Routing/ChainMatcher.php b/core/lib/Drupal/Core/Routing/ChainMatcher.php index 2ef53b4dc51..23410bfe347 100644 --- a/core/lib/Drupal/Core/Routing/ChainMatcher.php +++ b/core/lib/Drupal/Core/Routing/ChainMatcher.php @@ -1,5 +1,10 @@ context = $context; @@ -87,8 +90,10 @@ class ChainMatcher implements RequestMatcherInterface, RequestContextAwareInterf * * @return array An array of parameters * - * @throws ResourceNotFoundException If no matching resource could be found - * @throws MethodNotAllowedException If a matching resource was found but the request method is not allowed + * @throws \Symfony\Component\Routing\Exception\ResourceNotFoundException + * If no matching resource could be found + * @throws \Symfony\Component\Routing\Exception\MethodNotAllowedException + * If a matching resource was found but the request method is not allowed */ public function matchRequest(Request $request) { $methodNotAllowed = null; diff --git a/core/lib/Drupal/Core/Routing/CompiledRoute.php b/core/lib/Drupal/Core/Routing/CompiledRoute.php index cd0998a7a07..c5cdde8a994 100644 --- a/core/lib/Drupal/Core/Routing/CompiledRoute.php +++ b/core/lib/Drupal/Core/Routing/CompiledRoute.php @@ -1,5 +1,10 @@ route; } /** - * Returns the pattern. - * - * @return string - * The pattern. - */ + * Returns the pattern. + * + * @return string + * The pattern. + */ public function getPattern() { return $this->route->getPattern(); } /** - * Returns the options. - * - * @return array - * The options. - */ + * Returns the options. + * + * @return array + * The options. + */ public function getOptions() { return $this->route->getOptions(); } /** - * Returns the defaults. - * - * @return array - * The defaults. - */ + * Returns the defaults. + * + * @return array + * The defaults. + */ public function getDefaults() { return $this->route->getDefaults(); } /** - * Returns the requirements. - * - * @return array - * The requirements. - */ + * Returns the requirements. + * + * @return array + * The requirements. + */ public function getRequirements() { return $this->route->getRequirements(); } diff --git a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php index 69683f59fbe..8b85c21847a 100644 --- a/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/FinalMatcherInterface.php @@ -1,5 +1,10 @@ routes)) { @@ -74,10 +80,10 @@ class MatcherDumper implements MatcherDumperInterface { * Available options: * - route_set: The route grouping that is being dumped. All existing * routes with this route set will be deleted on dump. - * - base_class: The base class name + * - base_class: The base class name. * * @param array $options - * An array of options + * An array of options. */ public function dump(array $options = array()) { $options += array( @@ -136,7 +142,7 @@ class MatcherDumper implements MatcherDumperInterface { /** * Gets the routes to match. * - * @return RouteCollection + * @return \Symfony\Component\Routing\RouteCollection * A RouteCollection instance representing all routes currently in the * dumper. */ diff --git a/core/lib/Drupal/Core/Routing/NestedMatcher.php b/core/lib/Drupal/Core/Routing/NestedMatcher.php index f8aed116154..d1bc91d66ff 100644 --- a/core/lib/Drupal/Core/Routing/NestedMatcher.php +++ b/core/lib/Drupal/Core/Routing/NestedMatcher.php @@ -43,14 +43,13 @@ class NestedMatcher implements NestedMatcherInterface { */ protected $context; - /** * Adds a partial matcher to the matching plan. * * Partial matchers will be run in the order in which they are added. * - * @param PartialMatcherInterface $matcher - * A partial. + * @param \Drupal\Core\Routing\PartialMatcherInterface $matcher + * A partial matcher. * * @return NestedMatcherInterface * The current matcher. @@ -64,11 +63,11 @@ class NestedMatcher implements NestedMatcherInterface { /** * Sets the final matcher for the matching plan. * - * @param UrlMatcherInterface $final + * @param \Drupal\Core\Routing\FinalMatcherInterface $final * The matcher that will be called last to ensure only a single route is * found. * - * @return NestedMatcherInterface + * @return \Drupal\Core\Routing\NestedMatcherInterface * The current matcher. */ public function setFinalMatcher(FinalMatcherInterface $final) { @@ -82,11 +81,11 @@ class NestedMatcher implements NestedMatcherInterface { * * Partial matchers will be run in the order in which they are added. * - * @param InitialMatcherInterface $matcher + * @param \Drupal\Core\Routing\InitialMatcherInterface $matcher * An initial matcher. It is responsible for its own configuration and * initial route collection * - * @return NestedMatcherInterface + * @return \Drupal\Core\Routing\NestedMatcherInterface * The current matcher. */ public function setInitialMatcher(InitialMatcherInterface $initial) { @@ -96,20 +95,22 @@ class NestedMatcher implements NestedMatcherInterface { } /** - * Tries to match a request with a set of routes. - * - * If the matcher can not find information, it must throw one of the exceptions documented - * below. - * - * @param Request $request - * The request to match. - * - * @return array - * An array of parameters. - * - * @throws ResourceNotFoundException If no matching resource could be found - * @throws MethodNotAllowedException If a matching resource was found but the request method is not allowed - */ + * Tries to match a request with a set of routes. + * + * If the matcher can not find information, it must throw one of the + * exceptions documented below. + * + * @param \Symfony\Component\HttpFoundation\Request $request + * The request to match. + * + * @return array + * An array of parameters. + * + * @throws ResourceNotFoundException + * If no matching resource could be found. + * @throws MethodNotAllowedException + * If a matching resource was found but the request method is not allowed. + */ public function matchRequest(Request $request) { $collection = $this->initialMatcher->matchRequestPartial($request); @@ -130,7 +131,8 @@ class NestedMatcher implements NestedMatcherInterface { * * This method is unused. It is here only to satisfy the interface. * - * @param RequestContext $context The context + * @param \Symfony\Component\Routing\RequestContext $context + * The context */ public function setContext(RequestContext $context) { $this->context = $context; @@ -141,7 +143,8 @@ class NestedMatcher implements NestedMatcherInterface { * * This method is unused. It is here only to satisfy the interface. * - * @return RequestContext The context + * @return \Symfony\Component\Routing\RequestContext + * The context */ public function getContext() { return $this->context; diff --git a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php index cd55d32dd8c..34018cc07f4 100644 --- a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php @@ -1,5 +1,10 @@ Date: Sat, 22 Sep 2012 15:15:47 -0500 Subject: [PATCH 68/72] Document hook_route_info(). --- core/modules/system/system.api.php | 45 ++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/core/modules/system/system.api.php b/core/modules/system/system.api.php index 66aa5e8c451..7957a648a63 100644 --- a/core/modules/system/system.api.php +++ b/core/modules/system/system.api.php @@ -565,6 +565,51 @@ function hook_menu_get_item_alter(&$router_item, $path, $original_map) { } } +/** + * Defines routes in the new router system. + * + * A route is a Symfony Route object. See the Symfony documentation for more + * details on the available options. Of specific note: + * - _controller: This is the PHP callable that will handle a request matching + * the route. + * - _content: This is the PHP callable that will handle the body of a request + * matching this route. A default controller will provide the page + * rendering around it. + * + * Typically you will only specify one or the other of those properties. + * + * @deprecated + * This mechanism for registering routes is temporary. It will be replaced + * by a more robust mechanism in the near future. It is documented here + * only for completeness. + */ +function hook_route_info() { + $collection = new RouteCollection(); + + $route = new Route('router_test/test1', array( + '_controller' => '\Drupal\router_test\TestControllers::test1' + )); + $collection->add('router_test_1', $route); + + $route = new Route('router_test/test2', array( + '_content' => '\Drupal\router_test\TestControllers::test2' + )); + $collection->add('router_test_2', $route); + + $route = new Route('router_test/test3/{value}', array( + '_content' => '\Drupal\router_test\TestControllers::test3' + )); + $collection->add('router_test_3', $route); + + $route = new Route('router_test/test4/{value}', array( + '_content' => '\Drupal\router_test\TestControllers::test4', + 'value' => 'narf', + )); + $collection->add('router_test_4', $route); + + return $collection; +} + /** * Define menu items and page callbacks. * From e2f99d4ec8ba96eeb5d5f2ff2692255c78a39557 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Fri, 28 Sep 2012 11:34:24 -0500 Subject: [PATCH 69/72] Properly escape SQL table. --- core/lib/Drupal/Core/Routing/PathMatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index ede6bde1f82..cef1a0a07cc 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -62,7 +62,7 @@ class PathMatcher implements InitialMatcherInterface { $ancestors = $this->getCandidateOutlines($parts); - $routes = $this->connection->query("SELECT name, route FROM {{$this->tableName}} WHERE pattern_outline IN (:patterns) ORDER BY fit", array( + $routes = $this->connection->query("SELECT name, route FROM {" . $this->connection->escapeTable($this->tableName) . "} WHERE pattern_outline IN (:patterns) ORDER BY fit", array( ':patterns' => $ancestors, )) ->fetchAllKeyed(); From 854a48bf6b7cc57b350359324865eee04bebaf90 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Fri, 28 Sep 2012 11:55:12 -0500 Subject: [PATCH 70/72] Remove workarounds for Symfony limitations that have been fixed. --- core/lib/Drupal/Core/Routing/MatcherDumper.php | 7 +------ core/lib/Drupal/Core/Routing/PathMatcher.php | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/MatcherDumper.php b/core/lib/Drupal/Core/Routing/MatcherDumper.php index 693c356dec0..48b2d8c1d16 100644 --- a/core/lib/Drupal/Core/Routing/MatcherDumper.php +++ b/core/lib/Drupal/Core/Routing/MatcherDumper.php @@ -111,12 +111,7 @@ class MatcherDumper implements MatcherDumperInterface { 'pattern' => $compiled->getPattern(), 'pattern_outline' => $compiled->getPatternOutline(), 'number_parts' => $compiled->getNumParts(), - // This is only temporary. We need to strip off the compiled route from - // route object in order to serialize it. Cloning strips off the - // compiled route object. Remove this once - // https://github.com/symfony/symfony/pull/4755 is merged and brought - // back downstream. - 'route' => serialize(clone($route)), + 'route' => serialize($route), ); $insert->values($values); } diff --git a/core/lib/Drupal/Core/Routing/PathMatcher.php b/core/lib/Drupal/Core/Routing/PathMatcher.php index cef1a0a07cc..f037d9acb41 100644 --- a/core/lib/Drupal/Core/Routing/PathMatcher.php +++ b/core/lib/Drupal/Core/Routing/PathMatcher.php @@ -75,7 +75,7 @@ class PathMatcher implements InitialMatcherInterface { } } - if (!count($collection->all())) { + if (!count($collection)) { throw new ResourceNotFoundException(); } From bf586d4f3337dcb427ed4bb6bfec6b12c398bad4 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Fri, 28 Sep 2012 12:21:03 -0500 Subject: [PATCH 71/72] Add priority support to partial matchers in a nested matcher. --- .../lib/Drupal/Core/Routing/NestedMatcher.php | 54 +++++++++++++++++-- .../Core/Routing/NestedMatcherInterface.php | 7 ++- .../Tests/Routing/NestedMatcherTest.php | 2 +- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/core/lib/Drupal/Core/Routing/NestedMatcher.php b/core/lib/Drupal/Core/Routing/NestedMatcher.php index d1bc91d66ff..ff53715426e 100644 --- a/core/lib/Drupal/Core/Routing/NestedMatcher.php +++ b/core/lib/Drupal/Core/Routing/NestedMatcher.php @@ -29,6 +29,13 @@ class NestedMatcher implements NestedMatcherInterface { */ protected $partialMatchers = array(); + /** + * Array of PartialMatcherInterface objects, sorted. + * + * @var type + */ + protected $sortedMatchers = array(); + /** * The initial matcher to match against. * @@ -50,14 +57,20 @@ class NestedMatcher implements NestedMatcherInterface { * * @param \Drupal\Core\Routing\PartialMatcherInterface $matcher * A partial matcher. + * @param int $priority + * (optional) The priority of the matcher. Higher number matchers will be checked + * first. Default to 0. * * @return NestedMatcherInterface * The current matcher. */ - public function addPartialMatcher(PartialMatcherInterface $matcher) { - $this->partialMatchers[] = $matcher; + public function addPartialMatcher(PartialMatcherInterface $matcher, $priority = 0) { + if (empty($this->matchers[$priority])) { + $this->matchers[$priority] = array(); + } - return $this; + $this->matchers[$priority][] = $matcher; + $this->sortedMatchers = array(); } /** @@ -114,7 +127,7 @@ class NestedMatcher implements NestedMatcherInterface { public function matchRequest(Request $request) { $collection = $this->initialMatcher->matchRequestPartial($request); - foreach ($this->partialMatchers as $matcher) { + foreach ($this->getPartialMatchers() as $matcher) { if ($collection) { $matcher->setCollection($collection); } @@ -126,6 +139,39 @@ class NestedMatcher implements NestedMatcherInterface { return $attributes; } + /** + * Sorts the matchers and flattens them. + * + * @return array + * An array of RequestMatcherInterface objects. + */ + public function getPartialMatchers() { + if (empty($this->sortedMatchers)) { + $this->sortedMatchers = $this->sortMatchers(); + } + + return $this->sortedMatchers; + } + + /** + * Sort matchers by priority. + * + * The highest priority number is the highest priority (reverse sorting). + * + * @return \Symfony\Component\Routing\RequestMatcherInterface[] + * An array of Matcher objects in the order they should be used. + */ + protected function sortMatchers() { + $sortedMatchers = array(); + krsort($this->matchers); + + foreach ($this->matchers as $matchers) { + $sortedMatchers = array_merge($sortedMatchers, $matchers); + } + + return $sortedMatchers; + } + /** * Sets the request context. * diff --git a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php index 34018cc07f4..6ae09954af7 100644 --- a/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php +++ b/core/lib/Drupal/Core/Routing/NestedMatcherInterface.php @@ -35,11 +35,14 @@ interface NestedMatcherInterface extends RequestMatcherInterface { * * @param \Drupal\Core\Routing\PartialMatcherInterface $matcher * A partial matcher. + * @param int $priority + * (optional) The priority of the matcher. Higher number matchers will be checked + * first. Default to 0. * - * @return \Drupal\Core\Routing\NestedMatcherInterface + * @return NestedMatcherInterface * The current matcher. */ - public function addPartialMatcher(PartialMatcherInterface $matcher); + public function addPartialMatcher(PartialMatcherInterface $matcher, $priority = 0); /** * Sets the final matcher for the matching plan. diff --git a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php index ad17ea7c1a6..444785c49f0 100644 --- a/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php +++ b/core/modules/system/lib/Drupal/system/Tests/Routing/NestedMatcherTest.php @@ -53,7 +53,7 @@ class NestedMatcherTest extends UnitTestBase { $matcher = new NestedMatcher(); $matcher->setInitialMatcher(new MockPathMatcher($this->fixtures->sampleRouteCollection())); - $matcher->addPartialMatcher(new HttpMethodMatcher()); + $matcher->addPartialMatcher(new HttpMethodMatcher(), 1); $matcher->setFinalMatcher(new FirstEntryFinalMatcher()); $request = Request::create('/path/one', 'GET'); From ff6804ed8229ab2911edab603c1c155bbfd3be43 Mon Sep 17 00:00:00 2001 From: Larry Garfield Date: Fri, 28 Sep 2012 12:59:52 -0500 Subject: [PATCH 72/72] Chasing update hook numbers. --- core/modules/system/system.install | 124 ++++++++++++++--------------- 1 file changed, 62 insertions(+), 62 deletions(-) diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 1c0d43cdb25..05459d03d43 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -1971,10 +1971,71 @@ function system_update_8019() { db_drop_table('registry_file'); } +/** + * Conditionally enable the new Ban module. + */ +function system_update_8020() { + $blocked_ips_exists = db_query_range('SELECT 1 FROM {blocked_ips}', 0, 1)->fetchField(); + if ($blocked_ips_exists) { + // Rename the permission name. + db_update('role_permission') + ->fields(array( + 'permission' => 'ban IP addresses', + 'module' => 'ban', + )) + ->condition('permission', 'block IP addresses') + ->execute(); + // Rename {blocked_ips} table into {ban_ip}. + db_rename_table('blocked_ips', 'ban_ip'); + // Rename all references to the action callback. + db_update('actions') + ->fields(array('callback' => 'ban_ip_action')) + ->condition('callback', 'system_block_ip_action') + ->execute(); + // Rename the action's aid. + db_update('actions') + ->fields(array('aid' => 'ban_ip_action')) + ->condition('aid', 'system_block_ip_action') + ->execute(); + // Enable the new Ban module. + update_module_enable(array('ban')); + } + else { + // Drop old table. + db_drop_table('blocked_ips'); + } +} + +/** + * Enable the Actions module. + */ +function system_update_8021() { + // Enable the module without re-installing the schema. + update_module_enable(array('action')); + // Rename former System module actions. + $map = array( + 'system_message_action' => 'action_message_action', + 'system_send_email_action' => 'action_send_email_action', + 'system_goto_action' => 'action_goto_action', + ); + foreach ($map as $old => $new) { + // Rename all references to the action callback. + db_update('actions') + ->fields(array('callback' => $new)) + ->condition('callback', $old) + ->execute(); + // Rename the action's aid. + db_update('actions') + ->fields(array('aid' => $new)) + ->condition('aid', $old) + ->execute(); + } +} + /* * Create the new routing table. */ -function system_update_8020() { +function system_update_8022() { $tables['router'] = array( 'description' => 'Maps paths to various callbacks (access, page and title)', @@ -2040,67 +2101,6 @@ function system_update_8020() { $schema->createTable('router', $tables['router']); } -/** - * Conditionally enable the new Ban module. - */ -function system_update_8020() { - $blocked_ips_exists = db_query_range('SELECT 1 FROM {blocked_ips}', 0, 1)->fetchField(); - if ($blocked_ips_exists) { - // Rename the permission name. - db_update('role_permission') - ->fields(array( - 'permission' => 'ban IP addresses', - 'module' => 'ban', - )) - ->condition('permission', 'block IP addresses') - ->execute(); - // Rename {blocked_ips} table into {ban_ip}. - db_rename_table('blocked_ips', 'ban_ip'); - // Rename all references to the action callback. - db_update('actions') - ->fields(array('callback' => 'ban_ip_action')) - ->condition('callback', 'system_block_ip_action') - ->execute(); - // Rename the action's aid. - db_update('actions') - ->fields(array('aid' => 'ban_ip_action')) - ->condition('aid', 'system_block_ip_action') - ->execute(); - // Enable the new Ban module. - update_module_enable(array('ban')); - } - else { - // Drop old table. - db_drop_table('blocked_ips'); - } -} - -/** - * Enable the Actions module. - */ -function system_update_8021() { - // Enable the module without re-installing the schema. - update_module_enable(array('action')); - // Rename former System module actions. - $map = array( - 'system_message_action' => 'action_message_action', - 'system_send_email_action' => 'action_send_email_action', - 'system_goto_action' => 'action_goto_action', - ); - foreach ($map as $old => $new) { - // Rename all references to the action callback. - db_update('actions') - ->fields(array('callback' => $new)) - ->condition('callback', $old) - ->execute(); - // Rename the action's aid. - db_update('actions') - ->fields(array('aid' => $new)) - ->condition('aid', $old) - ->execute(); - } -} - /** * @} End of "defgroup updates-7.x-to-8.x". * The next series of updates should start at 9000.