diff --git a/core/core.services.yml b/core/core.services.yml index 6b8334a..f828e5b 100644 --- a/core/core.services.yml +++ b/core/core.services.yml @@ -508,11 +508,6 @@ services: calls: - [setContext, ['@router.request_context']] - [add, ['@router.dynamic']] - router.path_roots_subscriber: - class: Drupal\Core\EventSubscriber\PathRootsSubscriber - arguments: ['@state'] - tags: - - { name: event_subscriber } entity.query: class: Drupal\Core\Entity\Query\QueryFactory arguments: ['@entity.manager'] @@ -581,11 +576,6 @@ services: class: Drupal\Core\ParamConverter\ParamConverterManager tags: - { name: service_collector, tag: paramconverter, call: addConverter } - paramconverter_subscriber: - class: Drupal\Core\EventSubscriber\ParamConverterSubscriber - tags: - - { name: event_subscriber } - arguments: ['@paramconverter_manager'] paramconverter.entity: class: Drupal\Core\ParamConverter\EntityConverter tags: @@ -605,11 +595,6 @@ services: resolver_manager.entity: class: Drupal\Core\Entity\EntityResolverManager arguments: ['@entity.manager', '@class_resolver'] - route_subscriber.entity: - class: Drupal\Core\EventSubscriber\EntityRouteAlterSubscriber - tags: - - { name: event_subscriber } - arguments: ['@resolver_manager.entity'] ajax_subscriber: class: Drupal\Core\EventSubscriber\AjaxSubscriber tags: @@ -645,10 +630,6 @@ services: class: Drupal\Core\EventSubscriber\SpecialAttributesRouteSubscriber tags: - { name: event_subscriber } - route_http_method_subscriber: - class: Drupal\Core\EventSubscriber\RouteMethodSubscriber - tags: - - { name: event_subscriber } controller.page: class: Drupal\Core\Controller\HtmlPageController arguments: ['@controller_resolver', '@title_resolver', '@render_html_renderer'] @@ -698,11 +679,6 @@ services: arguments: ['@router.route_provider', '@paramconverter_manager', '@access_arguments_resolver_factory', '@current_user'] calls: - [setContainer, ['@service_container']] - access_route_subscriber: - class: Drupal\Core\EventSubscriber\AccessRouteSubscriber - tags: - - { name: event_subscriber } - arguments: ['@access_manager'] access_check.default: class: Drupal\Core\Access\DefaultAccessCheck tags: diff --git a/core/lib/Drupal/Core/EventSubscriber/AccessRouteSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/AccessRouteSubscriber.php deleted file mode 100644 index 240073b..0000000 --- a/core/lib/Drupal/Core/EventSubscriber/AccessRouteSubscriber.php +++ /dev/null @@ -1,61 +0,0 @@ -accessManager = $access_manager; - } - - /** - * Apply access checks to routes. - * - * @param \Drupal\Core\Routing\RouteBuildEvent $event - * The event to process. - */ - public function onRoutingRouteAlterSetAccessCheck(RouteBuildEvent $event) { - $this->accessManager->setChecks($event->getRouteCollection()); - } - - /** - * Registers the methods in this class that should be listeners. - * - * @return array - * An array of event listener definitions. - */ - static function getSubscribedEvents() { - // Setting very low priority to ensure access checks are run after alters. - $events[RoutingEvents::ALTER][] = array('onRoutingRouteAlterSetAccessCheck', -1000); - - return $events; - } - -} diff --git a/core/lib/Drupal/Core/EventSubscriber/EntityRouteAlterSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/EntityRouteAlterSubscriber.php deleted file mode 100644 index 120520b..0000000 --- a/core/lib/Drupal/Core/EventSubscriber/EntityRouteAlterSubscriber.php +++ /dev/null @@ -1,64 +0,0 @@ -resolverManager = $entity_resolver_manager; - } - - /** - * Applies parameter converters to route parameters. - * - * @param \Drupal\Core\Routing\RouteBuildEvent $event - * The event to process. - */ - public function onRoutingRouteAlterSetType(RouteBuildEvent $event) { - foreach ($event->getRouteCollection() as $route) { - $this->resolverManager->setRouteOptions($route); - } - } - - /** - * {@inheritdoc} - */ - static function getSubscribedEvents() { - $events[RoutingEvents::ALTER][] = array('onRoutingRouteAlterSetType', -150); - return $events; - } -} diff --git a/core/lib/Drupal/Core/EventSubscriber/ParamConverterSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/ParamConverterSubscriber.php deleted file mode 100644 index eaa248a..0000000 --- a/core/lib/Drupal/Core/EventSubscriber/ParamConverterSubscriber.php +++ /dev/null @@ -1,56 +0,0 @@ -paramConverterManager = $param_converter_manager; - } - - /** - * Applies parameter converters to route parameters. - * - * @param \Drupal\Core\Routing\RouteBuildEvent $event - * The event to process. - */ - public function onRoutingRouteAlterSetParameterConverters(RouteBuildEvent $event) { - $this->paramConverterManager->setRouteParameterConverters($event->getRouteCollection()); - } - - /** - * {@inheritdoc} - */ - static function getSubscribedEvents() { - // Run after \Drupal\system\EventSubscriber\AdminRouteSubscriber. - $events[RoutingEvents::ALTER][] = array('onRoutingRouteAlterSetParameterConverters', -220); - return $events; - } -} diff --git a/core/lib/Drupal/Core/EventSubscriber/PathRootsSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/PathRootsSubscriber.php deleted file mode 100644 index 8d96767..0000000 --- a/core/lib/Drupal/Core/EventSubscriber/PathRootsSubscriber.php +++ /dev/null @@ -1,81 +0,0 @@ -state = $state; - } - - /** - * Collects all path roots. - * - * @param \Drupal\Core\Routing\RouteBuildEvent $event - * The route build event. - */ - public function onRouteAlter(RouteBuildEvent $event) { - $collection = $event->getRouteCollection(); - foreach ($collection->all() as $route) { - $bits = explode('/', ltrim($route->getPath(), '/')); - $this->pathRoots[$bits[0]] = $bits[0]; - } - } - - /** - * {@inheritdoc} - */ - public function onRouteFinished() { - $this->state->set('router.path_roots', array_keys($this->pathRoots)); - unset($this->pathRoots); - } - - /** - * {@inheritdoc} - */ - public static function getSubscribedEvents() { - $events = array(); - // Try to set a low priority to ensure that all routes are already added. - $events[RoutingEvents::ALTER][] = array('onRouteAlter', -1024); - $events[RoutingEvents::FINISHED][] = array('onRouteFinished'); - return $events; - } - -} diff --git a/core/lib/Drupal/Core/EventSubscriber/RouteMethodSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/RouteMethodSubscriber.php deleted file mode 100644 index f422b2a..0000000 --- a/core/lib/Drupal/Core/EventSubscriber/RouteMethodSubscriber.php +++ /dev/null @@ -1,50 +0,0 @@ -getRouteCollection() as $route) { - $methods = $route->getMethods(); - if (empty($methods)) { - $route->setMethods(array('GET', 'POST')); - } - } - } - - /** - * {@inheritdoc} - */ - static function getSubscribedEvents() { - // Set a higher priority to ensure that routes get the default HTTP methods - // as early as possible. - $events[RoutingEvents::ALTER][] = array('onRouteBuilding', 5000); - return $events; - } - -} diff --git a/core/lib/Drupal/Core/Routing/RouteBuilder.php b/core/lib/Drupal/Core/Routing/RouteBuilder.php index 5bea0a5..c9d2fdb 100644 --- a/core/lib/Drupal/Core/Routing/RouteBuilder.php +++ b/core/lib/Drupal/Core/Routing/RouteBuilder.php @@ -8,7 +8,10 @@ namespace Drupal\Core\Routing; use Drupal\Component\Discovery\YamlDiscovery; +use Drupal\Core\Access\AccessManagerInterface; use Drupal\Core\Controller\ControllerResolverInterface; +use Drupal\Core\Entity\EntityResolverManager; +use Drupal\Core\ParamConverter\ParamConverterManagerInterface; use Drupal\Core\State\StateInterface; use Symfony\Component\EventDispatcher\Event; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -92,7 +95,7 @@ class RouteBuilder implements RouteBuilderInterface { * The module handler. * @param \Drupal\Core\Controller\ControllerResolverInterface $controller_resolver * The controller resolver. - * @param \Drupal\Core\KeyValueStore\StateInterface $state + * @param \Drupal\Core\State\StateInterface $state * The state. */ public function __construct(MatcherDumperInterface $dumper, LockBackendInterface $lock, EventDispatcherInterface $dispatcher, ModuleHandlerInterface $module_handler, ControllerResolverInterface $controller_resolver, StateInterface $state = NULL) { @@ -105,6 +108,119 @@ public function __construct(MatcherDumperInterface $dumper, LockBackendInterface } /** + * Provides a default value for the HTTP method restriction on routes. + * + * Most routes will only deal with GET and POST requests, so we restrict them to + * those two if nothing else is specified. This is necessary to give other + * routes a chance during the route matching process when they are listening + * for example to DELETE requests on the same path. A typical use case are REST + * web service routes that use the full spectrum of HTTP methods. + * + * @param RouteCollection $collection + */ + public static function setDefaultMethods(RouteCollection $collection) { + foreach ($collection->all() as $route) { + if (!$route->getMethods()) { + $route->setMethods(array('GET', 'POST')); + } + } + } + + /** + * Registers the 'type' of route parameter names that match an entity type. + * + * @todo Matching on parameter *name* is not ideal, because it breaks + * encapsulation: parameter names are local to the controller and route, and + * controllers and routes can't be expected to know what all possible entity + * types might exist across all modules in order to pick names that don't + * conflict. Instead, the 'type' should be determined from introspecting what + * kind of PHP variable (e.g., a type hinted interface) the controller + * requires: https://drupal.org/node/2041907. + */ + public static function setEntityType(RouteCollection $collection, EntityResolverManager $entity_resolver_manager = NULL) { + if (!$entity_resolver_manager) { + $entity_resolver_manager = \Drupal::service('resolver_manager.entity'); + } + foreach ($collection as $route) { + $entity_resolver_manager->setRouteOptions($route);; + } + } + + /** + * Applies parameter converters to route parameters. + * + * @param \Symfony\Component\Routing\RouteCollection $collection + * @param ParamConverterManagerInterface $param_converter_manager + * The parameter converter manager that will be responsible for upcasting + * request attributes. + */ + public static function setParameterConverters(RouteCollection $collection, ParamConverterManagerInterface $param_converter_manager = NULL) { + if (!$param_converter_manager) { + $param_converter_manager = \Drupal::service('paramconverter_manager'); + } + $param_converter_manager->setRouteParameterConverters($collection); + } + + /** + * Apply access checks to routes. + * + * @param \Symfony\Component\Routing\RouteCollection $collection + * A collection of routes to apply checks to. + * @param \Drupal\Core\Access\AccessManagerInterface $access_manager + * The access check manager that will be responsible for applying + * AccessCheckers against routes. + */ + public static function addChecks(RouteCollection $collection, AccessManagerInterface $access_manager = NULL) { + if (!$access_manager) { + $access_manager = \Drupal::accessManager(); + } + $access_manager->setChecks($collection); + } + + /** + * Collects all path roots. + * + * @param \Symfony\Component\Routing\RouteCollection $collection + * A collection of routes to collect path roots from. + * @param \Drupal\Core\State\StateInterface $state + */ + public static function collectPathRoots(RouteCollection $collection, StateInterface $state = NULL) { + if (!$state) { + $state = \Drupal::state(); + } + $path_roots = []; + foreach ($collection->all() as $route) { + $path_root = explode('/', ltrim($route->getPath(), '/'))[0]; + $path_roots[$path_root] = $path_root; + } + $state->set('router.path_roots', array_keys($path_roots)); + } + + /** + * Adds the _admin_route option to each admin route. + * + * @param \Symfony\Component\Routing\RouteCollection $collection + * @param \Drupal\Core\State\StateInterface $state + */ + public static function separateAdminRoutes(RouteCollection $collection, StateInterface $state = NULL) { + if (!$state) { + $state = \Drupal::state(); + } + $non_admin_routes = []; + foreach ($collection->all() as $name => $route) { + if (strpos($route->getPath(), '/admin/') === 0 || $route->getPath() === 'admin' || $route->getOption('_admin_route')) { + // This might not be necessary but there is no harm in setting it + // again. + $route->setOption('_admin_route', TRUE); + } + else { + $non_admin_routes[$name] = $name; + } + } + $state->set('routing.non_admin_routes', array_keys($non_admin_routes)); + } + + /** * {@inheritdoc} */ public function rebuild() { @@ -172,6 +288,13 @@ public function rebuild() { // make it clear. $this->dispatcher->dispatch(RoutingEvents::ALTER, new RouteBuildEvent($collection)); + static::setDefaultMethods($collection); + static::setEntityType($collection); + static::setParameterConverters($collection); + static::addChecks($collection); + static::collectPathRoots($collection, $this->state); + static::separateAdminRoutes($collection, $this->state); + $this->dumper->addRoutes($collection); $this->dumper->dump(); diff --git a/core/lib/Drupal/Core/Routing/RoutePreloader.php b/core/lib/Drupal/Core/Routing/RoutePreloader.php index 7952d17..78fdb0b 100644 --- a/core/lib/Drupal/Core/Routing/RoutePreloader.php +++ b/core/lib/Drupal/Core/Routing/RoutePreloader.php @@ -90,39 +90,9 @@ protected function loadNonAdminRoutes() { } /** - * Alters existing routes for a specific collection. - * - * @param \Drupal\Core\Routing\RouteBuildEvent $event - * The route build event. - */ - public function onAlterRoutes(RouteBuildEvent $event) { - $collection = $event->getRouteCollection(); - foreach ($collection->all() as $name => $route) { - if (strpos($route->getPath(), '/admin/') !== 0 && $route->getPath() != '/admin') { - $this->nonAdminRoutesOnRebuild[] = $name; - } - } - $this->nonAdminRoutesOnRebuild = array_unique($this->nonAdminRoutesOnRebuild); - } - - /** - * Store the non admin routes in state when the route building is finished. - * - * @param \Symfony\Component\EventDispatcher\Event $event - * The route finish event. - */ - public function onFinishedRoutes(Event $event) { - $this->state->set('routing.non_admin_routes', $this->nonAdminRoutesOnRebuild); - $this->nonAdminRoutesOnRebuild = array(); - } - - /** * {@inheritdoc} */ public static function getSubscribedEvents() { - // Set a really low priority to catch as many as possible routes. - $events[RoutingEvents::ALTER] = array('onAlterRoutes', -1024); - $events[RoutingEvents::FINISHED] = array('onFinishedRoutes'); // Load the routes before the controller is executed (which happens after // the kernel request event). $events[KernelEvents::REQUEST][] = array('onRequest'); diff --git a/core/modules/system/src/EventSubscriber/AdminRouteSubscriber.php b/core/modules/system/src/EventSubscriber/AdminRouteSubscriber.php deleted file mode 100644 index eef05e1..0000000 --- a/core/modules/system/src/EventSubscriber/AdminRouteSubscriber.php +++ /dev/null @@ -1,45 +0,0 @@ -all() as $route) { - if (strpos($route->getPath(), '/admin') === 0 && !$route->hasOption('_admin_route')) { - $route->setOption('_admin_route', TRUE); - } - } - } - - /** - * {@inheritdoc} - */ - public static function getSubscribedEvents() { - $events = parent::getSubscribedEvents(); - - // Use a higher priority than \Drupal\field_ui\Routing\RouteSubscriber or - // \Drupal\views\EventSubscriber\RouteSubscriber to ensure we add the - // option to their routes. - // @todo https://drupal.org/node/2158571 - $events[RoutingEvents::ALTER] = array('onAlterRoutes', -200); - - return $events; - } - -} diff --git a/core/modules/system/system.services.yml b/core/modules/system/system.services.yml index 453eea4..7cd9ae6 100644 --- a/core/modules/system/system.services.yml +++ b/core/modules/system/system.services.yml @@ -21,10 +21,6 @@ services: class: Drupal\system\PathProcessor\PathProcessorFiles tags: - { name: path_processor_inbound, priority: 200 } - system.admin_path.route_subscriber: - class: Drupal\system\EventSubscriber\AdminRouteSubscriber - tags: - - { name: event_subscriber } theme.negotiator.system.batch: class: Drupal\system\Theme\BatchNegotiator arguments: ['@batch.storage', '@request_stack'] diff --git a/core/tests/Drupal/Tests/Core/EventSubscriber/PathRootsSubscriberTest.php b/core/tests/Drupal/Tests/Core/EventSubscriber/PathRootsSubscriberTest.php deleted file mode 100644 index 02acb66..0000000 --- a/core/tests/Drupal/Tests/Core/EventSubscriber/PathRootsSubscriberTest.php +++ /dev/null @@ -1,74 +0,0 @@ -state = $this->getMock('Drupal\Core\State\StateInterface'); - $this->pathRootsSubscriber = new PathRootsSubscriber($this->state); - } - - /** - * Tests altering and finished event. - * - * @covers ::onRouteAlter() - * @covers ::onRouteFinished() - */ - public function testSubscribing() { - $route_collection = new RouteCollection(); - $route_collection->add('test_route1', new Route('/test/bar')); - $route_collection->add('test_route2', new Route('/test/baz')); - $route_collection->add('test_route3', new Route('/test2/bar/baz')); - - $event = new RouteBuildEvent($route_collection, 'provider'); - $this->pathRootsSubscriber->onRouteAlter($event); - - $route_collection = new RouteCollection(); - $route_collection->add('test_route4', new Route('/test1/bar')); - $route_collection->add('test_route5', new Route('/test2/baz')); - $route_collection->add('test_route6', new Route('/test2/bar/baz')); - - $event = new RouteBuildEvent($route_collection, 'provider'); - $this->pathRootsSubscriber->onRouteAlter($event); - - $this->state->expects($this->once()) - ->method('set') - ->with('router.path_roots', array('test', 'test2', 'test1')); - - $this->pathRootsSubscriber->onRouteFinished(); - } - -} diff --git a/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php b/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php index aff98962..85247ce 100644 --- a/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php @@ -91,6 +91,14 @@ protected function setUp() { $this->routeBuilder = new TestRouteBuilder($this->dumper, $this->lock, $this->dispatcher, $this->moduleHandler, $this->controllerResolver, $this->state); $this->routeBuilder->setYamlDiscovery($this->yamlDiscovery); + $container = new ContainerBuilder(); + $entity_resolver_manager = $this->getMockBuilder('Drupal\Core\Entity\EntityResolverManager') + ->disableOriginalConstructor() + ->getMock(); + $container->set('resolver_manager.entity', $entity_resolver_manager); + $container->set('paramconverter_manager', $this->getMock('Drupal\Core\ParamConverter\ParamConverterManagerInterface')); + $container->set('access_manager', $this->getMock('Drupal\Core\Access\AccessManagerInterface')); + \Drupal::setContainer($container); } /** @@ -168,16 +176,19 @@ public function testRebuildWithStaticModuleRoutes() { $this->dispatcher->expects($this->at(1)) ->method('dispatch') ->with(RoutingEvents::ALTER, $route_build_event); + $expected_collection = clone $route_collection; + RouteBuilder::setDefaultMethods($expected_collection); // Ensure that the routes are set to the dumper and dumped. $this->dumper->expects($this->at(0)) ->method('addRoutes') - ->with($route_collection); + ->with($expected_collection); $this->dumper->expects($this->at(1)) ->method('dump') ->with(); $this->assertTrue($this->routeBuilder->rebuild()); + RouteBuilder::setDefaultMethods($route_collection); } /** @@ -233,15 +244,18 @@ public function testRebuildWithProviderBasedRoutes() { $this->dispatcher->expects($this->at(1)) ->method('dispatch') ->with(RoutingEvents::ALTER, $route_build_event); + $expected_collection = clone $route_collection_filled; + RouteBuilder::setDefaultMethods($expected_collection); // Ensure that the routes are set to the dumper and dumped. $this->dumper->expects($this->at(0)) ->method('addRoutes') - ->with($route_collection_filled); + ->with($expected_collection); $this->dumper->expects($this->at(1)) ->method('dump'); $this->assertTrue($this->routeBuilder->rebuild()); + RouteBuilder::setDefaultMethods($route_collection_filled); } /** @@ -257,9 +271,8 @@ public function testRebuildIfNecessary() { ->method('release') ->with('router_rebuild'); - $this->state->expects($this->once()) - ->method('set') - ->with('router_rebuild_needed'); + $this->state->expects($this->exactly(3)) + ->method('set'); $this->state->expects($this->once()) ->method('delete') diff --git a/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php b/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php index a928c97..324d2cb 100644 --- a/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php @@ -61,74 +61,6 @@ protected function setUp() { } /** - * Tests onAlterRoutes with just admin routes. - */ - public function testOnAlterRoutesWithAdminRoutes() { - $event = $this->getMockBuilder('Drupal\Core\Routing\RouteBuildEvent') - ->disableOriginalConstructor() - ->getMock(); - $route_collection = new RouteCollection(); - $route_collection->add('test', new Route('/admin/foo', array('_content' => 'Drupal\ExampleController'))); - $route_collection->add('test2', new Route('/admin/bar', array('_content' => 'Drupal\ExampleController'))); - $event->expects($this->once()) - ->method('getRouteCollection') - ->will($this->returnValue($route_collection)); - - $this->state->expects($this->once()) - ->method('set') - ->with('routing.non_admin_routes', array()); - $this->preloader->onAlterRoutes($event); - $this->preloader->onFinishedRoutes(new Event()); - } - - /** - * Tests onAlterRoutes with "admin" appearing in the path. - */ - public function testOnAlterRoutesWithAdminPathNoAdminRoute() { - $event = $this->getMockBuilder('Drupal\Core\Routing\RouteBuildEvent') - ->disableOriginalConstructor() - ->getMock(); - $route_collection = new RouteCollection(); - $route_collection->add('test', new Route('/foo/admin/foo', array('_content' => 'Drupal\ExampleController'))); - $route_collection->add('test2', new Route('/bar/admin/bar', array('_content' => 'Drupal\ExampleController'))); - $route_collection->add('test3', new Route('/administrator/a', array('_content' => 'Drupal\ExampleController'))); - $route_collection->add('test4', new Route('/admin', array('_content' => 'Drupal\ExampleController'))); - $event->expects($this->once()) - ->method('getRouteCollection') - ->will($this->returnValue($route_collection)); - - $this->state->expects($this->once()) - ->method('set') - ->with('routing.non_admin_routes', array('test', 'test2', 'test3')); - $this->preloader->onAlterRoutes($event); - $this->preloader->onFinishedRoutes(new Event()); - } - - - /** - * Tests onAlterRoutes with admin routes and non admin routes. - */ - public function testOnAlterRoutesWithNonAdminRoutes() { - $event = $this->getMockBuilder('Drupal\Core\Routing\RouteBuildEvent') - ->disableOriginalConstructor() - ->getMock(); - $route_collection = new RouteCollection(); - $route_collection->add('test', new Route('/admin/foo', array('_content' => 'Drupal\ExampleController'))); - $route_collection->add('test2', new Route('/bar', array('_content' => 'Drupal\ExampleController'))); - // Non content routes, like ajax callbacks should be ignored. - $route_collection->add('test3', new Route('/bar', array('_controller' => 'Drupal\ExampleController'))); - $event->expects($this->once()) - ->method('getRouteCollection') - ->will($this->returnValue($route_collection)); - - $this->state->expects($this->once()) - ->method('set') - ->with('routing.non_admin_routes', array('test2', 'test3')); - $this->preloader->onAlterRoutes($event); - $this->preloader->onFinishedRoutes(new Event()); - } - - /** * Tests onRequest on a non html request. */ public function testOnRequestNonHtml() {