diff --git a/core/lib/Drupal/Core/Routing/RouteBuilder.php b/core/lib/Drupal/Core/Routing/RouteBuilder.php index d720f07..2e02fe8 100644 --- a/core/lib/Drupal/Core/Routing/RouteBuilder.php +++ b/core/lib/Drupal/Core/Routing/RouteBuilder.php @@ -142,12 +142,6 @@ public function rebuild() { $this->dumper->dump(array('provider' => $provider)); } - // Now allow modules to register additional, dynamic routes. - $collection = new RouteCollection(); - $this->dispatcher->dispatch(RoutingEvents::ALTER, new RouteBuildEvent($collection, 'dynamic_routes')); - $this->dumper->addRoutes($collection); - $this->dumper->dump(array('provider' => 'dynamic_routes')); - $this->lock->release('router_rebuild'); return TRUE; } diff --git a/core/lib/Drupal/Core/Routing/RouteSubscriberBase.php b/core/lib/Drupal/Core/Routing/RouteSubscriberBase.php index 1172428..3e94e8f 100644 --- a/core/lib/Drupal/Core/Routing/RouteSubscriberBase.php +++ b/core/lib/Drupal/Core/Routing/RouteSubscriberBase.php @@ -22,8 +22,7 @@ * @param \Symfony\Component\Routing\RouteCollection $collection * The route collection for adding routes. * @param string $provider - * The provider these routes belong to. For dynamically added routes, the - * provider name will be 'dynamic_routes'. + * The provider these routes belong to. */ protected function alterRoutes(RouteCollection $collection, $provider) { } diff --git a/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php b/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php index 537b186..c4a9e7c 100644 --- a/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php +++ b/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php @@ -37,12 +37,6 @@ public function __construct(ConfigMapperManagerInterface $mapper_manager) { * {@inheritdoc} */ protected function alterRoutes(RouteCollection $collection, $provider) { - // Because ConfigMapperManagerInterface uses the route provider to determine - // the routes to add here this must only run during the dynamic alter stage. - if ($provider != 'dynamic_routes') { - return; - } - $mappers = $this->mapperManager->getMappers(); foreach ($mappers as $mapper) { $collection->add($mapper->getOverviewRouteName(), $mapper->getOverviewRoute()); diff --git a/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php b/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php index 5768dd3..347b802 100644 --- a/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php +++ b/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php @@ -95,9 +95,13 @@ protected function getViewsDisplayIDsWithRoute() { } /** - * {@inheritdoc} + * Returns a set of route objects. + * + * @return \Symfony\Component\Routing\RouteCollection + * A route collection. */ - protected function alterRoutes(RouteCollection $collection, $provider) { + public function routes() { + $collection = new RouteCollection(); foreach ($this->getViewsDisplayIDsWithRoute() as $pair) { list($view_id, $display_id) = explode('.', $pair); $view = $this->viewStorageController->load($view_id); @@ -105,10 +109,28 @@ protected function alterRoutes(RouteCollection $collection, $provider) { if (($view = $view->getExecutable()) && $view instanceof ViewExecutable) { if ($view->setDisplay($display_id) && $display = $view->displayHandlers->get($display_id)) { if ($display instanceof DisplayRouterInterface) { - if ($provider == 'dynamic_routes') { - $this->viewRouteNames += (array) $display->collectRoutes($collection); - } + $this->viewRouteNames += (array) $display->collectRoutes($collection); + } + } + $view->destroy(); + } + } + + $this->state->set('views.view_route_names', $this->viewRouteNames); + return $collection; + } + /** + * {@inheritdoc} + */ + protected function alterRoutes(RouteCollection $collection, $module) { + foreach ($this->getViewsDisplayIDsWithRoute() as $pair) { + list($view_id, $display_id) = explode('.', $pair); + $view = $this->viewStorageController->load($view_id); + // @todo This should have an executable factory injected. + if (($view = $view->getExecutable()) && $view instanceof ViewExecutable) { + if ($view->setDisplay($display_id) && $display = $view->displayHandlers->get($display_id)) { + if ($display instanceof DisplayRouterInterface) { // If the display returns TRUE a route item was found, so it does not // have to be added. $view_route_names = $display->alterRoutes($collection); @@ -121,9 +143,6 @@ protected function alterRoutes(RouteCollection $collection, $provider) { $view->destroy(); } } - if ($provider == 'dynamic_routes') { - $this->state->set('views.view_route_names', $this->viewRouteNames); - } } /** diff --git a/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayPageTest.php b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayPageTest.php index a3f0f49..b0d830b 100644 --- a/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayPageTest.php +++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayPageTest.php @@ -96,9 +96,7 @@ public function testPageResponses() { * Checks that the router items are properly registered */ public function testPageRouterItems() { - $subscriber = new RouteSubscriber($this->container->get('entity.manager'), $this->container->get('state')); - $collection = new RouteCollection(); - $subscriber->onAlterRoutes(new RouteBuildEvent($collection, 'dynamic_routes')); + $collection = \Drupal::service('views.route_subscriber')->routes(); // Check the controller defaults. foreach ($collection as $id => $route) { diff --git a/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php b/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php index 64f9c5a..a7759a7 100644 --- a/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php +++ b/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php @@ -73,13 +73,40 @@ protected function setUp() { } /** + * Tests the onDynamicRoutes method. + * + * @see \Drupal\views\EventSubscriber\RouteSubscriber::onDynamicRoutes() + */ + public function testDynamicRoutes() { + list($view, $executable, $display_1, $display_2) = $this->setupMocks(); + + $display_1->expects($this->once()) + ->method('collectRoutes') + ->will($this->returnValue(array('test_id.page_1' => 'views.test_id.page_1'))); + $display_2->expects($this->once()) + ->method('collectRoutes') + ->will($this->returnValue(array('test_id.page_2' => 'views.test_id.page_2'))); + + $this->routeSubscriber->routes(); + + $this->state->expects($this->once()) + ->method('set') + ->with('views.view_route_names', array('test_id.page_1' => 'views.test_id.page_1', 'test_id.page_2' => 'views.test_id.page_2')); + $this->routeSubscriber->destruct(); + } + + /** * Tests the onAlterRoutes method. * * @see \Drupal\views\EventSubscriber\RouteSubscriber::onAlterRoutes() */ public function testOnAlterRoutes() { $collection = new RouteCollection(); - $route_event = new RouteBuildEvent($collection, 'dynamic_routes'); + $collection->add('test_route', new Route('test_route', array('_controller' => 'Drupal\Tests\Core\Controller\TestController'))); + $route_2 = new Route('test_route/example', array('_controller' => 'Drupal\Tests\Core\Controller\TestController')); + $collection->add('test_route_2', $route_2); + + $route_event = new RouteBuildEvent($collection, 'views'); list($view, $executable, $display_1, $display_2) = $this->setupMocks(); @@ -88,9 +115,8 @@ public function testOnAlterRoutes() { $display_1->expects($this->once()) ->method('alterRoutes') ->will($this->returnValue(array('test_id.page_1' => 'test_route'))); - $display_1->expects($this->once()) - ->method('collectRoutes') - ->will($this->returnValue(array('test_id.page_1' => 'views.test_id.page_1'))); + $display_1->expects($this->never()) + ->method('collectRoutes'); $display_2->expects($this->once()) ->method('alterRoutes') @@ -101,6 +127,11 @@ public function testOnAlterRoutes() { $this->assertNull($this->routeSubscriber->onAlterRoutes($route_event)); + // Ensure that after the alterRoutes the collectRoutes method is just called + // once (not for page_1 anymore). + + $this->routeSubscriber->routes(); + $this->state->expects($this->once()) ->method('set') ->with('views.view_route_names', array('test_id.page_1' => 'test_route', 'test_id.page_2' => 'views.test_id.page_2')); diff --git a/core/modules/views/views.routing.yml b/core/modules/views/views.routing.yml index 9abe5d5..e720603 100644 --- a/core/modules/views/views.routing.yml +++ b/core/modules/views/views.routing.yml @@ -4,3 +4,6 @@ views.ajax: _controller: '\Drupal\views\Controller\ViewAjaxController::ajaxView' requirements: _access: 'TRUE' + +route_callbacks: + - 'views.route_subscriber:routes' diff --git a/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php b/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php index 98cbfda..7fb3091 100644 --- a/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php +++ b/core/tests/Drupal/Tests/Core/Routing/RouteBuilderTest.php @@ -10,7 +10,6 @@ use Drupal\Component\Discovery\YamlDiscovery; use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Routing\RouteBuilder; -use Drupal\Core\Routing\RouteBuildEvent; use Drupal\Core\Routing\RoutingEvents; use Drupal\Tests\UnitTestCase; use Symfony\Component\Routing\Route; @@ -162,14 +161,6 @@ public function testRebuildWithStaticModuleRoutes() { ->method('dispatch') ->with($this->equalTo(RoutingEvents::ALTER), $this->isInstanceOf('Drupal\Core\Routing\RouteBuildEvent')); - $empty_collection = new RouteCollection(); - $route_build_event = new RouteBuildEvent($empty_collection, 'dynamic_routes'); - - // Ensure that the alter routes events are fired. - $this->dispatcher->expects($this->at(1)) - ->method('dispatch') - ->with(RoutingEvents::ALTER, $route_build_event); - // Ensure that the routes are set to the dumper and dumped. $this->dumper->expects($this->at(0)) ->method('addRoutes') @@ -177,12 +168,6 @@ public function testRebuildWithStaticModuleRoutes() { $this->dumper->expects($this->at(1)) ->method('dump') ->with(array('provider' => 'test_module')); - $this->dumper->expects($this->at(2)) - ->method('addRoutes') - ->with($empty_collection); - $this->dumper->expects($this->at(3)) - ->method('dump') - ->with(array('provider' => 'dynamic_routes')); $this->assertTrue($this->routeBuilder->rebuild()); } @@ -235,14 +220,6 @@ public function testRebuildWithProviderBasedRoutes() { ->method('dispatch') ->with($this->equalTo(RoutingEvents::ALTER), $this->isInstanceOf('Drupal\Core\Routing\RouteBuildEvent')); - $empty_collection = new RouteCollection(); - $route_build_event = new RouteBuildEvent($empty_collection, 'dynamic_routes'); - - // Ensure that the alter routes events are fired. - $this->dispatcher->expects($this->at(1)) - ->method('dispatch') - ->with(RoutingEvents::ALTER, $route_build_event); - // Ensure that the routes are set to the dumper and dumped. $this->dumper->expects($this->at(0)) ->method('addRoutes')